mbox series

[v3,0/6] PCI/treewide: Cleanup/streamline PCI error code handling

Message ID 20230911125354.25501-1-ilpo.jarvinen@linux.intel.com (mailing list archive)
Headers show
Series PCI/treewide: Cleanup/streamline PCI error code handling | expand

Message

Ilpo Järvinen Sept. 11, 2023, 12:53 p.m. UTC
As the first step towards converting PCI accessor function return codes
into normal errnos this series cleans up related code paths which have
complicated multi-line construct to handle the PCI error checking.

I'd prefer these (the remaining ones) to be routed through PCI tree due
to PCI accessor function return code conversion being built on top of
them.

v3:
- Return pci_generic_config_read32()'s error code directly
- Removed already accepted patches

v2:
- Moved ret local var to the inner block (I2C: ali15x3)
- Removed already accepted patches


Ilpo Järvinen (6):
  alpha: Streamline convoluted PCI error handling
  sh: pci: Do PCI error check on own line
  atm: iphase: Do PCI error checks on own line
  PCI: Do error check on own line to split long if conditions
  PCI: xgene: Do PCI error check on own line & keep return value
  scsi: ipr: Do PCI error checks on own line

 arch/alpha/kernel/sys_miata.c      | 17 +++++++++--------
 arch/sh/drivers/pci/common.c       |  7 ++++---
 drivers/atm/iphase.c               | 20 +++++++++++---------
 drivers/pci/controller/pci-xgene.c |  7 ++++---
 drivers/pci/pci.c                  |  9 ++++++---
 drivers/pci/probe.c                |  6 +++---
 drivers/pci/quirks.c               |  6 +++---
 drivers/scsi/ipr.c                 | 12 ++++++++----
 8 files changed, 48 insertions(+), 36 deletions(-)

Comments

Bjorn Helgaas Oct. 10, 2023, 10:35 p.m. UTC | #1
[+cc Tadeusz; updates to quirk_intel_qat_vf_cap()]

On Mon, Sep 11, 2023 at 03:53:48PM +0300, Ilpo Järvinen wrote:
> As the first step towards converting PCI accessor function return codes
> into normal errnos this series cleans up related code paths which have
> complicated multi-line construct to handle the PCI error checking.
> 
> I'd prefer these (the remaining ones) to be routed through PCI tree due
> to PCI accessor function return code conversion being built on top of
> them.
> 
> v3:
> - Return pci_generic_config_read32()'s error code directly
> - Removed already accepted patches
> 
> v2:
> - Moved ret local var to the inner block (I2C: ali15x3)
> - Removed already accepted patches
> 
> 
> Ilpo Järvinen (6):
>   alpha: Streamline convoluted PCI error handling
>   sh: pci: Do PCI error check on own line
>   atm: iphase: Do PCI error checks on own line
>   PCI: Do error check on own line to split long if conditions
>   PCI: xgene: Do PCI error check on own line & keep return value
>   scsi: ipr: Do PCI error checks on own line
> 
>  arch/alpha/kernel/sys_miata.c      | 17 +++++++++--------
>  arch/sh/drivers/pci/common.c       |  7 ++++---
>  drivers/atm/iphase.c               | 20 +++++++++++---------
>  drivers/pci/controller/pci-xgene.c |  7 ++++---
>  drivers/pci/pci.c                  |  9 ++++++---
>  drivers/pci/probe.c                |  6 +++---
>  drivers/pci/quirks.c               |  6 +++---
>  drivers/scsi/ipr.c                 | 12 ++++++++----
>  8 files changed, 48 insertions(+), 36 deletions(-)

Applied all to pci/config-errs for v6.7, thanks!

I made the tweaks below; heads-up to John Paul and Tadeusz.

Oh, and weird experience applying these via b4, git am: the
Signed-off-by was corrupted on these patches:

  https://lore.kernel.org/r/20230911125354.25501-7-ilpo.jarvinen@linux.intel.com  https://lore.kernel.org/r/20230911125354.25501-6-ilpo.jarvinen@linux.intel.com  https://lore.kernel.org/r/20230911125354.25501-3-ilpo.jarvinen@linux.intel.com

It looked like this:

  Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Not sure why this happened; maybe one of the mailing lists screwed it
up and the order of arrival determines which one b4 uses?  The ones
from linux-alpha look like:

  Signed-off-by: Ilpo J=C3=A4rvinen <ilpo.jarvinen@linux.intel.com>

which I think corresponds to the bad rendering.  I think I fixed them
all.

Bjorn

diff --git a/arch/sh/drivers/pci/common.c b/arch/sh/drivers/pci/common.c
index f59e5b9a6a80..ab9e791070b4 100644
--- a/arch/sh/drivers/pci/common.c
+++ b/arch/sh/drivers/pci/common.c
@@ -50,7 +50,7 @@ int __init pci_is_66mhz_capable(struct pci_channel *hose,
 				int top_bus, int current_bus)
 {
 	u32 pci_devfn;
-	unsigned short vid;
+	u16 vid;
 	int cap66 = -1;
 	u16 stat;
 	int ret;
@@ -64,7 +64,7 @@ int __init pci_is_66mhz_capable(struct pci_channel *hose,
 					     pci_devfn, PCI_VENDOR_ID, &vid);
 		if (ret != PCIBIOS_SUCCESSFUL)
 			continue;
-		if (vid == 0xffff)
+		if (PCI_POSSIBLE_ERROR(vid))
 			continue;
 
 		/* check 66MHz capability */
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 81f3da536a3c..f5fc92441194 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5430,7 +5430,7 @@ static void quirk_intel_qat_vf_cap(struct pci_dev *pdev)
 
 		pdev->cfg_size = PCI_CFG_SPACE_EXP_SIZE;
 		ret = pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &status);
-		if ((ret != PCIBIOS_SUCCESSFUL) || (status == 0xffffffff))
+		if ((ret != PCIBIOS_SUCCESSFUL) || (PCI_POSSIBLE_ERROR(status)))
 			pdev->cfg_size = PCI_CFG_SPACE_SIZE;
 
 		if (pci_find_saved_cap(pdev, PCI_CAP_ID_EXP))
Ilpo Järvinen Oct. 11, 2023, 11:07 a.m. UTC | #2
On Tue, 10 Oct 2023, Bjorn Helgaas wrote:

> [+cc Tadeusz; updates to quirk_intel_qat_vf_cap()]
> 
> On Mon, Sep 11, 2023 at 03:53:48PM +0300, Ilpo Järvinen wrote:
> > As the first step towards converting PCI accessor function return codes
> > into normal errnos this series cleans up related code paths which have
> > complicated multi-line construct to handle the PCI error checking.
> > 
> > I'd prefer these (the remaining ones) to be routed through PCI tree due
> > to PCI accessor function return code conversion being built on top of
> > them.
> > 
> > v3:
> > - Return pci_generic_config_read32()'s error code directly
> > - Removed already accepted patches
> > 
> > v2:
> > - Moved ret local var to the inner block (I2C: ali15x3)
> > - Removed already accepted patches
> > 
> > 
> > Ilpo Järvinen (6):
> >   alpha: Streamline convoluted PCI error handling
> >   sh: pci: Do PCI error check on own line
> >   atm: iphase: Do PCI error checks on own line
> >   PCI: Do error check on own line to split long if conditions
> >   PCI: xgene: Do PCI error check on own line & keep return value
> >   scsi: ipr: Do PCI error checks on own line
> > 
> >  arch/alpha/kernel/sys_miata.c      | 17 +++++++++--------
> >  arch/sh/drivers/pci/common.c       |  7 ++++---
> >  drivers/atm/iphase.c               | 20 +++++++++++---------
> >  drivers/pci/controller/pci-xgene.c |  7 ++++---
> >  drivers/pci/pci.c                  |  9 ++++++---
> >  drivers/pci/probe.c                |  6 +++---
> >  drivers/pci/quirks.c               |  6 +++---
> >  drivers/scsi/ipr.c                 | 12 ++++++++----
> >  8 files changed, 48 insertions(+), 36 deletions(-)
> 
> Applied all to pci/config-errs for v6.7, thanks!
> 
> I made the tweaks below; heads-up to John Paul and Tadeusz.
> 
> Oh, and weird experience applying these via b4, git am: the
> Signed-off-by was corrupted on these patches:
> 
>   https://lore.kernel.org/r/20230911125354.25501-7-ilpo.jarvinen@linux.intel.com  https://lore.kernel.org/r/20230911125354.25501-6-ilpo.jarvinen@linux.intel.com  https://lore.kernel.org/r/20230911125354.25501-3-ilpo.jarvinen@linux.intel.com
> 
> It looked like this:
> 
>   Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> Not sure why this happened; maybe one of the mailing lists screwed it
> up and the order of arrival determines which one b4 uses?  The ones
> from linux-alpha look like:
> 
>   Signed-off-by: Ilpo J=C3=A4rvinen <ilpo.jarvinen@linux.intel.com>
> 
> which I think corresponds to the bad rendering.  I think I fixed them
> all.

Thanks for letting me know. It seems that copies of the same mail

https://lore.kernel.org/linux-alpha/...
https://lore.kernel.org/linux-pci/...

do indeed differ for some reason. I'll probably have to experiment to see 
if I can reproduce problem with the linux-alpha list.