Re: [PATCH 1/2] iio: adc: qcom-spmi-adc5-gen3: Share SDAM0 IRQ with ADC_TM auxiliary driver

[Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>]

Hi Jonathan,

On 5/15/2026 7:24 PM, Jonathan Cameron wrote:
> On Fri, 15 May 2026 14:23:44 +0530
> Jishnu Prakash <jishnu.prakash@oss.qualcomm.com> wrote:
> 
>> The SDAM0 IRQ can be triggered for both EOC (end of conversion) events for
>> immediate ADC reads done in this driver and for threshold violation events,
>> based on ADC_TM thresholds configured from the auxiliary ADC_TM driver on
>> TM channels on the first SDAM.
>>
>> At present, this interrupt is handled only in the ISR in the main ADC driver.
>> When the ISR is triggered for an ADC_TM event, this driver notifies the ADC_TM
>> driver by calling a notifier callback exposed from it for this purpose.
>>
>> To simplify the interrupt handling in both drivers, share the interrupt between
>> the drivers. With this, ADC_TM interrupts on SDAM0 will be handled directly in
>> the ADC_TM driver, so remove the notifier callback and all TM interrupt
>> handling in the main ADC ISR.
>>
>> Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
>> ---
> 
> Some stuff from Sashiko on this one:
> https://sashiko.dev/#/patchset/20260515-gen3_adc_tm-v1-0-39ba29f9b4ab%40oss.qualcomm.com
> 
> Given I assume you didn't see the warning (I'm fairly sure the bots analysis is correct
> as we've been busy fixing similar cases all cycle), can I just check, have you tested
> this on latest upstream?

I had tested on a build based on top of Linux 7.1-rc2 and verified the driver's basic
functionality, but I think I overlooked the warning from the interrupt management code,
sorry about the miss.

> 
> Thanks,
> 
> Jonathan
> 
> 
>>  drivers/iio/adc/qcom-spmi-adc5-gen3.c         | 52 +++++----------------------
>>  include/linux/iio/adc/qcom-adc5-gen3-common.h |  2 --
>>  2 files changed, 8 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/iio/adc/qcom-spmi-adc5-gen3.c b/drivers/iio/adc/qcom-spmi-adc5-gen3.c
>> index f8168a14b907..a819c3e627a0 100644
>> --- a/drivers/iio/adc/qcom-spmi-adc5-gen3.c
>> +++ b/drivers/iio/adc/qcom-spmi-adc5-gen3.c
> 
>>  static int adc5_gen3_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> @@ -818,7 +782,7 @@ static int adc5_gen3_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	ret = devm_request_irq(dev, adc->dev_data.base[ADC5_GEN3_VADC_SDAM].irq,
>> -			       adc5_gen3_isr, 0,
>> +			       adc5_gen3_isr, IRQF_ONESHOT | IRQF_SHARED,
> 
> Sashikio points out that IRQF_ONESHOT is never correct for a non threaded
> interrupt.  The point of that flag is to ensure we don't handle another interrupt
> until the thread is done. If there isn't a thread then it doesn't do anything
> (other than omit a warning!)

I tried at first keeping only the IRQF_SHARED flag here, but it seems that
shared interrupts need to agree on the ONESHOT flag configuration, else the
second interrupt's IRQ request call fails.

And the ADC_TM interrupt needs to be ONESHOT, since we don't want that interrupt to
be rearmed before we have notified the thermal framework from the threaded
part of the handler. So I had to add the IRQF_ONESHOT here too, though it is
not useful here.

I think it's best to use a threaded IRQ handler in this driver too. I don't really
see any meaningful way to split the actions in the interrupt handler here into a primary
handler and a threaded handler, so is it fine if I just make the primary handler NULL
and move all the ISR functionality into the threaded handler part ?

Thanks,
Jishnu


> 
>>  			       adc->dev_data.base[ADC5_GEN3_VADC_SDAM].irq_name,
>>  			       adc);
>>  	if (ret)
>> diff --git a/include/linux/iio/adc/qcom-adc5-gen3-common.h b/include/linux/iio/adc/qcom-adc5-gen3-common.h
>> index 6303eaa6640b..39cbfcbdb101 100644
>> --- a/include/linux/iio/adc/qcom-adc5-gen3-common.h
>> +++ b/include/linux/iio/adc/qcom-adc5-gen3-common.h
>> @@ -205,7 +205,5 @@ int adc5_gen3_get_scaled_reading(struct device *dev,
>>  int adc5_gen3_therm_code_to_temp(struct device *dev,
>>  				 struct adc5_channel_common_prop *common_props,
>>  				 u16 code, int *val);
>> -void adc5_gen3_register_tm_event_notifier(struct device *dev,
>> -					  void (*handler)(struct auxiliary_device *));
>>  
>>  #endif /* QCOM_ADC5_GEN3_COMMON_H */
>>
>