On Mon, May 19, 2025 at 05:49:12PM +0200, Quentin Schulz wrote:
> On 5/19/25 5:43 PM, Tom Rini wrote:
> > On Mon, May 19, 2025 at 05:09:07PM +0200, Quentin Schulz wrote:
> > > Hi Tom,
> > >
> > > On 5/19/25 4:48 PM, Tom Rini wrote:
> > > > On Thu, May 15, 2025 at 05:55:44PM +0200, Lukasz Czechowski wrote:
> > > > > Initialize the debug uart only in case CONFIG_DEBUG_UART is
> > > > > enabled. The _debug_uart_putc is used internally by debug uart
> > > > > functions, so it must be also included inside #ifdef block,
> > > > > otherwise it will cause compilation warnings.
> > > > >
> > > > > Signed-off-by: Lukasz Czechowski <lukasz.czechow...@thaumatec.com>
> > > > > ---
> > > > > lib/efi/efi_stub.c | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c
> > > > > index 40fc29d9adf7..5172cd78a7c0 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)
> > > > > {
> > > >
> > > > I notice that this, and the uniphier implementations are the only
> > > > non-inline ones. Is the problem we later conflict with your changes,
> > > > perhaps because this wasn't marked inline? Thanks.
> > > >
> > >
> > > https://lore.kernel.org/u-boot/d63d53bd-3f63-430d-b0bc-2379b7bde...@rock-chips.com/
> > >
> > > so essentially
> > >
> > > lib/efi/efi_stub.c:86:13: error: '_debug_uart_putc' defined but not
> > > used [-Werror=unused-function]
> > > 86 | static void _debug_uart_putc(int ch)
> > >
> > > I tested by adding inline and indeed it fixes the warning.
> > >
> > > Somehow the non-inlined _debug_uart_init doesn't seem to be triggering any
> > > warning though.
> > >
> > > Would you recommend to inline the _debug_uart_putc function instead of
> > > guarding the whole thing with ifdefery?
> >
> > Yes please, and also fixing up uniphier for consistency (it won't have
> > the warning because it doesn't descend to that directory unless the
> > option is enabled).
>
> I actually added an #error in the function and it is compiled and doesn't
> show a warning, so not sure what I'm missing?It's on efi-x86_payload32 and efi-x86_payload64 where the warning happens, and in that case inline also fixes the warning. So yes, please lets go with that for consistency, and also to match up include/debug_uart.h says it should be written. -- Tom
signature.asc
Description: PGP signature
