Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk.

View Change

5 comments:

• File flash.h:

  • Patch Set #15, Line 170:

    	INVALID_REG = 0,
    	STATUS1,
    	STATUS2,
    	CONFIG1,
    

    Please add entries in the commits that make use of them. Took me rather
    long to realize that I can't find a commit for CONFIG1 probably because
    it's not used?

• File flash.h:

  • Patch Set #14, Line 169:

    enum flash_reg {
    	INVALID_REG = 0,
    	STATUS1,
    	STATUS2,
    	CONFIG1,
    };
    #define MAX_REGISTERS 4
    

    Also, this is specific to SPI flash chips. I'm not sure if it should be here.

    `MAX_REGISTERS` is only used in `writeprotect.c` AFAICS, so it would belong
    there. Or maybe as a last enum element? It would automatically have the
    correct value if starting at 0.

    IMO, the enum belongs to the API functions.

• File spi25_statusreg.c:

  • Patch Set #14, Line 51: int result = spi_send_multicommand(flash, cmds);

    Would be good to check with manibuilder if this causes problems with unusual toolchains (e.g. […]

    What do you mean? What could cause problems?

• File spi25_statusreg.c:

  • Patch Set #15, Line 63: */

    I have doubts that this is true for any modern chip. "allow running RDSR
    only once"? when? only once after an erase/write command? how should one
    keep track of it?

    So I would pretty much prefer to start testing writes to registers != SR1
    without this delay. Otherwise we'd have to keep assuming forever that
    it's necessary.

  • Patch Set #15, Line 84: *

    Please drop this dangling asterisk or add a line break after the first
    / before the last.

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