All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.