[2/6] thermal/drivers/tsens: Add TSENS enable and calibration support for V2

Message ID 20230710103735.1375847-3-quic_ipkumar@quicinc.com
State New
Headers
Series Add IPQ5332 TSENS support |

Commit Message

Praveenkumar I July 10, 2023, 10:37 a.m. UTC
  SoCs without RPM have to enable sensors and calibrate from the kernel.
Though TSENS IP supports 16 sensors, not all are used. So added
sensors_to_en in tsens data help enable the relevant sensors.

Added new calibration function for V2 as the tsens.c calib function
only supports V1.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 drivers/thermal/qcom/tsens-v2.c | 116 ++++++++++++++++++++++++++++++++
 drivers/thermal/qcom/tsens.c    |  37 +++++++++-
 drivers/thermal/qcom/tsens.h    |  56 +++++++++++++++
 3 files changed, 208 insertions(+), 1 deletion(-)
  

Comments

Dmitry Baryshkov July 10, 2023, 11:19 a.m. UTC | #1
On 10/07/2023 13:37, Praveenkumar I wrote:
> SoCs without RPM have to enable sensors and calibrate from the kernel.
> Though TSENS IP supports 16 sensors, not all are used. So added
> sensors_to_en in tsens data help enable the relevant sensors.
> 
> Added new calibration function for V2 as the tsens.c calib function
> only supports V1.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>   drivers/thermal/qcom/tsens-v2.c | 116 ++++++++++++++++++++++++++++++++
>   drivers/thermal/qcom/tsens.c    |  37 +++++++++-
>   drivers/thermal/qcom/tsens.h    |  56 +++++++++++++++
>   3 files changed, 208 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index 29a61d2d6ca3..db48b1d95348 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -6,11 +6,20 @@
>   
>   #include <linux/bitops.h>
>   #include <linux/regmap.h>
> +#include <linux/nvmem-consumer.h>
>   #include "tsens.h"
>   
>   /* ----- SROT ------ */
>   #define SROT_HW_VER_OFF	0x0000
>   #define SROT_CTRL_OFF		0x0004
> +#define SROT_MEASURE_PERIOD	0x0008
> +#define SROT_Sn_CONVERSION	0x0060
> +#define V2_SHIFT_DEFAULT	0x0003
> +#define V2_SLOPE_DEFAULT	0x0cd0
> +#define V2_CZERO_DEFAULT	0x016a
> +#define ONE_PT_SLOPE		0x0cd0
> +#define TWO_PT_SHIFTED_GAIN	921600
> +#define ONE_PT_CZERO_CONST	94
>   
>   /* ----- TM ------ */
>   #define TM_INT_EN_OFF			0x0004
> @@ -59,6 +68,16 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
>   	/* CTRL_OFF */
>   	[TSENS_EN]     = REG_FIELD(SROT_CTRL_OFF,    0,  0),
>   	[TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF,    1,  1),
> +	[SENSOR_EN]    = REG_FIELD(SROT_CTRL_OFF,    3,  18),
> +	[CODE_OR_TEMP] = REG_FIELD(SROT_CTRL_OFF,    21, 21),
> +
> +	/* MAIN_MEASURE_PERIOD */
> +	[MAIN_MEASURE_PERIOD] = REG_FIELD(SROT_MEASURE_PERIOD, 0, 7),
> +
> +	/* Sn Conversion */
> +	REG_FIELD_FOR_EACH_SENSOR16(SHIFT, SROT_Sn_CONVERSION, 23, 24),
> +	REG_FIELD_FOR_EACH_SENSOR16(SLOPE, SROT_Sn_CONVERSION, 10, 22),
> +	REG_FIELD_FOR_EACH_SENSOR16(CZERO, SROT_Sn_CONVERSION, 0, 9),
>   
>   	/* ----- TM ------ */
>   	/* INTERRUPT ENABLE */
> @@ -104,6 +123,103 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
>   	[TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>   };
>   
> +static int tsens_v2_calibration(struct tsens_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	u32 mode, base0, base1;
> +	u32 slope, czero;
> +	char name[15];
> +	int i, j, ret;
> +
> +	if (priv->num_sensors > MAX_SENSORS)
> +		return -EINVAL;
> +
> +	ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
> +	if (ret == -ENOENT)
> +		dev_warn(priv->dev, "Calibration data not present in DT\n");
> +	if (ret < 0)
> +		return ret;
> +
> +	dev_dbg(priv->dev, "calibration mode is %d\n", mode);
> +
> +	ret = nvmem_cell_read_variable_le_u32(priv->dev, "base0", &base0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Read offset values and allocate SHIFT, SLOPE & CZERO regmap for enabled sensors */
> +	for (i = 0; i < priv->num_sensors; i++) {
> +		if (!(priv->sensors_to_en & (0x1 << i)))
> +			continue;
> +
> +		ret = snprintf(name, sizeof(name), "s%d_offset", priv->sensor[i].hw_id);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &priv->sensor[i].offset);
> +		if (ret)
> +			return ret;
> +
> +		for (j = SHIFT_0; j <= CZERO_0; j++) {
> +			int idx = (i * 3) + j;
> +
> +			priv->rf[idx] = devm_regmap_field_alloc(dev, priv->srot_map,
> +								priv->fields[idx]);
> +			if (IS_ERR(priv->rf[idx]))
> +				return PTR_ERR(priv->rf[idx]);

I think, allocating data structures for 48 regfields, which are written 
just once, to be an overkill.

> +		}
> +	}
> +
> +	/* Based on calib mode, program SHIFT, SLOPE and CZERO for enabled sensors */
> +	switch (mode) {
> +	case TWO_PT_CALIB:
> +		slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
> +
> +		for (i = 0; i < priv->num_sensors; i++) {
> +			if (!(priv->sensors_to_en & (0x1 << i)))
> +				continue;
> +
> +			int idx = i * 3;
> +
> +			czero = (base0 + priv->sensor[i].offset - ((base1 - base0) / 3));
> +			regmap_field_write(priv->rf[SHIFT_0 + idx], V2_SHIFT_DEFAULT);
> +			regmap_field_write(priv->rf[SLOPE_0 + idx], slope);
> +			regmap_field_write(priv->rf[CZERO_0 + idx], czero);
> +		}
> +		fallthrough;
> +	case ONE_PT_CALIB2:
> +		for (i = 0; i < priv->num_sensors; i++) {
> +			if (!(priv->sensors_to_en & (0x1 << i)))
> +				continue;
> +
> +			int idx = i * 3;
> +
> +			czero = base0 + priv->sensor[i].offset - ONE_PT_CZERO_CONST;
> +			regmap_field_write(priv->rf[SHIFT_0 + idx], V2_SHIFT_DEFAULT);
> +			regmap_field_write(priv->rf[SLOPE_0 + idx], ONE_PT_SLOPE);
> +			regmap_field_write(priv->rf[CZERO_0 + idx], czero);
> +		}
> +		break;
> +	default:
> +		dev_dbg(priv->dev, "calibrationless mode\n");
> +		for (i = 0; i < priv->num_sensors; i++) {
> +			if (!(priv->sensors_to_en & (0x1 << i)))
> +				continue;
> +
> +			int idx = i * 3;
> +
> +			regmap_field_write(priv->rf[SHIFT_0 + idx], V2_SHIFT_DEFAULT);
> +			regmap_field_write(priv->rf[SLOPE_0 + idx], V2_SLOPE_DEFAULT);
> +			regmap_field_write(priv->rf[CZERO_0 + idx], V2_CZERO_DEFAULT);
> +		}
> +	}

This code iterates over the sensors field several times. Please consider 
extracting a function that handles all setup for a single sensor, then 
calling it in a loop (I should probably do the same for tsens-v0/v1 too).

> +
> +	return 0;
> +}
> +
>   static const struct tsens_ops ops_generic_v2 = {
>   	.init		= init_common,
>   	.get_temp	= get_temp_tsens_valid,
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 98c356acfe98..169690355dad 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -974,7 +974,7 @@ int __init init_common(struct tsens_priv *priv)
>   	ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
>   	if (ret)
>   		goto err_put_device;
> -	if (!enabled) {
> +	if (!enabled && !priv->sensors_to_en) {
>   		dev_err(dev, "%s: device not enabled\n", __func__);
>   		ret = -ENODEV;
>   		goto err_put_device;
> @@ -1006,6 +1006,40 @@ int __init init_common(struct tsens_priv *priv)
>   		goto err_put_device;
>   	}
>   
> +	/* Do TSENS initialization if required */
> +	if (priv->sensors_to_en) {

Maybe it would be better to explicitly add VER_2_X_NO_RPM and check it here?

> +		priv->rf[CODE_OR_TEMP] = devm_regmap_field_alloc(dev, priv->srot_map,
> +								 priv->fields[CODE_OR_TEMP]);
> +		if (IS_ERR(priv->rf[CODE_OR_TEMP])) {
> +			ret = PTR_ERR(priv->rf[CODE_OR_TEMP]);
> +			goto err_put_device;
> +		}
> +
> +		priv->rf[MAIN_MEASURE_PERIOD] =
> +			devm_regmap_field_alloc(dev, priv->srot_map,
> +						priv->fields[MAIN_MEASURE_PERIOD]);
> +		if (IS_ERR(priv->rf[MAIN_MEASURE_PERIOD])) {
> +			ret = PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
> +			goto err_put_device;
> +		}
> +
> +		regmap_field_write(priv->rf[TSENS_SW_RST], 0x1);
> +
> +		/* Update measure period to 2ms */
> +		regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], 0x1);
> +
> +		/* Enable available sensors */
> +		regmap_field_write(priv->rf[SENSOR_EN], priv->sensors_to_en);
> +
> +		/* Real temperature format */
> +		regmap_field_write(priv->rf[CODE_OR_TEMP], 0x1);
> +
> +		regmap_field_write(priv->rf[TSENS_SW_RST], 0x0);
> +
> +		/* Enable TSENS */
> +		regmap_field_write(priv->rf[TSENS_EN], 0x1);
> +	}
> +
>   	/* This loop might need changes if enum regfield_ids is reordered */
>   	for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
>   		for (i = 0; i < priv->feat->max_sensors; i++) {
> @@ -1282,6 +1316,7 @@ static int tsens_probe(struct platform_device *pdev)
>   
>   	priv->dev = dev;
>   	priv->num_sensors = num_sensors;
> +	priv->sensors_to_en = data->sensors_to_en;
>   	priv->ops = data->ops;
>   	for (i = 0;  i < priv->num_sensors; i++) {
>   		if (data->hw_ids)
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 2805de1c6827..f8897bc8944e 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -168,6 +168,58 @@ enum regfield_ids {
>   	TSENS_SW_RST,
>   	SENSOR_EN,
>   	CODE_OR_TEMP,
> +	/* MEASURE_PERIOD */
> +	MAIN_MEASURE_PERIOD,
> +
> +	/* Sn_CONVERSION */
> +	SHIFT_0,
> +	SLOPE_0,
> +	CZERO_0,
> +	SHIFT_1,
> +	SLOPE_1,
> +	CZERO_1,
> +	SHIFT_2,
> +	SLOPE_2,
> +	CZERO_2,
> +	SHIFT_3,
> +	SLOPE_3,
> +	CZERO_3,
> +	SHIFT_4,
> +	SLOPE_4,
> +	CZERO_4,
> +	SHIFT_5,
> +	SLOPE_5,
> +	CZERO_5,
> +	SHIFT_6,
> +	SLOPE_6,
> +	CZERO_6,
> +	SHIFT_7,
> +	SLOPE_7,
> +	CZERO_7,
> +	SHIFT_8,
> +	SLOPE_8,
> +	CZERO_8,
> +	SHIFT_9,
> +	SLOPE_9,
> +	CZERO_9,
> +	SHIFT_10,
> +	SLOPE_10,
> +	CZERO_10,
> +	SHIFT_11,
> +	SLOPE_11,
> +	CZERO_11,
> +	SHIFT_12,
> +	SLOPE_12,
> +	CZERO_12,
> +	SHIFT_13,
> +	SLOPE_13,
> +	CZERO_13,
> +	SHIFT_14,
> +	SLOPE_14,
> +	CZERO_14,
> +	SHIFT_15,
> +	SLOPE_15,
> +	CZERO_15,
>   
>   	/* ----- TM ------ */
>   	/* TRDY */
> @@ -524,6 +576,7 @@ struct tsens_features {
>   /**
>    * struct tsens_plat_data - tsens compile-time platform data
>    * @num_sensors: Number of sensors supported by platform
> + * @sensors_to_en: Sensors to be enabled. Each bit represent a sensor
>    * @ops: operations the tsens instance supports
>    * @hw_ids: Subset of sensors ids supported by platform, if not the first n
>    * @feat: features of the IP
> @@ -531,6 +584,7 @@ struct tsens_features {
>    */
>   struct tsens_plat_data {
>   	const u32		num_sensors;
> +	const u16		sensors_to_en;

There is already a similar field, hw_ids. Can it be used instead?

>   	const struct tsens_ops	*ops;
>   	unsigned int		*hw_ids;
>   	struct tsens_features	*feat;
> @@ -551,6 +605,7 @@ struct tsens_context {
>    * struct tsens_priv - private data for each instance of the tsens IP
>    * @dev: pointer to struct device
>    * @num_sensors: number of sensors enabled on this device
> + * @sensors_to_en: sensors to be enabled. Each bit represents a sensor
>    * @tm_map: pointer to TM register address space
>    * @srot_map: pointer to SROT register address space
>    * @tm_offset: deal with old device trees that don't address TM and SROT
> @@ -569,6 +624,7 @@ struct tsens_context {
>   struct tsens_priv {
>   	struct device			*dev;
>   	u32				num_sensors;
> +	u16				sensors_to_en;
>   	struct regmap			*tm_map;
>   	struct regmap			*srot_map;
>   	u32				tm_offset;
  
Dmitry Baryshkov July 10, 2023, 11:22 a.m. UTC | #2
On 10/07/2023 14:19, Dmitry Baryshkov wrote:
> On 10/07/2023 13:37, Praveenkumar I wrote:
>> SoCs without RPM have to enable sensors and calibrate from the kernel.
>> Though TSENS IP supports 16 sensors, not all are used. So added
>> sensors_to_en in tsens data help enable the relevant sensors.
>>
>> Added new calibration function for V2 as the tsens.c calib function
>> only supports V1.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>   drivers/thermal/qcom/tsens-v2.c | 116 ++++++++++++++++++++++++++++++++
>>   drivers/thermal/qcom/tsens.c    |  37 +++++++++-
>>   drivers/thermal/qcom/tsens.h    |  56 +++++++++++++++
>>   3 files changed, 208 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/qcom/tsens-v2.c 
>> b/drivers/thermal/qcom/tsens-v2.c
>> index 29a61d2d6ca3..db48b1d95348 100644
>> --- a/drivers/thermal/qcom/tsens-v2.c
>> +++ b/drivers/thermal/qcom/tsens-v2.c
>> @@ -6,11 +6,20 @@
>>   #include <linux/bitops.h>
>>   #include <linux/regmap.h>
>> +#include <linux/nvmem-consumer.h>
>>   #include "tsens.h"
>>   /* ----- SROT ------ */
>>   #define SROT_HW_VER_OFF    0x0000
>>   #define SROT_CTRL_OFF        0x0004
>> +#define SROT_MEASURE_PERIOD    0x0008
>> +#define SROT_Sn_CONVERSION    0x0060
>> +#define V2_SHIFT_DEFAULT    0x0003
>> +#define V2_SLOPE_DEFAULT    0x0cd0
>> +#define V2_CZERO_DEFAULT    0x016a
>> +#define ONE_PT_SLOPE        0x0cd0
>> +#define TWO_PT_SHIFTED_GAIN    921600
>> +#define ONE_PT_CZERO_CONST    94
>>   /* ----- TM ------ */
>>   #define TM_INT_EN_OFF            0x0004
>> @@ -59,6 +68,16 @@ static const struct reg_field 
>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>       /* CTRL_OFF */
>>       [TSENS_EN]     = REG_FIELD(SROT_CTRL_OFF,    0,  0),
>>       [TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF,    1,  1),
>> +    [SENSOR_EN]    = REG_FIELD(SROT_CTRL_OFF,    3,  18),
>> +    [CODE_OR_TEMP] = REG_FIELD(SROT_CTRL_OFF,    21, 21),
>> +
>> +    /* MAIN_MEASURE_PERIOD */
>> +    [MAIN_MEASURE_PERIOD] = REG_FIELD(SROT_MEASURE_PERIOD, 0, 7),
>> +
>> +    /* Sn Conversion */
>> +    REG_FIELD_FOR_EACH_SENSOR16(SHIFT, SROT_Sn_CONVERSION, 23, 24),
>> +    REG_FIELD_FOR_EACH_SENSOR16(SLOPE, SROT_Sn_CONVERSION, 10, 22),
>> +    REG_FIELD_FOR_EACH_SENSOR16(CZERO, SROT_Sn_CONVERSION, 0, 9),
>>       /* ----- TM ------ */
>>       /* INTERRUPT ENABLE */
>> @@ -104,6 +123,103 @@ static const struct reg_field 
>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>       [TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>>   };
>> +static int tsens_v2_calibration(struct tsens_priv *priv)
>> +{
>> +    struct device *dev = priv->dev;
>> +    u32 mode, base0, base1;
>> +    u32 slope, czero;
>> +    char name[15];
>> +    int i, j, ret;
>> +
>> +    if (priv->num_sensors > MAX_SENSORS)
>> +        return -EINVAL;
>> +
>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
>> +    if (ret == -ENOENT)
>> +        dev_warn(priv->dev, "Calibration data not present in DT\n");
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    dev_dbg(priv->dev, "calibration mode is %d\n", mode);
>> +
>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base0", &base0);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    /* Read offset values and allocate SHIFT, SLOPE & CZERO regmap 
>> for enabled sensors */
>> +    for (i = 0; i < priv->num_sensors; i++) {
>> +        if (!(priv->sensors_to_en & (0x1 << i)))
>> +            continue;
>> +
>> +        ret = snprintf(name, sizeof(name), "s%d_offset", 
>> priv->sensor[i].hw_id);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        ret = nvmem_cell_read_variable_le_u32(priv->dev, name, 
>> &priv->sensor[i].offset);
>> +        if (ret)
>> +            return ret;
>> +
>> +        for (j = SHIFT_0; j <= CZERO_0; j++) {
>> +            int idx = (i * 3) + j;
>> +
>> +            priv->rf[idx] = devm_regmap_field_alloc(dev, priv->srot_map,
>> +                                priv->fields[idx]);
>> +            if (IS_ERR(priv->rf[idx]))
>> +                return PTR_ERR(priv->rf[idx]);
> 
> I think, allocating data structures for 48 regfields, which are written 
> just once, to be an overkill.
> 
>> +        }
>> +    }
>> +
>> +    /* Based on calib mode, program SHIFT, SLOPE and CZERO for 
>> enabled sensors */
>> +    switch (mode) {
>> +    case TWO_PT_CALIB:
>> +        slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
>> +
>> +        for (i = 0; i < priv->num_sensors; i++) {
>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>> +                continue;
>> +
>> +            int idx = i * 3;
>> +
>> +            czero = (base0 + priv->sensor[i].offset - ((base1 - 
>> base0) / 3));
>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>> V2_SHIFT_DEFAULT);
>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], slope);
>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>> +        }
>> +        fallthrough;
>> +    case ONE_PT_CALIB2:
>> +        for (i = 0; i < priv->num_sensors; i++) {
>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>> +                continue;
>> +
>> +            int idx = i * 3;
>> +
>> +            czero = base0 + priv->sensor[i].offset - ONE_PT_CZERO_CONST;
>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>> V2_SHIFT_DEFAULT);
>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], ONE_PT_SLOPE);
>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>> +        }
>> +        break;
>> +    default:
>> +        dev_dbg(priv->dev, "calibrationless mode\n");
>> +        for (i = 0; i < priv->num_sensors; i++) {
>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>> +                continue;
>> +
>> +            int idx = i * 3;
>> +
>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>> V2_SHIFT_DEFAULT);
>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], 
>> V2_SLOPE_DEFAULT);
>> +            regmap_field_write(priv->rf[CZERO_0 + idx], 
>> V2_CZERO_DEFAULT);
>> +        }
>> +    }
> 
> This code iterates over the sensors field several times. Please consider 
> extracting a function that handles all setup for a single sensor, then 
> calling it in a loop (I should probably do the same for tsens-v0/v1 too).
> 
>> +
>> +    return 0;
>> +}
>> +
>>   static const struct tsens_ops ops_generic_v2 = {
>>       .init        = init_common,
>>       .get_temp    = get_temp_tsens_valid,
>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
>> index 98c356acfe98..169690355dad 100644
>> --- a/drivers/thermal/qcom/tsens.c
>> +++ b/drivers/thermal/qcom/tsens.c
>> @@ -974,7 +974,7 @@ int __init init_common(struct tsens_priv *priv)
>>       ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
>>       if (ret)
>>           goto err_put_device;
>> -    if (!enabled) {
>> +    if (!enabled && !priv->sensors_to_en) {
>>           dev_err(dev, "%s: device not enabled\n", __func__);
>>           ret = -ENODEV;
>>           goto err_put_device;
>> @@ -1006,6 +1006,40 @@ int __init init_common(struct tsens_priv *priv)
>>           goto err_put_device;
>>       }
>> +    /* Do TSENS initialization if required */
>> +    if (priv->sensors_to_en) {
> 
> Maybe it would be better to explicitly add VER_2_X_NO_RPM and check it 
> here?

After taking a look at the patch 6, can we just add a separate init() 
function which will call init_common() and then initialize these fields?

> 
>> +        priv->rf[CODE_OR_TEMP] = devm_regmap_field_alloc(dev, 
>> priv->srot_map,
>> +                                 priv->fields[CODE_OR_TEMP]);
>> +        if (IS_ERR(priv->rf[CODE_OR_TEMP])) {
>> +            ret = PTR_ERR(priv->rf[CODE_OR_TEMP]);
>> +            goto err_put_device;
>> +        }
>> +
>> +        priv->rf[MAIN_MEASURE_PERIOD] =
>> +            devm_regmap_field_alloc(dev, priv->srot_map,
>> +                        priv->fields[MAIN_MEASURE_PERIOD]);
>> +        if (IS_ERR(priv->rf[MAIN_MEASURE_PERIOD])) {
>> +            ret = PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
>> +            goto err_put_device;
>> +        }
>> +
>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x1);
>> +
>> +        /* Update measure period to 2ms */
>> +        regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], 0x1);
>> +
>> +        /* Enable available sensors */
>> +        regmap_field_write(priv->rf[SENSOR_EN], priv->sensors_to_en);
>> +
>> +        /* Real temperature format */
>> +        regmap_field_write(priv->rf[CODE_OR_TEMP], 0x1);
>> +
>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x0);
>> +
>> +        /* Enable TSENS */
>> +        regmap_field_write(priv->rf[TSENS_EN], 0x1);
>> +    }
>> +
>>       /* This loop might need changes if enum regfield_ids is 
>> reordered */
>>       for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
>>           for (i = 0; i < priv->feat->max_sensors; i++) {
>> @@ -1282,6 +1316,7 @@ static int tsens_probe(struct platform_device 
>> *pdev)
>>       priv->dev = dev;
>>       priv->num_sensors = num_sensors;
>> +    priv->sensors_to_en = data->sensors_to_en;
>>       priv->ops = data->ops;
>>       for (i = 0;  i < priv->num_sensors; i++) {
>>           if (data->hw_ids)
>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
>> index 2805de1c6827..f8897bc8944e 100644
>> --- a/drivers/thermal/qcom/tsens.h
>> +++ b/drivers/thermal/qcom/tsens.h
>> @@ -168,6 +168,58 @@ enum regfield_ids {
>>       TSENS_SW_RST,
>>       SENSOR_EN,
>>       CODE_OR_TEMP,
>> +    /* MEASURE_PERIOD */
>> +    MAIN_MEASURE_PERIOD,
>> +
>> +    /* Sn_CONVERSION */
>> +    SHIFT_0,
>> +    SLOPE_0,
>> +    CZERO_0,
>> +    SHIFT_1,
>> +    SLOPE_1,
>> +    CZERO_1,
>> +    SHIFT_2,
>> +    SLOPE_2,
>> +    CZERO_2,
>> +    SHIFT_3,
>> +    SLOPE_3,
>> +    CZERO_3,
>> +    SHIFT_4,
>> +    SLOPE_4,
>> +    CZERO_4,
>> +    SHIFT_5,
>> +    SLOPE_5,
>> +    CZERO_5,
>> +    SHIFT_6,
>> +    SLOPE_6,
>> +    CZERO_6,
>> +    SHIFT_7,
>> +    SLOPE_7,
>> +    CZERO_7,
>> +    SHIFT_8,
>> +    SLOPE_8,
>> +    CZERO_8,
>> +    SHIFT_9,
>> +    SLOPE_9,
>> +    CZERO_9,
>> +    SHIFT_10,
>> +    SLOPE_10,
>> +    CZERO_10,
>> +    SHIFT_11,
>> +    SLOPE_11,
>> +    CZERO_11,
>> +    SHIFT_12,
>> +    SLOPE_12,
>> +    CZERO_12,
>> +    SHIFT_13,
>> +    SLOPE_13,
>> +    CZERO_13,
>> +    SHIFT_14,
>> +    SLOPE_14,
>> +    CZERO_14,
>> +    SHIFT_15,
>> +    SLOPE_15,
>> +    CZERO_15,
>>       /* ----- TM ------ */
>>       /* TRDY */
>> @@ -524,6 +576,7 @@ struct tsens_features {
>>   /**
>>    * struct tsens_plat_data - tsens compile-time platform data
>>    * @num_sensors: Number of sensors supported by platform
>> + * @sensors_to_en: Sensors to be enabled. Each bit represent a sensor
>>    * @ops: operations the tsens instance supports
>>    * @hw_ids: Subset of sensors ids supported by platform, if not the 
>> first n
>>    * @feat: features of the IP
>> @@ -531,6 +584,7 @@ struct tsens_features {
>>    */
>>   struct tsens_plat_data {
>>       const u32        num_sensors;
>> +    const u16        sensors_to_en;
> 
> There is already a similar field, hw_ids. Can it be used instead?
> 
>>       const struct tsens_ops    *ops;
>>       unsigned int        *hw_ids;
>>       struct tsens_features    *feat;
>> @@ -551,6 +605,7 @@ struct tsens_context {
>>    * struct tsens_priv - private data for each instance of the tsens IP
>>    * @dev: pointer to struct device
>>    * @num_sensors: number of sensors enabled on this device
>> + * @sensors_to_en: sensors to be enabled. Each bit represents a sensor
>>    * @tm_map: pointer to TM register address space
>>    * @srot_map: pointer to SROT register address space
>>    * @tm_offset: deal with old device trees that don't address TM and 
>> SROT
>> @@ -569,6 +624,7 @@ struct tsens_context {
>>   struct tsens_priv {
>>       struct device            *dev;
>>       u32                num_sensors;
>> +    u16                sensors_to_en;
>>       struct regmap            *tm_map;
>>       struct regmap            *srot_map;
>>       u32                tm_offset;
>
  
Praveenkumar I July 10, 2023, 1:22 p.m. UTC | #3
On 7/10/2023 4:49 PM, Dmitry Baryshkov wrote:
> On 10/07/2023 13:37, Praveenkumar I wrote:
>> SoCs without RPM have to enable sensors and calibrate from the kernel.
>> Though TSENS IP supports 16 sensors, not all are used. So added
>> sensors_to_en in tsens data help enable the relevant sensors.
>>
>> Added new calibration function for V2 as the tsens.c calib function
>> only supports V1.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>   drivers/thermal/qcom/tsens-v2.c | 116 ++++++++++++++++++++++++++++++++
>>   drivers/thermal/qcom/tsens.c    |  37 +++++++++-
>>   drivers/thermal/qcom/tsens.h    |  56 +++++++++++++++
>>   3 files changed, 208 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/qcom/tsens-v2.c 
>> b/drivers/thermal/qcom/tsens-v2.c
>> index 29a61d2d6ca3..db48b1d95348 100644
>> --- a/drivers/thermal/qcom/tsens-v2.c
>> +++ b/drivers/thermal/qcom/tsens-v2.c
>> @@ -6,11 +6,20 @@
>>     #include <linux/bitops.h>
>>   #include <linux/regmap.h>
>> +#include <linux/nvmem-consumer.h>
>>   #include "tsens.h"
>>     /* ----- SROT ------ */
>>   #define SROT_HW_VER_OFF    0x0000
>>   #define SROT_CTRL_OFF        0x0004
>> +#define SROT_MEASURE_PERIOD    0x0008
>> +#define SROT_Sn_CONVERSION    0x0060
>> +#define V2_SHIFT_DEFAULT    0x0003
>> +#define V2_SLOPE_DEFAULT    0x0cd0
>> +#define V2_CZERO_DEFAULT    0x016a
>> +#define ONE_PT_SLOPE        0x0cd0
>> +#define TWO_PT_SHIFTED_GAIN    921600
>> +#define ONE_PT_CZERO_CONST    94
>>     /* ----- TM ------ */
>>   #define TM_INT_EN_OFF            0x0004
>> @@ -59,6 +68,16 @@ static const struct reg_field 
>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>       /* CTRL_OFF */
>>       [TSENS_EN]     = REG_FIELD(SROT_CTRL_OFF,    0,  0),
>>       [TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF,    1,  1),
>> +    [SENSOR_EN]    = REG_FIELD(SROT_CTRL_OFF,    3,  18),
>> +    [CODE_OR_TEMP] = REG_FIELD(SROT_CTRL_OFF,    21, 21),
>> +
>> +    /* MAIN_MEASURE_PERIOD */
>> +    [MAIN_MEASURE_PERIOD] = REG_FIELD(SROT_MEASURE_PERIOD, 0, 7),
>> +
>> +    /* Sn Conversion */
>> +    REG_FIELD_FOR_EACH_SENSOR16(SHIFT, SROT_Sn_CONVERSION, 23, 24),
>> +    REG_FIELD_FOR_EACH_SENSOR16(SLOPE, SROT_Sn_CONVERSION, 10, 22),
>> +    REG_FIELD_FOR_EACH_SENSOR16(CZERO, SROT_Sn_CONVERSION, 0, 9),
>>         /* ----- TM ------ */
>>       /* INTERRUPT ENABLE */
>> @@ -104,6 +123,103 @@ static const struct reg_field 
>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>       [TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>>   };
>>   +static int tsens_v2_calibration(struct tsens_priv *priv)
>> +{
>> +    struct device *dev = priv->dev;
>> +    u32 mode, base0, base1;
>> +    u32 slope, czero;
>> +    char name[15];
>> +    int i, j, ret;
>> +
>> +    if (priv->num_sensors > MAX_SENSORS)
>> +        return -EINVAL;
>> +
>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
>> +    if (ret == -ENOENT)
>> +        dev_warn(priv->dev, "Calibration data not present in DT\n");
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    dev_dbg(priv->dev, "calibration mode is %d\n", mode);
>> +
>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base0", &base0);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    /* Read offset values and allocate SHIFT, SLOPE & CZERO regmap 
>> for enabled sensors */
>> +    for (i = 0; i < priv->num_sensors; i++) {
>> +        if (!(priv->sensors_to_en & (0x1 << i)))
>> +            continue;
>> +
>> +        ret = snprintf(name, sizeof(name), "s%d_offset", 
>> priv->sensor[i].hw_id);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        ret = nvmem_cell_read_variable_le_u32(priv->dev, name, 
>> &priv->sensor[i].offset);
>> +        if (ret)
>> +            return ret;
>> +
>> +        for (j = SHIFT_0; j <= CZERO_0; j++) {
>> +            int idx = (i * 3) + j;
>> +
>> +            priv->rf[idx] = devm_regmap_field_alloc(dev, 
>> priv->srot_map,
>> +                                priv->fields[idx]);
>> +            if (IS_ERR(priv->rf[idx]))
>> +                return PTR_ERR(priv->rf[idx]);
>
> I think, allocating data structures for 48 regfields, which are 
> written just once, to be an overkill.
Can we change it to single field for each sensor. For example, 
CONVERSION_0 instead of SHIFT_0, SLOPE_0 and CZERO_0? This way it will 
be max 16 regfields.
>
>> +        }
>> +    }
>> +
>> +    /* Based on calib mode, program SHIFT, SLOPE and CZERO for 
>> enabled sensors */
>> +    switch (mode) {
>> +    case TWO_PT_CALIB:
>> +        slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
>> +
>> +        for (i = 0; i < priv->num_sensors; i++) {
>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>> +                continue;
>> +
>> +            int idx = i * 3;
>> +
>> +            czero = (base0 + priv->sensor[i].offset - ((base1 - 
>> base0) / 3));
>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>> V2_SHIFT_DEFAULT);
>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], slope);
>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>> +        }
>> +        fallthrough;
>> +    case ONE_PT_CALIB2:
>> +        for (i = 0; i < priv->num_sensors; i++) {
>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>> +                continue;
>> +
>> +            int idx = i * 3;
>> +
>> +            czero = base0 + priv->sensor[i].offset - 
>> ONE_PT_CZERO_CONST;
>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>> V2_SHIFT_DEFAULT);
>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], ONE_PT_SLOPE);
>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>> +        }
>> +        break;
>> +    default:
>> +        dev_dbg(priv->dev, "calibrationless mode\n");
>> +        for (i = 0; i < priv->num_sensors; i++) {
>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>> +                continue;
>> +
>> +            int idx = i * 3;
>> +
>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>> V2_SHIFT_DEFAULT);
>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], 
>> V2_SLOPE_DEFAULT);
>> +            regmap_field_write(priv->rf[CZERO_0 + idx], 
>> V2_CZERO_DEFAULT);
>> +        }
>> +    }
>
> This code iterates over the sensors field several times. Please 
> consider extracting a function that handles all setup for a single 
> sensor, then calling it in a loop (I should probably do the same for 
> tsens-v0/v1 too).
Sure. After reading the mode0, base0 and base1 from QFPROM, we can call 
a function in a loop to setup the calibration for each sensor.
>
>> +
>> +    return 0;
>> +}
>> +
>>   static const struct tsens_ops ops_generic_v2 = {
>>       .init        = init_common,
>>       .get_temp    = get_temp_tsens_valid,
>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
>> index 98c356acfe98..169690355dad 100644
>> --- a/drivers/thermal/qcom/tsens.c
>> +++ b/drivers/thermal/qcom/tsens.c
>> @@ -974,7 +974,7 @@ int __init init_common(struct tsens_priv *priv)
>>       ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
>>       if (ret)
>>           goto err_put_device;
>> -    if (!enabled) {
>> +    if (!enabled && !priv->sensors_to_en) {
>>           dev_err(dev, "%s: device not enabled\n", __func__);
>>           ret = -ENODEV;
>>           goto err_put_device;
>> @@ -1006,6 +1006,40 @@ int __init init_common(struct tsens_priv *priv)
>>           goto err_put_device;
>>       }
>>   +    /* Do TSENS initialization if required */
>> +    if (priv->sensors_to_en) {
>
> Maybe it would be better to explicitly add VER_2_X_NO_RPM and check it 
> here?
Sure, will add a separate version macro.
>
>> +        priv->rf[CODE_OR_TEMP] = devm_regmap_field_alloc(dev, 
>> priv->srot_map,
>> + priv->fields[CODE_OR_TEMP]);
>> +        if (IS_ERR(priv->rf[CODE_OR_TEMP])) {
>> +            ret = PTR_ERR(priv->rf[CODE_OR_TEMP]);
>> +            goto err_put_device;
>> +        }
>> +
>> +        priv->rf[MAIN_MEASURE_PERIOD] =
>> +            devm_regmap_field_alloc(dev, priv->srot_map,
>> +                        priv->fields[MAIN_MEASURE_PERIOD]);
>> +        if (IS_ERR(priv->rf[MAIN_MEASURE_PERIOD])) {
>> +            ret = PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
>> +            goto err_put_device;
>> +        }
>> +
>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x1);
>> +
>> +        /* Update measure period to 2ms */
>> +        regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], 0x1);
>> +
>> +        /* Enable available sensors */
>> +        regmap_field_write(priv->rf[SENSOR_EN], priv->sensors_to_en);
>> +
>> +        /* Real temperature format */
>> +        regmap_field_write(priv->rf[CODE_OR_TEMP], 0x1);
>> +
>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x0);
>> +
>> +        /* Enable TSENS */
>> +        regmap_field_write(priv->rf[TSENS_EN], 0x1);
>> +    }
>> +
>>       /* This loop might need changes if enum regfield_ids is 
>> reordered */
>>       for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
>>           for (i = 0; i < priv->feat->max_sensors; i++) {
>> @@ -1282,6 +1316,7 @@ static int tsens_probe(struct platform_device 
>> *pdev)
>>         priv->dev = dev;
>>       priv->num_sensors = num_sensors;
>> +    priv->sensors_to_en = data->sensors_to_en;
>>       priv->ops = data->ops;
>>       for (i = 0;  i < priv->num_sensors; i++) {
>>           if (data->hw_ids)
>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
>> index 2805de1c6827..f8897bc8944e 100644
>> --- a/drivers/thermal/qcom/tsens.h
>> +++ b/drivers/thermal/qcom/tsens.h
>> @@ -168,6 +168,58 @@ enum regfield_ids {
>>       TSENS_SW_RST,
>>       SENSOR_EN,
>>       CODE_OR_TEMP,
>> +    /* MEASURE_PERIOD */
>> +    MAIN_MEASURE_PERIOD,
>> +
>> +    /* Sn_CONVERSION */
>> +    SHIFT_0,
>> +    SLOPE_0,
>> +    CZERO_0,
>> +    SHIFT_1,
>> +    SLOPE_1,
>> +    CZERO_1,
>> +    SHIFT_2,
>> +    SLOPE_2,
>> +    CZERO_2,
>> +    SHIFT_3,
>> +    SLOPE_3,
>> +    CZERO_3,
>> +    SHIFT_4,
>> +    SLOPE_4,
>> +    CZERO_4,
>> +    SHIFT_5,
>> +    SLOPE_5,
>> +    CZERO_5,
>> +    SHIFT_6,
>> +    SLOPE_6,
>> +    CZERO_6,
>> +    SHIFT_7,
>> +    SLOPE_7,
>> +    CZERO_7,
>> +    SHIFT_8,
>> +    SLOPE_8,
>> +    CZERO_8,
>> +    SHIFT_9,
>> +    SLOPE_9,
>> +    CZERO_9,
>> +    SHIFT_10,
>> +    SLOPE_10,
>> +    CZERO_10,
>> +    SHIFT_11,
>> +    SLOPE_11,
>> +    CZERO_11,
>> +    SHIFT_12,
>> +    SLOPE_12,
>> +    CZERO_12,
>> +    SHIFT_13,
>> +    SLOPE_13,
>> +    CZERO_13,
>> +    SHIFT_14,
>> +    SLOPE_14,
>> +    CZERO_14,
>> +    SHIFT_15,
>> +    SLOPE_15,
>> +    CZERO_15,
>>         /* ----- TM ------ */
>>       /* TRDY */
>> @@ -524,6 +576,7 @@ struct tsens_features {
>>   /**
>>    * struct tsens_plat_data - tsens compile-time platform data
>>    * @num_sensors: Number of sensors supported by platform
>> + * @sensors_to_en: Sensors to be enabled. Each bit represent a sensor
>>    * @ops: operations the tsens instance supports
>>    * @hw_ids: Subset of sensors ids supported by platform, if not the 
>> first n
>>    * @feat: features of the IP
>> @@ -531,6 +584,7 @@ struct tsens_features {
>>    */
>>   struct tsens_plat_data {
>>       const u32        num_sensors;
>> +    const u16        sensors_to_en;
>
> There is already a similar field, hw_ids. Can it be used instead?
Yes, it can be used. I missed to check this hw_ids. Will change the 
num_sensors to 5 and use the hw_ids.
>
>>       const struct tsens_ops    *ops;
>>       unsigned int        *hw_ids;
>>       struct tsens_features    *feat;
>> @@ -551,6 +605,7 @@ struct tsens_context {
>>    * struct tsens_priv - private data for each instance of the tsens IP
>>    * @dev: pointer to struct device
>>    * @num_sensors: number of sensors enabled on this device
>> + * @sensors_to_en: sensors to be enabled. Each bit represents a sensor
>>    * @tm_map: pointer to TM register address space
>>    * @srot_map: pointer to SROT register address space
>>    * @tm_offset: deal with old device trees that don't address TM and 
>> SROT
>> @@ -569,6 +624,7 @@ struct tsens_context {
>>   struct tsens_priv {
>>       struct device            *dev;
>>       u32                num_sensors;
>> +    u16                sensors_to_en;
>>       struct regmap            *tm_map;
>>       struct regmap            *srot_map;
>>       u32                tm_offset;
>
--
Thanks,
Praveenkumar
  
Praveenkumar I July 10, 2023, 1:24 p.m. UTC | #4
On 7/10/2023 4:52 PM, Dmitry Baryshkov wrote:
> On 10/07/2023 14:19, Dmitry Baryshkov wrote:
>> On 10/07/2023 13:37, Praveenkumar I wrote:
>>> SoCs without RPM have to enable sensors and calibrate from the kernel.
>>> Though TSENS IP supports 16 sensors, not all are used. So added
>>> sensors_to_en in tsens data help enable the relevant sensors.
>>>
>>> Added new calibration function for V2 as the tsens.c calib function
>>> only supports V1.
>>>
>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>>> ---
>>>   drivers/thermal/qcom/tsens-v2.c | 116 
>>> ++++++++++++++++++++++++++++++++
>>>   drivers/thermal/qcom/tsens.c    |  37 +++++++++-
>>>   drivers/thermal/qcom/tsens.h    |  56 +++++++++++++++
>>>   3 files changed, 208 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/thermal/qcom/tsens-v2.c 
>>> b/drivers/thermal/qcom/tsens-v2.c
>>> index 29a61d2d6ca3..db48b1d95348 100644
>>> --- a/drivers/thermal/qcom/tsens-v2.c
>>> +++ b/drivers/thermal/qcom/tsens-v2.c
>>> @@ -6,11 +6,20 @@
>>>   #include <linux/bitops.h>
>>>   #include <linux/regmap.h>
>>> +#include <linux/nvmem-consumer.h>
>>>   #include "tsens.h"
>>>   /* ----- SROT ------ */
>>>   #define SROT_HW_VER_OFF    0x0000
>>>   #define SROT_CTRL_OFF        0x0004
>>> +#define SROT_MEASURE_PERIOD    0x0008
>>> +#define SROT_Sn_CONVERSION    0x0060
>>> +#define V2_SHIFT_DEFAULT    0x0003
>>> +#define V2_SLOPE_DEFAULT    0x0cd0
>>> +#define V2_CZERO_DEFAULT    0x016a
>>> +#define ONE_PT_SLOPE        0x0cd0
>>> +#define TWO_PT_SHIFTED_GAIN    921600
>>> +#define ONE_PT_CZERO_CONST    94
>>>   /* ----- TM ------ */
>>>   #define TM_INT_EN_OFF            0x0004
>>> @@ -59,6 +68,16 @@ static const struct reg_field 
>>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>>       /* CTRL_OFF */
>>>       [TSENS_EN]     = REG_FIELD(SROT_CTRL_OFF,    0,  0),
>>>       [TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF,    1,  1),
>>> +    [SENSOR_EN]    = REG_FIELD(SROT_CTRL_OFF,    3,  18),
>>> +    [CODE_OR_TEMP] = REG_FIELD(SROT_CTRL_OFF,    21, 21),
>>> +
>>> +    /* MAIN_MEASURE_PERIOD */
>>> +    [MAIN_MEASURE_PERIOD] = REG_FIELD(SROT_MEASURE_PERIOD, 0, 7),
>>> +
>>> +    /* Sn Conversion */
>>> +    REG_FIELD_FOR_EACH_SENSOR16(SHIFT, SROT_Sn_CONVERSION, 23, 24),
>>> +    REG_FIELD_FOR_EACH_SENSOR16(SLOPE, SROT_Sn_CONVERSION, 10, 22),
>>> +    REG_FIELD_FOR_EACH_SENSOR16(CZERO, SROT_Sn_CONVERSION, 0, 9),
>>>       /* ----- TM ------ */
>>>       /* INTERRUPT ENABLE */
>>> @@ -104,6 +123,103 @@ static const struct reg_field 
>>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>>       [TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>>>   };
>>> +static int tsens_v2_calibration(struct tsens_priv *priv)
>>> +{
>>> +    struct device *dev = priv->dev;
>>> +    u32 mode, base0, base1;
>>> +    u32 slope, czero;
>>> +    char name[15];
>>> +    int i, j, ret;
>>> +
>>> +    if (priv->num_sensors > MAX_SENSORS)
>>> +        return -EINVAL;
>>> +
>>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
>>> +    if (ret == -ENOENT)
>>> +        dev_warn(priv->dev, "Calibration data not present in DT\n");
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    dev_dbg(priv->dev, "calibration mode is %d\n", mode);
>>> +
>>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base0", &base0);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    /* Read offset values and allocate SHIFT, SLOPE & CZERO regmap 
>>> for enabled sensors */
>>> +    for (i = 0; i < priv->num_sensors; i++) {
>>> +        if (!(priv->sensors_to_en & (0x1 << i)))
>>> +            continue;
>>> +
>>> +        ret = snprintf(name, sizeof(name), "s%d_offset", 
>>> priv->sensor[i].hw_id);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +
>>> +        ret = nvmem_cell_read_variable_le_u32(priv->dev, name, 
>>> &priv->sensor[i].offset);
>>> +        if (ret)
>>> +            return ret;
>>> +
>>> +        for (j = SHIFT_0; j <= CZERO_0; j++) {
>>> +            int idx = (i * 3) + j;
>>> +
>>> +            priv->rf[idx] = devm_regmap_field_alloc(dev, 
>>> priv->srot_map,
>>> +                                priv->fields[idx]);
>>> +            if (IS_ERR(priv->rf[idx]))
>>> +                return PTR_ERR(priv->rf[idx]);
>>
>> I think, allocating data structures for 48 regfields, which are 
>> written just once, to be an overkill.
>>
>>> +        }
>>> +    }
>>> +
>>> +    /* Based on calib mode, program SHIFT, SLOPE and CZERO for 
>>> enabled sensors */
>>> +    switch (mode) {
>>> +    case TWO_PT_CALIB:
>>> +        slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
>>> +
>>> +        for (i = 0; i < priv->num_sensors; i++) {
>>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>>> +                continue;
>>> +
>>> +            int idx = i * 3;
>>> +
>>> +            czero = (base0 + priv->sensor[i].offset - ((base1 - 
>>> base0) / 3));
>>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>>> V2_SHIFT_DEFAULT);
>>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], slope);
>>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>>> +        }
>>> +        fallthrough;
>>> +    case ONE_PT_CALIB2:
>>> +        for (i = 0; i < priv->num_sensors; i++) {
>>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>>> +                continue;
>>> +
>>> +            int idx = i * 3;
>>> +
>>> +            czero = base0 + priv->sensor[i].offset - 
>>> ONE_PT_CZERO_CONST;
>>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>>> V2_SHIFT_DEFAULT);
>>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], ONE_PT_SLOPE);
>>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>>> +        }
>>> +        break;
>>> +    default:
>>> +        dev_dbg(priv->dev, "calibrationless mode\n");
>>> +        for (i = 0; i < priv->num_sensors; i++) {
>>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>>> +                continue;
>>> +
>>> +            int idx = i * 3;
>>> +
>>> +            regmap_field_write(priv->rf[SHIFT_0 + idx], 
>>> V2_SHIFT_DEFAULT);
>>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], 
>>> V2_SLOPE_DEFAULT);
>>> +            regmap_field_write(priv->rf[CZERO_0 + idx], 
>>> V2_CZERO_DEFAULT);
>>> +        }
>>> +    }
>>
>> This code iterates over the sensors field several times. Please 
>> consider extracting a function that handles all setup for a single 
>> sensor, then calling it in a loop (I should probably do the same for 
>> tsens-v0/v1 too).
>>
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   static const struct tsens_ops ops_generic_v2 = {
>>>       .init        = init_common,
>>>       .get_temp    = get_temp_tsens_valid,
>>> diff --git a/drivers/thermal/qcom/tsens.c 
>>> b/drivers/thermal/qcom/tsens.c
>>> index 98c356acfe98..169690355dad 100644
>>> --- a/drivers/thermal/qcom/tsens.c
>>> +++ b/drivers/thermal/qcom/tsens.c
>>> @@ -974,7 +974,7 @@ int __init init_common(struct tsens_priv *priv)
>>>       ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
>>>       if (ret)
>>>           goto err_put_device;
>>> -    if (!enabled) {
>>> +    if (!enabled && !priv->sensors_to_en) {
>>>           dev_err(dev, "%s: device not enabled\n", __func__);
>>>           ret = -ENODEV;
>>>           goto err_put_device;
>>> @@ -1006,6 +1006,40 @@ int __init init_common(struct tsens_priv *priv)
>>>           goto err_put_device;
>>>       }
>>> +    /* Do TSENS initialization if required */
>>> +    if (priv->sensors_to_en) {
>>
>> Maybe it would be better to explicitly add VER_2_X_NO_RPM and check 
>> it here?
>
> After taking a look at the patch 6, can we just add a separate init() 
> function which will call init_common() and then initialize these fields?
Sure, will add separate init() function in tsens-v2.c
>
>>
>>> +        priv->rf[CODE_OR_TEMP] = devm_regmap_field_alloc(dev, 
>>> priv->srot_map,
>>> + priv->fields[CODE_OR_TEMP]);
>>> +        if (IS_ERR(priv->rf[CODE_OR_TEMP])) {
>>> +            ret = PTR_ERR(priv->rf[CODE_OR_TEMP]);
>>> +            goto err_put_device;
>>> +        }
>>> +
>>> +        priv->rf[MAIN_MEASURE_PERIOD] =
>>> +            devm_regmap_field_alloc(dev, priv->srot_map,
>>> + priv->fields[MAIN_MEASURE_PERIOD]);
>>> +        if (IS_ERR(priv->rf[MAIN_MEASURE_PERIOD])) {
>>> +            ret = PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
>>> +            goto err_put_device;
>>> +        }
>>> +
>>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x1);
>>> +
>>> +        /* Update measure period to 2ms */
>>> +        regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], 0x1);
>>> +
>>> +        /* Enable available sensors */
>>> +        regmap_field_write(priv->rf[SENSOR_EN], priv->sensors_to_en);
>>> +
>>> +        /* Real temperature format */
>>> +        regmap_field_write(priv->rf[CODE_OR_TEMP], 0x1);
>>> +
>>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x0);
>>> +
>>> +        /* Enable TSENS */
>>> +        regmap_field_write(priv->rf[TSENS_EN], 0x1);
>>> +    }
>>> +
>>>       /* This loop might need changes if enum regfield_ids is 
>>> reordered */
>>>       for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
>>>           for (i = 0; i < priv->feat->max_sensors; i++) {
>>> @@ -1282,6 +1316,7 @@ static int tsens_probe(struct platform_device 
>>> *pdev)
>>>       priv->dev = dev;
>>>       priv->num_sensors = num_sensors;
>>> +    priv->sensors_to_en = data->sensors_to_en;
>>>       priv->ops = data->ops;
>>>       for (i = 0;  i < priv->num_sensors; i++) {
>>>           if (data->hw_ids)
>>> diff --git a/drivers/thermal/qcom/tsens.h 
>>> b/drivers/thermal/qcom/tsens.h
>>> index 2805de1c6827..f8897bc8944e 100644
>>> --- a/drivers/thermal/qcom/tsens.h
>>> +++ b/drivers/thermal/qcom/tsens.h
>>> @@ -168,6 +168,58 @@ enum regfield_ids {
>>>       TSENS_SW_RST,
>>>       SENSOR_EN,
>>>       CODE_OR_TEMP,
>>> +    /* MEASURE_PERIOD */
>>> +    MAIN_MEASURE_PERIOD,
>>> +
>>> +    /* Sn_CONVERSION */
>>> +    SHIFT_0,
>>> +    SLOPE_0,
>>> +    CZERO_0,
>>> +    SHIFT_1,
>>> +    SLOPE_1,
>>> +    CZERO_1,
>>> +    SHIFT_2,
>>> +    SLOPE_2,
>>> +    CZERO_2,
>>> +    SHIFT_3,
>>> +    SLOPE_3,
>>> +    CZERO_3,
>>> +    SHIFT_4,
>>> +    SLOPE_4,
>>> +    CZERO_4,
>>> +    SHIFT_5,
>>> +    SLOPE_5,
>>> +    CZERO_5,
>>> +    SHIFT_6,
>>> +    SLOPE_6,
>>> +    CZERO_6,
>>> +    SHIFT_7,
>>> +    SLOPE_7,
>>> +    CZERO_7,
>>> +    SHIFT_8,
>>> +    SLOPE_8,
>>> +    CZERO_8,
>>> +    SHIFT_9,
>>> +    SLOPE_9,
>>> +    CZERO_9,
>>> +    SHIFT_10,
>>> +    SLOPE_10,
>>> +    CZERO_10,
>>> +    SHIFT_11,
>>> +    SLOPE_11,
>>> +    CZERO_11,
>>> +    SHIFT_12,
>>> +    SLOPE_12,
>>> +    CZERO_12,
>>> +    SHIFT_13,
>>> +    SLOPE_13,
>>> +    CZERO_13,
>>> +    SHIFT_14,
>>> +    SLOPE_14,
>>> +    CZERO_14,
>>> +    SHIFT_15,
>>> +    SLOPE_15,
>>> +    CZERO_15,
>>>       /* ----- TM ------ */
>>>       /* TRDY */
>>> @@ -524,6 +576,7 @@ struct tsens_features {
>>>   /**
>>>    * struct tsens_plat_data - tsens compile-time platform data
>>>    * @num_sensors: Number of sensors supported by platform
>>> + * @sensors_to_en: Sensors to be enabled. Each bit represent a sensor
>>>    * @ops: operations the tsens instance supports
>>>    * @hw_ids: Subset of sensors ids supported by platform, if not 
>>> the first n
>>>    * @feat: features of the IP
>>> @@ -531,6 +584,7 @@ struct tsens_features {
>>>    */
>>>   struct tsens_plat_data {
>>>       const u32        num_sensors;
>>> +    const u16        sensors_to_en;
>>
>> There is already a similar field, hw_ids. Can it be used instead?
>>
>>>       const struct tsens_ops    *ops;
>>>       unsigned int        *hw_ids;
>>>       struct tsens_features    *feat;
>>> @@ -551,6 +605,7 @@ struct tsens_context {
>>>    * struct tsens_priv - private data for each instance of the tsens IP
>>>    * @dev: pointer to struct device
>>>    * @num_sensors: number of sensors enabled on this device
>>> + * @sensors_to_en: sensors to be enabled. Each bit represents a sensor
>>>    * @tm_map: pointer to TM register address space
>>>    * @srot_map: pointer to SROT register address space
>>>    * @tm_offset: deal with old device trees that don't address TM 
>>> and SROT
>>> @@ -569,6 +624,7 @@ struct tsens_context {
>>>   struct tsens_priv {
>>>       struct device            *dev;
>>>       u32                num_sensors;
>>> +    u16                sensors_to_en;
>>>       struct regmap            *tm_map;
>>>       struct regmap            *srot_map;
>>>       u32                tm_offset;
>>
>
--
Thanks,
Praveenkumar
  
Dmitry Baryshkov July 10, 2023, 3:02 p.m. UTC | #5
On Mon, 10 Jul 2023 at 16:22, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
>
> On 7/10/2023 4:49 PM, Dmitry Baryshkov wrote:
> > On 10/07/2023 13:37, Praveenkumar I wrote:
> >> SoCs without RPM have to enable sensors and calibrate from the kernel.
> >> Though TSENS IP supports 16 sensors, not all are used. So added
> >> sensors_to_en in tsens data help enable the relevant sensors.
> >>
> >> Added new calibration function for V2 as the tsens.c calib function
> >> only supports V1.
> >>
> >> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> >> ---
> >>   drivers/thermal/qcom/tsens-v2.c | 116 ++++++++++++++++++++++++++++++++
> >>   drivers/thermal/qcom/tsens.c    |  37 +++++++++-
> >>   drivers/thermal/qcom/tsens.h    |  56 +++++++++++++++
> >>   3 files changed, 208 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/thermal/qcom/tsens-v2.c
> >> b/drivers/thermal/qcom/tsens-v2.c
> >> index 29a61d2d6ca3..db48b1d95348 100644
> >> --- a/drivers/thermal/qcom/tsens-v2.c
> >> +++ b/drivers/thermal/qcom/tsens-v2.c
> >> @@ -6,11 +6,20 @@
> >>     #include <linux/bitops.h>
> >>   #include <linux/regmap.h>
> >> +#include <linux/nvmem-consumer.h>
> >>   #include "tsens.h"
> >>     /* ----- SROT ------ */
> >>   #define SROT_HW_VER_OFF    0x0000
> >>   #define SROT_CTRL_OFF        0x0004
> >> +#define SROT_MEASURE_PERIOD    0x0008
> >> +#define SROT_Sn_CONVERSION    0x0060
> >> +#define V2_SHIFT_DEFAULT    0x0003
> >> +#define V2_SLOPE_DEFAULT    0x0cd0
> >> +#define V2_CZERO_DEFAULT    0x016a
> >> +#define ONE_PT_SLOPE        0x0cd0
> >> +#define TWO_PT_SHIFTED_GAIN    921600
> >> +#define ONE_PT_CZERO_CONST    94
> >>     /* ----- TM ------ */
> >>   #define TM_INT_EN_OFF            0x0004
> >> @@ -59,6 +68,16 @@ static const struct reg_field
> >> tsens_v2_regfields[MAX_REGFIELDS] = {
> >>       /* CTRL_OFF */
> >>       [TSENS_EN]     = REG_FIELD(SROT_CTRL_OFF,    0,  0),
> >>       [TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF,    1,  1),
> >> +    [SENSOR_EN]    = REG_FIELD(SROT_CTRL_OFF,    3,  18),
> >> +    [CODE_OR_TEMP] = REG_FIELD(SROT_CTRL_OFF,    21, 21),
> >> +
> >> +    /* MAIN_MEASURE_PERIOD */
> >> +    [MAIN_MEASURE_PERIOD] = REG_FIELD(SROT_MEASURE_PERIOD, 0, 7),
> >> +
> >> +    /* Sn Conversion */
> >> +    REG_FIELD_FOR_EACH_SENSOR16(SHIFT, SROT_Sn_CONVERSION, 23, 24),
> >> +    REG_FIELD_FOR_EACH_SENSOR16(SLOPE, SROT_Sn_CONVERSION, 10, 22),
> >> +    REG_FIELD_FOR_EACH_SENSOR16(CZERO, SROT_Sn_CONVERSION, 0, 9),
> >>         /* ----- TM ------ */
> >>       /* INTERRUPT ENABLE */
> >> @@ -104,6 +123,103 @@ static const struct reg_field
> >> tsens_v2_regfields[MAX_REGFIELDS] = {
> >>       [TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
> >>   };
> >>   +static int tsens_v2_calibration(struct tsens_priv *priv)
> >> +{
> >> +    struct device *dev = priv->dev;
> >> +    u32 mode, base0, base1;
> >> +    u32 slope, czero;
> >> +    char name[15];
> >> +    int i, j, ret;
> >> +
> >> +    if (priv->num_sensors > MAX_SENSORS)
> >> +        return -EINVAL;
> >> +
> >> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
> >> +    if (ret == -ENOENT)
> >> +        dev_warn(priv->dev, "Calibration data not present in DT\n");
> >> +    if (ret < 0)
> >> +        return ret;
> >> +
> >> +    dev_dbg(priv->dev, "calibration mode is %d\n", mode);
> >> +
> >> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base0", &base0);
> >> +    if (ret < 0)
> >> +        return ret;
> >> +
> >> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
> >> +    if (ret < 0)
> >> +        return ret;
> >> +
> >> +    /* Read offset values and allocate SHIFT, SLOPE & CZERO regmap
> >> for enabled sensors */
> >> +    for (i = 0; i < priv->num_sensors; i++) {
> >> +        if (!(priv->sensors_to_en & (0x1 << i)))
> >> +            continue;
> >> +
> >> +        ret = snprintf(name, sizeof(name), "s%d_offset",
> >> priv->sensor[i].hw_id);
> >> +        if (ret < 0)
> >> +            return ret;
> >> +
> >> +        ret = nvmem_cell_read_variable_le_u32(priv->dev, name,
> >> &priv->sensor[i].offset);
> >> +        if (ret)
> >> +            return ret;
> >> +
> >> +        for (j = SHIFT_0; j <= CZERO_0; j++) {
> >> +            int idx = (i * 3) + j;
> >> +
> >> +            priv->rf[idx] = devm_regmap_field_alloc(dev,
> >> priv->srot_map,
> >> +                                priv->fields[idx]);
> >> +            if (IS_ERR(priv->rf[idx]))
> >> +                return PTR_ERR(priv->rf[idx]);
> >
> > I think, allocating data structures for 48 regfields, which are
> > written just once, to be an overkill.
> Can we change it to single field for each sensor. For example,
> CONVERSION_0 instead of SHIFT_0, SLOPE_0 and CZERO_0? This way it will
> be max 16 regfields.

If you move writing of the registers to the loop, you won't need
regfields. You can just call regmap_update_bits. The point is that you
don't have to allocate a one-time instance.

> >
> >> +        }
> >> +    }
> >> +
> >> +    /* Based on calib mode, program SHIFT, SLOPE and CZERO for
> >> enabled sensors */
> >> +    switch (mode) {
> >> +    case TWO_PT_CALIB:
> >> +        slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
> >> +
> >> +        for (i = 0; i < priv->num_sensors; i++) {
> >> +            if (!(priv->sensors_to_en & (0x1 << i)))
> >> +                continue;
> >> +
> >> +            int idx = i * 3;
> >> +
> >> +            czero = (base0 + priv->sensor[i].offset - ((base1 -
> >> base0) / 3));
> >> +            regmap_field_write(priv->rf[SHIFT_0 + idx],
> >> V2_SHIFT_DEFAULT);
> >> +            regmap_field_write(priv->rf[SLOPE_0 + idx], slope);
> >> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
> >> +        }
> >> +        fallthrough;
> >> +    case ONE_PT_CALIB2:
> >> +        for (i = 0; i < priv->num_sensors; i++) {
> >> +            if (!(priv->sensors_to_en & (0x1 << i)))
> >> +                continue;
> >> +
> >> +            int idx = i * 3;
> >> +
> >> +            czero = base0 + priv->sensor[i].offset -
> >> ONE_PT_CZERO_CONST;
> >> +            regmap_field_write(priv->rf[SHIFT_0 + idx],
> >> V2_SHIFT_DEFAULT);
> >> +            regmap_field_write(priv->rf[SLOPE_0 + idx], ONE_PT_SLOPE);
> >> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
> >> +        }
> >> +        break;
> >> +    default:
> >> +        dev_dbg(priv->dev, "calibrationless mode\n");
> >> +        for (i = 0; i < priv->num_sensors; i++) {
> >> +            if (!(priv->sensors_to_en & (0x1 << i)))
> >> +                continue;
> >> +
> >> +            int idx = i * 3;
> >> +
> >> +            regmap_field_write(priv->rf[SHIFT_0 + idx],
> >> V2_SHIFT_DEFAULT);
> >> +            regmap_field_write(priv->rf[SLOPE_0 + idx],
> >> V2_SLOPE_DEFAULT);
> >> +            regmap_field_write(priv->rf[CZERO_0 + idx],
> >> V2_CZERO_DEFAULT);
> >> +        }
> >> +    }
> >
> > This code iterates over the sensors field several times. Please
> > consider extracting a function that handles all setup for a single
> > sensor, then calling it in a loop (I should probably do the same for
> > tsens-v0/v1 too).
> Sure. After reading the mode0, base0 and base1 from QFPROM, we can call
> a function in a loop to setup the calibration for each sensor.
> >
> >> +
> >> +    return 0;
> >> +}
> >> +
> >>   static const struct tsens_ops ops_generic_v2 = {
> >>       .init        = init_common,
> >>       .get_temp    = get_temp_tsens_valid,
> >> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> >> index 98c356acfe98..169690355dad 100644
> >> --- a/drivers/thermal/qcom/tsens.c
> >> +++ b/drivers/thermal/qcom/tsens.c
> >> @@ -974,7 +974,7 @@ int __init init_common(struct tsens_priv *priv)
> >>       ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
> >>       if (ret)
> >>           goto err_put_device;
> >> -    if (!enabled) {
> >> +    if (!enabled && !priv->sensors_to_en) {
> >>           dev_err(dev, "%s: device not enabled\n", __func__);
> >>           ret = -ENODEV;
> >>           goto err_put_device;
> >> @@ -1006,6 +1006,40 @@ int __init init_common(struct tsens_priv *priv)
> >>           goto err_put_device;
> >>       }
> >>   +    /* Do TSENS initialization if required */
> >> +    if (priv->sensors_to_en) {
> >
> > Maybe it would be better to explicitly add VER_2_X_NO_RPM and check it
> > here?
> Sure, will add a separate version macro.
> >
> >> +        priv->rf[CODE_OR_TEMP] = devm_regmap_field_alloc(dev,
> >> priv->srot_map,
> >> + priv->fields[CODE_OR_TEMP]);
> >> +        if (IS_ERR(priv->rf[CODE_OR_TEMP])) {
> >> +            ret = PTR_ERR(priv->rf[CODE_OR_TEMP]);
> >> +            goto err_put_device;
> >> +        }
> >> +
> >> +        priv->rf[MAIN_MEASURE_PERIOD] =
> >> +            devm_regmap_field_alloc(dev, priv->srot_map,
> >> +                        priv->fields[MAIN_MEASURE_PERIOD]);
> >> +        if (IS_ERR(priv->rf[MAIN_MEASURE_PERIOD])) {
> >> +            ret = PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
> >> +            goto err_put_device;
> >> +        }
> >> +
> >> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x1);
> >> +
> >> +        /* Update measure period to 2ms */
> >> +        regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], 0x1);
> >> +
> >> +        /* Enable available sensors */
> >> +        regmap_field_write(priv->rf[SENSOR_EN], priv->sensors_to_en);
> >> +
> >> +        /* Real temperature format */
> >> +        regmap_field_write(priv->rf[CODE_OR_TEMP], 0x1);
> >> +
> >> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x0);
> >> +
> >> +        /* Enable TSENS */
> >> +        regmap_field_write(priv->rf[TSENS_EN], 0x1);
> >> +    }
> >> +
> >>       /* This loop might need changes if enum regfield_ids is
> >> reordered */
> >>       for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
> >>           for (i = 0; i < priv->feat->max_sensors; i++) {
> >> @@ -1282,6 +1316,7 @@ static int tsens_probe(struct platform_device
> >> *pdev)
> >>         priv->dev = dev;
> >>       priv->num_sensors = num_sensors;
> >> +    priv->sensors_to_en = data->sensors_to_en;
> >>       priv->ops = data->ops;
> >>       for (i = 0;  i < priv->num_sensors; i++) {
> >>           if (data->hw_ids)
> >> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> >> index 2805de1c6827..f8897bc8944e 100644
> >> --- a/drivers/thermal/qcom/tsens.h
> >> +++ b/drivers/thermal/qcom/tsens.h
> >> @@ -168,6 +168,58 @@ enum regfield_ids {
> >>       TSENS_SW_RST,
> >>       SENSOR_EN,
> >>       CODE_OR_TEMP,
> >> +    /* MEASURE_PERIOD */
> >> +    MAIN_MEASURE_PERIOD,
> >> +
> >> +    /* Sn_CONVERSION */
> >> +    SHIFT_0,
> >> +    SLOPE_0,
> >> +    CZERO_0,
> >> +    SHIFT_1,
> >> +    SLOPE_1,
> >> +    CZERO_1,
> >> +    SHIFT_2,
> >> +    SLOPE_2,
> >> +    CZERO_2,
> >> +    SHIFT_3,
> >> +    SLOPE_3,
> >> +    CZERO_3,
> >> +    SHIFT_4,
> >> +    SLOPE_4,
> >> +    CZERO_4,
> >> +    SHIFT_5,
> >> +    SLOPE_5,
> >> +    CZERO_5,
> >> +    SHIFT_6,
> >> +    SLOPE_6,
> >> +    CZERO_6,
> >> +    SHIFT_7,
> >> +    SLOPE_7,
> >> +    CZERO_7,
> >> +    SHIFT_8,
> >> +    SLOPE_8,
> >> +    CZERO_8,
> >> +    SHIFT_9,
> >> +    SLOPE_9,
> >> +    CZERO_9,
> >> +    SHIFT_10,
> >> +    SLOPE_10,
> >> +    CZERO_10,
> >> +    SHIFT_11,
> >> +    SLOPE_11,
> >> +    CZERO_11,
> >> +    SHIFT_12,
> >> +    SLOPE_12,
> >> +    CZERO_12,
> >> +    SHIFT_13,
> >> +    SLOPE_13,
> >> +    CZERO_13,
> >> +    SHIFT_14,
> >> +    SLOPE_14,
> >> +    CZERO_14,
> >> +    SHIFT_15,
> >> +    SLOPE_15,
> >> +    CZERO_15,
> >>         /* ----- TM ------ */
> >>       /* TRDY */
> >> @@ -524,6 +576,7 @@ struct tsens_features {
> >>   /**
> >>    * struct tsens_plat_data - tsens compile-time platform data
> >>    * @num_sensors: Number of sensors supported by platform
> >> + * @sensors_to_en: Sensors to be enabled. Each bit represent a sensor
> >>    * @ops: operations the tsens instance supports
> >>    * @hw_ids: Subset of sensors ids supported by platform, if not the
> >> first n
> >>    * @feat: features of the IP
> >> @@ -531,6 +584,7 @@ struct tsens_features {
> >>    */
> >>   struct tsens_plat_data {
> >>       const u32        num_sensors;
> >> +    const u16        sensors_to_en;
> >
> > There is already a similar field, hw_ids. Can it be used instead?
> Yes, it can be used. I missed to check this hw_ids. Will change the
> num_sensors to 5 and use the hw_ids.
> >
> >>       const struct tsens_ops    *ops;
> >>       unsigned int        *hw_ids;
> >>       struct tsens_features    *feat;
> >> @@ -551,6 +605,7 @@ struct tsens_context {
> >>    * struct tsens_priv - private data for each instance of the tsens IP
> >>    * @dev: pointer to struct device
> >>    * @num_sensors: number of sensors enabled on this device
> >> + * @sensors_to_en: sensors to be enabled. Each bit represents a sensor
> >>    * @tm_map: pointer to TM register address space
> >>    * @srot_map: pointer to SROT register address space
> >>    * @tm_offset: deal with old device trees that don't address TM and
> >> SROT
> >> @@ -569,6 +624,7 @@ struct tsens_context {
> >>   struct tsens_priv {
> >>       struct device            *dev;
> >>       u32                num_sensors;
> >> +    u16                sensors_to_en;
> >>       struct regmap            *tm_map;
> >>       struct regmap            *srot_map;
> >>       u32                tm_offset;
> >
> --
> Thanks,
> Praveenkumar
  
Praveenkumar I July 11, 2023, 9:19 a.m. UTC | #6
On 7/10/2023 8:32 PM, Dmitry Baryshkov wrote:
> On Mon, 10 Jul 2023 at 16:22, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>>
>> On 7/10/2023 4:49 PM, Dmitry Baryshkov wrote:
>>> On 10/07/2023 13:37, Praveenkumar I wrote:
>>>> SoCs without RPM have to enable sensors and calibrate from the kernel.
>>>> Though TSENS IP supports 16 sensors, not all are used. So added
>>>> sensors_to_en in tsens data help enable the relevant sensors.
>>>>
>>>> Added new calibration function for V2 as the tsens.c calib function
>>>> only supports V1.
>>>>
>>>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>>>> ---
>>>>    drivers/thermal/qcom/tsens-v2.c | 116 ++++++++++++++++++++++++++++++++
>>>>    drivers/thermal/qcom/tsens.c    |  37 +++++++++-
>>>>    drivers/thermal/qcom/tsens.h    |  56 +++++++++++++++
>>>>    3 files changed, 208 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/thermal/qcom/tsens-v2.c
>>>> b/drivers/thermal/qcom/tsens-v2.c
>>>> index 29a61d2d6ca3..db48b1d95348 100644
>>>> --- a/drivers/thermal/qcom/tsens-v2.c
>>>> +++ b/drivers/thermal/qcom/tsens-v2.c
>>>> @@ -6,11 +6,20 @@
>>>>      #include <linux/bitops.h>
>>>>    #include <linux/regmap.h>
>>>> +#include <linux/nvmem-consumer.h>
>>>>    #include "tsens.h"
>>>>      /* ----- SROT ------ */
>>>>    #define SROT_HW_VER_OFF    0x0000
>>>>    #define SROT_CTRL_OFF        0x0004
>>>> +#define SROT_MEASURE_PERIOD    0x0008
>>>> +#define SROT_Sn_CONVERSION    0x0060
>>>> +#define V2_SHIFT_DEFAULT    0x0003
>>>> +#define V2_SLOPE_DEFAULT    0x0cd0
>>>> +#define V2_CZERO_DEFAULT    0x016a
>>>> +#define ONE_PT_SLOPE        0x0cd0
>>>> +#define TWO_PT_SHIFTED_GAIN    921600
>>>> +#define ONE_PT_CZERO_CONST    94
>>>>      /* ----- TM ------ */
>>>>    #define TM_INT_EN_OFF            0x0004
>>>> @@ -59,6 +68,16 @@ static const struct reg_field
>>>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>>>        /* CTRL_OFF */
>>>>        [TSENS_EN]     = REG_FIELD(SROT_CTRL_OFF,    0,  0),
>>>>        [TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF,    1,  1),
>>>> +    [SENSOR_EN]    = REG_FIELD(SROT_CTRL_OFF,    3,  18),
>>>> +    [CODE_OR_TEMP] = REG_FIELD(SROT_CTRL_OFF,    21, 21),
>>>> +
>>>> +    /* MAIN_MEASURE_PERIOD */
>>>> +    [MAIN_MEASURE_PERIOD] = REG_FIELD(SROT_MEASURE_PERIOD, 0, 7),
>>>> +
>>>> +    /* Sn Conversion */
>>>> +    REG_FIELD_FOR_EACH_SENSOR16(SHIFT, SROT_Sn_CONVERSION, 23, 24),
>>>> +    REG_FIELD_FOR_EACH_SENSOR16(SLOPE, SROT_Sn_CONVERSION, 10, 22),
>>>> +    REG_FIELD_FOR_EACH_SENSOR16(CZERO, SROT_Sn_CONVERSION, 0, 9),
>>>>          /* ----- TM ------ */
>>>>        /* INTERRUPT ENABLE */
>>>> @@ -104,6 +123,103 @@ static const struct reg_field
>>>> tsens_v2_regfields[MAX_REGFIELDS] = {
>>>>        [TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>>>>    };
>>>>    +static int tsens_v2_calibration(struct tsens_priv *priv)
>>>> +{
>>>> +    struct device *dev = priv->dev;
>>>> +    u32 mode, base0, base1;
>>>> +    u32 slope, czero;
>>>> +    char name[15];
>>>> +    int i, j, ret;
>>>> +
>>>> +    if (priv->num_sensors > MAX_SENSORS)
>>>> +        return -EINVAL;
>>>> +
>>>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
>>>> +    if (ret == -ENOENT)
>>>> +        dev_warn(priv->dev, "Calibration data not present in DT\n");
>>>> +    if (ret < 0)
>>>> +        return ret;
>>>> +
>>>> +    dev_dbg(priv->dev, "calibration mode is %d\n", mode);
>>>> +
>>>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base0", &base0);
>>>> +    if (ret < 0)
>>>> +        return ret;
>>>> +
>>>> +    ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
>>>> +    if (ret < 0)
>>>> +        return ret;
>>>> +
>>>> +    /* Read offset values and allocate SHIFT, SLOPE & CZERO regmap
>>>> for enabled sensors */
>>>> +    for (i = 0; i < priv->num_sensors; i++) {
>>>> +        if (!(priv->sensors_to_en & (0x1 << i)))
>>>> +            continue;
>>>> +
>>>> +        ret = snprintf(name, sizeof(name), "s%d_offset",
>>>> priv->sensor[i].hw_id);
>>>> +        if (ret < 0)
>>>> +            return ret;
>>>> +
>>>> +        ret = nvmem_cell_read_variable_le_u32(priv->dev, name,
>>>> &priv->sensor[i].offset);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +
>>>> +        for (j = SHIFT_0; j <= CZERO_0; j++) {
>>>> +            int idx = (i * 3) + j;
>>>> +
>>>> +            priv->rf[idx] = devm_regmap_field_alloc(dev,
>>>> priv->srot_map,
>>>> +                                priv->fields[idx]);
>>>> +            if (IS_ERR(priv->rf[idx]))
>>>> +                return PTR_ERR(priv->rf[idx]);
>>> I think, allocating data structures for 48 regfields, which are
>>> written just once, to be an overkill.
>> Can we change it to single field for each sensor. For example,
>> CONVERSION_0 instead of SHIFT_0, SLOPE_0 and CZERO_0? This way it will
>> be max 16 regfields.
> If you move writing of the registers to the loop, you won't need
> regfields. You can just call regmap_update_bits. The point is that you
> don't have to allocate a one-time instance.
Sure, will update in next patch.
>
>>>> +        }
>>>> +    }
>>>> +
>>>> +    /* Based on calib mode, program SHIFT, SLOPE and CZERO for
>>>> enabled sensors */
>>>> +    switch (mode) {
>>>> +    case TWO_PT_CALIB:
>>>> +        slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
>>>> +
>>>> +        for (i = 0; i < priv->num_sensors; i++) {
>>>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>>>> +                continue;
>>>> +
>>>> +            int idx = i * 3;
>>>> +
>>>> +            czero = (base0 + priv->sensor[i].offset - ((base1 -
>>>> base0) / 3));
>>>> +            regmap_field_write(priv->rf[SHIFT_0 + idx],
>>>> V2_SHIFT_DEFAULT);
>>>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], slope);
>>>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>>>> +        }
>>>> +        fallthrough;
>>>> +    case ONE_PT_CALIB2:
>>>> +        for (i = 0; i < priv->num_sensors; i++) {
>>>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>>>> +                continue;
>>>> +
>>>> +            int idx = i * 3;
>>>> +
>>>> +            czero = base0 + priv->sensor[i].offset -
>>>> ONE_PT_CZERO_CONST;
>>>> +            regmap_field_write(priv->rf[SHIFT_0 + idx],
>>>> V2_SHIFT_DEFAULT);
>>>> +            regmap_field_write(priv->rf[SLOPE_0 + idx], ONE_PT_SLOPE);
>>>> +            regmap_field_write(priv->rf[CZERO_0 + idx], czero);
>>>> +        }
>>>> +        break;
>>>> +    default:
>>>> +        dev_dbg(priv->dev, "calibrationless mode\n");
>>>> +        for (i = 0; i < priv->num_sensors; i++) {
>>>> +            if (!(priv->sensors_to_en & (0x1 << i)))
>>>> +                continue;
>>>> +
>>>> +            int idx = i * 3;
>>>> +
>>>> +            regmap_field_write(priv->rf[SHIFT_0 + idx],
>>>> V2_SHIFT_DEFAULT);
>>>> +            regmap_field_write(priv->rf[SLOPE_0 + idx],
>>>> V2_SLOPE_DEFAULT);
>>>> +            regmap_field_write(priv->rf[CZERO_0 + idx],
>>>> V2_CZERO_DEFAULT);
>>>> +        }
>>>> +    }
>>> This code iterates over the sensors field several times. Please
>>> consider extracting a function that handles all setup for a single
>>> sensor, then calling it in a loop (I should probably do the same for
>>> tsens-v0/v1 too).
>> Sure. After reading the mode0, base0 and base1 from QFPROM, we can call
>> a function in a loop to setup the calibration for each sensor.
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>    static const struct tsens_ops ops_generic_v2 = {
>>>>        .init        = init_common,
>>>>        .get_temp    = get_temp_tsens_valid,
>>>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
>>>> index 98c356acfe98..169690355dad 100644
>>>> --- a/drivers/thermal/qcom/tsens.c
>>>> +++ b/drivers/thermal/qcom/tsens.c
>>>> @@ -974,7 +974,7 @@ int __init init_common(struct tsens_priv *priv)
>>>>        ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
>>>>        if (ret)
>>>>            goto err_put_device;
>>>> -    if (!enabled) {
>>>> +    if (!enabled && !priv->sensors_to_en) {
>>>>            dev_err(dev, "%s: device not enabled\n", __func__);
>>>>            ret = -ENODEV;
>>>>            goto err_put_device;
>>>> @@ -1006,6 +1006,40 @@ int __init init_common(struct tsens_priv *priv)
>>>>            goto err_put_device;
>>>>        }
>>>>    +    /* Do TSENS initialization if required */
>>>> +    if (priv->sensors_to_en) {
>>> Maybe it would be better to explicitly add VER_2_X_NO_RPM and check it
>>> here?
>> Sure, will add a separate version macro.
>>>> +        priv->rf[CODE_OR_TEMP] = devm_regmap_field_alloc(dev,
>>>> priv->srot_map,
>>>> + priv->fields[CODE_OR_TEMP]);
>>>> +        if (IS_ERR(priv->rf[CODE_OR_TEMP])) {
>>>> +            ret = PTR_ERR(priv->rf[CODE_OR_TEMP]);
>>>> +            goto err_put_device;
>>>> +        }
>>>> +
>>>> +        priv->rf[MAIN_MEASURE_PERIOD] =
>>>> +            devm_regmap_field_alloc(dev, priv->srot_map,
>>>> +                        priv->fields[MAIN_MEASURE_PERIOD]);
>>>> +        if (IS_ERR(priv->rf[MAIN_MEASURE_PERIOD])) {
>>>> +            ret = PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
>>>> +            goto err_put_device;
>>>> +        }
>>>> +
>>>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x1);
>>>> +
>>>> +        /* Update measure period to 2ms */
>>>> +        regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], 0x1);
>>>> +
>>>> +        /* Enable available sensors */
>>>> +        regmap_field_write(priv->rf[SENSOR_EN], priv->sensors_to_en);
>>>> +
>>>> +        /* Real temperature format */
>>>> +        regmap_field_write(priv->rf[CODE_OR_TEMP], 0x1);
>>>> +
>>>> +        regmap_field_write(priv->rf[TSENS_SW_RST], 0x0);
>>>> +
>>>> +        /* Enable TSENS */
>>>> +        regmap_field_write(priv->rf[TSENS_EN], 0x1);
>>>> +    }
>>>> +
>>>>        /* This loop might need changes if enum regfield_ids is
>>>> reordered */
>>>>        for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
>>>>            for (i = 0; i < priv->feat->max_sensors; i++) {
>>>> @@ -1282,6 +1316,7 @@ static int tsens_probe(struct platform_device
>>>> *pdev)
>>>>          priv->dev = dev;
>>>>        priv->num_sensors = num_sensors;
>>>> +    priv->sensors_to_en = data->sensors_to_en;
>>>>        priv->ops = data->ops;
>>>>        for (i = 0;  i < priv->num_sensors; i++) {
>>>>            if (data->hw_ids)
>>>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
>>>> index 2805de1c6827..f8897bc8944e 100644
>>>> --- a/drivers/thermal/qcom/tsens.h
>>>> +++ b/drivers/thermal/qcom/tsens.h
>>>> @@ -168,6 +168,58 @@ enum regfield_ids {
>>>>        TSENS_SW_RST,
>>>>        SENSOR_EN,
>>>>        CODE_OR_TEMP,
>>>> +    /* MEASURE_PERIOD */
>>>> +    MAIN_MEASURE_PERIOD,
>>>> +
>>>> +    /* Sn_CONVERSION */
>>>> +    SHIFT_0,
>>>> +    SLOPE_0,
>>>> +    CZERO_0,
>>>> +    SHIFT_1,
>>>> +    SLOPE_1,
>>>> +    CZERO_1,
>>>> +    SHIFT_2,
>>>> +    SLOPE_2,
>>>> +    CZERO_2,
>>>> +    SHIFT_3,
>>>> +    SLOPE_3,
>>>> +    CZERO_3,
>>>> +    SHIFT_4,
>>>> +    SLOPE_4,
>>>> +    CZERO_4,
>>>> +    SHIFT_5,
>>>> +    SLOPE_5,
>>>> +    CZERO_5,
>>>> +    SHIFT_6,
>>>> +    SLOPE_6,
>>>> +    CZERO_6,
>>>> +    SHIFT_7,
>>>> +    SLOPE_7,
>>>> +    CZERO_7,
>>>> +    SHIFT_8,
>>>> +    SLOPE_8,
>>>> +    CZERO_8,
>>>> +    SHIFT_9,
>>>> +    SLOPE_9,
>>>> +    CZERO_9,
>>>> +    SHIFT_10,
>>>> +    SLOPE_10,
>>>> +    CZERO_10,
>>>> +    SHIFT_11,
>>>> +    SLOPE_11,
>>>> +    CZERO_11,
>>>> +    SHIFT_12,
>>>> +    SLOPE_12,
>>>> +    CZERO_12,
>>>> +    SHIFT_13,
>>>> +    SLOPE_13,
>>>> +    CZERO_13,
>>>> +    SHIFT_14,
>>>> +    SLOPE_14,
>>>> +    CZERO_14,
>>>> +    SHIFT_15,
>>>> +    SLOPE_15,
>>>> +    CZERO_15,
>>>>          /* ----- TM ------ */
>>>>        /* TRDY */
>>>> @@ -524,6 +576,7 @@ struct tsens_features {
>>>>    /**
>>>>     * struct tsens_plat_data - tsens compile-time platform data
>>>>     * @num_sensors: Number of sensors supported by platform
>>>> + * @sensors_to_en: Sensors to be enabled. Each bit represent a sensor
>>>>     * @ops: operations the tsens instance supports
>>>>     * @hw_ids: Subset of sensors ids supported by platform, if not the
>>>> first n
>>>>     * @feat: features of the IP
>>>> @@ -531,6 +584,7 @@ struct tsens_features {
>>>>     */
>>>>    struct tsens_plat_data {
>>>>        const u32        num_sensors;
>>>> +    const u16        sensors_to_en;
>>> There is already a similar field, hw_ids. Can it be used instead?
>> Yes, it can be used. I missed to check this hw_ids. Will change the
>> num_sensors to 5 and use the hw_ids.
>>>>        const struct tsens_ops    *ops;
>>>>        unsigned int        *hw_ids;
>>>>        struct tsens_features    *feat;
>>>> @@ -551,6 +605,7 @@ struct tsens_context {
>>>>     * struct tsens_priv - private data for each instance of the tsens IP
>>>>     * @dev: pointer to struct device
>>>>     * @num_sensors: number of sensors enabled on this device
>>>> + * @sensors_to_en: sensors to be enabled. Each bit represents a sensor
>>>>     * @tm_map: pointer to TM register address space
>>>>     * @srot_map: pointer to SROT register address space
>>>>     * @tm_offset: deal with old device trees that don't address TM and
>>>> SROT
>>>> @@ -569,6 +624,7 @@ struct tsens_context {
>>>>    struct tsens_priv {
>>>>        struct device            *dev;
>>>>        u32                num_sensors;
>>>> +    u16                sensors_to_en;
>>>>        struct regmap            *tm_map;
>>>>        struct regmap            *srot_map;
>>>>        u32                tm_offset;
>> --
>> Thanks,
>> Praveenkumar
>
>
  

Patch

diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index 29a61d2d6ca3..db48b1d95348 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -6,11 +6,20 @@ 
 
 #include <linux/bitops.h>
 #include <linux/regmap.h>
+#include <linux/nvmem-consumer.h>
 #include "tsens.h"
 
 /* ----- SROT ------ */
 #define SROT_HW_VER_OFF	0x0000
 #define SROT_CTRL_OFF		0x0004
+#define SROT_MEASURE_PERIOD	0x0008
+#define SROT_Sn_CONVERSION	0x0060
+#define V2_SHIFT_DEFAULT	0x0003
+#define V2_SLOPE_DEFAULT	0x0cd0
+#define V2_CZERO_DEFAULT	0x016a
+#define ONE_PT_SLOPE		0x0cd0
+#define TWO_PT_SHIFTED_GAIN	921600
+#define ONE_PT_CZERO_CONST	94
 
 /* ----- TM ------ */
 #define TM_INT_EN_OFF			0x0004
@@ -59,6 +68,16 @@  static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
 	/* CTRL_OFF */
 	[TSENS_EN]     = REG_FIELD(SROT_CTRL_OFF,    0,  0),
 	[TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF,    1,  1),
+	[SENSOR_EN]    = REG_FIELD(SROT_CTRL_OFF,    3,  18),
+	[CODE_OR_TEMP] = REG_FIELD(SROT_CTRL_OFF,    21, 21),
+
+	/* MAIN_MEASURE_PERIOD */
+	[MAIN_MEASURE_PERIOD] = REG_FIELD(SROT_MEASURE_PERIOD, 0, 7),
+
+	/* Sn Conversion */
+	REG_FIELD_FOR_EACH_SENSOR16(SHIFT, SROT_Sn_CONVERSION, 23, 24),
+	REG_FIELD_FOR_EACH_SENSOR16(SLOPE, SROT_Sn_CONVERSION, 10, 22),
+	REG_FIELD_FOR_EACH_SENSOR16(CZERO, SROT_Sn_CONVERSION, 0, 9),
 
 	/* ----- TM ------ */
 	/* INTERRUPT ENABLE */
@@ -104,6 +123,103 @@  static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
 	[TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
 };
 
+static int tsens_v2_calibration(struct tsens_priv *priv)
+{
+	struct device *dev = priv->dev;
+	u32 mode, base0, base1;
+	u32 slope, czero;
+	char name[15];
+	int i, j, ret;
+
+	if (priv->num_sensors > MAX_SENSORS)
+		return -EINVAL;
+
+	ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
+	if (ret == -ENOENT)
+		dev_warn(priv->dev, "Calibration data not present in DT\n");
+	if (ret < 0)
+		return ret;
+
+	dev_dbg(priv->dev, "calibration mode is %d\n", mode);
+
+	ret = nvmem_cell_read_variable_le_u32(priv->dev, "base0", &base0);
+	if (ret < 0)
+		return ret;
+
+	ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
+	if (ret < 0)
+		return ret;
+
+	/* Read offset values and allocate SHIFT, SLOPE & CZERO regmap for enabled sensors */
+	for (i = 0; i < priv->num_sensors; i++) {
+		if (!(priv->sensors_to_en & (0x1 << i)))
+			continue;
+
+		ret = snprintf(name, sizeof(name), "s%d_offset", priv->sensor[i].hw_id);
+		if (ret < 0)
+			return ret;
+
+		ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &priv->sensor[i].offset);
+		if (ret)
+			return ret;
+
+		for (j = SHIFT_0; j <= CZERO_0; j++) {
+			int idx = (i * 3) + j;
+
+			priv->rf[idx] = devm_regmap_field_alloc(dev, priv->srot_map,
+								priv->fields[idx]);
+			if (IS_ERR(priv->rf[idx]))
+				return PTR_ERR(priv->rf[idx]);
+		}
+	}
+
+	/* Based on calib mode, program SHIFT, SLOPE and CZERO for enabled sensors */
+	switch (mode) {
+	case TWO_PT_CALIB:
+		slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
+
+		for (i = 0; i < priv->num_sensors; i++) {
+			if (!(priv->sensors_to_en & (0x1 << i)))
+				continue;
+
+			int idx = i * 3;
+
+			czero = (base0 + priv->sensor[i].offset - ((base1 - base0) / 3));
+			regmap_field_write(priv->rf[SHIFT_0 + idx], V2_SHIFT_DEFAULT);
+			regmap_field_write(priv->rf[SLOPE_0 + idx], slope);
+			regmap_field_write(priv->rf[CZERO_0 + idx], czero);
+		}
+		fallthrough;
+	case ONE_PT_CALIB2:
+		for (i = 0; i < priv->num_sensors; i++) {
+			if (!(priv->sensors_to_en & (0x1 << i)))
+				continue;
+
+			int idx = i * 3;
+
+			czero = base0 + priv->sensor[i].offset - ONE_PT_CZERO_CONST;
+			regmap_field_write(priv->rf[SHIFT_0 + idx], V2_SHIFT_DEFAULT);
+			regmap_field_write(priv->rf[SLOPE_0 + idx], ONE_PT_SLOPE);
+			regmap_field_write(priv->rf[CZERO_0 + idx], czero);
+		}
+		break;
+	default:
+		dev_dbg(priv->dev, "calibrationless mode\n");
+		for (i = 0; i < priv->num_sensors; i++) {
+			if (!(priv->sensors_to_en & (0x1 << i)))
+				continue;
+
+			int idx = i * 3;
+
+			regmap_field_write(priv->rf[SHIFT_0 + idx], V2_SHIFT_DEFAULT);
+			regmap_field_write(priv->rf[SLOPE_0 + idx], V2_SLOPE_DEFAULT);
+			regmap_field_write(priv->rf[CZERO_0 + idx], V2_CZERO_DEFAULT);
+		}
+	}
+
+	return 0;
+}
+
 static const struct tsens_ops ops_generic_v2 = {
 	.init		= init_common,
 	.get_temp	= get_temp_tsens_valid,
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 98c356acfe98..169690355dad 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -974,7 +974,7 @@  int __init init_common(struct tsens_priv *priv)
 	ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
 	if (ret)
 		goto err_put_device;
-	if (!enabled) {
+	if (!enabled && !priv->sensors_to_en) {
 		dev_err(dev, "%s: device not enabled\n", __func__);
 		ret = -ENODEV;
 		goto err_put_device;
@@ -1006,6 +1006,40 @@  int __init init_common(struct tsens_priv *priv)
 		goto err_put_device;
 	}
 
+	/* Do TSENS initialization if required */
+	if (priv->sensors_to_en) {
+		priv->rf[CODE_OR_TEMP] = devm_regmap_field_alloc(dev, priv->srot_map,
+								 priv->fields[CODE_OR_TEMP]);
+		if (IS_ERR(priv->rf[CODE_OR_TEMP])) {
+			ret = PTR_ERR(priv->rf[CODE_OR_TEMP]);
+			goto err_put_device;
+		}
+
+		priv->rf[MAIN_MEASURE_PERIOD] =
+			devm_regmap_field_alloc(dev, priv->srot_map,
+						priv->fields[MAIN_MEASURE_PERIOD]);
+		if (IS_ERR(priv->rf[MAIN_MEASURE_PERIOD])) {
+			ret = PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
+			goto err_put_device;
+		}
+
+		regmap_field_write(priv->rf[TSENS_SW_RST], 0x1);
+
+		/* Update measure period to 2ms */
+		regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], 0x1);
+
+		/* Enable available sensors */
+		regmap_field_write(priv->rf[SENSOR_EN], priv->sensors_to_en);
+
+		/* Real temperature format */
+		regmap_field_write(priv->rf[CODE_OR_TEMP], 0x1);
+
+		regmap_field_write(priv->rf[TSENS_SW_RST], 0x0);
+
+		/* Enable TSENS */
+		regmap_field_write(priv->rf[TSENS_EN], 0x1);
+	}
+
 	/* This loop might need changes if enum regfield_ids is reordered */
 	for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
 		for (i = 0; i < priv->feat->max_sensors; i++) {
@@ -1282,6 +1316,7 @@  static int tsens_probe(struct platform_device *pdev)
 
 	priv->dev = dev;
 	priv->num_sensors = num_sensors;
+	priv->sensors_to_en = data->sensors_to_en;
 	priv->ops = data->ops;
 	for (i = 0;  i < priv->num_sensors; i++) {
 		if (data->hw_ids)
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 2805de1c6827..f8897bc8944e 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -168,6 +168,58 @@  enum regfield_ids {
 	TSENS_SW_RST,
 	SENSOR_EN,
 	CODE_OR_TEMP,
+	/* MEASURE_PERIOD */
+	MAIN_MEASURE_PERIOD,
+
+	/* Sn_CONVERSION */
+	SHIFT_0,
+	SLOPE_0,
+	CZERO_0,
+	SHIFT_1,
+	SLOPE_1,
+	CZERO_1,
+	SHIFT_2,
+	SLOPE_2,
+	CZERO_2,
+	SHIFT_3,
+	SLOPE_3,
+	CZERO_3,
+	SHIFT_4,
+	SLOPE_4,
+	CZERO_4,
+	SHIFT_5,
+	SLOPE_5,
+	CZERO_5,
+	SHIFT_6,
+	SLOPE_6,
+	CZERO_6,
+	SHIFT_7,
+	SLOPE_7,
+	CZERO_7,
+	SHIFT_8,
+	SLOPE_8,
+	CZERO_8,
+	SHIFT_9,
+	SLOPE_9,
+	CZERO_9,
+	SHIFT_10,
+	SLOPE_10,
+	CZERO_10,
+	SHIFT_11,
+	SLOPE_11,
+	CZERO_11,
+	SHIFT_12,
+	SLOPE_12,
+	CZERO_12,
+	SHIFT_13,
+	SLOPE_13,
+	CZERO_13,
+	SHIFT_14,
+	SLOPE_14,
+	CZERO_14,
+	SHIFT_15,
+	SLOPE_15,
+	CZERO_15,
 
 	/* ----- TM ------ */
 	/* TRDY */
@@ -524,6 +576,7 @@  struct tsens_features {
 /**
  * struct tsens_plat_data - tsens compile-time platform data
  * @num_sensors: Number of sensors supported by platform
+ * @sensors_to_en: Sensors to be enabled. Each bit represent a sensor
  * @ops: operations the tsens instance supports
  * @hw_ids: Subset of sensors ids supported by platform, if not the first n
  * @feat: features of the IP
@@ -531,6 +584,7 @@  struct tsens_features {
  */
 struct tsens_plat_data {
 	const u32		num_sensors;
+	const u16		sensors_to_en;
 	const struct tsens_ops	*ops;
 	unsigned int		*hw_ids;
 	struct tsens_features	*feat;
@@ -551,6 +605,7 @@  struct tsens_context {
  * struct tsens_priv - private data for each instance of the tsens IP
  * @dev: pointer to struct device
  * @num_sensors: number of sensors enabled on this device
+ * @sensors_to_en: sensors to be enabled. Each bit represents a sensor
  * @tm_map: pointer to TM register address space
  * @srot_map: pointer to SROT register address space
  * @tm_offset: deal with old device trees that don't address TM and SROT
@@ -569,6 +624,7 @@  struct tsens_context {
 struct tsens_priv {
 	struct device			*dev;
 	u32				num_sensors;
+	u16				sensors_to_en;
 	struct regmap			*tm_map;
 	struct regmap			*srot_map;
 	u32				tm_offset;