[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4D9C12BF.9090402@ladisch.de>
Date: Wed, 06 Apr 2011 09:14:07 +0200
From: Clemens Ladisch <clemens@...isch.de>
To: Guenter Roeck <guenter.roeck@...csson.com>,
Andreas Herrmann <herrmann.der.user@...glemail.com>
CC: Jean Delvare <khali@...ux-fr.org>,
Thomas Renninger <trenn@...e.de>,
"lm-sensors@...sensors.org" <lm-sensors@...sensors.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] hwmon: Add driver for AMD family 15h processor power
information
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists