[PATCH v4 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller

[PATCH v4 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller

[Maxime Ripard]

Hi,

This patch serie enable the NAND support on the Armada 385 Access
Point DB.

In the process, some timeouts were found when we were accessing a
freshly erased NAND page, which turned out to be an issue when
draining the read FIFO where we were not following the datasheet.

This has been fixed with the first patch, with stable CC'd. The second
patch just enables the NAND controller in the DT.

Thanks,
Maxime

Changes from v3:
  - Fixed a typo in the commit log
  - Reworked the FIFO draining function to not poll the RDDREQ
    register on the last 32 bytes chunk, and handle non 32 bytes
    aligned reads

Changes from v2:
  - Read the status bits only every 32 bytes read, and not 32 bits
    like was done before.
  - Changed the timeout routine code not use the jiffies that won't
    change in an interrupt context.

Changes from v1:
  - Added a timeout to the busy waiting loop for RDDREQ

Maxime Ripard (2):
  mtd: nand: pxa3xx: Fix PIO FIFO draining
  ARM: mvebu: a385-db-ap: Enable the NAND

 arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++
 drivers/mtd/nand/pxa3xx_nand.c         | 48 +++++++++++++++++++++++++++++-----
 2 files changed, 55 insertions(+), 6 deletions(-)

-- 
2.3.0

[PATCH v4 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller

[Maxime Ripard]

Hi,

This patch serie enable the NAND support on the Armada 385 Access
Point DB.

In the process, some timeouts were found when we were accessing a
freshly erased NAND page, which turned out to be an issue when
draining the read FIFO where we were not following the datasheet.

This has been fixed with the first patch, with stable CC'd. The second
patch just enables the NAND controller in the DT.

Thanks,
Maxime

Changes from v3:
  - Fixed a typo in the commit log
  - Reworked the FIFO draining function to not poll the RDDREQ
    register on the last 32 bytes chunk, and handle non 32 bytes
    aligned reads

Changes from v2:
  - Read the status bits only every 32 bytes read, and not 32 bits
    like was done before.
  - Changed the timeout routine code not use the jiffies that won't
    change in an interrupt context.

Changes from v1:
  - Added a timeout to the busy waiting loop for RDDREQ

Maxime Ripard (2):
  mtd: nand: pxa3xx: Fix PIO FIFO draining
  ARM: mvebu: a385-db-ap: Enable the NAND

 arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++
 drivers/mtd/nand/pxa3xx_nand.c         | 48 +++++++++++++++++++++++++++++-----
 2 files changed, 55 insertions(+), 6 deletions(-)

-- 
2.3.0

[PATCH v4 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller

[Maxime Ripard]

Hi,

This patch serie enable the NAND support on the Armada 385 Access
Point DB.

In the process, some timeouts were found when we were accessing a
freshly erased NAND page, which turned out to be an issue when
draining the read FIFO where we were not following the datasheet.

This has been fixed with the first patch, with stable CC'd. The second
patch just enables the NAND controller in the DT.

Thanks,
Maxime

Changes from v3:
  - Fixed a typo in the commit log
  - Reworked the FIFO draining function to not poll the RDDREQ
    register on the last 32 bytes chunk, and handle non 32 bytes
    aligned reads

Changes from v2:
  - Read the status bits only every 32 bytes read, and not 32 bits
    like was done before.
  - Changed the timeout routine code not use the jiffies that won't
    change in an interrupt context.

Changes from v1:
  - Added a timeout to the busy waiting loop for RDDREQ

Maxime Ripard (2):
  mtd: nand: pxa3xx: Fix PIO FIFO draining
  ARM: mvebu: a385-db-ap: Enable the NAND

 arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++
 drivers/mtd/nand/pxa3xx_nand.c         | 48 +++++++++++++++++++++++++++++-----
 2 files changed, 55 insertions(+), 6 deletions(-)

-- 
2.3.0

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Maxime Ripard]

The NDDB register holds the data that are needed by the read and write
commands.

However, during a read PIO access, the datasheet specifies that after each 32
bytes read in that register, when BCH is enabled, we have to make sure that the
RDDREQ bit is set in the NDSR register.

This fixes an issue that was seen on the Armada 385, and presumably other mvebu
SoCs, when a read on a newly erased page would end up in the driver reporting a
timeout from the NAND.

Cc: <stable@vger.kernel.org> # v3.14
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 96b0b1d27df1..bc677362bc73 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 	nand_writel(info, NDCR, ndcr | int_mask);
 }
 
+static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
+{
+	if (info->ecc_bch) {
+		int timeout;
+
+		/*
+		 * According to the datasheet, when reading from NDDB
+		 * with BCH enabled, after each 32 bytes reads, we
+		 * have to make sure that the NDSR.RDDREQ bit is set.
+		 *
+		 * Drain the FIFO 8 32 bits reads at a time, and skip
+		 * the polling on the last read.
+		 */
+		while (len > 8) {
+			__raw_readsl(info->mmio_base + NDDB, data, 8);
+
+			for (timeout = 0;
+			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
+			     timeout++) {
+				if (timeout >= 5) {
+					dev_err(&info->pdev->dev,
+						"Timeout on RDDREQ while draining the FIFO\n");
+					return;
+				}
+
+				mdelay(1);
+			}
+
+			data += 32;
+			len -= 8;
+		}
+	}
+
+	__raw_readsl(info->mmio_base + NDDB, data, len);
+}
+
 static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
 	unsigned int do_bytes = min(info->data_size, info->chunk_size);
@@ -496,14 +532,14 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
 				      DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	case STATE_PIO_READING:
-		__raw_readsl(info->mmio_base + NDDB,
-			     info->data_buff + info->data_buff_pos,
-			     DIV_ROUND_UP(do_bytes, 4));
+		drain_fifo(info,
+			   info->data_buff + info->data_buff_pos,
+			   DIV_ROUND_UP(do_bytes, 4));
 
 		if (info->oob_size > 0)
-			__raw_readsl(info->mmio_base + NDDB,
-				     info->oob_buff + info->oob_buff_pos,
-				     DIV_ROUND_UP(info->oob_size, 4));
+			drain_fifo(info,
+				   info->oob_buff + info->oob_buff_pos,
+				   DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	default:
 		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,
-- 
2.3.0

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Maxime Ripard]

The NDDB register holds the data that are needed by the read and write
commands.

However, during a read PIO access, the datasheet specifies that after each 32
bytes read in that register, when BCH is enabled, we have to make sure that the
RDDREQ bit is set in the NDSR register.

This fixes an issue that was seen on the Armada 385, and presumably other mvebu
SoCs, when a read on a newly erased page would end up in the driver reporting a
timeout from the NAND.

Cc: <stable@vger.kernel.org> # v3.14
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 96b0b1d27df1..bc677362bc73 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 	nand_writel(info, NDCR, ndcr | int_mask);
 }
 
+static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
+{
+	if (info->ecc_bch) {
+		int timeout;
+
+		/*
+		 * According to the datasheet, when reading from NDDB
+		 * with BCH enabled, after each 32 bytes reads, we
+		 * have to make sure that the NDSR.RDDREQ bit is set.
+		 *
+		 * Drain the FIFO 8 32 bits reads at a time, and skip
+		 * the polling on the last read.
+		 */
+		while (len > 8) {
+			__raw_readsl(info->mmio_base + NDDB, data, 8);
+
+			for (timeout = 0;
+			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
+			     timeout++) {
+				if (timeout >= 5) {
+					dev_err(&info->pdev->dev,
+						"Timeout on RDDREQ while draining the FIFO\n");
+					return;
+				}
+
+				mdelay(1);
+			}
+
+			data += 32;
+			len -= 8;
+		}
+	}
+
+	__raw_readsl(info->mmio_base + NDDB, data, len);
+}
+
 static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
 	unsigned int do_bytes = min(info->data_size, info->chunk_size);
@@ -496,14 +532,14 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
 				      DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	case STATE_PIO_READING:
-		__raw_readsl(info->mmio_base + NDDB,
-			     info->data_buff + info->data_buff_pos,
-			     DIV_ROUND_UP(do_bytes, 4));
+		drain_fifo(info,
+			   info->data_buff + info->data_buff_pos,
+			   DIV_ROUND_UP(do_bytes, 4));
 
 		if (info->oob_size > 0)
-			__raw_readsl(info->mmio_base + NDDB,
-				     info->oob_buff + info->oob_buff_pos,
-				     DIV_ROUND_UP(info->oob_size, 4));
+			drain_fifo(info,
+				   info->oob_buff + info->oob_buff_pos,
+				   DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	default:
 		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,
-- 
2.3.0

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Maxime Ripard]

The NDDB register holds the data that are needed by the read and write
commands.

However, during a read PIO access, the datasheet specifies that after each 32
bytes read in that register, when BCH is enabled, we have to make sure that the
RDDREQ bit is set in the NDSR register.

This fixes an issue that was seen on the Armada 385, and presumably other mvebu
SoCs, when a read on a newly erased page would end up in the driver reporting a
timeout from the NAND.

Cc: <stable@vger.kernel.org> # v3.14
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 96b0b1d27df1..bc677362bc73 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 	nand_writel(info, NDCR, ndcr | int_mask);
 }
 
+static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
+{
+	if (info->ecc_bch) {
+		int timeout;
+
+		/*
+		 * According to the datasheet, when reading from NDDB
+		 * with BCH enabled, after each 32 bytes reads, we
+		 * have to make sure that the NDSR.RDDREQ bit is set.
+		 *
+		 * Drain the FIFO 8 32 bits reads at a time, and skip
+		 * the polling on the last read.
+		 */
+		while (len > 8) {
+			__raw_readsl(info->mmio_base + NDDB, data, 8);
+
+			for (timeout = 0;
+			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
+			     timeout++) {
+				if (timeout >= 5) {
+					dev_err(&info->pdev->dev,
+						"Timeout on RDDREQ while draining the FIFO\n");
+					return;
+				}
+
+				mdelay(1);
+			}
+
+			data += 32;
+			len -= 8;
+		}
+	}
+
+	__raw_readsl(info->mmio_base + NDDB, data, len);
+}
+
 static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
 	unsigned int do_bytes = min(info->data_size, info->chunk_size);
@@ -496,14 +532,14 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
 				      DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	case STATE_PIO_READING:
-		__raw_readsl(info->mmio_base + NDDB,
-			     info->data_buff + info->data_buff_pos,
-			     DIV_ROUND_UP(do_bytes, 4));
+		drain_fifo(info,
+			   info->data_buff + info->data_buff_pos,
+			   DIV_ROUND_UP(do_bytes, 4));
 
 		if (info->oob_size > 0)
-			__raw_readsl(info->mmio_base + NDDB,
-				     info->oob_buff + info->oob_buff_pos,
-				     DIV_ROUND_UP(info->oob_size, 4));
+			drain_fifo(info,
+				   info->oob_buff + info->oob_buff_pos,
+				   DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	default:
 		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,
-- 
2.3.0

[PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Maxime Ripard]

The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
Micron as its main storage. Enable it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-db-ap.dts b/arch/arm/boot/dts/armada-385-db-ap.dts
index b891b4c897f5..ee648fb19075 100644
--- a/arch/arm/boot/dts/armada-385-db-ap.dts
+++ b/arch/arm/boot/dts/armada-385-db-ap.dts
@@ -130,6 +130,19 @@
 				phy-mode = "rgmii-id";
 			};
 
+			nfc: flash@d0000 {
+				status = "okay";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				num-cs = <1>;
+				nand-ecc-strength = <4>;
+				nand-ecc-step-size = <512>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+			};
+
 			usb3@f0000 {
 				status = "okay";
 				usb-phy = <&usb3_phy>;
-- 
2.3.0

[PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Maxime Ripard]

The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
Micron as its main storage. Enable it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-db-ap.dts b/arch/arm/boot/dts/armada-385-db-ap.dts
index b891b4c897f5..ee648fb19075 100644
--- a/arch/arm/boot/dts/armada-385-db-ap.dts
+++ b/arch/arm/boot/dts/armada-385-db-ap.dts
@@ -130,6 +130,19 @@
 				phy-mode = "rgmii-id";
 			};
 
+			nfc: flash@d0000 {
+				status = "okay";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				num-cs = <1>;
+				nand-ecc-strength = <4>;
+				nand-ecc-step-size = <512>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+			};
+
 			usb3@f0000 {
 				status = "okay";
 				usb-phy = <&usb3_phy>;
-- 
2.3.0

[PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Maxime Ripard]

The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
Micron as its main storage. Enable it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-db-ap.dts b/arch/arm/boot/dts/armada-385-db-ap.dts
index b891b4c897f5..ee648fb19075 100644
--- a/arch/arm/boot/dts/armada-385-db-ap.dts
+++ b/arch/arm/boot/dts/armada-385-db-ap.dts
@@ -130,6 +130,19 @@
 				phy-mode = "rgmii-id";
 			};
 
+			nfc: flash at d0000 {
+				status = "okay";
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				num-cs = <1>;
+				nand-ecc-strength = <4>;
+				nand-ecc-step-size = <512>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+			};
+
 			usb3 at f0000 {
 				status = "okay";
 				usb-phy = <&usb3_phy>;
-- 
2.3.0

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Boris Brezillon]

On Wed, 18 Feb 2015 11:32:07 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

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

> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 96b0b1d27df1..bc677362bc73 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
>  	nand_writel(info, NDCR, ndcr | int_mask);
>  }
>  
> +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> +{
> +	if (info->ecc_bch) {
> +		int timeout;
> +
> +		/*
> +		 * According to the datasheet, when reading from NDDB
> +		 * with BCH enabled, after each 32 bytes reads, we
> +		 * have to make sure that the NDSR.RDDREQ bit is set.
> +		 *
> +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> +		 * the polling on the last read.
> +		 */
> +		while (len > 8) {
> +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> +
> +			for (timeout = 0;
> +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> +			     timeout++) {
> +				if (timeout >= 5) {
> +					dev_err(&info->pdev->dev,
> +						"Timeout on RDDREQ while draining the FIFO\n");
> +					return;
> +				}
> +
> +				mdelay(1);
> +			}
> +
> +			data += 32;
> +			len -= 8;
> +		}
> +	}
> +
> +	__raw_readsl(info->mmio_base + NDDB, data, len);
> +}
> +
>  static void handle_data_pio(struct pxa3xx_nand_info *info)
>  {
>  	unsigned int do_bytes = min(info->data_size, info->chunk_size);
> @@ -496,14 +532,14 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
>  				      DIV_ROUND_UP(info->oob_size, 4));
>  		break;
>  	case STATE_PIO_READING:
> -		__raw_readsl(info->mmio_base + NDDB,
> -			     info->data_buff + info->data_buff_pos,
> -			     DIV_ROUND_UP(do_bytes, 4));
> +		drain_fifo(info,
> +			   info->data_buff + info->data_buff_pos,
> +			   DIV_ROUND_UP(do_bytes, 4));
>  
>  		if (info->oob_size > 0)
> -			__raw_readsl(info->mmio_base + NDDB,
> -				     info->oob_buff + info->oob_buff_pos,
> -				     DIV_ROUND_UP(info->oob_size, 4));
> +			drain_fifo(info,
> +				   info->oob_buff + info->oob_buff_pos,
> +				   DIV_ROUND_UP(info->oob_size, 4));
>  		break;
>  	default:
>  		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Boris Brezillon]

On Wed, 18 Feb 2015 11:32:07 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

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

> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 96b0b1d27df1..bc677362bc73 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
>  	nand_writel(info, NDCR, ndcr | int_mask);
>  }
>  
> +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> +{
> +	if (info->ecc_bch) {
> +		int timeout;
> +
> +		/*
> +		 * According to the datasheet, when reading from NDDB
> +		 * with BCH enabled, after each 32 bytes reads, we
> +		 * have to make sure that the NDSR.RDDREQ bit is set.
> +		 *
> +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> +		 * the polling on the last read.
> +		 */
> +		while (len > 8) {
> +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> +
> +			for (timeout = 0;
> +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> +			     timeout++) {
> +				if (timeout >= 5) {
> +					dev_err(&info->pdev->dev,
> +						"Timeout on RDDREQ while draining the FIFO\n");
> +					return;
> +				}
> +
> +				mdelay(1);
> +			}
> +
> +			data += 32;
> +			len -= 8;
> +		}
> +	}
> +
> +	__raw_readsl(info->mmio_base + NDDB, data, len);
> +}
> +
>  static void handle_data_pio(struct pxa3xx_nand_info *info)
>  {
>  	unsigned int do_bytes = min(info->data_size, info->chunk_size);
> @@ -496,14 +532,14 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
>  				      DIV_ROUND_UP(info->oob_size, 4));
>  		break;
>  	case STATE_PIO_READING:
> -		__raw_readsl(info->mmio_base + NDDB,
> -			     info->data_buff + info->data_buff_pos,
> -			     DIV_ROUND_UP(do_bytes, 4));
> +		drain_fifo(info,
> +			   info->data_buff + info->data_buff_pos,
> +			   DIV_ROUND_UP(do_bytes, 4));
>  
>  		if (info->oob_size > 0)
> -			__raw_readsl(info->mmio_base + NDDB,
> -				     info->oob_buff + info->oob_buff_pos,
> -				     DIV_ROUND_UP(info->oob_size, 4));
> +			drain_fifo(info,
> +				   info->oob_buff + info->oob_buff_pos,
> +				   DIV_ROUND_UP(info->oob_size, 4));
>  		break;
>  	default:
>  		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Boris Brezillon]

On Wed, 18 Feb 2015 11:32:07 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

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

> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 96b0b1d27df1..bc677362bc73 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
>  	nand_writel(info, NDCR, ndcr | int_mask);
>  }
>  
> +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> +{
> +	if (info->ecc_bch) {
> +		int timeout;
> +
> +		/*
> +		 * According to the datasheet, when reading from NDDB
> +		 * with BCH enabled, after each 32 bytes reads, we
> +		 * have to make sure that the NDSR.RDDREQ bit is set.
> +		 *
> +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> +		 * the polling on the last read.
> +		 */
> +		while (len > 8) {
> +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> +
> +			for (timeout = 0;
> +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> +			     timeout++) {
> +				if (timeout >= 5) {
> +					dev_err(&info->pdev->dev,
> +						"Timeout on RDDREQ while draining the FIFO\n");
> +					return;
> +				}
> +
> +				mdelay(1);
> +			}
> +
> +			data += 32;
> +			len -= 8;
> +		}
> +	}
> +
> +	__raw_readsl(info->mmio_base + NDDB, data, len);
> +}
> +
>  static void handle_data_pio(struct pxa3xx_nand_info *info)
>  {
>  	unsigned int do_bytes = min(info->data_size, info->chunk_size);
> @@ -496,14 +532,14 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
>  				      DIV_ROUND_UP(info->oob_size, 4));
>  		break;
>  	case STATE_PIO_READING:
> -		__raw_readsl(info->mmio_base + NDDB,
> -			     info->data_buff + info->data_buff_pos,
> -			     DIV_ROUND_UP(do_bytes, 4));
> +		drain_fifo(info,
> +			   info->data_buff + info->data_buff_pos,
> +			   DIV_ROUND_UP(do_bytes, 4));
>  
>  		if (info->oob_size > 0)
> -			__raw_readsl(info->mmio_base + NDDB,
> -				     info->oob_buff + info->oob_buff_pos,
> -				     DIV_ROUND_UP(info->oob_size, 4));
> +			drain_fifo(info,
> +				   info->oob_buff + info->oob_buff_pos,
> +				   DIV_ROUND_UP(info->oob_size, 4));
>  		break;
>  	default:
>  		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

On 02/18/2015 07:32 AM, Maxime Ripard wrote:
> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 96b0b1d27df1..bc677362bc73 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
>  	nand_writel(info, NDCR, ndcr | int_mask);
>  }
>  
> +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> +{
> +	if (info->ecc_bch) {
> +		int timeout;
> +
> +		/*
> +		 * According to the datasheet, when reading from NDDB
> +		 * with BCH enabled, after each 32 bytes reads, we
> +		 * have to make sure that the NDSR.RDDREQ bit is set.
> +		 *
> +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> +		 * the polling on the last read.
> +		 */
> +		while (len > 8) {
> +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> +
> +			for (timeout = 0;
> +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> +			     timeout++) {
> +				if (timeout >= 5) {
> +					dev_err(&info->pdev->dev,
> +						"Timeout on RDDREQ while draining the FIFO\n");
> +					return;
> +				}
> +
> +				mdelay(1);

This is probably a stupid nit.. but here it goes is it any difference if
udelay is used here?

Does this makes anything better/worse?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

On 02/18/2015 07:32 AM, Maxime Ripard wrote:
> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 96b0b1d27df1..bc677362bc73 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
>  	nand_writel(info, NDCR, ndcr | int_mask);
>  }
>  
> +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> +{
> +	if (info->ecc_bch) {
> +		int timeout;
> +
> +		/*
> +		 * According to the datasheet, when reading from NDDB
> +		 * with BCH enabled, after each 32 bytes reads, we
> +		 * have to make sure that the NDSR.RDDREQ bit is set.
> +		 *
> +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> +		 * the polling on the last read.
> +		 */
> +		while (len > 8) {
> +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> +
> +			for (timeout = 0;
> +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> +			     timeout++) {
> +				if (timeout >= 5) {
> +					dev_err(&info->pdev->dev,
> +						"Timeout on RDDREQ while draining the FIFO\n");
> +					return;
> +				}
> +
> +				mdelay(1);

This is probably a stupid nit.. but here it goes is it any difference if
udelay is used here?

Does this makes anything better/worse?
-- 
Ezequiel Garc�a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

On 02/18/2015 07:32 AM, Maxime Ripard wrote:
> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 96b0b1d27df1..bc677362bc73 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
>  	nand_writel(info, NDCR, ndcr | int_mask);
>  }
>  
> +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> +{
> +	if (info->ecc_bch) {
> +		int timeout;
> +
> +		/*
> +		 * According to the datasheet, when reading from NDDB
> +		 * with BCH enabled, after each 32 bytes reads, we
> +		 * have to make sure that the NDSR.RDDREQ bit is set.
> +		 *
> +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> +		 * the polling on the last read.
> +		 */
> +		while (len > 8) {
> +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> +
> +			for (timeout = 0;
> +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> +			     timeout++) {
> +				if (timeout >= 5) {
> +					dev_err(&info->pdev->dev,
> +						"Timeout on RDDREQ while draining the FIFO\n");
> +					return;
> +				}
> +
> +				mdelay(1);

This is probably a stupid nit.. but here it goes is it any difference if
udelay is used here?

Does this makes anything better/worse?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

On 02/18/2015 07:32 AM, Maxime Ripard wrote:
> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 96b0b1d27df1..bc677362bc73 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
>  	nand_writel(info, NDCR, ndcr | int_mask);
>  }
>  
> +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> +{
> +	if (info->ecc_bch) {
> +		int timeout;
> +
> +		/*
> +		 * According to the datasheet, when reading from NDDB
> +		 * with BCH enabled, after each 32 bytes reads, we
> +		 * have to make sure that the NDSR.RDDREQ bit is set.
> +		 *
> +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> +		 * the polling on the last read.
> +		 */
> +		while (len > 8) {
> +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> +
> +			for (timeout = 0;
> +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> +			     timeout++) {
> +				if (timeout >= 5) {
> +					dev_err(&info->pdev->dev,
> +						"Timeout on RDDREQ while draining the FIFO\n");
> +					return;
> +				}
> +
> +				mdelay(1);

This is probably a stupid nit.. but here it goes is it any difference if
udelay is used here?

Does this makes anything better/worse?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]

On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
> > The NDDB register holds the data that are needed by the read and write
> > commands.
> > 
> > However, during a read PIO access, the datasheet specifies that after each 32
> > bytes read in that register, when BCH is enabled, we have to make sure that the
> > RDDREQ bit is set in the NDSR register.
> > 
> > This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> > SoCs, when a read on a newly erased page would end up in the driver reporting a
> > timeout from the NAND.
> > 
> > Cc: <stable@vger.kernel.org> # v3.14
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 42 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index 96b0b1d27df1..bc677362bc73 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
> >  	nand_writel(info, NDCR, ndcr | int_mask);
> >  }
> >  
> > +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> > +{
> > +	if (info->ecc_bch) {
> > +		int timeout;
> > +
> > +		/*
> > +		 * According to the datasheet, when reading from NDDB
> > +		 * with BCH enabled, after each 32 bytes reads, we
> > +		 * have to make sure that the NDSR.RDDREQ bit is set.
> > +		 *
> > +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> > +		 * the polling on the last read.
> > +		 */
> > +		while (len > 8) {
> > +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> > +
> > +			for (timeout = 0;
> > +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> > +			     timeout++) {
> > +				if (timeout >= 5) {
> > +					dev_err(&info->pdev->dev,
> > +						"Timeout on RDDREQ while draining the FIFO\n");
> > +					return;
> > +				}
> > +
> > +				mdelay(1);
> 
> This is probably a stupid nit.. but here it goes is it any
> difference if udelay is used here?
> 
> Does this makes anything better/worse?

It doesn't make any difference. On the board I've been using, we never
hit the delay.

So I really don't care about the number of retries and the sleep
behind them. I made these numbers up, feel free to come up with others
if it makes you more comfortable, but could we settle this?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]

On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
> > The NDDB register holds the data that are needed by the read and write
> > commands.
> > 
> > However, during a read PIO access, the datasheet specifies that after each 32
> > bytes read in that register, when BCH is enabled, we have to make sure that the
> > RDDREQ bit is set in the NDSR register.
> > 
> > This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> > SoCs, when a read on a newly erased page would end up in the driver reporting a
> > timeout from the NAND.
> > 
> > Cc: <stable@vger.kernel.org> # v3.14
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 42 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index 96b0b1d27df1..bc677362bc73 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
> >  	nand_writel(info, NDCR, ndcr | int_mask);
> >  }
> >  
> > +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> > +{
> > +	if (info->ecc_bch) {
> > +		int timeout;
> > +
> > +		/*
> > +		 * According to the datasheet, when reading from NDDB
> > +		 * with BCH enabled, after each 32 bytes reads, we
> > +		 * have to make sure that the NDSR.RDDREQ bit is set.
> > +		 *
> > +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> > +		 * the polling on the last read.
> > +		 */
> > +		while (len > 8) {
> > +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> > +
> > +			for (timeout = 0;
> > +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> > +			     timeout++) {
> > +				if (timeout >= 5) {
> > +					dev_err(&info->pdev->dev,
> > +						"Timeout on RDDREQ while draining the FIFO\n");
> > +					return;
> > +				}
> > +
> > +				mdelay(1);
> 
> This is probably a stupid nit.. but here it goes is it any
> difference if udelay is used here?
> 
> Does this makes anything better/worse?

It doesn't make any difference. On the board I've been using, we never
hit the delay.

So I really don't care about the number of retries and the sleep
behind them. I made these numbers up, feel free to come up with others
if it makes you more comfortable, but could we settle this?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Maxime Ripard]

On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
> > The NDDB register holds the data that are needed by the read and write
> > commands.
> > 
> > However, during a read PIO access, the datasheet specifies that after each 32
> > bytes read in that register, when BCH is enabled, we have to make sure that the
> > RDDREQ bit is set in the NDSR register.
> > 
> > This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> > SoCs, when a read on a newly erased page would end up in the driver reporting a
> > timeout from the NAND.
> > 
> > Cc: <stable@vger.kernel.org> # v3.14
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/mtd/nand/pxa3xx_nand.c | 48 ++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 42 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index 96b0b1d27df1..bc677362bc73 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -480,6 +480,42 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
> >  	nand_writel(info, NDCR, ndcr | int_mask);
> >  }
> >  
> > +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
> > +{
> > +	if (info->ecc_bch) {
> > +		int timeout;
> > +
> > +		/*
> > +		 * According to the datasheet, when reading from NDDB
> > +		 * with BCH enabled, after each 32 bytes reads, we
> > +		 * have to make sure that the NDSR.RDDREQ bit is set.
> > +		 *
> > +		 * Drain the FIFO 8 32 bits reads at a time, and skip
> > +		 * the polling on the last read.
> > +		 */
> > +		while (len > 8) {
> > +			__raw_readsl(info->mmio_base + NDDB, data, 8);
> > +
> > +			for (timeout = 0;
> > +			     !(nand_readl(info, NDSR) & NDSR_RDDREQ);
> > +			     timeout++) {
> > +				if (timeout >= 5) {
> > +					dev_err(&info->pdev->dev,
> > +						"Timeout on RDDREQ while draining the FIFO\n");
> > +					return;
> > +				}
> > +
> > +				mdelay(1);
> 
> This is probably a stupid nit.. but here it goes is it any
> difference if udelay is used here?
> 
> Does this makes anything better/worse?

It doesn't make any difference. On the board I've been using, we never
hit the delay.

So I really don't care about the number of retries and the sleep
behind them. I made these numbers up, feel free to come up with others
if it makes you more comfortable, but could we settle this?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150218/b2ce9e4e/attachment.sig>

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 02/18/2015 11:01 AM, Maxime Ripard wrote:
> On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
>> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
>>> The NDDB register holds the data that are needed by the read
>>> and write commands.
>>> 
>>> However, during a read PIO access, the datasheet specifies that
>>> after each 32 bytes read in that register, when BCH is enabled,
>>> we have to make sure that the RDDREQ bit is set in the NDSR
>>> register.
>>> 
>>> This fixes an issue that was seen on the Armada 385, and
>>> presumably other mvebu SoCs, when a read on a newly erased page
>>> would end up in the driver reporting a timeout from the NAND.
>>> 
>>> Cc: <stable@vger.kernel.org> # v3.14 Signed-off-by: Maxime
>>> Ripard <maxime.ripard@free-electrons.com> --- 
>>> drivers/mtd/nand/pxa3xx_nand.c | 48
>>> ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42
>>> insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>>> b/drivers/mtd/nand/pxa3xx_nand.c index
>>> 96b0b1d27df1..bc677362bc73 100644 ---
>>> a/drivers/mtd/nand/pxa3xx_nand.c +++
>>> b/drivers/mtd/nand/pxa3xx_nand.c @@ -480,6 +480,42 @@ static
>>> void disable_int(struct pxa3xx_nand_info *info, uint32_t
>>> int_mask) nand_writel(info, NDCR, ndcr | int_mask); }
>>> 
>>> +static void drain_fifo(struct pxa3xx_nand_info *info, void
>>> *data, int len) +{ +	if (info->ecc_bch) { +		int timeout; + +
>>> /* +		 * According to the datasheet, when reading from NDDB +
>>> * with BCH enabled, after each 32 bytes reads, we +		 * have to
>>> make sure that the NDSR.RDDREQ bit is set. +		 * +		 * Drain
>>> the FIFO 8 32 bits reads at a time, and skip +		 * the polling
>>> on the last read. +		 */ +		while (len > 8) { +
>>> __raw_readsl(info->mmio_base + NDDB, data, 8); + +			for
>>> (timeout = 0; +			     !(nand_readl(info, NDSR) &
>>> NDSR_RDDREQ); +			     timeout++) { +				if (timeout >= 5) { +
>>> dev_err(&info->pdev->dev, +						"Timeout on RDDREQ while
>>> draining the FIFO\n"); +					return; +				} + +				mdelay(1);
>> 
>> This is probably a stupid nit.. but here it goes is it any 
>> difference if udelay is used here?
>> 
>> Does this makes anything better/worse?
> 
> It doesn't make any difference. On the board I've been using, we
> never hit the delay.
> 
> So I really don't care about the number of retries and the sleep 
> behind them. I made these numbers up, feel free to come up with
> others if it makes you more comfortable, but could we settle this?
> 

OK, let's stop the bikeshedding. For both patches:

Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
- -- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJU5JxaAAoJEIOKbhOEIHKi9HYP/jxU75zI6QjNk1yGL7Y3qwEy
M2UYrepXTwgRVRxAIN3nhGqERuBVOJCIVKlb3CUSiq9auNAO8rsRs6JTAossV781
LTUniAA0nIBFbTn/k2Wc6yGQizSy7iJRXu73MrLJrcSFO8JxFFqu04V1EYuZbh5s
fuVpeLJEX8Gfy6gj85ybFs7+wkD/XnENKlnDzD6y/n4ewBC3yDPLNh436hZpEVDO
ky8lYjGPHMQs0yEDcp1rfFejLAmxJ4SY6hVSKAxq3/Bn58S4y3Pgkm4cJtP8nHYe
UN4q9UfOBIHWrQIVbBlViK//n3zyEtaPojDSKUiSvI2+Hmz9+eortlYEvpN1HCP3
Xqy1gzFto9FpP4Wp3KpyF609JNVUeQxAsUQZMXM6yaH2oz35szZhBq2xlIpsyo3C
BDbjaYKFk3hVg+jAE0iitZW8BiZS+WZ/pzX4A8rwQBSTMcbrLTuRV611R4E/nEBL
sfCwDWc1gxDDiM8pMJBGC97gwtHEibJqxES9y+T3LrhxtqPl1kUczHFDPgd3uoVw
fT71PsZBtGUeOu3ysymNPc+mF9b9G9KRHYhSiOjnEIle9yvXh6kaGWv93ZM3RCUG
JASv01+gqb+Bz5ZU3MMU+xjHxeoWBq7KfSWcshEHpD8QCuiZzNZ3yt0dZaYXao+M
YsLlm5s62fDVILb2Q3+d
=MA8V
-----END PGP SIGNATURE-----

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 02/18/2015 11:01 AM, Maxime Ripard wrote:
> On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
>> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
>>> The NDDB register holds the data that are needed by the read
>>> and write commands.
>>> 
>>> However, during a read PIO access, the datasheet specifies that
>>> after each 32 bytes read in that register, when BCH is enabled,
>>> we have to make sure that the RDDREQ bit is set in the NDSR
>>> register.
>>> 
>>> This fixes an issue that was seen on the Armada 385, and
>>> presumably other mvebu SoCs, when a read on a newly erased page
>>> would end up in the driver reporting a timeout from the NAND.
>>> 
>>> Cc: <stable@vger.kernel.org> # v3.14 Signed-off-by: Maxime
>>> Ripard <maxime.ripard@free-electrons.com> --- 
>>> drivers/mtd/nand/pxa3xx_nand.c | 48
>>> ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42
>>> insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>>> b/drivers/mtd/nand/pxa3xx_nand.c index
>>> 96b0b1d27df1..bc677362bc73 100644 ---
>>> a/drivers/mtd/nand/pxa3xx_nand.c +++
>>> b/drivers/mtd/nand/pxa3xx_nand.c @@ -480,6 +480,42 @@ static
>>> void disable_int(struct pxa3xx_nand_info *info, uint32_t
>>> int_mask) nand_writel(info, NDCR, ndcr | int_mask); }
>>> 
>>> +static void drain_fifo(struct pxa3xx_nand_info *info, void
>>> *data, int len) +{ +	if (info->ecc_bch) { +		int timeout; + +
>>> /* +		 * According to the datasheet, when reading from NDDB +
>>> * with BCH enabled, after each 32 bytes reads, we +		 * have to
>>> make sure that the NDSR.RDDREQ bit is set. +		 * +		 * Drain
>>> the FIFO 8 32 bits reads at a time, and skip +		 * the polling
>>> on the last read. +		 */ +		while (len > 8) { +
>>> __raw_readsl(info->mmio_base + NDDB, data, 8); + +			for
>>> (timeout = 0; +			     !(nand_readl(info, NDSR) &
>>> NDSR_RDDREQ); +			     timeout++) { +				if (timeout >= 5) { +
>>> dev_err(&info->pdev->dev, +						"Timeout on RDDREQ while
>>> draining the FIFO\n"); +					return; +				} + +				mdelay(1);
>> 
>> This is probably a stupid nit.. but here it goes is it any 
>> difference if udelay is used here?
>> 
>> Does this makes anything better/worse?
> 
> It doesn't make any difference. On the board I've been using, we
> never hit the delay.
> 
> So I really don't care about the number of retries and the sleep 
> behind them. I made these numbers up, feel free to come up with
> others if it makes you more comfortable, but could we settle this?
> 

OK, let's stop the bikeshedding. For both patches:

Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
- -- 
Ezequiel Garc�a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJU5JxaAAoJEIOKbhOEIHKi9HYP/jxU75zI6QjNk1yGL7Y3qwEy
M2UYrepXTwgRVRxAIN3nhGqERuBVOJCIVKlb3CUSiq9auNAO8rsRs6JTAossV781
LTUniAA0nIBFbTn/k2Wc6yGQizSy7iJRXu73MrLJrcSFO8JxFFqu04V1EYuZbh5s
fuVpeLJEX8Gfy6gj85ybFs7+wkD/XnENKlnDzD6y/n4ewBC3yDPLNh436hZpEVDO
ky8lYjGPHMQs0yEDcp1rfFejLAmxJ4SY6hVSKAxq3/Bn58S4y3Pgkm4cJtP8nHYe
UN4q9UfOBIHWrQIVbBlViK//n3zyEtaPojDSKUiSvI2+Hmz9+eortlYEvpN1HCP3
Xqy1gzFto9FpP4Wp3KpyF609JNVUeQxAsUQZMXM6yaH2oz35szZhBq2xlIpsyo3C
BDbjaYKFk3hVg+jAE0iitZW8BiZS+WZ/pzX4A8rwQBSTMcbrLTuRV611R4E/nEBL
sfCwDWc1gxDDiM8pMJBGC97gwtHEibJqxES9y+T3LrhxtqPl1kUczHFDPgd3uoVw
fT71PsZBtGUeOu3ysymNPc+mF9b9G9KRHYhSiOjnEIle9yvXh6kaGWv93ZM3RCUG
JASv01+gqb+Bz5ZU3MMU+xjHxeoWBq7KfSWcshEHpD8QCuiZzNZ3yt0dZaYXao+M
YsLlm5s62fDVILb2Q3+d
=MA8V
-----END PGP SIGNATURE-----

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 02/18/2015 11:01 AM, Maxime Ripard wrote:
> On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
>> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
>>> The NDDB register holds the data that are needed by the read
>>> and write commands.
>>> 
>>> However, during a read PIO access, the datasheet specifies that
>>> after each 32 bytes read in that register, when BCH is enabled,
>>> we have to make sure that the RDDREQ bit is set in the NDSR
>>> register.
>>> 
>>> This fixes an issue that was seen on the Armada 385, and
>>> presumably other mvebu SoCs, when a read on a newly erased page
>>> would end up in the driver reporting a timeout from the NAND.
>>> 
>>> Cc: <stable@vger.kernel.org> # v3.14 Signed-off-by: Maxime
>>> Ripard <maxime.ripard@free-electrons.com> --- 
>>> drivers/mtd/nand/pxa3xx_nand.c | 48
>>> ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42
>>> insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>>> b/drivers/mtd/nand/pxa3xx_nand.c index
>>> 96b0b1d27df1..bc677362bc73 100644 ---
>>> a/drivers/mtd/nand/pxa3xx_nand.c +++
>>> b/drivers/mtd/nand/pxa3xx_nand.c @@ -480,6 +480,42 @@ static
>>> void disable_int(struct pxa3xx_nand_info *info, uint32_t
>>> int_mask) nand_writel(info, NDCR, ndcr | int_mask); }
>>> 
>>> +static void drain_fifo(struct pxa3xx_nand_info *info, void
>>> *data, int len) +{ +	if (info->ecc_bch) { +		int timeout; + +
>>> /* +		 * According to the datasheet, when reading from NDDB +
>>> * with BCH enabled, after each 32 bytes reads, we +		 * have to
>>> make sure that the NDSR.RDDREQ bit is set. +		 * +		 * Drain
>>> the FIFO 8 32 bits reads at a time, and skip +		 * the polling
>>> on the last read. +		 */ +		while (len > 8) { +
>>> __raw_readsl(info->mmio_base + NDDB, data, 8); + +			for
>>> (timeout = 0; +			     !(nand_readl(info, NDSR) &
>>> NDSR_RDDREQ); +			     timeout++) { +				if (timeout >= 5) { +
>>> dev_err(&info->pdev->dev, +						"Timeout on RDDREQ while
>>> draining the FIFO\n"); +					return; +				} + +				mdelay(1);
>> 
>> This is probably a stupid nit.. but here it goes is it any 
>> difference if udelay is used here?
>> 
>> Does this makes anything better/worse?
> 
> It doesn't make any difference. On the board I've been using, we
> never hit the delay.
> 
> So I really don't care about the number of retries and the sleep 
> behind them. I made these numbers up, feel free to come up with
> others if it makes you more comfortable, but could we settle this?
> 

OK, let's stop the bikeshedding. For both patches:

Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
- -- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJU5JxaAAoJEIOKbhOEIHKi9HYP/jxU75zI6QjNk1yGL7Y3qwEy
M2UYrepXTwgRVRxAIN3nhGqERuBVOJCIVKlb3CUSiq9auNAO8rsRs6JTAossV781
LTUniAA0nIBFbTn/k2Wc6yGQizSy7iJRXu73MrLJrcSFO8JxFFqu04V1EYuZbh5s
fuVpeLJEX8Gfy6gj85ybFs7+wkD/XnENKlnDzD6y/n4ewBC3yDPLNh436hZpEVDO
ky8lYjGPHMQs0yEDcp1rfFejLAmxJ4SY6hVSKAxq3/Bn58S4y3Pgkm4cJtP8nHYe
UN4q9UfOBIHWrQIVbBlViK//n3zyEtaPojDSKUiSvI2+Hmz9+eortlYEvpN1HCP3
Xqy1gzFto9FpP4Wp3KpyF609JNVUeQxAsUQZMXM6yaH2oz35szZhBq2xlIpsyo3C
BDbjaYKFk3hVg+jAE0iitZW8BiZS+WZ/pzX4A8rwQBSTMcbrLTuRV611R4E/nEBL
sfCwDWc1gxDDiM8pMJBGC97gwtHEibJqxES9y+T3LrhxtqPl1kUczHFDPgd3uoVw
fT71PsZBtGUeOu3ysymNPc+mF9b9G9KRHYhSiOjnEIle9yvXh6kaGWv93ZM3RCUG
JASv01+gqb+Bz5ZU3MMU+xjHxeoWBq7KfSWcshEHpD8QCuiZzNZ3yt0dZaYXao+M
YsLlm5s62fDVILb2Q3+d
=MA8V
-----END PGP SIGNATURE-----

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Ezequiel Garcia]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 02/18/2015 11:01 AM, Maxime Ripard wrote:
> On Wed, Feb 18, 2015 at 10:40:02AM -0300, Ezequiel Garcia wrote:
>> On 02/18/2015 07:32 AM, Maxime Ripard wrote:
>>> The NDDB register holds the data that are needed by the read
>>> and write commands.
>>> 
>>> However, during a read PIO access, the datasheet specifies that
>>> after each 32 bytes read in that register, when BCH is enabled,
>>> we have to make sure that the RDDREQ bit is set in the NDSR
>>> register.
>>> 
>>> This fixes an issue that was seen on the Armada 385, and
>>> presumably other mvebu SoCs, when a read on a newly erased page
>>> would end up in the driver reporting a timeout from the NAND.
>>> 
>>> Cc: <stable@vger.kernel.org> # v3.14 Signed-off-by: Maxime
>>> Ripard <maxime.ripard@free-electrons.com> --- 
>>> drivers/mtd/nand/pxa3xx_nand.c | 48
>>> ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42
>>> insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>>> b/drivers/mtd/nand/pxa3xx_nand.c index
>>> 96b0b1d27df1..bc677362bc73 100644 ---
>>> a/drivers/mtd/nand/pxa3xx_nand.c +++
>>> b/drivers/mtd/nand/pxa3xx_nand.c @@ -480,6 +480,42 @@ static
>>> void disable_int(struct pxa3xx_nand_info *info, uint32_t
>>> int_mask) nand_writel(info, NDCR, ndcr | int_mask); }
>>> 
>>> +static void drain_fifo(struct pxa3xx_nand_info *info, void
>>> *data, int len) +{ +	if (info->ecc_bch) { +		int timeout; + +
>>> /* +		 * According to the datasheet, when reading from NDDB +
>>> * with BCH enabled, after each 32 bytes reads, we +		 * have to
>>> make sure that the NDSR.RDDREQ bit is set. +		 * +		 * Drain
>>> the FIFO 8 32 bits reads at a time, and skip +		 * the polling
>>> on the last read. +		 */ +		while (len > 8) { +
>>> __raw_readsl(info->mmio_base + NDDB, data, 8); + +			for
>>> (timeout = 0; +			     !(nand_readl(info, NDSR) &
>>> NDSR_RDDREQ); +			     timeout++) { +				if (timeout >= 5) { +
>>> dev_err(&info->pdev->dev, +						"Timeout on RDDREQ while
>>> draining the FIFO\n"); +					return; +				} + +				mdelay(1);
>> 
>> This is probably a stupid nit.. but here it goes is it any 
>> difference if udelay is used here?
>> 
>> Does this makes anything better/worse?
> 
> It doesn't make any difference. On the board I've been using, we
> never hit the delay.
> 
> So I really don't care about the number of retries and the sleep 
> behind them. I made these numbers up, feel free to come up with
> others if it makes you more comfortable, but could we settle this?
> 

OK, let's stop the bikeshedding. For both patches:

Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
- -- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJU5JxaAAoJEIOKbhOEIHKi9HYP/jxU75zI6QjNk1yGL7Y3qwEy
M2UYrepXTwgRVRxAIN3nhGqERuBVOJCIVKlb3CUSiq9auNAO8rsRs6JTAossV781
LTUniAA0nIBFbTn/k2Wc6yGQizSy7iJRXu73MrLJrcSFO8JxFFqu04V1EYuZbh5s
fuVpeLJEX8Gfy6gj85ybFs7+wkD/XnENKlnDzD6y/n4ewBC3yDPLNh436hZpEVDO
ky8lYjGPHMQs0yEDcp1rfFejLAmxJ4SY6hVSKAxq3/Bn58S4y3Pgkm4cJtP8nHYe
UN4q9UfOBIHWrQIVbBlViK//n3zyEtaPojDSKUiSvI2+Hmz9+eortlYEvpN1HCP3
Xqy1gzFto9FpP4Wp3KpyF609JNVUeQxAsUQZMXM6yaH2oz35szZhBq2xlIpsyo3C
BDbjaYKFk3hVg+jAE0iitZW8BiZS+WZ/pzX4A8rwQBSTMcbrLTuRV611R4E/nEBL
sfCwDWc1gxDDiM8pMJBGC97gwtHEibJqxES9y+T3LrhxtqPl1kUczHFDPgd3uoVw
fT71PsZBtGUeOu3ysymNPc+mF9b9G9KRHYhSiOjnEIle9yvXh6kaGWv93ZM3RCUG
JASv01+gqb+Bz5ZU3MMU+xjHxeoWBq7KfSWcshEHpD8QCuiZzNZ3yt0dZaYXao+M
YsLlm5s62fDVILb2Q3+d
=MA8V
-----END PGP SIGNATURE-----

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Brian Norris]

On Wed, Feb 18, 2015 at 11:32:07AM +0100, Maxime Ripard wrote:
> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Pushed this one to linux-mtd.git. I'll try to get it out in the 4.0
cycle. I assume patch 2 (the DT addition) will go through arm-soc.

Brian

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Brian Norris]

On Wed, Feb 18, 2015 at 11:32:07AM +0100, Maxime Ripard wrote:
> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Pushed this one to linux-mtd.git. I'll try to get it out in the 4.0
cycle. I assume patch 2 (the DT addition) will go through arm-soc.

Brian

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Brian Norris]

On Wed, Feb 18, 2015 at 11:32:07AM +0100, Maxime Ripard wrote:
> The NDDB register holds the data that are needed by the read and write
> commands.
> 
> However, during a read PIO access, the datasheet specifies that after each 32
> bytes read in that register, when BCH is enabled, we have to make sure that the
> RDDREQ bit is set in the NDSR register.
> 
> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
> SoCs, when a read on a newly erased page would end up in the driver reporting a
> timeout from the NAND.
> 
> Cc: <stable@vger.kernel.org> # v3.14
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Pushed this one to linux-mtd.git. I'll try to get it out in the 4.0
cycle. I assume patch 2 (the DT addition) will go through arm-soc.

Brian

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Gregory CLEMENT]

On 28/02/2015 10:01, Brian Norris wrote:
> On Wed, Feb 18, 2015 at 11:32:07AM +0100, Maxime Ripard wrote:
>> The NDDB register holds the data that are needed by the read and write
>> commands.
>>
>> However, during a read PIO access, the datasheet specifies that after each 32
>> bytes read in that register, when BCH is enabled, we have to make sure that the
>> RDDREQ bit is set in the NDSR register.
>>
>> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
>> SoCs, when a read on a newly erased page would end up in the driver reporting a
>> timeout from the NAND.
>>
>> Cc: <stable@vger.kernel.org> # v3.14
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Pushed this one to linux-mtd.git. I'll try to get it out in the 4.0
> cycle. I assume patch 2 (the DT addition) will go through arm-soc.

Yes, now that you took the driver part,  I will apply it on mvebu and then push it
to arm-soc.

Thanks,

Gregory


> 
> Brian
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Gregory CLEMENT]

On 28/02/2015 10:01, Brian Norris wrote:
> On Wed, Feb 18, 2015 at 11:32:07AM +0100, Maxime Ripard wrote:
>> The NDDB register holds the data that are needed by the read and write
>> commands.
>>
>> However, during a read PIO access, the datasheet specifies that after each 32
>> bytes read in that register, when BCH is enabled, we have to make sure that the
>> RDDREQ bit is set in the NDSR register.
>>
>> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
>> SoCs, when a read on a newly erased page would end up in the driver reporting a
>> timeout from the NAND.
>>
>> Cc: <stable@vger.kernel.org> # v3.14
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Pushed this one to linux-mtd.git. I'll try to get it out in the 4.0
> cycle. I assume patch 2 (the DT addition) will go through arm-soc.

Yes, now that you took the driver part,  I will apply it on mvebu and then push it
to arm-soc.

Thanks,

Gregory


> 
> Brian
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH v4 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining

[Gregory CLEMENT]

On 28/02/2015 10:01, Brian Norris wrote:
> On Wed, Feb 18, 2015 at 11:32:07AM +0100, Maxime Ripard wrote:
>> The NDDB register holds the data that are needed by the read and write
>> commands.
>>
>> However, during a read PIO access, the datasheet specifies that after each 32
>> bytes read in that register, when BCH is enabled, we have to make sure that the
>> RDDREQ bit is set in the NDSR register.
>>
>> This fixes an issue that was seen on the Armada 385, and presumably other mvebu
>> SoCs, when a read on a newly erased page would end up in the driver reporting a
>> timeout from the NAND.
>>
>> Cc: <stable@vger.kernel.org> # v3.14
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Pushed this one to linux-mtd.git. I'll try to get it out in the 4.0
> cycle. I assume patch 2 (the DT addition) will go through arm-soc.

Yes, now that you took the driver part,  I will apply it on mvebu and then push it
to arm-soc.

Thanks,

Gregory


> 
> Brian
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Gregory CLEMENT]

Hi Maxime,

On 18/02/2015 11:32, Maxime Ripard wrote:
> The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
> Micron as its main storage. Enable it.

I wanted applying your patch but it failed. Actually it depends on your
other patch "ARM: mvebu: armada-385-ap: Enable USB3 port" and this patch
depend on an other patch "usb: xhci: plat: Add USB phy support" which
hadn't been merged.

So either you send a new version which apply on 4.0-rc1, or we can wait for
that "usb: xhci: plat: Add USB phy support" will be merged then I will be able
applying "ARM: mvebu: armada-385-ap: Enable USB3 port" and finally this patch.
As you want.


Thanks,

Gregory


> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-385-db-ap.dts b/arch/arm/boot/dts/armada-385-db-ap.dts
> index b891b4c897f5..ee648fb19075 100644
> --- a/arch/arm/boot/dts/armada-385-db-ap.dts
> +++ b/arch/arm/boot/dts/armada-385-db-ap.dts
> @@ -130,6 +130,19 @@
>  				phy-mode = "rgmii-id";
>  			};
>  
> +			nfc: flash@d0000 {
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +
> +				num-cs = <1>;
> +				nand-ecc-strength = <4>;
> +				nand-ecc-step-size = <512>;
> +				marvell,nand-keep-config;
> +				marvell,nand-enable-arbiter;
> +				nand-on-flash-bbt;
> +			};
> +
>  			usb3@f0000 {
>  				status = "okay";
>  				usb-phy = <&usb3_phy>;
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Gregory CLEMENT]

Hi Maxime,

On 18/02/2015 11:32, Maxime Ripard wrote:
> The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
> Micron as its main storage. Enable it.

I wanted applying your patch but it failed. Actually it depends on your
other patch "ARM: mvebu: armada-385-ap: Enable USB3 port" and this patch
depend on an other patch "usb: xhci: plat: Add USB phy support" which
hadn't been merged.

So either you send a new version which apply on 4.0-rc1, or we can wait for
that "usb: xhci: plat: Add USB phy support" will be merged then I will be able
applying "ARM: mvebu: armada-385-ap: Enable USB3 port" and finally this patch.
As you want.


Thanks,

Gregory


> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-385-db-ap.dts b/arch/arm/boot/dts/armada-385-db-ap.dts
> index b891b4c897f5..ee648fb19075 100644
> --- a/arch/arm/boot/dts/armada-385-db-ap.dts
> +++ b/arch/arm/boot/dts/armada-385-db-ap.dts
> @@ -130,6 +130,19 @@
>  				phy-mode = "rgmii-id";
>  			};
>  
> +			nfc: flash@d0000 {
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +
> +				num-cs = <1>;
> +				nand-ecc-strength = <4>;
> +				nand-ecc-step-size = <512>;
> +				marvell,nand-keep-config;
> +				marvell,nand-enable-arbiter;
> +				nand-on-flash-bbt;
> +			};
> +
>  			usb3@f0000 {
>  				status = "okay";
>  				usb-phy = <&usb3_phy>;
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Gregory CLEMENT]

Hi Maxime,

On 18/02/2015 11:32, Maxime Ripard wrote:
> The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
> Micron as its main storage. Enable it.

I wanted applying your patch but it failed. Actually it depends on your
other patch "ARM: mvebu: armada-385-ap: Enable USB3 port" and this patch
depend on an other patch "usb: xhci: plat: Add USB phy support" which
hadn't been merged.

So either you send a new version which apply on 4.0-rc1, or we can wait for
that "usb: xhci: plat: Add USB phy support" will be merged then I will be able
applying "ARM: mvebu: armada-385-ap: Enable USB3 port" and finally this patch.
As you want.


Thanks,

Gregory


> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-385-db-ap.dts b/arch/arm/boot/dts/armada-385-db-ap.dts
> index b891b4c897f5..ee648fb19075 100644
> --- a/arch/arm/boot/dts/armada-385-db-ap.dts
> +++ b/arch/arm/boot/dts/armada-385-db-ap.dts
> @@ -130,6 +130,19 @@
>  				phy-mode = "rgmii-id";
>  			};
>  
> +			nfc: flash at d0000 {
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +
> +				num-cs = <1>;
> +				nand-ecc-strength = <4>;
> +				nand-ecc-step-size = <512>;
> +				marvell,nand-keep-config;
> +				marvell,nand-enable-arbiter;
> +				nand-on-flash-bbt;
> +			};
> +
>  			usb3 at f0000 {
>  				status = "okay";
>  				usb-phy = <&usb3_phy>;
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

On Tue, Mar 03, 2015 at 09:10:15AM +0100, Gregory CLEMENT wrote:
> Hi Maxime,
> 
> On 18/02/2015 11:32, Maxime Ripard wrote:
> > The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
> > Micron as its main storage. Enable it.
> 
> I wanted applying your patch but it failed. Actually it depends on your
> other patch "ARM: mvebu: armada-385-ap: Enable USB3 port" and this patch
> depend on an other patch "usb: xhci: plat: Add USB phy support" which
> hadn't been merged.

Hmmmm, indeed.

> So either you send a new version which apply on 4.0-rc1, or we can wait for
> that "usb: xhci: plat: Add USB phy support" will be merged then I will be able
> applying "ARM: mvebu: armada-385-ap: Enable USB3 port" and finally this patch.
> As you want.

I'll send a new version.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

On Tue, Mar 03, 2015 at 09:10:15AM +0100, Gregory CLEMENT wrote:
> Hi Maxime,
> 
> On 18/02/2015 11:32, Maxime Ripard wrote:
> > The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
> > Micron as its main storage. Enable it.
> 
> I wanted applying your patch but it failed. Actually it depends on your
> other patch "ARM: mvebu: armada-385-ap: Enable USB3 port" and this patch
> depend on an other patch "usb: xhci: plat: Add USB phy support" which
> hadn't been merged.

Hmmmm, indeed.

> So either you send a new version which apply on 4.0-rc1, or we can wait for
> that "usb: xhci: plat: Add USB phy support" will be merged then I will be able
> applying "ARM: mvebu: armada-385-ap: Enable USB3 port" and finally this patch.
> As you want.

I'll send a new version.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v4 2/2] ARM: mvebu: a385-db-ap: Enable the NAND

[Maxime Ripard]

On Tue, Mar 03, 2015 at 09:10:15AM +0100, Gregory CLEMENT wrote:
> Hi Maxime,
> 
> On 18/02/2015 11:32, Maxime Ripard wrote:
> > The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from
> > Micron as its main storage. Enable it.
> 
> I wanted applying your patch but it failed. Actually it depends on your
> other patch "ARM: mvebu: armada-385-ap: Enable USB3 port" and this patch
> depend on an other patch "usb: xhci: plat: Add USB phy support" which
> hadn't been merged.

Hmmmm, indeed.

> So either you send a new version which apply on 4.0-rc1, or we can wait for
> that "usb: xhci: plat: Add USB phy support" will be merged then I will be able
> applying "ARM: mvebu: armada-385-ap: Enable USB3 port" and finally this patch.
> As you want.

I'll send a new version.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150303/dce0daea/attachment.sig>