Re: [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/parpor...

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