[PATCH 6/7] ddr: altera: arria10: Change %i to %u for printf

[Tom Rini]

On Wed, Apr 15, 2020 at 08:06:45PM +0200, Marek Vasut wrote:
> On 4/15/20 7:44 PM, Tom Rini wrote:
> > On Wed, Apr 15, 2020 at 05:16:52PM +0200, Marek Vasut wrote:
> >> On 4/15/20 5:14 PM, Tom Rini wrote:
> >>> On Wed, Apr 15, 2020 at 04:58:31PM +0200, Marek Vasut wrote:
> >>>> On 4/15/20 4:56 PM, Tom Rini wrote:
> >>>>> On Wed, Apr 15, 2020 at 02:45:08PM +0200, Marek Vasut wrote:
> >>>>>> On 4/15/20 11:00 AM, Ley Foon Tan wrote:
> >>>>>>> Tiny printf doesn't support %i, change to %u.
> >>>>>>>
> >>>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> >>>>>>> ---
> >>>>>>>  drivers/ddr/altera/sdram_arria10.c | 2 +-
> >>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/ddr/altera/sdram_arria10.c b/drivers/ddr/altera/sdram_arria10.c
> >>>>>>> index e3f11984a978..8acf324117af 100644
> >>>>>>> --- a/drivers/ddr/altera/sdram_arria10.c
> >>>>>>> +++ b/drivers/ddr/altera/sdram_arria10.c
> >>>>>>> @@ -195,7 +195,7 @@ static void sdram_init_ecc_bits(u32 size)
> >>>>>>>  
> >>>>>>>  	dcache_enable();
> >>>>>>>  
> >>>>>>> -	printf("DDRCAL: Scrubbing ECC RAM (%i MiB).\n", size >> 20);
> >>>>>>> +	printf("DDRCAL: Scrubbing ECC RAM (%u MiB).\n", size >> 20);
> >>>>>>>  	memset((void *)0x8000, 0, size - 0x8000);
> >>>>>>>  	flush_dcache_all();
> >>>>>>>  	printf("DDRCAL: Scrubbing ECC RAM done.\n");
> >>>>>>
> >>>>>> Yes, sadly, tiny printf is broken so we need to patch code to work
> >>>>>> around that breakage.
> >>>>>
> >>>>> Yes, limited by design, thanks for changing.
> >>>>
> >>>> This code could be used without tiny printf, so this change is unnecessary.
> >>>
> >>> You've got it backwards.  Code that could be used by tiny printf needs
> >>> to use the more limited set of formats.  But this should have been using
> >>> %u all along?  %i is for int, %u is unsigned int.
> >>
> >> That would mean most of U-Boot needs to be limited to the subset of
> >> formatting characters supported by tiny printf, which is unrealistic.
> > 
> > Not at all.  Only the code that's used in SPL and in tiny printf
> > situations needs to be limited to reduced set.  Which is why we're at
> > 4.5 years in and just now "oh, %i doesn't work?".
> 
> I keep running into "oh, %i, such a basic C printf() feature, doesn't
> work" again and again, and it makes my work with U-Boot real annoying.
> This should be fixed in the printf implementation, not all over the code
> base. Also, it prevents sane code reuse, unless we start adding #ifdef
> TINY_PRINTF all over the place, which is just ew ...

I hear you saying "I type %i not %d without thinking about it", but I'm
telling you, think about it.  I will not grow 200+ boards when there's
an easy way not to.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200416/d0897cfd/attachment.sig>