[PATCH] mtd: spi-nor: fixed spansion quad enable

[Esponde, Joel]

Hi Cyrille,

> -----Message d'origine-----
> De : Cyrille Pitchen [mailto:cyrille.pitchen at atmel.com]
> Envoyé : vendredi 21 octobre 2016 15:45
> À : Esponde, Joel <Joel.Esponde at Honeywell.com>; linux-
> mtd at lists.infradead.org
> Objet : Re: [PATCH] mtd: spi-nor: fixed spansion quad enable
> 
> > That's a very good remark and I think the reason is that the current
> > implementation of spansion_quad_enable() suffers from a bug but this
> > one is almost harmless:
> >
> > I assume there is pull-up resistor on the MISO line:
> > spansion_quad_enable() sends a 35h command, which is not supported
> > hence ignored by some memories, then it reads a 1-byte value of FFh
> > due to the pull-up so we pass the test which checks that the Quad Enable
> bit has successfully been set.
> >
> > Then if we now add the same test before sending the Write Status (01h)
> > command with 2 bytes (SR1 then CR/SR2), we also read a FFh value for
> > the Configuration Register hence we skip the Write Status command
> > because we think the Quad Enable bit has already been set but if we
> > are using a brand new memory still using its factory settings the Quad
> > Enable has never been set and won't be set since we now skip the Write
> Status command.
> >
> > I think this what currently happens but I need to check with one of my
> > memory samples if I can find one which doesn't support the Read CR
> > command but I not sure to have such a sample...
> 
> The JESD216B specification clearly deals with 2 cases: a first one where the
> 35h command is not used, I guess because not supported, and a second case
> using both the 05h and 35h commands to read the SR1 and SR2/CR1 memory
> registers before writing both SR1 and SR2/CR1 with the 01h command.
> 
> However I can't provide any example of memory sample falling into the first
> case. I guess case 1 only applies to old and buggy memories and maybe not
> so used in production.
> Besides, your patch is great so I wonder whether it makes sense to reject it
> just because it might introduce a regression with some unidentified
> memories...
> If so, I think we could still find a workaround later to handle those very few
> (deprecated) parts.
> 
> The S25FL127S memory is still recommended by Cypress/Spansion for new
> designs.
> 

I cannot help here as I have only one part number of flash memory...
But from a performance point of view, it would be really great to have this pre test before writing quad enable bit to memory.
In my case, I will have to keep this patch because I cannot add useless 200ms delay at boot time.

Best regards,

Joël