Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Nicholas Chin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 )
Change subject: ichspi: Split very long init function into two ......................................................................
Patch Set 8:
(3 comments)
Patchset:
PS7:
Thank you so much Nicolas for help!
Nicolas, sorry for delay, I uploaded new version which (hopefully) fixes ich_spibar and ich_generation initialisation. Is it possible for you to try run it again on your machine? Thank you so much!
PS7:
My memory tries to tell me something... back in 2012 we were […]
Wow this is a very special computer that you found! ;) Does it still work? 10 years ago... this is impressive! Maybe you can keep it as a memory even if it doesn't work anymore :) If it still works... maybe you can keep it to test things sometimes?
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/58735/comment/0a290381_292b00e5 PS7, Line 1771: ich_generation = ich_gen; : ich_spibar = spibar;
These two are globals and also affect the ICH7 paths. I guess we […]
Thank you so much for your analysis! This is helpful. ich_spibar is used in program_opcodes ichspi.c#L566, which is called at some point of init (as I understand?), and ich_generation is needed for run_opcode ichspi.c#L1081 ? I missed this :( global state makes it hard to understand what is used where, I am hoping to remove global state one day.
I moved initialisation of both ich_spibar and ich_generation back into entry point ich_init_spi. This last patchset I tested on the device that I have, and now of course it needs to be tested on ICH7.
The question that I have in mind: does the code looks good enough to re-test it again? At the moment I have no way to run init_ich7_spi code, so I only know that the code builds.
-
Anastasia Klimchuk (Code Review)