linux-kernel - Re: [PATCH v5 01/22] genirq/devres: Add devm_request_threaded_irq_probe() and devm_request_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87r0a7a2d0.ffs@tglx>
Date: Thu, 29 Aug 2024 15:24:27 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Yangtao Li <frank.li@...o.com>
Cc: Yangtao Li <frank.li@...o.com>, Krzysztof Kozlowski <krzk@...nel.org>,
 Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
 Jonathan Cameron
 <Jonathan.Cameron@...wei.com>, AngeloGioacchino Del Regno
 <angelogioacchino.delregno@...labora.com>, Geert Uytterhoeven
 <geert+renesas@...der.be>, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 01/22] genirq/devres: Add
 devm_request_threaded_irq_probe() and devm_request_irq_probe()

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