Re: [flashrom] [PATCH] add support for Pm25LQ032

[Stefan Tauner]

On Sat, 4 Apr 2015 10:57:24 +0100
Steven Honeyman  wrote:
> Thanks for the feedback. I've made the changes and added support for
> the rest of the Pm25LQ family, new patch is below.
> The "C" part number thing is a bit strange - the *032 datasheet is the
> only one to have a C at the end, and has a smaller voltage range and
> smaller OTP size than the rest.

Hi,

thanks for the refined patch and sorry for the long response time.
Please do not post inline patches because they often get mangled by
auto-wrapping of lines (see quoted example on the bottom). I had
another problem with the first hunk of your patch I could not fix
and ended up applying it manually, thus I can't tell what the exact
cause is/was. I have added comments regarding multi-i/o support and
committed the result in r1891. Thanks! There is an unsupported Pm25WD
series as well btw ;)
> On 4 April 2015 at 00:05, Stefan Tauner
>  wrote:
> > >            .block_erase = spi_block_erase_20,    /* sector erase */
> >
> > All these comments are redundant. Experienced flashrom hacker will
> > instantly deduce that from the erase block size(s).
>
> Maybe... but that doesn't mean newbies will!

No, but they need to study the datasheets themselves... the wording in
your comments as well as the wording in the datasheets is not 
standardized at all. So these comments could create even more
confusion :)
> Signed-off-by: Steven Honeyman
> Index: flashchips.c
>
> diff --git a/trunk/flashchips.c b/trunk/flashchips.c
> --- a/trunk/flashchips.c    (revision 1889)
> +++ b/trunk/flashchips.c    (working copy)
> @@ -9903,8 +9903,203 @@
>          .voltage    = {2700, 3600}, /* 2.3-3.6V for Pm25LD040 */
>      },
>
> -{
> {
>     .vendor        = "PMC",
>    .name        = "Pm25LQ020",
>    .bustype    = BUS_SPI,
>    .manufacture_id    = PMC_ID,
>    .model_id    = PMC_PM25LQ020,
>    .total_size    = 256,
>    .page_size    = 256,
>    /* OTP: 256B total; read 0x4B, write 0xB1 */
>    .feature_bits    = FEATURE_WRSR_WREN | FEATURE_OTP,
>    .tested        = TEST_UNTESTED,
>    .probe        = probe_spi_rdid,
>    .probe_timing    = TIMING_ZERO,
>    .block_erasers    =
>    {
>        {
>            .eraseblocks = { {4 * 1024, 64} },
>            .block_erase = spi_block_erase_20,
>        }, {
>            .eraseblocks = { {4 * 1024, 64} },
>            .block_erase = spi_block_erase_d7,
>        }, {
>            .eraseblocks = { {64 * 1024, 4} },
>            .block_erase = spi_block_erase_d8,
>        }, {
>            .eraseblocks = { {256 * 1024, 1} },
>            .block_erase = spi_block_erase_60,
>        }, {
>            .eraseblocks = { {256 * 1024, 1} },
>            .block_erase = spi_block_erase_c7,
>        }
>    },

E.g.:
>    .printlock    = spi_prettyprint_status_register_bp3_srwd, /*
>
> bit6 is quad enable */