* [PATCH v2 0/2] of: Clean up naming of platform devices @ 2014-05-22 23:36 Grant Likely [not found] ` <1400801769-4506-1-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-05-22 23:36 ` [PATCH v2 2/2] of: Stop naming platform_device using dcr address Grant Likely 0 siblings, 2 replies; 9+ messages in thread From: Grant Likely @ 2014-05-22 23:36 UTC (permalink / raw) To: devicetree, linux-kernel This series makes of_device_make_bus_id() more reliable in choosing a unique name and clears out some historical cruft. I've rebased it onto Rob's for-next tree so that it plays well with the removal of of_can_translate_address() ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <1400801769-4506-1-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* [PATCH v2 1/2] of: Ensure unique names without sacrificing determinism [not found] ` <1400801769-4506-1-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2014-05-22 23:36 ` Grant Likely 2014-05-23 13:59 ` Rob Herring 2014-05-24 18:10 ` Ezequiel Garcia 0 siblings, 2 replies; 9+ messages in thread From: Grant Likely @ 2014-05-22 23:36 UTC (permalink / raw) To: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Grant Likely, Ezequiel Garcia, Rob Herring 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> Cc: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- drivers/of/platform.c | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index d0009b3614af..95c133a0554b 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -68,17 +68,15 @@ EXPORT_SYMBOL(of_find_device_by_node); * of_device_make_bus_id - Use the device node data to assign a unique 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. + * This routine will first try using the translated bus address to + * derive a unique name. If it cannot, then it will prepend names from + * parent nodes until a unique name can be derived. */ 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 /* @@ -100,25 +98,25 @@ 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) { - addr = of_translate_address(node, reg); - if (addr != OF_BAD_ADDR) { - dev_set_name(dev, "%llx.%s", - (unsigned long long)addr, node->name); + /* 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 && (addr = of_translate_address(node, reg)) != OF_BAD_ADDR) { + 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; + } } /** -- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] of: Ensure unique names without sacrificing determinism 2014-05-22 23:36 ` [PATCH v2 1/2] of: Ensure unique names without sacrificing determinism Grant Likely @ 2014-05-23 13:59 ` Rob Herring 2014-05-24 18:10 ` Ezequiel Garcia 1 sibling, 0 replies; 9+ messages in thread From: Rob Herring @ 2014-05-23 13:59 UTC (permalink / raw) To: Grant Likely Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Ezequiel Garcia On Thu, May 22, 2014 at 6:36 PM, Grant Likely <grant.likely@linaro.org> wrote: > 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@linaro.org> > Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > Cc: Rob Herring <robh@kernel.org> Acked-by: Rob Herring <robh@kernel.org> > --- > drivers/of/platform.c | 40 +++++++++++++++++++--------------------- > 1 file changed, 19 insertions(+), 21 deletions(-) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index d0009b3614af..95c133a0554b 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -68,17 +68,15 @@ EXPORT_SYMBOL(of_find_device_by_node); > * of_device_make_bus_id - Use the device node data to assign a unique 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. > + * This routine will first try using the translated bus address to > + * derive a unique name. If it cannot, then it will prepend names from > + * parent nodes until a unique name can be derived. > */ > 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 > /* > @@ -100,25 +98,25 @@ 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) { > - addr = of_translate_address(node, reg); > - if (addr != OF_BAD_ADDR) { > - dev_set_name(dev, "%llx.%s", > - (unsigned long long)addr, node->name); > + /* 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 && (addr = of_translate_address(node, reg)) != OF_BAD_ADDR) { > + 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; > + } > } > > /** > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] of: Ensure unique names without sacrificing determinism 2014-05-22 23:36 ` [PATCH v2 1/2] of: Ensure unique names without sacrificing determinism Grant Likely 2014-05-23 13:59 ` Rob Herring @ 2014-05-24 18:10 ` Ezequiel Garcia 1 sibling, 0 replies; 9+ messages in thread From: Ezequiel Garcia @ 2014-05-24 18:10 UTC (permalink / raw) To: Grant Likely; +Cc: devicetree, linux-kernel, Rob Herring On 23 May 08:36 AM, Grant Likely wrote: > 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@linaro.org> Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] of: Stop naming platform_device using dcr address 2014-05-22 23:36 [PATCH v2 0/2] of: Clean up naming of platform devices Grant Likely [not found] ` <1400801769-4506-1-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2014-05-22 23:36 ` Grant Likely [not found] ` <1400801769-4506-3-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 1 sibling, 1 reply; 9+ messages in thread From: Grant Likely @ 2014-05-22 23:36 UTC (permalink / raw) To: devicetree, linux-kernel Cc: Grant Likely, Rob Herring, Benjamin Herrenschmidt There is now a way to ensure all platform devices get a unique name when populated from the device tree, and the DCR_NATIVE code path is broken anyway. PowerPC Cell (PS3) is the only platform that actually uses this path. Most likely nobody will notice if it is killed. Remove the code and associated ugly #ifdef. The user-visible impact of this patch is that any DCR device on Cell will get a new name in the /sys/devices hierarchy. Signed-off-by: Grant Likely <grant.likely@linaro.org> Cc: Rob Herring <robh@kernel.org> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- arch/powerpc/include/asm/dcr-mmio.h | 4 ---- arch/powerpc/sysdev/dcr.c | 6 +++--- drivers/of/platform.c | 24 ------------------------ 3 files changed, 3 insertions(+), 31 deletions(-) diff --git a/arch/powerpc/include/asm/dcr-mmio.h b/arch/powerpc/include/asm/dcr-mmio.h index acd491dbd45a..93a68b28e695 100644 --- a/arch/powerpc/include/asm/dcr-mmio.h +++ b/arch/powerpc/include/asm/dcr-mmio.h @@ -51,10 +51,6 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host, out_be32(host.token + ((host.base + dcr_n) * host.stride), value); } -extern u64 of_translate_dcr_address(struct device_node *dev, - unsigned int dcr_n, - unsigned int *stride); - #endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_DCR_MMIO_H */ diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c index 1bd0eba4d355..e9056e438575 100644 --- a/arch/powerpc/sysdev/dcr.c +++ b/arch/powerpc/sysdev/dcr.c @@ -152,9 +152,9 @@ EXPORT_SYMBOL_GPL(dcr_resource_len); #ifdef CONFIG_PPC_DCR_MMIO -u64 of_translate_dcr_address(struct device_node *dev, - unsigned int dcr_n, - unsigned int *out_stride) +static u64 of_translate_dcr_address(struct device_node *dev, + unsigned int dcr_n, + unsigned int *out_stride) { struct device_node *dp; const u32 *p; diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 95c133a0554b..52780a72d09d 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -51,10 +51,6 @@ struct platform_device *of_find_device_by_node(struct device_node *np) } EXPORT_SYMBOL(of_find_device_by_node); -#if defined(CONFIG_PPC_DCR) -#include <asm/dcr.h> -#endif - #ifdef CONFIG_OF_ADDRESS /* * The following routines scan a subtree and registers a device for @@ -78,26 +74,6 @@ void of_device_make_bus_id(struct device *dev) const __be32 *reg; u64 addr; -#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 */ - /* Construct the name, using parent nodes if necessary to ensure uniqueness */ while (node->parent) { /* -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <1400801769-4506-3-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2 2/2] of: Stop naming platform_device using dcr address [not found] ` <1400801769-4506-3-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2014-05-23 14:07 ` Arnd Bergmann 2014-05-27 12:47 ` Grant Likely 2014-05-24 4:10 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2014-05-23 14:07 UTC (permalink / raw) To: Grant Likely Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Benjamin Herrenschmidt On Friday 23 May 2014 08:36:09 Grant Likely wrote: > There is now a way to ensure all platform devices get a unique name when > populated from the device tree, and the DCR_NATIVE code path is broken > anyway. PowerPC Cell (PS3) is the only platform that actually uses this > path. Most likely nobody will notice if it is killed. Remove the code > and associated ugly #ifdef. > > The user-visible impact of this patch is that any DCR device on Cell > will get a new name in the /sys/devices hierarchy. Isn't this used on all ppc440+ as well? I don't think PS3 has it either, it should only be the second generation Cell blade doing this, which uses ppc440-derived I/O devices. We will probably see a bunch of the same devices on APM's X-gene ARM64 machines as well, but we don't have to maintain a stable device name for those yet. Arnd -- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] of: Stop naming platform_device using dcr address 2014-05-23 14:07 ` Arnd Bergmann @ 2014-05-27 12:47 ` Grant Likely 0 siblings, 0 replies; 9+ messages in thread From: Grant Likely @ 2014-05-27 12:47 UTC (permalink / raw) To: Arnd Bergmann Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Benjamin Herrenschmidt On Fri, 23 May 2014 16:07:08 +0200, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote: > On Friday 23 May 2014 08:36:09 Grant Likely wrote: > > There is now a way to ensure all platform devices get a unique name when > > populated from the device tree, and the DCR_NATIVE code path is broken > > anyway. PowerPC Cell (PS3) is the only platform that actually uses this > > path. Most likely nobody will notice if it is killed. Remove the code > > and associated ugly #ifdef. > > > > The user-visible impact of this patch is that any DCR device on Cell > > will get a new name in the /sys/devices hierarchy. > > Isn't this used on all ppc440+ as well? Yes, but the 440 code path is broken so that it always uses the default name instead. Therefore only Cell is affected. 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] of: Stop naming platform_device using dcr address [not found] ` <1400801769-4506-3-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-05-23 14:07 ` Arnd Bergmann @ 2014-05-24 4:10 ` Benjamin Herrenschmidt 2014-05-26 9:21 ` Grant Likely 1 sibling, 1 reply; 9+ messages in thread From: Benjamin Herrenschmidt @ 2014-05-24 4:10 UTC (permalink / raw) To: Grant Likely Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring On Fri, 2014-05-23 at 08:36 +0900, Grant Likely wrote: > There is now a way to ensure all platform devices get a unique name when > populated from the device tree, and the DCR_NATIVE code path is broken > anyway. PowerPC Cell (PS3) is the only platform that actually uses this > path. Most likely nobody will notice if it is killed. Remove the code > and associated ugly #ifdef. > > The user-visible impact of this patch is that any DCR device on Cell > will get a new name in the /sys/devices hierarchy. I don't understand. First DCR devices cover more than Cell (and not even PS3). All 4xx embedded have some form of DCR based devices so I don't know how much we use as "platform". What do you mean by "the DCR_NATIVE" code path is broken anyway ? Changing user visible platform device names has broken stuff in the past, I'm not saying it is now but I'd like a better justification for removing this. Cheers, Ben. > Signed-off-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Cc: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> > --- > arch/powerpc/include/asm/dcr-mmio.h | 4 ---- > arch/powerpc/sysdev/dcr.c | 6 +++--- > drivers/of/platform.c | 24 ------------------------ > 3 files changed, 3 insertions(+), 31 deletions(-) > > diff --git a/arch/powerpc/include/asm/dcr-mmio.h b/arch/powerpc/include/asm/dcr-mmio.h > index acd491dbd45a..93a68b28e695 100644 > --- a/arch/powerpc/include/asm/dcr-mmio.h > +++ b/arch/powerpc/include/asm/dcr-mmio.h > @@ -51,10 +51,6 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host, > out_be32(host.token + ((host.base + dcr_n) * host.stride), value); > } > > -extern u64 of_translate_dcr_address(struct device_node *dev, > - unsigned int dcr_n, > - unsigned int *stride); > - > #endif /* __KERNEL__ */ > #endif /* _ASM_POWERPC_DCR_MMIO_H */ > > diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c > index 1bd0eba4d355..e9056e438575 100644 > --- a/arch/powerpc/sysdev/dcr.c > +++ b/arch/powerpc/sysdev/dcr.c > @@ -152,9 +152,9 @@ EXPORT_SYMBOL_GPL(dcr_resource_len); > > #ifdef CONFIG_PPC_DCR_MMIO > > -u64 of_translate_dcr_address(struct device_node *dev, > - unsigned int dcr_n, > - unsigned int *out_stride) > +static u64 of_translate_dcr_address(struct device_node *dev, > + unsigned int dcr_n, > + unsigned int *out_stride) > { > struct device_node *dp; > const u32 *p; > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 95c133a0554b..52780a72d09d 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -51,10 +51,6 @@ struct platform_device *of_find_device_by_node(struct device_node *np) > } > EXPORT_SYMBOL(of_find_device_by_node); > > -#if defined(CONFIG_PPC_DCR) > -#include <asm/dcr.h> > -#endif > - > #ifdef CONFIG_OF_ADDRESS > /* > * The following routines scan a subtree and registers a device for > @@ -78,26 +74,6 @@ void of_device_make_bus_id(struct device *dev) > const __be32 *reg; > u64 addr; > > -#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 */ > - > /* Construct the name, using parent nodes if necessary to ensure uniqueness */ > while (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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] of: Stop naming platform_device using dcr address 2014-05-24 4:10 ` Benjamin Herrenschmidt @ 2014-05-26 9:21 ` Grant Likely 0 siblings, 0 replies; 9+ messages in thread From: Grant Likely @ 2014-05-26 9:21 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux Kernel Mailing List, Rob Herring On Sat, May 24, 2014 at 5:10 AM, Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> wrote: > On Fri, 2014-05-23 at 08:36 +0900, Grant Likely wrote: >> There is now a way to ensure all platform devices get a unique name when >> populated from the device tree, and the DCR_NATIVE code path is broken >> anyway. PowerPC Cell (PS3) is the only platform that actually uses this >> path. Most likely nobody will notice if it is killed. Remove the code >> and associated ugly #ifdef. >> >> The user-visible impact of this patch is that any DCR device on Cell >> will get a new name in the /sys/devices hierarchy. > > I don't understand. First DCR devices cover more than Cell (and not even > PS3). All 4xx embedded have some form of DCR based devices so I don't > know how much we use as "platform". > > What do you mean by "the DCR_NATIVE" code path is broken anyway ? Take a look at the function. In the PPC_DCR_NATIVE path, the device name is set using format "d%x.%s", but the function doesn't return. Instead, it falls through to the end of the function that sets the device name again using "%s.%d", so the first name never sticks. The last time that code was touched was in 2010 by me when I moved it out of arch/powerpc (94c0931983ee9, "of: Merge of_device_alloc() and of_device_make_bus_id()"). It was already broken then. As far as I can tell, it was broken as far back as 7eebde70, "[POWERPC] Souped-up of_platform_device support" from 2006 which is when the code was added. Did it ever work? The only thing that selects the other option, PPC_DCR_MMIO is Cell in arch/powerpc/platforms/cell/Kconfig. Every other DCR device is PPC_DCR_NATIVE, all of which are broken. > Changing user visible platform device names has broken stuff in the > past, I'm not saying it is now but I'd like a better justification > for removing this. The whole purpose of this function was to create unique names. The previous patch solves that, so I'm not convinced that this extra code path is still warranted. Given that nobody has complained about PPC_DCR_NATIVE seeming never has worked, that path should be just dropped outright since fixing it would be an user visible change itself! If you are really bothered about removing it on Cell, then I'll put it back in. I just don't think it is worth it. g. > > Cheers, > Ben. > > >> Signed-off-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> >> Cc: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> >> --- >> arch/powerpc/include/asm/dcr-mmio.h | 4 ---- >> arch/powerpc/sysdev/dcr.c | 6 +++--- >> drivers/of/platform.c | 24 ------------------------ >> 3 files changed, 3 insertions(+), 31 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/dcr-mmio.h b/arch/powerpc/include/asm/dcr-mmio.h >> index acd491dbd45a..93a68b28e695 100644 >> --- a/arch/powerpc/include/asm/dcr-mmio.h >> +++ b/arch/powerpc/include/asm/dcr-mmio.h >> @@ -51,10 +51,6 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host, >> out_be32(host.token + ((host.base + dcr_n) * host.stride), value); >> } >> >> -extern u64 of_translate_dcr_address(struct device_node *dev, >> - unsigned int dcr_n, >> - unsigned int *stride); >> - >> #endif /* __KERNEL__ */ >> #endif /* _ASM_POWERPC_DCR_MMIO_H */ >> >> diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c >> index 1bd0eba4d355..e9056e438575 100644 >> --- a/arch/powerpc/sysdev/dcr.c >> +++ b/arch/powerpc/sysdev/dcr.c >> @@ -152,9 +152,9 @@ EXPORT_SYMBOL_GPL(dcr_resource_len); >> >> #ifdef CONFIG_PPC_DCR_MMIO >> >> -u64 of_translate_dcr_address(struct device_node *dev, >> - unsigned int dcr_n, >> - unsigned int *out_stride) >> +static u64 of_translate_dcr_address(struct device_node *dev, >> + unsigned int dcr_n, >> + unsigned int *out_stride) >> { >> struct device_node *dp; >> const u32 *p; >> diff --git a/drivers/of/platform.c b/drivers/of/platform.c >> index 95c133a0554b..52780a72d09d 100644 >> --- a/drivers/of/platform.c >> +++ b/drivers/of/platform.c >> @@ -51,10 +51,6 @@ struct platform_device *of_find_device_by_node(struct device_node *np) >> } >> EXPORT_SYMBOL(of_find_device_by_node); >> >> -#if defined(CONFIG_PPC_DCR) >> -#include <asm/dcr.h> >> -#endif >> - >> #ifdef CONFIG_OF_ADDRESS >> /* >> * The following routines scan a subtree and registers a device for >> @@ -78,26 +74,6 @@ void of_device_make_bus_id(struct device *dev) >> const __be32 *reg; >> u64 addr; >> >> -#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 */ >> - >> /* Construct the name, using parent nodes if necessary to ensure uniqueness */ >> while (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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-05-27 12:47 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-22 23:36 [PATCH v2 0/2] of: Clean up naming of platform devices Grant Likely [not found] ` <1400801769-4506-1-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-05-22 23:36 ` [PATCH v2 1/2] of: Ensure unique names without sacrificing determinism Grant Likely 2014-05-23 13:59 ` Rob Herring 2014-05-24 18:10 ` Ezequiel Garcia 2014-05-22 23:36 ` [PATCH v2 2/2] of: Stop naming platform_device using dcr address Grant Likely [not found] ` <1400801769-4506-3-git-send-email-grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-05-23 14:07 ` Arnd Bergmann 2014-05-27 12:47 ` Grant Likely 2014-05-24 4:10 ` Benjamin Herrenschmidt 2014-05-26 9:21 ` 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).