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. Thanks, Jagan. > > 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 [email protected] http://lists.denx.de/mailman/listinfo/u-boot
