[PATCH v2 00/11] Supporting restricted NAND controllers

[PATCH v2 00/11] Supporting restricted NAND controllers

[Miquel Raynal]

Hello,

The first 6 patches are just miscellaneous changes, that do not bring
functional changes but clarify a few core areas.

Patches 7 and 8 change the NAND operation used to read ONFI/JEDEC
parameter pages. I expect all controllers to support it.

Patch 9 add new nand_monolithic_read/write_page_raw() helpers, that
could be used by drivers which are "constrained".

Finally, patches 10 and 11 soften the rules so that the hooks linked
to the above helpers are not overwritten by the core or NAND chip
drivers.

This series is needed in order to support controllers like Arasan's.

Thanks,
Miquèl

Changes in v2:
* Fixed the two wrong conversions of flag values from 8-bit digits
  into BIT() macros.
* Dropped "Help supporting controllers that are not able to split
  operations". Instead, decided on the fly for the read_param_page()
  uses (ONFI and JEDEC discovery) and wrote separate helpers for
  read/write_page_raw() (the Arasan driver will use them).
* Prevent the core and NAND chip drivers to overload the
  ecc->read/write_page_raw() hooks.
* Added Reviewed-by tags.

Miquel Raynal (11):
  mtd: rawnand: Translate obscure bitfields into readable macros
  mtd: rawnand: Reorder the nand_chip->options flags
  mtd: rawnand: Rename a NAND chip option
  mtd: rawnand: Fix comments about the use of bufpoi
  mtd: rawnand: Rename the use_bufpoi variables
  mtd: rawnand: Avoid indirect access to ->data_buf()
  mtd: rawnand: onfi: Adapt the parameter page read to constraint
    controllers
  mtd: rawnand: jedec: Adapt the parameter page read to constraint
    controllers
  mtd: rawnand: Expose monolithic read/write_page_raw() helpers
  mtd: rawnand: Allow controllers to overload soft ECC hooks
  mtd: rawnand: micron: Allow controllers to overload raw accessors

 drivers/mtd/nand/raw/atmel/nand-controller.c |   2 +-
 drivers/mtd/nand/raw/brcmnand/brcmnand.c     |   2 +-
 drivers/mtd/nand/raw/denali.c                |   2 +-
 drivers/mtd/nand/raw/meson_nand.c            |   2 +-
 drivers/mtd/nand/raw/mtk_nand.c              |   2 +-
 drivers/mtd/nand/raw/nand_base.c             | 122 +++++++++++++++----
 drivers/mtd/nand/raw/nand_jedec.c            |  28 +++--
 drivers/mtd/nand/raw/nand_micron.c           |   6 +-
 drivers/mtd/nand/raw/nand_onfi.c             |  14 +--
 drivers/mtd/nand/raw/qcom_nandc.c            |   2 +-
 drivers/mtd/nand/raw/stm32_fmc2_nand.c       |   2 +-
 drivers/mtd/nand/raw/sunxi_nand.c            |   2 +-
 drivers/mtd/nand/raw/tango_nand.c            |   2 +-
 drivers/mtd/nand/raw/tegra_nand.c            |   2 +-
 include/linux/mtd/rawnand.h                  |  97 ++++++++-------
 15 files changed, 184 insertions(+), 103 deletions(-)

-- 
2.20.1


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

[PATCH v2 01/11] mtd: rawnand: Translate obscure bitfields into readable macros

[Miquel Raynal]

Use the BIT() macro instead of defining a 8-digit value.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 include/linux/mtd/rawnand.h | 38 ++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 21873168ba4d..4b58de842340 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -129,36 +129,36 @@ enum nand_ecc_algo {
  * features.
  */
 /* Buswidth is 16 bit */
-#define NAND_BUSWIDTH_16	0x00000002
+#define NAND_BUSWIDTH_16	BIT(1)
 /* Chip has cache program function */
-#define NAND_CACHEPRG		0x00000008
+#define NAND_CACHEPRG		BIT(3)
 /*
  * Chip requires ready check on read (for auto-incremented sequential read).
  * True only for small page devices; large page devices do not support
  * autoincrement.
  */
-#define NAND_NEED_READRDY	0x00000100
+#define NAND_NEED_READRDY	BIT(8)
 
 /* Chip does not allow subpage writes */
-#define NAND_NO_SUBPAGE_WRITE	0x00000200
+#define NAND_NO_SUBPAGE_WRITE	BIT(9)
 
 /* Device is one of 'new' xD cards that expose fake nand command set */
-#define NAND_BROKEN_XD		0x00000400
+#define NAND_BROKEN_XD		BIT(10)
 
 /* Device behaves just like nand, but is readonly */
-#define NAND_ROM		0x00000800
+#define NAND_ROM		BIT(11)
 
 /* Device supports subpage reads */
-#define NAND_SUBPAGE_READ	0x00001000
+#define NAND_SUBPAGE_READ	BIT(12)
 
 /*
  * Some MLC NANDs need data scrambling to limit bitflips caused by repeated
  * patterns.
  */
-#define NAND_NEED_SCRAMBLING	0x00002000
+#define NAND_NEED_SCRAMBLING	BIT(13)
 
 /* Device needs 3rd row address cycle */
-#define NAND_ROW_ADDR_3		0x00004000
+#define NAND_ROW_ADDR_3		BIT(14)
 
 /* Options valid for Samsung large page devices */
 #define NAND_SAMSUNG_LP_OPTIONS NAND_CACHEPRG
@@ -173,9 +173,9 @@ enum nand_ecc_algo {
  * Position within the block: Each of these pages needs to be checked for a
  * bad block marking pattern.
  */
-#define NAND_BBM_FIRSTPAGE		0x01000000
-#define NAND_BBM_SECONDPAGE		0x02000000
-#define NAND_BBM_LASTPAGE		0x04000000
+#define NAND_BBM_FIRSTPAGE	BIT(24)
+#define NAND_BBM_SECONDPAGE	BIT(25)
+#define NAND_BBM_LASTPAGE	BIT(26)
 
 /* Position within the OOB data of the page */
 #define NAND_BBM_POS_SMALL		5
@@ -183,21 +183,21 @@ enum nand_ecc_algo {
 
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
-#define NAND_SKIP_BBTSCAN	0x00010000
+#define NAND_SKIP_BBTSCAN	BIT(16)
 /* Chip may not exist, so silence any errors in scan */
-#define NAND_SCAN_SILENT_NODEV	0x00040000
+#define NAND_SCAN_SILENT_NODEV	BIT(18)
 /*
  * Autodetect nand buswidth with readid/onfi.
  * This suppose the driver will configure the hardware in 8 bits mode
  * when calling nand_scan_ident, and update its configuration
  * before calling nand_scan_tail.
  */
-#define NAND_BUSWIDTH_AUTO      0x00080000
+#define NAND_BUSWIDTH_AUTO      BIT(19)
 /*
  * This option could be defined by controller drivers to protect against
  * kmap'ed, vmalloc'ed highmem buffers being passed from upper layers
  */
-#define NAND_USE_BOUNCE_BUFFER	0x00100000
+#define NAND_USE_BOUNCE_BUFFER	BIT(20)
 
 /*
  * In case your controller is implementing ->legacy.cmd_ctrl() and is relying
@@ -207,20 +207,20 @@ enum nand_ecc_algo {
  * If your controller already takes care of this delay, you don't need to set
  * this flag.
  */
-#define NAND_WAIT_TCCS		0x00200000
+#define NAND_WAIT_TCCS		BIT(21)
 
 /*
  * Whether the NAND chip is a boot medium. Drivers might use this information
  * to select ECC algorithms supported by the boot ROM or similar restrictions.
  */
-#define NAND_IS_BOOT_MEDIUM	0x00400000
+#define NAND_IS_BOOT_MEDIUM	BIT(22)
 
 /*
  * Do not try to tweak the timings at runtime. This is needed when the
  * controller initializes the timings on itself or when it relies on
  * configuration done by the bootloader.
  */
-#define NAND_KEEP_TIMINGS	0x00800000
+#define NAND_KEEP_TIMINGS	BIT(23)
 
 /* Cell info constants */
 #define NAND_CI_CHIPNR_MSK	0x03
-- 
2.20.1


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

[PATCH v2 02/11] mtd: rawnand: Reorder the nand_chip->options flags

[Miquel Raynal]

These flags are in a strange order, reorder the list, add spaces when
it is relevant, pack definitions that are related.

There is no functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 include/linux/mtd/rawnand.h | 57 +++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 4b58de842340..e70fea67030b 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -118,20 +118,25 @@ enum nand_ecc_algo {
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
 #define NAND_ECC_MAXIMIZE		BIT(1)
 
-/*
- * When using software implementation of Hamming, we can specify which byte
- * ordering should be used.
- */
-#define NAND_ECC_SOFT_HAMMING_SM_ORDER	BIT(2)
-
 /*
  * Option constants for bizarre disfunctionality and real
  * features.
  */
+
 /* Buswidth is 16 bit */
 #define NAND_BUSWIDTH_16	BIT(1)
+
+/*
+ * When using software implementation of Hamming, we can specify which byte
+ * ordering should be used.
+ */
+#define NAND_ECC_SOFT_HAMMING_SM_ORDER	BIT(2)
+
 /* Chip has cache program function */
 #define NAND_CACHEPRG		BIT(3)
+/* Options valid for Samsung large page devices */
+#define NAND_SAMSUNG_LP_OPTIONS NAND_CACHEPRG
+
 /*
  * Chip requires ready check on read (for auto-incremented sequential read).
  * True only for small page devices; large page devices do not support
@@ -150,6 +155,8 @@ enum nand_ecc_algo {
 
 /* Device supports subpage reads */
 #define NAND_SUBPAGE_READ	BIT(12)
+/* Macros to identify the above */
+#define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
 
 /*
  * Some MLC NANDs need data scrambling to limit bitflips caused by repeated
@@ -160,32 +167,12 @@ enum nand_ecc_algo {
 /* Device needs 3rd row address cycle */
 #define NAND_ROW_ADDR_3		BIT(14)
 
-/* Options valid for Samsung large page devices */
-#define NAND_SAMSUNG_LP_OPTIONS NAND_CACHEPRG
-
-/* Macros to identify the above */
-#define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
-
-/*
- * There are different places where the manufacturer stores the factory bad
- * block markers.
- *
- * Position within the block: Each of these pages needs to be checked for a
- * bad block marking pattern.
- */
-#define NAND_BBM_FIRSTPAGE	BIT(24)
-#define NAND_BBM_SECONDPAGE	BIT(25)
-#define NAND_BBM_LASTPAGE	BIT(26)
-
-/* Position within the OOB data of the page */
-#define NAND_BBM_POS_SMALL		5
-#define NAND_BBM_POS_LARGE		0
-
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
 #define NAND_SKIP_BBTSCAN	BIT(16)
 /* Chip may not exist, so silence any errors in scan */
 #define NAND_SCAN_SILENT_NODEV	BIT(18)
+
 /*
  * Autodetect nand buswidth with readid/onfi.
  * This suppose the driver will configure the hardware in 8 bits mode
@@ -193,6 +180,7 @@ enum nand_ecc_algo {
  * before calling nand_scan_tail.
  */
 #define NAND_BUSWIDTH_AUTO      BIT(19)
+
 /*
  * This option could be defined by controller drivers to protect against
  * kmap'ed, vmalloc'ed highmem buffers being passed from upper layers
@@ -222,11 +210,26 @@ enum nand_ecc_algo {
  */
 #define NAND_KEEP_TIMINGS	BIT(23)
 
+/*
+ * There are different places where the manufacturer stores the factory bad
+ * block markers.
+ *
+ * Position within the block: Each of these pages needs to be checked for a
+ * bad block marking pattern.
+ */
+#define NAND_BBM_FIRSTPAGE	BIT(24)
+#define NAND_BBM_SECONDPAGE	BIT(25)
+#define NAND_BBM_LASTPAGE	BIT(26)
+
 /* Cell info constants */
 #define NAND_CI_CHIPNR_MSK	0x03
 #define NAND_CI_CELLTYPE_MSK	0x0C
 #define NAND_CI_CELLTYPE_SHIFT	2
 
+/* Position within the OOB data of the page */
+#define NAND_BBM_POS_SMALL		5
+#define NAND_BBM_POS_LARGE		0
+
 /**
  * struct nand_parameters - NAND generic parameters from the parameter page
  * @model: Model name
-- 
2.20.1


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

[PATCH v2 03/11] mtd: rawnand: Rename a NAND chip option

[Miquel Raynal]

NAND controller drivers can set the NAND_USE_BOUNCE_BUFFER flag to a
chip 'option' field. With this flag, the core is responsible of
providing DMA-able buffers.

The current behavior is to not force the use of a bounce buffer when
the core thinks this is not needed. So in the end the name is a bit
misleading, because in theory we will always have a DMA buffer but in
practice it will not always be a bounce buffer.

Rename this flag NAND_USE_DMA_BUFFER to be more accurate.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/atmel/nand-controller.c | 2 +-
 drivers/mtd/nand/raw/brcmnand/brcmnand.c     | 2 +-
 drivers/mtd/nand/raw/denali.c                | 2 +-
 drivers/mtd/nand/raw/meson_nand.c            | 2 +-
 drivers/mtd/nand/raw/mtk_nand.c              | 2 +-
 drivers/mtd/nand/raw/nand_base.c             | 4 ++--
 drivers/mtd/nand/raw/qcom_nandc.c            | 2 +-
 drivers/mtd/nand/raw/stm32_fmc2_nand.c       | 2 +-
 drivers/mtd/nand/raw/sunxi_nand.c            | 2 +-
 drivers/mtd/nand/raw/tango_nand.c            | 2 +-
 drivers/mtd/nand/raw/tegra_nand.c            | 2 +-
 include/linux/mtd/rawnand.h                  | 2 +-
 12 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 3ba17a98df4d..95d106fdb54f 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1494,7 +1494,7 @@ static void atmel_nand_init(struct atmel_nand_controller *nc,
 	 * suitable for DMA.
 	 */
 	if (nc->dmac)
-		chip->options |= NAND_USE_BOUNCE_BUFFER;
+		chip->options |= NAND_USE_DMA_BUFFER;
 
 	/* Default to HW ECC if pmecc is available. */
 	if (nc->pmecc)
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index e4e3ceeac38f..6bb927c512a9 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2577,7 +2577,7 @@ static int brcmnand_attach_chip(struct nand_chip *chip)
 	 * to/from, and have nand_base pass us a bounce buffer instead, as
 	 * needed.
 	 */
-	chip->options |= NAND_USE_BOUNCE_BUFFER;
+	chip->options |= NAND_USE_DMA_BUFFER;
 
 	if (chip->bbt_options & NAND_BBT_USE_FLASH)
 		chip->bbt_options |= NAND_BBT_NO_OOB;
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 6a6c919b2569..4d199fbae800 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1203,7 +1203,7 @@ int denali_chip_init(struct denali_controller *denali,
 		mtd->name = "denali-nand";
 
 	if (denali->dma_avail) {
-		chip->options |= NAND_USE_BOUNCE_BUFFER;
+		chip->options |= NAND_USE_DMA_BUFFER;
 		chip->buf_align = 16;
 	}
 
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index e961f7bebf0a..69af30cdb6eb 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -1269,7 +1269,7 @@ meson_nfc_nand_chip_init(struct device *dev,
 	nand_set_flash_node(nand, np);
 	nand_set_controller_data(nand, nfc);
 
-	nand->options |= NAND_USE_BOUNCE_BUFFER;
+	nand->options |= NAND_USE_DMA_BUFFER;
 	mtd = nand_to_mtd(nand);
 	mtd->owner = THIS_MODULE;
 	mtd->dev.parent = dev;
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index ef149e8b26d0..b02377ec12f2 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -1380,7 +1380,7 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 	nand_set_flash_node(nand, np);
 	nand_set_controller_data(nand, nfc);
 
-	nand->options |= NAND_USE_BOUNCE_BUFFER | NAND_SUBPAGE_READ;
+	nand->options |= NAND_USE_DMA_BUFFER | NAND_SUBPAGE_READ;
 	nand->legacy.dev_ready = mtk_nfc_dev_ready;
 	nand->legacy.select_chip = mtk_nfc_select_chip;
 	nand->legacy.write_byte = mtk_nfc_write_byte;
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 27ed6189f227..db2745cf4f15 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3191,7 +3191,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 
 		if (!aligned)
 			use_bufpoi = 1;
-		else if (chip->options & NAND_USE_BOUNCE_BUFFER)
+		else if (chip->options & NAND_USE_DMA_BUFFER)
 			use_bufpoi = !virt_addr_valid(buf) ||
 				     !IS_ALIGNED((unsigned long)buf,
 						 chip->buf_align);
@@ -4017,7 +4017,7 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
 
 		if (part_pagewr)
 			use_bufpoi = 1;
-		else if (chip->options & NAND_USE_BOUNCE_BUFFER)
+		else if (chip->options & NAND_USE_DMA_BUFFER)
 			use_bufpoi = !virt_addr_valid(buf) ||
 				     !IS_ALIGNED((unsigned long)buf,
 						 chip->buf_align);
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 5b11c7061497..9be1bd719da4 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2836,7 +2836,7 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
 	chip->legacy.block_markbad	= qcom_nandc_block_markbad;
 
 	chip->controller = &nandc->controller;
-	chip->options |= NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER |
+	chip->options |= NAND_NO_SUBPAGE_WRITE | NAND_USE_DMA_BUFFER |
 			 NAND_SKIP_BBTSCAN;
 
 	/* set up initial status value */
diff --git a/drivers/mtd/nand/raw/stm32_fmc2_nand.c b/drivers/mtd/nand/raw/stm32_fmc2_nand.c
index 46b7d04e2c87..496bac45c695 100644
--- a/drivers/mtd/nand/raw/stm32_fmc2_nand.c
+++ b/drivers/mtd/nand/raw/stm32_fmc2_nand.c
@@ -1987,7 +1987,7 @@ static int stm32_fmc2_probe(struct platform_device *pdev)
 
 	chip->controller = &fmc2->base;
 	chip->options |= NAND_BUSWIDTH_AUTO | NAND_NO_SUBPAGE_WRITE |
-			 NAND_USE_BOUNCE_BUFFER;
+			 NAND_USE_DMA_BUFFER;
 
 	/* Default ECC settings */
 	chip->ecc.mode = NAND_ECC_HW;
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 18ac0b36abfa..3eaf5526628b 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1698,7 +1698,7 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 		ecc->read_page = sunxi_nfc_hw_ecc_read_page_dma;
 		ecc->read_subpage = sunxi_nfc_hw_ecc_read_subpage_dma;
 		ecc->write_page = sunxi_nfc_hw_ecc_write_page_dma;
-		nand->options |= NAND_USE_BOUNCE_BUFFER;
+		nand->options |= NAND_USE_DMA_BUFFER;
 	} else {
 		ecc->read_page = sunxi_nfc_hw_ecc_read_page;
 		ecc->read_subpage = sunxi_nfc_hw_ecc_read_subpage;
diff --git a/drivers/mtd/nand/raw/tango_nand.c b/drivers/mtd/nand/raw/tango_nand.c
index 9acf2de37ee0..026db1be2cba 100644
--- a/drivers/mtd/nand/raw/tango_nand.c
+++ b/drivers/mtd/nand/raw/tango_nand.c
@@ -568,7 +568,7 @@ static int chip_init(struct device *dev, struct device_node *np)
 	chip->legacy.select_chip = tango_select_chip;
 	chip->legacy.cmd_ctrl = tango_cmd_ctrl;
 	chip->legacy.dev_ready = tango_dev_ready;
-	chip->options = NAND_USE_BOUNCE_BUFFER |
+	chip->options = NAND_USE_DMA_BUFFER |
 			NAND_NO_SUBPAGE_WRITE |
 			NAND_WAIT_TCCS;
 	chip->controller = &nfc->hw;
diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
index 6a255ba0f288..1b9ea0225047 100644
--- a/drivers/mtd/nand/raw/tegra_nand.c
+++ b/drivers/mtd/nand/raw/tegra_nand.c
@@ -1115,7 +1115,7 @@ static int tegra_nand_chips_init(struct device *dev,
 	if (!mtd->name)
 		mtd->name = "tegra_nand";
 
-	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER;
+	chip->options = NAND_NO_SUBPAGE_WRITE | NAND_USE_DMA_BUFFER;
 
 	ret = nand_scan(chip, 1);
 	if (ret)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index e70fea67030b..d40ad19ba0f6 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -185,7 +185,7 @@ enum nand_ecc_algo {
  * This option could be defined by controller drivers to protect against
  * kmap'ed, vmalloc'ed highmem buffers being passed from upper layers
  */
-#define NAND_USE_BOUNCE_BUFFER	BIT(20)
+#define NAND_USE_DMA_BUFFER	BIT(20)
 
 /*
  * In case your controller is implementing ->legacy.cmd_ctrl() and is relying
-- 
2.20.1


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

Re: [PATCH v2 03/11] mtd: rawnand: Rename a NAND chip option

[Boris Brezillon]

On Wed, 29 Apr 2020 17:55:32 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> NAND controller drivers can set the NAND_USE_BOUNCE_BUFFER flag to a
> chip 'option' field. With this flag, the core is responsible of
> providing DMA-able buffers.
> 
> The current behavior is to not force the use of a bounce buffer when
> the core thinks this is not needed. So in the end the name is a bit
> misleading, because in theory we will always have a DMA buffer but in
> practice it will not always be a bounce buffer.
> 
> Rename this flag NAND_USE_DMA_BUFFER to be more accurate.

I still think this one should be named NAND_CONTROLLER_USES_DMA.

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

Re: [PATCH v2 03/11] mtd: rawnand: Rename a NAND chip option

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
2020 18:08:16 +0200:

> On Wed, 29 Apr 2020 17:55:32 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > NAND controller drivers can set the NAND_USE_BOUNCE_BUFFER flag to a
> > chip 'option' field. With this flag, the core is responsible of
> > providing DMA-able buffers.
> > 
> > The current behavior is to not force the use of a bounce buffer when
> > the core thinks this is not needed. So in the end the name is a bit
> > misleading, because in theory we will always have a DMA buffer but in
> > practice it will not always be a bounce buffer.
> > 
> > Rename this flag NAND_USE_DMA_BUFFER to be more accurate.  
> 
> I still think this one should be named NAND_CONTROLLER_USES_DMA.

Actually I want to rework all the flags and prefix them with
NAND_CONTROLLER, that's why I am keeping the NAND_ prefix. I can change
the _USE_DMA_BUFFER into _USES_DMA though.

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

Re: [PATCH v2 03/11] mtd: rawnand: Rename a NAND chip option

[Boris Brezillon]

On Wed, 29 Apr 2020 18:22:00 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> 2020 18:08:16 +0200:
> 
> > On Wed, 29 Apr 2020 17:55:32 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > NAND controller drivers can set the NAND_USE_BOUNCE_BUFFER flag to a
> > > chip 'option' field. With this flag, the core is responsible of
> > > providing DMA-able buffers.
> > > 
> > > The current behavior is to not force the use of a bounce buffer when
> > > the core thinks this is not needed. So in the end the name is a bit
> > > misleading, because in theory we will always have a DMA buffer but in
> > > practice it will not always be a bounce buffer.
> > > 
> > > Rename this flag NAND_USE_DMA_BUFFER to be more accurate.    
> > 
> > I still think this one should be named NAND_CONTROLLER_USES_DMA.  
> 
> Actually I want to rework all the flags and prefix them with
> NAND_CONTROLLER, that's why I am keeping the NAND_ prefix. I can change
> the _USE_DMA_BUFFER into _USES_DMA though.

Ack on NAND_USES_DMA.

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

Re: [PATCH v2 03/11] mtd: rawnand: Rename a NAND chip option

[Boris Brezillon]

On Wed, 29 Apr 2020 18:32:31 +0200
Boris Brezillon <boris.brezillon@collabora.com> wrote:

> On Wed, 29 Apr 2020 18:22:00 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> > 2020 18:08:16 +0200:
> >   
> > > On Wed, 29 Apr 2020 17:55:32 +0200
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > NAND controller drivers can set the NAND_USE_BOUNCE_BUFFER flag to a
> > > > chip 'option' field. With this flag, the core is responsible of
> > > > providing DMA-able buffers.
> > > > 
> > > > The current behavior is to not force the use of a bounce buffer when
> > > > the core thinks this is not needed. So in the end the name is a bit
> > > > misleading, because in theory we will always have a DMA buffer but in
> > > > practice it will not always be a bounce buffer.
> > > > 
> > > > Rename this flag NAND_USE_DMA_BUFFER to be more accurate.      
> > > 
> > > I still think this one should be named NAND_CONTROLLER_USES_DMA.    
> > 
> > Actually I want to rework all the flags and prefix them with
> > NAND_CONTROLLER, that's why I am keeping the NAND_ prefix. I can change
> > the _USE_DMA_BUFFER into _USES_DMA though.  
> 
> Ack on NAND_USES_DMA.

But then I wonder if it's really worth renaming this field now if you
plan to rename it again later :-).

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

Re: [PATCH v2 03/11] mtd: rawnand: Rename a NAND chip option

[Miquel Raynal]

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
2020 18:36:42 +0200:

> On Wed, 29 Apr 2020 18:32:31 +0200
> Boris Brezillon <boris.brezillon@collabora.com> wrote:
> 
> > On Wed, 29 Apr 2020 18:22:00 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Boris,
> > > 
> > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> > > 2020 18:08:16 +0200:
> > >     
> > > > On Wed, 29 Apr 2020 17:55:32 +0200
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >       
> > > > > NAND controller drivers can set the NAND_USE_BOUNCE_BUFFER flag to a
> > > > > chip 'option' field. With this flag, the core is responsible of
> > > > > providing DMA-able buffers.
> > > > > 
> > > > > The current behavior is to not force the use of a bounce buffer when
> > > > > the core thinks this is not needed. So in the end the name is a bit
> > > > > misleading, because in theory we will always have a DMA buffer but in
> > > > > practice it will not always be a bounce buffer.
> > > > > 
> > > > > Rename this flag NAND_USE_DMA_BUFFER to be more accurate.        
> > > > 
> > > > I still think this one should be named NAND_CONTROLLER_USES_DMA.      
> > > 
> > > Actually I want to rework all the flags and prefix them with
> > > NAND_CONTROLLER, that's why I am keeping the NAND_ prefix. I can change
> > > the _USE_DMA_BUFFER into _USES_DMA though.    
> > 
> > Ack on NAND_USES_DMA.  
> 
> But then I wonder if it's really worth renaming this field now if you
> plan to rename it again later :-).

Hehe, well, it "fixes" the meaning of the flag, later changes will only
be "cosmetic" :)

Saying we "use a bounce buffer" is not accurate as the code first checks
if the buffer is compliant with DMA constraints, and uses a bounce
buffer only if it is not.

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

Re: [PATCH v2 03/11] mtd: rawnand: Rename a NAND chip option

[Boris Brezillon]

On Wed, 29 Apr 2020 18:54:59 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> 2020 18:36:42 +0200:
> 
> > On Wed, 29 Apr 2020 18:32:31 +0200
> > Boris Brezillon <boris.brezillon@collabora.com> wrote:
> >   
> > > On Wed, 29 Apr 2020 18:22:00 +0200
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Hi Boris,
> > > > 
> > > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> > > > 2020 18:08:16 +0200:
> > > >       
> > > > > On Wed, 29 Apr 2020 17:55:32 +0200
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >         
> > > > > > NAND controller drivers can set the NAND_USE_BOUNCE_BUFFER flag to a
> > > > > > chip 'option' field. With this flag, the core is responsible of
> > > > > > providing DMA-able buffers.
> > > > > > 
> > > > > > The current behavior is to not force the use of a bounce buffer when
> > > > > > the core thinks this is not needed. So in the end the name is a bit
> > > > > > misleading, because in theory we will always have a DMA buffer but in
> > > > > > practice it will not always be a bounce buffer.
> > > > > > 
> > > > > > Rename this flag NAND_USE_DMA_BUFFER to be more accurate.          
> > > > > 
> > > > > I still think this one should be named NAND_CONTROLLER_USES_DMA.        
> > > > 
> > > > Actually I want to rework all the flags and prefix them with
> > > > NAND_CONTROLLER, that's why I am keeping the NAND_ prefix. I can change
> > > > the _USE_DMA_BUFFER into _USES_DMA though.      
> > > 
> > > Ack on NAND_USES_DMA.    
> > 
> > But then I wonder if it's really worth renaming this field now if you
> > plan to rename it again later :-).  
> 
> Hehe, well, it "fixes" the meaning of the flag, later changes will only
> be "cosmetic" :)
> 
> Saying we "use a bounce buffer" is not accurate as the code first checks
> if the buffer is compliant with DMA constraints, and uses a bounce
> buffer only if it is not.

I'm not questioning the need to fix the macro name, just the need to do
it twice. Anyway, it's not a big deal if you think it's necessary to do
it in 2 steps.

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

[PATCH v2 04/11] mtd: rawnand: Fix comments about the use of bufpoi

[Miquel Raynal]

Clarify these comments which are not very accurate (even wrong in the
read case).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/nand_base.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index db2745cf4f15..4d8a4a20df63 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3229,7 +3229,10 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 				break;
 			}
 
-			/* Transfer not aligned data */
+			/*
+			 * Copy back the data in the initial buffer when reading
+			 * partial pages or when a bounce buffer is required.
+			 */
 			if (use_bufpoi) {
 				if (!NAND_HAS_SUBPAGE_READ(chip) && !oob &&
 				    !(mtd->ecc_stats.failed - ecc_failures) &&
@@ -4024,7 +4027,10 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
 		else
 			use_bufpoi = 0;
 
-		/* Partial page write?, or need to use bounce buffer */
+		/*
+		 * Copy the data from the initial buffer when doing partial page
+		 * writes or when a bounce buffer is required.
+		 */
 		if (use_bufpoi) {
 			pr_debug("%s: using write bounce buffer for buf@%p\n",
 					 __func__, buf);
-- 
2.20.1


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

[PATCH v2 05/11] mtd: rawnand: Rename the use_bufpoi variables

[Miquel Raynal]

Both in nand_do_read_ops() and nand_do_write_ops() there is a boolean
called use_bufpoi which is set to true in case of unaligned request or
when there is a need for a DMA-able buffer. It basically means "use a
bounce buffer".

Depending on the value of use_bufpoi, the bufpoi variable is always
used and will either point to the original buffer or to the nand_chip
structure "internal data buffer" (this buffer is allocated with
kmalloc() on purpose so that it will be DMA-compliant).

In all cases bufpoi is used so the boolean name is misleading. Rename
use_bufpoi to be use_bouce_buf to be more accurate.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/nand_base.c | 34 ++++++++++++++++----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 4d8a4a20df63..0e2dd4c1b44c 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3166,7 +3166,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 	uint32_t max_oobsize = mtd_oobavail(mtd, ops);
 
 	uint8_t *bufpoi, *oob, *buf;
-	int use_bufpoi;
+	int use_bounce_buf;
 	unsigned int max_bitflips = 0;
 	int retry_mode = 0;
 	bool ecc_fail = false;
@@ -3190,19 +3190,19 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 		aligned = (bytes == mtd->writesize);
 
 		if (!aligned)
-			use_bufpoi = 1;
+			use_bounce_buf = 1;
 		else if (chip->options & NAND_USE_DMA_BUFFER)
-			use_bufpoi = !virt_addr_valid(buf) ||
-				     !IS_ALIGNED((unsigned long)buf,
-						 chip->buf_align);
+			use_bounce_buf = !virt_addr_valid(buf) ||
+					 !IS_ALIGNED((unsigned long)buf,
+						     chip->buf_align);
 		else
-			use_bufpoi = 0;
+			use_bounce_buf = 0;
 
 		/* Is the current page in the buffer? */
 		if (realpage != chip->pagecache.page || oob) {
-			bufpoi = use_bufpoi ? chip->data_buf : buf;
+			bufpoi = use_bounce_buf ? chip->data_buf : buf;
 
-			if (use_bufpoi && aligned)
+			if (use_bounce_buf && aligned)
 				pr_debug("%s: using read bounce buffer for buf@%p\n",
 						 __func__, buf);
 
@@ -3223,7 +3223,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 				ret = chip->ecc.read_page(chip, bufpoi,
 							  oob_required, page);
 			if (ret < 0) {
-				if (use_bufpoi)
+				if (use_bounce_buf)
 					/* Invalidate page cache */
 					chip->pagecache.page = -1;
 				break;
@@ -3233,7 +3233,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 			 * Copy back the data in the initial buffer when reading
 			 * partial pages or when a bounce buffer is required.
 			 */
-			if (use_bufpoi) {
+			if (use_bounce_buf) {
 				if (!NAND_HAS_SUBPAGE_READ(chip) && !oob &&
 				    !(mtd->ecc_stats.failed - ecc_failures) &&
 				    (ops->mode != MTD_OPS_RAW)) {
@@ -4015,23 +4015,23 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
 	while (1) {
 		int bytes = mtd->writesize;
 		uint8_t *wbuf = buf;
-		int use_bufpoi;
+		int use_bounce_buf;
 		int part_pagewr = (column || writelen < mtd->writesize);
 
 		if (part_pagewr)
-			use_bufpoi = 1;
+			use_bounce_buf = 1;
 		else if (chip->options & NAND_USE_DMA_BUFFER)
-			use_bufpoi = !virt_addr_valid(buf) ||
-				     !IS_ALIGNED((unsigned long)buf,
-						 chip->buf_align);
+			use_bounce_buf = !virt_addr_valid(buf) ||
+					 !IS_ALIGNED((unsigned long)buf,
+						     chip->buf_align);
 		else
-			use_bufpoi = 0;
+			use_bounce_buf = 0;
 
 		/*
 		 * Copy the data from the initial buffer when doing partial page
 		 * writes or when a bounce buffer is required.
 		 */
-		if (use_bufpoi) {
+		if (use_bounce_buf) {
 			pr_debug("%s: using write bounce buffer for buf@%p\n",
 					 __func__, buf);
 			if (part_pagewr)
-- 
2.20.1


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

[PATCH v2 06/11] mtd: rawnand: Avoid indirect access to ->data_buf()

[Miquel Raynal]

The logic in nand_do_read_ops() is to use a bufpoi variable, either
set to the original buffer, or set to a bounce buffer which in the end
happens to be chip->data_buf depending on the value of the
use_bounce_buf boolean. This is not a reason to call chip->data_buf
directly when we know that we are using the bounce buffer. Let's use
bufpoi instead to be consistent.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/nand_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 0e2dd4c1b44c..15a9189b2307 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3243,7 +3243,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 					/* Invalidate page cache */
 					chip->pagecache.page = -1;
 				}
-				memcpy(buf, chip->data_buf + col, bytes);
+				memcpy(buf, bufpoi + col, bytes);
 			}
 
 			if (unlikely(oob)) {
-- 
2.20.1


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

[PATCH v2 07/11] mtd: rawnand: onfi: Adapt the parameter page read to constraint controllers

[Miquel Raynal]

We already know that there are controllers not able to read the three
copies of the parameter page in one go. The workaround was to first
request the controller to assert command and address cycles on the
NAND bus to trigger a parameter page read, and then do a read
operation for each page.

But there are also controllers which are not able to split the
parameter page read between the command/address cycles and the actual
data operation.

All controllers are expected to be able to change the read column
though. So let's use a regular parameter page read operation for the
first iteration and use a change read column operation for the
following copies.

The extra command and address cycles sent over the NAND bus are
negligible compared to the amount of data that is being transferred
anyway.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_onfi.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index ee0f2c2549c1..19e1be94c7e3 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -160,14 +160,14 @@ int nand_onfi_detect(struct nand_chip *chip)
 	if (!pbuf)
 		return -ENOMEM;
 
-	ret = nand_read_param_page_op(chip, 0, NULL, 0);
-	if (ret) {
-		ret = 0;
-		goto free_onfi_param_page;
-	}
-
 	for (i = 0; i < ONFI_PARAM_PAGES; i++) {
-		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
+		if (!i)
+			ret = nand_read_param_page_op(chip, 0, &pbuf[i],
+						      sizeof(*pbuf));
+		else
+			ret = nand_change_read_column_op(chip, sizeof(*pbuf) * i,
+							 &pbuf[i], sizeof(*pbuf),
+							 true);
 		if (ret) {
 			ret = 0;
 			goto free_onfi_param_page;
-- 
2.20.1


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

Re: [PATCH v2 07/11] mtd: rawnand: onfi: Adapt the parameter page read to constraint controllers

[Boris Brezillon]

On Wed, 29 Apr 2020 17:55:36 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> We already know that there are controllers not able to read the three
> copies of the parameter page in one go. The workaround was to first
> request the controller to assert command and address cycles on the
> NAND bus to trigger a parameter page read, and then do a read
> operation for each page.
> 
> But there are also controllers which are not able to split the
> parameter page read between the command/address cycles and the actual
> data operation.
> 
> All controllers are expected to be able to change the read column
> though. So let's use a regular parameter page read operation for the
> first iteration and use a change read column operation for the
> following copies.
> 
> The extra command and address cycles sent over the NAND bus are
> negligible compared to the amount of data that is being transferred
> anyway.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/nand_onfi.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index ee0f2c2549c1..19e1be94c7e3 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -160,14 +160,14 @@ int nand_onfi_detect(struct nand_chip *chip)
>  	if (!pbuf)
>  		return -ENOMEM;
>  
> -	ret = nand_read_param_page_op(chip, 0, NULL, 0);
> -	if (ret) {
> -		ret = 0;
> -		goto free_onfi_param_page;
> -	}
> -
>  	for (i = 0; i < ONFI_PARAM_PAGES; i++) {
> -		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
> +		if (!i)
> +			ret = nand_read_param_page_op(chip, 0, &pbuf[i],
> +						      sizeof(*pbuf));
> +		else
> +			ret = nand_change_read_column_op(chip, sizeof(*pbuf) * i,
> +							 &pbuf[i], sizeof(*pbuf),
> +							 true);
>  		if (ret) {
>  			ret = 0;
>  			goto free_onfi_param_page;


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

Re: [PATCH v2 07/11] mtd: rawnand: onfi: Adapt the parameter page read to constraint controllers

[Boris Brezillon]

On Wed, 29 Apr 2020 17:55:36 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> We already know that there are controllers not able to read the three
> copies of the parameter page in one go. The workaround was to first
> request the controller to assert command and address cycles on the
> NAND bus to trigger a parameter page read, and then do a read
> operation for each page.
> 
> But there are also controllers which are not able to split the
> parameter page read between the command/address cycles and the actual
> data operation.
> 
> All controllers are expected to be able to change the read column
> though. So let's use a regular parameter page read operation for the
> first iteration and use a change read column operation for the
> following copies.
> 
> The extra command and address cycles sent over the NAND bus are
> negligible compared to the amount of data that is being transferred
> anyway.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_onfi.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index ee0f2c2549c1..19e1be94c7e3 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -160,14 +160,14 @@ int nand_onfi_detect(struct nand_chip *chip)
>  	if (!pbuf)
>  		return -ENOMEM;
>  
> -	ret = nand_read_param_page_op(chip, 0, NULL, 0);
> -	if (ret) {
> -		ret = 0;
> -		goto free_onfi_param_page;
> -	}
> -
>  	for (i = 0; i < ONFI_PARAM_PAGES; i++) {
> -		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
> +		if (!i)
> +			ret = nand_read_param_page_op(chip, 0, &pbuf[i],
> +						      sizeof(*pbuf));
> +		else
> +			ret = nand_change_read_column_op(chip, sizeof(*pbuf) * i,
> +							 &pbuf[i], sizeof(*pbuf),
> +							 true);

Oops! Looks like this this change will break at least 3 drivers which
support NAND_CMD_PARAM but don't support NAND_CMD_RNDOUT:
fsl_ifc_nand, mxc_nand and qcom_nandc.

>  		if (ret) {
>  			ret = 0;
>  			goto free_onfi_param_page;


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

Re: [PATCH v2 07/11] mtd: rawnand: onfi: Adapt the parameter page read to constraint controllers

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sat, 2 May
2020 10:12:15 +0200:

> On Wed, 29 Apr 2020 17:55:36 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > We already know that there are controllers not able to read the three
> > copies of the parameter page in one go. The workaround was to first
> > request the controller to assert command and address cycles on the
> > NAND bus to trigger a parameter page read, and then do a read
> > operation for each page.
> > 
> > But there are also controllers which are not able to split the
> > parameter page read between the command/address cycles and the actual
> > data operation.
> > 
> > All controllers are expected to be able to change the read column
> > though. So let's use a regular parameter page read operation for the
> > first iteration and use a change read column operation for the
> > following copies.
> > 
> > The extra command and address cycles sent over the NAND bus are
> > negligible compared to the amount of data that is being transferred
> > anyway.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_onfi.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> > index ee0f2c2549c1..19e1be94c7e3 100644
> > --- a/drivers/mtd/nand/raw/nand_onfi.c
> > +++ b/drivers/mtd/nand/raw/nand_onfi.c
> > @@ -160,14 +160,14 @@ int nand_onfi_detect(struct nand_chip *chip)
> >  	if (!pbuf)
> >  		return -ENOMEM;
> >  
> > -	ret = nand_read_param_page_op(chip, 0, NULL, 0);
> > -	if (ret) {
> > -		ret = 0;
> > -		goto free_onfi_param_page;
> > -	}
> > -
> >  	for (i = 0; i < ONFI_PARAM_PAGES; i++) {
> > -		ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf), true);
> > +		if (!i)
> > +			ret = nand_read_param_page_op(chip, 0, &pbuf[i],
> > +						      sizeof(*pbuf));
> > +		else
> > +			ret = nand_change_read_column_op(chip, sizeof(*pbuf) * i,
> > +							 &pbuf[i], sizeof(*pbuf),
> > +							 true);  
> 
> Oops! Looks like this this change will break at least 3 drivers which
> support NAND_CMD_PARAM but don't support NAND_CMD_RNDOUT:
> fsl_ifc_nand, mxc_nand and qcom_nandc.


That's right, here is the new implementation that I am going to share:

-> if (!i) -> READ PARAM PAGE
-> if (use_datain) -> DATA IN
-> else -> RNDOUT.

The use_datain boolean is set to true in the following cases:
- the driver is compliant with exec-op and supports simple reads
- the driver is not compliant with exec-op (we keep the behavior
  unchanged for these drivers).

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

[PATCH v2 08/11] mtd: rawnand: jedec: Adapt the parameter page read to constraint controllers

[Miquel Raynal]

We already know that there are controllers not able to read the three
copies of the parameter page in one go. The workaround was to first
request the controller to assert command and address cycles on the
NAND bus to trigger a parameter page read, and then do a read
operation for each page.

But there are also controllers which are not able to split the
parameter page read between the command/address cycles and the actual
data operation.

All controllers are expected to be able to change the read column
though. So let's use a regular parameter page read operation for the
first iteration and use a change read column operation for the
following copies.

The extra command and address cycles sent over the NAND bus are
negligible compared to the amount of data that is being transferred
anyway.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_jedec.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
index 15937e02c64f..b2be9056759a 100644
--- a/drivers/mtd/nand/raw/nand_jedec.c
+++ b/drivers/mtd/nand/raw/nand_jedec.c
@@ -25,7 +25,7 @@ int nand_jedec_detect(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_memory_organization *memorg;
-	struct nand_jedec_params *p;
+	struct nand_jedec_params *p = NULL, *pbuf;
 	struct jedec_ecc_info *ecc;
 	int jedec_version = 0;
 	char id[5];
@@ -40,25 +40,25 @@ int nand_jedec_detect(struct nand_chip *chip)
 		return 0;
 
 	/* JEDEC chip: allocate a buffer to hold its parameter page */
-	p = kzalloc(sizeof(*p), GFP_KERNEL);
-	if (!p)
+	pbuf = kzalloc(sizeof(*pbuf) * JEDEC_PARAM_PAGES, GFP_KERNEL);
+	if (!pbuf)
 		return -ENOMEM;
 
-	ret = nand_read_param_page_op(chip, 0x40, NULL, 0);
-	if (ret) {
-		ret = 0;
-		goto free_jedec_param_page;
-	}
-
 	for (i = 0; i < JEDEC_PARAM_PAGES; i++) {
-		ret = nand_read_data_op(chip, p, sizeof(*p), true);
+		if (!i)
+			ret = nand_read_param_page_op(chip, 0x40, &pbuf[i],
+						      sizeof(*pbuf));
+		else
+			ret = nand_change_read_column_op(chip, sizeof(*pbuf) * i,
+							 &pbuf[i], sizeof(*pbuf),
+							 true);
 		if (ret) {
 			ret = 0;
 			goto free_jedec_param_page;
 		}
 
-		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 510);
-		if (crc == le16_to_cpu(p->crc))
+		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 510);
+		if (crc == le16_to_cpu(pbuf[i].crc))
 			break;
 	}
 
@@ -67,6 +67,8 @@ int nand_jedec_detect(struct nand_chip *chip)
 		goto free_jedec_param_page;
 	}
 
+	p = pbuf;
+
 	/* Check version */
 	val = le16_to_cpu(p->revision);
 	if (val & (1 << 2))
@@ -122,6 +124,6 @@ int nand_jedec_detect(struct nand_chip *chip)
 	ret = 1;
 
 free_jedec_param_page:
-	kfree(p);
+	kfree(pbuf);
 	return ret;
 }
-- 
2.20.1


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

Re: [PATCH v2 08/11] mtd: rawnand: jedec: Adapt the parameter page read to constraint controllers

[Boris Brezillon]

On Wed, 29 Apr 2020 17:55:37 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> We already know that there are controllers not able to read the three
> copies of the parameter page in one go. The workaround was to first
> request the controller to assert command and address cycles on the
> NAND bus to trigger a parameter page read, and then do a read
> operation for each page.
> 
> But there are also controllers which are not able to split the
> parameter page read between the command/address cycles and the actual
> data operation.
> 
> All controllers are expected to be able to change the read column
> though. So let's use a regular parameter page read operation for the
> first iteration and use a change read column operation for the
> following copies.
> 
> The extra command and address cycles sent over the NAND bus are
> negligible compared to the amount of data that is being transferred
> anyway.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_jedec.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> index 15937e02c64f..b2be9056759a 100644
> --- a/drivers/mtd/nand/raw/nand_jedec.c
> +++ b/drivers/mtd/nand/raw/nand_jedec.c
> @@ -25,7 +25,7 @@ int nand_jedec_detect(struct nand_chip *chip)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct nand_memory_organization *memorg;
> -	struct nand_jedec_params *p;
> +	struct nand_jedec_params *p = NULL, *pbuf;

Looks like you've merged 2 different commits here. I remember you had a
separate commit adding pbuf to avoid an extra memcpy().

>  	struct jedec_ecc_info *ecc;
>  	int jedec_version = 0;
>  	char id[5];
> @@ -40,25 +40,25 @@ int nand_jedec_detect(struct nand_chip *chip)
>  		return 0;
>  
>  	/* JEDEC chip: allocate a buffer to hold its parameter page */
> -	p = kzalloc(sizeof(*p), GFP_KERNEL);
> -	if (!p)
> +	pbuf = kzalloc(sizeof(*pbuf) * JEDEC_PARAM_PAGES, GFP_KERNEL);
> +	if (!pbuf)
>  		return -ENOMEM;
>  
> -	ret = nand_read_param_page_op(chip, 0x40, NULL, 0);
> -	if (ret) {
> -		ret = 0;
> -		goto free_jedec_param_page;
> -	}
> -
>  	for (i = 0; i < JEDEC_PARAM_PAGES; i++) {
> -		ret = nand_read_data_op(chip, p, sizeof(*p), true);
> +		if (!i)
> +			ret = nand_read_param_page_op(chip, 0x40, &pbuf[i],
> +						      sizeof(*pbuf));
> +		else
> +			ret = nand_change_read_column_op(chip, sizeof(*pbuf) * i,
> +							 &pbuf[i], sizeof(*pbuf),
> +							 true);
>  		if (ret) {
>  			ret = 0;
>  			goto free_jedec_param_page;
>  		}
>  
> -		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 510);
> -		if (crc == le16_to_cpu(p->crc))
> +		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 510);
> +		if (crc == le16_to_cpu(pbuf[i].crc))
>  			break;
>  	}
>  
> @@ -67,6 +67,8 @@ int nand_jedec_detect(struct nand_chip *chip)
>  		goto free_jedec_param_page;
>  	}
>  
> +	p = pbuf;
> +
>  	/* Check version */
>  	val = le16_to_cpu(p->revision);
>  	if (val & (1 << 2))
> @@ -122,6 +124,6 @@ int nand_jedec_detect(struct nand_chip *chip)
>  	ret = 1;
>  
>  free_jedec_param_page:
> -	kfree(p);
> +	kfree(pbuf);
>  	return ret;
>  }


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

Re: [PATCH v2 08/11] mtd: rawnand: jedec: Adapt the parameter page read to constraint controllers

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
2020 18:04:05 +0200:

> On Wed, 29 Apr 2020 17:55:37 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > We already know that there are controllers not able to read the three
> > copies of the parameter page in one go. The workaround was to first
> > request the controller to assert command and address cycles on the
> > NAND bus to trigger a parameter page read, and then do a read
> > operation for each page.
> > 
> > But there are also controllers which are not able to split the
> > parameter page read between the command/address cycles and the actual
> > data operation.
> > 
> > All controllers are expected to be able to change the read column
> > though. So let's use a regular parameter page read operation for the
> > first iteration and use a change read column operation for the
> > following copies.
> > 
> > The extra command and address cycles sent over the NAND bus are
> > negligible compared to the amount of data that is being transferred
> > anyway.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_jedec.c | 28 +++++++++++++++-------------
> >  1 file changed, 15 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> > index 15937e02c64f..b2be9056759a 100644
> > --- a/drivers/mtd/nand/raw/nand_jedec.c
> > +++ b/drivers/mtd/nand/raw/nand_jedec.c
> > @@ -25,7 +25,7 @@ int nand_jedec_detect(struct nand_chip *chip)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> >  	struct nand_memory_organization *memorg;
> > -	struct nand_jedec_params *p;
> > +	struct nand_jedec_params *p = NULL, *pbuf;  
> 
> Looks like you've merged 2 different commits here. I remember you had a
> separate commit adding pbuf to avoid an extra memcpy().
> 

Oops. Indeed I merged two commits by mistake, this is a leftover of the
operation, I'll fix it.

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

Re: [PATCH v2 08/11] mtd: rawnand: jedec: Adapt the parameter page read to constraint controllers

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
2020 18:04:05 +0200:

> On Wed, 29 Apr 2020 17:55:37 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > We already know that there are controllers not able to read the three
> > copies of the parameter page in one go. The workaround was to first
> > request the controller to assert command and address cycles on the
> > NAND bus to trigger a parameter page read, and then do a read
> > operation for each page.
> > 
> > But there are also controllers which are not able to split the
> > parameter page read between the command/address cycles and the actual
> > data operation.
> > 
> > All controllers are expected to be able to change the read column
> > though. So let's use a regular parameter page read operation for the
> > first iteration and use a change read column operation for the
> > following copies.
> > 
> > The extra command and address cycles sent over the NAND bus are
> > negligible compared to the amount of data that is being transferred
> > anyway.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_jedec.c | 28 +++++++++++++++-------------
> >  1 file changed, 15 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> > index 15937e02c64f..b2be9056759a 100644
> > --- a/drivers/mtd/nand/raw/nand_jedec.c
> > +++ b/drivers/mtd/nand/raw/nand_jedec.c
> > @@ -25,7 +25,7 @@ int nand_jedec_detect(struct nand_chip *chip)
> >  {
> >  	struct mtd_info *mtd = nand_to_mtd(chip);
> >  	struct nand_memory_organization *memorg;
> > -	struct nand_jedec_params *p;
> > +	struct nand_jedec_params *p = NULL, *pbuf;  
> 
> Looks like you've merged 2 different commits here. I remember you had a
> separate commit adding pbuf to avoid an extra memcpy().

Actually this was only fixed in the onfi detection routine. The jedec
equivalent does not copy the page (the copy came from the 3-way merge
that has only been added to onfi detection).

Here pbuf represents the buffer containing the three pages while p now
only points to the actual page that is correct.

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

Re: [PATCH v2 08/11] mtd: rawnand: jedec: Adapt the parameter page read to constraint controllers

[Boris Brezillon]

On Sun, 3 May 2020 21:06:23 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> 2020 18:04:05 +0200:
> 
> > On Wed, 29 Apr 2020 17:55:37 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > We already know that there are controllers not able to read the three
> > > copies of the parameter page in one go. The workaround was to first
> > > request the controller to assert command and address cycles on the
> > > NAND bus to trigger a parameter page read, and then do a read
> > > operation for each page.
> > > 
> > > But there are also controllers which are not able to split the
> > > parameter page read between the command/address cycles and the actual
> > > data operation.
> > > 
> > > All controllers are expected to be able to change the read column
> > > though. So let's use a regular parameter page read operation for the
> > > first iteration and use a change read column operation for the
> > > following copies.
> > > 
> > > The extra command and address cycles sent over the NAND bus are
> > > negligible compared to the amount of data that is being transferred
> > > anyway.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/mtd/nand/raw/nand_jedec.c | 28 +++++++++++++++-------------
> > >  1 file changed, 15 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> > > index 15937e02c64f..b2be9056759a 100644
> > > --- a/drivers/mtd/nand/raw/nand_jedec.c
> > > +++ b/drivers/mtd/nand/raw/nand_jedec.c
> > > @@ -25,7 +25,7 @@ int nand_jedec_detect(struct nand_chip *chip)
> > >  {
> > >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > >  	struct nand_memory_organization *memorg;
> > > -	struct nand_jedec_params *p;
> > > +	struct nand_jedec_params *p = NULL, *pbuf;    
> > 
> > Looks like you've merged 2 different commits here. I remember you had a
> > separate commit adding pbuf to avoid an extra memcpy().  
> 
> Actually this was only fixed in the onfi detection routine. The jedec
> equivalent does not copy the page (the copy came from the 3-way merge
> that has only been added to onfi detection).
> 
> Here pbuf represents the buffer containing the three pages while p now
> only points to the actual page that is correct.

Oh, I see. Looks like the p = pbuf assignment is wrong BTW (should be
p = &pbuf[i]).

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

Re: [PATCH v2 08/11] mtd: rawnand: jedec: Adapt the parameter page read to constraint controllers

[Miquel Raynal]

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 3 May
2020 21:44:03 +0200:

> On Sun, 3 May 2020 21:06:23 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> > 2020 18:04:05 +0200:
> >   
> > > On Wed, 29 Apr 2020 17:55:37 +0200
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > We already know that there are controllers not able to read the three
> > > > copies of the parameter page in one go. The workaround was to first
> > > > request the controller to assert command and address cycles on the
> > > > NAND bus to trigger a parameter page read, and then do a read
> > > > operation for each page.
> > > > 
> > > > But there are also controllers which are not able to split the
> > > > parameter page read between the command/address cycles and the actual
> > > > data operation.
> > > > 
> > > > All controllers are expected to be able to change the read column
> > > > though. So let's use a regular parameter page read operation for the
> > > > first iteration and use a change read column operation for the
> > > > following copies.
> > > > 
> > > > The extra command and address cycles sent over the NAND bus are
> > > > negligible compared to the amount of data that is being transferred
> > > > anyway.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/mtd/nand/raw/nand_jedec.c | 28 +++++++++++++++-------------
> > > >  1 file changed, 15 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
> > > > index 15937e02c64f..b2be9056759a 100644
> > > > --- a/drivers/mtd/nand/raw/nand_jedec.c
> > > > +++ b/drivers/mtd/nand/raw/nand_jedec.c
> > > > @@ -25,7 +25,7 @@ int nand_jedec_detect(struct nand_chip *chip)
> > > >  {
> > > >  	struct mtd_info *mtd = nand_to_mtd(chip);
> > > >  	struct nand_memory_organization *memorg;
> > > > -	struct nand_jedec_params *p;
> > > > +	struct nand_jedec_params *p = NULL, *pbuf;      
> > > 
> > > Looks like you've merged 2 different commits here. I remember you had a
> > > separate commit adding pbuf to avoid an extra memcpy().    
> > 
> > Actually this was only fixed in the onfi detection routine. The jedec
> > equivalent does not copy the page (the copy came from the 3-way merge
> > that has only been added to onfi detection).
> > 
> > Here pbuf represents the buffer containing the three pages while p now
> > only points to the actual page that is correct.  
> 
> Oh, I see. Looks like the p = pbuf assignment is wrong BTW (should be
> p = &pbuf[i]).

Good catch!

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

[PATCH v2 09/11] mtd: rawnand: Expose monolithic read/write_page_raw() helpers

[Miquel Raynal]

The current nand_read/write_page_raw() helpers are already widely used
but do not fit the purpose of "constrained" controllers which cannot,
for instance, separate command/address cycles with data cycles.

Workaround this issue by proposing alternative helpers that cannot be
used by controller drivers instead.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 60 ++++++++++++++++++++++++++++++++
 include/linux/mtd/rawnand.h      |  8 +++--
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 15a9189b2307..2e525cb5a4e4 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2629,6 +2629,39 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
 }
 EXPORT_SYMBOL(nand_read_page_raw);
 
+/**
+ * nand_monolithic_read_page_raw - Read raw page data without ECC in one go
+ * @chip: nand chip info structure
+ * @buf: buffer to store read data
+ * @oob_required: caller requires OOB data read to chip->oob_poi
+ * @page: page number to read
+ */
+int nand_monolithic_read_page_raw(struct nand_chip *chip, u8 *buf,
+				  int oob_required, int page)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	unsigned int size = mtd->writesize;
+	u8 *read_buf = buf;
+	int ret;
+
+	if (oob_required) {
+		size += mtd->oobsize;
+
+		if (buf != chip->data_buf)
+			read_buf = nand_get_data_buf(chip);
+	}
+
+	ret = nand_read_page_op(chip, page, 0, read_buf, size);
+	if (ret)
+		return ret;
+
+	if (buf != chip->data_buf)
+		memcpy(buf, read_buf, mtd->writesize);
+
+	return 0;
+}
+EXPORT_SYMBOL(nand_monolithic_read_page_raw);
+
 /**
  * nand_read_page_raw_syndrome - [INTERN] read raw page data without ecc
  * @chip: nand chip info structure
@@ -3636,6 +3669,33 @@ int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
 }
 EXPORT_SYMBOL(nand_write_page_raw);
 
+/**
+ * nand_monolithic_write_page_raw - Raw page write in one go
+ * @chip: NAND chip info structure
+ * @buf: data buffer
+ * @oob_required: must write chip->oob_poi to OOB
+ * @page: page number to write
+ */
+int nand_monolithic_write_page_raw(struct nand_chip *chip, const u8 *buf,
+				   int oob_required, int page)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	unsigned int size = mtd->writesize;
+	u8 *write_buf = (u8 *)buf;
+
+	if (oob_required) {
+		size += mtd->oobsize;
+
+		if (buf != chip->data_buf) {
+			write_buf = nand_get_data_buf(chip);
+			memcpy(write_buf, buf, mtd->writesize);
+		}
+	}
+
+	return nand_prog_page_op(chip, page, 0, write_buf, size);
+}
+EXPORT_SYMBOL(nand_monolithic_write_page_raw);
+
 /**
  * nand_write_page_raw_syndrome - [INTERN] raw page write function
  * @chip: nand chip info structure
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index d40ad19ba0f6..0da10e99cf39 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1328,13 +1328,17 @@ int nand_read_oob_std(struct nand_chip *chip, int page);
 int nand_get_set_features_notsupp(struct nand_chip *chip, int addr,
 				  u8 *subfeature_param);
 
-/* Default read_page_raw implementation */
+/* Default read_page_raw implementations */
 int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
 		       int page);
+int nand_monolithic_read_page_raw(struct nand_chip *chip, uint8_t *buf,
+				  int oob_required, int page);
 
-/* Default write_page_raw implementation */
+/* Default write_page_raw implementations */
 int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
 			int oob_required, int page);
+int nand_monolithic_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
+				   int oob_required, int page);
 
 /* Reset and initialize a NAND device */
 int nand_reset(struct nand_chip *chip, int chipnr);
-- 
2.20.1


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

Re: [PATCH v2 09/11] mtd: rawnand: Expose monolithic read/write_page_raw() helpers

[Boris Brezillon]

On Wed, 29 Apr 2020 17:55:38 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> The current nand_read/write_page_raw() helpers are already widely used
> but do not fit the purpose of "constrained" controllers which cannot,
> for instance, separate command/address cycles with data cycles.
> 
> Workaround this issue by proposing alternative helpers that cannot be
> used by controller drivers instead.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 60 ++++++++++++++++++++++++++++++++
>  include/linux/mtd/rawnand.h      |  8 +++--
>  2 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 15a9189b2307..2e525cb5a4e4 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -2629,6 +2629,39 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
>  }
>  EXPORT_SYMBOL(nand_read_page_raw);
>  
> +/**
> + * nand_monolithic_read_page_raw - Read raw page data without ECC in one go

Maybe

"Read the full page (data + OOB) with ECC engine disabled"

?

> + * @chip: nand chip info structure
> + * @buf: buffer to store read data
> + * @oob_required: caller requires OOB data read to chip->oob_poi
> + * @page: page number to read
> + */
> +int nand_monolithic_read_page_raw(struct nand_chip *chip, u8 *buf,
> +				  int oob_required, int page)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	unsigned int size = mtd->writesize;
> +	u8 *read_buf = buf;
> +	int ret;
> +
> +	if (oob_required) {
> +		size += mtd->oobsize;
> +
> +		if (buf != chip->data_buf)
> +			read_buf = nand_get_data_buf(chip);
> +	}
> +
> +	ret = nand_read_page_op(chip, page, 0, read_buf, size);
> +	if (ret)
> +		return ret;
> +
> +	if (buf != chip->data_buf)
> +		memcpy(buf, read_buf, mtd->writesize);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(nand_monolithic_read_page_raw);
> +
>  /**
>   * nand_read_page_raw_syndrome - [INTERN] read raw page data without ecc
>   * @chip: nand chip info structure
> @@ -3636,6 +3669,33 @@ int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
>  }
>  EXPORT_SYMBOL(nand_write_page_raw);
>  
> +/**
> + * nand_monolithic_write_page_raw - Raw page write in one go

Maybe we should have a consistent description for those helpers:

"Write the full page (data + OOB) with ECC engine disabled"

?

> + * @chip: NAND chip info structure
> + * @buf: data buffer
> + * @oob_required: must write chip->oob_poi to OOB
> + * @page: page number to write
> + */
> +int nand_monolithic_write_page_raw(struct nand_chip *chip, const u8 *buf,
> +				   int oob_required, int page)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +	unsigned int size = mtd->writesize;
> +	u8 *write_buf = (u8 *)buf;
> +
> +	if (oob_required) {
> +		size += mtd->oobsize;
> +
> +		if (buf != chip->data_buf) {
> +			write_buf = nand_get_data_buf(chip);
> +			memcpy(write_buf, buf, mtd->writesize);
> +		}
> +	}
> +
> +	return nand_prog_page_op(chip, page, 0, write_buf, size);
> +}
> +EXPORT_SYMBOL(nand_monolithic_write_page_raw);
> +
>  /**
>   * nand_write_page_raw_syndrome - [INTERN] raw page write function
>   * @chip: nand chip info structure
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index d40ad19ba0f6..0da10e99cf39 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1328,13 +1328,17 @@ int nand_read_oob_std(struct nand_chip *chip, int page);
>  int nand_get_set_features_notsupp(struct nand_chip *chip, int addr,
>  				  u8 *subfeature_param);
>  
> -/* Default read_page_raw implementation */
> +/* Default read_page_raw implementations */

Well, nand_monolithic_read_page_raw() is not the default :p. Just drop
the "Default " and it should be good.

>  int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
>  		       int page);
> +int nand_monolithic_read_page_raw(struct nand_chip *chip, uint8_t *buf,
> +				  int oob_required, int page);
>  
> -/* Default write_page_raw implementation */
> +/* Default write_page_raw implementations */

Ditto.

>  int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
>  			int oob_required, int page);
> +int nand_monolithic_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
> +				   int oob_required, int page);
>  
>  /* Reset and initialize a NAND device */
>  int nand_reset(struct nand_chip *chip, int chipnr);


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

Re: [PATCH v2 09/11] mtd: rawnand: Expose monolithic read/write_page_raw() helpers

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
2020 18:15:09 +0200:

> On Wed, 29 Apr 2020 17:55:38 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > The current nand_read/write_page_raw() helpers are already widely used
> > but do not fit the purpose of "constrained" controllers which cannot,
> > for instance, separate command/address cycles with data cycles.
> > 
> > Workaround this issue by proposing alternative helpers that cannot be
> > used by controller drivers instead.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/raw/nand_base.c | 60 ++++++++++++++++++++++++++++++++
> >  include/linux/mtd/rawnand.h      |  8 +++--
> >  2 files changed, 66 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index 15a9189b2307..2e525cb5a4e4 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -2629,6 +2629,39 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
> >  }
> >  EXPORT_SYMBOL(nand_read_page_raw);
> >  
> > +/**
> > + * nand_monolithic_read_page_raw - Read raw page data without ECC in one go  
> 
> Maybe
> 
> "Read the full page (data + OOB) with ECC engine disabled"
> 
> ?

This is not accurate as we don't enforce OOB read.

Don't you find "in one go" explicit enough?

> 
> > + * @chip: nand chip info structure
> > + * @buf: buffer to store read data
> > + * @oob_required: caller requires OOB data read to chip->oob_poi
> > + * @page: page number to read
> > + */
> > +int nand_monolithic_read_page_raw(struct nand_chip *chip, u8 *buf,
> > +				  int oob_required, int page)
> > +{
> > +	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	unsigned int size = mtd->writesize;
> > +	u8 *read_buf = buf;
> > +	int ret;
> > +
> > +	if (oob_required) {
> > +		size += mtd->oobsize;
> > +
> > +		if (buf != chip->data_buf)
> > +			read_buf = nand_get_data_buf(chip);
> > +	}
> > +
> > +	ret = nand_read_page_op(chip, page, 0, read_buf, size);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (buf != chip->data_buf)
> > +		memcpy(buf, read_buf, mtd->writesize);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(nand_monolithic_read_page_raw);
> > +
> >  /**
> >   * nand_read_page_raw_syndrome - [INTERN] read raw page data without ecc
> >   * @chip: nand chip info structure
> > @@ -3636,6 +3669,33 @@ int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
> >  }
> >  EXPORT_SYMBOL(nand_write_page_raw);
> >  
> > +/**
> > + * nand_monolithic_write_page_raw - Raw page write in one go  
> 
> Maybe we should have a consistent description for those helpers:
> 
> "Write the full page (data + OOB) with ECC engine disabled"
> 
> ?*
> 
> > + * @chip: NAND chip info structure
> > + * @buf: data buffer
> > + * @oob_required: must write chip->oob_poi to OOB
> > + * @page: page number to write
> > + */
> > +int nand_monolithic_write_page_raw(struct nand_chip *chip, const u8 *buf,
> > +				   int oob_required, int page)
> > +{
> > +	struct mtd_info *mtd = nand_to_mtd(chip);
> > +	unsigned int size = mtd->writesize;
> > +	u8 *write_buf = (u8 *)buf;
> > +
> > +	if (oob_required) {
> > +		size += mtd->oobsize;
> > +
> > +		if (buf != chip->data_buf) {
> > +			write_buf = nand_get_data_buf(chip);
> > +			memcpy(write_buf, buf, mtd->writesize);
> > +		}
> > +	}
> > +
> > +	return nand_prog_page_op(chip, page, 0, write_buf, size);
> > +}
> > +EXPORT_SYMBOL(nand_monolithic_write_page_raw);
> > +
> >  /**
> >   * nand_write_page_raw_syndrome - [INTERN] raw page write function
> >   * @chip: nand chip info structure
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index d40ad19ba0f6..0da10e99cf39 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -1328,13 +1328,17 @@ int nand_read_oob_std(struct nand_chip *chip, int page);
> >  int nand_get_set_features_notsupp(struct nand_chip *chip, int addr,
> >  				  u8 *subfeature_param);
> >  
> > -/* Default read_page_raw implementation */
> > +/* Default read_page_raw implementations */  
> 
> Well, nand_monolithic_read_page_raw() is not the default :p. Just drop
> the "Default " and it should be good.

Fair enough :)

> 
> >  int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
> >  		       int page);
> > +int nand_monolithic_read_page_raw(struct nand_chip *chip, uint8_t *buf,
> > +				  int oob_required, int page);
> >  
> > -/* Default write_page_raw implementation */
> > +/* Default write_page_raw implementations */  
> 
> Ditto.
> 
> >  int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
> >  			int oob_required, int page);
> > +int nand_monolithic_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
> > +				   int oob_required, int page);
> >  
> >  /* Reset and initialize a NAND device */
> >  int nand_reset(struct nand_chip *chip, int chipnr);  
> 




Thanks,
Miquèl

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

Re: [PATCH v2 09/11] mtd: rawnand: Expose monolithic read/write_page_raw() helpers

[Boris Brezillon]

On Wed, 29 Apr 2020 18:26:31 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> 2020 18:15:09 +0200:
> 
> > On Wed, 29 Apr 2020 17:55:38 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > The current nand_read/write_page_raw() helpers are already widely used
> > > but do not fit the purpose of "constrained" controllers which cannot,
> > > for instance, separate command/address cycles with data cycles.
> > > 
> > > Workaround this issue by proposing alternative helpers that cannot be
> > > used by controller drivers instead.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/mtd/nand/raw/nand_base.c | 60 ++++++++++++++++++++++++++++++++
> > >  include/linux/mtd/rawnand.h      |  8 +++--
> > >  2 files changed, 66 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > > index 15a9189b2307..2e525cb5a4e4 100644
> > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > @@ -2629,6 +2629,39 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
> > >  }
> > >  EXPORT_SYMBOL(nand_read_page_raw);
> > >  
> > > +/**
> > > + * nand_monolithic_read_page_raw - Read raw page data without ECC in one go    
> > 
> > Maybe
> > 
> > "Read the full page (data + OOB) with ECC engine disabled"
> > 
> > ?  
> 
> This is not accurate as we don't enforce OOB read.

Right

> 
> Don't you find "in one go" explicit enough?

Well, "raw" and "without ECC" is redundant, that's the part I wanted to
see addressed. And the 'in one go' refers to the data/OOB split, which
is not clear here.


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

Re: [PATCH v2 09/11] mtd: rawnand: Expose monolithic read/write_page_raw() helpers

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
2020 18:31:37 +0200:

> On Wed, 29 Apr 2020 18:26:31 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> > 2020 18:15:09 +0200:
> >   
> > > On Wed, 29 Apr 2020 17:55:38 +0200
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > The current nand_read/write_page_raw() helpers are already widely used
> > > > but do not fit the purpose of "constrained" controllers which cannot,
> > > > for instance, separate command/address cycles with data cycles.
> > > > 
> > > > Workaround this issue by proposing alternative helpers that cannot be
> > > > used by controller drivers instead.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/mtd/nand/raw/nand_base.c | 60 ++++++++++++++++++++++++++++++++
> > > >  include/linux/mtd/rawnand.h      |  8 +++--
> > > >  2 files changed, 66 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > > > index 15a9189b2307..2e525cb5a4e4 100644
> > > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > > @@ -2629,6 +2629,39 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
> > > >  }
> > > >  EXPORT_SYMBOL(nand_read_page_raw);
> > > >  
> > > > +/**
> > > > + * nand_monolithic_read_page_raw - Read raw page data without ECC in one go      
> > > 
> > > Maybe
> > > 
> > > "Read the full page (data + OOB) with ECC engine disabled"
> > > 
> > > ?    
> > 
> > This is not accurate as we don't enforce OOB read.  
> 
> Right
> 
> > 
> > Don't you find "in one go" explicit enough?  
> 
> Well, "raw" and "without ECC" is redundant, that's the part I wanted to
> see addressed. And the 'in one go' refers to the data/OOB split, which
> is not clear here.
> 


What about

    "Send a single request to the controller driver to read raw data"

the end of the sentence might also be

    "...to read a page plus eventually the OOB area"

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

Re: [PATCH v2 09/11] mtd: rawnand: Expose monolithic read/write_page_raw() helpers

[Boris Brezillon]

On Wed, 29 Apr 2020 18:52:52 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> 2020 18:31:37 +0200:
> 
> > On Wed, 29 Apr 2020 18:26:31 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Boris,
> > > 
> > > Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 29 Apr
> > > 2020 18:15:09 +0200:
> > >     
> > > > On Wed, 29 Apr 2020 17:55:38 +0200
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >       
> > > > > The current nand_read/write_page_raw() helpers are already widely used
> > > > > but do not fit the purpose of "constrained" controllers which cannot,
> > > > > for instance, separate command/address cycles with data cycles.
> > > > > 
> > > > > Workaround this issue by proposing alternative helpers that cannot be
> > > > > used by controller drivers instead.
> > > > > 
> > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > ---
> > > > >  drivers/mtd/nand/raw/nand_base.c | 60 ++++++++++++++++++++++++++++++++
> > > > >  include/linux/mtd/rawnand.h      |  8 +++--
> > > > >  2 files changed, 66 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > > > > index 15a9189b2307..2e525cb5a4e4 100644
> > > > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > > > @@ -2629,6 +2629,39 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
> > > > >  }
> > > > >  EXPORT_SYMBOL(nand_read_page_raw);
> > > > >  
> > > > > +/**
> > > > > + * nand_monolithic_read_page_raw - Read raw page data without ECC in one go        
> > > > 
> > > > Maybe
> > > > 
> > > > "Read the full page (data + OOB) with ECC engine disabled"
> > > > 
> > > > ?      
> > > 
> > > This is not accurate as we don't enforce OOB read.    
> > 
> > Right
> >   
> > > 
> > > Don't you find "in one go" explicit enough?    
> > 
> > Well, "raw" and "without ECC" is redundant, that's the part I wanted to
> > see addressed. And the 'in one go' refers to the data/OOB split, which
> > is not clear here.
> >   
> 
> 
> What about
> 
>     "Send a single request to the controller driver to read raw data"
> 
> the end of the sentence might also be
> 
>     "...to read a page plus eventually the OOB area"

I think we should stick to something shorter here, like "Do a monolithic
page read in raw mode" and have a detailed description (basically
defining what 'raw' and 'monolithic' mean in this context) after the
arguments.

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

[PATCH v2 10/11] mtd: rawnand: Allow controllers to overload soft ECC hooks

[Miquel Raynal]

Some controller drivers do not support executing regular
nand_read/write_page_raw() helpers. For that, we created
nand_monolithic_read/write_page_raw() alternatives. Let's now allow
the driver to overload the ECC ->read/write_page_raw() hooks.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 2e525cb5a4e4..b265bdd38265 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5206,8 +5206,10 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 		ecc->read_page = nand_read_page_swecc;
 		ecc->read_subpage = nand_read_subpage;
 		ecc->write_page = nand_write_page_swecc;
-		ecc->read_page_raw = nand_read_page_raw;
-		ecc->write_page_raw = nand_write_page_raw;
+		if (!ecc->read_page_raw)
+			ecc->read_page_raw = nand_read_page_raw;
+		if (!ecc->write_page_raw)
+			ecc->write_page_raw = nand_write_page_raw;
 		ecc->read_oob = nand_read_oob_std;
 		ecc->write_oob = nand_write_oob_std;
 		if (!ecc->size)
@@ -5229,8 +5231,10 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
 		ecc->read_page = nand_read_page_swecc;
 		ecc->read_subpage = nand_read_subpage;
 		ecc->write_page = nand_write_page_swecc;
-		ecc->read_page_raw = nand_read_page_raw;
-		ecc->write_page_raw = nand_write_page_raw;
+		if (!ecc->read_page_raw)
+			ecc->read_page_raw = nand_read_page_raw;
+		if (!ecc->write_page_raw)
+			ecc->write_page_raw = nand_write_page_raw;
 		ecc->read_oob = nand_read_oob_std;
 		ecc->write_oob = nand_write_oob_std;
 
-- 
2.20.1


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

Re: [PATCH v2 10/11] mtd: rawnand: Allow controllers to overload soft ECC hooks

[Boris Brezillon]

On Wed, 29 Apr 2020 17:55:39 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Some controller drivers do not support executing regular
> nand_read/write_page_raw() helpers. For that, we created
> nand_monolithic_read/write_page_raw() alternatives. Let's now allow
> the driver to overload the ECC ->read/write_page_raw() hooks.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/nand_base.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 2e525cb5a4e4..b265bdd38265 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5206,8 +5206,10 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
>  		ecc->read_page = nand_read_page_swecc;
>  		ecc->read_subpage = nand_read_subpage;
>  		ecc->write_page = nand_write_page_swecc;
> -		ecc->read_page_raw = nand_read_page_raw;
> -		ecc->write_page_raw = nand_write_page_raw;
> +		if (!ecc->read_page_raw)
> +			ecc->read_page_raw = nand_read_page_raw;
> +		if (!ecc->write_page_raw)
> +			ecc->write_page_raw = nand_write_page_raw;
>  		ecc->read_oob = nand_read_oob_std;
>  		ecc->write_oob = nand_write_oob_std;
>  		if (!ecc->size)
> @@ -5229,8 +5231,10 @@ static int nand_set_ecc_soft_ops(struct nand_chip *chip)
>  		ecc->read_page = nand_read_page_swecc;
>  		ecc->read_subpage = nand_read_subpage;
>  		ecc->write_page = nand_write_page_swecc;
> -		ecc->read_page_raw = nand_read_page_raw;
> -		ecc->write_page_raw = nand_write_page_raw;
> +		if (!ecc->read_page_raw)
> +			ecc->read_page_raw = nand_read_page_raw;
> +		if (!ecc->write_page_raw)
> +			ecc->write_page_raw = nand_write_page_raw;
>  		ecc->read_oob = nand_read_oob_std;
>  		ecc->write_oob = nand_write_oob_std;
>  


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

[PATCH v2 11/11] mtd: rawnand: micron: Allow controllers to overload raw accessors

[Miquel Raynal]

Some controller drivers do not support executing regular
nand_read/write_page_raw() helpers. For that, we created
nand_monolithic_read/write_page_raw() alternatives. Let's now allow
the driver to overload the ECC ->read/write_page_raw() hooks when
these hooks are supported.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_micron.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 56654030ec7f..3f109ab31fa1 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -508,8 +508,10 @@ static int micron_nand_init(struct nand_chip *chip)
 			chip->ecc.read_page_raw = nand_read_page_raw_notsupp;
 			chip->ecc.write_page_raw = nand_write_page_raw_notsupp;
 		} else {
-			chip->ecc.read_page_raw = nand_read_page_raw;
-			chip->ecc.write_page_raw = nand_write_page_raw;
+			if (!chip->ecc.read_page_raw)
+				chip->ecc.read_page_raw = nand_read_page_raw;
+			if (!chip->ecc.write_page_raw)
+				chip->ecc.write_page_raw = nand_write_page_raw;
 		}
 	}
 
-- 
2.20.1


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

Re: [PATCH v2 11/11] mtd: rawnand: micron: Allow controllers to overload raw accessors

[Boris Brezillon]

On Wed, 29 Apr 2020 17:55:40 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Some controller drivers do not support executing regular
> nand_read/write_page_raw() helpers. For that, we created
> nand_monolithic_read/write_page_raw() alternatives. Let's now allow
> the driver to overload the ECC ->read/write_page_raw() hooks when
> these hooks are supported.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

I would have merged this patch in patch 10, but that's not a big deal.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/nand/raw/nand_micron.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index 56654030ec7f..3f109ab31fa1 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -508,8 +508,10 @@ static int micron_nand_init(struct nand_chip *chip)
>  			chip->ecc.read_page_raw = nand_read_page_raw_notsupp;
>  			chip->ecc.write_page_raw = nand_write_page_raw_notsupp;
>  		} else {
> -			chip->ecc.read_page_raw = nand_read_page_raw;
> -			chip->ecc.write_page_raw = nand_write_page_raw;
> +			if (!chip->ecc.read_page_raw)
> +				chip->ecc.read_page_raw = nand_read_page_raw;
> +			if (!chip->ecc.write_page_raw)
> +				chip->ecc.write_page_raw = nand_write_page_raw;
>  		}
>  	}
>  


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