diff mbox

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

Message ID 1447684616-10297-5-git-send-email-thomas@wytron.com.tw
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Thomas Chou Nov. 16, 2015, 2:36 p.m. UTC
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(+)

Comments

Tom Rini Nov. 16, 2015, 6:22 p.m. UTC | #1
On Mon, Nov 16, 2015 at 10:36:48PM +0800, Thomas Chou 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>

Reviewed-by: Tom Rini <trini@konsulko.com>
Bin Meng Nov. 18, 2015, 1:03 a.m. UTC | #2
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
Thomas Chou Nov. 18, 2015, 1:59 a.m. UTC | #3
Hi Bin,

On 2015年11月18日 09:03, Bin Meng wrote:
>> 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.

These #ifdef is needed during the transition. They will be removed as a 
follow-up of this series.

>
>> +#if CONFIG_IS_ENABLED(OF_CONTROL)
>
> This line should be removed too?
>

Some spl build with DM platdata but not of_control. So this is needed.

Best regards,
Thomas
diff mbox

Patch

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
+	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
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+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 */