On Fri, Apr 27, 2007 at 04:44:10PM +0200, Mondrian Nuessle wrote:
Actually you do not need to set the index reg twice. I just used the code pattern from the Agami Aruma board, which does the same thing...
In that case, we should introduce functions like:
void w83xxx_ext_enter(void); void w83xxx_ext_leave(void);
/* Enter extended functions before using these! */ unsigned char w83xxx_ext_read(unsigned char index); void w83xxx_ext_write(unsigned char index, unsigned char data); void w83xxx_ext_mask(unsigned char index, unsigned char data, unsigned char mask);
A _mask function is not that common, but this plainly provides a bitmask for data to be written. I don't see it around much, but i very much enjoy having them around in my code.
This replaces:
/* set GPIO regs... */ outb(0x2b, EFIR); /* GPIO multiplexed pin reg. */ b = inb(EFDR) | 0xd0; outb(0x2b, EFIR); outb(b, EFDR);
which would've been changed to:
/* set GPIO regs... */ b= w83xxx_ext_read(0x2b); /* GPIO multiplexed pin reg. */ b |= 0xd0; w83xxx_ext_write(0x2b, b);
with: w83xxx_ext_mask(0x2B, 0xD0, 0xD0); /* Set GPIO regs... */
I don't think that these functions need to live outside board_enable.c The size of them will already largely be made up for by the code we save with this.
Luc Verhaegen.
PS: Mondrian, there is no need for your patch to also implement this. With the whitespace comments, i didn't expect you to go over the whole file, but looking back, it's good that you did so though, as i was unaware of the spaces still being there.