[U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Lukasz Majewski
lukma at denx.de
Tue Jan 8 07:30:45 UTC 2019
Hi Simon,
> 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.
Just to be clear here - the FPGA SDK (Quartus ?) generates clock info
in different way than "normal" SoCs and it cannot use "clocks" and
"clocks-names" properties to define needed clocks?
>
> This might be valid for socfpga only, and this driver migth be used
> on other boards as well.
I think yes. IMHO the goal here is to reuse DM versions of drivers in
the SPL (as non DM ones will be removed finally).
> 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...
This is my point - the OF_PLATDATA allows using the DM ready driver as
the standalone (no DTS parsing nor appending) entity in SPL.
The problem (at least for iMX6) is the dependency on pinmux (which
properties by default are stripped from SPL's version of DTB).
I could hack it with having relevant pins defined as "hog" group,
which would be configured when pinctrl driver is probed in SPL. However,
this would prevent me from re-using kernel DTS out of the box.
>
> > 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.
Thanks for the info. I will look into that.
>
> Regards,
> Simon
>
> >
> > > +};
> > > +#endif
> >
> >
> >
> >
> > Best regards,
> >
> > Lukasz Majewski
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190108/54451572/attachment.sig>
More information about the U-Boot
mailing list