Re: [PATCH v1 2/2] PM: devfreq: handle invalid parameters gracefully in simpleondemand

[luriwen]

在 2025/11/23 23:24, Chanwoo Choi 写道:
> 25. 11. 18. 12:23에 Riwen Lu 이(가) 쓴 글:
> > Instead of returning -EINVAL when upthreshold > 100 or upthreshold <
> > downdifferential, fall back to default threshold values to ensure the
> > governor continues functioning.
> >
> > Additionally, the validation is now scoped to the if (data) block,
> > preventing unnecessary checks when no user data is provided, while the
> > fallback mechanism ensures reliability with invalid configurations.
> >
> > Signed-off-by: Riwen Lu <luriwen@xxxxxxxxxx>
> > ---
> >   drivers/devfreq/governor_simpleondemand.c | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> > index b4d7be766f33..7205891d2ec6 100644
> > --- a/drivers/devfreq/governor_simpleondemand.c
> > +++ b/drivers/devfreq/governor_simpleondemand.c
> > @@ -36,10 +36,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
> >   			dfso_upthreshold = data->upthreshold;
> >   		if (data->downdifferential)
> >   			dfso_downdifferential = data->downdifferential;
> > +
> > +		if (dfso_upthreshold > 100 ||
> > +		    dfso_upthreshold < dfso_downdifferential) {
> > +			dfso_upthreshold = DFSO_UPTHRESHOLD;
> > +			dfso_downdifferential = DFSO_DOWNDIFFERENTIAL;
> > +			pr_debug("Invalid thresholds, using defaults: up = %u, down = %u\n",
> > +				dfso_upthreshold, dfso_downdifferential);
> > +		}
> >   	}
> > -	if (dfso_upthreshold > 100 ||
> > -	    dfso_upthreshold < dfso_downdifferential)
> > -		return -EINVAL;
> >   
> >   	/* Assume MAX if it is going to be divided by zero */
> >   	if (stat->total_time == 0) {
> If there are wrong initialization of devfreq_simple_ondemand,
> it should point out what is wrong because it makes some confusion if there are no error.
The original intention behind falling back to default values was to ensure the governor remains functional even with invalid configurations. However, I agree that silently using defaults could hide configuration issues from users.

I'd like to keep fallback but add explicit warning or error.

```c

if (dfso_upthreshold > 100 ||
    dfso_upthreshold < dfso_downdifferential) {
        pr_warn("Invalid thresholds (up = %u, down = %u), using 
defaults: up = %u, down = %u\n",
                dfso_upthreshold, dfso_downdifferential,
                DFSO_UPTHRESHOLD, DFSO_DOWNDIFFERENTIAL);
        dfso_upthreshold = DFSO_UPTHRESHOLD;
        dfso_downdifferential = DFSO_DOWNDIFFERENTIAL;
}

--

Best Regards,

Riwen Lu