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

[Stefan Roese <sr@denx.de>]

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