From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Brodkin Date: Fri, 11 Jan 2019 09:03:33 +0000 Subject: [U-Boot] [PATCH v1 3/4] serial: add an of-platdata driver for "snps, dw-apb-uart" In-Reply-To: References: <20190107211423.10151-1-simon.k.r.goldschmidt@gmail.com> <20190107211423.10151-4-simon.k.r.goldschmidt@gmail.com> <4881796E12491D4BB15146FE0209CE64681C1E9A@DE02WEMBXB.internal.synopsys.com> <02cacbce-5906-a2ae-3559-f0e0a22391b7@gmail.com> <4881796E12491D4BB15146FE0209CE64681C361D@DE02WEMBXB.internal.synopsys.com> Message-ID: <4881796E12491D4BB15146FE0209CE64681C3697@DE02WEMBXB.internal.synopsys.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi Simon, > -----Original Message----- > From: Simon Goldschmidt [mailto:simon.k.r.goldschmidt at 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 at 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&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=wyhoU5_3rGv5y > 373_cpetmUHK9_ALMCC29QnKS2As5k&s=uMDuaOZ__YoyqakveKqByAtb1lfRQBYktcVgGH3oawE&e= > > > > > > 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