Message-ID: <vzoyeyhzrmvkhjeif6yuyxjc4moq6yzc5zuz7izeipz27f6cd4@csaqkjur3r3r>
Date: Sat, 6 Dec 2025 04:22:26 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
Cc: jic23@...nel.org, robh@...nel.org, krzysztof.kozlowski@...aro.org,
        krzk+dt@...nel.org, conor+dt@...nel.org, agross@...nel.org,
        andersson@...nel.org, lumag@...nel.org, konradybcio@...nel.org,
        daniel.lezcano@...aro.org, sboyd@...nel.org, amitk@...nel.org,
        thara.gopinath@...il.com, lee@...nel.org, rafael@...nel.org,
        subbaraman.narayanamurthy@....qualcomm.com,
        david.collins@....qualcomm.com, anjelique.melendez@....qualcomm.com,
        kamal.wadhwa@....qualcomm.com, rui.zhang@...el.com,
        lukasz.luba@....com, devicetree@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        cros-qcom-dts-watchers@...omium.org, quic_kotarake@...cinc.com,
        neil.armstrong@...aro.org, stephan.gerhold@...aro.org
Subject: Re: [PATCH V8 3/4] iio: adc: Add support for QCOM PMIC5 Gen3 ADC

On Thu, Nov 27, 2025 at 07:10:35PM +0530, Jishnu Prakash wrote:
> The ADC architecture on PMIC5 Gen3 is similar to that on PMIC5 Gen2,
> with all SW communication to ADC going through PMK8550 which
> communicates with other PMICs through PBS.
> 
> One major difference is that the register interface used here is that
> of an SDAM (Shared Direct Access Memory) peripheral present on PMK8550.
> There may be more than one SDAM used for ADC5 Gen3 and each has eight
> channels, which may be used for either immediate reads (same functionality
> as previous PMIC5 and PMIC5 Gen2 ADC peripherals) or recurring measurements
> (same as ADC_TM functionality).
> 
> By convention, we reserve the first channel of the first SDAM for all
> immediate reads and use the remaining channels across all SDAMs for
> ADC_TM monitoring functionality.
> 
> Add support for PMIC5 Gen3 ADC driver for immediate read functionality.
> ADC_TM is implemented as an auxiliary thermal driver under this ADC
> driver.
> 
> Signed-off-by: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
> ---
> Changes since v7:
> - Addressed following comments from Jonathan:
>   - Included regmap header file in drivers/iio/adc/qcom-adc5-gen3-common.c.
>   - Increased comment wrap length in adc5_gen3_configure() and 
>     struct adc5_chip definition.
>   - Updated error checks in adc5_gen3_isr() to remove NULL check for
>     adrv_tm and keep (!adrv_tm->tm_event_notify) error check alone
> 	within if() condition.
>   - Removed sid initialization in adc5_gen3_get_fw_channel_data()
> - Added definitions for ADC channel macros used in adc5_gen3_chans_pmic[]
>   in include/linux/iio/adc/qcom-adc5-gen3-common.h instead of 
>   include/dt-bindings/iio/adc/qcom,spmi-vadc.h, as this latter file
>   will be moved out of bindings folder in a separate change. Also
>   removed its inclusion in drivers/iio/adc/qcom-spmi-adc5-gen3.c.
> - Cleaned up local variable declarations in adc5_gen3_isr() and
>   adc5_gen3_get_fw_channel_data() and added local variable for
>   adc->dev in adc5_get_fw_data().
> - Fixed error message after platform_get_irq() call in adc5_gen3_probe()
>   to print IRQ number correctly.
> - Added a check in adc5_gen3_get_fw_channel_data() to exit with error
>   if ADC channel value obtained from `reg` channel property is not
>   among the supported ones in the array adc5_gen3_chans_pmic[].
> - Corrected the value used in checking for max valid ADC channel value,
>   in adc5_gen3_get_fw_channel_data().
> 
> Changes since v6:
> - Addressed following comments from Jonathan:
>   - Moved functions exported in drivers/iio/adc/qcom-adc5-gen3-common.c
>     into namespace "QCOM_SPMI_ADC5_GEN3".
>   - Increased line wrap length for comments.
>   - Added local variable for adc->dev in adc5_gen3_isr().
>   - Shifted debug print showing IRQ status registers in adc5_gen3_isr()
>     to before tm_status[] check.
>   - Fixed indentation and brackets in adc5_gen3_get_fw_channel_data().
>   - Cleaned up array formatting in adc5_gen3_data_pmic struct.
>   - Used scoped variant of device_for_each_child_node() in adc5_get_fw_data().
>   - Updated auxiliary device cleanup handling to fix memory freeing
>     issues, by adding empty auxiliary device release function.
>   - Used devm_mutex_init() in adc5_gen3_probe().
>   - Updated virtual channel macro name from V_CHAN to ADC5_GEN3_V_CHAN.
>   - Set IIO device name to "spmi-adc5-gen3".
> - Added __acquires and __releases macros for exported mutex lock
>   and unlock functions in drivers/iio/adc/qcom-spmi-adc5-gen3.c.
> - Added error check to fail probe in case adding auxiliary TM device fails.
> - Replaced 2025 copyright in newly added files with yearless copyright,
>   following new internal guidelines.
> 
> Changes since v5:
> - Addressed following comments from Jonathan:
>   - Corrected line wrap length in Kconfig and driver files.
>   - Replaced usleep_range() with fsleep() in adc5_gen3_poll_wait_hs()
>   - Corrected all files to follow kernel-doc formatting fully.
>   - Removed IIO_CHAN_INFO_RAW case in adc5_gen3_read_raw()
>   - Cleaned up formatting in adc5_gen3_data_pmic struct and in other
>     struct definitions.
>   - Updated adc5_gen3_add_aux_tm_device() to keep errors alone out of line.
>   - Split mutex function exported to ADC_TM driver into separate functions
>     for acquiring and releasing mutex.
>   - Removed num_sdams member from struct adc5_chip.
>   - Fixed dev_err_probe() print in adc5_gen3_probe().
>   - Updated logic for acquiring IRQ numbers to account for removing
>     "interrupt-names" DT property.
> - Included bitfield.h header file in drivers/iio/adc/qcom-adc5-gen3-common.c
>   to fix kernel bot error.
> 
> Changes since v4:
> - Moved out common funtions from newly added .h file into a separate .c
>   file to avoid duplicating them and updated interrupt name, as suggested
>   by Krzysztof. Updated namespace export symbol statement to have a string
>   as second argument to follow framework change.
> 
> Changes since v3:
> - Split out TM functionality into auxiliary driver in separate patch and
>   added required changes in main driver, as suggested by Dmitry.
> - Addressed other reviewer comments in main driver patch. 
> 
> Changes since v1:
> - Removed datashet_name usage and implemented read_label() function
> - In probe, updated channel property in iio_chan_spec from individual
>   channel to virtual channel and set indexed property to 1, due to the
>   above change.
> - Updated order of checks in ISR
> - Removed the driver remove callback and replaced with callbacks in a
>   devm_add_action call in probe.
> - Addressed other comments from reviewers.
> 
>  drivers/iio/adc/Kconfig                       |  30 +
>  drivers/iio/adc/Makefile                      |   2 +
>  drivers/iio/adc/qcom-adc5-gen3-common.c       | 107 +++
>  drivers/iio/adc/qcom-spmi-adc5-gen3.c         | 767 ++++++++++++++++++
>  include/linux/iio/adc/qcom-adc5-gen3-common.h | 216 +++++
>  5 files changed, 1122 insertions(+)
>  create mode 100644 drivers/iio/adc/qcom-adc5-gen3-common.c
>  create mode 100644 drivers/iio/adc/qcom-spmi-adc5-gen3.c
>  create mode 100644 include/linux/iio/adc/qcom-adc5-gen3-common.h
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 58a14e6833f6..da201a9a6950 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1319,6 +1319,36 @@ config QCOM_SPMI_ADC5
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called qcom-spmi-adc5.
>  
> +config QCOM_ADC5_GEN3_COMMON
> +	tristate

This Kconfig (and the module) are used only by QCOM_SPMI_ADC5_GEN3. Why
do you need to separate them? Your thermal module doesn't depend on the
common functions.

> +
> +config QCOM_SPMI_ADC5_GEN3
> +	tristate "Qualcomm Technologies Inc. SPMI PMIC5 GEN3 ADC"
> +	depends on SPMI && THERMAL
> +	select REGMAP_SPMI
> +	select QCOM_VADC_COMMON
> +	select QCOM_ADC5_GEN3_COMMON
> +	select AUXILIARY_BUS
> +	help
> 

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists