Re: [PATCH v6 7/9] media: chips-media: wave6: Add Wave6 thermal cooling device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Ignored error return in set_cur_state callback causes thermal framework desynchronization.
- [Medium] Initial thermal state changes during registration are silently dropped due to a check for `!thermal->cooling`.
--

commit f2e1f0ee6d5cf6a10bb958433178a8d9e3143413
Author: Nas Chung <nas.chung@chipsnmedia.com>

media: chips-media: wave6: Add Wave6 thermal cooling device

Add a thermal cooling device for the Wave6 VPU. The device operates within
the Linux thermal framework, adjusting the VPU performance state based on
thermal conditions.

> diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c
> new file mode 100644
> index 0000000000000..91cd826e81195
> --- /dev/null
> +++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c

[ ... ]

> +static int wave6_vpu_thermal_cooling_update(struct vpu_thermal_cooling *thermal,
> +					    int state)
> +{
> +	unsigned long new_clock_rate;
> +	int ret;
> +
> +	if (state > thermal->thermal_max || !thermal->cooling)
> +		return 0;

[Severity: Medium]
Will this silently drop initial thermal state changes during registration?

When devm_thermal_of_cooling_device_register() is called, it can synchronously
trigger a state update before the registration function returns and assigns the
pointer to thermal->cooling. Because thermal->cooling is not yet assigned,
this check evaluates to true and skips the hardware update.

[ ... ]

> +static int wave6_vpu_cooling_set_cur_state(struct thermal_cooling_device *cdev,
> +					   unsigned long state)
> +{
> +	struct vpu_thermal_cooling *thermal = cdev->devdata;
> +
> +	thermal->thermal_event = state;
> +	wave6_vpu_thermal_cooling_update(thermal, state);
> +
> +	return 0;
> +}

[Severity: Medium]
Should the return value of wave6_vpu_thermal_cooling_update() be checked before
returning success to the thermal framework?

If wave6_vpu_thermal_cooling_update() fails, this function still unconditionally
returns 0 and updates the local thermal->thermal_event state. This could leave
the thermal framework thinking the hardware state has changed when the hardware
is actually stuck in its previous state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072043.238-1-nas.chung@chipsnmedia.com?part=7