linux-kernel - Re: [PATCH v3 1/2] counter: microchip-tcb-capture: Add IRQ handling

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: <3f424600-33d3-403b-9df6-85f7cb97c9a6@prolan.hu>
Date: Mon, 10 Feb 2025 10:49:07 +0100
From: Csókás Bence <csokas.bence@...lan.hu>
To: <Dharma.B@...rochip.com>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: <kamel.bouhara@...tlin.com>, <wbg@...nel.org>
Subject: Re: [PATCH v3 1/2] counter: microchip-tcb-capture: Add IRQ handling

Hi,

On 2025. 02. 07. 9:12, Dharma.B@...rochip.com wrote:
> can we have macros for the channel (3rd argument) for better clarity?

I myself have also thought about that, even about adding an uapi header. 
Thoughts?

>> +static int mchp_tc_irq_enable(struct counter_device *const counter)
> 
> Can we have it as mchp_tc_irq_init ?

Why, what's wrong with the current name? It requests IRQ servicing from 
Linux, then writes the peripheral's Interrupt Enable Register.

>> +{
>> +	struct mchp_tc_data *const priv = counter_priv(counter);
>> +	int ret = devm_request_irq(counter->parent, priv->irq, mchp_tc_isr, 0,
>> +				   dev_name(counter->parent), counter);
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = regmap_write(priv->regmap, ATMEL_TC_REG(priv->channel[0], IER), ATMEL_TC_DEF_IRQS);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = devm_add_action_or_reset(counter->parent, mchp_tc_irq_remove, priv);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>>    static void mchp_tc_clk_remove(void *ptr)
>>    {
>>    	clk_disable_unprepare((struct clk *)ptr);
>> @@ -378,6 +438,13 @@ static int mchp_tc_probe(struct platform_device *pdev)
>>    	counter->num_signals = ARRAY_SIZE(mchp_tc_count_signals);
>>    	counter->signals = mchp_tc_count_signals;
>>    
>> +	priv->irq = of_irq_get(np->parent, 0);
>> +	if (priv->irq > 0) {
>> +		ret = mchp_tc_irq_enable(counter);
> 
> missing error handling in irq retrieval (check for -EPROBE_DEFER).

Hmm, what should happen on `priv->irq == -EPROBE_DEFER`? `return 
-EPROBE_DEFER`?

Bence