[PATCH V2 0/4] Add support for IPQ5018 tsens

[PATCH V2 0/4] Add support for IPQ5018 tsens

[Sricharan Ramabadhran]

IPQ5018 has tsens V1.0 IP with 4 sensors and 1 interrupt.
There is no RPM present in the soc to do tsens early enable.
Adding support for the same here.

[v2]
	*) Sorted the compatible and removed example
	*) Fixed the name for new tsens_feature
	*) Used tsend_calibrate_common instead of legacy
	   and addressed comments from Dmitry.
	*) Squashed patch 3 & 4
	*) Fixed node names, order and added qfprom cells
            for points seprately
	*) Squashed patch 6 & 7 

Sricharan Ramabadhran (4):
  dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible
  thermal/drivers/qcom: Add new feat for soc without rpm
  thermal/drivers/tsens: Add support for IPQ5018 tsens
  arm64: dts: qcom: ipq5018: Add tsens node

 .../bindings/nvmem/qcom,qfprom.yaml           |   1 +
 .../bindings/thermal/qcom-tsens.yaml          |   1 +
 arch/arm64/boot/dts/qcom/ipq5018.dtsi         | 169 ++++++++++++++++++
 drivers/thermal/qcom/tsens-v1.c               |  58 ++++++
 drivers/thermal/qcom/tsens.c                  |   5 +-
 drivers/thermal/qcom/tsens.h                  |   5 +-
 6 files changed, 237 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Sricharan Ramabadhran]

IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 [v2] Sorted the compatible and removed example

 Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index 27e9e16e6455..c9586b2fbba4 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -39,6 +39,7 @@ properties:
       - description: v1 of TSENS
         items:
           - enum:
+              - qcom,ipq5018-tsens
               - qcom,msm8956-tsens
               - qcom,msm8976-tsens
               - qcom,qcs404-tsens
-- 
2.34.1

[PATCH V2 2/4] thermal/drivers/qcom: Add new feat for soc without rpm

[Sricharan Ramabadhran]

In IPQ5018, Tsens IP doesn't have RPM. Hence the early init to
enable tsens would not be done. So add a flag for that in feat
and skip enable checks. Without this, tsens probe fails.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 [v2] Fixed the name for new tsens_feature without rpm

 drivers/thermal/qcom/tsens.c | 2 +-
 drivers/thermal/qcom/tsens.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 98c356acfe98..0a43ccf02ec4 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->feat->ignore_enable)) {
 		dev_err(dev, "%s: device not enabled\n", __func__);
 		ret = -ENODEV;
 		goto err_put_device;
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 2805de1c6827..e254cd2df904 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -505,6 +505,8 @@ enum regfield_ids {
  * @srot_split: does the IP neatly splits the register space into SROT and TM,
  *              with SROT only being available to secure boot firmware?
  * @has_watchdog: does this IP support watchdog functionality?
+ * @ignore_enable: does this IP reside in a soc that does not have rpm to
+ *                 do pre-init.
  * @max_sensors: maximum sensors supported by this version of the IP
  * @trip_min_temp: minimum trip temperature supported by this version of the IP
  * @trip_max_temp: maximum trip temperature supported by this version of the IP
@@ -516,6 +518,7 @@ struct tsens_features {
 	unsigned int adc:1;
 	unsigned int srot_split:1;
 	unsigned int has_watchdog:1;
+	unsigned int ignore_enable:1;
 	unsigned int max_sensors;
 	int trip_min_temp;
 	int trip_max_temp;
-- 
2.34.1

[PATCH V2 3/4] thermal/drivers/tsens: Add support for IPQ5018 tsens

[Sricharan Ramabadhran]

IPQ5018 has tsens IP V1.0, 4 sensors and 1 interrupt.
The soc does not have a RPM, hence tsens has to be reset and
enabled in the driver init. Adding the driver support for same.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 [v2] Used tsens_calibrate_common instead of legacy
      and addressed comments from Dmitry.

 drivers/thermal/qcom/tsens-v1.c | 58 +++++++++++++++++++++++++++++++++
 drivers/thermal/qcom/tsens.c    |  3 ++
 drivers/thermal/qcom/tsens.h    |  2 +-
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
index dc1c4ae2d8b0..ed5c017905ab 100644
--- a/drivers/thermal/qcom/tsens-v1.c
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -79,6 +79,18 @@ static struct tsens_features tsens_v1_feat = {
 	.trip_max_temp	= 120000,
 };
 
+static struct tsens_features tsens_v1_ipq5018_feat = {
+	.ver_major	= VER_1_X,
+	.crit_int	= 0,
+	.combo_int	= 0,
+	.adc		= 1,
+	.srot_split	= 1,
+	.max_sensors	= 11,
+	.trip_min_temp	= -40000,
+	.trip_max_temp	= 120000,
+	.ignore_enable	= 1,
+};
+
 static const struct reg_field tsens_v1_regfields[MAX_REGFIELDS] = {
 	/* ----- SROT ------ */
 	/* VERSION */
@@ -150,6 +162,39 @@ static int __init init_8956(struct tsens_priv *priv) {
 	return init_common(priv);
 }
 
+static int init_ipq5018(struct tsens_priv *priv)
+{
+	int ret;
+	u32 mask;
+
+	init_common(priv);
+	if (!priv->tm_map)
+		return -ENODEV;
+
+	ret = regmap_field_write(priv->rf[TSENS_SW_RST], 1);
+	if (ret) {
+		dev_err(priv->dev, "Reset failed\n");
+		return ret;
+	}
+
+	mask = GENMASK(priv->num_sensors, 0);
+	ret = regmap_field_update_bits(priv->rf[SENSOR_EN], mask, mask);
+	if (ret) {
+		dev_err(priv->dev, "Sensor Enable failed\n");
+		return ret;
+	}
+
+	ret = regmap_field_write(priv->rf[TSENS_EN], 1);
+	if (ret) {
+		dev_err(priv->dev, "Enable failed\n");
+		return ret;
+	}
+
+	ret = regmap_field_write(priv->rf[TSENS_SW_RST], 0);
+
+	return ret;
+}
+
 static const struct tsens_ops ops_generic_v1 = {
 	.init		= init_common,
 	.calibrate	= calibrate_v1,
@@ -187,3 +232,16 @@ struct tsens_plat_data data_8976 = {
 	.feat		= &tsens_v1_feat,
 	.fields		= tsens_v1_regfields,
 };
+
+const struct tsens_ops ops_ipq5018 = {
+	.init		= init_ipq5018,
+	.calibrate	= tsens_calibrate_common,
+	.get_temp	= get_temp_tsens_valid,
+};
+
+struct tsens_plat_data data_ipq5018 = {
+	.num_sensors	= 5,
+	.ops		= &ops_ipq5018,
+	.feat		= &tsens_v1_ipq5018_feat,
+	.fields		= tsens_v1_regfields,
+};
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 0a43ccf02ec4..c792b9dc6676 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -1101,6 +1101,9 @@ static SIMPLE_DEV_PM_OPS(tsens_pm_ops, tsens_suspend, tsens_resume);
 
 static const struct of_device_id tsens_table[] = {
 	{
+		.compatible = "qcom,ipq5018-tsens",
+		.data = &data_ipq5018,
+	}, {
 		.compatible = "qcom,ipq8064-tsens",
 		.data = &data_8960,
 	}, {
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index e254cd2df904..b6594b546d11 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -645,7 +645,7 @@ extern struct tsens_plat_data data_8960;
 extern struct tsens_plat_data data_8226, data_8909, data_8916, data_8939, data_8974, data_9607;
 
 /* TSENS v1 targets */
-extern struct tsens_plat_data data_tsens_v1, data_8976, data_8956;
+extern struct tsens_plat_data data_tsens_v1, data_8976, data_8956, data_ipq5018;
 
 /* TSENS v2 targets */
 extern struct tsens_plat_data data_8996, data_ipq8074, data_tsens_v2;
-- 
2.34.1

[PATCH V2 4/4] arm64: dts: qcom: ipq5018: Add tsens node

[Sricharan Ramabadhran]

IPQ5018 has tsens V1.0 IP with 4 sensors.
There is no RPM, so tsens has to be manually enabled. Adding the tsens
and nvmem node and IPQ5018 has 4 thermal sensors (zones). With the
critical temperature being 120'C and action is to reboot. Adding all
the 4 zones here. 

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
---
 [v2] Fixed node names, order and added qfprom cells for points
      seperately to use the calibrate_common and squashed thermal_zone
      nodes here

 arch/arm64/boot/dts/qcom/ipq5018.dtsi | 169 ++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 9f13d2dcdfd5..d53aea5342e2 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -93,6 +93,117 @@ soc: soc@0 {
 		#size-cells = <1>;
 		ranges = <0 0 0 0xffffffff>;
 
+		qfprom: qfprom@a0000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";
+			reg = <0xa0000 0x1000>;
+
+			tsens_base1: base1@249 {
+				reg = <0x249 2>;
+				bits = <3 8>;
+			};
+
+			tsens_base2: base2@24a {
+				reg = <0x24a 2>;
+				bits = <3 8>;
+			};
+
+			tsens_mode: mode@249 {
+				reg = <0x249 1>;
+				bits = <0 3>;
+			};
+
+			tsens_s0_p1: s0-p1@24b {
+				reg = <0x24b 0x2>;
+				bits = <2 6>;
+			};
+
+			tsens_s0_p2: s0-p2@24c {
+				reg = <0x24c 0x1>;
+				bits = <1 6>;
+			};
+
+			tsens_s1_p1: s1-p1@24c {
+				reg = <0x24c 0x2>;
+				bits = <7 6>;
+			};
+
+			tsens_s1_p2: s1-p2@24d {
+				reg = <0x24d 0x2>;
+				bits = <5 6>;
+			};
+
+			tsens_s2_p1: s2-p1@24e {
+				reg = <0x24e 0x2>;
+				bits = <3 6>;
+			};
+
+			tsens_s2_p2: s2-p2@24f {
+				reg = <0x24f 0x1>;
+				bits = <1 6>;
+			};
+
+			tsens_s3_p1: s3-p1@24f {
+				reg = <0x24f 0x2>;
+				bits = <7 6>;
+			};
+
+			tsens_s3_p2: s3-p2@250 {
+				reg = <0x250 0x2>;
+				bits = <5 6>;
+			};
+
+			tsens_s4_p1: s4-p1@251 {
+				reg = <0x251 0x2>;
+				bits = <3 6>;
+			};
+
+			tsens_s4_p2: s4-p2@254 {
+				reg = <0x254 0x1>;
+				bits = <0 6>;
+			};
+		};
+
+		tsens: thermal-sensor@4a9000 {
+			compatible = "qcom,ipq5018-tsens", "qcom,tsens-v1";
+			reg = <0x4a9000 0x1000>, /* TM */
+			      <0x4a8000 0x1000>; /* SORT */
+
+			nvmem-cells = <&tsens_mode>,
+				      <&tsens_base1>,
+				      <&tsens_base2>,
+				      <&tsens_s0_p1>,
+				      <&tsens_s0_p2>,
+				      <&tsens_s1_p1>,
+				      <&tsens_s1_p2>,
+				      <&tsens_s2_p1>,
+				      <&tsens_s2_p2>,
+				      <&tsens_s3_p1>,
+				      <&tsens_s3_p2>,
+				      <&tsens_s4_p1>,
+				      <&tsens_s4_p2>;
+
+			nvmem-cell-names = "mode",
+					   "base1",
+					   "base2",
+					   "s0_p1",
+					   "s0_p2",
+					   "s1_p1",
+					   "s1_p2",
+					   "s2_p1",
+					   "s2_p2",
+					   "s3_p1",
+					   "s3_p2",
+					   "s4_p1",
+					   "s4_p2";
+
+			interrupts = <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "uplow";
+			#qcom,sensors = <5>;
+			#thermal-sensor-cells = <1>;
+		};
+
 		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq5018-tlmm";
 			reg = <0x01000000 0x300000>;
@@ -240,6 +351,64 @@ frame@b128000 {
 		};
 	};
 
+	thermal-zones {
+		ubi32-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 1>;
+
+			trips {
+				ubi32-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+			};
+		};
+
+		cpu-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 2>;
+
+			trips {
+				cpu-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+			};
+		};
+
+		top-glue-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 3>;
+
+			trips {
+				top_glue-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+			};
+		};
+
+		gephy-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&tsens 4>;
+
+			trips {
+				gephy-critical {
+					temperature = <120000>;
+					hysteresis = <2>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
-- 
2.34.1

Re: [PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Krzysztof Kozlowski]

On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---
>  [v2] Sorted the compatible and removed example
> 

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

Best regards,
Krzysztof

Re: [PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Krzysztof Kozlowski]

On 15/09/2023 14:43, Krzysztof Kozlowski wrote:
> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> ---
>>  [v2] Sorted the compatible and removed example
>>
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

No, unreviewed. Your driver says it is not compatible with
qcom,tsens-v1. This does not look right :/

Best regards,
Krzysztof

Re: [PATCH V2 4/4] arm64: dts: qcom: ipq5018: Add tsens node

[Krzysztof Kozlowski]

On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
> IPQ5018 has tsens V1.0 IP with 4 sensors.
> There is no RPM, so tsens has to be manually enabled. Adding the tsens
> and nvmem node and IPQ5018 has 4 thermal sensors (zones). With the
> critical temperature being 120'C and action is to reboot. Adding all
> the 4 zones here. 
> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---
>  [v2] Fixed node names, order and added qfprom cells for points
>       seperately to use the calibrate_common and squashed thermal_zone
>       nodes here
> 
>  arch/arm64/boot/dts/qcom/ipq5018.dtsi | 169 ++++++++++++++++++++++++++
>  1 file changed, 169 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
> index 9f13d2dcdfd5..d53aea5342e2 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
> @@ -93,6 +93,117 @@ soc: soc@0 {
>  		#size-cells = <1>;
>  		ranges = <0 0 0 0xffffffff>;
>  
> +		qfprom: qfprom@a0000 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof

Re: [PATCH V2 3/4] thermal/drivers/tsens: Add support for IPQ5018 tsens

[Dmitry Baryshkov]

On Fri, 15 Sept 2023 at 15:16, Sricharan Ramabadhran
<quic_srichara@quicinc.com> wrote:
>
> IPQ5018 has tsens IP V1.0, 4 sensors and 1 interrupt.
> The soc does not have a RPM, hence tsens has to be reset and
> enabled in the driver init. Adding the driver support for same.
>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---
>  [v2] Used tsens_calibrate_common instead of legacy
>       and addressed comments from Dmitry.
>
>  drivers/thermal/qcom/tsens-v1.c | 58 +++++++++++++++++++++++++++++++++
>  drivers/thermal/qcom/tsens.c    |  3 ++
>  drivers/thermal/qcom/tsens.h    |  2 +-
>  3 files changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
> index dc1c4ae2d8b0..ed5c017905ab 100644
> --- a/drivers/thermal/qcom/tsens-v1.c
> +++ b/drivers/thermal/qcom/tsens-v1.c
> @@ -79,6 +79,18 @@ static struct tsens_features tsens_v1_feat = {
>         .trip_max_temp  = 120000,
>  };
>
> +static struct tsens_features tsens_v1_ipq5018_feat = {
> +       .ver_major      = VER_1_X,
> +       .crit_int       = 0,
> +       .combo_int      = 0,
> +       .adc            = 1,
> +       .srot_split     = 1,
> +       .max_sensors    = 11,
> +       .trip_min_temp  = -40000,
> +       .trip_max_temp  = 120000,
> +       .ignore_enable  = 1,
> +};
> +
>  static const struct reg_field tsens_v1_regfields[MAX_REGFIELDS] = {
>         /* ----- SROT ------ */
>         /* VERSION */
> @@ -150,6 +162,39 @@ static int __init init_8956(struct tsens_priv *priv) {
>         return init_common(priv);
>  }
>
> +static int init_ipq5018(struct tsens_priv *priv)
> +{
> +       int ret;
> +       u32 mask;
> +
> +       init_common(priv);

Please take care of init_common()'s return code. Don't continue init
if init_common() fails.

> +       if (!priv->tm_map)
> +               return -ENODEV;

Why?

> +
> +       ret = regmap_field_write(priv->rf[TSENS_SW_RST], 1);
> +       if (ret) {
> +               dev_err(priv->dev, "Reset failed\n");
> +               return ret;
> +       }
> +
> +       mask = GENMASK(priv->num_sensors, 0);
> +       ret = regmap_field_update_bits(priv->rf[SENSOR_EN], mask, mask);
> +       if (ret) {
> +               dev_err(priv->dev, "Sensor Enable failed\n");
> +               return ret;
> +       }
> +
> +       ret = regmap_field_write(priv->rf[TSENS_EN], 1);
> +       if (ret) {
> +               dev_err(priv->dev, "Enable failed\n");
> +               return ret;
> +       }
> +
> +       ret = regmap_field_write(priv->rf[TSENS_SW_RST], 0);
> +
> +       return ret;
> +}
> +
>  static const struct tsens_ops ops_generic_v1 = {
>         .init           = init_common,
>         .calibrate      = calibrate_v1,
> @@ -187,3 +232,16 @@ struct tsens_plat_data data_8976 = {
>         .feat           = &tsens_v1_feat,
>         .fields         = tsens_v1_regfields,
>  };
> +
> +const struct tsens_ops ops_ipq5018 = {
> +       .init           = init_ipq5018,
> +       .calibrate      = tsens_calibrate_common,
> +       .get_temp       = get_temp_tsens_valid,
> +};
> +
> +struct tsens_plat_data data_ipq5018 = {
> +       .num_sensors    = 5,
> +       .ops            = &ops_ipq5018,
> +       .feat           = &tsens_v1_ipq5018_feat,
> +       .fields         = tsens_v1_regfields,
> +};
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 0a43ccf02ec4..c792b9dc6676 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -1101,6 +1101,9 @@ static SIMPLE_DEV_PM_OPS(tsens_pm_ops, tsens_suspend, tsens_resume);
>
>  static const struct of_device_id tsens_table[] = {
>         {
> +               .compatible = "qcom,ipq5018-tsens",
> +               .data = &data_ipq5018,
> +       }, {
>                 .compatible = "qcom,ipq8064-tsens",
>                 .data = &data_8960,
>         }, {
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index e254cd2df904..b6594b546d11 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -645,7 +645,7 @@ extern struct tsens_plat_data data_8960;
>  extern struct tsens_plat_data data_8226, data_8909, data_8916, data_8939, data_8974, data_9607;
>
>  /* TSENS v1 targets */
> -extern struct tsens_plat_data data_tsens_v1, data_8976, data_8956;
> +extern struct tsens_plat_data data_tsens_v1, data_8976, data_8956, data_ipq5018;
>
>  /* TSENS v2 targets */
>  extern struct tsens_plat_data data_8996, data_ipq8074, data_tsens_v2;
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH V2 2/4] thermal/drivers/qcom: Add new feat for soc without rpm

[Dmitry Baryshkov]

On Fri, 15 Sept 2023 at 15:15, Sricharan Ramabadhran
<quic_srichara@quicinc.com> wrote:
>
> In IPQ5018, Tsens IP doesn't have RPM. Hence the early init to
> enable tsens would not be done. So add a flag for that in feat
> and skip enable checks. Without this, tsens probe fails.
>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>

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


-- 
With best wishes
Dmitry

Re: [PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Sricharan Ramabadhran]

On 9/15/2023 6:15 PM, Krzysztof Kozlowski wrote:
> On 15/09/2023 14:43, Krzysztof Kozlowski wrote:
>> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>>
>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>> ---
>>>   [v2] Sorted the compatible and removed example
>>>
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> No, unreviewed. Your driver says it is not compatible with
> qcom,tsens-v1. This does not look right :/
> 

  Yes it is V1 IP, but since there is no RPM, to enable the IP/SENSORS
  have to do those steps after calling init_common. Similar reason
  added a new feat as well in patch #2 as well. Hence for this,
  new compatible was required.

Regards,
  Sricharan

Re: [PATCH V2 3/4] thermal/drivers/tsens: Add support for IPQ5018 tsens

[Sricharan Ramabadhran]

<..>

>> +static int init_ipq5018(struct tsens_priv *priv)
>> +{
>> +       int ret;
>> +       u32 mask;
>> +
>> +       init_common(priv);
> 
> Please take care of init_common()'s return code. Don't continue init
> if init_common() fails.

  ok

> 
>> +       if (!priv->tm_map)
>> +               return -ENODEV;
> 
> Why?
> 
  ok, redundant, will remove.

Regards,
  Sricharan

Re: [PATCH V2 4/4] arm64: dts: qcom: ipq5018: Add tsens node

[Sricharan Ramabadhran]

On 9/15/2023 6:16 PM, Krzysztof Kozlowski wrote:
> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>> IPQ5018 has tsens V1.0 IP with 4 sensors.
>> There is no RPM, so tsens has to be manually enabled. Adding the tsens
>> and nvmem node and IPQ5018 has 4 thermal sensors (zones). With the
>> critical temperature being 120'C and action is to reboot. Adding all
>> the 4 zones here.
>>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> ---
>>   [v2] Fixed node names, order and added qfprom cells for points
>>        seperately to use the calibrate_common and squashed thermal_zone
>>        nodes here
>>
>>   arch/arm64/boot/dts/qcom/ipq5018.dtsi | 169 ++++++++++++++++++++++++++
>>   1 file changed, 169 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>> index 9f13d2dcdfd5..d53aea5342e2 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>> @@ -93,6 +93,117 @@ soc: soc@0 {
>>   		#size-cells = <1>;
>>   		ranges = <0 0 0 0xffffffff>;
>>   
>> +		qfprom: qfprom@a0000 {
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
> 

  oops, moved the compatible to first, but missed it on posting version.
  Will fix it in V3.

Regards,
  Sricharan

Re: [PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Krzysztof Kozlowski]

On 19/09/2023 09:22, Sricharan Ramabadhran wrote:
> 
> 
> On 9/15/2023 6:15 PM, Krzysztof Kozlowski wrote:
>> On 15/09/2023 14:43, Krzysztof Kozlowski wrote:
>>> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>>>
>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>> ---
>>>>   [v2] Sorted the compatible and removed example
>>>>
>>>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> No, unreviewed. Your driver says it is not compatible with
>> qcom,tsens-v1. This does not look right :/
>>
> 
>   Yes it is V1 IP, but since there is no RPM, to enable the IP/SENSORS
>   have to do those steps after calling init_common. Similar reason
>   added a new feat as well in patch #2 as well. Hence for this,
>   new compatible was required.

I dud not write about new or old compatible ("compatible" as noun). I
wrote that it is not compatible ("compatible" as adjective) with v1.

Best regards,
Krzysztof

Re: [PATCH V2 4/4] arm64: dts: qcom: ipq5018: Add tsens node

[Krzysztof Kozlowski]

On 19/09/2023 09:28, Sricharan Ramabadhran wrote:
> 
> 
> On 9/15/2023 6:16 PM, Krzysztof Kozlowski wrote:
>> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>>> IPQ5018 has tsens V1.0 IP with 4 sensors.
>>> There is no RPM, so tsens has to be manually enabled. Adding the tsens
>>> and nvmem node and IPQ5018 has 4 thermal sensors (zones). With the
>>> critical temperature being 120'C and action is to reboot. Adding all
>>> the 4 zones here.
>>>
>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>> ---
>>>   [v2] Fixed node names, order and added qfprom cells for points
>>>        seperately to use the calibrate_common and squashed thermal_zone
>>>        nodes here
>>>
>>>   arch/arm64/boot/dts/qcom/ipq5018.dtsi | 169 ++++++++++++++++++++++++++
>>>   1 file changed, 169 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>>> index 9f13d2dcdfd5..d53aea5342e2 100644
>>> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>>> @@ -93,6 +93,117 @@ soc: soc@0 {
>>>   		#size-cells = <1>;
>>>   		ranges = <0 0 0 0xffffffff>;
>>>   
>>> +		qfprom: qfprom@a0000 {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>> +			compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";
>>
>> This is a friendly reminder during the review process.
>>
>> It seems my previous comments were not fully addressed. Maybe my
>> feedback got lost between the quotes, maybe you just forgot to apply it.
>> Please go back to the previous discussion and either implement all
>> requested changes or keep discussing them.
>>
> 
>   oops, moved the compatible to first, but missed it on posting version.
>   Will fix it in V3.

What do you mean by "posting version"? If it is not the same as your Git
version, then your process is buggy. You must work on mainline tree and
send patches from that tree. Not edit patches and edit Git separately...

Best regards,
Krzysztof

Re: [PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Sricharan Ramabadhran]

On 9/19/2023 6:02 PM, Krzysztof Kozlowski wrote:
> On 19/09/2023 09:22, Sricharan Ramabadhran wrote:
>>
>>
>> On 9/15/2023 6:15 PM, Krzysztof Kozlowski wrote:
>>> On 15/09/2023 14:43, Krzysztof Kozlowski wrote:
>>>> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>>>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>>>>
>>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>> ---
>>>>>    [v2] Sorted the compatible and removed example
>>>>>
>>>>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>
>>> No, unreviewed. Your driver says it is not compatible with
>>> qcom,tsens-v1. This does not look right :/
>>>
>>
>>    Yes it is V1 IP, but since there is no RPM, to enable the IP/SENSORS
>>    have to do those steps after calling init_common. Similar reason
>>    added a new feat as well in patch #2 as well. Hence for this,
>>    new compatible was required.
> 
> I dud not write about new or old compatible ("compatible" as noun). I
> wrote that it is not compatible ("compatible" as adjective) with v1.
> 

  Ho, in that case, yes it is not compatible with V1 init and features
  because of 'no rpm'. So in that case, should this be documented
  as a separate version of 'V1 without rpm' ?

Regards,
  Sricharan

Re: [PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Krzysztof Kozlowski]

On 19/09/2023 14:48, Sricharan Ramabadhran wrote:
> 
> 
> On 9/19/2023 6:02 PM, Krzysztof Kozlowski wrote:
>> On 19/09/2023 09:22, Sricharan Ramabadhran wrote:
>>>
>>>
>>> On 9/15/2023 6:15 PM, Krzysztof Kozlowski wrote:
>>>> On 15/09/2023 14:43, Krzysztof Kozlowski wrote:
>>>>> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>>>>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>>>>>
>>>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>>> ---
>>>>>>    [v2] Sorted the compatible and removed example
>>>>>>
>>>>>
>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>
>>>> No, unreviewed. Your driver says it is not compatible with
>>>> qcom,tsens-v1. This does not look right :/
>>>>
>>>
>>>    Yes it is V1 IP, but since there is no RPM, to enable the IP/SENSORS
>>>    have to do those steps after calling init_common. Similar reason
>>>    added a new feat as well in patch #2 as well. Hence for this,
>>>    new compatible was required.
>>
>> I dud not write about new or old compatible ("compatible" as noun). I
>> wrote that it is not compatible ("compatible" as adjective) with v1.
>>
> 
>   Ho, in that case, yes it is not compatible with V1 init and features
>   because of 'no rpm'. So in that case, should this be documented
>   as a separate version of 'V1 without rpm' ?

It should not be mixed with regular v1, just as new entry there. I don't
think fallback is needed - just use SoC specific compatible.

Best regards,
Krzysztof

Re: [PATCH V2 4/4] arm64: dts: qcom: ipq5018: Add tsens node

[Sricharan Ramabadhran]

On 9/19/2023 6:07 PM, Krzysztof Kozlowski wrote:
> On 19/09/2023 09:28, Sricharan Ramabadhran wrote:
>>
>>
>> On 9/15/2023 6:16 PM, Krzysztof Kozlowski wrote:
>>> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>>>> IPQ5018 has tsens V1.0 IP with 4 sensors.
>>>> There is no RPM, so tsens has to be manually enabled. Adding the tsens
>>>> and nvmem node and IPQ5018 has 4 thermal sensors (zones). With the
>>>> critical temperature being 120'C and action is to reboot. Adding all
>>>> the 4 zones here.
>>>>
>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>> ---
>>>>    [v2] Fixed node names, order and added qfprom cells for points
>>>>         seperately to use the calibrate_common and squashed thermal_zone
>>>>         nodes here
>>>>
>>>>    arch/arm64/boot/dts/qcom/ipq5018.dtsi | 169 ++++++++++++++++++++++++++
>>>>    1 file changed, 169 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>>>> index 9f13d2dcdfd5..d53aea5342e2 100644
>>>> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>>>> @@ -93,6 +93,117 @@ soc: soc@0 {
>>>>    		#size-cells = <1>;
>>>>    		ranges = <0 0 0 0xffffffff>;
>>>>    
>>>> +		qfprom: qfprom@a0000 {
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <1>;
>>>> +			compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";
>>>
>>> This is a friendly reminder during the review process.
>>>
>>> It seems my previous comments were not fully addressed. Maybe my
>>> feedback got lost between the quotes, maybe you just forgot to apply it.
>>> Please go back to the previous discussion and either implement all
>>> requested changes or keep discussing them.
>>>
>>
>>    oops, moved the compatible to first, but missed it on posting version.
>>    Will fix it in V3.
> 
> What do you mean by "posting version"? If it is not the same as your Git
> version, then your process is buggy. You must work on mainline tree and
> send patches from that tree. Not edit patches and edit Git separately...
> 
   Working on mainline tree only, just that i had 2 different build
   servers (one build machine and other local machine). Usually develop
   all on build server, copy/apply patches to local machine (mainline)
   and send. This time missed copying to local finally.

Regards,
  Sricharan

   one for

Re: [PATCH V2 1/4] dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible

[Sricharan Ramabadhran]

On 9/19/2023 6:26 PM, Krzysztof Kozlowski wrote:
> On 19/09/2023 14:48, Sricharan Ramabadhran wrote:
>>
>>
>> On 9/19/2023 6:02 PM, Krzysztof Kozlowski wrote:
>>> On 19/09/2023 09:22, Sricharan Ramabadhran wrote:
>>>>
>>>>
>>>> On 9/15/2023 6:15 PM, Krzysztof Kozlowski wrote:
>>>>> On 15/09/2023 14:43, Krzysztof Kozlowski wrote:
>>>>>> On 15/09/2023 14:15, Sricharan Ramabadhran wrote:
>>>>>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>>>>>>
>>>>>>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>>>> ---
>>>>>>>     [v2] Sorted the compatible and removed example
>>>>>>>
>>>>>>
>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>
>>>>> No, unreviewed. Your driver says it is not compatible with
>>>>> qcom,tsens-v1. This does not look right :/
>>>>>
>>>>
>>>>     Yes it is V1 IP, but since there is no RPM, to enable the IP/SENSORS
>>>>     have to do those steps after calling init_common. Similar reason
>>>>     added a new feat as well in patch #2 as well. Hence for this,
>>>>     new compatible was required.
>>>
>>> I dud not write about new or old compatible ("compatible" as noun). I
>>> wrote that it is not compatible ("compatible" as adjective) with v1.
>>>
>>
>>    Ho, in that case, yes it is not compatible with V1 init and features
>>    because of 'no rpm'. So in that case, should this be documented
>>    as a separate version of 'V1 without rpm' ?
> 
> It should not be mixed with regular v1, just as new entry there. I don't
> think fallback is needed - just use SoC specific compatible.
> 
  ok, sure, will add in V3.

Regards,
  Sricharan