From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C43A13CDBD8; Fri, 22 May 2026 10:47:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779446856; cv=none; b=KpdiCFJH5W+41+A1NE5yO68YioUmAWIRGW8A87uECjHEOVDB8HbaFdG0quhSo1YoPIn9MFKU0lgcn1UTDgTn09DWNh0OScKId1kGHkudI1Fi8Vs9M/qJrLWUA7x8+9dHyQZrZI4SbV+12hFI1CuQuqiZb339mCKWalS0IRWVrC8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779446856; c=relaxed/simple; bh=QNdletnnGNblfetjlRW6ZCm9/bq4IaOYIpJ8Y2ip6so=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=g6VdhHle3D/sbh9M+LLq0ILBL+hGlO9xdB6EaR7yNpIT73JRPxMxT9VPVy0xYnSK0fpbBpHonxlc284vkt41nHrMwPaj/udsjs3srF/Ghtah5Nw9LUayeHH5DxNspG7Smb1UP+mxVl2KR5HrIy6ObHJbSEAPKlr02SlfIu5nfto= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=NqoOW6Ru; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="NqoOW6Ru" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A1BE51F000E9; Fri, 22 May 2026 10:47:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1779446854; bh=K7QkYbeKzf0diPzwFSy35esfS3DNOr7gU6+VWzSEYXw=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=NqoOW6Ru5ewCnOZl4JPJNgROCMEbS2VYkGBvVV05HDjXUsmqvcZ/btGQX2aIUXrZ6 8QvFlJ315J+uyGU+OY/HXdSDD6BJuriRtn0dMGKo2uVGcnnnaUJo8fs1OjHKP53H4M EvzTHTxJGlL4TywdE8zamWHjSZS8anCKmxL0wVDNTVz/YhoNaMG9+Fr7MEC7C2tlgJ SUWW/WdDiTNa1GIBkEwmxo62oWLLcoOtPIlSs7cVo2RVrUWpGFRDOpwsb9GfFhwGPg j6lLTBi7b/KDSfXbKVacEfpPmNc7ECF8OBYo2FuJbZXHkykbB/S7pAxQOv4YdRYD1r 35AZXSnBLBsZA== Date: Fri, 22 May 2026 11:47:22 +0100 From: Jonathan Cameron To: Jishnu Prakash Cc: David Lechner , Nuno =?UTF-8?B?U8Oh?= , Andy Shevchenko , Amit Kucheria , Thara Gopinath , "Rafael J. Wysocki" , Daniel Lezcano , Zhang Rui , Lukasz Luba , linux-arm-msm@vger.kernel.org, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Kamal Wadhwa , David Collins , Anjelique Melendez , Neil Armstrong , Stephan Gerhold Subject: Re: [PATCH 1/2] iio: adc: qcom-spmi-adc5-gen3: Share SDAM0 IRQ with ADC_TM auxiliary driver Message-ID: <20260522114722.49b6055e@jic23-huawei> In-Reply-To: <6c876a92-e572-4cb9-8c31-bbf681a4770b@oss.qualcomm.com> References: <20260515-gen3_adc_tm-v1-0-39ba29f9b4ab@oss.qualcomm.com> <20260515-gen3_adc_tm-v1-1-39ba29f9b4ab@oss.qualcomm.com> <20260515145450.3b1e9c3d@jic23-huawei> <6c876a92-e572-4cb9-8c31-bbf681a4770b@oss.qualcomm.com> X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 21 May 2026 16:16:17 +0530 Jishnu Prakash wrote: > Hi Jonathan, > > On 5/15/2026 7:24 PM, Jonathan Cameron wrote: > > On Fri, 15 May 2026 14:23:44 +0530 > > Jishnu Prakash 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 > >> --- > > > > 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 */ > >> > > >