Hi Jagan, On Thu, Apr 25, 2013 at 6:55 AM, Jagan Teki <jagannadh.t...@gmail.com> wrote: > Hi Simon, > > On Wed, Apr 24, 2013 at 6:03 AM, Simon Glass <s...@chromium.org> wrote: >> Hi, >> >> On Tue, Apr 23, 2013 at 3:11 PM, Jagan Teki <jagannadh.t...@gmail.com> wrote: >>> Hi Simon, >>> >>> On Wed, Apr 24, 2013 at 3:25 AM, Jagan Teki <jagannadh.t...@gmail.com> >>> wrote: >>>> Hi Simon, >>>> >>>> On Wed, Apr 24, 2013 at 3:10 AM, Simon Glass <s...@chromium.org> wrote: >>>>> Hi Jagan, >>>>> >>>>> On Tue, Apr 23, 2013 at 2:31 PM, Jagan Teki <jagannadh.t...@gmail.com> >>>>> wrote: >>>>>> Hi Simon, >>>>>> >>>>>> On Wed, Apr 24, 2013 at 2:48 AM, Simon Glass <s...@chromium.org> wrote: >>>>>>> Hi Jagan, >>>>>>> >>>>>>> On Tue, Apr 23, 2013 at 2:15 PM, Jagan Teki <jagannadh.t...@gmail.com> >>>>>>> wrote: >>>>>>>> Hi Simon, >>>>>>>> >>>>>>>> On Wed, Apr 24, 2013 at 2:43 AM, Simon Glass <s...@chromium.org> wrote: >>>>>>>>> Hi Jagan, >>>>>>>>> >>>>>>>>> On Tue, Apr 23, 2013 at 2:01 PM, Jagan Teki >>>>>>>>> <jagannadh.t...@gmail.com> wrote: >>>>>>>>>> Hi Simon, >>>>>>>>>> >>>>>>>>>> On Wed, Apr 24, 2013 at 2:20 AM, Simon Glass <s...@chromium.org> >>>>>>>>>> wrote: >>>>>>>>>>> Hi Jagan, >>>>>>>>>>> >>>>>>>>>>> On Tue, Apr 23, 2013 at 1:44 PM, Jagan Teki >>>>>>>>>>> <jagannadh.t...@gmail.com> wrote: >>>>>>>>>>>> Hi Simon, >>>>>>>>>>>> >>>>>>>>>>>> On Mon, Mar 11, 2013 at 9:38 PM, Simon Glass <s...@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 <s...@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. Regards, Simon > > Thanks, > Jagan. > >> >> Regards, >> Simon >> >>> >>> Thanks, >>> Jagan. >>> >>>> >>>> May be the new code handle this situation as earlier may not have.? _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot