[U-Boot] [PATCH 1/4] spiflash: at25: using common spi flash operation

[Andreas Bießmann]

Dear Bo Shen,

On 22.08.2012 03:33, Bo Shen wrote:
> Hi Andreas,
> 
> On 8/21/2012 19:26, Andreas Bießmann wrote:
>> Dear Bo Shen,
>>
>> On 20.08.2012 08:32, Bo Shen wrote:
>>> Using common spi flash operation function to replace private operation
>>> funtion
>>>
>>> This patch is based on http://patchwork.ozlabs.org/patch/177896/
>>> which has been merged by Mike frysinger
>>
>> Mike, do you think this is a fix? Should it go into 2012.10? Will you
>> take it?
>>
>>> Signed-off-by: Bo Shen <voice.shen at atmel.com>
>>> ---
>>>   drivers/mtd/spi/atmel.c |   11 ++++++++---
>>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi/atmel.c b/drivers/mtd/spi/atmel.c
>>> index 89ebe9d..006f6d5 100644
>>> --- a/drivers/mtd/spi/atmel.c
>>> +++ b/drivers/mtd/spi/atmel.c
>>> @@ -518,13 +518,19 @@ struct spi_flash *spi_flash_probe_atmel(struct
>>> spi_slave *spi, u8 *idcode)
>>>               asf->flash.erase = dataflash_erase_p2;
>>>           }
>>>
>>> +        asf->flash.page_size = page_size;
>>> +        asf->flash.sector_size = page_size;
>>>           break;
>>>
>>>       case DF_FAMILY_AT26F:
>>>       case DF_FAMILY_AT26DF:
>>>           asf->flash.read = spi_flash_cmd_read_fast;
>>> -        asf->flash.write = dataflash_write_p2;
>>> -        asf->flash.erase = dataflash_erase_p2;
>>> +        asf->flash.write = spi_flash_cmd_write_multi;
>>> +        asf->flash.erase = spi_flash_cmd_erase;
>>> +        asf->flash.page_size = page_size;
>>> +        asf->flash.sector_size = 4096;
>>
>> why do you take fixed value for sector size here?
> 
> The fixed number fit for both AT25 and AT26 serials. So, I take it.
> 
> Actually, we can calculate it as:
q> asf->flash.sector_size = page_size * params->pages_per_block *
> params->blocks_per_sector
> So, may I need to change it?

I got it. The current driver for AT45 is written to only support page
erase, therefore the page_size == sector_size.
The AT26 on the other hand will now use the generic API which uses
sector erase instead. Therefore we have different values at all, am I right?

The only supported DF of at26 type is currently [1] which in fact has
4KiB erase size. Unfortunately others (at least [2]) have another
layout. But this could be adopted if such a chip is supported in the future.

>>
>>> +        /* clear SPRL# bit for locked flash */
>>> +        spi_flash_cmd_write_status(&asf->flash, 0);
>>>           break;
>>>
>>>       default:
>>> @@ -532,7 +538,6 @@ struct spi_flash *spi_flash_probe_atmel(struct
>>> spi_slave *spi, u8 *idcode)
>>>           goto err;
>>>       }
>>>
>>> -    asf->flash.sector_size = page_size;
>>>       asf->flash.size = page_size * params->pages_per_block
>>>                   * params->blocks_per_sector
>>>                   * params->nr_sectors;
>>
>> And here you correlate number of sectors with page size ... but we may
>> have page_size != sector_size for some at26 devices, aren't we?
> 
> This is from old driver and this line is moved away.
> For at26 devices, it is used the fixed number 4096 for sector size.
> Please take following as a reference.
> ---------------------------------------------
>        case DF_FAMILY_AT26F:
>     case DF_FAMILY_AT26DF:
>            asf->flash.read = spi_flash_cmd_read_fast;
>  -        asf->flash.write = dataflash_write_p2;
>  -        asf->flash.erase = dataflash_erase_p2;
>  +        asf->flash.write = spi_flash_cmd_write_multi;
>  +        asf->flash.erase = spi_flash_cmd_erase;
>  +        asf->flash.page_size = page_size;
>  +        asf->flash.sector_size = 4096;
> ---------------------------------------------

Yea, I got that already. The only thing I asked why you correlate these
sector related parameters (blocks_per_sector and nr_sectors) with the
page_size.

I think this change is Ok.

Acked-by: Andreas Bießmann <andreas.devel at googlemail.com>

@Mike: As I understand you I should take patch #2 to #4 while you will
take this one, am I right?

[1] http://www.atmel.com/Images/doc3633.pdf
[2] http://www.atmel.com/Images/doc3495.pdf