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

[Stefan Roese]

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?

Thanks,
Stefan