Hi Bin,
On 22 April 2015 at 15:02, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Jagan,
>
> On Wed, Apr 22, 2015 at 5:15 PM, Jagan Teki <jagannadh.t...@gmail.com> wrote:
>> Hi Bin,
>>
>> On 22 April 2015 at 14:13, Bin Meng <bmeng...@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Wed, Apr 22, 2015 at 4:06 PM, Jagan Teki <jagannadh.t...@gmail.com>
>>> wrote:
>>>> Hi Bin,
>>>>
>>>> On 22 April 2015 at 12:44, Bin Meng <bmeng...@gmail.com> wrote:
>>>>> Hi Jagan,
>>>>>
>>>>> On Wed, Apr 22, 2015 at 3:03 PM, Jagan Teki <jagannadh.t...@gmail.com>
>>>>> wrote:
>>>>>> Hi Bin,
>>>>>>
>>>>>> On 22 April 2015 at 12:14, Bin Meng <bmeng...@gmail.com> wrote:
>>>>>>> Hi Jagan,
>>>>>>>
>>>>>>> On Tue, Apr 21, 2015 at 8:47 PM, Jagan Teki <jagannadh.t...@gmail.com>
>>>>>>> wrote:
>>>>>>>> Hi Bin,
>>>>>>>>
>>>>>>>> On 20 April 2015 at 15:02, Bin Meng <bmeng...@gmail.com> wrote:
>>>>>>>>> Hi Jagan,
>>>>>>>>>
>>>>>>>>> On Fri, Apr 17, 2015 at 4:48 PM, Jagan Teki
>>>>>>>>> <jagannadh.t...@gmail.com> wrote:
>>>>>>>>>> Hi Bin,
>>>>>>>>>>
>>>>>>>>>> On 17 April 2015 at 07:14, Bin Meng <bmeng...@gmail.com> wrote:
>>>>>>>>>>> Hi Jagan,
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Apr 17, 2015 at 2:09 AM, Jagan Teki
>>>>>>>>>>> <jagannadh.t...@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.
>>>>>
>>>>> You previous email already said: the 'sf erase' command depends on
>>>>> *flash->sector_size*
>>>>>
>>>>>> /* 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;
>>>>>> }
>>>>>
>>>>> Here the codes says: *flash->erase_size*
>>>>>
>>>>> So when I give a 'sf erase 0 100' it erase 64KB even if you have SECT_4K.
>>>>
>>>> Example:
>>>> "SST25WF080", 0xbf2505, 0x0, 64 * 1024, 16, RD_NORM,
>>>> SECT_4K | SST_WR},
>>>>
>>>> sf probe gives
>>>> sector_size = 64 * 1024 and erase_size = 4096
>>>>
>>>> sf erase 0 100
>>>> sf_parse_len_arg len returns 100 and spi_flash_cmd_erase_ops returns
>>>> "SF: Erase offset/length not multiple of erase size"
>>>
>>> sf erase 0 +100. Sorry for the typo. But looks like you are not really
>>> reading the codes.
>>>
>>
>> Worked on too-many overclocked issue, sorry for that.
>>
>> So, something fixed in sf_probe.c will fix this I guess.
>
> Good, you finally got it! So you prefer fixing this inconsistency in
> sf_probe.c? I guess by overriding flash->sector_size and
> flash->nr_sectors if SECT_4K?
I'm posting patch.
>
>>> => sf probe
>>> SF: Detected SST25VF016B with page size 256 Bytes, erase size 4 KiB,
>>> total 2 MiB, mapped at ffe00000
>>>
>>> => sf erase 0 +100
>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>
>>> Tested on two boards, and both shows 64K was erased.
>>>
>>>> Example:
>>>> "SST25WF080", 0xbf2505, 0x0, 64 * 1024, 16, RD_NORM,
>>>> SST_WR},
>>>>
>>>> sf probe gives
>>>> sector_size = 64 * 1024 and erase_size = 64 * 1024
>>>>
>>>> sf erase 0 100
>>>> sf_parse_len_arg len returns 100 and spi_flash_cmd_erase_ops returns
>>>> "SF: Erase offset/length not multiple of erase size"
>>>>
>>>> Still have any concerns, please come to IRC for more discussion
>>
>
> As you see I have rebased this patch once for v2/v3 and lots of effort
> were spent on this series. I remember you said this patch series needs
> some testing on your side, but this comment shows that you may want to
> do it in another way. I really hope such comments could be sent months
> ago. Today I can't even remember all of the details in this series.
> Luckily I don't lose interest to get this upstream so I kept asking
> for an update.
Yes - I did some testing for Micron patch at-least and even I post the comment
for the same. I have a plan to test the remaining patches as well and
in-fact these
questions I got it from sst patch(this patch). I agree some delay in
my side but as
these are core changes and I need to see each and test them
respectively, that is
my main Job. I'm very much thankful to you for keeping me updates.
thanks!
--
Jagan.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
