linux-kernel - Re: [PATCH v6 2/3] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <9fcf9cf4-ba02-4583-9e14-bd650dbe32ee@prolan.hu>
Date: Tue, 4 Mar 2025 10:57:05 +0100
From: Csókás Bence <csokas.bence@...lan.hu>
To: William Breathitt Gray <wbg@...nel.org>
CC: <linux-arm-kernel@...ts.infradead.org>, <linux-iio@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Kamel Bouhara <kamel.bouhara@...tlin.com>
Subject: Re: [PATCH v6 2/3] counter: microchip-tcb-capture: Add IRQ handling

Hi,

On 2025. 03. 04. 8:02, William Breathitt Gray wrote:
> In theory, the error code could be something else if of_irq_get() failed
> for any other reason. Handle all those error cases at once by checking
> IS_ERR(priv->irq) rather than just -EPROBE_DEFER. Then you can just
> return dev_err_probe() with priv->irq for the error code.

Yes, `of_irq_get()` can return an error, for example if the IRQ is not 
defined in the DT. In these cases, we just don't do IRQ, but still allow 
the device to probe. -EPROBE_DEFER is special in this case, because it 
signifies that there *is* an IRQ to set up, just not now.

>> +enum counter_mchp_event_channels {
>> +	COUNTER_MCHP_EVCHN_CV = 0,
>> +	COUNTER_MCHP_EVCHN_RA = 0,
>> +	COUNTER_MCHP_EVCHN_RB,
>> +	COUNTER_MCHP_EVCHN_RC,
>> +};
> 
> These would be better as preprocessor defines in case we need to
> introduce new events to channel 1 or 2 in the future. That would allow
> us to insert new events easily to existing channels without having to
> worry about its actual position in an enum list.

Okay. I personally like the enum interface more, as it is much cleaner, 
but if there's a good reason to use #define's, then so be it.

> One additional benefit is if we do end up introducing more Counts for
> the module. In that situation we would have multiple CV and RA/RB/RC per
> Counter device, but we can easily define a preprocessor macro to
> calculate the channel offset given the Count index. However, with enum
> structure we would have to manually add and maintain redundant defines
> for each Count, which is far less ideal.
> 
> William Breathitt Gray

Bence