Hi, Grant,
On Tue, Mar 8, 2011 at 12:22 AM, Grant Likely <grant.lik...@secretlab.ca> wrote:
> On Mon, Mar 7, 2011 at 6:06 AM, Jason Liu <jason....@linaro.org> wrote:
>> Signed-off-by: Jason Liu <r64...@freescale.com>
>> Signed-off-by: Jeremy Kerr <jeremy.k...@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.
>
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
