2012-11-12 15:00:37

by Per Förlin

[permalink] [raw]
Subject: [PATCH] dt: platform: Extract device name from device tree blob

Add support to extract device name from device tree blob.
If the property "dev-name" is set in the DTS this name will
be used when creating the device.
The auxdata_lookup has precedence and will override
the "dev-name" property.

Adding support to parse DTS "dev-name" will make it possible to
replace these kind of structures with a dev-name in the DTS.
OF_DEV_AUXDATA("compatible", BASE_ADDRESS, "dev-name", NULL)

Examples
OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),
OF_DEV_AUXDATA("marvell,mv64xxx-i2c", 0xf1011000, "mv64xxx_i2c.0",
OF_DEV_AUXDATA("marvell,orion-sata", 0xf10a0000, "sata_mv.0", NULL),
OF_DEV_AUXDATA("marvell,dove-sdhci", 0xf1092000, "sdhci-dove.0", NULL),
OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_PDMA0, "dma-pl330.0", NULL),
OF_DEV_AUXDATA("arm,pl330", EXYNOS5_PA_PDMA0, "dma-pl330.0", NULL),
OF_DEV_AUXDATA("fsl,imx27-uart", MX27_UART1_BASE_ADDR, "imx21-uart.0", NULL),
OF_DEV_AUXDATA("fsl,imx27-fec", MX27_FEC_BASE_ADDR, "imx27-fec.0", NULL),
OF_DEV_AUXDATA("fsl,imx27-i2c", MX27_I2C1_BASE_ADDR, "imx-i2c.0", NULL),
OF_DEV_AUXDATA("fsl,imx27-cspi", MX27_CSPI1_BASE_ADDR, "imx27-cspi.0", NULL),
OF_DEV_AUXDATA("fsl,imx27-wdt", MX27_WDOG_BASE_ADDR, "imx2-wdt.0", NULL),
OF_DEV_AUXDATA("fsl,imx27-nand", MX27_NFC_BASE_ADDR, "mxc_nand.0", NULL),
OF_DEV_AUXDATA("fsl,imx51-uart", MX51_UART1_BASE_ADDR, "imx21-uart.0", NULL),
OF_DEV_AUXDATA("fsl,imx51-fec", MX51_FEC_BASE_ADDR, "imx27-fec.0", NULL),
OF_DEV_AUXDATA("fsl,imx51-esdhc", MX51_ESDHC1_BASE_ADDR, "sdhci-esdhc-imx51.0", NULL),
OF_DEV_AUXDATA("fsl,imx51-ecspi", MX51_ECSPI1_BASE_ADDR, "imx51-ecspi.0", NULL),
OF_DEV_AUXDATA("fsl,imx51-cspi", MX51_CSPI_BASE_ADDR, "imx35-cspi.0", NULL),
OF_DEV_AUXDATA("fsl,imx51-i2c", MX51_I2C1_BASE_ADDR, "imx-i2c.0", NULL),
OF_DEV_AUXDATA("fsl,imx51-wdt", MX51_WDOG1_BASE_ADDR, "imx2-wdt.0", NULL),
OF_DEV_AUXDATA("fsl,imx53-uart", MX53_UART1_BASE_ADDR, "imx21-uart.0", NULL),
OF_DEV_AUXDATA("fsl,imx53-fec", MX53_FEC_BASE_ADDR, "imx25-fec.0", NULL),
OF_DEV_AUXDATA("fsl,imx53-esdhc", MX53_ESDHC1_BASE_ADDR, "sdhci-esdhc-imx53.0", NULL),
OF_DEV_AUXDATA("fsl,imx53-ecspi", MX53_ECSPI1_BASE_ADDR, "imx51-ecspi.0", NULL),
OF_DEV_AUXDATA("fsl,imx53-cspi", MX53_CSPI_BASE_ADDR, "imx35-cspi.0", NULL),
OF_DEV_AUXDATA("fsl,imx53-i2c", MX53_I2C1_BASE_ADDR, "imx-i2c.0", NULL),
OF_DEV_AUXDATA("fsl,imx53-wdt", MX53_WDOG1_BASE_ADDR, "imx2-wdt.0", NULL),
OF_DEV_AUXDATA("marvell,orion-spi", 0xf1010600, "orion_spi.0", NULL),
OF_DEV_AUXDATA("marvell,mv64xxx-i2c", 0xf1011000, "mv64xxx_i2c.0",
OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4017000, "pxa2xx-uart.0", NULL),
OF_DEV_AUXDATA("mrvl,mmp-twsi", 0xd4011000, "pxa2xx-i2c.0", NULL),
OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4017000, "pxa2xx-uart.0", NULL),
OF_DEV_AUXDATA("mrvl,mmp-twsi", 0xd4011000, "pxa2xx-i2c.0", NULL),
OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4030000, "pxa2xx-uart.0", NULL),
OF_DEV_AUXDATA("mrvl,mmp-twsi", 0xd4011000, "pxa2xx-i2c.0", NULL),
OF_DEV_AUXDATA("mrvl,pxa-uart", 0x40100000, "pxa2xx-uart.0", NULL),
OF_DEV_AUXDATA("marvell,pxa-mmc", 0x41100000, "pxa2xx-mci.0", NULL),
OF_DEV_AUXDATA("mrvl,pxa-i2c", 0x40301680, "pxa2xx-i2c.0", NULL),
OF_DEV_AUXDATA("nvidia,tegra20-sdhci", TEGRA_SDMMC1_BASE, "sdhci-tegra.0", NULL),
OF_DEV_AUXDATA("nvidia,tegra20-i2c", TEGRA_I2C_BASE, "tegra-i2c.0", NULL),
OF_DEV_AUXDATA("nvidia,tegra20-i2s", TEGRA_I2S1_BASE, "tegra20-i2s.0", NULL),
OF_DEV_AUXDATA("nvidia,tegra20-ehci", TEGRA_USB_BASE, "tegra-ehci.0",
OF_DEV_AUXDATA("nvidia,tegra20-sdhci", 0x78000000, "sdhci-tegra.0", NULL),
OF_DEV_AUXDATA("nvidia,tegra20-i2c", 0x7000C000, "tegra-i2c.0", NULL),
OF_DEV_AUXDATA("st,nomadik-gpio", 0x8012e000, "gpio.0", NULL),
OF_DEV_AUXDATA("st,nomadik-i2c", 0x80004000, "nmk-i2c.0", NULL),

Signed-off-by: Per Forlin <[email protected]>
---
drivers/of/platform.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9600480..999ee95 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -169,6 +169,8 @@ struct platform_device *of_device_alloc(struct device_node *np,

if (bus_id)
dev_set_name(&dev->dev, "%s", bus_id);
+ else if (of_property_read_string(np, "dev-name", &bus_id) == 0)
+ dev_set_name(&dev->dev, "%s", bus_id);
else
of_device_make_bus_id(&dev->dev);

@@ -264,6 +266,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev->dev.platform_data = platform_data;
if (bus_id)
dev_set_name(&dev->dev, "%s", bus_id);
+ else if (of_property_read_string(node, "dev-name", &bus_id) == 0)
+ dev_set_name(&dev->dev, "%s", bus_id);
else
of_device_make_bus_id(&dev->dev);

--
1.7.11.3


2012-11-12 15:20:24

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH] dt: platform: Extract device name from device tree blob

On Mon, Nov 12, 2012 at 2:59 PM, Per Forlin <[email protected]> wrote:
> Add support to extract device name from device tree blob.
> If the property "dev-name" is set in the DTS this name will
> be used when creating the device.
> The auxdata_lookup has precedence and will override
> the "dev-name" property.

Using a 'dev-name' property has the same problem that the 'cell-index'
properties have in that it is encoding part of the global namespace
local to the node and it becomes easy to create collisions. Instead of
this check to see if one of the properties in /aliases points to the
node and use that for the name.

g.

2012-11-12 22:55:04

by Per Förlin

[permalink] [raw]
Subject: Re: [PATCH] dt: platform: Extract device name from device tree blob

On 11/12/2012 04:20 PM, Grant Likely wrote:
> On Mon, Nov 12, 2012 at 2:59 PM, Per Forlin <[email protected]> wrote:
>> Add support to extract device name from device tree blob.
>> If the property "dev-name" is set in the DTS this name will
>> be used when creating the device.
>> The auxdata_lookup has precedence and will override
>> the "dev-name" property.
>
> Using a 'dev-name' property has the same problem that the 'cell-index'
> properties have in that it is encoding part of the global namespace
> local to the node and it becomes easy to create collisions. Instead of
> this check to see if one of the properties in /aliases points to the
> node and use that for the name.
>
> g.
>
Thanks Grant for your feedback,

Extract from exynos5250.dtsi:
-----------
aliases {
spi0 = &spi_0;
spi1 = &spi_1;
spi2 = &spi_2;
};

spi_0: spi@12d20000 {
...
};

spi_1: spi@12d30000 {
...
};

spi_2: spi@12d40000 {
...
};
---------------

Alias refers to the device node. The device node is not aware of the alias.

How to get a device name from the aliases.
1. Traverse all aliases for each device node (time consuming if there are many aliases)
2. Make a new function of_alias_get_name(), today there is only of_alias_get_id()
3. The functionality of setting device_name based on alias name needs to be optional because one may want to use aliases without changing the name of the device.
All this is feasible but perhaps not optimal.

I don't really see how come name space is a big issue in this case. The name space of "dev-name" is local to the device node. A child device node can use the same dev-name as the parent (unless I'm mistaken which happens quite often). Introducing yet another property name pollutes the name space of the device node. Still I think the pros are stronger than the cons.

Do you still prefer to use the name of the Alias? Could you please elaborate a bit more how this can be done in practice?
I would agree with you if there was a reference from the device node to the alias.

My dev-name patch can be made smaller by moving read_string to:
static int of_platform_bus_create(struct device_node *bus,
platform_data = auxdata->platform_data;
}

+ if (!bus_id)
+ of_property_read_string(bus, "dev-name", &bus_id);
+

It would be nice to add something in the DTS to explicitly enable this feature to avoid collisions. Default this feature is disabled.
Any suggestions?

BR
Per

2012-11-13 14:34:13

by Per Förlin

[permalink] [raw]
Subject: Re: [PATCH] dt: platform: Extract device name from device tree blob

On 11/12/2012 11:54 PM, Per F?rlin wrote:
> On 11/12/2012 04:20 PM, Grant Likely wrote:
>> On Mon, Nov 12, 2012 at 2:59 PM, Per Forlin <[email protected]> wrote:
>>> Add support to extract device name from device tree blob.
>>> If the property "dev-name" is set in the DTS this name will
>>> be used when creating the device.
>>> The auxdata_lookup has precedence and will override
>>> the "dev-name" property.
>>
>> Using a 'dev-name' property has the same problem that the 'cell-index'
>> properties have in that it is encoding part of the global namespace
>> local to the node and it becomes easy to create collisions. Instead of
>> this check to see if one of the properties in /aliases points to the
>> node and use that for the name.
>>
>> g.
>>
> Thanks Grant for your feedback,
>
> Extract from exynos5250.dtsi:
> -----------
> aliases {
> spi0 = &spi_0;
> spi1 = &spi_1;
> spi2 = &spi_2;
> };
>
> spi_0: spi@12d20000 {
> ...
> };
>
> spi_1: spi@12d30000 {
> ...
> };
>
> spi_2: spi@12d40000 {
> ...
> };
> ---------------
>
> Alias refers to the device node. The device node is not aware of the alias.
>
> How to get a device name from the aliases.
> 1. Traverse all aliases for each device node (time consuming if there are many aliases)
> 2. Make a new function of_alias_get_name(), today there is only of_alias_get_id()
> 3. The functionality of setting device_name based on alias name needs to be optional because one may want to use aliases without changing the name of the device.
> All this is feasible but perhaps not optimal.
>
> I don't really see how come name space is a big issue in this case. The name space of "dev-name" is local to the device node. A child device node can use the same dev-name as the parent (unless I'm mistaken which happens quite often). Introducing yet another property name pollutes the name space of the device node. Still I think the pros are stronger than the cons.
>
> Do you still prefer to use the name of the Alias? Could you please elaborate a bit more how this can be done in practice?
> I would agree with you if there was a reference from the device node to the alias.
>
> My dev-name patch can be made smaller by moving read_string to:
> static int of_platform_bus_create(struct device_node *bus,
> platform_data = auxdata->platform_data;
> }
>
> + if (!bus_id)
> + of_property_read_string(bus, "dev-name", &bus_id);
> +
>
> It would be nice to add something in the DTS to explicitly enable this feature to avoid collisions. Default this feature is disabled.
> Any suggestions?
Hi,

I'm starting to feel that this patch is only necessary in case the regulator tree or the clock tree lives out side the DTS. In this case one must use a specific dev-name to map device and clock.
Are there other reasons for wanting to set a specific device name?

If regulator-tree or clock-tree are defined in the DTS one can simply refer directly to the clock from the device node.

Example:
serial@fff36000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0xfff36000 0x1000>;
interrupts = <0 20 4>;
clocks = <&pclk>;
clock-names = "apb_pclk";
};

BR
Per

2012-11-14 09:22:07

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH] dt: platform: Extract device name from device tree blob

On Mon, 12 Nov 2012, Per Forlin wrote:

> Add support to extract device name from device tree blob.
> If the property "dev-name" is set in the DTS this name will
> be used when creating the device.
> The auxdata_lookup has precedence and will override
> the "dev-name" property.
>
> Adding support to parse DTS "dev-name" will make it possible to
> replace these kind of structures with a dev-name in the DTS.
> OF_DEV_AUXDATA("compatible", BASE_ADDRESS, "dev-name", NULL)

This patch isn't required. The correct action is to make the driver
DT aware. Also, is naming devices "devname.id" a Linux only thing,
or do other OSes do the same? If the former is true, then we
definitely do not want to add this binding.

> OF_DEV_AUXDATA("st,nomadik-gpio", 0x8012e000, "gpio.0", NULL),
> OF_DEV_AUXDATA("st,nomadik-i2c", 0x80004000, "nmk-i2c.0", NULL),

In this case, once we have added common clk functionality to our
Device Trees, these will vanish.

Kind regards,
Lee

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2012-11-15 16:15:40

by Per Förlin

[permalink] [raw]
Subject: Re: [PATCH] dt: platform: Extract device name from device tree blob

On 11/15/2012 04:52 PM, Grant Likely wrote:
> On Mon, 12 Nov 2012 23:54:40 +0100, Per Förlin <[email protected]> wrote:
>> On 11/12/2012 04:20 PM, Grant Likely wrote:
>>> On Mon, Nov 12, 2012 at 2:59 PM, Per Forlin <[email protected]> wrote:
>>>> Add support to extract device name from device tree blob.
>>>> If the property "dev-name" is set in the DTS this name will
>>>> be used when creating the device.
>>>> The auxdata_lookup has precedence and will override
>>>> the "dev-name" property.
>>>
>>> Using a 'dev-name' property has the same problem that the 'cell-index'
>>> properties have in that it is encoding part of the global namespace
>>> local to the node and it becomes easy to create collisions. Instead of
>>> this check to see if one of the properties in /aliases points to the
>>> node and use that for the name.
>>>
>>> g.
>>>
>> Thanks Grant for your feedback,
>>
>> Extract from exynos5250.dtsi:
>> -----------
>> aliases {
>> spi0 = &spi_0;
>> spi1 = &spi_1;
>> spi2 = &spi_2;
>> };
>>
>> spi_0: spi@12d20000 {
>> ...
>> };
>>
>> spi_1: spi@12d30000 {
>> ...
>> };
>>
>> spi_2: spi@12d40000 {
>> ...
>> };
>> ---------------
>>
>> Alias refers to the device node. The device node is not aware of the alias.
>>
>> How to get a device name from the aliases.
>> 1. Traverse all aliases for each device node (time consuming if there are many aliases)
>> 2. Make a new function of_alias_get_name(), today there is only of_alias_get_id()
>> 3. The functionality of setting device_name based on alias name needs to be optional because one may want to use aliases without changing the name of the device.
>> All this is feasible but perhaps not optimal.
>>
>> I don't really see how come name space is a big issue in this case. The name space of "dev-name" is local to the device node. A child device node can use the same dev-name as the parent (unless I'm mistaken which happens quite often). Introducing yet another property name pollutes the name space of the device node. Still I think the pros are stronger than the cons.
>>
>> Do you still prefer to use the name of the Alias? Could you please elaborate a bit more how this can be done in practice?
>> I would agree with you if there was a reference from the device node to the alias.
>
> Oh, I see what you're trying to do. As Lee pointed out you're trying to
> make the Linux internal way of matching up clocks and regulators happy.
> That is very much a Linux-kernel internal thing and should be solved in
> the kernel. Trying to solve it with fixed names in the device tree will
> cause problems down the road.
>
> I though you were wanting to have logical names for the devices that
> make sense to the user which is how aliases is used now.
>
> So, no, don't do this.
>
> g.
>
Hi,

I came to the same conclusion when I dag into it some more. I replied to my own comment and concluded (https://lkml.org/lkml/2012/11/13/309).
The solution is to move clocks and regulators into the DTS. When this is done there will be no need for setting a specific device-name (all those auxdata_lookup can be removed)

BR
Per

2012-11-16 08:15:44

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH] dt: platform: Extract device name from device tree blob

On Thu, 15 Nov 2012, Per Förlin wrote:

> On 11/15/2012 04:52 PM, Grant Likely wrote:
> > On Mon, 12 Nov 2012 23:54:40 +0100, Per Förlin <[email protected]> wrote:
> >> On 11/12/2012 04:20 PM, Grant Likely wrote:
> >>> On Mon, Nov 12, 2012 at 2:59 PM, Per Forlin <[email protected]> wrote:
> >>>> Add support to extract device name from device tree blob.
> >>>> If the property "dev-name" is set in the DTS this name will
> >>>> be used when creating the device.
> >>>> The auxdata_lookup has precedence and will override
> >>>> the "dev-name" property.
> >>>
> >>> Using a 'dev-name' property has the same problem that the 'cell-index'
> >>> properties have in that it is encoding part of the global namespace
> >>> local to the node and it becomes easy to create collisions. Instead of
> >>> this check to see if one of the properties in /aliases points to the
> >>> node and use that for the name.
> >>>
> >>> g.
> >>>
> >> Thanks Grant for your feedback,
> >>
> >> Extract from exynos5250.dtsi:
> >> -----------
> >> aliases {
> >> spi0 = &spi_0;
> >> spi1 = &spi_1;
> >> spi2 = &spi_2;
> >> };
> >>
> >> spi_0: spi@12d20000 {
> >> ...
> >> };
> >>
> >> spi_1: spi@12d30000 {
> >> ...
> >> };
> >>
> >> spi_2: spi@12d40000 {
> >> ...
> >> };
> >> ---------------
> >>
> >> Alias refers to the device node. The device node is not aware of the alias.
> >>
> >> How to get a device name from the aliases.
> >> 1. Traverse all aliases for each device node (time consuming if there are many aliases)
> >> 2. Make a new function of_alias_get_name(), today there is only of_alias_get_id()
> >> 3. The functionality of setting device_name based on alias name needs to be optional because one may want to use aliases without changing the name of the device.
> >> All this is feasible but perhaps not optimal.
> >>
> >> I don't really see how come name space is a big issue in this case. The name space of "dev-name" is local to the device node. A child device node can use the same dev-name as the parent (unless I'm mistaken which happens quite often). Introducing yet another property name pollutes the name space of the device node. Still I think the pros are stronger than the cons.
> >>
> >> Do you still prefer to use the name of the Alias? Could you please elaborate a bit more how this can be done in practice?
> >> I would agree with you if there was a reference from the device node to the alias.
> >
> > Oh, I see what you're trying to do. As Lee pointed out you're trying to
> > make the Linux internal way of matching up clocks and regulators happy.
> > That is very much a Linux-kernel internal thing and should be solved in
> > the kernel. Trying to solve it with fixed names in the device tree will
> > cause problems down the road.
> >
> > I though you were wanting to have logical names for the devices that
> > make sense to the user which is how aliases is used now.
> >
> > So, no, don't do this.
> >
> > g.
> >
> Hi,
>
> I came to the same conclusion when I dag into it some more. I replied to my own comment and concluded (https://lkml.org/lkml/2012/11/13/309).
> The solution is to move clocks and regulators into the DTS. When this is done there will be no need for setting a specific device-name (all those auxdata_lookup can be removed)

Right, something I will do when we have all the pieces in place.

FWIW, all of the regulators are in the DTS(I) files and are happy.
We're still working on Clocks, I believe Ulf will deal with these.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog