Re: [PATCH for-4.4 1/2] mtd: spi-nor: fix Spansion regressions (aliased with Winbond)

[Brian Norris <computersforpeace@gmail.com>]

On Mon, Mar 28, 2016 at 12:52:51AM +0200, Matthias Schiffer wrote:
> On 03/26/2016 07:57 PM, Matthias Schiffer wrote:
> > On 12/15/2015 07:48 PM, Brian Norris wrote:
> >> Spansion and Winbond have occasionally used the same manufacturer ID,
> >> and they don't support the same features. Particularly, writing SR=0
> >> seems to break read access for Spansion's s25fl064k. Unfortunately, we
> >> don't currently have a way to differentiate these Spansion and Winbond
> >> parts, so rather than regressing support for these Spansion flash, let's
> >> drop the new Winbond lock/unlock support for now. We can try to address
> >> Winbond support during the next release cycle.
> >>
> >> Original discussion:
> >>
> >> http://patchwork.ozlabs.org/patch/549173/
> >> http://patchwork.ozlabs.org/patch/553683/
> >>
> > 
> > I have a few devices with a s25fl064k lying around, and I was not able to
> > reproduce this issue. I've re-applied "mtd: spi-nor: disable protection for
> > Winbond flash at startup" and the flash is readable just fine.
> > 
> > On the contrary, I've come across a board with a s25fl064k that comes up
> > locked, so removing the protection bits would be necessary. (I was not yet
> > able to check if the patch actually fixes writing to the flash on this
> > board, as I don't have access to the device myself, but I hope to get a
> > response on that soon.)
> > 
> > Regards,
> > Matthias
> > 
> 
> I made the mistake of trusting the kernel log and OpenWrt Wiki when making
> my previous tests.
> 
> All of the boards I was talking about in my last mail actually have a
> Winbond w25q64, not a s25fl064k (two board I tested the patch on, and the
> board that was reported to come up locked). The kernel detects the w25q64
> as s25fl064k, as these two flash chips have the same JEDEC ID 0xef4017.

That's interesting; I didn't notice we had duplicate entries for the
same ID. But apparently, the committers did:

  commit f2df1ae3fe8d ("mtd: m25p80: Add support for two new Spansion
  SPI devices (S25FL-K)")
  ...
  "Note that both parts exhibit a Winbond manufacturer ID so they might
  also be added to that section."

But this is interesting: I see the latest datasheet for Spansion
s25fl064k says it supports the Block Protect bits in the Status
Register, so presumably *some* version of s25fl064k should support
write_sr(nor, 0) to unlock it at boot...

If Felix's initial report is indeed correct, then I think we have:
(1) Spansion s25fl064k without Block Protect support (that breaks if you
    try to write SR=0)
(2) Spansion s25fl064k with Block Protect support (that requires you to
    unlock at boot by writing SR=0 (?))
(3) Winbond w25q64 with Block Protect support (that requires you to
    unlock at boot by writing SR=0)

And (1)-(3) all report the same ID, and (1) is incompatible with (2) and
(3). Am I right? Are flash vendors really this insane? Should we all
just give up and go home?

Brian