Messages in this thread | | | From | Grant Likely <> | Date | Tue, 21 Jun 2011 13:38:03 -0600 | Subject | Re: [PATCH 1/3] serial/imx: add device tree support |
| |
On Tue, Jun 21, 2011 at 1:35 PM, Mitch Bradley <wmb@firmworks.com> wrote: > What is the problem with > > aliases{ > serial0 = "/uart@7000c000"; > } > > 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?
g.
> > > On 6/21/2011 9:13 AM, Grant Likely wrote: >> >> On Tue, Jun 21, 2011 at 7:55 AM, Shawn Guo<shawn.guo@freescale.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@12340000 { /*...*/ }; >> >> .... 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@7000c000 { >>> + uart2: uart@7000c000 { >>> compatible = "fsl,imx51-uart", >>> "fsl,imx21-uart"; >>> reg =<0x7000c000 0x4000>; >>> interrupts =<33>; >>> id =<3>; >>> - fsl,has-rts-cts; >>> + fsl,uart-has-rtscts; >>> }; >>> }; >>> >>> - uart@73fbc000 { >>> + uart0: uart@73fbc000 { >>> compatible = "fsl,imx51-uart", "fsl,imx21-uart"; >>> reg =<0x73fbc000 0x4000>; >>> interrupts =<31>; >>> id =<1>; >>> - fsl,has-rts-cts; >>> + fsl,uart-has-rtscts; >>> }; >>> >>> - uart@73fc0000 { >>> + uart1: uart@73fc0000 { >>> 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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
-- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |