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?
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?
Thanks, Stefan