Re: [PATCH v5 01/22] genirq/devres: Add devm_request_threaded_irq_probe() and devm_request_irq_probe()

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, Jul 21 2023 at 17:46, Yangtao Li wrote:
> +/**
> + *	devm_request_threaded_irq_probe - allocate an interrupt line for a managed device(recommended)

What does device(recommended) mean? This short description should
explain comprehensivly what this function does. You chose the name
_probe() for a reason, no?

> + *	@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
> + *	@info: Optional additional error log

Please make the argument doc tabular.

	@dev:		Device to request interrupt for
	@irq: 		Interrupt line to allocate
        ....
	@irqflags:	Interrupt type flags
        ...

> + *
> + *	This is a variant of the devm_request_threaded_irq function.
> + *	It will print an error message by default when the request fails,
> + *	and the consumer can add a special error msg.
> + *
> + *	Except for the extra @info argument, this function takes the
> + *	same arguments and performs the same function as
> + *	devm_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.

Lacks documentation of the return value.

> + */
> +int devm_request_threaded_irq_probe(struct device *dev, unsigned int irq,
> +				    irq_handler_t handler, irq_handler_t thread_fn,
> +				    unsigned long irqflags, const char *devname,
> +				    void *dev_id, const char *info)
> +{
> +	int rc;
> +
> +	rc = devm_request_threaded_irq(dev, irq, handler, NULL, irqflags, devname, dev_id);
> +	if (rc)
> +		return dev_err_probe(dev, rc, "Failed to request %sinterrupt %u %s %s\n",
> +				     thread_fn ? "threaded " : "", irq, devname ? : dev_name(dev),
> +				     info ? : "");

        if (rc) {
        	return ....;
        }

See https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#bracket-rules

> +	return 0;
> +}
> +EXPORT_SYMBOL(devm_request_threaded_irq_probe);

EXPORT_SYMBOL_GPL();

> +
>  /**
>   *	devm_request_any_context_irq - allocate an interrupt line for a managed device
>   *	@dev: device to request interrupt for

Thanks,

        tglx