Re: [PATCH v7 5/8] thermal/drivers/loongson2: Use bitfields and per-SoC thermal ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] thermal: loongson2: API misuse of FIELD_PREP triggers potential BUILD_BUG_ON
--

commit 9fa005fd4a6e6873cf9a1897453c930665ef1a63
Author: Binbin Zhou <zhoubinbin@loongson.cn>

thermal/drivers/loongson2: Use bitfields and per-SoC thermal ops
    
This commit replaces open-coded masks and shifts with GENMASK() and FIELD_* 
helpers. It also separates the temperature read and thermal zone ops into 
SoC-specific variants to prepare for further expansion.

> diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c
> index 94259d88da8a0..02f7545eaac1b 100644
> --- a/drivers/thermal/loongson2_thermal.c
> +++ b/drivers/thermal/loongson2_thermal.c

[ ... ]

> @@ -121,17 +135,18 @@ static void ls2k2000_thermal_set_trips_regs(struct loongson2_thermal_data *data,
>  					    int temp, bool low, bool enable)
>  {
>  	unsigned int sel = data->chip_data->thermal_sensor_sel;
>  	int hi_reg, ctrl_reg, val, gate;
>  
> -	gate = (temp + 311) * 0x4000 / 820;
> +	gate = (temp + LS2K2000_TEMP_OFFSET) * LS2K2000_TEMP_DIV / LS2K2000_TEMP_MUL;
>  
>  	/* Low byte of the gate value goes to the control register */
>  	ctrl_reg = low ? LOONGSON2_THSENS_CTRL_LOW_REG : LOONGSON2_THSENS_CTRL_HI_REG;
> -	val = gate & 0xff;
> -	val |= enable ? 0x100 : 0;
> -	val |= sel << 10;
> +	val = FIELD_PREP(LOONGSON2_THSENS_CTRL_GATE_MASK, gate) |
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Could this FIELD_PREP() macro usage break the build? It appears to pass a
16-bit value (gate) into an 8-bit mask (LOONGSON2_THSENS_CTRL_GATE_MASK
which is defined as GENMASK(7, 0)).

The FIELD_PREP() macro strictly expects the value to fit inside the mask
boundary. For ls2k2000, the calculated gate value is a 16-bit integer (for
example, at 0 degrees Celsius, it calculates to 6213).

If the compiler performs speculative devirtualization or constant propagation
(such as during LTO across loongson2_thermal_probe() calling
loongson2_thermal_set_trip_temp(data, 0, 0, false)), __builtin_constant_p(gate)
would evaluate to true. This can trigger the BUILD_BUG_ON_MSG("value too large
for the field") in bitfield.h and fail compilation.

> +	      FIELD_PREP(LOONGSON2_THSENS_CTRL_INTEN, enable) |
> +	      FIELD_PREP(LOONGSON2_THSENS_CTRL_SEL_MASK, sel);
>  	writew(val, data->ctrl_reg + ctrl_reg + sel * 2);
>  
>  	/* High byte of the gate value goes to the low byte of the HI_GATE register */
>  	hi_reg = low ? LS2K2000_THSENS_HI_GATE_LOW_REG : LS2K2000_THSENS_HI_GATE_HI_REG;
> -	val = (gate >> 8) & 0xff;
> +	val = FIELD_GET(GENMASK(15, 8), gate);
>  	writeb(val, data->ctrl_reg + hi_reg + sel);
>  }

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