[LinuxBIOS] [PATCH] Fix ITE IT8705F support

Corey Osgood corey_osgood at verizon.net
Fri Mar 30 21:16:23 CEST 2007


Uwe Hermann wrote:
> On Thu, Mar 29, 2007 at 06:26:16AM -0400, Corey Osgood wrote:
>> I'm still not entirely convinced that the previous version was flawed,
>> but this one has more features, and covers some of what Uwe had marked
>> as TODO anyways, with more that I'll fix up later.
> 
> My guess is that the problem was that the
>   pnp_set_iobase(dev, PNP_IDX_IO0, iobase);
> was not done explicitly before. I think I'll be able to test some code
> this evening...
> 
> I'd prefer a less invasive patch, though. We should leave superio.c as
> is for now, until we can really test that part of the code.
> Let's just fix it8705f_early_serial.c for now.

Sounds good. I'll do some more work on the superio once I can boot this
damn board to boot (spd not reading correctly).

> 
> 
>> +/*----------------------------------------------------------------------------------
>> + * Function:    	pnp_enter_conf_state
>> + * Parameters:  	dev - high 8 bits = Super I/O port
>> + * Return Value:	None
>> + * Description: 	Enable access to the IT8705F's configuration registers.
>> + */
> 
> Please don't copy these code comments. We should use Doxygen-style
> comments for all new code we write (or "normal" code comments).
> 

Okay, I'll have to learn a little more about Doxygen, I guess. I hate
these comments, personally, I think a quick little description like
"Enable config registers" usually works best, and isn't so ugly.

> 
>> +static inline void pnp_enter_conf_state(device_t dev) {
>> +	/* Port 0x2e is constant, no matter what */
> 
> Nope, I think for the IT8705F it can be 0x4e, too. The special address
> used for entering MB PnP mode is always 0x2e, though (if I'm reading the
> data sheet correctly). However, this is not the same as the configuration
> port which will be used later...

Hmm, I'll have to tinker with this, I don't think I understand it entirely.

> 
> 
>> +static void pnp_exit_conf_state(device_t dev) {
>> +	outb(0xaa, 0x2e);
>>  }
> 
> I'm not sure this is correct. The datasheet suggests that the current
> version is correct. Is this a copy-n-paste error from the other Super I/O?
> 

Yep, it is, you're version is correct. The ambition to port another
superio was because I had just finished a similar hack for Magnus's
Jetway, and I was looking at those datasheets when I checked that
function, I guess.

>   
>> +	pnp_enter_conf_state(dev);
>> +	pnp_set_logical_device(dev);
>> +	pnp_set_enable(dev, 0);
>> +	pnp_set_iobase(dev, PNP_IDX_IO0, iobase);
>> +	pnp_set_enable(dev, 1);
>> +	pnp_exit_conf_state(dev);
> 
> Ok, this looks good. Let's do it this way, if it's enough to make the
> Super I/O work...
> 
> 
> See attached (untested, yet) patch for a new version which fixes several
> issues I was doing wrong in the ITE Super I/Os (the others need fixing,
> too; I'll post patches).
> 
> Does this patch work on your board?
> 

I'll test this this either later tonight or this weekend, unless "the
other project" arrives. For now, I'm off to work again.

-Corey




More information about the coreboot mailing list