| Message ID | 1309679760-22796-3-git-send-email-shawn.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jul 03, 2011 at 03:56:00PM +0800, Shawn Guo wrote: > It adds device tree probe support for imx tty/serial driver. > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > Signed-off-by: Jason Liu <jason.hui@linaro.org> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > Cc: Grant Likely <grant.likely@secretlab.ca> Minor comments below, but otherwise looks okay to me. Feel free to add the following when you respin with the first patch: Acked-by: Grant Likely <grant.likely@secretlab.ca> I don't see any reason preventing this from being ready to be queued up for v3.1 g. > --- > .../bindings/tty/serial/fsl-imx-uart.txt | 19 +++++ > drivers/tty/serial/imx.c | 82 +++++++++++++++++--- > 2 files changed, 91 insertions(+), 10 deletions(-) > create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > new file mode 100644 > index 0000000..a9c0406 > --- /dev/null > +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > @@ -0,0 +1,19 @@ > +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) > + > +Required properties: > +- compatible : Should be "fsl,<soc>-uart" > +- reg : Address and length of the register set for the device > +- interrupts : Should contain uart interrupt > + > +Optional properties: > +- fsl,uart-has-rtscts : Indicate the uart has rts and cts > +- fsl,irda-mode : Indicate the uart supports irda mode > + > +Example: > + > +uart@73fbc000 { > + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > + reg = <0x73fbc000 0x4000>; > + interrupts = <31>; > + fsl,uart-has-rtscts; > +}; > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 983f3bd..483c447 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -45,6 +45,8 @@ > #include <linux/delay.h> > #include <linux/rational.h> > #include <linux/slab.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > > #include <asm/io.h> > #include <asm/irq.h> > @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { > } > }; > > +static struct of_device_id imx_uart_dt_ids[] = { > + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, > + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, > + { /* sentinel */ }, Nit: drop the ',' from the last entry so that nobody inadvertently adds another entry to the list. > +}; > + > /* > * Handle any change of modem status signal since we were last called. > */ > @@ -1261,6 +1269,58 @@ 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 *np = pdev->dev.of_node; > + const struct of_device_id *of_id = > + of_match_device(imx_uart_dt_ids, &pdev->dev); > + > + if (!np) > + return -ENODEV; > + > + pdev->id = of_alias_get_id(np, "serial"); > + if (pdev->id < 0) > + return -ENODEV; If you're bailing here, it would be friendly to spit out a klog message saying why the driver has bailed. > + if (of_get_property(np, "fsl,uart-has-rtscts", NULL)) > + sport->have_rtscts = 1; > + > + if (of_get_property(np, "fsl,irda-mode", NULL)) > + sport->use_irda = 1; > + > + sport->devdata = of_id->data; > + > + return 0; > +} > +#else > +static inline 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; > + > + if (pdata->flags & IMXUART_IRDA) > + sport->use_irda = 1; > + > + sport->devdata = &imx_uart_devdata[pdev->id_entry->driver_data]; > + > + return 0; > +} > + > static int serial_imx_probe(struct platform_device *pdev) > { > struct imx_port *sport; > @@ -1273,6 +1333,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; > @@ -1301,7 +1367,6 @@ static int serial_imx_probe(struct platform_device *pdev) > init_timer(&sport->timer); > sport->timer.function = imx_timeout; > sport->timer.data = (unsigned long)sport; > - sport->devdata = &imx_uart_devdata[pdev->id_entry->driver_data]; > > sport->clk = clk_get(&pdev->dev, "uart"); > if (IS_ERR(sport->clk)) { > @@ -1312,17 +1377,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) > @@ -1384,6 +1445,7 @@ static struct platform_driver serial_imx_driver = { > .driver = { > .name = "imx-uart", > .owner = THIS_MODULE, > + .of_match_table = imx_uart_dt_ids, > }, > }; > > -- > 1.7.4.1 >
On Sun, Jul 03, 2011 at 03:14:34PM -0600, Grant Likely wrote: > On Sun, Jul 03, 2011 at 03:56:00PM +0800, Shawn Guo wrote: > > It adds device tree probe support for imx tty/serial driver. > > > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > > Signed-off-by: Jason Liu <jason.hui@linaro.org> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > > Cc: Grant Likely <grant.likely@secretlab.ca> > > Minor comments below, but otherwise looks okay to me. Feel free to > add the following when you respin with the first patch: > > Acked-by: Grant Likely <grant.likely@secretlab.ca> > > I don't see any reason preventing this from being ready to be queued > up for v3.1 > > g. > > > --- > > .../bindings/tty/serial/fsl-imx-uart.txt | 19 +++++ > > drivers/tty/serial/imx.c | 82 +++++++++++++++++--- > > 2 files changed, 91 insertions(+), 10 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > > diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > new file mode 100644 > > index 0000000..a9c0406 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > @@ -0,0 +1,19 @@ > > +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) > > + > > +Required properties: > > +- compatible : Should be "fsl,<soc>-uart" > > +- reg : Address and length of the register set for the device > > +- interrupts : Should contain uart interrupt > > + > > +Optional properties: > > +- fsl,uart-has-rtscts : Indicate the uart has rts and cts > > +- fsl,irda-mode : Indicate the uart supports irda mode > > + > > +Example: > > + > > +uart@73fbc000 { > > + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > > + reg = <0x73fbc000 0x4000>; > > + interrupts = <31>; > > + fsl,uart-has-rtscts; > > +}; > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > index 983f3bd..483c447 100644 > > --- a/drivers/tty/serial/imx.c > > +++ b/drivers/tty/serial/imx.c > > @@ -45,6 +45,8 @@ > > #include <linux/delay.h> > > #include <linux/rational.h> > > #include <linux/slab.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > > > #include <asm/io.h> > > #include <asm/irq.h> > > @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { > > } > > }; > > > > +static struct of_device_id imx_uart_dt_ids[] = { > > + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, > > + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, > > + { /* sentinel */ }, > > Nit: drop the ',' from the last entry so that nobody inadvertently > adds another entry to the list. > Ok, will make the change on other patches where the comment applies. > > +}; > > + > > /* > > * Handle any change of modem status signal since we were last called. > > */ > > @@ -1261,6 +1269,58 @@ 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 *np = pdev->dev.of_node; > > + const struct of_device_id *of_id = > > + of_match_device(imx_uart_dt_ids, &pdev->dev); > > + > > + if (!np) > > + return -ENODEV; > > + > > + pdev->id = of_alias_get_id(np, "serial"); > > + if (pdev->id < 0) > > + return -ENODEV; > > If you're bailing here, it would be friendly to spit out a klog > message saying why the driver has bailed. > Ok, will make the change on other patches where the comment applies. Regards, Shawn
On Sun, Jul 03, 2011 at 03:56:00PM +0800, Shawn Guo wrote: > It adds device tree probe support for imx tty/serial driver. > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > Signed-off-by: Jason Liu <jason.hui@linaro.org> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > Cc: Grant Likely <grant.likely@secretlab.ca> > --- > .../bindings/tty/serial/fsl-imx-uart.txt | 19 +++++ > drivers/tty/serial/imx.c | 82 +++++++++++++++++--- > 2 files changed, 91 insertions(+), 10 deletions(-) > create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > new file mode 100644 > index 0000000..a9c0406 > --- /dev/null > +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > @@ -0,0 +1,19 @@ > +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) > + > +Required properties: > +- compatible : Should be "fsl,<soc>-uart" > +- reg : Address and length of the register set for the device > +- interrupts : Should contain uart interrupt > + > +Optional properties: > +- fsl,uart-has-rtscts : Indicate the uart has rts and cts > +- fsl,irda-mode : Indicate the uart supports irda mode > + > +Example: > + > +uart@73fbc000 { > + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > + reg = <0x73fbc000 0x4000>; > + interrupts = <31>; > + fsl,uart-has-rtscts; > +}; > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 983f3bd..483c447 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -45,6 +45,8 @@ > #include <linux/delay.h> > #include <linux/rational.h> > #include <linux/slab.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > > #include <asm/io.h> > #include <asm/irq.h> > @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { > } > }; > > +static struct of_device_id imx_uart_dt_ids[] = { > + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, > + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, > + { /* sentinel */ }, > +}; Isn't this table a bit too small? What about the other SoCs? Sascha
On Mon, Jul 04, 2011 at 08:58:20AM +0200, Sascha Hauer wrote: > On Sun, Jul 03, 2011 at 03:56:00PM +0800, Shawn Guo wrote: > > It adds device tree probe support for imx tty/serial driver. > > > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > > Signed-off-by: Jason Liu <jason.hui@linaro.org> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > > Cc: Grant Likely <grant.likely@secretlab.ca> > > --- > > .../bindings/tty/serial/fsl-imx-uart.txt | 19 +++++ > > drivers/tty/serial/imx.c | 82 +++++++++++++++++--- > > 2 files changed, 91 insertions(+), 10 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > > diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > new file mode 100644 > > index 0000000..a9c0406 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > @@ -0,0 +1,19 @@ > > +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) > > + > > +Required properties: > > +- compatible : Should be "fsl,<soc>-uart" > > +- reg : Address and length of the register set for the device > > +- interrupts : Should contain uart interrupt > > + > > +Optional properties: > > +- fsl,uart-has-rtscts : Indicate the uart has rts and cts > > +- fsl,irda-mode : Indicate the uart supports irda mode > > + > > +Example: > > + > > +uart@73fbc000 { > > + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > > + reg = <0x73fbc000 0x4000>; > > + interrupts = <31>; > > + fsl,uart-has-rtscts; > > +}; > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > index 983f3bd..483c447 100644 > > --- a/drivers/tty/serial/imx.c > > +++ b/drivers/tty/serial/imx.c > > @@ -45,6 +45,8 @@ > > #include <linux/delay.h> > > #include <linux/rational.h> > > #include <linux/slab.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > > > #include <asm/io.h> > > #include <asm/irq.h> > > @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { > > } > > }; > > > > +static struct of_device_id imx_uart_dt_ids[] = { > > + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, > > + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, > > + { /* sentinel */ }, > > +}; > > Isn't this table a bit too small? What about the other SoCs? I'd expect that using fsl,imx21-uart on imx51 would result in a working port, but it feels stange and if that's the intention the example above needs fixing. Best regards Uwe
On Mon, Jul 04, 2011 at 09:38:26AM +0200, Uwe Kleine-König wrote: > On Mon, Jul 04, 2011 at 08:58:20AM +0200, Sascha Hauer wrote: > > On Sun, Jul 03, 2011 at 03:56:00PM +0800, Shawn Guo wrote: > > > It adds device tree probe support for imx tty/serial driver. > > > > > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > > > Signed-off-by: Jason Liu <jason.hui@linaro.org> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > > > Cc: Grant Likely <grant.likely@secretlab.ca> > > > --- > > > .../bindings/tty/serial/fsl-imx-uart.txt | 19 +++++ > > > drivers/tty/serial/imx.c | 82 +++++++++++++++++--- > > > 2 files changed, 91 insertions(+), 10 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > > > > diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > new file mode 100644 > > > index 0000000..a9c0406 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > @@ -0,0 +1,19 @@ > > > +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) > > > + > > > +Required properties: > > > +- compatible : Should be "fsl,<soc>-uart" > > > +- reg : Address and length of the register set for the device > > > +- interrupts : Should contain uart interrupt > > > + > > > +Optional properties: > > > +- fsl,uart-has-rtscts : Indicate the uart has rts and cts > > > +- fsl,irda-mode : Indicate the uart supports irda mode > > > + > > > +Example: > > > + > > > +uart@73fbc000 { > > > + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > > > + reg = <0x73fbc000 0x4000>; > > > + interrupts = <31>; > > > + fsl,uart-has-rtscts; > > > +}; > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > > index 983f3bd..483c447 100644 > > > --- a/drivers/tty/serial/imx.c > > > +++ b/drivers/tty/serial/imx.c > > > @@ -45,6 +45,8 @@ > > > #include <linux/delay.h> > > > #include <linux/rational.h> > > > #include <linux/slab.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > > > > #include <asm/io.h> > > > #include <asm/irq.h> > > > @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { > > > } > > > }; > > > > > > +static struct of_device_id imx_uart_dt_ids[] = { > > > + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, > > > + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, > > > + { /* sentinel */ }, > > > +}; > > > > Isn't this table a bit too small? What about the other SoCs? > I'd expect that using fsl,imx21-uart on imx51 would result in a working > port, but it feels stange and if that's the intention the example above > needs fixing. > Here is what I have in dts file for uart. The 'compatible' tells that it's a imx51-uart and compatible with imx21-uart. uart0: uart@73fbc000 { compatible = "fsl,imx51-uart", "fsl,imx21-uart"; reg = <0x73fbc000 0x4000>; interrupts = <31>; fsl,uart-has-rtscts; }; I did not buy it until I got convinced by Grant here [1]. We do not need this long table until there are some soc specific thing to distinguish. Currently, they are all compatible with imx21-uart and share imx_uart_devdata[IMX2_UART]. { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, { .compatible = "fsl,imx25-uart", .data = &imx_uart_devdata[IMX2_UART], }, { .compatible = "fsl,imx27-uart", .data = &imx_uart_devdata[IMX2_UART], }, { .compatible = "fsl,imx31-uart", .data = &imx_uart_devdata[IMX2_UART], }, { .compatible = "fsl,imx35-uart", .data = &imx_uart_devdata[IMX2_UART], }, { .compatible = "fsl,imx50-uart", .data = &imx_uart_devdata[IMX2_UART], }, { .compatible = "fsl,imx51-uart", .data = &imx_uart_devdata[IMX2_UART], }, { .compatible = "fsl,imx53-uart", .data = &imx_uart_devdata[IMX2_UART], }, [1] http://permalink.gmane.org/gmane.linux.network/198935
On Mon, Jul 04, 2011 at 05:05:27PM +0800, Shawn Guo wrote: > On Mon, Jul 04, 2011 at 09:38:26AM +0200, Uwe Kleine-König wrote: > > On Mon, Jul 04, 2011 at 08:58:20AM +0200, Sascha Hauer wrote: > > > On Sun, Jul 03, 2011 at 03:56:00PM +0800, Shawn Guo wrote: > > > > It adds device tree probe support for imx tty/serial driver. > > > > > > > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > > > > Signed-off-by: Jason Liu <jason.hui@linaro.org> > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > > > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > > > > Cc: Grant Likely <grant.likely@secretlab.ca> > > > > --- > > > > .../bindings/tty/serial/fsl-imx-uart.txt | 19 +++++ > > > > drivers/tty/serial/imx.c | 82 +++++++++++++++++--- > > > > 2 files changed, 91 insertions(+), 10 deletions(-) > > > > create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > > new file mode 100644 > > > > index 0000000..a9c0406 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > > @@ -0,0 +1,19 @@ > > > > +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) > > > > + > > > > +Required properties: > > > > +- compatible : Should be "fsl,<soc>-uart" > > > > +- reg : Address and length of the register set for the device > > > > +- interrupts : Should contain uart interrupt > > > > + > > > > +Optional properties: > > > > +- fsl,uart-has-rtscts : Indicate the uart has rts and cts > > > > +- fsl,irda-mode : Indicate the uart supports irda mode > > > > + > > > > +Example: > > > > + > > > > +uart@73fbc000 { > > > > + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > > > > + reg = <0x73fbc000 0x4000>; > > > > + interrupts = <31>; > > > > + fsl,uart-has-rtscts; > > > > +}; > > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > > > index 983f3bd..483c447 100644 > > > > --- a/drivers/tty/serial/imx.c > > > > +++ b/drivers/tty/serial/imx.c > > > > @@ -45,6 +45,8 @@ > > > > #include <linux/delay.h> > > > > #include <linux/rational.h> > > > > #include <linux/slab.h> > > > > +#include <linux/of.h> > > > > +#include <linux/of_device.h> > > > > > > > > #include <asm/io.h> > > > > #include <asm/irq.h> > > > > @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { > > > > } > > > > }; > > > > > > > > +static struct of_device_id imx_uart_dt_ids[] = { > > > > + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, > > > > + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, > > > > + { /* sentinel */ }, > > > > +}; > > > > > > Isn't this table a bit too small? What about the other SoCs? > > I'd expect that using fsl,imx21-uart on imx51 would result in a working > > port, but it feels stange and if that's the intention the example above > > needs fixing. > > > Here is what I have in dts file for uart. The 'compatible' tells > that it's a imx51-uart and compatible with imx21-uart. > > uart0: uart@73fbc000 { > compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > reg = <0x73fbc000 0x4000>; > interrupts = <31>; > fsl,uart-has-rtscts; > }; /me learned something new about dt. Sorry for the noise. Best regards Uwe
On Mon, Jul 04, 2011 at 09:38:26AM +0200, Uwe Kleine-König wrote: > On Mon, Jul 04, 2011 at 08:58:20AM +0200, Sascha Hauer wrote: > > On Sun, Jul 03, 2011 at 03:56:00PM +0800, Shawn Guo wrote: > > > It adds device tree probe support for imx tty/serial driver. > > > > > > Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> > > > Signed-off-by: Jason Liu <jason.hui@linaro.org> > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > > Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> > > > Cc: Grant Likely <grant.likely@secretlab.ca> > > > --- > > > .../bindings/tty/serial/fsl-imx-uart.txt | 19 +++++ > > > drivers/tty/serial/imx.c | 82 +++++++++++++++++--- > > > 2 files changed, 91 insertions(+), 10 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > > > > diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > new file mode 100644 > > > index 0000000..a9c0406 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt > > > @@ -0,0 +1,19 @@ > > > +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) > > > + > > > +Required properties: > > > +- compatible : Should be "fsl,<soc>-uart" > > > +- reg : Address and length of the register set for the device > > > +- interrupts : Should contain uart interrupt > > > + > > > +Optional properties: > > > +- fsl,uart-has-rtscts : Indicate the uart has rts and cts > > > +- fsl,irda-mode : Indicate the uart supports irda mode > > > + > > > +Example: > > > + > > > +uart@73fbc000 { > > > + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; > > > + reg = <0x73fbc000 0x4000>; > > > + interrupts = <31>; > > > + fsl,uart-has-rtscts; > > > +}; > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > > index 983f3bd..483c447 100644 > > > --- a/drivers/tty/serial/imx.c > > > +++ b/drivers/tty/serial/imx.c > > > @@ -45,6 +45,8 @@ > > > #include <linux/delay.h> > > > #include <linux/rational.h> > > > #include <linux/slab.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > > > > #include <asm/io.h> > > > #include <asm/irq.h> > > > @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { > > > } > > > }; > > > > > > +static struct of_device_id imx_uart_dt_ids[] = { > > > + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, > > > + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, > > > + { /* sentinel */ }, > > > +}; > > > > Isn't this table a bit too small? What about the other SoCs? > I'd expect that using fsl,imx21-uart on imx51 would result in a working > port, but it feels stange and if that's the intention the example above > needs fixing. If the newer parts claim compatibility with the older, then have a terse of_device_id table is just fine. ie, the compatible property for an imx51 would be: compatible = "fsl,imx51-uart", "fsl,imx21-uart"; It would only be wrong if the device tree node omitted the "fsl,imx51-uart" value from the list. g.
diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt new file mode 100644 index 0000000..a9c0406 --- /dev/null +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt @@ -0,0 +1,19 @@ +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART) + +Required properties: +- compatible : Should be "fsl,<soc>-uart" +- reg : Address and length of the register set for the device +- interrupts : Should contain uart interrupt + +Optional properties: +- fsl,uart-has-rtscts : Indicate the uart has rts and cts +- fsl,irda-mode : Indicate the uart supports irda mode + +Example: + +uart@73fbc000 { + compatible = "fsl,imx51-uart", "fsl,imx21-uart"; + reg = <0x73fbc000 0x4000>; + interrupts = <31>; + fsl,uart-has-rtscts; +}; diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 983f3bd..483c447 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -45,6 +45,8 @@ #include <linux/delay.h> #include <linux/rational.h> #include <linux/slab.h> +#include <linux/of.h> +#include <linux/of_device.h> #include <asm/io.h> #include <asm/irq.h> @@ -239,6 +241,12 @@ static struct platform_device_id imx_uart_devtype[] = { } }; +static struct of_device_id imx_uart_dt_ids[] = { + { .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], }, + { .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX2_UART], }, + { /* sentinel */ }, +}; + /* * Handle any change of modem status signal since we were last called. */ @@ -1261,6 +1269,58 @@ 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 *np = pdev->dev.of_node; + const struct of_device_id *of_id = + of_match_device(imx_uart_dt_ids, &pdev->dev); + + if (!np) + return -ENODEV; + + pdev->id = of_alias_get_id(np, "serial"); + if (pdev->id < 0) + return -ENODEV; + + if (of_get_property(np, "fsl,uart-has-rtscts", NULL)) + sport->have_rtscts = 1; + + if (of_get_property(np, "fsl,irda-mode", NULL)) + sport->use_irda = 1; + + sport->devdata = of_id->data; + + return 0; +} +#else +static inline 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; + + if (pdata->flags & IMXUART_IRDA) + sport->use_irda = 1; + + sport->devdata = &imx_uart_devdata[pdev->id_entry->driver_data]; + + return 0; +} + static int serial_imx_probe(struct platform_device *pdev) { struct imx_port *sport; @@ -1273,6 +1333,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; @@ -1301,7 +1367,6 @@ static int serial_imx_probe(struct platform_device *pdev) init_timer(&sport->timer); sport->timer.function = imx_timeout; sport->timer.data = (unsigned long)sport; - sport->devdata = &imx_uart_devdata[pdev->id_entry->driver_data]; sport->clk = clk_get(&pdev->dev, "uart"); if (IS_ERR(sport->clk)) { @@ -1312,17 +1377,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) @@ -1384,6 +1445,7 @@ static struct platform_driver serial_imx_driver = { .driver = { .name = "imx-uart", .owner = THIS_MODULE, + .of_match_table = imx_uart_dt_ids, }, };