[U-Boot] [PATCH v3 1/9] sf: Update SST flash params

[Jagan Teki]

Hi Bin,

On 20 April 2015 at 15:02, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Jagan,
>
> On Fri, Apr 17, 2015 at 4:48 PM, Jagan Teki <jagannadh.teki at gmail.com> wrote:
>> Hi Bin,
>>
>> On 17 April 2015 at 07:14, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Fri, Apr 17, 2015 at 2:09 AM, Jagan Teki <jagannadh.teki at 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.

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.

>
> static int sf_parse_len_arg(char *arg, ulong *len)
> {
>     char *ep;
>     char round_up_len; /* indicates if the "+length" form used */
>     ulong len_arg;
>
>     round_up_len = 0;
>     if (*arg == '+') {
>         round_up_len = 1;
>         ++arg;
>     }
>
>     len_arg = simple_strtoul(arg, &ep, 16);
>     if (ep == arg || *ep != '\0')
>         return -1;
>
>     if (round_up_len && flash->sector_size > 0)
>         *len = ROUND(len_arg, flash->sector_size);
>     else
>         *len = len_arg;
>
>     return 1;
> }
>
> So even you are passing 4KB in the flash params, the 'sf erase'
> command WILL erase 64KB.
>
>>         /* Compute erase sector and command */
>>         if (params->flags & SECT_4K) {
>>                 flash->erase_cmd = CMD_ERASE_4K;
>>                 flash->erase_size = 4096 << flash->shift;
>>         } else if (params->flags & SECT_32K) {
>>                 flash->erase_cmd = CMD_ERASE_32K;
>>                 flash->erase_size = 32768 << flash->shift;
>>         } else {
>>                 flash->erase_cmd = CMD_ERASE_64K;
>>                 flash->erase_size = flash->sector_size;
>>         }
>>
>> And to be honest, these flashes were tested with lowest ie 4KB so even if they
>> do support 64KB, we mentioned on 4KB on 'params->flags' as we tested
>> well with that
>> and it works consistently.

thanks!
-- 
Jagan.