Patchwork [v2,1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members

login
register
mail settings
Submitter Guillaume LECERF
Date March 30, 2010, 1:34 p.m.
Message ID <20100330133448.20107.1077.stgit@shiryu.yomgui.biz>
Download mbox | patch
Permalink /patch/48960/
State New
Headers show

Comments

Guillaume LECERF - March 30, 2010, 1:34 p.m.
Move the code to enter Auto Select Mode down to be able to use cfi->cfiq
members to add support for chips using alternative sequence / unlock
addresses.

Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
 drivers/mtd/chips/cfi_probe.c |   46 +++++++++++++++++++++--------------------
 1 files changed, 23 insertions(+), 23 deletions(-)
Artem Bityutskiy - March 31, 2010, 1:28 p.m.
On Tue, 2010-03-30 at 15:34 +0200, Guillaume LECERF wrote:
> Move the code to enter Auto Select Mode down to be able to use cfi->cfiq
> members to add support for chips using alternative sequence / unlock
> addresses.
> 
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>

Pushed the series to the "dunno" branch of my l2-mtd-2.6.git tree,
thanks.
Guillaume LECERF - April 2, 2010, 11:46 a.m.
2010/3/31 Artem Bityutskiy <dedekind1@gmail.com>:
> Pushed the series to the "dunno" branch of my l2-mtd-2.6.git tree,
> thanks.

Thanks.
May I ask you what is the status of this branch ? Do you plan to merge
it to mtd-2.6 (and finally to the official kernel.git) ?
Artem Bityutskiy - April 2, 2010, 12:11 p.m.
On Fri, 2010-04-02 at 13:46 +0200, Guillaume LECERF wrote:
> 2010/3/31 Artem Bityutskiy <dedekind1@gmail.com>:
> > Pushed the series to the "dunno" branch of my l2-mtd-2.6.git tree,
> > thanks.
> 
> Thanks.
> May I ask you what is the status of this branch ? Do you plan to merge
> it to mtd-2.6 (and finally to the official kernel.git) ?

dwwm2 should at some point look at that branch and take your patches,
and then merge them upstream, unless he does not like something, in
which case I guess he should let you know what is wrong.
Wolfram Sang - April 8, 2010, 8:59 a.m.
Hi Guillaume,

thanks for this patch series! I will test it right away.
A few comments/questions after having a look at the code:

On Tue, Mar 30, 2010 at 03:34:48PM +0200, Guillaume LECERF wrote:
> Move the code to enter Auto Select Mode down to be able to use cfi->cfiq
> members to add support for chips using alternative sequence / unlock
> addresses.
> 
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> ---
>  drivers/mtd/chips/cfi_probe.c |   46 +++++++++++++++++++++--------------------
>  1 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
> index e63e674..9b511ef 100644
> --- a/drivers/mtd/chips/cfi_probe.c
> +++ b/drivers/mtd/chips/cfi_probe.c
> @@ -181,29 +181,6 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>  	for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
>  		((unsigned char *)cfi->cfiq)[i] = cfi_read_query(map,base + (0x10 + i)*ofs_factor);
>  
> -	/* Note we put the device back into Read Mode BEFORE going into Auto
> -	 * Select Mode, as some devices support nesting of modes, others
> -	 * don't. This way should always work.
> -	 * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
> -	 * so should be treated as nops or illegal (and so put the device
> -	 * back into Read Mode, which is a nop in this case).
> -	 */
> -	cfi_send_gen_cmd(0xf0,     0, base, map, cfi, cfi->device_type, NULL);
> -	cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
> -	cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
> -	cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
> -	cfi->mfr = cfi_read_query16(map, base);
> -	cfi->id = cfi_read_query16(map, base + ofs_factor);
> -
> -	/* Get AMD/Spansion extended JEDEC ID */
> -	if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
> -		cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
> -			  cfi_read_query(map, base + 0xf * ofs_factor);
> -
> -	/* Put it back into Read Mode */
> -	cfi_qry_mode_off(base, map, cfi);
> -	xip_allowed(base, map);
> -
>  	/* Do any necessary byteswapping */
>  	cfi->cfiq->P_ID = le16_to_cpu(cfi->cfiq->P_ID);
>  
> @@ -228,6 +205,29 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>  #endif
>  	}
>  
> +	/* Note we put the device back into Read Mode BEFORE going into Auto
> +	 * Select Mode, as some devices support nesting of modes, others
> +	 * don't. This way should always work.
> +	 * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
> +	 * so should be treated as nops or illegal (and so put the device
> +	 * back into Read Mode, which is a nop in this case).
> +	 */

Very minor: If you touch this code anyhow, maybe you could adapt it to
standard kernel-multiline comments?

> +	cfi_send_gen_cmd(0xf0,     0, base, map, cfi, cfi->device_type, NULL);
> +	cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
> +	cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
> +	cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
> +	cfi->mfr = cfi_read_query16(map, base);
> +	cfi->id = cfi_read_query16(map, base + ofs_factor);
> +
> +	/* Get AMD/Spansion extended JEDEC ID */
> +	if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
> +		cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
> +			  cfi_read_query(map, base + 0xf * ofs_factor);
> +
> +	/* Put it back into Read Mode */
> +	cfi_qry_mode_off(base, map, cfi);
> +	xip_allowed(base, map);
> +
>  	printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit bank\n",
>  	       map->name, cfi->interleave, cfi->device_type*8, base,
>  	       map->bankwidth*8);
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Patch

diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index e63e674..9b511ef 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -181,29 +181,6 @@  static int __xipram cfi_chip_setup(struct map_info *map,
 	for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
 		((unsigned char *)cfi->cfiq)[i] = cfi_read_query(map,base + (0x10 + i)*ofs_factor);
 
-	/* Note we put the device back into Read Mode BEFORE going into Auto
-	 * Select Mode, as some devices support nesting of modes, others
-	 * don't. This way should always work.
-	 * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
-	 * so should be treated as nops or illegal (and so put the device
-	 * back into Read Mode, which is a nop in this case).
-	 */
-	cfi_send_gen_cmd(0xf0,     0, base, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
-	cfi->mfr = cfi_read_query16(map, base);
-	cfi->id = cfi_read_query16(map, base + ofs_factor);
-
-	/* Get AMD/Spansion extended JEDEC ID */
-	if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
-		cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
-			  cfi_read_query(map, base + 0xf * ofs_factor);
-
-	/* Put it back into Read Mode */
-	cfi_qry_mode_off(base, map, cfi);
-	xip_allowed(base, map);
-
 	/* Do any necessary byteswapping */
 	cfi->cfiq->P_ID = le16_to_cpu(cfi->cfiq->P_ID);
 
@@ -228,6 +205,29 @@  static int __xipram cfi_chip_setup(struct map_info *map,
 #endif
 	}
 
+	/* Note we put the device back into Read Mode BEFORE going into Auto
+	 * Select Mode, as some devices support nesting of modes, others
+	 * don't. This way should always work.
+	 * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
+	 * so should be treated as nops or illegal (and so put the device
+	 * back into Read Mode, which is a nop in this case).
+	 */
+	cfi_send_gen_cmd(0xf0,     0, base, map, cfi, cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
+	cfi->mfr = cfi_read_query16(map, base);
+	cfi->id = cfi_read_query16(map, base + ofs_factor);
+
+	/* Get AMD/Spansion extended JEDEC ID */
+	if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
+		cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
+			  cfi_read_query(map, base + 0xf * ofs_factor);
+
+	/* Put it back into Read Mode */
+	cfi_qry_mode_off(base, map, cfi);
+	xip_allowed(base, map);
+
 	printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit bank\n",
 	       map->name, cfi->interleave, cfi->device_type*8, base,
 	       map->bankwidth*8);