[U-Boot] [PATCH 04/12] ns16550: add generic binding to unify the drivers

Bin Meng
2015-11-18T01:03:34Z

Hi Thomas,

On Mon, Nov 16, 2015 at 10:36 PM, Thomas Chou <thomas@wytron.com.tw> wrote:
> Add generic binding to unify ns16550 drivers. There are
> several drivers using almost the same code, such as serial_dw,
> serial_keystone, serial_omap, serial_ppc, serial_rockchip,
> serial_tegra.c, and serial_x86. But each is platform specific.
>
> The key difference between these drivers is the way to get
> input clock frequency. With this unified approach, fixed clock
> frequency should be extracted from "clock-frequency" property of
> device tree blob. If this property is not available, the macro
> CONFIG_SYS_NS16550_CLK will be used. It can be a constant or a
> function to get clock, eg, get_serial_clock().
>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
> drivers/serial/Kconfig | 11 +++++++++++
> drivers/serial/ns16550.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+)
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 9476a00..1261356 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -186,6 +186,17 @@ config ROCKCHIP_SERIAL
> your board config header. The clock input is automatically set to
> use the oscillator (24MHz).
>
> +config NS16550_SERIAL
> + bool "NS16550 UART or compatible"
> + depends on DM_SERIAL
> + help
> + Support NS16550 UART or compatible with driver model. This can be
> + enabled in the device tree with the correct input clock frequency.
> + If the input clock frequency is not defined in the device tree,
> + the macro CONFIG_SYS_NS16550_CLK defined in a legacy board header
> + file will be used. It can be a constant or a function to get clock,
> + eg, get_serial_clock().
> +
> config SANDBOX_SERIAL
> bool "Sandbox UART support"
> depends on SANDBOX
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 8d028de..f0a9aac 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -56,6 +56,10 @@ DECLARE_GLOBAL_DATA_PTR;
>
> #ifdef CONFIG_DM_SERIAL
>
> +#ifndef CONFIG_SYS_NS16550_CLK
> +#define CONFIG_SYS_NS16550_CLK 0
> +#endif
> +
> static inline void serial_out_shift(void *addr, int shift, int value)
> {
> #ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> @@ -400,6 +404,15 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
> plat->base = addr;
> plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> "reg-shift", 1);
> +#ifdef CONFIG_NS16550_SERIAL

Is this #ifdef necessary?

> + plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> + "clock-frequency",
> + CONFIG_SYS_NS16550_CLK);
> + if (!plat->clock) {
> + debug("ns16550 clock not defined\n");
> + return -EINVAL;
> + }
> +#endif /* CONFIG_NS16550_SERIAL */
>
> return 0;
> }
> @@ -411,4 +424,35 @@ const struct dm_serial_ops ns16550_serial_ops = {
> .getc = ns16550_serial_getc,
> .setbrg = ns16550_serial_setbrg,
> };
> +
> +#ifdef CONFIG_NS16550_SERIAL

Ditto.

> +#if CONFIG_IS_ENABLED(OF_CONTROL)

This line should be removed too?

> +static const struct udevice_id ns16550_serial_ids[] = {
> + { .compatible = "ns16550" },
> + { .compatible = "ns16550a" },
> + { .compatible = "nvidia,tegra20-uart" },
> + { .compatible = "snps,dw-apb-uart" },
> + { .compatible = "ti,omap2-uart" },
> + { .compatible = "ti,omap3-uart" },
> + { .compatible = "ti,omap4-uart" },
> + { .compatible = "ti,am3352-uart" },
> + { .compatible = "ti,am4372-uart" },
> + { .compatible = "ti,dra742-uart" },
> + {}
> +};
> +#endif
> +
> +U_BOOT_DRIVER(ns16550_serial) = {
> + .name = "ns16550_serial",
> + .id = UCLASS_SERIAL,
> +#if CONFIG_IS_ENABLED(OF_CONTROL)
> + .of_match = ns16550_serial_ids,
> + .ofdata_to_platdata = ns16550_serial_ofdata_to_platdata,
> + .platdata_auto_alloc_size = sizeof(struct ns16550_platdata),
> +#endif
> + .priv_auto_alloc_size = sizeof(struct NS16550),
> + .probe = ns16550_serial_probe,
> + .ops = &ns16550_serial_ops,
> +};
> +#endif /* CONFIG_NS16550_SERIAL */
> #endif /* CONFIG_DM_SERIAL */
> --

Regards,
Bin