platform: Faciliatate the creation of pseduo-platform busses
in [Kernel]
|
Prev: timer: Added usleep_range timer
Next: [tip:x86/hwmon] x86, hwmon: Package Level Thermal/Power: pkgtemp documentation
From: Greg KH on 3 Aug 2010 20:00 On Tue, Aug 03, 2010 at 04:35:06PM -0700, Patrick Pannuto wrote: > Inspiration for this comes from: > http://www.mail-archive.com/linux-omap(a)vger.kernel.org/msg31161.html > > 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); You really want a bus hanging off of a bus? Normally you need a device to do that, which is what I think you have here, but the naming is a bit odd to me. What would you do with this "sub bus"? It's just a device, but you are wanting it to be around for something. > > 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_CPY(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, > }; Ah, you put devices on this "sub bus". But why? Why not just put it on your "normal" bus that you created? What's with the extra level of nesting here? Other than that, it looks like a nice idea, are there portions of kernel code that can be simplified because of this? > @@ -539,12 +541,12 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv, > * if the probe was successful, and make sure any forced probes of > * new devices fail. > */ > - spin_lock(&platform_bus_type.p->klist_drivers.k_lock); > + spin_lock(&drv->driver.bus->p->klist_drivers.k_lock); > drv->probe = NULL; > if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list)) > retval = -ENODEV; > drv->driver.probe = platform_drv_probe_fail; > - spin_unlock(&platform_bus_type.p->klist_drivers.k_lock); > + spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock); > > if (code != retval) > platform_driver_unregister(drv); I'm guessing that this chunk can be applied now, right? > @@ -1017,6 +1019,26 @@ struct bus_type platform_bus_type = { > }; > EXPORT_SYMBOL_GPL(platform_bus_type); > > +/** platform_bus_type_init - fill in a pseudo-platform-bus > + * @bus: foriegn bus type > + * > + * This init is basically a selective memcpy that > + * won't overwrite any user-defined attributes and > + * only copies things that platform bus defines anyway > + */ > +void platform_bus_type_init(struct bus_type *bus) > +{ > + if (!bus->dev_attrs) > + bus->dev_attrs = platform_bus_type.dev_attrs; > + if (!bus->match) > + bus->match = platform_bus_type.match; > + if (!bus->uevent) > + bus->uevent = platform_bus_type.uevent; > + if (!bus->pm) > + bus->pm = platform_bus_type.pm; Watch out for things in "write only" memory here. That could cause problems. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 3 Aug 2010 20:20 On Tue, Aug 03, 2010 at 05:02:29PM -0700, Patrick Pannuto wrote: > On 08/03/2010 04:56 PM, Greg KH wrote: > > On Tue, Aug 03, 2010 at 04:35:06PM -0700, Patrick Pannuto wrote: > >> Inspiration for this comes from: > >> http://www.mail-archive.com/linux-omap(a)vger.kernel.org/msg31161.html > >> > >> 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); For your question below, this could be in write-only memory. Well, I guess it never is as we modify things in the bus structure, so nevermind about that, false alarm. > >> > >> struct platform_device sub_bus1 = { > >> .name = "sub_bus1", > >> .id = -1, > >> .dev.bus = &my_bus_type, > >> } > >> EXPORT_SYMBOL_GPL(sub_bus1); > > > > You really want a bus hanging off of a bus? Normally you need a device > > to do that, which is what I think you have here, but the naming is a bit > > odd to me. > > > > What would you do with this "sub bus"? It's just a device, but you are > > wanting it to be around for something. > > > > It's for power management stuff, basically, there are actual physical buses > involved that can be completely powered off IFF all of their devices are > not in use. Plus it actually matches bus topology this way. Then create a real bus hanging off of a device, not another device that "acts" like a bus here, right? Or am I missing the point? > >> +void platform_bus_type_init(struct bus_type *bus) > >> +{ > >> + if (!bus->dev_attrs) > >> + bus->dev_attrs = platform_bus_type.dev_attrs; > >> + if (!bus->match) > >> + bus->match = platform_bus_type.match; > >> + if (!bus->uevent) > >> + bus->uevent = platform_bus_type.uevent; > >> + if (!bus->pm) > >> + bus->pm = platform_bus_type.pm; > > > > Watch out for things in "write only" memory here. That could cause > > problems. > > Pardon my ignorance (I'm quite new to kernel work), what do you mean > here? What memory could be "write only"? See above. I was thinking that struct bus would be a constant or something. Sorry. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 4 Aug 2010 05:30 > (possibly in a header) > #ifdef CONFIG_MY_BUS > #define MY_BUS_TYPE &my_bus_type > #else > #define MY_BUS_TYPE NULL > #endif > > /drivers/my_driver.c > static struct platform_driver my_driver = { > .driver = { > .name = "my-driver", > .owner = THIS_MODULE, > .bus = MY_BUS_TYPE, > }, > }; > > Which will allow the same driver to easily to used on either > the platform bus or the newly defined bus type. At compile time. I suspect there is an argument for having an "ANY_BUS_TYPE" value for the devices so that you can runtime wildcard stuff which works whatever sub-bus it is hung off. > I believe this to be a fairly elegant and simple solution to the > problem, but humbly RFC It's exactly what I need to tidy up the SCU IPC devices on the new Intel MID platforms certainly. It's also a very small patch to achieve it Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 6 Aug 2010 10:40 On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote: > (On that point Greg, what is the reason for even having the > /sys/devices/platform/ parent? Why not just let the platform devices > sit at the root of the device tree? In the OF case (granted, I'm > biased) all of the platform_device registrations reflect the actual > device hierarchy expressed in the device tree data.) If we sat them at the "root", there would be a bunch of them there. I don't know, we could drop the parent, I guess whoever created the platform device oh so long ago, decided that it would look nicer to be in this type of structure. > Now, having gone on this whole long tirade, it looks like having > separate platform bus types may not be the best approach after all. I totally agree, and thanks for the detailed explaination, it saved me from having to write up the same thing :) greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 6 Aug 2010 20:00
On Fri, Aug 06, 2010 at 09:12:27AM -0600, Grant Likely wrote: > On Fri, Aug 6, 2010 at 8:27 AM, Greg KH <gregkh(a)suse.de> wrote: > > On Thu, Aug 05, 2010 at 04:59:35PM -0600, Grant Likely wrote: > >> (On that point Greg, what is the reason for even having the > >> /sys/devices/platform/ parent? �Why not just let the platform devices > >> sit at the root of the device tree? �In the OF case (granted, I'm > >> biased) all of the platform_device registrations reflect the actual > >> device hierarchy expressed in the device tree data.) > > > > If we sat them at the "root", there would be a bunch of them there. �I > > don't know, we could drop the parent, I guess whoever created the > > platform device oh so long ago, decided that it would look nicer to be > > in this type of structure. > > Personally I'd rather see a meaningful structure used here. Maybe > having them all in the root will encourage people to find realistic > parents for their platform devices. :-) That would be nice, but take your "standard" PC today: > ls /sys/devices/platform/ Fixed MDIO bus.0 i8042 pcspkr power serial8250 uevent vesafb.0 There are tty devices below the serial port, which is nice to see, but the others? I don't know what type of bus they would be on, do you? > Why don't I float a patch to remove this and see if anybody freaks > out. Should I wrap it with a CONFIG_ so that it can be configurable > for a release or to, or just make it unconditional? If you can figure out a structure for the desktop/server machines, sure, I say just always enable it :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |