Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov.

Patch set 7:Code-Review +1

View Change

3 comments:

• File flashrom.c:

  • Patch Set #5, Line 220: void programmer_delay(const struct flashctx *flash, unsigned int usecs)

    Currently the commit message doesn't give any hint why this is necessary. […]

    The commit message to this patch is hard to have both ways, it is either very forward looking and mention things in the future or largely mention nothing at all.

    I am happy with it as-is however if you have something specific in mind could you express the insight here Felix?

• File flashrom.c:

  • Patch Set #6, Line 226: return internal_delay(flash, usecs);

    Since we don't use `internal_delay` as a callback, we may not provide `flashctx`. […]

    If we put CB:67393 on top of this patch then `programmer_delay()` has been largely dealt with from the prospective of getting things into a re-entrant state of affairs.

    The follow on would be to flatten things out a bit and remove `programmer_delay()` from being a callback and have the drivers naively call their respective delay implementations internally or core just call the default implementation however that is less urgent. Overall with your patch and CB:67393 the fire of `programmer_delay()` subsides.

• File ichspi.c:

  • Patch Set #5, Line 860: truct flashctx *flas

    Is it maybe sufficient to just use `NULL` in the case of ichspi as it is the internal timer anyway.

    Ack

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