[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 08:49:23 UTC 2019
Hi Simon,
> On Tue, Jan 8, 2019 at 8:30 AM Lukasz Majewski <lukma at denx.de> wrote:
> >
> > 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?
>
> I'm afraid I don't know what the "normal" SoC way is in Linux. Coming
> from smaller embedded systems like STM32, what Quartus generates here
> is not uncommon :-)
Only Intel (ALtera) knows what Quartus generates (to some extend :-) ).
>
> The DTS for socfpga has clock references but at least for the gen5
> devices, these are not used.
Ok. I see.
>
> Anyway, how do "normal" SoCs select the target speed of IP cores? The
> DTS only describes what can be done. I.e. how do I tell U-Boot that I
> want the CPU at say, 400 MHz, not 800 MHz? (This question of course
> applies to peripherals as well).
In case of IMX6 there is a set of "clock" functions in
mach-imx/mx6/clock.c
Those functions are called by board code/drivers.
In Linux the CLK hierarchy is modelled in a more verbose way and
describes all the needed MUXes, PLLs, Gates, etc.
However, IMHO we don't need such verbose description in U-boot as at
most we need to increase the ROM default clock and enable it (to
improve boot time).
>
> > >
> > > 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).
>
> Well, this *is* a DM serial driver.
Yes. That was my point to reuse this DM code in SPL.
> It is just lacking clock and
> pinctrl references
> for some platforms...
And this is problematic to have full DM reuse in SPL with reasonable
size.
In one of IMX6Q boards the SPL grown from 35 KiB to 51 KiB when the
DM/DTS conversion has been done (without OF_PLATDATA, tiny
printf/malloc).
>
> >
> > > 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).
>
> You need to stop it stripping that, of course ;-)
Yes :-).
>
> I'd like that for socfpga as well. Unfortunately, there is no
> documentation of the pinmux configuration for these Intel FPGAs and
> Quartus only generates 4 big u32 arrays :-(
Binary blob is the best way to configure things :-) -> Then just use
single "for" and everything just works :-).
>
> >
> > 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.
>
> It's actually pretty much the same like this, only the names change.
> That might be worth combining, but the device/driver binding code
> would need to allow checking for multiple compatible strings.
Ok. I see.
>
> Regards,
> Simon
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/4a29706f/attachment.sig>
More information about the U-Boot
mailing list