Patchwork [v2,3/7] mtd: sh_flctl: Expand the READID command to 8 bytes

login
register
mail settings
Submitter Bastian Hecht
Date Feb. 11, 2012, 11:45 a.m.
Message ID <1328960705-18699-4-git-send-email-hechtb@gmail.com>
Download mbox | patch
Permalink /patch/140779/
State New
Headers show

Comments

Bastian Hecht - Feb. 11, 2012, 11:45 a.m.
The nand base code wants to read out 8 bytes in the READID command.
Reflect this in the driver code.

Signed-off-by: Bastian Hecht <hechtb@gmail.com>
---
changelog: exactly same as patch v1.

 drivers/mtd/nand/sh_flctl.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
Laurent Pinchart - Feb. 18, 2012, 2:20 a.m.
Hi Bastian,

Thanks for the patch.

On Saturday 11 February 2012 12:45:01 Bastian Hecht wrote:
> The nand base code wants to read out 8 bytes in the READID command.
> Reflect this in the driver code.
> 
> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
> ---
> changelog: exactly same as patch v1.
> 
>  drivers/mtd/nand/sh_flctl.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
> index 8c97367..407acb5 100644
> --- a/drivers/mtd/nand/sh_flctl.c
> +++ b/drivers/mtd/nand/sh_flctl.c
> @@ -320,6 +320,7 @@ static void set_cmd_regs(struct mtd_info *mtd, uint32_t
> cmd, uint32_t flcmcdr_va break;
>  	case NAND_CMD_READID:
>  		flcmncr_val &= ~SNAND_E;
> +		flcmdcr_val |= CDSRC_E;

this bit is marked as reserved in my SH7372 documentation, and should be set 
to 0. Won't this break compatibility with the SH7372 ?

>  		addr_len_bytes = ADRCNT_1;
>  		break;
>  	case NAND_CMD_STATUS:
> @@ -559,12 +560,18 @@ static void flctl_cmdfunc(struct mtd_info *mtd,
> unsigned int command,
> 
>  	case NAND_CMD_READID:
>  		set_cmd_regs(mtd, command, command);
> -		set_addr(mtd, 0, 0);
> 
> -		flctl->read_bytes = 4;
> +		/* READID is always performed using an 8-bit bus */
> +		if (flctl->chip.options & NAND_BUSWIDTH_16)
> +			column <<= 1;
> +		set_addr(mtd, column, 0);
> +
> +		flctl->read_bytes = 8;
>  		writel(flctl->read_bytes, FLDTCNTR(flctl)); /* set read size */
> +		empty_fifo(flctl);
>  		start_translation(flctl);
> -		read_datareg(flctl, 0);	/* read and end */
> +		read_fiforeg(flctl, flctl->read_bytes, 0);

Not saying it's a bad thing, but why do you switch from read_datareg() to 
read_fiforeg() ?

> +		wait_completion(flctl);
>  		break;
> 
>  	case NAND_CMD_ERASE1:
Bastian Hecht - Feb. 19, 2012, 10:46 a.m.
Hello Laurent,

2012/2/18 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Hi Bastian,
>
> Thanks for the patch.
>
> On Saturday 11 February 2012 12:45:01 Bastian Hecht wrote:
>> The nand base code wants to read out 8 bytes in the READID command.
>> Reflect this in the driver code.
>>
>> Signed-off-by: Bastian Hecht <hechtb@gmail.com>
>> ---
>> changelog: exactly same as patch v1.
>>
>>  drivers/mtd/nand/sh_flctl.c |   13 ++++++++++---
>>  1 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
>> index 8c97367..407acb5 100644
>> --- a/drivers/mtd/nand/sh_flctl.c
>> +++ b/drivers/mtd/nand/sh_flctl.c
>> @@ -320,6 +320,7 @@ static void set_cmd_regs(struct mtd_info *mtd, uint32_t
>> cmd, uint32_t flcmcdr_va break;
>>       case NAND_CMD_READID:
>>               flcmncr_val &= ~SNAND_E;
>> +             flcmdcr_val |= CDSRC_E;
>
> this bit is marked as reserved in my SH7372 documentation, and should be set
> to 0. Won't this break compatibility with the SH7372 ?

It's flcm*D*cr not flcmncr. Took me some time to separate these
beautiful abbreviations too ;)
It's to tell the flctl to save the read in the fifo instead of a register.

>>               addr_len_bytes = ADRCNT_1;
>>               break;
>>       case NAND_CMD_STATUS:
>> @@ -559,12 +560,18 @@ static void flctl_cmdfunc(struct mtd_info *mtd,
>> unsigned int command,
>>
>>       case NAND_CMD_READID:
>>               set_cmd_regs(mtd, command, command);
>> -             set_addr(mtd, 0, 0);
>>
>> -             flctl->read_bytes = 4;
>> +             /* READID is always performed using an 8-bit bus */
>> +             if (flctl->chip.options & NAND_BUSWIDTH_16)
>> +                     column <<= 1;
>> +             set_addr(mtd, column, 0);
>> +
>> +             flctl->read_bytes = 8;
>>               writel(flctl->read_bytes, FLDTCNTR(flctl)); /* set read size */
>> +             empty_fifo(flctl);
>>               start_translation(flctl);
>> -             read_datareg(flctl, 0); /* read and end */
>> +             read_fiforeg(flctl, flctl->read_bytes, 0);
>
> Not saying it's a bad thing, but why do you switch from read_datareg() to
> read_fiforeg() ?

We can only read 4 bytes from a register read, while we need 8 bytes
for the ID. The extra 4 bytes are used to determine the correct NAND
chip size in nand_base.c.

>> +             wait_completion(flctl);
>>               break;
>>
>>       case NAND_CMD_ERASE1:
> --
> Regards,
>
> Laurent Pinchart

thanks,

 Bastian

Patch

diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index 8c97367..407acb5 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -320,6 +320,7 @@  static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_va
 		break;
 	case NAND_CMD_READID:
 		flcmncr_val &= ~SNAND_E;
+		flcmdcr_val |= CDSRC_E;
 		addr_len_bytes = ADRCNT_1;
 		break;
 	case NAND_CMD_STATUS:
@@ -559,12 +560,18 @@  static void flctl_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 	case NAND_CMD_READID:
 		set_cmd_regs(mtd, command, command);
-		set_addr(mtd, 0, 0);
 
-		flctl->read_bytes = 4;
+		/* READID is always performed using an 8-bit bus */
+		if (flctl->chip.options & NAND_BUSWIDTH_16)
+			column <<= 1;
+		set_addr(mtd, column, 0);
+
+		flctl->read_bytes = 8;
 		writel(flctl->read_bytes, FLDTCNTR(flctl)); /* set read size */
+		empty_fifo(flctl);
 		start_translation(flctl);
-		read_datareg(flctl, 0);	/* read and end */
+		read_fiforeg(flctl, flctl->read_bytes, 0);
+		wait_completion(flctl);
 		break;
 
 	case NAND_CMD_ERASE1: