From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bin Meng <bmeng.cn@gmail.com>
Date: Mon, 20 Apr 2015 17:32:50 +0800
Subject: [U-Boot] [PATCH v3 1/9] sf: Update SST flash params
In-Reply-To: <CAD6G_RSoUtaWZihJLEnyDTxsBVZ_xdxut72=kFYdntAsGn7dUg@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>
Message-ID: <CAEUhbmWBfpg_c5oQ7NziCOK0U0YTSjb16mKj2+HQq_TjuxC6gQ@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 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:

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

Regards,
Bin