Re: [PATCH] mtd: spi-nor: stop printing superfluous debug info

[Miquel Raynal]

Hello,

michael@xxxxxxxx wrote on Tue, 28 Nov 2023 09:47:28 +0100:

> Hi,
> 
> > The mtd data can be obtain with the mtd ioctls and the SPI NOR
> > flash name can be determined interrogating the sysfs entries.
> > Stop polluting the kernel log.
> > 
> > Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>
> > 
> > ---
> >  drivers/mtd/spi-nor/core.c | 19 -------------------
> >  1 file changed, 19 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 25a64c65717d..6de76fd009d1 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -3517,25 +3517,6 @@ int spi_nor_scan(struct spi_nor *nor, const char > *name,
> >  	/* No mtd_info fields should be used up to this point. */
> >  	spi_nor_set_mtd_info(nor);
> > 
> > -	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
> > -			(long long)mtd->size >> 10);  
> 
> I'd lower this to dev_dbg() and print the jedec id. It might come in
> handy for a quick glance during bootup if debug is enabled.

Ack. Although, your boot time will almost be unaffected if you don't
print the info messages to the console. What takes the most time is not
writing to the kernel buffer, it's to display the lines on a serial
console, and dev_info() are by default discarded, you need to select a
lower log level manually, and if you do that it means you're not
looking for quick boot times but rather more for additional information.

> > -	dev_dbg(dev,
> > -		"mtd .name = %s, .size = 0x%llx (%lldMiB), "
> > -		".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
> > -		mtd->name, (long long)mtd->size, (long long)(mtd->size >> 20),
> > -		mtd->erasesize, mtd->erasesize / 1024, mtd->numeraseregions);
> > -
> > -	if (mtd->numeraseregions)
> > -		for (i = 0; i < mtd->numeraseregions; i++)
> > -			dev_dbg(dev,
> > -				"mtd.eraseregions[%d] = { .offset = 0x%llx, "
> > -				".erasesize = 0x%.8x (%uKiB), "
> > -				".numblocks = %d }\n",
> > -				i, (long long)mtd->eraseregions[i].offset,
> > -				mtd->eraseregions[i].erasesize,
> > -				mtd->eraseregions[i].erasesize / 1024,
> > -				mtd->eraseregions[i].numblocks);
> >  	return 0;  
> 
> Part of this is already available through the spi-nor debugfs, although not
> the actual mtd properties. These I think, should go into the mtdcore
> itself if really needed. Either through dev_dbg() or debugfs.

Maybe we don't need this at all, as long as one message remains about
the JEDEC ID, but keep in mind that spi-nors are commonly storing the
rootfs and if your spi-nor does not boot you don't have a userspace yet
and all the debugfs entries are purely useless.

Thanks,
Miquèl