Hi, On Thu, Apr 25, 2013 at 12:06 PM, Jagan Teki <[email protected]> wrote: > Hi Simon, > > On Fri, Apr 26, 2013 at 12:22 AM, Simon Glass <[email protected]> wrote: >> Hi Jagan, >> >> On Thu, Apr 25, 2013 at 6:55 AM, Jagan Teki <[email protected]> wrote: >>> Hi Simon, >>> >>> On Wed, Apr 24, 2013 at 6:03 AM, Simon Glass <[email protected]> wrote: >>>> Hi, >>>> >>>> On Tue, Apr 23, 2013 at 3:11 PM, Jagan Teki <[email protected]> >>>> wrote: >>>>> Hi Simon, >>>>> >>>>> On Wed, Apr 24, 2013 at 3:25 AM, Jagan Teki <[email protected]> >>>>> wrote: >>>>>> Hi Simon, >>>>>> >>>>>> On Wed, Apr 24, 2013 at 3:10 AM, Simon Glass <[email protected]> wrote: >>>>>>> Hi Jagan, >>>>>>> >>>>>>> On Tue, Apr 23, 2013 at 2:31 PM, Jagan Teki <[email protected]> >>>>>>> wrote: >>>>>>>> Hi Simon, >>>>>>>> >>>>>>>> On Wed, Apr 24, 2013 at 2:48 AM, Simon Glass <[email protected]> wrote: >>>>>>>>> Hi Jagan, >>>>>>>>> >>>>>>>>> On Tue, Apr 23, 2013 at 2:15 PM, Jagan Teki >>>>>>>>> <[email protected]> wrote: >>>>>>>>>> Hi Simon, >>>>>>>>>> >>>>>>>>>> On Wed, Apr 24, 2013 at 2:43 AM, Simon Glass <[email protected]> >>>>>>>>>> wrote: >>>>>>>>>>> Hi Jagan, >>>>>>>>>>> >>>>>>>>>>> On Tue, Apr 23, 2013 at 2:01 PM, Jagan Teki >>>>>>>>>>> <[email protected]> wrote: >>>>>>>>>>>> Hi Simon, >>>>>>>>>>>> >>>>>>>>>>>> On Wed, Apr 24, 2013 at 2:20 AM, Simon Glass <[email protected]> >>>>>>>>>>>> wrote: >>>>>>>>>>>>> Hi Jagan, >>>>>>>>>>>>> >>>>>>>>>>>>> On Tue, Apr 23, 2013 at 1:44 PM, Jagan Teki >>>>>>>>>>>>> <[email protected]> wrote: >>>>>>>>>>>>>> Hi Simon, >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Mon, Mar 11, 2013 at 9:38 PM, Simon Glass <[email protected]> >>>>>>>>>>>>>> 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 <[email protected]> >>>>>>>>>>>>>>> --- >>>>>>>>>>>>>>> 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()? Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
