Hi Bin,

On 19.01.2016 12:02, Bin Meng wrote:
Hi Stefan,

On Tue, Jan 19, 2016 at 6:54 PM, Stefan Roese <s...@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 <s...@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...@gmail.com> wrote:

Hi Stefan,

On Mon, Jan 18, 2016 at 5:56 PM, Stefan Roese <s...@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 <s...@denx.de>
Cc: Bin Meng <bmeng...@gmail.com>
Cc: Simon Glass <s...@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.

I've just created a new patchset, with 2 patches now and the
suggested change from above implemented.

Thanks,
Stefan

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to