On Wed, Apr 06, 2011 at 09:14:07AM +0200, Clemens Ladisch wrote:
> 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.
Agreed, using bool as return type would have been the better choice.
I'll adapt it.
> > 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.
Except under virtualization. But common practice is to not pass any
such devices into a guest. So either all CPU northbridge functions are
there or no function at all. I'll adapt the code to your suggestion.
> 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?
Yes and I think Andre has just sent a patch to add support for 15h to
k10temp.
Thanks,
Andreas
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
