2021-10-18 07:19:55

by Jishnu Prakash

[permalink] [raw]
Subject: [PATCH 0/2] thermal: qcom: Add support for PMIC5 Gen2 ADC_TM

PMIC5 Gen2 ADC_TM is supported on PMIC7 chips and is a close
counterpart of PMIC7 ADC. It has the same functionality as
PMIC5 ADC_TM, to support generating interrupts on ADC value
crossing upper or lower thresholds for monitored channels.

Jishnu Prakash (2):
dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings
thermal: qcom: add support for PMIC5 Gen2 ADCTM

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

--
2.7.4


2021-10-18 07:20:53

by Jishnu Prakash

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: thermal: qcom: add PMIC5 Gen2 ADC_TM bindings

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 <[email protected]>
---
.../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

2021-10-18 07:22:32

by Jishnu Prakash

[permalink] [raw]
Subject: [PATCH 2/2] thermal: qcom: add support for PMIC5 Gen2 ADCTM

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 <[email protected]>
---
drivers/iio/adc/qcom-vadc-common.c | 187 ++++++++++++++
drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 431 ++++++++++++++++++++++++++++++-
include/linux/iio/adc/qcom-vadc-common.h | 2 +
3 files changed, 611 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/qcom-vadc-common.c b/drivers/iio/adc/qcom-vadc-common.c
index 1472389..e192736 100644
--- a/drivers/iio/adc/qcom-vadc-common.c
+++ b/drivers/iio/adc/qcom-vadc-common.c
@@ -100,6 +100,182 @@ static const struct vadc_map_pt adcmap_100k_104ef_104fb_1875_vref[] = {
{ 46, 125000 },
};

+/*
+ * Resistance to temperature table for NTCG104EF104 thermistor with
+ * 100k pull-up.
+ */
+static const struct vadc_map_pt adcmap_100k_adc7[] = {
+ { 4250657, -40960 },
+ { 3962085, -39936 },
+ { 3694875, -38912 },
+ { 3447322, -37888 },
+ { 3217867, -36864 },
+ { 3005082, -35840 },
+ { 2807660, -34816 },
+ { 2624405, -33792 },
+ { 2454218, -32768 },
+ { 2296094, -31744 },
+ { 2149108, -30720 },
+ { 2012414, -29696 },
+ { 1885232, -28672 },
+ { 1766846, -27648 },
+ { 1656598, -26624 },
+ { 1553884, -25600 },
+ { 1458147, -24576 },
+ { 1368873, -23552 },
+ { 1285590, -22528 },
+ { 1207863, -21504 },
+ { 1135290, -20480 },
+ { 1067501, -19456 },
+ { 1004155, -18432 },
+ { 944935, -17408 },
+ { 889550, -16384 },
+ { 837731, -15360 },
+ { 789229, -14336 },
+ { 743813, -13312 },
+ { 701271, -12288 },
+ { 661405, -11264 },
+ { 624032, -10240 },
+ { 588982, -9216 },
+ { 556100, -8192 },
+ { 525239, -7168 },
+ { 496264, -6144 },
+ { 469050, -5120 },
+ { 443480, -4096 },
+ { 419448, -3072 },
+ { 396851, -2048 },
+ { 375597, -1024 },
+ { 355598, 0 },
+ { 336775, 1024 },
+ { 319052, 2048 },
+ { 302359, 3072 },
+ { 286630, 4096 },
+ { 271806, 5120 },
+ { 257829, 6144 },
+ { 244646, 7168 },
+ { 232209, 8192 },
+ { 220471, 9216 },
+ { 209390, 10240 },
+ { 198926, 11264 },
+ { 189040, 12288 },
+ { 179698, 13312 },
+ { 170868, 14336 },
+ { 162519, 15360 },
+ { 154622, 16384 },
+ { 147150, 17408 },
+ { 140079, 18432 },
+ { 133385, 19456 },
+ { 127046, 20480 },
+ { 121042, 21504 },
+ { 115352, 22528 },
+ { 109960, 23552 },
+ { 104848, 24576 },
+ { 100000, 25600 },
+ { 95402, 26624 },
+ { 91038, 27648 },
+ { 86897, 28672 },
+ { 82965, 29696 },
+ { 79232, 30720 },
+ { 75686, 31744 },
+ { 72316, 32768 },
+ { 69114, 33792 },
+ { 66070, 34816 },
+ { 63176, 35840 },
+ { 60423, 36864 },
+ { 57804, 37888 },
+ { 55312, 38912 },
+ { 52940, 39936 },
+ { 50681, 40960 },
+ { 48531, 41984 },
+ { 46482, 43008 },
+ { 44530, 44032 },
+ { 42670, 45056 },
+ { 40897, 46080 },
+ { 39207, 47104 },
+ { 37595, 48128 },
+ { 36057, 49152 },
+ { 34590, 50176 },
+ { 33190, 51200 },
+ { 31853, 52224 },
+ { 30577, 53248 },
+ { 29358, 54272 },
+ { 28194, 55296 },
+ { 27082, 56320 },
+ { 26020, 57344 },
+ { 25004, 58368 },
+ { 24033, 59392 },
+ { 23104, 60416 },
+ { 22216, 61440 },
+ { 21367, 62464 },
+ { 20554, 63488 },
+ { 19776, 64512 },
+ { 19031, 65536 },
+ { 18318, 66560 },
+ { 17636, 67584 },
+ { 16982, 68608 },
+ { 16355, 69632 },
+ { 15755, 70656 },
+ { 15180, 71680 },
+ { 14628, 72704 },
+ { 14099, 73728 },
+ { 13592, 74752 },
+ { 13106, 75776 },
+ { 12640, 76800 },
+ { 12192, 77824 },
+ { 11762, 78848 },
+ { 11350, 79872 },
+ { 10954, 80896 },
+ { 10574, 81920 },
+ { 10209, 82944 },
+ { 9858, 83968 },
+ { 9521, 84992 },
+ { 9197, 86016 },
+ { 8886, 87040 },
+ { 8587, 88064 },
+ { 8299, 89088 },
+ { 8023, 90112 },
+ { 7757, 91136 },
+ { 7501, 92160 },
+ { 7254, 93184 },
+ { 7017, 94208 },
+ { 6789, 95232 },
+ { 6570, 96256 },
+ { 6358, 97280 },
+ { 6155, 98304 },
+ { 5959, 99328 },
+ { 5770, 100352 },
+ { 5588, 101376 },
+ { 5412, 102400 },
+ { 5243, 103424 },
+ { 5080, 104448 },
+ { 4923, 105472 },
+ { 4771, 106496 },
+ { 4625, 107520 },
+ { 4484, 108544 },
+ { 4348, 109568 },
+ { 4217, 110592 },
+ { 4090, 111616 },
+ { 3968, 112640 },
+ { 3850, 113664 },
+ { 3736, 114688 },
+ { 3626, 115712 },
+ { 3519, 116736 },
+ { 3417, 117760 },
+ { 3317, 118784 },
+ { 3221, 119808 },
+ { 3129, 120832 },
+ { 3039, 121856 },
+ { 2952, 122880 },
+ { 2868, 123904 },
+ { 2787, 124928 },
+ { 2709, 125952 },
+ { 2633, 126976 },
+ { 2560, 128000 },
+ { 2489, 129024 },
+ { 2420, 130048 }
+};
+
+
static const struct vadc_map_pt adcmap7_die_temp[] = {
{ 857300, 160000 },
{ 820100, 140000 },
@@ -677,6 +853,17 @@ u16 qcom_adc_tm5_temp_volt_scale(unsigned int prescale_ratio,
}
EXPORT_SYMBOL(qcom_adc_tm5_temp_volt_scale);

+u16 qcom_adc_tm5_temp_res_scale(int temp)
+{
+ int64_t resistance;
+
+ resistance = qcom_vadc_map_temp_voltage(adcmap_100k_adc7,
+ ARRAY_SIZE(adcmap_100k_adc7), temp);
+
+ return div64_s64((resistance * RATIO_MAX_ADC7), (resistance + R_PU_100K));
+}
+EXPORT_SYMBOL(qcom_adc_tm5_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/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
index 8494cc0..f344976f 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,
@@ -79,6 +133,7 @@ enum adc5_timer_select {
};

struct adc_tm5_data {
+ const struct adc_tm_ops *ops;
const u32 full_scale_code_volt;
unsigned int *decimation;
unsigned int *hw_settle;
@@ -100,6 +155,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 +171,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;
+ bool low_thr_en;
+ bool meas_en;
struct iio_channel *iio;
struct adc_tm5_chip *chip;
struct thermal_zone_device *tzd;
@@ -123,9 +189,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,9 +205,34 @@ struct adc_tm5_chip {
unsigned int decimation;
unsigned int avg_samples;
u16 base;
+ struct mutex adc_mutex_lock;
+};
+
+enum adc_tm_gen2_time_select {
+ MEAS_INT_50MS = 0,
+ MEAS_INT_100MS,
+ MEAS_INT_1S,
+ MEAS_INT_SET,
+ MEAS_INT_NONE,
+};
+
+struct adc_tm_ops {
+ int (*disable_channel)(struct adc_tm5_channel *channel);
+ int (*configure)(struct adc_tm5_channel *channel, int low,
+ int high);
+ int (*init)(struct adc_tm5_chip *chip);
};

+enum adc_tm_index {
+ ADC_TM5,
+ ADC_TM5_GEN2,
+ ADC_TM5_MAX
+};
+
+static const struct adc_tm_ops ops_adc_tm5[ADC_TM5_MAX];
+
static const struct adc_tm5_data adc_tm5_data_pmic = {
+ .ops = &ops_adc_tm5[ADC_TM5],
.full_scale_code_volt = 0x70e4,
.decimation = (unsigned int []) { 250, 420, 840 },
.hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
@@ -146,6 +240,15 @@ static const struct adc_tm5_data adc_tm5_data_pmic = {
64000, 128000 },
};

+static const struct adc_tm5_data adc_tm5_gen2_data_pmic = {
+ .ops = &ops_adc_tm5[ADC_TM5_GEN2],
+ .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 },
+};
+
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);
@@ -210,6 +313,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 +398,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 +576,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 measurment 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_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_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 +670,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->ops->disable_channel(channel);
else
- ret = adc_tm5_configure(channel, low, high);
+ ret = chip->data->ops->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 +693,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",
@@ -410,12 +746,37 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
return ret;
}

+static int adc_tm5_gen2_init(struct adc_tm5_chip *chip)
+{
+ u8 channels_available;
+ int ret;
+ unsigned int i;
+
+ ret = adc_tm5_read(chip, ADC_TM5_NUM_BTM,
+ &channels_available, sizeof(channels_available));
+ if (ret) {
+ dev_err(chip->dev, "read failed for BTM channels\n");
+ return ret;
+ }
+
+ mutex_init(&chip->adc_mutex_lock);
+
+ for (i = 0; i < chip->nchannels; i++) {
+ if (chip->channels[i].channel >= channels_available) {
+ dev_err(chip->dev, "Invalid channel %d\n", chip->channels[i].channel);
+ return -EINVAL;
+ }
+ }
+
+ return ret;
+}
+
static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
struct adc_tm5_channel *channel,
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 +806,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->ops == &ops_adc_tm5[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,6 +856,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->ops == &ops_adc_tm5[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;
}

@@ -591,7 +982,7 @@ static int adc_tm5_probe(struct platform_device *pdev)
return ret;
}

- ret = adc_tm5_init(adc_tm);
+ ret = adc_tm->data->ops->init(adc_tm);
if (ret) {
dev_err(dev, "adc-tm init failed\n");
return ret;
@@ -603,15 +994,37 @@ static int adc_tm5_probe(struct platform_device *pdev)
return ret;
}

+ if (adc_tm->data->ops == &ops_adc_tm5[ADC_TM5_GEN2])
+ return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_gen2_isr,
+ IRQF_ONESHOT, "pm-adc-tm5-gen2", adc_tm);
+
return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_isr,
- IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
+ IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
}

+static const struct adc_tm_ops ops_adc_tm5[ADC_TM5_MAX] = {
+ [ADC_TM5] = {
+ .disable_channel = adc_tm5_disable_channel,
+ .configure = adc_tm5_configure,
+ .init = adc_tm5_init,
+ },
+
+ [ADC_TM5_GEN2] = {
+ .disable_channel = adc_tm5_gen2_disable_channel,
+ .configure = adc_tm5_gen2_configure,
+ .init = adc_tm5_gen2_init,
+ },
+};
+
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);
diff --git a/include/linux/iio/adc/qcom-vadc-common.h b/include/linux/iio/adc/qcom-vadc-common.h
index 33f60f4..fa2eabe 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_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

2021-10-18 11:49:34

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 2/2] thermal: qcom: add support for PMIC5 Gen2 ADCTM

On Mon, 18 Oct 2021 at 10:18, Jishnu Prakash <[email protected]> wrote:
>
> Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a

It looks to me like the Gen2 suffix is a bit misleading. I haven't
checked the docs, but following the ADC example, I think we should
name it ADC_TM7.
Or maybe my original naming was wrong and we should use ADC5_TM and ADC7_TM.

> 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 <[email protected]>
> ---
> drivers/iio/adc/qcom-vadc-common.c | 187 ++++++++++++++
> drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 431 ++++++++++++++++++++++++++++++-
> include/linux/iio/adc/qcom-vadc-common.h | 2 +

Please split iio changes to the separate patch.

> 3 files changed, 611 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/qcom-vadc-common.c b/drivers/iio/adc/qcom-vadc-common.c
> index 1472389..e192736 100644
> --- a/drivers/iio/adc/qcom-vadc-common.c
> +++ b/drivers/iio/adc/qcom-vadc-common.c
> @@ -100,6 +100,182 @@ static const struct vadc_map_pt adcmap_100k_104ef_104fb_1875_vref[] = {
> { 46, 125000 },
> };
>
> +/*
> + * Resistance to temperature table for NTCG104EF104 thermistor with
> + * 100k pull-up.
> + */
> +static const struct vadc_map_pt adcmap_100k_adc7[] = {

There is the adcmap7_100k table, which looks identical. Do you need
this extra table?

> + { 4250657, -40960 },
> + { 3962085, -39936 },
> + { 3694875, -38912 },
> + { 3447322, -37888 },
> + { 3217867, -36864 },
> + { 3005082, -35840 },
> + { 2807660, -34816 },
> + { 2624405, -33792 },
> + { 2454218, -32768 },
> + { 2296094, -31744 },
> + { 2149108, -30720 },
> + { 2012414, -29696 },
> + { 1885232, -28672 },
> + { 1766846, -27648 },
> + { 1656598, -26624 },
> + { 1553884, -25600 },
> + { 1458147, -24576 },
> + { 1368873, -23552 },
> + { 1285590, -22528 },
> + { 1207863, -21504 },
> + { 1135290, -20480 },
> + { 1067501, -19456 },
> + { 1004155, -18432 },
> + { 944935, -17408 },
> + { 889550, -16384 },
> + { 837731, -15360 },
> + { 789229, -14336 },
> + { 743813, -13312 },
> + { 701271, -12288 },
> + { 661405, -11264 },
> + { 624032, -10240 },
> + { 588982, -9216 },
> + { 556100, -8192 },
> + { 525239, -7168 },
> + { 496264, -6144 },
> + { 469050, -5120 },
> + { 443480, -4096 },
> + { 419448, -3072 },
> + { 396851, -2048 },
> + { 375597, -1024 },
> + { 355598, 0 },
> + { 336775, 1024 },
> + { 319052, 2048 },
> + { 302359, 3072 },
> + { 286630, 4096 },
> + { 271806, 5120 },
> + { 257829, 6144 },
> + { 244646, 7168 },
> + { 232209, 8192 },
> + { 220471, 9216 },
> + { 209390, 10240 },
> + { 198926, 11264 },
> + { 189040, 12288 },
> + { 179698, 13312 },
> + { 170868, 14336 },
> + { 162519, 15360 },
> + { 154622, 16384 },
> + { 147150, 17408 },
> + { 140079, 18432 },
> + { 133385, 19456 },
> + { 127046, 20480 },
> + { 121042, 21504 },
> + { 115352, 22528 },
> + { 109960, 23552 },
> + { 104848, 24576 },
> + { 100000, 25600 },
> + { 95402, 26624 },
> + { 91038, 27648 },
> + { 86897, 28672 },
> + { 82965, 29696 },
> + { 79232, 30720 },
> + { 75686, 31744 },
> + { 72316, 32768 },
> + { 69114, 33792 },
> + { 66070, 34816 },
> + { 63176, 35840 },
> + { 60423, 36864 },
> + { 57804, 37888 },
> + { 55312, 38912 },
> + { 52940, 39936 },
> + { 50681, 40960 },
> + { 48531, 41984 },
> + { 46482, 43008 },
> + { 44530, 44032 },
> + { 42670, 45056 },
> + { 40897, 46080 },
> + { 39207, 47104 },
> + { 37595, 48128 },
> + { 36057, 49152 },
> + { 34590, 50176 },
> + { 33190, 51200 },
> + { 31853, 52224 },
> + { 30577, 53248 },
> + { 29358, 54272 },
> + { 28194, 55296 },
> + { 27082, 56320 },
> + { 26020, 57344 },
> + { 25004, 58368 },
> + { 24033, 59392 },
> + { 23104, 60416 },
> + { 22216, 61440 },
> + { 21367, 62464 },
> + { 20554, 63488 },
> + { 19776, 64512 },
> + { 19031, 65536 },
> + { 18318, 66560 },
> + { 17636, 67584 },
> + { 16982, 68608 },
> + { 16355, 69632 },
> + { 15755, 70656 },
> + { 15180, 71680 },
> + { 14628, 72704 },
> + { 14099, 73728 },
> + { 13592, 74752 },
> + { 13106, 75776 },
> + { 12640, 76800 },
> + { 12192, 77824 },
> + { 11762, 78848 },
> + { 11350, 79872 },
> + { 10954, 80896 },
> + { 10574, 81920 },
> + { 10209, 82944 },
> + { 9858, 83968 },
> + { 9521, 84992 },
> + { 9197, 86016 },
> + { 8886, 87040 },
> + { 8587, 88064 },
> + { 8299, 89088 },
> + { 8023, 90112 },
> + { 7757, 91136 },
> + { 7501, 92160 },
> + { 7254, 93184 },
> + { 7017, 94208 },
> + { 6789, 95232 },
> + { 6570, 96256 },
> + { 6358, 97280 },
> + { 6155, 98304 },
> + { 5959, 99328 },
> + { 5770, 100352 },
> + { 5588, 101376 },
> + { 5412, 102400 },
> + { 5243, 103424 },
> + { 5080, 104448 },
> + { 4923, 105472 },
> + { 4771, 106496 },
> + { 4625, 107520 },
> + { 4484, 108544 },
> + { 4348, 109568 },
> + { 4217, 110592 },
> + { 4090, 111616 },
> + { 3968, 112640 },
> + { 3850, 113664 },
> + { 3736, 114688 },
> + { 3626, 115712 },
> + { 3519, 116736 },
> + { 3417, 117760 },
> + { 3317, 118784 },
> + { 3221, 119808 },
> + { 3129, 120832 },
> + { 3039, 121856 },
> + { 2952, 122880 },
> + { 2868, 123904 },
> + { 2787, 124928 },
> + { 2709, 125952 },
> + { 2633, 126976 },
> + { 2560, 128000 },
> + { 2489, 129024 },
> + { 2420, 130048 }
> +};
> +
> +

Extra newline

> static const struct vadc_map_pt adcmap7_die_temp[] = {
> { 857300, 160000 },
> { 820100, 140000 },
> @@ -677,6 +853,17 @@ u16 qcom_adc_tm5_temp_volt_scale(unsigned int prescale_ratio,
> }
> EXPORT_SYMBOL(qcom_adc_tm5_temp_volt_scale);
>
> +u16 qcom_adc_tm5_temp_res_scale(int temp)

This is definitely not an qcom_adc_tm5-kind of function. What about
qcom_adc_tm7_temp_res_scale() ?

> +{
> + int64_t resistance;
> +
> + resistance = qcom_vadc_map_temp_voltage(adcmap_100k_adc7,
> + ARRAY_SIZE(adcmap_100k_adc7), temp);
> +
> + return div64_s64((resistance * RATIO_MAX_ADC7), (resistance + R_PU_100K));

Internal brackets are not needed.

> +}
> +EXPORT_SYMBOL(qcom_adc_tm5_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/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> index 8494cc0..f344976f 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,
> @@ -79,6 +133,7 @@ enum adc5_timer_select {
> };
>
> struct adc_tm5_data {
> + const struct adc_tm_ops *ops;
> const u32 full_scale_code_volt;
> unsigned int *decimation;
> unsigned int *hw_settle;
> @@ -100,6 +155,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 +171,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;
> + bool low_thr_en;
> + bool meas_en;

Is there any reason for using the data here rather than reading it
back from the register?
And if there is one, adc_tm5 code should be converted to use them too.

> struct iio_channel *iio;
> struct adc_tm5_chip *chip;
> struct thermal_zone_device *tzd;
> @@ -123,9 +189,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,9 +205,34 @@ struct adc_tm5_chip {
> unsigned int decimation;
> unsigned int avg_samples;
> u16 base;
> + struct mutex adc_mutex_lock;
> +};
> +
> +enum adc_tm_gen2_time_select {
> + MEAS_INT_50MS = 0,
> + MEAS_INT_100MS,
> + MEAS_INT_1S,
> + MEAS_INT_SET,
> + MEAS_INT_NONE,
> +};
> +
> +struct adc_tm_ops {
> + int (*disable_channel)(struct adc_tm5_channel *channel);
> + int (*configure)(struct adc_tm5_channel *channel, int low,
> + int high);
> + int (*init)(struct adc_tm5_chip *chip);
> };
>
> +enum adc_tm_index {
> + ADC_TM5,
> + ADC_TM5_GEN2,
> + ADC_TM5_MAX
> +};
> +
> +static const struct adc_tm_ops ops_adc_tm5[ADC_TM5_MAX];

There is no need for this extra array, please use ops directly. And if
you introduce flags into the data, you won't have to forward declare
the ops here.

> +
> static const struct adc_tm5_data adc_tm5_data_pmic = {
> + .ops = &ops_adc_tm5[ADC_TM5],
> .full_scale_code_volt = 0x70e4,
> .decimation = (unsigned int []) { 250, 420, 840 },
> .hw_settle = (unsigned int []) { 15, 100, 200, 300, 400, 500, 600, 700,
> @@ -146,6 +240,15 @@ static const struct adc_tm5_data adc_tm5_data_pmic = {
> 64000, 128000 },
> };
>
> +static const struct adc_tm5_data adc_tm5_gen2_data_pmic = {
> + .ops = &ops_adc_tm5[ADC_TM5_GEN2],
> + .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 },
> +};
> +
> 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);
> @@ -210,6 +313,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;

Any reason for having the meas_en here rather than reading the MEAS_EN
bit like the ADC_TM5 code does?

> +
> + lower_set = (status_low & BIT(ch)) &&
> + (chip->channels[i].low_thr_en);
> +
> + upper_set = (status_high & BIT(ch)) &&
> + (chip->channels[i].high_thr_en);

And the high_thr_en/low_thr_en too.

> +
> + 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 +398,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.
> + */

Should this section be protected by the mutex then, so that any other
possible user won't queue the conversion?

> +
> + 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 +576,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);

Either introduce mutex into the adc_tm5 code too, or drop it here.

> +
> + 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 measurment interval */

measurement

> + 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_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_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 +670,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->ops->disable_channel(channel);
> else
> - ret = adc_tm5_configure(channel, low, high);
> + ret = chip->data->ops->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 +693,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",
> @@ -410,12 +746,37 @@ static int adc_tm5_init(struct adc_tm5_chip *chip)
> return ret;
> }
>
> +static int adc_tm5_gen2_init(struct adc_tm5_chip *chip)
> +{
> + u8 channels_available;
> + int ret;
> + unsigned int i;
> +
> + ret = adc_tm5_read(chip, ADC_TM5_NUM_BTM,
> + &channels_available, sizeof(channels_available));
> + if (ret) {
> + dev_err(chip->dev, "read failed for BTM channels\n");
> + return ret;
> + }
> +
> + mutex_init(&chip->adc_mutex_lock);
> +
> + for (i = 0; i < chip->nchannels; i++) {
> + if (chip->channels[i].channel >= channels_available) {
> + dev_err(chip->dev, "Invalid channel %d\n", chip->channels[i].channel);
> + return -EINVAL;
> + }
> + }
> +
> + return ret;

This is the adc_tm5_init(), but with the ADC_DIG_PARAM bits left off.
So it might be better to split the adc_tm5_init() rather than
introducing the duplicate.

> +}
> +
> static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
> struct adc_tm5_channel *channel,
> 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 +806,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) {

This does not apply either against current linux master or against
linux-next master.

> + adc_channel = args.args[0];
> + if (adc_tm->data->ops == &ops_adc_tm5[ADC_TM5_GEN2])

Please replace ops comparison with flags.

> + 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,6 +856,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->ops == &ops_adc_tm5[ADC_TM5_GEN2]) {

And here too, please replace it with flags.

> + 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;
> }
>
> @@ -591,7 +982,7 @@ static int adc_tm5_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = adc_tm5_init(adc_tm);
> + ret = adc_tm->data->ops->init(adc_tm);
> if (ret) {
> dev_err(dev, "adc-tm init failed\n");
> return ret;
> @@ -603,15 +994,37 @@ static int adc_tm5_probe(struct platform_device *pdev)
> return ret;
> }
>
> + if (adc_tm->data->ops == &ops_adc_tm5[ADC_TM5_GEN2])
> + return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_gen2_isr,
> + IRQF_ONESHOT, "pm-adc-tm5-gen2", adc_tm);
> +
> return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_isr,
> - IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
> + IRQF_ONESHOT, "pm-adc-tm5", adc_tm);

Push ISR into TM data.

> }
>
> +static const struct adc_tm_ops ops_adc_tm5[ADC_TM5_MAX] = {
> + [ADC_TM5] = {
> + .disable_channel = adc_tm5_disable_channel,
> + .configure = adc_tm5_configure,
> + .init = adc_tm5_init,
> + },
> +
> + [ADC_TM5_GEN2] = {
> + .disable_channel = adc_tm5_gen2_disable_channel,
> + .configure = adc_tm5_gen2_configure,
> + .init = adc_tm5_gen2_init,
> + },
> +};
> +
> 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);
> diff --git a/include/linux/iio/adc/qcom-vadc-common.h b/include/linux/iio/adc/qcom-vadc-common.h
> index 33f60f4..fa2eabe 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_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
>


--
With best wishes
Dmitry

2021-10-25 12:11:47

by Jishnu Prakash

[permalink] [raw]
Subject: Re: [PATCH 2/2] thermal: qcom: add support for PMIC5 Gen2 ADCTM

Hi Dmitry,

On 10/18/2021 5:16 PM, Dmitry Baryshkov wrote:
> On Mon, 18 Oct 2021 at 10:18, Jishnu Prakash <[email protected]> wrote:
>> Add support for PMIC5 Gen2 ADC_TM, used on PMIC7 chips. It is a
> It looks to me like the Gen2 suffix is a bit misleading. I haven't
> checked the docs, but following the ADC example, I think we should
> name it ADC_TM7.
> Or maybe my original naming was wrong and we should use ADC5_TM and ADC7_TM.

The name used for this ADC version was amended internally, it should be
called PMIC5 Gen2 ADC.

Keeping future ADC versions in mind, it would be better to call this
peripheral PMIC5 Gen2 ADC_TM.

>
>> 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 <[email protected]>
>> ---
>> drivers/iio/adc/qcom-vadc-common.c | 187 ++++++++++++++
>> drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 431 ++++++++++++++++++++++++++++++-
>> include/linux/iio/adc/qcom-vadc-common.h | 2 +
> Please split iio changes to the separate patch.
I'll split them in the next post.
>
>> 3 files changed, 611 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/adc/qcom-vadc-common.c b/drivers/iio/adc/qcom-vadc-common.c
>> index 1472389..e192736 100644
>> --- a/drivers/iio/adc/qcom-vadc-common.c
>> +++ b/drivers/iio/adc/qcom-vadc-common.c
>> @@ -100,6 +100,182 @@ static const struct vadc_map_pt adcmap_100k_104ef_104fb_1875_vref[] = {
>> { 46, 125000 },
>> };
>>
>> +/*
>> + * Resistance to temperature table for NTCG104EF104 thermistor with
>> + * 100k pull-up.
>> + */
>> +static const struct vadc_map_pt adcmap_100k_adc7[] = {
> There is the adcmap7_100k table, which looks identical. Do you need
> this extra table?

You're right, I'll remove it in the next post.

>
>> + { 4250657, -40960 },
>> + { 3962085, -39936 },
>> + { 3694875, -38912 },
>> + { 3447322, -37888 },
>> + { 3217867, -36864 },
>> + { 3005082, -35840 },

>> + { 2633, 126976 },
>> + { 2560, 128000 },
>> + { 2489, 129024 },
>> + { 2420, 130048 }
>> +};
>> +
>> +
> Extra newline
Will remove this.
>
>> static const struct vadc_map_pt adcmap7_die_temp[] = {
>> { 857300, 160000 },
>> { 820100, 140000 },
>> @@ -677,6 +853,17 @@ u16 qcom_adc_tm5_temp_volt_scale(unsigned int prescale_ratio,
>> }
>> EXPORT_SYMBOL(qcom_adc_tm5_temp_volt_scale);
>>
>> +u16 qcom_adc_tm5_temp_res_scale(int temp)
> This is definitely not an qcom_adc_tm5-kind of function. What about
> qcom_adc_tm7_temp_res_scale() ?


I'll rename it using adc_tm5_gen2 in the next post.

>> +{
>> + int64_t resistance;
>> +
>> + resistance = qcom_vadc_map_temp_voltage(adcmap_100k_adc7,
>> + ARRAY_SIZE(adcmap_100k_adc7), temp);
>> +
>> + return div64_s64((resistance * RATIO_MAX_ADC7), (resistance + R_PU_100K));
> Internal brackets are not needed.
Will fix it in the next post.
>
>> struct adc_tm5_data {
>> + const struct adc_tm_ops *ops;
>> const u32 full_scale_code_volt;
>> unsigned int *decimation;
>> unsigned int *hw_settle;
>> @@ -100,6 +155,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 +171,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;
>> + bool low_thr_en;
>> + bool meas_en;
> Is there any reason for using the data here rather than reading it
> back from the register?
> And if there is one, adc_tm5 code should be converted to use them too.
That is one major difference between PMIC5 and PMIC5 Gen2 ADC_TM. In
PMIC5, there was a separate set of
configuration registers for each ADC_TM channel, but in Gen2 ADC_TM,
there is only one common set of
registers which can be used to configure any one of the 8 channels at a
time. Reading back these
values for any one of the channels is not possible, the registers can
only be used to write a
configuration which is read and applied by PBS on the actual ADC HW to
monitor the channel,
as ADC_TM functionality is implemented completely within PBS. Therefore
the channel data has to be
stored in SW.
>> struct iio_channel *iio;
>> struct adc_tm5_chip *chip;
>> struct thermal_zone_device *tzd;
>> @@ -123,9 +189,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.

>>
>> +enum adc_tm_index {
>> + ADC_TM5,
>> + ADC_TM5_GEN2,
>> + ADC_TM5_MAX
>> +};
>> +
>> +static const struct adc_tm_ops ops_adc_tm5[ADC_TM5_MAX];
> There is no need for this extra array, please use ops directly. And if
> you introduce flags into the data, you won't have to forward declare
> the ops here.
Yes, I'll use the existing adc_tm5_data directly for these in the next
post.
>> +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;
> Any reason for having the meas_en here rather than reading the MEAS_EN
> bit like the ADC_TM5 code does?


As mentioned in the earlier comment above, the channel status cannot be
read back from

registers for Gen2 ADC_TM, it needs to be stored in SW.

>
>> +
>> + lower_set = (status_low & BIT(ch)) &&
>> + (chip->channels[i].low_thr_en);
>> +
>> + upper_set = (status_high & BIT(ch)) &&
>> + (chip->channels[i].high_thr_en);
> And the high_thr_en/low_thr_en too.
Same as above comment.
>
>> +
>> + 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 +398,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.
>> + */
> Should this section be protected by the mutex then, so that any other
> possible user won't queue the conversion?


This API is only called within the mutex-protected region, in two other
places in this file.

>
>> +
>> + 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_enable(struct adc_tm5_chip *chip)
>> {
>> int ret;
>> @@ -320,6 +576,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);
> Either introduce mutex into the adc_tm5 code too, or drop it here.
I believe the mutex lock is not really needed for PMIC5 ADC_TM
configuration because there we have one separate set of registers per TM
channel, so only
one channel (and its set of registers) would be configured at a time. It
could be a problem here if there are multiple thermal zones mapped to same
ADC_TM channel, but I understand the thermal framework supports only one
thermal zone for one registered sensor, so this should not be not a problem.
For PMIC5 Gen2 ADC_TM, we need a mutex, as it is a common set of
registers shared between all TM channels, which could be a problem if
two different
ADC_TM thermal zones are configured simultaneously.

Considering the above points, which do you think is better, to add the
mutex for ADC_TM5 too or not ?

>
>> +
>> + 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 measurment interval */
> measurement
Will fix it in the next post.

> +static int adc_tm5_gen2_init(struct adc_tm5_chip *chip)
> +{
> + u8 channels_available;
> + int ret;
> + unsigned int i;
> +
> + ret = adc_tm5_read(chip, ADC_TM5_NUM_BTM,
> + &channels_available, sizeof(channels_available));
> + if (ret) {
> + dev_err(chip->dev, "read failed for BTM channels\n");
> + return ret;
> + }
> +
> + mutex_init(&chip->adc_mutex_lock);
> +
> + for (i = 0; i < chip->nchannels; i++) {
> + if (chip->channels[i].channel >= channels_available) {
> + dev_err(chip->dev, "Invalid channel %d\n", chip->channels[i].channel);
> + return -EINVAL;
> + }
> + }
> +
> + return ret;
> This is the adc_tm5_init(), but with the ADC_DIG_PARAM bits left off.
> So it might be better to split the adc_tm5_init() rather than
> introducing the duplicate.
Will do it in the next post.
>
>> +}
>> +
>> static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
>> struct adc_tm5_channel *channel,
>> 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 +806,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) {
> This does not apply either against current linux master or against
> linux-next master.
I'm not sure what you mean here, can you please explain some more? Which
part exactly needs to change?
>
>> + adc_channel = args.args[0];
>> + if (adc_tm->data->ops == &ops_adc_tm5[ADC_TM5_GEN2])
> Please replace ops comparison with flags.
Will do it in the next post.
>
>> + 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,6 +856,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->ops == &ops_adc_tm5[ADC_TM5_GEN2]) {
> And here too, please replace it with flags.
Will do it in the next post.
>
>

>> @@ -591,7 +982,7 @@ static int adc_tm5_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> - ret = adc_tm5_init(adc_tm);
>> + ret = adc_tm->data->ops->init(adc_tm);
>> if (ret) {
>> dev_err(dev, "adc-tm init failed\n");
>> return ret;
>> @@ -603,15 +994,37 @@ static int adc_tm5_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> + if (adc_tm->data->ops == &ops_adc_tm5[ADC_TM5_GEN2])
>> + return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_gen2_isr,
>> + IRQF_ONESHOT, "pm-adc-tm5-gen2", adc_tm);
>> +
>> return devm_request_threaded_irq(dev, irq, NULL, adc_tm5_isr,
>> - IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
>> + IRQF_ONESHOT, "pm-adc-tm5", adc_tm);
> Push ISR into TM data.
Will do it in the next post.
>
>> }
>>
Thanks,

Jishnu