[PATCH v4 2/5] debug_uart: Replace debug functions with dummies if CONFIG_DEBUG_UART is not set

[Quentin Schulz]

Hi Lukasz,

On 5/19/25 12:32 PM, Łukasz Czechowski wrote:
> Hi Quentin,
> 
> pt., 16 maj 2025 o 17:27 Quentin Schulz <quentin.schulz at cherry.de> napisał(a):
>>
>> Hi Lukasz,
>>
>> On 5/15/25 5:55 PM, Lukasz Czechowski wrote:
>>> In case DEBUG UART is not used, define dummy macros replacing
>>> the actual function implementations that will not be available.
>>> This allows to compile code and avoid linker errors.
>>> Redefine the DEBUG_UART_FUNCS macro if DEBUG UART is not available,
>>> to avoid compilation errors.
>>>
>>> Signed-off-by: Lukasz Czechowski <lukasz.czechowski at thaumatec.com>
>>> ---
>>>    include/debug_uart.h | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/include/debug_uart.h b/include/debug_uart.h
>>> index 714b369e6fed..4526dca24ac6 100644
>>> --- a/include/debug_uart.h
>>> +++ b/include/debug_uart.h
>>> @@ -128,6 +128,8 @@ void printdec(unsigned int value);
>>>                        (1 << CONFIG_DEBUG_UART_SHIFT), \
>>>                CONFIG_DEBUG_UART_SHIFT)
>>>
>>> +#ifdef CONFIG_DEBUG_UART
>>> +
>>>    /*
>>>     * Now define some functions - this should be inserted into the serial driver
>>>     */
>>> @@ -197,4 +199,17 @@ void printdec(unsigned int value);
>>>                _DEBUG_UART_ANNOUNCE \
>>>        } \
>>>
>>> +#else
>>> +
>>> +#define DEBUG_UART_FUNCS
>>> +
>>> +#define printch(ch) (void)(ch)
>>> +#define printascii(str) (void)(str)
>>> +#define printhex2(value) (void)(value)
>>> +#define printhex4(value) (void)(value)
>>> +#define printhex8(value) (void)(value)
>>> +#define printdec(value) (void)(value)
>>> +
>>
>> There are a few additional functions that aren't redefined here:
>>
>> _printch
>> printhex1
>> printhex
>> debug_uart_init
>>
>> any particular reason for not having included them?
> 
> Looking at the original code, _printch, printhex1 and printhex functions
> seem to be only used internally by other print functions, hence they are
> static (there are also no forward declarations of them in linux_uart,h,
> and I haven't seen any usage in the U-Boot code).
> 

If I haven't misunderstood how C header works, I believe the fact that 
the functions are static and used only in functions inside the header is 
irrelevant. A static function in a header will still be available in the 
C files that include it, even if there are no user of the function in 
the header file. I believe this means anyone who includes that header 
file and use one of those functions will have a build failure when 
deselecting CONFIG_DEBUG_UART.

> The debug_uart_init function calls are, in most cases, guarded by
> CONFIG_DEBUG_UART, so it didn't trigger compilation errors for me.
> Do you think it's worth redefining it for consistency?
> 

I think it makes sense, maybe Tom or Simon have another opinion on that.

Cheers,
Quentin