diff mbox series

[v7] mtd: rawnand: brcmnand: Add support for getting ecc setting from strap

Message ID 20240227201253.31978-1-william.zhang@broadcom.com (mailing list archive)
State New, archived
Headers show
Series [v7] mtd: rawnand: brcmnand: Add support for getting ecc setting from strap | expand

Commit Message

William Zhang Feb. 27, 2024, 8:12 p.m. UTC
This is the update to the patch 12 in the original series [1] that was
applied to nand-next except this patch.

BCMBCA broadband SoC based board design does not specify ecc setting in
dts but rather use the SoC NAND strap info to obtain the ecc strength
and spare area size setting. Add brcm,nand-ecc-use-strap dts propety for
this purpose and update driver to support this option. However these two
options can not be used at the same time.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
Reviewed-by: David Regan <dregan@broadcom.com>

[1] https://lore.kernel.org/lkml/20240223034758.13753-1-william.zhang@broadcom.com/

---

Changes in v7:
- Simply the logic for sector size detection
- Update the error message

Changes in v6:
- Combine the ecc step size and ecc strength into one get function
- Treat it as error condition if both brcm,nand-ecc-use-strap and nand
ecc dts properties are set
- Add intermediate steps to get the sector size bitfield

Changes in v4:
- Update the comments for ecc setting selection

Changes in v2:
- Minor cosmetic fixes

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 83 ++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 6 deletions(-)

Comments

Miquel Raynal Feb. 29, 2024, 10:28 a.m. UTC | #1
Hi William,

william.zhang@broadcom.com wrote on Tue, 27 Feb 2024 12:12:52 -0800:

> This is the update to the patch 12 in the original series [1] that was
> applied to nand-next except this patch.

This is context. While interesting, it should not be part of the commit
log but instead located...

> 
> BCMBCA broadband SoC based board design does not specify ecc setting in
> dts but rather use the SoC NAND strap info to obtain the ecc strength
> and spare area size setting. Add brcm,nand-ecc-use-strap dts propety for
> this purpose and update driver to support this option. However these two
> options can not be used at the same time.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Reviewed-by: David Regan <dregan@broadcom.com>
> 
> [1] https://lore.kernel.org/lkml/20240223034758.13753-1-william.zhang@broadcom.com/

This as well should be below.
> 
> ---

...here.

Thanks,
Miquèl
William Zhang Feb. 29, 2024, 11:39 p.m. UTC | #2
Hi Miquel,

On 2/29/24 02:28, Miquel Raynal wrote:
> Hi William,
> 
> william.zhang@broadcom.com wrote on Tue, 27 Feb 2024 12:12:52 -0800:
> 
>> This is the update to the patch 12 in the original series [1] that was
>> applied to nand-next except this patch.
> 
> This is context. While interesting, it should not be part of the commit
> log but instead located...
> 
>>
>> BCMBCA broadband SoC based board design does not specify ecc setting in
>> dts but rather use the SoC NAND strap info to obtain the ecc strength
>> and spare area size setting. Add brcm,nand-ecc-use-strap dts propety for
>> this purpose and update driver to support this option. However these two
>> options can not be used at the same time.
>>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> Reviewed-by: David Regan <dregan@broadcom.com>
>>
>> [1] https://lore.kernel.org/lkml/20240223034758.13753-1-william.zhang@broadcom.com/
> 
> This as well should be below.
>>
>> ---
> 
> ...here.
> 
Sorry accidentally put these info in the commit log.  Should be in the 
revision history.  Let me know if you want to send a new version.

> Thanks,
> Miquèl
Miquel Raynal March 1, 2024, 8:58 a.m. UTC | #3
Hi William,

william.zhang@broadcom.com wrote on Thu, 29 Feb 2024 15:39:25 -0800:

> Hi Miquel,
> 
> On 2/29/24 02:28, Miquel Raynal wrote:
> > Hi William,
> > 
> > william.zhang@broadcom.com wrote on Tue, 27 Feb 2024 12:12:52 -0800:
> >   
> >> This is the update to the patch 12 in the original series [1] that was
> >> applied to nand-next except this patch.  
> > 
> > This is context. While interesting, it should not be part of the commit
> > log but instead located...
> >   
> >>
> >> BCMBCA broadband SoC based board design does not specify ecc setting in
> >> dts but rather use the SoC NAND strap info to obtain the ecc strength
> >> and spare area size setting. Add brcm,nand-ecc-use-strap dts propety for
> >> this purpose and update driver to support this option. However these two
> >> options can not be used at the same time.
> >>
> >> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> >> Reviewed-by: David Regan <dregan@broadcom.com>
> >>
> >> [1] https://lore.kernel.org/lkml/20240223034758.13753-1-william.zhang@broadcom.com/  
> > 
> > This as well should be below.  
> >>
> >> ---  
> > 
> > ...here.
> >   
> Sorry accidentally put these info in the commit log.  Should be in the revision history.  Let me know if you want to send a new version.

Yes please, it's easier for me.

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index ef7d340475be..8a26ff97c8e2 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1038,6 +1038,22 @@  static inline int brcmnand_sector_1k_shift(struct brcmnand_controller *ctrl)
 		return -1;
 }
 
+static bool brcmnand_get_sector_size_1k(struct brcmnand_host *host)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	int sector_size_bit = brcmnand_sector_1k_shift(ctrl);
+	u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+						  BRCMNAND_CS_ACC_CONTROL);
+	u32 acc_control;
+
+	if (sector_size_bit < 0)
+		return false;
+
+	acc_control = nand_readreg(ctrl, acc_control_offs);
+
+	return ((acc_control & BIT(sector_size_bit)) != 0);
+}
+
 static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
 {
 	struct brcmnand_controller *ctrl = host->ctrl;
@@ -1055,6 +1071,43 @@  static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
 	nand_writereg(ctrl, acc_control_offs, tmp);
 }
 
+static int brcmnand_get_spare_size(struct brcmnand_host *host)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+						  BRCMNAND_CS_ACC_CONTROL);
+	u32 acc = nand_readreg(ctrl, acc_control_offs);
+
+	return (acc & brcmnand_spare_area_mask(ctrl));
+}
+
+static void brcmnand_get_ecc_settings(struct brcmnand_host *host, struct nand_chip *chip)
+{
+	struct brcmnand_controller *ctrl = host->ctrl;
+	u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+						  BRCMNAND_CS_ACC_CONTROL);
+	bool sector_size_1k = brcmnand_get_sector_size_1k(host);
+	int spare_area_size, ecc_level;
+	u32 acc;
+
+	spare_area_size = brcmnand_get_spare_size(host);
+	acc = nand_readreg(ctrl, acc_control_offs);
+	ecc_level = (acc & brcmnand_ecc_level_mask(ctrl)) >> ctrl->ecc_level_shift;
+	if (sector_size_1k)
+		chip->ecc.strength = ecc_level * 2;
+	else if (spare_area_size == 16 && ecc_level == 15)
+		chip->ecc.strength = 1; /* hamming */
+	else
+		chip->ecc.strength = ecc_level;
+
+	if (chip->ecc.size == 0) {
+		if (sector_size_1k)
+			chip->ecc.size = 1024;
+		else
+			chip->ecc.size = 512;
+	}
+}
+
 /***********************************************************************
  * CS_NAND_SELECT
  ***********************************************************************/
@@ -2625,19 +2678,37 @@  static int brcmnand_setup_dev(struct brcmnand_host *host)
 		nanddev_get_memorg(&chip->base);
 	struct brcmnand_controller *ctrl = host->ctrl;
 	struct brcmnand_cfg *cfg = &host->hwcfg;
-	char msg[128];
+	struct device_node *np = nand_get_flash_node(chip);
 	u32 offs, tmp, oob_sector;
+	bool use_strap = false;
+	char msg[128];
 	int ret;
 
 	memset(cfg, 0, sizeof(*cfg));
+	use_strap = of_property_read_bool(np, "brcm,nand-ecc-use-strap");
 
-	ret = of_property_read_u32(nand_get_flash_node(chip),
-				   "brcm,nand-oob-sector-size",
+	/*
+	 * Either nand-ecc-xxx or brcm,nand-ecc-use-strap can be set. Error out
+	 * if both exist.
+	 */
+	if (chip->ecc.strength && use_strap) {
+		dev_err(ctrl->dev,
+			"ECC strap and DT ECC configuration properties are mutually exclusive\n");
+		return -EINVAL;
+	}
+
+	if (use_strap)
+		brcmnand_get_ecc_settings(host, chip);
+
+	ret = of_property_read_u32(np, "brcm,nand-oob-sector-size",
 				   &oob_sector);
 	if (ret) {
-		/* Use detected size */
-		cfg->spare_area_size = mtd->oobsize /
-					(mtd->writesize >> FC_SHIFT);
+		if (use_strap)
+			cfg->spare_area_size = brcmnand_get_spare_size(host);
+		else
+			/* Use detected size */
+			cfg->spare_area_size = mtd->oobsize /
+						(mtd->writesize >> FC_SHIFT);
 	} else {
 		cfg->spare_area_size = oob_sector;
 	}