Re: [PATCH v2 1/6] mtd: ubi: Do not zero out EC and VID on ECC-ed NOR flashes
On 4/29/24 15:17, Pratyush Yadav wrote:
> On Thu, Apr 25 2024, tkuw584...@gmail.com wrote:
>
>> From: Takahiro Kuwano
>
> 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.
Re: [PATCH v2 3/6] mtd: spi-nor: Check nor->info before setting macronix_octal_fixups
On 4/25/24 05:52, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> 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
> Cc: JaimeLiao
> ---
> 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 = _octal_fixups;
> + if (JEDEC_MFR(nor->info) == SNOR_MFR_MACRONIX &&
> + nor->info->flags & SPI_NOR_OCTAL_DTR_READ)
> + nor->fixups = _octal_fixups;
we still have the mfr checks in u-boot, sigh.
sounds sane:
Acked-by: Tudor Ambarus
> #endif /* SPI_FLASH_MACRONIX */
> }
>
Re: [PATCH v2 5/6] mtd: spi-nor: Call spi_nor_post_sfdp_fixups() only after spi_nor_parse_sfdp()
On 4/25/24 05:52, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> 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
>
> Signed-off-by: Takahiro Kuwano
> ---
> 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, _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;
Re: [PATCH v2 6/6] mtd: spi-nor: Set ECC unit size to MTD writesize in Infineon SEMPER flashes
On 4/25/24 05:52, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> 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
Acked-by: Tudor Ambarus
> ---
> 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,
Re: [PATCH v2 4/6] mtd: spi-nor: Replace default_init() hook with late_init()
On 4/25/24 05:52, tkuw584...@gmail.com wrote: > From: Takahiro Kuwano > > 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 > Signed-off-by: Takahiro Kuwano Acked-by: Tudor Ambarus
Re: [PATCH 3/4] mtd: spi-nor-core: Rework default_init() to take flash_parameter
On 4/15/24 08:09, Takahiro Kuwano wrote: > Hi Tudor, Hi! > > On 4/15/2024 3:47 PM, Tudor Ambarus wrote: >> >> >> On 4/15/24 05:33, tkuw584...@gmail.com wrote: >>> From: Takahiro Kuwano >>> >>> default_init() fixup hook should be used to initialize flash parameters >>> when its information is not provided in SFDP. To support that case, it >>> needs to take flash_parameter structure like as other hooks. >>> >>> Signed-off-by: Takahiro Kuwano >>> --- >> >> I'd like to get rid of the default_init hook, let's not extend it if >> possible. Can you use the late_init hook instead? >> > It looks easy to migrate from default_init to late_init so I will do it. > Could you provide the links to related discussion in Linux MTD side so that > I can summarize it in commit message? > I can't, I don't remember if I brought this up or when, but I can explain why. 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. Feel free to use this in the commit message if you think it helps. Cheers, ta
Re: [PATCH 0/4] mtd: Make sure UBIFS does not do multi-pass page programming on flashes that don't support it
On 4/15/24 05:33, tkuw584...@gmail.com wrote: > From: Takahiro Kuwano > > This series is equivalent to the one for Linux MTD submitted by > Pratyush Yadav. > > https://patchwork.ozlabs.org/project/linux-mtd/list/?series=217759=* Ah, I see you specified it here. I'd argue it's better to mention it in the commit message itself, it spares people searching on the ml archive. > > Takahiro Kuwano (4): > 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-core: Rework default_init() to take flash_parameter > mtd: spi-nor: Set ECC unit size to MTD writesize in Infineon SEMPER > flashes > > drivers/mtd/spi/spi-nor-core.c | 45 +- > drivers/mtd/ubi/build.c| 4 +-- > drivers/mtd/ubi/io.c | 9 ++- > include/linux/mtd/spi-nor.h| 1 + > 4 files changed, 44 insertions(+), 15 deletions(-) >
Re: [PATCH 4/4] mtd: spi-nor: Set ECC unit size to MTD writesize in Infineon SEMPER flashes
On 4/15/24 05:33, tkuw584...@gmail.com wrote:
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 8f371a5213..773afd4040 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -3459,6 +3459,13 @@ static void s25_default_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;
> }
Use late_init() please. Looks good.
ta
Re: [PATCH 2/4] mtd: spi-nor: Allow flashes to specify MTD writesize
Hi, Takahiro!
On 4/15/24 05:33, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> 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.
>
> Signed-off-by: Takahiro Kuwano
Please specify when a patch follows linux upstream. This follows the
following upstream linux commit:
afd473e85827 ("mtd: spi-nor: core: Allow flashes to specify MTD writesize")
Acked-by: Tudor Ambarus
Re: [PATCH 3/4] mtd: spi-nor-core: Rework default_init() to take flash_parameter
On 4/15/24 05:33, tkuw584...@gmail.com wrote: > From: Takahiro Kuwano > > default_init() fixup hook should be used to initialize flash parameters > when its information is not provided in SFDP. To support that case, it > needs to take flash_parameter structure like as other hooks. > > Signed-off-by: Takahiro Kuwano > --- I'd like to get rid of the default_init hook, let's not extend it if possible. Can you use the late_init hook instead? Cheers, ta
Re: [PATCH 1/4] mtd: ubi: Do not zero out EC and VID on ECC-ed NOR flashes
Hi, Takahiro,
On 4/15/24 05:33, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> 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.
>
I'm not familiar with the u-boot requirements, but I think a good
practice would be to specify if/when a commit follows the upstream linux
implementation. It helps reviewers, gives a peace of mind to the
maintainer(s), and gives credit to the author. If something breaks all
parties can be involved.
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
Cheers,
ta
Re: [PATCH v2 2/2] mtd: spi-nor-core: Make CFRx reg fields generic
On 1/20/23 03:28, tkuw584...@gmail.com wrote: From: Takahiro Kuwano Cypress defines two flavors of configuration registers, volatile and non volatile, and both use the same bit fields. Rename the bitfields in the configuration registers so that they can be used for both flavors. Suggested-by: Tudor Ambarus Signed-off-by: Takahiro Kuwano Reviewed-by: Tudor Ambarus --- drivers/mtd/spi/spi-nor-core.c | 8 include/linux/mtd/spi-nor.h| 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 1ea8363d9f89..a3198253ca63 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3373,7 +3373,7 @@ static int spi_nor_cypress_octal_dtr_enable(struct spi_nor *nor) if (ret) return ret; - buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24; + buf = SPINOR_REG_CYPRESS_CFR2_MEMLAT_11_24; op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1), SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_CYPRESS_CFR2V, 1), SPI_MEM_OP_NO_DUMMY, @@ -3396,7 +3396,7 @@ static int spi_nor_cypress_octal_dtr_enable(struct spi_nor *nor) if (ret) return ret; - buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN; + buf = SPINOR_REG_CYPRESS_CFR5_OCT_DTR_EN; op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1), SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_CYPRESS_CFR5V, 1), SPI_MEM_OP_NO_DUMMY, @@ -3444,7 +3444,7 @@ static int s28hx_t_setup(struct spi_nor *nor, const struct flash_info *info, if (ret) return ret; - if (!(buf & SPINOR_REG_CYPRESS_CFR3V_UNISECT)) + if (!(buf & SPINOR_REG_CYPRESS_CFR3_UNISECT)) nor->erase = s28hx_t_erase_non_uniform; return spi_nor_default_setup(nor, info, params); @@ -3511,7 +3511,7 @@ static int s28hx_t_post_bfpt_fixup(struct spi_nor *nor, if (ret) return ret; - if (buf & SPINOR_REG_CYPRESS_CFR3V_PGSZ) + if (buf & SPINOR_REG_CYPRESS_CFR3_PGSZ) params->page_size = 512; else params->page_size = 256; diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 2fb4595fc756..605cddef4d06 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -189,15 +189,15 @@ #define SPINOR_OP_WR_ANY_REG 0x71/* Write any register */ #define SPINOR_OP_S28_SE_4K 0x21 #define SPINOR_REG_CYPRESS_CFR2V 0x0083 -#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24 0xb +#define SPINOR_REG_CYPRESS_CFR2_MEMLAT_11_24 0xb #define SPINOR_REG_CYPRESS_CFR3V 0x0084 -#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */ -#define SPINOR_REG_CYPRESS_CFR3V_UNISECT BIT(3) /* Uniform sector mode */ +#define SPINOR_REG_CYPRESS_CFR3_PGSZ BIT(4) /* Page size. */ +#define SPINOR_REG_CYPRESS_CFR3_UNISECTBIT(3) /* Uniform sector mode */ #define SPINOR_REG_CYPRESS_CFR5V 0x0086 #define SPINOR_REG_CYPRESS_CFR5_BIT6 BIT(6) #define SPINOR_REG_CYPRESS_CFR5_DDR BIT(1) #define SPINOR_REG_CYPRESS_CFR5_OPI BIT(0) -#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN\ +#define SPINOR_REG_CYPRESS_CFR5_OCT_DTR_EN \ (SPINOR_REG_CYPRESS_CFR5_BIT6 | SPINOR_REG_CYPRESS_CFR5_DDR | \ SPINOR_REG_CYPRESS_CFR5_OPI) #define SPINOR_OP_CYPRESS_RD_FAST 0xee
Re: [PATCH v2 1/2] mtd: spi-nor-core: Consider reserved bits in CFR5 register
On 1/20/23 03:28, tkuw584...@gmail.com wrote:
From: Takahiro Kuwano
CFR5[6] is reserved bit and must be always 1. Set it to comply with flash
requirements. While fixing SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN definition,
stop using magic numbers and describe the missing bit fields in CFR5
register. This is useful for both readability and future possible addition
of Octal STR mode support.
Fixes: ea9a22f7e79c ("mtd: spi-nor-core: Add support for Cypress Semper flash")
Suggested-by: Tudor Ambarus
Signed-off-by: Takahiro Kuwano
Reviewed-by: Tudor Ambarus
---
include/linux/mtd/spi-nor.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 30f15452aa68..2fb4595fc756 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -194,7 +194,12 @@
#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
#define SPINOR_REG_CYPRESS_CFR3V_UNISECT BIT(3) /* Uniform sector mode */
#define SPINOR_REG_CYPRESS_CFR5V 0x0086
-#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN0x3
+#define SPINOR_REG_CYPRESS_CFR5_BIT6 BIT(6)
+#define SPINOR_REG_CYPRESS_CFR5_DDRBIT(1)
+#define SPINOR_REG_CYPRESS_CFR5_OPIBIT(0)
+#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN\
+ (SPINOR_REG_CYPRESS_CFR5_BIT6 | SPINOR_REG_CYPRESS_CFR5_DDR | \
+SPINOR_REG_CYPRESS_CFR5_OPI)
#define SPINOR_OP_CYPRESS_RD_FAST 0xee
/* Supported SPI protocols */
[PATCH 1/3] Revert "sama5d3: Fix Galois Field Table offsets"
This reverts commit 786f888b743e9b83c9095cb9b5548ebe2e29afc5.
Looks like the datasheet at
https://ww1.microchip.com/downloads/en/DeviceDoc/SAMA5D3-Series-Data-sheet-DS60001609b.pdf
is wrong, and the testing was poorly done, because the PMECC did not raise
any error, but also didn't correct any bitflips. Restoring the offsets
as they were before, makes the PMECC on sama5d3x capable of correcting
bitflips.
Fixes: 786f888b74 ("sama5d3: Fix Galois Field Table offsets")
Signed-off-by: Tudor Ambarus
---
arch/arm/mach-at91/include/mach/sama5d3.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-at91/include/mach/sama5d3.h
b/arch/arm/mach-at91/include/mach/sama5d3.h
index f4f05676f7..83f18a8148 100644
--- a/arch/arm/mach-at91/include/mach/sama5d3.h
+++ b/arch/arm/mach-at91/include/mach/sama5d3.h
@@ -190,8 +190,8 @@
/*
* PMECC table in ROM
*/
-#define ATMEL_PMECC_INDEX_OFFSET_512 0x8000
-#define ATMEL_PMECC_INDEX_OFFSET_1024 0x1
+#define ATMEL_PMECC_INDEX_OFFSET_512 0x1
+#define ATMEL_PMECC_INDEX_OFFSET_1024 0x18000
/*
* SAMA5D3 specific prototypes
--
2.25.1
[PATCH 2/3] configs: sam9x60ek: Enable NAND on mmc defconfig
Enable NAND on mmc defconfig for greater flexibility and for consistency reasons. All our other boards that have a NAND flash integrated, enable NAND regardless of the type of the defconfig. Signed-off-by: Tudor Ambarus --- configs/sam9x60ek_mmc_defconfig | 7 +++ 1 file changed, 7 insertions(+) diff --git a/configs/sam9x60ek_mmc_defconfig b/configs/sam9x60ek_mmc_defconfig index e97b20aeb3..e5edf45fee 100644 --- a/configs/sam9x60ek_mmc_defconfig +++ b/configs/sam9x60ek_mmc_defconfig @@ -25,6 +25,8 @@ CONFIG_CMD_BOOTZ=y CONFIG_CMD_DM=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y +CONFIG_CMD_NAND=y +CONFIG_CMD_NAND_TRIMFFS=y # CONFIG_CMD_SETEXPR is not set CONFIG_CMD_DHCP=y CONFIG_CMD_MII=y @@ -48,6 +50,11 @@ CONFIG_MICROCHIP_FLEXCOM=y CONFIG_DM_MMC=y CONFIG_MMC_SDHCI=y CONFIG_MMC_SDHCI_ATMEL=y +CONFIG_MTD=y +CONFIG_MTD_RAW_NAND=y +CONFIG_NAND_ATMEL=y +CONFIG_ATMEL_NAND_HW_PMECC=y +CONFIG_PMECC_CAP=8 CONFIG_PHY_MICREL=y CONFIG_DM_ETH=y CONFIG_MACB=y -- 2.25.1
[PATCH 3/3] nand: atmel: Correct bitflips in erased pages
From: "Kai Stuhlemmer (ebee Engineering)"
Not correcting anything in case of empty ECC data area
is not an appropriate strategy, because an uncorrected bit-flip
in an empty sector may cause upper layers (namely UBI) fail to work
properly. Therefore the approach chosen in Linux kernel and other
u-boot mtd drivers has been adopted, where a heuristic implemented
by nand_check_erased_ecc_chunk() is used in order to detect and
correct empty sectors.
Tested with sama5d3_xplained and sam9x60-ek.
Signed-off-by: Kai Stuhlemmer (ebee Engineering)
Tested-by: Tudor Ambarus
[ta: reorder if conditions, change commit subject, s/uint8_t/u8.]
Signed-off-by: Tudor Ambarus
---
drivers/mtd/nand/raw/atmel_nand.c | 37 +++
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel_nand.c
b/drivers/mtd/nand/raw/atmel_nand.c
index abc432c862..6541c3bea8 100644
--- a/drivers/mtd/nand/raw/atmel_nand.c
+++ b/drivers/mtd/nand/raw/atmel_nand.c
@@ -493,21 +493,9 @@ static int pmecc_correction(struct mtd_info *mtd, u32
pmecc_stat, uint8_t *buf,
{
struct nand_chip *nand_chip = mtd_to_nand(mtd);
struct atmel_nand_host *host = nand_get_controller_data(nand_chip);
- int i, err_nbr, eccbytes;
- uint8_t *buf_pos;
-
- /* SAMA5D4 PMECC IP can correct errors for all 0xff page */
- if (host->pmecc_version >= PMECC_VERSION_SAMA5D4)
- goto normal_check;
-
- eccbytes = nand_chip->ecc.bytes;
- for (i = 0; i < eccbytes; i++)
- if (ecc[i] != 0xff)
- goto normal_check;
- /* Erased page, return OK */
- return 0;
+ int i, err_nbr;
+ u8 *buf_pos, *ecc_pos;
-normal_check:
for (i = 0; i < host->pmecc_sector_number; i++) {
err_nbr = 0;
if (pmecc_stat & 0x1) {
@@ -518,15 +506,26 @@ normal_check:
pmecc_get_sigma(mtd);
err_nbr = pmecc_err_location(mtd);
- if (err_nbr == -1) {
+ if (err_nbr >= 0) {
+ pmecc_correct_data(mtd, buf_pos, ecc, i,
+ host->pmecc_bytes_per_sector,
+ err_nbr);
+ } else if (host->pmecc_version < PMECC_VERSION_SAMA5D4)
{
+ ecc_pos = ecc + i *
host->pmecc_bytes_per_sector;
+
+ err_nbr = nand_check_erased_ecc_chunk(
+ buf_pos, host->pmecc_sector_size,
+ ecc_pos, host->pmecc_bytes_per_sector,
+ NULL, 0, host->pmecc_corr_cap);
+ }
+
+ if (err_nbr < 0) {
dev_err(mtd->dev, "PMECC: Too many errors\n");
mtd->ecc_stats.failed++;
return -EBADMSG;
- } else {
- pmecc_correct_data(mtd, buf_pos, ecc, i,
- host->pmecc_bytes_per_sector, err_nbr);
- mtd->ecc_stats.corrected += err_nbr;
}
+
+ mtd->ecc_stats.corrected += err_nbr;
}
pmecc_stat >>= 1;
}
--
2.25.1
[PATCH 3/4] sama5d3: Fix Galois Field Table offsets
Offsets are described in the datasheet at section:
"11.4.4.2 NAND Flash Boot: PMECC Error Detection and Correction".
For testing I "injected" bit flips into u-boot NAND memory area,
and then read back. PMECC could not correct the errors. With the
offsets updated everything is fine.
Fixes: 3225f34e5c ("ARM: atmel: add sama5d3xek support")
Signed-off-by: Tudor Ambarus
---
arch/arm/mach-at91/include/mach/sama5d3.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-at91/include/mach/sama5d3.h
b/arch/arm/mach-at91/include/mach/sama5d3.h
index 83f18a8148..f4f05676f7 100644
--- a/arch/arm/mach-at91/include/mach/sama5d3.h
+++ b/arch/arm/mach-at91/include/mach/sama5d3.h
@@ -190,8 +190,8 @@
/*
* PMECC table in ROM
*/
-#define ATMEL_PMECC_INDEX_OFFSET_512 0x1
-#define ATMEL_PMECC_INDEX_OFFSET_1024 0x18000
+#define ATMEL_PMECC_INDEX_OFFSET_512 0x8000
+#define ATMEL_PMECC_INDEX_OFFSET_1024 0x1
/*
* SAMA5D3 specific prototypes
--
2.25.1
[PATCH 4/4] sam9x60.h: Fix Galois Field Table offsets
From: "Kai Stuhlemmer (ebee Engineering)" Because ATMEL_BASE_ROM is defined to 0x10, it already points to the begin of the index table for 512 byte sectors correction. Thus its offset must be zero and the index of the table for 1024 byte sectors must start at offset 0x8000. Signed-off-by: Kai Stuhlemmer (ebee Engineering) [ta: update commit message] Signed-off-by: Tudor Ambarus --- arch/arm/mach-at91/include/mach/sam9x60.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-at91/include/mach/sam9x60.h b/arch/arm/mach-at91/include/mach/sam9x60.h index b7f43226b7..c08d19c691 100644 --- a/arch/arm/mach-at91/include/mach/sam9x60.h +++ b/arch/arm/mach-at91/include/mach/sam9x60.h @@ -154,8 +154,8 @@ /* * PMECC table in ROM */ -#define ATMEL_PMECC_INDEX_OFFSET_512 0x8000 -#define ATMEL_PMECC_INDEX_OFFSET_1024 0x1 +#define ATMEL_PMECC_INDEX_OFFSET_512 0x +#define ATMEL_PMECC_INDEX_OFFSET_1024 0x8000 /* * SAM9X60 specific prototypes -- 2.25.1
[PATCH 2/4] configs: at91: Fix wrong definitions for CONFIG_PMECC_CAP
When CONFIG_ATMEL_NAND_HW_PMECC is set, CONFIG_PMECC_CAP defaults
to the value of 2. At the conversion to Kconfig for the PMECC config
values, some boards/defconfigs were wrongly configured.
Update CONFIG_PMECC_CAP to the PMECC_CAP value before the conversion.
Fixes: 49ad40298c ("ARM: at91: Convert SPL_GENERATE_ATMEL_PMECC_HEADER to
Kconfig")
Signed-off-by: Tudor Ambarus
---
configs/sama5d36ek_cmp_mmc_defconfig | 1 +
configs/sama5d36ek_cmp_nandflash_defconfig | 1 +
configs/sama5d36ek_cmp_spiflash_defconfig | 1 +
configs/sama5d3_xplained_mmc_defconfig | 1 +
configs/sama5d4ek_mmc_defconfig| 1 +
5 files changed, 5 insertions(+)
diff --git a/configs/sama5d36ek_cmp_mmc_defconfig
b/configs/sama5d36ek_cmp_mmc_defconfig
index 177b020469..32580ce8f0 100644
--- a/configs/sama5d36ek_cmp_mmc_defconfig
+++ b/configs/sama5d36ek_cmp_mmc_defconfig
@@ -48,6 +48,7 @@ CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
CONFIG_ATMEL_NAND_HW_PMECC=y
+CONFIG_PMECC_CAP=4
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/sama5d36ek_cmp_nandflash_defconfig
b/configs/sama5d36ek_cmp_nandflash_defconfig
index d38a0bea6d..478720f72f 100644
--- a/configs/sama5d36ek_cmp_nandflash_defconfig
+++ b/configs/sama5d36ek_cmp_nandflash_defconfig
@@ -47,6 +47,7 @@ CONFIG_GENERIC_ATMEL_MCI=y
CONFIG_MTD=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_PMECC_CAP=4
CONFIG_SPL_GENERATE_ATMEL_PMECC_HEADER=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
diff --git a/configs/sama5d36ek_cmp_spiflash_defconfig
b/configs/sama5d36ek_cmp_spiflash_defconfig
index fe1600b798..10227fe3bb 100644
--- a/configs/sama5d36ek_cmp_spiflash_defconfig
+++ b/configs/sama5d36ek_cmp_spiflash_defconfig
@@ -50,6 +50,7 @@ CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
CONFIG_ATMEL_NAND_HW_PMECC=y
+CONFIG_PMECC_CAP=4
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/sama5d3_xplained_mmc_defconfig
b/configs/sama5d3_xplained_mmc_defconfig
index dd36d809d0..d713af77b1 100644
--- a/configs/sama5d3_xplained_mmc_defconfig
+++ b/configs/sama5d3_xplained_mmc_defconfig
@@ -67,6 +67,7 @@ CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
CONFIG_ATMEL_NAND_HW_PMECC=y
+CONFIG_PMECC_CAP=4
CONFIG_DM_ETH=y
CONFIG_MACB=y
CONFIG_PINCTRL=y
diff --git a/configs/sama5d4ek_mmc_defconfig b/configs/sama5d4ek_mmc_defconfig
index 9aa37e5bae..c1ee3793c6 100644
--- a/configs/sama5d4ek_mmc_defconfig
+++ b/configs/sama5d4ek_mmc_defconfig
@@ -63,6 +63,7 @@ CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
CONFIG_ATMEL_NAND_HW_PMECC=y
+CONFIG_PMECC_CAP=8
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
--
2.25.1
[PATCH 1/4] configs: at91: Fix the involuntarily disablement of NAND PMECC
SPL_GENERATE_ATMEL_PMECC_HEADER selects:
ATMEL_NAND_HWECC [=y] && ATMEL_NAND_HW_PMECC [=y].
With the removal of SPL_GENERATE_ATMEL_PMECC_HEADER,
ATMEL_NAND_HW_PMECC and ATMEL_NAND_HWECC were no longer
selected. Also, when the SPL_GENERATE_ATMEL_PMECC_HEADER was removed,
the configs were not updated using savedefconfig, thus the
'commit d168bcb6fe39 ("configs: Resync with savedefconfig")'
further removes the CONFIG_PMECC_CAP value.
Update defconfigs and add CONFIG_ATMEL_NAND_HW_PMECC,
which selects ATMEL_NAND_HWECC, in order to restore NAND PMECC
support. Restore CONFIG_PMECC_CAP value.
Fixes: 57f76c2a47 ("configs: at91: remove SPL_GENERATE_ATMEL_PMECC_HEADER from
non-nand configs")
Signed-off-by: Tudor Ambarus
---
configs/at91sam9n12ek_mmc_defconfig | 1 +
configs/at91sam9n12ek_spiflash_defconfig| 1 +
configs/at91sam9x5ek_mmc_defconfig | 1 +
configs/at91sam9x5ek_spiflash_defconfig | 1 +
configs/sama5d36ek_cmp_mmc_defconfig| 1 +
configs/sama5d36ek_cmp_spiflash_defconfig | 1 +
configs/sama5d3_xplained_mmc_defconfig | 1 +
configs/sama5d3xek_mmc_defconfig| 2 ++
configs/sama5d3xek_spiflash_defconfig | 2 ++
configs/sama5d4_xplained_spiflash_defconfig | 2 ++
configs/sama5d4ek_mmc_defconfig | 1 +
configs/sama5d4ek_spiflash_defconfig| 2 ++
12 files changed, 16 insertions(+)
diff --git a/configs/at91sam9n12ek_mmc_defconfig
b/configs/at91sam9n12ek_mmc_defconfig
index fa802c3625..0ddf8fcb56 100644
--- a/configs/at91sam9n12ek_mmc_defconfig
+++ b/configs/at91sam9n12ek_mmc_defconfig
@@ -43,6 +43,7 @@ CONFIG_MTD=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/at91sam9n12ek_spiflash_defconfig
b/configs/at91sam9n12ek_spiflash_defconfig
index 997594720b..4af64d8ac2 100644
--- a/configs/at91sam9n12ek_spiflash_defconfig
+++ b/configs/at91sam9n12ek_spiflash_defconfig
@@ -45,6 +45,7 @@ CONFIG_MTD=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/at91sam9x5ek_mmc_defconfig
b/configs/at91sam9x5ek_mmc_defconfig
index 7f8fa22dc2..92b653f7d7 100644
--- a/configs/at91sam9x5ek_mmc_defconfig
+++ b/configs/at91sam9x5ek_mmc_defconfig
@@ -46,6 +46,7 @@ CONFIG_MTD=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/at91sam9x5ek_spiflash_defconfig
b/configs/at91sam9x5ek_spiflash_defconfig
index 8575a33943..ca3ae1fb62 100644
--- a/configs/at91sam9x5ek_spiflash_defconfig
+++ b/configs/at91sam9x5ek_spiflash_defconfig
@@ -48,6 +48,7 @@ CONFIG_MTD=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/sama5d36ek_cmp_mmc_defconfig
b/configs/sama5d36ek_cmp_mmc_defconfig
index 846f96b595..177b020469 100644
--- a/configs/sama5d36ek_cmp_mmc_defconfig
+++ b/configs/sama5d36ek_cmp_mmc_defconfig
@@ -47,6 +47,7 @@ CONFIG_MTD=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/sama5d36ek_cmp_spiflash_defconfig
b/configs/sama5d36ek_cmp_spiflash_defconfig
index 6a63a74162..fe1600b798 100644
--- a/configs/sama5d36ek_cmp_spiflash_defconfig
+++ b/configs/sama5d36ek_cmp_spiflash_defconfig
@@ -49,6 +49,7 @@ CONFIG_MTD=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_ATMEL=y
diff --git a/configs/sama5d3_xplained_mmc_defconfig
b/configs/sama5d3_xplained_mmc_defconfig
index ec117ee693..dd36d809d0 100644
--- a/configs/sama5d3_xplained_mmc_defconfig
+++ b/configs/sama5d3_xplained_mmc_defconfig
@@ -66,6 +66,7 @@ CONFIG_MTD=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
CONFIG_DM_ETH=y
CONFIG_MACB=y
CONFIG_PINCTRL=y
diff --git a/configs/sama5d3xek_mmc_defconfig b/configs/sama5d3xek_mmc_defconfig
index 50f97e68e1..dcb413 100644
--- a/configs/sama5d3xek_mmc_defconfig
+++ b/configs/sama5d3xek_mmc_defconfig
@@ -71,6 +71,8 @@ CONFIG_SYS_FLASH_CFI=y
CONFIG_MTD_RAW_NAND=y
# CONFIG_SYS_NAND_USE_FLASH_BBT is not set
CONFIG_NAND_ATMEL=y
+CONFIG_ATMEL_NAND_HW_PMECC=y
+CONFIG_PMECC_CAP=4
CONFIG_DM_SPI_FLASH=y
CONFIG_SF_DEFAULT_SPEED=3000
CONFIG_SPI_FLASH_
[PATCH 0/4] at91: Fix NAND PMECC on various SoCs/boards
Fix the Galois Field Table offsets for sama5d3 and sam9x60 SoCs. PMECC could not correct errors. Fix the involuntarily disablement of NAND PMECC on some boards, and fix some wrong definitions for CONFIG_PMECC_CAP. Tested on sam9x60ek and sama5d3_xplained. I/we "injected" bit flips into u-boot NAND memory area, and then read back. PMECC could not correct the errors. With these everything is fine. Kai Stuhlemmer (ebee Engineering) (1): sam9x60.h: Fix Galois Field Table offsets Tudor Ambarus (3): configs: at91: Fix the involuntarily disablement of NAND PMECC configs: at91: Fix wrong definitions for CONFIG_PMECC_CAP sama5d3: Fix Galois Field Table offsets arch/arm/mach-at91/include/mach/sam9x60.h | 4 ++-- arch/arm/mach-at91/include/mach/sama5d3.h | 4 ++-- configs/at91sam9n12ek_mmc_defconfig | 1 + configs/at91sam9n12ek_spiflash_defconfig| 1 + configs/at91sam9x5ek_mmc_defconfig | 1 + configs/at91sam9x5ek_spiflash_defconfig | 1 + configs/sama5d36ek_cmp_mmc_defconfig| 2 ++ configs/sama5d36ek_cmp_nandflash_defconfig | 1 + configs/sama5d36ek_cmp_spiflash_defconfig | 2 ++ configs/sama5d3_xplained_mmc_defconfig | 2 ++ configs/sama5d3xek_mmc_defconfig| 2 ++ configs/sama5d3xek_spiflash_defconfig | 2 ++ configs/sama5d4_xplained_spiflash_defconfig | 2 ++ configs/sama5d4ek_mmc_defconfig | 2 ++ configs/sama5d4ek_spiflash_defconfig| 2 ++ 15 files changed, 25 insertions(+), 4 deletions(-) -- 2.25.1
