Hi Michael,
On 08.07.2010 16:24, Michael Karcher wrote:
currently flashrom programs GPIO pins it needs for a board enable generally for output and chooses the GPIO function if that pin is multiplexed (as long as selecting output and multiplexing is implemented for that type of GPIO). Another policy would be to *require* that the GPIO pin is *already* set to output and GPIO and abort otherwise. I consider the second approach much safer than the first one, but I suspect there are boards where flashrom finds the pins in the wrong state and reprogramming them is really needed.
Still I think that disallowing GPIOs that are not already set to output would be a good idea (and have gpio=force as programmer option) to protect against accidental board matches disabling core functionality.
Some boards may rely on the output of a GPIO in special function mode (e.g. if the special function always outputs high level), or they may rely on a GPIO set to GPI mode if tristate is in fact a weak pullup/pulldown.
The concrete background of this post is the Asus P2B-N board. It has write enable on GPO18 (lower to enable write). Currently intel_piix4_gpo_set rejects this GPO pin, because it is a "dual function pin which is most likely in use already". In fact, it is multiplexed with PCI_STP#, an output signal to stop the PCI bus clock. It would be fatal on a board that really stops the PCI clock on lowering the pin to set this pin to GPO and lower it. Just lowering it without touching the multiplex bit would be safe, because it won't do anything if the PCI_STP# functionality of that pin is in use - but this would be inconsistent with what flashrom is doing now.
Your suggestion of allowing GPIO writes only if the pin is in output mode looks sane and safe, but I'd like board enables to be able to specify an override if we know the GPIO mode to be incorrect. This mode should _not_ require a parameter from the user because users are unlikely to know if such a setting is correct or not. Can we add a force_unsafe parameter to all GPO functions, and set it to GPO_SAFE for all existing functions, and only set it to GPO_UNSAFE if a board definitely requires it?
Regards, Carl-Daniel