[PATCH v1 2/2] mtd: spi-nor: hide flash name when the flash name does not exist

[Tudor Ambarus]

On 11/27/23 15:28, Michael Walle wrote:
> Hi,
> 
>>>>> > (null) will print when flash ID founded in ID table but
>>>>> > the flash name didn't include in it.
>>>>> >
>>>>> > Make info->name optional in the print for showing flash name
>>>>> > and flash size.
>>>>> >
>>>>> > Signed-off-by: JaimeLiao <jaimeliao at mxic.com.tw>
>>>>> > ---
>>>>> >  drivers/mtd/spi-nor/core.c | 2 +-
>>>>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> >
>>>>> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>>>>> > index 1c443fe568cf..73405bed2a5a 100644
>>>>> > --- a/drivers/mtd/spi-nor/core.c
>>>>> > +++ b/drivers/mtd/spi-nor/core.c
>>>>> > @@ -3518,7 +3518,7 @@ 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,
>>>>> > +    dev_info(dev, "%s (%lld Kbytes)\n", info->name ?: "",
>>>>>
>>>>> Maybe print the jedec id if the name is empty.
>>>>
>>>> What about always printing the jedec id if names aren't reliable
>>>> enough?
>>>
>>> I'm fine with that, too. Actually I've considered that myself, but
>>> maybe Tudor or Pratyush want to keep the output backwards compatible.
>>>
>>
>> We won't remove the names for flashes that already have a name defined,
>> won't we?
> 
> No.
> 
>> We just deprecate the name field and not use it anymore with
>> new flash additions. No backward compatibility problem.
> 
> I mean in the kernel console output. Someone out there might parse it.
> 
> If you switch from
>   dev_info(dev, "%s (%lld Kbytes)\n", info->name, ...)
> to
>   dev_info(dev, ""%*phN (%lld Kbytes)\n", id_len, id, ...)
> 

no problem in changing what we print to kernel log, there's no
guarantees associated with it.

>> BTW, that print should be lowered to dev_dbg, let's no longer pollute
>> the kernel log. Drivers should be quiet if all goes well.
> 
> Well, ok, that will also answer my question :) So let's go
> with the dev_dbg() and id only.
> 

I looked again and I saw that together with the name we printed the
flash size in Kbytes. But then the next print printed the same flash
size in bytes and then in MBytes. And then we printed some mtd data that
should be obtained with the mtd ioctls. I didn't like that mess and
ended up removing all those prints. We shall now have a faster kernel
boot (got rid of the print info), and cleaner kernel log. All that debug
data shall be obtained with the mtd ioctls and the SPI NOR sysfs and
debugfs entries, so hopefully more people will start using them.

Here's the patch that I mentioned:
https://lore.kernel.org/linux-mtd/20231127165908.1734951-1-tudor.ambarus@linaro.org/T/#u

Cheers,
ta