[PATCH 0/2] nand: pxa3xx: A couple device identification fixes

[PATCH 0/2] nand: pxa3xx: A couple device identification fixes

[Ezequiel Garcia]

Brian,

A couple fixes. The first one to increase the size of the READ_ID
buffer, which is required to read 'ONFI' and 'JEDEC'.
The second one, to allow to read the ONFI and JEDEC PARAM pages.

This is tested on:
 * Armada XP GP board (w/ONFI device)
 * PXA3xx CM-X300 board (non-ONFI/JEDEC device)

Can't test on any JEDEC device, but at least I can't see any regressions.

Ezequiel Garcia (2):
  nand: pxa3xx: Increase READ_ID buffer and make the size static
  nand: pxa3xx: Increase initial buffer size

 drivers/mtd/nand/pxa3xx_nand.c | 43 +++++++++++++++---------------------------
 1 file changed, 15 insertions(+), 28 deletions(-)

-- 
2.4.6

[PATCH 1/2] nand: pxa3xx: Increase READ_ID buffer and make the size static

[Ezequiel Garcia]

The read ID count should be made as large as the maximum READ_ID size,
so there's no need to have dynamic size. This commit sets the hardware
maximum read ID count, which should be more than enough on all cases.
Also, we get rid of the read_id_bytes, and use a macro instead.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1259cc5..0815f11 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -126,6 +126,8 @@
 #define EXT_CMD_TYPE_LAST_RW	1 /* Last naked read/write */
 #define EXT_CMD_TYPE_MONO	0 /* Monolithic read/write */
 
+#define READ_ID_BYTES		7 /* Large enough to read 'JEDEC' */
+
 /* macros for registers read/write */
 #define nand_writel(info, off, val)	\
 	writel_relaxed((val), (info)->mmio_base + (off))
@@ -173,8 +175,6 @@ struct pxa3xx_nand_host {
 	/* calculated from pxa3xx_nand_flash data */
 	unsigned int		col_addr_cycles;
 	unsigned int		row_addr_cycles;
-	size_t			read_id_bytes;
-
 };
 
 struct pxa3xx_nand_info {
@@ -910,13 +910,13 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 		break;
 
 	case NAND_CMD_READID:
-		info->buf_count = host->read_id_bytes;
+		info->buf_count = READ_ID_BYTES;
 		info->ndcb0 |= NDCB0_CMD_TYPE(3)
 				| NDCB0_ADDR_CYC(1)
 				| command;
 		info->ndcb1 = (column & 0xFF);
 
-		info->data_size = 8;
+		info->data_size = READ_ID_BYTES;
 		break;
 	case NAND_CMD_STATUS:
 		info->buf_count = 1;
@@ -1247,9 +1247,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 		return -EINVAL;
 	}
 
-	/* calculate flash information */
-	host->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
-
 	/* calculate addressing information */
 	host->col_addr_cycles = (f->page_size == 2048) ? 2 : 1;
 
@@ -1265,7 +1262,7 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 	ndcr |= (f->flash_width == 16) ? NDCR_DWIDTH_M : 0;
 	ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0;
 
-	ndcr |= NDCR_RD_ID_CNT(host->read_id_bytes);
+	ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
 	ndcr |= NDCR_SPARE_EN; /* enable spare by default */
 
 	info->reg_ndcr = ndcr;
@@ -1276,23 +1273,10 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 
 static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 {
-	/*
-	 * We set 0 by hard coding here, for we don't support keep_config
-	 * when there is more than one chip attached to the controller
-	 */
-	struct pxa3xx_nand_host *host = info->host[0];
 	uint32_t ndcr = nand_readl(info, NDCR);
 
-	if (ndcr & NDCR_PAGE_SZ) {
-		/* Controller's FIFO size */
-		info->chunk_size = 2048;
-		host->read_id_bytes = 4;
-	} else {
-		info->chunk_size = 512;
-		host->read_id_bytes = 2;
-	}
-
 	/* Set an initial chunk size */
+	info->chunk_size = ndcr & NDCR_PAGE_SZ ? 2048 : 512;
 	info->reg_ndcr = ndcr & ~NDCR_INT_MASK;
 	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
 	info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
-- 
2.4.6

Re: [PATCH 1/2] nand: pxa3xx: Increase READ_ID buffer and make the size static

[Brian Norris]

On Mon, Aug 03, 2015 at 11:31:25AM -0300, Ezequiel Garcia wrote:
> The read ID count should be made as large as the maximum READ_ID size,
> so there's no need to have dynamic size. This commit sets the hardware
> maximum read ID count, which should be more than enough on all cases.
> Also, we get rid of the read_id_bytes, and use a macro instead.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 28 ++++++----------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 1259cc5..0815f11 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -126,6 +126,8 @@
>  #define EXT_CMD_TYPE_LAST_RW	1 /* Last naked read/write */
>  #define EXT_CMD_TYPE_MONO	0 /* Monolithic read/write */
>  
> +#define READ_ID_BYTES		7 /* Large enough to read 'JEDEC' */
> +

For the future: nand_base likes to read 8 bytes of ID for
regular ID decoding. But pxa3xx_nand doesn't use the standard ID code
still, so that's not a problem yet.

Brian

Re: [PATCH 1/2] nand: pxa3xx: Increase READ_ID buffer and make the size static

[Ezequiel Garcia]

On 12 August 2015 at 22:23, Brian Norris <computersforpeace@gmail.com> wrote:
> On Mon, Aug 03, 2015 at 11:31:25AM -0300, Ezequiel Garcia wrote:
>> The read ID count should be made as large as the maximum READ_ID size,
>> so there's no need to have dynamic size. This commit sets the hardware
>> maximum read ID count, which should be more than enough on all cases.
>> Also, we get rid of the read_id_bytes, and use a macro instead.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>> ---
>>  drivers/mtd/nand/pxa3xx_nand.c | 28 ++++++----------------------
>>  1 file changed, 6 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
>> index 1259cc5..0815f11 100644
>> --- a/drivers/mtd/nand/pxa3xx_nand.c
>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> @@ -126,6 +126,8 @@
>>  #define EXT_CMD_TYPE_LAST_RW 1 /* Last naked read/write */
>>  #define EXT_CMD_TYPE_MONO    0 /* Monolithic read/write */
>>
>> +#define READ_ID_BYTES                7 /* Large enough to read 'JEDEC' */
>> +
>
> For the future: nand_base likes to read 8 bytes of ID for
> regular ID decoding. But pxa3xx_nand doesn't use the standard ID code
> still, so that's not a problem yet.
>

And in any case, 7 bytes is the maximum count for READ_ID supported
by this controller :-)

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

Re: [PATCH 1/2] nand: pxa3xx: Increase READ_ID buffer and make the size static

[Jonas Gorski]

Hi,

On Mon, Aug 3, 2015 at 4:31 PM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> The read ID count should be made as large as the maximum READ_ID size,
> so there's no need to have dynamic size. This commit sets the hardware
> maximum read ID count, which should be more than enough on all cases.
> Also, we get rid of the read_id_bytes, and use a macro instead.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 28 ++++++----------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 1259cc5..0815f11 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -126,6 +126,8 @@
>  #define EXT_CMD_TYPE_LAST_RW   1 /* Last naked read/write */
>  #define EXT_CMD_TYPE_MONO      0 /* Monolithic read/write */
>
> +#define READ_ID_BYTES          7 /* Large enough to read 'JEDEC' */
> +
>  /* macros for registers read/write */
>  #define nand_writel(info, off, val)    \
>         writel_relaxed((val), (info)->mmio_base + (off))
> @@ -173,8 +175,6 @@ struct pxa3xx_nand_host {
>         /* calculated from pxa3xx_nand_flash data */
>         unsigned int            col_addr_cycles;
>         unsigned int            row_addr_cycles;
> -       size_t                  read_id_bytes;
> -
>  };
>
>  struct pxa3xx_nand_info {
> @@ -910,13 +910,13 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
>                 break;
>
>         case NAND_CMD_READID:
> -               info->buf_count = host->read_id_bytes;
> +               info->buf_count = READ_ID_BYTES;
>                 info->ndcb0 |= NDCB0_CMD_TYPE(3)
>                                 | NDCB0_ADDR_CYC(1)
>                                 | command;
>                 info->ndcb1 = (column & 0xFF);
>
> -               info->data_size = 8;
> +               info->data_size = READ_ID_BYTES;

You are reducing ->data_size from 8 to 7 here, is this intentional?


Jonas

Re: [PATCH 1/2] nand: pxa3xx: Increase READ_ID buffer and make the size static

[Robert Jarzmik]

Jonas Gorski <jogo@openwrt.org> writes:

>>
>> -               info->data_size = 8;
>> +               info->data_size = READ_ID_BYTES;
>
> You are reducing ->data_size from 8 to 7 here, is this intentional?
I bet it's a small mistake.

The pxa3xx NFC controller requires to read at least 8 bytes, as it is one entry
FIFO (chapter 3.7.1.2 "Non-DMA Operating mode" of PXA3xx VolII developer
manual).

Same constraint applies to DMA btw.

Cheers.

-- 
Robert

Re: [PATCH 1/2] nand: pxa3xx: Increase READ_ID buffer and make the size static

[Ezequiel Garcia]

On 14 August 2015 at 13:18, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Jonas Gorski <jogo@openwrt.org> writes:
>
>>>
>>> -               info->data_size = 8;
>>> +               info->data_size = READ_ID_BYTES;
>>
>> You are reducing ->data_size from 8 to 7 here, is this intentional?
> I bet it's a small mistake.
>

Ah, good catch. Yes, as Robert explains this is a mistake.
Brian, if you can drop this patch, I'll send a new one.

> The pxa3xx NFC controller requires to read at least 8 bytes, as it is one entry
> FIFO (chapter 3.7.1.2 "Non-DMA Operating mode" of PXA3xx VolII developer
> manual).
>
> Same constraint applies to DMA btw.
>

This is also explained on the A370 specs [1], section 14.4 NAND, Operation

""
For read ID and read status commands, software reads 8B from NDDB,
because the NFCv2
allocates one buffer entry (8B) to hold the read data for these
commands. Valid data is aligned to the
LSB; users should discard non-valid bytes, for example, for a 5B
read-ID command, bytes 0 through
4 are valid while bytes 5 through 7 should be ignored.
""

Read ID count should be set to 7 (the maximum allowed read id count),
buf_count will be set to 7 too, but data_size should be set to 8.

[1] http://www.marvell.com/embedded-processors/armada-300/assets/ARMADA370-FunctionalSpec-datasheet.pdf

Thanks a lot for the catch,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH 1/2] nand: pxa3xx: Increase READ_ID buffer and make the size static

[Brian Norris]

On Fri, Aug 14, 2015 at 03:07:51PM -0300, Ezequiel Garcia wrote:
> Brian, if you can drop this patch, I'll send a new one.

Dropped. Because I am occasionally a git heretic, I have rewritten the
history.

Brian

[PATCH 2/2] nand: pxa3xx: Increase initial buffer size

[Ezequiel Garcia]

The initial buffer is used for the initial commands used to detect
a flash device (STATUS, READID and PARAM).

ONFI param page is 256 bytes, and there are three redundant copies
to be read. JEDEC param page is 512 bytes, and there are also three
redundant copies to be read. Hence this buffer should be at least
512 x 3. This commits rounds the buffer size to 2048.

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

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 0815f11..9dac28d 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -45,10 +45,13 @@
 
 /*
  * Define a buffer size for the initial command that detects the flash device:
- * STATUS, READID and PARAM. The largest of these is the PARAM command,
- * needing 256 bytes.
+ * STATUS, READID and PARAM.
+ * ONFI param page is 256 bytes, and there are three redundant copies
+ * to be read. JEDEC param page is 512 bytes, and there are also three
+ * redundant copies to be read.
+ * Hence this buffer should be at least 512 x 3. Let's pick 2048.
  */
-#define INIT_BUFFER_SIZE	256
+#define INIT_BUFFER_SIZE	2048
 
 /* registers and bit definitions */
 #define NDCR		(0x00) /* Control register */
@@ -899,14 +902,14 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
 		break;
 
 	case NAND_CMD_PARAM:
-		info->buf_count = 256;
+		info->buf_count = INIT_BUFFER_SIZE;
 		info->ndcb0 |= NDCB0_CMD_TYPE(0)
 				| NDCB0_ADDR_CYC(1)
 				| NDCB0_LEN_OVRD
 				| command;
 		info->ndcb1 = (column & 0xFF);
-		info->ndcb3 = 256;
-		info->data_size = 256;
+		info->ndcb3 = INIT_BUFFER_SIZE;
+		info->data_size = INIT_BUFFER_SIZE;
 		break;
 
 	case NAND_CMD_READID:
-- 
2.4.6

Re: [PATCH 2/2] nand: pxa3xx: Increase initial buffer size

[Robert Jarzmik]

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

> The initial buffer is used for the initial commands used to detect
> a flash device (STATUS, READID and PARAM).
>
> ONFI param page is 256 bytes, and there are three redundant copies
> to be read. JEDEC param page is 512 bytes, and there are also three
> redundant copies to be read. Hence this buffer should be at least
> 512 x 3. This commits rounds the buffer size to 2048.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

-
Robert

Re: [PATCH 2/2] nand: pxa3xx: Increase initial buffer size

[Brian Norris]

On Mon, Aug 03, 2015 at 11:31:26AM -0300, Ezequiel Garcia wrote:
> The initial buffer is used for the initial commands used to detect
> a flash device (STATUS, READID and PARAM).
> 
> ONFI param page is 256 bytes, and there are three redundant copies
> to be read. JEDEC param page is 512 bytes, and there are also three
> redundant copies to be read. Hence this buffer should be at least
> 512 x 3. This commits rounds the buffer size to 2048.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 0815f11..9dac28d 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -45,10 +45,13 @@
>  
>  /*
>   * Define a buffer size for the initial command that detects the flash device:
> - * STATUS, READID and PARAM. The largest of these is the PARAM command,
> - * needing 256 bytes.
> + * STATUS, READID and PARAM.
> + * ONFI param page is 256 bytes, and there are three redundant copies
> + * to be read. JEDEC param page is 512 bytes, and there are also three
> + * redundant copies to be read.
> + * Hence this buffer should be at least 512 x 3. Let's pick 2048.
>   */
> -#define INIT_BUFFER_SIZE	256
> +#define INIT_BUFFER_SIZE	2048

Not sure if you're going to run into problems with extended parameter
pages. For one, you don't support NAND_CMD_RNDOUT, it seems.

Brian

>  
>  /* registers and bit definitions */
>  #define NDCR		(0x00) /* Control register */
> @@ -899,14 +902,14 @@ static int prepare_set_command(struct pxa3xx_nand_info *info, int command,
>  		break;
>  
>  	case NAND_CMD_PARAM:
> -		info->buf_count = 256;
> +		info->buf_count = INIT_BUFFER_SIZE;
>  		info->ndcb0 |= NDCB0_CMD_TYPE(0)
>  				| NDCB0_ADDR_CYC(1)
>  				| NDCB0_LEN_OVRD
>  				| command;
>  		info->ndcb1 = (column & 0xFF);
> -		info->ndcb3 = 256;
> -		info->data_size = 256;
> +		info->ndcb3 = INIT_BUFFER_SIZE;
> +		info->data_size = INIT_BUFFER_SIZE;
>  		break;
>  
>  	case NAND_CMD_READID:
> -- 
> 2.4.6
> 

Re: [PATCH 0/2] nand: pxa3xx: A couple device identification fixes

[Brian Norris]

On Mon, Aug 03, 2015 at 11:31:24AM -0300, Ezequiel Garcia wrote:
> Brian,
> 
> A couple fixes. The first one to increase the size of the READ_ID
> buffer, which is required to read 'ONFI' and 'JEDEC'.
> The second one, to allow to read the ONFI and JEDEC PARAM pages.
> 
> This is tested on:
>  * Armada XP GP board (w/ONFI device)
>  * PXA3xx CM-X300 board (non-ONFI/JEDEC device)
> 
> Can't test on any JEDEC device, but at least I can't see any regressions.

Applied both to l2-mtd.git.