Re: [coreboot] [PATCH] i82801gx updates

[Carl-Daniel Hailfinger]

I don't know the code well, so this is not a real review. Just a nitpick.

On 11.03.2009 13:23, Stefan Reinauer wrote:
> +static void smbus_set_subsystem(device_t dev, unsigned vendor, unsigned device)
> +{
> if (!vendor || !device) {

This case should output a diagnostic message at SPEW or DEBUG level.
> pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID,
> 		pci_read_config32(dev, 0));

PCI_VENDOR_ID instead of 0 for consistency.
> } else {
> pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID,
> 		((device & 0xffff) << 16) | (vendor & 0xffff));
> }
>
> +}

Same for the other *_set_subsystem. Maybe we even want a generic
function to handle this since the *_set_subsystem functions in the patch
seem to be identical.

Regards,
Carl-Daniel