Re: [PATCH] tiny-printf: Add support for upper case hex values

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]

On Fri, Apr 04, 2025 at 10:33:25AM +0200, Michael Walle wrote:
> Hi,
> 
> > > I wouldn't mind printing the pointer for %p[mMI], but %pa prints the
> > > *content* of the pointer which is really confusing. I.e. in
> > > pinctrl-single.c the reg value pairs are printed like
> > > 
> > >   dev_dbg(dev, "reg/val %pa/0x%08x\n", &reg, val);
> > > 
> > > with reg being a pointer to a physical address. So with tiny_printf
> > > the address of reg (which is a pointer to the stack) is printed in
> > > this case.
> > > 
> > > I don't think we can print %p without putting more logic into the
> > > decoding. I think the culprit here is the fallthrough to %x, which
> > > then leads to the confusing behavior shown above. IMHO if we want to
> > > avoid that, we'd have to make %p entirely unsupported.
> > > 
> > > diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> > > index faf55d7f327..8147ffa2c1b 100644
> > > --- a/lib/tiny-printf.c
> > > +++ b/lib/tiny-printf.c
> > > @@ -269,21 +269,18 @@ static int _vprintf(struct printf_info *info, const char *fmt,
> > > va_list va)
> > >  						div_out(info, &num, div);
> > >  				}
> > >  				break;
> > > +#if CONFIG_IS_ENABLED(NET) || CONFIG_IS_ENABLED(NET_LWIP) || _DEBUG
> >
> > What if we fine-tune tinyprinf via config here?
> > For example SPL_USE_TINY_PRINTF_POINTER_SUPPORT and
> > select it by NET or NET_LWIP. If someone needs it,
> > the pointer output can be enabled, otherwise '?' for
> > unsupported is output.
> 
> Yeah I had a similar idea, but I'm not sure if yet another config
> symbol is worth it. That's up to the maintainer to decide :)
> 
> In any case, we have a different behavior to what is printed
> right now, as we drop the fallthrough to %x. Tom? Simon?

A Kconfig symbol that NET||NET_LWIP select seems fine, and fall through
to printing "?".

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]