[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support
Simon Glass
sjg at chromium.org
Wed Jan 6 01:25:25 CET 2016
Hi Bin,
On 31 December 2015 at 01:53, Bin Meng <bmeng.cn at gmail.com> wrote:
> This adds driver model support to lpuart serial driver.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
> doc/driver-model/serial-howto.txt | 1 -
> drivers/serial/serial_lpuart.c | 157 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 157 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass <sjg at chromium.org>
But please see below.
>
> diff --git a/doc/driver-model/serial-howto.txt b/doc/driver-model/serial-howto.txt
> index 76ad629..b933836 100644
> --- a/doc/driver-model/serial-howto.txt
> +++ b/doc/driver-model/serial-howto.txt
> @@ -13,7 +13,6 @@ is time for maintainers to start converting over the remaining serial drivers:
> opencores_yanu.c
> serial_bfin.c
> serial_imx.c
> - serial_lpuart.c
> serial_max3100.c
> serial_pxa.c
> serial_s3c24x0.c
> diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
> index fed83a6..b1ba1bc 100644
> --- a/drivers/serial/serial_lpuart.c
> +++ b/drivers/serial/serial_lpuart.c
> @@ -5,6 +5,7 @@
> */
>
> #include <common.h>
> +#include <dm.h>
> #include <watchdog.h>
> #include <asm/io.h>
> #include <serial.h>
> @@ -49,6 +50,10 @@ DECLARE_GLOBAL_DATA_PTR;
>
> struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
>
> +struct lpuart_serial_platdata {
> + struct lpuart_fsl *reg;
> +};
> +
> #ifndef CONFIG_LPUART_32B_REG
> static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
> {
> @@ -122,6 +127,7 @@ static int _lpuart_serial_init(struct lpuart_fsl *reg)
> return 0;
> }
>
> +#ifndef CONFIG_DM_SERIAL
> static void lpuart_serial_setbrg(void)
> {
> _lpuart_serial_setbrg(base, gd->baudrate);
> @@ -157,6 +163,54 @@ static struct serial_device lpuart_serial_drv = {
> .getc = lpuart_serial_getc,
> .tstc = lpuart_serial_tstc,
> };
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + _lpuart_serial_setbrg(reg, baudrate);
> +
> + return 0;
> +}
> +
> +static int lpuart_serial_getc(struct udevice *dev)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + return _lpuart_serial_getc(reg);
> +}
> +
> +static int lpuart_serial_putc(struct udevice *dev, const char c)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + _lpuart_serial_putc(reg, c);
> +
> + return 0;
> +}
> +
> +static int lpuart_serial_pending(struct udevice *dev, bool input)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + if (input)
> + return _lpuart_serial_tstc(reg);
> + else
> + return __raw_readb(®->us1) & US1_TDRE ? 0 : 1;
> +}
> +
> +static int lpuart_serial_probe(struct udevice *dev)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + return _lpuart_serial_init(reg);
> +}
> +#endif /* CONFIG_DM_SERIAL */
> #else
> static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
> {
> @@ -227,6 +281,7 @@ static int _lpuart32_serial_init(struct lpuart_fsl *reg)
> return 0;
> }
>
> +#ifndef CONFIG_DM_SERIAL
> static void lpuart32_serial_setbrg(void)
> {
> _lpuart32_serial_setbrg(base, gd->baudrate);
> @@ -262,8 +317,57 @@ static struct serial_device lpuart32_serial_drv = {
> .getc = lpuart32_serial_getc,
> .tstc = lpuart32_serial_tstc,
> };
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart32_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + _lpuart32_serial_setbrg(reg, baudrate);
> +
> + return 0;
> +}
> +
> +static int lpuart32_serial_getc(struct udevice *dev)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + return _lpuart32_serial_getc(reg);
> +}
> +
> +static int lpuart32_serial_putc(struct udevice *dev, const char c)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + _lpuart32_serial_putc(reg, c);
> +
> + return 0;
> +}
> +
> +static int lpuart32_serial_pending(struct udevice *dev, bool input)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + if (input)
> + return _lpuart32_serial_tstc(reg);
> + else
> + return in_be32(®->stat) & STAT_TDRE ? 0 : 1;
> +}
> +
> +static int lpuart32_serial_probe(struct udevice *dev)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + struct lpuart_fsl *reg = plat->reg;
> +
> + return _lpuart32_serial_init(reg);
> +}
> +#endif /* CONFIG_DM_SERIAL */
> #endif
>
> +#ifndef CONFIG_DM_SERIAL
> void lpuart_serial_initialize(void)
> {
> #ifdef CONFIG_LPUART_32B_REG
> @@ -281,3 +385,56 @@ __weak struct serial_device *default_serial_console(void)
> return &lpuart_serial_drv;
> #endif
> }
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart_serial_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct lpuart_serial_platdata *plat = dev->platdata;
> + fdt_addr_t addr;
> +
> + addr = dev_get_addr(dev);
> + if (addr == FDT_ADDR_T_NONE)
> + return -EINVAL;
> +
> + plat->reg = (struct lpuart_fsl *)addr;
> +
> + return 0;
> +}
> +
> +static const struct dm_serial_ops lpuart_serial_ops = {
> +#ifdef CONFIG_LPUART_32B_REG
> + .putc = lpuart32_serial_putc,
> + .pending = lpuart32_serial_pending,
> + .getc = lpuart32_serial_getc,
> + .setbrg = lpuart32_serial_setbrg,
> +#else
> + .putc = lpuart_serial_putc,
> + .pending = lpuart_serial_pending,
> + .getc = lpuart_serial_getc,
> + .setbrg = lpuart_serial_setbrg,
> +#endif
> +};
> +
> +static const struct udevice_id lpuart_serial_ids[] = {
> +#ifdef CONFIG_LPUART_32B_REG
> + { .compatible = "fsl,ls1021a-lpuart" },
> +#else
> + { .compatible = "fsl,vf610-lpuart" },
> +#endif
Shouldn't we have both of these in there, with a .data member to
distinguish between them? Or two separate U_BOOT_DRIVER() entries each
with their own operations.
> + { }
> +};
> +
> +U_BOOT_DRIVER(serial_lpuart) = {
> + .name = "serial_lpuart",
> + .id = UCLASS_SERIAL,
> + .of_match = lpuart_serial_ids,
> + .ofdata_to_platdata = lpuart_serial_ofdata_to_platdata,
> + .platdata_auto_alloc_size = sizeof(struct lpuart_serial_platdata),
> +#ifdef CONFIG_LPUART_32B_REG
> + .probe = lpuart32_serial_probe,
> +#else
> + .probe = lpuart_serial_probe,
> +#endif
> + .ops = &lpuart_serial_ops,
> + .flags = DM_FLAG_PRE_RELOC,
> +};
> +#endif /* CONFIG_DM_SERIAL */
> --
> 1.8.2.1
>
Regards,
Simon
More information about the U-Boot
mailing list