Hi Simon,
On 18.07.2016 13:24, Simon Glass wrote:
On 18 July 2016 at 02:17, Stefan Roese sr@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@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@denx.de Cc: Simon Glass sjg@chromium.org Cc: Bin Meng bmeng.cn@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 theinternal
* 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);} }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.
Thanks. Let me check if I can cook up a nicer patch version shortly...
Thanks, Stefan