Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Hi Simon, On Fri, 11 Jan 2019 at 03:02, Simon Goldschmidt wrote: > > On Fri, Jan 11, 2019 at 10:22 AM Andy Shevchenko > wrote: > > > > On Wed, Jan 9, 2019 at 10:36 AM Alexey Brodkin > > wrote: > > > > > > > Might be a bit naïve question but why depend on SPL_OF_PLATDATA only? > > > What about CONFIG_OF_EMBED? > > > > No driver should depend on OF_EMBED IIUC. New U-Boot spils a warning > > when production boards are using it. > > By now, actually I even dislike the idea of depending on OF_PLATDATA myself. > Maybe we should fix the OF_PLATDATA to driver matching code to better handle > drivers with multiple compatible strings instead of adding dedicated drivers. Sounds good. One option I rejected with the initial impl was to add tags to the DT to tell dtoc what to do. I still hope to avoid that, but it is an option, if helpful. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
On Fri, Jan 11, 2019 at 10:22 AM Andy Shevchenko wrote: > > On Wed, Jan 9, 2019 at 10:36 AM Alexey Brodkin > wrote: > > > > Might be a bit naïve question but why depend on SPL_OF_PLATDATA only? > > What about CONFIG_OF_EMBED? > > No driver should depend on OF_EMBED IIUC. New U-Boot spils a warning > when production boards are using it. By now, actually I even dislike the idea of depending on OF_PLATDATA myself. Maybe we should fix the OF_PLATDATA to driver matching code to better handle drivers with multiple compatible strings instead of adding dedicated drivers. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
On Fri, Jan 11, 2019 at 10:03 AM Alexey Brodkin
wrote:
>
> Hi Simon,
>
> > -Original Message-
> > From: Simon Goldschmidt [mailto:simon.k.r.goldschm...@gmail.com]
> > Sent: Friday, January 11, 2019 11:41 AM
> > To: Alexey Brodkin
> > Cc: Patrice Chotard ; Simon Glass
> > ; Anup Patel
> > ; Lokesh Vutla ; Patrick Delaunay
> > ;
> > Marek Vasut ; u-boot@lists.denx.de; Álvaro Fernández
> > Rojas ;
> > Ryder Lee ; Vikas Manocha ;
> > Alexander Graf
> > ; Weijie Gao ; Marek Vasut
> >
> > Subject: Re: [PATCH v1 3/4] serial: add an of-platdata driver for
> > "snps,dw-apb-uart"
> >
> > On Fri, Jan 11, 2019 at 9:33 AM Alexey Brodkin
> > 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://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_u-
> > 2Dboot_latest_source_drivers_serial_ns16550.c-
> > 23L456=DwIFaQ=DPL6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I=wyhoU5_3rGv5y
> > 373_cpetmUHK9_ALMCC29QnKS2As5k=uMDuaOZ__YoyqakveKqByAtb1lfRQBYktcVgGH3oawE=
> > >
> > > > > 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>"?
>
> Well actually CONFIG_SYS_NS16550_MEM32 is an alias to "reg-io-width = <4>"
> which selects exactly accessor (right as CONFIG_SYS_NS16550_MEM32 in
> serial_{in|out}_shift()). But even though we do read "reg-io-width"
> from .dtb it is not used in the driver. It was added by Andy Shevchenko
> recently, see
> http://git.denx.de/?p=u-boot.git;a=commit;h=4e7207791c05b3ecff916c1b1b1b21401ec29b0b
>
> BTW as I may see some SoCFPGA configs do mention this define:
> -->8-
> # git grep CONFIG_SYS_NS16550_MEM32 | grep socfpga
> include/configs/socfpga_arria10_socdk.h:33:#define CONFIG_SYS_NS16550_MEM32
> include/configs/socfpga_stratix10_socdk.h:146:#define CONFIG_SYS_NS16550_MEM32
> -->8-
Right. Sorry for being unclear. I'm working on the "sub-mach" "Gen5" only. Those
two are different types that I can't really test. Seems like Arria10
and Stratix10 might
not be converted to DM yet?
>
> as well as this DT property:
> -->8-
> # git grep reg-io-width | grep socfpga
> arch/arm/dts/socfpga.dtsi:877: reg-io-width = <4>;
> arch/arm/dts/socfpga.dtsi:889: reg-io-width = <4>;
Those two are of interest to my boards.
> arch/arm/dts/socfpga_arria10.dtsi:829: reg-io-width = <4>;
> arch/arm/dts/socfpga_arria10.dtsi:840: reg-io-width = <4>;
> arch/arm/dts/socfpga_stratix10.dtsi:252:reg-io-width
> = <4>;
> arch/arm/dts/socfpga_stratix10.dtsi:265:reg-io-width
> = <4>;
> arch/arm/dts/socfpga_stratix10.dtsi:314:reg-io-width
> = <4>;
> arch/arm/dts/socfpga_stratix10.dtsi:326:reg-io-width
> = <4>;
> -->8-
>
> So I guess you may experiment with it a bit.
Ok, you're right there. I guess it just works for me when using 8-bit access,
but you're right that the driver should use 32-bit access when configured like
that via 'reg-io-width'. I'll have a look at that.
Thanks for insisting ;-)
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
On Wed, Jan 9, 2019 at 10:36 AM Alexey Brodkin wrote: > Might be a bit naïve question but why depend on SPL_OF_PLATDATA only? > What about CONFIG_OF_EMBED? No driver should depend on OF_EMBED IIUC. New U-Boot spils a warning when production boards are using it. -- With Best Regards, Andy Shevchenko ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Hi Simon,
> -Original Message-
> From: Simon Goldschmidt [mailto:simon.k.r.goldschm...@gmail.com]
> Sent: Friday, January 11, 2019 11:41 AM
> To: Alexey Brodkin
> Cc: Patrice Chotard ; Simon Glass
> ; Anup Patel
> ; Lokesh Vutla ; Patrick Delaunay
> ;
> Marek Vasut ; u-boot@lists.denx.de; Álvaro Fernández
> Rojas ;
> Ryder Lee ; Vikas Manocha ;
> Alexander Graf
> ; Weijie Gao ; Marek Vasut
>
> Subject: Re: [PATCH v1 3/4] serial: add an of-platdata driver for
> "snps,dw-apb-uart"
>
> On Fri, Jan 11, 2019 at 9:33 AM Alexey Brodkin
> 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://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_u-
> 2Dboot_latest_source_drivers_serial_ns16550.c-
> 23L456=DwIFaQ=DPL6_X_6JkXFx7AXWqB0tg=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I=wyhoU5_3rGv5y
> 373_cpetmUHK9_ALMCC29QnKS2As5k=uMDuaOZ__YoyqakveKqByAtb1lfRQBYktcVgGH3oawE=
> >
> > > > 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>"?
Well actually CONFIG_SYS_NS16550_MEM32 is an alias to "reg-io-width = <4>"
which selects exactly accessor (right as CONFIG_SYS_NS16550_MEM32 in
serial_{in|out}_shift()). But even though we do read "reg-io-width"
from .dtb it is not used in the driver. It was added by Andy Shevchenko
recently, see
http://git.denx.de/?p=u-boot.git;a=commit;h=4e7207791c05b3ecff916c1b1b1b21401ec29b0b
BTW as I may see some SoCFPGA configs do mention this define:
-->8-
# git grep CONFIG_SYS_NS16550_MEM32 | grep socfpga
include/configs/socfpga_arria10_socdk.h:33:#define CONFIG_SYS_NS16550_MEM32
include/configs/socfpga_stratix10_socdk.h:146:#define CONFIG_SYS_NS16550_MEM32
-->8-
as well as this DT property:
-->8-
# git grep reg-io-width | grep socfpga
arch/arm/dts/socfpga.dtsi:877: reg-io-width = <4>;
arch/arm/dts/socfpga.dtsi:889: reg-io-width = <4>;
arch/arm/dts/socfpga_arria10.dtsi:829: reg-io-width = <4>;
arch/arm/dts/socfpga_arria10.dtsi:840: reg-io-width = <4>;
arch/arm/dts/socfpga_stratix10.dtsi:252:reg-io-width =
<4>;
arch/arm/dts/socfpga_stratix10.dtsi:265:reg-io-width =
<4>;
arch/arm/dts/socfpga_stratix10.dtsi:314:reg-io-width =
<4>;
arch/arm/dts/socfpga_stratix10.dtsi:326:reg-io-width =
<4>;
-->8-
So I guess you may experiment with it a bit.
-Alexey
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
On Fri, Jan 11, 2019 at 9:33 AM Alexey Brodkin
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
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.
That said accessors in ns16550 are begging for significant rework to make
the driver completely OF-driven.
-Alexey
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
On Mon, 7 Jan 2019 at 14:14, Simon Goldschmidt wrote: > > 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 > --- > > 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 > Reviewed-by: Simon Glass ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Hi Alexey, Am 09.01.2019 um 09:35 schrieb Alexey Brodkin: Hi Simon, -Original Message- From: Simon Goldschmidt [mailto:simon.k.r.goldschm...@gmail.com] Sent: Tuesday, January 8, 2019 12:14 AM To: Marek Vasut Cc: Simon Goldschmidt ; Patrice Chotard ; Simon Glass ; Anup Patel ; Lokesh Vutla ; Alexey Brodkin ; Patrick Delaunay ; Marek Vasut ; u-boot@lists.denx.de; Álvaro Fernández Rojas ; Ryder Lee ; Vikas Manocha ; Alexander Graf ; Weijie Gao Subject: [PATCH v1 3/4] serial: add an of-platdata driver for "snps,dw-apb-uart" 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 --- 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 Might be a bit naïve question but why depend on SPL_OF_PLATDATA only? What about CONFIG_OF_EMBED? 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 seems it's not a matter of a new driver but a matter of cleaning up the boards to use DM_SERIAL, I guess? Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Hi Alexey, On Wed, Jan 9, 2019 at 9:36 AM Alexey Brodkin wrote: > > Hi Simon, > > > -Original Message- > > From: Simon Goldschmidt [mailto:simon.k.r.goldschm...@gmail.com] > > Sent: Tuesday, January 8, 2019 12:14 AM > > To: Marek Vasut > > Cc: Simon Goldschmidt ; Patrice Chotard > > ; > > Simon Glass ; Anup Patel ; Lokesh > > Vutla ; > > Alexey Brodkin ; Patrick Delaunay > > ; Marek Vasut > > ; u-boot@lists.denx.de; Álvaro Fernández Rojas > > ; Ryder Lee > > ; Vikas Manocha ; Alexander > > Graf ; Weijie > > Gao > > Subject: [PATCH v1 3/4] serial: add an of-platdata driver for > > "snps,dw-apb-uart" > > > > 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 > > --- > > > > 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 > > Might be a bit naïve question but why depend on SPL_OF_PLATDATA only? Because my focus has been to get OF_PLATDATA running on my socfpga board. But since Marek seems to be opposed, I guess I'll drop this series and try to find a different way to squeeze out the bytes I need. > What about CONFIG_OF_EMBED? > > 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 You're right there, this would be a good idea for socfpga as well. Maybe I'll continue that line some day, but I don't have immediate plans for this. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Hi Simon, > -Original Message- > From: Simon Goldschmidt [mailto:simon.k.r.goldschm...@gmail.com] > Sent: Tuesday, January 8, 2019 12:14 AM > To: Marek Vasut > Cc: Simon Goldschmidt ; Patrice Chotard > ; > Simon Glass ; Anup Patel ; Lokesh > Vutla ; > Alexey Brodkin ; Patrick Delaunay > ; Marek Vasut > ; u-boot@lists.denx.de; Álvaro Fernández Rojas > ; Ryder Lee > ; Vikas Manocha ; Alexander > Graf ; Weijie > Gao > Subject: [PATCH v1 3/4] serial: add an of-platdata driver for > "snps,dw-apb-uart" > > 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 > --- > > 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 Might be a bit naïve question but why depend on SPL_OF_PLATDATA only? What about CONFIG_OF_EMBED? 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 -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Hi Simon,
> On Tue, Jan 8, 2019 at 8:30 AM Lukasz Majewski wrote:
> >
> > Hi Simon,
> >
> > > On Mon, Jan 7, 2019 at 11:12 PM Lukasz Majewski
> > > 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
> > > > > ---
> > > > >
> > > > > 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 00..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
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +
> > > > > +#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;
> > > > > +
> > > > > + 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= _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"
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
On Tue, Jan 8, 2019 at 8:30 AM Lukasz Majewski wrote:
>
> Hi Simon,
>
> > On Mon, Jan 7, 2019 at 11:12 PM Lukasz Majewski 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
> > > > ---
> > > >
> > > > 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 00..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
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +#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;
> > > > +
> > > > + 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= _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 :-)
The DTS for socfpga has clock references but at least for the gen5 devices,
these are not used.
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
Hi Simon,
> On Mon, Jan 7, 2019 at 11:12 PM Lukasz Majewski 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
> > > ---
> > >
> > > 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 00..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
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#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;
> > > +
> > > + 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= _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
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
On Mon, Jan 7, 2019 at 11:12 PM Lukasz Majewski 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
> > ---
> >
> > 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 00..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
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#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;
> > +
> > + 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= _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
Re: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
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
> ---
>
> 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 00..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
> +#include
> +#include
> +#include
> +#include
> +
> +#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;
> +
> + 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= _serial_ops,
> + .flags = DM_FLAG_PRE_RELOC,
Is it possible/or is your application requiring the pinmux/clock setting
for this serial?
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.
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
---> pinctrl (to config pins)
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).
> +};
> +#endif
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: lu...@denx.de
pgppVERp0tTP7.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart"
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
---
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 00..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
+#include
+#include
+#include
+#include
+
+#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;
+
+ 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= _serial_ops,
+ .flags = DM_FLAG_PRE_RELOC,
+};
+#endif
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
