Re: [PATCH v3] mtd: spinand: Add support for AllianceMemory AS5F34G04SND

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Mario,

dev@kicherer.org wrote on Wed, 25 Jan 2023 13:22:04 +0100:

> Add support for AllianceMemory AS5F34G04SND SPI NAND flash
> 
> Datasheet:
> - https://www.alliancememory.com/wp-content/uploads/pdf/flash/AllianceMemory_SPI_NAND_Flash_July2020_Rev1.0.pdf
> 
> Signed-off-by: Mario Kicherer <dev@kicherer.org>
> Reviewed-by: Dhruva Gole <d-gole@ti.com>
> ---
>  drivers/mtd/nand/spi/Makefile         |   2 +-
>  drivers/mtd/nand/spi/alliancememory.c | 110 ++++++++++++++++++++++++++
>  drivers/mtd/nand/spi/core.c           |   1 +
>  include/linux/mtd/spinand.h           |   1 +
>  4 files changed, 113 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/mtd/nand/spi/alliancememory.c
> 
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> index b520fe634041..4ec973b8b6bf 100644
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,3 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
> -spinand-objs := core.o ato.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o xtx.o
> +spinand-objs := core.o alliancememory.o ato.o gigadevice.o macronix.o micron.o paragon.o toshiba.o winbond.o xtx.o
>  obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
> diff --git a/drivers/mtd/nand/spi/alliancememory.c b/drivers/mtd/nand/spi/alliancememory.c
> new file mode 100644
> index 000000000000..3430ac4d18fc
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/alliancememory.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Author: Mario Kicherer <dev@kicherer.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +#define SPINAND_MFR_ALLIANCEMEMORY	0x52
> +
> +#define AM_STATUS_ECC_BITMASK		(3 << 4)
> +
> +#define AM_STATUS_ECC_NONE_DETECTED	(0 << 4)
> +#define AM_STATUS_ECC_1_CORRECTED	(1 << 4)
> +#define AM_STATUS_ECC_1_DETECTED	(2 << 4)
> +#define AM_STATUS_ECC_MAX_CORRECTED	(3 << 4)
> +
> +static SPINAND_OP_VARIANTS(read_cache_variants,
> +		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> +			   SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> +			   SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> +			   SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> +			   SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
> +static int as5f34g04snd_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				      struct mtd_oob_region *region)
> +{
> +	region->offset = 0x48;
> +	region->length = 0x38;

This NAND series uses the end of the oob area to store the ecc bytes.
You can just hardcode how much ECC bytes are used because that's only
what matters and do something more readable:

ecc_bytes = [32 if page is 2k and OOB 64B] || [56 if page is 2k and OOB
is 128B] || [112 if page is 4k and OOB 256]

offset = mtd->oobsize - ecc_bytes
length = ecc_bytes

> +
> +	return 0;
> +}
> +
> +static int as5f34g04snd_ooblayout_free(struct mtd_info *mtd, int section,
> +				       struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	/*
> +	 * It is unclear how many bytes are used for the bad block marker. We
> +	 * reserve the common two bytes here.
> +	 */

You could maybe indicate in the comment that the free area is made of
sequential 4 or 8 identical chunks where each first 4 bytes are
unprotected.

> +
> +	region->offset = 2;
> +	region->length = 0x48 - 2;

And here again:
offset = 2
length = mtd->oobsize - 2 - ecc_bytes

> +
> +	return 0;
> +}

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/