Looks good, save for a few nits

Patch set 6:Code-Review +1

View Change

7 comments:

• File flashchips.h:

  • Patch Set #6, Line 40: #define VARIABLE_SIZE_DEVICE_ID 0x10af

    What are these for?

    If unused, please drop.

  • Patch Set #6, Line 278: #define EON_EN25D16 0x3015

    Spurious double entry?

    Definitely. I'd keep the bottom one, and optionally rename it to Q16 for consistency (doesn't need to be in this patch)

  • Patch Set #6, Line 504: #define MACRONIX_MX25L12805 0x2018 /* MX25L12805 */

    Does it really exist?

    Looks like it's the same as the line below, but following the style of the lines above, so it seems to be duplicated.

  • Patch Set #6, Line 962: #define WINBOND_NEX_W25Q128_W 0x6018 /* Same as W25Q128FV (QPI mode), W25R128FV */

    Spurious, double entry?

    Looks like it, please keep the "W25Q128_W" entry

• File flashchips.h:

  • Patch Set #5, Line 949: W25Q128

    I'm unfamiliar with these parts. […]

    I don't think this comment needs to be updated (see the other comment thread), but I wanted to note that this is not specific to the W25Q128.

  • Patch Set #5, Line 951: */

    I would prefer to just drop the comment. It's merely a random detail […]

    Agreed.

• File flashchips.c:

  • Patch Set #5, Line 5892: CD

    I think it's common to use the name of the chip that introduced […]

    Agreed. I wouldn't change the model_id define, but rather the string name (on a separate patch).

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