[U-Boot] [PATCH v2] serial: ns16550: Add register shift variable

[Tom Rini]

On Tue, Jul 17, 2018 at 03:38:17PM +0200, Alexander Graf wrote:
> On 07/17/2018 03:34 PM, Felix Brack wrote:
> >
> >On 17.07.2018 15:21, Tom Rini wrote:
> >>On Tue, Jul 17, 2018 at 12:45:51PM +0000, Alexey Brodkin wrote:
> >>>Hi Felix,
> >>>
> >>>>-----Original Message-----
> >>>>From: Felix Brack [mailto:fb at ltec.ch]
> >>>>Sent: Tuesday, July 17, 2018 3:13 PM
> >>>>To: Alexander Graf <agraf at suse.de>; Lokesh Vutla <lokeshvutla at ti.com>; u-boot at lists.denx.de
> >>>>Cc: Wolfgang Denk <wd at denx.de>; Tom Rini <trini at konsulko.com>; Marek Vasut <marek.vasut at gmail.com>; Patrice Chotard
> >>>><patrice.chotard at st.com>; Michal Simek <michal.simek at xilinx.com>; Simon Glass <sjg at chromium.org>; Alexey Brodkin
> >>>><Alexey.Brodkin at synopsys.com>; Bin Meng <bmeng.cn at gmail.com>; Ley Foon Tan <ley.foon.tan at intel.com>; Patrick Delaunay
> >>>><patrick.delaunay at st.com>; Mario Six <mario.six at gdsys.cc>; Stefan Roese <sr at denx.de>; Bernhard Messerklinger
> >>>><bernhard.messerklinger at br-automation.com>
> >>>>Subject: Re: [PATCH v2] serial: ns16550: Add register shift variable
> >>>[snip]
> >>>>Adding a separate PORT in ns16550_serial_ids for a particular
> >>>>architecture, platform or SoC would be an option. However the patch I
> >>>>posted is much more generic as it offers to set the reg-shift property
> >>>>for no matter what architecture, platform or SoC. It can also easily be
> >>>>extended by adding more conditional defaults to the Kconfig file.
> >>>I'd say we're dealing with just one corner-case here.
> >>>If I understand a concept of Device Tree it is supposed to describe your
> >>>hardware. Thus if reg shift exists in your HW it should be explicitly mentioned in
> >>>your .dts. If for some [historical] reason you have to deal with "incorrect" .dts then
> >>>I'd prefer to have mentioned quirk with a separate PORT in ns16550_serial_ids
> >>>instead of adding yet another Kconfig option.
> >>So, this is part of the problem I suppose.  I don't know _why_ we can't
> >>just add the correct and valid reg-shift property to the dtsi file in
> >>Linux and be done with it.  Then the U-Boot driver would work because we
> >>parse that property.
> >>
> >The only reason I can see why the <reg-shift> property "can't be added"
> >to the Linux .dtsi file is that there is nothing broken in Linux. Hence
> >we would actually ask Linux to add a property required by U-Boot.
> 
> In the DT you can describe hardware specifics by either a different
> compatible string or by additional properties on a generic compatible. So
> 
>   compatible = "ti,omap3-uart";
> 
> is correct, as is
> 
>   compatible = "ns16550";
>   reg-shift = 2;
> 
> There might be more that gets implied by the omap3-uart compatible that I'm
> not aware of, but in a nutshell it's all about whether you use a generic
> compatible string or a device specific one. Linux went for the specific one,
> so it didn't need reg-shift. U-Boot (incorrectly) treats the device specific
> compatible string as generic which is why you see the failure.

So, to answer my own questions, drivers/tty/serial/8250/8250_omap.c
takes the compatible and forces the reg-shift.  Honestly I assume they
do this because there's other things being handled in those SoC-specific
files in there.  I guess we need to follow suit, sigh.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180717/1bc01fda/attachment.sig>