Re: [PATCH] cmd: nand: Add support to print the manufacturer, model and size

[Alexander Dahl <ada@thorsis.com>]

Hello Mihai,

adding the raw nand maintainers to Cc, maybe cmd/nand.c should be
added to MAINTAINERS file section raw nand?  See my comments below.

Am Mon, Mar 18, 2024 at 02:26:47PM +0200 schrieb Mihai Sain:
> Add support to nand info for printing the manufacturer,model and size.
> 
> U-Boot> nand info
> 
> Device 0: nand0, sector size 256 KiB
>   Manufacturer  MACRONIX
>   Model         MX30LF4G28AD
>   Device size        512 MiB
>   Page size         4096 b
>   OOB size           256 b
>   Erase size      262144 b
>   ecc strength         8 bits
>   ecc step size      512 b
>   subpagesize       4096 b
>   options       0x00004200
>   bbt options   0x00028000

Tested here:

    Device 0: nand0, sector size 128 KiB
      Manufacturer  SPANSION 
      Model         S34ML02G1 
      Device size        256 MiB
      Page size         2048 b
      OOB size            64 b
      Erase size      131072 b
      ecc strength         8 bits
      ecc step size      512 b
      subpagesize       2048 b
      options       0x40004200
      bbt options   0x00028000

> Signed-off-by: Mihai Sain <mihai.sain@microchip.com>
> ---
>  cmd/nand.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/cmd/nand.c b/cmd/nand.c
> index fe834c4ac5..f5cfaab37c 100644
> --- a/cmd/nand.c
> +++ b/cmd/nand.c
> @@ -418,6 +418,9 @@ static void nand_print_and_set_info(int idx)
>  		printf("%dx ", chip->numchips);
>  	printf("%s, sector size %u KiB\n",
>  	       mtd->name, mtd->erasesize >> 10);
> +	printf("  Manufacturer  %s \n", chip->onfi_params.manufacturer);
> +	printf("  Model         %s \n", chip->onfi_params.model);

This probably only works for ONFI flash and might fail otherwise.  See
this code in nand_base.c:

https://elixir.bootlin.com/u-boot/v2024.01/source/drivers/mtd/nand/raw/nand_base.c#L4455

The manufacturer could always be printed from struct nand_chip
->manufacturer.desc.name here.

Don't know if the third fallback for model ("type->name") is somehow
accessible after nand_detect() but I would at least test on
chip->onfi_version or chip->jedec_version here, and maybe just drop
the model line if none is set.

> +	printf("  Device size   %8d MiB\n", (int)(chip->chipsize >> 20));

Fine.

Greets
Alex

>  	printf("  Page size     %8d b\n", mtd->writesize);
>  	printf("  OOB size      %8d b\n", mtd->oobsize);
>  	printf("  Erase size    %8d b\n", mtd->erasesize);
> -- 
> 2.44.0
>