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

[Tom Rini <trini@konsulko.com>]

On Sun, Nov 15, 2015 at 06:34:51PM -0700, Simon Glass wrote:
> Hi,
> 
> On 13 November 2015 at 03:41, Bin Meng <bmeng.cn@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Nov 11, 2015 at 10:04 PM, Fabio Estevam <festevam@gmail.com> wrote:
> >> On Wed, Nov 11, 2015 at 12:56 AM, Simon Glass <sjg@chromium.org> wrote:
> >>> Hi Fabio,
> >>>
> >>> On 10 November 2015 at 16:51, Fabio Estevam <festevam@gmail.com> wrote:
> >>>>
> >>>> Hi Simon,
> >>>>
> >>>> On Tue, Nov 10, 2015 at 10:09 PM, Simon Glass <sjg@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?

Hang on, didn't we have this discussion on one of the earlier threads
about this?  Part of the problem here is that everything else required
to do this on DM isn't quite there and Fabio has agreed to (under pain
of feature removal later) do the conversion when possible but to not
otherwise block getting this feature (and thus some platform(s))
integrated already.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151115/821d54d5/attachment.sig>