Re: [PATCH] thermal: qcom-spmi-adc-tm5-gen3: move hardirq actions into threaded handler

[Andy Shevchenko]

On Thu, Nov 12, 2026 at 01:54:13PM +0530, Jishnu Prakash wrote:
> On 11/11/2026 7:53 PM, Andy Shevchenko wrote:
> > On Wed, Nov 11, 2026 at 02:12:01PM +0530, Jishnu Prakash wrote:

...

> >> -static irqreturn_t adctm5_gen3_isr(int irq, void *dev_id)
> >> +static irqreturn_t adctm5_gen3_isr_thread(int irq, void *dev_id)
> >>  {
> >>  	struct adc_tm5_gen3_chip *adc_tm5 = dev_id;
> >> -	int ret, sdam_num;
> >> +	int sdam_index, ret;
> > 
> > Was the variable rename so important for the fix? Can we defer it to the later
> > cleanups, if ever needed?
> > 
> >>  	u8 tm_status[2];
> >>  	u8 status, val;

...

> >> -	struct adc_tm5_gen3_chip *adc_tm5 = dev_id;
> >> -	u8 tm_status[2];
> >> -	int sdam_index;
> > 
> > Okay, I see the reason now why it's done, but I noticed no mention of this in
> > the commit message. Also it's unclear which variable is used more often and
> > what is the real semantics of its value. Is it number or index?

> > With all this
> > in mind, please at bare minimum elaborate in the commit message and, if it's
> > actually a number, replace the other part instead.

^^^

> In both adctm5_gen3_isr() and adctm5_gen3_isr_thread(), we needed a variable to
> store the index of the SDAM whose IRQ was triggered. We called it 'sdam_num'
> in adctm5_gen3_isr() and 'sdam_index' in adctm5_gen3_isr_thread(). In the rest
> of the file, it is called 'sdam_index', so ideally that same name should have
> been used in adctm5_gen3_isr() too, as it is more accurate than 'sdam_num'.
> 
> As I mentioned at the end of the commit message, I just meant to drop adctm5_gen3_isr()
> and expand adctm5_gen3_isr_thread() to include the actions formerly done in
> adctm5_gen3_isr(), but I see the diff makes it look more like I modified adctm5_gen3_isr()
> into adctm5_gen3_isr_thread() with a variable rename, which is a bit misleading.
> 
> I can add a line in the commit message to emphasize that 'sdam_num' is the same as
> 'sdam_index' if you think it helps.

Yes, see above.

-- 
With Best Regards,
Andy Shevchenko