Hi Tom/Simon/Febio, On 17 November 2015 at 02:37, Simon Glass <s...@chromium.org> wrote: > Hi Tom, > > On 15 November 2015 at 18:58, Tom Rini <tr...@konsulko.com> wrote: >> On Sun, Nov 15, 2015 at 06:34:51PM -0700, Simon Glass wrote: >>> Hi, >>> >>> On 13 November 2015 at 03:41, Bin Meng <bmeng...@gmail.com> wrote: >>> > Hi, >>> > >>> > On Wed, Nov 11, 2015 at 10:04 PM, Fabio Estevam <feste...@gmail.com> >>> > wrote: >>> >> On Wed, Nov 11, 2015 at 12:56 AM, Simon Glass <s...@chromium.org> wrote: >>> >>> Hi Fabio, >>> >>> >>> >>> On 10 November 2015 at 16:51, Fabio Estevam <feste...@gmail.com> wrote: >>> >>>> >>> >>>> Hi Simon, >>> >>>> >>> >>>> On Tue, Nov 10, 2015 at 10:09 PM, Simon Glass <s...@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. > > OK that sounds good, thanks.
Idea of making lock ops to common for both dm and non-dm(w/o adding them to dm_spi_ops) is that all generic spi_flash ops are going to use mtd ops similar to nand and Linux spi-nor. I did that mtd conversion in this [1] series and going forward spi_flash{} doesn't have generic ops all are inherited from mtd_info this is my initial plan for moving to spi-nor core addition. [1] http://u-boot.10912.n7.nabble.com/PATCH-v6-00-23-sf-MTD-support-td233769.html thanks! -- Jagan | openedev. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot