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

View Change

3 comments:

• Commit Message:

  • Patch Set #4, Line 13:

    I modified the signature as above and updated the commit message to explain that programmer specific […]

    I re-read the discussion, and it seems resolved to me.
    The last question from Angel:
    > If the long-term goal is to remove the global state, in which struct should these currently-global variables go?

    These currently-global variables should move together with ex-global variables, which is going into a data struct that every registered master has already.
    But Edward gave a lot more more detailed explanation above.

    I am resolving this thread (I added two other comments, but that's a different story :) ). However, Angel, if you have more thoughts please tell us!

• File flashrom.c:

  • Patch Set #9, Line 265:

    	if (flash->mst->buses_supported & BUS_SPI) {
    		if (flash->mst->spi.delay)
    			flash->mst->spi.delay(flash, usecs);
    	} else if (flash->mst->buses_supported & BUS_PARALLEL) {
    		if (flash->mst->par.delay)
    			flash->mst->par.delay(flash, usecs);
    	} else
    		internal_delay(usecs);
    

    This changes the logic, it seems to me?
    If we need to keep the logic "internal_delay is default unless custom delay defined" then internal_delay needs to always be called unless custom delay defined.
    Which would be (in my mind):

    ```
    if (flash->mst->buses_supported & BUS_SPI)
            if (flash->mst->spi.delay)
    		return flash->mst->spi.delay(flash, usecs);

    if (flash->mst->buses_supported & BUS_PARALLEL)
    	if (flash->mst->par.delay)
    		return flash->mst->par.delay(flash, usecs);
    	
    return internal_delay(usecs);
    ```

    "If there is nothing custom, call internal_delay"

    Does this seem right to you?

• File serprog.c:

  • Patch Set #9, Line 456: static void serprog_delay(const struct flashctx *flash, unsigned int usecs);

    I am missing something, why the static forward declaration is needed? Can it be the `serprog_delay` itself here?

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