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

[Felix Brack]

On 17.07.2018 11:27, Alexander Graf wrote:
> On 07/17/2018 11:21 AM, Lokesh Vutla wrote:
>>
>> On Tuesday 17 July 2018 02:42 PM, Alexander Graf wrote:
>>> On 07/17/2018 09:55 AM, Lokesh Vutla wrote:
>>>> On Tuesday 17 July 2018 12:32 PM, Felix Brack wrote:
>>>>> This patch adds a new Kconfig variable that allows setting
>>>>> the register offset shift value for the ns16550 driver to some
>>>>> other value then 0 if not defined by the DT. All credit for this
>>>>> patch goes to Lokesh Vutla as it was his idea.
>>>>>
>>>>> 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.
>>>>>
>>>>> Signed-off-by: Felix Brack <fb at ltec.ch>
>>> NAK. Please determine the shift value from the compatible instead.
>>> Ideally you would create a subclass of the ns16550 device class and set
>>> reg_shift in there.
>> There was a separate driver for omap_serial initially but was deleted by
>> the below commit:
>>
>> commit c7b9686d5d482c8e952598841ea467e6ec0ec0de
>> Author: Thomas Chou <thomas at wytron.com.tw>
>> Date:   Thu Nov 19 21:48:12 2015 +0800
>>
>>      ns16550: unify serial_omap
>>
>>      Unify serial_omap, and use the generic binding.
>>
>>      Signed-off-by: Thomas Chou <thomas at wytron.com.tw>
>>      Reviewed-by: Tom Rini <trini at konsulko.com>
>>      Acked-by: Simon Glass <sjg at chromium.org>
> 
> Sounds like that wasn't a terribly smart move :).
> 
> If you really don't want a separate driver (and I'm not sure why you
> wouldn't), declare a separate PORT for ti,omap3-uart in
> ns16550_serial_ids and set the shift value based on that in
> ns16550_serial_ofdata_to_platdata() if DT doesn't describe one.
> 
Adding a separate driver when you can use a generic one is of no
benefit, it just bloats the source code.

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.

regards Felix