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

[Simon Goldschmidt]

On Fri, Jan 11, 2019 at 9:33 AM Alexey Brodkin
<alexey.brodkin at synopsys.com> wrote:
>
> Hi Simon,
>
> [snip]
>
> > >> +config DESIGNWARE_SERIAL
> > >> +  bool "DesignWare UART support"
> > >> +  depends on DM_SERIAL && SPL_OF_PLATDATA
> > >
> > > Might be a bit naïve question but why depend on SPL_OF_PLATDATA only?
> > > What about CONFIG_OF_EMBED?
>
> Ok I completely forgot that standard ns16550 driver already covers DW APB UART,
> see https://elixir.bootlin.com/u-boot/latest/source/drivers/serial/ns16550.c#L456
>
> > > I'd happily switch my ARC boards on this driver and get rid of all
> > > CONFIG_SYS_NS16550_xxx nonsense in include/configs/myboardname.h
> >
> > I checked include/configs/socfpga_common.h again and by its using
> > Kconfig and DM_SERIAL, there are no CONFIG_SYS_NS16550_xxx defines left
> > (other than CONFIG_SYS_NS16550_SERIAL, which I will remove).
>
> So it's not that easy apparently :)
> At least CONFIG_SYS_NS16550_MEM32 is still used really required
> for getting correct accessor used, see implementation of
> serial_{in|out}_shift() in drivers/serial/ns16550.c.
>
> If CONFIG_SYS_NS16550_MEM32 is not set then simple readb() is used so
> that 8-bit data offset in 32-bit word is lost and we're dead in the water.

Isn't that covered by 'plat->reg_shift' which is read from dts property
"reg-shift = <2>"?

Remember that CONFIG_SYS_NS16550_REG_SIZE is not used/required
for 32-bit socfpga as well!

> That said accessors in ns16550 are begging for significant rework to make
> the driver completely OF-driven.

I think much of the code in ns16550.c could be removed if non-DM support
was dropped. That might clean things up as well.

Regards,
Simon