linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] driver-core: Remove dummy 'platform_bus'
@ 2012-11-21 14:44 Grant Likely
  2012-11-21 14:52 ` Greg Kroah-Hartman
  2014-04-21 21:05 ` Rob Herring
  0 siblings, 2 replies; 10+ messages in thread
From: Grant Likely @ 2012-11-21 14:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: Grant Likely, Greg Kroah-Hartman, Kay Sievers

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

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kay Sievers <kay.sievers@vrfy.org>
---
 arch/arm/plat-mxc/devices.c         |    2 --
 arch/unicore32/kernel/puv3-core.c   |    2 +-
 arch/unicore32/kernel/puv3-nb0916.c |    6 +++---
 drivers/base/platform.c             |   16 +---------------
 drivers/char/tile-srom.c            |    2 +-
 drivers/mmc/host/sdhci-pltfm.c      |    6 ++----
 drivers/scsi/hosts.c                |    2 +-
 include/linux/platform_device.h     |    1 -
 8 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/arch/arm/plat-mxc/devices.c b/arch/arm/plat-mxc/devices.c
index 4d55a7a..7df592b 100644
--- a/arch/arm/plat-mxc/devices.c
+++ b/arch/arm/plat-mxc/devices.c
@@ -25,12 +25,10 @@
 
 struct device mxc_aips_bus = {
 	.init_name	= "mxc_aips",
-	.parent		= &platform_bus,
 };
 
 struct device mxc_ahb_bus = {
 	.init_name	= "mxc_ahb",
-	.parent		= &platform_bus,
 };
 
 static int __init mxc_device_init(void)
diff --git a/arch/unicore32/kernel/puv3-core.c b/arch/unicore32/kernel/puv3-core.c
index 254adee..438dd2e 100644
--- a/arch/unicore32/kernel/puv3-core.c
+++ b/arch/unicore32/kernel/puv3-core.c
@@ -272,7 +272,7 @@ void __init puv3_core_init(void)
 	platform_device_register_simple("PKUnity-v3-UART", 1,
 			puv3_uart1_resources, ARRAY_SIZE(puv3_uart1_resources));
 	platform_device_register_simple("PKUnity-v3-AC97", -1, NULL, 0);
-	platform_device_register_resndata(&platform_bus, "musb_hdrc", -1,
+	platform_device_register_resndata(NULL, "musb_hdrc", -1,
 			puv3_usb_resources, ARRAY_SIZE(puv3_usb_resources),
 			&puv3_usb_plat, sizeof(puv3_usb_plat));
 }
diff --git a/arch/unicore32/kernel/puv3-nb0916.c b/arch/unicore32/kernel/puv3-nb0916.c
index 181108b..e9991b6 100644
--- a/arch/unicore32/kernel/puv3-nb0916.c
+++ b/arch/unicore32/kernel/puv3-nb0916.c
@@ -111,13 +111,13 @@ int __init mach_nb0916_init(void)
 	platform_device_register_simple("PKUnity-v3-I2C", -1,
 			puv3_i2c_resources, ARRAY_SIZE(puv3_i2c_resources));
 
-	platform_device_register_data(&platform_bus, "pwm-backlight", -1,
+	platform_device_register_data(NULL, "pwm-backlight", -1,
 			&nb0916_backlight_data, sizeof(nb0916_backlight_data));
 
-	platform_device_register_data(&platform_bus, "gpio-keys", -1,
+	platform_device_register_data(NULL, "gpio-keys", -1,
 			&nb0916_gpio_button_data, sizeof(nb0916_gpio_button_data));
 
-	platform_device_register_resndata(&platform_bus, "physmap-flash", -1,
+	platform_device_register_resndata(NULL, "physmap-flash", -1,
 			&physmap_flash_resource, 1,
 			&physmap_flash_data, sizeof(physmap_flash_data));
 
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 72c776f..4f1c969 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -31,11 +31,6 @@ static DEFINE_IDA(platform_devid_ida);
 #define to_platform_driver(drv)	(container_of((drv), struct platform_driver, \
 				 driver))
 
-struct device platform_bus = {
-	.init_name	= "platform",
-};
-EXPORT_SYMBOL_GPL(platform_bus);
-
 /**
  * arch_setup_pdev_archdata - Allow manipulation of archdata before its used
  * @pdev: platform device
@@ -283,9 +278,6 @@ int platform_device_add(struct platform_device *pdev)
 	if (!pdev)
 		return -EINVAL;
 
-	if (!pdev->dev.parent)
-		pdev->dev.parent = &platform_bus;
-
 	pdev->dev.bus = &platform_bus_type;
 
 	switch (pdev->id) {
@@ -883,13 +875,7 @@ int __init platform_bus_init(void)
 
 	early_platform_cleanup();
 
-	error = device_register(&platform_bus);
-	if (error)
-		return error;
-	error =  bus_register(&platform_bus_type);
-	if (error)
-		device_unregister(&platform_bus);
-	return error;
+	return bus_register(&platform_bus_type);
 }
 
 #ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK
diff --git a/drivers/char/tile-srom.c b/drivers/char/tile-srom.c
index 3b22a60..854ab2b 100644
--- a/drivers/char/tile-srom.c
+++ b/drivers/char/tile-srom.c
@@ -369,7 +369,7 @@ static int srom_setup_minor(struct srom_dev *srom, int index)
 		       SROM_PAGE_SIZE_OFF, sizeof(srom->page_size)) < 0)
 		return -EIO;
 
-	dev = device_create(srom_class, &platform_bus,
+	dev = device_create(srom_class, NULL,
 			    MKDEV(srom_major, index), srom, "%d", index);
 	return IS_ERR(dev) ? PTR_ERR(dev) : 0;
 }
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 2716445..841425c 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -115,10 +115,8 @@ struct sdhci_host *sdhci_pltfm_init(struct platform_device *pdev,
 		dev_err(&pdev->dev, "Invalid iomem size!\n");
 
 	/* Some PCI-based MFD need the parent here */
-	if (pdev->dev.parent != &platform_bus && !np)
-		host = sdhci_alloc_host(pdev->dev.parent, sizeof(*pltfm_host));
-	else
-		host = sdhci_alloc_host(&pdev->dev, sizeof(*pltfm_host));
+	host = sdhci_alloc_host(pdev->dev.parent ? pdev->dev.parent : &pdev->dev,
+				sizeof(*pltfm_host));
 
 	if (IS_ERR(host)) {
 		ret = PTR_ERR(host);
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 593085a..c1f9966 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -217,7 +217,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
 		goto fail;
 
 	if (!shost->shost_gendev.parent)
-		shost->shost_gendev.parent = dev ? dev : &platform_bus;
+		shost->shost_gendev.parent = dev;
 	if (!dma_dev)
 		dma_dev = shost->shost_gendev.parent;
 
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 5711e95..e01efb3 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -44,7 +44,6 @@ extern int platform_device_register(struct platform_device *);
 extern void platform_device_unregister(struct platform_device *);
 
 extern struct bus_type platform_bus_type;
-extern struct device platform_bus;
 
 extern void arch_setup_pdev_archdata(struct platform_device *);
 extern struct resource *platform_get_resource(struct platform_device *, unsigned int, unsigned int);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2012-11-21 14:44 [RFC] driver-core: Remove dummy 'platform_bus' Grant Likely
@ 2012-11-21 14:52 ` Greg Kroah-Hartman
  2012-11-22 19:17   ` Kay Sievers
  2014-04-21 21:05 ` Rob Herring
  1 sibling, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-21 14:52 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-kernel, Kay Sievers

On Wed, Nov 21, 2012 at 02:44:31PM +0000, Grant Likely 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

If the devices don't show up under platform/ where are they going to be
at now, virtual/ ?  That doesn't sound like a good plan, they should be
somewhere "useful".

And yes, odds are this will break userspace, but we might not know until
we try it, have you tried it on different distros to see what happens?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2012-11-21 14:52 ` Greg Kroah-Hartman
@ 2012-11-22 19:17   ` Kay Sievers
  2012-11-22 21:20     ` Grant Likely
  0 siblings, 1 reply; 10+ messages in thread
From: Kay Sievers @ 2012-11-22 19:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Grant Likely, linux-kernel

On Wed, Nov 21, 2012 at 3:52 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Nov 21, 2012 at 02:44:31PM +0000, Grant Likely 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
>
> If the devices don't show up under platform/ where are they going to be
> at now, virtual/ ?  That doesn't sound like a good plan, they should be
> somewhere "useful".

Just a note to keep in mind: We usually need and want devices to have
a bus or class. Devices without a "subsystem" are invisible to udev,
and do not get proper coldplug support at bootup.

Kay

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2012-11-22 19:17   ` Kay Sievers
@ 2012-11-22 21:20     ` Grant Likely
  2012-11-23 14:39       ` Kay Sievers
  0 siblings, 1 reply; 10+ messages in thread
From: Grant Likely @ 2012-11-22 21:20 UTC (permalink / raw)
  To: Kay Sievers; +Cc: Greg Kroah-Hartman, Linux Kernel Mailing List

On Thu, Nov 22, 2012 at 7:17 PM, Kay Sievers <kay@vrfy.org> wrote:
> On Wed, Nov 21, 2012 at 3:52 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Wed, Nov 21, 2012 at 02:44:31PM +0000, Grant Likely 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
>>
>> If the devices don't show up under platform/ where are they going to be
>> at now, virtual/ ?  That doesn't sound like a good plan, they should be
>> somewhere "useful".
>
> Just a note to keep in mind: We usually need and want devices to have
> a bus or class. Devices without a "subsystem" are invisible to udev,
> and do not get proper coldplug support at bootup.

Note: this patch is only about the "platform_bus" dummy device. It has
nothing to do with platform_bus_type.

g.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2012-11-22 21:20     ` Grant Likely
@ 2012-11-23 14:39       ` Kay Sievers
  0 siblings, 0 replies; 10+ messages in thread
From: Kay Sievers @ 2012-11-23 14:39 UTC (permalink / raw)
  To: Grant Likely; +Cc: Greg Kroah-Hartman, Linux Kernel Mailing List

On Thu, Nov 22, 2012 at 10:20 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Thu, Nov 22, 2012 at 7:17 PM, Kay Sievers <kay@vrfy.org> wrote:
>> On Wed, Nov 21, 2012 at 3:52 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

>>> If the devices don't show up under platform/ where are they going to be
>>> at now, virtual/ ?  That doesn't sound like a good plan, they should be
>>> somewhere "useful".
>>
>> Just a note to keep in mind: We usually need and want devices to have
>> a bus or class. Devices without a "subsystem" are invisible to udev,
>> and do not get proper coldplug support at bootup.
>
> Note: this patch is only about the "platform_bus" dummy device. It has
> nothing to do with platform_bus_type.

Ah, I see now.

Why do you want to remove the "platform_bus" fake-parent, entirely? I
understand and agree that drivers should not fiddle with that
directly. But I don't see a real reason why it should not be private
to the platform_bus_type. It's nothing really wrong with it, I guess.

I have on x86:
  coretemp.0 -> ../../../devices/platform/coretemp.0
  dock.0 -> ../../../devices/platform/dock.0
  dock.1 -> ../../../devices/platform/dock.1
  efifb.0 -> ../../../devices/platform/efifb.0
  i8042 -> ../../../devices/platform/i8042
  iTCO_wdt -> ../../../devices/pci0000:00/0000:00:1f.0/iTCO_wdt
  pcspkr -> ../../../devices/platform/pcspkr
  serial8250 -> ../../../devices/platform/serial8250
  thinkpad_acpi -> ../../../devices/platform/thinkpad_acpi
  thinkpad_hwmon -> ../../../devices/platform/thinkpad_hwmon

which look pretty reasonable in a "platform parent" instead of ending
up in virtual/.

We should probably remove the explicit assignment in the drivers like
your patch does, unexport "platform_bus" from the core, so nobody can
use it anymore in the future. The /sys/bus/platform/devices/ directory
can then be created on-demand only, with the first registration of a
device without a parent, we do that in other parts of the core
already.

Wouldn't that solve your problem and still do not touch any other
stuff visible in /sys?

Also, it seems there are devices without any subsystem in the list of
things your patch touches? That is really not how things should work.
All devices should have a bus or class, so userspace receives proper
events, and can enumerate them.
The /sys/devices/ hierarchy is not really meant to be used directly,
it can for some devices even change at runtime. It's not considered a
stable or reasonable predictable ABI, all lookups should start in the
flat device symlink lists of the class/ and bus/, and not in the
hierarchical tree in devices/.

Kay

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2012-11-21 14:44 [RFC] driver-core: Remove dummy 'platform_bus' Grant Likely
  2012-11-21 14:52 ` Greg Kroah-Hartman
@ 2014-04-21 21:05 ` Rob Herring
  2014-04-23 14:05   ` Grant Likely
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2014-04-21 21:05 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-kernel, Greg Kroah-Hartman, Kay Sievers

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.

Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2014-04-21 21:05 ` Rob Herring
@ 2014-04-23 14:05   ` Grant Likely
  2014-04-23 14:16     ` Rob Herring
  2014-04-23 14:44     ` Grant Likely
  0 siblings, 2 replies; 10+ messages in thread
From: Grant Likely @ 2014-04-23 14:05 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel, Greg Kroah-Hartman, Kay Sievers

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.

---

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);


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2014-04-23 14:05   ` Grant Likely
@ 2014-04-23 14:16     ` Rob Herring
  2014-04-23 14:49       ` Grant Likely
  2014-04-23 14:44     ` Grant Likely
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2014-04-23 14:16 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-kernel, Greg Kroah-Hartman, Kay Sievers

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2014-04-23 14:05   ` Grant Likely
  2014-04-23 14:16     ` Rob Herring
@ 2014-04-23 14:44     ` Grant Likely
  1 sibling, 0 replies; 10+ messages in thread
From: Grant Likely @ 2014-04-23 14:44 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel, Greg Kroah-Hartman, Kay Sievers

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);
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC] driver-core: Remove dummy 'platform_bus'
  2014-04-23 14:16     ` Rob Herring
@ 2014-04-23 14:49       ` Grant Likely
  0 siblings, 0 replies; 10+ messages in thread
From: Grant Likely @ 2014-04-23 14:49 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel, Greg Kroah-Hartman, Kay Sievers

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-04-23 14:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-21 14:44 [RFC] driver-core: Remove dummy 'platform_bus' Grant Likely
2012-11-21 14:52 ` Greg Kroah-Hartman
2012-11-22 19:17   ` Kay Sievers
2012-11-22 21:20     ` Grant Likely
2012-11-23 14:39       ` Kay Sievers
2014-04-21 21:05 ` Rob Herring
2014-04-23 14:05   ` Grant Likely
2014-04-23 14:16     ` Rob Herring
2014-04-23 14:49       ` Grant Likely
2014-04-23 14:44     ` Grant Likely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).