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
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
