Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons.

View Change

20 comments:

• Commit Message:

  • Patch Set #1, Line 7: .

    nit: no period at the end of commit summaries

    Done

• Patchset:

  • Patch Set #1:

    Hi Neill, thanks for your patch. […]

    • It definitely looks like there's an opportunity for code sharing with mstarddc_spi, but the ISP process for our specific chip (TSUMOP82JUQ) requires extra steps that aren't included in that driver.
    • We had two different board designs, one of which connects the TSUM* to SMBus and the other to raw I2C. This driver is tested on both. The finished product will be using I2C. I've left the SMBus code in as a convenience so we can continue to flash our earlier prototypes.
    • GPIO numbers are specific to TSUMOP82JUQ. My understanding is that these particular GPIO lines, for that chip, are standard for connecting to SPI WP#. Documentation that we received from PanVision just says to poke them and doesn't provide further detail.

• Patchset:

  • Patch Set #2:

    Thanks for the review!

• File mediatek_i2c_spi.c:

  • Patch Set #1, Line 29:

    #define ISP_PORT 0x92 >> 1
    #define DEBUG_PORT 0xb2 >> 1
    

    Please wrap these in parentheses to avoid operation order problems: […]

    Done

  • Patch Set #1, Line 36:

    nit: double blank line

    Done

  • Patch Set #1, Line 55: uint8_t* buf

    The `*` unary operator should have a space before, not after: […]

    Done

  • Patch Set #1, Line 66: d

    with ret value so we know how the ioctl actually failed.

    Done

  • Patch Set #1, Line 96: args.data->block[0]

    just 'len' here, avoids needing to do that indirection there to check we are in fact memcpy to a hea […]

    Done

  • Patch Set #1, Line 96: args.data->block

    args. […]

    This is copying the data out of the i2c_smbus_data, starting at 1, using the size specified by [0].

  • Patch Set #1, Line 181: 2

    ARRAY_SIZE(data) […]

    Done

  • Patch Set #1, Line 211: 3

    ARRAY_SIZE(data)

    Done

  • Patch Set #1, Line 242: GPIO line

    +1

    This is specific to the revision of chip that our board uses (TSUMOP82JUQ).

  • Patch Set #1, Line 249: 0x426, 7

    What are these magic numbers?

    The TSUMOP82JUQ has a GPIO line going to WP# on SPI, and these two registers (0x426 and 0x428) control its state. We're following a ISP guide given to us by PanVision that doesn't provide any additional details.

  • Patch Set #1, Line 266: {

    Please move the brace to the following line

    Done

  • Patch Set #1, Line 269: unsigned char

    This assumes that the size of `unsigned char` is always 1 byte. I'd use `uint8_t`.

    Done

  • Patch Set #1, Line 271: sizeof

    Along with the changes above, this should then be `ARRAY_SIZE`

    Done

  • Patch Set #1, Line 295: 0x7F

    Please use lowercase for hex

    Done

  • Patch Set #1, Line 330: {

    Please move the brace to the following line

    Done

  • Patch Set #1, Line 348: }

    If both attempts fail, can we assume that entering ISP has failed?

    Observed behavior is that this can fail if the chip is already in ISP mode.

  • Patch Set #1, Line 360: {

    Please move the brace to the following line

    Done

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