There are some serious problems with this code. And things only become transparent when you introduce those helper functions.
This is what your function is reduced to:
/* * Disable the flash write protect (which is connected to the * Winbond W83627HF GPIOs). * * There are some severe discrepancies in this code! */ static int board_iwill_dk8_htx(const char *name) { w836xx_ext_enter();
/* set pins to SCL or GPIO. */ wbsio_mask(0x2B, 0xD0, 0xD0); /* GPIO 21,22,24 */
wbsio_write(0x07, 0x08); /* select logical device 8: GPIO port 2. */
wbsio_mask(0x30, 0x01, 0x01); /* Activate logical device */
wbsio_mask(0xF0, 0xEF, 0xEF); /* GPIO 20,21,22,23,25,26,27 = input */
wbsio_mask(0xF1, 0x16, 0x16); /* Raise GPIO 21,22,24 */
#if 0 wbsio_mask(0xF2, 0x00, 0x00); /* GPIO inversion reg */ #endif
w836xx_ext_leave();
return 0; }
Let me run down through things one by one.
First of all, which GPIO pin is the culprit here? Which one needs to be raised/dropped for your rom to be writeable? Do you know this? Or are you not sure which one it is.
Secondly, you are using | <value> all the time on all but one register access. Clearly this is not what is wanted in all cases and it leads to some pretty crappy results when using the helper functions.
Let me step through the code:
/* set pins to SCL or GPIO. */ wbsio_mask(0x2B, 0xD0, 0xD0); /* GPIO 21,22,24 */
This was moved up. You don't need to select a logical device for this to work. Aruma code was correct here. But which GPIO is it?
wbsio_write(0x07, 0x08); /* select logical device 8: GPIO port 2. */
This is fully correct.
wbsio_mask(0x30, 0x01, 0x01); /* Activate logical device */
This is correct, but could've just as well been a write. I prefer this over what the aruma code uses as it clearly marks this as a single bit to be raised.
wbsio_mask(0xF0, 0xEF, 0xEF); /* GPIO 20,21,22,23,25,26,27 = input */
It seems as if you wanted to set GPIO24 as an output. But instead you did | 0xEF. Which is quite the opposite of & 0xEF or & ~0x10.
This line should be replaced by: wbsio_mask(0xF0, 0x00, 0x10); /* GPIO24 = output */
So is your gpio line GPIO24 like the aruma?
Next line:
wbsio_mask(0xF1, 0x16, 0x16); /* Raise GPIO 21,22,24 */
Which one is it?
And this: #if 0 wbsio_mask(0xF2, 0x00, 0x00); /* GPIO inversion reg */ #endif
This was the conversion of the useless | 0x00 you did there. Were you trying to do this: wbsio_write(0xF2, 0x00, 0x10); /* Clear GPIO24 inversion */
All in all, the way this patch was handled was very very poor. We've all been talking about formatting yet no-one bothered to look at what actually was going on (and i too acked this code).
This is not how code should get handled.
I've attached the full patch here. Except for the ordering of the 0x2B write, this is a direct translation, with just some comments added.
I will look into writing static int W83627HFGPIORaise(int Port); which handles the raising of GPIO20-27 on this very superIO.
This would reduce the two relevant board enables and stop mistakes like this from happening ever again.
Luc Verhaegen.