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

[Michael Walle <michael@xxxxxxxx>]

Hi,

Am 2022-05-02 20:56, schrieb Biju Das:
> Add support for Renesas AT25QL128A serial nor flash.
> Details of flash chip can be found here [1]
>
> [1] 
> https://www.dialog-semiconductor.com/sites/default/files/2022-04/DS-AT25QL128A-129F-022022.pdf
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> $  xxd -p sfdp
> 53464450060101ff00060110300000ff1f00010280000001ffffffffffff
> ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b
> 083b80bbfeffffffffff00ffffff42eb0c200f5210d800ff3362d5008429
> 01ceeca1073d7a757a75f7a2d55c19f61cffe810c080ffffffffffffffff
> ffffffffffffffff501650190000ffff

Thanks!

> :~# md5sum
> /sys/devices/platform/soc/10060000.spi/rpc-if-spi/spi_master/spi1/spi1.0/spi-nor/sfdp
> 23e3ec56b5b8f986d0488ba4727239dd
>
> ~# cat
> /sys/devices/platform/soc/10060000.spi/rpc-if-spi/spi_master/spi1/spi1.0/spi-nor/jedec_id
> 1f4218
>
> ~# cat
> /sys/devices/platform/soc/10060000.spi/rpc-if-spi/spi_master/spi1/spi1.0/spi-nor/partname
> at25ql128a
>
> ~# cat
> /sys/devices/platform/soc/10060000.spi/rpc-if-spi/spi_master/spi1/spi1.0/spi-nor/manufacturer
> renesas
>
> RFC->v1:
>  * Moved the flash definitions to atmel.c
>  * Remove macro related to locking as it is untested.
>  * Replaced INFO macro with SNOR_ID3
>
> RFC:
>  *
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220501185512.51158-1-biju.das.jz@xxxxxxxxxxxxxx/
> ---
>  drivers/mtd/spi-nor/atmel.c | 55 +++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/core.c  |  1 +
>  drivers/mtd/spi-nor/core.h  |  1 +
>  3 files changed, 57 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
> index 656dd80a0be7..61cff6de7db2 100644
> --- a/drivers/mtd/spi-nor/atmel.c
> +++ b/drivers/mtd/spi-nor/atmel.c
> @@ -10,6 +10,15 @@
>
>  #define ATMEL_SR_GLOBAL_PROTECT_MASK GENMASK(5, 2)
>
> +#define SNOR_ID3(_jedec_id) \
> +	.id = { \
> +		((_jedec_id) >> 16) & 0xff, \
> +		((_jedec_id) >> 8) & 0xff, \
> +		(_jedec_id) & 0xff, \
> +	}, \
> +	.id_len = 3, \
> +	.parse_sfdp = true

Put that into core.h below the INFO macros.

> +
>  /*
>   * The Atmel AT25FS010/AT25FS040 parts have some weird configuration 
> for the
>   * block protection bits. We don't support them. But legacy behavior 
> in linux
> @@ -204,6 +213,52 @@ static const struct flash_info atmel_nor_parts[] = 
> {
>  		NO_SFDP_FLAGS(SECT_4K) },
>  };
>
> +static const struct flash_info renesas_nor_parts[] = {
> +	{ "at25ql128a", SNOR_ID3(0x1f4218) },

Put that into the atmel list, no need for an own list.

> +};
> +
> +/**
> + * renesas_nor_set_4byte_addr_mode() - Set 4-byte address mode for 
> Renesas
> + * flashes.
> + * @nor:	pointer to 'struct spi_nor'.
> + * @enable:	true to enter the 4-byte address mode, false to exit the 
> 4-byte
> + *		address mode.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int renesas_nor_set_4byte_addr_mode(struct spi_nor *nor, bool 
> enable)

Please expand the SFDP parser. This should be contained in 16th dword,
there.

> +{
> +	int ret;
> +
> +	ret = spi_nor_write_enable(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_nor_set_4byte_addr_mode(nor, enable);
> +	if (ret)
> +		return ret;
> +
> +	return spi_nor_write_disable(nor);
> +}
> +
> +static void renesas_nor_default_init(struct spi_nor *nor)
> +{
> +	nor->flags &= ~SNOR_F_HAS_16BIT_SR;

Why is that?

> +	nor->params->quad_enable = NULL;
> +	nor->params->set_4byte_addr_mode = renesas_nor_set_4byte_addr_mode;
> +}
> +
> +static const struct spi_nor_fixups renesas_nor_fixups = {
> +	.default_init = renesas_nor_default_init,
> +};
> +
> +const struct spi_nor_manufacturer spi_nor_renesas = {
> +	.name = "renesas",
> +	.parts = renesas_nor_parts,
> +	.nparts = ARRAY_SIZE(renesas_nor_parts),
> +	.fixups = &renesas_nor_fixups,
> +};

Not needed.

-michael

> +
>  const struct spi_nor_manufacturer spi_nor_atmel = {
>  	.name = "atmel",
>  	.parts = atmel_nor_parts,
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index b4f141ad9c9c..ba9f222da00b 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1621,6 +1621,7 @@ static const struct spi_nor_manufacturer
> *manufacturers[] = {
>  	&spi_nor_issi,
>  	&spi_nor_macronix,
>  	&spi_nor_micron,
> +	&spi_nor_renesas,
>  	&spi_nor_st,
>  	&spi_nor_spansion,
>  	&spi_nor_sst,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index b7fd760e3b47..3d2e39329079 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -511,6 +511,7 @@ extern const struct spi_nor_manufacturer 
> spi_nor_intel;
>  extern const struct spi_nor_manufacturer spi_nor_issi;
>  extern const struct spi_nor_manufacturer spi_nor_macronix;
>  extern const struct spi_nor_manufacturer spi_nor_micron;
> +extern const struct spi_nor_manufacturer spi_nor_renesas;
>  extern const struct spi_nor_manufacturer spi_nor_st;
>  extern const struct spi_nor_manufacturer spi_nor_spansion;
>  extern const struct spi_nor_manufacturer spi_nor_sst;