[U-Boot] [PATCH 2/3] x86: BayTrail: Add function to disable the internal legacy UART

[Bin Meng]

Hi Stefan,

On Tue, Jan 19, 2016 at 5:29 PM, Stefan Roese <sr at 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 at gmail.com> wrote:
>>> Hi Stefan,
>>>
>>> On Mon, Jan 18, 2016 at 5:56 PM, Stefan Roese <sr at 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 at denx.de>
>>>> Cc: Bin Meng <bmeng.cn at gmail.com>
>>>> Cc: Simon Glass <sjg at 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. 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.

Regards,
Bin