Linux-MIPS Archive mirror
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Sergio Paracuellos <sergio.paracuellos@gmail.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	 linux-mips@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/4] MIPS: PCI: Return PCIBIOS_* from tx4927_pci_config_read/write()
Date: Tue, 6 Feb 2024 16:25:06 +0200 (EET)	[thread overview]
Message-ID: <a0db7b09-75e0-2e31-5287-60f6be180d60@linux.intel.com> (raw)
In-Reply-To: <CAMhs-H9QaQCz70dmBfMWJ4xoSagrYCJLqejA72fqb50uDwZY9A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2202 bytes --]

On Mon, 5 Feb 2024, Sergio Paracuellos wrote:
> On Mon, Feb 5, 2024 at 3:35 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > pci_ops .read/.write must return PCIBIOS_* codes but
> > tx4927_pci_config_read/write() return -1 when mkaddr() cannot find
> > devfn from the root bus. Return PCIBIOS_DEVICE_NOT_FOUND instead and
> > pass that onward in the call chain instead of overwriting the return
> > value.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  arch/mips/pci/ops-tx4927.c | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/mips/pci/ops-tx4927.c b/arch/mips/pci/ops-tx4927.c
> > index f7802f100401..4dd8b93985fb 100644
> > --- a/arch/mips/pci/ops-tx4927.c
> > +++ b/arch/mips/pci/ops-tx4927.c
> > @@ -60,7 +60,7 @@ static int mkaddr(struct pci_bus *bus, unsigned int devfn, int where,
> >  {
> >         if (bus->parent == NULL &&
> >             devfn >= PCI_DEVFN(TX4927_PCIC_MAX_DEVNU, 0))
> > -               return -1;
> > +               return PCIBIOS_DEVICE_NOT_FOUND;
> >         __raw_writel(((bus->number & 0xff) << 0x10)
> >                      | ((devfn & 0xff) << 0x08) | (where & 0xfc)
> >                      | (bus->parent ? 1 : 0),
> 
> Should we also return PCIBIOS_SUCCESSFUL instead of 'return 0' in
> 'mkaddr' for coherency?

Yeah right, I'll change it too.

I didn't take notice of that because the reason for all this is that I 
intend to convert these functions to return generic errno and push the 
PCIBIOS error code -> errno conversion into where it's really needed (real 
PCIBIOS access functions in arch/x86/pci/pcbios.c). Returning 0 as literal 
is very common cosmetic "error" in these functions. While calculating the 
error rate in return values of these functions (I'm able to do that 
because of the audit), those were not even included to 15% returning 
-Esomething instead of PCIBIOS_*. It would be way above that if I'd count 
return 0 also as an error.

> Other than that, changes look good to me.
> 
> Reviewed-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>

Thanks for the review.

-- 
 i.

  reply	other threads:[~2024-02-06 14:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-05 13:34 [PATCH 0/4] MIPS: Cleanups to PCI related code Ilpo Järvinen
2024-02-05 13:34 ` [PATCH 1/4] MIPS: lantiq: Remove unused function pointer variables Ilpo Järvinen
2024-02-05 13:34 ` [PATCH 2/4] MIPS: ath79: Don't return PCIBIOS_* code from pcibios_enable_device() Ilpo Järvinen
2024-02-05 13:34 ` [PATCH 3/4] MIPS: PCI: Return PCIBIOS_* from tx4927_pci_config_read/write() Ilpo Järvinen
2024-02-05 14:52   ` Sergio Paracuellos
2024-02-06 14:25     ` Ilpo Järvinen [this message]
2024-02-05 13:34 ` [PATCH 4/4] MIPS: TXx9: Use PCI_SET_ERROR_RESPONSE() Ilpo Järvinen

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=a0db7b09-75e0-2e31-5287-60f6be180d60@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=sergio.paracuellos@gmail.com \
    --cc=tsbogend@alpha.franken.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 read-only IMAP folder(s) and NNTP newsgroup(s).