On 6/21/2011 9:38 AM, Grant Likely wrote:

On Tue, Jun 21, 2011 at 1:35 PM, Mitch Bradley[off-list ref]  wrote:

quoted

What is the problem with

aliases{
   serial0 = "/uart at 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?

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.

quoted

On 6/21/2011 9:13 AM, Grant Likely wrote:

quoted

On Tue, Jun 21, 2011 at 7:55 AM, Shawn Guo[off-list ref]
  wrote:

quoted

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

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

        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

+       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

-       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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel