From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Mon, 18 Jul 2016 13:46:33 +0200 Subject: [U-Boot] [PATCH] serial: ns16550: Add support for the BayTrail internal HS UART In-Reply-To: References: <20160713060336.30386-1-sr@denx.de> <578C9091.7080305@denx.de> Message-ID: <578CC199.9060608@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 18.07.2016 13:24, Simon Glass wrote: > On 18 July 2016 at 02:17, Stefan Roese wrote: >> Hi Bin, >> >> >> On 14.07.2016 02:52, Bin Meng wrote: >>> >>> On Wed, Jul 13, 2016 at 2:03 PM, Stefan Roese 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 >>>> Cc: Simon Glass >>>> Cc: Bin Meng >>>> --- >>>> 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. Thanks. Let me check if I can cook up a nicer patch version shortly... Thanks, Stefan