[U-Boot] [PATCH v7 21/21] sf: Add SPI NOR protection mechanism

[Simon Glass]

Hi,

On 13 November 2015 at 03:41, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi,
>
> On Wed, Nov 11, 2015 at 10:04 PM, Fabio Estevam <festevam at gmail.com> wrote:
>> On Wed, Nov 11, 2015 at 12:56 AM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Fabio,
>>>
>>> On 10 November 2015 at 16:51, Fabio Estevam <festevam at gmail.com> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On Tue, Nov 10, 2015 at 10:09 PM, Simon Glass <sjg at chromium.org> wrote:
>>>>
>>>> > This patch breaks chromebook_link - I think because it adds a new
>>>> > operation which is not supported by all flash chips. Those that are
>>>> > not supported (i.e that don't have the 'flash_is_locked' method)
>>>> > should still work.
>>>>
>>>> What is the symptom you are seeing? Which SPI NOR flash does this board have?
>>>
>>> It crashes reading the environment:
>>>
>>> U-Boot 2015.10-00544-gcad0499 (Nov 10 2015 - 17:06:00 -0700)
>>>
>>> CPU:   Intel(R) Core(TM) i5-3427U CPU @ 1.80GHz
>>> DRAM:  2.7 GiB
>>> SF: Detected W25Q64CV with page size 256 Bytes, erase size 4 KiB, total 8 MiB
>>> *** Warning - bad CRC, using default environment
>>>
>>> Video: 1280x1024x16
>>> Model: Google Link
>>> SF: Detected W25Q64CV with page size 256 Bytes, erase size 4 KiB, total 8 MiB
>>> Invalid Opcode (Undefined Opcode)
>>
>> I am wondering if this invalid opcode is caused by 6c2f758cee266f7648.
>>
>> Could you please try this?
>>
>
> No, this does not resolve this issue.
>
>> --- a/arch/x86/include/asm/bitops.h
>> +++ b/arch/x86/include/asm/bitops.h
>> @@ -364,7 +364,7 @@ static __inline__ int ffs(int x)
>>         __asm__("bsfl %1,%0\n\t"
>>                 "jnz 1f\n\t"
>>                 "movl $-1,%0\n"
>> -               "1:" : "=r" (r) : "rm" (x));
>> +               "1:" : "=r" (r) : "g" (x));
>>
>>         return r+1;
>>  }
>
> It turns out it is a NULL pointer exception! Fixing this NULL pointer
> makes the crash disappear, but 'saveenv' does not actually work on the
> SST flash. Something is broken again, gosh!

Bin thank you for fixing this.

We still have a big problem with this patch though - it adds features
to the pre-driver-model SPI flash implementation and not the driver
model implementation! If I somehow have the wrong end of the stick
please let me know.

If we accept this sort of patch we will never be done with driver
model conversions, as we make it impossible for boards to move
forward.

Fabio can you please rework this to remove the pre-driver-model
support, and add your new functions to struct dm_spi_flash_ops
instead, then convert the affected boards to driver model?

Regards,
Simon