diff mbox

[U-Boot] cfi_flash driver: fixed addressing for 8-bit flash buses.

Message ID 1302594796-26276-1-git-send-email-aaron.williams@caviumnetworks.com
State Changes Requested
Delegated to: Stefan Roese
Headers show

Commit Message

Aaron Williams April 12, 2011, 7:53 a.m. UTC 
This patch corrects the addresses used when working with Spansion/AMD FLASH
chips.  Addressing for 8 and 16 bits is almost identical except in the 16
bit case the LSB of the address is always 0.  The confusion arose because
the addresses in the datasheet for the 16 bit mode are word addresses but
the CFI driver assumed they were byte addresses.

I have only been able to test this on our Octeon boards which use either 
an 8 or 16 bit bus.  I have not tested 32-bits or when there are multiple
parts in parallel on a wider bus.

Several other people have also reported success with this patch.

-Aaron Williams


Signed-off-by: Aaron Williams <aaron.williams@caviumnetworks.com>
---
 drivers/mtd/cfi_flash.c |   66 +++++++++++++++++++++++++++++++---------------
 include/mtd/cfi_flash.h |   43 ++++++++++++++++--------------
 2 files changed, 67 insertions(+), 42 deletions(-)

Comments

Albert ARIBAUD April 12, 2011, 8:14 a.m. UTC | #1 
Hi Aaron,

Le 12/04/2011 09:53, Aaron Williams a écrit :
> This patch corrects the addresses used when working with Spansion/AMD FLASH
> chips.  Addressing for 8 and 16 bits is almost identical except in the 16
> bit case the LSB of the address is always 0.  The confusion arose because
> the addresses in the datasheet for the 16 bit mode are word addresses but
> the CFI driver assumed they were byte addresses.
>
> I have only been able to test this on our Octeon boards which use either
> an 8 or 16 bit bus.  I have not tested 32-bits or when there are multiple
> parts in parallel on a wider bus.
>
> Several other people have also reported success with this patch.
>
> -Aaron Williams
>
>
> Signed-off-by: Aaron Williams<aaron.williams@caviumnetworks.com>
> ---
>   drivers/mtd/cfi_flash.c |   66 +++++++++++++++++++++++++++++++---------------
>   include/mtd/cfi_flash.h |   43 ++++++++++++++++--------------
>   2 files changed, 67 insertions(+), 42 deletions(-)

That's V2 of http://patchwork.ozlabs.org/patch/89429/, isn't it ?
Then it should have V2 tag and patch history.

Amicalement,
Wolfgang Denk April 30, 2011, 7:12 p.m. UTC | #2 
Dear Aaron Williams,

In message <1302594796-26276-1-git-send-email-aaron.williams@caviumnetworks.com> you wrote:
> This patch corrects the addresses used when working with Spansion/AMD FLASH
> chips.  Addressing for 8 and 16 bits is almost identical except in the 16
> bit case the LSB of the address is always 0.  The confusion arose because
> the addresses in the datasheet for the 16 bit mode are word addresses but
> the CFI driver assumed they were byte addresses.
> 
> I have only been able to test this on our Octeon boards which use either 
> an 8 or 16 bit bus.  I have not tested 32-bits or when there are multiple
> parts in parallel on a wider bus.
> 
> Several other people have also reported success with this patch.
> 
> -Aaron Williams
> 
> 
> Signed-off-by: Aaron Williams <aaron.williams@caviumnetworks.com>
> ---
>  drivers/mtd/cfi_flash.c |   66 +++++++++++++++++++++++++++++++---------------
>  include/mtd/cfi_flash.h |   43 ++++++++++++++++--------------
>  2 files changed, 67 insertions(+), 42 deletions(-)

This patch has a number of coding style isses (trailing whitespace,
space between function name and open parenthesis, line over 80
characters, etc.)

Please clean up and resubmit (after verifying the patch using
checkpatch).

Best regards,

Wolfgang Denk
Aaron Williams April 30, 2011, 7:14 p.m. UTC | #3 
Will do. I've been quite busy lately bringing up a 32-core chip and
getting ready for a release but I should have some time now.

I've got some other patches I also need to submit.

-Aaron

On 04/30/2011 12:12 PM, Wolfgang Denk wrote:
> Dear Aaron Williams,
>
> In message <1302594796-26276-1-git-send-email-aaron.williams@caviumnetworks.com> you wrote:
>> This patch corrects the addresses used when working with Spansion/AMD FLASH
>> chips.  Addressing for 8 and 16 bits is almost identical except in the 16
>> bit case the LSB of the address is always 0.  The confusion arose because
>> the addresses in the datasheet for the 16 bit mode are word addresses but
>> the CFI driver assumed they were byte addresses.
>>
>> I have only been able to test this on our Octeon boards which use either 
>> an 8 or 16 bit bus.  I have not tested 32-bits or when there are multiple
>> parts in parallel on a wider bus.
>>
>> Several other people have also reported success with this patch.
>>
>> -Aaron Williams
>>
>>
>> Signed-off-by: Aaron Williams <aaron.williams@caviumnetworks.com>
>> ---
>>  drivers/mtd/cfi_flash.c |   66 +++++++++++++++++++++++++++++++---------------
>>  include/mtd/cfi_flash.h |   43 ++++++++++++++++--------------
>>  2 files changed, 67 insertions(+), 42 deletions(-)
> This patch has a number of coding style isses (trailing whitespace,
> space between function name and open parenthesis, line over 80
> characters, etc.)
>
> Please clean up and resubmit (after verifying the patch using
> checkpatch).
>
> Best regards,
>
> Wolfgang Denk
>
Stefan Roese Sept. 30, 2011, 8:12 a.m. UTC | #4 
Aaron,

On Saturday 30 April 2011 21:14:49 Aaron Williams wrote:
> Will do. I've been quite busy lately bringing up a 32-core chip and
> getting ready for a release but I should have some time now.
> 
> I've got some other patches I also need to submit.

Is this patch still on your list for a re-spin? IIRC, it fixes a general 
problem with 8-bit devices. Or is it not needed any more?

Thanks,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
diff mbox

Patch

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 5788328..9ee639e 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -10,6 +10,9 @@ 
  *
  * Copyright (C) 2006
  * Tolunay Orkun <listmember@orkun.us>
+ * 
+ * Copyright (C) 2011 Cavium Networks, Inc.
+ * Aaron Williams <Aaron.Williams@caviumnetworks.com>
  *
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -209,9 +212,11 @@  unsigned long flash_sector_size(flash_info_t *info, flash_sect_t sect)
 static inline void *
 flash_map (flash_info_t * info, flash_sect_t sect, uint offset)
 {
-	unsigned int byte_offset = offset * info->portwidth;
-
-	return (void *)(info->start[sect] + byte_offset);
+	unsigned int byte_offset = offset * info->portwidth / info->chipwidth;
+	unsigned int addr = (info->start[sect] + byte_offset);
+	unsigned int mask = 0xffffffff << (info->portwidth - 1);
+	
+	return (void *)(addr & mask);
 }
 
 static inline void flash_unmap(flash_info_t *info, flash_sect_t sect,
@@ -1082,6 +1087,7 @@  int flash_erase (flash_info_t * info, int s_first, int s_last)
 				break;
 			case CFI_CMDSET_AMD_STANDARD:
 			case CFI_CMDSET_AMD_EXTENDED:
+				flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
 				flash_unlock_seq (info, sect);
 				flash_write_cmd (info, sect,
 						info->addr_unlock1,
@@ -1719,7 +1725,7 @@  static void flash_read_cfi (flash_info_t *info, void *buf,
 	unsigned int i;
 
 	for (i = 0; i < len; i++)
-		p[i] = flash_read_uchar(info, start + i);
+		p[i] = flash_read_uchar(info, start + (i * 2));
 }
 
 void __flash_cmd_reset(flash_info_t *info)
@@ -1739,21 +1745,38 @@  static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry)
 {
 	int cfi_offset;
 
-	/* Issue FLASH reset command */
-	flash_cmd_reset(info);
-
 	for (cfi_offset=0;
 	     cfi_offset < sizeof(flash_offset_cfi) / sizeof(uint);
 	     cfi_offset++) {
+		/* Issue FLASH reset command */
+		flash_cmd_reset(info);
 		flash_write_cmd (info, 0, flash_offset_cfi[cfi_offset],
 				 FLASH_CMD_CFI);
 		if (flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP, 'Q')
-		    && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 1, 'R')
-		    && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 2, 'Y')) {
+		    && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 2, 'R')
+		    && flash_isequal (info, 0, FLASH_OFFSET_CFI_RESP + 4, 'Y')) {
 			flash_read_cfi(info, qry, FLASH_OFFSET_CFI_RESP,
 					sizeof(struct cfi_qry));
+#ifdef CONFIG_SYS_FLASH_INTERFACE_WIDTH
+			info->interface = CONFIG_SYS_FLASH_INTERFACE_WIDTH;
+#else
 			info->interface	= le16_to_cpu(qry->interface_desc);
-
+			/* Some flash chips can support multiple bus widths.
+			 * In this case, override the interface width and
+			 * limit it to the port width.
+			 */
+			if ((info->interface == FLASH_CFI_X8X16) &&
+			    (info->portwidth == FLASH_CFI_8BIT)) {
+				debug("Overriding 16-bit interface width to "
+				      " 8-bit port width.\n");
+				info->interface = FLASH_CFI_X8;
+			} else if ((info->interface == FLASH_CFI_X16X32) &&
+				 (info->portwidth == FLASH_CFI_16BIT)) {
+				debug("Overriding 16-bit interface width to "
+				      " 16-bit port width.\n");
+				info->interface = FLASH_CFI_X16;
+			}
+#endif
 			info->cfi_offset = flash_offset_cfi[cfi_offset];
 			debug ("device interface is %d\n",
 			       info->interface);
@@ -1764,8 +1787,8 @@  static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry)
 			       info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
 
 			/* calculate command offsets as in the Linux driver */
-			info->addr_unlock1 = 0x555;
-			info->addr_unlock2 = 0x2aa;
+			info->addr_unlock1 = 0xaaa;
+			info->addr_unlock2 = 0x555;
 
 			/*
 			 * modify the unlock address if we are
@@ -1819,7 +1842,7 @@  static void flash_fixup_amd(flash_info_t *info, struct cfi_qry *qry)
 			/* CFI < 1.1, try to guess from device id */
 			if ((info->device_id & 0x80) != 0)
 				cfi_reverse_geometry(qry);
-		} else if (flash_read_uchar(info, info->ext_addr + 0xf) == 3) {
+		} else if (flash_read_uchar(info, info->ext_addr + 0x1e) == 3) {
 			/* CFI >= 1.1, deduct from top/bottom flag */
 			/* note: ext_addr is valid since cfi_version > 0 */
 			cfi_reverse_geometry(qry);
@@ -1891,14 +1914,15 @@  ulong flash_get_size (phys_addr_t base, int banknum)
 
 	if (flash_detect_cfi (info, &qry)) {
 		info->vendor = le16_to_cpu(qry.p_id);
-		info->ext_addr = le16_to_cpu(qry.p_adr);
+		info->ext_addr = le16_to_cpu(qry.p_adr) * 2;
+		debug("extended address is 0x%x\n", info->ext_addr);
 		num_erase_regions = qry.num_erase_regions;
 
 		if (info->ext_addr) {
 			info->cfi_version = (ushort) flash_read_uchar (info,
-						info->ext_addr + 3) << 8;
+						info->ext_addr + 6) << 8;
 			info->cfi_version |= (ushort) flash_read_uchar (info,
-						info->ext_addr + 4);
+						info->ext_addr + 8);
 		}
 
 #ifdef DEBUG
@@ -1946,13 +1970,9 @@  ulong flash_get_size (phys_addr_t base, int banknum)
 		debug ("device id is 0x%x\n", info->device_id);
 		debug ("device id2 is 0x%x\n", info->device_id2);
 		debug ("cfi version is 0x%04x\n", info->cfi_version);
-
+		debug ("port width: %d, chipwidth: %d, interface: %d\n",
+		       info->portwidth, info->chipwidth, info->interface);
 		size_ratio = info->portwidth / info->chipwidth;
-		/* if the chip is x8/x16 reduce the ratio by half */
-		if ((info->interface == FLASH_CFI_X8X16)
-		    && (info->chipwidth == FLASH_CFI_BY8)) {
-			size_ratio >>= 1;
-		}
 		debug ("size_ratio %d port %d bits chip %d bits\n",
 		       size_ratio, info->portwidth << CFI_FLASH_SHIFT_WIDTH,
 		       info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
@@ -2024,6 +2044,8 @@  ulong flash_get_size (phys_addr_t base, int banknum)
 				sect_cnt++;
 			}
 		}
+		debug ("port width: %d, chipwidth: %d, interface: %d\n",
+		       info->portwidth, info->chipwidth, info->interface);
 
 		info->sector_count = sect_cnt;
 		info->buffer_size = 1 << le16_to_cpu(qry.max_buf_write_size);
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h
index 3245b44..932fadd 100644
--- a/include/mtd/cfi_flash.h
+++ b/include/mtd/cfi_flash.h
@@ -70,30 +70,33 @@ 
 
 #define FLASH_CONTINUATION_CODE		0x7F
 
+/* All offsets are for the 8-bit bus option.  When a 16 bit bus is used
+ * the addresses are the same except that the LSB is cleared.
+ */
 #define FLASH_OFFSET_MANUFACTURER_ID	0x00
-#define FLASH_OFFSET_DEVICE_ID		0x01
-#define FLASH_OFFSET_DEVICE_ID2		0x0E
-#define FLASH_OFFSET_DEVICE_ID3		0x0F
-#define FLASH_OFFSET_CFI		0x55
+#define FLASH_OFFSET_DEVICE_ID		0x02
+#define FLASH_OFFSET_DEVICE_ID2		0x1C
+#define FLASH_OFFSET_DEVICE_ID3		0x1E
+#define FLASH_OFFSET_CFI		0xAA
 #define FLASH_OFFSET_CFI_ALT		0x555
-#define FLASH_OFFSET_CFI_RESP		0x10
-#define FLASH_OFFSET_PRIMARY_VENDOR	0x13
+#define FLASH_OFFSET_CFI_RESP		0x20
+#define FLASH_OFFSET_PRIMARY_VENDOR	0x26
 /* extended query table primary address */
-#define FLASH_OFFSET_EXT_QUERY_T_P_ADDR	0x15
+#define FLASH_OFFSET_EXT_QUERY_T_P_ADDR	0x2A
 #define FLASH_OFFSET_WTOUT		0x1F
-#define FLASH_OFFSET_WBTOUT		0x20
-#define FLASH_OFFSET_ETOUT		0x21
-#define FLASH_OFFSET_CETOUT		0x22
-#define FLASH_OFFSET_WMAX_TOUT		0x23
-#define FLASH_OFFSET_WBMAX_TOUT		0x24
-#define FLASH_OFFSET_EMAX_TOUT		0x25
-#define FLASH_OFFSET_CEMAX_TOUT		0x26
-#define FLASH_OFFSET_SIZE		0x27
-#define FLASH_OFFSET_INTERFACE		0x28
-#define FLASH_OFFSET_BUFFER_SIZE	0x2A
-#define FLASH_OFFSET_NUM_ERASE_REGIONS	0x2C
-#define FLASH_OFFSET_ERASE_REGIONS	0x2D
-#define FLASH_OFFSET_PROTECT		0x02
+#define FLASH_OFFSET_WBTOUT		0x40
+#define FLASH_OFFSET_ETOUT		0x4A
+#define FLASH_OFFSET_CETOUT		0x44
+#define FLASH_OFFSET_WMAX_TOUT		0x46
+#define FLASH_OFFSET_WBMAX_TOUT		0x48
+#define FLASH_OFFSET_EMAX_TOUT		0x4A
+#define FLASH_OFFSET_CEMAX_TOUT		0x4C
+#define FLASH_OFFSET_SIZE		0x4E
+#define FLASH_OFFSET_INTERFACE		0x50
+#define FLASH_OFFSET_BUFFER_SIZE	0x54
+#define FLASH_OFFSET_NUM_ERASE_REGIONS	0x58
+#define FLASH_OFFSET_ERASE_REGIONS	0x5A
+#define FLASH_OFFSET_PROTECT		0x04
 #define FLASH_OFFSET_USER_PROTECTION	0x85
 #define FLASH_OFFSET_INTEL_PROTECTION	0x81