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:


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


> >       /* 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);


Patrick

Reply via email to