archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <>
To: Saheed Bolarinwa <>
Cc: Bjorn Helgaas <>,
	Shuah Khan <>,
Subject: Re: Linux Kernel Mentorship Summer 2020
Date: Wed, 18 Mar 2020 10:04:32 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[+cc linux-pci because there are lots of interesting wrinkles in this
issue -- as background, my idea for this project was to make
pcie_capability_read_word() return errors  more like
pci_read_config_word() does.  When a PCI error occurs,
pci_read_config_word() returns ~0 data, but
pcie_capability_read_word() return 0 data.]

On Mon, Mar 16, 2020 at 02:03:57PM +0100, Saheed Bolarinwa wrote:
> I have checked the function the  pcie_capability_read_word() that
> sets *val = 0  when pci_read_config_word() fails.
> I am still trying to get familiar to the code, I just wondering why
> the result of pci_read_config_word()  is not being returned directly
> since  *val  is passed into it.

pci_read_config_word() needs to return two things:

  1) A success/failure indication.  This is either 0 or an error like
  the function return value.

  2) A 16-bit value read from PCI config space.  This is what goes in

pcie_capability_read_word() is similar.  The idea of this project is
to change part 2, the value in *val.

The reason is that sometimes the config read fails on PCI.  This can
happen because the device has been physically removed, it's been
electrically isolated, or some other PCI error.  When a config read
fails, the PCI host bridge generally returns ~0 data to satisfy the
CPU load instruction.

The PCI core, i.e., pci_read_config_word() can't tell whether ~0 means
(a) the config read was successful and the register on the PCI card
happened to contain ~0, or (b) the config read failed because of a PCI

Only the driver knows whether ~0 is a valid value for a config space
register, so the driver has to be involved in looking for this error.

My idea for this project is to make this checking more uniform between
pci_read_config_word() and pcie_capability_read_word().

For example, pci_read_config_word() sets *val = ~0 when it returns

On the other hand, pcie_capability_read_word() sets *val = 0 when it

> This is what is done inside pci_read_config_word() when
> pci_bus_read_config_word() is called. I tried to find the definition
> of  pci_bus_read_config_word() but I couldn't find it.  I am not
> sure I need it, though.

pci_bus_read_config_word() and similar functions are defined by the
PCI_OP_READ() macro in drivers/pci/access.c.  It's sort of annoying
because grep/cscope/etc don't find those functions very well.  But
you're right; they aren't really relevant to this project.

> I am also confused with the last statement of the Project
> description on the Project List
> <> page. It
> says, "*This will require changes to some callers of
> pci_read_config_word() that assume the read returns 0 on error.*" I
> think the callers pcie_capability_read_word() are supposedly being
> referred to here.

Yes, that's a typo.  The idea is to change pcie_capability_read_*(),
so we'd probably need to change some callers to match.

       reply	other threads:[~2020-03-18 15:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
2020-03-18 15:04 ` Bjorn Helgaas [this message]
     [not found]   ` <>
2020-03-19 15:57     ` Fwd: Linux Kernel Mentorship Summer 2020 Saheed Bolarinwa
2020-03-19 20:55     ` Bjorn Helgaas

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

* 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).