public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: "Andreas Bießmann" <andreas.devel@googlemail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/4] spiflash: at25: using common spi flash operation
Date: Mon, 27 Aug 2012 13:29:12 +0200	[thread overview]
Message-ID: <503B5A08.2060406@gmail.com> (raw)
In-Reply-To: <503436D0.9090600@atmel.com>

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@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@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

  reply	other threads:[~2012-08-27 11:29 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-20  6:32 [U-Boot] [PATCH 1/4] spiflash: at25: using common spi flash operation Bo Shen
2012-08-20  6:32 ` [U-Boot] [PATCH 2/4] spi: atmel: add WDRBT bit to avoid receive overrun Bo Shen
2012-08-21 11:11   ` Andreas Bießmann
2012-08-21 18:56     ` Mike Frysinger
2012-08-24  9:46       ` Bo Shen
2012-08-27 10:46       ` Andreas Bießmann
2012-08-20  6:32 ` [U-Boot] [PATCH 3/4] atmel: at91sam9x5: fix name error for spi Bo Shen
2012-08-21 11:04   ` Andreas Bießmann
2012-08-20  6:32 ` [U-Boot] [PATCH 4/4] atmel: at91sam9x5: add spi flash boot support Bo Shen
2012-08-21 11:26 ` [U-Boot] [PATCH 1/4] spiflash: at25: using common spi flash operation Andreas Bießmann
2012-08-21 18:55   ` Mike Frysinger
2012-08-24  9:51     ` Bo Shen
2012-08-22  1:33   ` Bo Shen
2012-08-27 11:29     ` Andreas Bießmann [this message]
2012-09-17 17:03 ` Andreas Bießmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503B5A08.2060406@gmail.com \
    --to=andreas.devel@googlemail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox