Attention is currently required from: Nico Huber, Nikolai Artemiev.

View Change

1 comment:

• File ichspi.c:

  • Patch Set #12, Line 135: #define HSFC_CYCLE_RDID HSFC_FCYCLE_MASK(6)

    This belongs to PCH100_HSFC_FCYCLE*. It doesn't fit in the 2 bits for ICH9. […]

    My dyslexic brain can hardly keep track of all this pre-processor nonsense going on here but this all looks very suspicious to me;

    ```
    /* Changed HSFC Control bits */
    /*
     * 4 bits to represents the FCYCLE operation for PCH as:
     * 0: SPI Read
     * 2: SPI Write
     * 3: SPI Erase 4K
     * 4: SPI Erase 64K
     * 6: SPI RDID
     * 7: SPI Write Status
     * 8: SPI Read Status
      */
    #define PCH100_HSFC_FCYCLE_BIT_WIDTH	0xf
    [..]
    #define PCH100_HSFC_FCYCLE	HSFC_FCYCLE_MASK(PCH100_HSFC_FCYCLE_BIT_WIDTH)
    [..]
    /*
     * 2 bits to represents the FCYCLE operation for ICH9 as:
     * 0: SPI Read
     * 2: SPI Write
     * 3: SPI Block Erase
     */
    #define ICH9_HSFC_FCYCLE_BIT_WIDTH	3
    #define HSFC_FCYCLE_OFF		1	/* 1-2: FLASH Cycle */
    #define HSFC_FCYCLE_MASK(n)	((n) << HSFC_FCYCLE_OFF)
    #define HSFC_FCYCLE		HSFC_FCYCLE_MASK(ICH9_HSFC_FCYCLE_BIT_WIDTH)
    ```

    At minimum `HSFC_FCYCLE` seems like it should be called `ICH9_HSFC_FCYCLE`, the definitions of `HSFC_FCYCLE_MASK(n) && HSFC_FCYCLE_OFF` moved up? The nomenclature is completely jumbled up as well as the ordering and the two sets of comments for FCYCLE are confusing in their current form.

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