Hi Simon,
On Fri, Apr 26, 2013 at 12:22 AM, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On Thu, Apr 25, 2013 at 6:55 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Simon,
On Wed, Apr 24, 2013 at 6:03 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, Apr 23, 2013 at 3:11 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Simon,
On Wed, Apr 24, 2013 at 3:25 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Simon,
On Wed, Apr 24, 2013 at 3:10 AM, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On Tue, Apr 23, 2013 at 2:31 PM, Jagan Teki jagannadh.teki@gmail.com wrote: > Hi Simon, > > On Wed, Apr 24, 2013 at 2:48 AM, Simon Glass sjg@chromium.org wrote: >> Hi Jagan, >> >> On Tue, Apr 23, 2013 at 2:15 PM, Jagan Teki jagannadh.teki@gmail.com wrote: >>> Hi Simon, >>> >>> On Wed, Apr 24, 2013 at 2:43 AM, Simon Glass sjg@chromium.org wrote: >>>> Hi Jagan, >>>> >>>> On Tue, Apr 23, 2013 at 2:01 PM, Jagan Teki jagannadh.teki@gmail.com wrote: >>>>> Hi Simon, >>>>> >>>>> On Wed, Apr 24, 2013 at 2:20 AM, Simon Glass sjg@chromium.org wrote: >>>>>> Hi Jagan, >>>>>> >>>>>> On Tue, Apr 23, 2013 at 1:44 PM, Jagan Teki jagannadh.teki@gmail.com wrote: >>>>>>> Hi Simon, >>>>>>> >>>>>>> On Mon, Mar 11, 2013 at 9:38 PM, Simon Glass sjg@chromium.org wrote: >>>>>>>> Some SPI flash controllers (e.g. Intel ICH) have a limit on the number of >>>>>>>> bytes that can be in a write transaction. Support this by breaking the >>>>>>>> writes into multiple transactions. >>>>>>>> >>>>>>>> Signed-off-by: Simon Glass sjg@chromium.org >>>>>>>> --- >>>>>>>> Changes in v2: None >>>>>>>> >>>>>>>> drivers/mtd/spi/spi_flash.c | 10 ++++++++-- >>>>>>>> 1 file changed, 8 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >>>>>>>> index 17f3d3c..b82011d 100644 >>>>>>>> --- a/drivers/mtd/spi/spi_flash.c >>>>>>>> +++ b/drivers/mtd/spi/spi_flash.c >>>>>>>> @@ -87,6 +87,9 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset, >>>>>>>> for (actual = 0; actual < len; actual += chunk_len) { >>>>>>>> chunk_len = min(len - actual, page_size - byte_addr); >>>>>>>> >>>>>>>> + if (flash->spi->max_write_size) >>>>>>>> + chunk_len = min(chunk_len, flash->spi->max_write_size); >>>>>>>> + >>>>>>>> cmd[1] = page_addr >> 8; >>>>>>>> cmd[2] = page_addr; >>>>>>>> cmd[3] = byte_addr; >>>>>>>> @@ -111,8 +114,11 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset, >>>>>>>> if (ret) >>>>>>>> break; >>>>>>>> >>>>>>>> - page_addr++; >>>>>>>> - byte_addr = 0; >>>>>>>> + byte_addr += chunk_len; >>>>>>>> + if (byte_addr == page_size) { >>>>>>>> + page_addr++; >>>>>>>> + byte_addr = 0; >>>>>>>> + } >>>>>>> >>>>>>> Does this change required to handle < page_size writes, means if the >>>>>>> user is giving an offset other than >>>>>>> multiples of page_sizes? >>>>>> >>>>>> I'm not quite sure what you are saying, but let me try to response. >>>>>> >>>>>> I believe what should happen is that byte_addr should become aligned >>>>>> to the page_size after the first transfer, and from then on it should >>>>>> start at 0 for each page. >>>>>> >>>>>> Are you seeing a problem? >>>>> >>>>> My question,what if this change doesn't have.? >>>>> Can't I able to write data starts from unaligned page_sizes? >>>> >>>> It should work fine - I did in fact find a problem in the driver in >>>> this case, which I fixed. Let me know if you see any problem. >>> >>> Means this change is for proper handling of write data starts from >>> unaligned page_sizes, is it? >> >> Not really - it was designed to handle the case where the driver >> cannot write a whole page at once. The Intel ICH peripheral has this >> problem. >> >> I believe that writing starting from unaligned page sizes worked OK >> before this change. > > Thanks. > > But I am some how confusing instead of this change may be you may > place the existing code as it is > page_addr++; > byte_addr = 0; > prior to above may be you can place intel ICH per hack as other will > do whole page at once, i may be wrong.
The old code assumed that it could skip to the start of the next page after each write. The new code skips forward by chunk_len, which generally takes as to the start of the next page, but not always (e.g. with Intel ICH).
Yes, AFAIK every other SPI driver can still do a whole page at a time, with or without this patch.
Thats what if the user is giving an unaligned page size suppose 0x80 with 512 bytes (if the page_size=256) sf write 0x100 0x80 0x200 the loop will goes 2 non page_sizes and 1 pages_size like this iteration 1--- 128 iteration 2-- 256 iteration 3-- 128
I was tested the old and new code w.r.t this unaligned page_size as a start the result is same uboot> sf write 0x100 0x80 0x200 actual = 0.....chunk_len = 128 actual = 128.....chunk_len = 256 actual = 384.....chunk_len = 128 SF: program success 512 bytes @ 0x80
Means the old and new code does the same thing, but still i couldn't understand. What exactly this change is for, if it is specific to intel flash what is state in above condition.
Yes it is for the Intel SPI controller which has a strange limitation that it can only write 64 bytes at a time.
So I need to initialize slave.max_write_size to 0 on my controller driver? is that the good idea to make change in all drivers with this issue, may be i am wrong.?
If your driver is using spi_flash_alloc(), as it now should be, then it should work OK.
Sorry I couldn't understand. As per as i know spi_flash_alloc is a generic call used for spi_flash read/write/erase calls.
When i use the code as it is i got the garbage value for slave.max_write_size and chunk_len on write call has 1 due to this flash write failed. I fixed when I explicitly assign 0 to slave.max_write_size on my controller driver.
Request for your comments.
Thanks, Jagan.
Regards, Simon
Thanks, Jagan.
Regards, Simon
Thanks, Jagan.
May be the new code handle this situation as earlier may not have.?