Nico Huber has posted comments on this change. ( https://review.coreboot.org/23262 )
Change subject: Add a SPI command class to `struct flashchip` ......................................................................
Patch Set 5:
(1 comment)
Note that naming the field spi_command_set breaks the current tab alignment in the chips declaration, e.g.: .spi_command_set = EDI, .probe = edi_probe_kb9012,
I think naming it "command_set" would be enough since SPI25 already has SPI in the name and we could generalize the issue with other protoocls anyway.
I would like to keep the `spi_` in the name to make it clear which values to expect. Though, I shortened it to `spi_cmd_set`.
Also, I think it would be more convenient to be able to specify the command set to use from the command line instead of specifying the exact chip. One might want to just hook EDI to the keyboard pins for reflashing the EC and let flashrom detect which ENE chip it is.
Right, I kept that in mind. Should be easy to add to the CLI in a follow up.
If you want to pick up that idea, that'd be great, otherwise I'll probably end up writing and submitting something in this direction.
Please, go ahead.
https://review.coreboot.org/#/c/23262/4/flash.h File flash.h:
https://review.coreboot.org/#/c/23262/4/flash.h@186 PS4, Line 186: /*
By the way, it looks like the dummy structure commented out in the flashchips struct array declarati […]
It's obviously unmaintained because it documents this struct in a very different place. I wouldn't object to drop it or move it above the definition here.
-
Nico Huber (Code Review)