On Tue, 12 Apr 2011 12:58:35 -0700
David Hendricks <[email protected]> wrote:

> Hello,
> The attached patch adds support for the following Eon SPI flash chips:
> EN25Q40, EN25Q80, EN25Q32A/B, EN25Q64, and EN25Q128
> 
> I was going to add support for the EN25Q16 as well, but found that it shares
> the same ID as the EN25D16 but has different write protection capabilities.
> So I added a comment instead.
> 
> I do not have real hardware to test with at the moment, so I left the status
> as untested.
> 
> Datasheets available from Eon @ http://www.eonssi.com/products/products.aspx
> 

hello david and thanks for your patch.

i can confirm that the EN25D16 shares the ID with EN25Q16.
there is also a EN25QH16 but with another ID 7015h (and an upcoming
EN25QH256 w/o data sheet yet).

> Index: flashchips.h
> ===================================================================
> --- flashchips.h      (revision 1285)
> +++ flashchips.h      (working copy)
> @@ -232,7 +232,12 @@
>  #define EON_EN25B64          0x2017  /* Same as P64 */
>  #define EON_EN25B64T         0x46
>  #define EON_EN25B64B         0x36
> +#define EON_EN25Q40          0x3013
ok
> +#define EON_EN25Q80          0x3014
i could only find references to a chip named EN25Q80A... either add a
comment or rename to EON_EN25Q80A?

>  #define EON_EN25D16          0x3015
i think the twin's name should be noted here too.
since the D is untested i would also suggest using Q as the variable
name here, because there is no other D supported yet... so
-#define EON_EN25D16            0x3015
+#define EON_EN25Q16            0x3015 /* Same as D16 */
or so..

> +#define EON_EN25Q32          0x3016
correct; similar to the EON_EN25Q80 there seem to exist 3 versions: 'A',
'B' and no suffix.
> +#define EON_EN25Q64          0x3017
ok
> +#define EON_EN25Q128         0x3018
ok
>  #define EON_EN25F05          0x3110
>  #define EON_EN25F10          0x3111
>  #define EON_EN25F20          0x3112

somewhere below...
+#define EON_EN25QH16           0x7015

> Index: flashchips.c
> ===================================================================
> --- flashchips.c      (revision 1285)
> +++ flashchips.c      (working copy)
> @@ -2807,6 +2807,8 @@
>       },
>  
>       {
> +             /* Note: EN25Q16 is an evil twin which shares the model ID
> +                but has different write protection capabilities */
>               .vendor         = "Eon",
>               .name           = "EN25D16",
change according to the macro name chosen (if applicable see above)

>               .bustype        = CHIP_BUSTYPE_SPI,
> @@ -2814,6 +2816,7 @@
>               .model_id       = EON_EN25D16,
>               .total_size     = 2048,
>               .page_size      = 256,
> +             .feature_bits   = FEATURE_WRSR_WREN,
true for the Q, did not check D

>               .tested         = TEST_UNTESTED,
>               .probe          = probe_spi_rdid,
>               .probe_timing   = TIMING_ZERO,
.block_erase = spi_block_erase_52, is not supported by the Q version.
add a comment please.
other erasers seem to be ok with the Q version.

                .voltage        = {2700, 3600}, (for the Q version)

> @@ -3083,6 +3086,171 @@
>  
>       {
>               .vendor         = "Eon",
> +             .name           = "EN25Q40",
> +             .bustype        = CHIP_BUSTYPE_SPI,
> +             .manufacture_id = EON_ID_NOPREFIX,
> +             .model_id       = EON_EN25Q40,
> +             .total_size     = 512,
> +             .page_size      = 256,
> +             .feature_bits   = FEATURE_WRSR_WREN,
> +             .tested         = TEST_UNTESTED,
> +             .probe          = probe_spi_rdid,
> +             .probe_timing   = TIMING_ZERO,
> +             .block_erasers  =
> +             {
> +                     {
> +                             .eraseblocks = { {4 * 1024, 128} },
> +                             .block_erase = spi_block_erase_20,
> +                     }, {
> +                             .eraseblocks = { {64 * 1024, 8} },
> +                             .block_erase = spi_block_erase_d8,
> +                     }, {
> +                             .eraseblocks = { {512 * 1024, 1} },
> +                             .block_erase = spi_block_erase_60,
> +                     }, {
> +                             .eraseblocks = { {512 * 1024, 1} },
> +                             .block_erase = spi_block_erase_c7,
> +                     }
> +             },
> +             .unlock         = spi_disable_blockprotect,
> +             .write          = spi_chip_write_256,
> +             .read           = spi_chip_read,
                .voltage        = {2700, 3600},
> +     },
> +
> +     {
> +             .vendor         = "Eon",
> +             .name           = "EN25Q80(A)",
> +             .bustype        = CHIP_BUSTYPE_SPI,
> +             .manufacture_id = EON_ID_NOPREFIX,
> +             .model_id       = EON_EN25Q80,
depends on macro name choice above
> +             .total_size     = 1024,
> +             .page_size      = 256,
> +             .feature_bits   = FEATURE_WRSR_WREN,
> +             .tested         = TEST_UNTESTED,
> +             .probe          = probe_spi_rdid,
> +             .probe_timing   = TIMING_ZERO,
> +             .block_erasers  =
> +             {
> +                     {
> +                             .eraseblocks = { {4 * 1024, 256} },
> +                             .block_erase = spi_block_erase_20,
> +                     }, {
> +                             .eraseblocks = { {64 * 1024, 16} },
> +                             .block_erase = spi_block_erase_d8,
> +                     }, {
> +                             .eraseblocks = { {1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_60,
> +                     }, {
> +                             .eraseblocks = { {1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_c7,
> +                     }
> +             },
> +             .unlock         = spi_disable_blockprotect,
> +             .write          = spi_chip_write_256,
> +             .read           = spi_chip_read,
                .voltage        = {2700, 3600},
> +     },
> +
> +     {
> +             .vendor         = "Eon",
> +             .name           = "EN25Q32(A)(B)",
i think that should be (A/B)
> +             .bustype        = CHIP_BUSTYPE_SPI,
> +             .manufacture_id = EON_ID_NOPREFIX,
> +             .model_id       = EON_EN25Q32,
> +             .total_size     = 4096,
> +             .page_size      = 256,
> +             .feature_bits   = FEATURE_WRSR_WREN,
> +             .tested         = TEST_UNTESTED,
> +             .probe          = probe_spi_rdid,
> +             .probe_timing   = TIMING_ZERO,
> +             .block_erasers  =
> +             {
> +                     {
> +                             .eraseblocks = { {4 * 1024, 1024} },
> +                             .block_erase = spi_block_erase_20,
> +                     }, {
> +                             .eraseblocks = { {64 * 1024, 64} },
> +                             .block_erase = spi_block_erase_d8,
> +                     }, {
> +                             .eraseblocks = { {4 * 1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_60,
> +                     }, {
> +                             .eraseblocks = { {4 * 1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_c7,
> +                     }
> +             },
> +             .unlock         = spi_disable_blockprotect,
> +             .write          = spi_chip_write_256,
> +             .read           = spi_chip_read,
                .voltage        = {2700, 3600},
> +     },
> +
> +     {
> +             .vendor         = "Eon",
> +             .name           = "EN25Q64",
> +             .bustype        = CHIP_BUSTYPE_SPI,
> +             .manufacture_id = EON_ID_NOPREFIX,
> +             .model_id       = EON_EN25Q64,
> +             .total_size     = 8192,
> +             .page_size      = 256,
> +             .feature_bits   = FEATURE_WRSR_WREN,
> +             .tested         = TEST_UNTESTED,
> +             .probe          = probe_spi_rdid,
> +             .probe_timing   = TIMING_ZERO,
> +             .block_erasers  =
> +             {
> +                     {
> +                             .eraseblocks = { {4 * 1024, 2048} },
> +                             .block_erase = spi_block_erase_20,
> +                     }, {
> +                             .eraseblocks = { {64 * 1024, 128} },
> +                             .block_erase = spi_block_erase_d8,
> +                     }, {
> +                             .eraseblocks = { {8 * 1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_60,
> +                     }, {
> +                             .eraseblocks = { {8 * 1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_c7,
> +                     }
> +             },
> +             .unlock         = spi_disable_blockprotect,
> +             .write          = spi_chip_write_256,
> +             .read           = spi_chip_read,
                .voltage        = {2700, 3600},
> +     },
> +
> +     {
> +             .vendor         = "Eon",
> +             .name           = "EN25Q128",
> +             .bustype        = CHIP_BUSTYPE_SPI,
> +             .manufacture_id = EON_ID_NOPREFIX,
> +             .model_id       = EON_EN25Q128,
> +             .total_size     = 16384,
> +             .page_size      = 256,
> +             .feature_bits   = FEATURE_WRSR_WREN,
> +             .tested         = TEST_UNTESTED,
> +             .probe          = probe_spi_rdid,
> +             .probe_timing   = TIMING_ZERO,
> +             .block_erasers  =
> +             {
> +                     {
> +                             .eraseblocks = { {4 * 1024, 4096} },
> +                             .block_erase = spi_block_erase_20,
> +                     }, {
> +                             .eraseblocks = { {64 * 1024, 256} },
> +                             .block_erase = spi_block_erase_d8,
> +                     }, {
> +                             .eraseblocks = { {16 * 1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_60,
> +                     }, {
> +                             .eraseblocks = { {16 * 1024 * 1024, 1} },
> +                             .block_erase = spi_block_erase_c7,
> +                     }
> +             },
> +             .unlock         = spi_disable_blockprotect,
> +             .write          = spi_chip_write_256,
> +             .read           = spi_chip_read,
                .voltage        = {2700, 3600},
> +     },
> +
> +     {
> +             .vendor         = "Eon",
>               .name           = "EN29F010",
>               .bustype        = CHIP_BUSTYPE_PARALLEL,
>               .manufacture_id = EON_ID,

although the comment of the array indicates "alphabetically" sorted, it
seems most of it is sorted by chipsize (if pushed). that means the
entries above should be sorted like the IDs in flashchips.h

also please add a comment to all chips with the respective OTP (=one
time programmable) area size.
we don't support that yet, nor do we warn the user (yet), but i am sure
there is a patch to lure with such a comment... ;)
512B:
Q128
Q64
Q32B
(QH16)

256B:
Q80A
Q40

128B:
Q16 (but D16 has 512B, oh my...)

most of this are rather small problems. hence if everything i put into
this review was at least briefly considered.. it is
Acked-by: Stefan Tauner <[email protected]>
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to