Re: [RFC] mtd: spi-nor: Add Renesas AT25QL128A serial nor flash

[Michael Walle <michael@xxxxxxxx>]

Am 2022-05-02 13:29, schrieb Michael Walle:
> >  drivers/mtd/spi-nor/Makefile  |  1 +
> >  drivers/mtd/spi-nor/core.c    |  1 +
> >  drivers/mtd/spi-nor/core.h    |  1 +
> >  drivers/mtd/spi-nor/renesas.c | 57 
> > +++++++++++++++++++++++++++++++++++
>
> Please put the flash in atmel.c. There are all the Atmel/Adesto/now 
> Renesas
> flashes.
>
> > +static const struct flash_info renesas_nor_parts[] = {
> > +	{ "at25ql128a", INFO(0x1f4218, 0, 64 * 1024, 256)
> > +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> > +};
>
> I suppose this flash supports SFDP. Therefore,
> 	{ "at25ql128a", PARSE_SFDP },

The id is missing. Sorry. Seems like we need a new macro.

#define SNOR_ID3(_jedec_id) \
    .id = { \
        ((_jedec_id) >> 16) & 0xff, \
         ((_jedec_id) >> 8) & 0xff, \
          (_jedec_id) & 0xff, \
    } \
    .id_len = 3,

Then it would be
	{ "at25ql128a", SNOR_ID3(0x1f4218) PARSE_SFDP },

Now we'll get bitten by that missing comma (or the additional
comma in the defines).. I'd love to see that one removed, so
we can have the usual declarations:
	{ "at25ql128a", SNOR_ID3(0x1f4218), PARSE_SFDP },

Also it might make sense to have the PARSE_SFDP in the
SNOR_ID3() macro, because without SFDP, that macro cannot
be used. Tudor, Pratyush, what do you think about:

#define SNOR_ID3(_jedec_id) \
    .id = { \
        ((_jedec_id) >> 16) & 0xff, \
         ((_jedec_id) >> 8) & 0xff, \
          (_jedec_id) & 0xff, \
    } \
    .id_len = 3, \
    .parse_sfdp = true


-michael