View Change

11 comments:

• File chipdrivers.h:

  • Patch Set #2, Line 24: #include "writeprotect.h" /* for modifier_bits */

    I don't think this is needed in this commit for adding the chip initial support.

    It provides the modifier_bits struct, used by the s25f_{set,get}_modifier_bits() prototypes. We could leave them out, but I think it's worth adding them now.

• File s25f.c:

  • Patch Set #2, Line 34:

     * s25f.c - Helper functions for Spansion S25FL and S25FS SPI flash chips.
     * Uses 24 bit addressing for the FS chips and 32 bit addressing for the FL
     * chips (which is required by the overlayed sector size devices).
     * TODO: Implement fancy hybrid sector architecture helpers.
    

    separate out comment from license header.

    Done

  • Patch Set #2, Line 43: #include "hwaccess.h"

    is this needed?

    It was needed for the `be_to_cpu32` macro, but I have changed the code that used it now.

  • Patch Set #2, Line 45: #include "writeprotect.h"

    ditto.

    It provides the `struct modifier_bits` type.

  • Patch Set #2, Line 192:

    unsigned char read_cr_cmd[] = {
    					CMD_RDAR,
    					(addr >> 16) & 0xff,
    					(addr >> 8) & 0xff,
    					(addr & 0xff),
    					0x00, 0x00, 0x00, 0x00,
    					0x00, 0x00, 0x00, 0x00,
    	};
    

    looks like these various cmd blocks though the file are badly formatted. […]

    I've updated this one. I'm not sure what the indent levels should be for the others (e.g. in s25fs_write_cr), I can't find a comparable example in spi25.c. It seems like a weird mix of tabulation and regular indentation rules.

  • Patch Set #2, Line 489: et;

    = 0;

    This was never really used to store a return value, only to store the return code from spi_send_command(). I've changed it accordingly.

  • Patch Set #2, Line 494: if (!ret) {

    invert the ret check logic to avoid such a big block under a if branch. […]

    Done

  • Patch Set #2, Line 531: return

    ret =

    Restructured, no longer have ret variable.

  • Patch Set #2, Line 531: return

    ret =

    ditto

  • Patch Set #2, Line 534: 0;

    ret

    ditto

• File s25f.c:

  • Patch Set #4, Line 487: int probe_spi_big_spansion(struct flashctx *flash)

    n.b. I have changed this function quite a bit:

    • Added context to msg_gdbg() debugging calls
    • Replaced type punned union + memcpy + endianness conversion with bitshift + bitwise or
    • Flattened control flow
    • Changed unsigned char -> uint8_t

    Compare at patchset 2 to patchset 4 to see the changes.

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