[U-Boot] [PATCH] serial: ns16550: Add support for the BayTrail internal HS UART

[Simon Glass]

Hi,

On 18 July 2016 at 02:17, Stefan Roese <sr at denx.de> wrote:
> Hi Bin,
>
>
> On 14.07.2016 02:52, Bin Meng wrote:
>>
>> On Wed, Jul 13, 2016 at 2:03 PM, Stefan Roese <sr at denx.de> wrote:
>>>
>>> To support the BayTrail internal SIO HS UART, the internal UART clock
>>> needs to get configured. This patch adds support for this clock
>>> configuration which will be done, if a compatible DT node is found.
>>>
>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>> Cc: Simon Glass <sjg at chromium.org>
>>> Cc: Bin Meng <bmeng.cn at gmail.com>
>>> ---
>>>   drivers/serial/ns16550.c | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>>> index c6cb3eb..12499ec 100644
>>> --- a/drivers/serial/ns16550.c
>>> +++ b/drivers/serial/ns16550.c
>>> @@ -94,6 +94,13 @@ static inline int serial_in_shift(void *addr, int
>>> shift)
>>>   #define CONFIG_SYS_NS16550_CLK  0
>>>   #endif
>>>
>>> +/* Intel BayTrail defines */
>>> +#define BYT_PRV_CLK                    0x800
>>> +#define BYT_PRV_CLK_EN                 (1 << 0)
>>> +#define BYT_PRV_CLK_M_VAL_SHIFT                1
>>> +#define BYT_PRV_CLK_N_VAL_SHIFT                16
>>> +#define BYT_PRV_CLK_UPDATE             (1 << 31)
>>> +
>>>   static void ns16550_writeb(NS16550_t port, int offset, int value)
>>>   {
>>>          struct ns16550_platdata *plat = port->plat;
>>> @@ -381,6 +388,25 @@ int ns16550_serial_ofdata_to_platdata(struct udevice
>>> *dev)
>>>                          return ret;
>>>
>>>                  addr = bar;
>>> +
>>> +               /* Setup BayTrail UART clock */
>>> +               ret = fdt_node_check_compatible(gd->fdt_blob,
>>> dev->of_offset,
>>> +                                               "baytrail-hs-uart");
>>> +               if (ret == 0) {
>>> +                       u32 m, n, reg;
>>> +
>>> +                       /*
>>> +                        * Configure the BayTrail UART clock for the
>>> internal
>>> +                        * HS UARTs (PCI devices) to 58982400 Hz
>>> +                        */
>>> +                       m = 0x2400;
>>> +                       n = 0x3d09;
>>> +                       reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) |
>>> +                               (n << BYT_PRV_CLK_N_VAL_SHIFT);
>>> +                       writel(reg, bar + BYT_PRV_CLK);
>>> +                       reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE;
>>> +                       writel(reg, bar + BYT_PRV_CLK);
>>> +               }
>>>          }
>>>   #endif
>>
>>
>> I prefer moving these clock setup codes to the SoC codes
>> (arch/x86/cpu/baytrail/). Is that possible?
>
>
> It is possible, but I'm not so sure if this is "better". First, I can't
> find a file in arch/x86/cpu/baytrail/ that is really fitting for such
> a UART clock setup. Sure, I could create a new one, but I'm not so sure
> if its worth it. Also, in the Linux 8250 serial driver, the clock setup
> is also done in this driver. It would be cleaner to move this clock
> setup into a separate function in this file. But this would mean
> additional #ifdef's for this function, which I'm hesitant to add.
>
> So what do you think? Can this code stay in this file? Or where
> exactly do you want me to move it into?

Perhaps arch_cpu_init_dm()? That's where ivybridge puts it. This
function is called immediately after driver model is inited.

Regards,
Simon