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. 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
