mbox series

[v6,0/7] Add TSENS support for IPQ5332, IPQ5424

Message ID 20241104124413.2012794-1-quic_mmanikan@quicinc.com
Headers show
Series Add TSENS support for IPQ5332, IPQ5424 | expand

Message

Manikanta Mylavarapu Nov. 4, 2024, 12:44 p.m. UTC
IPQ5332 uses tsens v2.3.3 IP with combined interrupt for
upper/lower and critical. IPQ5332 does not have RPM and
kernel has to take care of TSENS enablement and calibration.

IPQ5424 also uses same tsens v2.3.3 IP and it's similar to IPQ5332
(no RPM) hence add IPQ5424 support in this series itself.

This patch series adds the temperature sensor enablement,
calibration support for IPQ5332 and IPQ5424.

Depends On:
https://lore.kernel.org/linux-arm-msm/20241028060506.246606-1-quic_srichara@quicinc.com/

Changes in V6:
	- Patches are re-ordered as follows
	  1) dt-bindings
	  2) driver
	  3) dts
	- Patch #1 and Patch #5 from V5 have been merged and incorporated into
	  Patch #3 in V6
	- Added TSENS support for IPQ5424
	- Detailed change logs are added to the respective patches

V5 can be found at:
https://lore.kernel.org/linux-arm-msm/20230721054619.2366510-1-quic_ipkumar@quicinc.com/

V4 can be found at:
https://lore.kernel.org/linux-arm-msm/20230719104041.126718-1-quic_ipkumar@quicinc.com/

V3 can be found at:
https://lore.kernel.org/linux-arm-msm/20230713052732.787853-1-quic_ipkumar@quicinc.com/

V2 can be found at:
https://lore.kernel.org/linux-arm-msm/20230712113539.4029941-1-quic_ipkumar@quicinc.com/

Manikanta Mylavarapu (3):
  dt-bindings: nvmem: Add compatible for IPQ5424
  arm64: dts: qcom: ipq5424: Add tsens node
  arm64: dts: qcom: ipq5424: Add thermal zone nodes

Praveenkumar I (4):
  dt-bindings: thermal: tsens: Add ipq5332, ipq5424 compatible
  thermal/drivers/tsens: Add TSENS enable and calibration support for V2
  arm64: dts: qcom: ipq5332: Add tsens node
  arm64: dts: qcom: ipq5332: Add thermal zone nodes

 .../bindings/nvmem/qcom,qfprom.yaml           |   1 +
 .../bindings/thermal/qcom-tsens.yaml          |  18 ++
 arch/arm64/boot/dts/qcom/ipq5332.dtsi         | 144 ++++++++++++
 arch/arm64/boot/dts/qcom/ipq5424.dtsi         | 211 ++++++++++++++++++
 drivers/thermal/qcom/tsens-v2.c               | 182 +++++++++++++++
 drivers/thermal/qcom/tsens.c                  |   8 +-
 drivers/thermal/qcom/tsens.h                  |   4 +-
 7 files changed, 566 insertions(+), 2 deletions(-)


base-commit: 1ffec08567f426a1c593e038cadc61bdc38cb467
prerequisite-patch-id: 1090fe9cee19a52dc8595e2fecef659199828246
prerequisite-patch-id: 491c9f6c32738c4eb4398962c1ae7c625cd43238
prerequisite-patch-id: 1651c75547b539eb46eb4d02630e364f262860bf
prerequisite-patch-id: 7ce54f0af6083e897067a7e5cd9561198f3d4d41
prerequisite-patch-id: 0a04fdee4b5b76cd5b734c666f7c8f5561e3e9d8
prerequisite-patch-id: 084f6dced27c39b600711dde2f797b43393cde73

Comments

Konrad Dybcio Nov. 4, 2024, 1:51 p.m. UTC | #1
On 4.11.2024 1:44 PM, Manikanta Mylavarapu wrote:
> Add thermal zone nodes for sensors present in IPQ5424.
> 
> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
> ---
[...]

> +
> +		cpu3-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +			thermal-sensors = <&tsens 13>;
> +
> +			trips {
> +				cpu-critical {
> +					temperature = <120000>;
> +					hysteresis = <9000>;
> +					type = "critical";
> +				};
> +
> +				cpu-passive {
> +					temperature = <110000>;
> +					hysteresis = <9000>;
> +					type = "passive";

You have a passive trip point without passive polling

> +				};
> +			};
> +		};
> +
> +		wcss-tile2-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;

These are the defaults

> +			thermal-sensors = <&tsens 9>;
> +
> +			trips {
> +				wcss_tile2-critical {

Node names must not contain underscores

Konrad
Dmitry Baryshkov Nov. 4, 2024, 7:42 p.m. UTC | #2
On Mon, Nov 04, 2024 at 06:14:09PM +0530, Manikanta Mylavarapu wrote:
> From: Praveenkumar I <quic_ipkumar@quicinc.com>
> 
> SoCs without RPM need to enable sensors and calibrate them from the kernel.
> The IPQ5332 and IPQ5424 use the tsens v2.3.3 IP and do not have RPM.
> Therefore, add a new calibration function for V2, as the tsens.c calib
> function only supports V1. Also add new feature_config, ops and data for
> IPQ5332, IPQ5424.
> 
> Although the TSENS IP supports 16 sensors, not all are used. The hw_id
> is used to enable the relevant sensors.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
> ---
> Changes in V6:
> 	- Add IPQ5424 support
> 	- Patch #1 and Patch #5 from V5 have been merged and
> 	  incorporated into Patch #3 in V6 to resolve below
> 	  compilation warnings.
> drivers/thermal/qcom/tsens-v2.c:216:19: warning: ‘init_tsens_v2_no_rpm’
> defined but not used [-Wunused-function]
>  static int __init init_tsens_v2_no_rpm(struct tsens_priv *priv)
> drivers/thermal/qcom/tsens-v2.c:180:12: warning: ‘tsens_v2_calibration’
> defined but not used [-Wunused-function]
>  static int tsens_v2_calibration(struct tsens_priv *priv)
> 
>  drivers/thermal/qcom/tsens-v2.c | 182 ++++++++++++++++++++++++++++++++
>  drivers/thermal/qcom/tsens.c    |   8 +-
>  drivers/thermal/qcom/tsens.h    |   4 +-
>  3 files changed, 192 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index 0cb7301eca6e..90b6915607ea 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -4,13 +4,32 @@
>   * Copyright (c) 2018, Linaro Limited
>   */
>  
> +#include <linux/bitfield.h>
>  #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
> +#define SW_RST_DEASSERT		0x0
> +#define SW_RST_ASSERT		0x1
> +#define MEASURE_PERIOD_2mSEC	0x1
> +#define RSEULT_FORMAT_TEMP	0x1
> +#define TSENS_ENABLE		0x1
> +#define SENSOR_CONVERSION(n)	(((n) * 4) + SROT_Sn_CONVERSION)
> +#define CONVERSION_SHIFT_MASK	GENMASK(24, 23)
> +#define CONVERSION_SLOPE_MASK	GENMASK(22, 10)
> +#define CONVERSION_CZERO_MASK	GENMASK(9, 0)
>  
>  /* ----- TM ------ */
>  #define TM_INT_EN_OFF			0x0004
> @@ -50,6 +69,17 @@ static struct tsens_features ipq8074_feat = {
>  	.trip_max_temp	= 204000,
>  };
>  
> +static struct tsens_features ipq5332_feat = {
> +	.ver_major	= VER_2_X_NO_RPM,
> +	.crit_int	= 1,
> +	.combo_int	= 1,
> +	.adc		= 0,
> +	.srot_split	= 1,
> +	.max_sensors	= 16,
> +	.trip_min_temp	= 0,
> +	.trip_max_temp	= 204000,
> +};
> +
>  static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
>  	/* ----- SROT ------ */
>  	/* VERSION */
> @@ -59,6 +89,11 @@ 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),
>  
>  	/* ----- TM ------ */
>  	/* INTERRUPT ENABLE */
> @@ -104,6 +139,131 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
>  	[TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>  };
>  
> +static int tsens_v2_calibrate_sensor(struct device *dev, struct tsens_sensor *sensor,
> +				     struct regmap *map,  u32 mode, u32 base0, u32 base1)
> +{
> +	u32 slope, czero, val;
> +	char name[8];
> +	int ret;
> +
> +	/* Read offset value */
> +	ret = snprintf(name, sizeof(name), "s%d", sensor->hw_id);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = nvmem_cell_read_variable_le_u32(dev, name, &sensor->offset);
> +	if (ret)
> +		return ret;
> +
> +	/* Based on calib mode, program SHIFT, SLOPE and CZERO */
> +	switch (mode) {
> +	case TWO_PT_CALIB:
> +		slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
> +
> +		czero = (base0 + sensor->offset - ((base1 - base0) / 3));
> +
> +		val = FIELD_PREP(CONVERSION_SHIFT_MASK, V2_SHIFT_DEFAULT) |
> +		      FIELD_PREP(CONVERSION_SLOPE_MASK, slope) |
> +		      FIELD_PREP(CONVERSION_CZERO_MASK, czero);
> +
> +		fallthrough;
> +	case ONE_PT_CALIB2:
> +		czero = base0 + sensor->offset - ONE_PT_CZERO_CONST;
> +
> +		val = FIELD_PREP(CONVERSION_SHIFT_MASK, V2_SHIFT_DEFAULT) |
> +		      FIELD_PREP(CONVERSION_SLOPE_MASK, ONE_PT_SLOPE) |
> +		      FIELD_PREP(CONVERSION_CZERO_MASK, czero);
> +
> +		break;
> +	default:
> +		dev_dbg(dev, "calibrationless mode\n");
> +
> +		val = FIELD_PREP(CONVERSION_SHIFT_MASK, V2_SHIFT_DEFAULT) |
> +		      FIELD_PREP(CONVERSION_SLOPE_MASK, V2_SLOPE_DEFAULT) |
> +		      FIELD_PREP(CONVERSION_CZERO_MASK, V2_CZERO_DEFAULT);

Move val calculation out of switch-case, assign default values to shift,
slope and czero and then change them under switch-case.

> +	}
> +
> +	regmap_write(map, SENSOR_CONVERSION(sensor->hw_id), val);
> +
> +	return 0;
> +}
> +
> +static int tsens_v2_calibration(struct tsens_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	u32 mode, base0, base1;
> +	int i, 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);

Is this actually base0 / base1 or rather base1 / base2 as used by the
tsens v0.1 / v1 ?

> +	if (ret < 0)
> +		return ret;
> +
> +	/* Calibrate each sensor */
> +	for (i = 0; i < priv->num_sensors; i++) {
> +		ret = tsens_v2_calibrate_sensor(dev, &priv->sensor[i], priv->srot_map,
> +						mode, base0, base1);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int __init init_tsens_v2_no_rpm(struct tsens_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	int i, ret;
> +	u32 val = 0;
> +
> +	ret = init_common(priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	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]))
> +		return PTR_ERR(priv->rf[CODE_OR_TEMP]);
> +
> +	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]))
> +		return PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
> +
> +	regmap_field_write(priv->rf[TSENS_SW_RST], SW_RST_ASSERT);
> +
> +	regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], MEASURE_PERIOD_2mSEC);
> +
> +	/* Enable available sensors */
> +	for (i = 0; i < priv->num_sensors; i++)
> +		val |= 1 << priv->sensor[i].hw_id;
> +
> +	regmap_field_write(priv->rf[SENSOR_EN], val);
> +
> +	/* Select temperature format, unit is deci-Celsius */
> +	regmap_field_write(priv->rf[CODE_OR_TEMP], RSEULT_FORMAT_TEMP);
> +
> +	regmap_field_write(priv->rf[TSENS_SW_RST], SW_RST_DEASSERT);
> +
> +	regmap_field_write(priv->rf[TSENS_EN], TSENS_ENABLE);
> +
> +	return 0;
> +}
> +
>  static const struct tsens_ops ops_generic_v2 = {
>  	.init		= init_common,
>  	.get_temp	= get_temp_tsens_valid,
> @@ -122,6 +282,28 @@ struct tsens_plat_data data_ipq8074 = {
>  	.fields	= tsens_v2_regfields,
>  };
>  
> +static const struct tsens_ops ops_ipq5332 = {
> +	.init		= init_tsens_v2_no_rpm,
> +	.get_temp	= get_temp_tsens_valid,
> +	.calibrate	= tsens_v2_calibration,
> +};
> +
> +struct tsens_plat_data data_ipq5332 = {
> +	.num_sensors	= 5,
> +	.ops		= &ops_ipq5332,
> +	.hw_ids		= (unsigned int []){11, 12, 13, 14, 15},
> +	.feat		= &ipq5332_feat,
> +	.fields		= tsens_v2_regfields,
> +};
> +
> +struct tsens_plat_data data_ipq5424 = {
> +	.num_sensors	= 7,
> +	.ops		= &ops_ipq5332,
> +	.hw_ids		= (unsigned int []){9, 10, 11, 12, 13, 14, 15},
> +	.feat		= &ipq5332_feat,
> +	.fields		= tsens_v2_regfields,
> +};
> +
>  /* Kept around for backward compatibility with old msm8996.dtsi */
>  struct tsens_plat_data data_8996 = {
>  	.num_sensors	= 13,
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 0b4421bf4785..c5409225756f 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -975,7 +975,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 && (tsens_version(priv) != VER_2_X_NO_RPM)) {
>  		dev_err(dev, "%s: device not enabled\n", __func__);
>  		ret = -ENODEV;
>  		goto err_put_device;
> @@ -1102,6 +1102,12 @@ static SIMPLE_DEV_PM_OPS(tsens_pm_ops, tsens_suspend, tsens_resume);
>  
>  static const struct of_device_id tsens_table[] = {
>  	{
> +		.compatible = "qcom,ipq5332-tsens",
> +		.data = &data_ipq5332,
> +	}, {
> +		.compatible = "qcom,ipq5424-tsens",
> +		.data = &data_ipq5424,
> +	}, {
>  		.compatible = "qcom,ipq8064-tsens",
>  		.data = &data_8960,
>  	}, {
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index cab39de045b1..ac5358e48f74 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -35,6 +35,7 @@ enum tsens_ver {
>  	VER_0_1,
>  	VER_1_X,
>  	VER_2_X,
> +	VER_2_X_NO_RPM,
>  };
>  
>  enum tsens_irq_type {
> @@ -168,6 +169,7 @@ enum regfield_ids {
>  	TSENS_SW_RST,
>  	SENSOR_EN,
>  	CODE_OR_TEMP,
> +	MAIN_MEASURE_PERIOD,
>  
>  	/* ----- TM ------ */
>  	/* TRDY */
> @@ -650,6 +652,6 @@ extern struct tsens_plat_data data_8226, data_8909, data_8916, data_8939, data_8
>  extern struct tsens_plat_data data_tsens_v1, data_8976, data_8956;
>  
>  /* TSENS v2 targets */
> -extern struct tsens_plat_data data_8996, data_ipq8074, data_tsens_v2;
> +extern struct tsens_plat_data data_8996, data_ipq8074, data_ipq5332, data_ipq5424, data_tsens_v2;
>  
>  #endif /* __QCOM_TSENS_H__ */
> -- 
> 2.34.1
>
Krzysztof Kozlowski Nov. 5, 2024, 8:45 a.m. UTC | #3
On Mon, Nov 04, 2024 at 06:14:07PM +0530, Manikanta Mylavarapu wrote:
> From: Praveenkumar I <quic_ipkumar@quicinc.com>
> 
> The IPQ5332 and IPQ5424 use TSENS v2.3.3 IP with combined interrupt.
> RPM is not available in these SoCs, hence adding new compatible
> to have the sensor enablement and calibration function. Also add
> nvmem-cell-names.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
> ---
> Changes in V6:
> 	- Remove the R-b tag because support for IPQ5424 has been added
> 

I wish we rather finish with this patchset and add ipq5424 later, so we
won't have to re-review.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Manikanta Mylavarapu Nov. 6, 2024, 6:16 a.m. UTC | #4
On 11/5/2024 1:12 AM, Dmitry Baryshkov wrote:
> On Mon, Nov 04, 2024 at 06:14:09PM +0530, Manikanta Mylavarapu wrote:
>> From: Praveenkumar I <quic_ipkumar@quicinc.com>
>>
>> SoCs without RPM need to enable sensors and calibrate them from the kernel.
>> The IPQ5332 and IPQ5424 use the tsens v2.3.3 IP and do not have RPM.
>> Therefore, add a new calibration function for V2, as the tsens.c calib
>> function only supports V1. Also add new feature_config, ops and data for
>> IPQ5332, IPQ5424.
>>
>> Although the TSENS IP supports 16 sensors, not all are used. The hw_id
>> is used to enable the relevant sensors.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>> ---
>> Changes in V6:
>> 	- Add IPQ5424 support
>> 	- Patch #1 and Patch #5 from V5 have been merged and
>> 	  incorporated into Patch #3 in V6 to resolve below
>> 	  compilation warnings.
>> drivers/thermal/qcom/tsens-v2.c:216:19: warning: ‘init_tsens_v2_no_rpm’
>> defined but not used [-Wunused-function]
>>  static int __init init_tsens_v2_no_rpm(struct tsens_priv *priv)
>> drivers/thermal/qcom/tsens-v2.c:180:12: warning: ‘tsens_v2_calibration’
>> defined but not used [-Wunused-function]
>>  static int tsens_v2_calibration(struct tsens_priv *priv)
>>
>>  drivers/thermal/qcom/tsens-v2.c | 182 ++++++++++++++++++++++++++++++++
>>  drivers/thermal/qcom/tsens.c    |   8 +-
>>  drivers/thermal/qcom/tsens.h    |   4 +-
>>  3 files changed, 192 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
>> index 0cb7301eca6e..90b6915607ea 100644
>> --- a/drivers/thermal/qcom/tsens-v2.c
>> +++ b/drivers/thermal/qcom/tsens-v2.c
>> @@ -4,13 +4,32 @@
>>   * Copyright (c) 2018, Linaro Limited
>>   */
>>  
>> +#include <linux/bitfield.h>
>>  #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
>> +#define SW_RST_DEASSERT		0x0
>> +#define SW_RST_ASSERT		0x1
>> +#define MEASURE_PERIOD_2mSEC	0x1
>> +#define RSEULT_FORMAT_TEMP	0x1
>> +#define TSENS_ENABLE		0x1
>> +#define SENSOR_CONVERSION(n)	(((n) * 4) + SROT_Sn_CONVERSION)
>> +#define CONVERSION_SHIFT_MASK	GENMASK(24, 23)
>> +#define CONVERSION_SLOPE_MASK	GENMASK(22, 10)
>> +#define CONVERSION_CZERO_MASK	GENMASK(9, 0)
>>  
>>  /* ----- TM ------ */
>>  #define TM_INT_EN_OFF			0x0004
>> @@ -50,6 +69,17 @@ static struct tsens_features ipq8074_feat = {
>>  	.trip_max_temp	= 204000,
>>  };
>>  
>> +static struct tsens_features ipq5332_feat = {
>> +	.ver_major	= VER_2_X_NO_RPM,
>> +	.crit_int	= 1,
>> +	.combo_int	= 1,
>> +	.adc		= 0,
>> +	.srot_split	= 1,
>> +	.max_sensors	= 16,
>> +	.trip_min_temp	= 0,
>> +	.trip_max_temp	= 204000,
>> +};
>> +
>>  static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
>>  	/* ----- SROT ------ */
>>  	/* VERSION */
>> @@ -59,6 +89,11 @@ 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),
>>  
>>  	/* ----- TM ------ */
>>  	/* INTERRUPT ENABLE */
>> @@ -104,6 +139,131 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
>>  	[TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0),
>>  };
>>  
>> +static int tsens_v2_calibrate_sensor(struct device *dev, struct tsens_sensor *sensor,
>> +				     struct regmap *map,  u32 mode, u32 base0, u32 base1)
>> +{
>> +	u32 slope, czero, val;
>> +	char name[8];
>> +	int ret;
>> +
>> +	/* Read offset value */
>> +	ret = snprintf(name, sizeof(name), "s%d", sensor->hw_id);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = nvmem_cell_read_variable_le_u32(dev, name, &sensor->offset);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Based on calib mode, program SHIFT, SLOPE and CZERO */
>> +	switch (mode) {
>> +	case TWO_PT_CALIB:
>> +		slope = (TWO_PT_SHIFTED_GAIN / (base1 - base0));
>> +
>> +		czero = (base0 + sensor->offset - ((base1 - base0) / 3));
>> +
>> +		val = FIELD_PREP(CONVERSION_SHIFT_MASK, V2_SHIFT_DEFAULT) |
>> +		      FIELD_PREP(CONVERSION_SLOPE_MASK, slope) |
>> +		      FIELD_PREP(CONVERSION_CZERO_MASK, czero);
>> +
>> +		fallthrough;
>> +	case ONE_PT_CALIB2:
>> +		czero = base0 + sensor->offset - ONE_PT_CZERO_CONST;
>> +
>> +		val = FIELD_PREP(CONVERSION_SHIFT_MASK, V2_SHIFT_DEFAULT) |
>> +		      FIELD_PREP(CONVERSION_SLOPE_MASK, ONE_PT_SLOPE) |
>> +		      FIELD_PREP(CONVERSION_CZERO_MASK, czero);
>> +
>> +		break;
>> +	default:
>> +		dev_dbg(dev, "calibrationless mode\n");
>> +
>> +		val = FIELD_PREP(CONVERSION_SHIFT_MASK, V2_SHIFT_DEFAULT) |
>> +		      FIELD_PREP(CONVERSION_SLOPE_MASK, V2_SLOPE_DEFAULT) |
>> +		      FIELD_PREP(CONVERSION_CZERO_MASK, V2_CZERO_DEFAULT);
> 
> Move val calculation out of switch-case, assign default values to shift,
> slope and czero and then change them under switch-case.
> 

Sure. I will update.

>> +	}
>> +
>> +	regmap_write(map, SENSOR_CONVERSION(sensor->hw_id), val);
>> +
>> +	return 0;
>> +}
>> +
>> +static int tsens_v2_calibration(struct tsens_priv *priv)
>> +{
>> +	struct device *dev = priv->dev;
>> +	u32 mode, base0, base1;
>> +	int i, 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);
> 
> Is this actually base0 / base1 or rather base1 / base2 as used by the
> tsens v0.1 / v1 ?
> 

As per qualcomm internal register guide, these registers are named as base0 and base1.

Thanks & Regards,
Manikanta.

>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* Calibrate each sensor */
>> +	for (i = 0; i < priv->num_sensors; i++) {
>> +		ret = tsens_v2_calibrate_sensor(dev, &priv->sensor[i], priv->srot_map,
>> +						mode, base0, base1);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int __init init_tsens_v2_no_rpm(struct tsens_priv *priv)
>> +{
>> +	struct device *dev = priv->dev;
>> +	int i, ret;
>> +	u32 val = 0;
>> +
>> +	ret = init_common(priv);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	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]))
>> +		return PTR_ERR(priv->rf[CODE_OR_TEMP]);
>> +
>> +	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]))
>> +		return PTR_ERR(priv->rf[MAIN_MEASURE_PERIOD]);
>> +
>> +	regmap_field_write(priv->rf[TSENS_SW_RST], SW_RST_ASSERT);
>> +
>> +	regmap_field_write(priv->rf[MAIN_MEASURE_PERIOD], MEASURE_PERIOD_2mSEC);
>> +
>> +	/* Enable available sensors */
>> +	for (i = 0; i < priv->num_sensors; i++)
>> +		val |= 1 << priv->sensor[i].hw_id;
>> +
>> +	regmap_field_write(priv->rf[SENSOR_EN], val);
>> +
>> +	/* Select temperature format, unit is deci-Celsius */
>> +	regmap_field_write(priv->rf[CODE_OR_TEMP], RSEULT_FORMAT_TEMP);
>> +
>> +	regmap_field_write(priv->rf[TSENS_SW_RST], SW_RST_DEASSERT);
>> +
>> +	regmap_field_write(priv->rf[TSENS_EN], TSENS_ENABLE);
>> +
>> +	return 0;
>> +}
>> +
>>  static const struct tsens_ops ops_generic_v2 = {
>>  	.init		= init_common,
>>  	.get_temp	= get_temp_tsens_valid,
>> @@ -122,6 +282,28 @@ struct tsens_plat_data data_ipq8074 = {
>>  	.fields	= tsens_v2_regfields,
>>  };
>>  
>> +static const struct tsens_ops ops_ipq5332 = {
>> +	.init		= init_tsens_v2_no_rpm,
>> +	.get_temp	= get_temp_tsens_valid,
>> +	.calibrate	= tsens_v2_calibration,
>> +};
>> +
>> +struct tsens_plat_data data_ipq5332 = {
>> +	.num_sensors	= 5,
>> +	.ops		= &ops_ipq5332,
>> +	.hw_ids		= (unsigned int []){11, 12, 13, 14, 15},
>> +	.feat		= &ipq5332_feat,
>> +	.fields		= tsens_v2_regfields,
>> +};
>> +
>> +struct tsens_plat_data data_ipq5424 = {
>> +	.num_sensors	= 7,
>> +	.ops		= &ops_ipq5332,
>> +	.hw_ids		= (unsigned int []){9, 10, 11, 12, 13, 14, 15},
>> +	.feat		= &ipq5332_feat,
>> +	.fields		= tsens_v2_regfields,
>> +};
>> +
>>  /* Kept around for backward compatibility with old msm8996.dtsi */
>>  struct tsens_plat_data data_8996 = {
>>  	.num_sensors	= 13,
>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
>> index 0b4421bf4785..c5409225756f 100644
>> --- a/drivers/thermal/qcom/tsens.c
>> +++ b/drivers/thermal/qcom/tsens.c
>> @@ -975,7 +975,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 && (tsens_version(priv) != VER_2_X_NO_RPM)) {
>>  		dev_err(dev, "%s: device not enabled\n", __func__);
>>  		ret = -ENODEV;
>>  		goto err_put_device;
>> @@ -1102,6 +1102,12 @@ static SIMPLE_DEV_PM_OPS(tsens_pm_ops, tsens_suspend, tsens_resume);
>>  
>>  static const struct of_device_id tsens_table[] = {
>>  	{
>> +		.compatible = "qcom,ipq5332-tsens",
>> +		.data = &data_ipq5332,
>> +	}, {
>> +		.compatible = "qcom,ipq5424-tsens",
>> +		.data = &data_ipq5424,
>> +	}, {
>>  		.compatible = "qcom,ipq8064-tsens",
>>  		.data = &data_8960,
>>  	}, {
>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
>> index cab39de045b1..ac5358e48f74 100644
>> --- a/drivers/thermal/qcom/tsens.h
>> +++ b/drivers/thermal/qcom/tsens.h
>> @@ -35,6 +35,7 @@ enum tsens_ver {
>>  	VER_0_1,
>>  	VER_1_X,
>>  	VER_2_X,
>> +	VER_2_X_NO_RPM,
>>  };
>>  
>>  enum tsens_irq_type {
>> @@ -168,6 +169,7 @@ enum regfield_ids {
>>  	TSENS_SW_RST,
>>  	SENSOR_EN,
>>  	CODE_OR_TEMP,
>> +	MAIN_MEASURE_PERIOD,
>>  
>>  	/* ----- TM ------ */
>>  	/* TRDY */
>> @@ -650,6 +652,6 @@ extern struct tsens_plat_data data_8226, data_8909, data_8916, data_8939, data_8
>>  extern struct tsens_plat_data data_tsens_v1, data_8976, data_8956;
>>  
>>  /* TSENS v2 targets */
>> -extern struct tsens_plat_data data_8996, data_ipq8074, data_tsens_v2;
>> +extern struct tsens_plat_data data_8996, data_ipq8074, data_ipq5332, data_ipq5424, data_tsens_v2;
>>  
>>  #endif /* __QCOM_TSENS_H__ */
>> -- 
>> 2.34.1
>>
>
Dmitry Baryshkov Nov. 6, 2024, 6:24 a.m. UTC | #5
On Wed, Nov 06, 2024 at 11:46:26AM +0530, Manikanta Mylavarapu wrote:
> 
> 
> On 11/5/2024 1:12 AM, Dmitry Baryshkov wrote:
> > On Mon, Nov 04, 2024 at 06:14:09PM +0530, Manikanta Mylavarapu wrote:

> >> +static int tsens_v2_calibration(struct tsens_priv *priv)
> >> +{
> >> +	struct device *dev = priv->dev;
> >> +	u32 mode, base0, base1;
> >> +	int i, 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);
> > 
> > Is this actually base0 / base1 or rather base1 / base2 as used by the
> > tsens v0.1 / v1 ?
> > 
> 
> As per qualcomm internal register guide, these registers are named as base0 and base1.

Ack

> 
> Thanks & Regards,
> Manikanta.
Manikanta Mylavarapu Nov. 6, 2024, 8:47 a.m. UTC | #6
On 11/4/2024 7:21 PM, Konrad Dybcio wrote:
> On 4.11.2024 1:44 PM, Manikanta Mylavarapu wrote:
>> Add thermal zone nodes for sensors present in IPQ5424.
>>
>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>> ---
> [...]
> 
>> +
>> +		cpu3-thermal {
>> +			polling-delay-passive = <0>;
>> +			polling-delay = <0>;
>> +			thermal-sensors = <&tsens 13>;
>> +
>> +			trips {
>> +				cpu-critical {
>> +					temperature = <120000>;
>> +					hysteresis = <9000>;
>> +					type = "critical";
>> +				};
>> +
>> +				cpu-passive {
>> +					temperature = <110000>;
>> +					hysteresis = <9000>;
>> +					type = "passive";
> 
> You have a passive trip point without passive polling
> 

Okay, will remove this.

>> +				};
>> +			};
>> +		};
>> +
>> +		wcss-tile2-thermal {
>> +			polling-delay-passive = <0>;
>> +			polling-delay = <0>;
> 
> These are the defaults
> 

Okay, sure. I will remove.

>> +			thermal-sensors = <&tsens 9>;
>> +
>> +			trips {
>> +				wcss_tile2-critical {
> 
> Node names must not contain underscores
> 

Okay, sure. I will remove.

Thanks & Regards,
Manikanta.
Konrad Dybcio Nov. 6, 2024, 9:12 a.m. UTC | #7
On 11/6/24 09:47, Manikanta Mylavarapu wrote:
> 
> 
> On 11/4/2024 7:21 PM, Konrad Dybcio wrote:
>> On 4.11.2024 1:44 PM, Manikanta Mylavarapu wrote:
>>> Add thermal zone nodes for sensors present in IPQ5424.
>>>
>>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>>> ---
>> [...]
>>
>>> +
>>> +		cpu3-thermal {
>>> +			polling-delay-passive = <0>;
>>> +			polling-delay = <0>;
>>> +			thermal-sensors = <&tsens 13>;
>>> +
>>> +			trips {
>>> +				cpu-critical {
>>> +					temperature = <120000>;
>>> +					hysteresis = <9000>;
>>> +					type = "critical";
>>> +				};
>>> +
>>> +				cpu-passive {
>>> +					temperature = <110000>;
>>> +					hysteresis = <9000>;
>>> +					type = "passive";
>>
>> You have a passive trip point without passive polling
>>
> 
> Okay, will remove this.

You most likely want to preserve it, while keeping a sensible
polling frequency, so that userspace will be aware of the current
CPU temperature. <100> sounds like a sensible value here.

Konrad
Manikanta Mylavarapu Nov. 6, 2024, 10:25 a.m. UTC | #8
On 11/6/2024 2:42 PM, Konrad Dybcio wrote:
> 
> 
> On 11/6/24 09:47, Manikanta Mylavarapu wrote:
>>
>>
>> On 11/4/2024 7:21 PM, Konrad Dybcio wrote:
>>> On 4.11.2024 1:44 PM, Manikanta Mylavarapu wrote:
>>>> Add thermal zone nodes for sensors present in IPQ5424.
>>>>
>>>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>>>> ---
>>> [...]
>>>
>>>> +
>>>> +        cpu3-thermal {
>>>> +            polling-delay-passive = <0>;
>>>> +            polling-delay = <0>;
>>>> +            thermal-sensors = <&tsens 13>;
>>>> +
>>>> +            trips {
>>>> +                cpu-critical {
>>>> +                    temperature = <120000>;
>>>> +                    hysteresis = <9000>;
>>>> +                    type = "critical";
>>>> +                };
>>>> +
>>>> +                cpu-passive {
>>>> +                    temperature = <110000>;
>>>> +                    hysteresis = <9000>;
>>>> +                    type = "passive";
>>>
>>> You have a passive trip point without passive polling
>>>
>>
>> Okay, will remove this.
> 
> You most likely want to preserve it, while keeping a sensible
> polling frequency, so that userspace will be aware of the current
> CPU temperature. <100> sounds like a sensible value here.
> 
> Konrad

Temperature sensor's present in IPQ5424 supports interrupts.
Hence no need to configure polling frequency.
I will remove polling-delay-passive/polling-delay.

Thanks & Regards,
Manikanta.
Konrad Dybcio Nov. 7, 2024, 2:47 p.m. UTC | #9
On 6.11.2024 11:25 AM, Manikanta Mylavarapu wrote:
> 
> 
> On 11/6/2024 2:42 PM, Konrad Dybcio wrote:
>>
>>
>> On 11/6/24 09:47, Manikanta Mylavarapu wrote:
>>>
>>>
>>> On 11/4/2024 7:21 PM, Konrad Dybcio wrote:
>>>> On 4.11.2024 1:44 PM, Manikanta Mylavarapu wrote:
>>>>> Add thermal zone nodes for sensors present in IPQ5424.
>>>>>
>>>>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>>>>> ---
>>>> [...]
>>>>
>>>>> +
>>>>> +        cpu3-thermal {
>>>>> +            polling-delay-passive = <0>;
>>>>> +            polling-delay = <0>;
>>>>> +            thermal-sensors = <&tsens 13>;
>>>>> +
>>>>> +            trips {
>>>>> +                cpu-critical {
>>>>> +                    temperature = <120000>;
>>>>> +                    hysteresis = <9000>;
>>>>> +                    type = "critical";
>>>>> +                };
>>>>> +
>>>>> +                cpu-passive {
>>>>> +                    temperature = <110000>;
>>>>> +                    hysteresis = <9000>;
>>>>> +                    type = "passive";
>>>>
>>>> You have a passive trip point without passive polling
>>>>
>>>
>>> Okay, will remove this.
>>
>> You most likely want to preserve it, while keeping a sensible
>> polling frequency, so that userspace will be aware of the current
>> CPU temperature. <100> sounds like a sensible value here.
>>
>> Konrad
> 
> Temperature sensor's present in IPQ5424 supports interrupts.

Correct.

> Hence no need to configure polling frequency.

No, that interrupt firing signifies crossing the temp threshold (meaning
no updates beyond that) or the tsens watchdog barking.

Konrad
Manikanta Mylavarapu Nov. 15, 2024, 10:38 a.m. UTC | #10
On 11/12/2024 4:32 PM, Konrad Dybcio wrote:
> 
> 
> On 11-Nov-24 12:51, Manikanta Mylavarapu wrote:
>>
>>
>> On 11/7/2024 8:17 PM, Konrad Dybcio wrote:
>>> On 6.11.2024 11:25 AM, Manikanta Mylavarapu wrote:
>>>>
>>>>
>>>> On 11/6/2024 2:42 PM, Konrad Dybcio wrote:
>>>>>
>>>>>
>>>>> On 11/6/24 09:47, Manikanta Mylavarapu wrote:
>>>>>>
>>>>>>
>>>>>> On 11/4/2024 7:21 PM, Konrad Dybcio wrote:
>>>>>>> On 4.11.2024 1:44 PM, Manikanta Mylavarapu wrote:
>>>>>>>> Add thermal zone nodes for sensors present in IPQ5424.
>>>>>>>>
>>>>>>>> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
>>>>>>>> ---
>>>>>>> [...]
>>>>>>>
>>>>>>>> +
>>>>>>>> +        cpu3-thermal {
>>>>>>>> +            polling-delay-passive = <0>;
>>>>>>>> +            polling-delay = <0>;
>>>>>>>> +            thermal-sensors = <&tsens 13>;
>>>>>>>> +
>>>>>>>> +            trips {
>>>>>>>> +                cpu-critical {
>>>>>>>> +                    temperature = <120000>;
>>>>>>>> +                    hysteresis = <9000>;
>>>>>>>> +                    type = "critical";
>>>>>>>> +                };
>>>>>>>> +
>>>>>>>> +                cpu-passive {
>>>>>>>> +                    temperature = <110000>;
>>>>>>>> +                    hysteresis = <9000>;
>>>>>>>> +                    type = "passive";
>>>>>>>
>>>>>>> You have a passive trip point without passive polling
>>>>>>>
>>>>>>
>>>>>> Okay, will remove this.
>>>>>
>>>>> You most likely want to preserve it, while keeping a sensible
>>>>> polling frequency, so that userspace will be aware of the current
>>>>> CPU temperature. <100> sounds like a sensible value here.
>>>>>
>>>>> Konrad
>>>>
>>>> Temperature sensor's present in IPQ5424 supports interrupts.
>>>
>>> Correct.
>>>
>>>> Hence no need to configure polling frequency.
>>>
>>> No, that interrupt firing signifies crossing the temp threshold (meaning
>>> no updates beyond that) or the tsens watchdog barking.
>>>
>>> Konrad
>>
>> An interrupt fires when the temperature crosses a threshold.
> 
> Which means you can't monitor the temperature at runtime
> without polling..
> 
> Konrad

Hi Konrad,

I got the point. Will configure polling-delay-passive with 100.

Thanks & Regards,
Manikanta.