[PATCH 00/10] Supporting restricted NAND controllers

[PATCH 00/10] 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.

Path 7 adds a way to test what the controller ->exec_op()
implementation actually supports and decide to either "pack" the NAND
operations as much as possible, otherwise continue to split operations
as usual.

Patches 8 and 9 use this flag to fallback on packed operations during
ONFI/JEDEC parameter page reads (the idea of these two patches is
imported from a previous series).

Finally, patch 10 does the same in the core, in the read/write_page
helpers.

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

Thanks,
Miquèl

Miquel Raynal (10):
  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: Help supporting controllers that are not able to split
    operations
  mtd: rawnand: onfi: Add an alternative parameter page read
  mtd: rawnand: jedec: Add an alternative parameter page read
  mtd: rawnand: Fallback on easier operations when needed

 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/internals.h             |   5 +
 drivers/mtd/nand/raw/meson_nand.c            |   2 +-
 drivers/mtd/nand/raw/mtk_nand.c              |   2 +-
 drivers/mtd/nand/raw/nand_base.c             | 110 +++++++++++++++----
 drivers/mtd/nand/raw/nand_jedec.c            |  31 ++++--
 drivers/mtd/nand/raw/nand_onfi.c             |  17 ++-
 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                  | 105 ++++++++++--------
 15 files changed, 191 insertions(+), 97 deletions(-)

-- 
2.20.1


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

[PATCH 01/10] 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>
---
 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..c92e7ecebdea 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(22)
 
 /*
  * 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(20)
 
 /*
  * 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 02/10] 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>
---
 include/linux/mtd/rawnand.h | 69 +++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index c92e7ecebdea..dee4578d2389 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,11 +180,12 @@ 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
+ * 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_USE_BOUNCE_BUFFER	BIT(22)
+#define NAND_IS_BOOT_MEDIUM	BIT(20)
 
 /*
  * In case your controller is implementing ->legacy.cmd_ctrl() and is relying
@@ -210,10 +198,10 @@ enum nand_ecc_algo {
 #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.
+ * This option could be defined by controller drivers to protect against
+ * kmap'ed, vmalloc'ed highmem buffers being passed from upper layers
  */
-#define NAND_IS_BOOT_MEDIUM	BIT(20)
+#define NAND_USE_BOUNCE_BUFFER	BIT(22)
 
 /*
  * Do not try to tweak the timings at runtime. This is needed when the
@@ -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 03/10] 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 dee4578d2389..21753b83d536 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -201,7 +201,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(22)
+#define NAND_USE_DMA_BUFFER	BIT(22)
 
 /*
  * Do not try to tweak the timings at runtime. This is needed when the
-- 
2.20.1


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

[PATCH 04/10] 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>
---
 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 05/10] 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>
---
 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 06/10] 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>
---
 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 07/10] mtd: rawnand: Help supporting controllers that are not able to split operations

[Miquel Raynal]

While performing any NAND operation is as simple as following the
cores order and send "command", "address" and "data" cycles as
provided in a list of instructions, certain controllers are "too
clever" and are not able to split the sending of these cycles.

Try to find out at boot time if the controller will be problematic and
flag it. Additional changes will make use of this flag to workaround
the capricious controllers by proposing "packed" operations as an
alternative.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/internals.h |  5 ++++
 drivers/mtd/nand/raw/nand_base.c | 44 ++++++++++++++++++++++++++++++++
 include/linux/mtd/rawnand.h      |  8 ++++++
 3 files changed, 57 insertions(+)

diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index 9d0caadf940e..38898b8639ee 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -130,6 +130,11 @@ static inline bool nand_has_setup_data_iface(struct nand_chip *chip)
 	return true;
 }
 
+static inline bool nand_pack_ops(struct nand_chip *chip)
+{
+	return (chip->options & NAND_PACK_OPS);
+}
+
 /* BBT functions */
 int nand_markbad_bbt(struct nand_chip *chip, loff_t offs);
 int nand_isreserved_bbt(struct nand_chip *chip, loff_t offs);
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 15a9189b2307..6e4eabb9dc11 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5031,6 +5031,44 @@ static int nand_dt_init(struct nand_chip *chip)
 	return 0;
 }
 
+/**
+ * nand_controller_needs_packed_op - Check the controller habilities to perform
+ *                                   a set of split operations that the core is
+ *                                   very likely to try. If one of them do not
+ *                                   pass, then try to pack operations together.
+ * @chip: The NAND chip
+ *
+ * Returns @true if packing is needed, false otherwise.
+ */
+static bool nand_controller_needs_packed_op(struct nand_chip *chip)
+{
+	u8 tmp[8];
+	struct nand_op_instr data_in_instrs[] = {
+		NAND_OP_DATA_IN(8, tmp, 0),
+	};
+	struct nand_op_instr data_out_instrs[] = {
+		NAND_OP_DATA_OUT(8, tmp, 0),
+	};
+	struct nand_operation ops[] = {
+		NAND_OPERATION(0, data_in_instrs),
+		NAND_OPERATION(0, data_out_instrs),
+	};
+	int ret, i;
+
+	if (!nand_has_exec_op(chip))
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(ops); i++) {
+		ret = chip->controller->ops->exec_op(chip, &ops[i], true);
+		if (ret) {
+			pr_debug("Using ->exec_op() packed operations only\n");
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /**
  * nand_scan_ident - Scan for the NAND device
  * @chip: NAND chip object
@@ -5052,6 +5090,7 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
 	struct nand_memory_organization *memorg;
 	int nand_maf_id, nand_dev_id;
 	unsigned int i;
+	bool pack_ops;
 	int ret;
 
 	memorg = nanddev_get_memorg(&chip->base);
@@ -5080,6 +5119,11 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
 
 	memorg->ntargets = maxchips;
 
+	/* Verify the controller's abilities */
+	pack_ops = nand_controller_needs_packed_op(chip);
+	if (pack_ops)
+		chip->options |= NAND_PACK_OPS;
+
 	/* Read the flash type */
 	ret = nand_detect(chip, table);
 	if (ret) {
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 21753b83d536..4ecc6be434e0 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -170,6 +170,14 @@ enum nand_ecc_algo {
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
 #define NAND_SKIP_BBTSCAN	BIT(16)
+
+/*
+ * Controller does not support "naked" operations, the core should try to pack
+ * the NAND commands as much as possible thanks to the constant use of a bounce
+ * buffer. This flag must be set by the core only.
+ */
+#define NAND_PACK_OPS		BIT(17)
+
 /* Chip may not exist, so silence any errors in scan */
 #define NAND_SCAN_SILENT_NODEV	BIT(18)
 
-- 
2.20.1


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

[PATCH 08/10] mtd: rawnand: onfi: Add an alternative parameter page read

[Miquel Raynal]

Some controllers are not able to read the parameter page in separate chunks.

As there is no need for separate parameter page reads (the delay penalty
for reading the three copies in one go being negligible), the
temptation to just do a monolithic read is high.

But we are afraid of controllers not supporting reading a parameter
page of 768 bytes neither.

So, despite darkening a little bit this portion, the final solution to
support as many controllers as possible is to check if there is an
actual need for such monolithic read, otherwise we keep the current
behavior.

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

diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index 2fc71b7c361f..e6be19e1afde 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -160,17 +160,24 @@ int nand_onfi_detect(struct nand_chip *chip)
 	if (!pbuf)
 		return -ENOMEM;
 
-	ret = nand_read_param_page_op(chip, 0, NULL, 0);
+	if (nand_pack_ops(chip))
+		ret = nand_read_param_page_op(chip, 0, pbuf,
+					      sizeof(*pbuf) * ONFI_PARAM_PAGES);
+	else
+		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 (ret) {
-			ret = 0;
-			goto free_onfi_param_page;
+		if (!nand_pack_ops(chip)) {
+			ret = nand_read_data_op(chip, &pbuf[i], sizeof(*pbuf),
+						true);
+			if (ret) {
+				ret = 0;
+				goto free_onfi_param_page;
+			}
 		}
 
 		crc = onfi_crc16(ONFI_CRC_BASE, (u8 *)&pbuf[i], 254);
-- 
2.20.1


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

[PATCH 09/10] mtd: rawnand: jedec: Add an alternative parameter page read

[Miquel Raynal]

Some controllers are not able to read the parameter page in separate chunks.

As there is no need for separate parameter page reads (the delay penalty
for reading the three copies in one go being negligible), the
temptation to just do a monolithic read is high.

But we are afraid of controllers not supporting reading a parameter
page of 1536 bytes neither.

So, despite darkening a little bit this portion, the final solution to
support as many controllers as possible is to check if there is an
actual need for such monolithic read, otherwise we keep the current
behavior.

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

diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
index 15937e02c64f..c88a5f1049d2 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,32 @@ 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 (nand_pack_ops(chip))
+		ret = nand_read_param_page_op(chip, 0x40, pbuf,
+					      sizeof(*pbuf) * JEDEC_PARAM_PAGES);
+	else
+		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 (ret) {
-			ret = 0;
-			goto free_jedec_param_page;
+		if (!nand_pack_ops(chip)) {
+			ret = nand_read_data_op(chip, &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 +74,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 +131,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/

[PATCH 10/10] mtd: rawnand: Fallback on easier operations when needed

[Miquel Raynal]

nand_read/write_page_raw() helpers are extensively used when working
with software ECC engines, hence they must support almost any kind of
controller.

Now that we have a way to distinguish between a controller that
supports split operations and one that do not, let's support both.

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

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 6e4eabb9dc11..145b3059e5ff 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2614,6 +2614,12 @@ int nand_read_page_raw(struct nand_chip *chip, uint8_t *buf, int oob_required,
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int ret;
 
+	if (nand_pack_ops(chip)) {
+		int len = mtd->writesize + (oob_required ? mtd->oobsize : 0);
+
+		return nand_read_page_op(chip, page, 0, buf, len);
+	}
+
 	ret = nand_read_page_op(chip, page, 0, buf, mtd->writesize);
 	if (ret)
 		return ret;
@@ -3189,7 +3195,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 		bytes = min(mtd->writesize - col, readlen);
 		aligned = (bytes == mtd->writesize);
 
-		if (!aligned)
+		if (!aligned || nand_pack_ops(chip))
 			use_bounce_buf = 1;
 		else if (chip->options & NAND_USE_DMA_BUFFER)
 			use_bounce_buf = !virt_addr_valid(buf) ||
@@ -3621,6 +3627,12 @@ int nand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int ret;
 
+	if (nand_pack_ops(chip)) {
+		int len = mtd->writesize + (oob_required ? mtd->oobsize : 0);
+
+		return nand_prog_page_op(chip, page, 0, buf, len);
+	}
+
 	ret = nand_prog_page_begin_op(chip, page, 0, buf, mtd->writesize);
 	if (ret)
 		return ret;
@@ -4018,7 +4030,7 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
 		int use_bounce_buf;
 		int part_pagewr = (column || writelen < mtd->writesize);
 
-		if (part_pagewr)
+		if (part_pagewr || nand_pack_ops(chip))
 			use_bounce_buf = 1;
 		else if (chip->options & NAND_USE_DMA_BUFFER)
 			use_bounce_buf = !virt_addr_valid(buf) ||
-- 
2.20.1


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

Re: [PATCH 01/10] mtd: rawnand: Translate obscure bitfields into readable macros

[Boris Brezillon]

On Fri, 24 Apr 2020 19:36:22 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

>  /*
>   * 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(22)

					^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(20)

				^ BIT(22)

The rest looks good, feel fre to add

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

once those 2 mistakes are fixed.

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

Re: [PATCH 02/10] mtd: rawnand: Reorder the nand_chip->options flags

[Boris Brezillon]

On Fri, 24 Apr 2020 19:36:23 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> 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>

With the correct USE_BOUNCE_BUF and BOOT_MEDIUM original value, this
patch is

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



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

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

[Boris Brezillon]

On Fri, 24 Apr 2020 19:36:24 +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 would suggest renaming it NAND_CONTROLLER_NEEDS_DMAABLE_BUFFER or
NAND_CONTROLLER_USES_DMA, and maybe we should introduce NAND controller
flags (nand_controller.flags) instead of hijacking the NAND chip flags.

> 
> 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 dee4578d2389..21753b83d536 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -201,7 +201,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(22)
> +#define NAND_USE_DMA_BUFFER	BIT(22)
>  
>  /*
>   * Do not try to tweak the timings at runtime. This is needed when the


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

Re: [PATCH 04/10] mtd: rawnand: Fix comments about the use of bufpoi

[Boris Brezillon]

On Fri, 24 Apr 2020 19:36:25 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> 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);


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

Re: [PATCH 05/10] mtd: rawnand: Rename the use_bufpoi variables

[Boris Brezillon]

On Fri, 24 Apr 2020 19:36:26 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> 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>

I wonder if we shouldn't find a better name for bufpoi too. Not sure
what the poi means here (pointer?). So maybe just rename those into
read_buf, write_buf (since buf seems to be declared already).

> ---
>  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)


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

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

[Boris Brezillon]

On Fri, 24 Apr 2020 19:36:27 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> 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)) {


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

Re: [PATCH 07/10] mtd: rawnand: Help supporting controllers that are not able to split operations

[Boris Brezillon]

On Fri, 24 Apr 2020 19:36:28 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> While performing any NAND operation is as simple as following the
> cores order and send "command", "address" and "data" cycles as
> provided in a list of instructions, certain controllers are "too
> clever" and are not able to split the sending of these cycles.
> 
> Try to find out at boot time if the controller will be problematic and
> flag it. Additional changes will make use of this flag to workaround
> the capricious controllers by proposing "packed" operations as an
> alternative.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/internals.h |  5 ++++
>  drivers/mtd/nand/raw/nand_base.c | 44 ++++++++++++++++++++++++++++++++
>  include/linux/mtd/rawnand.h      |  8 ++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 9d0caadf940e..38898b8639ee 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -130,6 +130,11 @@ static inline bool nand_has_setup_data_iface(struct nand_chip *chip)
>  	return true;
>  }
>  
> +static inline bool nand_pack_ops(struct nand_chip *chip)
> +{
> +	return (chip->options & NAND_PACK_OPS);
> +}
> +
>  /* BBT functions */
>  int nand_markbad_bbt(struct nand_chip *chip, loff_t offs);
>  int nand_isreserved_bbt(struct nand_chip *chip, loff_t offs);
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 15a9189b2307..6e4eabb9dc11 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5031,6 +5031,44 @@ static int nand_dt_init(struct nand_chip *chip)
>  	return 0;
>  }
>  
> +/**
> + * nand_controller_needs_packed_op - Check the controller habilities to perform
> + *                                   a set of split operations that the core is
> + *                                   very likely to try. If one of them do not
> + *                                   pass, then try to pack operations together.
> + * @chip: The NAND chip
> + *
> + * Returns @true if packing is needed, false otherwise.
> + */
> +static bool nand_controller_needs_packed_op(struct nand_chip *chip)
> +{
> +	u8 tmp[8];
> +	struct nand_op_instr data_in_instrs[] = {
> +		NAND_OP_DATA_IN(8, tmp, 0),
> +	};
> +	struct nand_op_instr data_out_instrs[] = {
> +		NAND_OP_DATA_OUT(8, tmp, 0),
> +	};
> +	struct nand_operation ops[] = {
> +		NAND_OPERATION(0, data_in_instrs),
> +		NAND_OPERATION(0, data_out_instrs),
> +	};
> +	int ret, i;
> +
> +	if (!nand_has_exec_op(chip))
> +		return false;
> +
> +	for (i = 0; i < ARRAY_SIZE(ops); i++) {
> +		ret = chip->controller->ops->exec_op(chip, &ops[i], true);
> +		if (ret) {
> +			pr_debug("Using ->exec_op() packed operations only\n");
> +			return true;
> +		}
> +	}

Hm, I'm not sure that's enough to detect all weird cases that the
controller might support or not. The check should really be done on
actual operations with accurate sizes instead of using a randomly chosen
8byte data in/out pattern to decide whether all ops need to be
monolithic or not.

So, let's take a step back and analyze your use cases. You seem to have
3 here:

1/ read param page
2/ program page
3/ read page

For #1, we can just do the check before executing the operation because
it's only done once at init time (not in the read/write/erase path
where we care about perfs). For that one I'd suggest extending the
nand_read_param_page_op() function to take a check_only parameter and
doing the check directly in nand_{onfi,jedec}_detect().

For #2 and #3, I'd rather have per operation flags to pick the right
variant, but maybe a simpler option would be to add new helpers for
those monolithic read/write until we come up with a generic way to
determine which variants of each perf-sensitive sequences should be
used based on exec_op() checks.

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) {
			chip->pagecache.page = -1;
			read_buf = chip->data_buf;
		}
	}

	ret = nand_read_page_op(chip, page, 0, read_buf, size);
	if (ret)
		return ret;

	if (buf != read_buf)
		memcpy(buf, read_buf, mtd->writesize)

	return 0;
}

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;
	const u8 *write_buf = buf;
	int ret;

	if (oob_required) {
		size += mtd->oobsize;

		if (buf != chip->data_buf) {
			chip->pagecache.page = -1;
			memcpy(chip->data_buf, buf, mtd->writesize);
			write_buf = chip->data_buf;
		}
	}

	return nand_prog_page_op(chip, page, 0, write_buf, size);
}

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

Re: [PATCH 05/10] mtd: rawnand: Rename the use_bufpoi variables

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sat, 25 Apr
2020 10:44:40 +0200:

> On Fri, 24 Apr 2020 19:36:26 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > 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>
> 
> I wonder if we shouldn't find a better name for bufpoi too. Not sure
> what the poi means here (pointer?). So maybe just rename those into
> read_buf, write_buf (since buf seems to be declared already).

My first patch also renamed bufpoi.

Actually I read it like "buf pointer" and it makes sense and is used
all across nand_base.c so I decided to let it as-is for now.

Thanks,
Miquèl

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

Re: [PATCH 05/10] mtd: rawnand: Rename the use_bufpoi variables

[Boris Brezillon]

On Tue, 28 Apr 2020 11:05:01 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Sat, 25 Apr
> 2020 10:44:40 +0200:
> 
> > On Fri, 24 Apr 2020 19:36:26 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > 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>
> > 
> > I wonder if we shouldn't find a better name for bufpoi too. Not sure
> > what the poi means here (pointer?). So maybe just rename those into
> > read_buf, write_buf (since buf seems to be declared already).  
> 
> My first patch also renamed bufpoi.
> 
> Actually I read it like "buf pointer" and it makes sense and is used
> all across nand_base.c so I decided to let it as-is for now.

Fair enough.

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

Re: [PATCH 01/10] mtd: rawnand: Translate obscure bitfields into readable macros

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sat, 25 Apr
2020 10:33:20 +0200:

> On Fri, 24 Apr 2020 19:36:22 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> >  /*
> >   * 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(22)  
> 
> 					^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(20)  
> 
> 				^ BIT(22)
> 
> The rest looks good, feel fre to add
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> 
> once those 2 mistakes are fixed.

Mmh yeah absolutely, fixed.


Thanks,
Miquèl

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

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

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sat, 25 Apr
2020 10:39:56 +0200:

> On Fri, 24 Apr 2020 19:36:24 +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 would suggest renaming it NAND_CONTROLLER_NEEDS_DMAABLE_BUFFER or
> NAND_CONTROLLER_USES_DMA, and maybe we should introduce NAND controller
> flags (nand_controller.flags) instead of hijacking the NAND chip flags.

Agreed, in fact I would like to entirely rework all the NAND flags. I
will do it in a separate series.


Thanks,
Miquèl

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