[RFC/PATCH] of: platform: Remove unique device name enforcement

[RFC/PATCH] of: platform: Remove unique device name enforcement

[Ezequiel Garcia]

When creating a device object for a devicetree node, the device name
is created by using the node name and the 'reg' property, to make a name
such as "a000.foo_device".

For certain devices without an associated address, and hence no 'reg' property,
the current code attempts to make a unique name, by using a global integer.
Names look like "foo_device.1", "bar_device.2", and so on.
Examples of such devices are: gpio-keys', backlights and rotary-encoders.

The system cannot know such devices name before hand, given they are determined
by the kernel probe order and by the nodes present in the devicetree. This can
be problematic, on systems that are tied to the device's name, e.g. when
catching hotplug events.

In order to fix the name, this commit removes the global integer uniqueness
enforcement and instead uses the node name for the device name. The rationale
behind such change, is that there's no way two nodes with the same node name
are associated to two different devices, by the nature of the devicetree.
In other words, it's impossible to produce a name collision for these kind
of devices.

Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
Hello Grant, Rob:

I'm pretty sure there's a good reason for the existence of the atomic magic
integer, so this is not a *real* patch, but rather a bold attempt at asking you
some explanations for this.

Under what circumstances does the magic integer avoids a name collision?
Will this propposal introduce a regression?

 drivers/of/platform.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 60292e1..0f091a3 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -65,20 +65,17 @@ EXPORT_SYMBOL(of_find_device_by_node);
  */
 
 /**
- * of_device_make_bus_id - Use the device node data to assign a unique name
+ * of_device_make_bus_id - Use the device node data to assign name
  * @dev: pointer to device structure that is linked to a device tree node
  *
  * This routine will first try using either the dcr-reg or the reg property
- * value to derive a unique name.  As a last resort it will use the node
- * name followed by a unique number.
+ * value to derive the device name.
  */
 void of_device_make_bus_id(struct device *dev)
 {
-	static atomic_t bus_no_reg_magic;
 	struct device_node *node = dev->of_node;
 	const __be32 *reg;
 	u64 addr;
-	int magic;
 
 #ifdef CONFIG_PPC_DCR
 	/*
@@ -113,12 +110,7 @@ void of_device_make_bus_id(struct device *dev)
 		}
 	}
 
-	/*
-	 * No BusID, use the node name and add a globally incremented
-	 * counter (and pray...)
-	 */
-	magic = atomic_add_return(1, &bus_no_reg_magic);
-	dev_set_name(dev, "%s.%d", node->name, magic - 1);
+	dev_set_name(dev, "%s", node->name);
 }
 
 /**
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Rob Herring]

On Sun, May 18, 2014 at 12:01 PM, Ezequiel Garcia
<ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> When creating a device object for a devicetree node, the device name
> is created by using the node name and the 'reg' property, to make a name
> such as "a000.foo_device".
>
> For certain devices without an associated address, and hence no 'reg' property,
> the current code attempts to make a unique name, by using a global integer.
> Names look like "foo_device.1", "bar_device.2", and so on.
> Examples of such devices are: gpio-keys', backlights and rotary-encoders.
>
> The system cannot know such devices name before hand, given they are determined
> by the kernel probe order and by the nodes present in the devicetree. This can
> be problematic, on systems that are tied to the device's name, e.g. when
> catching hotplug events.
>
> In order to fix the name, this commit removes the global integer uniqueness
> enforcement and instead uses the node name for the device name. The rationale
> behind such change, is that there's no way two nodes with the same node name
> are associated to two different devices, by the nature of the devicetree.
> In other words, it's impossible to produce a name collision for these kind
> of devices.

This is perfectly legal:

/bus1/device
/bus2/device

Only the full name has to be unique.

>
> Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
> ---
> Hello Grant, Rob:
>
> I'm pretty sure there's a good reason for the existence of the atomic magic
> integer, so this is not a *real* patch, but rather a bold attempt at asking you
> some explanations for this.
>
> Under what circumstances does the magic integer avoids a name collision?
> Will this propposal introduce a regression?

See the recent changes to this function in linux-next and the
associated discussion of those commits.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Grant Likely]

On Sun, 18 May 2014 18:11:07 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Sun, May 18, 2014 at 12:01 PM, Ezequiel Garcia
> <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> > When creating a device object for a devicetree node, the device name
> > is created by using the node name and the 'reg' property, to make a name
> > such as "a000.foo_device".
> >
> > For certain devices without an associated address, and hence no 'reg' property,
> > the current code attempts to make a unique name, by using a global integer.
> > Names look like "foo_device.1", "bar_device.2", and so on.
> > Examples of such devices are: gpio-keys', backlights and rotary-encoders.
> >
> > The system cannot know such devices name before hand, given they are determined
> > by the kernel probe order and by the nodes present in the devicetree. This can
> > be problematic, on systems that are tied to the device's name, e.g. when
> > catching hotplug events.

The device's uevent file in sysfs contains both the ->name and
->full_name values for a node. Does that help you?

The big problem is not the structure under /sys/devices, but rather the
symlinks to devices that appear under /sys/bus/*/devices. If two leaf
nodes have the same name, then they will conflict when they get added to
a bus_type's array of devices.

Another way to handle it is to only add the suffix when a conflict
actually occurs. That requires checking first whether or not a name will
conflict and ammending it only when that happens. I don't know if that
can be done nicely. I'll take a look.

> >
> > In order to fix the name, this commit removes the global integer uniqueness
> > enforcement and instead uses the node name for the device name. The rationale
> > behind such change, is that there's no way two nodes with the same node name
> > are associated to two different devices, by the nature of the devicetree.
> > In other words, it's impossible to produce a name collision for these kind
> > of devices.
> 
> This is perfectly legal:
> 
> /bus1/device
> /bus2/device
> 
> Only the full name has to be unique.
> 
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
> > ---
> > Hello Grant, Rob:
> >
> > I'm pretty sure there's a good reason for the existence of the atomic magic
> > integer, so this is not a *real* patch, but rather a bold attempt at asking you
> > some explanations for this.
> >
> > Under what circumstances does the magic integer avoids a name collision?
> > Will this propposal introduce a regression?
> 
> See the recent changes to this function in linux-next and the
> associated discussion of those commits.
> 
> Rob

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Rob Herring]

On Tue, May 20, 2014 at 1:50 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Sun, 18 May 2014 18:11:07 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Sun, May 18, 2014 at 12:01 PM, Ezequiel Garcia
>> <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
>> > When creating a device object for a devicetree node, the device name
>> > is created by using the node name and the 'reg' property, to make a name
>> > such as "a000.foo_device".
>> >
>> > For certain devices without an associated address, and hence no 'reg' property,
>> > the current code attempts to make a unique name, by using a global integer.
>> > Names look like "foo_device.1", "bar_device.2", and so on.
>> > Examples of such devices are: gpio-keys', backlights and rotary-encoders.
>> >
>> > The system cannot know such devices name before hand, given they are determined
>> > by the kernel probe order and by the nodes present in the devicetree. This can
>> > be problematic, on systems that are tied to the device's name, e.g. when
>> > catching hotplug events.
>
> The device's uevent file in sysfs contains both the ->name and
> ->full_name values for a node. Does that help you?
>
> The big problem is not the structure under /sys/devices, but rather the
> symlinks to devices that appear under /sys/bus/*/devices. If two leaf
> nodes have the same name, then they will conflict when they get added to
> a bus_type's array of devices.
>
> Another way to handle it is to only add the suffix when a conflict
> actually occurs. That requires checking first whether or not a name will
> conflict and ammending it only when that happens. I don't know if that
> can be done nicely. I'll take a look.

That was my idea as well and to move to a local number so you have
something like deviceA, deviceA.1, deviceB, deviceB.1 (maybe the
number is always appended). Perhaps a random number instead so no one
expects the names to be an ABI. ;)

I think it should be doable.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Ezequiel Garcia]

On 20 May 01:30 PM, Rob Herring wrote:
> On Tue, May 20, 2014 at 1:50 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > On Sun, 18 May 2014 18:11:07 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> On Sun, May 18, 2014 at 12:01 PM, Ezequiel Garcia
> >> <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> >> > When creating a device object for a devicetree node, the device name
> >> > is created by using the node name and the 'reg' property, to make a name
> >> > such as "a000.foo_device".
> >> >
> >> > For certain devices without an associated address, and hence no 'reg' property,
> >> > the current code attempts to make a unique name, by using a global integer.
> >> > Names look like "foo_device.1", "bar_device.2", and so on.
> >> > Examples of such devices are: gpio-keys', backlights and rotary-encoders.
> >> >
> >> > The system cannot know such devices name before hand, given they are determined
> >> > by the kernel probe order and by the nodes present in the devicetree. This can
> >> > be problematic, on systems that are tied to the device's name, e.g. when
> >> > catching hotplug events.
> >
> > The device's uevent file in sysfs contains both the ->name and
> > ->full_name values for a node. Does that help you?
> >
> > The big problem is not the structure under /sys/devices, but rather the
> > symlinks to devices that appear under /sys/bus/*/devices. If two leaf
> > nodes have the same name, then they will conflict when they get added to
> > a bus_type's array of devices.
> >
> > Another way to handle it is to only add the suffix when a conflict
> > actually occurs. That requires checking first whether or not a name will
> > conflict and ammending it only when that happens. I don't know if that
> > can be done nicely. I'll take a look.
> 
> That was my idea as well and to move to a local number so you have
> something like deviceA, deviceA.1, deviceB, deviceB.1 (maybe the
> number is always appended). Perhaps a random number instead so no one
> expects the names to be an ABI. ;)
> 
> I think it should be doable.
> 

That would be very nice :)
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Grant Likely]

On Tue, 20 May 2014 13:30:02 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, May 20, 2014 at 1:50 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > On Sun, 18 May 2014 18:11:07 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> On Sun, May 18, 2014 at 12:01 PM, Ezequiel Garcia
> >> <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> >> > When creating a device object for a devicetree node, the device name
> >> > is created by using the node name and the 'reg' property, to make a name
> >> > such as "a000.foo_device".
> >> >
> >> > For certain devices without an associated address, and hence no 'reg' property,
> >> > the current code attempts to make a unique name, by using a global integer.
> >> > Names look like "foo_device.1", "bar_device.2", and so on.
> >> > Examples of such devices are: gpio-keys', backlights and rotary-encoders.
> >> >
> >> > The system cannot know such devices name before hand, given they are determined
> >> > by the kernel probe order and by the nodes present in the devicetree. This can
> >> > be problematic, on systems that are tied to the device's name, e.g. when
> >> > catching hotplug events.
> >
> > The device's uevent file in sysfs contains both the ->name and
> > ->full_name values for a node. Does that help you?
> >
> > The big problem is not the structure under /sys/devices, but rather the
> > symlinks to devices that appear under /sys/bus/*/devices. If two leaf
> > nodes have the same name, then they will conflict when they get added to
> > a bus_type's array of devices.
> >
> > Another way to handle it is to only add the suffix when a conflict
> > actually occurs. That requires checking first whether or not a name will
> > conflict and ammending it only when that happens. I don't know if that
> > can be done nicely. I'll take a look.
> 
> That was my idea as well and to move to a local number so you have
> something like deviceA, deviceA.1, deviceB, deviceB.1 (maybe the
> number is always appended). Perhaps a random number instead so no one
> expects the names to be an ABI. ;)

Another idea: how about fix the name to alwasy be the final component of
the path name, followed by phandle. Anything that doesn't have a phandle
can be assigned one at boot. That would guarantee uniqueness without the
cost of trying to find duplicates.

for example:
	uart@10043000:0012

g.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Grant Likely]

On Wed, 21 May 2014 09:53:57 +0900, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Tue, 20 May 2014 13:30:02 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Tue, May 20, 2014 at 1:50 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > > On Sun, 18 May 2014 18:11:07 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > >> On Sun, May 18, 2014 at 12:01 PM, Ezequiel Garcia
> > >> <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> > >> > When creating a device object for a devicetree node, the device name
> > >> > is created by using the node name and the 'reg' property, to make a name
> > >> > such as "a000.foo_device".
> > >> >
> > >> > For certain devices without an associated address, and hence no 'reg' property,
> > >> > the current code attempts to make a unique name, by using a global integer.
> > >> > Names look like "foo_device.1", "bar_device.2", and so on.
> > >> > Examples of such devices are: gpio-keys', backlights and rotary-encoders.
> > >> >
> > >> > The system cannot know such devices name before hand, given they are determined
> > >> > by the kernel probe order and by the nodes present in the devicetree. This can
> > >> > be problematic, on systems that are tied to the device's name, e.g. when
> > >> > catching hotplug events.
> > >
> > > The device's uevent file in sysfs contains both the ->name and
> > > ->full_name values for a node. Does that help you?
> > >
> > > The big problem is not the structure under /sys/devices, but rather the
> > > symlinks to devices that appear under /sys/bus/*/devices. If two leaf
> > > nodes have the same name, then they will conflict when they get added to
> > > a bus_type's array of devices.
> > >
> > > Another way to handle it is to only add the suffix when a conflict
> > > actually occurs. That requires checking first whether or not a name will
> > > conflict and ammending it only when that happens. I don't know if that
> > > can be done nicely. I'll take a look.
> > 
> > That was my idea as well and to move to a local number so you have
> > something like deviceA, deviceA.1, deviceB, deviceB.1 (maybe the
> > number is always appended). Perhaps a random number instead so no one
> > expects the names to be an ABI. ;)
> 
> Another idea: how about fix the name to alwasy be the final component of
> the path name, followed by phandle. Anything that doesn't have a phandle
> can be assigned one at boot. That would guarantee uniqueness without the
> cost of trying to find duplicates.
> 
> for example:
> 	uart@10043000:0012

The following actually works. I don't like it because it results in very
long names, but it does work.

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9c121819e813..6e11c70b0616 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -74,60 +74,7 @@ EXPORT_SYMBOL(of_find_device_by_node);
  */
 void of_device_make_bus_id(struct device *dev)
 {
-	static atomic_t bus_no_reg_magic;
-	struct device_node *node = dev->of_node;
-	const __be32 *reg;
-	u64 addr;
-	const __be32 *addrp;
-	int magic;
-
-#ifdef CONFIG_PPC_DCR
-	/*
-	 * If it's a DCR based device, use 'd' for native DCRs
-	 * and 'D' for MMIO DCRs.
-	 */
-	reg = of_get_property(node, "dcr-reg", NULL);
-	if (reg) {
-#ifdef CONFIG_PPC_DCR_NATIVE
-		dev_set_name(dev, "d%x.%s", *reg, node->name);
-#else /* CONFIG_PPC_DCR_NATIVE */
-		u64 addr = of_translate_dcr_address(node, *reg, NULL);
-		if (addr != OF_BAD_ADDR) {
-			dev_set_name(dev, "D%llx.%s",
-				     (unsigned long long)addr, node->name);
-			return;
-		}
-#endif /* !CONFIG_PPC_DCR_NATIVE */
-	}
-#endif /* CONFIG_PPC_DCR */
-
-	/*
-	 * For MMIO, get the physical address
-	 */
-	reg = of_get_property(node, "reg", NULL);
-	if (reg) {
-		if (of_can_translate_address(node)) {
-			addr = of_translate_address(node, reg);
-		} else {
-			addrp = of_get_address(node, 0, NULL, NULL);
-			if (addrp)
-				addr = of_read_number(addrp, 1);
-			else
-				addr = OF_BAD_ADDR;
-		}
-		if (addr != OF_BAD_ADDR) {
-			dev_set_name(dev, "%llx.%s",
-				     (unsigned long long)addr, node->name);
-			return;
-		}
-	}
-
-	/*
-	 * No BusID, use the node name and add a globally incremented
-	 * counter (and pray...)
-	 */
-	magic = atomic_add_return(1, &bus_no_reg_magic);
-	dev_set_name(dev, "%s.%d", node->name, magic - 1);
+	dev_set_name(dev, dev->of_node->full_name);
 }
 
 /**

> 
> g.
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Grant Likely]

On Wed, 21 May 2014 09:53:57 +0900, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Tue, 20 May 2014 13:30:02 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Tue, May 20, 2014 at 1:50 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > > On Sun, 18 May 2014 18:11:07 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > >> On Sun, May 18, 2014 at 12:01 PM, Ezequiel Garcia
> > >> <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> wrote:
> > >> > When creating a device object for a devicetree node, the device name
> > >> > is created by using the node name and the 'reg' property, to make a name
> > >> > such as "a000.foo_device".
> > >> >
> > >> > For certain devices without an associated address, and hence no 'reg' property,
> > >> > the current code attempts to make a unique name, by using a global integer.
> > >> > Names look like "foo_device.1", "bar_device.2", and so on.
> > >> > Examples of such devices are: gpio-keys', backlights and rotary-encoders.
> > >> >
> > >> > The system cannot know such devices name before hand, given they are determined
> > >> > by the kernel probe order and by the nodes present in the devicetree. This can
> > >> > be problematic, on systems that are tied to the device's name, e.g. when
> > >> > catching hotplug events.
> > >
> > > The device's uevent file in sysfs contains both the ->name and
> > > ->full_name values for a node. Does that help you?
> > >
> > > The big problem is not the structure under /sys/devices, but rather the
> > > symlinks to devices that appear under /sys/bus/*/devices. If two leaf
> > > nodes have the same name, then they will conflict when they get added to
> > > a bus_type's array of devices.
> > >
> > > Another way to handle it is to only add the suffix when a conflict
> > > actually occurs. That requires checking first whether or not a name will
> > > conflict and ammending it only when that happens. I don't know if that
> > > can be done nicely. I'll take a look.
> > 
> > That was my idea as well and to move to a local number so you have
> > something like deviceA, deviceA.1, deviceB, deviceB.1 (maybe the
> > number is always appended). Perhaps a random number instead so no one
> > expects the names to be an ABI. ;)
> 
> Another idea: how about fix the name to alwasy be the final component of
> the path name, followed by phandle. Anything that doesn't have a phandle
> can be assigned one at boot. That would guarantee uniqueness without the
> cost of trying to find duplicates.
> 
> for example:
> 	uart@10043000:0012

Yet another approach. How about this patch? If the unit address cannot
be translated, then append the translated name of the parent.

Before:
# ls -l /sys/bus/platform/devices/
10002000.i2c -> ../../../devices/platform/10002000.i2c
10003000.intc -> ../../../devices/platform/amba.0/10003000.intc
10008000.lcd -> ../../../devices/platform/10008000.lcd
10010000.net -> ../../../devices/platform/10010000.net
10140000.intc -> ../../../devices/platform/amba.0/10140000.intc
34000000.flash -> ../../../devices/platform/34000000.flash
alarmtimer -> ../../../devices/platform/alarmtimer
amba.0 -> ../../../devices/platform/amba.0
fpga.1 -> ../../../devices/platform/amba.0/fpga.1
testcase-device1.2 -> ../../../devices/platform/testcase-device1.2
testcase-device2.3 -> ../../../devices/platform/testcase-device2.3

After:
# ls -l /sys/bus/platform/devices/
10002000.i2c -> ../../../devices/platform/10002000.i2c
10003000.intc -> ../../../devices/platform/amba/10003000.intc
10008000.lcd -> ../../../devices/platform/10008000.lcd
10010000.net -> ../../../devices/platform/10010000.net
10140000.intc -> ../../../devices/platform/amba/10140000.intc
34000000.flash -> ../../../devices/platform/34000000.flash
alarmtimer -> ../../../devices/platform/alarmtimer
amba -> ../../../devices/platform/amba
amba:fpga -> ../../../devices/platform/amba/amba:fpga
fffff000.testcase-data:testcase-device1 -> ../../../devices/platform/fffff000.testcase-data:testcase-device1
fffff000.testcase-data:testcase-device2 -> ../../../devices/platform/fffff000.testcase-data:testcase-device2

commit 177e5ff131639c8568248e5b9b2380066ce305d6
Author: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Date:   Wed May 21 15:40:31 2014 +0900

    of: Ensure unique names without sacrificing determinism
    
    The way the driver core is implemented, every device using the same bus
    type is required to have a unique name because a symlink to each device
    is created in the appropriate /sys/bus/*/devices directory, and two
    identical names causes a collision.
    
    The current code handles the requirement by using an globally
    incremented counter that is appended to the device name. It works, but
    it means any change to device registration will change the assigned
    numbers. Instead, if we build up the name by using information from the
    parent nodes, then it can be guaranteed to be unique without adding a
    random number to the end of it.
    
    Signed-off-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9c121819e813..e74fccbeb177 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -74,12 +74,9 @@ EXPORT_SYMBOL(of_find_device_by_node);
  */
 void of_device_make_bus_id(struct device *dev)
 {
-	static atomic_t bus_no_reg_magic;
 	struct device_node *node = dev->of_node;
 	const __be32 *reg;
 	u64 addr;
-	const __be32 *addrp;
-	int magic;
 
 #ifdef CONFIG_PPC_DCR
 	/*
@@ -101,33 +98,26 @@ void of_device_make_bus_id(struct device *dev)
 	}
 #endif /* CONFIG_PPC_DCR */
 
-	/*
-	 * For MMIO, get the physical address
-	 */
-	reg = of_get_property(node, "reg", NULL);
-	if (reg) {
-		if (of_can_translate_address(node)) {
+	/* Construct the name, using parent nodes if necessary to ensure uniqueness */
+	while (node->parent) {
+		/*
+		 * If the address can be translated, then that is as much
+		 * uniqueness as we need. Make it the first component and return
+		 */
+		reg = of_get_property(node, "reg", NULL);
+		if (reg && of_can_translate_address(node)) {
 			addr = of_translate_address(node, reg);
-		} else {
-			addrp = of_get_address(node, 0, NULL, NULL);
-			if (addrp)
-				addr = of_read_number(addrp, 1);
-			else
-				addr = OF_BAD_ADDR;
-		}
-		if (addr != OF_BAD_ADDR) {
-			dev_set_name(dev, "%llx.%s",
-				     (unsigned long long)addr, node->name);
+			dev_set_name(dev, dev_name(dev) ? "%llx.%s:%s" : "%llx.%s",
+				     (unsigned long long)addr, node->name,
+				     dev_name(dev));
 			return;
 		}
-	}
 
-	/*
-	 * No BusID, use the node name and add a globally incremented
-	 * counter (and pray...)
-	 */
-	magic = atomic_add_return(1, &bus_no_reg_magic);
-	dev_set_name(dev, "%s.%d", node->name, magic - 1);
+		/* format arguments only used if dev_name() resolves to NULL */
+		dev_set_name(dev, dev_name(dev) ? "%s:%s" : "%s",
+			     strrchr(node->full_name, '/') + 1, dev_name(dev));
+		node = node->parent;
+	}
 }
 
 /**

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Ezequiel Garcia]

On 21 May 04:43 PM, Grant Likely wrote:
[..]
> 
> Yet another approach. How about this patch? If the unit address cannot
> be translated, then append the translated name of the parent.
> 
[..]
> 
> commit 177e5ff131639c8568248e5b9b2380066ce305d6
> Author: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Date:   Wed May 21 15:40:31 2014 +0900
> 
>     of: Ensure unique names without sacrificing determinism
>     
>     The way the driver core is implemented, every device using the same bus
>     type is required to have a unique name because a symlink to each device
>     is created in the appropriate /sys/bus/*/devices directory, and two
>     identical names causes a collision.
>     
>     The current code handles the requirement by using an globally
>     incremented counter that is appended to the device name. It works, but
>     it means any change to device registration will change the assigned
>     numbers. Instead, if we build up the name by using information from the
>     parent nodes, then it can be guaranteed to be unique without adding a
>     random number to the end of it.
>     
>     Signed-off-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 

This solves the issue I complained about. Now the names of the devices match
the name of nodes, so the system knows what names to expect.

Before:

$ ls /sys/devices/
44e10800.pinmux   gpio-keys.8       pmu.0             system
ARMv7 Cortex-A8   hdmi.5            rotary-encoder.9  userhelper.10
backlight.6       ocp.3             soc.1             virtual
breakpoint        panel.7           soc0
fixedregulator.4  platform          software

After:

$ ls /sys/devices/
44e10800.pinmux   gpio-keys         pmu               system
ARMv7 Cortex-A8   hdmi              rotary-encoder    userhelper
backlight         ocp               soc               virtual
breakpoint        panel             soc0
fixedregulator@0  platform          software

Tested-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>

It would be great if this patch could be merged.

Thanks a lot Grant,
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Rob Herring]

On Wed, May 21, 2014 at 2:43 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Wed, 21 May 2014 09:53:57 +0900, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> On Tue, 20 May 2014 13:30:02 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > On Tue, May 20, 2014 at 1:50 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:

[...]

>> > > Another way to handle it is to only add the suffix when a conflict
>> > > actually occurs. That requires checking first whether or not a name will
>> > > conflict and ammending it only when that happens. I don't know if that
>> > > can be done nicely. I'll take a look.
>> >
>> > That was my idea as well and to move to a local number so you have
>> > something like deviceA, deviceA.1, deviceB, deviceB.1 (maybe the
>> > number is always appended). Perhaps a random number instead so no one
>> > expects the names to be an ABI. ;)
>>
>> Another idea: how about fix the name to alwasy be the final component of
>> the path name, followed by phandle. Anything that doesn't have a phandle
>> can be assigned one at boot. That would guarantee uniqueness without the
>> cost of trying to find duplicates.
>>
>> for example:
>>       uart@10043000:0012
>
> Yet another approach. How about this patch? If the unit address cannot
> be translated, then append the translated name of the parent.

I Ilke this, but...

> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9c121819e813..e74fccbeb177 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -74,12 +74,9 @@ EXPORT_SYMBOL(of_find_device_by_node);
>   */
>  void of_device_make_bus_id(struct device *dev)
>  {
> -       static atomic_t bus_no_reg_magic;
>         struct device_node *node = dev->of_node;
>         const __be32 *reg;
>         u64 addr;
> -       const __be32 *addrp;
> -       int magic;
>
>  #ifdef CONFIG_PPC_DCR
>         /*
> @@ -101,33 +98,26 @@ void of_device_make_bus_id(struct device *dev)
>         }
>  #endif /* CONFIG_PPC_DCR */
>
> -       /*
> -        * For MMIO, get the physical address
> -        */
> -       reg = of_get_property(node, "reg", NULL);
> -       if (reg) {
> -               if (of_can_translate_address(node)) {
> +       /* Construct the name, using parent nodes if necessary to ensure uniqueness */
> +       while (node->parent) {
> +               /*
> +                * If the address can be translated, then that is as much
> +                * uniqueness as we need. Make it the first component and return
> +                */
> +               reg = of_get_property(node, "reg", NULL);
> +               if (reg && of_can_translate_address(node)) {
>                         addr = of_translate_address(node, reg);

This needs to be rebased on my fix and killing off of of_can_translate_address.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] of: platform: Remove unique device name enforcement

[Grant Likely]

On Thu, 22 May 2014 13:31:37 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, May 21, 2014 at 2:43 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > On Wed, 21 May 2014 09:53:57 +0900, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> >> On Tue, 20 May 2014 13:30:02 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> > On Tue, May 20, 2014 at 1:50 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> 
> [...]
> 
> >> > > Another way to handle it is to only add the suffix when a conflict
> >> > > actually occurs. That requires checking first whether or not a name will
> >> > > conflict and ammending it only when that happens. I don't know if that
> >> > > can be done nicely. I'll take a look.
> >> >
> >> > That was my idea as well and to move to a local number so you have
> >> > something like deviceA, deviceA.1, deviceB, deviceB.1 (maybe the
> >> > number is always appended). Perhaps a random number instead so no one
> >> > expects the names to be an ABI. ;)
> >>
> >> Another idea: how about fix the name to alwasy be the final component of
> >> the path name, followed by phandle. Anything that doesn't have a phandle
> >> can be assigned one at boot. That would guarantee uniqueness without the
> >> cost of trying to find duplicates.
> >>
> >> for example:
> >>       uart@10043000:0012
> >
> > Yet another approach. How about this patch? If the unit address cannot
> > be translated, then append the translated name of the parent.
> 
> I Ilke this, but...
> 
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 9c121819e813..e74fccbeb177 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > +               reg = of_get_property(node, "reg", NULL);
> > +               if (reg && of_can_translate_address(node)) {
> >                         addr = of_translate_address(node, reg);
> 
> This needs to be rebased on my fix and killing off of of_can_translate_address.

Done. I've rebased this and all the other stuff I've got queued up for
v3.16 on top of your for-next tree.

g.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html