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

[Jonathan Cameron]

On Thu, 21 May 2026 16:16:17 +0530
Jishnu Prakash <jishnu.prakash@xxxxxxxxxxxxxxxx> wrote:

> 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@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> >> ---  
> > 
> > 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.
That's an interesting corner case.  Maybe the warning needs to be more refined?
(I don't think it checks for shared?)

> 
> 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 ?

That's fine by me.  Just add some comments on why.

J
> 
> 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 */
> >>  
> >   
>