[V2,3/3] serial/imx: parse from device tree support

Message ID 1299503160-9910-4-git-send-email-jason.hui@linaro.org
State New
Headers show

Commit Message

Jason Hui March 7, 2011, 1:06 p.m.
Signed-off-by: Jason Liu <r64343@freescale.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
---
 drivers/tty/serial/imx.c |   79 ++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 69 insertions(+), 10 deletions(-)

Comments

Grant Likely March 7, 2011, 4:22 p.m. | #1
On Mon, Mar 7, 2011 at 6:06 AM, Jason Liu <jason.hui@linaro.org> wrote:
> Signed-off-by: Jason Liu <r64343@freescale.com>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

Hi Jason,

comments below.

> ---
>  drivers/tty/serial/imx.c |   79 ++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 69 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index dfcf4b1..1ebca28 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -53,6 +53,9 @@
>  #include <mach/hardware.h>
>  #include <mach/imx-uart.h>
>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
>  /* Register definitions */
>  #define URXD0 0x0  /* Receiver Register */
>  #define URTX0 0x40 /* Transmitter Register */
> @@ -1224,6 +1227,53 @@ static int serial_imx_resume(struct platform_device *dev)
>        return 0;
>  }
>
> +#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;
> +       static int line;
> +
> +       if (!node)
> +               return -ENODEV;
> +
> +       if (of_get_property(node, "rts-cts", NULL))
> +               sport->have_rtscts = 1;
> +
> +       if (of_get_property(node, "irda", NULL))
> +               sport->use_irda = 1;

These properties need to be documented in
Documentation/devicetree/bindings/serial.  Also, for device specific
properties, it is considered good practice to prefix the property name
with the vendor name; so "fsl,rts-cts" and "fsl,irda".  Also, slightly
more descriptive property names would probably be helpful:
"fsl,has-rts-cts" and "fsl,irda-mode"

> +
> +       sport->port.line = line++;
> +
> +       return 0;
> +}
> +#else
> +static int serial_imx_probe_dt(struct imx_port *sport,
> +               struct platform_device *pdev)
> +{
> +       return -ENODEV;
> +}
> +#endif
> +
> +static int serial_imx_probe_pdata(struct imx_port *sport,
> +               struct platform_device *pdev)
> +{
> +       struct imxuart_platform_data *pdata = pdev->dev.platform_data;
> +
> +       if (!pdata)
> +               return 0;
> +
> +       if (pdata->flags & IMXUART_HAVE_RTSCTS)
> +               sport->have_rtscts = 1;
> +
> +#ifdef CONFIG_IRDA
> +       if (pdata->flags & IMXUART_IRDA)
> +               sport->use_irda = 1;
> +#endif
> +       sport->port.line = pdev->id;
> +
> +       return 0;
> +}
>  static int serial_imx_probe(struct platform_device *pdev)
>  {
>        struct imx_port *sport;
> @@ -1236,6 +1286,12 @@ static int serial_imx_probe(struct platform_device *pdev)
>        if (!sport)
>                return -ENOMEM;
>
> +       ret = serial_imx_probe_dt(sport, pdev);
> +       if (ret == -ENODEV)
> +               ret = serial_imx_probe_pdata(sport, pdev);
> +       if (ret)
> +               goto free;
> +
>        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>        if (!res) {
>                ret = -ENODEV;
> @@ -1260,7 +1316,6 @@ static int serial_imx_probe(struct platform_device *pdev)
>        sport->port.fifosize = 32;
>        sport->port.ops = &imx_pops;
>        sport->port.flags = UPF_BOOT_AUTOCONF;
> -       sport->port.line = pdev->id;
>        init_timer(&sport->timer);
>        sport->timer.function = imx_timeout;
>        sport->timer.data     = (unsigned long)sport;
> @@ -1274,17 +1329,13 @@ static int serial_imx_probe(struct platform_device *pdev)
>
>        sport->port.uartclk = clk_get_rate(sport->clk);
>
> -       imx_ports[pdev->id] = sport;
> +       if (imx_ports[sport->port.line]) {
> +               ret = -EBUSY;
> +               goto clkput;
> +       }
> +       imx_ports[sport->port.line] = sport;
>
>        pdata = pdev->dev.platform_data;
> -       if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
> -               sport->have_rtscts = 1;
> -
> -#ifdef CONFIG_IRDA
> -       if (pdata && (pdata->flags & IMXUART_IRDA))
> -               sport->use_irda = 1;
> -#endif
> -
>        if (pdata && pdata->init) {
>                ret = pdata->init(pdev);
>                if (ret)
> @@ -1336,6 +1387,11 @@ static int serial_imx_remove(struct platform_device *pdev)
>        return 0;
>  }
>
> +static struct of_device_id imx_uart_matches[] = {
> +       { .compatible = "imx-uart" },

Should be "fsl,imx51-uart".  It is always a good idea to specify the exact part.

> +       {},
> +};
> +
>  static struct platform_driver serial_imx_driver = {
>        .probe          = serial_imx_probe,
>        .remove         = serial_imx_remove,
> @@ -1345,6 +1401,9 @@ static struct platform_driver serial_imx_driver = {
>        .driver         = {
>                .name   = "imx-uart",
>                .owner  = THIS_MODULE,
> +#if defined(CONFIG_OF)
> +               .of_match_table = imx_uart_matches,
> +#endif
>        },
>  };
>
> --
> 1.7.0.4
>
>
Jason Hui March 8, 2011, 2:56 a.m. | #2
Hi, Grant,

On Tue, Mar 8, 2011 at 12:22 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Mar 7, 2011 at 6:06 AM, Jason Liu <jason.hui@linaro.org> wrote:
>> Signed-off-by: Jason Liu <r64343@freescale.com>
>> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
>
> Hi Jason,
>
> comments below.
>
>> ---
>>  drivers/tty/serial/imx.c |   79 ++++++++++++++++++++++++++++++++++++++++------
>>  1 files changed, 69 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index dfcf4b1..1ebca28 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -53,6 +53,9 @@
>>  #include <mach/hardware.h>
>>  #include <mach/imx-uart.h>
>>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +
>>  /* Register definitions */
>>  #define URXD0 0x0  /* Receiver Register */
>>  #define URTX0 0x40 /* Transmitter Register */
>> @@ -1224,6 +1227,53 @@ static int serial_imx_resume(struct platform_device *dev)
>>        return 0;
>>  }
>>
>> +#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;
>> +       static int line;
>> +
>> +       if (!node)
>> +               return -ENODEV;
>> +
>> +       if (of_get_property(node, "rts-cts", NULL))
>> +               sport->have_rtscts = 1;
>> +
>> +       if (of_get_property(node, "irda", NULL))
>> +               sport->use_irda = 1;
>
> These properties need to be documented in
> Documentation/devicetree/bindings/serial.  Also, for device specific
> properties, it is considered good practice to prefix the property name
> with the vendor name; so "fsl,rts-cts" and "fsl,irda".  Also, slightly
> more descriptive property names would probably be helpful:
> "fsl,has-rts-cts" and "fsl,irda-mode"

OK,

>
>> +
>> +       sport->port.line = line++;
>> +
>> +       return 0;
>> +}
>> +#else
>> +static int serial_imx_probe_dt(struct imx_port *sport,
>> +               struct platform_device *pdev)
>> +{
>> +       return -ENODEV;
>> +}
>> +#endif
>> +
>> +static int serial_imx_probe_pdata(struct imx_port *sport,
>> +               struct platform_device *pdev)
>> +{
>> +       struct imxuart_platform_data *pdata = pdev->dev.platform_data;
>> +
>> +       if (!pdata)
>> +               return 0;
>> +
>> +       if (pdata->flags & IMXUART_HAVE_RTSCTS)
>> +               sport->have_rtscts = 1;
>> +
>> +#ifdef CONFIG_IRDA
>> +       if (pdata->flags & IMXUART_IRDA)
>> +               sport->use_irda = 1;
>> +#endif
>> +       sport->port.line = pdev->id;
>> +
>> +       return 0;
>> +}
>>  static int serial_imx_probe(struct platform_device *pdev)
>>  {
>>        struct imx_port *sport;
>> @@ -1236,6 +1286,12 @@ static int serial_imx_probe(struct platform_device *pdev)
>>        if (!sport)
>>                return -ENOMEM;
>>
>> +       ret = serial_imx_probe_dt(sport, pdev);
>> +       if (ret == -ENODEV)
>> +               ret = serial_imx_probe_pdata(sport, pdev);
>> +       if (ret)
>> +               goto free;
>> +
>>        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>        if (!res) {
>>                ret = -ENODEV;
>> @@ -1260,7 +1316,6 @@ static int serial_imx_probe(struct platform_device *pdev)
>>        sport->port.fifosize = 32;
>>        sport->port.ops = &imx_pops;
>>        sport->port.flags = UPF_BOOT_AUTOCONF;
>> -       sport->port.line = pdev->id;
>>        init_timer(&sport->timer);
>>        sport->timer.function = imx_timeout;
>>        sport->timer.data     = (unsigned long)sport;
>> @@ -1274,17 +1329,13 @@ static int serial_imx_probe(struct platform_device *pdev)
>>
>>        sport->port.uartclk = clk_get_rate(sport->clk);
>>
>> -       imx_ports[pdev->id] = sport;
>> +       if (imx_ports[sport->port.line]) {
>> +               ret = -EBUSY;
>> +               goto clkput;
>> +       }
>> +       imx_ports[sport->port.line] = sport;
>>
>>        pdata = pdev->dev.platform_data;
>> -       if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
>> -               sport->have_rtscts = 1;
>> -
>> -#ifdef CONFIG_IRDA
>> -       if (pdata && (pdata->flags & IMXUART_IRDA))
>> -               sport->use_irda = 1;
>> -#endif
>> -
>>        if (pdata && pdata->init) {
>>                ret = pdata->init(pdev);
>>                if (ret)
>> @@ -1336,6 +1387,11 @@ static int serial_imx_remove(struct platform_device *pdev)
>>        return 0;
>>  }
>>
>> +static struct of_device_id imx_uart_matches[] = {
>> +       { .compatible = "imx-uart" },
>
> Should be "fsl,imx51-uart".  It is always a good idea to specify the exact part.

OK,
>
>> +       {},
>> +};
>> +
>>  static struct platform_driver serial_imx_driver = {
>>        .probe          = serial_imx_probe,
>>        .remove         = serial_imx_remove,
>> @@ -1345,6 +1401,9 @@ static struct platform_driver serial_imx_driver = {
>>        .driver         = {
>>                .name   = "imx-uart",
>>                .owner  = THIS_MODULE,
>> +#if defined(CONFIG_OF)
>> +               .of_match_table = imx_uart_matches,
>> +#endif
>>        },
>>  };
>>
>> --
>> 1.7.0.4
>>
>>
>
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>

Patch

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dfcf4b1..1ebca28 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -53,6 +53,9 @@ 
 #include <mach/hardware.h>
 #include <mach/imx-uart.h>
 
+#include <linux/of.h>
+#include <linux/of_address.h>
+
 /* Register definitions */
 #define URXD0 0x0  /* Receiver Register */
 #define URTX0 0x40 /* Transmitter Register */
@@ -1224,6 +1227,53 @@  static int serial_imx_resume(struct platform_device *dev)
 	return 0;
 }
 
+#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;
+	static int line;
+
+	if (!node)
+		return -ENODEV;
+
+	if (of_get_property(node, "rts-cts", NULL))
+		sport->have_rtscts = 1;
+
+	if (of_get_property(node, "irda", NULL))
+		sport->use_irda = 1;
+
+	sport->port.line = line++;
+
+	return 0;
+}
+#else
+static int serial_imx_probe_dt(struct imx_port *sport,
+		struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+#endif
+
+static int serial_imx_probe_pdata(struct imx_port *sport,
+		struct platform_device *pdev)
+{
+	struct imxuart_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!pdata)
+		return 0;
+
+	if (pdata->flags & IMXUART_HAVE_RTSCTS)
+		sport->have_rtscts = 1;
+
+#ifdef CONFIG_IRDA
+	if (pdata->flags & IMXUART_IRDA)
+		sport->use_irda = 1;
+#endif
+	sport->port.line = pdev->id;
+
+	return 0;
+}
 static int serial_imx_probe(struct platform_device *pdev)
 {
 	struct imx_port *sport;
@@ -1236,6 +1286,12 @@  static int serial_imx_probe(struct platform_device *pdev)
 	if (!sport)
 		return -ENOMEM;
 
+	ret = serial_imx_probe_dt(sport, pdev);
+	if (ret == -ENODEV)
+		ret = serial_imx_probe_pdata(sport, pdev);
+	if (ret)
+		goto free;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		ret = -ENODEV;
@@ -1260,7 +1316,6 @@  static int serial_imx_probe(struct platform_device *pdev)
 	sport->port.fifosize = 32;
 	sport->port.ops = &imx_pops;
 	sport->port.flags = UPF_BOOT_AUTOCONF;
-	sport->port.line = pdev->id;
 	init_timer(&sport->timer);
 	sport->timer.function = imx_timeout;
 	sport->timer.data     = (unsigned long)sport;
@@ -1274,17 +1329,13 @@  static int serial_imx_probe(struct platform_device *pdev)
 
 	sport->port.uartclk = clk_get_rate(sport->clk);
 
-	imx_ports[pdev->id] = sport;
+	if (imx_ports[sport->port.line]) {
+		ret = -EBUSY;
+		goto clkput;
+	}
+	imx_ports[sport->port.line] = sport;
 
 	pdata = pdev->dev.platform_data;
-	if (pdata && (pdata->flags & IMXUART_HAVE_RTSCTS))
-		sport->have_rtscts = 1;
-
-#ifdef CONFIG_IRDA
-	if (pdata && (pdata->flags & IMXUART_IRDA))
-		sport->use_irda = 1;
-#endif
-
 	if (pdata && pdata->init) {
 		ret = pdata->init(pdev);
 		if (ret)
@@ -1336,6 +1387,11 @@  static int serial_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id imx_uart_matches[] = {
+	{ .compatible = "imx-uart" },
+	{},
+};
+
 static struct platform_driver serial_imx_driver = {
 	.probe		= serial_imx_probe,
 	.remove		= serial_imx_remove,
@@ -1345,6 +1401,9 @@  static struct platform_driver serial_imx_driver = {
 	.driver		= {
 		.name	= "imx-uart",
 		.owner	= THIS_MODULE,
+#if defined(CONFIG_OF)
+		.of_match_table = imx_uart_matches,
+#endif
 	},
 };