linux-kernel - Re: [Patch v2 7/9] cpufreq: tegra194: add OPP support and set bandwidth

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]

Date:   Tue, 28 Feb 2023 06:48:32 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Thierry Reding <treding@...dia.com>
Cc:     Sumit Gupta <sumitg@...dia.com>, krzysztof.kozlowski@...aro.org,
        dmitry.osipenko@...labora.com, rafael@...nel.org,
        jonathanh@...dia.com, robh+dt@...nel.org, lpieralisi@...nel.org,
        linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-pci@...r.kernel.org, mmaddireddy@...dia.com, kw@...ux.com,
        bhelgaas@...gle.com, vidyas@...dia.com, sanjayc@...dia.com,
        ksitaraman@...dia.com, ishah@...dia.com, bbasu@...dia.com
Subject: Re: [Patch v2 7/9] cpufreq: tegra194: add OPP support and set
 bandwidth

On 27-02-23, 13:44, Thierry Reding wrote:
> On Thu, Feb 23, 2023 at 03:06:26PM +0530, Sumit Gupta wrote:
> > On 22/02/23 09:33, Viresh Kumar wrote:
> > Tried using it and got below crash. It seems to be coming because we don't
> > have clocks property within CPU node for SoC's having BPMP-FW.
> > 
> >  Unable to handle kernel NULL pointer dereference at virtual address
> > 000000000000002e
> >  ....
> >  Call trace:
> >   clk_round_rate+0x38/0xd8
> >   dev_pm_opp_set_rate+0xe4/0x1a8
> >   tegra194_cpufreq_set_target+0x74/0x88
> >   __cpufreq_driver_target+0x154/0x250
> >   cpufreq_online+0x7b4/0x9ac
> 
> Can you try to find out what exactly is happening here? The clock
> framework should be able to deal with NULL clock pointers just fine.
> Although, looking at the OPP table code, it seems like we don't use
> clk_get_optional(), so opp_table->clk may end up being a pointer-
> encoded error. Perhaps we need something like this:
> 
> --- >8 ---
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index e87567dbe99f..d7baeb6ac697 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1397,6 +1397,7 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
>  		 * frequency in opp->rates and also parse the entries in DT.
>  		 */
>  		opp_table->clk_count = 1;
> +		opp_table->clk = NULL;
>  
>  		dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret);
>  		return opp_table;

I didn't reply earlier as I had nothing more to say and Sumit's
initial approach was correct. Maybe I should have I have clarified
this then.

The OPP core supports dev_pm_opp_set_rate() only for devices that can
set the rate, for everything else dev_pm_opp_set_opp() is the right
choice. I suggested dev_pm_opp_set_rate() earlier as I thought rate is
supported here.

-- 
viresh