Hi Simon,
On Fri, Apr 26, 2013 at 5:51 PM, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On Thu, Apr 25, 2013 at 11:16 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Simon,
On Fri, Apr 26, 2013 at 7:13 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On Thu, Apr 25, 2013 at 12:06 PM, Jagan Teki jagannadh.teki@gmail.com wrote:
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.
Which SPI driver please? Does your SPI driver call spi_alloc_slave()?
I am using xilinx zynq qspi controller driver, not mainline yet, planning to do.
Yes we have a spi_alloc_slave func where I am initializing slave and spi_dev members. in that I have initialized slave.max_write_size = 0.
OK, but this should be done by spi_do_alloc_slave() for you, if you are calling the mainline spi_alloc_slave() macro. Please see how other SPI drivers set themselves up.
Thanks for your help, got the point. Could you please any reference driver, I am planning to make this qspi driver to push on mainline.
Thanks, Jagan.
Regards, Simon
Thanks, Jagan.
Regards, Simon