Re: [PATCH 3/3] serial/imx: parse from device tree support

[Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>]

Hi Jason,

The patch looks good, but I noticed a few details that can be improved.

On Friday 18 February 2011, Jason Liu wrote:
> index dfcf4b1..3388599
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -52,6 +52,10 @@
>  #include <asm/irq.h>
>  #include <mach/hardware.h>
>  #include <mach/imx-uart.h>
> +#ifdef CONFIG_OF
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#endif /* CONFIG_OF */
>  
>  /* Register definitions */
>  #define URXD0 0x0  /* Receiver Register */

There is generally no need to enclose any header incudes in #ifdef.
If there is a problem in the header when included without CONFIG_OF set,
that should be fixed in the header.

> @@ -1224,6 +1228,54 @@ 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;
> +
> +#ifdef CONFIG_IRDA
> +       if (of_get_property(node, "irda", NULL))
> +               sport->use_irda = 1;
> +#endif
> +       sport->port.line = line++;
> +
> +       return 0;
> +}
> +#else
> +static int serial_imx_probe_dt(struct imx_port *sport,
> +               struct platform_device *pdev)
> +{
> +       return -ENODEV;
> +}
> +#endif

Similarly, there is no need to have the #ifdef CONFIG_IRDA here.
This one takes up a few bytes of probe code, but otherwise makes
the code more readable.

	Arnd