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

["Csókás Bence" <csokas.bence@prolan.hu>]

Hi,

On 2025. 02. 07. 9:12, Dharma.B@microchip.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