Re: [PATCH 30/39] PCI: Bulk conversion to generic_handle_domain_irq()

[Marc Zyngier]

On Thu, 20 May 2021 18:47:06 +0100,
Rob Herring <robh@xxxxxxxxxx> wrote:
> 
> On Thu, May 20, 2021 at 11:57 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> >
> > Wherever possible, replace constructs that match either
> > generic_handle_irq(irq_find_mapping()) or
> > generic_handle_irq(irq_linear_revmap()) to a single call to
> > generic_handle_domain_irq().
> >
> > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> > ---
> >  drivers/pci/controller/dwc/pci-dra7xx.c        | 14 +++++---------
> >  drivers/pci/controller/dwc/pci-keystone.c      |  5 ++---
> >  .../pci/controller/dwc/pcie-designware-host.c  |  9 ++++-----
> >  drivers/pci/controller/dwc/pcie-uniphier.c     |  6 ++----
> >  .../controller/mobiveil/pcie-mobiveil-host.c   | 15 ++++++---------
> >  drivers/pci/controller/pci-aardvark.c          |  5 ++---
> >  drivers/pci/controller/pci-ftpci100.c          |  2 +-
> >  drivers/pci/controller/pci-tegra.c             |  8 +++-----
> >  drivers/pci/controller/pci-xgene-msi.c         |  9 +++------
> >  drivers/pci/controller/pcie-altera-msi.c       | 10 ++++------
> >  drivers/pci/controller/pcie-altera.c           | 10 ++++------
> >  drivers/pci/controller/pcie-brcmstb.c          |  9 ++++-----
> >  drivers/pci/controller/pcie-iproc-msi.c        |  4 +---
> >  drivers/pci/controller/pcie-mediatek-gen3.c    | 13 ++++---------
> >  drivers/pci/controller/pcie-mediatek.c         | 12 ++++--------
> >  drivers/pci/controller/pcie-microchip-host.c   | 18 +++++++-----------
> >  drivers/pci/controller/pcie-rcar-host.c        |  8 +++-----
> >  drivers/pci/controller/pcie-rockchip-host.c    |  8 +++-----
> >  drivers/pci/controller/pcie-xilinx-cpm.c       |  4 ++--
> >  drivers/pci/controller/pcie-xilinx-nwl.c       | 13 +++----------
> >  drivers/pci/controller/pcie-xilinx.c           |  9 ++++-----
> >  21 files changed, 71 insertions(+), 120 deletions(-)
> 
> 
> > diff --git a/drivers/pci/controller/pci-xgene-msi.c b/drivers/pci/controller/pci-xgene-msi.c
> > index 1c34c897a7e2..cf3832b905e8 100644
> > --- a/drivers/pci/controller/pci-xgene-msi.c
> > +++ b/drivers/pci/controller/pci-xgene-msi.c
> > @@ -291,8 +291,7 @@ static void xgene_msi_isr(struct irq_desc *desc)
> >         struct irq_chip *chip = irq_desc_get_chip(desc);
> >         struct xgene_msi_group *msi_groups;
> >         struct xgene_msi *xgene_msi;
> > -       unsigned int virq;
> > -       int msir_index, msir_val, hw_irq;
> > +       int msir_index, msir_val, hw_irq, ret;
> >         u32 intr_index, grp_select, msi_grp;
> >
> >         chained_irq_enter(chip, desc);
> > @@ -330,10 +329,8 @@ static void xgene_msi_isr(struct irq_desc *desc)
> >                          * CPU0
> >                          */
> >                         hw_irq = hwirq_to_canonical_hwirq(hw_irq);
> > -                       virq = irq_find_mapping(xgene_msi->inner_domain, hw_irq);
> > -                       WARN_ON(!virq);
> > -                       if (virq != 0)
> > -                               generic_handle_irq(virq);
> > +                       ret = generic_handle_domain_irq(xgene_msi->inner_domain, hw_irq);
> > +                       WARN_ON(ret);
> 
> There's various error prints in some of the handlers. I think they
> should be moved to the core. I can't imagine handling the irq is ever
> optional.

Printing stuff like this is a sure recipe for disaster, and there is
no way I'm moving such crap into core code. If the interrupt handling
fails (most likely because there is no mapping for this interrupt), it
is the driver's responsibility to handle the error (either disabling
the input or the output of the secondary irqchip). There isn't much
the core code can do about it.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.