Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 1/2] cpufreq: dt: Don't (ab)use
 dev_pm_opp_get_opp_table() to create OPP table
To:     Viresh Kumar <viresh.kumar@linaro.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc:     linux-pm@vger.kernel.org,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Stephen Boyd <sboyd@kernel.org>, Nishanth Menon <nm@ti.com>,
        digetx@gmail.com, Stephan Gerhold <stephan@gerhold.net>,
        linux-kernel@vger.kernel.org,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
From:   Marek Szyprowski <m.szyprowski@samsung.com>
Message-ID: <2924bddd-d237-aa57-abb1-a67723770e97@samsung.com>
Date:   Mon, 9 Nov 2020 13:42:24 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0)
        Gecko/20100101 Thunderbird/78.4.0
MIME-Version: 1.0
In-Reply-To: <684ff01900180c0a40ec307dacc673b24eab593b.1604643714.git.viresh.kumar@linaro.org>
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <684ff01900180c0a40ec307dacc673b24eab593b.1604643714.git.viresh.kumar@linaro.org>
        <CGME20201109124218eucas1p1b8948a9bf2cf107b17b500b1603905e8@eucas1p1.samsung.com>
Precedence: bulk

Hi Viresh,

On 06.11.2020 07:24, Viresh Kumar wrote:
> Initially, the helper dev_pm_opp_get_opp_table() was supposed to be used
> only for the OPP core's internal use (it tries to find an existing OPP
> table and if it doesn't find one, then it allocates the OPP table).
>
> Sometime back, the cpufreq-dt driver started using it to make sure all
> the relevant resources required by the OPP core are available earlier
> during initialization process to properly propagate -EPROBE_DEFER.
>
> It worked but it also abused the API to create an OPP table, which
> should be created with the help of other helpers provided by the OPP
> core.
>
> The OPP core will be updated in a later commit to limit the scope of
> dev_pm_opp_get_opp_table() to only finding an existing OPP table and not
> create one. This commit updates the cpufreq-dt driver before that
> happens.
>
> Now the cpufreq-dt driver creates the OPP and cpufreq tables for all the
> CPUs from driver's init callback itself.
>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

This patch landed in linux next-20201109 as commit e8f7703f8fe5 
("cpufreq: dt: Don't (ab)use dev_pm_opp_get_opp_table() to create OPP 
table"). Sadly it causes regression on some Samsung Exynos based boards:

8<--- cut here ---
Unable to handle kernel paging request at virtual address ffffff37
pgd = (ptrval)
[ffffff37] *pgd=4ffff841, *pte=00000000, *ppte=00000000
Internal error: Oops: 27 [#1] PREEMPT SMP ARM
Modules linked in:
usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc1-00007-ge8f7703f8fe5 
#1908
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at dev_pm_opp_put_regulators+0x8/0xf0
LR is at dt_cpufreq_probe+0x19c/0x3fc
pc : [<c0847694>]    lr : [<c08538b8>]    psr: a0000013
...
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
Stack: (0xc1d29da8 to 0xc1d2a000)
...
[<c0847694>] (dev_pm_opp_put_regulators) from [<c08538b8>] 
(dt_cpufreq_probe+0x19c/0x3fc)
[<c08538b8>] (dt_cpufreq_probe) from [<c068eecc>] 
(platform_drv_probe+0x6c/0xa4)
[<c068eecc>] (platform_drv_probe) from [<c068c490>] 
(really_probe+0x200/0x4fc)
[<c068c490>] (really_probe) from [<c068c954>] 
(driver_probe_device+0x78/0x1fc)
[<c068c954>] (driver_probe_device) from [<c068cd3c>] 
(device_driver_attach+0x58/0x60)
[<c068cd3c>] (device_driver_attach) from [<c068ce20>] 
(__driver_attach+0xdc/0x174)
[<c068ce20>] (__driver_attach) from [<c068a218>] 
(bus_for_each_dev+0x68/0xb4)
[<c068a218>] (bus_for_each_dev) from [<c068b54c>] 
(bus_add_driver+0x158/0x214)
[<c068b54c>] (bus_add_driver) from [<c068dc80>] (driver_register+0x78/0x110)
[<c068dc80>] (driver_register) from [<c0102484>] 
(do_one_initcall+0x8c/0x42c)
[<c0102484>] (do_one_initcall) from [<c11011c0>] 
(kernel_init_freeable+0x190/0x1dc)
[<c11011c0>] (kernel_init_freeable) from [<c0b208f8>] 
(kernel_init+0x8/0x118)
[<c0b208f8>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
Exception stack(0xc1d29fb0 to 0xc1d29ff8)
...
---[ end trace 5879c43bc748d0d3 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
CPU0: stopping

Reverting this patch and d44aca126b03 ("cpufreq: dt: 
dev_pm_opp_put_regulators() accepts NULL argument"), which depends on 
it, fixes the panic on current linux-next.

> ---
>   drivers/cpufreq/cpufreq-dt.c | 158 +++++++++++++++--------------------
>   1 file changed, 68 insertions(+), 90 deletions(-)
>
> ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland