Patchworkβ Add Winbond SuperI/O detection

Add Winbond Super I/O detection and hook it up to the generic Super I/O
detection framework.
Incomplete and untested, but at least it shouldn't break anything.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

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.

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.

> +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?

Regards,
  Michael Karcher

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