* [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add A523 THS0/1 controllers
2026-07-04 17:14 [PATCH v5 0/5] Allwinner: A523: add support for A523 THS0/1 controllers Mikhail Kalashnikov
@ 2026-07-04 17:14 ` Mikhail Kalashnikov
2026-07-04 17:23 ` sashiko-bot
2026-07-05 9:24 ` Krzysztof Kozlowski
2026-07-04 17:14 ` [PATCH v5 2/5] thermal/drivers/sun8i: replace devm_reset_control_get to devm_reset_control_get_shared_deasserted Mikhail Kalashnikov
` (3 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Mikhail Kalashnikov @ 2026-07-04 17:14 UTC (permalink / raw)
To: Vasily Khoruzhick, Yangtao Li, Rafael J . Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel
Cc: linux-pm, devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
Add dt-bindings description of the thermal sensors in the A523 processor.
The controllers require activation of the additional frequency of the
associated gpadc controller, so a mod clock property required.
The calibration data for both thermal controllers (THS0, THS1) is split
across two nvmem cells at different offsets, requiring the two cells
to be read and merged. To maintain backward compatibility, the name of
the old cell remains "calibration" and the new cell is named
"calibration-second-part".
Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
---
.../thermal/allwinner,sun8i-a83t-ths.yaml | 54 +++++++++++++++++--
1 file changed, 51 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
index 3e61689f6..6f5b2f3b9 100644
--- a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
+++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
@@ -24,6 +24,8 @@ properties:
- allwinner,sun50i-h5-ths
- allwinner,sun50i-h6-ths
- allwinner,sun50i-h616-ths
+ - allwinner,sun55i-a523-ths0
+ - allwinner,sun55i-a523-ths1
clocks:
minItems: 1
@@ -47,11 +49,16 @@ properties:
maxItems: 1
nvmem-cells:
- maxItems: 1
- description: Calibration data for thermal sensors
+ minItems: 1
+ items:
+ - description: Calibration data for thermal sensors
+ - description: Additional cell in case of separate calibration data
nvmem-cell-names:
- const: calibration
+ minItems: 1
+ items:
+ - const: calibration
+ - const: calibration-second-part
allwinner,sram:
maxItems: 1
@@ -107,6 +114,7 @@ allOf:
enum:
- allwinner,sun8i-h3-ths
- allwinner,sun20i-d1-ths
+ - allwinner,sun55i-a523-ths0
then:
properties:
@@ -132,6 +140,29 @@ allOf:
- clock-names
- resets
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - allwinner,sun55i-a523-ths0
+ - allwinner,sun55i-a523-ths1
+ then:
+ properties:
+ clocks:
+ minItems: 2
+ nvmem-cells:
+ minItems: 2
+ nvmem-cell-names:
+ minItems: 2
+ else:
+ properties:
+ nvmem-cells:
+ maxItems: 1
+ nvmem-cell-names:
+ items:
+ - const: calibration
+
required:
- compatible
- reg
@@ -176,4 +207,21 @@ examples:
#thermal-sensor-cells = <1>;
};
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/sun55i-a523-ccu.h>
+ #include <dt-bindings/reset/sun55i-a523-ccu.h>
+
+ thermal-sensor@2009400 {
+ compatible = "allwinner,sun55i-a523-ths1";
+ reg = <0x02009400 0x400>;
+ interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_GPADC1>;
+ clock-names = "bus", "mod";
+ resets = <&ccu RST_BUS_THS>;
+ nvmem-cells = <&ths_calibration0>, <&ths_calibration1>;
+ nvmem-cell-names = "calibration",
+ "calibration-second-part";
+ #thermal-sensor-cells = <1>;
+ };
...
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add A523 THS0/1 controllers
2026-07-04 17:14 ` [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add " Mikhail Kalashnikov
@ 2026-07-04 17:23 ` sashiko-bot
2026-07-05 9:24 ` Krzysztof Kozlowski
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-04 17:23 UTC (permalink / raw)
To: Mikhail Kalashnikov; +Cc: linux-sunxi, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The fallback `else` branch fails to explicitly restrict `nvmem-cell-names` to `maxItems: 1` for legacy devices.
- [Low] The `clocks: minItems: 2` constraint in the new `allOf` block for A523 devices is redundant.
--
commit 3b55695ed8b61c3a70dec40b0b70e9958c234bfc
Author: Mikhail Kalashnikov <iuncuim@gmail.com>
dt-bindings: thermal: sun8i: Add A523 THS0/1 controllers
This commit adds device tree binding descriptions for the thermal sensors
in the A523 processor. It updates the schema to require an additional
clock for these variants and supports split calibration data across two
nvmem cells.
> diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> index 3e61689f6dd41..6f5b2f3b94f80 100644
> --- a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
[ ... ]
> @@ -132,6 +140,29 @@ allOf:
> - clock-names
> - resets
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - allwinner,sun55i-a523-ths0
> + - allwinner,sun55i-a523-ths1
> + then:
> + properties:
> + clocks:
> + minItems: 2
[Severity: Low]
Is this constraint redundant? The first allOf block in this file checks
for single-clock devices and its else branch already applies minItems: 2
for clocks and clock-names to all other devices. Since the A523 variant
is not in that single-clock list, shouldn't it already be constrained to
2 clocks?
> + nvmem-cells:
> + minItems: 2
> + nvmem-cell-names:
> + minItems: 2
> + else:
> + properties:
> + nvmem-cells:
> + maxItems: 1
> + nvmem-cell-names:
> + items:
> + - const: calibration
[Severity: Low]
Does this else block also need to specify maxItems: 1 for
nvmem-cell-names?
Redefining items as a tuple only validates the specified elements but
does not restrict the total array length. Without maxItems: 1, legacy
device trees could erroneously define two nvmem-cell-names and still
pass validation.
> +
> required:
> - compatible
> - reg
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260704171411.1413349-1-iuncuim@gmail.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add A523 THS0/1 controllers
2026-07-04 17:14 ` [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add " Mikhail Kalashnikov
2026-07-04 17:23 ` sashiko-bot
@ 2026-07-05 9:24 ` Krzysztof Kozlowski
1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-07-05 9:24 UTC (permalink / raw)
To: Mikhail Kalashnikov
Cc: Vasily Khoruzhick, Yangtao Li, Rafael J . Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel, linux-pm, devicetree, linux-arm-kernel,
linux-sunxi, linux-kernel
On Sun, Jul 05, 2026 at 01:14:07AM +0800, Mikhail Kalashnikov wrote:
> Add dt-bindings description of the thermal sensors in the A523 processor.
>
> The controllers require activation of the additional frequency of the
> associated gpadc controller, so a mod clock property required.
>
> The calibration data for both thermal controllers (THS0, THS1) is split
> across two nvmem cells at different offsets, requiring the two cells
> to be read and merged. To maintain backward compatibility, the name of
> the old cell remains "calibration" and the new cell is named
> "calibration-second-part".
>
> Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
> Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
> ---
> .../thermal/allwinner,sun8i-a83t-ths.yaml | 54 +++++++++++++++++--
> 1 file changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> index 3e61689f6..6f5b2f3b9 100644
> --- a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml
> @@ -24,6 +24,8 @@ properties:
> - allwinner,sun50i-h5-ths
> - allwinner,sun50i-h6-ths
> - allwinner,sun50i-h616-ths
> + - allwinner,sun55i-a523-ths0
> + - allwinner,sun55i-a523-ths1
>
> clocks:
> minItems: 1
> @@ -47,11 +49,16 @@ properties:
> maxItems: 1
>
> nvmem-cells:
> - maxItems: 1
> - description: Calibration data for thermal sensors
> + minItems: 1
> + items:
> + - description: Calibration data for thermal sensors
> + - description: Additional cell in case of separate calibration data
>
> nvmem-cell-names:
> - const: calibration
> + minItems: 1
> + items:
> + - const: calibration
> + - const: calibration-second-part
>
> allwinner,sram:
> maxItems: 1
> @@ -107,6 +114,7 @@ allOf:
> enum:
> - allwinner,sun8i-h3-ths
> - allwinner,sun20i-d1-ths
> + - allwinner,sun55i-a523-ths0
>
> then:
> properties:
> @@ -132,6 +140,29 @@ allOf:
> - clock-names
> - resets
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - allwinner,sun55i-a523-ths0
> + - allwinner,sun55i-a523-ths1
> + then:
> + properties:
> + clocks:
> + minItems: 2
Missing clock-names
Look at other variants here, how they did it.
> + nvmem-cells:
> + minItems: 2
> + nvmem-cell-names:
> + minItems: 2
> + else:
> + properties:
> + nvmem-cells:
> + maxItems: 1
> + nvmem-cell-names:
maxItems instead
> + items:
> + - const: calibration
> +
> required:
> - compatible
> - reg
> @@ -176,4 +207,21 @@ examples:
> #thermal-sensor-cells = <1>;
> };
>
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/sun55i-a523-ccu.h>
> + #include <dt-bindings/reset/sun55i-a523-ccu.h>
> +
> + thermal-sensor@2009400 {
You already have three examples, it is enough. Drop. Actually two would be
enough already...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 2/5] thermal/drivers/sun8i: replace devm_reset_control_get to devm_reset_control_get_shared_deasserted
2026-07-04 17:14 [PATCH v5 0/5] Allwinner: A523: add support for A523 THS0/1 controllers Mikhail Kalashnikov
2026-07-04 17:14 ` [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add " Mikhail Kalashnikov
@ 2026-07-04 17:14 ` Mikhail Kalashnikov
2026-07-04 17:14 ` [PATCH v5 3/5] thermal/drivers/sun8i: get calibration data from two nvmem cells Mikhail Kalashnikov
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Mikhail Kalashnikov @ 2026-07-04 17:14 UTC (permalink / raw)
To: Vasily Khoruzhick, Yangtao Li, Rafael J . Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel
Cc: linux-pm, devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
The A523 processor has two temperature controllers, but they share a
common reset line. Make it shared with the shared variant of
devm_reset_control_get(), and also simplify the driver by switching to
devm_reset_control_get_shared_deasserted().
Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
---
drivers/thermal/sun8i_thermal.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
index 226747906..df0c26970 100644
--- a/drivers/thermal/sun8i_thermal.c
+++ b/drivers/thermal/sun8i_thermal.c
@@ -342,11 +342,6 @@ static int sun8i_ths_calibrate(struct ths_device *tmdev)
return ret;
}
-static void sun8i_ths_reset_control_assert(void *data)
-{
- reset_control_assert(data);
-}
-
static struct regmap *sun8i_ths_get_sram_regmap(struct device_node *node)
{
struct platform_device *sram_pdev;
@@ -389,19 +384,10 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev)
return PTR_ERR(tmdev->regmap);
if (tmdev->chip->has_bus_clk_reset) {
- tmdev->reset = devm_reset_control_get(dev, NULL);
+ tmdev->reset = devm_reset_control_get_shared_deasserted(dev, NULL);
if (IS_ERR(tmdev->reset))
return PTR_ERR(tmdev->reset);
- ret = reset_control_deassert(tmdev->reset);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev, sun8i_ths_reset_control_assert,
- tmdev->reset);
- if (ret)
- return ret;
-
tmdev->bus_clk = devm_clk_get_enabled(&pdev->dev, "bus");
if (IS_ERR(tmdev->bus_clk))
return PTR_ERR(tmdev->bus_clk);
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v5 3/5] thermal/drivers/sun8i: get calibration data from two nvmem cells
2026-07-04 17:14 [PATCH v5 0/5] Allwinner: A523: add support for A523 THS0/1 controllers Mikhail Kalashnikov
2026-07-04 17:14 ` [PATCH v5 1/5] dt-bindings: thermal: sun8i: Add " Mikhail Kalashnikov
2026-07-04 17:14 ` [PATCH v5 2/5] thermal/drivers/sun8i: replace devm_reset_control_get to devm_reset_control_get_shared_deasserted Mikhail Kalashnikov
@ 2026-07-04 17:14 ` Mikhail Kalashnikov
2026-07-04 17:28 ` sashiko-bot
2026-07-04 17:14 ` [PATCH v5 4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers Mikhail Kalashnikov
2026-07-04 17:14 ` [PATCH v5 5/5] arm64: dts: allwinner: sun55i: add thermal sensors Mikhail Kalashnikov
4 siblings, 1 reply; 11+ messages in thread
From: Mikhail Kalashnikov @ 2026-07-04 17:14 UTC (permalink / raw)
To: Vasily Khoruzhick, Yangtao Li, Rafael J . Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel
Cc: linux-pm, devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
The A523 processor has calibration data in two nvmem cells. To be able to
add support, the ability to add data from two cells into one array must be
added.
Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
---
drivers/thermal/sun8i_thermal.c | 80 +++++++++++++++++++++------------
1 file changed, 51 insertions(+), 29 deletions(-)
diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
index df0c26970..3bdd62aa8 100644
--- a/drivers/thermal/sun8i_thermal.c
+++ b/drivers/thermal/sun8i_thermal.c
@@ -301,43 +301,65 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
static int sun8i_ths_calibrate(struct ths_device *tmdev)
{
- struct nvmem_cell *calcell;
+ struct nvmem_cell *calcell = NULL;
struct device *dev = tmdev->dev;
- u16 *caldata;
- size_t callen;
+ struct device_node *np = dev_of_node(dev);
+ struct property *prop;
+ const char *cellname;
+ u8 *caldata = NULL;
+ size_t callen = 0;
int ret = 0;
- calcell = nvmem_cell_get(dev, "calibration");
- if (IS_ERR(calcell)) {
- if (PTR_ERR(calcell) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- /*
- * Even if the external calibration data stored in sid is
- * not accessible, the THS hardware can still work, although
- * the data won't be so accurate.
- *
- * The default value of calibration register is 0x800 for
- * every sensor, and the calibration value is usually 0x7xx
- * or 0x8xx, so they won't be away from the default value
- * for a lot.
- *
- * So here we do not return error if the calibration data is
- * not available, except the probe needs deferring.
- */
- goto out;
- }
+ of_property_for_each_string(np, "nvmem-cell-names", prop, cellname) {
+ size_t len;
+ u8 *caldatapart;
+
+ calcell = of_nvmem_cell_get(np, cellname);
+ if (IS_ERR(calcell)) {
+ if (PTR_ERR(calcell) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ /*
+ * Even if the external calibration data stored in sid is
+ * not accessible, the THS hardware can still work, although
+ * the data won't be so accurate.
+ *
+ * The default value of calibration register is 0x800 for
+ * every sensor, and the calibration value is usually 0x7xx
+ * or 0x8xx, so they won't be away from the default value
+ * for a lot.
+ *
+ * So here we do not return error if the calibration data is
+ * not available, except the probe needs deferring.
+ */
+ goto out;
+ }
+
+ caldatapart = nvmem_cell_read(calcell, &len);
+ nvmem_cell_put(calcell);
+ calcell = NULL;
+ if (IS_ERR(caldatapart)) {
+ ret = PTR_ERR(caldatapart);
+ goto out;
+ }
+
+ caldata = devm_krealloc(dev, caldata, callen + len, GFP_KERNEL);
+ if (!caldata) {
+ kfree(caldatapart);
+ ret = -ENOMEM;
+ goto out;
+ }
- caldata = nvmem_cell_read(calcell, &callen);
- if (IS_ERR(caldata)) {
- ret = PTR_ERR(caldata);
- goto out;
+ memcpy(caldata + callen, caldatapart, len);
+ callen += len;
+ kfree(caldatapart);
}
- tmdev->chip->calibrate(tmdev, caldata, callen);
+ tmdev->chip->calibrate(tmdev, (u16 *)caldata, callen);
- kfree(caldata);
+ devm_kfree(dev, caldata);
+ caldata = NULL;
out:
- if (!IS_ERR(calcell))
+ if (!IS_ERR_OR_NULL(calcell))
nvmem_cell_put(calcell);
return ret;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 3/5] thermal/drivers/sun8i: get calibration data from two nvmem cells
2026-07-04 17:14 ` [PATCH v5 3/5] thermal/drivers/sun8i: get calibration data from two nvmem cells Mikhail Kalashnikov
@ 2026-07-04 17:28 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-04 17:28 UTC (permalink / raw)
To: Mikhail Kalashnikov; +Cc: linux-sunxi, devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing or empty 'nvmem-cell-names' property causes an unconditional NULL pointer dereference in the calibration callback.
- [Medium] The newly introduced A523 calibration function accesses fixed indices of the dynamically sized caldata array without validating callen.
--
commit 385d056a66304901aadb5675fbf878e2aae0cfc7
Author: Mikhail Kalashnikov <iuncuim@gmail.com>
thermal/drivers/sun8i: get calibration data from two nvmem cells
This commit updates the sun8i thermal driver to support reading calibration
data from multiple nvmem cells by iterating over nvmem-cell-names. It appends
the data from each cell into a single dynamically allocated buffer which is
passed to the calibration callbacks. This provides the ability to support the
A523 processor which has calibration data split across two cells.
> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index df0c269708ea7..3bdd62aa8bdf1 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c
> @@ -301,43 +301,65 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
>
> static int sun8i_ths_calibrate(struct ths_device *tmdev)
> {
> - struct nvmem_cell *calcell;
> + struct nvmem_cell *calcell = NULL;
> struct device *dev = tmdev->dev;
> - u16 *caldata;
> - size_t callen;
> + struct device_node *np = dev_of_node(dev);
> + struct property *prop;
> + const char *cellname;
> + u8 *caldata = NULL;
> + size_t callen = 0;
> int ret = 0;
>
> - calcell = nvmem_cell_get(dev, "calibration");
> - if (IS_ERR(calcell)) {
[ ... ]
> + memcpy(caldata + callen, caldatapart, len);
> + callen += len;
> + kfree(caldatapart);
> }
>
> - tmdev->chip->calibrate(tmdev, caldata, callen);
> + tmdev->chip->calibrate(tmdev, (u16 *)caldata, callen);
[Severity: High]
If the nvmem-cell-names property is missing from the device tree, the
of_property_for_each_string() loop is bypassed entirely and caldata remains
NULL.
Does this unconditionally pass a NULL pointer to the backend calibration
callback?
Backend callbacks like sun8i_h3_ths_calibrate() dereference the array directly:
if (!caldata[0] || callen < 2 * tmdev->chip->sensor_num)
return -EINVAL;
If the device tree does not contain calibration nvmem cells, this will cause a
kernel panic on boot.
[Severity: Medium]
For the A523 processor, does sun55i_a523_ths_calibrate() validate that callen
is large enough before accessing the array?
Looking at sun55i_a523_ths_calibrate() which was recently introduced:
if (tmdev->chip->sensor_num == 1) {
sensor_reg = ((caldata[5] >> 8) | (caldata[6] << 8)) & TEMP_CALIB_MASK;
} else {
It accesses elements up to caldata[6] without verifying that callen covers it.
If a device tree defines nvmem cells that yield less than 14 bytes of data,
will these lookups read out of bounds?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260704171411.1413349-1-iuncuim@gmail.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers
2026-07-04 17:14 [PATCH v5 0/5] Allwinner: A523: add support for A523 THS0/1 controllers Mikhail Kalashnikov
` (2 preceding siblings ...)
2026-07-04 17:14 ` [PATCH v5 3/5] thermal/drivers/sun8i: get calibration data from two nvmem cells Mikhail Kalashnikov
@ 2026-07-04 17:14 ` Mikhail Kalashnikov
2026-07-04 17:29 ` sashiko-bot
2026-07-04 17:14 ` [PATCH v5 5/5] arm64: dts: allwinner: sun55i: add thermal sensors Mikhail Kalashnikov
4 siblings, 1 reply; 11+ messages in thread
From: Mikhail Kalashnikov @ 2026-07-04 17:14 UTC (permalink / raw)
To: Vasily Khoruzhick, Yangtao Li, Rafael J . Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel
Cc: linux-pm, devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
The A523 processor has two temperature controllers, THS0 and THS1.
THS0 has only one temperature sensor, which is located in the DRAM
controller. THS1 does have 3 sensors:
ths1_0 - "big" cores
ths1_1 - "little" cores
ths1_2 - gpu
The datasheet mentions a fourth sensor in the NPU, but lacks any registers
for operation other than calibration registers. The vendor code reads the
value from ths1_2, but uses separate calibration data, so we get two
different values from real one.
Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
---
drivers/thermal/sun8i_thermal.c | 133 ++++++++++++++++++++++++++++++++
1 file changed, 133 insertions(+)
diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
index 3bdd62aa8..f48ed9eae 100644
--- a/drivers/thermal/sun8i_thermal.c
+++ b/drivers/thermal/sun8i_thermal.c
@@ -59,6 +59,12 @@
#define SUN50I_H6_THS_PC_TEMP_PERIOD(x) ((GENMASK(19, 0) & (x)) << 12)
#define SUN50I_H6_THS_DATA_IRQ_STS(x) BIT(x)
+#define SUN55I_A523_DELIMITER 0x7c8
+#define SUN55I_A523_OFFSET_ABOVE 2736
+#define SUN55I_A523_OFFSET_BELOW 2825
+#define SUN55I_A523_SCALE_ABOVE 74
+#define SUN55I_A523_SCALE_BELOW 65
+
struct tsensor {
struct ths_device *tmdev;
struct thermal_zone_device *tzd;
@@ -114,6 +120,15 @@ static int sun50i_h5_calc_temp(struct ths_device *tmdev,
return -1590 * reg / 10 + 276000;
}
+static int sun55i_a523_calc_temp(struct ths_device *tmdev,
+ int id, int reg)
+{
+ if (reg >= SUN55I_A523_DELIMITER)
+ return SUN55I_A523_SCALE_ABOVE * (SUN55I_A523_OFFSET_ABOVE - reg);
+ else
+ return SUN55I_A523_SCALE_BELOW * (SUN55I_A523_OFFSET_BELOW - reg);
+}
+
static int sun8i_ths_get_temp(struct thermal_zone_device *tz, int *temp)
{
struct tsensor *s = thermal_zone_device_priv(tz);
@@ -299,6 +314,97 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
return 0;
}
+/*
+ * The A523 nvmem calibration values. The ths1_3 is not used as it
+ * doesn't have its own sensor and doesn't have any internal switch.
+ * Instead, the value from the ths1_2 sensor is used, which gives the
+ * illusion of an independent sensor for NPU and GPU when using
+ * different calibration values.
+ *
+ * efuse layout 0x38-0x3F (caldata[0..3]):
+ * caldata[0] caldata[1] caldata[2] caldata[3]
+ * 0 16 24 32 36 48 60 64
+ * +---------------+---------------+---------------+---------------+
+ * | | | temp | ths1_0 | ths1_1 | +
+ * +---------------+---------------+---------------+---------------+
+ *
+ * efuse layout 0x44-0x4B (caldata[4..7]):
+ * caldata[4] caldata[5] caldata[6] caldata[7]
+ * 0 12 16 24 32 36 48 64
+ * +---------------+---------------+---------------+---------------+
+ * | ths1_2 | ths1_3 | ths0 | | +
+ * +---------------+---------------+---------------+---------------+
+ */
+static int sun55i_a523_ths_calibrate(struct ths_device *tmdev,
+ u16 *caldata, int callen)
+{
+ struct device *dev = tmdev->dev;
+ int i, ft_temp;
+
+ if (!caldata[0])
+ return -EINVAL;
+
+ ft_temp = (((caldata[2] << 8) | (caldata[1] >> 8)) & FT_TEMP_MASK) * 100;
+
+ for (i = 0; i < tmdev->chip->sensor_num; i++) {
+ int sensor_reg, sensor_temp, cdata, offset;
+ /*
+ * Chips ths0 and ths1 have common parameters for value
+ * calibration. To separate them we can use the number of
+ * temperature sensors on each chip.
+ * For ths0 this value is 1.
+ */
+ if (tmdev->chip->sensor_num == 1) {
+ sensor_reg = ((caldata[5] >> 8) | (caldata[6] << 8)) & TEMP_CALIB_MASK;
+ } else {
+ switch (i) {
+ case 0:
+ sensor_reg = (caldata[2] >> 4) & TEMP_CALIB_MASK;
+ break;
+ case 1:
+ sensor_reg = caldata[3] & TEMP_CALIB_MASK;
+ break;
+ case 2:
+ sensor_reg = caldata[4] & TEMP_CALIB_MASK;
+ break;
+ default:
+ sensor_reg = 0;
+ break;
+ }
+ }
+
+ sensor_temp = tmdev->chip->calc_temp(tmdev, i, sensor_reg);
+
+ /*
+ * Calibration data is CALIBRATE_DEFAULT - (calculated
+ * temperature from sensor reading at factory temperature
+ * minus actual factory temperature) * X (scale from
+ * temperature to register values)
+ */
+ cdata = CALIBRATE_DEFAULT -
+ ((sensor_temp - ft_temp) / SUN55I_A523_SCALE_ABOVE);
+
+ if (cdata & ~TEMP_CALIB_MASK) {
+ /*
+ * Calibration value more than 12-bit, but calibration
+ * register is 12-bit. In this case, ths hardware can
+ * still work without calibration, although the data
+ * won't be so accurate.
+ */
+ dev_warn(dev, "sensor%d is not calibrated.\n", i);
+ continue;
+ }
+
+ offset = (i % 2) * 16;
+ regmap_update_bits(tmdev->regmap,
+ SUN50I_H6_THS_TEMP_CALIB + (i / 2 * 4),
+ TEMP_CALIB_MASK << offset,
+ cdata << offset);
+ }
+
+ return 0;
+}
+
static int sun8i_ths_calibrate(struct ths_device *tmdev)
{
struct nvmem_cell *calcell = NULL;
@@ -717,6 +823,31 @@ static const struct ths_thermal_chip sun50i_h616_ths = {
.calc_temp = sun8i_ths_calc_temp,
};
+/* The A523 has a shared reset line for both chips */
+static const struct ths_thermal_chip sun55i_a523_ths0 = {
+ .sensor_num = 1,
+ .has_bus_clk_reset = true,
+ .has_mod_clk = true,
+ .ft_deviation = 5000,
+ .temp_data_base = SUN50I_H6_THS_TEMP_DATA,
+ .calibrate = sun55i_a523_ths_calibrate,
+ .init = sun50i_h6_thermal_init,
+ .irq_ack = sun50i_h6_irq_ack,
+ .calc_temp = sun55i_a523_calc_temp,
+};
+
+static const struct ths_thermal_chip sun55i_a523_ths1 = {
+ .sensor_num = 3,
+ .has_bus_clk_reset = true,
+ .has_mod_clk = true,
+ .ft_deviation = 5000,
+ .temp_data_base = SUN50I_H6_THS_TEMP_DATA,
+ .calibrate = sun55i_a523_ths_calibrate,
+ .init = sun50i_h6_thermal_init,
+ .irq_ack = sun50i_h6_irq_ack,
+ .calc_temp = sun55i_a523_calc_temp,
+};
+
static const struct of_device_id of_ths_match[] = {
{ .compatible = "allwinner,sun8i-a83t-ths", .data = &sun8i_a83t_ths },
{ .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths },
@@ -727,6 +858,8 @@ static const struct of_device_id of_ths_match[] = {
{ .compatible = "allwinner,sun50i-h6-ths", .data = &sun50i_h6_ths },
{ .compatible = "allwinner,sun20i-d1-ths", .data = &sun20i_d1_ths },
{ .compatible = "allwinner,sun50i-h616-ths", .data = &sun50i_h616_ths },
+ { .compatible = "allwinner,sun55i-a523-ths0", .data = &sun55i_a523_ths0 },
+ { .compatible = "allwinner,sun55i-a523-ths1", .data = &sun55i_a523_ths1 },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, of_ths_match);
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers
2026-07-04 17:14 ` [PATCH v5 4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers Mikhail Kalashnikov
@ 2026-07-04 17:29 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-04 17:29 UTC (permalink / raw)
To: Mikhail Kalashnikov; +Cc: robh, devicetree, conor+dt, linux-sunxi
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The calibration check `if (!caldata[0])` will always fail and abort calibration if bits 0-15 of the A523 eFuse are zero.
- [High] NULL pointer dereference if the `nvmem-cell-names` property is missing from the Device Tree.
- [Medium] Out-of-bounds heap read due to missing `callen` validation in `sun55i_a523_ths_calibrate`.
- [Medium] Calibration logic incorrectly hardcodes `SUN55I_A523_SCALE_ABOVE` to convert temperature error into a register offset.
--
commit ed04dff3819d46435d0d0c6fa95dfd782738eb1e
Author: Mikhail Kalashnikov <iuncuim@gmail.com>
thermal/drivers/sun8i: Add support for A523 THS0/1 controllers
The A523 processor has two temperature controllers, THS0 and THS1.
THS0 has only one temperature sensor, which is located in the DRAM
controller. THS1 does have 3 sensors:
ths1_0 - "big" cores
ths1_1 - "little" cores
ths1_2 - gpu
The datasheet mentions a fourth sensor in the NPU, but lacks any registers
for operation other than calibration registers. The vendor code reads the
value from ths1_2, but uses separate calibration data, so we get two
different values from real one.
> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index 3bdd62aa8bdf1..f48ed9eae6c4f 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c
[ ... ]
> @@ -299,6 +314,97 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
[ ... ]
> +static int sun55i_a523_ths_calibrate(struct ths_device *tmdev,
> + u16 *caldata, int callen)
> +{
> + struct device *dev = tmdev->dev;
> + int i, ft_temp;
> +
> + if (!caldata[0])
[Severity: High]
If the device tree lacks the nvmem-cell-names property, the loop inside
sun8i_ths_calibrate() will be skipped entirely. This leaves caldata as
NULL when it is passed to the chip-specific calibrate callback:
sun8i_ths_calibrate() {
...
of_property_for_each_string(...) {
...
}
tmdev->chip->calibrate(tmdev, (u16 *)caldata, callen);
...
}
Will checking !caldata[0] here cause a NULL pointer dereference in that
scenario?
[Severity: High]
According to the eFuse layout diagram in the comment block just above this
function, bits 0-15 are empty/unused, while the actual factory temperature
is stored in bits 24-35 (caldata[1] and caldata[2]).
If those unused eFuse bits are typically zero, will this check silently
abort the calibration process on the A523?
> + return -EINVAL;
> +
> + ft_temp = (((caldata[2] << 8) | (caldata[1] >> 8)) & FT_TEMP_MASK) * 100;
> +
> + for (i = 0; i < tmdev->chip->sensor_num; i++) {
> + int sensor_reg, sensor_temp, cdata, offset;
> + /*
> + * Chips ths0 and ths1 have common parameters for value
> + * calibration. To separate them we can use the number of
> + * temperature sensors on each chip.
> + * For ths0 this value is 1.
> + */
> + if (tmdev->chip->sensor_num == 1) {
> + sensor_reg = ((caldata[5] >> 8) | (caldata[6] << 8)) & TEMP_CALIB_MASK;
[Severity: Medium]
Is there a chance we might read out of bounds here if the provided nvmem
data is smaller than expected?
If the device tree or nvmem cell provides fewer bytes than the 14 bytes
required to access caldata[6], this could result in an out-of-bounds heap
read.
Should we validate callen before accessing these offsets?
> + } else {
> + switch (i) {
> + case 0:
> + sensor_reg = (caldata[2] >> 4) & TEMP_CALIB_MASK;
> + break;
[ ... ]
> + sensor_temp = tmdev->chip->calc_temp(tmdev, i, sensor_reg);
> +
> + /*
> + * Calibration data is CALIBRATE_DEFAULT - (calculated
> + * temperature from sensor reading at factory temperature
> + * minus actual factory temperature) * X (scale from
> + * temperature to register values)
> + */
> + cdata = CALIBRATE_DEFAULT -
> + ((sensor_temp - ft_temp) / SUN55I_A523_SCALE_ABOVE);
[Severity: Medium]
The sun55i_a523_calc_temp() function uses a piecewise formula depending
on the register value, applying either SUN55I_A523_SCALE_ABOVE or
SUN55I_A523_SCALE_BELOW.
However, the calibration offset calculation here unconditionally divides
the temperature error by SUN55I_A523_SCALE_ABOVE.
If the factory temperature corresponds to a register value in the
SUN55I_A523_SCALE_BELOW region, will dividing by the SCALE_ABOVE
value result in an inaccurate calibration offset?
> +
> + if (cdata & ~TEMP_CALIB_MASK) {
> + /*
> + * Calibration value more than 12-bit, but calibration
> + * register is 12-bit. In this case, ths hardware can
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260704171411.1413349-1-iuncuim@gmail.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 5/5] arm64: dts: allwinner: sun55i: add thermal sensors
2026-07-04 17:14 [PATCH v5 0/5] Allwinner: A523: add support for A523 THS0/1 controllers Mikhail Kalashnikov
` (3 preceding siblings ...)
2026-07-04 17:14 ` [PATCH v5 4/5] thermal/drivers/sun8i: Add support for A523 THS0/1 controllers Mikhail Kalashnikov
@ 2026-07-04 17:14 ` Mikhail Kalashnikov
2026-07-04 17:21 ` sashiko-bot
4 siblings, 1 reply; 11+ messages in thread
From: Mikhail Kalashnikov @ 2026-07-04 17:14 UTC (permalink / raw)
To: Vasily Khoruzhick, Yangtao Li, Rafael J . Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel
Cc: linux-pm, devicetree, linux-arm-kernel, linux-sunxi, linux-kernel
The A523 processor has two temperature controllers, THS0 and THS1.
THS0 has only one temperature sensor, which is located in the DRAM
controller.
THS1 does have 3 sensors:
ths1_0 - "big" cores
ths1_1 - "little" cores
ths1_2 - gpu
Add the thermal sensor configuration and the thermal zones.
Trips temperature, polling-delay and sustainable-power parameters are
derived from the manufacturer's BSP.
Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
---
.../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 160 ++++++++++++++++++
1 file changed, 160 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
index ca6a16807..2ed00e7a7 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
@@ -11,6 +11,7 @@
#include <dt-bindings/reset/sun55i-a523-r-ccu.h>
#include <dt-bindings/power/allwinner,sun55i-a523-ppu.h>
#include <dt-bindings/power/allwinner,sun55i-a523-pck-600.h>
+#include <dt-bindings/thermal/thermal.h>
/ {
interrupt-parent = <&gic>;
@@ -26,6 +27,7 @@ cpu0: cpu@0 {
device_type = "cpu";
reg = <0x000>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
cpu1: cpu@100 {
@@ -33,6 +35,7 @@ cpu1: cpu@100 {
device_type = "cpu";
reg = <0x100>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
cpu2: cpu@200 {
@@ -40,6 +43,7 @@ cpu2: cpu@200 {
device_type = "cpu";
reg = <0x200>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
cpu3: cpu@300 {
@@ -47,6 +51,7 @@ cpu3: cpu@300 {
device_type = "cpu";
reg = <0x300>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
cpu4: cpu@400 {
@@ -54,6 +59,7 @@ cpu4: cpu@400 {
device_type = "cpu";
reg = <0x400>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
cpu5: cpu@500 {
@@ -61,6 +67,7 @@ cpu5: cpu@500 {
device_type = "cpu";
reg = <0x500>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
cpu6: cpu@600 {
@@ -68,6 +75,7 @@ cpu6: cpu@600 {
device_type = "cpu";
reg = <0x600>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
cpu7: cpu@700 {
@@ -75,6 +83,7 @@ cpu7: cpu@700 {
device_type = "cpu";
reg = <0x700>;
enable-method = "psci";
+ #cooling-cells = <2>;
};
};
@@ -355,6 +364,32 @@ gpadc: adc@2009000 {
status = "disabled";
};
+ ths1: thermal-sensor@2009400 {
+ compatible = "allwinner,sun55i-a523-ths1";
+ reg = <0x02009400 0x400>;
+ interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_GPADC1>;
+ clock-names = "bus", "mod";
+ resets = <&ccu RST_BUS_THS>;
+ nvmem-cells = <&ths_calibration0>, <&ths_calibration1>;
+ nvmem-cell-names = "calibration",
+ "calibration-second-part";
+ #thermal-sensor-cells = <1>;
+ };
+
+ ths0: thermal-sensor@200a000 {
+ compatible = "allwinner,sun55i-a523-ths0";
+ reg = <0x0200a000 0x400>;
+ interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_GPADC0>;
+ clock-names = "bus", "mod";
+ resets = <&ccu RST_BUS_THS>;
+ nvmem-cells = <&ths_calibration0>, <&ths_calibration1>;
+ nvmem-cell-names = "calibration",
+ "calibration-second-part";
+ #thermal-sensor-cells = <0>;
+ };
+
wdt: watchdog@2050000 {
compatible = "allwinner,sun55i-a523-wdt";
reg = <0x2050000 0x20>;
@@ -586,6 +621,14 @@ sid: efuse@3006000 {
reg = <0x03006000 0x1000>;
#address-cells = <1>;
#size-cells = <1>;
+
+ ths_calibration0: ths-calibration0@38 {
+ reg = <0x38 0x8>;
+ };
+
+ ths_calibration1: ths-calibration1@44 {
+ reg = <0x44 0x8>;
+ };
};
gic: interrupt-controller@3400000 {
@@ -1102,4 +1145,121 @@ npu: npu@7122000 {
power-domains = <&ppu PD_NPU>;
};
};
+
+ thermal-zones {
+ cpu0_thermal: cpu0-thermal {
+ polling-delay-passive = <100>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths1 1>;
+ sustainable-power = <1200>;
+
+ trips {
+ cpu0_threshold: cpu-trip-0 {
+ temperature = <70000>;
+ type = "passive";
+ hysteresis = <0>;
+ };
+
+ cpu0_target: cpu-trip-1 {
+ temperature = <90000>;
+ type = "passive";
+ hysteresis = <0>;
+ };
+
+ cpu0_critical: cpu-trip-2 {
+ temperature = <110000>;
+ type = "critical";
+ hysteresis = <0>;
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu0_target>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ cpu4_thermal: cpu4-thermal {
+ polling-delay-passive = <100>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths1 0>;
+ sustainable-power = <1600>;
+
+ trips {
+ cpu4_threshold: cpu-trip-0 {
+ temperature = <70000>;
+ type = "passive";
+ hysteresis = <0>;
+ };
+
+ cpu4_target: cpu-trip-1 {
+ temperature = <90000>;
+ type = "passive";
+ hysteresis = <0>;
+ };
+
+ cpu4_critical: cpu-trip-2 {
+ temperature = <110000>;
+ type = "critical";
+ hysteresis = <0>;
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&cpu4_target>;
+ cooling-device = <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ gpu-thermal {
+ polling-delay-passive = <100>;
+ polling-delay = <1000>;
+ thermal-sensors = <&ths1 2>;
+ sustainable-power = <2400>;
+
+ gpu-trips {
+ gpu_temp_threshold: gpu-trip-0 {
+ temperature = <60000>;
+ type = "passive";
+ hysteresis = <0>;
+ };
+
+ gpu_temp_target: gpu-trip-1 {
+ temperature = <90000>;
+ type = "passive";
+ hysteresis = <0>;
+ };
+
+ gpu_temp_critical: gpu-trip-2 {
+ temperature = <110000>;
+ type = "critical";
+ hysteresis = <0>;
+ };
+ };
+ };
+
+ ddr-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <&ths0>;
+
+ trips {
+ ddr_temp_critical: ddr-trip-0 {
+ temperature = <110000>;
+ type = "critical";
+ hysteresis = <0>;
+ };
+ };
+ };
+ };
};
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v5 5/5] arm64: dts: allwinner: sun55i: add thermal sensors
2026-07-04 17:14 ` [PATCH v5 5/5] arm64: dts: allwinner: sun55i: add thermal sensors Mikhail Kalashnikov
@ 2026-07-04 17:21 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-04 17:21 UTC (permalink / raw)
To: Mikhail Kalashnikov; +Cc: devicetree, linux-sunxi, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The GPU thermal zone incorrectly names its trip points node 'gpu-trips' instead of 'trips', causing the thermal core to silently ignore all GPU trip points.
--
commit 2bc13bc055e98065b94ef6b295284d00ab1541b4
Author: Mikhail Kalashnikov <iuncuim@gmail.com>
arm64: dts: allwinner: sun55i: add thermal sensors
This commit adds thermal sensor configuration and thermal zones for the A523
processor, including THS0 and THS1 sensors. It defines trip temperatures,
polling delays, and cooling maps for the CPUs, GPU, and DDR.
> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> index 5afa8d92acbfb..07ff74a955441 100644
> --- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
[ ... ]
> + gpu-thermal {
> + polling-delay-passive = <100>;
> + polling-delay = <1000>;
> + thermal-sensors = <&ths1 2>;
> + sustainable-power = <2400>;
> +
> + gpu-trips {
[Severity: High]
Will this node name cause the thermal OF framework to skip parsing the GPU
trip points?
Looking at thermal_of_trips_init() in drivers/thermal/thermal_of.c, it
specifically looks for a child node named exactly "trips" using
of_get_child_by_name(np, "trips").
Because this is named "gpu-trips", it appears the GPU trip points will be
silently ignored by the parser. This could leave the GPU without thermal
protection, including the 110-degree critical shutdown threshold. Should
this be renamed to "trips"?
> + gpu_temp_threshold: gpu-trip-0 {
> + temperature = <60000>;
> + type = "passive";
> + hysteresis = <0>;
> + };
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260704171411.1413349-1-iuncuim@gmail.com?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread