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

[Łukasz Czechowski]

Hi,

On 2024/10/25 14:30, Kever Yang wrote:
>
> Hi Tom,
>
>      This is regression of "#ifdef CONFIG", is it possible for us to go
> back to use "#ifdef CONFIG" in this case?
>
> Or do you have any other suggestion for this issue?
>
> On 2024/9/30 16:55, Quentin Schulz wrote:
> > Hi Kever,
> >
> > On 9/29/24 3:53 AM, Kever Yang wrote:
> >> Hi Lukasz,
> >>
> >>      I think this will make the error happen like this:
> >>
> >> +common/console.c: In function 'puts':
> >> +common/console.c:746:29: error: unused variable 'ch'
> >> [-Werror=unused- variable]
> >> +  746 |                         int ch = *s++;
> >> +      |                             ^~
> >> +cc1: all warnings being treated as errors
> >> +make[2]: *** [scripts/Makefile.build:257: common/console.o] Error 1
> >>
> >>
> >> The main reason is that below patch removes "#ifdef":
> >>
> >> c04f856822a console: remove #ifdef CONFIG when it is possible
> >>
> >
> > Can you please always share the link to the pipelines that fail so
> > people have an idea on how to reproduce it locally?
> >
> > Here I assume it is:
> > https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/22455
> >
> > A simple way is to apply the patches, build the pine64-lts for example
> > and then you'll see warnings (which aren't failing builds locally I
> > believe but in CI, yes).
> >
> > I think we can fool the compiler with the following:
> >
> > diff --git a/include/debug_uart.h b/include/debug_uart.h
> > index dc0f1aa4c98..b19e44d6d0f 100644
> > --- a/include/debug_uart.h
> > +++ b/include/debug_uart.h
> > @@ -204,12 +204,12 @@ void printdec(unsigned int value);
> >  #define DEBUG_UART_FUNCS \
> >      #warning "DEBUG_UART not defined!"
> >
> > -#define printch(ch) do{}while(0);
> > -#define printascii(str) do{}while(0);
> > -#define printhex2(value) do{}while(0);
> > -#define printhex4(value) do{}while(0);
> > -#define printhex8(value) do{}while(0);
> > -#define printdec(value) do{}while(0);
> > +#define printch(ch) do{ (void)(ch); }while(0);
> > +#define printascii(str) do{ (void)(str); }while(0);
> > +#define printhex2(value) do{ (void)(value); }while(0);
> > +#define printhex4(value) do{ (void)(value); }while(0);
> > +#define printhex8(value) do{ (void)(value); }while(0);
> > +#define printdec(value) do{ (void)(value); }while(0);
> >
> >  #endif
> >
> > Does this make sense?
>
> Hi Quentin,
>
>      Thanks for your information about pipeline and suggestion for code
> change, but this workaround does not looks good :(
>

Thanks for the suggestions. I think this code can be even simplified
to just i.e.:

@@ -204,12 +204,12 @@ void printdec(unsigned int value);
 #define DEBUG_UART_FUNCS \
        #warning "DEBUG_UART not defined!"

-#define printch(ch) do{}while(0);
-#define printascii(str) do{}while(0);
-#define printhex2(value) do{}while(0);
-#define printhex4(value) do{}while(0);
-#define printhex8(value) do{}while(0);
-#define printdec(value) do{}while(0);
+#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;

 #endif

That will allow us to get rid of warnings. What do you think? I can't
think of another method besides creating a lot of #ifdefs in every
place debug functions are used, which will look even worse than the
dummy macros.


>
> Thanks,
>
> - Kever
>
> >
> > Cheers,
> > Quentin

Best regards,
Lukasz