Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52364 )
Change subject: programmer.h,chipset_enable.c: Make penable fn ptr more descript ......................................................................
Patch Set 2:
(1 comment)
File programmer.h:
https://review.coreboot.org/c/flashrom/+/52364/comment/7c1e71b9_b36e5467 PS1, Line 228: enable_flash_xxx
Example: https://github.com/torvalds/linux/blob/fcadab740480e0e0e9fa9bd272acd409884d4...
Huh? that's documentation examples, I don't see there any `xxx` in an API?
Also, the syntax alone makes it obvious that it's not a specific implementation that is called. Why introduce placeholders? Imagine I would have used outb_xxx, inb_xxx, outw_xxx, etc. in CB:52604. Would there be any benefit?
I see the whole thing here as a usual command pattern. The function references are often called run() or execute(); doit() would work for me too. For me it's a chipset enable and what we do is run the chipset enable.
NB. I think enable_flash_* is not a good name for these functions in general.