Re: [PATCH v3 1/4] debug_uart: Replace debug functions with dummies if CONFIG_DEBUG_UART is not set

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Łukasz,

On 1/17/25 5:18 PM, Quentin Schulz wrote:
> Hi Łukasz, Kever,
> 
> On 11/27/24 11:21 PM, Łukasz Czechowski wrote:
>> Hi Kever,
>>
>> On 2024/11/27 03:42 Kever Yang <kever.yang@rock-chips.com> wrote:
>>>
>>> Hi Lukasz,
>>>
>>>       I got a new error base on patch [1], see full log here [2].
>>
>> Looking at the file efi_stub.c that is used in the failing
>> configuration it looks to me
>> that some functions from debug_uart.h are used here for convenience
>> (i.e. printhex),
>> even though the DEBUG_UART is not used. This is contrary to my 
>> assumption that
>> DEBUG_UART_FUNCS macro should not be used if DEBUG UART is not available.
>> In order to apply the patch then, either the efi_stub code must be
>> updated to not
>> use debug functions or the assumption must be relaxed - I'm not yet
>> sure what shall be
>> the correct approach.
>>
> 
> """
> diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c
> index 40fc29d9adf..5172cd78a7c 100644
> --- a/lib/efi/efi_stub.c
> +++ b/lib/efi/efi_stub.c
> @@ -83,12 +83,14 @@ void puts(const char *str)
>           putc(*str++);
>   }
> 
> +#ifdef CONFIG_DEBUG_UART
>   static void _debug_uart_putc(int ch)
>   {
>       putc(ch);
>   }
> 
>   DEBUG_UART_FUNCS
> +#endif
> 
>   void *memcpy(void *dest, const void *src, size_t size)
>   {
> """
> 
> Fixes it. It builds fine with and without CONFIG_DEBUG_UART set. Not 
> sure if it's proper but I guess that's fine?
> 
> While reading the patch again, I think we made a small oversight.
> 
> #define printhex8(value) do{}while(0);
> 
> is an issue because ch may actually have side effects.
> 
> Take for example an interrupt register where reading is acknowledging, 
> if one does:
> 
> printhex8(readl(INT_REG))
> 
> will read the register when CONFIG_DEBUG_UART is set, but not when it's 
> not.
> 
> I am not sure if and how we can handle that so that things are executed 
> but only print is not called. Is this something we should care about?
> 

Can you send a v4 that fixes that? I'm planning on having a release on 
the upcoming v2025.07 for our devices, so maybe you'll want to rebase 
for your client's product too? I'm pretty sure your client will not be 
the only one who doesn't want anything on UART in U-Boot on Rockchip :)

Cheers,
Quentin