[PATCH V3 0/4] thermal: qcom: Add support for PMIC5 Gen2 ADC_TM

[PATCH V3 0/4] thermal: qcom: Add support for PMIC5 Gen2 ADC_TM

[Jishnu Prakash]

Made following changes in this post:
Addressed comments given by Jonathan for qcom-spmi-adc-tm5.yaml.
Addressed comments given by Dmitry and Jonathan for qcom-spmi-adc-tm5.c.
Split patch for qcom-spmi-adc-tm5.c into two parts, one to refactor
code to support multiple device generations and the second to add
actual Gen2 ADC_TM changes.

Jishnu Prakash (4):
  dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings
  iio: adc: qcom-vadc-common: add reverse scaling for PMIC5 Gen2 ADC_TM
  thermal: qcom: Add support for multiple generations of devices
  thermal: qcom: add support for PMIC5 Gen2 ADCTM

 .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 110 ++++-
 drivers/iio/adc/qcom-vadc-common.c                 |  11 +
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c           | 451 +++++++++++++++++++--
 include/linux/iio/adc/qcom-vadc-common.h           |   2 +
 4 files changed, 541 insertions(+), 33 deletions(-)

-- 
2.7.4

[PATCH V3 1/4] dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings

[Jishnu Prakash]

Add documentation for PMIC5 Gen2 ADC_TM peripheral.
It is used for monitoring ADC channel thresholds for PMIC7-type
PMICs. It is present on PMK8350, like PMIC7 ADC and can be used
to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
on a target, through PBS(Programmable Boot Sequence).

Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
---
 .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 110 ++++++++++++++++++++-
 1 file changed, 108 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
index 3ea8c0c..feb390d 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
@@ -10,7 +10,9 @@ maintainers:
 
 properties:
   compatible:
-    const: qcom,spmi-adc-tm5
+    enum:
+      - qcom,spmi-adc-tm5
+      - qcom,spmi-adc-tm5-gen2
 
   reg:
     maxItems: 1
@@ -33,6 +35,7 @@ properties:
   qcom,avg-samples:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: Number of samples to be used for measurement.
+            Not applicable for Gen2 ADC_TM peripheral.
     enum:
       - 1
       - 2
@@ -45,6 +48,7 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: This parameter is used to decrease ADC sampling rate.
             Quicker measurements can be made by reducing decimation ratio.
+            Not applicable for Gen2 ADC_TM peripheral.
     enum:
       - 250
       - 420
@@ -93,6 +97,29 @@ patternProperties:
           - const: 1
           - enum: [ 1, 3, 4, 6, 20, 8, 10 ]
 
+      qcom,avg-samples:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Number of samples to be used for measurement.
+          This property in child node is applicable only for Gen2 ADC_TM peripheral.
+        enum:
+          - 1
+          - 2
+          - 4
+          - 8
+          - 16
+        default: 1
+
+      qcom,decimation:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: This parameter is used to decrease ADC sampling rate.
+          Quicker measurements can be made by reducing decimation ratio.
+          This property in child node is applicable only for Gen2 ADC_TM peripheral.
+        enum:
+          - 85
+          - 340
+          - 1360
+        default: 1360
+
     required:
       - reg
       - io-channels
@@ -100,6 +127,31 @@ patternProperties:
     additionalProperties:
       false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,spmi-adc-tm5
+
+    then:
+      patternProperties:
+        "^([-a-z0-9]*)@[0-7]$":
+          properties:
+            qcom,decimation: false
+            qcom,avg-samples: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,spmi-adc-tm5-gen2
+
+    then:
+      properties:
+        qcom,avg-samples: false
+        qcom,decimation: false
+
 required:
   - compatible
   - reg
@@ -124,7 +176,7 @@ examples:
             #size-cells = <0>;
             #io-channel-cells = <1>;
 
-            /* Other propreties are omitted */
+            /* Other properties are omitted */
             conn-therm@4f {
                 reg = <ADC5_AMUX_THM3_100K_PU>;
                 qcom,ratiometric;
@@ -148,4 +200,58 @@ examples:
             };
         };
     };
+
+  - |
+    #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
+    #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spmi_bus {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pmk8350_vadc: adc@3100 {
+            reg = <0x3100>;
+            compatible = "qcom,spmi-adc7";
+            #address-cells = <1>;
+            #size-cells = <0>;
+            #io-channel-cells = <1>;
+
+            /* Other properties are omitted */
+            xo-therm@44 {
+                reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
+                qcom,ratiometric;
+                qcom,hw-settle-time = <200>;
+            };
+
+            conn-therm@47 {
+                reg = <PM8350_ADC7_AMUX_THM4_100K_PU>;
+                qcom,ratiometric;
+                qcom,hw-settle-time = <200>;
+            };
+        };
+
+        pmk8350_adc_tm: adc-tm@3400 {
+            compatible = "qcom,spmi-adc-tm5-gen2";
+            reg = <0x3400>;
+            interrupts = <0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
+            #thermal-sensor-cells = <1>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            pmk8350-xo-therm@0 {
+                reg = <0>;
+                io-channels = <&pmk8350_vadc PMK8350_ADC7_AMUX_THM1_100K_PU>;
+                qcom,decimation = <340>;
+                qcom,ratiometric;
+                qcom,hw-settle-time-us = <200>;
+            };
+
+            conn-therm@1 {
+                reg = <1>;
+                io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM4_100K_PU>;
+                qcom,avg-samples = <2>;
+                qcom,ratiometric;
+                qcom,hw-settle-time-us = <200>;
+            };
+        };
+    };
 ...
-- 
2.7.4

[PATCH V3 2/4] iio: adc: qcom-vadc-common: add reverse scaling for PMIC5 Gen2 ADC_TM

[Jishnu Prakash]

Add reverse scaling function for PMIC5 Gen2 ADC_TM, to convert
temperature to raw ADC code, for setting thresholds for
thermistor channels.

Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/qcom-vadc-common.c       | 11 +++++++++++
 include/linux/iio/adc/qcom-vadc-common.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/iio/adc/qcom-vadc-common.c b/drivers/iio/adc/qcom-vadc-common.c
index 1472389..194d7c3 100644
--- a/drivers/iio/adc/qcom-vadc-common.c
+++ b/drivers/iio/adc/qcom-vadc-common.c
@@ -677,6 +677,17 @@ u16 qcom_adc_tm5_temp_volt_scale(unsigned int prescale_ratio,
 }
 EXPORT_SYMBOL(qcom_adc_tm5_temp_volt_scale);
 
+u16 qcom_adc_tm5_gen2_temp_res_scale(int temp)
+{
+	int64_t resistance;
+
+	resistance = qcom_vadc_map_temp_voltage(adcmap7_100k,
+		ARRAY_SIZE(adcmap7_100k), temp);
+
+	return div64_s64(resistance * RATIO_MAX_ADC7, resistance + R_PU_100K);
+}
+EXPORT_SYMBOL(qcom_adc_tm5_gen2_temp_res_scale);
+
 int qcom_adc5_hw_scale(enum vadc_scale_fn_type scaletype,
 		    unsigned int prescale_ratio,
 		    const struct adc5_data *data,
diff --git a/include/linux/iio/adc/qcom-vadc-common.h b/include/linux/iio/adc/qcom-vadc-common.h
index 33f60f4..598a5d2 100644
--- a/include/linux/iio/adc/qcom-vadc-common.h
+++ b/include/linux/iio/adc/qcom-vadc-common.h
@@ -161,6 +161,8 @@ int qcom_adc5_hw_scale(enum vadc_scale_fn_type scaletype,
 u16 qcom_adc_tm5_temp_volt_scale(unsigned int prescale_ratio,
 				 u32 full_scale_code_volt, int temp);
 
+u16 qcom_adc_tm5_gen2_temp_res_scale(int temp);
+
 int qcom_adc5_prescaling_from_dt(u32 num, u32 den);
 
 int qcom_adc5_hw_settle_time_from_dt(u32 value, const unsigned int *hw_settle);
-- 
2.7.4

[PATCH V3 3/4] thermal: qcom: Add support for multiple generations of devices

[Jishnu Prakash]

Refactor code to support multiple generations of ADC_TM devices
by defining gen number, irq name and disable, configure and isr
APIs in the individual data structs.

Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
---
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 76 ++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 28 deletions(-)

diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
index 824671c..fc8cd45 100644
--- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
+++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
@@ -78,11 +78,10 @@ enum adc5_timer_select {
 	ADC5_TIMER_SEL_NONE,
 };
 
-struct adc_tm5_data {
-	const u32	full_scale_code_volt;
-	unsigned int	*decimation;
-	unsigned int	*hw_settle;
-	bool		is_hc;
+enum adc5_gen {
+	ADC_TM5,
+	ADC_TM_HC,
+	ADC_TM5_MAX
 };
 
 enum adc_tm5_cal_method {
@@ -92,6 +91,18 @@ enum adc_tm5_cal_method {
 };
 
 struct adc_tm5_chip;
+struct adc_tm5_channel;
+
+struct adc_tm5_data {
+	const u32 full_scale_code_volt;
+	unsigned int *decimation;
+	unsigned int *hw_settle;
+	int (*disable_channel)(struct adc_tm5_channel *channel);
+	int (*configure)(struct adc_tm5_channel *channel, int low, int high);
+	irqreturn_t (*isr)(int irq, void *data);
+	char *irq_name;
+	int gen;
+};
 
 /**
  * struct adc_tm5_channel - ADC Thermal Monitoring channel data.
@@ -139,22 +150,6 @@ struct adc_tm5_chip {
 	u16			base;
 };
 
-static const struct adc_tm5_data adc_tm5_data_pmic = {
-	.full_scale_code_volt = 0x70e4,
-	.decimation = (unsigned int []) { 250, 420, 840 },
-	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
-					 1000, 2000, 4000, 8000, 16000, 32000,
-					 64000, 128000 },
-};
-
-static const struct adc_tm5_data adc_tm_hc_data_pmic = {
-	.full_scale_code_volt = 0x70e4,
-	.decimation = (unsigned int []) { 256, 512, 1024 },
-	.hw_settle = (unsigned int []) { 0, 100, 200, 300, 400, 500, 600, 700,
-					 1000, 2000, 4000, 6000, 8000, 10000 },
-	.is_hc = true,
-};
-
 static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
 {
 	return regmap_bulk_read(adc_tm->regmap, adc_tm->base + offset, data, len);
@@ -343,14 +338,14 @@ static int adc_tm5_set_trips(void *data, int low, int high)
 		channel->channel, low, high);
 
 	if (high == INT_MAX && low <= -INT_MAX)
-		ret = adc_tm5_disable_channel(channel);
+		ret = chip->data->disable_channel(channel);
 	else
-		ret = adc_tm5_configure(channel, low, high);
+		ret = chip->data->configure(channel, low, high);
 
 	return ret;
 }
 
-static struct thermal_zone_of_device_ops adc_tm5_ops = {
+static struct thermal_zone_of_device_ops adc_tm5_thermal_ops = {
 	.get_temp = adc_tm5_get_temp,
 	.set_trips = adc_tm5_set_trips,
 };
@@ -366,7 +361,7 @@ static int adc_tm5_register_tzd(struct adc_tm5_chip *adc_tm)
 		tzd = devm_thermal_zone_of_sensor_register(adc_tm->dev,
 							   adc_tm->channels[i].channel,
 							   &adc_tm->channels[i],
-							   &adc_tm5_ops);
+							   &adc_tm5_thermal_ops);
 		if (IS_ERR(tzd)) {
 			if (PTR_ERR(tzd) == -ENODEV) {
 				dev_warn(adc_tm->dev, "thermal sensor on channel %d is not used\n",
@@ -526,6 +521,31 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
 	return 0;
 }
 
+static const struct adc_tm5_data adc_tm5_data_pmic = {
+	.full_scale_code_volt = 0x70e4,
+	.decimation = (unsigned int []) { 250, 420, 840 },
+	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
+					 1000, 2000, 4000, 8000, 16000, 32000,
+					 64000, 128000 },
+	.disable_channel = adc_tm5_disable_channel,
+	.configure = adc_tm5_configure,
+	.isr = adc_tm5_isr,
+	.irq_name = "pm-adc-tm5",
+	.gen = ADC_TM5,
+};
+
+static const struct adc_tm5_data adc_tm_hc_data_pmic = {
+	.full_scale_code_volt = 0x70e4,
+	.decimation = (unsigned int []) { 256, 512, 1024 },
+	.hw_settle = (unsigned int []) { 0, 100, 200, 300, 400, 500, 600, 700,
+					 1000, 2000, 4000, 6000, 8000, 10000 },
+	.disable_channel = adc_tm5_disable_channel,
+	.configure = adc_tm5_configure,
+	.isr = adc_tm5_isr,
+	.irq_name = "pm-adc-tm5",
+	.gen = ADC_TM_HC,
+};
+
 static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node *node)
 {
 	struct adc_tm5_channel *channels;
@@ -623,7 +643,7 @@ static int adc_tm5_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (adc_tm->data->is_hc)
+	if (adc_tm->data->gen == ADC_TM_HC)
 		ret = adc_tm_hc_init(adc_tm);
 	else
 		ret = adc_tm5_init(adc_tm);
@@ -638,8 +658,8 @@ static int adc_tm5_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_isr,
-					 IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
+	return devm_request_threaded_irq(dev, irq, NULL, adc_tm->data->isr,
+			IRQF_ONESHOT, adc_tm->data->irq_name, adc_tm);
 }
 
 static const struct of_device_id adc_tm5_match_table[] = {
-- 
2.7.4

[PATCH V3 4/4] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Jishnu Prakash]

Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a
close counterpart of PMIC7 ADC and has the same functionality as
PMIC5 ADC_TM, for threshold monitoring and interrupt generation.
It is present on PMK8350 alone, like PMIC7 ADC and can be used
to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
having ADC on a target, through PBS(Programmable Boot Sequence).

Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
---
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 375 ++++++++++++++++++++++++++++++-
 1 file changed, 372 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
index fc8cd45..a7b33a8 100644
--- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
+++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
@@ -4,7 +4,10 @@
  *
  * Based on original driver:
  * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
+ *
+ * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved.
  */
+
 #include <linux/bitfield.h>
 #include <linux/iio/adc/qcom-vadc-common.h>
 #include <linux/iio/consumer.h>
@@ -71,6 +74,60 @@
 #define ADC_TM5_M_HIGH_THR_INT_EN			BIT(1)
 #define ADC_TM5_M_LOW_THR_INT_EN			BIT(0)
 
+#define ADC_TM_GEN2_STATUS1			0x08
+#define ADC_TM_GEN2_STATUS_LOW_SET		0x09
+#define ADC_TM_GEN2_STATUS_LOW_CLR		0x0a
+#define ADC_TM_GEN2_STATUS_HIGH_SET		0x0b
+#define ADC_TM_GEN2_STATUS_HIGH_CLR		0x0c
+
+#define ADC_TM_GEN2_CFG_HS_SET			0x0d
+#define ADC_TM_GEN2_CFG_HS_FLAG			BIT(0)
+#define ADC_TM_GEN2_CFG_HS_CLR			0x0e
+
+#define ADC_TM_GEN2_SID				0x40
+
+#define ADC_TM_GEN2_CH_CTL			0x41
+#define ADC_TM_GEN2_TM_CH_SEL			GENMASK(7, 5)
+#define ADC_TM_GEN2_MEAS_INT_SEL		GENMASK(3, 2)
+
+#define ADC_TM_GEN2_ADC_DIG_PARAM		0x42
+#define ADC_TM_GEN2_CTL_CAL_SEL			GENMASK(5, 4)
+#define ADC_TM_GEN2_CTL_DEC_RATIO_MASK		GENMASK(3, 2)
+
+#define ADC_TM_GEN2_FAST_AVG_CTL		0x43
+#define ADC_TM_GEN2_FAST_AVG_EN			BIT(7)
+
+#define ADC_TM_GEN2_ADC_CH_SEL_CTL		0x44
+
+#define ADC_TM_GEN2_DELAY_CTL			0x45
+#define ADC_TM_GEN2_HW_SETTLE_DELAY		GENMASK(3, 0)
+
+#define ADC_TM_GEN2_EN_CTL1			0x46
+#define ADC_TM_GEN2_EN				BIT(7)
+
+#define ADC_TM_GEN2_CONV_REQ			0x47
+#define ADC_TM_GEN2_CONV_REQ_EN			BIT(7)
+
+#define ADC_TM_GEN2_LOW_THR0			0x49
+#define ADC_TM_GEN2_LOW_THR1			0x4a
+#define ADC_TM_GEN2_HIGH_THR0			0x4b
+#define ADC_TM_GEN2_HIGH_THR1			0x4c
+#define ADC_TM_GEN2_LOWER_MASK(n)		((n) & GENMASK(7, 0))
+#define ADC_TM_GEN2_UPPER_MASK(n)		(((n) & GENMASK(15, 8)) >> 8)
+
+#define ADC_TM_GEN2_MEAS_IRQ_EN			0x4d
+#define ADC_TM_GEN2_MEAS_EN			BIT(7)
+#define ADC_TM5_GEN2_HIGH_THR_INT_EN		BIT(1)
+#define ADC_TM5_GEN2_LOW_THR_INT_EN		BIT(0)
+
+#define ADC_TM_GEN2_MEAS_INT_LSB		0x50
+#define ADC_TM_GEN2_MEAS_INT_MSB		0x51
+#define ADC_TM_GEN2_MEAS_INT_MODE		0x52
+
+#define ADC_TM_GEN2_Mn_DATA0(n)			((n * 2) + 0xa0)
+#define ADC_TM_GEN2_Mn_DATA1(n)			((n * 2) + 0xa1)
+#define ADC_TM_GEN2_DATA_SHIFT			8
+
 enum adc5_timer_select {
 	ADC5_TIMER_SEL_1 = 0,
 	ADC5_TIMER_SEL_2,
@@ -81,6 +138,7 @@ enum adc5_timer_select {
 enum adc5_gen {
 	ADC_TM5,
 	ADC_TM_HC,
+	ADC_TM5_GEN2,
 	ADC_TM5_MAX
 };
 
@@ -90,6 +148,14 @@ enum adc_tm5_cal_method {
 	ADC_TM5_ABSOLUTE_CAL
 };
 
+enum adc_tm_gen2_time_select {
+	MEAS_INT_50MS = 0,
+	MEAS_INT_100MS,
+	MEAS_INT_1S,
+	MEAS_INT_SET,
+	MEAS_INT_NONE,
+};
+
 struct adc_tm5_chip;
 struct adc_tm5_channel;
 
@@ -112,6 +178,12 @@ struct adc_tm5_data {
  * @prescale: channel scaling performed on the input signal.
  * @hw_settle_time: the time between AMUX being configured and the
  *	start of conversion.
+ * @decimation: sampling rate supported for the channel.
+ * @avg_samples: ability to provide single result from the ADC
+ *	that is an average of multiple measurements.
+ * @high_thr_en: channel upper voltage threshold enable state.
+ * @low_thr_en: channel lower voltage threshold enable state.
+ * @meas_en: recurring measurement enable state
  * @iio: IIO channel instance used by this channel.
  * @chip: ADC TM chip instance.
  * @tzd: thermal zone device used by this channel.
@@ -122,6 +194,11 @@ struct adc_tm5_channel {
 	enum adc_tm5_cal_method	cal_method;
 	unsigned int		prescale;
 	unsigned int		hw_settle_time;
+	unsigned int		decimation;	/* For Gen2 ADC_TM */
+	unsigned int		avg_samples;	/* For Gen2 ADC_TM */
+	bool			high_thr_en;	/* For Gen2 ADC_TM */
+	bool			low_thr_en;	/* For Gen2 ADC_TM */
+	bool			meas_en;	/* For Gen2 ADC_TM */
 	struct iio_channel	*iio;
 	struct adc_tm5_chip	*chip;
 	struct thermal_zone_device *tzd;
@@ -135,9 +212,15 @@ struct adc_tm5_channel {
  * @channels: array of ADC TM channel data.
  * @nchannels: amount of channels defined/allocated
  * @decimation: sampling rate supported for the channel.
+ *      Applies to all channels, used only on Gen1 ADC_TM.
  * @avg_samples: ability to provide single result from the ADC
- *	that is an average of multiple measurements.
+ *      that is an average of multiple measurements. Applies to all
+ *      channels, used only on Gen1 ADC_TM.
  * @base: base address of TM registers.
+ * @adc_mutex_lock: ADC_TM mutex lock, used only on Gen2 ADC_TM.
+ *      It is used to ensure only one ADC channel configuration
+ *      is done at a time using the shared set of configuration
+ *      registers.
  */
 struct adc_tm5_chip {
 	struct regmap		*regmap;
@@ -148,6 +231,7 @@ struct adc_tm5_chip {
 	unsigned int		decimation;
 	unsigned int		avg_samples;
 	u16			base;
+	struct mutex		adc_mutex_lock;
 };
 
 static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
@@ -214,6 +298,61 @@ static irqreturn_t adc_tm5_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t adc_tm5_gen2_isr(int irq, void *data)
+{
+	struct adc_tm5_chip *chip = data;
+	u8 status_low, status_high;
+	int ret, i;
+
+	ret = adc_tm5_read(chip, ADC_TM_GEN2_STATUS_LOW_CLR, &status_low, sizeof(status_low));
+	if (ret) {
+		dev_err(chip->dev, "read status_low failed: %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	ret = adc_tm5_read(chip, ADC_TM_GEN2_STATUS_HIGH_CLR, &status_high, sizeof(status_high));
+	if (ret) {
+		dev_err(chip->dev, "read status_high failed: %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_STATUS_LOW_CLR, &status_low, sizeof(status_low));
+	if (ret < 0) {
+		dev_err(chip->dev, "clear status low failed with %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_STATUS_HIGH_CLR, &status_high, sizeof(status_high));
+	if (ret < 0) {
+		dev_err(chip->dev, "clear status high failed with %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	for (i = 0; i < chip->nchannels; i++) {
+		bool upper_set = false, lower_set = false;
+		unsigned int ch = chip->channels[i].channel;
+
+		/* No TZD, we warned at the boot time */
+		if (!chip->channels[i].tzd)
+			continue;
+
+		if (!chip->channels[i].meas_en)
+			continue;
+
+		lower_set = (status_low & BIT(ch)) &&
+			(chip->channels[i].low_thr_en);
+
+		upper_set = (status_high & BIT(ch)) &&
+			(chip->channels[i].high_thr_en);
+
+		if (upper_set || lower_set)
+			thermal_zone_device_update(chip->channels[i].tzd,
+						   THERMAL_EVENT_UNSPECIFIED);
+	}
+
+	return IRQ_HANDLED;
+}
+
 static int adc_tm5_get_temp(void *data, int *temp)
 {
 	struct adc_tm5_channel *channel = data;
@@ -244,6 +383,104 @@ static int adc_tm5_disable_channel(struct adc_tm5_channel *channel)
 				  0);
 }
 
+#define ADC_TM_GEN2_POLL_DELAY_MIN_US		100
+#define ADC_TM_GEN2_POLL_DELAY_MAX_US		110
+#define ADC_TM_GEN2_POLL_RETRY_COUNT		3
+
+static int32_t adc_tm5_gen2_conv_req(struct adc_tm5_chip *chip)
+{
+	int ret;
+	u8 data;
+	unsigned int count;
+
+	data = ADC_TM_GEN2_EN;
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_EN_CTL1, &data, 1);
+	if (ret < 0) {
+		dev_err(chip->dev, "adc-tm enable failed with %d\n", ret);
+		return ret;
+	}
+
+	data = ADC_TM_GEN2_CFG_HS_FLAG;
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_CFG_HS_SET, &data, 1);
+	if (ret < 0) {
+		dev_err(chip->dev, "adc-tm handshake failed with %d\n", ret);
+		return ret;
+	}
+
+	data = ADC_TM_GEN2_CONV_REQ_EN;
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_CONV_REQ, &data, 1);
+	if (ret < 0) {
+		dev_err(chip->dev, "adc-tm request conversion failed with %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * SW sets a handshake bit and waits for PBS to clear it
+	 * before the next conversion request can be queued.
+	 */
+
+	for (count = 0; count < ADC_TM_GEN2_POLL_RETRY_COUNT; count++) {
+		ret = adc_tm5_read(chip, ADC_TM_GEN2_CFG_HS_SET, &data, sizeof(data));
+		if (ret < 0) {
+			dev_err(chip->dev, "adc-tm read failed with %d\n", ret);
+			return ret;
+		}
+
+		if (!(data & ADC_TM_GEN2_CFG_HS_FLAG))
+			return ret;
+		usleep_range(ADC_TM_GEN2_POLL_DELAY_MIN_US,
+			ADC_TM_GEN2_POLL_DELAY_MAX_US);
+	}
+
+	dev_err(chip->dev, "adc-tm conversion request handshake timed out\n");
+
+	return -ETIMEDOUT;
+}
+
+static int adc_tm5_gen2_disable_channel(struct adc_tm5_channel *channel)
+{
+	struct adc_tm5_chip *chip = channel->chip;
+	int ret;
+	u8 val;
+
+	mutex_lock(&chip->adc_mutex_lock);
+
+	channel->meas_en = false;
+	channel->high_thr_en = false;
+	channel->low_thr_en = false;
+
+	ret = adc_tm5_read(chip, ADC_TM_GEN2_CH_CTL, &val, sizeof(val));
+	if (ret < 0) {
+		dev_err(chip->dev, "adc-tm block read failed with %d\n", ret);
+		goto disable_fail;
+	}
+
+	val &= ~ADC_TM_GEN2_TM_CH_SEL;
+	val |= FIELD_PREP(ADC_TM_GEN2_TM_CH_SEL, channel->channel);
+
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_CH_CTL, &val, 1);
+	if (ret < 0) {
+		dev_err(chip->dev, "adc-tm channel disable failed with %d\n", ret);
+		goto disable_fail;
+	}
+
+	val = 0;
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_MEAS_IRQ_EN, &val, 1);
+	if (ret < 0) {
+		dev_err(chip->dev, "adc-tm interrupt disable failed with %d\n", ret);
+		goto disable_fail;
+	}
+
+
+	ret = adc_tm5_gen2_conv_req(channel->chip);
+	if (ret < 0)
+		dev_err(chip->dev, "adc-tm channel configure failed with %d\n", ret);
+
+disable_fail:
+	mutex_unlock(&chip->adc_mutex_lock);
+	return ret;
+}
+
 static int adc_tm5_enable(struct adc_tm5_chip *chip)
 {
 	int ret;
@@ -324,6 +561,86 @@ static int adc_tm5_configure(struct adc_tm5_channel *channel, int low, int high)
 	return adc_tm5_enable(chip);
 }
 
+static int adc_tm5_gen2_configure(struct adc_tm5_channel *channel, int low, int high)
+{
+	struct adc_tm5_chip *chip = channel->chip;
+	int ret;
+	u8 buf[14];
+	u16 adc_code;
+
+	mutex_lock(&chip->adc_mutex_lock);
+
+	channel->meas_en = true;
+
+	ret = adc_tm5_read(chip, ADC_TM_GEN2_SID, buf, sizeof(buf));
+	if (ret < 0) {
+		dev_err(chip->dev, "adc-tm block read failed with %d\n", ret);
+		goto config_fail;
+	}
+
+	/* Set SID from virtual channel number */
+	buf[0] = channel->adc_channel >> 8;
+
+	/* Set TM channel number used and measurement interval */
+	buf[1] &= ~ADC_TM_GEN2_TM_CH_SEL;
+	buf[1] |= FIELD_PREP(ADC_TM_GEN2_TM_CH_SEL, channel->channel);
+	buf[1] &= ~ADC_TM_GEN2_MEAS_INT_SEL;
+	buf[1] |= FIELD_PREP(ADC_TM_GEN2_MEAS_INT_SEL, MEAS_INT_1S);
+
+	buf[2] &= ~ADC_TM_GEN2_CTL_DEC_RATIO_MASK;
+	buf[2] |= FIELD_PREP(ADC_TM_GEN2_CTL_DEC_RATIO_MASK, channel->decimation);
+	buf[2] &= ~ADC_TM_GEN2_CTL_CAL_SEL;
+	buf[2] |= FIELD_PREP(ADC_TM_GEN2_CTL_CAL_SEL, channel->cal_method);
+
+	buf[3] = channel->avg_samples | ADC_TM_GEN2_FAST_AVG_EN;
+
+	buf[4] = channel->adc_channel & 0xff;
+
+	buf[5] = channel->hw_settle_time & ADC_TM_GEN2_HW_SETTLE_DELAY;
+
+	/* High temperature corresponds to low voltage threshold */
+	if (high != INT_MAX) {
+		channel->low_thr_en = true;
+		adc_code = qcom_adc_tm5_gen2_temp_res_scale(high);
+
+		buf[9] = adc_code & 0xff;
+		buf[10] = adc_code >> 8;
+	} else {
+		channel->low_thr_en = false;
+	}
+
+	/* Low temperature corresponds to high voltage threshold */
+	if (low != -INT_MAX) {
+		channel->high_thr_en = true;
+		adc_code = qcom_adc_tm5_gen2_temp_res_scale(low);
+
+		buf[11] = adc_code & 0xff;
+		buf[12] = adc_code >> 8;
+	} else {
+		channel->high_thr_en = false;
+	}
+
+	buf[13] = ADC_TM_GEN2_MEAS_EN;
+	if (channel->high_thr_en)
+		buf[13] |= ADC_TM5_GEN2_HIGH_THR_INT_EN;
+	if (channel->low_thr_en)
+		buf[13] |= ADC_TM5_GEN2_LOW_THR_INT_EN;
+
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_SID, buf, sizeof(buf));
+	if (ret) {
+		dev_err(chip->dev, "channel %d params write failed: %d\n", channel->channel, ret);
+		goto config_fail;
+	}
+
+	ret = adc_tm5_gen2_conv_req(channel->chip);
+	if (ret < 0)
+		dev_err(chip->dev, "adc-tm channel configure failed with %d\n", ret);
+
+config_fail:
+	mutex_unlock(&chip->adc_mutex_lock);
+	return ret;
+}
+
 static int adc_tm5_set_trips(void *data, int low, int high)
 {
 	struct adc_tm5_channel *channel = data;
@@ -422,6 +739,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
 		}
 	}
 
+	mutex_init(&chip->adc_mutex_lock);
+
+	if (chip->data->gen == ADC_TM5_GEN2)
+		return ret;
+
 	buf[0] = chip->decimation;
 	buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
 	buf[2] = ADC_TM5_TIMER1;
@@ -442,7 +764,7 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
 				       struct device_node *node)
 {
 	const char *name = node->name;
-	u32 chan, value, varr[2];
+	u32 chan, value, adc_channel, varr[2];
 	int ret;
 	struct device *dev = adc_tm->dev;
 	struct of_phandle_args args;
@@ -472,7 +794,11 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
 	}
 	of_node_put(args.np);
 
-	if (args.args_count != 1 || args.args[0] >= ADC5_MAX_CHANNEL) {
+	adc_channel = args.args[0];
+	if (adc_tm->data->gen == ADC_TM5_GEN2)
+		adc_channel &= 0xff;
+
+	if (args.args_count != 1 || adc_channel >= ADC5_MAX_CHANNEL) {
 		dev_err(dev, "%s: invalid ADC channel number %d\n", name, chan);
 		return -EINVAL;
 	}
@@ -518,6 +844,32 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
 	else
 		channel->cal_method = ADC_TM5_ABSOLUTE_CAL;
 
+	if (adc_tm->data->gen == ADC_TM5_GEN2) {
+		ret = of_property_read_u32(node, "qcom,decimation", &value);
+		if (!ret) {
+			ret = qcom_adc5_decimation_from_dt(value, adc_tm->data->decimation);
+			if (ret < 0) {
+				dev_err(dev, "invalid decimation %d\n", value);
+				return ret;
+			}
+			channel->decimation = ret;
+		} else {
+			channel->decimation = ADC5_DECIMATION_DEFAULT;
+		}
+
+		ret = of_property_read_u32(node, "qcom,avg-samples", &value);
+		if (!ret) {
+			ret = qcom_adc5_avg_samples_from_dt(value);
+			if (ret < 0) {
+				dev_err(dev, "invalid avg-samples %d\n", value);
+				return ret;
+			}
+			channel->avg_samples = ret;
+		} else {
+			channel->avg_samples = VADC_DEF_AVG_SAMPLES;
+		}
+	}
+
 	return 0;
 }
 
@@ -546,6 +898,19 @@ static const struct adc_tm5_data adc_tm_hc_data_pmic = {
 	.gen = ADC_TM_HC,
 };
 
+static const struct adc_tm5_data adc_tm5_gen2_data_pmic = {
+	.full_scale_code_volt = 0x70e4,
+	.decimation = (unsigned int []) { 85, 340, 1360 },
+	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
+					 1000, 2000, 4000, 8000, 16000, 32000,
+					 64000, 128000 },
+	.disable_channel = adc_tm5_gen2_disable_channel,
+	.configure = adc_tm5_gen2_configure,
+	.isr = adc_tm5_gen2_isr,
+	.irq_name = "pm-adc-tm5-gen2",
+	.gen = ADC_TM5_GEN2,
+};
+
 static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node *node)
 {
 	struct adc_tm5_channel *channels;
@@ -671,6 +1036,10 @@ static const struct of_device_id adc_tm5_match_table[] = {
 		.compatible = "qcom,spmi-adc-tm-hc",
 		.data = &adc_tm_hc_data_pmic,
 	},
+	{
+		.compatible = "qcom,spmi-adc-tm5-gen2",
+		.data = &adc_tm5_gen2_data_pmic,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, adc_tm5_match_table);
-- 
2.7.4

Re: [PATCH V3 0/4] thermal: qcom: Add support for PMIC5 Gen2 ADC_TM

[Jonathan Cameron]

On Tue, 23 Nov 2021 11:27:00 +0530
Jishnu Prakash <quic_jprakash@quicinc.com> wrote:

> Made following changes in this post:
> Addressed comments given by Jonathan for qcom-spmi-adc-tm5.yaml.
> Addressed comments given by Dmitry and Jonathan for qcom-spmi-adc-tm5.c.
> Split patch for qcom-spmi-adc-tm5.c into two parts, one to refactor
> code to support multiple device generations and the second to add
> actual Gen2 ADC_TM changes.

Series is missing a change log.  Either in cover letter, or in
individual patches after the --

Jonathan

> 
> Jishnu Prakash (4):
>   dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings
>   iio: adc: qcom-vadc-common: add reverse scaling for PMIC5 Gen2 ADC_TM
>   thermal: qcom: Add support for multiple generations of devices
>   thermal: qcom: add support for PMIC5 Gen2 ADCTM
> 
>  .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 110 ++++-
>  drivers/iio/adc/qcom-vadc-common.c                 |  11 +
>  drivers/thermal/qcom/qcom-spmi-adc-tm5.c           | 451 +++++++++++++++++++--
>  include/linux/iio/adc/qcom-vadc-common.h           |   2 +
>  4 files changed, 541 insertions(+), 33 deletions(-)
> 

Re: [PATCH V3 1/4] dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings

[Jonathan Cameron]

On Tue, 23 Nov 2021 11:27:01 +0530
Jishnu Prakash <quic_jprakash@quicinc.com> wrote:

> Add documentation for PMIC5 Gen2 ADC_TM peripheral.
> It is used for monitoring ADC channel thresholds for PMIC7-type
> PMICs. It is present on PMK8350, like PMIC7 ADC and can be used
> to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
> on a target, through PBS(Programmable Boot Sequence).
> 
> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
This addressed my comments (still open question on whether Rob or anyone
else cares about hiding a typo fix in here, but I'll leave that to others
to comment on).

Thanks,

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 110 ++++++++++++++++++++-
>  1 file changed, 108 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
> index 3ea8c0c..feb390d 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml
> @@ -10,7 +10,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: qcom,spmi-adc-tm5
> +    enum:
> +      - qcom,spmi-adc-tm5
> +      - qcom,spmi-adc-tm5-gen2
>  
>    reg:
>      maxItems: 1
> @@ -33,6 +35,7 @@ properties:
>    qcom,avg-samples:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: Number of samples to be used for measurement.
> +            Not applicable for Gen2 ADC_TM peripheral.
>      enum:
>        - 1
>        - 2
> @@ -45,6 +48,7 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: This parameter is used to decrease ADC sampling rate.
>              Quicker measurements can be made by reducing decimation ratio.
> +            Not applicable for Gen2 ADC_TM peripheral.
>      enum:
>        - 250
>        - 420
> @@ -93,6 +97,29 @@ patternProperties:
>            - const: 1
>            - enum: [ 1, 3, 4, 6, 20, 8, 10 ]
>  
> +      qcom,avg-samples:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: Number of samples to be used for measurement.
> +          This property in child node is applicable only for Gen2 ADC_TM peripheral.
> +        enum:
> +          - 1
> +          - 2
> +          - 4
> +          - 8
> +          - 16
> +        default: 1
> +
> +      qcom,decimation:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: This parameter is used to decrease ADC sampling rate.
> +          Quicker measurements can be made by reducing decimation ratio.
> +          This property in child node is applicable only for Gen2 ADC_TM peripheral.
> +        enum:
> +          - 85
> +          - 340
> +          - 1360
> +        default: 1360
> +
>      required:
>        - reg
>        - io-channels
> @@ -100,6 +127,31 @@ patternProperties:
>      additionalProperties:
>        false
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,spmi-adc-tm5
> +
> +    then:
> +      patternProperties:
> +        "^([-a-z0-9]*)@[0-7]$":
> +          properties:
> +            qcom,decimation: false
> +            qcom,avg-samples: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,spmi-adc-tm5-gen2
> +
> +    then:
> +      properties:
> +        qcom,avg-samples: false
> +        qcom,decimation: false
> +
>  required:
>    - compatible
>    - reg
> @@ -124,7 +176,7 @@ examples:
>              #size-cells = <0>;
>              #io-channel-cells = <1>;
>  
> -            /* Other propreties are omitted */
> +            /* Other properties are omitted */
>              conn-therm@4f {
>                  reg = <ADC5_AMUX_THM3_100K_PU>;
>                  qcom,ratiometric;
> @@ -148,4 +200,58 @@ examples:
>              };
>          };
>      };
> +
> +  - |
> +    #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
> +    #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spmi_bus {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        pmk8350_vadc: adc@3100 {
> +            reg = <0x3100>;
> +            compatible = "qcom,spmi-adc7";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            #io-channel-cells = <1>;
> +
> +            /* Other properties are omitted */
> +            xo-therm@44 {
> +                reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
> +                qcom,ratiometric;
> +                qcom,hw-settle-time = <200>;
> +            };
> +
> +            conn-therm@47 {
> +                reg = <PM8350_ADC7_AMUX_THM4_100K_PU>;
> +                qcom,ratiometric;
> +                qcom,hw-settle-time = <200>;
> +            };
> +        };
> +
> +        pmk8350_adc_tm: adc-tm@3400 {
> +            compatible = "qcom,spmi-adc-tm5-gen2";
> +            reg = <0x3400>;
> +            interrupts = <0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>;
> +            #thermal-sensor-cells = <1>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            pmk8350-xo-therm@0 {
> +                reg = <0>;
> +                io-channels = <&pmk8350_vadc PMK8350_ADC7_AMUX_THM1_100K_PU>;
> +                qcom,decimation = <340>;
> +                qcom,ratiometric;
> +                qcom,hw-settle-time-us = <200>;
> +            };
> +
> +            conn-therm@1 {
> +                reg = <1>;
> +                io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM4_100K_PU>;
> +                qcom,avg-samples = <2>;
> +                qcom,ratiometric;
> +                qcom,hw-settle-time-us = <200>;
> +            };
> +        };
> +    };
>  ...

Re: [PATCH V3 3/4] thermal: qcom: Add support for multiple generations of devices

[Jonathan Cameron]

On Tue, 23 Nov 2021 11:27:03 +0530
Jishnu Prakash <quic_jprakash@quicinc.com> wrote:

> Refactor code to support multiple generations of ADC_TM devices
> by defining gen number, irq name and disable, configure and isr
> APIs in the individual data structs.
> 
> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 76 ++++++++++++++++++++------------
>  1 file changed, 48 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> index 824671c..fc8cd45 100644
> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> @@ -78,11 +78,10 @@ enum adc5_timer_select {
>  	ADC5_TIMER_SEL_NONE,
>  };
>  
> -struct adc_tm5_data {
> -	const u32	full_scale_code_volt;
> -	unsigned int	*decimation;
> -	unsigned int	*hw_settle;
> -	bool		is_hc;
> +enum adc5_gen {
> +	ADC_TM5,
> +	ADC_TM_HC,
> +	ADC_TM5_MAX
>  };
>  
>  enum adc_tm5_cal_method {
> @@ -92,6 +91,18 @@ enum adc_tm5_cal_method {
>  };
>  
>  struct adc_tm5_chip;
> +struct adc_tm5_channel;
> +
> +struct adc_tm5_data {
> +	const u32 full_scale_code_volt;
> +	unsigned int *decimation;
> +	unsigned int *hw_settle;
> +	int (*disable_channel)(struct adc_tm5_channel *channel);
> +	int (*configure)(struct adc_tm5_channel *channel, int low, int high);
> +	irqreturn_t (*isr)(int irq, void *data);
> +	char *irq_name;
> +	int gen;
> +};
>  
>  /**
>   * struct adc_tm5_channel - ADC Thermal Monitoring channel data.
> @@ -139,22 +150,6 @@ struct adc_tm5_chip {
>  	u16			base;
>  };
>  
> -static const struct adc_tm5_data adc_tm5_data_pmic = {
> -	.full_scale_code_volt = 0x70e4,
> -	.decimation = (unsigned int []) { 250, 420, 840 },
> -	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
> -					 1000, 2000, 4000, 8000, 16000, 32000,
> -					 64000, 128000 },
> -};
> -
> -static const struct adc_tm5_data adc_tm_hc_data_pmic = {
> -	.full_scale_code_volt = 0x70e4,
> -	.decimation = (unsigned int []) { 256, 512, 1024 },
> -	.hw_settle = (unsigned int []) { 0, 100, 200, 300, 400, 500, 600, 700,
> -					 1000, 2000, 4000, 6000, 8000, 10000 },
> -	.is_hc = true,
> -};
> -
>  static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
>  {
>  	return regmap_bulk_read(adc_tm->regmap, adc_tm->base + offset, data, len);
> @@ -343,14 +338,14 @@ static int adc_tm5_set_trips(void *data, int low, int high)
>  		channel->channel, low, high);
>  
>  	if (high == INT_MAX && low <= -INT_MAX)
> -		ret = adc_tm5_disable_channel(channel);
> +		ret = chip->data->disable_channel(channel);
>  	else
> -		ret = adc_tm5_configure(channel, low, high);
> +		ret = chip->data->configure(channel, low, high);
>  
>  	return ret;
>  }
>  
> -static struct thermal_zone_of_device_ops adc_tm5_ops = {
> +static struct thermal_zone_of_device_ops adc_tm5_thermal_ops = {
>  	.get_temp = adc_tm5_get_temp,
>  	.set_trips = adc_tm5_set_trips,
>  };
> @@ -366,7 +361,7 @@ static int adc_tm5_register_tzd(struct adc_tm5_chip *adc_tm)
>  		tzd = devm_thermal_zone_of_sensor_register(adc_tm->dev,
>  							   adc_tm->channels[i].channel,
>  							   &adc_tm->channels[i],
> -							   &adc_tm5_ops);
> +							   &adc_tm5_thermal_ops);
>  		if (IS_ERR(tzd)) {
>  			if (PTR_ERR(tzd) == -ENODEV) {
>  				dev_warn(adc_tm->dev, "thermal sensor on channel %d is not used\n",
> @@ -526,6 +521,31 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>  	return 0;
>  }
>  
> +static const struct adc_tm5_data adc_tm5_data_pmic = {
> +	.full_scale_code_volt = 0x70e4,
> +	.decimation = (unsigned int []) { 250, 420, 840 },
> +	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
> +					 1000, 2000, 4000, 8000, 16000, 32000,
> +					 64000, 128000 },
> +	.disable_channel = adc_tm5_disable_channel,
> +	.configure = adc_tm5_configure,
> +	.isr = adc_tm5_isr,
> +	.irq_name = "pm-adc-tm5",
> +	.gen = ADC_TM5,
> +};
> +
> +static const struct adc_tm5_data adc_tm_hc_data_pmic = {
> +	.full_scale_code_volt = 0x70e4,
> +	.decimation = (unsigned int []) { 256, 512, 1024 },
> +	.hw_settle = (unsigned int []) { 0, 100, 200, 300, 400, 500, 600, 700,
> +					 1000, 2000, 4000, 6000, 8000, 10000 },
> +	.disable_channel = adc_tm5_disable_channel,
> +	.configure = adc_tm5_configure,
> +	.isr = adc_tm5_isr,
> +	.irq_name = "pm-adc-tm5",
> +	.gen = ADC_TM_HC,
> +};
> +
>  static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node *node)
>  {
>  	struct adc_tm5_channel *channels;
> @@ -623,7 +643,7 @@ static int adc_tm5_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	if (adc_tm->data->is_hc)
> +	if (adc_tm->data->gen == ADC_TM_HC)
>  		ret = adc_tm_hc_init(adc_tm);
>  	else
>  		ret = adc_tm5_init(adc_tm);
> @@ -638,8 +658,8 @@ static int adc_tm5_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_isr,
> -					 IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
> +	return devm_request_threaded_irq(dev, irq, NULL, adc_tm->data->isr,
> +			IRQF_ONESHOT, adc_tm->data->irq_name, adc_tm);
>  }
>  
>  static const struct of_device_id adc_tm5_match_table[] = {

Re: [PATCH V3 4/4] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Jonathan Cameron]

On Tue, 23 Nov 2021 11:27:04 +0530
Jishnu Prakash <quic_jprakash@quicinc.com> wrote:

> Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a
> close counterpart of PMIC7 ADC and has the same functionality as
> PMIC5 ADC_TM, for threshold monitoring and interrupt generation.
> It is present on PMK8350 alone, like PMIC7 ADC and can be used
> to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
> having ADC on a target, through PBS(Programmable Boot Sequence).
> 
> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>

Just one note on using put_unaligned_le16() below.  Otherwise, from
a drive by review point of view it looks fine to someone not that
familiar with the driver or thermal :)

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 375 ++++++++++++++++++++++++++++++-
>  1 file changed, 372 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> index fc8cd45..a7b33a8 100644
> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> @@ -4,7 +4,10 @@
>   *
>   * Based on original driver:
>   * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
> + *
> + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
> +
>  #include <linux/bitfield.h>
>  #include <linux/iio/adc/qcom-vadc-common.h>
>  #include <linux/iio/consumer.h>
> @@ -71,6 +74,60 @@
>  #define ADC_TM5_M_HIGH_THR_INT_EN			BIT(1)
>  #define ADC_TM5_M_LOW_THR_INT_EN			BIT(0)
>  
> +#define ADC_TM_GEN2_STATUS1			0x08
> +#define ADC_TM_GEN2_STATUS_LOW_SET		0x09
> +#define ADC_TM_GEN2_STATUS_LOW_CLR		0x0a
> +#define ADC_TM_GEN2_STATUS_HIGH_SET		0x0b
> +#define ADC_TM_GEN2_STATUS_HIGH_CLR		0x0c
> +
> +#define ADC_TM_GEN2_CFG_HS_SET			0x0d
> +#define ADC_TM_GEN2_CFG_HS_FLAG			BIT(0)
> +#define ADC_TM_GEN2_CFG_HS_CLR			0x0e
> +
> +#define ADC_TM_GEN2_SID				0x40
> +
> +#define ADC_TM_GEN2_CH_CTL			0x41
> +#define ADC_TM_GEN2_TM_CH_SEL			GENMASK(7, 5)
> +#define ADC_TM_GEN2_MEAS_INT_SEL		GENMASK(3, 2)
> +
> +#define ADC_TM_GEN2_ADC_DIG_PARAM		0x42
> +#define ADC_TM_GEN2_CTL_CAL_SEL			GENMASK(5, 4)
> +#define ADC_TM_GEN2_CTL_DEC_RATIO_MASK		GENMASK(3, 2)
> +
> +#define ADC_TM_GEN2_FAST_AVG_CTL		0x43
> +#define ADC_TM_GEN2_FAST_AVG_EN			BIT(7)
> +
> +#define ADC_TM_GEN2_ADC_CH_SEL_CTL		0x44
> +
> +#define ADC_TM_GEN2_DELAY_CTL			0x45
> +#define ADC_TM_GEN2_HW_SETTLE_DELAY		GENMASK(3, 0)
> +
> +#define ADC_TM_GEN2_EN_CTL1			0x46
> +#define ADC_TM_GEN2_EN				BIT(7)
> +
> +#define ADC_TM_GEN2_CONV_REQ			0x47
> +#define ADC_TM_GEN2_CONV_REQ_EN			BIT(7)
> +
> +#define ADC_TM_GEN2_LOW_THR0			0x49
> +#define ADC_TM_GEN2_LOW_THR1			0x4a
> +#define ADC_TM_GEN2_HIGH_THR0			0x4b
> +#define ADC_TM_GEN2_HIGH_THR1			0x4c
> +#define ADC_TM_GEN2_LOWER_MASK(n)		((n) & GENMASK(7, 0))
> +#define ADC_TM_GEN2_UPPER_MASK(n)		(((n) & GENMASK(15, 8)) >> 8)
> +
> +#define ADC_TM_GEN2_MEAS_IRQ_EN			0x4d
> +#define ADC_TM_GEN2_MEAS_EN			BIT(7)
> +#define ADC_TM5_GEN2_HIGH_THR_INT_EN		BIT(1)
> +#define ADC_TM5_GEN2_LOW_THR_INT_EN		BIT(0)
> +
> +#define ADC_TM_GEN2_MEAS_INT_LSB		0x50
> +#define ADC_TM_GEN2_MEAS_INT_MSB		0x51
> +#define ADC_TM_GEN2_MEAS_INT_MODE		0x52
> +
> +#define ADC_TM_GEN2_Mn_DATA0(n)			((n * 2) + 0xa0)
> +#define ADC_TM_GEN2_Mn_DATA1(n)			((n * 2) + 0xa1)
> +#define ADC_TM_GEN2_DATA_SHIFT			8
> +
>  enum adc5_timer_select {
>  	ADC5_TIMER_SEL_1 = 0,
>  	ADC5_TIMER_SEL_2,
> @@ -81,6 +138,7 @@ enum adc5_timer_select {
>  enum adc5_gen {
>  	ADC_TM5,
>  	ADC_TM_HC,
> +	ADC_TM5_GEN2,
>  	ADC_TM5_MAX
>  };
>  
> @@ -90,6 +148,14 @@ enum adc_tm5_cal_method {
>  	ADC_TM5_ABSOLUTE_CAL
>  };
>  
> +enum adc_tm_gen2_time_select {
> +	MEAS_INT_50MS = 0,
> +	MEAS_INT_100MS,
> +	MEAS_INT_1S,
> +	MEAS_INT_SET,
> +	MEAS_INT_NONE,
> +};
> +
>  struct adc_tm5_chip;
>  struct adc_tm5_channel;
>  
> @@ -112,6 +178,12 @@ struct adc_tm5_data {
>   * @prescale: channel scaling performed on the input signal.
>   * @hw_settle_time: the time between AMUX being configured and the
>   *	start of conversion.
> + * @decimation: sampling rate supported for the channel.
> + * @avg_samples: ability to provide single result from the ADC
> + *	that is an average of multiple measurements.
> + * @high_thr_en: channel upper voltage threshold enable state.
> + * @low_thr_en: channel lower voltage threshold enable state.
> + * @meas_en: recurring measurement enable state
>   * @iio: IIO channel instance used by this channel.
>   * @chip: ADC TM chip instance.
>   * @tzd: thermal zone device used by this channel.
> @@ -122,6 +194,11 @@ struct adc_tm5_channel {
>  	enum adc_tm5_cal_method	cal_method;
>  	unsigned int		prescale;
>  	unsigned int		hw_settle_time;
> +	unsigned int		decimation;	/* For Gen2 ADC_TM */
> +	unsigned int		avg_samples;	/* For Gen2 ADC_TM */
> +	bool			high_thr_en;	/* For Gen2 ADC_TM */
> +	bool			low_thr_en;	/* For Gen2 ADC_TM */
> +	bool			meas_en;	/* For Gen2 ADC_TM */
>  	struct iio_channel	*iio;
>  	struct adc_tm5_chip	*chip;
>  	struct thermal_zone_device *tzd;
> @@ -135,9 +212,15 @@ struct adc_tm5_channel {
>   * @channels: array of ADC TM channel data.
>   * @nchannels: amount of channels defined/allocated
>   * @decimation: sampling rate supported for the channel.
> + *      Applies to all channels, used only on Gen1 ADC_TM.
>   * @avg_samples: ability to provide single result from the ADC
> - *	that is an average of multiple measurements.
> + *      that is an average of multiple measurements. Applies to all
> + *      channels, used only on Gen1 ADC_TM.
>   * @base: base address of TM registers.
> + * @adc_mutex_lock: ADC_TM mutex lock, used only on Gen2 ADC_TM.
> + *      It is used to ensure only one ADC channel configuration
> + *      is done at a time using the shared set of configuration
> + *      registers.

Much better. Thanks.

>   */
>  struct adc_tm5_chip {
>  	struct regmap		*regmap;
> @@ -148,6 +231,7 @@ struct adc_tm5_chip {
>  	unsigned int		decimation;
>  	unsigned int		avg_samples;
>  	u16			base;
> +	struct mutex		adc_mutex_lock;
>  };
>  
>  static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
> @@ -214,6 +298,61 @@ static irqreturn_t adc_tm5_isr(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t adc_tm5_gen2_isr(int irq, void *data)
> +{
> +	struct adc_tm5_chip *chip = data;
> +	u8 status_low, status_high;
> +	int ret, i;
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_STATUS_LOW_CLR, &status_low, sizeof(status_low));
> +	if (ret) {
> +		dev_err(chip->dev, "read status_low failed: %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_STATUS_HIGH_CLR, &status_high, sizeof(status_high));
> +	if (ret) {
> +		dev_err(chip->dev, "read status_high failed: %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_STATUS_LOW_CLR, &status_low, sizeof(status_low));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "clear status low failed with %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_STATUS_HIGH_CLR, &status_high, sizeof(status_high));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "clear status high failed with %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	for (i = 0; i < chip->nchannels; i++) {
> +		bool upper_set = false, lower_set = false;
> +		unsigned int ch = chip->channels[i].channel;
> +
> +		/* No TZD, we warned at the boot time */
> +		if (!chip->channels[i].tzd)
> +			continue;
> +
> +		if (!chip->channels[i].meas_en)
> +			continue;
> +
> +		lower_set = (status_low & BIT(ch)) &&
> +			(chip->channels[i].low_thr_en);
> +
> +		upper_set = (status_high & BIT(ch)) &&
> +			(chip->channels[i].high_thr_en);
> +
> +		if (upper_set || lower_set)
> +			thermal_zone_device_update(chip->channels[i].tzd,
> +						   THERMAL_EVENT_UNSPECIFIED);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int adc_tm5_get_temp(void *data, int *temp)
>  {
>  	struct adc_tm5_channel *channel = data;
> @@ -244,6 +383,104 @@ static int adc_tm5_disable_channel(struct adc_tm5_channel *channel)
>  				  0);
>  }
>  
> +#define ADC_TM_GEN2_POLL_DELAY_MIN_US		100
> +#define ADC_TM_GEN2_POLL_DELAY_MAX_US		110
> +#define ADC_TM_GEN2_POLL_RETRY_COUNT		3
> +
> +static int32_t adc_tm5_gen2_conv_req(struct adc_tm5_chip *chip)
> +{
> +	int ret;
> +	u8 data;
> +	unsigned int count;
> +
> +	data = ADC_TM_GEN2_EN;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_EN_CTL1, &data, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm enable failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	data = ADC_TM_GEN2_CFG_HS_FLAG;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_CFG_HS_SET, &data, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm handshake failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	data = ADC_TM_GEN2_CONV_REQ_EN;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_CONV_REQ, &data, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm request conversion failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * SW sets a handshake bit and waits for PBS to clear it
> +	 * before the next conversion request can be queued.
> +	 */
> +
> +	for (count = 0; count < ADC_TM_GEN2_POLL_RETRY_COUNT; count++) {
> +		ret = adc_tm5_read(chip, ADC_TM_GEN2_CFG_HS_SET, &data, sizeof(data));
> +		if (ret < 0) {
> +			dev_err(chip->dev, "adc-tm read failed with %d\n", ret);
> +			return ret;
> +		}
> +
> +		if (!(data & ADC_TM_GEN2_CFG_HS_FLAG))
> +			return ret;
> +		usleep_range(ADC_TM_GEN2_POLL_DELAY_MIN_US,
> +			ADC_TM_GEN2_POLL_DELAY_MAX_US);
> +	}
> +
> +	dev_err(chip->dev, "adc-tm conversion request handshake timed out\n");
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int adc_tm5_gen2_disable_channel(struct adc_tm5_channel *channel)
> +{
> +	struct adc_tm5_chip *chip = channel->chip;
> +	int ret;
> +	u8 val;
> +
> +	mutex_lock(&chip->adc_mutex_lock);
> +
> +	channel->meas_en = false;
> +	channel->high_thr_en = false;
> +	channel->low_thr_en = false;
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_CH_CTL, &val, sizeof(val));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm block read failed with %d\n", ret);
> +		goto disable_fail;
> +	}
> +
> +	val &= ~ADC_TM_GEN2_TM_CH_SEL;
> +	val |= FIELD_PREP(ADC_TM_GEN2_TM_CH_SEL, channel->channel);
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_CH_CTL, &val, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm channel disable failed with %d\n", ret);
> +		goto disable_fail;
> +	}
> +
> +	val = 0;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_MEAS_IRQ_EN, &val, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm interrupt disable failed with %d\n", ret);
> +		goto disable_fail;
> +	}
> +
> +
> +	ret = adc_tm5_gen2_conv_req(channel->chip);
> +	if (ret < 0)
> +		dev_err(chip->dev, "adc-tm channel configure failed with %d\n", ret);
> +
> +disable_fail:
> +	mutex_unlock(&chip->adc_mutex_lock);
> +	return ret;
> +}
> +
>  static int adc_tm5_enable(struct adc_tm5_chip *chip)
>  {
>  	int ret;
> @@ -324,6 +561,86 @@ static int adc_tm5_configure(struct adc_tm5_channel *channel, int low, int high)
>  	return adc_tm5_enable(chip);
>  }
>  
> +static int adc_tm5_gen2_configure(struct adc_tm5_channel *channel, int low, int high)
> +{
> +	struct adc_tm5_chip *chip = channel->chip;
> +	int ret;
> +	u8 buf[14];
> +	u16 adc_code;
> +
> +	mutex_lock(&chip->adc_mutex_lock);
> +
> +	channel->meas_en = true;
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_SID, buf, sizeof(buf));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm block read failed with %d\n", ret);
> +		goto config_fail;
> +	}
> +
> +	/* Set SID from virtual channel number */
> +	buf[0] = channel->adc_channel >> 8;
> +
> +	/* Set TM channel number used and measurement interval */
> +	buf[1] &= ~ADC_TM_GEN2_TM_CH_SEL;
> +	buf[1] |= FIELD_PREP(ADC_TM_GEN2_TM_CH_SEL, channel->channel);
> +	buf[1] &= ~ADC_TM_GEN2_MEAS_INT_SEL;
> +	buf[1] |= FIELD_PREP(ADC_TM_GEN2_MEAS_INT_SEL, MEAS_INT_1S);
> +
> +	buf[2] &= ~ADC_TM_GEN2_CTL_DEC_RATIO_MASK;
> +	buf[2] |= FIELD_PREP(ADC_TM_GEN2_CTL_DEC_RATIO_MASK, channel->decimation);
> +	buf[2] &= ~ADC_TM_GEN2_CTL_CAL_SEL;
> +	buf[2] |= FIELD_PREP(ADC_TM_GEN2_CTL_CAL_SEL, channel->cal_method);
> +
> +	buf[3] = channel->avg_samples | ADC_TM_GEN2_FAST_AVG_EN;
> +
> +	buf[4] = channel->adc_channel & 0xff;
> +
> +	buf[5] = channel->hw_settle_time & ADC_TM_GEN2_HW_SETTLE_DELAY;
> +
> +	/* High temperature corresponds to low voltage threshold */
> +	if (high != INT_MAX) {
> +		channel->low_thr_en = true;
> +		adc_code = qcom_adc_tm5_gen2_temp_res_scale(high);
> +
> +		buf[9] = adc_code & 0xff;
> +		buf[10] = adc_code >> 8;
> +	} else {
> +		channel->low_thr_en = false;
> +	}
> +
> +	/* Low temperature corresponds to high voltage threshold */
> +	if (low != -INT_MAX) {
> +		channel->high_thr_en = true;
> +		adc_code = qcom_adc_tm5_gen2_temp_res_scale(low);
> +
> +		buf[11] = adc_code & 0xff;
> +		buf[12] = adc_code >> 8;

looks like a little endian put though not necessarily aligned so
put_unaligned_le16() preferred to open coding it. Same in similar places.
Not my area though so maintainer may not care as much.

> +	} else {
> +		channel->high_thr_en = false;
> +	}
> +
> +	buf[13] = ADC_TM_GEN2_MEAS_EN;
> +	if (channel->high_thr_en)
> +		buf[13] |= ADC_TM5_GEN2_HIGH_THR_INT_EN;
> +	if (channel->low_thr_en)
> +		buf[13] |= ADC_TM5_GEN2_LOW_THR_INT_EN;
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_SID, buf, sizeof(buf));
> +	if (ret) {
> +		dev_err(chip->dev, "channel %d params write failed: %d\n", channel->channel, ret);
> +		goto config_fail;
> +	}
> +
> +	ret = adc_tm5_gen2_conv_req(channel->chip);
> +	if (ret < 0)
> +		dev_err(chip->dev, "adc-tm channel configure failed with %d\n", ret);
> +
> +config_fail:
> +	mutex_unlock(&chip->adc_mutex_lock);
> +	return ret;
> +}
> +
>  static int adc_tm5_set_trips(void *data, int low, int high)
>  {
>  	struct adc_tm5_channel *channel = data;
> @@ -422,6 +739,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
>  		}
>  	}
>  
> +	mutex_init(&chip->adc_mutex_lock);
> +
> +	if (chip->data->gen == ADC_TM5_GEN2)
> +		return ret;
> +
>  	buf[0] = chip->decimation;
>  	buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
>  	buf[2] = ADC_TM5_TIMER1;
> @@ -442,7 +764,7 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>  				       struct device_node *node)
>  {
>  	const char *name = node->name;
> -	u32 chan, value, varr[2];
> +	u32 chan, value, adc_channel, varr[2];
>  	int ret;
>  	struct device *dev = adc_tm->dev;
>  	struct of_phandle_args args;
> @@ -472,7 +794,11 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>  	}
>  	of_node_put(args.np);
>  
> -	if (args.args_count != 1 || args.args[0] >= ADC5_MAX_CHANNEL) {
> +	adc_channel = args.args[0];
> +	if (adc_tm->data->gen == ADC_TM5_GEN2)
> +		adc_channel &= 0xff;
> +
> +	if (args.args_count != 1 || adc_channel >= ADC5_MAX_CHANNEL) {
>  		dev_err(dev, "%s: invalid ADC channel number %d\n", name, chan);
>  		return -EINVAL;
>  	}
> @@ -518,6 +844,32 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>  	else
>  		channel->cal_method = ADC_TM5_ABSOLUTE_CAL;
>  
> +	if (adc_tm->data->gen == ADC_TM5_GEN2) {
> +		ret = of_property_read_u32(node, "qcom,decimation", &value);
> +		if (!ret) {
> +			ret = qcom_adc5_decimation_from_dt(value, adc_tm->data->decimation);
> +			if (ret < 0) {
> +				dev_err(dev, "invalid decimation %d\n", value);
> +				return ret;
> +			}
> +			channel->decimation = ret;
> +		} else {
> +			channel->decimation = ADC5_DECIMATION_DEFAULT;
> +		}
> +
> +		ret = of_property_read_u32(node, "qcom,avg-samples", &value);
> +		if (!ret) {
> +			ret = qcom_adc5_avg_samples_from_dt(value);
> +			if (ret < 0) {
> +				dev_err(dev, "invalid avg-samples %d\n", value);
> +				return ret;
> +			}
> +			channel->avg_samples = ret;
> +		} else {
> +			channel->avg_samples = VADC_DEF_AVG_SAMPLES;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -546,6 +898,19 @@ static const struct adc_tm5_data adc_tm_hc_data_pmic = {
>  	.gen = ADC_TM_HC,
>  };
>  
> +static const struct adc_tm5_data adc_tm5_gen2_data_pmic = {
> +	.full_scale_code_volt = 0x70e4,
> +	.decimation = (unsigned int []) { 85, 340, 1360 },
> +	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
> +					 1000, 2000, 4000, 8000, 16000, 32000,
> +					 64000, 128000 },
> +	.disable_channel = adc_tm5_gen2_disable_channel,
> +	.configure = adc_tm5_gen2_configure,
> +	.isr = adc_tm5_gen2_isr,
> +	.irq_name = "pm-adc-tm5-gen2",
> +	.gen = ADC_TM5_GEN2,
> +};
> +
>  static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node *node)
>  {
>  	struct adc_tm5_channel *channels;
> @@ -671,6 +1036,10 @@ static const struct of_device_id adc_tm5_match_table[] = {
>  		.compatible = "qcom,spmi-adc-tm-hc",
>  		.data = &adc_tm_hc_data_pmic,
>  	},
> +	{
> +		.compatible = "qcom,spmi-adc-tm5-gen2",
> +		.data = &adc_tm5_gen2_data_pmic,
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, adc_tm5_match_table);

Re: [PATCH V3 1/4] dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings

[Rob Herring]

On Tue, 23 Nov 2021 11:27:01 +0530, Jishnu Prakash wrote:
> Add documentation for PMIC5 Gen2 ADC_TM peripheral.
> It is used for monitoring ADC channel thresholds for PMIC7-type
> PMICs. It is present on PMK8350, like PMIC7 ADC and can be used
> to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
> on a target, through PBS(Programmable Boot Sequence).
> 
> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
> ---
>  .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 110 ++++++++++++++++++++-
>  1 file changed, 108 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH V3 3/4] thermal: qcom: Add support for multiple generations of devices

[Dmitry Baryshkov]

On 23/11/2021 08:57, Jishnu Prakash wrote:
> Refactor code to support multiple generations of ADC_TM devices
> by defining gen number, irq name and disable, configure and isr
> APIs in the individual data structs.
> 
> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Minor question below.

> ---
>   drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 76 ++++++++++++++++++++------------
>   1 file changed, 48 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> index 824671c..fc8cd45 100644
> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> @@ -78,11 +78,10 @@ enum adc5_timer_select {
>   	ADC5_TIMER_SEL_NONE,
>   };
>   
> -struct adc_tm5_data {
> -	const u32	full_scale_code_volt;
> -	unsigned int	*decimation;
> -	unsigned int	*hw_settle;
> -	bool		is_hc;
> +enum adc5_gen {
> +	ADC_TM5,
> +	ADC_TM_HC,
> +	ADC_TM5_MAX
>   };
>   
>   enum adc_tm5_cal_method {
> @@ -92,6 +91,18 @@ enum adc_tm5_cal_method {
>   };
>   
>   struct adc_tm5_chip;
> +struct adc_tm5_channel;
> +
> +struct adc_tm5_data {
> +	const u32 full_scale_code_volt;
> +	unsigned int *decimation;
> +	unsigned int *hw_settle;
> +	int (*disable_channel)(struct adc_tm5_channel *channel);
> +	int (*configure)(struct adc_tm5_channel *channel, int low, int high);
> +	irqreturn_t (*isr)(int irq, void *data);
> +	char *irq_name;
> +	int gen;
> +};

Any reason for moving the adc_tm5_data definition? It is still prefix 
with the adc_tmp5_channel declaration.

>   
>   /**
>    * struct adc_tm5_channel - ADC Thermal Monitoring channel data.
> @@ -139,22 +150,6 @@ struct adc_tm5_chip {
>   	u16			base;
>   };
>   
> -static const struct adc_tm5_data adc_tm5_data_pmic = {
> -	.full_scale_code_volt = 0x70e4,
> -	.decimation = (unsigned int []) { 250, 420, 840 },
> -	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
> -					 1000, 2000, 4000, 8000, 16000, 32000,
> -					 64000, 128000 },
> -};
> -
> -static const struct adc_tm5_data adc_tm_hc_data_pmic = {
> -	.full_scale_code_volt = 0x70e4,
> -	.decimation = (unsigned int []) { 256, 512, 1024 },
> -	.hw_settle = (unsigned int []) { 0, 100, 200, 300, 400, 500, 600, 700,
> -					 1000, 2000, 4000, 6000, 8000, 10000 },
> -	.is_hc = true,
> -};
> -
>   static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
>   {
>   	return regmap_bulk_read(adc_tm->regmap, adc_tm->base + offset, data, len);
> @@ -343,14 +338,14 @@ static int adc_tm5_set_trips(void *data, int low, int high)
>   		channel->channel, low, high);
>   
>   	if (high == INT_MAX && low <= -INT_MAX)
> -		ret = adc_tm5_disable_channel(channel);
> +		ret = chip->data->disable_channel(channel);
>   	else
> -		ret = adc_tm5_configure(channel, low, high);
> +		ret = chip->data->configure(channel, low, high);
>   
>   	return ret;
>   }
>   
> -static struct thermal_zone_of_device_ops adc_tm5_ops = {
> +static struct thermal_zone_of_device_ops adc_tm5_thermal_ops = {
>   	.get_temp = adc_tm5_get_temp,
>   	.set_trips = adc_tm5_set_trips,
>   };
> @@ -366,7 +361,7 @@ static int adc_tm5_register_tzd(struct adc_tm5_chip *adc_tm)
>   		tzd = devm_thermal_zone_of_sensor_register(adc_tm->dev,
>   							   adc_tm->channels[i].channel,
>   							   &adc_tm->channels[i],
> -							   &adc_tm5_ops);
> +							   &adc_tm5_thermal_ops);
>   		if (IS_ERR(tzd)) {
>   			if (PTR_ERR(tzd) == -ENODEV) {
>   				dev_warn(adc_tm->dev, "thermal sensor on channel %d is not used\n",
> @@ -526,6 +521,31 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>   	return 0;
>   }
>   
> +static const struct adc_tm5_data adc_tm5_data_pmic = {
> +	.full_scale_code_volt = 0x70e4,
> +	.decimation = (unsigned int []) { 250, 420, 840 },
> +	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
> +					 1000, 2000, 4000, 8000, 16000, 32000,
> +					 64000, 128000 },
> +	.disable_channel = adc_tm5_disable_channel,
> +	.configure = adc_tm5_configure,
> +	.isr = adc_tm5_isr,
> +	.irq_name = "pm-adc-tm5",
> +	.gen = ADC_TM5,
> +};
> +
> +static const struct adc_tm5_data adc_tm_hc_data_pmic = {
> +	.full_scale_code_volt = 0x70e4,
> +	.decimation = (unsigned int []) { 256, 512, 1024 },
> +	.hw_settle = (unsigned int []) { 0, 100, 200, 300, 400, 500, 600, 700,
> +					 1000, 2000, 4000, 6000, 8000, 10000 },
> +	.disable_channel = adc_tm5_disable_channel,
> +	.configure = adc_tm5_configure,
> +	.isr = adc_tm5_isr,
> +	.irq_name = "pm-adc-tm5",
> +	.gen = ADC_TM_HC,
> +};
> +
>   static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node *node)
>   {
>   	struct adc_tm5_channel *channels;
> @@ -623,7 +643,7 @@ static int adc_tm5_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>   
> -	if (adc_tm->data->is_hc)
> +	if (adc_tm->data->gen == ADC_TM_HC)
>   		ret = adc_tm_hc_init(adc_tm);
>   	else
>   		ret = adc_tm5_init(adc_tm);
> @@ -638,8 +658,8 @@ static int adc_tm5_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>   
> -	return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_isr,
> -					 IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
> +	return devm_request_threaded_irq(dev, irq, NULL, adc_tm->data->isr,
> +			IRQF_ONESHOT, adc_tm->data->irq_name, adc_tm);
>   }
>   
>   static const struct of_device_id adc_tm5_match_table[] = {
> 


-- 
With best wishes
Dmitry

Re: [PATCH V3 4/4] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Dmitry Baryshkov]

On 23/11/2021 08:57, Jishnu Prakash wrote:
> Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a
> close counterpart of PMIC7 ADC and has the same functionality as
> PMIC5 ADC_TM, for threshold monitoring and interrupt generation.
> It is present on PMK8350 alone, like PMIC7 ADC and can be used
> to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
> having ADC on a target, through PBS(Programmable Boot Sequence).
> 
> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
> ---
>   drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 375 ++++++++++++++++++++++++++++++-
>   1 file changed, 372 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> index fc8cd45..a7b33a8 100644
> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> @@ -4,7 +4,10 @@
>    *
>    * Based on original driver:
>    * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
> + *
> + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved.
>    */
> +
>   #include <linux/bitfield.h>
>   #include <linux/iio/adc/qcom-vadc-common.h>
>   #include <linux/iio/consumer.h>
> @@ -71,6 +74,60 @@
>   #define ADC_TM5_M_HIGH_THR_INT_EN			BIT(1)
>   #define ADC_TM5_M_LOW_THR_INT_EN			BIT(0)
>   
> +#define ADC_TM_GEN2_STATUS1			0x08
> +#define ADC_TM_GEN2_STATUS_LOW_SET		0x09
> +#define ADC_TM_GEN2_STATUS_LOW_CLR		0x0a
> +#define ADC_TM_GEN2_STATUS_HIGH_SET		0x0b
> +#define ADC_TM_GEN2_STATUS_HIGH_CLR		0x0c
> +
> +#define ADC_TM_GEN2_CFG_HS_SET			0x0d
> +#define ADC_TM_GEN2_CFG_HS_FLAG			BIT(0)
> +#define ADC_TM_GEN2_CFG_HS_CLR			0x0e
> +
> +#define ADC_TM_GEN2_SID				0x40
> +
> +#define ADC_TM_GEN2_CH_CTL			0x41
> +#define ADC_TM_GEN2_TM_CH_SEL			GENMASK(7, 5)
> +#define ADC_TM_GEN2_MEAS_INT_SEL		GENMASK(3, 2)
> +
> +#define ADC_TM_GEN2_ADC_DIG_PARAM		0x42
> +#define ADC_TM_GEN2_CTL_CAL_SEL			GENMASK(5, 4)
> +#define ADC_TM_GEN2_CTL_DEC_RATIO_MASK		GENMASK(3, 2)
> +
> +#define ADC_TM_GEN2_FAST_AVG_CTL		0x43
> +#define ADC_TM_GEN2_FAST_AVG_EN			BIT(7)
> +
> +#define ADC_TM_GEN2_ADC_CH_SEL_CTL		0x44
> +
> +#define ADC_TM_GEN2_DELAY_CTL			0x45
> +#define ADC_TM_GEN2_HW_SETTLE_DELAY		GENMASK(3, 0)
> +
> +#define ADC_TM_GEN2_EN_CTL1			0x46
> +#define ADC_TM_GEN2_EN				BIT(7)
> +
> +#define ADC_TM_GEN2_CONV_REQ			0x47
> +#define ADC_TM_GEN2_CONV_REQ_EN			BIT(7)
> +
> +#define ADC_TM_GEN2_LOW_THR0			0x49
> +#define ADC_TM_GEN2_LOW_THR1			0x4a
> +#define ADC_TM_GEN2_HIGH_THR0			0x4b
> +#define ADC_TM_GEN2_HIGH_THR1			0x4c
> +#define ADC_TM_GEN2_LOWER_MASK(n)		((n) & GENMASK(7, 0))
> +#define ADC_TM_GEN2_UPPER_MASK(n)		(((n) & GENMASK(15, 8)) >> 8)
> +
> +#define ADC_TM_GEN2_MEAS_IRQ_EN			0x4d
> +#define ADC_TM_GEN2_MEAS_EN			BIT(7)
> +#define ADC_TM5_GEN2_HIGH_THR_INT_EN		BIT(1)
> +#define ADC_TM5_GEN2_LOW_THR_INT_EN		BIT(0)
> +
> +#define ADC_TM_GEN2_MEAS_INT_LSB		0x50
> +#define ADC_TM_GEN2_MEAS_INT_MSB		0x51
> +#define ADC_TM_GEN2_MEAS_INT_MODE		0x52
> +
> +#define ADC_TM_GEN2_Mn_DATA0(n)			((n * 2) + 0xa0)
> +#define ADC_TM_GEN2_Mn_DATA1(n)			((n * 2) + 0xa1)
> +#define ADC_TM_GEN2_DATA_SHIFT			8
> +
>   enum adc5_timer_select {
>   	ADC5_TIMER_SEL_1 = 0,
>   	ADC5_TIMER_SEL_2,
> @@ -81,6 +138,7 @@ enum adc5_timer_select {
>   enum adc5_gen {
>   	ADC_TM5,
>   	ADC_TM_HC,
> +	ADC_TM5_GEN2,
>   	ADC_TM5_MAX
>   };
>   
> @@ -90,6 +148,14 @@ enum adc_tm5_cal_method {
>   	ADC_TM5_ABSOLUTE_CAL
>   };
>   
> +enum adc_tm_gen2_time_select {
> +	MEAS_INT_50MS = 0,
> +	MEAS_INT_100MS,
> +	MEAS_INT_1S,
> +	MEAS_INT_SET,
> +	MEAS_INT_NONE,
> +};
> +
>   struct adc_tm5_chip;
>   struct adc_tm5_channel;
>   
> @@ -112,6 +178,12 @@ struct adc_tm5_data {
>    * @prescale: channel scaling performed on the input signal.
>    * @hw_settle_time: the time between AMUX being configured and the
>    *	start of conversion.
> + * @decimation: sampling rate supported for the channel.
> + * @avg_samples: ability to provide single result from the ADC
> + *	that is an average of multiple measurements.
> + * @high_thr_en: channel upper voltage threshold enable state.
> + * @low_thr_en: channel lower voltage threshold enable state.
> + * @meas_en: recurring measurement enable state
>    * @iio: IIO channel instance used by this channel.
>    * @chip: ADC TM chip instance.
>    * @tzd: thermal zone device used by this channel.
> @@ -122,6 +194,11 @@ struct adc_tm5_channel {
>   	enum adc_tm5_cal_method	cal_method;
>   	unsigned int		prescale;
>   	unsigned int		hw_settle_time;
> +	unsigned int		decimation;	/* For Gen2 ADC_TM */
> +	unsigned int		avg_samples;	/* For Gen2 ADC_TM */
> +	bool			high_thr_en;	/* For Gen2 ADC_TM */
> +	bool			low_thr_en;	/* For Gen2 ADC_TM */
> +	bool			meas_en;	/* For Gen2 ADC_TM */
>   	struct iio_channel	*iio;
>   	struct adc_tm5_chip	*chip;
>   	struct thermal_zone_device *tzd;
> @@ -135,9 +212,15 @@ struct adc_tm5_channel {
>    * @channels: array of ADC TM channel data.
>    * @nchannels: amount of channels defined/allocated
>    * @decimation: sampling rate supported for the channel.
> + *      Applies to all channels, used only on Gen1 ADC_TM.
>    * @avg_samples: ability to provide single result from the ADC
> - *	that is an average of multiple measurements.
> + *      that is an average of multiple measurements. Applies to all
> + *      channels, used only on Gen1 ADC_TM.
>    * @base: base address of TM registers.
> + * @adc_mutex_lock: ADC_TM mutex lock, used only on Gen2 ADC_TM.
> + *      It is used to ensure only one ADC channel configuration
> + *      is done at a time using the shared set of configuration
> + *      registers.
>    */
>   struct adc_tm5_chip {
>   	struct regmap		*regmap;
> @@ -148,6 +231,7 @@ struct adc_tm5_chip {
>   	unsigned int		decimation;
>   	unsigned int		avg_samples;
>   	u16			base;
> +	struct mutex		adc_mutex_lock;
>   };
>   
>   static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
> @@ -214,6 +298,61 @@ static irqreturn_t adc_tm5_isr(int irq, void *data)
>   	return IRQ_HANDLED;
>   }
>   
> +static irqreturn_t adc_tm5_gen2_isr(int irq, void *data)
> +{
> +	struct adc_tm5_chip *chip = data;
> +	u8 status_low, status_high;
> +	int ret, i;
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_STATUS_LOW_CLR, &status_low, sizeof(status_low));
> +	if (ret) {
> +		dev_err(chip->dev, "read status_low failed: %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_STATUS_HIGH_CLR, &status_high, sizeof(status_high));
> +	if (ret) {
> +		dev_err(chip->dev, "read status_high failed: %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_STATUS_LOW_CLR, &status_low, sizeof(status_low));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "clear status low failed with %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_STATUS_HIGH_CLR, &status_high, sizeof(status_high));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "clear status high failed with %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	for (i = 0; i < chip->nchannels; i++) {
> +		bool upper_set = false, lower_set = false;
> +		unsigned int ch = chip->channels[i].channel;
> +
> +		/* No TZD, we warned at the boot time */
> +		if (!chip->channels[i].tzd)
> +			continue;
> +
> +		if (!chip->channels[i].meas_en)
> +			continue;
> +
> +		lower_set = (status_low & BIT(ch)) &&
> +			(chip->channels[i].low_thr_en);
> +
> +		upper_set = (status_high & BIT(ch)) &&
> +			(chip->channels[i].high_thr_en);
> +
> +		if (upper_set || lower_set)
> +			thermal_zone_device_update(chip->channels[i].tzd,
> +						   THERMAL_EVENT_UNSPECIFIED);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
>   static int adc_tm5_get_temp(void *data, int *temp)
>   {
>   	struct adc_tm5_channel *channel = data;
> @@ -244,6 +383,104 @@ static int adc_tm5_disable_channel(struct adc_tm5_channel *channel)
>   				  0);
>   }
>   
> +#define ADC_TM_GEN2_POLL_DELAY_MIN_US		100
> +#define ADC_TM_GEN2_POLL_DELAY_MAX_US		110
> +#define ADC_TM_GEN2_POLL_RETRY_COUNT		3
> +
> +static int32_t adc_tm5_gen2_conv_req(struct adc_tm5_chip *chip)
> +{
> +	int ret;
> +	u8 data;
> +	unsigned int count;
> +
> +	data = ADC_TM_GEN2_EN;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_EN_CTL1, &data, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm enable failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	data = ADC_TM_GEN2_CFG_HS_FLAG;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_CFG_HS_SET, &data, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm handshake failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	data = ADC_TM_GEN2_CONV_REQ_EN;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_CONV_REQ, &data, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm request conversion failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * SW sets a handshake bit and waits for PBS to clear it
> +	 * before the next conversion request can be queued.
> +	 */
> +
> +	for (count = 0; count < ADC_TM_GEN2_POLL_RETRY_COUNT; count++) {
> +		ret = adc_tm5_read(chip, ADC_TM_GEN2_CFG_HS_SET, &data, sizeof(data));
> +		if (ret < 0) {
> +			dev_err(chip->dev, "adc-tm read failed with %d\n", ret);
> +			return ret;
> +		}
> +
> +		if (!(data & ADC_TM_GEN2_CFG_HS_FLAG))
> +			return ret;
> +		usleep_range(ADC_TM_GEN2_POLL_DELAY_MIN_US,
> +			ADC_TM_GEN2_POLL_DELAY_MAX_US);
> +	}
> +
> +	dev_err(chip->dev, "adc-tm conversion request handshake timed out\n");
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int adc_tm5_gen2_disable_channel(struct adc_tm5_channel *channel)
> +{
> +	struct adc_tm5_chip *chip = channel->chip;
> +	int ret;
> +	u8 val;
> +
> +	mutex_lock(&chip->adc_mutex_lock);
> +
> +	channel->meas_en = false;
> +	channel->high_thr_en = false;
> +	channel->low_thr_en = false;
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_CH_CTL, &val, sizeof(val));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm block read failed with %d\n", ret);
> +		goto disable_fail;
> +	}
> +
> +	val &= ~ADC_TM_GEN2_TM_CH_SEL;
> +	val |= FIELD_PREP(ADC_TM_GEN2_TM_CH_SEL, channel->channel);
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_CH_CTL, &val, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm channel disable failed with %d\n", ret);
> +		goto disable_fail;
> +	}
> +
> +	val = 0;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_MEAS_IRQ_EN, &val, 1);
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm interrupt disable failed with %d\n", ret);
> +		goto disable_fail;
> +	}
> +
> +
> +	ret = adc_tm5_gen2_conv_req(channel->chip);
> +	if (ret < 0)
> +		dev_err(chip->dev, "adc-tm channel configure failed with %d\n", ret);
> +
> +disable_fail:
> +	mutex_unlock(&chip->adc_mutex_lock);
> +	return ret;
> +}
> +
>   static int adc_tm5_enable(struct adc_tm5_chip *chip)
>   {
>   	int ret;
> @@ -324,6 +561,86 @@ static int adc_tm5_configure(struct adc_tm5_channel *channel, int low, int high)
>   	return adc_tm5_enable(chip);
>   }
>   
> +static int adc_tm5_gen2_configure(struct adc_tm5_channel *channel, int low, int high)
> +{
> +	struct adc_tm5_chip *chip = channel->chip;
> +	int ret;
> +	u8 buf[14];
> +	u16 adc_code;
> +
> +	mutex_lock(&chip->adc_mutex_lock);
> +
> +	channel->meas_en = true;
> +
> +	ret = adc_tm5_read(chip, ADC_TM_GEN2_SID, buf, sizeof(buf));
> +	if (ret < 0) {
> +		dev_err(chip->dev, "adc-tm block read failed with %d\n", ret);
> +		goto config_fail;
> +	}
> +
> +	/* Set SID from virtual channel number */
> +	buf[0] = channel->adc_channel >> 8;
> +
> +	/* Set TM channel number used and measurement interval */
> +	buf[1] &= ~ADC_TM_GEN2_TM_CH_SEL;
> +	buf[1] |= FIELD_PREP(ADC_TM_GEN2_TM_CH_SEL, channel->channel);
> +	buf[1] &= ~ADC_TM_GEN2_MEAS_INT_SEL;
> +	buf[1] |= FIELD_PREP(ADC_TM_GEN2_MEAS_INT_SEL, MEAS_INT_1S);
> +
> +	buf[2] &= ~ADC_TM_GEN2_CTL_DEC_RATIO_MASK;
> +	buf[2] |= FIELD_PREP(ADC_TM_GEN2_CTL_DEC_RATIO_MASK, channel->decimation);
> +	buf[2] &= ~ADC_TM_GEN2_CTL_CAL_SEL;
> +	buf[2] |= FIELD_PREP(ADC_TM_GEN2_CTL_CAL_SEL, channel->cal_method);
> +
> +	buf[3] = channel->avg_samples | ADC_TM_GEN2_FAST_AVG_EN;
> +
> +	buf[4] = channel->adc_channel & 0xff;
> +
> +	buf[5] = channel->hw_settle_time & ADC_TM_GEN2_HW_SETTLE_DELAY;
> +
> +	/* High temperature corresponds to low voltage threshold */
> +	if (high != INT_MAX) {
> +		channel->low_thr_en = true;
> +		adc_code = qcom_adc_tm5_gen2_temp_res_scale(high);
> +
> +		buf[9] = adc_code & 0xff;
> +		buf[10] = adc_code >> 8;
> +	} else {
> +		channel->low_thr_en = false;
> +	}
> +
> +	/* Low temperature corresponds to high voltage threshold */
> +	if (low != -INT_MAX) {
> +		channel->high_thr_en = true;
> +		adc_code = qcom_adc_tm5_gen2_temp_res_scale(low);
> +
> +		buf[11] = adc_code & 0xff;
> +		buf[12] = adc_code >> 8;
> +	} else {
> +		channel->high_thr_en = false;
> +	}
> +
> +	buf[13] = ADC_TM_GEN2_MEAS_EN;
> +	if (channel->high_thr_en)
> +		buf[13] |= ADC_TM5_GEN2_HIGH_THR_INT_EN;
> +	if (channel->low_thr_en)
> +		buf[13] |= ADC_TM5_GEN2_LOW_THR_INT_EN;
> +
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_SID, buf, sizeof(buf));
> +	if (ret) {
> +		dev_err(chip->dev, "channel %d params write failed: %d\n", channel->channel, ret);
> +		goto config_fail;
> +	}
> +
> +	ret = adc_tm5_gen2_conv_req(channel->chip);
> +	if (ret < 0)
> +		dev_err(chip->dev, "adc-tm channel configure failed with %d\n", ret);
> +
> +config_fail:
> +	mutex_unlock(&chip->adc_mutex_lock);
> +	return ret;
> +}
> +
>   static int adc_tm5_set_trips(void *data, int low, int high)
>   {
>   	struct adc_tm5_channel *channel = data;
> @@ -422,6 +739,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
>   		}
>   	}
>   
> +	mutex_init(&chip->adc_mutex_lock);

Minor issue. This way, the mutex is left uninitialized for ADC_TM5_HC 
devices. I'd move the mutex_init() call to the _probe function itself.

> +
> +	if (chip->data->gen == ADC_TM5_GEN2)
> +		return ret;
> +

Please do not do this. Create a separate adc_tm5_gen2_init function.
Add init() callback to adc_tm5_data structure.

>   	buf[0] = chip->decimation;
>   	buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
>   	buf[2] = ADC_TM5_TIMER1;
> @@ -442,7 +764,7 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>   				       struct device_node *node)
>   {
>   	const char *name = node->name;
> -	u32 chan, value, varr[2];
> +	u32 chan, value, adc_channel, varr[2];
>   	int ret;
>   	struct device *dev = adc_tm->dev;
>   	struct of_phandle_args args;
> @@ -472,7 +794,11 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>   	}
>   	of_node_put(args.np);
>   
> -	if (args.args_count != 1 || args.args[0] >= ADC5_MAX_CHANNEL) {
> +	adc_channel = args.args[0];
> +	if (adc_tm->data->gen == ADC_TM5_GEN2)
> +		adc_channel &= 0xff;
> +
> +	if (args.args_count != 1 || adc_channel >= ADC5_MAX_CHANNEL) {

Here you read the data (args.args[0]) before checking that it is 
actually available (args.args_count is not zero). Please correct the 
sequence.

>   		dev_err(dev, "%s: invalid ADC channel number %d\n", name, chan);
>   		return -EINVAL;
>   	}
> @@ -518,6 +844,32 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>   	else
>   		channel->cal_method = ADC_TM5_ABSOLUTE_CAL;
>   
> +	if (adc_tm->data->gen == ADC_TM5_GEN2) {
> +		ret = of_property_read_u32(node, "qcom,decimation", &value);
> +		if (!ret) {
> +			ret = qcom_adc5_decimation_from_dt(value, adc_tm->data->decimation);
> +			if (ret < 0) {
> +				dev_err(dev, "invalid decimation %d\n", value);
> +				return ret;
> +			}
> +			channel->decimation = ret;
> +		} else {
> +			channel->decimation = ADC5_DECIMATION_DEFAULT;
> +		}
> +
> +		ret = of_property_read_u32(node, "qcom,avg-samples", &value);
> +		if (!ret) {
> +			ret = qcom_adc5_avg_samples_from_dt(value);
> +			if (ret < 0) {
> +				dev_err(dev, "invalid avg-samples %d\n", value);
> +				return ret;
> +			}
> +			channel->avg_samples = ret;
> +		} else {
> +			channel->avg_samples = VADC_DEF_AVG_SAMPLES;
> +		}
> +	}
> +
>   	return 0;
>   }
>   
> @@ -546,6 +898,19 @@ static const struct adc_tm5_data adc_tm_hc_data_pmic = {
>   	.gen = ADC_TM_HC,
>   };
>   
> +static const struct adc_tm5_data adc_tm5_gen2_data_pmic = {
> +	.full_scale_code_volt = 0x70e4,
> +	.decimation = (unsigned int []) { 85, 340, 1360 },
> +	.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
> +					 1000, 2000, 4000, 8000, 16000, 32000,
> +					 64000, 128000 },
> +	.disable_channel = adc_tm5_gen2_disable_channel,
> +	.configure = adc_tm5_gen2_configure,
> +	.isr = adc_tm5_gen2_isr,
> +	.irq_name = "pm-adc-tm5-gen2",
> +	.gen = ADC_TM5_GEN2,
> +};
> +
>   static int adc_tm5_get_dt_data(struct adc_tm5_chip *adc_tm, struct device_node *node)
>   {
>   	struct adc_tm5_channel *channels;
> @@ -671,6 +1036,10 @@ static const struct of_device_id adc_tm5_match_table[] = {
>   		.compatible = "qcom,spmi-adc-tm-hc",
>   		.data = &adc_tm_hc_data_pmic,
>   	},
> +	{
> +		.compatible = "qcom,spmi-adc-tm5-gen2",
> +		.data = &adc_tm5_gen2_data_pmic,
> +	},
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, adc_tm5_match_table);
> 


-- 
With best wishes
Dmitry

Re: [PATCH V3 0/4] thermal: qcom: Add support for PMIC5 Gen2 ADC_TM

[Jishnu Prakash]

Hi Jonathan,

On 11/26/2021 11:59 PM, Jonathan Cameron wrote:
> On Tue, 23 Nov 2021 11:27:00 +0530
> Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
>
>> Made following changes in this post:
>> Addressed comments given by Jonathan for qcom-spmi-adc-tm5.yaml.
>> Addressed comments given by Dmitry and Jonathan for qcom-spmi-adc-tm5.c.
>> Split patch for qcom-spmi-adc-tm5.c into two parts, one to refactor
>> code to support multiple device generations and the second to add
>> actual Gen2 ADC_TM changes.
> Series is missing a change log.  Either in cover letter, or in
> individual patches after the --
>
> Jonathan


I'll add the change log in the cover letter in the next post.


>
>> Jishnu Prakash (4):
>>    dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings
>>    iio: adc: qcom-vadc-common: add reverse scaling for PMIC5 Gen2 ADC_TM
>>    thermal: qcom: Add support for multiple generations of devices
>>    thermal: qcom: add support for PMIC5 Gen2 ADCTM
>>
>>   .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 110 ++++-
>>   drivers/iio/adc/qcom-vadc-common.c                 |  11 +
>>   drivers/thermal/qcom/qcom-spmi-adc-tm5.c           | 451 +++++++++++++++++++--
>>   include/linux/iio/adc/qcom-vadc-common.h           |   2 +
>>   4 files changed, 541 insertions(+), 33 deletions(-)
>>
Thanks,

Jishnu

Re: [PATCH V3 4/4] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Jishnu Prakash]

Hi Jonathan,

On 11/27/2021 12:16 AM, Jonathan Cameron wrote:
> On Tue, 23 Nov 2021 11:27:04 +0530
> Jishnu Prakash <quic_jprakash@quicinc.com> wrote:
>
>> Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a
>> close counterpart of PMIC7 ADC and has the same functionality as
>> PMIC5 ADC_TM, for threshold monitoring and interrupt generation.
>> It is present on PMK8350 alone, like PMIC7 ADC and can be used
>> to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
>> having ADC on a target, through PBS(Programmable Boot Sequence).
>>
>> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
> Just one note on using put_unaligned_le16() below.  Otherwise, from
> a drive by review point of view it looks fine to someone not that
> familiar with the driver or thermal :)
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
>> ---
>>   drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 375 ++++++++++++++++++++++++++++++-
>>   1 file changed, 372 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
>> index fc8cd45..a7b33a8 100644
>> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
>> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
>> @@ -4,7 +4,10 @@
>>    *
>>    * Based on original driver:
>>    * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved.
>> + *
>> + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved.
>>    */

>> +
>> +	/* Low temperature corresponds to high voltage threshold */
>> +	if (low != -INT_MAX) {
>> +		channel->high_thr_en = true;
>> +		adc_code = qcom_adc_tm5_gen2_temp_res_scale(low);
>> +
>> +		buf[11] = adc_code & 0xff;
>> +		buf[12] = adc_code >> 8;
> looks like a little endian put though not necessarily aligned so
> put_unaligned_le16() preferred to open coding it. Same in similar places.
> Not my area though so maintainer may not care as much.


I'll use put_unaligned_le16 as suggested, in similar places in the next 
post.


>> +	} else {
>> +		channel->high_thr_en = false;
>> +	}
>> +
>> +	buf[13] = ADC_TM_GEN2_MEAS_EN;
>> +	if (channel->high_thr_en)
>> +		buf[13] |= ADC_TM5_GEN2_HIGH_THR_INT_EN;

Thanks,

Jishnu

Re: [PATCH V3 3/4] thermal: qcom: Add support for multiple generations of devices

[Jishnu Prakash]

Hi Dmitry,


On 11/29/2021 7:55 AM, Dmitry Baryshkov wrote:
> On 23/11/2021 08:57, Jishnu Prakash wrote:
>> Refactor code to support multiple generations of ADC_TM devices
>> by defining gen number, irq name and disable, configure and isr
>> APIs in the individual data structs.
>>
>> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Minor question below.
>
>> ---
>>   drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 76 
>> ++++++++++++++++++++------------
>>   1 file changed, 48 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c 
>> b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
>> index 824671c..fc8cd45 100644
>> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
>> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
>> @@ -78,11 +78,10 @@ enum adc5_timer_select {
>>       ADC5_TIMER_SEL_NONE,
>>   };
>>   -struct adc_tm5_data {
>> -    const u32    full_scale_code_volt;
>> -    unsigned int    *decimation;
>> -    unsigned int    *hw_settle;
>> -    bool        is_hc;
>> +enum adc5_gen {
>> +    ADC_TM5,
>> +    ADC_TM_HC,
>> +    ADC_TM5_MAX
>>   };
>>     enum adc_tm5_cal_method {
>> @@ -92,6 +91,18 @@ enum adc_tm5_cal_method {
>>   };
>>     struct adc_tm5_chip;
>> +struct adc_tm5_channel;
>> +
>> +struct adc_tm5_data {
>> +    const u32 full_scale_code_volt;
>> +    unsigned int *decimation;
>> +    unsigned int *hw_settle;
>> +    int (*disable_channel)(struct adc_tm5_channel *channel);
>> +    int (*configure)(struct adc_tm5_channel *channel, int low, int 
>> high);
>> +    irqreturn_t (*isr)(int irq, void *data);
>> +    char *irq_name;
>> +    int gen;
>> +};
>
> Any reason for moving the adc_tm5_data definition? It is still prefix 
> with the adc_tmp5_channel declaration.


There's no strong reason, I just thought it would look better to keep 
all the major struct definitions together after the enum definitions.


>
>>     /**
>>    * struct adc_tm5_channel - ADC Thermal Monitoring channel data.
>> @@ -139,22 +150,6 @@ struct adc_tm5_chip {
>>       u16            base;
>>   };
>>   -static const struct adc_tm5_data adc_tm5_data_pmic = {
>> -    .full_scale_code_volt = 0x70e4,
>> -    .decimation = (unsigned int []) { 250, 420, 840 },
>> -    .hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 
>> 600, 700,
>> -                     1000, 2000, 4000, 8000, 16000, 32000,
>> -                     64000, 128000 },
>> -};
>> -
>> -static const struct adc_tm5_data adc_tm_hc_data_pmic = {
>> -    .full_scale_code_volt = 0x70e4,
>> -    .decimation = (unsigned int []) { 256, 512, 1024 },
>> -    .hw_settle = (unsigned int []) { 0, 100, 200, 300, 400, 500, 
>> 600, 700,
>> -                     1000, 2000, 4000, 6000, 8000, 10000 },
>> -    .is_hc = true,
>> -};
>> -

Thanks,

Jishnu

Re: [PATCH V3 4/4] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Jishnu Prakash]

Hi Dmitry,

On 11/29/2021 8:02 AM, Dmitry Baryshkov wrote:
> On 23/11/2021 08:57, Jishnu Prakash wrote:
>> Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a
>> close counterpart of PMIC7 ADC and has the same functionality as
>> PMIC5 ADC_TM, for threshold monitoring and interrupt generation.
>> It is present on PMK8350 alone, like PMIC7 ADC and can be used
>> to monitor up to 8 ADC channels, from any of the PMIC7 PMICs
>> having ADC on a target, through PBS(Programmable Boot Sequence).
>>
>> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.com>
>> ---
>>   drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 375 
>> ++++++++++++++++++++++++++++++-
>>   1 file changed, 372 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c 
>> b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
>> index fc8cd45..a7b33a8 100644

>>   static int adc_tm5_set_trips(void *data, int low, int high)
>>   {
>>       struct adc_tm5_channel *channel = data;
>> @@ -422,6 +739,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
>>           }
>>       }
>>   +    mutex_init(&chip->adc_mutex_lock);
>
> Minor issue. This way, the mutex is left uninitialized for ADC_TM5_HC 
> devices. I'd move the mutex_init() call to the _probe function itself.


The mutex is needed only for Gen2 ADC_TM devices, I have mentioned this 
in the adc_tm5_chip struct description. I'll keep it in the Gen2 ADC_TM 
init function.


>
>> +
>> +    if (chip->data->gen == ADC_TM5_GEN2)
>> +        return ret;
>> +
>
> Please do not do this. Create a separate adc_tm5_gen2_init function.
> Add init() callback to adc_tm5_data structure.


Will make this change in next post.


>
>>       buf[0] = chip->decimation;
>>       buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
>>       buf[2] = ADC_TM5_TIMER1;
>> @@ -442,7 +764,7 @@ static int adc_tm5_get_dt_channel_data(struct 
>> adc_tm5_chip *adc_tm,
>>                          struct device_node *node)
>>   {
>>       const char *name = node->name;
>> -    u32 chan, value, varr[2];
>> +    u32 chan, value, adc_channel, varr[2];
>>       int ret;
>>       struct device *dev = adc_tm->dev;
>>       struct of_phandle_args args;
>> @@ -472,7 +794,11 @@ static int adc_tm5_get_dt_channel_data(struct 
>> adc_tm5_chip *adc_tm,
>>       }
>>       of_node_put(args.np);
>>   -    if (args.args_count != 1 || args.args[0] >= ADC5_MAX_CHANNEL) {
>> +    adc_channel = args.args[0];
>> +    if (adc_tm->data->gen == ADC_TM5_GEN2)
>> +        adc_channel &= 0xff;
>> +
>> +    if (args.args_count != 1 || adc_channel >= ADC5_MAX_CHANNEL) {
>
> Here you read the data (args.args[0]) before checking that it is 
> actually available (args.args_count is not zero). Please correct the 
> sequence.


Will correct this in next post.


>
>>           dev_err(dev, "%s: invalid ADC channel number %d\n", name, 
>> chan);
>>           return -EINVAL;
>>       }
>> @@ -518,6 +844,32 @@ static int adc_tm5_get_dt_channel_data(struct 
>> adc_tm5_chip *adc_tm,
>>       else
>>           channel->cal_method = ADC_TM5_ABSOLUTE_CAL;
>>   +    if (adc_tm->data->gen == ADC_TM5_GEN2) {
>> +        ret = of_property_read_u32(node, "qcom,decimation", &value);
>> +        if (!ret) {
>> +            ret = qcom_adc5_decimation_from_dt(value, 
>> adc_tm->data->decimation);
>> +            if (ret < 0) {
>> +                dev_err(dev, "invalid decimation %d\n", value);
>> +                return ret;

Thanks,

Jishnu