linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Yicong Yang <yangyicong@hisilicon.com>
Cc: Saheed Bolarinwa <refactormyself@gmail.com>,
	bjorn@helgaas.com, skhan@linuxfoundation.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH RFC] pci: Make return value of pcie_capability_read*() consistent
Date: Fri, 24 Apr 2020 10:12:20 -0500	[thread overview]
Message-ID: <20200424151220.GA134293@google.com> (raw)
In-Reply-To: <d80f8d9e-0676-5661-6031-39fe4460b66c@hisilicon.com>

On Fri, Apr 24, 2020 at 05:11:44PM +0800, Yicong Yang wrote:
> On 2020/4/24 14:02, Saheed Bolarinwa wrote:
> > On 4/24/20 12:38 AM, Bjorn Helgaas wrote:
> >> On Thu, Apr 23, 2020 at 07:55:17PM +0800, Yicong Yang wrote:

> >>> BTW, pci_{read, write}_config_*() may also have the issues that
> >>> export the private err code outside. You may want to solve these in
> >>> a series along with this patch.
> >>
> >> If you see a specific issue, please point it out.
> 
> arch/x86/platform/intel/iosf_mbi.c, iosf_mbi_pci_read_mdr():
>         result = pci_read_config_dword(mbi_pdev, MBI_MDR_OFFSET, mdr);
>         if (result < 0)
>             goto fail_read;
>         return 0;
>     fail_read:
>         dev_err(&mbi_pdev->dev, "PCI config access failed with %d\n", result);
>         return result;

This is a problem in the caller, not in pci_read_config*().  This
caller is definitely broken, but fixing it is material for other
patches, not the current effort to align pcie_capability_read*() and
pci_read_config*().

> >> I looked at pci_read_config_word(), and it can return
> >> PCIBIOS_DEVICE_NOT_FOUND, PCIBIOS_BAD_REGISTER_NUMBER, or the return
> >> value from bus->ops->read().
> >>
> >> I looked at all the users of PCIBIOS_*.  There's really no interesting
> >> use of any of them except by pcibios_err_to_errno() and
> >> xen_pcibios_err_to_errno(), so I'm not sure it's even worth keeping
> >> them.
> 
> maybe we can mark them as deprecated. I can send a RFC one to do so.

Let's put this on a list for later.  I want to make sure this first
effort is successful before throwing more stuff into the mix.

Bjorn

      reply	other threads:[~2020-04-24 15:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-19  6:51 [PATCH RFC] pci: Make return value of pcie_capability_read*() consistent Bolarinwa Olayemi Saheed
2020-04-23 11:55 ` Yicong Yang
2020-04-23 22:38   ` Bjorn Helgaas
2020-04-24  6:02     ` Saheed Bolarinwa
2020-04-24  9:11       ` Yicong Yang
2020-04-24 15:12         ` Bjorn Helgaas [this message]

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=20200424151220.GA134293@google.com \
    --to=helgaas@kernel.org \
    --cc=bjorn@helgaas.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=refactormyself@gmail.com \
    --cc=skhan@linuxfoundation.org \
    --cc=yangyicong@hisilicon.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).