From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jagan Teki <jagannadh.teki@gmail.com>
Date: Wed, 22 Apr 2015 12:33:59 +0530
Subject: [U-Boot] [PATCH v3 1/9] sf: Update SST flash params
In-Reply-To: <CAEUhbmWR+7jhGED3NZDLS5Uzopb-C-vEEOvHd7pFb9zLuHvvHg@mail.gmail.com>
References: <1418215892-17617-1-git-send-email-bmeng.cn@gmail.com>
 <1418215892-17617-2-git-send-email-bmeng.cn@gmail.com>
 <CAD6G_RQm3=aDQ2ROYW5SzjDrfcrJOV3i5G8OFf8W6OMi88LAWA@mail.gmail.com>
 <CAEUhbmU-hW2vBfLXDG4LqU-Zmu5P-yfMR2UBa53=55QmoOOBdw@mail.gmail.com>
 <CAD6G_RSoUtaWZihJLEnyDTxsBVZ_xdxut72=kFYdntAsGn7dUg@mail.gmail.com>
 <CAEUhbmWBfpg_c5oQ7NziCOK0U0YTSjb16mKj2+HQq_TjuxC6gQ@mail.gmail.com>
 <CAD6G_RQJYWH9kvojSwcTS0jU2p_4L710YO=v493-_fi=Ny=uow@mail.gmail.com>
 <CAEUhbmWR+7jhGED3NZDLS5Uzopb-C-vEEOvHd7pFb9zLuHvvHg@mail.gmail.com>
Message-ID: <CAD6G_RQDzngfd6unhs6gqxiutqfW-VKG-irh7o23n2f9ZynZjw@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Bin,

On 22 April 2015 at 12:14, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Tue, Apr 21, 2015 at 8:47 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> Hi Bin,
>>
>> On 20 April 2015 at 15:02, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Fri, Apr 17, 2015 at 4:48 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>> Hi Bin,
>>>>
>>>> On 17 April 2015 at 07:14, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> Hi Jagan,
>>>>>
>>>>> On Fri, Apr 17, 2015 at 2:09 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>> Hi Bin,
>>>>>>
>>>>>> I think you have a different interpretation of sector size here-
>>>>>>
>>>>>> /* The size listed here is what works with SPINOR_OP_SE, which isn't
>>>>>>  * necessarily called a "sector" by the vendor.
>>>>>>  */
>>>>>> Say for example SST25VF040B has 8 sectors of which each sector size is
>>>>>> 64 * 1024 out of this we can use 4K sector erase or 32K sector erase or
>>>>>> 64K sector erase through flags.
>>>>>>
>>>>>> Linux does follow the same-
>>>>>>         /* SST -- large erase sizes are "overlays", "sectors" are 4K */
>>>>>>         { "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024,  8, SECT_4K |
>>>>>> SST_WRITE) },
>>>>>>         { "sst25vf080b", INFO(0xbf258e, 0, 64 * 1024, 16, SECT_4K |
>>>>>> SST_WRITE) },
>>>>>>         { "sst25vf016b", INFO(0xbf2541, 0, 64 * 1024, 32, SECT_4K |
>>>>>> SST_WRITE) },
>>>>>>         { "sst25vf032b", INFO(0xbf254a, 0, 64 * 1024, 64, SECT_4K |
>>>>>> SST_WRITE) },
>>>>>>
>>>>>> Please check it.
>>>>>
>>>>>
>>>>> Yes, I know this pretty well. And I want to change this behavior, as
>>>>> my cover letter says.
>>>>>
>>>>> Currently the 'sf erase' command operates on a 64KB granularity, while
>>>>> the actual erase command is 4KB granularity, which is inconsistent and
>>>>> causes confusion.
>>>>
>>>> It never related to 'sf erase' instead based on the 'params->flags'
>>>> sf_probe will decide which erase_cmd with erase_size will use.
>>>
>>> No, it is related. See cmd_sf.c:
>>
>> I'm not getting your point- how could it erase use 64K sector size
>> instead of 4K.
>
> It indeed erases 64K sector size. You need check the logic in
> spi_flash_validate_params().

We're assigning erase_size to sector_size only when SECT_4K and SECT_32K
and for these erase_size becomes direct values, please check this.

       /* Compute erase sector and command */
        if (params->flags & SECT_4K) {
                flash->erase_cmd = CMD_ERASE_4K;
                flash->erase_size = 4096;
        } else if (params->flags & SECT_32K) {
                flash->erase_cmd = CMD_ERASE_32K;
                flash->erase_size = 32768;
        } else {
                flash->erase_cmd = CMD_ERASE_64K;
                flash->erase_size = flash->sector_size;
        }

>
>> Suppose the sector size is 4K
>>
>> flash->sector_size = 0x1000
>> 1.  erase 4K  len flash (it's total erase length)
>>
>> # sf erase 0x0 0x1000
>>
>>   len_arg = simple_strtoul(arg, &ep, 16);
>>  gives - 0x1000
>>
>>    *len becomes 0x1000
>>
>> 2.  erase 4K+1 len flash
>>
>> # sf erase 0x0 +0x1001
>>
>>   len_arg = simple_strtoul(arg, &ep, 16);
>>  gives - 0x1001
>>
>>  *len becomes 0x2000
>>
>> All the way when it goes to sf_ops.c erase will take by means of
>> erase_size which is assigned in sf_probe.c based on flags like 4K
>> 32K or 64K.

thanks!
-- 
Jagan.