Patch Set 3:

(5 comments)

I like the USB code and its potential to unify programmer
parameters. It belongs into a separate patch, though, to
ease review.

Ah I squashed it in so this is a functional bit of code upon it's own merits. Maybe we can separate out in the final round? I would like to address all the higher level rubrics required to get a patch of this nature moved from the cros/flashrom tree into the upstream code. license/style/etc as I predicted there was going to be a lot of back and forth to get it into what it needs to be like?

View Change

7 comments:

• File raiden_debug_spi.c:

  • Patch Set #3, Line 35: * Software Foundation.

    I guess one could infer GPLv3 compatibility from the 3-clause above. But […]

    That is a complex question that I would need to ask legal about? The implementation herein was stripped out of the cros/flashrom fork and isn't just something I wrote vanilla with this header. I'll talk to Stefan for guidance here if you think this blocks getting this merged?

  • Patch Set #3, Line 154: * maximum USB packet size for the device.

    NB. Writing and reviewing such comments takes much more time than fixing the […]

    I am not the author and I don't have enough control myself to change all the servo's firmwares to make this perfect from the getgo.. I simply don't think fixing interfaces is attainable in the initial support patch but maybe as a longer term goal.

  • Patch Set #3, Line 157: command header maximum size

    That's kind of true, but highly confusing when one knows what the macros […]

    Sorry I wasn't clear on the expectation on what you wish me to change here if anything?

  • Patch Set #3, Line 238: )

    (void)

    Done

• File usb_device.h:

  • Patch Set #3, Line 64: return

    I agree, this macro is a blocker. Hidden control flow statements are not […]

    I brought up the same point in the cros/flashrom tree. I very much agree. Can it be it's own patch though or do I need to fix this first in our tree then rebase this patch from our tree again? It's hard to juggle two trees when my goal is to make one go away in the end.

• File usb_device.c:

  • Patch Set #3, Line 35: * Software Foundation.

    See `raiden_debug_spi.c`.

    ditto, see there.

  • Patch Set #3, Line 86: void usb_match_value_default

    Why are all externally visible functions undocumented (here, not in the . […]

    I am aware, however this is the situation in the cros/flashrom tree. If I can get the functional differences sorted I can address these things as follow ups. However doing them together adds more diff and is too much up front work in one go to make that goal attainable since there is so many cases such as these.

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