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
