[Uboot-stm32] [PATCH v3 2/2] mtd: spi: nor: force mtd name to "nor%d"

[Patrick DELAUNAY]

Hi Marek,

On 9/17/21 3:39 PM, Marek Vasut wrote:
> On 9/17/21 3:06 PM, Patrick DELAUNAY wrote:
>> Hi Marek,
>>
>>  > Marek VasutSept. 16, 2021, 5:27 p.m. UTC | #3
>>  > On 9/16/21 4:01 PM, Patrick Delaunay wrote:
>>
>>  > [...]
>>
>>  > > @@ -3664,6 +3666,11 @@ int spi_nor_scan(struct spi_nor *nor)
>>  > >       struct mtd_info *mtd = &nor->mtd;
>>  > >       struct spi_slave *spi = nor->spi;
>>  > >       int ret;
>>  > > +    int cfi_mtd_nb = 0;
>>  > > +
>>  > > +#ifdef CONFIG_SYS_MAX_FLASH_BANKS
>>  > > +    cfi_mtd_nb = CONFIG_SYS_MAX_FLASH_BANKS;
>>  > > +#endif
>>
>>  > Are we covering all the NORs (HF and co.) with this ?
>>
>>
>> Yes, except if I miss something
>>
>>
>> any NOR (including hyperflash) wich use the CFI interface / 
>> CONFIG_FLASH_CFI_MTD
>>
>> define the the 'nor%d' name with the calling stack:
>
> Yes
>
>> initr_flash()
>>
>> => flash_init()
>>
>> ==> cfi_flash_init_dm()
>>
>> ==> cfi_mtd_init()   "nor%d" wich use loop on CONFIG_SYS_MAX_FLASH_BANKS
>>
>>
>> I have only one concern today...
>>
>>
>> if one cfi bank is missing (not activated in DT by example)
>>
>> and CONFIG_SYS_MAX_FLASH_BANKS_DETECT is not activated
>>
>> some holes can be done in index
>>
>>
>> example:
>>
>> with CONFIG_SYS_MAX_FLASH_BANKS = 2 but with only one NOR on the board
>>
>> => "nor1" is absent and we have only 2 MTD device named "nor"
>>
>> - "nor0" => NOR or HF, first CFI bank
>>
>> - "nor2" => SPI-NOR (first)
>>
>>
>> but I don't think that it is blocking
>
> Wasn't the old behavior exactly the same anyway ?


I don't sure, I think "nor0" was defined 2 times:

- "nor0" => NOR or HF, first CFI bank

- "nor0" => SPI-NOR (first)


And that cause issue => CFI and SPI-NOR wasn't activated at the same time.


>
>>  > >       /* Reset SPI protocol for all commands. */
>>  > >       nor->reg_proto = SNOR_PROTO_1_1_1;
>>  > > @@ -3715,8 +3722,10 @@ int spi_nor_scan(struct spi_nor *nor)
>>  > >       if (ret)
>>  > >           return ret;
>>  > >
>>  > > -    if (!mtd->name)
>>  > > -        mtd->name = info->name;
>>  > > +    if (!mtd->name) {
>>  > > +        sprintf(nor->mtd_name, "nor%d", cfi_mtd_nb + 
>> dev_seq(nor->dev));
>>  > > +        mtd->name = nor->mtd_name;
>>  > > +    }
>>  > >       mtd->dev = nor->dev;
>>  > >       mtd->priv = nor;
>>  > >       mtd->type = MTD_NORFLASH;
>>  > > @@ -3821,7 +3830,7 @@ int spi_nor_scan(struct spi_nor *nor)
>>  > >
>>  > >       nor->rdsr_dummy = params.rdsr_dummy;
>>  > >       nor->rdsr_addr_nbytes = params.rdsr_addr_nbytes;
>>  > > -    nor->name = mtd->name;
>>  > > +    nor->name = info->name;
>>  > >       nor->size = mtd->size;
>>  > >       nor->erase_size = mtd->erasesize;
>>  > >       nor->sector_size = mtd->erasesize;
>>  > > diff --git a/include/linux/mtd/spi-nor.h 
>> b/include/linux/mtd/spi-nor.h
>>  > > index 7ddc4ba2bf..8c3d5032e3 100644
>>  > > --- a/include/linux/mtd/spi-nor.h
>>  > > +++ b/include/linux/mtd/spi-nor.h
>>  > > @@ -561,6 +561,7 @@ struct spi_nor {
>>  > >       int (*ready)(struct spi_nor *nor);
>>  > >
>>  > >       void *priv;
>>  > > +    char mtd_name[10];
>>
>>  > should be 14, because nor%d\0 can be up to 14 bytes long.
>>
>> normally DM_MAX_SEQ = 999 (but never used)
>>
>> but Ok with you for 14 with "nor" = 3 + "%d" = 10 for max u32 value + 
>> "/0" = 1
>>
>>
>> for cfi_mtd_names => 16 byte used with "nor%d"
>>
>>      static char cfi_mtd_names[CFI_MAX_FLASH_BANKS][16];
>>
>> for nand => 8 bytes (./drivers/mtd/nand/raw/nand.c:59)
>>
>>      static char dev_name[CONFIG_SYS_MAX_NAND_DEVICE][8];
>>
>> for spi-nand => 20 bytes (drivers/mtd/nand/spi/core.c:1169)
>>
>>      mtd->name = malloc(20);
>
> Maybe we need a macro for this length of DM seq string which we could 
> use in these embedded array cases.


Ok, something as

/* Maximum sequence number supported */
#define DM_MAX_SEQ    999

+ #define DM_MAX_SEQ_STR    3


....

#define MTD_NAME_SIZE(type)    (sizeof(MTD_DEV_TYPE(type)) + 
DM_MAX_SEQ_STR))


=> MTD_NAME_SIZE(MTD_DEV_TYPE_NOR)


I need to test it tomorrow before to push it.


> _______________________________________________
> Uboot-stm32 mailing list
> Uboot-stm32 at st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32

Patrick