[bug report] cpufreq: mediatek: Link CCI device to CPU

[Dan Carpenter]

Hello Rex-BC Chen,

The patch 0daa47325bae: "cpufreq: mediatek: Link CCI device to CPU"
from May 5, 2022, leads to the following Smatch static checker
warning:

	drivers/cpufreq/mediatek-cpufreq.c:405 mtk_cpu_dvfs_info_init()
	warn: passing zero to 'PTR_ERR'

drivers/cpufreq/mediatek-cpufreq.c
    387 static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu)
    388 {
    389         struct device *cpu_dev;
    390         struct dev_pm_opp *opp;
    391         unsigned long rate;
    392         int ret;
    393 
    394         cpu_dev = get_cpu_device(cpu);
    395         if (!cpu_dev) {
    396                 dev_err(cpu_dev, "failed to get cpu%d device\n", cpu);
    397                 return -ENODEV;
    398         }
    399         info->cpu_dev = cpu_dev;
    400 
    401         info->ccifreq_bound = false;
    402         if (info->soc_data->ccifreq_supported) {
    403                 info->cci_dev = of_get_cci(info->cpu_dev);
    404                 if (IS_ERR_OR_NULL(info->cci_dev)) {
--> 405                         ret = PTR_ERR(info->cci_dev);
    406                         dev_err(cpu_dev, "cpu%d: failed to get cci device\n", cpu);
    407                         return -ENODEV;

This code is "harmless" in terms of run time but it's also wrong.  Linus
once complained about this Smatch check that actually passing zero to
PTR_ERR() is allowed but I explained to him that in real life 95% or
19 times out of 20 then when people have this warning there is something
wrong.  Today I have seen many of these bugs and 1 non-bug.

[Btw, this code will also trigger another warning because the "ret = "
assignment is never used.]

This driver does a lot of checking for IS_ERR_OR_NULL() and in each case
the code is doing something wrong.  I have explained this in my blog:

https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/

The existance of the IS_ERR_OR_NULL() function doesn't mean that we
don't have to care if the function returns NULL or if it returns error
pointers...

If a function returns NULL, then we have to check for NULL.  If a
function returns error pointers we have to check for error pointers.
If a function returns a mix of error pointers and NULL the *most* of the
time the errors need to be handled differently from the NULL returns.
For these functions a NULL is *not* an error but is instead a special
kind of success where the feature is not enabled.

Q: Give me the feature
A: The feature is disables so here is a NULL.  Success!

The driver then has to written to handle an optional feature which has
been disabled.  Do not print an error message.  Do not return an error
code.

    408                 }
    409         }

regards,
dan carpenter