| Message ID | 20140423140508.2575AC408D2@trevor.secretlab.ca |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 23, 2014 at 9:05 AM, Grant Likely <grant.likely@linaro.org> wrote: > On Mon, 21 Apr 2014 16:05:29 -0500, Rob Herring <robherring2@gmail.com> wrote: >> On Wed, Nov 21, 2012 at 8:44 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> > The "platform_bus" (note: not platform_bus_type) only exists as an empty >> > directory to put platform devices into. However, it really doesn't make >> > sense to segregate all the platform devices into a sub directory when >> > typically they are memory mapped devices that doen't go through any >> > particular bus. Particularly on embedded type platforms the platform_bus >> > directory doesn't add anything. >> > >> > However, this will probably just end up breaking some userspace that >> > depends on the /sys/devices/platform/ path to be present (no matter how >> > much we protest that userspace must not depend on paths in sysfs). So >> > while I'm seriously proposing this change, it may just be unacceptable >> > ABI breakage >> >> An old thread, but was there ever a conclusion to this? We now have a >> mixture of using platform_bus as the parent or not on various ARM >> platforms. > > We kind of concluded in the opposite direction. Instead of removing the > /sys/device/platform directory, the drivers/of code should be changed to > use it. > > The following patch is sufficient to have the same effect. It doesn't > unify the OF and non-OF paths of platform device addition, but it gets > them closer. I've been nervous about applying it because I'm concerned > about userspace breakage, but maybe it just needs to be merged and we > can quirk out systems that break. Given that we've changed practically all device names in converting to DT and I haven't heard of any complaints, we may be okay. We also have some platforms (imx6 for example) setting the parent to an soc device. I still need to understand why the soc device needs to be the parent, but it is pointless platform variation in my book. It would also change the paths when someone has the whim to add an soc device. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Wed, Apr 23, 2014 at 3:05 PM, Grant Likely <grant.likely@linaro.org> wrote: > On Mon, 21 Apr 2014 16:05:29 -0500, Rob Herring <robherring2@gmail.com> wrote: >> On Wed, Nov 21, 2012 at 8:44 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> > The "platform_bus" (note: not platform_bus_type) only exists as an empty >> > directory to put platform devices into. However, it really doesn't make >> > sense to segregate all the platform devices into a sub directory when >> > typically they are memory mapped devices that doen't go through any >> > particular bus. Particularly on embedded type platforms the platform_bus >> > directory doesn't add anything. >> > >> > However, this will probably just end up breaking some userspace that >> > depends on the /sys/devices/platform/ path to be present (no matter how >> > much we protest that userspace must not depend on paths in sysfs). So >> > while I'm seriously proposing this change, it may just be unacceptable >> > ABI breakage >> >> An old thread, but was there ever a conclusion to this? We now have a >> mixture of using platform_bus as the parent or not on various ARM >> platforms. > > We kind of concluded in the opposite direction. Instead of removing the > /sys/device/platform directory, the drivers/of code should be changed to > use it. > > The following patch is sufficient to have the same effect. It doesn't > unify the OF and non-OF paths of platform device addition, but it gets > them closer. I've been nervous about applying it because I'm concerned > about userspace breakage, but maybe it just needs to be merged and we > can quirk out systems that break. Ugh, no matter what we do, something is going to be inconsistent. With this patch, the platform_devices get moved under /sys/devices/platform, but an amba_device at the top level stays where it is. We could move amba devices under the same hierarchy, but that doesn't seem appropriate. We could have a separate /sys/devices/of hierarchy for generating devices into if this really is a problem that needs to be solved. I'm not convinced that a real problem exists though. PowerPC has been creating the devices directly under /sys/devices at least since 2.6.12. With the DT being hierarchical to begin with, and the dt code preserving the hierarchy, on most platforms there won't be a lot of things at the root. Anyway, whatever. I don't care enough to argue anymore. Move them if you like, but make sure the root AMBA devices get created in the same place. g. > > --- > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 404d1daebefa..40a85b85c932 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -175,7 +175,7 @@ struct platform_device *of_device_alloc(struct device_node *np, > #if defined(CONFIG_MICROBLAZE) > dev->dev.dma_mask = &dev->archdata.dma_mask; > #endif > - dev->dev.parent = parent; > + dev->dev.parent = parent ? parent : &platform_bus; > > if (bus_id) > dev_set_name(&dev->dev, "%s", bus_id); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Wed, Apr 23, 2014 at 3:16 PM, Rob Herring <robherring2@gmail.com> wrote: > On Wed, Apr 23, 2014 at 9:05 AM, Grant Likely <grant.likely@linaro.org> wrote: >> On Mon, 21 Apr 2014 16:05:29 -0500, Rob Herring <robherring2@gmail.com> wrote: >>> On Wed, Nov 21, 2012 at 8:44 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>> > The "platform_bus" (note: not platform_bus_type) only exists as an empty >>> > directory to put platform devices into. However, it really doesn't make >>> > sense to segregate all the platform devices into a sub directory when >>> > typically they are memory mapped devices that doen't go through any >>> > particular bus. Particularly on embedded type platforms the platform_bus >>> > directory doesn't add anything. >>> > >>> > However, this will probably just end up breaking some userspace that >>> > depends on the /sys/devices/platform/ path to be present (no matter how >>> > much we protest that userspace must not depend on paths in sysfs). So >>> > while I'm seriously proposing this change, it may just be unacceptable >>> > ABI breakage >>> >>> An old thread, but was there ever a conclusion to this? We now have a >>> mixture of using platform_bus as the parent or not on various ARM >>> platforms. >> >> We kind of concluded in the opposite direction. Instead of removing the >> /sys/device/platform directory, the drivers/of code should be changed to >> use it. >> >> The following patch is sufficient to have the same effect. It doesn't >> unify the OF and non-OF paths of platform device addition, but it gets >> them closer. I've been nervous about applying it because I'm concerned >> about userspace breakage, but maybe it just needs to be merged and we >> can quirk out systems that break. > > Given that we've changed practically all device names in converting to > DT and I haven't heard of any complaints, we may be okay. > > We also have some platforms (imx6 for example) setting the parent to > an soc device. I still need to understand why the soc device needs to > be the parent, but it is pointless platform variation in my book. It > would also change the paths when someone has the whim to add an soc > device. Yes, that is just dumb and should be discouraged. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 404d1daebefa..40a85b85c932 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -175,7 +175,7 @@ struct platform_device *of_device_alloc(struct device_node *np, #if defined(CONFIG_MICROBLAZE) dev->dev.dma_mask = &dev->archdata.dma_mask; #endif - dev->dev.parent = parent; + dev->dev.parent = parent ? parent : &platform_bus; if (bus_id) dev_set_name(&dev->dev, "%s", bus_id);