[PATCH v2 0/6] mtd: Make sure UBIFS does not do multi-pass page programming on flashes that don't support it

[PATCH v2 0/6] mtd: Make sure UBIFS does not do multi-pass page programming on flashes that don't support it

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

This series is equivalent to the one for Linux MTD submitted by
Pratyush Yadav.

https://patchwork.ozlabs.org/project/linux-mtd/list/?series=217759&state=*

Changes in v2:
  - Fix an issue in setting macronix_octal_fixups
  - Rework fixup hooks

Takahiro Kuwano (6):
  mtd: ubi: Do not zero out EC and VID on ECC-ed NOR flashes
  mtd: spi-nor: Allow flashes to specify MTD writesize
  mtd: spi-nor: Check nor->info before setting macronix_octal_fixups
  mtd: spi-nor: Replace default_init() hook with late_init()
  mtd: spi-nor: Call spi_nor_post_sfdp_fixups() only after
    spi_nor_parse_sfdp()
  mtd: spi-nor: Set ECC unit size to MTD writesize in Infineon SEMPER
    flashes

 drivers/mtd/spi/spi-nor-core.c | 88 +++++++++++++++++++++-------------
 drivers/mtd/ubi/build.c        |  4 +-
 drivers/mtd/ubi/io.c           |  9 +++-
 include/linux/mtd/spi-nor.h    |  1 +
 4 files changed, 65 insertions(+), 37 deletions(-)

-- 
2.34.1

[PATCH v2 1/6] mtd: ubi: Do not zero out EC and VID on ECC-ed NOR flashes

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

For NOR flashes EC and VID are zeroed out before an erase is issued to
make sure UBI does not mistakenly treat the PEB as used and associate it
with an LEB.

But on some flashes, like the Infineon Semper NOR flash family,
multi-pass page programming is not allowed on the default ECC scheme.
This means zeroing out these magic numbers will result in the flash
throwing a page programming error.

Do not zero out EC and VID for such flashes. A writesize > 1 is an
indication of an ECC-ed flash.

This patch replicates the following upstream linux commit:
f669e74be820 ("ubi: Do not zero out EC and VID on ECC-ed NOR flashes")

Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/ubi/build.c | 4 +---
 drivers/mtd/ubi/io.c    | 9 ++++++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index a1941b8eb8..81c1b7bdbc 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -679,10 +679,8 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 		ubi->bad_peb_limit = get_bad_peb_limit(ubi, max_beb_per1024);
 	}
 
-	if (ubi->mtd->type == MTD_NORFLASH) {
-		ubi_assert(ubi->mtd->writesize == 1);
+	if (ubi->mtd->type == MTD_NORFLASH)
 		ubi->nor_flash = 1;
-	}
 
 	ubi->min_io_size = ubi->mtd->writesize;
 	ubi->hdrs_min_io_size = ubi->mtd->writesize >> ubi->mtd->subpage_sft;
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 14be95b74b..45699b4a47 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -563,7 +563,14 @@ int ubi_io_sync_erase(struct ubi_device *ubi, int pnum, int torture)
 		return -EROFS;
 	}
 
-	if (ubi->nor_flash) {
+	/*
+	 * If the flash is ECC-ed then we have to erase the ECC block before we
+	 * can write to it. But the write is in preparation to an erase in the
+	 * first place. This means we cannot zero out EC and VID before the
+	 * erase and we just have to hope the flash starts erasing from the
+	 * start of the page.
+	 */
+	if (ubi->nor_flash && ubi->mtd->writesize == 1) {
 		err = nor_erase_prepare(ubi, pnum);
 		if (err)
 			return err;
-- 
2.34.1

Re: [PATCH v2 1/6] mtd: ubi: Do not zero out EC and VID on ECC-ed NOR flashes

[Pratyush Yadav]

On Thu, Apr 25 2024, tkuw584924@gmail.com wrote:

> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

I wonder how authorship should work for such patches. Patches 1, 2, and
6 in this series are very close to what my patches did for Linux. So I
wonder who should get authorship in this case: the person porting the
patch or the person who wrote the original one. Same for patch 5 but
that is still a little more changed from its Linux counterpart.

>
> For NOR flashes EC and VID are zeroed out before an erase is issued to
> make sure UBI does not mistakenly treat the PEB as used and associate it
> with an LEB.
>
> But on some flashes, like the Infineon Semper NOR flash family,
> multi-pass page programming is not allowed on the default ECC scheme.
> This means zeroing out these magic numbers will result in the flash
> throwing a page programming error.
>
> Do not zero out EC and VID for such flashes. A writesize > 1 is an
> indication of an ECC-ed flash.
>
> This patch replicates the following upstream linux commit:
> f669e74be820 ("ubi: Do not zero out EC and VID on ECC-ed NOR flashes")
>
> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Apart from my comment above,

Acked-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

Re: [PATCH v2 1/6] mtd: ubi: Do not zero out EC and VID on ECC-ed NOR flashes

[Tudor Ambarus]

On 4/29/24 15:17, Pratyush Yadav wrote:
> On Thu, Apr 25 2024, tkuw584924@gmail.com wrote:
> 
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> I wonder how authorship should work for such patches. Patches 1, 2, and
> 6 in this series are very close to what my patches did for Linux. So I
> wonder who should get authorship in this case: the person porting the
> patch or the person who wrote the original one. Same for patch 5 but
> that is still a little more changed from its Linux counterpart.
> 

I saw it's the one that ports the patch to u-boot even when it's a
1-to-1 match. I guess it's because u-boot has its own dedicated repo.

cut

>>
>> This patch replicates the following upstream linux commit:
>> f669e74be820 ("ubi: Do not zero out EC and VID on ECC-ed NOR flashes")

This paragraph shall give credit to the linux author and at the same
time help others review the changes.

[PATCH v2 2/6] mtd: spi-nor: Allow flashes to specify MTD writesize

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Some flashes like the Infineon SEMPER NOR flash family use ECC. Under
this ECC scheme, multi-pass writes to an ECC block is not allowed.
In other words, once data is programmed to an ECC block, it can't be
programmed again without erasing it first.

Upper layers like file systems need to be given this information so they
do not cause error conditions on the flash by attempting multi-pass
programming. This can be done by setting 'writesize' in 'struct
mtd_info'.

Set the default to 1 but allow flashes to modify it in fixup hooks. If
more flashes show up with this constraint in the future it might be
worth it to add it to 'struct flash_info', but for now increasing its
size is not worth it.

This patch replicates the following upstream linux commit:
afd473e85827 ("mtd: spi-nor: core: Allow flashes to specify MTD writesize")

Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi/spi-nor-core.c | 3 ++-
 include/linux/mtd/spi-nor.h    | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index f86003ca8c..1bfef6797f 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -2789,6 +2789,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
 	memset(params, 0, sizeof(*params));
 
 	/* Set SPI NOR sizes. */
+	params->writesize = 1;
 	params->size = info->sector_size * info->n_sectors;
 	params->page_size = info->page_size;
 
@@ -4078,7 +4079,7 @@ int spi_nor_scan(struct spi_nor *nor)
 	mtd->dev = nor->dev;
 	mtd->priv = nor;
 	mtd->type = MTD_NORFLASH;
-	mtd->writesize = 1;
+	mtd->writesize = params.writesize;
 	mtd->flags = MTD_CAP_NORFLASH;
 	mtd->size = params.size;
 	mtd->_erase = spi_nor_erase;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d1dbf3eadb..0d37a806c4 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -436,6 +436,7 @@ enum spi_nor_pp_command_index {
 
 struct spi_nor_flash_parameter {
 	u64				size;
+	u32				writesize;
 	u32				page_size;
 	u8				rdsr_dummy;
 	u8				rdsr_addr_nbytes;
-- 
2.34.1

[PATCH v2 3/6] mtd: spi-nor: Check nor->info before setting macronix_octal_fixups

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The macronix_octal_fixups should be set only when mfr and flags match.

Fixes: df3d5f9e41 ("mtd: spi-nor: add support for Macronix Octal flash")
Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Cc: JaimeLiao <jaimeliao.tw@gmail.com>
---
 drivers/mtd/spi/spi-nor-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 1bfef6797f..c2d2ddf0c8 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -4003,7 +4003,9 @@ void spi_nor_set_fixups(struct spi_nor *nor)
 #endif
 
 #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
-	nor->fixups = &macronix_octal_fixups;
+	if (JEDEC_MFR(nor->info) == SNOR_MFR_MACRONIX &&
+	    nor->info->flags & SPI_NOR_OCTAL_DTR_READ)
+		nor->fixups = &macronix_octal_fixups;
 #endif /* SPI_FLASH_MACRONIX */
 }
 
-- 
2.34.1

Re: [PATCH v2 3/6] mtd: spi-nor: Check nor->info before setting macronix_octal_fixups

[Tudor Ambarus]

On 4/25/24 05:52, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> The macronix_octal_fixups should be set only when mfr and flags match.
> 
> Fixes: df3d5f9e41 ("mtd: spi-nor: add support for Macronix Octal flash")
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> Cc: JaimeLiao <jaimeliao.tw@gmail.com>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 1bfef6797f..c2d2ddf0c8 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -4003,7 +4003,9 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>  #endif
>  
>  #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> -	nor->fixups = &macronix_octal_fixups;
> +	if (JEDEC_MFR(nor->info) == SNOR_MFR_MACRONIX &&
> +	    nor->info->flags & SPI_NOR_OCTAL_DTR_READ)
> +		nor->fixups = &macronix_octal_fixups;

we still have the mfr checks in u-boot, sigh.

sounds sane:
Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>  #endif /* SPI_FLASH_MACRONIX */
>  }
>  

[PATCH v2 4/6] mtd: spi-nor: Replace default_init() hook with late_init()

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

default_init() is wrong, it contributes to the maze of initializing
flash parameters. We'd like to get rid of it because the flash
parameters that it initializes are not really used at SFDP parsing time,
thus they can be initialized later.

Ideally we want SFDP to initialize all the flash parameters. If (when)
SFDP tables are wrong, we fix them with the post_sfdp/bfpt hooks, to
emphasize that SFDP is indeed wrong. When there are parameters that are
not covered by SFDP, we initialize them in late_init() - these
parameters have nothing to do with SFDP and they are not needed earlier.
With this we'll have a clearer view of who initializes what.

There are six default_init() hooks implemented just for initializing
octal_dtr_enable() and/or setup() hooks that called later on.
Just moving those to late_init() does not change functionality.

Suggested-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi/spi-nor-core.c | 48 +++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index c2d2ddf0c8..fda879f3a3 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -189,27 +189,27 @@ struct sfdp_bfpt {
 
 /**
  * struct spi_nor_fixups - SPI NOR fixup hooks
- * @default_init: called after default flash parameters init. Used to tweak
- *                flash parameters when information provided by the flash_info
- *                table is incomplete or wrong.
  * @post_bfpt: called after the BFPT table has been parsed
  * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
  *             that do not support RDSFDP). Typically used to tweak various
  *             parameters that could not be extracted by other means (i.e.
  *             when information provided by the SFDP/flash_info tables are
  *             incomplete or wrong).
+ * @late_init: used to initialize flash parameters that are not declared in the
+ *             JESD216 SFDP standard, or where SFDP tables not defined at all.
  *
  * Those hooks can be used to tweak the SPI NOR configuration when the SFDP
  * table is broken or not available.
  */
 struct spi_nor_fixups {
-	void (*default_init)(struct spi_nor *nor);
 	int (*post_bfpt)(struct spi_nor *nor,
 			 const struct sfdp_parameter_header *bfpt_header,
 			 const struct sfdp_bfpt *bfpt,
 			 struct spi_nor_flash_parameter *params);
 	void (*post_sfdp)(struct spi_nor *nor,
 			  struct spi_nor_flash_parameter *params);
+	void (*late_init)(struct spi_nor *nor,
+			  struct spi_nor_flash_parameter *params);
 };
 
 #define SPI_NOR_SRST_SLEEP_LEN			200
@@ -2775,10 +2775,11 @@ static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
 		nor->fixups->post_sfdp(nor, params);
 }
 
-static void spi_nor_default_init_fixups(struct spi_nor *nor)
+static void spi_nor_late_init_fixups(struct spi_nor *nor,
+				     struct spi_nor_flash_parameter *params)
 {
-	if (nor->fixups && nor->fixups->default_init)
-		nor->fixups->default_init(nor);
+	if (nor->fixups && nor->fixups->late_init)
+		nor->fixups->late_init(nor, params);
 }
 
 static int spi_nor_init_params(struct spi_nor *nor,
@@ -2885,8 +2886,6 @@ static int spi_nor_init_params(struct spi_nor *nor,
 		}
 	}
 
-	spi_nor_default_init_fixups(nor);
-
 	/* Override the parameters with data read from SFDP tables. */
 	nor->addr_width = 0;
 	nor->mtd.erasesize = 0;
@@ -2905,6 +2904,7 @@ static int spi_nor_init_params(struct spi_nor *nor,
 	}
 
 	spi_nor_post_sfdp_fixups(nor, params);
+	spi_nor_late_init_fixups(nor, params);
 
 	return 0;
 }
@@ -3328,7 +3328,8 @@ static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info,
 	return spi_nor_default_setup(nor, info, params);
 }
 
-static void s25fs_s_default_init(struct spi_nor *nor)
+static void s25fs_s_late_init(struct spi_nor *nor,
+			      struct spi_nor_flash_parameter *params)
 {
 	nor->setup = s25fs_s_setup;
 }
@@ -3366,9 +3367,9 @@ static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor,
 }
 
 static struct spi_nor_fixups s25fs_s_fixups = {
-	.default_init = s25fs_s_default_init,
 	.post_bfpt = s25fs_s_post_bfpt_fixup,
 	.post_sfdp = s25fs_s_post_sfdp_fixup,
+	.late_init = s25fs_s_late_init,
 };
 
 static int s25_s28_mdp_ready(struct spi_nor *nor)
@@ -3452,7 +3453,8 @@ static int s25_s28_setup(struct spi_nor *nor, const struct flash_info *info,
 	return spi_nor_default_setup(nor, info, params);
 }
 
-static void s25_default_init(struct spi_nor *nor)
+static void s25_late_init(struct spi_nor *nor,
+			  struct spi_nor_flash_parameter *params)
 {
 	nor->setup = s25_s28_setup;
 }
@@ -3533,9 +3535,9 @@ static void s25_post_sfdp_fixup(struct spi_nor *nor,
 }
 
 static struct spi_nor_fixups s25_fixups = {
-	.default_init = s25_default_init,
 	.post_bfpt = s25_s28_post_bfpt_fixup,
 	.post_sfdp = s25_post_sfdp_fixup,
+	.late_init = s25_late_init,
 };
 
 static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info,
@@ -3544,13 +3546,14 @@ static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info,
 	return -ENOTSUPP; /* Bank Address Register is not supported */
 }
 
-static void s25fl256l_default_init(struct spi_nor *nor)
+static void s25fl256l_late_init(struct spi_nor *nor,
+				struct spi_nor_flash_parameter *params)
 {
 	nor->setup = s25fl256l_setup;
 }
 
 static struct spi_nor_fixups s25fl256l_fixups = {
-	.default_init = s25fl256l_default_init,
+	.late_init = s25fl256l_late_init,
 };
 #endif
 
@@ -3613,7 +3616,8 @@ static int spi_nor_cypress_octal_dtr_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static void s28hx_t_default_init(struct spi_nor *nor)
+static void s28hx_t_late_init(struct spi_nor *nor,
+			      struct spi_nor_flash_parameter *params)
 {
 	nor->octal_dtr_enable = spi_nor_cypress_octal_dtr_enable;
 	nor->setup = s25_s28_setup;
@@ -3651,9 +3655,9 @@ static void s28hx_t_post_sfdp_fixup(struct spi_nor *nor,
 }
 
 static struct spi_nor_fixups s28hx_t_fixups = {
-	.default_init = s28hx_t_default_init,
 	.post_sfdp = s28hx_t_post_sfdp_fixup,
 	.post_bfpt = s25_s28_post_bfpt_fixup,
+	.late_init = s28hx_t_late_init,
 };
 #endif /* CONFIG_SPI_FLASH_S28HX_T */
 
@@ -3705,7 +3709,8 @@ static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static void mt35xu512aba_default_init(struct spi_nor *nor)
+static void mt35xu512aba_late_init(struct spi_nor *nor,
+				   struct spi_nor_flash_parameter *params)
 {
 	nor->octal_dtr_enable = spi_nor_micron_octal_dtr_enable;
 }
@@ -3734,8 +3739,8 @@ static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor,
 }
 
 static struct spi_nor_fixups mt35xu512aba_fixups = {
-	.default_init = mt35xu512aba_default_init,
 	.post_sfdp = mt35xu512aba_post_sfdp_fixup,
+	.late_init = mt35xu512aba_late_init,
 };
 #endif /* CONFIG_SPI_FLASH_MT35XU */
 
@@ -3795,7 +3800,8 @@ static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static void macronix_octal_default_init(struct spi_nor *nor)
+static void macronix_octal_late_init(struct spi_nor *nor,
+				     struct spi_nor_flash_parameter *params)
 {
 	nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
 }
@@ -3812,8 +3818,8 @@ static void macronix_octal_post_sfdp_fixup(struct spi_nor *nor,
 }
 
 static struct spi_nor_fixups macronix_octal_fixups = {
-	.default_init = macronix_octal_default_init,
 	.post_sfdp = macronix_octal_post_sfdp_fixup,
+	.late_init = macronix_octal_late_init,
 };
 #endif /* CONFIG_SPI_FLASH_MACRONIX */
 
-- 
2.34.1

Re: [PATCH v2 4/6] mtd: spi-nor: Replace default_init() hook with late_init()

[Tudor Ambarus]

On 4/25/24 05:52, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> default_init() is wrong, it contributes to the maze of initializing
> flash parameters. We'd like to get rid of it because the flash
> parameters that it initializes are not really used at SFDP parsing time,
> thus they can be initialized later.
> 
> Ideally we want SFDP to initialize all the flash parameters. If (when)
> SFDP tables are wrong, we fix them with the post_sfdp/bfpt hooks, to
> emphasize that SFDP is indeed wrong. When there are parameters that are
> not covered by SFDP, we initialize them in late_init() - these
> parameters have nothing to do with SFDP and they are not needed earlier.
> With this we'll have a clearer view of who initializes what.
> 
> There are six default_init() hooks implemented just for initializing
> octal_dtr_enable() and/or setup() hooks that called later on.
> Just moving those to late_init() does not change functionality.
> 
> Suggested-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>

[PATCH v2 5/6] mtd: spi-nor: Call spi_nor_post_sfdp_fixups() only after spi_nor_parse_sfdp()

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

spi_nor_post_sfdp_fixups() was called regardless of if
spi_nor_parse_sfdp() had been called or not. late_init() should be
instead used to initialize the parameters that are not defined in SFDP.

Ideally spi_nor_post_sfdp_fixups() is called only after successful parse
of SFDP. However, in case SFDP support is disabled by .config, that can
break current functionality. Therefore, we would call it after
spi_nor_parse_sfdp() regardless of its return value.

This patch follows the upstream linux commit:
5273cc6df984("mtd: spi-nor: core: Call spi_nor_post_sfdp_fixups() only
when SFDP is defined")

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi/spi-nor-core.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index fda879f3a3..ee968c10e4 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -190,11 +190,10 @@ struct sfdp_bfpt {
 /**
  * struct spi_nor_fixups - SPI NOR fixup hooks
  * @post_bfpt: called after the BFPT table has been parsed
- * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
- *             that do not support RDSFDP). Typically used to tweak various
- *             parameters that could not be extracted by other means (i.e.
- *             when information provided by the SFDP/flash_info tables are
- *             incomplete or wrong).
+ * @post_sfdp: called after SFDP has been parsed. Typically used to tweak
+ *             various parameters that could not be extracted by other means
+ *             (i.e. when information provided by the SFDP tables are incomplete
+ *             or wrong).
  * @late_init: used to initialize flash parameters that are not declared in the
  *             JESD216 SFDP standard, or where SFDP tables not defined at all.
  *
@@ -2760,13 +2759,12 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
 
 /**
  * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings
- * after SFDP has been parsed (is also called for SPI NORs that do not
- * support RDSFDP).
+ * after SFDP has been parsed.
  * @nor:	pointer to a 'struct spi_nor'
  *
  * Typically used to tweak various parameters that could not be extracted by
- * other means (i.e. when information provided by the SFDP/flash_info tables
- * are incomplete or wrong).
+ * other means (i.e. when information provided by the SFDP tables are incomplete
+ * or wrong).
  */
 static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
 				     struct spi_nor_flash_parameter *params)
@@ -2901,9 +2899,10 @@ static int spi_nor_init_params(struct spi_nor *nor,
 		} else {
 			memcpy(params, &sfdp_params, sizeof(*params));
 		}
+
+		spi_nor_post_sfdp_fixups(nor, params);
 	}
 
-	spi_nor_post_sfdp_fixups(nor, params);
 	spi_nor_late_init_fixups(nor, params);
 
 	return 0;
-- 
2.34.1

Re: [PATCH v2 5/6] mtd: spi-nor: Call spi_nor_post_sfdp_fixups() only after spi_nor_parse_sfdp()

[Tudor Ambarus]

On 4/25/24 05:52, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> spi_nor_post_sfdp_fixups() was called regardless of if
> spi_nor_parse_sfdp() had been called or not. late_init() should be
> instead used to initialize the parameters that are not defined in SFDP.
> 
> Ideally spi_nor_post_sfdp_fixups() is called only after successful parse
> of SFDP. However, in case SFDP support is disabled by .config, that can
> break current functionality. Therefore, we would call it after
> spi_nor_parse_sfdp() regardless of its return value.
> 
> This patch follows the upstream linux commit:
> 5273cc6df984("mtd: spi-nor: core: Call spi_nor_post_sfdp_fixups() only
> when SFDP is defined")

this shall be the first information in the commit message.

Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index fda879f3a3..ee968c10e4 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -190,11 +190,10 @@ struct sfdp_bfpt {
>  /**
>   * struct spi_nor_fixups - SPI NOR fixup hooks
>   * @post_bfpt: called after the BFPT table has been parsed
> - * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
> - *             that do not support RDSFDP). Typically used to tweak various
> - *             parameters that could not be extracted by other means (i.e.
> - *             when information provided by the SFDP/flash_info tables are
> - *             incomplete or wrong).
> + * @post_sfdp: called after SFDP has been parsed. Typically used to tweak
> + *             various parameters that could not be extracted by other means
> + *             (i.e. when information provided by the SFDP tables are incomplete
> + *             or wrong).
>   * @late_init: used to initialize flash parameters that are not declared in the
>   *             JESD216 SFDP standard, or where SFDP tables not defined at all.
>   *
> @@ -2760,13 +2759,12 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>  
>  /**
>   * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings
> - * after SFDP has been parsed (is also called for SPI NORs that do not
> - * support RDSFDP).
> + * after SFDP has been parsed.
>   * @nor:	pointer to a 'struct spi_nor'
>   *
>   * Typically used to tweak various parameters that could not be extracted by
> - * other means (i.e. when information provided by the SFDP/flash_info tables
> - * are incomplete or wrong).
> + * other means (i.e. when information provided by the SFDP tables are incomplete
> + * or wrong).
>   */
>  static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
>  				     struct spi_nor_flash_parameter *params)
> @@ -2901,9 +2899,10 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  		} else {
>  			memcpy(params, &sfdp_params, sizeof(*params));
>  		}
> +
> +		spi_nor_post_sfdp_fixups(nor, params);
>  	}
>  
> -	spi_nor_post_sfdp_fixups(nor, params);
>  	spi_nor_late_init_fixups(nor, params);
>  
>  	return 0;

[PATCH v2 6/6] mtd: spi-nor: Set ECC unit size to MTD writesize in Infineon SEMPER flashes

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The Infineon SEMPER NOR flash family uses 2-bit ECC by default with each
ECC block being 16 bytes. Under this scheme multi-pass programming to an
ECC block is not allowed. Set the writesize to make sure multi-pass
programming is not attempted on the flash.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi/spi-nor-core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index ee968c10e4..7985ca70ff 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3456,6 +3456,13 @@ static void s25_late_init(struct spi_nor *nor,
 			  struct spi_nor_flash_parameter *params)
 {
 	nor->setup = s25_s28_setup;
+
+	/*
+	 * Programming is supported only in 16-byte ECC data unit granularity.
+	 * Byte-programming, bit-walking, or multiple program operations to the
+	 * same ECC data unit without an erase are not allowed.
+	 */
+	params->writesize = 16;
 }
 
 static int s25_s28_post_bfpt_fixup(struct spi_nor *nor,
@@ -3620,6 +3627,13 @@ static void s28hx_t_late_init(struct spi_nor *nor,
 {
 	nor->octal_dtr_enable = spi_nor_cypress_octal_dtr_enable;
 	nor->setup = s25_s28_setup;
+
+	/*
+	 * Programming is supported only in 16-byte ECC data unit granularity.
+	 * Byte-programming, bit-walking, or multiple program operations to the
+	 * same ECC data unit without an erase are not allowed.
+	 */
+	params->writesize = 16;
 }
 
 static void s28hx_t_post_sfdp_fixup(struct spi_nor *nor,
-- 
2.34.1

Re: [PATCH v2 6/6] mtd: spi-nor: Set ECC unit size to MTD writesize in Infineon SEMPER flashes

[Tudor Ambarus]

On 4/25/24 05:52, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> The Infineon SEMPER NOR flash family uses 2-bit ECC by default with each
> ECC block being 16 bytes. Under this scheme multi-pass programming to an
> ECC block is not allowed. Set the writesize to make sure multi-pass
> programming is not attempted on the flash.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ee968c10e4..7985ca70ff 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -3456,6 +3456,13 @@ static void s25_late_init(struct spi_nor *nor,
>  			  struct spi_nor_flash_parameter *params)
>  {
>  	nor->setup = s25_s28_setup;
> +
> +	/*
> +	 * Programming is supported only in 16-byte ECC data unit granularity.
> +	 * Byte-programming, bit-walking, or multiple program operations to the
> +	 * same ECC data unit without an erase are not allowed.
> +	 */
> +	params->writesize = 16;
>  }
>  
>  static int s25_s28_post_bfpt_fixup(struct spi_nor *nor,
> @@ -3620,6 +3627,13 @@ static void s28hx_t_late_init(struct spi_nor *nor,
>  {
>  	nor->octal_dtr_enable = spi_nor_cypress_octal_dtr_enable;
>  	nor->setup = s25_s28_setup;
> +
> +	/*
> +	 * Programming is supported only in 16-byte ECC data unit granularity.
> +	 * Byte-programming, bit-walking, or multiple program operations to the
> +	 * same ECC data unit without an erase are not allowed.
> +	 */
> +	params->writesize = 16;
>  }
>  
>  static void s28hx_t_post_sfdp_fixup(struct spi_nor *nor,