Guenter Roeck wrote: > On Tue, Apr 05, 2011 at 10:45:36AM -0400, Andreas Herrmann wrote: > > +static int __devinit f15h_power_is_internal_node0(struct pci_dev *f4) > > +{ > > + u32 val; > > + struct pci_dev *f3; > > + > > + f3 = pci_get_slot(f4->bus, PCI_DEVFN(PCI_SLOT(f4->devfn), 3)); > > + if (!f3) { > > + dev_err(&f4->dev, "no function 3 available on this slot\n"); > > + return 0; > > It is a common practice to return a negative value on errors. Why not here ? Apparently, this function returns a boolean value. Using "bool"/"true"/ "false" would have made this more obvious. > Also, is this really an error which asks for an error message, or just a CPU > or system which does not support the attribute ? AFAICT all F15h CPUs are _known_ to have all these PCI functions; the error should never occur in practice. What I do in the k10temp driver in this situation is to trust the CPU to be there, omitting the pci_get_slot/pci_dev_put and just replacing this: > > + pci_read_config_dword(f3, REG_NORTHBRIDGE_CAP, &val); > > + pci_dev_put(f3); with the equivalent of: pci_bus_read_config_dword(f4->bus, PCI_DEVFN(PCI_SLOT(f4->devfn), 3), REG_NORTHBRIDGE_CAP, &val); BTW: The family 15h CPUs have the same temperature sensor registers (D18F3xA4 and D18F3x64) as the earlier families, haven't they? Regards, Clemens _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors