Received-SPF: pass (google.com: domain of linux-kernel+bounces-55106-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 6 Feb 2024 16:25:06 +0200 (EET)
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()
In-Reply-To: <CAMhs-H9QaQCz70dmBfMWJ4xoSagrYCJLqejA72fqb50uDwZY9A@mail.gmail.com>
Message-ID: <a0db7b09-75e0-2e31-5287-60f6be180d60@linux.intel.com>
References: <20240205133450.5222-1-ilpo.jarvinen@linux.intel.com> <20240205133450.5222-4-ilpo.jarvinen@linux.intel.com> <CAMhs-H9QaQCz70dmBfMWJ4xoSagrYCJLqejA72fqb50uDwZY9A@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; BOUNDARY="8323328-1474976237-1707152415=:1027"
Content-ID: <a6079b97-9e10-d27a-72a2-25462cc0603c@linux.intel.com>

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1474976237-1707152415=:1027
Content-Type: text/plain; CHARSET=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Content-ID: <76afa036-4192-9aa8-2f0d-bde05d65a403@linux.intel.com>

On Mon, 5 Feb 2024, Sergio Paracuellos wrote:
> On Mon, Feb 5, 2024 at 3:35=E2=80=AFPM Ilpo J=C3=A4rvinen
> <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=C3=A4rvinen <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 d=
evfn, int where,
> >  {
> >         if (bus->parent =3D=3D NULL &&
> >             devfn >=3D 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),
>=20
> 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=20
intend to convert these functions to return generic errno and push the=20
PCIBIOS error code -> errno conversion into where it's really needed (real=
=20
PCIBIOS access functions in arch/x86/pci/pcbios.c). Returning 0 as literal=
=20
is very common cosmetic "error" in these functions. While calculating the=
=20
error rate in return values of these functions (I'm able to do that=20
because of the audit), those were not even included to 15% returning=20
-Esomething instead of PCIBIOS_*. It would be way above that if I'd count=
=20
return 0 also as an error.

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

Thanks for the review.

--=20
 i.
--8323328-1474976237-1707152415=:1027--