[PATCH V2 0/3] thermal: qcom: Add support for PMIC5 Gen2 ADC_TM

[PATCH V2 0/3] thermal: qcom: Add support for PMIC5 Gen2 ADC_TM

[Jishnu Prakash]

Made following changes in this post:
Split IIO file changes into separate patch.
Addressed comments given by Dmitry for qcom-spmi-adc-tm5.c.

Jishnu Prakash (3):
  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 PMIC5 Gen2 ADCTM

 .../bindings/thermal/qcom-spmi-adc-tm5.yaml        |  83 +++-
 drivers/iio/adc/qcom-vadc-common.c                 |  11 +
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c           | 418 ++++++++++++++++++++-
 include/linux/iio/adc/qcom-vadc-common.h           |   2 +
 4 files changed, 493 insertions(+), 21 deletions(-)

-- 
2.7.4

[PATCH V2 1/3] 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        | 83 +++++++++++++++++++++-
 1 file changed, 81 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..71a05a3 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
@@ -124,7 +151,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 +175,56 @@ 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,ratiometric;
+                qcom,hw-settle-time-us = <200>;
+            };
+
+            conn-therm@1 {
+                reg = <1>;
+                io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM4_100K_PU>;
+                qcom,ratiometric;
+                qcom,hw-settle-time-us = <200>;
+            };
+        };
+    };
 ...
-- 
2.7.4

[PATCH V2 2/3] 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>
---
 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 V2 3/3] 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 | 418 +++++++++++++++++++++++++++++--
 1 file changed, 399 insertions(+), 19 deletions(-)

diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
index 8494cc0..cdc4405 100644
--- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
+++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
@@ -71,6 +71,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,
@@ -78,10 +132,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;
+enum adc5_gen {
+	ADC_TM5,
+	ADC_TM5_GEN2,
+	ADC_TM5_MAX
 };
 
 enum adc_tm5_cal_method {
@@ -91,6 +145,19 @@ 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.
@@ -100,6 +167,12 @@ struct adc_tm5_chip;
  * @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.
@@ -110,6 +183,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;
@@ -123,9 +201,12 @@ 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.
  */
 struct adc_tm5_chip {
 	struct regmap		*regmap;
@@ -136,14 +217,15 @@ struct adc_tm5_chip {
 	unsigned int		decimation;
 	unsigned int		avg_samples;
 	u16			base;
+	struct mutex		adc_mutex_lock;
 };
 
-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 },
+enum adc_tm_gen2_time_select {
+	MEAS_INT_50MS = 0,
+	MEAS_INT_100MS,
+	MEAS_INT_1S,
+	MEAS_INT_SET,
+	MEAS_INT_NONE,
 };
 
 static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
@@ -210,6 +292,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;
@@ -240,6 +377,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 = 0;
+	u8 data = 0;
+	unsigned int count;
+
+	data = ADC_TM_GEN2_EN;
+	ret = adc_tm5_write(chip, ADC_TM_GEN2_EN_CTL1, &data, 1);
+	if (ret < 0) {
+		pr_err("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) {
+		pr_err("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) {
+		pr_err("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) {
+			pr_err("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);
+	}
+
+	pr_err("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) {
+		pr_err("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;
@@ -320,6 +555,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) {
+		pr_err("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;
@@ -334,14 +649,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,
 };
@@ -357,7 +672,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",
@@ -395,6 +710,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
 		}
 	}
 
+	if (chip-->data->gen == ADC_TM5_GEN2) {
+		mutex_init(&chip->adc_mutex_lock);
+		return ret;
+	}
+
 	buf[0] = chip->decimation;
 	buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
 	buf[2] = ADC_TM5_TIMER1;
@@ -415,7 +735,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;
@@ -445,7 +765,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;
 	}
@@ -491,9 +815,61 @@ 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;
 }
 
+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_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;
@@ -603,8 +979,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[] = {
@@ -612,6 +988,10 @@ static const struct of_device_id adc_tm5_match_table[] = {
 		.compatible = "qcom,spmi-adc-tm5",
 		.data = &adc_tm5_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 V2 3/3] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Dmitry Baryshkov]

On 26/10/2021 19:04, 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 | 418 +++++++++++++++++++++++++++++--
>   1 file changed, 399 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> index 8494cc0..cdc4405 100644
> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> @@ -71,6 +71,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,
> @@ -78,10 +132,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;
> +enum adc5_gen {
> +	ADC_TM5,
> +	ADC_TM5_GEN2,
> +	ADC_TM5_MAX
>   };
>   
>   enum adc_tm5_cal_method {
> @@ -91,6 +145,19 @@ 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.
> @@ -100,6 +167,12 @@ struct adc_tm5_chip;
>    * @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.
> @@ -110,6 +183,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;
> @@ -123,9 +201,12 @@ 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.

Please specify that it is used only for gen2 and that it keeps written 
and cached channel setup in sync (feel free to correct this description 
according to your understanding, I might be wrong here).

>    */
>   struct adc_tm5_chip {
>   	struct regmap		*regmap;
> @@ -136,14 +217,15 @@ struct adc_tm5_chip {
>   	unsigned int		decimation;
>   	unsigned int		avg_samples;
>   	u16			base;
> +	struct mutex		adc_mutex_lock;
>   };
>   
> -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 },
> +enum adc_tm_gen2_time_select {
> +	MEAS_INT_50MS = 0,
> +	MEAS_INT_100MS,
> +	MEAS_INT_1S,
> +	MEAS_INT_SET,
> +	MEAS_INT_NONE,
>   };

Move this enum to the top, closer to the rest of definitions.

>   
>   static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, u8 *data, int len)
> @@ -210,6 +292,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;
> @@ -240,6 +377,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 = 0;
> +	u8 data = 0;
> +	unsigned int count;
> +
> +	data = ADC_TM_GEN2_EN;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_EN_CTL1, &data, 1);
> +	if (ret < 0) {
> +		pr_err("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) {
> +		pr_err("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) {
> +		pr_err("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) {
> +			pr_err("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);
> +	}
> +
> +	pr_err("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) {
> +		pr_err("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;
> @@ -320,6 +555,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) {
> +		pr_err("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;
> @@ -334,14 +649,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,
>   };
> @@ -357,7 +672,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",
> @@ -395,6 +710,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
>   		}
>   	}
>   
> +	if (chip-->data->gen == ADC_TM5_GEN2) {
> +		mutex_init(&chip->adc_mutex_lock);
> +		return ret;
> +	}
> +

Just init the mutex always, there is no need to be so picky in the init 
code.

>   	buf[0] = chip->decimation;
>   	buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
>   	buf[2] = ADC_TM5_TIMER1;
> @@ -415,7 +735,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;
> @@ -445,7 +765,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;
>   	}
> @@ -491,9 +815,61 @@ 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;
>   }
>   
> +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_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;
> @@ -603,8 +979,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[] = {
> @@ -612,6 +988,10 @@ static const struct of_device_id adc_tm5_match_table[] = {
>   		.compatible = "qcom,spmi-adc-tm5",
>   		.data = &adc_tm5_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 V2 1/3] dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings

[Jonathan Cameron]

On Tue, 26 Oct 2021 21:34:33 +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>

Hi Jishnu,

A few comments inline.

Thanks,

Jonathan

> ---
>  .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 83 +++++++++++++++++++++-
>  1 file changed, 81 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..71a05a3 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.

Why not use an matching statement to set
qcom,avg_samples: false
for that compatible rather than relying on the fuzzy nature of a coment.


>      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
> @@ -124,7 +151,7 @@ examples:
>              #size-cells = <0>;
>              #io-channel-cells = <1>;
>  
> -            /* Other propreties are omitted */
> +            /* Other properties are omitted */

Should really be a separate patch, but up to Rob.


>              conn-therm@4f {
>                  reg = <ADC5_AMUX_THM3_100K_PU>;
>                  qcom,ratiometric;
> @@ -148,4 +175,56 @@ 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,ratiometric;
> +                qcom,hw-settle-time-us = <200>;

Perhaps include the new properties you are defining in the example?

> +            };
> +
> +            conn-therm@1 {
> +                reg = <1>;
> +                io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM4_100K_PU>;
> +                qcom,ratiometric;
> +                qcom,hw-settle-time-us = <200>;
> +            };
> +        };
> +    };
>  ...

Re: [PATCH V2 2/3] iio: adc: qcom-vadc-common: add reverse scaling for PMIC5 Gen2 ADC_TM

[Jonathan Cameron]

On Tue, 26 Oct 2021 21:34:34 +0530
Jishnu Prakash <quic_jprakash@quicinc.com> wrote:

> 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>
This little utility function looks fine to me.

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);

Re: [PATCH V2 3/3] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Jonathan Cameron]

On Tue, 26 Oct 2021 21:34:35 +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>

Drive be review as I was looking at the other bits of the series.
Personally I'd prefer to see this as 2 patches, 1st one does
the refactoring and 2nd adds the gen2 specific parts.

Jonathan


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

Indenting in here is inconsistent.
Personally I'd just stop bothering with the lining everything up.

> +	int (*disable_channel)(struct adc_tm5_channel *channel);
> +	int (*configure)(struct adc_tm5_channel *channel, int low,
> +					int high);

Why the line break?  It's under 80 chars without it.

> +	irqreturn_t (*isr)(int irq, void *data);
> +	char *irq_name;
> +	int gen;
> +};
>  
>  /**
>   * struct adc_tm5_channel - ADC Thermal Monitoring channel data.
> @@ -100,6 +167,12 @@ struct adc_tm5_chip;
>   * @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.
> @@ -110,6 +183,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;
> @@ -123,9 +201,12 @@ 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.

Not very informative.  What is it protecting access to?
Ah. I see this one has already been commented on by Dmitry

>   */
>  struct adc_tm5_chip {
>  	struct regmap		*regmap;
> @@ -136,14 +217,15 @@ struct adc_tm5_chip {
>  	unsigned int		decimation;
>  	unsigned int		avg_samples;
>  	u16			base;
> +	struct mutex		adc_mutex_lock;
>  };
>  

...

> +
> +static int32_t adc_tm5_gen2_conv_req(struct adc_tm5_chip *chip)
> +{
> +	int ret = 0;

No need to initialise as set in all paths.

> +	u8 data = 0;
> +	unsigned int count;
> +
> +	data = ADC_TM_GEN2_EN;
> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_EN_CTL1, &data, 1);
> +	if (ret < 0) {
> +		pr_err("adc-tm enable failed with %d\n", ret);

General mix of pr_err and dev_err.  Should all be dev_err unless
they aren't associated with a particular device for some reason.

> +		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) {
> +		pr_err("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) {
> +		pr_err("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) {
> +			pr_err("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);
> +	}
> +
> +	pr_err("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) {
> +		pr_err("adc-tm block read failed with %d\n", ret);

Why pr_err rather then dev_err?

> +		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 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,
This might all look nicer if split into two patches.
1st of which does the various structure moves and addes the callbacks for the
TM5.  2nd of which introduces the gen2 version of everything.

Patch 1 would be a noop which is usually easy to tell, and patch 2 would be
clean addition of new code.

> +	.isr = adc_tm5_isr,
> +	.irq_name = "pm-adc-tm5",
> +	.gen = ADC_TM5,
> +};
> +

Re: [PATCH V2 3/3] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Jishnu Prakash]

Hi Dmitry,

On 10/27/2021 8:04 AM, Dmitry Baryshkov wrote:
> On 26/10/2021 19:04, 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 | 418 
>> +++++++++++++++++++++++++++++--
>>   1 file changed, 399 insertions(+), 19 deletions(-)
>>
>>       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;
>> @@ -123,9 +201,12 @@ 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.
>
> Please specify that it is used only for gen2 and that it keeps written 
> and cached channel setup in sync (feel free to correct this 
> description according to your understanding, I might be wrong here).


I'll add to the description in the next post.


>>    */
>>   struct adc_tm5_chip {
>>       struct regmap        *regmap;
>> @@ -136,14 +217,15 @@ struct adc_tm5_chip {
>>       unsigned int        decimation;
>>       unsigned int        avg_samples;
>>       u16            base;
>> +    struct mutex        adc_mutex_lock;
>>   };
>>   -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 },
>> +enum adc_tm_gen2_time_select {
>> +    MEAS_INT_50MS = 0,
>> +    MEAS_INT_100MS,
>> +    MEAS_INT_1S,
>> +    MEAS_INT_SET,
>> +    MEAS_INT_NONE,
>>   };
>
> Move this enum to the top, closer to the rest of definitions.


Will move it in the next post


>
>>     static int adc_tm5_read(struct adc_tm5_chip *adc_tm, u16 offset, 
>> u8 *data, int len)
>> @@ -210,6 +292,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)
>> +{
>>           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",
>> @@ -395,6 +710,11 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
>>           }
>>       }
>>   +    if (chip-->data->gen == ADC_TM5_GEN2) {
>> +        mutex_init(&chip->adc_mutex_lock);
>> +        return ret;
>> +    }
>> +
>
> Just init the mutex always, there is no need to be so picky in the 
> init code.
Will fix it in the next post.
>
>>       buf[0] = chip->decimation;
>>       buf[1] = chip->avg_samples | ADC_TM5_FAST_AVG_EN;
>>       buf[2] = ADC_TM5_TIMER1;
>> @@ -415,7 +735,7 @@ static int adc_tm5_get_dt_channel_data(struct 
>> adc_tm5_chip *adc_tm,
>>                          struct device_node *node)
>>       { }
>>   };
>>   MODULE_DEVICE_TABLE(of, adc_tm5_match_table);
>>
Thanks,
Jishnu

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

[Jishnu Prakash]

Hi Jonathan,

On 10/27/2021 11:21 PM, Jonathan Cameron wrote:
> On Tue, 26 Oct 2021 21:34:33 +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>
> Hi Jishnu,
>
> A few comments inline.
>
> Thanks,
>
> Jonathan
>
>> ---
>>   .../bindings/thermal/qcom-spmi-adc-tm5.yaml        | 83 +++++++++++++++++++++-
>>   1 file changed, 81 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..71a05a3 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.
> Why not use an matching statement to set
> qcom,avg_samples: false
> for that compatible rather than relying on the fuzzy nature of a coment.


I'll add this condition in the next post.


>
>
>>       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
>> @@ -124,7 +151,7 @@ examples:
>>               #size-cells = <0>;
>>               #io-channel-cells = <1>;
>>   
>> -            /* Other propreties are omitted */
>> +            /* Other properties are omitted */
> Should really be a separate patch, but up to Rob.
>
>
>>               conn-therm@4f {
>>                   reg = <ADC5_AMUX_THM3_100K_PU>;
>>                   qcom,ratiometric;
>> @@ -148,4 +175,56 @@ 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,ratiometric;
>> +                qcom,hw-settle-time-us = <200>;
> Perhaps include the new properties you are defining in the example?


I'll add them in the next post.


>
>> +            };
>> +
>> +            conn-therm@1 {
>> +                reg = <1>;
>> +                io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM4_100K_PU>;
>> +                qcom,ratiometric;
>> +                qcom,hw-settle-time-us = <200>;
>> +            };
>> +        };
>> +    };
>>   ...

Thanks,

Jishnu

Re: [PATCH V2 3/3] thermal: qcom: add support for PMIC5 Gen2 ADCTM

[Jishnu Prakash]

Hi Jonathan,

On 10/27/2021 11:35 PM, Jonathan Cameron wrote:
> On Tue, 26 Oct 2021 21:34:35 +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>
> Drive be review as I was looking at the other bits of the series.
> Personally I'd prefer to see this as 2 patches, 1st one does
> the refactoring and 2nd adds the gen2 specific parts.
>
> Jonathan
>
>
>>   
>>   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;
> Indenting in here is inconsistent.
> Personally I'd just stop bothering with the lining everything up.
Will fix this in the next post.
>
>> +	int (*disable_channel)(struct adc_tm5_channel *channel);
>> +	int (*configure)(struct adc_tm5_channel *channel, int low,
>> +					int high);
> Why the line break?  It's under 80 chars without it.
Will fix this in the next post.
>
>> +	irqreturn_t (*isr)(int irq, void *data);
>> +	char *irq_name;
>> +	int gen;
>> +};
>>   
>>   /**
>>    * struct adc_tm5_channel - ADC Thermal Monitoring channel data.
>> @@ -100,6 +167,12 @@ struct adc_tm5_chip;
>>    * @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.
>> @@ -110,6 +183,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;
>> @@ -123,9 +201,12 @@ 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.
> Not very informative.  What is it protecting access to?
> Ah. I see this one has already been commented on by Dmitry
Will expand this desciption in the next post.
>
>>    */
>>   struct adc_tm5_chip {
>>   	struct regmap		*regmap;
>> @@ -136,14 +217,15 @@ struct adc_tm5_chip {
>>   	unsigned int		decimation;
>>   	unsigned int		avg_samples;
>>   	u16			base;
>> +	struct mutex		adc_mutex_lock;
>>   };
>>   
> ...
>
>> +
>> +static int32_t adc_tm5_gen2_conv_req(struct adc_tm5_chip *chip)
>> +{
>> +	int ret = 0;
> No need to initialise as set in all paths.
Will fix this in the next post.
>
>> +	u8 data = 0;
>> +	unsigned int count;
>> +
>> +	data = ADC_TM_GEN2_EN;
>> +	ret = adc_tm5_write(chip, ADC_TM_GEN2_EN_CTL1, &data, 1);
>> +	if (ret < 0) {
>> +		pr_err("adc-tm enable failed with %d\n", ret);
> General mix of pr_err and dev_err.  Should all be dev_err unless
> they aren't associated with a particular device for some reason.
I'll change them all to dev_err in the next post.
>
>> +		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) {
>> +		pr_err("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) {
>> +		pr_err("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) {
>> +			pr_err("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);
>> +	}
>> +
>> +	pr_err("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) {
>> +		pr_err("adc-tm block read failed with %d\n", ret);
> Why pr_err rather then dev_err?
Will fix this in the next post.
>
>> +		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 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,
> This might all look nicer if split into two patches.
> 1st of which does the various structure moves and addes the callbacks for the
> TM5.  2nd of which introduces the gen2 version of everything.
>
> Patch 1 would be a noop which is usually easy to tell, and patch 2 would be
> clean addition of new code.


Yes, I'll split this patch as you recommended in the next post.


>
>> +	.isr = adc_tm5_isr,
>> +	.irq_name = "pm-adc-tm5",
>> +	.gen = ADC_TM5,
>> +};
>> +


Thanks,

Jishnu