[PATCH 1/2] efi_stub: Sync the debug UART definition like other platform

[Kever Yang]

Hi Heinrich,

On 2025/1/3 23:43, Heinrich Schuchardt wrote:
> On 28.11.24 04:47, Kever Yang wrote:
>> The debug UART interface is available when CONFIG_DEBUG_UART is defined,
>> sync with the other platforms to use the same definition.
>>
>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>> ---
>>
>>   lib/efi/efi_stub.c | 16 ++++++++++------
>>   1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c
>> index 40fc29d9adf..5179c5b2c09 100644
>> --- a/lib/efi/efi_stub.c
>> +++ b/lib/efi/efi_stub.c
>> @@ -9,7 +9,6 @@
>>    * EFI application. It can be built either in 32-bit or 64-bit mode.
>>    */
>>
>> -#include <debug_uart.h>
>>   #include <efi.h>
>>   #include <efi_api.h>
>>   #include <errno.h>
>> @@ -55,10 +54,6 @@ struct __packed desctab_info {
>>    * considering if we start needing more U-Boot functionality. Note 
>> that we
>>    * could then move get_codeseg32() to arch/x86/cpu/cpu.c.
>>    */
>> -void _debug_uart_init(void)
>> -{
>> -}
>> -
>>   void putc(const char ch)
>>   {
>>       struct efi_priv *priv = efi_get_priv();
>> @@ -83,12 +78,21 @@ void puts(const char *str)
>>           putc(*str++);
>>   }
>>
>> -static void _debug_uart_putc(int ch)
>> +#ifdef CONFIG_DEBUG_UART
>
> Why do we need this #ifdef? In other places we leave it to the linker to
> remove unused functions.

I think you are right for most of modules, but for this DEBUG_UART, it's 
a little different

because it's for early debug, and not so stand alone to disable.

And now we are try to make it clean and able to disable, see patch[1].

>
>> +
>> +#include <debug_uart.h>
>
> You cannot consume function _debug_uart_putc() before defining it.
>
This is following what all other serial driver have done.

And I think put all the code support for DEBUG_UART together is reasonable.

This efi_stub is a little bit different and cause the DEBUG_UART not 
able to disable, that's

why I'm doing this "sync" commit.


Thanks,

- Kever

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20240918130155.299353-2-lukasz.czechowski@thaumatec.com/

> So either the #include statement must follow the implementations or you
> have to define the function prototypes in the include.
>
> I would prefer adding the missing definitions to the include and leaving
> it at the top of the code.
>
> Best regards
>
> Heinrich
>
>> +
>> +void _debug_uart_init(void)
>> +{
>> +}
>> +
>> +static inline void _debug_uart_putc(int ch)
>>   {
>>       putc(ch);
>>   }
>>
>>   DEBUG_UART_FUNCS
>> +#endif
>>
>>   void *memcpy(void *dest, const void *src, size_t size)
>>   {
>
>