Inspiration for this comes from: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html RFC: http://lkml.org/lkml/2010/8/3/496 Patch is unchanged from the RFC. Reviews seemed generally positive and it seemed this was desired functionality. INTRO As SOCs become more popular, the desire to quickly define a simple, but functional, bus type with only a few unique properties becomes desirable. As they become more complicated, the ability to nest these simple busses and otherwise orchestrate them to match the actual topology also becomes desirable. EXAMPLE USAGE /arch/ARCH/MY_ARCH/my_bus.c: #include <linux/device.h> #include <linux/platform_device.h> struct bus_type my_bus_type = { .name = "mybus", }; EXPORT_SYMBOL_GPL(my_bus_type); struct platform_device sub_bus1 = { .name = "sub_bus1", .id = -1, .dev.bus = &my_bus_type, } EXPORT_SYMBOL_GPL(sub_bus1); struct platform_device sub_bus2 = { .name = "sub_bus2", .id = -1, .dev.bus = &my_bus_type, } EXPORT_SYMBOL_GPL(sub_bus2); static int __init my_bus_init(void) { int error; platform_bus_type_init(&my_bus_type); error = bus_register(&my_bus_type); if (error) return error; error = platform_device_register(&sub_bus1); if (error) goto fail_sub_bus1; error = platform_device_register(&sub_bus2); if (error) goto fail_sub_bus2; return error; fail_sub_bus2: platform_device_unregister(&sub_bus1); fail_sub_bus2: bus_unregister(&my_bus_type); return error; } postcore_initcall(my_bus_init); EXPORT_SYMBOL_GPL(my_bus_init); /drivers/my_driver.c static struct platform_driver my_driver = { .driver = { .name = "my-driver", .owner = THIS_MODULE, .bus = &my_bus_type, }, }; /somewhere/my_device.c static struct platform_device my_device = { .name = "my-device", .id = -1, .dev.bus = &my_bus_type, .dev.parent = &sub_bus_1.dev, }; Notice that for a device / driver, only 3 lines ...
Also, later in that thread I also wrote[1] what seems to be the core of what you've done here: namely, allow platform_devices and platform_drivers to to be used on custom busses. Patch is at the end of this mail with a more focused changelog. As Greg suggested in his reply to your first version, this part could be merged today, and the platform_bus_init stuff could be added later, after some more review. Except it requires a re-compile. Rather than doing this at compile time, it would be better to support legacy devices at runtime. You could handle this by simply registering the driver on the custom bus and the platform_bus and let the bus matching code handle it. Then, the same binary would work on both With this approach, you should note in the comments/changelog that any selective customization of the bus PM methods must be done after calling platform_bus_type_init(). Also, You've left out the legacy PM methods here. That implies that moving a driver from the platform_bus to the custom bus is not entirely transparent. If the driver still has legacy PM methods, it would stop working on the custom bus. While this is good motivation for converting a driver to dev_pm_ops, at a minimum it should be clear in the changelog that the derivative busses do not support legacy PM methods. However, since it's quite easy to do, and you want the derivative busses to be *exactly* like the platform bus except where explicitly changed, I'd suggest you also check/copy the legacy PM methods. In addition, you've missed several fields in 'struct bus_type' (bus_attr, drv_attr, p, etc.) Without digging deeper into the driver core, I'm not sure if they are all needed at init time, but it should be clear in the comments why they can be excluded. Kevin [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31289.html From b784009af1d0a7065dc5e58a13ce29afa3432d3e Mon Sep 17 00:00:00 2001 From: Kevin Hilman <khilman@deeprootsystems.com> Date: Mon, 28 Jun 2010 ...
I can split this into 2 patches.
Can you safely register a driver on more than one bus? I didn't think
that was safe -- normally it's impossible since you're calling
struct BUS_TYPE_driver mydriver;
BUS_TYPE_driver_register(&mydriver)
but now we have multiple "bus types" that are all actually platform type; still,
at a minimum you would need:
struct platform_driver mydrvier1 = {
.driver.bus = &sub_bus1,
};
struct platform_driver mydrvier2 = {
.driver.bus = &sub_bus2,
};
which would all point to the same driver functions, yet the respective devices
attached for the "same" driver would be on different buses. I fear this might
confuse some drivers. I don't think dynamic bus assignment is this easy
In short: I do not believe the same driver can be registered on multiple
No they don't. If you call platform_bus_type_init first then you'll
just overwrite them with new values; if you call it second then they
I copied everything that was defined for platform_bus_type:
struct bus_type platform_bus_type = {
.name = "platform",
.dev_attrs = platform_dev_attrs,
.match = platform_match,
.uevent = platform_uevent,
.pm = &platform_dev_pm_ops,
};
EXPORT_SYMBOL_GPL(platform_bus_type);
struct bus_type {
const char *name;
struct bus_attribute *bus_attrs;
struct device_attribute *dev_attrs;
struct driver_attribute *drv_attrs;
int (*match)(struct device *dev, struct device_driver *drv);
int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
int (*probe)(struct device *dev);
int (*remove)(struct device *dev);
void (*shutdown)(struct device *dev);
int (*suspend)(struct device *dev, pm_message_t state);
int (*resume)(struct device *dev);
const struct dev_pm_ops *pm;
struct bus_type_private *p;
};
It is my understanding that everything that I did not copy *should* remain
unique to each bus; remaining fields will be filled in by bus_register and
If you would like to lead this effort, please do so; I did not mean to ...It is possible, and currently done in powerpc land where some drivers handle devices on the platform_bus and the custom OF bus. However, as noted by Magnus, what we really need here is a way for drivers to not care at all what kind of bus they are on. There are an increasing number of drivers that are re-used not just across different SoCs in the same family, but across totally different SoCs (e.g. drivers Ah, OK. The changelog was missing credits to that affect, but I was more concerned that you hadn't seen my example and didn't want to be Right, they will not be overwritten, but you'll be left with a mostly empty dev_pm_ops on the custom bus. IOW, Most of these custom busses will only want to customize a small subset of the dev_pm_ops methods (e.g. only the runtime PM methods.) If you setup your sparsly populated custom dev_pm_ops and then call platform_bus_type_init() second, dev_pm_ops on the new buswill have *only* your custom fields, and none of the defaults from platform_dev_pm_ops. So, what I was getting at is that it should probably be clearer to the users of platform_bus_type_init() that any customization of dev_pm_ops Great, I'm glad you forged ahead. There is definitely a broader need for something like this, and I have no personal attachment to the code. I have no problems with you continuing the work (in fact, I'd prefer it. I have lots of other things to catch up on after my vacation.) In the future though, it's common (and kind) to note the original author in the changelog when basing a patch on previous work. Something like "originally written by..." or "based on the work of..." etc. Thanks, Kevin --
Ok, I can do that; that was the intention of the "original inspiration from" line at the beginning. Is there a more formal way of indicating this in the next version of the patch? Should I add you as a "From:" or an "Author:"? -Pat -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum --
I don't know if there is an official way of doing this. I typically add something like "based on the idea/code originally proposed/written by John Doe" in the changelog, and then add Cc: John Doe <...> in the changelog too, but AFAIK, none of this is formalized. Kevin --
On Thu, Aug 5, 2010 at 9:57 AM, Kevin Hilman As of now, the of_platform_bus_type has been removed. It was a bad idea because it tried to encode non-bus-specific information into something that was just a clone of the platform_bus. Drivers that worked on both had to be bound to both busses. I do actually have code that automatically registers a driver on more than one bus, but it is rather a hack and was only a temporary measure. The relevant question before going down this path is, "Is the omap/sh/other-soc behaviour something fundamentally different from the platform bus? Or is it something complementary that would be better handled with a notifier or some orthogonal method of adding new behaviour?" I don't have a problem with multiple platform_bus instances using the same code (I did suggest it after all), but I do worry about muddying the Linux device model or making it overly complex. Binding single drivers to multiple device types could be messy. Cheers, g. --
From your other email, the distinction of /bus_types/ versus /physical
or logical buses/ was very valuable (thanks). I am becoming less
convinced that I need this infrastructure or the ability to create
pseudo-platform buses. Let me outline some thoughts below, which I
think at least solves my problems ( ;) ), and if some of the OMAP folks
and Alan could chime in as to whether they can apply something similar,
or if they still have need of creating actual new bus types?
As we are exploring all of this, let me put a concrete (ish) scenario
out there to discuss:
SUB_BUS1---DEVICE1
/
CPU ---
\
SUB_BUS2---DEVICE2
DEVICE1 and DEVICE2 are the same device (say, usb host controller, or
whatever), and they should be driven by the same driver. However,
physically they are attached to different buses.
SUB_BUS1 and SUB_BUS2 are *different* and let's say require a different
suspend method. If I understand correctly, the right way to build the
topology would be:
struct device_type SUB_BUS1_type = {
.name = "sub-bus1-type",
.pm = &sub_bus1_pm_ops;
};
struct platform_device SUB_BUS1 = {
.init_name = "sub-bus1",
.dev.type = &sub_bus1_type,
};
struct platform_device DEVICE1 = {
.name = "device1",
.dev.parent = &SUB_BUS1.dev,
};
platform_device_register(&SUB_BUS1);
platform_device_register(&DEVICE1);
[analogous for *2]
which would yield:
/sys/bus/platform/devices/
|
|-SUB_BUS1
| |
| |-DEVICE1
|
|-SUB_BUS2
| |
| |-DEVICE2
Which is the correct topology, and logically provides all the correct
interfaces. The only thing that becomes confusing now, is that SUB_BUS*
is *actually* a physically different bus, yet it's not in /sys/bus;
although I suppose this is actually how the world works presently (you
don't see an individual entry in /sys/bus for each usb host controller,
which is technically defining a sub-bus...)
Thoughts? Am I understanding everything correctly?
Is there anything more not ...Not quite. The device_type is intended to collect similar, but different things (ie, all keyboards, regardless of how they are attached to the system). Trying to capture the device-specific behavour really belongs in the specific device driver attached to the SUB_BUS* device. In fact, the way you've constructed your example, SUB_BUS1 and SUB_BUS2 don't really need to be platform_devices at all; you could have used a plain struct device because in this example you aren't binding them to a device driver). That being said, because each bus *does* have custom behaviour, it probably does make sense to either bind each to a device driver, or to do something to each child device that changes the PM behaviour. I haven't dug into the problem domain deep enough to give you absolute answers, but the devres approach looks promising, as does creating a derived platform_bus_type (although I'm not fond of sharing device drivers between multiple bus_types). Another option might be giving the parent struct device some runtime PM operations that it performs You're looking in the wrong place. Look in /sys/devices instead of /sys/bus is for bus_types, not bus instances. All USB devices will be listed in /sys/bus/usb/devices regardless of the bus instance that they attached to. However, look carefully at the files in /sys/bus/*/devices. Notice that they are all symlinks? Notice that the all of them point to directories in /sys/devices? /sys/bus tells you which devices are registered against each bus type, but /sys/devices is the actual structure. Always look in /sys/devices when you want to know the topology of the system. Cheers, g. --
Thanks for your patch, it's really nice to see work done in this area! I'd like to see something like this merged in the not so distant future. At this point I'm not so concerned about the details, so I'll I would really prefer not to have the bus type in the here. I understand it's needed at this point, but I wonder if it's possible to adjust the device<->driver matching for platform devices to allow any type of pseudo-platform bus_type. The reason why I'd like to avoid having the bus type in the driver is that I'd like to reuse the platform driver across multiple architectures and buses. For instance, on the SH architecture and SH-Mobile ARM we have SoCs with SCIF hardware blocks driven by the sh-sci.c serial driver. The sh-sci.c platform driver supports a wide range of different SCI(F)(A)(B) hardware blocks, and on any given SoC there is a mix of SCIF blocks spread out on different buses. At this point our SH platform drivers are unaware where their driver instanced are located on the SoC. The I/O address and IRQs are assigned via struct resource and clocks are managed through clkdev. I believe that adding the bus type in the driver will violate this abstraction and make it more difficult to just instantiate a driver This I don't mind at all. Actually, this is the place where the topology should be defined IMO. Cheers, / magnus --
I totally agree here. Keeping the drivers ignorant of the bus (or SoC) they are on will make them much more portable. Kevin --
So would I :). That's where this was all heading eventually, I was just originally doing it in two passes. I have some ideas for how to do this -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum --
| Greg KH | Og dreams of kernels |
| Jens Axboe | [PATCH 31/33] Fusion: sg chaining support |
| Arnd Bergmann | Re: finding your own dead "CONFIG_" variables |
| Mark Brown | [PATCH 2/2] Subject: natsemi: Allow users to disable workaround for DspCfg reset |
| Tony Breeds | [LGUEST] Look in object dir for .config |
git: | |
| Brian Downing | Re: Git in a Nutshell guide |
| John Benes | Re: master has some toys |
| Matthias Lederhofer | [PATCH 4/7] introduce GIT_WORK_TREE to specify the work tree |
| Alexander Sulfrian | [RFC/PATCH] RE: git calls SSH_ASKPASS even if DISPLAY is not set |
| Junio C Hamano | Re: Rss produced by git is not valid xml? |
| Linux Kernel Mailing List |
