[PATCH v9 0/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[PATCH v9 0/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[Pan Chuang]

There are over 700 calls to devm_request_threaded_irq() and more than 1000
calls to devm_request_irq() in the kernel. Currently, most drivers implement
repetitive and inconsistent error handling for these functions:

1. Over 2000 lines of code are dedicated to error messages
2. Analysis shows 519 unique error messages with 323 variants after normalization
3. 186 messages provide no useful debugging information
4. Only a small fraction deliver meaningful error context

As tglx pointed out:
  "It's not a general allocator like kmalloc(). It's specialized and in the
   vast majority of cases failing to request the interrupt causes the device
   probe to fail. So having proper and consistent information why the device
   cannot be used is useful."

This patch implements a standardized error reporting approach[1]:

1. Renames existing functions to __devm_request_threaded_irq() and
   __devm_request_any_context_irq()
2. Creates new devm_request_threaded_irq() and devm_request_any_context_irq()
   that:
   a) Invoke the underscore-prefixed variants
   b) On error, call dev_err_probe() to provide consistent diagnostics

The new error format provides complete debugging context:
  "<device>: error -<errcode>: request_irq(<irq>) <handler> <thread_fn> <devname>"

Example from our QEMU testing:
  test_irq_device: error -EINVAL: request_irq(1001) test_handler [test_irq] test_thread_fn [test_irq] irq-1001-failure

Based on the v7 and v8, standardize coding style without logical change.
https://lore.kernel.org/all/20250728123251.384375-2-panchuang@vivo.com/

[1]https://lore.kernel.org/all/87qzy9tvso.ffs@tglx/

Pan Chuang (1):
  genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and
    devm_request_any_context_irq()

 kernel/irq/devres.c | 121 +++++++++++++++++++++++++++++---------------
 1 file changed, 81 insertions(+), 40 deletions(-)

-- 
2.34.1

[PATCH v9 1/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[Pan Chuang]

The devm_request_threaded_irq() and devm_request_any_context_irq() currently
don't print any error when interrupt registration fails. This forces each
driver to implement redundant error logging - over 2,000 lines of error
messages exist across drivers. Additionally, when upper-layer functions
propagate these errors without logging, critical debugging information is lost.

Add automatic error logging to these functions via dev_err_probe(), printing
device name, IRQ number, handler functions, and error code on failure.

Co-developed-by: Yangtao Li <frank.li@vivo.com>
Signed-off-by: Yangtao Li <frank.li@vivo.com>
Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 kernel/irq/devres.c | 121 +++++++++++++++++++++++++++++---------------
 1 file changed, 81 insertions(+), 40 deletions(-)

diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
index eb16a58e0322..dcbb9d0cd736 100644
--- a/kernel/irq/devres.c
+++ b/kernel/irq/devres.c
@@ -30,29 +30,11 @@ static int devm_irq_match(struct device *dev, void *res, void *data)
 	return this->irq == match->irq && this->dev_id == match->dev_id;
 }
 
-/**
- *	devm_request_threaded_irq - allocate an interrupt line for a managed device
- *	@dev: device to request interrupt for
- *	@irq: Interrupt line to allocate
- *	@handler: Function to be called when the IRQ occurs
- *	@thread_fn: function to be called in a threaded interrupt context. NULL
- *		    for devices which handle everything in @handler
- *	@irqflags: Interrupt type flags
- *	@devname: An ascii name for the claiming device, dev_name(dev) if NULL
- *	@dev_id: A cookie passed back to the handler function
- *
- *	Except for the extra @dev argument, this function takes the
- *	same arguments and performs the same function as
- *	request_threaded_irq().  IRQs requested with this function will be
- *	automatically freed on driver detach.
- *
- *	If an IRQ allocated with this function needs to be freed
- *	separately, devm_free_irq() must be used.
- */
-int devm_request_threaded_irq(struct device *dev, unsigned int irq,
-			      irq_handler_t handler, irq_handler_t thread_fn,
-			      unsigned long irqflags, const char *devname,
-			      void *dev_id)
+static int __devm_request_threaded_irq(struct device *dev, unsigned int irq,
+				       irq_handler_t handler,
+				       irq_handler_t thread_fn,
+				       unsigned long irqflags,
+				       const char *devname, void *dev_id)
 {
 	struct irq_devres *dr;
 	int rc;
@@ -78,28 +60,50 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq,
 
 	return 0;
 }
-EXPORT_SYMBOL(devm_request_threaded_irq);
 
 /**
- *	devm_request_any_context_irq - allocate an interrupt line for a managed device
- *	@dev: device to request interrupt for
- *	@irq: Interrupt line to allocate
- *	@handler: Function to be called when the IRQ occurs
- *	@irqflags: Interrupt type flags
- *	@devname: An ascii name for the claiming device, dev_name(dev) if NULL
- *	@dev_id: A cookie passed back to the handler function
+ * devm_request_threaded_irq - allocate an interrupt line for a managed device with error logging
+ * @dev:	Device to request interrupt for
+ * @irq:	Interrupt line to allocate
+ * @handler:	Function to be called when the IRQ occurs
+ * @thread_fn:	Function to be called in a threaded interrupt context. NULL
+ *		for devices which handle everything in @handler
+ * @irqflags:	Interrupt type flags
+ * @devname:	An ascii name for the claiming device, dev_name(dev) if NULL
+ * @dev_id:	A cookie passed back to the handler function
  *
- *	Except for the extra @dev argument, this function takes the
- *	same arguments and performs the same function as
- *	request_any_context_irq().  IRQs requested with this function will be
- *	automatically freed on driver detach.
+ * Except for the extra @dev argument, this function takes the same arguments
+ * and performs the same function as request_threaded_irq().  IRQs requested
+ * with this function will be automatically freed on driver detach.
+ *
+ * If an IRQ allocated with this function needs to be freed separately,
+ * devm_free_irq() must be used.
+ *
+ * When the request fails, an error message is printed with contextual
+ * information (device name, interrupt number, handler functions and
+ * error code). Don't add extra error messages at the call sites.
  *
- *	If an IRQ allocated with this function needs to be freed
- *	separately, devm_free_irq() must be used.
+ * Return: 0 on success or a negative error number.
  */
-int devm_request_any_context_irq(struct device *dev, unsigned int irq,
-			      irq_handler_t handler, unsigned long irqflags,
-			      const char *devname, void *dev_id)
+int devm_request_threaded_irq(struct device *dev, unsigned int irq,
+			      irq_handler_t handler, irq_handler_t thread_fn,
+			      unsigned long irqflags, const char *devname,
+			      void *dev_id)
+{
+	int rc = __devm_request_threaded_irq(dev, irq, handler, thread_fn,
+					     irqflags, devname, dev_id);
+	if (!rc)
+		return 0;
+
+	return dev_err_probe(dev, rc, "request_irq(%u) %ps %ps %s\n",
+			     irq, handler, thread_fn, devname ? : "");
+}
+EXPORT_SYMBOL(devm_request_threaded_irq);
+
+static int __devm_request_any_context_irq(struct device *dev, unsigned int irq,
+					  irq_handler_t handler,
+					  unsigned long irqflags,
+					  const char *devname, void *dev_id)
 {
 	struct irq_devres *dr;
 	int rc;
@@ -124,6 +128,43 @@ int devm_request_any_context_irq(struct device *dev, unsigned int irq,
 
 	return rc;
 }
+
+/**
+ * devm_request_any_context_irq - allocate an interrupt line for a managed device with error logging
+ * @dev:	Device to request interrupt for
+ * @irq:	Interrupt line to allocate
+ * @handler:	Function to be called when the IRQ occurs
+ * @irqflags:	Interrupt type flags
+ * @devname:	An ascii name for the claiming device, dev_name(dev) if NULL
+ * @dev_id:	A cookie passed back to the handler function
+ *
+ * Except for the extra @dev argument, this function takes the same arguments
+ * and performs the same function as request_any_context_irq().  IRQs requested
+ * with this function will be automatically freed on driver detach.
+ *
+ * If an IRQ allocated with this function needs to be freed separately,
+ * devm_free_irq() must be used.
+ *
+ * When the request fails, an error message is printed with contextual
+ * information (device name, interrupt number, handler functions and
+ * error code). Don't add extra error messages at the call sites.
+ *
+ * Return: IRQC_IS_HARDIRQ or IRQC_IS_NESTED on success, or a negative error
+ * number.
+ */
+int devm_request_any_context_irq(struct device *dev, unsigned int irq,
+				 irq_handler_t handler, unsigned long irqflags,
+				 const char *devname, void *dev_id)
+{
+	int rc = __devm_request_any_context_irq(dev, irq, handler, irqflags,
+						devname, dev_id);
+	if (rc < 0) {
+		return dev_err_probe(dev, rc, "request_irq(%u) %ps %s\n",
+				     irq, handler, devname ? : "");
+	}
+
+	return rc;
+}
 EXPORT_SYMBOL(devm_request_any_context_irq);
 
 /**
-- 
2.34.1

Re: [PATCH v9 0/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[Markus Elfring]

…
> This patch implements a standardized error reporting approach[1]:
…

Do you imagine that another patch series would become helpful
for the presented software transformation approach?

Regards,
Markus

Re: [PATCH v9 1/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[Christophe JAILLET]

Le 30/07/2025 à 08:25, Pan Chuang a écrit :
> The devm_request_threaded_irq() and devm_request_any_context_irq() currently
> don't print any error when interrupt registration fails. This forces each
> driver to implement redundant error logging - over 2,000 lines of error
> messages exist across drivers. Additionally, when upper-layer functions
> propagate these errors without logging, critical debugging information is lost.
> 
> Add automatic error logging to these functions via dev_err_probe(), printing
> device name, IRQ number, handler functions, and error code on failure.
> 
> Co-developed-by: Yangtao Li <frank.li@vivo.com>
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> Signed-off-by: Pan Chuang <panchuang@vivo.com>
> ---
>   kernel/irq/devres.c | 121 +++++++++++++++++++++++++++++---------------
>   1 file changed, 81 insertions(+), 40 deletions(-)
> 
> diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
> index eb16a58e0322..dcbb9d0cd736 100644
> --- a/kernel/irq/devres.c
> +++ b/kernel/irq/devres.c
> @@ -30,29 +30,11 @@ static int devm_irq_match(struct device *dev, void *res, void *data)
>   	return this->irq == match->irq && this->dev_id == match->dev_id;
>   }
>   
> -/**
> - *	devm_request_threaded_irq - allocate an interrupt line for a managed device
> - *	@dev: device to request interrupt for
> - *	@irq: Interrupt line to allocate
> - *	@handler: Function to be called when the IRQ occurs
> - *	@thread_fn: function to be called in a threaded interrupt context. NULL
> - *		    for devices which handle everything in @handler
> - *	@irqflags: Interrupt type flags
> - *	@devname: An ascii name for the claiming device, dev_name(dev) if NULL
> - *	@dev_id: A cookie passed back to the handler function
> - *
> - *	Except for the extra @dev argument, this function takes the
> - *	same arguments and performs the same function as
> - *	request_threaded_irq().  IRQs requested with this function will be
> - *	automatically freed on driver detach.
> - *
> - *	If an IRQ allocated with this function needs to be freed
> - *	separately, devm_free_irq() must be used.
> - */
> -int devm_request_threaded_irq(struct device *dev, unsigned int irq,
> -			      irq_handler_t handler, irq_handler_t thread_fn,
> -			      unsigned long irqflags, const char *devname,
> -			      void *dev_id)
> +static int __devm_request_threaded_irq(struct device *dev, unsigned int irq,
> +				       irq_handler_t handler,
> +				       irq_handler_t thread_fn,
> +				       unsigned long irqflags,
> +				       const char *devname, void *dev_id)
>   {
>   	struct irq_devres *dr;
>   	int rc;
> @@ -78,28 +60,50 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq,
>   
>   	return 0;
>   }
> -EXPORT_SYMBOL(devm_request_threaded_irq);
>   
>   /**
> - *	devm_request_any_context_irq - allocate an interrupt line for a managed device
> - *	@dev: device to request interrupt for
> - *	@irq: Interrupt line to allocate
> - *	@handler: Function to be called when the IRQ occurs
> - *	@irqflags: Interrupt type flags
> - *	@devname: An ascii name for the claiming device, dev_name(dev) if NULL
> - *	@dev_id: A cookie passed back to the handler function
> + * devm_request_threaded_irq - allocate an interrupt line for a managed device with error logging
> + * @dev:	Device to request interrupt for
> + * @irq:	Interrupt line to allocate
> + * @handler:	Function to be called when the IRQ occurs
> + * @thread_fn:	Function to be called in a threaded interrupt context. NULL
> + *		for devices which handle everything in @handler
> + * @irqflags:	Interrupt type flags
> + * @devname:	An ascii name for the claiming device, dev_name(dev) if NULL
> + * @dev_id:	A cookie passed back to the handler function
>    *
> - *	Except for the extra @dev argument, this function takes the
> - *	same arguments and performs the same function as
> - *	request_any_context_irq().  IRQs requested with this function will be
> - *	automatically freed on driver detach.
> + * Except for the extra @dev argument, this function takes the same arguments
> + * and performs the same function as request_threaded_irq().  IRQs requested
> + * with this function will be automatically freed on driver detach.
> + *
> + * If an IRQ allocated with this function needs to be freed separately,
> + * devm_free_irq() must be used.
> + *
> + * When the request fails, an error message is printed with contextual
> + * information (device name, interrupt number, handler functions and
> + * error code). Don't add extra error messages at the call sites.
>    *
> - *	If an IRQ allocated with this function needs to be freed
> - *	separately, devm_free_irq() must be used.
> + * Return: 0 on success or a negative error number.
>    */
> -int devm_request_any_context_irq(struct device *dev, unsigned int irq,
> -			      irq_handler_t handler, unsigned long irqflags,
> -			      const char *devname, void *dev_id)
> +int devm_request_threaded_irq(struct device *dev, unsigned int irq,
> +			      irq_handler_t handler, irq_handler_t thread_fn,
> +			      unsigned long irqflags, const char *devname,
> +			      void *dev_id)
> +{
> +	int rc = __devm_request_threaded_irq(dev, irq, handler, thread_fn,
> +					     irqflags, devname, dev_id);
> +	if (!rc)
> +		return 0;
> +
> +	return dev_err_probe(dev, rc, "request_irq(%u) %ps %ps %s\n",
> +			     irq, handler, thread_fn, devname ? : "");
> +}
> +EXPORT_SYMBOL(devm_request_threaded_irq);
> +
> +static int __devm_request_any_context_irq(struct device *dev, unsigned int irq,
> +					  irq_handler_t handler,
> +					  unsigned long irqflags,
> +					  const char *devname, void *dev_id)
>   {
>   	struct irq_devres *dr;
>   	int rc;
> @@ -124,6 +128,43 @@ int devm_request_any_context_irq(struct device *dev, unsigned int irq,
>   
>   	return rc;
>   }
> +
> +/**
> + * devm_request_any_context_irq - allocate an interrupt line for a managed device with error logging
> + * @dev:	Device to request interrupt for
> + * @irq:	Interrupt line to allocate
> + * @handler:	Function to be called when the IRQ occurs
> + * @irqflags:	Interrupt type flags
> + * @devname:	An ascii name for the claiming device, dev_name(dev) if NULL
> + * @dev_id:	A cookie passed back to the handler function
> + *
> + * Except for the extra @dev argument, this function takes the same arguments
> + * and performs the same function as request_any_context_irq().  IRQs requested
> + * with this function will be automatically freed on driver detach.
> + *
> + * If an IRQ allocated with this function needs to be freed separately,
> + * devm_free_irq() must be used.
> + *
> + * When the request fails, an error message is printed with contextual
> + * information (device name, interrupt number, handler functions and
> + * error code). Don't add extra error messages at the call sites.
> + *
> + * Return: IRQC_IS_HARDIRQ or IRQC_IS_NESTED on success, or a negative error
> + * number.
> + */
> +int devm_request_any_context_irq(struct device *dev, unsigned int irq,
> +				 irq_handler_t handler, unsigned long irqflags,
> +				 const char *devname, void *dev_id)
> +{
> +	int rc = __devm_request_any_context_irq(dev, irq, handler, irqflags,
> +						devname, dev_id);
> +	if (rc < 0) {
> +		return dev_err_probe(dev, rc, "request_irq(%u) %ps %s\n",
> +				     irq, handler, devname ? : "");
> +	}

Extra { } should be removed.

 From my PoV, it would look more logical to have the same logic in 
devm_request_threaded_irq() and in devm_request_any_context_irq().

Why "if (!rc) SUCCESS" in one case, and "if (rc < 0) FAILURE" in the 
other case?

Personally, I would change in devm_request_threaded_irq() above to have
	if (rc)
		return dev_err_probe();
	return 0;

> +
> +	return rc;
> +}
>   EXPORT_SYMBOL(devm_request_any_context_irq);

On version 5 of the patch, there was a comment related to using 
EXPORT_SYMBOL_GPL(), does it still make sense?
(no strong opinion from me, just noted that and wondered if done on purpose)

CJ

>   
>   /**

Re: [PATCH v9 1/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[Thomas Gleixner]

On Wed, Jul 30 2025 at 18:48, Christophe JAILLET wrote:
> Le 30/07/2025 à 08:25, Pan Chuang a écrit :
>> +int devm_request_any_context_irq(struct device *dev, unsigned int irq,
>> +				 irq_handler_t handler, unsigned long irqflags,
>> +				 const char *devname, void *dev_id)
>> +{
>> +	int rc = __devm_request_any_context_irq(dev, irq, handler, irqflags,
>> +						devname, dev_id);
>> +	if (rc < 0) {
>> +		return dev_err_probe(dev, rc, "request_irq(%u) %ps %s\n",
>> +				     irq, handler, devname ? : "");
>> +	}
>
> Extra { } should be removed.

No. Even when C does not require it, brackets should only be omitted
when there is truly a single line after the condition. That's just
simpler to read because w/o brackets the brain pattern recognition
mechanism expects _one_ line not two or more and if there are more the
reading flow is interrupted as you have to parse it. With the brackets
it's obvious that there are more lines to read than one.

>  From my PoV, it would look more logical to have the same logic in 
> devm_request_threaded_irq() and in devm_request_any_context_irq().

As they print the same thing the right thing to do is:

        int rc = __devm_request_any_context_irq(....);

        return devm_request_result(dev, rc, irq, handler, NULL, devname);

and in devm_request_threaded_irq() invoke it with:

        return devm_request_result(dev, rc, irq, handler, thread_fn, devname);

and let that function return rc if (rc >= 0), which handles both cases.

>>   EXPORT_SYMBOL(devm_request_any_context_irq);
>
> On version 5 of the patch, there was a comment related to using 
> EXPORT_SYMBOL_GPL(), does it still make sense?
> (no strong opinion from me, just noted that and wondered if done on purpose)

That's an existing export. If at all that needs to be a seperate patch.

Thanks,

        tglx

Re: [PATCH v9 1/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[PanChuang]

Hi, Christophe

在 2025/7/31 0:48, Christophe JAILLET 写道:
> Le 30/07/2025 à 08:25, Pan Chuang a écrit :
>> + * Return: IRQC_IS_HARDIRQ or IRQC_IS_NESTED on success, or a 
>> negative error
>> + * number.
>> + */
>> +int devm_request_any_context_irq(struct device *dev, unsigned int irq,
>> +                 irq_handler_t handler, unsigned long irqflags,
>> +                 const char *devname, void *dev_id)
>> +{
>> +    int rc = __devm_request_any_context_irq(dev, irq, handler, 
>> irqflags,
>> +                        devname, dev_id);
>> +    if (rc < 0) {
>> +        return dev_err_probe(dev, rc, "request_irq(%u) %ps %s\n",
>> +                     irq, handler, devname ? : "");
>> +    }
>
> Extra { } should be removed.
>
> From my PoV, it would look more logical to have the same logic in 
> devm_request_threaded_irq() and in devm_request_any_context_irq().
>
> Why "if (!rc) SUCCESS" in one case, and "if (rc < 0) FAILURE" in the 
> other case?
>
> Personally, I would change in devm_request_threaded_irq() above to have
>     if (rc)
>         return dev_err_probe();
>     return 0;
>
Thanks for your suggestion.

The return value of __devm_request_any_context_irq on success is either 
IRQC_IS_HARDIRQ or IRQC_IS_NESTED (>= 0).

Therefore, we cannot directly use "if (rc)" to check for errors.

>> +
>> +    return rc;
>> +}
>>   EXPORT_SYMBOL(devm_request_any_context_irq);
>
> On version 5 of the patch, there was a comment related to using 
> EXPORT_SYMBOL_GPL(), does it still make sense?
> (no strong opinion from me, just noted that and wondered if done on 
> purpose)
>
Since it is an existing export, I will keep it as-is.

Thanks,

     Pan Chuang

Re: [PATCH v9 1/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[PanChuang]

Hi, tglx

在 2025/7/31 1:27, Thomas Gleixner 写道:
>>   From my PoV, it would look more logical to have the same logic in
>> devm_request_threaded_irq() and in devm_request_any_context_irq().
> As they print the same thing the right thing to do is:
>
>          int rc = __devm_request_any_context_irq(....);
>
>          return devm_request_result(dev, rc, irq, handler, NULL, devname);
>
> and in devm_request_threaded_irq() invoke it with:
>
>          return devm_request_result(dev, rc, irq, handler, thread_fn, devname);
>
> and let that function return rc if (rc >= 0), which handles both cases.

Could you please confirm if this implementation aligns with your vision?

I'm happy to refine it further based on your guidance.

 >        int rc = __devm_request_any_context_irq(dev, irq, handler, 
irqflags,
 >                                                devname, dev_id);
 >-       if (rc < 0) {
 >-               return dev_err_probe(dev, rc, "request_irq(%u) %ps %s\n",
 >-                                    irq, handler, devname ? : "");
 >-       }
 >+       if (rc >= 0)
 >+               return rc;
 >
 >-       return rc;
 >+       return dev_err_probe(dev, rc, "request_irq(%u) %ps %s\n",
 >+                            irq, handler, devname ? : "");
  >}


Thanks,

     Pan Chuang

Re: [PATCH v9 1/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context_irq()

[PanChuang]

Hi, tglx

在 2025/7/31 1:27, Thomas Gleixner 写道:
> As they print the same thing the right thing to do is:
>
>          int rc = __devm_request_any_context_irq(....);
>
>          return devm_request_result(dev, rc, irq, handler, NULL, devname);
>
> and in devm_request_threaded_irq() invoke it with:
>
>          return devm_request_result(dev, rc, irq, handler, thread_fn, devname);
>
> and let that function return rc if (rc >= 0), which handles both cases.

Or do you mean that we should add a new API devm_request_result() as 
follows:

static int devm_request_result(struct device *dev, int rc, unsigned int 
irq, irq_handler_t handler, irq_handler_t thread_fn, const char 
*devname) { if (rc >= 0) return rc; return dev_err_probe(dev, rc, 
"request_irq(%u) %ps %ps %s\n", irq, handler, thread_fn, devname ? : ""); }

And this function is called by devm_request_any_context_irq() and 
devm_request_thread_irq().

I'm concerned that when we call devm_request_any_context_irq(), it 
always logs a NULL parameter (thread_fn)."

I would appreciate your guidance on this issue.

Thanks,

Pan Chuang