Re: [PATCH v2 1/4] PM / devfreq: Fall back to previous_freq when get_cur_freq() fails

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] devfreq->resume_freq is incorrectly overwritten with devfreq->suspend_freq during system suspend, causing devices to resume to their suspend frequency rather than their active frequency.
--

commit 252cf4045ad4d642d45961b4d6845aa317e8ee57
Author: Bean Huo <beanhuo@micron.com>

PM / devfreq: Fall back to previous_freq when get_cur_freq() fails

This commit updates devfreq_set_target() to fall back to the previous
frequency when get_cur_freq() returns an error. This prevents uninitialized
stack values from being passed to notifiers during frequency changes.

> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f08fc6966eaee..f20d9a6607791 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -348,9 +348,8 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
>  	unsigned long cur_freq;
>  	int err = 0;
>  
> -	if (devfreq->profile->get_cur_freq)
> -		devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq);
> -	else
> +	if (!devfreq->profile->get_cur_freq ||
> +	    devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq))
>  		cur_freq = devfreq->previous_freq;
>  
>  	freqs.old = cur_freq;

[Severity: Medium]
This is a pre-existing issue, but since we are modifying devfreq_set_target(),
I noticed a potential issue with how resume frequencies are stored.

Further down in devfreq_set_target(), there is this unconditional assignment:

drivers/devfreq/devfreq.c:devfreq_set_target() {
    ...
    if (devfreq->suspend_freq)
        devfreq->resume_freq = new_freq;

    return err;
}

When a device defines a suspend_freq, system suspend invokes
devfreq_suspend_device(), which places the device in the low-power state
by calling:

drivers/devfreq/devfreq.c:devfreq_suspend_device() {
    ...
    ret = devfreq_set_target(devfreq, devfreq->suspend_freq, 0);
    ...
}

Does this cause devfreq_set_target() to unintentionally overwrite the saved
active frequency with the suspend frequency?

It appears that during every system suspend cycle, resume_freq gets updated to
the suspend_freq target. Will this cause devices to resume to their lowest
suspend frequency instead of their pre-suspend active frequency, degrading
performance until the governor recalculates a new target?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907192140.2701755-1-beanhuo@iokpp.de?part=1