From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bartlomiej Sieka <tur@semihalf.com>
Date: Tue, 11 Dec 2007 15:25:25 +0100
Subject: [U-Boot-Users] [PATCH] CFI: synchronize command offsets with
 Linux CFI driver
In-Reply-To: <200712111511.49605.sr@denx.de>
References: <20071211125955.GB27980@frozen.semihalf.com>
	<200712111511.49605.sr@denx.de>
Message-ID: <475E9DD5.3040108@semihalf.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Stefan Roese wrote:
[...]
>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>> index 66754cd..b748a9f 100644
>> --- a/drivers/mtd/cfi_flash.c
>> +++ b/drivers/mtd/cfi_flash.c
>> @@ -1231,9 +1231,26 @@ static int flash_detect_cfi (flash_info_t * info)
>>  					debug ("port %d bits chip %d bits\n",
>>  						info->portwidth << CFI_FLASH_SHIFT_WIDTH,
>>  						info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
>> -					/* this probably only works if info->interface == FLASH_CFI_X8X16 */
>> -					info->addr_unlock1 = (info->portwidth == FLASH_CFI_8BIT) ? 0xAAA :
>> 0x555; -					info->addr_unlock2 = (info->portwidth == FLASH_CFI_8BIT) ?
>> 0x555 : 0x2AA; +
>> +					/* calculate command offsets as in the Linux driver */
>> +					info->addr_unlock1 = 0x555;
>> +					info->addr_unlock2 = 0x2aa;
>> +
>> +					/*
>> +					 * modify the unlock address if we are
>> +					 * in compatibility mode
>> +					 */
>> +					if (	/* x8/x16 in x8 mode */
>> +						((info->chipwidth == FLASH_CFI_BY8) &&
>> +							(info->interface == FLASH_CFI_X8X16)) ||
>> +						/* x16/x32 in x16 mode */
>> +						((info->chipwidth == FLASH_CFI_BY16) &&
>> +							(info->interface == FLASH_CFI_X16X32)))
>> +					{
>> +						info->addr_unlock1 = 0xaaa;
>> +						info->addr_unlock2 = 0x555;
>> +					}
>> +
> 
> Why not like this:
> 
> 					if (((info->chipwidth == FLASH_CFI_BY8) &&
> 					     (info->interface == FLASH_CFI_X8X16)) ||
> 					    ((info->chipwidth == FLASH_CFI_BY16) &&
> 					     (info->interface == FLASH_CFI_X16X32))) {
> 						/*
> 						 * Compatibility mode:
> 						 * x8/x16 in x8 mode or
> 						 * x16/x32 in x16 mode
> 						 */
> 						info->addr_unlock1 = 0xaaa;
> 						info->addr_unlock2 = 0x555;
> 					} else {
> 						/* default */
> 						info->addr_unlock1 = 0x555;
> 						info->addr_unlock2 = 0x2aa;
> 					}

I copied the code from Linux CFI driver and kept it as is (modulo
#define names). I'm fine with either of the two forms, although I think
that there is some benefit in keeping borrowed code as intact as possible.

Regards,
Bartlomiej