Hi Stefan,
On Tue, Jan 19, 2016 at 6:54 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 19.01.2016 11:15, Bin Meng wrote:
On Tue, Jan 19, 2016 at 5:29 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
(added Simon again to Cc)
On 19.01.2016 09:44, Bin Meng wrote:
Hi Stefan,
On Tue, Jan 19, 2016 at 4:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Stefan,
On Mon, Jan 18, 2016 at 5:56 PM, Stefan Roese sr@denx.de wrote:
Some BayTrail boards may want to use a different legacy UART than the internal one. E.g. one provided by a Winbond Super IO chip, like the W83627. This patch adds a function to disable this BayTrail internal UART for this purpose.
Signed-off-by: Stefan Roese sr@denx.de Cc: Bin Meng bmeng.cn@gmail.com Cc: Simon Glass sjg@chromium.org
arch/x86/cpu/baytrail/early_uart.c | 9 +++++++++ arch/x86/include/asm/u-boot-x86.h | 3 +++ 2 files changed, 12 insertions(+)
diff --git a/arch/x86/cpu/baytrail/early_uart.c b/arch/x86/cpu/baytrail/early_uart.c index b64a3a9..716783c 100644 --- a/arch/x86/cpu/baytrail/early_uart.c +++ b/arch/x86/cpu/baytrail/early_uart.c @@ -76,3 +76,12 @@ int setup_early_uart(void)
return 0;}
+int disable_internal_uart(void) +{
/* Disable the legacy UART hardware. */nits: please remove the ending peirod.
x86_pci_write_config32(PCI_DEV_CONFIG(0, LPC_DEV, LPC_FUNC),UART_CONT,
0);return 0;+} diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h index dbf8e95..0c95796 100644 --- a/arch/x86/include/asm/u-boot-x86.h +++ b/arch/x86/include/asm/u-boot-x86.h @@ -47,6 +47,9 @@ int default_print_cpuinfo(void); /* Set up a UART which can be used with printch(), printhex8(), etc. */ int setup_early_uart(void);
+/* Disable the internal legacy UART */ +int disable_internal_uart(void);
If we can call disable_internal_uart() in board-specific codes, then this declaration can be moved to SoC-specific header instead of x86 generic one.
Do you have a preferred header for this in mind?
How about arch/x86/include/asm/arch-baytrail/baytrail.h?
Another idea would be, to add a parameter to the existing function setup_early_uart() to either enable or disable the internal UART:
Like this:
int setup_early_uart(int enable) { /* Enable the legacy UART hardware. */ x86_pci_write_config32(PCI_DEV_CONFIG(0, LPC_DEV, LPC_FUNC), UART_CONT, enable); if (!enable) return 0; ...
What do you think? Should I change it this way?
The issue with setup_early_uart() is that it is only called when CONFIG_DEBUG_UART is on.
In fsp_init(), yes. But I can nevertheless call it from the board specific code in my case to *disable* the internal legacy UART.
Ah, yes! I thought you wanted to use the existing call.
I think CONFIG_DEBUG_UART is only for debug purpose IOW it's still legal to have a U-Boot booting without CONFIG_DEBUG_UART.
Correct. But as mentioned above, I can call it from my board code before the Windond enable function to disable the internal UART (when changed to provide this functionality as suggested in the last mail).
Yes.
Or am I missing something? The function naming would be not really matching its purpose any more. Perhaps I should also rename it to setup_internal_uart() instead?
Yep, makes sense. Let's see how Simon thinks.
Regards, Bin