Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 24 May 2021 09:38:03 +0100
Message-ID: <87tumswmqc.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Rob Herring <robh@kernel.org>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>,
        Ley Foon Tan <ley.foon.tan@intel.com>,
        Chris Zankel <chris@zankel.net>,
        Max Filippov <jcmvbkbc@gmail.com>,
        Vineet Gupta <vgupta@synopsys.com>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Robert Jarzmik <robert.jarzmik@free.fr>,
        Russell King <linux@armlinux.org.uk>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Yoshinori Sato <ysato@users.sourceforge.jp>,
        Rich Felker <dalias@libc.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Alex Deucher <alexander.deucher@amd.com>,
        Christian =?UTF-8?B?S8O2bmln?= <christian.koenig@amd.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Rob Clark <robdclark@gmail.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Lee Jones <lee.jones@linaro.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        Android Kernel Team <kernel-team@android.com>
Subject: Re: [PATCH 30/39] PCI: Bulk conversion to generic_handle_domain_irq()
In-Reply-To: <CAL_Jsq+nu8PmONzx2AfysRWuhJDV9Xn3O5rCOfEZL0KoC12_qw@mail.gmail.com>
References: <20210520163751.27325-1-maz@kernel.org>
        <20210520163751.27325-31-maz@kernel.org>
        <CAL_Jsq+nu8PmONzx2AfysRWuhJDV9Xn3O5rCOfEZL0KoC12_qw@mail.gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Thu, 20 May 2021 18:47:06 +0100,
Rob Herring <robh@kernel.org> wrote:
> 
> On Thu, May 20, 2021 at 11:57 AM Marc Zyngier <maz@kernel.org> 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@kernel.org>
> > ---
> >  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.