Re: [PATCH] Revert "mtd: spi-nor: disable protection for Winbond flash at startup"

[Brian Norris <computersforpeace@gmail.com>]

Hi,

On Sat, Nov 28, 2015 at 09:59:43AM +0100, Felix Fietkau wrote:
> On 2015-11-27 23:27, Brian Norris wrote:
> > On Thu, Nov 26, 2015 at 05:27:01PM +0100, Felix Fietkau wrote:
> >> This reverts commit c6fc2171b249e73745c497b578b417a2946f1b2f.
> >> 
> >> This commit is breaking read access on at least s25fl064k, but also
> >> possibly other Spansion flash chips.
> > 
> > This makes no sense. This patch is about Winbond flash, but you're
> > referencing Spansion flash in the commit subject. I suspect you have
> > either some more explaining, or some better investigation to do, because
> > I don't understand how the commit in question could be the problem for
> > you.
> Take a look at these Spansion flash ids:
> 
>     { "s25fl004k",  INFO(0xef4013,      0,  64 * 1024,   8, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>     { "s25fl008k",  INFO(0xef4014,      0,  64 * 1024,  16, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>     { "s25fl016k",  INFO(0xef4015,      0,  64 * 1024,  32, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>     { "s25fl064k",  INFO(0xef4017,      0,  64 * 1024, 128, SECT_4K) },
> 
> Note how they start with 0xef (SNOR_MFR_WINBOND).

Ugh, that really sucks :(

Maybe we should do the revert [*] now, and fix it for the next release.
Maybe we can add something to the table entry to (optionally) specify
the manufacturer, so we can specify that even if these look like they're
Winbond, they're actually Spansion.

However, it seems like the lock/unlock features (e.g., top/bottom; 4K
sector ranges) supported vary significantly across not only vendors, but
generation. So maybe we just need some more opt-in flags in the table,
so we don't have to try to make blanket statements, like "all Winbond
flash support flash locking in SR."

Brian

[*] I have a comment for this patch. I'll reply to the top-level.