[PATCH 0/8] Improve MTK NAND driver and ->calc_ecc_bytes() hook

[PATCH 0/8] Improve MTK NAND driver and ->calc_ecc_bytes() hook

[Xiaolei Li]

These patches are mainly to:
  - Fix some MTK NAND driver DT binding description errors.
  - Support FDM ECC protected bytes setting by DT properity.
  - Support nand-ecc-maximize DT properity. And improve
    ->calc_ecc_bytes() hook.
  - Add maintainer entry for Mediatek NAND controller driver.

Changes relative to:
--------------------

tree    : https://github.com/bbrezillon/linux-0day
branch  : nand/next
commit  :
	'commit ba4150b1ec2a ("mtd: rawnand: s3c2410: enhance the probe
	 function error path")'

Tests:
------
  - ubifs and jffs2 are validated on NAND device MT29F16G08ADBCA
    by 'dd' command.
  - all drivers/mtd/tests/* pass.

Xiaolei Li (8):
  dt-bindings: mtd: mtk-nand: Update properties description
  MAINTAINERS: Add entry for Mediatek NAND controller driver
  mtd: rawnand: mtk: Add DT property mtk,fdm-ecc-size
  mtd: rawnand: mtk: Remove max_sector_size from struct mtk_nfc_caps
  mtd: rawnand: Modify ->calc_ecc_bytes() hook in nand_ecc_caps
  mtd: rawnand: mtk: Introduce mtk_ecc_calc_parity_bytes() function
  mtd: rawnand: mtk: Introduce mtk_ecc_get_strength_num(),
    mtk_ecc_get_strength()
  mtd: rawnand: mtk: Use generic helpers to calculate ecc size and
    strength

 Documentation/devicetree/bindings/mtd/mtk-nand.txt |  24 ++-
 MAINTAINERS                                        |   7 +
 drivers/mtd/nand/raw/denali.c                      |   3 +-
 drivers/mtd/nand/raw/denali.h                      |   2 +-
 drivers/mtd/nand/raw/mtk_ecc.c                     |  43 ++---
 drivers/mtd/nand/raw/mtk_ecc.h                     |   5 +-
 drivers/mtd/nand/raw/mtk_nand.c                    | 187 ++++++++++++++-------
 drivers/mtd/nand/raw/nand_base.c                   |  11 +-
 include/linux/mtd/rawnand.h                        |  77 ++++-----
 9 files changed, 223 insertions(+), 136 deletions(-)

-- 
1.9.1

[PATCH 1/8] dt-bindings: mtd: mtk-nand: Update properties description

[Xiaolei Li]

Update ecc step size, ecc strength, and parity bits supported on
each MTK NAND controller.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 Documentation/devicetree/bindings/mtd/mtk-nand.txt | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/mtk-nand.txt b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
index 1c88526..ef786568 100644
--- a/Documentation/devicetree/bindings/mtd/mtk-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
@@ -50,14 +50,19 @@ Optional:
 - nand-on-flash-bbt:	Store BBT on NAND Flash.
 - nand-ecc-mode:	the NAND ecc mode (check driver for supported modes)
 - nand-ecc-step-size:	Number of data bytes covered by a single ECC step.
-			valid values: 512 and 1024.
+			valid values:
+			512 and 1024 on mt2701 and mt2712.
+			512 only on mt7622.
 			1024 is recommended for large page NANDs.
 - nand-ecc-strength:	Number of bits to correct per ECC step.
-			The valid values that the controller supports are: 4, 6,
-			8, 10, 12, 14, 16, 18, 20, 22, 24, 28, 32, 36, 40, 44,
-			48, 52, 56, 60.
+			The valid values that each controller supports:
+			mt2701: 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28,
+				32, 36, 40, 44, 48, 52, 56, 60.
+			mt2712: 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28,
+				32, 36, 40, 44, 48, 52, 56, 60, 68, 72, 80.
+			mt7622: 4, 6, 8, 10, 12, 14, 16.
 			The strength should be calculated as follows:
-			E = (S - F) * 8 / 14
+			E = (S - F) * 8 / B
 			S = O / (P / Q)
 				E :	nand-ecc-strength.
 				S :	spare size per sector.
@@ -66,6 +71,9 @@ Optional:
 				O :	oob size.
 				P :	page size.
 				Q :	nand-ecc-step-size.
+				B :	parity bits to do 1 bit ECC.
+					Be 14 on mt2701 and mt2712.
+					Be 13 on mt7622.
 			If the result does not match any one of the listed
 			choices above, please select the smaller valid value from
 			the list.
-- 
1.9.1

[PATCH 2/8] MAINTAINERS: Add entry for Mediatek NAND controller driver

[Xiaolei Li]

Add entry for Mediatek NAND controller driver and its bindings.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d262f17..429ef09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8838,6 +8838,13 @@ L:	linux-wireless@vger.kernel.org
 S:	Maintained
 F:	drivers/net/wireless/mediatek/mt7601u/
 
+MEDIATEK NAND CONTROLLER DRIVER
+M:	Xiaolei Li <xiaolei.li@mediatek.com>
+L:	linux-mtd@lists.infradead.org
+S:	Maintained
+F:	drivers/mtd/nand/raw/mtk_*
+F:	Documentation/devicetree/bindings/mtd/mtk-nand.txt
+
 MEDIATEK RANDOM NUMBER GENERATOR SUPPORT
 M:	Sean Wang <sean.wang@mediatek.com>
 S:	Maintained
-- 
1.9.1

[PATCH 3/8] mtd: rawnand: mtk: Add DT property mtk,fdm-ecc-size

[Xiaolei Li]

For some MTK NAND chips, BootROM may access more than one byte
ECC protected FDM data, but now we fix ECC protected FDM byte as 1.
This will make some chips be failed to boot up.

With this DT property setting, different MTK NAND chips with the same
NAND controller IP can work well.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 Documentation/devicetree/bindings/mtd/mtk-nand.txt |  6 ++++++
 drivers/mtd/nand/raw/mtk_nand.c                    | 25 ++++++++++++++++------
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/mtk-nand.txt b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
index ef786568..a8e4136 100644
--- a/Documentation/devicetree/bindings/mtd/mtk-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
@@ -47,6 +47,12 @@ Children nodes properties:
 - reg:			Chip Select Signal, default 0.
 			Set as reg = <0>, <1> when need 2 CS.
 Optional:
+- mtk,fdm-ecc-size:	Integer representing ECC protected FDM bytes.
+			Should be in the range [1,8], if not present 1.
+			On some MTK NAND chips, BootROM may access more than
+			one byte ECC protected FDM data. Different MTK chips
+			with the same NAND controller IP will work well with
+			this properity setting.
 - nand-on-flash-bbt:	Store BBT on NAND Flash.
 - nand-ecc-mode:	the NAND ecc mode (check driver for supported modes)
 - nand-ecc-step-size:	Number of data bytes covered by a single ECC step.
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 6977da3..b05f619 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -86,7 +86,6 @@
 #define NFI_FDML(x)		(0xA0 + (x) * sizeof(u32) * 2)
 #define NFI_FDMM(x)		(0xA4 + (x) * sizeof(u32) * 2)
 #define NFI_FDM_MAX_SIZE	(8)
-#define NFI_FDM_MIN_SIZE	(1)
 #define NFI_MASTER_STA		(0x224)
 #define		MASTER_STA_MASK		(0x0FFF)
 #define NFI_EMPTY_THRESH	(0x23C)
@@ -1118,7 +1117,7 @@ static int mtk_nfc_ooblayout_ecc(struct mtd_info *mtd, int section,
 	.ecc = mtk_nfc_ooblayout_ecc,
 };
 
-static void mtk_nfc_set_fdm(struct mtk_nfc_fdm *fdm, struct mtd_info *mtd)
+static int mtk_nfc_set_fdm(struct mtk_nfc_fdm *fdm, struct mtd_info *mtd)
 {
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct mtk_nfc_nand_chip *chip = to_mtk_nand(nand);
@@ -1129,11 +1128,15 @@ static void mtk_nfc_set_fdm(struct mtk_nfc_fdm *fdm, struct mtd_info *mtd)
 				 mtk_ecc_get_parity_bits(nfc->ecc), 8);
 
 	fdm->reg_size = chip->spare_per_sector - ecc_bytes;
-	if (fdm->reg_size > NFI_FDM_MAX_SIZE)
+	if (fdm->reg_size > NFI_FDM_MAX_SIZE) {
 		fdm->reg_size = NFI_FDM_MAX_SIZE;
+	} else if (fdm->reg_size < fdm->ecc_size) {
+		dev_err(nfc->dev, "fdm reg size(%u) is less than fdm ecc size(%u)\n",
+			fdm->reg_size, fdm->ecc_size);
+		return -ENOTSUPP;
+	}
 
-	/* bad block mark storage */
-	fdm->ecc_size = 1;
+	return 0;
 }
 
 static void mtk_nfc_set_bad_mark_ctl(struct mtk_nfc_bad_mark_ctl *bm_ctl,
@@ -1185,6 +1188,7 @@ static int mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)
 static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 {
 	struct nand_chip *nand = mtd_to_nand(mtd);
+	struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(nand);
 	struct mtk_nfc *nfc = nand_get_controller_data(nand);
 	u32 spare;
 	int free, ret;
@@ -1236,7 +1240,7 @@ static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 			nand->ecc.strength = (spare << 3) /
 					     mtk_ecc_get_parity_bits(nfc->ecc);
 		} else if (free < 0) {
-			spare -= NFI_FDM_MIN_SIZE;
+			spare -= mtk_nand->fdm.ecc_size;
 			nand->ecc.strength = (spare << 3) /
 					     mtk_ecc_get_parity_bits(nfc->ecc);
 		}
@@ -1285,6 +1289,10 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 		chip->sels[i] = tmp;
 	}
 
+	/* If not present, set 1 for bad block mark storage */
+	if (of_property_read_u32(np, "mtk,fdm-ecc-size", &chip->fdm.ecc_size))
+		chip->fdm.ecc_size = 1;
+
 	nand = &chip->nand;
 	nand->controller = &nfc->controller;
 
@@ -1345,7 +1353,10 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 	if (ret)
 		return ret;
 
-	mtk_nfc_set_fdm(&chip->fdm, mtd);
+	ret = mtk_nfc_set_fdm(&chip->fdm, mtd);
+	if (ret)
+		return ret;
+
 	mtk_nfc_set_bad_mark_ctl(&chip->bad_mark, mtd);
 
 	len = mtd->writesize + mtd->oobsize;
-- 
1.9.1

[PATCH 4/8] mtd: rawnand: mtk: Remove max_sector_size from struct mtk_nfc_caps

[Xiaolei Li]

Remove max_sector_size from struct mtk_nfc_caps, and use sector size
array and number of sector size to show sector sizes that controller
supports.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index b05f619..2c244c2 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -105,10 +105,11 @@
 struct mtk_nfc_caps {
 	const u8 *spare_size;
 	u8 num_spare_size;
+	const int *sector_size;
+	int num_sector_size;
 	u8 pageformat_spare_shift;
 	u8 nfi_clk_div;
 	u8 max_sector;
-	u32 max_sector_size;
 };
 
 struct mtk_nfc_bad_mark_ctl {
@@ -177,6 +178,14 @@ struct mtk_nfc {
 	16, 26, 27, 28
 };
 
+static const int sector_size_max_1k[] = {
+	1024, 512
+};
+
+static const int sector_size_max_512[] = {
+	512
+};
+
 static inline struct mtk_nfc_nand_chip *to_mtk_nand(struct nand_chip *nand)
 {
 	return container_of(nand, struct mtk_nfc_nand_chip, nand);
@@ -257,6 +266,16 @@ static inline u8 nfi_readb(struct mtk_nfc *nfc, u32 reg)
 	return readb_relaxed(nfc->regs + reg);
 }
 
+static int mtk_nfc_max_sector_size(struct mtk_nfc *nfc)
+{
+	int i, sector_size = 0;
+
+	for (i = 0; i < nfc->caps->num_sector_size; i++)
+		sector_size = max(sector_size, nfc->caps->sector_size[i]);
+
+	return sector_size;
+}
+
 static void mtk_nfc_hw_reset(struct mtk_nfc *nfc)
 {
 	struct device *dev = nfc->dev;
@@ -1211,7 +1230,7 @@ static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 		 */
 		if (nand->ecc.size < 1024) {
 			if (mtd->writesize > 512 &&
-			    nfc->caps->max_sector_size > 512) {
+			    mtk_nfc_max_sector_size(nfc) > 512) {
 				nand->ecc.size = 1024;
 				nand->ecc.strength <<= 1;
 			} else {
@@ -1400,28 +1419,31 @@ static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
 static const struct mtk_nfc_caps mtk_nfc_caps_mt2701 = {
 	.spare_size = spare_size_mt2701,
 	.num_spare_size = 16,
+	.sector_size = sector_size_max_1k,
+	.num_sector_size = 2,
 	.pageformat_spare_shift = 4,
 	.nfi_clk_div = 1,
 	.max_sector = 16,
-	.max_sector_size = 1024,
 };
 
 static const struct mtk_nfc_caps mtk_nfc_caps_mt2712 = {
 	.spare_size = spare_size_mt2712,
 	.num_spare_size = 19,
+	.sector_size = sector_size_max_1k,
+	.num_sector_size = 2,
 	.pageformat_spare_shift = 16,
 	.nfi_clk_div = 2,
 	.max_sector = 16,
-	.max_sector_size = 1024,
 };
 
 static const struct mtk_nfc_caps mtk_nfc_caps_mt7622 = {
 	.spare_size = spare_size_mt7622,
 	.num_spare_size = 4,
+	.sector_size = sector_size_max_512,
+	.num_sector_size = 1,
 	.pageformat_spare_shift = 4,
 	.nfi_clk_div = 1,
 	.max_sector = 8,
-	.max_sector_size = 512,
 };
 
 static const struct of_device_id mtk_nfc_id_table[] = {
-- 
1.9.1

[PATCH 5/8] mtd: rawnand: Modify ->calc_ecc_bytes() hook in nand_ecc_caps

[Xiaolei Li]

Maybe some controllers need more information besides step size and ecc
strength to calculate ECC bytes.

struct nand_ecc_ctrl provides lots of ECC control information include
private ECC control data. So, add it into ->calc_ecc_bytes() interface,
to make this hook be more flexible.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/denali.c    |  3 +-
 drivers/mtd/nand/raw/denali.h    |  2 +-
 drivers/mtd/nand/raw/nand_base.c | 11 ++++--
 include/linux/mtd/rawnand.h      | 77 ++++++++++++++++++++--------------------
 4 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 2a302a1..907609c 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1113,7 +1113,8 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	iowrite32(0xffff, denali->reg + SPARE_AREA_MARKER);
 }
 
-int denali_calc_ecc_bytes(int step_size, int strength)
+int denali_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
+			  int strength)
 {
 	/* BCH code.  Denali requires ecc.bytes to be multiple of 2 */
 	return DIV_ROUND_UP(strength * fls(step_size * 8), 16) * 2;
diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h
index 9ad33d2..644dffd 100644
--- a/drivers/mtd/nand/raw/denali.h
+++ b/drivers/mtd/nand/raw/denali.h
@@ -328,7 +328,7 @@ struct denali_nand_info {
 #define DENALI_CAP_HW_ECC_FIXUP			BIT(0)
 #define DENALI_CAP_DMA_64BIT			BIT(1)
 
-int denali_calc_ecc_bytes(int step_size, int strength);
+int denali_calc_ecc_bytes(struct nand_ecc_ctrl *, int, int);
 int denali_init(struct denali_nand_info *denali);
 void denali_remove(struct denali_nand_info *denali);
 
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d0b993f..e17abc8 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -6052,6 +6052,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
 			const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	const struct nand_ecc_step_info *stepinfo;
 	int preset_step = chip->ecc.size;
 	int preset_strength = chip->ecc.strength;
@@ -6076,7 +6077,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
 			if (stepinfo->strengths[j] != preset_strength)
 				continue;
 
-			ecc_bytes = caps->calc_ecc_bytes(preset_step,
+			ecc_bytes = caps->calc_ecc_bytes(ecc, preset_step,
 							 preset_strength);
 			if (WARN_ON_ONCE(ecc_bytes < 0))
 				return ecc_bytes;
@@ -6114,6 +6115,7 @@ int nand_match_ecc_req(struct nand_chip *chip,
 		       const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	const struct nand_ecc_step_info *stepinfo;
 	int req_step = chip->ecc_step_ds;
 	int req_strength = chip->ecc_strength_ds;
@@ -6152,7 +6154,8 @@ int nand_match_ecc_req(struct nand_chip *chip,
 
 			nsteps = mtd->writesize / step_size;
 
-			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
+			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
+							 strength);
 			if (WARN_ON_ONCE(ecc_bytes < 0))
 				continue;
 			ecc_bytes_total = ecc_bytes * nsteps;
@@ -6198,6 +6201,7 @@ int nand_maximize_ecc(struct nand_chip *chip,
 		      const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	const struct nand_ecc_step_info *stepinfo;
 	int step_size, strength, nsteps, ecc_bytes, corr;
 	int best_corr = 0;
@@ -6224,7 +6228,8 @@ int nand_maximize_ecc(struct nand_chip *chip,
 
 			nsteps = mtd->writesize / step_size;
 
-			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
+			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
+							 strength);
 			if (WARN_ON_ONCE(ecc_bytes < 0))
 				continue;
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 5dad59b..e7c7fdb 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -507,44 +507,6 @@ static inline void nand_hw_control_init(struct nand_hw_control *nfc)
 }
 
 /**
- * struct nand_ecc_step_info - ECC step information of ECC engine
- * @stepsize: data bytes per ECC step
- * @strengths: array of supported strengths
- * @nstrengths: number of supported strengths
- */
-struct nand_ecc_step_info {
-	int stepsize;
-	const int *strengths;
-	int nstrengths;
-};
-
-/**
- * struct nand_ecc_caps - capability of ECC engine
- * @stepinfos: array of ECC step information
- * @nstepinfos: number of ECC step information
- * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
- */
-struct nand_ecc_caps {
-	const struct nand_ecc_step_info *stepinfos;
-	int nstepinfos;
-	int (*calc_ecc_bytes)(int step_size, int strength);
-};
-
-/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
-#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
-static const int __name##_strengths[] = { __VA_ARGS__ };	\
-static const struct nand_ecc_step_info __name##_stepinfo = {	\
-	.stepsize = __step,					\
-	.strengths = __name##_strengths,			\
-	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
-};								\
-static const struct nand_ecc_caps __name = {			\
-	.stepinfos = &__name##_stepinfo,			\
-	.nstepinfos = 1,					\
-	.calc_ecc_bytes = __calc,				\
-}
-
-/**
  * struct nand_ecc_ctrl - Control structure for ECC
  * @mode:	ECC mode
  * @algo:	ECC algorithm
@@ -639,6 +601,45 @@ struct nand_ecc_ctrl {
 };
 
 /**
+ * struct nand_ecc_step_info - ECC step information of ECC engine
+ * @stepsize: data bytes per ECC step
+ * @strengths: array of supported strengths
+ * @nstrengths: number of supported strengths
+ */
+struct nand_ecc_step_info {
+	int stepsize;
+	const int *strengths;
+	int nstrengths;
+};
+
+/**
+ * struct nand_ecc_caps - capability of ECC engine
+ * @stepinfos: array of ECC step information
+ * @nstepinfos: number of ECC step information
+ * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
+ */
+struct nand_ecc_caps {
+	const struct nand_ecc_step_info *stepinfos;
+	int nstepinfos;
+	int (*calc_ecc_bytes)(struct nand_ecc_ctrl *ecc, int step_size,
+			      int strength);
+};
+
+/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
+#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
+static const int __name##_strengths[] = { __VA_ARGS__ };	\
+static const struct nand_ecc_step_info __name##_stepinfo = {	\
+	.stepsize = __step,					\
+	.strengths = __name##_strengths,			\
+	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
+};								\
+static const struct nand_ecc_caps __name = {			\
+	.stepinfos = &__name##_stepinfo,			\
+	.nstepinfos = 1,					\
+	.calc_ecc_bytes = __calc,				\
+}
+
+/**
  * struct nand_sdr_timings - SDR NAND chip timings
  *
  * This struct defines the timing requirements of a SDR NAND chip.
-- 
1.9.1

[PATCH 5/8] mtd: rawnand: Modify ->calc_ecc_bytes() hook in nand_ecc_caps

[Xiaolei Li]

Maybe some controllers need more information besides step size and ecc
strength to calculate ECC bytes.

struct nand_ecc_ctrl provides lots of ECC control information include
private ECC control data. So, add it into ->calc_ecc_bytes() interface,
to make this hook be more flexible.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/denali.c    |  3 +-
 drivers/mtd/nand/raw/denali.h    |  2 +-
 drivers/mtd/nand/raw/nand_base.c | 11 ++++--
 include/linux/mtd/rawnand.h      | 77 ++++++++++++++++++++--------------------
 4 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 2a302a1..907609c 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1113,7 +1113,8 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	iowrite32(0xffff, denali->reg + SPARE_AREA_MARKER);
 }
 
-int denali_calc_ecc_bytes(int step_size, int strength)
+int denali_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
+			  int strength)
 {
 	/* BCH code.  Denali requires ecc.bytes to be multiple of 2 */
 	return DIV_ROUND_UP(strength * fls(step_size * 8), 16) * 2;
diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h
index 9ad33d2..644dffd 100644
--- a/drivers/mtd/nand/raw/denali.h
+++ b/drivers/mtd/nand/raw/denali.h
@@ -328,7 +328,7 @@ struct denali_nand_info {
 #define DENALI_CAP_HW_ECC_FIXUP			BIT(0)
 #define DENALI_CAP_DMA_64BIT			BIT(1)
 
-int denali_calc_ecc_bytes(int step_size, int strength);
+int denali_calc_ecc_bytes(struct nand_ecc_ctrl *, int, int);
 int denali_init(struct denali_nand_info *denali);
 void denali_remove(struct denali_nand_info *denali);
 
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d0b993f..e17abc8 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -6052,6 +6052,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
 			const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	const struct nand_ecc_step_info *stepinfo;
 	int preset_step = chip->ecc.size;
 	int preset_strength = chip->ecc.strength;
@@ -6076,7 +6077,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
 			if (stepinfo->strengths[j] != preset_strength)
 				continue;
 
-			ecc_bytes = caps->calc_ecc_bytes(preset_step,
+			ecc_bytes = caps->calc_ecc_bytes(ecc, preset_step,
 							 preset_strength);
 			if (WARN_ON_ONCE(ecc_bytes < 0))
 				return ecc_bytes;
@@ -6114,6 +6115,7 @@ int nand_match_ecc_req(struct nand_chip *chip,
 		       const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	const struct nand_ecc_step_info *stepinfo;
 	int req_step = chip->ecc_step_ds;
 	int req_strength = chip->ecc_strength_ds;
@@ -6152,7 +6154,8 @@ int nand_match_ecc_req(struct nand_chip *chip,
 
 			nsteps = mtd->writesize / step_size;
 
-			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
+			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
+							 strength);
 			if (WARN_ON_ONCE(ecc_bytes < 0))
 				continue;
 			ecc_bytes_total = ecc_bytes * nsteps;
@@ -6198,6 +6201,7 @@ int nand_maximize_ecc(struct nand_chip *chip,
 		      const struct nand_ecc_caps *caps, int oobavail)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	const struct nand_ecc_step_info *stepinfo;
 	int step_size, strength, nsteps, ecc_bytes, corr;
 	int best_corr = 0;
@@ -6224,7 +6228,8 @@ int nand_maximize_ecc(struct nand_chip *chip,
 
 			nsteps = mtd->writesize / step_size;
 
-			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
+			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
+							 strength);
 			if (WARN_ON_ONCE(ecc_bytes < 0))
 				continue;
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 5dad59b..e7c7fdb 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -507,44 +507,6 @@ static inline void nand_hw_control_init(struct nand_hw_control *nfc)
 }
 
 /**
- * struct nand_ecc_step_info - ECC step information of ECC engine
- * @stepsize: data bytes per ECC step
- * @strengths: array of supported strengths
- * @nstrengths: number of supported strengths
- */
-struct nand_ecc_step_info {
-	int stepsize;
-	const int *strengths;
-	int nstrengths;
-};
-
-/**
- * struct nand_ecc_caps - capability of ECC engine
- * @stepinfos: array of ECC step information
- * @nstepinfos: number of ECC step information
- * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
- */
-struct nand_ecc_caps {
-	const struct nand_ecc_step_info *stepinfos;
-	int nstepinfos;
-	int (*calc_ecc_bytes)(int step_size, int strength);
-};
-
-/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
-#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
-static const int __name##_strengths[] = { __VA_ARGS__ };	\
-static const struct nand_ecc_step_info __name##_stepinfo = {	\
-	.stepsize = __step,					\
-	.strengths = __name##_strengths,			\
-	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
-};								\
-static const struct nand_ecc_caps __name = {			\
-	.stepinfos = &__name##_stepinfo,			\
-	.nstepinfos = 1,					\
-	.calc_ecc_bytes = __calc,				\
-}
-
-/**
  * struct nand_ecc_ctrl - Control structure for ECC
  * @mode:	ECC mode
  * @algo:	ECC algorithm
@@ -639,6 +601,45 @@ struct nand_ecc_ctrl {
 };
 
 /**
+ * struct nand_ecc_step_info - ECC step information of ECC engine
+ * @stepsize: data bytes per ECC step
+ * @strengths: array of supported strengths
+ * @nstrengths: number of supported strengths
+ */
+struct nand_ecc_step_info {
+	int stepsize;
+	const int *strengths;
+	int nstrengths;
+};
+
+/**
+ * struct nand_ecc_caps - capability of ECC engine
+ * @stepinfos: array of ECC step information
+ * @nstepinfos: number of ECC step information
+ * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
+ */
+struct nand_ecc_caps {
+	const struct nand_ecc_step_info *stepinfos;
+	int nstepinfos;
+	int (*calc_ecc_bytes)(struct nand_ecc_ctrl *ecc, int step_size,
+			      int strength);
+};
+
+/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
+#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
+static const int __name##_strengths[] = { __VA_ARGS__ };	\
+static const struct nand_ecc_step_info __name##_stepinfo = {	\
+	.stepsize = __step,					\
+	.strengths = __name##_strengths,			\
+	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
+};								\
+static const struct nand_ecc_caps __name = {			\
+	.stepinfos = &__name##_stepinfo,			\
+	.nstepinfos = 1,					\
+	.calc_ecc_bytes = __calc,				\
+}
+
+/**
  * struct nand_sdr_timings - SDR NAND chip timings
  *
  * This struct defines the timing requirements of a SDR NAND chip.
-- 
1.9.1


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

[PATCH 6/8] mtd: rawnand: mtk: Introduce mtk_ecc_calc_parity_bytes() function

[Xiaolei Li]

Introduce mtk_ecc_calc_parity_bytes() function to calculate ECC
parity byte number.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_ecc.c  | 10 ++++++++--
 drivers/mtd/nand/raw/mtk_ecc.h  |  1 +
 drivers/mtd/nand/raw/mtk_nand.c |  6 ++----
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_ecc.c b/drivers/mtd/nand/raw/mtk_ecc.c
index 40d86a8..717d553 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.c
+++ b/drivers/mtd/nand/raw/mtk_ecc.c
@@ -408,7 +408,7 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 	mtk_ecc_wait_idle(ecc, ECC_ENCODE);
 
 	/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
-	len = (config->strength * ecc->caps->parity_bits + 7) >> 3;
+	len = mtk_ecc_calc_parity_bytes(ecc, config->strength);
 
 	/* write the parity bytes generated by the ECC back to temp buffer */
 	__ioread32_copy(ecc->eccdata,
@@ -451,6 +451,12 @@ unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc)
 }
 EXPORT_SYMBOL(mtk_ecc_get_parity_bits);
 
+int mtk_ecc_calc_parity_bytes(struct mtk_ecc *ecc, int ecc_strength)
+{
+	return DIV_ROUND_UP(ecc->caps->parity_bits * ecc_strength, 8);
+}
+EXPORT_SYMBOL(mtk_ecc_calc_parity_bytes);
+
 static const struct mtk_ecc_caps mtk_ecc_caps_mt2701 = {
 	.err_mask = 0x3f,
 	.ecc_strength = ecc_strength_mt2701,
@@ -516,7 +522,7 @@ static int mtk_ecc_probe(struct platform_device *pdev)
 
 	max_eccdata_size = ecc->caps->num_ecc_strength - 1;
 	max_eccdata_size = ecc->caps->ecc_strength[max_eccdata_size];
-	max_eccdata_size = (max_eccdata_size * ecc->caps->parity_bits + 7) >> 3;
+	max_eccdata_size = mtk_ecc_calc_parity_bytes(ecc, max_eccdata_size);
 	max_eccdata_size = round_up(max_eccdata_size, 4);
 	ecc->eccdata = devm_kzalloc(dev, max_eccdata_size, GFP_KERNEL);
 	if (!ecc->eccdata)
diff --git a/drivers/mtd/nand/raw/mtk_ecc.h b/drivers/mtd/nand/raw/mtk_ecc.h
index a455df0..c1546f9 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.h
+++ b/drivers/mtd/nand/raw/mtk_ecc.h
@@ -42,6 +42,7 @@ struct mtk_ecc_config {
 void mtk_ecc_disable(struct mtk_ecc *);
 void mtk_ecc_adjust_strength(struct mtk_ecc *ecc, u32 *p);
 unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc);
+int mtk_ecc_calc_parity_bytes(struct mtk_ecc *ecc, int ecc_strength);
 
 struct mtk_ecc *of_mtk_ecc_get(struct device_node *);
 void mtk_ecc_release(struct mtk_ecc *);
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 2c244c2..249b3fb 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -1143,8 +1143,7 @@ static int mtk_nfc_set_fdm(struct mtk_nfc_fdm *fdm, struct mtd_info *mtd)
 	struct mtk_nfc *nfc = nand_get_controller_data(nand);
 	u32 ecc_bytes;
 
-	ecc_bytes = DIV_ROUND_UP(nand->ecc.strength *
-				 mtk_ecc_get_parity_bits(nfc->ecc), 8);
+	ecc_bytes = mtk_ecc_calc_parity_bytes(nfc->ecc, nand->ecc.strength);
 
 	fdm->reg_size = chip->spare_per_sector - ecc_bytes;
 	if (fdm->reg_size > NFI_FDM_MAX_SIZE) {
@@ -1245,8 +1244,7 @@ static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 			return ret;
 
 		/* calculate oob bytes except ecc parity data */
-		free = (nand->ecc.strength * mtk_ecc_get_parity_bits(nfc->ecc)
-			+ 7) >> 3;
+		free = mtk_ecc_calc_parity_bytes(nfc->ecc, nand->ecc.strength);
 		free = spare - free;
 
 		/*
-- 
1.9.1

[PATCH 7/8] mtd: rawnand: mtk: Introduce mtk_ecc_get_strength_num(), mtk_ecc_get_strength()

[Xiaolei Li]

Introduce mtk_ecc_get_strength_num(), mtk_ecc_get_strength() functions,
and prepare to support struct nand_ecc_caps.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_ecc.c | 24 ++++++++++++++++++------
 drivers/mtd/nand/raw/mtk_ecc.h |  2 ++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_ecc.c b/drivers/mtd/nand/raw/mtk_ecc.c
index 717d553..71390e7 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.c
+++ b/drivers/mtd/nand/raw/mtk_ecc.c
@@ -51,9 +51,9 @@
 
 struct mtk_ecc_caps {
 	u32 err_mask;
-	const u8 *ecc_strength;
+	const int *ecc_strength;
 	const u32 *ecc_regs;
-	u8 num_ecc_strength;
+	int num_ecc_strength;
 	u8 ecc_mode_shift;
 	u32 parity_bits;
 	int pg_irq_sel;
@@ -73,17 +73,17 @@ struct mtk_ecc {
 };
 
 /* ecc strength that each IP supports */
-static const u8 ecc_strength_mt2701[] = {
+static const int ecc_strength_mt2701[] = {
 	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28, 32, 36,
 	40, 44, 48, 52, 56, 60
 };
 
-static const u8 ecc_strength_mt2712[] = {
+static const int ecc_strength_mt2712[] = {
 	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28, 32, 36,
 	40, 44, 48, 52, 56, 60, 68, 72, 80
 };
 
-static const u8 ecc_strength_mt7622[] = {
+static const int ecc_strength_mt7622[] = {
 	4, 6, 8, 10, 12, 14, 16
 };
 
@@ -428,7 +428,7 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 
 void mtk_ecc_adjust_strength(struct mtk_ecc *ecc, u32 *p)
 {
-	const u8 *ecc_strength = ecc->caps->ecc_strength;
+	const int *ecc_strength = ecc->caps->ecc_strength;
 	int i;
 
 	for (i = 0; i < ecc->caps->num_ecc_strength; i++) {
@@ -451,6 +451,18 @@ unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc)
 }
 EXPORT_SYMBOL(mtk_ecc_get_parity_bits);
 
+int mtk_ecc_get_strength_num(struct mtk_ecc *ecc)
+{
+	return ecc->caps->num_ecc_strength;
+}
+EXPORT_SYMBOL(mtk_ecc_get_strength_num);
+
+const int *mtk_ecc_get_strength(struct mtk_ecc *ecc)
+{
+	return ecc->caps->ecc_strength;
+}
+EXPORT_SYMBOL(mtk_ecc_get_strength);
+
 int mtk_ecc_calc_parity_bytes(struct mtk_ecc *ecc, int ecc_strength)
 {
 	return DIV_ROUND_UP(ecc->caps->parity_bits * ecc_strength, 8);
diff --git a/drivers/mtd/nand/raw/mtk_ecc.h b/drivers/mtd/nand/raw/mtk_ecc.h
index c1546f9..5687864 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.h
+++ b/drivers/mtd/nand/raw/mtk_ecc.h
@@ -42,6 +42,8 @@ struct mtk_ecc_config {
 void mtk_ecc_disable(struct mtk_ecc *);
 void mtk_ecc_adjust_strength(struct mtk_ecc *ecc, u32 *p);
 unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc);
+int mtk_ecc_get_strength_num(struct mtk_ecc *ecc);
+const int *mtk_ecc_get_strength(struct mtk_ecc *ecc);
 int mtk_ecc_calc_parity_bytes(struct mtk_ecc *ecc, int ecc_strength);
 
 struct mtk_ecc *of_mtk_ecc_get(struct device_node *);
-- 
1.9.1

[PATCH 8/8] mtd: rawnand: mtk: Use generic helpers to calculate ecc size and strength

[Xiaolei Li]

An optional DT properity named nand-ecc-maximize is used to choose whether
maximize ecc strength or just match ecc strength required.

But MTK nand driver always maximize ecc strength now.

This patch uses generic helpers to calculate ecc size and strength
automatically according to nand-ecc-maximize setting.

Please remember to enable nand-ecc-maximize DT setting if want to be
compatible with older Bootloader base on this patch.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_ecc.c  |  25 -------
 drivers/mtd/nand/raw/mtk_ecc.h  |   2 -
 drivers/mtd/nand/raw/mtk_nand.c | 152 ++++++++++++++++++++++++----------------
 3 files changed, 93 insertions(+), 86 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_ecc.c b/drivers/mtd/nand/raw/mtk_ecc.c
index 71390e7..cd68157 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.c
+++ b/drivers/mtd/nand/raw/mtk_ecc.c
@@ -426,31 +426,6 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 }
 EXPORT_SYMBOL(mtk_ecc_encode);
 
-void mtk_ecc_adjust_strength(struct mtk_ecc *ecc, u32 *p)
-{
-	const int *ecc_strength = ecc->caps->ecc_strength;
-	int i;
-
-	for (i = 0; i < ecc->caps->num_ecc_strength; i++) {
-		if (*p <= ecc_strength[i]) {
-			if (!i)
-				*p = ecc_strength[i];
-			else if (*p != ecc_strength[i])
-				*p = ecc_strength[i - 1];
-			return;
-		}
-	}
-
-	*p = ecc_strength[ecc->caps->num_ecc_strength - 1];
-}
-EXPORT_SYMBOL(mtk_ecc_adjust_strength);
-
-unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc)
-{
-	return ecc->caps->parity_bits;
-}
-EXPORT_SYMBOL(mtk_ecc_get_parity_bits);
-
 int mtk_ecc_get_strength_num(struct mtk_ecc *ecc)
 {
 	return ecc->caps->num_ecc_strength;
diff --git a/drivers/mtd/nand/raw/mtk_ecc.h b/drivers/mtd/nand/raw/mtk_ecc.h
index 5687864..e7f20f0 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.h
+++ b/drivers/mtd/nand/raw/mtk_ecc.h
@@ -40,8 +40,6 @@ struct mtk_ecc_config {
 int mtk_ecc_wait_done(struct mtk_ecc *, enum mtk_ecc_operation);
 int mtk_ecc_enable(struct mtk_ecc *, struct mtk_ecc_config *);
 void mtk_ecc_disable(struct mtk_ecc *);
-void mtk_ecc_adjust_strength(struct mtk_ecc *ecc, u32 *p);
-unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc);
 int mtk_ecc_get_strength_num(struct mtk_ecc *ecc);
 const int *mtk_ecc_get_strength(struct mtk_ecc *ecc);
 int mtk_ecc_calc_parity_bytes(struct mtk_ecc *ecc, int ecc_strength);
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 249b3fb..b535eee 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -157,6 +157,8 @@ struct mtk_nfc {
 	struct completion done;
 	struct list_head chips;
 
+	const struct nand_ecc_caps *ecccaps;
+
 	u8 *buffer;
 };
 
@@ -266,16 +268,6 @@ static inline u8 nfi_readb(struct mtk_nfc *nfc, u32 reg)
 	return readb_relaxed(nfc->regs + reg);
 }
 
-static int mtk_nfc_max_sector_size(struct mtk_nfc *nfc)
-{
-	int i, sector_size = 0;
-
-	for (i = 0; i < nfc->caps->num_sector_size; i++)
-		sector_size = max(sector_size, nfc->caps->sector_size[i]);
-
-	return sector_size;
-}
-
 static void mtk_nfc_hw_reset(struct mtk_nfc *nfc)
 {
 	struct device *dev = nfc->dev;
@@ -1171,14 +1163,15 @@ static void mtk_nfc_set_bad_mark_ctl(struct mtk_nfc_bad_mark_ctl *bm_ctl,
 	}
 }
 
-static int mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)
+static int mtk_nfc_set_spare_per_sector(int ecc_size, u32 *sps,
+					struct mtd_info *mtd)
 {
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct mtk_nfc *nfc = nand_get_controller_data(nand);
 	const u8 *spare = nfc->caps->spare_size;
 	u32 eccsteps, i, closest_spare = 0;
 
-	eccsteps = mtd->writesize / nand->ecc.size;
+	eccsteps = mtd->writesize / ecc_size;
 	*sps = mtd->oobsize / eccsteps;
 
 	if (nand->ecc.size == 1024)
@@ -1205,68 +1198,102 @@ static int mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)
 
 static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 {
-	struct nand_chip *nand = mtd_to_nand(mtd);
-	struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(nand);
-	struct mtk_nfc *nfc = nand_get_controller_data(nand);
-	u32 spare;
-	int free, ret;
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip);
+	struct mtk_nfc *nfc = nand_get_controller_data(chip);
+	const struct nand_ecc_caps *caps = nfc->ecccaps;
+	int tmp, oobavail, nsteps, ret;
 
 	/* support only ecc hw mode */
-	if (nand->ecc.mode != NAND_ECC_HW) {
+	if (ecc->mode != NAND_ECC_HW) {
 		dev_err(dev, "ecc.mode not supported\n");
 		return -EINVAL;
 	}
 
-	/* if optional dt settings not present */
-	if (!nand->ecc.size || !nand->ecc.strength) {
-		/* use datasheet requirements */
-		nand->ecc.strength = nand->ecc_strength_ds;
-		nand->ecc.size = nand->ecc_step_ds;
+	if (ecc->size && ecc->strength)
+		tmp = ecc->size;
+	else
+		tmp = chip->ecc_step_ds;
+	if (!tmp)
+		return -ENOTSUPP;
 
-		/*
-		 * align eccstrength and eccsize
-		 * this controller only supports 512 and 1024 sizes
-		 */
-		if (nand->ecc.size < 1024) {
-			if (mtd->writesize > 512 &&
-			    mtk_nfc_max_sector_size(nfc) > 512) {
-				nand->ecc.size = 1024;
-				nand->ecc.strength <<= 1;
-			} else {
-				nand->ecc.size = 512;
-			}
-		} else {
-			nand->ecc.size = 1024;
-		}
+	nsteps = mtd->writesize / tmp;
+	/* Adjust spare size per sector through required ecc size */
+	ret = mtk_nfc_set_spare_per_sector(tmp, &oobavail, mtd);
+	if (ret)
+		return ret;
+	oobavail -= mtk_nand->fdm.ecc_size;
+	oobavail *= nsteps;
+	if (ecc->size && ecc->strength)
+		ret = nand_check_ecc_caps(chip, caps, oobavail);
+	else
+		ret = nand_match_ecc_req(chip, caps, oobavail);
+	if (ret)
+		return ret;
 
-		ret = mtk_nfc_set_spare_per_sector(&spare, mtd);
+	if (ecc->options & NAND_ECC_MAXIMIZE) {
+		/* Maximize ecc strength */
+		ret = mtk_nfc_set_spare_per_sector(ecc->size, &oobavail,
+						   mtd);
 		if (ret)
 			return ret;
-
-		/* calculate oob bytes except ecc parity data */
-		free = mtk_ecc_calc_parity_bytes(nfc->ecc, nand->ecc.strength);
-		free = spare - free;
-
+		tmp = oobavail - caps->calc_ecc_bytes(ecc, ecc->size,
+						      ecc->strength);
 		/*
-		 * enhance ecc strength if oob left is bigger than max FDM size
-		 * or reduce ecc strength if oob size is not enough for ecc
-		 * parity data.
+		 * Only maximize ecc strength if free OOB size is more than
+		 * NFI_FDM_MAX_SIZE.
 		 */
-		if (free > NFI_FDM_MAX_SIZE) {
-			spare -= NFI_FDM_MAX_SIZE;
-			nand->ecc.strength = (spare << 3) /
-					     mtk_ecc_get_parity_bits(nfc->ecc);
-		} else if (free < 0) {
-			spare -= mtk_nand->fdm.ecc_size;
-			nand->ecc.strength = (spare << 3) /
-					     mtk_ecc_get_parity_bits(nfc->ecc);
+		if (tmp > NFI_FDM_MAX_SIZE) {
+			oobavail -= NFI_FDM_MAX_SIZE;
+			nsteps = mtd->writesize / ecc->size;
+			oobavail *= nsteps;
+			ret = nand_maximize_ecc(chip, caps, oobavail);
+			if (ret)
+				return ret;
 		}
 	}
 
-	mtk_ecc_adjust_strength(nfc->ecc, &nand->ecc.strength);
-
 	dev_info(dev, "eccsize %d eccstrength %d\n",
-		 nand->ecc.size, nand->ecc.strength);
+		 ecc->size, ecc->strength);
+
+	return 0;
+}
+
+static int mtk_nfc_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
+				  int strength)
+{
+	struct mtk_ecc *mtkecc = (struct mtk_ecc *)ecc->priv;
+
+	return mtk_ecc_calc_parity_bytes(mtkecc, strength);
+}
+
+static int mtk_nfc_ecc_caps_init(struct device *dev, struct mtk_nfc *nfc)
+{
+	struct nand_ecc_caps *ecccaps;
+	struct nand_ecc_step_info *stepinfos;
+	int i, nsector = nfc->caps->num_sector_size;
+
+	ecccaps = devm_kzalloc(dev, sizeof(*ecccaps), GFP_KERNEL);
+	if (!ecccaps)
+		return -ENOMEM;
+
+	stepinfos = devm_kzalloc(dev, sizeof(*stepinfos) * nsector, GFP_KERNEL);
+	if (!stepinfos)
+		return -ENOMEM;
+
+	nfc->ecccaps = ecccaps;
+
+	ecccaps->stepinfos = stepinfos;
+	ecccaps->nstepinfos = nsector;
+	ecccaps->calc_ecc_bytes = mtk_nfc_calc_ecc_bytes;
+
+	for (i = 0; i < nsector; i++) {
+		stepinfos->stepsize = nfc->caps->sector_size[i];
+		stepinfos->strengths = mtk_ecc_get_strength(nfc->ecc);
+		stepinfos->nstrengths = mtk_ecc_get_strength_num(nfc->ecc);
+		stepinfos++;
+	}
 
 	return 0;
 }
@@ -1329,6 +1356,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 	/* set default mode in case dt entry is missing */
 	nand->ecc.mode = NAND_ECC_HW;
 
+	nand->ecc.priv = (void *)nfc->ecc;
+
 	nand->ecc.write_subpage = mtk_nfc_write_subpage_hwecc;
 	nand->ecc.write_page_raw = mtk_nfc_write_page_raw;
 	nand->ecc.write_page = mtk_nfc_write_page_hwecc;
@@ -1366,7 +1395,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 		return -EINVAL;
 	}
 
-	ret = mtk_nfc_set_spare_per_sector(&chip->spare_per_sector, mtd);
+	ret = mtk_nfc_set_spare_per_sector(nand->ecc.size,
+					   &chip->spare_per_sector, mtd);
 	if (ret)
 		return ret;
 
@@ -1539,6 +1569,10 @@ static int mtk_nfc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, nfc);
 
+	ret = mtk_nfc_ecc_caps_init(dev, nfc);
+	if (ret)
+		goto clk_disable;
+
 	ret = mtk_nfc_nand_chips_init(dev, nfc);
 	if (ret) {
 		dev_err(dev, "failed to init nand chips\n");
-- 
1.9.1

[PATCH 8/8] mtd: rawnand: mtk: Use generic helpers to calculate ecc size and strength

[Xiaolei Li]

An optional DT properity named nand-ecc-maximize is used to choose whether
maximize ecc strength or just match ecc strength required.

But MTK nand driver always maximize ecc strength now.

This patch uses generic helpers to calculate ecc size and strength
automatically according to nand-ecc-maximize setting.

Please remember to enable nand-ecc-maximize DT setting if want to be
compatible with older Bootloader base on this patch.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_ecc.c  |  25 -------
 drivers/mtd/nand/raw/mtk_ecc.h  |   2 -
 drivers/mtd/nand/raw/mtk_nand.c | 152 ++++++++++++++++++++++++----------------
 3 files changed, 93 insertions(+), 86 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_ecc.c b/drivers/mtd/nand/raw/mtk_ecc.c
index 71390e7..cd68157 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.c
+++ b/drivers/mtd/nand/raw/mtk_ecc.c
@@ -426,31 +426,6 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
 }
 EXPORT_SYMBOL(mtk_ecc_encode);
 
-void mtk_ecc_adjust_strength(struct mtk_ecc *ecc, u32 *p)
-{
-	const int *ecc_strength = ecc->caps->ecc_strength;
-	int i;
-
-	for (i = 0; i < ecc->caps->num_ecc_strength; i++) {
-		if (*p <= ecc_strength[i]) {
-			if (!i)
-				*p = ecc_strength[i];
-			else if (*p != ecc_strength[i])
-				*p = ecc_strength[i - 1];
-			return;
-		}
-	}
-
-	*p = ecc_strength[ecc->caps->num_ecc_strength - 1];
-}
-EXPORT_SYMBOL(mtk_ecc_adjust_strength);
-
-unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc)
-{
-	return ecc->caps->parity_bits;
-}
-EXPORT_SYMBOL(mtk_ecc_get_parity_bits);
-
 int mtk_ecc_get_strength_num(struct mtk_ecc *ecc)
 {
 	return ecc->caps->num_ecc_strength;
diff --git a/drivers/mtd/nand/raw/mtk_ecc.h b/drivers/mtd/nand/raw/mtk_ecc.h
index 5687864..e7f20f0 100644
--- a/drivers/mtd/nand/raw/mtk_ecc.h
+++ b/drivers/mtd/nand/raw/mtk_ecc.h
@@ -40,8 +40,6 @@ struct mtk_ecc_config {
 int mtk_ecc_wait_done(struct mtk_ecc *, enum mtk_ecc_operation);
 int mtk_ecc_enable(struct mtk_ecc *, struct mtk_ecc_config *);
 void mtk_ecc_disable(struct mtk_ecc *);
-void mtk_ecc_adjust_strength(struct mtk_ecc *ecc, u32 *p);
-unsigned int mtk_ecc_get_parity_bits(struct mtk_ecc *ecc);
 int mtk_ecc_get_strength_num(struct mtk_ecc *ecc);
 const int *mtk_ecc_get_strength(struct mtk_ecc *ecc);
 int mtk_ecc_calc_parity_bytes(struct mtk_ecc *ecc, int ecc_strength);
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 249b3fb..b535eee 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -157,6 +157,8 @@ struct mtk_nfc {
 	struct completion done;
 	struct list_head chips;
 
+	const struct nand_ecc_caps *ecccaps;
+
 	u8 *buffer;
 };
 
@@ -266,16 +268,6 @@ static inline u8 nfi_readb(struct mtk_nfc *nfc, u32 reg)
 	return readb_relaxed(nfc->regs + reg);
 }
 
-static int mtk_nfc_max_sector_size(struct mtk_nfc *nfc)
-{
-	int i, sector_size = 0;
-
-	for (i = 0; i < nfc->caps->num_sector_size; i++)
-		sector_size = max(sector_size, nfc->caps->sector_size[i]);
-
-	return sector_size;
-}
-
 static void mtk_nfc_hw_reset(struct mtk_nfc *nfc)
 {
 	struct device *dev = nfc->dev;
@@ -1171,14 +1163,15 @@ static void mtk_nfc_set_bad_mark_ctl(struct mtk_nfc_bad_mark_ctl *bm_ctl,
 	}
 }
 
-static int mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)
+static int mtk_nfc_set_spare_per_sector(int ecc_size, u32 *sps,
+					struct mtd_info *mtd)
 {
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct mtk_nfc *nfc = nand_get_controller_data(nand);
 	const u8 *spare = nfc->caps->spare_size;
 	u32 eccsteps, i, closest_spare = 0;
 
-	eccsteps = mtd->writesize / nand->ecc.size;
+	eccsteps = mtd->writesize / ecc_size;
 	*sps = mtd->oobsize / eccsteps;
 
 	if (nand->ecc.size == 1024)
@@ -1205,68 +1198,102 @@ static int mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)
 
 static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 {
-	struct nand_chip *nand = mtd_to_nand(mtd);
-	struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(nand);
-	struct mtk_nfc *nfc = nand_get_controller_data(nand);
-	u32 spare;
-	int free, ret;
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip);
+	struct mtk_nfc *nfc = nand_get_controller_data(chip);
+	const struct nand_ecc_caps *caps = nfc->ecccaps;
+	int tmp, oobavail, nsteps, ret;
 
 	/* support only ecc hw mode */
-	if (nand->ecc.mode != NAND_ECC_HW) {
+	if (ecc->mode != NAND_ECC_HW) {
 		dev_err(dev, "ecc.mode not supported\n");
 		return -EINVAL;
 	}
 
-	/* if optional dt settings not present */
-	if (!nand->ecc.size || !nand->ecc.strength) {
-		/* use datasheet requirements */
-		nand->ecc.strength = nand->ecc_strength_ds;
-		nand->ecc.size = nand->ecc_step_ds;
+	if (ecc->size && ecc->strength)
+		tmp = ecc->size;
+	else
+		tmp = chip->ecc_step_ds;
+	if (!tmp)
+		return -ENOTSUPP;
 
-		/*
-		 * align eccstrength and eccsize
-		 * this controller only supports 512 and 1024 sizes
-		 */
-		if (nand->ecc.size < 1024) {
-			if (mtd->writesize > 512 &&
-			    mtk_nfc_max_sector_size(nfc) > 512) {
-				nand->ecc.size = 1024;
-				nand->ecc.strength <<= 1;
-			} else {
-				nand->ecc.size = 512;
-			}
-		} else {
-			nand->ecc.size = 1024;
-		}
+	nsteps = mtd->writesize / tmp;
+	/* Adjust spare size per sector through required ecc size */
+	ret = mtk_nfc_set_spare_per_sector(tmp, &oobavail, mtd);
+	if (ret)
+		return ret;
+	oobavail -= mtk_nand->fdm.ecc_size;
+	oobavail *= nsteps;
+	if (ecc->size && ecc->strength)
+		ret = nand_check_ecc_caps(chip, caps, oobavail);
+	else
+		ret = nand_match_ecc_req(chip, caps, oobavail);
+	if (ret)
+		return ret;
 
-		ret = mtk_nfc_set_spare_per_sector(&spare, mtd);
+	if (ecc->options & NAND_ECC_MAXIMIZE) {
+		/* Maximize ecc strength */
+		ret = mtk_nfc_set_spare_per_sector(ecc->size, &oobavail,
+						   mtd);
 		if (ret)
 			return ret;
-
-		/* calculate oob bytes except ecc parity data */
-		free = mtk_ecc_calc_parity_bytes(nfc->ecc, nand->ecc.strength);
-		free = spare - free;
-
+		tmp = oobavail - caps->calc_ecc_bytes(ecc, ecc->size,
+						      ecc->strength);
 		/*
-		 * enhance ecc strength if oob left is bigger than max FDM size
-		 * or reduce ecc strength if oob size is not enough for ecc
-		 * parity data.
+		 * Only maximize ecc strength if free OOB size is more than
+		 * NFI_FDM_MAX_SIZE.
 		 */
-		if (free > NFI_FDM_MAX_SIZE) {
-			spare -= NFI_FDM_MAX_SIZE;
-			nand->ecc.strength = (spare << 3) /
-					     mtk_ecc_get_parity_bits(nfc->ecc);
-		} else if (free < 0) {
-			spare -= mtk_nand->fdm.ecc_size;
-			nand->ecc.strength = (spare << 3) /
-					     mtk_ecc_get_parity_bits(nfc->ecc);
+		if (tmp > NFI_FDM_MAX_SIZE) {
+			oobavail -= NFI_FDM_MAX_SIZE;
+			nsteps = mtd->writesize / ecc->size;
+			oobavail *= nsteps;
+			ret = nand_maximize_ecc(chip, caps, oobavail);
+			if (ret)
+				return ret;
 		}
 	}
 
-	mtk_ecc_adjust_strength(nfc->ecc, &nand->ecc.strength);
-
 	dev_info(dev, "eccsize %d eccstrength %d\n",
-		 nand->ecc.size, nand->ecc.strength);
+		 ecc->size, ecc->strength);
+
+	return 0;
+}
+
+static int mtk_nfc_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
+				  int strength)
+{
+	struct mtk_ecc *mtkecc = (struct mtk_ecc *)ecc->priv;
+
+	return mtk_ecc_calc_parity_bytes(mtkecc, strength);
+}
+
+static int mtk_nfc_ecc_caps_init(struct device *dev, struct mtk_nfc *nfc)
+{
+	struct nand_ecc_caps *ecccaps;
+	struct nand_ecc_step_info *stepinfos;
+	int i, nsector = nfc->caps->num_sector_size;
+
+	ecccaps = devm_kzalloc(dev, sizeof(*ecccaps), GFP_KERNEL);
+	if (!ecccaps)
+		return -ENOMEM;
+
+	stepinfos = devm_kzalloc(dev, sizeof(*stepinfos) * nsector, GFP_KERNEL);
+	if (!stepinfos)
+		return -ENOMEM;
+
+	nfc->ecccaps = ecccaps;
+
+	ecccaps->stepinfos = stepinfos;
+	ecccaps->nstepinfos = nsector;
+	ecccaps->calc_ecc_bytes = mtk_nfc_calc_ecc_bytes;
+
+	for (i = 0; i < nsector; i++) {
+		stepinfos->stepsize = nfc->caps->sector_size[i];
+		stepinfos->strengths = mtk_ecc_get_strength(nfc->ecc);
+		stepinfos->nstrengths = mtk_ecc_get_strength_num(nfc->ecc);
+		stepinfos++;
+	}
 
 	return 0;
 }
@@ -1329,6 +1356,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 	/* set default mode in case dt entry is missing */
 	nand->ecc.mode = NAND_ECC_HW;
 
+	nand->ecc.priv = (void *)nfc->ecc;
+
 	nand->ecc.write_subpage = mtk_nfc_write_subpage_hwecc;
 	nand->ecc.write_page_raw = mtk_nfc_write_page_raw;
 	nand->ecc.write_page = mtk_nfc_write_page_hwecc;
@@ -1366,7 +1395,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 		return -EINVAL;
 	}
 
-	ret = mtk_nfc_set_spare_per_sector(&chip->spare_per_sector, mtd);
+	ret = mtk_nfc_set_spare_per_sector(nand->ecc.size,
+					   &chip->spare_per_sector, mtd);
 	if (ret)
 		return ret;
 
@@ -1539,6 +1569,10 @@ static int mtk_nfc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, nfc);
 
+	ret = mtk_nfc_ecc_caps_init(dev, nfc);
+	if (ret)
+		goto clk_disable;
+
 	ret = mtk_nfc_nand_chips_init(dev, nfc);
 	if (ret) {
 		dev_err(dev, "failed to init nand chips\n");
-- 
1.9.1


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

Re: [PATCH 5/8] mtd: rawnand: Modify ->calc_ecc_bytes() hook in nand_ecc_caps

[Boris Brezillon]

On Wed, 11 Apr 2018 11:41:55 +0800
Xiaolei Li <xiaolei.li@mediatek.com> wrote:

> Maybe some controllers need more information besides step size and ecc
> strength to calculate ECC bytes.
> 
> struct nand_ecc_ctrl provides lots of ECC control information include
> private ECC control data. So, add it into ->calc_ecc_bytes() interface,
> to make this hook be more flexible.

I'm not fond of this approach. Why don't you provide a different
function for the 2 cases you have (parity = 13 and parity = 14)? 

> 
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/denali.c    |  3 +-
>  drivers/mtd/nand/raw/denali.h    |  2 +-
>  drivers/mtd/nand/raw/nand_base.c | 11 ++++--
>  include/linux/mtd/rawnand.h      | 77 ++++++++++++++++++++--------------------
>  4 files changed, 50 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
> index 2a302a1..907609c 100644
> --- a/drivers/mtd/nand/raw/denali.c
> +++ b/drivers/mtd/nand/raw/denali.c
> @@ -1113,7 +1113,8 @@ static void denali_hw_init(struct denali_nand_info *denali)
>  	iowrite32(0xffff, denali->reg + SPARE_AREA_MARKER);
>  }
>  
> -int denali_calc_ecc_bytes(int step_size, int strength)
> +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
> +			  int strength)
>  {
>  	/* BCH code.  Denali requires ecc.bytes to be multiple of 2 */
>  	return DIV_ROUND_UP(strength * fls(step_size * 8), 16) * 2;
> diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h
> index 9ad33d2..644dffd 100644
> --- a/drivers/mtd/nand/raw/denali.h
> +++ b/drivers/mtd/nand/raw/denali.h
> @@ -328,7 +328,7 @@ struct denali_nand_info {
>  #define DENALI_CAP_HW_ECC_FIXUP			BIT(0)
>  #define DENALI_CAP_DMA_64BIT			BIT(1)
>  
> -int denali_calc_ecc_bytes(int step_size, int strength);
> +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *, int, int);
>  int denali_init(struct denali_nand_info *denali);
>  void denali_remove(struct denali_nand_info *denali);
>  
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index d0b993f..e17abc8 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -6052,6 +6052,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
>  			const struct nand_ecc_caps *caps, int oobavail)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	const struct nand_ecc_step_info *stepinfo;
>  	int preset_step = chip->ecc.size;
>  	int preset_strength = chip->ecc.strength;
> @@ -6076,7 +6077,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
>  			if (stepinfo->strengths[j] != preset_strength)
>  				continue;
>  
> -			ecc_bytes = caps->calc_ecc_bytes(preset_step,
> +			ecc_bytes = caps->calc_ecc_bytes(ecc, preset_step,
>  							 preset_strength);
>  			if (WARN_ON_ONCE(ecc_bytes < 0))
>  				return ecc_bytes;
> @@ -6114,6 +6115,7 @@ int nand_match_ecc_req(struct nand_chip *chip,
>  		       const struct nand_ecc_caps *caps, int oobavail)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	const struct nand_ecc_step_info *stepinfo;
>  	int req_step = chip->ecc_step_ds;
>  	int req_strength = chip->ecc_strength_ds;
> @@ -6152,7 +6154,8 @@ int nand_match_ecc_req(struct nand_chip *chip,
>  
>  			nsteps = mtd->writesize / step_size;
>  
> -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> +							 strength);
>  			if (WARN_ON_ONCE(ecc_bytes < 0))
>  				continue;
>  			ecc_bytes_total = ecc_bytes * nsteps;
> @@ -6198,6 +6201,7 @@ int nand_maximize_ecc(struct nand_chip *chip,
>  		      const struct nand_ecc_caps *caps, int oobavail)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	const struct nand_ecc_step_info *stepinfo;
>  	int step_size, strength, nsteps, ecc_bytes, corr;
>  	int best_corr = 0;
> @@ -6224,7 +6228,8 @@ int nand_maximize_ecc(struct nand_chip *chip,
>  
>  			nsteps = mtd->writesize / step_size;
>  
> -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> +							 strength);
>  			if (WARN_ON_ONCE(ecc_bytes < 0))
>  				continue;
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 5dad59b..e7c7fdb 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -507,44 +507,6 @@ static inline void nand_hw_control_init(struct nand_hw_control *nfc)
>  }
>  
>  /**
> - * struct nand_ecc_step_info - ECC step information of ECC engine
> - * @stepsize: data bytes per ECC step
> - * @strengths: array of supported strengths
> - * @nstrengths: number of supported strengths
> - */
> -struct nand_ecc_step_info {
> -	int stepsize;
> -	const int *strengths;
> -	int nstrengths;
> -};
> -
> -/**
> - * struct nand_ecc_caps - capability of ECC engine
> - * @stepinfos: array of ECC step information
> - * @nstepinfos: number of ECC step information
> - * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> - */
> -struct nand_ecc_caps {
> -	const struct nand_ecc_step_info *stepinfos;
> -	int nstepinfos;
> -	int (*calc_ecc_bytes)(int step_size, int strength);
> -};
> -
> -/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> -#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> -static const int __name##_strengths[] = { __VA_ARGS__ };	\
> -static const struct nand_ecc_step_info __name##_stepinfo = {	\
> -	.stepsize = __step,					\
> -	.strengths = __name##_strengths,			\
> -	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> -};								\
> -static const struct nand_ecc_caps __name = {			\
> -	.stepinfos = &__name##_stepinfo,			\
> -	.nstepinfos = 1,					\
> -	.calc_ecc_bytes = __calc,				\
> -}
> -
> -/**
>   * struct nand_ecc_ctrl - Control structure for ECC
>   * @mode:	ECC mode
>   * @algo:	ECC algorithm
> @@ -639,6 +601,45 @@ struct nand_ecc_ctrl {
>  };
>  
>  /**
> + * struct nand_ecc_step_info - ECC step information of ECC engine
> + * @stepsize: data bytes per ECC step
> + * @strengths: array of supported strengths
> + * @nstrengths: number of supported strengths
> + */
> +struct nand_ecc_step_info {
> +	int stepsize;
> +	const int *strengths;
> +	int nstrengths;
> +};
> +
> +/**
> + * struct nand_ecc_caps - capability of ECC engine
> + * @stepinfos: array of ECC step information
> + * @nstepinfos: number of ECC step information
> + * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> + */
> +struct nand_ecc_caps {
> +	const struct nand_ecc_step_info *stepinfos;
> +	int nstepinfos;
> +	int (*calc_ecc_bytes)(struct nand_ecc_ctrl *ecc, int step_size,
> +			      int strength);
> +};
> +
> +/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> +#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> +static const int __name##_strengths[] = { __VA_ARGS__ };	\
> +static const struct nand_ecc_step_info __name##_stepinfo = {	\
> +	.stepsize = __step,					\
> +	.strengths = __name##_strengths,			\
> +	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> +};								\
> +static const struct nand_ecc_caps __name = {			\
> +	.stepinfos = &__name##_stepinfo,			\
> +	.nstepinfos = 1,					\
> +	.calc_ecc_bytes = __calc,				\
> +}
> +
> +/**
>   * struct nand_sdr_timings - SDR NAND chip timings
>   *
>   * This struct defines the timing requirements of a SDR NAND chip.

Re: [PATCH 5/8] mtd: rawnand: Modify ->calc_ecc_bytes() hook in nand_ecc_caps

[Boris Brezillon]

On Wed, 11 Apr 2018 11:41:55 +0800
Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> Maybe some controllers need more information besides step size and ecc
> strength to calculate ECC bytes.
> 
> struct nand_ecc_ctrl provides lots of ECC control information include
> private ECC control data. So, add it into ->calc_ecc_bytes() interface,
> to make this hook be more flexible.

I'm not fond of this approach. Why don't you provide a different
function for the 2 cases you have (parity = 13 and parity = 14)? 

> 
> Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/mtd/nand/raw/denali.c    |  3 +-
>  drivers/mtd/nand/raw/denali.h    |  2 +-
>  drivers/mtd/nand/raw/nand_base.c | 11 ++++--
>  include/linux/mtd/rawnand.h      | 77 ++++++++++++++++++++--------------------
>  4 files changed, 50 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
> index 2a302a1..907609c 100644
> --- a/drivers/mtd/nand/raw/denali.c
> +++ b/drivers/mtd/nand/raw/denali.c
> @@ -1113,7 +1113,8 @@ static void denali_hw_init(struct denali_nand_info *denali)
>  	iowrite32(0xffff, denali->reg + SPARE_AREA_MARKER);
>  }
>  
> -int denali_calc_ecc_bytes(int step_size, int strength)
> +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
> +			  int strength)
>  {
>  	/* BCH code.  Denali requires ecc.bytes to be multiple of 2 */
>  	return DIV_ROUND_UP(strength * fls(step_size * 8), 16) * 2;
> diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h
> index 9ad33d2..644dffd 100644
> --- a/drivers/mtd/nand/raw/denali.h
> +++ b/drivers/mtd/nand/raw/denali.h
> @@ -328,7 +328,7 @@ struct denali_nand_info {
>  #define DENALI_CAP_HW_ECC_FIXUP			BIT(0)
>  #define DENALI_CAP_DMA_64BIT			BIT(1)
>  
> -int denali_calc_ecc_bytes(int step_size, int strength);
> +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *, int, int);
>  int denali_init(struct denali_nand_info *denali);
>  void denali_remove(struct denali_nand_info *denali);
>  
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index d0b993f..e17abc8 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -6052,6 +6052,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
>  			const struct nand_ecc_caps *caps, int oobavail)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	const struct nand_ecc_step_info *stepinfo;
>  	int preset_step = chip->ecc.size;
>  	int preset_strength = chip->ecc.strength;
> @@ -6076,7 +6077,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
>  			if (stepinfo->strengths[j] != preset_strength)
>  				continue;
>  
> -			ecc_bytes = caps->calc_ecc_bytes(preset_step,
> +			ecc_bytes = caps->calc_ecc_bytes(ecc, preset_step,
>  							 preset_strength);
>  			if (WARN_ON_ONCE(ecc_bytes < 0))
>  				return ecc_bytes;
> @@ -6114,6 +6115,7 @@ int nand_match_ecc_req(struct nand_chip *chip,
>  		       const struct nand_ecc_caps *caps, int oobavail)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	const struct nand_ecc_step_info *stepinfo;
>  	int req_step = chip->ecc_step_ds;
>  	int req_strength = chip->ecc_strength_ds;
> @@ -6152,7 +6154,8 @@ int nand_match_ecc_req(struct nand_chip *chip,
>  
>  			nsteps = mtd->writesize / step_size;
>  
> -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> +							 strength);
>  			if (WARN_ON_ONCE(ecc_bytes < 0))
>  				continue;
>  			ecc_bytes_total = ecc_bytes * nsteps;
> @@ -6198,6 +6201,7 @@ int nand_maximize_ecc(struct nand_chip *chip,
>  		      const struct nand_ecc_caps *caps, int oobavail)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	const struct nand_ecc_step_info *stepinfo;
>  	int step_size, strength, nsteps, ecc_bytes, corr;
>  	int best_corr = 0;
> @@ -6224,7 +6228,8 @@ int nand_maximize_ecc(struct nand_chip *chip,
>  
>  			nsteps = mtd->writesize / step_size;
>  
> -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> +							 strength);
>  			if (WARN_ON_ONCE(ecc_bytes < 0))
>  				continue;
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 5dad59b..e7c7fdb 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -507,44 +507,6 @@ static inline void nand_hw_control_init(struct nand_hw_control *nfc)
>  }
>  
>  /**
> - * struct nand_ecc_step_info - ECC step information of ECC engine
> - * @stepsize: data bytes per ECC step
> - * @strengths: array of supported strengths
> - * @nstrengths: number of supported strengths
> - */
> -struct nand_ecc_step_info {
> -	int stepsize;
> -	const int *strengths;
> -	int nstrengths;
> -};
> -
> -/**
> - * struct nand_ecc_caps - capability of ECC engine
> - * @stepinfos: array of ECC step information
> - * @nstepinfos: number of ECC step information
> - * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> - */
> -struct nand_ecc_caps {
> -	const struct nand_ecc_step_info *stepinfos;
> -	int nstepinfos;
> -	int (*calc_ecc_bytes)(int step_size, int strength);
> -};
> -
> -/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> -#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> -static const int __name##_strengths[] = { __VA_ARGS__ };	\
> -static const struct nand_ecc_step_info __name##_stepinfo = {	\
> -	.stepsize = __step,					\
> -	.strengths = __name##_strengths,			\
> -	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> -};								\
> -static const struct nand_ecc_caps __name = {			\
> -	.stepinfos = &__name##_stepinfo,			\
> -	.nstepinfos = 1,					\
> -	.calc_ecc_bytes = __calc,				\
> -}
> -
> -/**
>   * struct nand_ecc_ctrl - Control structure for ECC
>   * @mode:	ECC mode
>   * @algo:	ECC algorithm
> @@ -639,6 +601,45 @@ struct nand_ecc_ctrl {
>  };
>  
>  /**
> + * struct nand_ecc_step_info - ECC step information of ECC engine
> + * @stepsize: data bytes per ECC step
> + * @strengths: array of supported strengths
> + * @nstrengths: number of supported strengths
> + */
> +struct nand_ecc_step_info {
> +	int stepsize;
> +	const int *strengths;
> +	int nstrengths;
> +};
> +
> +/**
> + * struct nand_ecc_caps - capability of ECC engine
> + * @stepinfos: array of ECC step information
> + * @nstepinfos: number of ECC step information
> + * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> + */
> +struct nand_ecc_caps {
> +	const struct nand_ecc_step_info *stepinfos;
> +	int nstepinfos;
> +	int (*calc_ecc_bytes)(struct nand_ecc_ctrl *ecc, int step_size,
> +			      int strength);
> +};
> +
> +/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> +#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> +static const int __name##_strengths[] = { __VA_ARGS__ };	\
> +static const struct nand_ecc_step_info __name##_stepinfo = {	\
> +	.stepsize = __step,					\
> +	.strengths = __name##_strengths,			\
> +	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> +};								\
> +static const struct nand_ecc_caps __name = {			\
> +	.stepinfos = &__name##_stepinfo,			\
> +	.nstepinfos = 1,					\
> +	.calc_ecc_bytes = __calc,				\
> +}
> +
> +/**
>   * struct nand_sdr_timings - SDR NAND chip timings
>   *
>   * This struct defines the timing requirements of a SDR NAND chip.

Re: [PATCH 8/8] mtd: rawnand: mtk: Use generic helpers to calculate ecc size and strength

[Boris Brezillon]

On Wed, 11 Apr 2018 11:41:58 +0800
Xiaolei Li <xiaolei.li@mediatek.com> wrote:

> An optional DT properity named nand-ecc-maximize is used to choose whether
> maximize ecc strength or just match ecc strength required.
> 
> But MTK nand driver always maximize ecc strength now.
> 
> This patch uses generic helpers to calculate ecc size and strength
> automatically according to nand-ecc-maximize setting.
> 
> Please remember to enable nand-ecc-maximize DT setting if want to be
> compatible with older Bootloader base on this patch.

You're breaking backward compat here. Not sure this is a good idea, but
if that's what you want I'd like a few more acks to confirm mediatek
maintainers are okay with that.

And please add a patch updating the DT bindinds doc accordingly.

> 
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/mtk_ecc.c  |  25 -------
>  drivers/mtd/nand/raw/mtk_ecc.h  |   2 -
>  drivers/mtd/nand/raw/mtk_nand.c | 152 ++++++++++++++++++++++++----------------
>  3 files changed, 93 insertions(+), 86 deletions(-)
> 

> +
> +static int mtk_nfc_ecc_caps_init(struct device *dev, struct mtk_nfc *nfc)
> +{
> +	struct nand_ecc_caps *ecccaps;
> +	struct nand_ecc_step_info *stepinfos;
> +	int i, nsector = nfc->caps->num_sector_size;
> +
> +	ecccaps = devm_kzalloc(dev, sizeof(*ecccaps), GFP_KERNEL);
> +	if (!ecccaps)
> +		return -ENOMEM;
> +
> +	stepinfos = devm_kzalloc(dev, sizeof(*stepinfos) * nsector, GFP_KERNEL);
> +	if (!stepinfos)
> +		return -ENOMEM;
> +
> +	nfc->ecccaps = ecccaps;
> +
> +	ecccaps->stepinfos = stepinfos;
> +	ecccaps->nstepinfos = nsector;
> +	ecccaps->calc_ecc_bytes = mtk_nfc_calc_ecc_bytes;
> +
> +	for (i = 0; i < nsector; i++) {
> +		stepinfos->stepsize = nfc->caps->sector_size[i];
> +		stepinfos->strengths = mtk_ecc_get_strength(nfc->ecc);
> +		stepinfos->nstrengths = mtk_ecc_get_strength_num(nfc->ecc);
> +		stepinfos++;
> +	}

You seem to re-create generic tables from mtk's internal
representation. Why don't you directly store a static const version of
nand_ecc_caps in you caps struct. And maybe you can also get rid of the
mtk specific representation in favor of the generic one.

>  
>  	return 0;
>  }
> @@ -1329,6 +1356,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
>  	/* set default mode in case dt entry is missing */
>  	nand->ecc.mode = NAND_ECC_HW;
>  
> +	nand->ecc.priv = (void *)nfc->ecc;
> +
>  	nand->ecc.write_subpage = mtk_nfc_write_subpage_hwecc;
>  	nand->ecc.write_page_raw = mtk_nfc_write_page_raw;
>  	nand->ecc.write_page = mtk_nfc_write_page_hwecc;
> @@ -1366,7 +1395,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
>  		return -EINVAL;
>  	}
>  
> -	ret = mtk_nfc_set_spare_per_sector(&chip->spare_per_sector, mtd);
> +	ret = mtk_nfc_set_spare_per_sector(nand->ecc.size,
> +					   &chip->spare_per_sector, mtd);
>  	if (ret)
>  		return ret;
>  
> @@ -1539,6 +1569,10 @@ static int mtk_nfc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, nfc);
>  
> +	ret = mtk_nfc_ecc_caps_init(dev, nfc);
> +	if (ret)
> +		goto clk_disable;
> +
>  	ret = mtk_nfc_nand_chips_init(dev, nfc);
>  	if (ret) {
>  		dev_err(dev, "failed to init nand chips\n");

Re: [PATCH 8/8] mtd: rawnand: mtk: Use generic helpers to calculate ecc size and strength

[Boris Brezillon]

On Wed, 11 Apr 2018 11:41:58 +0800
Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> An optional DT properity named nand-ecc-maximize is used to choose whether
> maximize ecc strength or just match ecc strength required.
> 
> But MTK nand driver always maximize ecc strength now.
> 
> This patch uses generic helpers to calculate ecc size and strength
> automatically according to nand-ecc-maximize setting.
> 
> Please remember to enable nand-ecc-maximize DT setting if want to be
> compatible with older Bootloader base on this patch.

You're breaking backward compat here. Not sure this is a good idea, but
if that's what you want I'd like a few more acks to confirm mediatek
maintainers are okay with that.

And please add a patch updating the DT bindinds doc accordingly.

> 
> Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/mtd/nand/raw/mtk_ecc.c  |  25 -------
>  drivers/mtd/nand/raw/mtk_ecc.h  |   2 -
>  drivers/mtd/nand/raw/mtk_nand.c | 152 ++++++++++++++++++++++++----------------
>  3 files changed, 93 insertions(+), 86 deletions(-)
> 

> +
> +static int mtk_nfc_ecc_caps_init(struct device *dev, struct mtk_nfc *nfc)
> +{
> +	struct nand_ecc_caps *ecccaps;
> +	struct nand_ecc_step_info *stepinfos;
> +	int i, nsector = nfc->caps->num_sector_size;
> +
> +	ecccaps = devm_kzalloc(dev, sizeof(*ecccaps), GFP_KERNEL);
> +	if (!ecccaps)
> +		return -ENOMEM;
> +
> +	stepinfos = devm_kzalloc(dev, sizeof(*stepinfos) * nsector, GFP_KERNEL);
> +	if (!stepinfos)
> +		return -ENOMEM;
> +
> +	nfc->ecccaps = ecccaps;
> +
> +	ecccaps->stepinfos = stepinfos;
> +	ecccaps->nstepinfos = nsector;
> +	ecccaps->calc_ecc_bytes = mtk_nfc_calc_ecc_bytes;
> +
> +	for (i = 0; i < nsector; i++) {
> +		stepinfos->stepsize = nfc->caps->sector_size[i];
> +		stepinfos->strengths = mtk_ecc_get_strength(nfc->ecc);
> +		stepinfos->nstrengths = mtk_ecc_get_strength_num(nfc->ecc);
> +		stepinfos++;
> +	}

You seem to re-create generic tables from mtk's internal
representation. Why don't you directly store a static const version of
nand_ecc_caps in you caps struct. And maybe you can also get rid of the
mtk specific representation in favor of the generic one.

>  
>  	return 0;
>  }
> @@ -1329,6 +1356,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
>  	/* set default mode in case dt entry is missing */
>  	nand->ecc.mode = NAND_ECC_HW;
>  
> +	nand->ecc.priv = (void *)nfc->ecc;
> +
>  	nand->ecc.write_subpage = mtk_nfc_write_subpage_hwecc;
>  	nand->ecc.write_page_raw = mtk_nfc_write_page_raw;
>  	nand->ecc.write_page = mtk_nfc_write_page_hwecc;
> @@ -1366,7 +1395,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
>  		return -EINVAL;
>  	}
>  
> -	ret = mtk_nfc_set_spare_per_sector(&chip->spare_per_sector, mtd);
> +	ret = mtk_nfc_set_spare_per_sector(nand->ecc.size,
> +					   &chip->spare_per_sector, mtd);
>  	if (ret)
>  		return ret;
>  
> @@ -1539,6 +1569,10 @@ static int mtk_nfc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, nfc);
>  
> +	ret = mtk_nfc_ecc_caps_init(dev, nfc);
> +	if (ret)
> +		goto clk_disable;
> +
>  	ret = mtk_nfc_nand_chips_init(dev, nfc);
>  	if (ret) {
>  		dev_err(dev, "failed to init nand chips\n");

Re: [PATCH 3/8] mtd: rawnand: mtk: Add DT property mtk,fdm-ecc-size

[Boris Brezillon]

On Wed, 11 Apr 2018 11:41:53 +0800
Xiaolei Li <xiaolei.li@mediatek.com> wrote:

> For some MTK NAND chips, BootROM may access more than one byte
> ECC protected FDM data, but now we fix ECC protected FDM byte as 1.
> This will make some chips be failed to boot up.
> 
> With this DT property setting, different MTK NAND chips with the same
> NAND controller IP can work well.
> 
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  Documentation/devicetree/bindings/mtd/mtk-nand.txt |  6 ++++++
>  drivers/mtd/nand/raw/mtk_nand.c                    | 25 ++++++++++++++++------
>  2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-nand.txt b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> index ef786568..a8e4136 100644
> --- a/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> @@ -47,6 +47,12 @@ Children nodes properties:
>  - reg:			Chip Select Signal, default 0.
>  			Set as reg = <0>, <1> when need 2 CS.
>  Optional:
> +- mtk,fdm-ecc-size:	Integer representing ECC protected FDM bytes.
> +			Should be in the range [1,8], if not present 1.
> +			On some MTK NAND chips, BootROM may access more than
> +			one byte ECC protected FDM data. Different MTK chips
> +			with the same NAND controller IP will work well with
> +			this properity setting.

Is this something that changes on a per-SoC basis, or can a specific
SoC have a different behavior depending on the version of the BootROM
it embeds (that would be quite tricky to deal with since that would
mean having different dts if you start using newer revisions of the same
SoC).

If it's on a per-SoC basis, I'd recommend defining new compatibles
instead of adding this property.

Re: [PATCH 3/8] mtd: rawnand: mtk: Add DT property mtk,fdm-ecc-size

[Boris Brezillon]

On Wed, 11 Apr 2018 11:41:53 +0800
Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> For some MTK NAND chips, BootROM may access more than one byte
> ECC protected FDM data, but now we fix ECC protected FDM byte as 1.
> This will make some chips be failed to boot up.
> 
> With this DT property setting, different MTK NAND chips with the same
> NAND controller IP can work well.
> 
> Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mtd/mtk-nand.txt |  6 ++++++
>  drivers/mtd/nand/raw/mtk_nand.c                    | 25 ++++++++++++++++------
>  2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-nand.txt b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> index ef786568..a8e4136 100644
> --- a/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> @@ -47,6 +47,12 @@ Children nodes properties:
>  - reg:			Chip Select Signal, default 0.
>  			Set as reg = <0>, <1> when need 2 CS.
>  Optional:
> +- mtk,fdm-ecc-size:	Integer representing ECC protected FDM bytes.
> +			Should be in the range [1,8], if not present 1.
> +			On some MTK NAND chips, BootROM may access more than
> +			one byte ECC protected FDM data. Different MTK chips
> +			with the same NAND controller IP will work well with
> +			this properity setting.

Is this something that changes on a per-SoC basis, or can a specific
SoC have a different behavior depending on the version of the BootROM
it embeds (that would be quite tricky to deal with since that would
mean having different dts if you start using newer revisions of the same
SoC).

If it's on a per-SoC basis, I'd recommend defining new compatibles
instead of adding this property.

Re: [PATCH 3/8] mtd: rawnand: mtk: Add DT property mtk,fdm-ecc-size

[xiaolei li]

Hi Boris,

On Wed, 2018-04-11 at 21:13 +0200, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 11:41:53 +0800
> Xiaolei Li <xiaolei.li@mediatek.com> wrote:
> 
> > For some MTK NAND chips, BootROM may access more than one byte
> > ECC protected FDM data, but now we fix ECC protected FDM byte as 1.
> > This will make some chips be failed to boot up.
> > 
> > With this DT property setting, different MTK NAND chips with the same
> > NAND controller IP can work well.
> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/mtk-nand.txt |  6 ++++++
> >  drivers/mtd/nand/raw/mtk_nand.c                    | 25 ++++++++++++++++------
> >  2 files changed, 24 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/mtk-nand.txt b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> > index ef786568..a8e4136 100644
> > --- a/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> > @@ -47,6 +47,12 @@ Children nodes properties:
> >  - reg:			Chip Select Signal, default 0.
> >  			Set as reg = <0>, <1> when need 2 CS.
> >  Optional:
> > +- mtk,fdm-ecc-size:	Integer representing ECC protected FDM bytes.
> > +			Should be in the range [1,8], if not present 1.
> > +			On some MTK NAND chips, BootROM may access more than
> > +			one byte ECC protected FDM data. Different MTK chips
> > +			with the same NAND controller IP will work well with
> > +			this properity setting.
> 
> Is this something that changes on a per-SoC basis, or can a specific
> SoC have a different behavior depending on the version of the BootROM
> it embeds (that would be quite tricky to deal with since that would
> mean having different dts if you start using newer revisions of the same
> SoC).
> 
> If it's on a per-SoC basis, I'd recommend defining new compatibles
> instead of adding this property.
> 
Thanks for your advice. I will change to add new compatibles for this
setting next patch.

Thanks,
Xiaolei
> 

Re: [PATCH 3/8] mtd: rawnand: mtk: Add DT property mtk,fdm-ecc-size

[xiaolei li]

Hi Boris,

On Wed, 2018-04-11 at 21:13 +0200, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 11:41:53 +0800
> Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> > For some MTK NAND chips, BootROM may access more than one byte
> > ECC protected FDM data, but now we fix ECC protected FDM byte as 1.
> > This will make some chips be failed to boot up.
> > 
> > With this DT property setting, different MTK NAND chips with the same
> > NAND controller IP can work well.
> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/mtd/mtk-nand.txt |  6 ++++++
> >  drivers/mtd/nand/raw/mtk_nand.c                    | 25 ++++++++++++++++------
> >  2 files changed, 24 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/mtk-nand.txt b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> > index ef786568..a8e4136 100644
> > --- a/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
> > @@ -47,6 +47,12 @@ Children nodes properties:
> >  - reg:			Chip Select Signal, default 0.
> >  			Set as reg = <0>, <1> when need 2 CS.
> >  Optional:
> > +- mtk,fdm-ecc-size:	Integer representing ECC protected FDM bytes.
> > +			Should be in the range [1,8], if not present 1.
> > +			On some MTK NAND chips, BootROM may access more than
> > +			one byte ECC protected FDM data. Different MTK chips
> > +			with the same NAND controller IP will work well with
> > +			this properity setting.
> 
> Is this something that changes on a per-SoC basis, or can a specific
> SoC have a different behavior depending on the version of the BootROM
> it embeds (that would be quite tricky to deal with since that would
> mean having different dts if you start using newer revisions of the same
> SoC).
> 
> If it's on a per-SoC basis, I'd recommend defining new compatibles
> instead of adding this property.
> 
Thanks for your advice. I will change to add new compatibles for this
setting next patch.

Thanks,
Xiaolei
> 

Re: [PATCH 8/8] mtd: rawnand: mtk: Use generic helpers to calculate ecc size and strength

[xiaolei li]

On Wed, 2018-04-11 at 21:05 +0200, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 11:41:58 +0800
> Xiaolei Li <xiaolei.li@mediatek.com> wrote:
> 
> > An optional DT properity named nand-ecc-maximize is used to choose whether
> > maximize ecc strength or just match ecc strength required.
> > 
> > But MTK nand driver always maximize ecc strength now.
> > 
> > This patch uses generic helpers to calculate ecc size and strength
> > automatically according to nand-ecc-maximize setting.
> > 
> > Please remember to enable nand-ecc-maximize DT setting if want to be
> > compatible with older Bootloader base on this patch.
> 
> You're breaking backward compat here. Not sure this is a good idea, but
> if that's what you want I'd like a few more acks to confirm mediatek
> maintainers are okay with that.
> 
> And please add a patch updating the DT bindinds doc accordingly.

Yes. It is not backward compatibility. It is not good.
Or, just use generic helpers here but keep to maximize ecc strength like
before, what do you think about it?

> 
> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_ecc.c  |  25 -------
> >  drivers/mtd/nand/raw/mtk_ecc.h  |   2 -
> >  drivers/mtd/nand/raw/mtk_nand.c | 152 ++++++++++++++++++++++++----------------
> >  3 files changed, 93 insertions(+), 86 deletions(-)
> > 
> 
> > +
> > +static int mtk_nfc_ecc_caps_init(struct device *dev, struct mtk_nfc *nfc)
> > +{
> > +	struct nand_ecc_caps *ecccaps;
> > +	struct nand_ecc_step_info *stepinfos;
> > +	int i, nsector = nfc->caps->num_sector_size;
> > +
> > +	ecccaps = devm_kzalloc(dev, sizeof(*ecccaps), GFP_KERNEL);
> > +	if (!ecccaps)
> > +		return -ENOMEM;
> > +
> > +	stepinfos = devm_kzalloc(dev, sizeof(*stepinfos) * nsector, GFP_KERNEL);
> > +	if (!stepinfos)
> > +		return -ENOMEM;
> > +
> > +	nfc->ecccaps = ecccaps;
> > +
> > +	ecccaps->stepinfos = stepinfos;
> > +	ecccaps->nstepinfos = nsector;
> > +	ecccaps->calc_ecc_bytes = mtk_nfc_calc_ecc_bytes;
> > +
> > +	for (i = 0; i < nsector; i++) {
> > +		stepinfos->stepsize = nfc->caps->sector_size[i];
> > +		stepinfos->strengths = mtk_ecc_get_strength(nfc->ecc);
> > +		stepinfos->nstrengths = mtk_ecc_get_strength_num(nfc->ecc);
> > +		stepinfos++;
> > +	}
> 
> You seem to re-create generic tables from mtk's internal
> representation. Why don't you directly store a static const version of
> nand_ecc_caps in you caps struct. And maybe you can also get rid of the
> mtk specific representation in favor of the generic one.

OK. It is OK for me. Thanks.

> 
> >  
> >  	return 0;
> >  }
> > @@ -1329,6 +1356,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
> >  	/* set default mode in case dt entry is missing */
> >  	nand->ecc.mode = NAND_ECC_HW;
> >  
> > +	nand->ecc.priv = (void *)nfc->ecc;
> > +
> >  	nand->ecc.write_subpage = mtk_nfc_write_subpage_hwecc;
> >  	nand->ecc.write_page_raw = mtk_nfc_write_page_raw;
> >  	nand->ecc.write_page = mtk_nfc_write_page_hwecc;
> > @@ -1366,7 +1395,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
> >  		return -EINVAL;
> >  	}
> >  
> > -	ret = mtk_nfc_set_spare_per_sector(&chip->spare_per_sector, mtd);
> > +	ret = mtk_nfc_set_spare_per_sector(nand->ecc.size,
> > +					   &chip->spare_per_sector, mtd);
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -1539,6 +1569,10 @@ static int mtk_nfc_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, nfc);
> >  
> > +	ret = mtk_nfc_ecc_caps_init(dev, nfc);
> > +	if (ret)
> > +		goto clk_disable;
> > +
> >  	ret = mtk_nfc_nand_chips_init(dev, nfc);
> >  	if (ret) {
> >  		dev_err(dev, "failed to init nand chips\n");
> 

Re: [PATCH 8/8] mtd: rawnand: mtk: Use generic helpers to calculate ecc size and strength

[xiaolei li]

On Wed, 2018-04-11 at 21:05 +0200, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 11:41:58 +0800
> Xiaolei Li <xiaolei.li@mediatek.com> wrote:
> 
> > An optional DT properity named nand-ecc-maximize is used to choose whether
> > maximize ecc strength or just match ecc strength required.
> > 
> > But MTK nand driver always maximize ecc strength now.
> > 
> > This patch uses generic helpers to calculate ecc size and strength
> > automatically according to nand-ecc-maximize setting.
> > 
> > Please remember to enable nand-ecc-maximize DT setting if want to be
> > compatible with older Bootloader base on this patch.
> 
> You're breaking backward compat here. Not sure this is a good idea, but
> if that's what you want I'd like a few more acks to confirm mediatek
> maintainers are okay with that.
> 
> And please add a patch updating the DT bindinds doc accordingly.

Yes. It is not backward compatibility. It is not good.
Or, just use generic helpers here but keep to maximize ecc strength like
before, what do you think about it?

> 
> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_ecc.c  |  25 -------
> >  drivers/mtd/nand/raw/mtk_ecc.h  |   2 -
> >  drivers/mtd/nand/raw/mtk_nand.c | 152 ++++++++++++++++++++++++----------------
> >  3 files changed, 93 insertions(+), 86 deletions(-)
> > 
> 
> > +
> > +static int mtk_nfc_ecc_caps_init(struct device *dev, struct mtk_nfc *nfc)
> > +{
> > +	struct nand_ecc_caps *ecccaps;
> > +	struct nand_ecc_step_info *stepinfos;
> > +	int i, nsector = nfc->caps->num_sector_size;
> > +
> > +	ecccaps = devm_kzalloc(dev, sizeof(*ecccaps), GFP_KERNEL);
> > +	if (!ecccaps)
> > +		return -ENOMEM;
> > +
> > +	stepinfos = devm_kzalloc(dev, sizeof(*stepinfos) * nsector, GFP_KERNEL);
> > +	if (!stepinfos)
> > +		return -ENOMEM;
> > +
> > +	nfc->ecccaps = ecccaps;
> > +
> > +	ecccaps->stepinfos = stepinfos;
> > +	ecccaps->nstepinfos = nsector;
> > +	ecccaps->calc_ecc_bytes = mtk_nfc_calc_ecc_bytes;
> > +
> > +	for (i = 0; i < nsector; i++) {
> > +		stepinfos->stepsize = nfc->caps->sector_size[i];
> > +		stepinfos->strengths = mtk_ecc_get_strength(nfc->ecc);
> > +		stepinfos->nstrengths = mtk_ecc_get_strength_num(nfc->ecc);
> > +		stepinfos++;
> > +	}
> 
> You seem to re-create generic tables from mtk's internal
> representation. Why don't you directly store a static const version of
> nand_ecc_caps in you caps struct. And maybe you can also get rid of the
> mtk specific representation in favor of the generic one.

OK. It is OK for me. Thanks.

> 
> >  
> >  	return 0;
> >  }
> > @@ -1329,6 +1356,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
> >  	/* set default mode in case dt entry is missing */
> >  	nand->ecc.mode = NAND_ECC_HW;
> >  
> > +	nand->ecc.priv = (void *)nfc->ecc;
> > +
> >  	nand->ecc.write_subpage = mtk_nfc_write_subpage_hwecc;
> >  	nand->ecc.write_page_raw = mtk_nfc_write_page_raw;
> >  	nand->ecc.write_page = mtk_nfc_write_page_hwecc;
> > @@ -1366,7 +1395,8 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
> >  		return -EINVAL;
> >  	}
> >  
> > -	ret = mtk_nfc_set_spare_per_sector(&chip->spare_per_sector, mtd);
> > +	ret = mtk_nfc_set_spare_per_sector(nand->ecc.size,
> > +					   &chip->spare_per_sector, mtd);
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -1539,6 +1569,10 @@ static int mtk_nfc_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, nfc);
> >  
> > +	ret = mtk_nfc_ecc_caps_init(dev, nfc);
> > +	if (ret)
> > +		goto clk_disable;
> > +
> >  	ret = mtk_nfc_nand_chips_init(dev, nfc);
> >  	if (ret) {
> >  		dev_err(dev, "failed to init nand chips\n");
> 



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

Re: [PATCH 5/8] mtd: rawnand: Modify ->calc_ecc_bytes() hook in nand_ecc_caps

[xiaolei li]

On Wed, 2018-04-11 at 20:57 +0200, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 11:41:55 +0800
> Xiaolei Li <xiaolei.li@mediatek.com> wrote:
> 
> > Maybe some controllers need more information besides step size and ecc
> > strength to calculate ECC bytes.
> > 
> > struct nand_ecc_ctrl provides lots of ECC control information include
> > private ECC control data. So, add it into ->calc_ecc_bytes() interface,
> > to make this hook be more flexible.
> 
> I'm not fond of this approach. Why don't you provide a different
> function for the 2 cases you have (parity = 13 and parity = 14)? 
> 
OK. It is achievable. Will rollback this change in patch v2.
Thanks.

> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/denali.c    |  3 +-
> >  drivers/mtd/nand/raw/denali.h    |  2 +-
> >  drivers/mtd/nand/raw/nand_base.c | 11 ++++--
> >  include/linux/mtd/rawnand.h      | 77 ++++++++++++++++++++--------------------
> >  4 files changed, 50 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
> > index 2a302a1..907609c 100644
> > --- a/drivers/mtd/nand/raw/denali.c
> > +++ b/drivers/mtd/nand/raw/denali.c
> > @@ -1113,7 +1113,8 @@ static void denali_hw_init(struct denali_nand_info *denali)
> >  	iowrite32(0xffff, denali->reg + SPARE_AREA_MARKER);
> >  }
> >  
> > -int denali_calc_ecc_bytes(int step_size, int strength)
> > +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
> > +			  int strength)
> >  {
> >  	/* BCH code.  Denali requires ecc.bytes to be multiple of 2 */
> >  	return DIV_ROUND_UP(strength * fls(step_size * 8), 16) * 2;
> > diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h
> > index 9ad33d2..644dffd 100644
> > --- a/drivers/mtd/nand/raw/denali.h
> > +++ b/drivers/mtd/nand/raw/denali.h
> > @@ -328,7 +328,7 @@ struct denali_nand_info {
> >  #define DENALI_CAP_HW_ECC_FIXUP			BIT(0)
> >  #define DENALI_CAP_DMA_64BIT			BIT(1)
> >  
> > -int denali_calc_ecc_bytes(int step_size, int strength);
> > +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *, int, int);
> >  int denali_init(struct denali_nand_info *denali);
> >  void denali_remove(struct denali_nand_info *denali);
> >  
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index d0b993f..e17abc8 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -6052,6 +6052,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
> >  			const struct nand_ecc_caps *caps, int oobavail)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> >  	const struct nand_ecc_step_info *stepinfo;
> >  	int preset_step = chip->ecc.size;
> >  	int preset_strength = chip->ecc.strength;
> > @@ -6076,7 +6077,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
> >  			if (stepinfo->strengths[j] != preset_strength)
> >  				continue;
> >  
> > -			ecc_bytes = caps->calc_ecc_bytes(preset_step,
> > +			ecc_bytes = caps->calc_ecc_bytes(ecc, preset_step,
> >  							 preset_strength);
> >  			if (WARN_ON_ONCE(ecc_bytes < 0))
> >  				return ecc_bytes;
> > @@ -6114,6 +6115,7 @@ int nand_match_ecc_req(struct nand_chip *chip,
> >  		       const struct nand_ecc_caps *caps, int oobavail)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> >  	const struct nand_ecc_step_info *stepinfo;
> >  	int req_step = chip->ecc_step_ds;
> >  	int req_strength = chip->ecc_strength_ds;
> > @@ -6152,7 +6154,8 @@ int nand_match_ecc_req(struct nand_chip *chip,
> >  
> >  			nsteps = mtd->writesize / step_size;
> >  
> > -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> > +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> > +							 strength);
> >  			if (WARN_ON_ONCE(ecc_bytes < 0))
> >  				continue;
> >  			ecc_bytes_total = ecc_bytes * nsteps;
> > @@ -6198,6 +6201,7 @@ int nand_maximize_ecc(struct nand_chip *chip,
> >  		      const struct nand_ecc_caps *caps, int oobavail)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> >  	const struct nand_ecc_step_info *stepinfo;
> >  	int step_size, strength, nsteps, ecc_bytes, corr;
> >  	int best_corr = 0;
> > @@ -6224,7 +6228,8 @@ int nand_maximize_ecc(struct nand_chip *chip,
> >  
> >  			nsteps = mtd->writesize / step_size;
> >  
> > -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> > +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> > +							 strength);
> >  			if (WARN_ON_ONCE(ecc_bytes < 0))
> >  				continue;
> >  
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index 5dad59b..e7c7fdb 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -507,44 +507,6 @@ static inline void nand_hw_control_init(struct nand_hw_control *nfc)
> >  }
> >  
> >  /**
> > - * struct nand_ecc_step_info - ECC step information of ECC engine
> > - * @stepsize: data bytes per ECC step
> > - * @strengths: array of supported strengths
> > - * @nstrengths: number of supported strengths
> > - */
> > -struct nand_ecc_step_info {
> > -	int stepsize;
> > -	const int *strengths;
> > -	int nstrengths;
> > -};
> > -
> > -/**
> > - * struct nand_ecc_caps - capability of ECC engine
> > - * @stepinfos: array of ECC step information
> > - * @nstepinfos: number of ECC step information
> > - * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> > - */
> > -struct nand_ecc_caps {
> > -	const struct nand_ecc_step_info *stepinfos;
> > -	int nstepinfos;
> > -	int (*calc_ecc_bytes)(int step_size, int strength);
> > -};
> > -
> > -/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> > -#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> > -static const int __name##_strengths[] = { __VA_ARGS__ };	\
> > -static const struct nand_ecc_step_info __name##_stepinfo = {	\
> > -	.stepsize = __step,					\
> > -	.strengths = __name##_strengths,			\
> > -	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> > -};								\
> > -static const struct nand_ecc_caps __name = {			\
> > -	.stepinfos = &__name##_stepinfo,			\
> > -	.nstepinfos = 1,					\
> > -	.calc_ecc_bytes = __calc,				\
> > -}
> > -
> > -/**
> >   * struct nand_ecc_ctrl - Control structure for ECC
> >   * @mode:	ECC mode
> >   * @algo:	ECC algorithm
> > @@ -639,6 +601,45 @@ struct nand_ecc_ctrl {
> >  };
> >  
> >  /**
> > + * struct nand_ecc_step_info - ECC step information of ECC engine
> > + * @stepsize: data bytes per ECC step
> > + * @strengths: array of supported strengths
> > + * @nstrengths: number of supported strengths
> > + */
> > +struct nand_ecc_step_info {
> > +	int stepsize;
> > +	const int *strengths;
> > +	int nstrengths;
> > +};
> > +
> > +/**
> > + * struct nand_ecc_caps - capability of ECC engine
> > + * @stepinfos: array of ECC step information
> > + * @nstepinfos: number of ECC step information
> > + * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> > + */
> > +struct nand_ecc_caps {
> > +	const struct nand_ecc_step_info *stepinfos;
> > +	int nstepinfos;
> > +	int (*calc_ecc_bytes)(struct nand_ecc_ctrl *ecc, int step_size,
> > +			      int strength);
> > +};
> > +
> > +/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> > +#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> > +static const int __name##_strengths[] = { __VA_ARGS__ };	\
> > +static const struct nand_ecc_step_info __name##_stepinfo = {	\
> > +	.stepsize = __step,					\
> > +	.strengths = __name##_strengths,			\
> > +	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> > +};								\
> > +static const struct nand_ecc_caps __name = {			\
> > +	.stepinfos = &__name##_stepinfo,			\
> > +	.nstepinfos = 1,					\
> > +	.calc_ecc_bytes = __calc,				\
> > +}
> > +
> > +/**
> >   * struct nand_sdr_timings - SDR NAND chip timings
> >   *
> >   * This struct defines the timing requirements of a SDR NAND chip.
> 

Re: [PATCH 5/8] mtd: rawnand: Modify ->calc_ecc_bytes() hook in nand_ecc_caps

[xiaolei li]

On Wed, 2018-04-11 at 20:57 +0200, Boris Brezillon wrote:
> On Wed, 11 Apr 2018 11:41:55 +0800
> Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> > Maybe some controllers need more information besides step size and ecc
> > strength to calculate ECC bytes.
> > 
> > struct nand_ecc_ctrl provides lots of ECC control information include
> > private ECC control data. So, add it into ->calc_ecc_bytes() interface,
> > to make this hook be more flexible.
> 
> I'm not fond of this approach. Why don't you provide a different
> function for the 2 cases you have (parity = 13 and parity = 14)? 
> 
OK. It is achievable. Will rollback this change in patch v2.
Thanks.

> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/mtd/nand/raw/denali.c    |  3 +-
> >  drivers/mtd/nand/raw/denali.h    |  2 +-
> >  drivers/mtd/nand/raw/nand_base.c | 11 ++++--
> >  include/linux/mtd/rawnand.h      | 77 ++++++++++++++++++++--------------------
> >  4 files changed, 50 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
> > index 2a302a1..907609c 100644
> > --- a/drivers/mtd/nand/raw/denali.c
> > +++ b/drivers/mtd/nand/raw/denali.c
> > @@ -1113,7 +1113,8 @@ static void denali_hw_init(struct denali_nand_info *denali)
> >  	iowrite32(0xffff, denali->reg + SPARE_AREA_MARKER);
> >  }
> >  
> > -int denali_calc_ecc_bytes(int step_size, int strength)
> > +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *ecc, int step_size,
> > +			  int strength)
> >  {
> >  	/* BCH code.  Denali requires ecc.bytes to be multiple of 2 */
> >  	return DIV_ROUND_UP(strength * fls(step_size * 8), 16) * 2;
> > diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h
> > index 9ad33d2..644dffd 100644
> > --- a/drivers/mtd/nand/raw/denali.h
> > +++ b/drivers/mtd/nand/raw/denali.h
> > @@ -328,7 +328,7 @@ struct denali_nand_info {
> >  #define DENALI_CAP_HW_ECC_FIXUP			BIT(0)
> >  #define DENALI_CAP_DMA_64BIT			BIT(1)
> >  
> > -int denali_calc_ecc_bytes(int step_size, int strength);
> > +int denali_calc_ecc_bytes(struct nand_ecc_ctrl *, int, int);
> >  int denali_init(struct denali_nand_info *denali);
> >  void denali_remove(struct denali_nand_info *denali);
> >  
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index d0b993f..e17abc8 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -6052,6 +6052,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
> >  			const struct nand_ecc_caps *caps, int oobavail)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> >  	const struct nand_ecc_step_info *stepinfo;
> >  	int preset_step = chip->ecc.size;
> >  	int preset_strength = chip->ecc.strength;
> > @@ -6076,7 +6077,7 @@ int nand_check_ecc_caps(struct nand_chip *chip,
> >  			if (stepinfo->strengths[j] != preset_strength)
> >  				continue;
> >  
> > -			ecc_bytes = caps->calc_ecc_bytes(preset_step,
> > +			ecc_bytes = caps->calc_ecc_bytes(ecc, preset_step,
> >  							 preset_strength);
> >  			if (WARN_ON_ONCE(ecc_bytes < 0))
> >  				return ecc_bytes;
> > @@ -6114,6 +6115,7 @@ int nand_match_ecc_req(struct nand_chip *chip,
> >  		       const struct nand_ecc_caps *caps, int oobavail)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> >  	const struct nand_ecc_step_info *stepinfo;
> >  	int req_step = chip->ecc_step_ds;
> >  	int req_strength = chip->ecc_strength_ds;
> > @@ -6152,7 +6154,8 @@ int nand_match_ecc_req(struct nand_chip *chip,
> >  
> >  			nsteps = mtd->writesize / step_size;
> >  
> > -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> > +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> > +							 strength);
> >  			if (WARN_ON_ONCE(ecc_bytes < 0))
> >  				continue;
> >  			ecc_bytes_total = ecc_bytes * nsteps;
> > @@ -6198,6 +6201,7 @@ int nand_maximize_ecc(struct nand_chip *chip,
> >  		      const struct nand_ecc_caps *caps, int oobavail)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> >  	const struct nand_ecc_step_info *stepinfo;
> >  	int step_size, strength, nsteps, ecc_bytes, corr;
> >  	int best_corr = 0;
> > @@ -6224,7 +6228,8 @@ int nand_maximize_ecc(struct nand_chip *chip,
> >  
> >  			nsteps = mtd->writesize / step_size;
> >  
> > -			ecc_bytes = caps->calc_ecc_bytes(step_size, strength);
> > +			ecc_bytes = caps->calc_ecc_bytes(ecc, step_size,
> > +							 strength);
> >  			if (WARN_ON_ONCE(ecc_bytes < 0))
> >  				continue;
> >  
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index 5dad59b..e7c7fdb 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -507,44 +507,6 @@ static inline void nand_hw_control_init(struct nand_hw_control *nfc)
> >  }
> >  
> >  /**
> > - * struct nand_ecc_step_info - ECC step information of ECC engine
> > - * @stepsize: data bytes per ECC step
> > - * @strengths: array of supported strengths
> > - * @nstrengths: number of supported strengths
> > - */
> > -struct nand_ecc_step_info {
> > -	int stepsize;
> > -	const int *strengths;
> > -	int nstrengths;
> > -};
> > -
> > -/**
> > - * struct nand_ecc_caps - capability of ECC engine
> > - * @stepinfos: array of ECC step information
> > - * @nstepinfos: number of ECC step information
> > - * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> > - */
> > -struct nand_ecc_caps {
> > -	const struct nand_ecc_step_info *stepinfos;
> > -	int nstepinfos;
> > -	int (*calc_ecc_bytes)(int step_size, int strength);
> > -};
> > -
> > -/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> > -#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> > -static const int __name##_strengths[] = { __VA_ARGS__ };	\
> > -static const struct nand_ecc_step_info __name##_stepinfo = {	\
> > -	.stepsize = __step,					\
> > -	.strengths = __name##_strengths,			\
> > -	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> > -};								\
> > -static const struct nand_ecc_caps __name = {			\
> > -	.stepinfos = &__name##_stepinfo,			\
> > -	.nstepinfos = 1,					\
> > -	.calc_ecc_bytes = __calc,				\
> > -}
> > -
> > -/**
> >   * struct nand_ecc_ctrl - Control structure for ECC
> >   * @mode:	ECC mode
> >   * @algo:	ECC algorithm
> > @@ -639,6 +601,45 @@ struct nand_ecc_ctrl {
> >  };
> >  
> >  /**
> > + * struct nand_ecc_step_info - ECC step information of ECC engine
> > + * @stepsize: data bytes per ECC step
> > + * @strengths: array of supported strengths
> > + * @nstrengths: number of supported strengths
> > + */
> > +struct nand_ecc_step_info {
> > +	int stepsize;
> > +	const int *strengths;
> > +	int nstrengths;
> > +};
> > +
> > +/**
> > + * struct nand_ecc_caps - capability of ECC engine
> > + * @stepinfos: array of ECC step information
> > + * @nstepinfos: number of ECC step information
> > + * @calc_ecc_bytes: driver's hook to calculate ECC bytes per step
> > + */
> > +struct nand_ecc_caps {
> > +	const struct nand_ecc_step_info *stepinfos;
> > +	int nstepinfos;
> > +	int (*calc_ecc_bytes)(struct nand_ecc_ctrl *ecc, int step_size,
> > +			      int strength);
> > +};
> > +
> > +/* a shorthand to generate struct nand_ecc_caps with only one ECC stepsize */
> > +#define NAND_ECC_CAPS_SINGLE(__name, __calc, __step, ...)	\
> > +static const int __name##_strengths[] = { __VA_ARGS__ };	\
> > +static const struct nand_ecc_step_info __name##_stepinfo = {	\
> > +	.stepsize = __step,					\
> > +	.strengths = __name##_strengths,			\
> > +	.nstrengths = ARRAY_SIZE(__name##_strengths),		\
> > +};								\
> > +static const struct nand_ecc_caps __name = {			\
> > +	.stepinfos = &__name##_stepinfo,			\
> > +	.nstepinfos = 1,					\
> > +	.calc_ecc_bytes = __calc,				\
> > +}
> > +
> > +/**
> >   * struct nand_sdr_timings - SDR NAND chip timings
> >   *
> >   * This struct defines the timing requirements of a SDR NAND chip.
>