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

[Felix Brack]

Hi Tom,

On 14.07.2018 17:49, Tom Rini wrote:
> On Sat, Jul 14, 2018 at 12:47:21PM +0200, Wolfgang Denk wrote:
>> Dear Felix,
>>
>> In message <1531492980-16543-1-git-send-email-fb at ltec.ch> you wrote:
>>>
>>> The motivation for writing this patch originates in the
>>> effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs.
>>> The current am33xx.dtsi file from U-Boot defines the <reg-shift>
>>> property for all UART nodes. The actual (4.18+) am33xx.dtsi
>>> file from Linux does not define <reg-shift> anymore. To prevent
>>> (probably difficult) changes in many .dts and .dtsi files once
>>> the synchronization is done, one can use this new variable. For
>>> the pdu001 board, for example, SYS_NS16550_REG_SHIFT is set
>>> to 2; no need to clutter U-Boot and board specific dts files
>>> with <reg-shift> properties.
>>
>> Does this mean that U-Boot will not be able to use the same DTB as
>> Linux?
> 
> To be clear, it's the other way around.  We can't use the Linux dtb/dts
> files as they've dropped (and in other cases, aren't adding) these
> properties as it's handled differently.
> 
This is exactly the point. These files have diverged quite a lot between
U-Boot and Linux since the last synchronization. I am trying to find a
method to re-synchronize U-Boot generating minimal collateral damage -
this patch is supposed to be part of that effort.

In fact we could even set the default value of SYS_NS16550_REG_SHIFT to
2 for _all_ am33xx SOC boards. That would probably further reduce
possible problems.

>>
>>> +config SYS_NS16550_REG_SHIFT
>>> +	int "Number of bytes to shift register offsets"
>>> +	default 0
>>> +	depends on SYS_NS16550
>>> +	help
>>> +	  Use this to specify the amount of bytes between discrete
>>> +	  device registers. If, for example, the device registers are
>>> +	  located at 0x00, 0x04, 0x08, 0x0C and so forth than set
>>> +	  this to 2. The default value is 0.
>>
>> This description is inconsistent or misleading.  How do you define
>> "space between registers"?  The unused gaps?  In the example, the
>> registers are spaced at 4 bytes intervals, so a value of 2 would
>> only make sense of we have 16 bit registters and you count the gap
>> bytes.
>>
>> But this is a very strange and uncommon way to describe such a
>> situation, especially when you write that you "shift register
>> offsets".  Here I think about something like a LSL operation, so
>> shifing by 2 bits would result in a 2^2 = 4 byte spacing.
>>
>> This needs to be rewritten.
> 
> To try and help clarify, the property in question means "quantity to
> shift the register offsets by."  It should be clear in our Kconfig help
> entry as well that this is what we're looking for.
> 
Thanks for this! The help text will be fixed in v2.

regards Felix