| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4E011678.4030208@firmworks.com>
Date: Tue, 21 Jun 2011 18:08:56 -0400
From: Mitch Bradley <wmb@...mworks.com>
To: Grant Likely <grant.likely@...retlab.ca>
CC: patches@...aro.org, netdev@...r.kernel.org,
devicetree-discuss@...ts.ozlabs.org,
Jason Liu <jason.hui@...aro.org>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Jeremy Kerr <jeremy.kerr@...onical.com>,
Sascha Hauer <s.hauer@...gutronix.de>,
Shawn Guo <shawn.guo@...escale.com>
Subject: Re: [PATCH 1/3] serial/imx: add device tree support
On 6/21/2011 9:38 AM, Grant Likely wrote:
> On Tue, Jun 21, 2011 at 1:35 PM, Mitch Bradley<wmb@...mworks.com> wrote:
>> What is the problem with
>>
>> aliases{
>> serial0 = "/uart@...0c000";
>> }
>>
>> Properties in the alias node are supposed to have string values.
>
> ?
>
> Not sure I follow. Indeed, properties in the aliases node are string values.
>
> Are you referring to how I was proposing some dtc syntax for
> generating the alias strings?
The point is that if you refer to the node explicitly by its string
name, the need for a label disappears and the problem of overriding a
default alias disappears (assuming that a later redefinition of a
property takes precedence over an earlier one, as is the OFW convention).
>
> g.
>
>>
>>
>> On 6/21/2011 9:13 AM, Grant Likely wrote:
>>>
>>> On Tue, Jun 21, 2011 at 7:55 AM, Shawn Guo<shawn.guo@...escale.com>
>>> wrote:
>>>>
>>>> Hi Grant,
>>>>
>>>> I just gave a try to use aliases node for identify the device index.
>>>> Please take a look and let me know if it's what you expect.
>>>
>>> Thanks Shawn. This is definitely on track. Comments below...
>>>
>>>>
>>>> On Sun, Jun 19, 2011 at 03:30:02PM +0800, Shawn Guo wrote:
>>>> [...]
>>>>>>>
>>>>>>> +#ifdef CONFIG_OF
>>>>>>> +static int serial_imx_probe_dt(struct imx_port *sport,
>>>>>>> + struct platform_device *pdev)
>>>>>>> +{
>>>>>>> + struct device_node *node = pdev->dev.of_node;
>>>>>>> + const __be32 *line;
>>>>>>> +
>>>>>>> + if (!node)
>>>>>>> + return -ENODEV;
>>>>>>> +
>>>>>>> + line = of_get_property(node, "id", NULL);
>>>>>>> + if (!line)
>>>>>>> + return -ENODEV;
>>>>>>> +
>>>>>>> + sport->port.line = be32_to_cpup(line) - 1;
>>>>>>
>>>>>> Hmmm, I really would like to be rid of this. Instead, if uarts must
>>>>>> be enumerated, the driver should look for a /aliases/uart* property
>>>>>> that matches the of_node. Doing it that way is already established in
>>>>>> the OpenFirmware documentation, and it ensures there are no overlaps
>>>>>> in the global namespace.
>>>>>>
>>>>>
>>>>> I just gave one more try to avoid using 'aliases', and you gave a
>>>>> 'no' again. Now, I know how hard you are on this. Okay, I start
>>>>> thinking about your suggestion seriously :)
>>>>>
>>>>>> We do need some infrastructure to make that easier though. Would you
>>>>>> have time to help put that together?
>>>>>>
>>>>> Ok, I will give it a try.
>>>>>
>>>>
>>>> diff --git a/arch/arm/boot/dts/imx51-babbage.dts
>>>> b/arch/arm/boot/dts/imx51-babbage.dts
>>>> index da0381a..f4a5c3c 100644
>>>> --- a/arch/arm/boot/dts/imx51-babbage.dts
>>>> +++ b/arch/arm/boot/dts/imx51-babbage.dts
>>>> @@ -18,6 +18,12 @@
>>>> compatible = "fsl,imx51-babbage", "fsl,imx51";
>>>> interrupt-parent =<&tzic>;
>>>>
>>>> + aliases {
>>>> + serial0 =&uart0;
>>>> + serial1 =&uart1;
>>>> + serial2 =&uart2;
>>>> + };
>>>> +
>>>
>>> Hmmm. David Gibson had tossed out an idea of automatically generating
>>> aliases from labels. It may be time to revisit that idea.
>>>
>>> David, perhaps using this format for a label should turn it into an
>>> alias (prefix label with an asterisk):
>>>
>>> *thealias: i2c@...40000 { /*...*/ };
>>>
>>> .... although that approach gets *really* hairy when considering that
>>> different boards will want different enumeration. How would one
>>> override an automatic alias defined by an included .dts file?
>>>
>>>> chosen {
>>>> bootargs = "console=ttymxc0,115200 root=/dev/mmcblk0p3
>>>> rootwait";
>>>> };
>>>> @@ -47,29 +53,29 @@
>>>> reg =<0x70000000 0x40000>;
>>>> ranges;
>>>>
>>>> - uart@...0c000 {
>>>> + uart2: uart@...0c000 {
>>>> compatible = "fsl,imx51-uart",
>>>> "fsl,imx21-uart";
>>>> reg =<0x7000c000 0x4000>;
>>>> interrupts =<33>;
>>>> id =<3>;
>>>> - fsl,has-rts-cts;
>>>> + fsl,uart-has-rtscts;
>>>> };
>>>> };
>>>>
>>>> - uart@...bc000 {
>>>> + uart0: uart@...bc000 {
>>>> compatible = "fsl,imx51-uart", "fsl,imx21-uart";
>>>> reg =<0x73fbc000 0x4000>;
>>>> interrupts =<31>;
>>>> id =<1>;
>>>> - fsl,has-rts-cts;
>>>> + fsl,uart-has-rtscts;
>>>> };
>>>>
>>>> - uart@...c0000 {
>>>> + uart1: uart@...c0000 {
>>>> compatible = "fsl,imx51-uart", "fsl,imx21-uart";
>>>> reg =<0x73fc0000 0x4000>;
>>>> interrupts =<32>;
>>>> id =<2>;
>>>> - fsl,has-rts-cts;
>>>> + fsl,uart-has-rtscts;
>>>> };
>>>> };
>>>>
>>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>>> index 632ebae..13df5d2 100644
>>>> --- a/drivers/of/base.c
>>>> +++ b/drivers/of/base.c
>>>> @@ -737,6 +737,37 @@ err0:
>>>> EXPORT_SYMBOL(of_parse_phandles_with_args);
>>>>
>>>> /**
>>>> + * of_get_device_index - Get device index by looking up "aliases"
>>>> node
>>>> + * @np: Pointer to device node that asks for device index
>>>> + * @name: The device alias without index number
>>>> + *
>>>> + * Returns the device index if find it, else returns -ENODEV.
>>>> + */
>>>> +int of_get_device_index(struct device_node *np, const char *alias)
>>>> +{
>>>> + struct device_node *aliases = of_find_node_by_name(NULL,
>>>> "aliases");
>>>> + struct property *prop;
>>>> + char name[32];
>>>> + int index = 0;
>>>> +
>>>> + if (!aliases)
>>>> + return -ENODEV;
>>>> +
>>>> + while (1) {
>>>> + snprintf(name, sizeof(name), "%s%d", alias, index);
>>>> + prop = of_find_property(aliases, name, NULL);
>>>> + if (!prop)
>>>> + return -ENODEV;
>>>> + if (np == of_find_node_by_path(prop->value))
>>>> + break;
>>>> + index++;
>>>> + }
>>>
>>> Rather than parsing the alias strings everytime, it would probably be
>>> better to preprocess all the properties in the aliases node and create
>>> a lookup table of alias->node references that can be walked quickly
>>> and trivially.
>>>
>>> Also, when obtaining an enumeration for a device, you'll need to be
>>> careful about what number gets returned. If the node doesn't match a
>>> given alias, but aliases do exist for other devices of like type, then
>>> you need to be careful not to assign a number already assigned to
>>> another device via an alias (this of course assumes the driver
>>> supports dynamics enumeration, which many drivers will). It would be
>>>
>>> \> +
>>>>
>>>> + return index;
>>>> +}
>>>> +EXPORT_SYMBOL(of_get_device_index);
>>>> +
>>>> +/**
>>>> * prom_add_property - Add a property to a node
>>>> */
>>>> int prom_add_property(struct device_node *np, struct property *prop)
>>>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>>>> index da436e0..852668f 100644
>>>> --- a/drivers/tty/serial/imx.c
>>>> +++ b/drivers/tty/serial/imx.c
>>>> @@ -1271,18 +1271,18 @@ static int serial_imx_probe_dt(struct imx_port
>>>> *sport,
>>>> struct device_node *node = pdev->dev.of_node;
>>>> const struct of_device_id *of_id =
>>>> of_match_device(imx_uart_dt_ids,&pdev->dev);
>>>> - const __be32 *line;
>>>> + int line;
>>>>
>>>> if (!node)
>>>> return -ENODEV;
>>>>
>>>> - line = of_get_property(node, "id", NULL);
>>>> - if (!line)
>>>> + line = of_get_device_index(node, "serial");
>>>> + if (IS_ERR_VALUE(line))
>>>> return -ENODEV;
>>>
>>> Personally, it an alias isn't present, then I'd dynamically assign a port
>>> id.
>>>
>>>>
>>>> - sport->port.line = be32_to_cpup(line) - 1;
>>>> + sport->port.line = line;
>>>>
>>>> - if (of_get_property(node, "fsl,has-rts-cts", NULL))
>>>> + if (of_get_property(node, "fsl,uart-has-rtscts", NULL))
>>>> sport->have_rtscts = 1;
>>>>
>>>> if (of_get_property(node, "fsl,irda-mode", NULL))
>>>> diff --git a/include/linux/of.h b/include/linux/of.h
>>>> index bfc0ed1..3153752 100644
>>>> --- a/include/linux/of.h
>>>> +++ b/include/linux/of.h
>>>> @@ -213,6 +213,8 @@ extern int of_parse_phandles_with_args(struct
>>>> device_node *np,
>>>> const char *list_name, const char *cells_name, int index,
>>>> struct device_node **out_node, const void **out_args);
>>>>
>>>> +extern int of_get_device_index(struct device_node *np, const char
>>>> *alias);
>>>> +
>>>> extern int of_machine_is_compatible(const char *compat);
>>>>
>>>> extern int prom_add_property(struct device_node* np, struct property*
>>>> prop);
>>>>
>>>> --
>>>> Regards,
>>>> Shawn
>>>>
>>>>
>>>
>>>
>>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists