[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 at 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
=====================================================================