[U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"

Tue Jan 8 06:06:04 UTC 2019

On Mon, Jan 7, 2019 at 11:12 PM Lukasz Majewski <lukma at denx.de> wrote:
>
> Hi Simon,
>
> > Add a driver for the "snps,dw-apb-uart" used in socfpga and others.
> >
> > This driver is required to get OF_PLATDATA to work for socfpga.
> > It uses the ns16550 driver, converting the platdata from of-platdata
> > go the ns16550 format.
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > ---
> >
> >  drivers/serial/Kconfig         | 10 ++++++++
> >  drivers/serial/Makefile        |  1 +
> >  drivers/serial/serial_dw_apb.c | 42
> > ++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+)
> >  create mode 100644 drivers/serial/serial_dw_apb.c
> >
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > index b7ff2960ab..10addd3309 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -511,6 +511,16 @@ config BCM283X_PL011_SERIAL
> >         that supports automatic disable, so that it only gets used
> > when the UART is actually muxed.
> >
> > +config DESIGNWARE_SERIAL
> > +     bool "DesignWare UART support"
> > +     depends on DM_SERIAL && SPL_OF_PLATDATA
> > +     help
> > +       Select this to enable a UART driver for "snps,dw-apb-uart"
> > devices
> > +       when using CONFIG_SPL_OF_PLATDATA (i.e. a compiled-in
> > device tree
> > +       replacement).
> > +       This uses the ns16550 driver, converting the platdata from
> > of-platdata
> > +       to the ns16550 format.
> > +
> >  config BCM6345_SERIAL
> >       bool "Support for BCM6345 UART"
> >       depends on DM_SERIAL
> > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> > index 06ee30697d..f1ebb90d92 100644
> > --- a/drivers/serial/Makefile
> > +++ b/drivers/serial/Makefile
> > @@ -46,6 +46,7 @@ obj-$(CONFIG_MESON_SERIAL) += serial_meson.o
> >  obj-$(CONFIG_INTEL_MID_SERIAL) += serial_intel_mid.o
> >  ifdef CONFIG_SPL_BUILD
> >  obj-$(CONFIG_ROCKCHIP_SERIAL) += serial_rockchip.o
> > +obj-$(CONFIG_DESIGNWARE_SERIAL) += serial_dw_apb.o
> >  endif
> >  obj-$(CONFIG_XILINX_UARTLITE) += serial_xuartlite.o
> >  obj-$(CONFIG_SANDBOX_SERIAL) += sandbox.o
> > diff --git a/drivers/serial/serial_dw_apb.c
> > b/drivers/serial/serial_dw_apb.c new file mode 100644
> > index 0000000000..26580e5ba5
> > --- /dev/null
> > +++ b/drivers/serial/serial_dw_apb.c
> > @@ -0,0 +1,42 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2018 Simon Goldschmidt
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <dt-structs.h>
> > +#include <ns16550.h>
> > +#include <serial.h>
> > +
> > +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > +struct dw_apb_uart_platdata {
> > +     struct dtd_snps_dw_apb_uart dtplat;
> > +     struct ns16550_platdata plat;
> > +};
> > +
> > +static int dw_apb_serial_probe(struct udevice *dev)
> > +{
> > +     struct dw_apb_uart_platdata *plat = dev_get_platdata(dev);
> > +
> > +     /* Set up correct platform data for the standard driver */
> > +     plat->plat.base = plat->dtplat.reg[0];
> > +     plat->plat.reg_shift = plat->dtplat.reg_shift;
> > +     plat->plat.reg_width = plat->dtplat.reg_io_width;
> > +     plat->plat.clock = plat->dtplat.clock_frequency;
> > +     plat->plat.fcr = UART_FCR_DEFVAL;
> > +     dev->platdata = &plat->plat;
> > +
> > +     return ns16550_serial_probe(dev);
> > +}
> > +
> > +U_BOOT_DRIVER(snps_dw_apb_uart) = {
> > +     .name   = "snps_dw_apb_uart",
> > +     .id     = UCLASS_SERIAL,
> > +     .priv_auto_alloc_size = sizeof(struct NS16550),
> > +     .platdata_auto_alloc_size = sizeof(struct
> > dw_apb_uart_platdata),
> > +     .probe  = dw_apb_serial_probe,
> > +     .ops    = &ns16550_serial_ops,
> > +     .flags  = DM_FLAG_PRE_RELOC,
>
> Is it possible/or is your application requiring the pinmux/clock setting
> for this serial?

No, the socfpga gen5 platform is somewhat of a real bad example regarding
pinmux and clocks. These are implemented completely separate, based on
files generated by the FPGA toolchain...

And as you can see above, the clock must be encoded into the DTS (and
platdata) instead of calculating it via clock references.

This might be valid for socfpga only, and this driver migth be used on other
boards as well. But I think it wouldn't hurt to start it like this and continue
improving it once it is used on other platforms - it has to be actively enabled
and is only used for SPL with OF_PLATDATA.

> With OF_PLATDATA we don't need to parse (and provide) the DTS (and we
> use the DM/DTS driver's code in a way similar to "native" one).
>
> I'm wondering, if it would be possible to "mimic" the dependencies
> between DM drivers without DTS parsing / providing overhead.

I'm actually working on something like that as I want to use QSPI with of-
platdata and there I need the parent/child connection between the SPI
master and the spi-flash...

> At least in my case I would need in SPL:
>
> -> driver XXX (working with u-boot proper's DM/DTS)
>         ---> CLK (uclass-clk) to enable clocks

Clock binding might already work with dtoc, but I haven't tried.

>         ---> pinctrl (to config pins)

As I've written above, sadly socfpga does not use pinctrl.

>
> As an example: uart or eMMC.
>
> I'm aware that it would be possible to use the old "approach" to
> configure pinmux and clk separately (as it is done now) and only
> re-use the DM portion of driver with OF_PLATDATA having all the
> necessary info.
>
> However, I'm wondering if there is a better way to achieve it (or if I
> misunderstood something).

There might be a better way, but porting socfpga to pinctrl seems
nearly unpossible without further documentation from Intel. Using CLK
might work but needs to be done...

In fact this UART is inspired by Simon's Rockchip UART for of-platdata,
which behaves about the same but uses different compatible strings.

Regards,
Simon

>
> > +};
> > +#endif
>
>
>
>
> Best regards,
>
> Lukasz Majewski