[U-Boot-Users] [PATCH] (Re-re-submited) Added support to flash_real_protect for Atmel flash devices (tested with AT49BV6416)

[U-Boot-Users] [PATCH] (Re-re-submited) Added support to flash_real_protect for Atmel flash devices (tested with AT49BV6416)

[Rafael Campos]

Some of the flash memories produced by ATMEL start in read-only mode. We
need to unprotect it.
This patch allows the AT49BV6416 to work with cfi_flash memories. Tested
in the at91rm9200ek board.


Signed-off-by: Rafael Campos Las Heras <rafael.campos@hanscan.com>
---
 drivers/mtd/cfi_flash.c |   40 ++++++++++++++++++++++++++++++++++------
 1 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 4340b1b..8d12eb8 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -103,6 +103,10 @@
 #define AMD_STATUS_TOGGLE		0x40
 #define AMD_STATUS_ERROR		0x20
 
+#define ATM_CMD_UNLOCK_SECT		0x70
+#define ATM_CMD_SOFTLOCK_START		0x80
+#define ATM_CMD_LOCK_SECT		0x40
+
 #define FLASH_OFFSET_MANUFACTURER_ID	0x00
 #define FLASH_OFFSET_DEVICE_ID		0x01
 #define FLASH_OFFSET_DEVICE_ID2		0x0E
@@ -1348,12 +1352,36 @@ int flash_real_protect (flash_info_t * info, long sector, int prot)
 {
 	int retcode = 0;
 
-	flash_write_cmd (info, sector, 0, FLASH_CMD_CLEAR_STATUS);
-	flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT);
-	if (prot)
-		flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_SET);
-	else
-		flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_CLEAR);
+	switch (info->vendor) {
+		case CFI_CMDSET_INTEL_PROG_REGIONS:
+		case CFI_CMDSET_INTEL_STANDARD:
+			flash_write_cmd (info, sector, 0, FLASH_CMD_CLEAR_STATUS);
+			flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT);
+			if (prot)
+				flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_SET);
+			else
+				flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_CLEAR);
+			break;
+		case CFI_CMDSET_AMD_EXTENDED:
+		case CFI_CMDSET_AMD_STANDARD:
+#ifdef CONFIG_FLASH_CFI_LEGACY
+		case CFI_CMDSET_AMD_LEGACY:
+#endif
+		/* U-Boot only checks the first byte */
+		if (info->manufacturer_id == (uchar)ATM_MANUFACT ) {
+			if (prot) {
+				flash_unlock_seq(info, 0);
+				flash_write_cmd (info, 0, info->addr_unlock1, ATM_CMD_SOFTLOCK_START);
+				flash_unlock_seq(info, 0);
+				flash_write_cmd (info, sector, 0, ATM_CMD_LOCK_SECT);
+			} else {
+				flash_write_cmd (info, 0, info->addr_unlock1, AMD_CMD_UNLOCK_START);
+				if (info->device_id == ATM_ID_BV6416)
+					flash_write_cmd (info, sector, 0, ATM_CMD_UNLOCK_SECT);
+			}
+		}
+		break;
+	}
 
 	if ((retcode =
 	     flash_full_status_check (info, sector, info->erase_blk_tout,
-- 
1.5.6.2

[U-Boot-Users] [PATCH] (Re-re-submited) Added support to flash_real_protect for Atmel flash devices (tested with AT49BV6416)

[Wolfgang Denk]

In message <1217414143.29498.8.camel@ls06.HANSCAN.local> you wrote:
> Some of the flash memories produced by ATMEL start in read-only mode. We
> need to unprotect it.
> This patch allows the AT49BV6416 to work with cfi_flash memories. Tested
> in the at91rm9200ek board.

This is your 5th posting to the very same subject, without threading,
without patch version numbers, nor with any other indication why  you
are submitting this.

I'm sorry, but I have to  tell  you  that  it  is  becoming  a  major
nuisance  to track all your patches and find out what we are supposed
to do with these.

Please try being a bit more efficient here on the list.

Please see also the private mail I sent you before.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I don't want to be young again, I just don't want to get any older.

[U-Boot-Users] [PATCH] (Re-re-submited) Added support to flash_real_protect for Atmel flash devices (tested with AT49BV6416)

[Stefan Roese]

Rafael,

On Wednesday 30 July 2008, Rafael Campos wrote:
> Some of the flash memories produced by ATMEL start in read-only mode. We
> need to unprotect it.
> This patch allows the AT49BV6416 to work with cfi_flash memories. Tested
> in the at91rm9200ek board.

Thanks. I finally find the time to review this patch. Sorry for the delay. But 
as Wolfgang already mentioned, resending the same patch over and over again 
is not the best way to handle things. Please find some nitpicking comments 
below.

> Signed-off-by: Rafael Campos Las Heras <rafael.campos@hanscan.com>
> ---
>  drivers/mtd/cfi_flash.c |   40 ++++++++++++++++++++++++++++++++++------
>  1 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 4340b1b..8d12eb8 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -103,6 +103,10 @@
>  #define AMD_STATUS_TOGGLE		0x40
>  #define AMD_STATUS_ERROR		0x20
>
> +#define ATM_CMD_UNLOCK_SECT		0x70
> +#define ATM_CMD_SOFTLOCK_START		0x80
> +#define ATM_CMD_LOCK_SECT		0x40
> +
>  #define FLASH_OFFSET_MANUFACTURER_ID	0x00
>  #define FLASH_OFFSET_DEVICE_ID		0x01
>  #define FLASH_OFFSET_DEVICE_ID2		0x0E
> @@ -1348,12 +1352,36 @@ int flash_real_protect (flash_info_t * info, long
> sector, int prot) {
>  	int retcode = 0;
>
> -	flash_write_cmd (info, sector, 0, FLASH_CMD_CLEAR_STATUS);
> -	flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT);
> -	if (prot)
> -		flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_SET);
> -	else
> -		flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_CLEAR);
> +	switch (info->vendor) {
> +		case CFI_CMDSET_INTEL_PROG_REGIONS:
> +		case CFI_CMDSET_INTEL_STANDARD:
> +			flash_write_cmd (info, sector, 0, FLASH_CMD_CLEAR_STATUS);
> +			flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT);
> +			if (prot)
> +				flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_SET);
> +			else
> +				flash_write_cmd (info, sector, 0, FLASH_CMD_PROTECT_CLEAR);
> +			break;
> +		case CFI_CMDSET_AMD_EXTENDED:
> +		case CFI_CMDSET_AMD_STANDARD:
> +#ifdef CONFIG_FLASH_CFI_LEGACY
> +		case CFI_CMDSET_AMD_LEGACY:
> +#endif
> +		/* U-Boot only checks the first byte */
> +		if (info->manufacturer_id == (uchar)ATM_MANUFACT ) {

Drop the space before ')'.

> +			if (prot) {
> +				flash_unlock_seq(info, 0);
> +				flash_write_cmd (info, 0, info->addr_unlock1, ATM_CMD_SOFTLOCK_START);

This line gets pretty long. Please wrap this (and others) line that exceed the 
80 chars.

> +				flash_unlock_seq(info, 0);
> +				flash_write_cmd (info, sector, 0, ATM_CMD_LOCK_SECT);

Decide for one coding style: func () or func(). As this file used the func () 
version, please use it all the time.

> +			} else {
> +				flash_write_cmd (info, 0, info->addr_unlock1, AMD_CMD_UNLOCK_START);
> +				if (info->device_id == ATM_ID_BV6416)
> +					flash_write_cmd (info, sector, 0, ATM_CMD_UNLOCK_SECT);
> +			}
> +		}
> +		break;
> +	}
>
>  	if ((retcode =
>  	     flash_full_status_check (info, sector, info->erase_blk_tout,

BTW: Did you test this patch with other FLASH chips too (Intel, Spansion ...)? 
Or only the Atmel one?

And please CC me directly on CFI related patches.

Thanks.

Best regards,
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 at denx.de
=====================================================================

[U-Boot-Users] Comments to the patch

[Rafael Campos]

El mi?, 30-07-2008 a las 14:25 +0200, Stefan Roese escribi?:
> BTW: Did you test this patch with other FLASH chips too (Intel,
> Spansion ...)? 
> Or only the Atmel one?

Only with the ATMEL one (AT49BV6416), which is the only one that i have
access to. But i checked several other ATMEL flash datasheets and said
that they start in read-only mode. 

Rafael Campos Las Heras.
Dpto. Ingenieria
 
rafael.campos at hanscan.com

___________________________________________
Hanscan Spain S.A.
Avda. de la Vega 1 Edif. Veganova 2-3 ?
28108 Alcobendas (Madrid) Spain
Telf. +34 902 636 551
FAX. +34 902 636 552
www.hanscan.com - info at hanscan.com
___________________________________________

[U-Boot-Users] Comments to the patch

[Haavard Skinnemoen]

Rafael Campos <rafael.campos@hanscan.com> wrote:
> El mi?, 30-07-2008 a las 14:25 +0200, Stefan Roese escribi?:
> > BTW: Did you test this patch with other FLASH chips too (Intel,
> > Spansion ...)? 
> > Or only the Atmel one?  
> 
> Only with the ATMEL one (AT49BV6416), which is the only one that i have
> access to. But i checked several other ATMEL flash datasheets and said
> that they start in read-only mode.

Of course they start in read-only mode -- it's flash after all ;-)

But I don't think all Atmel flash needs this treatment. AT49BV6416 is
kinda special -- the replacement, AT49BV642D does not need this.

The ones using Intel command set usually do need to be unlocked, but
this is standard behaviour for Intel-compatible flash I think.

Btw, also note that since the CFI driver only reads the low 8 bits of
the device ID, there's currently no way to tell AT49BV6416 and
AT49BV642D apart. Or at least it wasn't last time I checked. So I think
this needs fixing first.

Oh, and you also need to reverse the erase regions reported by this
chip.

Haavard