[PATCH v3 0/5] thermal: loongson2: Add support for Loongson-2K0300 SoC

[PATCH v3 0/5] thermal: loongson2: Add support for Loongson-2K0300 SoC

[Binbin Zhou]

Hi all:

This patch series adds thermal zone support for the Loongson-2K0300 SoC.

The Loongson-2K0300's thermal sensor is generally compatible with the
existing Loongson-2K series, but it features several key differences:
1. It has an independent CPU ID register for reading chip version
information.
2. The raw temperature output field has been extended from 8 bits to 11
bits.
3. The temperature calculation formula has been updated to incorporate a
correction factor based on the version information.

To ensure robustness on older hardware revisions, a fallback mechanism
is included: if the calculated temperature falls outside the valid range
(-55 TO 125),  the driver falls back to a simpler calculation and prints
a warning.

The patches have been validated on the Loongson-2K0300 development board.

Thanks.
Binbin

=========
V3:
Patch (2/5):
 - New patch for moving thermal_ops to chip_data;
Patch (3/5):
 - Correct spelling errors;
Patch (4/5):
 - Add Acked-by from Conor, thanks;
Patch (5/5):
 - Keep loongson2_thermal_data->chip_data as const, and use
   data->flags as the private data.
 - Renmae LOONGSON2_THSENS_OUT_10B_MASK ->
   LOONGSON2_THSENS_OUT_11B_MASK;

Link to V2:
https://lore.kernel.org/all/cover.1784619512.git.zhoubinbin@loongson.cn/

V2:
Patch (1/4):
 - New patch form Sashiko;
Patch (2/4):
 - New patch for adding chipid syscon compatible;
Patch (3/4):
 - Use `loongson,chipid` phandle replace reg property;
Patch (3/4):
 - Refact `calib_offset` calculate routing;
 - Add LS2K0300_OLD_FUSE_FLAG flag.

Link to V1:
https://lore.kernel.org/all/cover.1783670011.git.zhoubinbin@loongson.cn/

Binbin Zhou (5):
  thermal/drivers/loongson2: Correct thermal sensor registration loop
  thermal/drivers/loongson2: Move thermal_ops to chip_data for SoC
    scalability
  dt-bindings: mfd: syscon: Allow syscon compatible for Loongson-2K0300
    chip id
  dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for
    Loongson-2K0300
  thermal/drivers/loongson2: Add support for Loongson-2K0300

 .../devicetree/bindings/mfd/syscon.yaml       |   2 +
 .../thermal/loongson,ls2k-thermal.yaml        |  73 +++++++++---
 drivers/thermal/loongson2_thermal.c           | 110 ++++++++++++++++--
 3 files changed, 157 insertions(+), 28 deletions(-)


base-commit: 795f1b1a91ae13ebc012a364075e42f486a1cafe
-- 
2.52.0

[PATCH v3 1/5] thermal/drivers/loongson2: Correct thermal sensor registration loop

[Binbin Zhou]

The registration loop in loongson2_thermal_probe() incorrectly uses
dev_err_probe() when the sensor is not present (-ENODEV). In that case,
the driver should continue to the next sensor index rather than treating
it as a fatal error.

Fix this by correctly handling -ENODEV and only returning on other
errors. Also add a final check to ensure at least one thermal zone was
registered.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/cover.1783670011.git.zhoubinbin@loongson.cn?part=2
Fixes: e7e3a7c35791 ("thermal/drivers/loongson-2: Add thermal management support")
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/thermal/loongson2_thermal.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c
index ea4dd2fb1f47..5f47fa51fb90 100644
--- a/drivers/thermal/loongson2_thermal.c
+++ b/drivers/thermal/loongson2_thermal.c
@@ -161,16 +161,18 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
 
 	for (i = 0; i <= LOONGSON2_MAX_SENSOR_SEL_NUM; i++) {
 		tzd = devm_thermal_of_zone_register(dev, i, data, thermal_ops);
-
 		if (!IS_ERR(tzd))
 			break;
 
-		if (PTR_ERR(tzd) != -ENODEV)
+		if (PTR_ERR(tzd) == -ENODEV)
 			continue;
 
-		return dev_err_probe(dev, PTR_ERR(tzd), "failed to register");
+		return dev_err_probe(dev, PTR_ERR(tzd), "failed to register sensor %d\n", i);
 	}
 
+	if (IS_ERR(tzd))
+		return dev_err_probe(dev, -ENODEV, "No thermal sensor registered\n");
+
 	ret = devm_request_threaded_irq(dev, irq, NULL, loongson2_thermal_irq_thread,
 					IRQF_ONESHOT, "loongson2_thermal", tzd);
 	if (ret < 0)
-- 
2.52.0

Re: [PATCH v3 1/5] thermal/drivers/loongson2: Correct thermal sensor registration loop

[sashiko-bot]

> The registration loop in loongson2_thermal_probe() incorrectly uses
> dev_err_probe() when the sensor is not present (-ENODEV). In that case,
> the driver should continue to the next sensor index rather than treating
> it as a fatal error.
> 
> Fix this by correctly handling -ENODEV and only returning on other
> errors. Also add a final check to ensure at least one thermal zone was
> registered.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/cover.1783670011.git.zhoubinbin@loongson.cn?part=2
> Fixes: e7e3a7c35791 ("thermal/drivers/loongson-2: Add thermal management support")
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785480242.git.zhoubinbin@loongson.cn?part=1

[PATCH v3 2/5] thermal/drivers/loongson2: Move thermal_ops to chip_data for SoC scalability

[Binbin Zhou]

In preparation for supporting additional Loongson SoCs, relocate the
thermal zone device operations pointer (thermal_ops) from the probe
routine to the per-SoC chip_data structure. This eliminates the need
to add conditional branches in probe() when a new SoC is introduced;
instead, the appropriate ops can be statically assigned in the match
data.

No functional change intended.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/thermal/loongson2_thermal.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c
index 5f47fa51fb90..1a8ca2cd06f1 100644
--- a/drivers/thermal/loongson2_thermal.c
+++ b/drivers/thermal/loongson2_thermal.c
@@ -39,6 +39,7 @@
 struct loongson2_thermal_chip_data {
 	unsigned int thermal_sensor_sel;
 	unsigned int flags;
+	const struct thermal_zone_device_ops *thermal_ops;
 };
 
 struct loongson2_thermal_data {
@@ -124,7 +125,6 @@ static const struct thermal_zone_device_ops loongson2_2k2000_of_thermal_ops = {
 
 static int loongson2_thermal_probe(struct platform_device *pdev)
 {
-	const struct thermal_zone_device_ops *thermal_ops;
 	struct device *dev = &pdev->dev;
 	struct loongson2_thermal_data *data;
 	struct thermal_zone_device *tzd;
@@ -145,10 +145,6 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
 		data->temp_reg = devm_platform_ioremap_resource(pdev, 1);
 		if (IS_ERR(data->temp_reg))
 			return PTR_ERR(data->temp_reg);
-
-		thermal_ops = &loongson2_2k2000_of_thermal_ops;
-	} else {
-		thermal_ops = &loongson2_2k1000_of_thermal_ops;
 	}
 
 	irq = platform_get_irq(pdev, 0);
@@ -160,7 +156,7 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
 	loongson2_thermal_set(data, 0, 0, false);
 
 	for (i = 0; i <= LOONGSON2_MAX_SENSOR_SEL_NUM; i++) {
-		tzd = devm_thermal_of_zone_register(dev, i, data, thermal_ops);
+		tzd = devm_thermal_of_zone_register(dev, i, data, data->chip_data->thermal_ops);
 		if (!IS_ERR(tzd))
 			break;
 
@@ -186,11 +182,13 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
 static const struct loongson2_thermal_chip_data loongson2_thermal_ls2k1000_data = {
 	.thermal_sensor_sel = 0,
 	.flags = 0,
+	.thermal_ops = &loongson2_2k1000_of_thermal_ops,
 };
 
 static const struct loongson2_thermal_chip_data loongson2_thermal_ls2k2000_data = {
 	.thermal_sensor_sel = 0,
 	.flags = LS2K2000_THSENS_OUT_FLAG,
+	.thermal_ops = &loongson2_2k2000_of_thermal_ops,
 };
 
 static const struct of_device_id of_loongson2_thermal_match[] = {
-- 
2.52.0

Re: [PATCH v3 2/5] thermal/drivers/loongson2: Move thermal_ops to chip_data for SoC scalability

[sashiko-bot]

> In preparation for supporting additional Loongson SoCs, relocate the
> thermal zone device operations pointer (thermal_ops) from the probe
> routine to the per-SoC chip_data structure. This eliminates the need
> to add conditional branches in probe() when a new SoC is introduced;
> instead, the appropriate ops can be statically assigned in the match
> data.
> 
> No functional change intended.
> 
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785480242.git.zhoubinbin@loongson.cn?part=2

[PATCH v3 3/5] dt-bindings: mfd: syscon: Allow syscon compatible for Loongson-2K0300 chip id

[Binbin Zhou]

The Loongson-2K0300 SoC exposes its chip ID registers through a syscon
interface. Add the specific compatible `loongson,ls2k0300-chipid-syscon`
to the allowed list of syscon bindings so that it can be referenced from
the thermal node via a phandle.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index e22867088063..18eb9defec8c 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -76,6 +76,7 @@ select:
           - hpe,gxp-sysreg
           - loongson,ls1b-syscon
           - loongson,ls1c-syscon
+          - loongson,ls2k0300-chipid-syscon
           - lsi,axxia-syscon
           - marvell,armada-3700-cpu-misc
           - marvell,armada-3700-nb-pm
@@ -189,6 +190,7 @@ properties:
               - hpe,gxp-sysreg
               - loongson,ls1b-syscon
               - loongson,ls1c-syscon
+              - loongson,ls2k0300-chipid-syscon
               - lsi,axxia-syscon
               - marvell,armada-3700-cpu-misc
               - marvell,armada-3700-nb-pm
-- 
2.52.0

Re: [PATCH v3 3/5] dt-bindings: mfd: syscon: Allow syscon compatible for Loongson-2K0300 chip id

[sashiko-bot]

> The Loongson-2K0300 SoC exposes its chip ID registers through a syscon
> interface. Add the specific compatible `loongson,ls2k0300-chipid-syscon`
> to the allowed list of syscon bindings so that it can be referenced from
> the thermal node via a phandle.
> 
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785480242.git.zhoubinbin@loongson.cn?part=3

[PATCH v3 4/5] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

[Binbin Zhou]

Add a new compatible string `loongson,ls2k0300-thermal` for the thermal
sensor found on the Loongson-2K0300 SoC.

The hardware differs from existing SoCs in that it requires a phandle to
a syscon node that provides the CHIP ID register, used as a compensation
source in the temperature calculation.

Update the binding to make `loongson,chipid` required for this new
compatible, and clarify the register region requirements for each
variant.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 .../thermal/loongson,ls2k-thermal.yaml        | 73 ++++++++++++++-----
 1 file changed, 55 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
index 79e691b08341..d312e27737a2 100644
--- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
@@ -10,13 +10,11 @@ maintainers:
   - zhanghongchen <zhanghongchen@loongson.cn>
   - Yinbo Zhu <zhuyinbo@loongson.cn>
 
-allOf:
-  - $ref: /schemas/thermal/thermal-sensor.yaml#
-
 properties:
   compatible:
     oneOf:
       - enum:
+          - loongson,ls2k0300-thermal
           - loongson,ls2k1000-thermal
           - loongson,ls2k2000-thermal
       - items:
@@ -34,28 +32,56 @@ properties:
   '#thermal-sensor-cells':
     const: 1
 
+  loongson,chipid:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the syscon with the Loongson-2K0300 CHIP ID.
+
 required:
   - compatible
   - reg
   - interrupts
 
-if:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - loongson,ls2k2000-thermal
+allOf:
+  - $ref: /schemas/thermal/thermal-sensor.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: loongson,ls2k0300-thermal
+    then:
+      properties:
+        reg:
+          items:
+            - description: Thermal base register region
+      required:
+        - loongson,chipid
 
-then:
-  properties:
-    reg:
-      minItems: 2
-      maxItems: 2
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: loongson,ls2k1000-thermal
+    then:
+      properties:
+        loongson,chipid: false
+        reg:
+          items:
+            - description: Thermal base register region
 
-else:
-  properties:
-    reg:
-      maxItems: 1
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: loongson,ls2k2000-thermal
+    then:
+      properties:
+        loongson,chipid: false
+        reg:
+          items:
+            - description: Thermal base register region
+            - description: Thermal data output register region
 
 unevaluatedProperties: false
 
@@ -69,3 +95,14 @@ examples:
         interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
         #thermal-sensor-cells = <1>;
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    thermal-sensor@16001500 {
+       compatible = "loongson,ls2k0300-thermal";
+       reg = <0x16001500 0x30>;
+       interrupt-parent = <&liointc1>;
+       interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
+       loongson,chipid = <&chipid>;
+       #thermal-sensor-cells = <1>;
+    };
-- 
2.52.0

Re: [PATCH v3 4/5] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check for `device_get_match_data()` return value in probe.
- [High] The driver registers a thermal sensor found at an arbitrary device tree index but completely ignores this index during hardware configuration, hardcoding register operations to sensor 0.
--

--- Patch [4]: [PATCH v3 4/5] dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300 ---
commit 52153439c55d2e646aae8a83ae212f55c09dcf24
Author: Binbin Zhou <zhoubinbin@loongson.cn>

dt-bindings: thermal: loongson,ls2k-thermal: Add compatible for Loongson-2K0300

This commit adds the loongson,ls2k0300-thermal compatible string to the
device tree bindings. It mandates the loongson,chipid property for this new
SoC variant to provide a syscon phandle for temperature calculation.

> diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> index 79e691b08341c..d312e27737a20 100644
> --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
> @@ -10,13 +10,11 @@ maintainers:
>    - zhanghongchen <zhanghongchen@loongson.cn>
>    - Yinbo Zhu <zhuyinbo@loongson.cn>
>  
> -allOf:
> -  - $ref: /schemas/thermal/thermal-sensor.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
>        - enum:
> +          - loongson,ls2k0300-thermal
>            - loongson,ls2k1000-thermal
>            - loongson,ls2k2000-thermal
>        - items:

[ ... ]

[Severity: High]
This is a pre-existing issue, but can this code result in a null pointer
dereference?

If the driver is bound to a device via the sysfs bind interface without a
matching device tree node, device_get_match_data() returns null:

drivers/thermal/loongson2_thermal.c:loongson2_thermal_probe() {
    ...
	data->chip_data = device_get_match_data(dev);

	data->ctrl_reg = devm_platform_ioremap_resource(pdev, 0);
	if (IS_ERR(data->ctrl_reg))
		return PTR_ERR(data->ctrl_reg);

	/* The temperature output register is separate for Loongson-2K2000 */
	if (data->chip_data->flags & LS2K2000_THSENS_OUT_FLAG) {
    ...
}

Does this path blindly dereference data->chip_data via
data->chip_data->flags before any validation?

Could this allow a root user or privileged process writing to sysfs to crash
the kernel?

[Severity: High]
This is a pre-existing issue, but does this driver ignore the dynamically
discovered sensor index during hardware configuration?

The probe loop successfully registers the first available sensor index found
in the device tree, but doesn't store it to update the internal sensor
selection state:

drivers/thermal/loongson2_thermal.c:loongson2_thermal_probe() {
    ...
	for (i = 0; i <= LOONGSON2_MAX_SENSOR_SEL_NUM; i++) {
		tzd = devm_thermal_of_zone_register(dev, i, data, data->chip_data->thermal_ops);
		if (!IS_ERR(tzd))
			break;
    ...
}

Later, the hardware operations calculate the offset using the statically
hardcoded thermal_sensor_sel from the constant match data:

drivers/thermal/loongson2_thermal.c:loongson2_set_ctrl_regs() {
    ...
	int reg_off  = data->chip_data->thermal_sensor_sel * 2;
    ...
}

Will this always configure temperature thresholds and read temperatures for
sensor 0, even if a thermal zone was registered for a different sensor index?

A related patch in this series (thermal/drivers/loongson2: Correct thermal
sensor registration loop) actively enables this buggy path to be reached by
continuing on -ENODEV instead of aborting.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785480242.git.zhoubinbin@loongson.cn?part=4

[PATCH v3 5/5] thermal/drivers/loongson2: Add support for Loongson-2K0300

[Binbin Zhou]

Add support for the thermal sensor found on the Loongson-2K0300 SoC.

The Loongson-2K0300 thermal sensor uses a 10-bit ADC and requires
per-chip calibration. The calibration offset is stored in the CHIP ID
registers, which are accessed via syscon. The driver reads this offset
and applies it when converting the raw ADC value to millicelsius.

To handle old fuse versions that cannot be calibrated correctly, the
driver includes a fallback formula and a validity check. Once invalid
data is detected, the driver falls back to the old formula for future
reads and warns the user.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/thermal/loongson2_thermal.c | 94 ++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c
index 1a8ca2cd06f1..51b29b60372a 100644
--- a/drivers/thermal/loongson2_thermal.c
+++ b/drivers/thermal/loongson2_thermal.c
@@ -2,9 +2,11 @@
 /*
  * Author: zhanghongchen <zhanghongchen@loongson.cn>
  *         Yinbo Zhu <zhuyinbo@loongson.cn>
+ *         Binbin Zhou <zhoubinbin@loongson.cn>
  * Copyright (C) 2022-2023 Loongson Technology Corporation Limited
  */
 
+#include <linux/bitfield.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/minmax.h>
@@ -14,6 +16,9 @@
 #include <linux/property.h>
 #include <linux/thermal.h>
 #include <linux/units.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/syscore_ops.h>
 
 #include "thermal_hwmon.h"
 
@@ -23,18 +28,34 @@
 #define LOONGSON2_THSENS_CTRL_LOW_REG	0x8
 #define LOONGSON2_THSENS_STATUS_REG	0x10
 #define LOONGSON2_THSENS_OUT_REG	0x14
+#define LOONGSON2_THSENS_CFG_REG	0x18
 
 #define LOONGSON2_THSENS_INT_LO		BIT(0)
 #define LOONGSON2_THSENS_INT_HIGH	BIT(1)
 #define LOONGSON2_THSENS_INT_EN		(LOONGSON2_THSENS_INT_LO | \
 					 LOONGSON2_THSENS_INT_HIGH)
-#define LOONGSON2_THSENS_OUT_MASK	0xFF
+#define LOONGSON2_THSENS_OUT_8B_MASK	0xFF
+#define LOONGSON2_THSENS_OUT_11B_MASK	GENMASK(10, 0)
+
+#define LS2K0300_CHIP_ID0		0x10
+#define LS2K0300_CHIP_ID1		0x14
+#define LS2K0300_EXTERN_ID		BIT(4)
+#define LS2K0300_ID0_VAL_MASK		GENMASK(31, 20)
+#define LS2K0300_ID1_VAL_MASK		GENMASK(15, 0)
+
+#define LS2K0300_COMP_VAL_MASK		GENMASK(14, 0)
+#define LS2K0300_COMP_SIGN_BIT		BIT(15)
+
+#define LS2K0300_LOWEST_VALID_TEMP	(-55000)
+#define LS2K0300_HIGHEST_VALID_TEMP	(125000)
 
 /*
  * This flag is used to indicate the temperature reading
  * method of the Loongson-2K2000
  */
 #define LS2K2000_THSENS_OUT_FLAG	BIT(0)
+#define LS2K0300_CHIP_ID_FLAG		BIT(1)
+#define LS2K0300_OLD_FUSE_FLAG		BIT(2)
 
 struct loongson2_thermal_chip_data {
 	unsigned int thermal_sensor_sel;
@@ -43,8 +64,11 @@ struct loongson2_thermal_chip_data {
 };
 
 struct loongson2_thermal_data {
+	struct device *dev;
 	void __iomem *ctrl_reg;
 	void __iomem *temp_reg;
+	struct regmap *regmap_cfg;
+	u32 flags;
 	const struct loongson2_thermal_chip_data *chip_data;
 };
 
@@ -72,13 +96,54 @@ static int loongson2_thermal_set(struct loongson2_thermal_data *data,
 	return 0;
 }
 
+static int loongson2_2k0300_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct loongson2_thermal_data *tdata = thermal_zone_device_priv(tz);
+	int calib_data, calib_offset, temp_mc, raw_adc;
+	u32 chip_id0 = 0, chip_id1 = 0;
+
+	writel(0xff03, tdata->ctrl_reg + LOONGSON2_THSENS_CFG_REG);
+	raw_adc = FIELD_GET(LOONGSON2_THSENS_OUT_11B_MASK,
+			    readl(tdata->ctrl_reg + LOONGSON2_THSENS_OUT_REG));
+
+	if (tdata->flags & LS2K0300_OLD_FUSE_FLAG) {
+		*temp = raw_adc * 569 - 394700;
+		return 0;
+	}
+
+	regmap_read(tdata->regmap_cfg, LS2K0300_CHIP_ID0, &chip_id0);
+	regmap_read(tdata->regmap_cfg, LS2K0300_CHIP_ID1, &chip_id1);
+
+	if (chip_id0 & LS2K0300_EXTERN_ID) {
+		calib_data = FIELD_GET(LS2K0300_ID1_VAL_MASK, chip_id1);
+		calib_offset = FIELD_GET(LS2K0300_COMP_VAL_MASK, calib_data);
+		if (calib_data & LS2K0300_COMP_SIGN_BIT)
+			calib_offset = -calib_offset;
+	} else {
+		calib_data = FIELD_GET(LS2K0300_ID0_VAL_MASK, chip_id0);
+		calib_offset = FIELD_GET(LS2K0300_COMP_VAL_MASK, calib_data);
+	}
+
+	temp_mc = (raw_adc + calib_offset) * 570 - 394700;
+
+	/* For old fuse which can not read right thermal data */
+	if (temp_mc < LS2K0300_LOWEST_VALID_TEMP || temp_mc > LS2K0300_HIGHEST_VALID_TEMP) {
+		dev_warn_once(tdata->dev, "It's an old fuse, thermal %d is not right\n", temp_mc);
+		tdata->flags |= LS2K0300_OLD_FUSE_FLAG;
+		temp_mc = raw_adc * 569 - 394700;
+	}
+	*temp = temp_mc;
+
+	return 0;
+}
+
 static int loongson2_2k1000_get_temp(struct thermal_zone_device *tz, int *temp)
 {
 	int val;
 	struct loongson2_thermal_data *data = thermal_zone_device_priv(tz);
 
 	val = readl(data->ctrl_reg + LOONGSON2_THSENS_OUT_REG);
-	*temp = ((val & LOONGSON2_THSENS_OUT_MASK) - HECTO) * KILO;
+	*temp = ((val & LOONGSON2_THSENS_OUT_8B_MASK) - HECTO) * KILO;
 
 	return 0;
 }
@@ -113,6 +178,11 @@ static int loongson2_thermal_set_trips(struct thermal_zone_device *tz, int low,
 	return loongson2_thermal_set(data, low/MILLI, high/MILLI, true);
 }
 
+static const struct thermal_zone_device_ops loongson2_2k0300_of_thermal_ops = {
+	.get_temp = loongson2_2k0300_get_temp,
+	.set_trips = loongson2_thermal_set_trips,
+};
+
 static const struct thermal_zone_device_ops loongson2_2k1000_of_thermal_ops = {
 	.get_temp = loongson2_2k1000_get_temp,
 	.set_trips = loongson2_thermal_set_trips,
@@ -135,6 +205,8 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data->chip_data = device_get_match_data(dev);
+	data->flags = data->chip_data->flags;
+	data->dev = dev;
 
 	data->ctrl_reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(data->ctrl_reg))
@@ -147,6 +219,14 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
 			return PTR_ERR(data->temp_reg);
 	}
 
+	/* The chip id register is needed for Loongson-2K0300 */
+	if (data->chip_data->flags & LS2K0300_CHIP_ID_FLAG) {
+		data->regmap_cfg =
+			syscon_regmap_lookup_by_phandle(dev->of_node, "loongson,chipid");
+		if (IS_ERR(data->regmap_cfg))
+			return PTR_ERR(data->regmap_cfg);
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
@@ -179,6 +259,12 @@ static int loongson2_thermal_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct loongson2_thermal_chip_data loongson2_thermal_ls2k0300_data = {
+	.thermal_sensor_sel = 0,
+	.flags = LS2K0300_CHIP_ID_FLAG,
+	.thermal_ops = &loongson2_2k0300_of_thermal_ops,
+};
+
 static const struct loongson2_thermal_chip_data loongson2_thermal_ls2k1000_data = {
 	.thermal_sensor_sel = 0,
 	.flags = 0,
@@ -192,6 +278,10 @@ static const struct loongson2_thermal_chip_data loongson2_thermal_ls2k2000_data
 };
 
 static const struct of_device_id of_loongson2_thermal_match[] = {
+	{
+		.compatible = "loongson,ls2k0300-thermal",
+		.data = &loongson2_thermal_ls2k0300_data,
+	},
 	{
 		.compatible = "loongson,ls2k1000-thermal",
 		.data = &loongson2_thermal_ls2k1000_data,
-- 
2.52.0

Re: [PATCH v3 5/5] thermal/drivers/loongson2: Add support for Loongson-2K0300

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Hardware Logic Bug: Incorrect hardware trip point thresholds programmed for Loongson-2K0300.
- [High] Logic Error: Asymmetric handling and missing sign check for chip_id0 calibration data.
- [Medium] Logic Error: Off-by-one bitmask creates an 11-bit mask for a 10-bit ADC.
- [Medium] Performance Regression: Inefficient polling of static hardware calibration fuses.
- [Medium] Missing Error Handling: Ignored regmap_read return values silently use default zero offset.
--

--- Patch [5]: [PATCH v3 5/5] thermal/drivers/loongson2: Add support for Loongson-2K0300 ---
commit c989d07a95637f605eb53c63ff3b7f9dca11196a
Author: Binbin Zhou <zhoubinbin@loongson.cn>
Subject: thermal/drivers/loongson2: Add support for Loongson-2K0300

Add support for the thermal sensor found on the Loongson-2K0300 SoC.

> diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c
> index 1a8ca2cd06f1b..51b29b60372a5 100644
> --- a/drivers/thermal/loongson2_thermal.c
> +++ b/drivers/thermal/loongson2_thermal.c
[ ... ]
> @@ -23,18 +28,34 @@
>  #define LOONGSON2_THSENS_CTRL_LOW_REG	0x8
>  #define LOONGSON2_THSENS_STATUS_REG	0x10
>  #define LOONGSON2_THSENS_OUT_REG	0x14
> +#define LOONGSON2_THSENS_CFG_REG	0x18
>  
>  #define LOONGSON2_THSENS_INT_LO		BIT(0)
>  #define LOONGSON2_THSENS_INT_HIGH	BIT(1)
>  #define LOONGSON2_THSENS_INT_EN		(LOONGSON2_THSENS_INT_LO | \
>  					 LOONGSON2_THSENS_INT_HIGH)
> -#define LOONGSON2_THSENS_OUT_MASK	0xFF
> +#define LOONGSON2_THSENS_OUT_8B_MASK	0xFF
> +#define LOONGSON2_THSENS_OUT_11B_MASK	GENMASK(10, 0)

[Severity: Medium]
The commit message states the thermal sensor uses a 10-bit ADC, but this macro
creates an 11-bit mask. Does applying this 11-bit mask risk pulling in adjacent
status bits and corrupting the raw_adc value?

> +
> +#define LS2K0300_CHIP_ID0		0x10
> +#define LS2K0300_CHIP_ID1		0x14
> +#define LS2K0300_EXTERN_ID		BIT(4)
> +#define LS2K0300_ID0_VAL_MASK		GENMASK(31, 20)

[Severity: High]
Does this 12-bit extraction mask incorrectly truncate the data if ID0 was
intended to hold 16 bits like ID1?

> +#define LS2K0300_ID1_VAL_MASK		GENMASK(15, 0)
> +
> +#define LS2K0300_COMP_VAL_MASK		GENMASK(14, 0)
> +#define LS2K0300_COMP_SIGN_BIT		BIT(15)
> +
> +#define LS2K0300_LOWEST_VALID_TEMP	(-55000)
> +#define LS2K0300_HIGHEST_VALID_TEMP	(125000)
[ ... ]
> @@ -72,13 +96,54 @@ static int loongson2_thermal_set(struct loongson2_thermal_data *data,
>  	return 0;
>  }
>  
> +static int loongson2_2k0300_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct loongson2_thermal_data *tdata = thermal_zone_device_priv(tz);
> +	int calib_data, calib_offset, temp_mc, raw_adc;
> +	u32 chip_id0 = 0, chip_id1 = 0;
> +
> +	writel(0xff03, tdata->ctrl_reg + LOONGSON2_THSENS_CFG_REG);
> +	raw_adc = FIELD_GET(LOONGSON2_THSENS_OUT_11B_MASK,
> +			    readl(tdata->ctrl_reg + LOONGSON2_THSENS_OUT_REG));
> +
> +	if (tdata->flags & LS2K0300_OLD_FUSE_FLAG) {
> +		*temp = raw_adc * 569 - 394700;
> +		return 0;
> +	}
> +
> +	regmap_read(tdata->regmap_cfg, LS2K0300_CHIP_ID0, &chip_id0);
> +	regmap_read(tdata->regmap_cfg, LS2K0300_CHIP_ID1, &chip_id1);

[Severity: Medium]
Do we need to re-read the static calibration fuses on every temperature poll?

Since chip_id0 and chip_id1 don't change at runtime, does polling them
continuously introduce unnecessary MMIO read overhead and lock contention on
the shared syscon regmap?

[Severity: Medium]
Is it safe to ignore the return values of regmap_read() here?

If the reads fail, chip_id0 and chip_id1 will remain 0, leading to a silent
uncalibrated calculation. If the resulting temperature falls within the -55C
to 125C range, will this silently return inaccurate temperature data without
triggering the fallback mechanism or reporting the error?

> +
> +	if (chip_id0 & LS2K0300_EXTERN_ID) {
> +		calib_data = FIELD_GET(LS2K0300_ID1_VAL_MASK, chip_id1);
> +		calib_offset = FIELD_GET(LS2K0300_COMP_VAL_MASK, calib_data);
> +		if (calib_data & LS2K0300_COMP_SIGN_BIT)
> +			calib_offset = -calib_offset;
> +	} else {
> +		calib_data = FIELD_GET(LS2K0300_ID0_VAL_MASK, chip_id0);
> +		calib_offset = FIELD_GET(LS2K0300_COMP_VAL_MASK, calib_data);

[Severity: High]
When extracting calibration data from chip_id0, are we incorrectly applying a
15-bit mask to a 12-bit value and omitting the sign bit check?

If ID0 contains signed offsets similar to ID1, will negative offsets be
treated as large positive values?

> +	}
> +
[ ... ]
> @@ -113,6 +178,11 @@ static int loongson2_thermal_set_trips(struct thermal_zone_device *tz, int low,
>  	return loongson2_thermal_set(data, low/MILLI, high/MILLI, true);
>  }
>  
> +static const struct thermal_zone_device_ops loongson2_2k0300_of_thermal_ops = {
> +	.get_temp = loongson2_2k0300_get_temp,
> +	.set_trips = loongson2_thermal_set_trips,

[Severity: High]
Does assigning the legacy loongson2_thermal_set_trips function to the 2K0300
ops structure program the hardware comparators with the wrong values?

loongson2_thermal_set_trips() calls loongson2_set_ctrl_regs() which uses the
legacy formula:
    reg_ctrl = ctrl_data + HECTO;

But the 2K0300 uses a different formula:
    temp = (raw_adc + calib_offset) * 570 - 394700;

Will this cause the hardware interrupts to trigger at incorrect ADC values and
break thermal protection for the new SoC?

> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785480242.git.zhoubinbin@loongson.cn?part=5