linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] pxa3xx_nand: Assorted changes
@ 2015-11-04 16:13 Ezequiel Garcia
  2015-11-04 16:13 ` [PATCH 1/6] mtd: pxa3xx_nand: Increase the initial chunk size Ezequiel Garcia
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 16:13 UTC (permalink / raw)
  To: linux-mtd; +Cc: Robert Jarzmik, Antoine Tenart, Brian Norris, Ezequiel Garcia

This series contains a number of fixes and cleanups.
As described below, this series doesn't seem to fix any
currently known regression. IMO, there's no need to pick
any of this for v4.4-rc.

The first patch fixes a timeout found when issuing a CMD_PARAM,
without using the keep-config option. This timeout happens
only on NFCv2 (i.e. Armada 370/375/XP) with the keep-config option.

Since all the Armada 370/375/XP boards currently specify
keep-config, no user should be affected by the timeout.

The second patch is a fix for some leftovers issues from the
recent cleanup series by Antoine. Altough the patch can be
considered a fix, I'm not aware of any regression so I can't
argue it should be queued as a fix.

The rest of the patches are either cleanups or improvements.

Patch 3 simplifies the code around keep-config and removes
a silly goto.

Patch 4 removes the open-coded NAND chip sensing.

Patch 5 removes an unused macro.

Path 6 uses enable/disable clk to save power in resume/suspend
paths.

Thanks,

Ezequiel Garcia (6):
  mtd: pxa3xx_nand: Increase the initial chunk size
  mtd: pxa3xx_nand: Fix initial controller configuration
  mtd: pxa3xx_nand: Simplify pxa3xx_nand_scan
  mtd: pxa3xx_nand: Remove redundant NAND sensing
  mtd: pxa3xx_nand: Remove dead code
  mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths

 drivers/mtd/nand/pxa3xx_nand.c | 101 +++++++++++++++++------------------------
 1 file changed, 42 insertions(+), 59 deletions(-)

-- 
2.5.2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] mtd: pxa3xx_nand: Increase the initial chunk size
  2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
@ 2015-11-04 16:13 ` Ezequiel Garcia
  2015-11-04 20:15   ` Robert Jarzmik
  2015-11-04 16:13 ` [PATCH 2/6] mtd: pxa3xx_nand: Fix initial controller configuration Ezequiel Garcia
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 16:13 UTC (permalink / raw)
  To: linux-mtd; +Cc: Robert Jarzmik, Antoine Tenart, Brian Norris, Ezequiel Garcia

The chunk size represents the size of the data chunks, which
is used by the controllers that allow to split transfered data.

However, the initial chunk size is used in a non-splitted way,
during device identification. Therefore, it must be large enough
for all the NAND commands issued during device identification.
This includes NAND_CMD_PARAM which was recently changed to
transfer up to 2048 bytes (for the redundant parameter pages).

Thus, the initial chunk size should be 2048 as well.

On Armada 370/XP platforms (NFCv2) booted without the keep-config
devicetree property, this commit fixes a timeout on the NAND_CMD_PARAM
command:

  [..]
  pxa3xx-nand f10d0000.nand: This platform can't do DMA on this device
  pxa3xx-nand f10d0000.nand: Wait time out!!!
  nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x38
  nand: Micron MT29F8G08ABABAWP
  nand: 1024 MiB, SLC, erase size: 512 KiB, page size: 4096, OOB size: 224

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/pxa3xx_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index e453ae9a17fa..3d3ae903eec8 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1592,7 +1592,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 		goto KEEP_CONFIG;
 
 	/* Set a default chunk size */
-	info->chunk_size = 512;
+	info->chunk_size = PAGE_CHUNK_SIZE;
 
 	ret = pxa3xx_nand_config_flash(info);
 	if (ret)
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] mtd: pxa3xx_nand: Fix initial controller configuration
  2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
  2015-11-04 16:13 ` [PATCH 1/6] mtd: pxa3xx_nand: Increase the initial chunk size Ezequiel Garcia
@ 2015-11-04 16:13 ` Ezequiel Garcia
  2015-11-04 16:13 ` [PATCH 3/6] nand: pxa3xx: Simplify pxa3xx_nand_scan Ezequiel Garcia
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 16:13 UTC (permalink / raw)
  To: linux-mtd; +Cc: Robert Jarzmik, Antoine Tenart, Brian Norris, Ezequiel Garcia

The Data Flash Control Register (NDCR) contains two types
of parameters: those that are needed for device identification,
and those that can only be set after device identification.

Therefore, the driver can't set them all at once and instead
needs to configure the first group before nand_scan_ident()
and the second group later.

Let's split pxa3xx_nand_config in two halves, and set the
parameters that depend on the device geometry once this is known.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/pxa3xx_nand.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 3d3ae903eec8..a82f52d150f8 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1387,34 +1387,43 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
 	return NAND_STATUS_READY;
 }
 
-static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info)
+static int pxa3xx_nand_config_ident(struct pxa3xx_nand_info *info)
 {
 	struct platform_device *pdev = info->pdev;
 	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
-	struct pxa3xx_nand_host *host = info->host[info->cs];
-	struct mtd_info *mtd = host->mtd;
-	struct nand_chip *chip = mtd->priv;
 
-	/* configure default flash values */
+	/* Configure default flash values */
+	info->chunk_size = PAGE_CHUNK_SIZE;
 	info->reg_ndcr = 0x0; /* enable all interrupts */
 	info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
 	info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
-	info->reg_ndcr |= NDCR_SPARE_EN; /* enable spare by default */
+	info->reg_ndcr |= NDCR_SPARE_EN;
+
+	return 0;
+}
+
+static void pxa3xx_nand_config_tail(struct pxa3xx_nand_info *info)
+{
+	struct pxa3xx_nand_host *host = info->host[info->cs];
+	struct mtd_info *mtd = host->mtd;
+	struct nand_chip *chip = mtd->priv;
+
 	info->reg_ndcr |= (host->col_addr_cycles == 2) ? NDCR_RA_START : 0;
 	info->reg_ndcr |= (chip->page_shift == 6) ? NDCR_PG_PER_BLK : 0;
 	info->reg_ndcr |= (mtd->writesize == 2048) ? NDCR_PAGE_SZ : 0;
-
-	return 0;
 }
 
 static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 {
+	struct platform_device *pdev = info->pdev;
+	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	uint32_t ndcr = nand_readl(info, NDCR);
 
 	/* Set an initial chunk size */
 	info->chunk_size = ndcr & NDCR_PAGE_SZ ? 2048 : 512;
 	info->reg_ndcr = ndcr &
 		~(NDCR_INT_MASK | NDCR_ND_ARB_EN | NFCV1_NDCR_ARB_CNTL);
+	info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
 	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
 	info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
 	return 0;
@@ -1591,10 +1600,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
 		goto KEEP_CONFIG;
 
-	/* Set a default chunk size */
-	info->chunk_size = PAGE_CHUNK_SIZE;
-
-	ret = pxa3xx_nand_config_flash(info);
+	ret = pxa3xx_nand_config_ident(info);
 	if (ret)
 		return ret;
 
@@ -1607,7 +1613,6 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	}
 
 KEEP_CONFIG:
-	info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
 	if (info->reg_ndcr & NDCR_DWIDTH_M)
 		chip->options |= NAND_BUSWIDTH_16;
 
@@ -1692,6 +1697,10 @@ KEEP_CONFIG:
 		host->row_addr_cycles = 3;
 	else
 		host->row_addr_cycles = 2;
+
+	if (!pdata->keep_config)
+		pxa3xx_nand_config_tail(info);
+
 	return nand_scan_tail(mtd);
 }
 
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] nand: pxa3xx: Simplify pxa3xx_nand_scan
  2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
  2015-11-04 16:13 ` [PATCH 1/6] mtd: pxa3xx_nand: Increase the initial chunk size Ezequiel Garcia
  2015-11-04 16:13 ` [PATCH 2/6] mtd: pxa3xx_nand: Fix initial controller configuration Ezequiel Garcia
@ 2015-11-04 16:13 ` Ezequiel Garcia
  2015-11-04 20:56   ` Robert Jarzmik
  2015-11-04 16:13 ` [PATCH 4/6] mtd: pxa3xx_nand: Remove redundant NAND sensing Ezequiel Garcia
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 16:13 UTC (permalink / raw)
  To: linux-mtd; +Cc: Robert Jarzmik, Antoine Tenart, Brian Norris, Ezequiel Garcia

This commit simplifies the initial configuration performed
by pxa3xx_nand_scan. No functionality change is intended.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/pxa3xx_nand.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index a82f52d150f8..fd8658a9281e 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1413,7 +1413,7 @@ static void pxa3xx_nand_config_tail(struct pxa3xx_nand_info *info)
 	info->reg_ndcr |= (mtd->writesize == 2048) ? NDCR_PAGE_SZ : 0;
 }
 
-static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
+static void pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 {
 	struct platform_device *pdev = info->pdev;
 	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -1426,7 +1426,6 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 	info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
 	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
 	info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
-	return 0;
 }
 
 static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
@@ -1597,22 +1596,21 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	int ret;
 	uint16_t ecc_strength, ecc_step;
 
-	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
-		goto KEEP_CONFIG;
-
-	ret = pxa3xx_nand_config_ident(info);
-	if (ret)
-		return ret;
-
-	ret = pxa3xx_nand_sensing(host);
-	if (ret) {
-		dev_info(&info->pdev->dev, "There is no chip on cs %d!\n",
-			 info->cs);
-
-		return ret;
+	if (pdata->keep_config) {
+		pxa3xx_nand_detect_config(info);
+	} else {
+		ret = pxa3xx_nand_config_ident(info);
+		if (ret)
+			return ret;
+		ret = pxa3xx_nand_sensing(host);
+		if (ret) {
+			dev_info(&info->pdev->dev,
+				 "There is no chip on cs %d!\n",
+				 info->cs);
+			return ret;
+		}
 	}
 
-KEEP_CONFIG:
 	if (info->reg_ndcr & NDCR_DWIDTH_M)
 		chip->options |= NAND_BUSWIDTH_16;
 
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] mtd: pxa3xx_nand: Remove redundant NAND sensing
  2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
                   ` (2 preceding siblings ...)
  2015-11-04 16:13 ` [PATCH 3/6] nand: pxa3xx: Simplify pxa3xx_nand_scan Ezequiel Garcia
@ 2015-11-04 16:13 ` Ezequiel Garcia
  2015-11-04 21:05   ` Robert Jarzmik
  2015-11-04 16:13 ` [PATCH 5/6] mtd: pxa3xx_nand: Remove dead code Ezequiel Garcia
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 16:13 UTC (permalink / raw)
  To: linux-mtd; +Cc: Robert Jarzmik, Antoine Tenart, Brian Norris, Ezequiel Garcia

Currently, the driver is trying to detect the presence of a chip
by issuing a RESET command before nand_scan_ident. This seems completely
redundant, and is also a layering violation as nand_scan_ident is in charge
of device detection.

This commit removes the RESET command use, and moves the initial
timing configuration to pxa3xx_nand_config_ident.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/pxa3xx_nand.c | 41 ++++++++---------------------------------
 1 file changed, 8 insertions(+), 33 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index fd8658a9281e..89c02f7057fd 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1389,8 +1389,10 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
 
 static int pxa3xx_nand_config_ident(struct pxa3xx_nand_info *info)
 {
+	struct pxa3xx_nand_host *host = info->host[info->cs];
 	struct platform_device *pdev = info->pdev;
 	struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	const struct nand_sdr_timings *timings;
 
 	/* Configure default flash values */
 	info->chunk_size = PAGE_CHUNK_SIZE;
@@ -1399,6 +1401,12 @@ static int pxa3xx_nand_config_ident(struct pxa3xx_nand_info *info)
 	info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
 	info->reg_ndcr |= NDCR_SPARE_EN;
 
+	/* use the common timing to make a try */
+	timings = onfi_async_timing_mode_to_sdr_timings(0);
+	if (IS_ERR(timings))
+		return PTR_ERR(timings);
+
+	pxa3xx_nand_set_sdr_timing(host, timings);
 	return 0;
 }
 
@@ -1491,32 +1499,6 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
 	kfree(info->data_buff);
 }
 
-static int pxa3xx_nand_sensing(struct pxa3xx_nand_host *host)
-{
-	struct pxa3xx_nand_info *info = host->info_data;
-	struct mtd_info *mtd;
-	struct nand_chip *chip;
-	const struct nand_sdr_timings *timings;
-	int ret;
-
-	mtd = info->host[info->cs]->mtd;
-	chip = mtd->priv;
-
-	/* use the common timing to make a try */
-	timings = onfi_async_timing_mode_to_sdr_timings(0);
-	if (IS_ERR(timings))
-		return PTR_ERR(timings);
-
-	pxa3xx_nand_set_sdr_timing(host, timings);
-
-	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
-	ret = chip->waitfunc(mtd, chip);
-	if (ret & NAND_STATUS_FAIL)
-		return -ENODEV;
-
-	return 0;
-}
-
 static int pxa_ecc_init(struct pxa3xx_nand_info *info,
 			struct nand_ecc_ctrl *ecc,
 			int strength, int ecc_stepsize, int page_size)
@@ -1602,13 +1584,6 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 		ret = pxa3xx_nand_config_ident(info);
 		if (ret)
 			return ret;
-		ret = pxa3xx_nand_sensing(host);
-		if (ret) {
-			dev_info(&info->pdev->dev,
-				 "There is no chip on cs %d!\n",
-				 info->cs);
-			return ret;
-		}
 	}
 
 	if (info->reg_ndcr & NDCR_DWIDTH_M)
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] mtd: pxa3xx_nand: Remove dead code
  2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
                   ` (3 preceding siblings ...)
  2015-11-04 16:13 ` [PATCH 4/6] mtd: pxa3xx_nand: Remove redundant NAND sensing Ezequiel Garcia
@ 2015-11-04 16:13 ` Ezequiel Garcia
  2015-11-04 21:06   ` Robert Jarzmik
  2015-11-04 16:13 ` [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths Ezequiel Garcia
  2015-11-16 23:04 ` [PATCH 0/6] pxa3xx_nand: Assorted changes Brian Norris
  6 siblings, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 16:13 UTC (permalink / raw)
  To: linux-mtd; +Cc: Robert Jarzmik, Antoine Tenart, Brian Norris, Ezequiel Garcia

This macro is not used anymore, so it's just dead code.
Remove it.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/pxa3xx_nand.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 89c02f7057fd..0e6eb6c1f9b3 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -30,11 +30,6 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_mtd.h>
-
-#if defined(CONFIG_ARM) && (defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP))
-#define ARCH_HAS_DMA
-#endif
-
 #include <linux/platform_data/mtd-nand-pxa3xx.h>
 
 #define	CHIP_DELAY_TIMEOUT	msecs_to_jiffies(200)
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths
  2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
                   ` (4 preceding siblings ...)
  2015-11-04 16:13 ` [PATCH 5/6] mtd: pxa3xx_nand: Remove dead code Ezequiel Garcia
@ 2015-11-04 16:13 ` Ezequiel Garcia
  2015-11-04 21:16   ` Robert Jarzmik
  2015-11-16 23:04 ` [PATCH 0/6] pxa3xx_nand: Assorted changes Brian Norris
  6 siblings, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 16:13 UTC (permalink / raw)
  To: linux-mtd; +Cc: Robert Jarzmik, Antoine Tenart, Brian Norris, Ezequiel Garcia

The NAND clock can be disabled on suspend and enabled on resume.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/mtd/nand/pxa3xx_nand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 0e6eb6c1f9b3..13371d6e9ba5 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1936,12 +1936,18 @@ static int pxa3xx_nand_suspend(struct device *dev)
 		return -EAGAIN;
 	}
 
+	clk_disable(info->clk);
 	return 0;
 }
 
 static int pxa3xx_nand_resume(struct device *dev)
 {
 	struct pxa3xx_nand_info *info = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_enable(info->clk);
+	if (ret < 0)
+		return ret;
 
 	/* We don't want to handle interrupt without calling mtd routine */
 	disable_int(info, NDCR_INT_MASK);
-- 
2.5.2

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/6] mtd: pxa3xx_nand: Increase the initial chunk size
  2015-11-04 16:13 ` [PATCH 1/6] mtd: pxa3xx_nand: Increase the initial chunk size Ezequiel Garcia
@ 2015-11-04 20:15   ` Robert Jarzmik
  0 siblings, 0 replies; 16+ messages in thread
From: Robert Jarzmik @ 2015-11-04 20:15 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-mtd, Antoine Tenart, Brian Norris

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> The chunk size represents the size of the data chunks, which
> is used by the controllers that allow to split transfered data.
>
> However, the initial chunk size is used in a non-splitted way,
> during device identification. Therefore, it must be large enough
> for all the NAND commands issued during device identification.
> This includes NAND_CMD_PARAM which was recently changed to
> transfer up to 2048 bytes (for the redundant parameter pages).
>
> Thus, the initial chunk size should be 2048 as well.
Ok.

Does anybody own an "512 bytes per page" chip to see if there is any regression
with this ? My own are 2048 ones, so I cannot test it myself.

Apart from that :
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

--
Robert

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/6] nand: pxa3xx: Simplify pxa3xx_nand_scan
  2015-11-04 16:13 ` [PATCH 3/6] nand: pxa3xx: Simplify pxa3xx_nand_scan Ezequiel Garcia
@ 2015-11-04 20:56   ` Robert Jarzmik
  2015-11-04 21:08     ` Ezequiel Garcia
  0 siblings, 1 reply; 16+ messages in thread
From: Robert Jarzmik @ 2015-11-04 20:56 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-mtd, Antoine Tenart, Brian Norris

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> This commit simplifies the initial configuration performed
> by pxa3xx_nand_scan. No functionality change is intended.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Yes, I like this one very much.

Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

--
Robert

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] mtd: pxa3xx_nand: Remove redundant NAND sensing
  2015-11-04 16:13 ` [PATCH 4/6] mtd: pxa3xx_nand: Remove redundant NAND sensing Ezequiel Garcia
@ 2015-11-04 21:05   ` Robert Jarzmik
  0 siblings, 0 replies; 16+ messages in thread
From: Robert Jarzmik @ 2015-11-04 21:05 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-mtd, Antoine Tenart, Brian Norris

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> Currently, the driver is trying to detect the presence of a chip
> by issuing a RESET command before nand_scan_ident. This seems completely
> redundant, and is also a layering violation as nand_scan_ident is in charge
> of device detection.
>
> This commit removes the RESET command use, and moves the initial
> timing configuration to pxa3xx_nand_config_ident.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
That one looks good to me. I have not taken the time to apply it to see the
overall change, but from a first sight it's ok.

Cheers.

--
Robert

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/6] mtd: pxa3xx_nand: Remove dead code
  2015-11-04 16:13 ` [PATCH 5/6] mtd: pxa3xx_nand: Remove dead code Ezequiel Garcia
@ 2015-11-04 21:06   ` Robert Jarzmik
  0 siblings, 0 replies; 16+ messages in thread
From: Robert Jarzmik @ 2015-11-04 21:06 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-mtd, Antoine Tenart, Brian Norris

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> This macro is not used anymore, so it's just dead code.
> Remove it.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

-- 
Robert

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/6] nand: pxa3xx: Simplify pxa3xx_nand_scan
  2015-11-04 20:56   ` Robert Jarzmik
@ 2015-11-04 21:08     ` Ezequiel Garcia
  0 siblings, 0 replies; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 21:08 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: linux-mtd@lists.infradead.org, Antoine Tenart, Brian Norris

On 4 November 2015 at 17:56, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> This commit simplifies the initial configuration performed
>> by pxa3xx_nand_scan. No functionality change is intended.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Yes, I like this one very much.
>
> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
>

Cool. BTW, The subject is wrong and should be:

"mtd: pxa3xx_nand: Simplify pxa3xx_nand_scan"

-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths
  2015-11-04 16:13 ` [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths Ezequiel Garcia
@ 2015-11-04 21:16   ` Robert Jarzmik
  2015-11-04 21:26     ` Ezequiel Garcia
  0 siblings, 1 reply; 16+ messages in thread
From: Robert Jarzmik @ 2015-11-04 21:16 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-mtd, Antoine Tenart, Brian Norris

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> The NAND clock can be disabled on suspend and enabled on resume.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Won't registers set by firmware (IPL/SPL) be lost if you cut the clock ? For
example, NDREDEL ? And if so, shouldn't they be saved upon suspend and restored
upon resume ?

Cheers.

--
Robert

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths
  2015-11-04 21:16   ` Robert Jarzmik
@ 2015-11-04 21:26     ` Ezequiel Garcia
  2015-11-04 21:31       ` Robert Jarzmik
  0 siblings, 1 reply; 16+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 21:26 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: linux-mtd@lists.infradead.org, Antoine Tenart, Brian Norris

On 4 November 2015 at 18:16, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:
>
>> The NAND clock can be disabled on suspend and enabled on resume.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> Won't registers set by firmware (IPL/SPL) be lost if you cut the clock ? For
> example, NDREDEL ? And if so, shouldn't they be saved upon suspend and restored
> upon resume ?
>

I'd say no, because we are gating the bus clock, not the interface clock.
And in that case, my tests would have failed, and they didn't.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths
  2015-11-04 21:26     ` Ezequiel Garcia
@ 2015-11-04 21:31       ` Robert Jarzmik
  0 siblings, 0 replies; 16+ messages in thread
From: Robert Jarzmik @ 2015-11-04 21:31 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-mtd@lists.infradead.org, Antoine Tenart, Brian Norris

Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes:

> On 4 November 2015 at 18:16, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> I'd say no, because we are gating the bus clock, not the interface clock.
> And in that case, my tests would have failed, and they didn't.

Then:
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

-- 
Robert

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] pxa3xx_nand: Assorted changes
  2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
                   ` (5 preceding siblings ...)
  2015-11-04 16:13 ` [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths Ezequiel Garcia
@ 2015-11-16 23:04 ` Brian Norris
  6 siblings, 0 replies; 16+ messages in thread
From: Brian Norris @ 2015-11-16 23:04 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-mtd, Robert Jarzmik, Antoine Tenart

On Wed, Nov 04, 2015 at 01:13:40PM -0300, Ezequiel Garcia wrote:
> This series contains a number of fixes and cleanups.
> As described below, this series doesn't seem to fix any
> currently known regression. IMO, there's no need to pick
> any of this for v4.4-rc.
> 
> The first patch fixes a timeout found when issuing a CMD_PARAM,
> without using the keep-config option. This timeout happens
> only on NFCv2 (i.e. Armada 370/375/XP) with the keep-config option.
> 
> Since all the Armada 370/375/XP boards currently specify
> keep-config, no user should be affected by the timeout.
> 
> The second patch is a fix for some leftovers issues from the
> recent cleanup series by Antoine. Altough the patch can be
> considered a fix, I'm not aware of any regression so I can't
> argue it should be queued as a fix.
> 
> The rest of the patches are either cleanups or improvements.
> 
> Patch 3 simplifies the code around keep-config and removes
> a silly goto.
> 
> Patch 4 removes the open-coded NAND chip sensing.
> 
> Patch 5 removes an unused macro.
> 
> Path 6 uses enable/disable clk to save power in resume/suspend
> paths.

Pushed all 6 to l2-mtd.git. Thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-11-16 23:05 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-04 16:13 [PATCH 0/6] pxa3xx_nand: Assorted changes Ezequiel Garcia
2015-11-04 16:13 ` [PATCH 1/6] mtd: pxa3xx_nand: Increase the initial chunk size Ezequiel Garcia
2015-11-04 20:15   ` Robert Jarzmik
2015-11-04 16:13 ` [PATCH 2/6] mtd: pxa3xx_nand: Fix initial controller configuration Ezequiel Garcia
2015-11-04 16:13 ` [PATCH 3/6] nand: pxa3xx: Simplify pxa3xx_nand_scan Ezequiel Garcia
2015-11-04 20:56   ` Robert Jarzmik
2015-11-04 21:08     ` Ezequiel Garcia
2015-11-04 16:13 ` [PATCH 4/6] mtd: pxa3xx_nand: Remove redundant NAND sensing Ezequiel Garcia
2015-11-04 21:05   ` Robert Jarzmik
2015-11-04 16:13 ` [PATCH 5/6] mtd: pxa3xx_nand: Remove dead code Ezequiel Garcia
2015-11-04 21:06   ` Robert Jarzmik
2015-11-04 16:13 ` [PATCH 6/6] mtd: pxa3xx_nand: Gate/ungate the NAND clock in suspend/resume paths Ezequiel Garcia
2015-11-04 21:16   ` Robert Jarzmik
2015-11-04 21:26     ` Ezequiel Garcia
2015-11-04 21:31       ` Robert Jarzmik
2015-11-16 23:04 ` [PATCH 0/6] pxa3xx_nand: Assorted changes Brian Norris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).