From: Brian Norris <briannorris@chromium.org>
To: Shawn Lin <shawn.lin@rock-chips.com>,
Bjorn Helgaas <bhelgaas@google.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, Jeffy Chen <jeffy.chen@rock-chips.com>,
linux-rockchip@lists.infradead.org,
Krishna Dhulipala <krishnad@fb.com>,
Keith Busch <keith.busch@intel.com>,
Christoph Hellwig <hch@lst.de>, Wei Zhang <wzhang@fb.com>
Subject: [PATCH] PCI: Make error code types consistent in pci_{read,write}_config_*
Date: Tue, 23 May 2017 12:36:58 -0700 [thread overview]
Message-ID: <20170523193655.GA144183@google.com> (raw)
In-Reply-To: <20170523184359.GB115572@google.com>
Callers normally treat the config space accessors as returning PCBIOS_*
error codes, not Linux error codes (or they don't look at them at all).
We have pcibios_err_to_errno(), in case the error code needs translated.
Fixes: 4b1038834739 ("PCI: Don't attempt config access to disconnected devices")
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
+ others, change subject
On Tue, May 23, 2017 at 11:44:01AM -0700, Brian Norris wrote:
> But the high level code doesn't handle this
> consistently. See, e.g., pci_read_config_byte() which can return regular
> Linux error codes (like -ENODEV), except it also passes up the return
> code of pci_read_config_byte() (a PCIBIOS_* code) directly.
Apparently this is new (inconsistent) behavior in 4.12-rc1. Seems like
an oversight to me.
> So callers don't really know whether to treat the value from
> pci_read_config_<foo>() as a PCIBIOS_* code (which should be translated
> with pcibios_err_to_errno()) or as a standard Linux errno.
>
> But then, there are relatively few callers (less than 10% of
> pci_read_config_<foo>(); even fewer for writes) that actually check the
> error codes...
>
> Maybe the "fix" is to replace -ENODEV with PCIBIOS_DEVICE_NOT_FOUND for
> the inconsistent cases (pci_{read,write}_config_{byte,word,dword}()).
Fix implemented in the surrounding patch.
drivers/pci/access.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 74cf5fffb1e1..c80e37a69305 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -896,7 +896,7 @@ int pci_read_config_byte(const struct pci_dev *dev, int where, u8 *val)
{
if (pci_dev_is_disconnected(dev)) {
*val = ~0;
- return -ENODEV;
+ return PCIBIOS_DEVICE_NOT_FOUND;
}
return pci_bus_read_config_byte(dev->bus, dev->devfn, where, val);
}
@@ -906,7 +906,7 @@ int pci_read_config_word(const struct pci_dev *dev, int where, u16 *val)
{
if (pci_dev_is_disconnected(dev)) {
*val = ~0;
- return -ENODEV;
+ return PCIBIOS_DEVICE_NOT_FOUND;
}
return pci_bus_read_config_word(dev->bus, dev->devfn, where, val);
}
@@ -917,7 +917,7 @@ int pci_read_config_dword(const struct pci_dev *dev, int where,
{
if (pci_dev_is_disconnected(dev)) {
*val = ~0;
- return -ENODEV;
+ return PCIBIOS_DEVICE_NOT_FOUND;
}
return pci_bus_read_config_dword(dev->bus, dev->devfn, where, val);
}
@@ -926,7 +926,7 @@ EXPORT_SYMBOL(pci_read_config_dword);
int pci_write_config_byte(const struct pci_dev *dev, int where, u8 val)
{
if (pci_dev_is_disconnected(dev))
- return -ENODEV;
+ return PCIBIOS_DEVICE_NOT_FOUND;
return pci_bus_write_config_byte(dev->bus, dev->devfn, where, val);
}
EXPORT_SYMBOL(pci_write_config_byte);
@@ -934,7 +934,7 @@ EXPORT_SYMBOL(pci_write_config_byte);
int pci_write_config_word(const struct pci_dev *dev, int where, u16 val)
{
if (pci_dev_is_disconnected(dev))
- return -ENODEV;
+ return PCIBIOS_DEVICE_NOT_FOUND;
return pci_bus_write_config_word(dev->bus, dev->devfn, where, val);
}
EXPORT_SYMBOL(pci_write_config_word);
@@ -943,7 +943,7 @@ int pci_write_config_dword(const struct pci_dev *dev, int where,
u32 val)
{
if (pci_dev_is_disconnected(dev))
- return -ENODEV;
+ return PCIBIOS_DEVICE_NOT_FOUND;
return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val);
}
EXPORT_SYMBOL(pci_write_config_dword);
--
2.13.0.219.gdb65acc882-goog
next prev parent reply other threads:[~2017-05-23 19:37 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-19 6:58 [PATCH] PCI: rockchip: check link status when validating device Shawn Lin
2017-05-23 18:00 ` Brian Norris
2017-05-24 0:54 ` Shawn Lin
2017-05-24 1:00 ` Brian Norris
2017-05-24 1:14 ` Shawn Lin
2017-05-24 1:25 ` Brian Norris
2017-05-23 18:44 ` Brian Norris
2017-05-23 19:36 ` Brian Norris [this message]
2017-05-25 6:50 ` [PATCH] PCI: Make error code types consistent in pci_{read,write}_config_* Keith Busch
2017-05-26 21:40 ` Bjorn Helgaas
2017-05-23 19:44 ` [PATCH] PCI: rockchip: check link status when validating device Bjorn Helgaas
2017-05-24 1:04 ` Shawn Lin
2017-05-24 1:15 ` Brian Norris
2017-05-24 21:33 ` Bjorn Helgaas
2017-05-24 21:43 ` Brian Norris
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=20170523193655.GA144183@google.com \
--to=briannorris@chromium.org \
--cc=bhelgaas@google.com \
--cc=hch@lst.de \
--cc=jeffy.chen@rock-chips.com \
--cc=keith.busch@intel.com \
--cc=krishnad@fb.com \
--cc=linux-pci@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=shawn.lin@rock-chips.com \
--cc=wzhang@fb.com \
/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).