Re: [PATCH v6 3/7] thermal/drivers/loongson2: Fix Loongson-2K2000 trip point programming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] thermal: torn write in loongson2_2k2000_set_trips_regs() can trigger spurious interrupts
--

commit 89df6fff6e860d7cc010f8ec79a9f1531d8e5cca
Author: Binbin Zhou <zhoubinbin@loongson.cn>

    thermal/drivers/loongson2: Fix Loongson-2K2000 trip point programming
    
    Loongson-2K2000 uses a different trip point layout than Loongson-2K1000:
    the low 8 bits of the gate value are written to the control register,
    while the high 8 bits are written to separate HI_GATE_HI/LOW registers.
    The gate value is calculated as (temp + 311) * 0x4000 / 820.
    
    Introduce a per-SoC set_trips_regs() callback. Keep the existing
    Loongson-2K1000 logic and add the Loongson-2K2000 implementation.
    Move the common clamping into loongson2_thermal_set_trips() and pass
    Celsius values to the callback.

> diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c
> index bcabca3601572..45fe6100134ee 100644
> --- a/drivers/thermal/loongson2_thermal.c
> +++ b/drivers/thermal/loongson2_thermal.c

[ ... ]

> @@ -81,6 +99,26 @@ static int loongson2_2k1000_get_temp(struct thermal_zone_device *tz, int *temp)
>  	return 0;
>  }
>  
> +static void loongson2_2k2000_set_trips_regs(struct loongson2_thermal_data *data,
> +					    int temp, bool low, bool enable)
> +{
> +	int hi_reg, ctrl_reg;
> +	int val = 0, gate;
> +
> +	ctrl_reg = low ? LOONGSON2_THSENS_CTRL_LOW_REG : LOONGSON2_THSENS_CTRL_HI_REG;
> +	gate = (temp + 311) * 0x4000 / 820;
> +
> +	val = gate & 0xff;
> +	val |= enable ? 0x100 : 0;
> +	writel(val, data->ctrl_reg + ctrl_reg);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Will this writel() cause the hardware to evaluate a torn threshold?

By writing the low 8 bits and immediately enabling the hardware comparator here,
before the upper 8 bits are written to hi_reg below, there is a time window
where the active hardware uses a torn threshold (new low bits, old high bits).

If this torn value is momentarily crossed by the current live temperature, could
it fire a spurious hardware interrupt?

> +
> +	hi_reg = low ? LS2K2000_THSENS_HI_GATE_LOW_REG : LS2K2000_THSENS_HI_GATE_HI_REG;
> +
> +	val = readl(data->ctrl_reg + hi_reg);
> +	val = (val & ~0xff) | ((gate >> 8) & 0xff);
> +	writel(val, data->ctrl_reg + hi_reg);
> +}
> +
>  static int loongson2_2k2000_get_temp(struct thermal_zone_device *tz, int *temp)
>  {

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