[flashrom] [PATCH] Add Winbond SuperI/O detection

[Carl-Daniel Hailfinger]

On 30.06.2010 14:48, Michael Karcher wrote:
> Am Mittwoch, den 30.06.2010, 14:12 +0200 schrieb Carl-Daniel Hailfinger:
>
>   
>> -static const struct winbond_chip * winbond_superio_detect(uint16_t base)
>> +uint8_t probe_id_winbond(uint16_t port)
>>  {
>>  	uint8_t chipid;
>> +
>> +	w836xx_ext_enter(port);
>> +	chipid = sio_read(port, 0x20);
>> +	w836xx_ext_leave(port);
>> +
>> +	return chipid;
>> +}
>>     
>
> If this code is to be run on any board, I would like some more checking
> in it. For example, you could check that sio_read(port, 0x20) returns
> 0xFF after the w836xx_ext_leave(). There are ECs that don't need any
> enables, so they will respond after the Winbond enable although they are
> not Winbond chips. But they still respond after the disable, so they can
> be told apart.
>   

That's a good idea, but my goal for this patch was to have a quick hack
for discussion. We want to use the discovery code from superiotool or
the Linux kernel to avoid a maintenance headache.


> I think the kernel has Super I/O detection in the parport-pc driver.
> Maybe one could also peek there for safe detection code that has been
> tested on thousands of boxes.
>   

Yes.
http://git.openvz.org/?p=linux-2.6.32-openvz;a=blob;f=drivers/parport/parport_pc.c;h=2597145a066e391fa988ecbbf44921f2f249ef1d;hb=HEAD#l1219

I'm not sure if that code is still tested at all. AFAIK most modern
systems offer Parport discovery via ACPI.


>> +static const struct winbond_chip *winbond_superio_chipdef(void)
>> +{
>>  	const struct winbond_chip * chip = NULL;
>>  	int i;
>>  
>> -	w836xx_ext_enter(base);
>> -	chipid = sio_read(base, 0x20);
>>  	for (i = 0; i < ARRAY_SIZE(winbond_chips); i++)
>> -		if (winbond_chips[i].device_id == chipid)
>> +		if (winbond_chips[i].device_id == superio.model)
>>  		{
>>  			chip = &winbond_chips[i];
>>  			break;
>>  		}
>>  	
>> -	w836xx_ext_leave(base);
>>  	return chip;
>>  }
>>     
> Hmm. Shouldn't you check that superio.vendor is in fact
> SUPERIO_VENDOR_WINBOND here?
>   

Yes, thanks for pointing this out.

We should try to decide which exernal superio detection code we want
before we proceed.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/