linux-kernel - Re: [PATCH v3 21/22] cpufreq: add support for CPU DVFS based on SCMI message protocol

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a3pAFKM+Dq_yb7KYhB+xeneVuODBq_4nacEuw9creT6Yw@mail.gmail.com>
Date:   Wed, 4 Oct 2017 13:30:06 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Sudeep Holla <sudeep.holla@....com>
Cc:     ALKML <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        DTML <devicetree@...r.kernel.org>,
        Roy Franz <roy.franz@...ium.com>,
        Harb Abdulhamid <harba@...eaurora.org>,
        Nishanth Menon <nm@...com>, Loc Ho <lho@....com>,
        Alexey Klimov <alexey.klimov@....com>,
        Ryan Harkin <Ryan.Harkin@....com>,
        Jassi Brar <jassisinghbrar@...il.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 21/22] cpufreq: add support for CPU DVFS based on SCMI
 message protocol

> +static int scmi_cpufreq_probe(struct platform_device *pdev)
> +{
> +       int ret;
> +
> +       handle = devm_scmi_handle_get(&pdev->dev);
> +
> +       if (IS_ERR_OR_NULL(handle) || !handle->perf_ops)
> +               return -EPROBE_DEFER;

As mentioned before, never create an interface that needs to use
IS_ERR_OR_NULL(), make it return either NULL on error, or always
have an error code.

> +
> +static struct platform_driver scmi_cpufreq_platdrv = {
> +       .driver = {
> +               .name   = "scmi-cpufreq",
> +       },
> +       .probe          = scmi_cpufreq_probe,
> +       .remove         = scmi_cpufreq_remove,
> +};

You appear to have split this driver into the 'cpufreq' side and
the 'protocol' handler in a different file. I already commented that
the way the main scmi driver knows about all the protocols looks
bad, here we can see another aspect of the same problem.

Rather than manually register a platform_device for the purpose
of connecting it to the cpufreq driver, there should be a way
to dynamically register the protocol from the cpufreq driver
and then have both in the same file.

      Arnd