[PATCH 1/3] serial/imx: add device tree support
From: Grant Likely <hidden>
Date: 2011-06-21 19:14:14
Also in:
linux-devicetree, lkml, netdev
On Tue, Jun 21, 2011 at 7:55 AM, Shawn Guo [off-list ref] 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...
quoted hunk ↗ jump to hunk
On Sun, Jun 19, 2011 at 03:30:02PM +0800, Shawn Guo wrote: [...]quoted
quoted
quoted
+#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 :)quoted
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 at 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?
quoted hunk ↗ jump to hunk
? ? ? ?chosen { ? ? ? ? ? ? ? ?bootargs = "console=ttymxc0,115200 root=/dev/mmcblk0p3 rootwait"; ? ? ? ?};@@ -47,29 +53,29 @@? ? ? ? ? ? ? ? ? ? ? ?reg = <0x70000000 0x40000>; ? ? ? ? ? ? ? ? ? ? ? ?ranges; - ? ? ? ? ? ? ? ? ? ? ? uart at 7000c000 { + ? ? ? ? ? ? ? ? ? ? ? uart2: uart at 7000c000 { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?compatible = "fsl,imx51-uart", "fsl,imx21-uart"; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x7000c000 0x4000>; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <33>; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?id = <3>; - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? fsl,has-rts-cts; + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? fsl,uart-has-rtscts; ? ? ? ? ? ? ? ? ? ? ? ?}; ? ? ? ? ? ? ? ?}; - ? ? ? ? ? ? ? uart at 73fbc000 { + ? ? ? ? ? ? ? uart0: uart at 73fbc000 { ? ? ? ? ? ? ? ? ? ? ? ?compatible = "fsl,imx51-uart", "fsl,imx21-uart"; ? ? ? ? ? ? ? ? ? ? ? ?reg = <0x73fbc000 0x4000>; ? ? ? ? ? ? ? ? ? ? ? ?interrupts = <31>; ? ? ? ? ? ? ? ? ? ? ? ?id = <1>; - ? ? ? ? ? ? ? ? ? ? ? fsl,has-rts-cts; + ? ? ? ? ? ? ? ? ? ? ? fsl,uart-has-rtscts; ? ? ? ? ? ? ? ?}; - ? ? ? ? ? ? ? uart at 73fc0000 { + ? ? ? ? ? ? ? uart1: uart at 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 \> +
quoted hunk ↗ jump to hunk
+ ? ? ? 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.
quoted hunk ↗ jump to hunk
- ? ? ? 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
-- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.