pon., 19 maj 2025 o 12:40 Quentin Schulz <quentin.sch...@cherry.de> napisał(a): > > Hi Lukasz, > > On 5/19/25 12:32 PM, Łukasz Czechowski wrote: > > Hi Quentin, > > > > pt., 16 maj 2025 o 17:27 Quentin Schulz <quentin.sch...@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.czechow...@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.
My point was that even if using those functions is technically possible (because as you wrote 'static' is irrelevant here), it was likely not intended by the author. However if you think it's safer to redefine them as well, I can update the patch. > > > 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. Agree on that, thanks. > > Cheers, > Quentin Best regards, Lukasz
