lkml.org 
[lkml]   [2011]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/3] serial/imx: add device tree support
    What is the problem with

    aliases{
    serial0 = "/uart@7000c000";
    }

    Properties in the alias node are supposed to have string values.


    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
    >>
    >>
    >
    >
    >


    \
     
     \ /
      Last update: 2011-06-21 21:39    [W:0.041 / U:0.032 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site