Attention is currently required from: DZ, Nikolai Artemiev.

View Change

13 comments:

• Commit Message:

  • Patch Set #1, Line 9: We have tested read/write/erase and write protection function for following Macronix Flash.

    This is great, thank you so much!
    Could you please add info to commit message: which programmer you were using, and which operations you ran. Thanks!

• Patchset:

  • Patch Set #1:

    Daniel thank you the patch! I appreciate you are updating so many chip definitions!

    I have a suggestion, if you want, you can split this commit as one chip per commit. This way we can gradually review and submit them one by one.
    But you can keep it in one large commit, however in this case none of the chips will be submitted until all of them are ready.

    I added comments, and then I will have another round of review after these ones are fixed.

• File flashchips.c:

  • Patch Set #1, Line 6201: .tested = TEST_UNTESTED,

    This diff seems to be by mistake?
    That's Fudan chip, but also if the chip has been tested, you can't untest it :)

  • Patch Set #1, Line 9008: MX25L12845E/MX25L12865E

    These two models (MX25L12845E and MX25L12865E) have no configuration register, not in datasheets, which means:

    In the patch you update the current definition with `FEATURE_CFGR` and `reg_bits`, and now MX25L12845E and MX25L12865E do not match this new definition anymore.

    The easiest option is to add MX25L12850F as a new definition (and it will be reusing the same model ID, which is fine), and add `FEATURE_CFGR` and `reg_bits` into it. Old definition remains the same.

    Another option (harder, but more comprehensive) is to have old definition left for MX25L12845E and MX25L12865E, and create new definition with write-protect support for the remaining chips (including new one).

  • Patch Set #1, Line 9275: .tb = {CONFIG, 3, OTP}

    I didn't find configuration register in datasheets?

  • Patch Set #1, Line 9547:

    .eraseblocks = { {64 * 1024, 64} },
    				.block_erase = SPI_BLOCK_ERASE_52,
    

    this command is used for 32K block

  • Patch Set #1, Line 9564: {1650, 3600}

    datasheet says 2.65V-3.6V

  • Patch Set #1, Line 9567: .srp = {STATUS1, 7, RW},

    This bit is "Reserved" in datasheet

  • Patch Set #1, Line 9594:

    .eraseblocks = { {64 * 1024, 64} },
    				.block_erase = SPI_BLOCK_ERASE_52,
    

    this command is for 32K block

  • Patch Set #1, Line 9611: {1650, 3600},

    datasheet says 2.7V-3.6V

  • Patch Set #1, Line 10285: .name = "MX25R1635F",

    It seems like you moved MX25R1635F entry, and now it's out of alphabetical order. You need to return in back into its position in the list

• File include/flashchips.h:

  • Patch Set #1, Line 538: #define MACRONIX_MX25L3239E 0x2536

    for this and the other new lines: you need to keep the indentation between macro name and values, to keep value aligned

  • Patch Set #1, Line 546:

    #define MACRONIX_MX25R4035F 0x2813
    #define MACRONIX_MX25R1635F 0x2815
    

    it's a duplicate, the same macro exists already

To view, visit change 81096. To unsubscribe, or for help writing mail filters, visit settings.