Hi, On Fri, Apr 26, 2013 at 11:34 AM, Jagan Teki <jagannadh.t...@gmail.com> wrote: > Hi Simon, > > On Fri, Apr 26, 2013 at 5:51 PM, Simon Glass <s...@chromium.org> wrote: >> Hi Jagan, >> >> On Thu, Apr 25, 2013 at 11:16 PM, Jagan Teki <jagannadh.t...@gmail.com> >> wrote: >>> Hi Simon, >>> >>> On Fri, Apr 26, 2013 at 7:13 AM, Simon Glass <s...@chromium.org> wrote: >>>> Hi, >>>> >>>> On Thu, Apr 25, 2013 at 12:06 PM, Jagan Teki <jagannadh.t...@gmail.com> >>>> wrote: >>>>> Hi Simon, >>>>> >>>>> On Fri, Apr 26, 2013 at 12:22 AM, Simon Glass <s...@chromium.org> wrote: >>>>>> 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. >>>>> >>>>> 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.
drivers/spi/tegra_spi.c is a reasonable one to copy I think. Regards, Simon > > Thanks, > Jagan. > >> >> Regards, >> Simon >> >>> >>> Thanks, >>> Jagan. >>> >>>> >>>> Regards, >>>> Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot