Re: [flashrom] [PATCH] Add support for SST26VF064B

[Stefan Tauner]

On Mon, 5 May 2014 14:17:05 -0700
Wei Hu  wrote:
> On Sun, May 4, 2014 at 1:06 PM, Stefan Tauner
>  wrote:
> > On Mon, 31 Mar 2014 22:18:48 -0700
> > Wei Hu  wrote:
> > > Datasheet available at
> > > http://ww1.microchip.com/downloads/en/DeviceDoc/20005119D.pdf
> >
> > Hi,
> >
> > thank you for your patch! I have reviewed it and I think you got a few
> > things wrong:
> > Most importantly, I think the block erase sizes are non-uniform
> > (for opcode 0xD8). See note 11 of Table 5.1 and Section 3.0 etc.
>
> Thanks for fixing the non-uniform layout! Learned something new today.

This became rather uncommon with SPI chips but still there are even
new chips with this feature. It was very common in older flash chips. We
currently support over 100 chips with such a layout (look for
".eraseblocks = {\n" in flashchips.c).
> > Also, the ULBPR requires a WREN opcode sent before it. OTOH your
> > spi_disable_blockprotect_generic() calls seems useless (all bits of
> > the status register are read-only).
>
> I know spi_disable_blockprotect_generic() is useless here, but I
> thought it was a good idea to call the generic function first in
> general? As a side effect of calling this function I'm sending WREN,
> so I didn't send on explicitly.

There is no "generic" function that always should be called. The
spi_disable_blockprotect_generic() function is just generic in the
sense that most unprotect schemes can be implemented with it by
supplying the respective parameters.
Also, the WREN command is only active for one matching operation, and
in this case this is the WRSR command that immediately follows it
within spi_write_status_register_flag(). So your version would not do
what you expect.
> > The spi_prettyprint_status_register_sst25 function is completely
> > wrong for these chips.
> > The OTP memory, and other advanced features were not documented at
> > all.
>
> What's OTP? I didn't see it in the datasheet. Why do you want that?

One-time-programmable memory, it is noted on the front page of the
datahsset and is explained in more detail in 5.27 "Program Security ID".
If the flag FEATURE_OTP is set for a chip, we produce the following
message:
"This chip may contain one-time programmable memory. flashrom cannot
read and may never be able to write it, hence it may not be able to
completely clone the contents of this chip (see man page for details)."

Most users seem to not understand that, but I think nevertheless that
it is useful for those that do :)

[PATCH] Add support for SST26VF064B

[Wei Hu]

Stefan,

Thanks for your reply. Your patch looks good to me.

On Tue, May 6, 2014 at 1:55 PM, Stefan Tauner
 wrote:
> > I know spi_disable_blockprotect_generic() is useless here, but I
> > thought it was a good idea to call the generic function first in
> > general? As a side effect of calling this function I'm sending WREN,
> > so I didn't send on explicitly.
>
> There is no "generic" function that always should be called. The
> spi_disable_blockprotect_generic() function is just generic in the
> sense that most unprotect schemes can be implemented with it by
> supplying the respective parameters.
> Also, the WREN command is only active for one matching operation, and
> in this case this is the WRSR command that immediately follows it
> within spi_write_status_register_flag(). So your version would not do
> what you expect.

Maybe you're right, but I was able to erase/write the chip with my patch.
> > What's OTP? I didn't see it in the datasheet. Why do you want that?
>
> One-time-programmable memory, it is noted on the front page of the
> datahsset and is explained in more detail in 5.27 "Program Security ID".
> If the flag FEATURE_OTP is set for a chip, we produce the following
> message:

Sorry for not seeing OTP. I opened the file in a PDF reader and was
able to find the keyword. I wasn't able to search for it in Firefox
for some reason.