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

[Michael Karcher]

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