1. Patchew
  2. linux
  3. [PATCH 0/2] mailbox: qcom-ipcc: fix error reporting paths
  4. View patch

[PATCH 2/2] mailbox: qcom-ipcc: amend qcom_ipcc_irq_fn() to report errors

Gabriele Paoloni posted 2 patches 3 weeks ago
[PATCH 2/2] mailbox: qcom-ipcc: amend qcom_ipcc_irq_fn() to report errors
Posted by Gabriele Paoloni 3 weeks ago 
check the virq value returned by irq_find_mapping(), also
check the return value of generic_handle_irq(); return IRQ_NONE
if either of the checks fails.

Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
---
 drivers/mailbox/qcom-ipcc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mailbox/qcom-ipcc.c b/drivers/mailbox/qcom-ipcc.c
index c23efaaa64a1..0795184591f0 100644
--- a/drivers/mailbox/qcom-ipcc.c
+++ b/drivers/mailbox/qcom-ipcc.c
@@ -75,7 +75,7 @@ static irqreturn_t qcom_ipcc_irq_fn(int irq, void *data)
 {
 	struct qcom_ipcc *ipcc = data;
 	u32 hwirq;
-	int virq;
+	int virq, ret;
 
 	for (;;) {
 		hwirq = readl(ipcc->base + IPCC_REG_RECV_ID);
@@ -83,8 +83,14 @@ static irqreturn_t qcom_ipcc_irq_fn(int irq, void *data)
 			break;
 
 		virq = irq_find_mapping(ipcc->irq_domain, hwirq);
+		if (unlikely(!virq))
+			return IRQ_NONE;
+
 		writel(hwirq, ipcc->base + IPCC_REG_RECV_SIGNAL_CLEAR);
-		generic_handle_irq(virq);
+
+		ret = generic_handle_irq(virq);
+		if (unlikely(ret))
+			return IRQ_NONE;
 	}
 
 	return IRQ_HANDLED;
-- 
2.48.1
Re: [PATCH 2/2] mailbox: qcom-ipcc: amend qcom_ipcc_irq_fn() to report errors
Posted by Konrad Dybcio 2 weeks, 6 days ago 
On 3/16/26 11:26 AM, Gabriele Paoloni wrote:
> check the virq value returned by irq_find_mapping(), also
> check the return value of generic_handle_irq(); return IRQ_NONE
> if either of the checks fails.
> 
> Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
> ---

It certainly seems useful to inspect a function's return value, however I
noticed that out of 47 in-tree callers of generic_handle_irq() within
drivers/, only one other driver (intel_lpe_audio within drm/i915) checks
that..

Similarly, many other drivers never check if irq_find_mapping() actually
succeeded. This makes me wonder how you discovered there's an issue in
the first place. Could you please add some details in the commit message?

Konrad
Re: [PATCH 2/2] mailbox: qcom-ipcc: amend qcom_ipcc_irq_fn() to report errors
Posted by Gabriele Paoloni 2 weeks, 3 days ago 
Hi Konrad

On Tue, Mar 17, 2026 at 10:13 AM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 3/16/26 11:26 AM, Gabriele Paoloni wrote:
> > check the virq value returned by irq_find_mapping(), also
> > check the return value of generic_handle_irq(); return IRQ_NONE
> > if either of the checks fails.
> >
> > Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
> > ---
>
> It certainly seems useful to inspect a function's return value, however I
> noticed that out of 47 in-tree callers of generic_handle_irq() within
> drivers/, only one other driver (intel_lpe_audio within drm/i915) checks
> that..
>
> Similarly, many other drivers never check if irq_find_mapping() actually
> succeeded. This makes me wonder how you discovered there's an issue in
> the first place. Could you please add some details in the commit message?

Code inspection spotted the issue, and I could not find a justification for
irq_find_mapping() and generic_handle_irq() to always succeed; hence
I proposed this patch to follow good coding practices.
However if there is a justification for these functions to always succeed
in the context of use of the Qualcomm IPCC driver, I can drop this patch.

Thanks
Gab

>
> Konrad
>
Re: [PATCH 2/2] mailbox: qcom-ipcc: amend qcom_ipcc_irq_fn() to report errors
Posted by Konrad Dybcio 2 weeks ago 
On 3/20/26 12:24 PM, Gabriele Paoloni wrote:
> Hi Konrad
> 
> On Tue, Mar 17, 2026 at 10:13 AM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 3/16/26 11:26 AM, Gabriele Paoloni wrote:
>>> check the virq value returned by irq_find_mapping(), also
>>> check the return value of generic_handle_irq(); return IRQ_NONE
>>> if either of the checks fails.
>>>
>>> Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
>>> ---
>>
>> It certainly seems useful to inspect a function's return value, however I
>> noticed that out of 47 in-tree callers of generic_handle_irq() within
>> drivers/, only one other driver (intel_lpe_audio within drm/i915) checks
>> that..
>>
>> Similarly, many other drivers never check if irq_find_mapping() actually
>> succeeded. This makes me wonder how you discovered there's an issue in
>> the first place. Could you please add some details in the commit message?
> 
> Code inspection spotted the issue, and I could not find a justification for
> irq_find_mapping() and generic_handle_irq() to always succeed; hence
> I proposed this patch to follow good coding practices.

That's a good enough reason

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad
Re: [PATCH 2/2] mailbox: qcom-ipcc: amend qcom_ipcc_irq_fn() to report errors
Posted by Brian Masney 3 weeks ago 
On Mon, Mar 16, 2026 at 11:26:18AM +0100, Gabriele Paoloni wrote:
> check the virq value returned by irq_find_mapping(), also
> check the return value of generic_handle_irq(); return IRQ_NONE
> if either of the checks fails.
> 
> Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
> ---
>  drivers/mailbox/qcom-ipcc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mailbox/qcom-ipcc.c b/drivers/mailbox/qcom-ipcc.c
> index c23efaaa64a1..0795184591f0 100644
> --- a/drivers/mailbox/qcom-ipcc.c
> +++ b/drivers/mailbox/qcom-ipcc.c
> @@ -75,7 +75,7 @@ static irqreturn_t qcom_ipcc_irq_fn(int irq, void *data)
>  {
>  	struct qcom_ipcc *ipcc = data;
>  	u32 hwirq;
> -	int virq;
> +	int virq, ret;

Put variables in reverse Christmas tree order.

Brian


>  
>  	for (;;) {
>  		hwirq = readl(ipcc->base + IPCC_REG_RECV_ID);
> @@ -83,8 +83,14 @@ static irqreturn_t qcom_ipcc_irq_fn(int irq, void *data)
>  			break;
>  
>  		virq = irq_find_mapping(ipcc->irq_domain, hwirq);
> +		if (unlikely(!virq))
> +			return IRQ_NONE;
> +
>  		writel(hwirq, ipcc->base + IPCC_REG_RECV_SIGNAL_CLEAR);
> -		generic_handle_irq(virq);
> +
> +		ret = generic_handle_irq(virq);
> +		if (unlikely(ret))
> +			return IRQ_NONE;
>  	}
>  
>  	return IRQ_HANDLED;
> -- 
> 2.48.1
>
Re: [PATCH 2/2] mailbox: qcom-ipcc: amend qcom_ipcc_irq_fn() to report errors
Posted by Gabriele Paoloni 2 weeks, 3 days ago 
On Mon, Mar 16, 2026 at 12:05 PM Brian Masney <bmasney@redhat.com> wrote:
>
> On Mon, Mar 16, 2026 at 11:26:18AM +0100, Gabriele Paoloni wrote:
> > check the virq value returned by irq_find_mapping(), also
> > check the return value of generic_handle_irq(); return IRQ_NONE
> > if either of the checks fails.
> >
> > Signed-off-by: Gabriele Paoloni <gpaoloni@redhat.com>
> > ---
> >  drivers/mailbox/qcom-ipcc.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mailbox/qcom-ipcc.c b/drivers/mailbox/qcom-ipcc.c
> > index c23efaaa64a1..0795184591f0 100644
> > --- a/drivers/mailbox/qcom-ipcc.c
> > +++ b/drivers/mailbox/qcom-ipcc.c
> > @@ -75,7 +75,7 @@ static irqreturn_t qcom_ipcc_irq_fn(int irq, void *data)
> >  {
> >       struct qcom_ipcc *ipcc = data;
> >       u32 hwirq;
> > -     int virq;
> > +     int virq, ret;
>
> Put variables in reverse Christmas tree order.

Note, many thanks. I will fix this in v2

Gab

>
> Brian
>
>
> >
> >       for (;;) {
> >               hwirq = readl(ipcc->base + IPCC_REG_RECV_ID);
> > @@ -83,8 +83,14 @@ static irqreturn_t qcom_ipcc_irq_fn(int irq, void *data)
> >                       break;
> >
> >               virq = irq_find_mapping(ipcc->irq_domain, hwirq);
> > +             if (unlikely(!virq))
> > +                     return IRQ_NONE;
> > +
> >               writel(hwirq, ipcc->base + IPCC_REG_RECV_SIGNAL_CLEAR);
> > -             generic_handle_irq(virq);
> > +
> > +             ret = generic_handle_irq(virq);
> > +             if (unlikely(ret))
> > +                     return IRQ_NONE;
> >       }
> >
> >       return IRQ_HANDLED;
> > --
> > 2.48.1
> >
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.