From: Clemens Ladisch <clemens@ladisch.de>
To: Guenter Roeck <guenter.roeck@ericsson.com>,
Andreas Herrmann <herrmann.der.user@googlemail.com>
Cc: Jean Delvare <khali@linux-fr.org>,
Thomas Renninger <trenn@suse.de>,
"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] hwmon: Add driver for AMD family 15h processor power information
Date: Wed, 06 Apr 2011 09:14:07 +0200 [thread overview]
Message-ID: <4D9C12BF.9090402@ladisch.de> (raw)
In-Reply-To: <20110405201238.GA20551@ericsson.com>
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
next prev parent reply other threads:[~2011-04-06 7:13 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20110404160733.GA11818@alberich.amd.com>
2011-04-05 14:45 ` [PATCH v2] hwmon: Add driver for AMD family 15h processor power information Andreas Herrmann
2011-04-05 20:12 ` Guenter Roeck
2011-04-06 7:14 ` Clemens Ladisch [this message]
2011-04-06 9:38 ` Andreas Herrmann
2011-04-06 9:31 ` Andreas Herrmann
2011-04-06 13:54 ` [PATCH v3] " Andreas Herrmann
2011-04-06 16:50 ` Jean Delvare
2011-04-07 9:13 ` Andreas Herrmann
2011-04-08 13:54 ` [PATCH v4] " Andreas Herrmann
2011-04-13 13:06 ` Jean Delvare
2011-04-14 19:16 ` Andreas Herrmann
2011-04-14 19:20 ` [PATCH v5] " Andreas Herrmann
2011-04-15 9:31 ` Jean Delvare
2011-04-08 13:57 ` [PATCH] hwmon, fam15h_power: Add maintainers entry Andreas Herrmann
2011-04-13 13:08 ` Jean Delvare
2011-04-13 14:51 ` Andreas Herrmann
2011-04-06 14:14 ` [PATCH v2] hwmon: Add driver for AMD family 15h processor power information Jean Delvare
2011-04-06 15:19 ` Andreas Herrmann
2011-04-06 15:35 ` Jean Delvare
2011-04-06 16:14 ` Guenter Roeck
2011-04-06 16:20 ` Andreas Herrmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D9C12BF.9090402@ladisch.de \
--to=clemens@ladisch.de \
--cc=guenter.roeck@ericsson.com \
--cc=herrmann.der.user@googlemail.com \
--cc=khali@linux-fr.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lm-sensors@lm-sensors.org \
--cc=trenn@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).