| Message ID | 1299503160-9910-4-git-send-email-jason.hui@linaro.org |
|---|---|
| State | New |
| Headers | show |
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 > >
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. >
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 }, };