[PATCH v5 0/7] thermal/drivers: airoha: Add support for AN7583

[PATCH v5 0/7] thermal/drivers: airoha: Add support for AN7583

[Christian Marangi]

This small series implement all the required changes to
support Airoha AN7583 Thermal Sensor.

The SoC dropped the Monitor subsystem and only provide
temperature reading. Some generalization was required
to at least save the common code with also the help of
reg field API.

Changes v5:
- Fix alpabetical order in Documentation patch
- Address suggested change by AI Bot
Changes v4:
- Rebase on top of linux-next
Changes v3:
- Property use chip-scu for thermal sensor
Changes v2:
- Update DT schema patch to implement dedicated schema

Christian Marangi (7):
  thermal/drivers: airoha: fix copy paste error on clamp_t low temp
  thermal/drivers: airoha: fix copy paste error for sen internal
  thermal/drivers: airoha: Convert to regmap API
  thermal/drivers: airoha: Generalize probe function
  thermal/drivers: airoha: Generalize get_thermal_ADC and set_mux
    function
  dt-bindings: arm: airoha: Add the chip-scu node for AN7583 SoC
  thermal/drivers: airoha: Add support for AN7583 Thermal Sensor

 .../bindings/arm/airoha,en7581-chip-scu.yaml  |  14 +
 drivers/thermal/airoha_thermal.c              | 380 +++++++++++++++---
 2 files changed, 327 insertions(+), 67 deletions(-)

-- 
2.53.0

[PATCH v5 1/7] thermal/drivers: airoha: fix copy paste error on clamp_t low temp

[Christian Marangi]

In airoha_thermal_set_trips, there is a copy paste error on clamping the
value for the low trip temp point. Fix it to the correct value and actually
clamp for the low variable.

Fixes: 42de37f40e1b ("thermal/drivers: Add support for Airoha EN7581 thermal sensor")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/thermal/airoha_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
index b9fd6bfc88e5..439aa011b75c 100644
--- a/drivers/thermal/airoha_thermal.c
+++ b/drivers/thermal/airoha_thermal.c
@@ -273,7 +273,7 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
 
 	if (low != -INT_MAX) {
 		/* Validate low and clamp it to a supported value */
-		low = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
+		low = clamp_t(int, low, RAW_TO_TEMP(priv, 0),
 			      RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
 
 		/* We offset the low temp of 1°C to trigger correct event */
-- 
2.53.0

Re: [PATCH v5 1/7] thermal/drivers: airoha: fix copy paste error on clamp_t low temp

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver fails to clear old thresholds or disable the thermal monitor when trip points are disabled, potentially causing an interrupt storm.
- [High] The interrupt handler uses a `switch` statement on a bitmask, silently dropping events if both hot and cold interrupts are asserted simultaneously.
- [High] The thermal interrupt is requested before the thermal zone is registered, which can lead to a NULL pointer dereference.
--

commit e9247122d90d0d1f57f8075a6a171215a771b2d6
Author: Christian Marangi <ansuelsmth@gmail.com>

thermal/drivers: airoha: fix copy paste error on clamp_t low temp

In airoha_thermal_set_trips, there is a copy paste error on clamping the
value for the low trip temp point. Fix it to the correct value and actually
clamp for the low variable.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index b9fd6bfc88e5e..439aa011b75c7 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c
> @@ -273,7 +273,7 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
>  
>  	if (low != -INT_MAX) {
>  		/* Validate low and clamp it to a supported value */
> -		low = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
> +		low = clamp_t(int, low, RAW_TO_TEMP(priv, 0),
>  			      RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
>  
>  		/* We offset the low temp of 1°C to trigger correct event */

[Severity: High]
This is a pre-existing issue, but does this function properly handle the case
where trip points are disabled?

If a trip point boundary is disabled (e.g., passing INT_MAX for the high trip),
its corresponding threshold register (EN7581_TEMPOFFSETH) is not updated. If
both boundaries are disabled, enable_monitor is set to false, and the monitor is
never explicitly disabled via EN7581_TEMPMONCTL0.

This leaves stale thresholds active in hardware. If the temperature crosses
these stale thresholds, the hardware will continue asserting interrupts.
Because the trip is disabled in software, the thermal core will not take
action to adjust the boundaries, which could lead to an unhandled interrupt
storm that can render the system unusable.


[Severity: High]
This is another pre-existing issue, but could the interrupt handler silently
drop events if both hot and cold interrupts assert simultaneously?

Looking at airoha_thermal_irq():

drivers/thermal/airoha_thermal.c:airoha_thermal_irq() {
	...
	status = readl(priv->base + EN7581_TEMPMONINTSTS);
	switch (status & (EN7581_HOFSINTSTS0 | EN7581_LOFSINTSTS0)) {
	case EN7581_HOFSINTSTS0:
		event = THERMAL_TRIP_VIOLATED;
		update = true;
		break;
	case EN7581_LOFSINTSTS0:
		event = THERMAL_EVENT_UNSPECIFIED;
		update = true;
		break;
	default:
		/* Should be impossible as we enable only these Interrupt */
		break;
	}

	/* Reset Interrupt */
	writel(status, priv->base + EN7581_TEMPMONINTSTS);

	if (update)
		thermal_zone_device_update(priv->tz, event);
	...
}

If the hardware asserts both the hot and cold interrupt bits simultaneously
(e.g., due to a delayed IRQ handler combined with temperature fluctuations,
or a hardware glitch), the bitmask value matches neither case.

It will fall through to default, leaving update = false, and clear both
interrupts in hardware without notifying the thermal core.


[Severity: High]
This is also a pre-existing issue, but could requesting the interrupt before
registering the thermal zone lead to a NULL pointer dereference?

Looking at airoha_thermal_probe():

drivers/thermal/airoha_thermal.c:airoha_thermal_probe() {
	...
	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
					airoha_thermal_irq, IRQF_ONESHOT,
					pdev->name, priv);
	...
	priv->tz = devm_thermal_of_zone_register(dev, 0, priv, &thdev_ops);
	...
}

The driver does not explicitly disable or clear hardware interrupts prior to
requesting the IRQ.

If a bootloader or previous kernel left the thermal monitor running and
interrupts enabled, an interrupt can fire immediately upon request. The
handler airoha_thermal_irq() will execute and call
thermal_zone_device_update(priv->tz, event).

Because priv->tz is not yet populated, this would cause a NULL pointer
dereference during initialization.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520155525.22239-1-ansuelsmth@gmail.com?part=1

[PATCH v5 2/7] thermal/drivers: airoha: fix copy paste error for sen internal

[Christian Marangi]

In airoha_thermal_setup_monitor there is a copy paste error on configuring
the internval for temp monitor. Fix the error and use the correct mask for
the sen interval for the EN7581_TEMPMONCTL2 register.

Fixes: 42de37f40e1b ("thermal/drivers: Add support for Airoha EN7581 thermal sensor")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/thermal/airoha_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
index 439aa011b75c..829a7327fc40 100644
--- a/drivers/thermal/airoha_thermal.c
+++ b/drivers/thermal/airoha_thermal.c
@@ -403,7 +403,7 @@ static void airoha_thermal_setup_monitor(struct airoha_thermal_priv *priv)
 	 * sen interval is 379 * 52.715us = 19.97ms
 	 */
 	writel(FIELD_PREP(EN7581_FILT_INTERVAL, 1) |
-	       FIELD_PREP(EN7581_FILT_INTERVAL, 379),
+	       FIELD_PREP(EN7581_SEN_INTERVAL, 379),
 	       priv->base + EN7581_TEMPMONCTL2);
 
 	/* AHB poll is set to 146 * 68.64 = 10.02us */
-- 
2.53.0

[PATCH v5 3/7] thermal/drivers: airoha: Convert to regmap API

[Christian Marangi]

In preparation for support of Airoha AN7583, convert the driver to
regmap API. This is needed as Airoha AN7583 will be based on syscon
regmap.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/thermal/airoha_thermal.c | 77 +++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
index 829a7327fc40..b63893a8997a 100644
--- a/drivers/thermal/airoha_thermal.c
+++ b/drivers/thermal/airoha_thermal.c
@@ -194,7 +194,7 @@
 #define AIROHA_MAX_SAMPLES			6
 
 struct airoha_thermal_priv {
-	void __iomem *base;
+	struct regmap *map;
 	struct regmap *chip_scu;
 	struct resource scu_adc_res;
 
@@ -265,8 +265,8 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
 			       RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
 
 		/* We offset the high temp of 1°C to trigger correct event */
-		writel(TEMP_TO_RAW(priv, high) >> 4,
-		       priv->base + EN7581_TEMPOFFSETH);
+		regmap_write(priv->map, EN7581_TEMPOFFSETH,
+			     TEMP_TO_RAW(priv, high) >> 4);
 
 		enable_monitor = true;
 	}
@@ -277,15 +277,15 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
 			      RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
 
 		/* We offset the low temp of 1°C to trigger correct event */
-		writel(TEMP_TO_RAW(priv, low) >> 4,
-		       priv->base + EN7581_TEMPOFFSETL);
+		regmap_write(priv->map, EN7581_TEMPOFFSETL,
+			     TEMP_TO_RAW(priv, low) >> 4);
 
 		enable_monitor = true;
 	}
 
 	/* Enable sensor 0 monitor after trip are set */
 	if (enable_monitor)
-		writel(EN7581_SENSE0_EN, priv->base + EN7581_TEMPMONCTL0);
+		regmap_write(priv->map, EN7581_TEMPMONCTL0, EN7581_SENSE0_EN);
 
 	return 0;
 }
@@ -300,9 +300,9 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
 	struct airoha_thermal_priv *priv = data;
 	enum thermal_notify_event event;
 	bool update = false;
-	u32 status;
+	u32 status = 0;
 
-	status = readl(priv->base + EN7581_TEMPMONINTSTS);
+	regmap_read(priv->map, EN7581_TEMPMONINTSTS, &status);
 	switch (status & (EN7581_HOFSINTSTS0 | EN7581_LOFSINTSTS0)) {
 	case EN7581_HOFSINTSTS0:
 		event = THERMAL_TRIP_VIOLATED;
@@ -318,7 +318,7 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
 	}
 
 	/* Reset Interrupt */
-	writel(status, priv->base + EN7581_TEMPMONINTSTS);
+	regmap_write(priv->map, EN7581_TEMPMONINTSTS, status);
 
 	if (update)
 		thermal_zone_device_update(priv->tz, event);
@@ -329,18 +329,19 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
 static void airoha_thermal_setup_adc_val(struct device *dev,
 					 struct airoha_thermal_priv *priv)
 {
-	u32 efuse_calib_info, cpu_sensor;
+	u32 efuse_calib_info = 0;
+	u32 cpu_sensor = 0;
 
 	/* Setup thermal sensor to ADC mode and setup the mux to DIODE1 */
 	airoha_init_thermal_ADC_mode(priv);
 	/* sleep 10 ms for ADC to enable */
 	usleep_range(10 * USEC_PER_MSEC, 11 * USEC_PER_MSEC);
 
-	efuse_calib_info = readl(priv->base + EN7581_EFUSE_TEMP_OFFSET_REG);
+	regmap_read(priv->map, EN7581_EFUSE_TEMP_OFFSET_REG, &efuse_calib_info);
 	if (efuse_calib_info) {
 		priv->default_offset = FIELD_GET(EN7581_EFUSE_TEMP_OFFSET, efuse_calib_info);
 		/* Different slope are applied if the sensor is used for CPU or for package */
-		cpu_sensor = readl(priv->base + EN7581_EFUSE_TEMP_CPU_SENSOR_REG);
+		regmap_read(priv->map, EN7581_EFUSE_TEMP_CPU_SENSOR_REG, &cpu_sensor);
 		if (cpu_sensor) {
 			priv->default_slope = EN7581_SLOPE_X100_DIO_DEFAULT;
 			priv->init_temp = EN7581_INIT_TEMP_FTK_X10;
@@ -359,8 +360,8 @@ static void airoha_thermal_setup_adc_val(struct device *dev,
 static void airoha_thermal_setup_monitor(struct airoha_thermal_priv *priv)
 {
 	/* Set measure mode */
-	writel(FIELD_PREP(EN7581_MSRCTL0, EN7581_MSRCTL_6SAMPLE_MAX_MIX_AVG4),
-	       priv->base + EN7581_TEMPMSRCTL0);
+	regmap_write(priv->map, EN7581_TEMPMSRCTL0,
+		     FIELD_PREP(EN7581_MSRCTL0, EN7581_MSRCTL_6SAMPLE_MAX_MIX_AVG4));
 
 	/*
 	 * Configure ADC valid reading addr
@@ -375,15 +376,15 @@ static void airoha_thermal_setup_monitor(struct airoha_thermal_priv *priv)
 	 * We set valid instead of volt as we don't enable valid/volt
 	 * split reading and AHB read valid addr in such case.
 	 */
-	writel(priv->scu_adc_res.start + EN7581_DOUT_TADC,
-	       priv->base + EN7581_TEMPADCVALIDADDR);
+	regmap_write(priv->map, EN7581_TEMPADCVALIDADDR,
+		     priv->scu_adc_res.start + EN7581_DOUT_TADC);
 
 	/*
 	 * Configure valid bit on a fake value of bit 16. The ADC outputs
 	 * max of 2 bytes for voltage.
 	 */
-	writel(FIELD_PREP(EN7581_ADV_RD_VALID_POS, 16),
-	       priv->base + EN7581_TEMPADCVALIDMASK);
+	regmap_write(priv->map, EN7581_TEMPADCVALIDMASK,
+		     FIELD_PREP(EN7581_ADV_RD_VALID_POS, 16));
 
 	/*
 	 * AHB supports max 12 bytes for ADC voltage. Shift the read
@@ -391,40 +392,52 @@ static void airoha_thermal_setup_monitor(struct airoha_thermal_priv *priv)
 	 * in the order of half a °C and is acceptable in the context
 	 * of triggering interrupt in critical condition.
 	 */
-	writel(FIELD_PREP(EN7581_ADC_VOLTAGE_SHIFT, 4),
-	       priv->base + EN7581_TEMPADCVOLTAGESHIFT);
+	regmap_write(priv->map, EN7581_TEMPADCVOLTAGESHIFT,
+		     FIELD_PREP(EN7581_ADC_VOLTAGE_SHIFT, 4));
 
 	/* BUS clock is 300MHz counting unit is 3 * 68.64 * 256 = 52.715us */
-	writel(FIELD_PREP(EN7581_PERIOD_UNIT, 3),
-	       priv->base + EN7581_TEMPMONCTL1);
+	regmap_write(priv->map, EN7581_TEMPMONCTL1,
+		     FIELD_PREP(EN7581_PERIOD_UNIT, 3));
 
 	/*
 	 * filt interval is 1 * 52.715us = 52.715us,
 	 * sen interval is 379 * 52.715us = 19.97ms
 	 */
-	writel(FIELD_PREP(EN7581_FILT_INTERVAL, 1) |
-	       FIELD_PREP(EN7581_SEN_INTERVAL, 379),
-	       priv->base + EN7581_TEMPMONCTL2);
+	regmap_write(priv->map, EN7581_TEMPMONCTL2,
+		     FIELD_PREP(EN7581_FILT_INTERVAL, 1) |
+		     FIELD_PREP(EN7581_SEN_INTERVAL, 379));
 
 	/* AHB poll is set to 146 * 68.64 = 10.02us */
-	writel(FIELD_PREP(EN7581_ADC_POLL_INTVL, 146),
-	       priv->base + EN7581_TEMPAHBPOLL);
+	regmap_write(priv->map, EN7581_TEMPAHBPOLL,
+		     FIELD_PREP(EN7581_ADC_POLL_INTVL, 146));
 }
 
+static const struct regmap_config airoha_thermal_regmap_config = {
+	.reg_bits		= 32,
+	.reg_stride		= 4,
+	.val_bits		= 32,
+};
+
 static int airoha_thermal_probe(struct platform_device *pdev)
 {
 	struct airoha_thermal_priv *priv;
 	struct device_node *chip_scu_np;
 	struct device *dev = &pdev->dev;
+	void __iomem *base;
 	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
-	priv->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(priv->base))
-		return PTR_ERR(priv->base);
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->map = devm_regmap_init_mmio(dev, base,
+					  &airoha_thermal_regmap_config);
+	if (IS_ERR(priv->map))
+		return PTR_ERR(priv->map);
 
 	chip_scu_np = of_parse_phandle(dev->of_node, "airoha,chip-scu", 0);
 	if (!chip_scu_np)
@@ -462,8 +475,8 @@ static int airoha_thermal_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, priv);
 
 	/* Enable LOW and HIGH interrupt */
-	writel(EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0,
-	       priv->base + EN7581_TEMPMONINT);
+	regmap_write(priv->map, EN7581_TEMPMONINT,
+		     EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0);
 
 	return 0;
 }
-- 
2.53.0

Re: [PATCH v5 3/7] thermal/drivers: airoha: Convert to regmap API

[sashiko-bot]

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

Pre-existing issues:
- [High] Simultaneous temperature trip interrupts are dropped instead of being handled.
- [High] The IRQ is requested before the thermal zone is registered, allowing a potential NULL pointer dereference in the IRQ handler.
- [High] Non-atomic read-modify-restore sequence on a shared syscon regmap without external locking.
--

commit 952c01597265eea8ee25e21cc2c92bef3e5ca97e
Author: Christian Marangi <ansuelsmth@gmail.com>

thermal/drivers: airoha: Convert to regmap API

In preparation for support of Airoha AN7583, convert the driver to
regmap API. This is needed as Airoha AN7583 will be based on syscon
regmap.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index 829a7327fc403..b63893a8997a3 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c

[ ... ]

> @@ -300,9 +300,9 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
>  	struct airoha_thermal_priv *priv = data;
>  	enum thermal_notify_event event;
>  	bool update = false;
> -	u32 status;
> +	u32 status = 0;
>  
> -	status = readl(priv->base + EN7581_TEMPMONINTSTS);
> +	regmap_read(priv->map, EN7581_TEMPMONINTSTS, &status);
>  	switch (status & (EN7581_HOFSINTSTS0 | EN7581_LOFSINTSTS0)) {
>  	case EN7581_HOFSINTSTS0:
>  		event = THERMAL_TRIP_VIOLATED;

[Severity: High]
This is a pre-existing issue, but what happens if both high and low
temperature interrupt status bits are set simultaneously?

If both EN7581_HOFSINTSTS0 and EN7581_LOFSINTSTS0 are set, the bitwise AND
will yield a combination of both bits. Since there are only case statements
for the individual bits, the code will fall through to the default case.

The status will be written back to clear the interrupts, but the events
will be silently dropped without notifying the thermal core.

Would it be safer to replace this switch statement with independent if
checks for each interrupt flag?

> @@ -329,18 +329,19 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
>  static void airoha_thermal_setup_adc_val(struct device *dev,
>  					 struct airoha_thermal_priv *priv)
>  {
> -	u32 efuse_calib_info, cpu_sensor;
> +	u32 efuse_calib_info = 0;
> +	u32 cpu_sensor = 0;
>  
>  	/* Setup thermal sensor to ADC mode and setup the mux to DIODE1 */
>  	airoha_init_thermal_ADC_mode(priv);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the
chip_scu regmap sequence to race with other drivers?

Looking at airoha_init_thermal_ADC_mode(), it performs a read-modify-restore
sequence on a shared syscon regmap:

static void airoha_init_thermal_ADC_mode(struct airoha_thermal_priv *priv)
{
    ...
    regmap_read(priv->chip_scu, EN7581_PLLRG_PROTECT, &pllrg);
    regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT,
                 EN7581_SCU_THERMAL_PROTECT_KEY);
    ...
    regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, pllrg);
}

Because the internal regmap lock is only held during individual register
accesses, could another driver (like a clock or pinctrl driver) accessing
the SCU concurrently cause the protect key to be incorrectly overwritten or
prematurely removed?

[ ... ]

> @@ -462,8 +475,8 @@ static int airoha_thermal_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, priv);
>  
>  	/* Enable LOW and HIGH interrupt */
> -	writel(EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0,
> -	       priv->base + EN7581_TEMPMONINT);
> +	regmap_write(priv->map, EN7581_TEMPMONINT,
> +		     EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0);
>  
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but could we encounter a NULL pointer
dereference if an interrupt fires before the thermal zone is registered?

Looking earlier in airoha_thermal_probe():

    ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
                                    airoha_thermal_irq, IRQF_ONESHOT,
                                    pdev->name, priv);

    ...

    priv->tz = devm_thermal_of_zone_register(dev, 0, priv, &thdev_ops);

If an interrupt is already pending (for example, left active by the
bootloader) when the IRQ is requested, the handler will run immediately.

If an event is detected, airoha_thermal_irq() will call
thermal_zone_device_update(priv->tz, event). Since priv->tz is still NULL
at that point, it will crash.

Should the IRQ be requested after the thermal zone registration is complete?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520155525.22239-1-ansuelsmth@gmail.com?part=3

[PATCH v5 4/7] thermal/drivers: airoha: Generalize probe function

[Christian Marangi]

In preparation for support of Airoha AN7583, generalize the probe
function to address for the 2 SoC differece.

Implement a match_data struct where it's possible to define a more
specific probe and post_probe function and specific thermal ops and
pllrg protect value.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/thermal/airoha_thermal.c | 102 +++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 27 deletions(-)

diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
index b63893a8997a..ebb47ae5f2ce 100644
--- a/drivers/thermal/airoha_thermal.c
+++ b/drivers/thermal/airoha_thermal.c
@@ -198,12 +198,23 @@ struct airoha_thermal_priv {
 	struct regmap *chip_scu;
 	struct resource scu_adc_res;
 
+	u32 pllrg_protect;
+
 	struct thermal_zone_device *tz;
 	int init_temp;
 	int default_slope;
 	int default_offset;
 };
 
+struct airoha_thermal_soc_data {
+	u32 pllrg_protect;
+
+	const struct thermal_zone_device_ops *thdev_ops;
+	int (*probe)(struct platform_device *pdev,
+		     struct airoha_thermal_priv *priv);
+	int (*post_probe)(struct platform_device *pdev);
+};
+
 static int airoha_get_thermal_ADC(struct airoha_thermal_priv *priv)
 {
 	u32 val;
@@ -220,7 +231,8 @@ static void airoha_init_thermal_ADC_mode(struct airoha_thermal_priv *priv)
 	regmap_read(priv->chip_scu, EN7581_PLLRG_PROTECT, &pllrg);
 
 	/* Give access to thermal regs */
-	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, EN7581_SCU_THERMAL_PROTECT_KEY);
+	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT,
+		     priv->pllrg_protect);
 	adc_mux = FIELD_PREP(EN7581_MUX_TADC, EN7581_SCU_THERMAL_MUX_DIODE1);
 	regmap_write(priv->chip_scu, EN7581_PWD_TADC, adc_mux);
 
@@ -228,7 +240,7 @@ static void airoha_init_thermal_ADC_mode(struct airoha_thermal_priv *priv)
 	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, pllrg);
 }
 
-static int airoha_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
+static int en7581_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
 {
 	struct airoha_thermal_priv *priv = thermal_zone_device_priv(tz);
 	int min_value, max_value, avg_value, value;
@@ -253,7 +265,7 @@ static int airoha_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
 	return 0;
 }
 
-static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
+static int en7581_thermal_set_trips(struct thermal_zone_device *tz, int low,
 				    int high)
 {
 	struct airoha_thermal_priv *priv = thermal_zone_device_priv(tz);
@@ -290,12 +302,12 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
 	return 0;
 }
 
-static const struct thermal_zone_device_ops thdev_ops = {
-	.get_temp = airoha_thermal_get_temp,
-	.set_trips = airoha_thermal_set_trips,
+static const struct thermal_zone_device_ops en7581_thdev_ops = {
+	.get_temp = en7581_thermal_get_temp,
+	.set_trips = en7581_thermal_set_trips,
 };
 
-static irqreturn_t airoha_thermal_irq(int irq, void *data)
+static irqreturn_t en7581_thermal_irq(int irq, void *data)
 {
 	struct airoha_thermal_priv *priv = data;
 	enum thermal_notify_event event;
@@ -326,7 +338,7 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static void airoha_thermal_setup_adc_val(struct device *dev,
+static void en7581_thermal_setup_adc_val(struct device *dev,
 					 struct airoha_thermal_priv *priv)
 {
 	u32 efuse_calib_info = 0;
@@ -357,7 +369,7 @@ static void airoha_thermal_setup_adc_val(struct device *dev,
 	}
 }
 
-static void airoha_thermal_setup_monitor(struct airoha_thermal_priv *priv)
+static void en7581_thermal_setup_monitor(struct airoha_thermal_priv *priv)
 {
 	/* Set measure mode */
 	regmap_write(priv->map, EN7581_TEMPMSRCTL0,
@@ -412,30 +424,26 @@ static void airoha_thermal_setup_monitor(struct airoha_thermal_priv *priv)
 		     FIELD_PREP(EN7581_ADC_POLL_INTVL, 146));
 }
 
-static const struct regmap_config airoha_thermal_regmap_config = {
+static const struct regmap_config en7581_thermal_regmap_config = {
 	.reg_bits		= 32,
 	.reg_stride		= 4,
 	.val_bits		= 32,
 };
 
-static int airoha_thermal_probe(struct platform_device *pdev)
+static int en7581_thermal_probe(struct platform_device *pdev,
+				struct airoha_thermal_priv *priv)
 {
-	struct airoha_thermal_priv *priv;
 	struct device_node *chip_scu_np;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
 	int irq, ret;
 
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
 	priv->map = devm_regmap_init_mmio(dev, base,
-					  &airoha_thermal_regmap_config);
+					  &en7581_thermal_regmap_config);
 	if (IS_ERR(priv->map))
 		return PTR_ERR(priv->map);
 
@@ -455,18 +463,55 @@ static int airoha_thermal_probe(struct platform_device *pdev)
 		return irq;
 
 	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
-					airoha_thermal_irq, IRQF_ONESHOT,
+					en7581_thermal_irq, IRQF_ONESHOT,
 					pdev->name, priv);
 	if (ret) {
 		dev_err(dev, "Can't get interrupt working.\n");
 		return ret;
 	}
 
-	airoha_thermal_setup_monitor(priv);
-	airoha_thermal_setup_adc_val(dev, priv);
+	en7581_thermal_setup_monitor(priv);
+	en7581_thermal_setup_adc_val(dev, priv);
+
+	return 0;
+}
+
+static int en7581_thermal_post_probe(struct platform_device *pdev)
+{
+	struct airoha_thermal_priv *priv = platform_get_drvdata(pdev);
+
+	/* Enable LOW and HIGH interrupt (if supported) */
+	regmap_write(priv->map, EN7581_TEMPMONINT,
+		     EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0);
+
+	return 0;
+}
+
+static int airoha_thermal_probe(struct platform_device *pdev)
+{
+	const struct airoha_thermal_soc_data *soc_data;
+	struct airoha_thermal_priv *priv;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	soc_data = device_get_match_data(dev);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->pllrg_protect = soc_data->pllrg_protect;
+
+	if (!soc_data->probe)
+		return -EINVAL;
+
+	ret = soc_data->probe(pdev, priv);
+	if (ret)
+		return ret;
 
 	/* register of thermal sensor and get info from DT */
-	priv->tz = devm_thermal_of_zone_register(dev, 0, priv, &thdev_ops);
+	priv->tz = devm_thermal_of_zone_register(dev, 0, priv,
+						 soc_data->thdev_ops);
 	if (IS_ERR(priv->tz)) {
 		dev_err(dev, "register thermal zone sensor failed\n");
 		return PTR_ERR(priv->tz);
@@ -474,15 +519,18 @@ static int airoha_thermal_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
-	/* Enable LOW and HIGH interrupt */
-	regmap_write(priv->map, EN7581_TEMPMONINT,
-		     EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0);
-
-	return 0;
+	return soc_data->post_probe ? soc_data->post_probe(pdev) : 0;
 }
 
+static const struct airoha_thermal_soc_data en7581_data = {
+	.pllrg_protect = EN7581_SCU_THERMAL_PROTECT_KEY,
+	.thdev_ops = &en7581_thdev_ops,
+	.probe = &en7581_thermal_probe,
+	.post_probe = &en7581_thermal_post_probe,
+};
+
 static const struct of_device_id airoha_thermal_match[] = {
-	{ .compatible = "airoha,en7581-thermal" },
+	{ .compatible = "airoha,en7581-thermal", .data = &en7581_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, airoha_thermal_match);
-- 
2.53.0

Re: [PATCH v5 4/7] thermal/drivers: airoha: Generalize probe function

[sashiko-bot]

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

New issues:
- [High] Unchecked return value from device_get_match_data() leads to a NULL pointer dereference.

Pre-existing issues:
- [High] IRQ registered before thermal zone initialization is complete, causing a race condition and devm teardown use-after-free.
--

commit f9520e831723d0643c683a7acaaf6600c5487d0a
Author: Christian Marangi <ansuelsmth@gmail.com>

thermal/drivers: airoha: Generalize probe function

This commit generalizes the Airoha thermal probe function to prepare for
supporting the Airoha AN7583 SoC. It introduces a match_data struct
to define SoC-specific probe and post_probe functions, thermal ops, and
PLLRG protect values.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index b63893a8997a3..ebb47ae5f2cee 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c
[ ... ]
> @@ -455,18 +463,55 @@ static int airoha_thermal_probe(struct platform_device *pdev)
>  		return irq;
>  
>  	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> -					airoha_thermal_irq, IRQF_ONESHOT,
> +					en7581_thermal_irq, IRQF_ONESHOT,
>  					pdev->name, priv);

[Severity: High]
This is a pre-existing issue, but does requesting the interrupt here before
the thermal zone is registered create a race condition?

If an interrupt fires early (e.g., from a pending bootloader interrupt),
en7581_thermal_irq() will run and call thermal_zone_device_update() before
priv->tz is initialized, leading to a NULL pointer dereference.

Additionally, because devres teardown executes in reverse allocation order,
the thermal zone is destroyed before the IRQ is freed during module removal.
Could this lead to a use-after-free if an interrupt fires during teardown
and accesses the freed priv->tz?

[ ... ]
> +static int airoha_thermal_probe(struct platform_device *pdev)
> +{
> +	const struct airoha_thermal_soc_data *soc_data;
> +	struct airoha_thermal_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	soc_data = device_get_match_data(dev);
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->pllrg_protect = soc_data->pllrg_protect;

[Severity: High]
Does this code lead to a NULL pointer dereference if the driver is bound
manually via sysfs?

device_get_match_data() returns NULL when the driver is bound manually
rather than through Device Tree matching. If soc_data is NULL, accessing
soc_data->pllrg_protect will trigger a kernel panic. Should soc_data be
checked for NULL before being dereferenced?

> +
> +	if (!soc_data->probe)
> +		return -EINVAL;
> +
> +	ret = soc_data->probe(pdev, priv);
> +	if (ret)
> +		return ret;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520155525.22239-1-ansuelsmth@gmail.com?part=4

[PATCH v5 5/7] thermal/drivers: airoha: Generalize get_thermal_ADC and set_mux function

[Christian Marangi]

In preparation for support of Airoha AN7583, generalize
get_thermal_ADC() and set_thermal_mux() with the use of reg_field API.

This is to account the same logic between the current supported SoC and
the new one but with different register address.

While at it also further improve some comments and move sleep inside the
set_thermal_mux function.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/thermal/airoha_thermal.c | 56 +++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
index ebb47ae5f2ce..249abbbd46bc 100644
--- a/drivers/thermal/airoha_thermal.c
+++ b/drivers/thermal/airoha_thermal.c
@@ -193,9 +193,18 @@
 
 #define AIROHA_MAX_SAMPLES			6
 
+enum airoha_thermal_chip_scu_field {
+	AIROHA_THERMAL_DOUT_TADC,
+	AIROHA_THERMAL_MUX_TADC,
+
+	/* keep last */
+	AIROHA_THERMAL_FIELD_MAX,
+};
+
 struct airoha_thermal_priv {
 	struct regmap *map;
 	struct regmap *chip_scu;
+	struct regmap_field *chip_scu_fields[AIROHA_THERMAL_FIELD_MAX];
 	struct resource scu_adc_res;
 
 	u32 pllrg_protect;
@@ -219,25 +228,32 @@ static int airoha_get_thermal_ADC(struct airoha_thermal_priv *priv)
 {
 	u32 val;
 
-	regmap_read(priv->chip_scu, EN7581_DOUT_TADC, &val);
-	return FIELD_GET(EN7581_DOUT_TADC_MASK, val);
+	regmap_field_read(priv->chip_scu_fields[AIROHA_THERMAL_DOUT_TADC],
+			  &val);
+	return val;
 }
 
-static void airoha_init_thermal_ADC_mode(struct airoha_thermal_priv *priv)
+static void airoha_set_thermal_mux(struct airoha_thermal_priv *priv,
+				   int tdac_idx)
 {
-	u32 adc_mux, pllrg;
+	u32 pllrg;
 
 	/* Save PLLRG current value */
 	regmap_read(priv->chip_scu, EN7581_PLLRG_PROTECT, &pllrg);
 
-	/* Give access to thermal regs */
+	/* Give access to Thermal regs */
 	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT,
 		     priv->pllrg_protect);
-	adc_mux = FIELD_PREP(EN7581_MUX_TADC, EN7581_SCU_THERMAL_MUX_DIODE1);
-	regmap_write(priv->chip_scu, EN7581_PWD_TADC, adc_mux);
+
+	/* Configure Thermal ADC mux to tdac_idx */
+	regmap_field_write(priv->chip_scu_fields[AIROHA_THERMAL_MUX_TADC],
+			   tdac_idx);
 
 	/* Restore PLLRG value on exit */
 	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, pllrg);
+
+	/* Sleep 10 ms for Thermal ADC to enable */
+	usleep_range(10 * USEC_PER_MSEC, 11 * USEC_PER_MSEC);
 }
 
 static int en7581_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
@@ -344,10 +360,8 @@ static void en7581_thermal_setup_adc_val(struct device *dev,
 	u32 efuse_calib_info = 0;
 	u32 cpu_sensor = 0;
 
-	/* Setup thermal sensor to ADC mode and setup the mux to DIODE1 */
-	airoha_init_thermal_ADC_mode(priv);
-	/* sleep 10 ms for ADC to enable */
-	usleep_range(10 * USEC_PER_MSEC, 11 * USEC_PER_MSEC);
+	/* Setup Thermal Sensor to ADC mode and setup the mux to DIODE1 */
+	airoha_set_thermal_mux(priv, EN7581_SCU_THERMAL_MUX_DIODE1);
 
 	regmap_read(priv->map, EN7581_EFUSE_TEMP_OFFSET_REG, &efuse_calib_info);
 	if (efuse_calib_info) {
@@ -430,13 +444,18 @@ static const struct regmap_config en7581_thermal_regmap_config = {
 	.val_bits		= 32,
 };
 
+static const struct reg_field en7581_chip_scu_fields[AIROHA_THERMAL_FIELD_MAX] = {
+	[AIROHA_THERMAL_DOUT_TADC] = REG_FIELD(EN7581_DOUT_TADC, 0, 15),
+	[AIROHA_THERMAL_MUX_TADC] = REG_FIELD(EN7581_PWD_TADC, 1, 3),
+};
+
 static int en7581_thermal_probe(struct platform_device *pdev,
 				struct airoha_thermal_priv *priv)
 {
 	struct device_node *chip_scu_np;
 	struct device *dev = &pdev->dev;
 	void __iomem *base;
-	int irq, ret;
+	int i, irq, ret;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
@@ -455,6 +474,19 @@ static int en7581_thermal_probe(struct platform_device *pdev,
 	if (IS_ERR(priv->chip_scu))
 		return PTR_ERR(priv->chip_scu);
 
+	for (i = 0; i < AIROHA_THERMAL_FIELD_MAX; i++) {
+		struct regmap_field *field;
+
+		field = devm_regmap_field_alloc(dev, priv->chip_scu,
+						en7581_chip_scu_fields[i]);
+		if (IS_ERR(field)) {
+			of_node_put(chip_scu_np);
+			return PTR_ERR(field);
+		}
+
+		priv->chip_scu_fields[i] = field;
+	}
+
 	of_address_to_resource(chip_scu_np, 0, &priv->scu_adc_res);
 	of_node_put(chip_scu_np);
 
-- 
2.53.0

Re: [PATCH v5 5/7] thermal/drivers: airoha: Generalize get_thermal_ADC and set_mux function

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing `regmap_write()` with `regmap_field_write()` implicitly changes a full register overwrite to a read-modify-write, potentially failing to clear other bits in the register.
--

commit 156e68047b2cd59d81061c97136f7825e128ca91
Author: Christian Marangi <ansuelsmth@gmail.com>

thermal/drivers: airoha: Generalize get_thermal_ADC and set_mux function

This commit introduces the use of the regmap field API for get_thermal_ADC
and set_thermal_mux in preparation for Airoha AN7583 support. It also
improves comments and moves a sleep call into the mux function.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index ebb47ae5f2cee..249abbbd46bcd 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c

[ ... ]

> @@ -219,25 +228,32 @@ static int airoha_get_thermal_ADC(struct airoha_thermal_priv *priv)
> -static void airoha_init_thermal_ADC_mode(struct airoha_thermal_priv *priv)
> +static void airoha_set_thermal_mux(struct airoha_thermal_priv *priv,
> +				   int tdac_idx)
>  {
> -	u32 adc_mux, pllrg;
> +	u32 pllrg;
>  
>  	/* Save PLLRG current value */
>  	regmap_read(priv->chip_scu, EN7581_PLLRG_PROTECT, &pllrg);
>  
> -	/* Give access to thermal regs */
> +	/* Give access to Thermal regs */
>  	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT,
>  		     priv->pllrg_protect);
> -	adc_mux = FIELD_PREP(EN7581_MUX_TADC, EN7581_SCU_THERMAL_MUX_DIODE1);
> -	regmap_write(priv->chip_scu, EN7581_PWD_TADC, adc_mux);
> +
> +	/* Configure Thermal ADC mux to tdac_idx */
> +	regmap_field_write(priv->chip_scu_fields[AIROHA_THERMAL_MUX_TADC],
> +			   tdac_idx);

[Severity: High]
Does this unintentionally change a full register write into a
read-modify-write?

Before this patch, airoha_init_thermal_ADC_mode used regmap_write to
write the mux value directly to EN7581_PWD_TADC. Because adc_mux only
populated bits 1-3, this implicitly cleared all other bits to 0.

By switching to regmap_field_write, the operation becomes a
read-modify-write on bits 1-3. Given the register name EN7581_PWD_TADC, if
there is a power-down bit that defaults to 1 at reset, the new code will
preserve it as 1 instead of clearing it to 0.

Could this regression leave the ADC permanently powered down or in an
invalid state?

>  
>  	/* Restore PLLRG value on exit */
>  	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, pllrg);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520155525.22239-1-ansuelsmth@gmail.com?part=5

[PATCH v5 6/7] dt-bindings: arm: airoha: Add the chip-scu node for AN7583 SoC

[Christian Marangi]

Document support for Airoha AN7583 chip-scu node.

This is similar to Airoha EN7581 with the addition of the presence of
thermal sensor in addition to controlling HW PIN and other miscellaneous
pheriperals.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../bindings/arm/airoha,en7581-chip-scu.yaml       | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml b/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml
index 67c449d804c2..cc564dc7b414 100644
--- a/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml
+++ b/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml
@@ -19,15 +19,29 @@ properties:
     items:
       - enum:
           - airoha,en7581-chip-scu
+          - airoha,an7583-chip-scu
       - const: syscon
 
   reg:
     maxItems: 1
 
+  '#thermal-sensor-cells':
+    const: 0
+
 required:
   - compatible
   - reg
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: airoha,en7581-chip-scu
+
+then:
+  properties:
+    '#thermal-sensor-cells': false
+
 additionalProperties: false
 
 examples:
-- 
2.53.0

Re: [PATCH v5 6/7] dt-bindings: arm: airoha: Add the chip-scu node for AN7583 SoC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `#thermal-sensor-cells` property is not marked as required for the new `airoha,an7583-chip-scu` variant, allowing incomplete device tree nodes to silently pass schema validation.
--

commit 87c7f9a301f3391f2461e89c9c85f755bb4ba049
Author: Christian Marangi <ansuelsmth@gmail.com>

dt-bindings: arm: airoha: Add the chip-scu node for AN7583 SoC

Document support for the Airoha AN7583 chip-scu node. This variant is
similar to the EN7581 but adds a thermal sensor capability in addition
to controlling HW PIN and other peripherals.

> diff --git a/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml b/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml
> --- a/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml
> +++ b/Documentation/devicetree/bindings/arm/airoha,en7581-chip-scu.yaml

[ ... ]

>  required:
>    - compatible
>    - reg
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: airoha,en7581-chip-scu
> +
> +then:
> +  properties:
> +    '#thermal-sensor-cells': false
> +

[Severity: Medium]
Should #thermal-sensor-cells be explicitly added to the required list for
the airoha,an7583-chip-scu variant?

When a hardware variant adds a new provider capability, the corresponding
standard properties typically need to be explicitly required. Without it,
incomplete device tree nodes missing the thermal sensor cells property might
silently pass schema validation.

Could an else block be added here to enforce this requirement?

else:
  required:
    - '#thermal-sensor-cells'

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520155525.22239-1-ansuelsmth@gmail.com?part=6

[PATCH v5 7/7] thermal/drivers: airoha: Add support for AN7583 Thermal Sensor

[Christian Marangi]

Add support for Airoha AN7583 Thermal driver. This apply similar logic
on how to read the temperature but totally drop support for the
PTP_THERMAL subsystem. PTP_THERMAL subsystem was a way to trigger trip
point from hardware by configuring how to read the temperature
internally.

This subsystem has been totally removed from Airoha AN7583 permitting
only to read the temperature.

The SoC support up to 3 sensor but the original driver always read the
BGA sensor hence it's currently implemented reading only this specific
sensor. Reference and values for the other 2 sensor are defined for
further implementation if confirmed working.

set_thermal_mux() is extended to also address muxing the sensor as
AN7583 use a different way to read the temperature from 3 different
diode. The EN7581 code is updated to account for these changes.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/thermal/airoha_thermal.c | 161 ++++++++++++++++++++++++++++++-
 1 file changed, 157 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
index 249abbbd46bc..c84b5c36e880 100644
--- a/drivers/thermal/airoha_thermal.c
+++ b/drivers/thermal/airoha_thermal.c
@@ -18,6 +18,12 @@
 #define EN7581_DOUT_TADC			0x2f8
 #define   EN7581_DOUT_TADC_MASK			GENMASK(15, 0)
 
+#define AN7583_MUX_SENSOR			0x2a0
+#define   AN7583_LOAD_ADJ			GENMASK(3, 2)
+#define AN7583_MUX_TADC				0x2e4
+#define   AN7583_MUX_TADC_MASK			GENMASK(3, 1)
+#define AN7583_DOUT_TADC			0x2f0
+
 /* PTP_THERMAL regs */
 #define EN7581_TEMPMONCTL0			0x800
 #define   EN7581_SENSE3_EN			BIT(3)
@@ -181,6 +187,11 @@
 #define EN7581_SCU_THERMAL_PROTECT_KEY		0x12
 #define EN7581_SCU_THERMAL_MUX_DIODE1		0x7
 
+#define AN7583_SCU_THERMAL_PROTECT_KEY		0x80
+#define AN7583_NUM_SENSOR			3
+
+#define AIROHA_THERMAL_NO_MUX_SENSOR		-1
+
 /* Convert temp to raw value as read from ADC	((((temp / 100) - init) * slope) / 1000) + offset */
 #define TEMP_TO_RAW(priv, temp)			((((((temp) / 100) - (priv)->init_temp) * \
 						  (priv)->default_slope) / 1000) + \
@@ -193,8 +204,39 @@
 
 #define AIROHA_MAX_SAMPLES			6
 
+/*
+ * AN7583 supports all these ADC mux but the original driver
+ * always checked temp with the AN7583_BGP_TEMP_SENSOR.
+ * Assume using the other sensor temperature is invalid and
+ * always read from AN7583_BGP_TEMP_SENSOR.
+ *
+ * On top of this it's defined that AN7583 supports 3
+ * sensor: AN7583_BGP_TEMP_SENSOR, AN7583_GBE_TEMP_SENSOR,
+ * AN7583_CPU_TEMP_SENSOR.
+ *
+ * Provide the ADC mux for reference.
+ */
+enum an7583_thermal_adc_mux {
+	AN7583_BGP_TEMP_SENSOR,
+	AN7583_PAD_AVS,
+	AN7583_CORE_POWER,
+	AN7583_AVSDAC_OUT,
+	AN7583_VCM,
+	AN7583_GBE_TEMP_SENSOR,
+	AN7583_CPU_TEMP_SENSOR,
+
+	AN7583_ADC_MUX_MAX,
+};
+
+enum an7583_thermal_diode_mux {
+	AN7583_D0_TADC,
+	AN7583_ZERO_TADC,
+	AN7583_D1_TADC,
+};
+
 enum airoha_thermal_chip_scu_field {
 	AIROHA_THERMAL_DOUT_TADC,
+	AIROHA_THERMAL_MUX_SENSOR,
 	AIROHA_THERMAL_MUX_TADC,
 
 	/* keep last */
@@ -208,6 +250,7 @@ struct airoha_thermal_priv {
 	struct resource scu_adc_res;
 
 	u32 pllrg_protect;
+	int current_adc;
 
 	struct thermal_zone_device *tz;
 	int init_temp;
@@ -224,6 +267,24 @@ struct airoha_thermal_soc_data {
 	int (*post_probe)(struct platform_device *pdev);
 };
 
+static const unsigned int an7583_thermal_coeff[AN7583_ADC_MUX_MAX] = {
+	[AN7583_BGP_TEMP_SENSOR] = 973,
+	[AN7583_GBE_TEMP_SENSOR] = 995,
+	[AN7583_CPU_TEMP_SENSOR] = 1035,
+};
+
+static const unsigned int an7583_thermal_slope[AN7583_ADC_MUX_MAX] = {
+	[AN7583_BGP_TEMP_SENSOR] = 7440,
+	[AN7583_GBE_TEMP_SENSOR] = 7620,
+	[AN7583_CPU_TEMP_SENSOR] = 8390,
+};
+
+static const unsigned int an7583_thermal_offset[AN7583_ADC_MUX_MAX] = {
+	[AN7583_BGP_TEMP_SENSOR] = 294,
+	[AN7583_GBE_TEMP_SENSOR] = 298,
+	[AN7583_CPU_TEMP_SENSOR] = 344,
+};
+
 static int airoha_get_thermal_ADC(struct airoha_thermal_priv *priv)
 {
 	u32 val;
@@ -234,7 +295,7 @@ static int airoha_get_thermal_ADC(struct airoha_thermal_priv *priv)
 }
 
 static void airoha_set_thermal_mux(struct airoha_thermal_priv *priv,
-				   int tdac_idx)
+				   int tdac_idx, int sensor_idx)
 {
 	u32 pllrg;
 
@@ -245,9 +306,20 @@ static void airoha_set_thermal_mux(struct airoha_thermal_priv *priv,
 	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT,
 		     priv->pllrg_protect);
 
+	/*
+	 * Configure Thermal Sensor mux to sensor_idx.
+	 * (if not supported, sensor_idx is AIROHA_THERMAL_NO_MUX_SENSOR)
+	 */
+	if (sensor_idx != AIROHA_THERMAL_NO_MUX_SENSOR)
+		regmap_field_write(priv->chip_scu_fields[AIROHA_THERMAL_MUX_SENSOR],
+				   sensor_idx);
+
 	/* Configure Thermal ADC mux to tdac_idx */
-	regmap_field_write(priv->chip_scu_fields[AIROHA_THERMAL_MUX_TADC],
-			   tdac_idx);
+	if (priv->current_adc != tdac_idx) {
+		regmap_field_write(priv->chip_scu_fields[AIROHA_THERMAL_MUX_TADC],
+				   tdac_idx);
+		priv->current_adc = tdac_idx;
+	}
 
 	/* Restore PLLRG value on exit */
 	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, pllrg);
@@ -361,7 +433,8 @@ static void en7581_thermal_setup_adc_val(struct device *dev,
 	u32 cpu_sensor = 0;
 
 	/* Setup Thermal Sensor to ADC mode and setup the mux to DIODE1 */
-	airoha_set_thermal_mux(priv, EN7581_SCU_THERMAL_MUX_DIODE1);
+	airoha_set_thermal_mux(priv, EN7581_SCU_THERMAL_MUX_DIODE1,
+			       AIROHA_THERMAL_NO_MUX_SENSOR);
 
 	regmap_read(priv->map, EN7581_EFUSE_TEMP_OFFSET_REG, &efuse_calib_info);
 	if (efuse_calib_info) {
@@ -477,6 +550,10 @@ static int en7581_thermal_probe(struct platform_device *pdev,
 	for (i = 0; i < AIROHA_THERMAL_FIELD_MAX; i++) {
 		struct regmap_field *field;
 
+		/* Skip registering MUX_SENSOR field as not supported */
+		if (i == AIROHA_THERMAL_MUX_SENSOR)
+			continue;
+
 		field = devm_regmap_field_alloc(dev, priv->chip_scu,
 						en7581_chip_scu_fields[i]);
 		if (IS_ERR(field)) {
@@ -519,6 +596,74 @@ static int en7581_thermal_post_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int an7583_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct airoha_thermal_priv *priv = thermal_zone_device_priv(tz);
+	int sensor_idx;
+	int delta_diode, delta_gain;
+	int coeff, slope, offset;
+
+	int diode_zero, diode_d0, diode_d1;
+
+	/* Always read sensor AN7583_BGP_TEMP_SENSOR */
+	sensor_idx = AN7583_BGP_TEMP_SENSOR;
+
+	coeff = an7583_thermal_coeff[sensor_idx];
+	slope = an7583_thermal_slope[sensor_idx];
+	offset = an7583_thermal_offset[sensor_idx];
+
+	airoha_set_thermal_mux(priv, AN7583_ZERO_TADC, sensor_idx);
+	diode_zero = airoha_get_thermal_ADC(priv);
+	airoha_set_thermal_mux(priv, AN7583_D0_TADC, sensor_idx);
+	diode_d0 = airoha_get_thermal_ADC(priv);
+	airoha_set_thermal_mux(priv, AN7583_D1_TADC, sensor_idx);
+	diode_d1 = airoha_get_thermal_ADC(priv);
+
+	delta_diode = diode_d1 - diode_d0;
+	delta_gain = (delta_diode * coeff) / 100 + (diode_zero - diode_d1);
+	if (!delta_gain)
+		return -EINVAL;
+
+	*temp = (slope * delta_diode * 10) / delta_gain - offset * 10;
+	*temp *= 100;
+
+	return 0;
+}
+
+static const struct thermal_zone_device_ops an7583_tz_ops = {
+	.get_temp = an7583_thermal_get_temp,
+};
+
+static const struct reg_field an7583_chip_scu_fields[AIROHA_THERMAL_FIELD_MAX] = {
+	[AIROHA_THERMAL_DOUT_TADC] = REG_FIELD(AN7583_DOUT_TADC, 0, 31),
+	[AIROHA_THERMAL_MUX_TADC] = REG_FIELD(AN7583_MUX_TADC, 1, 3),
+	[AIROHA_THERMAL_MUX_SENSOR] = REG_FIELD(AN7583_MUX_SENSOR, 2, 3),
+};
+
+static int an7583_thermal_probe(struct platform_device *pdev,
+				struct airoha_thermal_priv *priv)
+{
+	struct device *dev = &pdev->dev;
+	int i;
+
+	priv->chip_scu = device_node_to_regmap(dev->of_node);
+	if (IS_ERR(priv->chip_scu))
+		return PTR_ERR(priv->chip_scu);
+
+	for (i = 0; i < AIROHA_THERMAL_FIELD_MAX; i++) {
+		struct regmap_field *field;
+
+		field = devm_regmap_field_alloc(dev, priv->chip_scu,
+						an7583_chip_scu_fields[i]);
+		if (IS_ERR(field))
+			return PTR_ERR(field);
+
+		priv->chip_scu_fields[i] = field;
+	}
+
+	return 0;
+}
+
 static int airoha_thermal_probe(struct platform_device *pdev)
 {
 	const struct airoha_thermal_soc_data *soc_data;
@@ -533,6 +678,7 @@ static int airoha_thermal_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv->pllrg_protect = soc_data->pllrg_protect;
+	priv->current_adc = -1;
 
 	if (!soc_data->probe)
 		return -EINVAL;
@@ -561,8 +707,15 @@ static const struct airoha_thermal_soc_data en7581_data = {
 	.post_probe = &en7581_thermal_post_probe,
 };
 
+static const struct airoha_thermal_soc_data an7583_data = {
+	.pllrg_protect = AN7583_SCU_THERMAL_PROTECT_KEY,
+	.thdev_ops = &an7583_tz_ops,
+	.probe = &an7583_thermal_probe,
+};
+
 static const struct of_device_id airoha_thermal_match[] = {
 	{ .compatible = "airoha,en7581-thermal", .data = &en7581_data },
+	{ .compatible = "airoha,an7583-chip-scu", .data = &an7583_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, airoha_thermal_match);
-- 
2.53.0

Re: [PATCH v5 7/7] thermal/drivers: airoha: Add support for AN7583 Thermal Sensor

[sashiko-bot]

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

New issues:
- [High] Potential 32-bit signed integer overflow in temperature calculation.
- [Medium] The MUX_SENSOR register field width is too narrow to hold all defined sensor indices, leading to silent truncation.

Pre-existing issues:
- [Critical] Missing `remove` callback causes devres Use-After-Free of thermal zone.
- [High] Unprotected read-modify-write sequence on a shared syscon register creates a race condition.
- [High] Unchecked regmap read operations lead to use of uninitialized stack variables.
- [High] IRQ requested before thermal zone pointer is initialized leads to potential NULL pointer dereference.
- [High] Missing NULL check on `device_get_match_data()` leads to kernel panic on sysfs bind.
--

commit 57a01e256a4324da608f512eaa2939419b755b71
Author: Christian Marangi <ansuelsmth@gmail.com>

thermal/drivers: airoha: Add support for AN7583 Thermal Sensor

Add support for Airoha AN7583 Thermal driver. This apply similar logic
on how to read the temperature but totally drop support for the
PTP_THERMAL subsystem.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index 249abbbd46bcd..c84b5c36e8803 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c
[ ... ]
>  static void airoha_set_thermal_mux(struct airoha_thermal_priv *priv,
> -				   int tdac_idx)
> +				   int tdac_idx, int sensor_idx)
>  {
>  	u32 pllrg;
>  
[ ... ]
>  	/* Configure Thermal ADC mux to tdac_idx */
> -	regmap_field_write(priv->chip_scu_fields[AIROHA_THERMAL_MUX_TADC],
> -			   tdac_idx);
> +	if (priv->current_adc != tdac_idx) {
> +		regmap_field_write(priv->chip_scu_fields[AIROHA_THERMAL_MUX_TADC],
> +				   tdac_idx);
> +		priv->current_adc = tdac_idx;
> +	}
>  
>  	/* Restore PLLRG value on exit */
>  	regmap_write(priv->chip_scu, EN7581_PLLRG_PROTECT, pllrg);

[Severity: High]
This is a pre-existing issue, but since this patch significantly increases
the frequency of this function for runtime polling, is there a race
condition with the read-modify-write sequence on the shared syscon register?

The regmap lock only protects individual reads and writes, making the
unlock-modify-lock sequence on EN7581_PLLRG_PROTECT non-atomic. If another
driver sharing the SCU performs operations concurrently, could they race and
leave the protection register permanently unlocked or restore an incorrect
state?

[Severity: High]
This is a pre-existing issue, but does the unchecked regmap read
operation lead to using an uninitialized stack variable?

In airoha_set_thermal_mux(), if the regmap_read() for EN7581_PLLRG_PROTECT
fails, the pllrg variable remains uninitialized and is later written back to
the hardware protection register.

Similarly, in airoha_get_thermal_ADC(), an unchecked regmap_field_read()
leaves the stack variable uninitialized before being returned and used in
temperature calculations.

[ ... ]
> +static int an7583_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct airoha_thermal_priv *priv = thermal_zone_device_priv(tz);
> +	int sensor_idx;
> +	int delta_diode, delta_gain;
> +	int coeff, slope, offset;
> +
> +	int diode_zero, diode_d0, diode_d1;
> +
[ ... ]
> +	delta_diode = diode_d1 - diode_d0;
> +	delta_gain = (delta_diode * coeff) / 100 + (diode_zero - diode_d1);
> +	if (!delta_gain)
> +		return -EINVAL;
> +
> +	*temp = (slope * delta_diode * 10) / delta_gain - offset * 10;
> +	*temp *= 100;
> +
> +	return 0;
> +}

[Severity: High]
Can the multiplication of slope and delta_diode overflow a 32-bit signed
integer?

The slope variable can be up to 8390, and delta_diode is derived from the
difference of two 32-bit ADC readings. A delta_diode value exceeding 25,595
would cause the (slope * delta_diode * 10) calculation to overflow the signed
32-bit maximum limit (2,147,483,647), leading to undefined behavior and
wildly incorrect temperature readings.

[ ... ]
> +static const struct thermal_zone_device_ops an7583_tz_ops = {
> +	.get_temp = an7583_thermal_get_temp,
> +};
> +
> +static const struct reg_field an7583_chip_scu_fields[AIROHA_THERMAL_FIELD_MAX] = {
> +	[AIROHA_THERMAL_DOUT_TADC] = REG_FIELD(AN7583_DOUT_TADC, 0, 31),
> +	[AIROHA_THERMAL_MUX_TADC] = REG_FIELD(AN7583_MUX_TADC, 1, 3),
> +	[AIROHA_THERMAL_MUX_SENSOR] = REG_FIELD(AN7583_MUX_SENSOR, 2, 3),
> +};

[Severity: Medium]
Is the AIROHA_THERMAL_MUX_SENSOR register field width too narrow to hold all
defined sensor indices?

The field is defined as a 2-bit field (bits 2 to 3), which can hold a
maximum value of 3. However, the driver enumerates sensor indices up to 6
(AN7583_CPU_TEMP_SENSOR = 6). If the driver is extended to use these other
sensors, calling regmap_field_write() with values larger than 3 will silently
truncate the values, causing the hardware to sample the wrong sensor.

[ ... ]
>  static int airoha_thermal_probe(struct platform_device *pdev)
>  {
>  	const struct airoha_thermal_soc_data *soc_data;
>  	struct airoha_thermal_priv *priv;
>  	struct device *dev = &pdev->dev;
>  	int ret;
>  
>  	soc_data = device_get_match_data(dev);
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
>  
>  	priv->pllrg_protect = soc_data->pllrg_protect;

[Severity: High]
This is a pre-existing issue, but could device_get_match_data() return NULL
and cause a kernel panic here?

If the driver is manually bound via sysfs to a device that lacks an
associated device tree node, device_get_match_data() returns NULL, leading
to a NULL pointer dereference when accessing soc_data->pllrg_protect.

> +	priv->current_adc = -1;
>  
>  	if (!soc_data->probe)
>  		return -EINVAL;
>  
>  	ret = soc_data->probe(pdev, priv);
>  	if (ret)
>  		return ret;
>  
>  	/* register of thermal sensor and get info from DT */
>  	priv->tz = devm_thermal_of_zone_register(dev, 0, priv,
>  						 soc_data->thdev_ops);

[Severity: High]
This is a pre-existing issue, but is the IRQ requested before the thermal
zone pointer is initialized?

In en7581_thermal_probe(), which is called by soc_data->probe(), the hardware
interrupt is registered via devm_request_threaded_irq() before priv->tz is
assigned here. If an interrupt fires immediately, en7581_thermal_irq() will
execute and pass the uninitialized NULL priv->tz to
thermal_zone_device_update(), causing a kernel panic.

[Severity: Critical]
This is a pre-existing issue, but does the lack of a remove callback cause
a use-after-free of the thermal zone?

The driver enables hardware interrupts but does not provide a remove function
to disable them. On module unload or manual unbind, devres cleans up
resources in reverse order: the thermal zone priv->tz is unregistered and
freed before the threaded IRQ handler is freed. If the interrupt fires
during this unbind window, en7581_thermal_irq() executes and accesses the
freed thermal zone.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520155525.22239-1-ansuelsmth@gmail.com?part=7