[LinuxBIOS] [PATCH] flashrom patch for iwill dk8_htx

[Mondrian Nuessle]

The attached patch enables flashing on the Iwill DK8-HTX board.
Basically, it configures the SuperIO to set the right GPIO pins, so
write protection is disabled.

Signed-off-by: Mondrian Nuessle <[e-mail]>
---

So, this is the second try :-)
Only pci vendor id/device id is given, no subsystem id. Therefor,
flashing when booted from factory BIOS will only work, if -m
iwill:dk8_htx switch is given. I guess this could go in the wiki doc to
this board...

Regards,
Mondrian

-- 
 Mondrian Nuessle                 University of Mannheim
 Phone: [phone]          Computer Architecture Group
 Fax:   [phone]          http://ra.ti.uni-mannheim.de
 mailto:[e-mail]

[Luc Verhaegen]

On Thu, Apr 26, 2007 at 06:24:06PM +0200, Mondrian Nuessle wrote:
> The attached patch enables flashing on the Iwill DK8-HTX board.
> Basically, it configures the SuperIO to set the right GPIO pins, so
> write protection is disabled.
>
> Signed-off-by: Mondrian Nuessle <[e-mail]>
> ---
>
> So, this is the second try :-)
> Only pci vendor id/device id is given, no subsystem id. Therefor,
> flashing when booted from factory BIOS will only work, if -m
> iwill:dk8_htx switch is given. I guess this could go in the wiki doc to
> this board...
>
> Regards,
> Mondrian

Ok, first some whitespace gestapoing.

There are 3 trailing whitespaces on empty lines. You probably also 
want to review comments. There's one place (GPIO inversion reg) where 
you start the line with spaces instead of a tab. Also, get a space in 
after a ',' in your in/outbs (activate logical device).

Is it really necessary to write the index register twice on the w83627?

Also:
> +        { 0x1022, 0x7468, 0x0, 0x0,  0x0, 0x0, 0x0, 0x0,
> +		"iwill", "dk8_htx", "IWILL DK8-HTX", board_iwill_dk8htx },

There is a hard strcmp in the linuxbios name matching. For the agami 
aruma, the linuxbios name is in all caps, but here it is in small 
letters. Is this correct, as in, does this correspond with the 
linuxbios config? Should board_enable be using strcasecmp?

As for documenting, README should be touched as well. You might want to 
add the AGAMI:ARUMA.

This name matching is rather safe now, as you have to match at least one 
set of main pci-ids. This will stop joe simple from simply running down 
the list of board enables.

Luc Verhaegen.

[Uwe Hermann]

On Thu, Apr 26, 2007 at 10:12:33PM +0200, Luc Verhaegen wrote:
> Also:
> > +        { 0x1022, 0x7468, 0x0, 0x0,  0x0, 0x0, 0x0, 0x0,
> > +		"iwill", "dk8_htx", "IWILL DK8-HTX", board_iwill_dk8htx },
>
> There is a hard strcmp in the linuxbios name matching. For the agami 
> aruma, the linuxbios name is in all caps, but here it is in small 
> letters. Is this correct, as in, does this correspond with the 
> linuxbios config? Should board_enable be using strcasecmp?

Good question. We should definately make sure to consistently use only
_one_ of the variants. IMHO the one which is the most "correct", as in:
the vendor uses that name in that exact variant (lower-/uppercase,
hyphens, etc.)


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[Mondrian Nuessle]

The attached patch enables flashing on the Iwill DK8-HTX board.
Basically, it configures the SuperIO to set the right GPIO pins, so
write protection is disabled.

Signed-off-by: Mondrian Nuessle <[e-mail]>
---

Ok, this is the third try :-)

* I changed the name argument to "const char*" (as suggested by Uwe,
applied that to all boards, of course)
* I fixed indent, as specified by the coding styles, notice that this
bloats the patch considerably, since a lot of existing code is touched
* fixed several typos, comments etc
* I tested this patch on real hardware, both booted by factory BIOS and
LinuxBIOS
* regarding the naming of the boards, these are the names as used in the
LinuxBIOS config and therefore correct (i.e. LinuxBIOS identifies the
dk8-htx board as iwill:dk8_htx, I did never check the the ARUMA...)
* added some explanation to the README

Regards,
Mondrian

-- 
 Mondrian Nuessle                 University of Mannheim
 Phone: [phone]          Computer Architecture Group
 Fax:   [phone]          http://ra.ti.uni-mannheim.de
 mailto:[e-mail]

[Luc Verhaegen]

On Fri, Apr 27, 2007 at 01:01:19PM +0200, Mondrian Nuessle wrote:
> The attached patch enables flashing on the Iwill DK8-HTX board.
> Basically, it configures the SuperIO to set the right GPIO pins, so
> write protection is disabled.
>
> Signed-off-by: Mondrian Nuessle <[e-mail]>
> ---
>
> Ok, this is the third try :-)
>
> Regards,
> Mondrian
> +	/* set GPIO regs... */
> +	outb(0x2b, EFIR);	/* GPIO multiplexed pin reg. */
> +	b = inb(EFDR) | 0xd0;
> +	outb(0x2b, EFIR);
> +	outb(b, EFDR);

Is it really necessary to write the index register twice here? Not that 
it matters much in codesize.
> -        /*  Disable the flash write protect.  The flash write protect is
> -         *  connected to the WinBond w83627hf GPIO 24.
> -         */
> -	outb(0x87, EFIR); /* sequence to unlock extended functions */
> +	/*  Disable the flash write protect.  The flash write protect is
> +	 *  connected to the WinBond w83627hf GPIO 24.
> +	 */
> +	outb(0x87, EFIR);	/* sequence to unlock extended functions */
>   outb(0x87, EFIR);

Ah, my old spaces for tabs setting, removed now. I had assumed that this 
was handled earlier, when stepan committed my code when i was too busy 
to fix things then. Sorry for that.

It will probably still exist in other places of flashrom too, nothing 
you should worry about though.

Luc Verhaegen.

[Luc Verhaegen]

On Fri, Apr 27, 2007 at 01:17:27PM +0200, Luc Verhaegen wrote:
> On Fri, Apr 27, 2007 at 01:01:19PM +0200, Mondrian Nuessle wrote:
> > The attached patch enables flashing on the Iwill DK8-HTX board.
> > Basically, it configures the SuperIO to set the right GPIO pins, so
> > write protection is disabled.
> >
> > Signed-off-by: Mondrian Nuessle <[e-mail]>
> > ---
> >
> > Ok, this is the third try :-)
> >
> > Regards,
> > Mondrian
> > +	/* set GPIO regs... */
> > +	outb(0x2b, EFIR);	/* GPIO multiplexed pin reg. */
> > +	b = inb(EFDR) | 0xd0;
> > +	outb(0x2b, EFIR);
> > +	outb(b, EFDR);
>
> Is it really necessary to write the index register twice here? Not that 
> it matters much in codesize.

I would like an answer there, just for reference, so don't worry about 
it patchwise :)

Acked-by: Luc Verhaegen <[e-mail]>

Luc Verhaegen.

[Uwe Hermann]

On Fri, Apr 27, 2007 at 01:25:11PM +0200, Luc Verhaegen wrote:
> Acked-by: Luc Verhaegen <[e-mail]>

One more revision, please :)

I fixed some more cosmetic issues and reduced the patch to just the
relevant code. Let's fix the random other coding style changes in an
extra patch.

(patch untested on hardware, please take this with a grain of salt)


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[Stefan Reinauer]

* Uwe Hermann <[e-mail]> [070428 01:59]:
> Signed-off-by: Mondrian Nuessle <[e-mail]>
> Signed-off-by: Uwe Hermann <[e-mail]>
> Acked-by: Stefan Reinauer <[e-mail]>

[Uwe Hermann]

On Thu, May 03, 2007 at 03:46:43AM +0200, Stefan Reinauer wrote:
> * Uwe Hermann <[e-mail]> [070428 01:59]:
> > Signed-off-by: Mondrian Nuessle <[e-mail]>
> > Signed-off-by: Uwe Hermann <[e-mail]>
> > Acked-by: Stefan Reinauer <[e-mail]>

Committed in r2624.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[Mondrian Nuessle]

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...

Mondrian

Luc Verhaegen wrote:
> On Fri, Apr 27, 2007 at 01:01:19PM +0200, Mondrian Nuessle wrote:
> > The attached patch enables flashing on the Iwill DK8-HTX board.
> > Basically, it configures the SuperIO to set the right GPIO pins, so
> > write protection is disabled.
> >
> > Signed-off-by: Mondrian Nuessle <[e-mail]>
> > ---
> >
> > Ok, this is the third try :-)
> >
> > Regards,
> > Mondrian
> > +	/* set GPIO regs... */
> > +	outb(0x2b, EFIR);	/* GPIO multiplexed pin reg. */
> > +	b = inb(EFDR) | 0xd0;
> > +	outb(0x2b, EFIR);
> > +	outb(b, EFDR);
>
> Is it really necessary to write the index register twice here? Not that 
> it matters much in codesize.
> > -        /*  Disable the flash write protect.  The flash write protect is
> > -         *  connected to the WinBond w83627hf GPIO 24.
> > -         */
> > -	outb(0x87, EFIR); /* sequence to unlock extended functions */
> > +	/*  Disable the flash write protect.  The flash write protect is
> > +	 *  connected to the WinBond w83627hf GPIO 24.
> > +	 */
> > +	outb(0x87, EFIR);	/* sequence to unlock extended functions */
> >   outb(0x87, EFIR);
>
> Ah, my old spaces for tabs setting, removed now. I had assumed that this 
> was handled earlier, when stepan committed my code when i was too busy 
> to fix things then. Sorry for that.
>
> It will probably still exist in other places of flashrom too, nothing 
> you should worry about though.
>
> Luc Verhaegen.

[Luc Verhaegen]

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.

[Uwe Hermann]

On Fri, Apr 27, 2007 at 10:33:53PM +0200, Luc Verhaegen wrote:
> 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);

Hm, not sure. Looks a bit like overkill in this case.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[Luc Verhaegen]

On Sat, Apr 28, 2007 at 02:00:42AM +0200, Uwe Hermann wrote:
> On Fri, Apr 27, 2007 at 10:33:53PM +0200, Luc Verhaegen wrote:
> > 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);
>
> Hm, not sure. Looks a bit like overkill in this case.
>
> Uwe.

Why?

Three out of four functions in there (when mondrians patch finally gets 
in) would make use of this already. That says something about what 
future board enables will look like too.

This isn't overkill in any way, it's formalizing what's there.

Without mondrians code:
 board_enable.c |  137 +++++++++++++++++++++++++++++++-------------------------- 1 file 
changed, 76 insertions(+), 61 deletions(-)

So when mondrians patch is in there, this is made up for completely, and 
the board enables really are a lot friendlier.

Luc Verhaegen.

[Mondrian Nuessle]

While I think that a clean function interface is nice, I can live
without it...
So I could go with the patch, as Uwe provided it.

Mondrian

Luc Verhaegen wrote:
> On Sat, Apr 28, 2007 at 02:00:42AM +0200, Uwe Hermann wrote:
> > On Fri, Apr 27, 2007 at 10:33:53PM +0200, Luc Verhaegen wrote:
> > > 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);
> > > Hm, not sure. Looks a bit like overkill in this case.
> >
> > Uwe.
>
> Why?
>
> Three out of four functions in there (when mondrians patch finally gets 
> in) would make use of this already. That says something about what 
> future board enables will look like too.
>
> This isn't overkill in any way, it's formalizing what's there.
>
> Without mondrians code:
>  board_enable.c |  137 +++++++++++++++++++++++++++++++-------------------------- 1 file 
> changed, 76 insertions(+), 61 deletions(-)
>
> So when mondrians patch is in there, this is made up for completely, and 
> the board enables really are a lot friendlier.
>
> Luc Verhaegen.

[Luc Verhaegen]

On Wed, May 02, 2007 at 06:45:42PM +0200, Mondrian Nuessle wrote:
> While I think that a clean function interface is nice, I can live
> without it...
> So I could go with the patch, as Uwe provided it.
>
> Mondrian

Well, as soon as this code is in, i'll put up a patch for the functions, 
and you'll see that it really makes a lot of difference, even on such a 
small file. 3 out of 4 board enables make use of it though.

Luc Verhaegen.

[Uwe Hermann]

On Wed, May 02, 2007 at 10:14:00PM +0200, Luc Verhaegen wrote:
> On Wed, May 02, 2007 at 06:45:42PM +0200, Mondrian Nuessle wrote:
> > While I think that a clean function interface is nice, I can live
> > without it...
> > So I could go with the patch, as Uwe provided it.
> >
> > Mondrian
>
> Well, as soon as this code is in, i'll put up a patch for the functions, 
> and you'll see that it really makes a lot of difference, even on such a 
> small file. 3 out of 4 board enables make use of it though.

Yep, thanks! It sounded like overkill when I read about it the first
time, but I think you may be right. Please post a patch...

As for _this_ patch: someone just needs to ACK it, IHMO it can be committed.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[Mondrian Nuessle]

> > Well, as soon as this code is in, i'll put up a patch for the functions, 
> > and you'll see that it really makes a lot of difference, even on such a 
> > small file. 3 out of 4 board enables make use of it though.
>
> Yep, thanks! It sounded like overkill when I read about it the first
> time, but I think you may be right. Please post a patch...
> I agree with you two. Great.

[Uwe Hermann]

On Thu, Apr 26, 2007 at 06:24:06PM +0200, Mondrian Nuessle wrote:
> The attached patch enables flashing on the Iwill DK8-HTX board.
> Basically, it configures the SuperIO to set the right GPIO pins, so
> write protection is disabled.
>
> Signed-off-by: Mondrian Nuessle <[e-mail]>
> ---
>
> So, this is the second try :-)
> Only pci vendor id/device id is given, no subsystem id. Therefor,
> flashing when booted from factory BIOS will only work, if -m
> iwill:dk8_htx switch is given. I guess this could go in the wiki doc to
> this board...

And in the flashrom README, yes.
> Index: util/flashrom/board_enable.c
> ===================================================================
> --- util/flashrom/board_enable.c	(revision 2618)
> +++ util/flashrom/board_enable.c	(working copy)
> @@ -23,6 +23,56 @@
>  #include "flash.h"
>  #include "debug.h"
>
> +static int board_iwill_dk8htx(char *name)

I think this can be
  const char *name
as you never modify 'name' (or use it at all).
> +{
> +	/* Extended function index register, either 0x2e or 0x4e    */

On what does this depend? Can it be detected at runtime? If so, we
should "probe" the value and handle both cases.
> +#define EFIR 0x2e
> +	/* Extended function data register, one plus the index reg. */
> +#define EFDR EFIR + 1
> +	char b;
> +
> +        /*  Disable the flash write protect.  The flash write protect is
> +         *  connected to the WinBond w83627hf GPIOs
> +         */
> +	outb(0x87, EFIR); /* sequence to unlock extended functions */
> +	outb(0x87, EFIR);
> +	
> +	/* activate logical devie */

device
> +	outb(0x7,EFIR);
> +	outb(8,EFDR);

Space after comma (and other stuff Luc mentioned in his mail). Please
use the Linux kernel coding style and/or run your code through indent,
see http://linuxbios.org/Development_Guidelines#Coding_Style.

(yes, I know, lots of code currently in LinuxBIOS does not yet follow
those guidelines, but we're slowly moving towards that direction)
> +	/*lock extended function sagain */

Typo.


Otherwise the patch looks good, but I cannot test it on real hardware,
of course.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[ron minnich]

I just want to thank the author for the patch, and the folks who
chimed in for the helpful corrections :-)

we've needed this patch for a long time. Thanks again

ron

[Luc Verhaegen]

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.

[Mondrian Nuessle]

Ok, I looked over everything again. I was blind!

Luc, you are absolutely right.
And we do not need a dk8_htx enable function. Just use the ARUMA one.

The code I posted was based on reverse engineering registers, it worked
for us very well for months. So I never bothered checking the other
boards. Sorry.

I tested using the Aruma function on real hw and it works as expected.

After your patch, everything is A LOT better readable.

Sorry for the hassle,
Mondrian
-- 
 Mondrian Nuessle                 University of Mannheim
 Phone: [phone]          Computer Architecture Group
 Fax:   [phone]          http://ra.ti.uni-mannheim.de
 mailto:[e-mail]

[Luc Verhaegen]

On Thu, May 03, 2007 at 08:11:43PM +0200, Mondrian Nuessle wrote:
> Ok, I looked over everything again. I was blind!

I acked your code, and i didn't really bother to look what you actually 
were doing. The old register accesses don't invite one to look over 
them. You were no blinder than i was.
> Luc, you are absolutely right.
> And we do not need a dk8_htx enable function. Just use the ARUMA one.

Smashing. New patch deletes your function, slightly alters the aruma one 
and renames it to w83627hf_gpio24_raise.
> The code I posted was based on reverse engineering registers, it worked
> for us very well for months. So I never bothered checking the other
> boards. Sorry.
>
> I tested using the Aruma function on real hw and it works as expected.
>
> After your patch, everything is A LOT better readable.

Yeah. I wouldn't have a single hair on my head left if i hadn't done 
this very early on for vga io in my unichrome driver :)
> Sorry for the hassle,
> Mondrian

Don't apologise. You saw this through to the end. Also, the code is 
cleaner, and we have what currently seems the best solution in place.
The result is there, that's what counts.

New patch will be sent in shortly.

Luc Verhaegen.