Skip to contentSkip to conversation

mtd: rawnand: brcmnand: driver and doc updates

PATCH Applied William Zhang 2 participants 11 messages Jul 6, 2023 – Jul 13, 2023

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v4 cover mtd: rawnand: brcmnand: driver and doc updates

This patch series include the accumulative updates and fixes for the brcmnand driver. The bcmbca SoC related updates are removed from v2 and will send separate patch series for that. v3 series adds a new patch for mtd oobsize fix.

Changes in v4:
- Move ACC_CONTROL_ECC*_SHIFT definitions out of the enum definition
- Add cc stable tag
- Update comment in the polling status and oob write function
- Update commit message

Changes in v3:
- Fix kernel test robot sparse warning:
drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse: expected unsigned int [usertype] data drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse: got restricted __be32 [usertype]

Changes in v2:
- Use driver static data for ECC level shift
- Handle the remaining unaligned oob data after the oob data write loop

William Zhang (5):
mtd: rawnand: brcmnand: Fix ECC level field setting for v7.2
controller
mtd: rawnand: brcmnand: Fix potential false time out warning
mtd: rawnand: brcmnand: Fix crash during the panic_write
mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob
write
mtd: rawnand: brcmnand: Fix mtd oobsize

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 119 +++++++++++++++--------
 1 file changed, 81 insertions(+), 38 deletions(-)
Signature · 1 line
2.37.3
#1 lore raw

PATCH v4 1/5 mtd: rawnand: brcmnand: Fix ECC level field setting for v7.2 controller

v7.2 controller has different ECC level field size and shift in the acc control register than its predecessor and successor controller. It needs to be set specifically.

Fixes decba6d47869 ("mtd: brcmnand: Add v7.2 controller support")

---

Changes in v4:
- Move ACC_CONTROL_ECC*_SHIFT definitions out of the enum definition
- Add cc stable tag

Changes in v3: None
Changes in v2:
- Use driver static data for ECC level shift

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 74 +++++++++++++-----------
 1 file changed, 41 insertions(+), 33 deletions(-)
drivers/mtd/nand/raw/brcmnand/brcmnand.c +41 -33
index 2e9c2e2d9c9f..9ea96911d16b 100644
@@ -272,6 +272,7 @@ struct brcmnand_controller {
 	const unsigned int	*page_sizes;
 	unsigned int		page_size_shift;
 	unsigned int		max_oob;
+	u32			ecc_level_shift;
 	u32			features;
 
 	/* for low-power standby/resume only */
@@ -596,6 +597,34 @@ enum {
 	INTFC_CTLR_READY		= BIT(31),
 };
 
+/***********************************************************************
+ * NAND ACC CONTROL bitfield
+ *
+ * Some bits have remained constant throughout hardware revision, while
+ * others have shifted around.
+ ***********************************************************************/
+
+/* Constant for all versions (where supported) */
+enum {
+	/* See BRCMNAND_HAS_CACHE_MODE */
+	ACC_CONTROL_CACHE_MODE				= BIT(22),
+
+	/* See BRCMNAND_HAS_PREFETCH */
+	ACC_CONTROL_PREFETCH				= BIT(23),
+
+	ACC_CONTROL_PAGE_HIT				= BIT(24),
+	ACC_CONTROL_WR_PREEMPT				= BIT(25),
+	ACC_CONTROL_PARTIAL_PAGE			= BIT(26),
+	ACC_CONTROL_RD_ERASED				= BIT(27),
+	ACC_CONTROL_FAST_PGM_RDIN			= BIT(28),
+	ACC_CONTROL_WR_ECC				= BIT(30),
+	ACC_CONTROL_RD_ECC				= BIT(31),
+};
+
+#define	ACC_CONTROL_ECC_SHIFT			16
+/* Only for v7.2 */
+#define	ACC_CONTROL_ECC_EXT_SHIFT		13
+
 static inline bool brcmnand_non_mmio_ops(struct brcmnand_controller *ctrl)
 {
 #if IS_ENABLED(CONFIG_MTD_NAND_BRCMNAND_BCMA)
@@ -737,6 +766,12 @@ static int brcmnand_revision_init(struct brcmnand_controller *ctrl)
 	else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
 		ctrl->features |= BRCMNAND_HAS_WP;
 
+	/* v7.2 has different ecc level shift in the acc register */
+	if (ctrl->nand_version == 0x0702)
+		ctrl->ecc_level_shift = ACC_CONTROL_ECC_EXT_SHIFT;
+	else
+		ctrl->ecc_level_shift = ACC_CONTROL_ECC_SHIFT;
+
 	return 0;
 }
 
@@ -931,30 +966,6 @@ static inline int brcmnand_cmd_shift(struct brcmnand_controller *ctrl)
 	return 0;
 }
 
-/***********************************************************************
- * NAND ACC CONTROL bitfield
- *
- * Some bits have remained constant throughout hardware revision, while
- * others have shifted around.
- ***********************************************************************/
-
-/* Constant for all versions (where supported) */
-enum {
-	/* See BRCMNAND_HAS_CACHE_MODE */
-	ACC_CONTROL_CACHE_MODE				= BIT(22),
-
-	/* See BRCMNAND_HAS_PREFETCH */
-	ACC_CONTROL_PREFETCH				= BIT(23),
-
-	ACC_CONTROL_PAGE_HIT				= BIT(24),
-	ACC_CONTROL_WR_PREEMPT				= BIT(25),
-	ACC_CONTROL_PARTIAL_PAGE			= BIT(26),
-	ACC_CONTROL_RD_ERASED				= BIT(27),
-	ACC_CONTROL_FAST_PGM_RDIN			= BIT(28),
-	ACC_CONTROL_WR_ECC				= BIT(30),
-	ACC_CONTROL_RD_ECC				= BIT(31),
-};
-
 static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
 {
 	if (ctrl->nand_version == 0x0702)
@@ -967,18 +978,15 @@ static inline u32 brcmnand_spare_area_mask(struct brcmnand_controller *ctrl)
 		return GENMASK(4, 0);
 }
 
-#define NAND_ACC_CONTROL_ECC_SHIFT	16
-#define NAND_ACC_CONTROL_ECC_EXT_SHIFT	13
-
 static inline u32 brcmnand_ecc_level_mask(struct brcmnand_controller *ctrl)
 {
 	u32 mask = (ctrl->nand_version >= 0x0600) ? 0x1f : 0x0f;
 
-	mask <<= NAND_ACC_CONTROL_ECC_SHIFT;
+	mask <<= ACC_CONTROL_ECC_SHIFT;
 
 	/* v7.2 includes additional ECC levels */
-	if (ctrl->nand_version >= 0x0702)
-		mask |= 0x7 << NAND_ACC_CONTROL_ECC_EXT_SHIFT;
+	if (ctrl->nand_version == 0x0702)
+		mask |= 0x7 << ACC_CONTROL_ECC_EXT_SHIFT;
 
 	return mask;
 }
@@ -992,8 +1000,8 @@ static void brcmnand_set_ecc_enabled(struct brcmnand_host *host, int en)
 
 	if (en) {
 		acc_control |= ecc_flags; /* enable RD/WR ECC */
-		acc_control |= host->hwcfg.ecc_level
-			       << NAND_ACC_CONTROL_ECC_SHIFT;
+		acc_control &= ~brcmnand_ecc_level_mask(ctrl);
+		acc_control |= host->hwcfg.ecc_level << ctrl->ecc_level_shift;
 	} else {
 		acc_control &= ~ecc_flags; /* disable RD/WR ECC */
 		acc_control &= ~brcmnand_ecc_level_mask(ctrl);
@@ -2561,7 +2569,7 @@ static int brcmnand_set_cfg(struct brcmnand_host *host,
 	tmp &= ~brcmnand_ecc_level_mask(ctrl);
 	tmp &= ~brcmnand_spare_area_mask(ctrl);
 	if (ctrl->nand_version >= 0x0302) {
-		tmp |= cfg->ecc_level << NAND_ACC_CONTROL_ECC_SHIFT;
+		tmp |= cfg->ecc_level << ctrl->ecc_level_shift;
 		tmp |= cfg->spare_area_size;
 	}
 	nand_writereg(ctrl, acc_control_offs, tmp);
Signature · 1 line
2.37.3
maintainer Applied
#2 lore raw

Re: [PATCH v4 1/5] mtd: rawnand: brcmnand: Fix ECC level field setting for v7.2 controller

Quoted 8 lines · William Zhang

v7.2 controller has different ECC level field size and shift in the acc control register than its predecessor and successor controller. It needs to be set specifically.

Fixes decba6d47869 ("mtd: brcmnand: Add v7.2 controller support")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

#3 lore raw

PATCH v4 2/5 mtd: rawnand: brcmnand: Fix potential false time out warning

In reply to William Zhang

If system is busy during the command status polling function, the driver may not get the chance to poll the status register till the end of time out and return the premature status. Do a final check after time out happens to ensure reading the correct status.

Fixes 9d2ee0a60b8b ("mtd: nand: brcmnand: Check flash #WP pin status before nand erase/program")

---

Changes in v4:
- Update comment in the polling status function
- Add cc stable tag

Changes in v3: None
Changes in v2: None

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mtd/nand/raw/brcmnand/brcmnand.c +8 -0
index 9ea96911d16b..9a373a10304d 100644
@@ -1080,6 +1080,14 @@ static int bcmnand_ctrl_poll_status(struct brcmnand_controller *ctrl,
 		cpu_relax();
 	} while (time_after(limit, jiffies));
 
+	/*
+	 * do a final check after time out in case the CPU was busy and the driver
+	 * did not get enough time to perform the polling to avoid false alarms
+	 */
+	val = brcmnand_read_reg(ctrl, BRCMNAND_INTFC_STATUS);
+	if ((val & mask) == expected_val)
+		return 0;
+
 	dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
 		 expected_val, val & mask);
 
Signature · 1 line
2.37.3
maintainer Applied
#4 lore raw

Re: [PATCH v4 2/5] mtd: rawnand: brcmnand: Fix potential false time out warning

Quoted 9 lines · William Zhang

If system is busy during the command status polling function, the driver may not get the chance to poll the status register till the end of time out and return the premature status. Do a final check after time out happens to ensure reading the correct status.

Fixes 9d2ee0a60b8b ("mtd: nand: brcmnand: Check flash #WP pin status before nand erase/program")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

#5 lore raw

PATCH v4 3/5 mtd: rawnand: brcmnand: Fix crash during the panic_write

In reply to William Zhang

When executing a NAND command within the panic write path, wait for any pending command instead of calling BUG_ON to avoid crashing while already crashing.

Fixes 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")

---

Changes in v4:
- Update commit message
- Add cc stable tag

Changes in v3: None
Changes in v2: None

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/mtd/nand/raw/brcmnand/brcmnand.c +11 -1
index 9a373a10304d..b2c6396060db 100644
@@ -1608,7 +1608,17 @@ static void brcmnand_send_cmd(struct brcmnand_host *host, int cmd)
 
 	dev_dbg(ctrl->dev, "send native cmd %d addr 0x%llx\n", cmd, cmd_addr);
 
-	BUG_ON(ctrl->cmd_pending != 0);
+	/*
+	 * If we came here through _panic_write and there is a pending
+	 * command, try to wait for it. If it times out, rather than
+	 * hitting BUG_ON, just return so we don't crash while crashing.
+	 */
+	if (oops_in_progress) {
+		if (ctrl->cmd_pending &&
+			bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, NAND_CTRL_RDY, 0))
+			return;
+	} else
+		BUG_ON(ctrl->cmd_pending != 0);
 	ctrl->cmd_pending = cmd;
 
 	ret = bcmnand_ctrl_poll_status(ctrl, NAND_CTRL_RDY, NAND_CTRL_RDY, 0);
Signature · 1 line
2.37.3
maintainer Applied
#6 lore raw

Re: [PATCH v4 3/5] mtd: rawnand: brcmnand: Fix crash during the panic_write

Quoted 10 lines · William Zhang

When executing a NAND command within the panic write path, wait for any pending command instead of calling BUG_ON to avoid crashing while already crashing.

Fixes 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

#7 lore raw

PATCH v4 4/5 mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write

In reply to William Zhang

When the oob buffer length is not in multiple of words, the oob write function does out-of-bounds read on the oob source buffer at the last iteration. Fix that by always checking length limit on the oob buffer read and fill with 0xff when reaching the end of the buffer to the oob registers.

Fixes 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")

---

Changes in v4:
- Add comments in the write_oob_to_regs function
- Add cc stable tag

Changes in v3:
- Fix kernel test robot sparse warning:
drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse: expected unsigned int [usertype] data drivers/mtd/nand/raw/brcmnand/brcmnand.c:1500:54: sparse: got restricted __be32 [usertype]

Changes in v2:
- Handle the remaining unaligned oob data after the oob data write loop

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/brcmnand/brcmnand.c +16 -2
index b2c6396060db..71d0ba652bee 100644
@@ -1477,19 +1477,33 @@ static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
 			     const u8 *oob, int sas, int sector_1k)
 {
 	int tbytes = sas << sector_1k;
-	int j;
+	int j, k = 0;
+	u32 last = 0xffffffff;
+	u8 *plast = (u8 *)&last;
 
 	/* Adjust OOB values for 1K sector size */
 	if (sector_1k && (i & 0x01))
 		tbytes = max(0, tbytes - (int)ctrl->max_oob);
 	tbytes = min_t(int, tbytes, ctrl->max_oob);
 
-	for (j = 0; j < tbytes; j += 4)
+	/*
+	 * tbytes may not be multiple of words. Make sure we don't read out of
+	 * the boundary and stop at last word.
+	 */
+	for (j = 0; (j + 3) < tbytes; j += 4)
 		oob_reg_write(ctrl, j,
 				(oob[j + 0] << 24) |
 				(oob[j + 1] << 16) |
 				(oob[j + 2] <<  8) |
 				(oob[j + 3] <<  0));
+
+	/* handle the remaing bytes */
+	while (j < tbytes)
+		plast[k++] = oob[j++];
+
+	if (tbytes & 0x3)
+		oob_reg_write(ctrl, (tbytes & ~0x3), (__force u32)cpu_to_be32(last));
+
 	return tbytes;
 }
 
Signature · 1 line
2.37.3
maintainer Applied
#8 lore raw

Re: [PATCH v4 4/5] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write

Quoted 10 lines · William Zhang

When the oob buffer length is not in multiple of words, the oob write function does out-of-bounds read on the oob source buffer at the last iteration. Fix that by always checking length limit on the oob buffer read and fill with 0xff when reaching the end of the buffer to the oob registers.

Fixes 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

#9 lore raw

PATCH v4 5/5 mtd: rawnand: brcmnand: Fix mtd oobsize

In reply to William Zhang

brcmnand controller can only access the flash spare area up to certain bytes based on the ECC level. It can be less than the actual flash spare area size. For example, for many NAND chip supporting ECC BCH-8, it has 226 bytes spare area. But controller can only uses 218 bytes. So brcmand driver overrides the mtd oobsize with the controller's accessible spare area size. When the nand base driver utilizes the nand_device object, it resets the oobsize back to the actual flash spare aprea size from nand_memory_organization structure and controller may not able to access all the oob area as mtd advises.

This change fixes the issue by overriding the oobsize in the nand_memory_organization structure to the controller's accessible spare area size.

Fixes a7ab085d7c16 ("mtd: rawnand: Initialize the nand_device object")

---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/mtd/nand/raw/brcmnand/brcmnand.c +5 -2
index 71d0ba652bee..39661e23d7d4 100644
@@ -2652,6 +2652,8 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
 	struct nand_chip *chip = &host->chip;
 	const struct nand_ecc_props *requirements =
 		nanddev_get_ecc_requirements(&chip->base);
+	struct nand_memory_organization *memorg =
+		nanddev_get_memorg(&chip->base);
 	struct brcmnand_controller *ctrl = host->ctrl;
 	struct brcmnand_cfg *cfg = &host->hwcfg;
 	char msg[128];
@@ -2673,10 +2675,11 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
 	if (cfg->spare_area_size > ctrl->max_oob)
 		cfg->spare_area_size = ctrl->max_oob;
 	/*
-	 * Set oobsize to be consistent with controller's spare_area_size, as
-	 * the rest is inaccessible.
+	 * Set mtd and memorg oobsize to be consistent with controller's
+	 * spare_area_size, as the rest is inaccessible.
 	 */
 	mtd->oobsize = cfg->spare_area_size * (mtd->writesize >> FC_SHIFT);
+	memorg->oobsize = mtd->oobsize;
 
 	cfg->device_size = mtd->size;
 	cfg->block_size = mtd->erasesize;
Signature · 1 line
2.37.3

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

maintainer Applied
#10 lore raw

Re: [PATCH v4 5/5] mtd: rawnand: brcmnand: Fix mtd oobsize

Quoted 10 lines · William Zhang

brcmnand controller can only access the flash spare area up to certain bytes based on the ECC level. It can be less than the actual flash spare area size. For example, for many NAND chip supporting ECC BCH-8, it has 226 bytes spare area. But controller can only uses 218 bytes. So brcmand driver overrides the mtd oobsize with the controller's accessible spare area size. When the nand base driver utilizes the nand_device object, it resets the oobsize back to the actual flash spare aprea size from nand_memory_organization structure and controller may not able to access all the oob area as mtd advises.

Quoted 6 lines

This change fixes the issue by overriding the oobsize in the nand_memory_organization structure to the controller's accessible spare area size.

Fixes a7ab085d7c16 ("mtd: rawnand: Initialize the nand_device object")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel