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

[Thomas Gleixner]

On Tue, Jul 29 2025 at 16:14, Pan Chuang wrote:

> The devm_request_threaded_irq() and devm_request_any_context_irq() functions

devm_request_threaded_irq() and devm_request_any_context_irq() ....

The '()' notation already makes it clear that these are functions, so no
'The ... functions' is redundant.

> 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 addresses, and error code on failure.

Again: %pS (or %ps) does NOT print the handler address. It prints the
symbol name. Feel free to ignore my review comments, but then accept
that I ignore your patches too.

> Signed-off-by: Yangtao Li <frank.li@xxxxxxxx>
> Signed-off-by: Pan Chuang <panchuang@xxxxxxxx>

This SOB chain is still incorrect. Again:

 https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

If anything is unclear, then please ask.

> +/**
> + * 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.
> + *
> + * On failure, it returns a negative value. On success, it returns either
> + * IRQC_IS_HARDIRQ or IRQC_IS_NESTED.

As you touch this, can you please convert this to the proper

   Returns:

formatting?

Thanks,

        tglx