22.01.2021 07:35, Viresh Kumar пишет: > On 21-01-21, 23:26, Dmitry Osipenko wrote: >> 21.01.2021 14:17, Viresh Kumar пишет: >>> In order to avoid conditional statements at the caller site, this patch >>> updates _generic_set_opp_clk_only() to work for devices that don't >>> change frequency (like power domains, etc.). Return 0 if the clk pointer >>> passed to this routine is not valid. >>> >>> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx> >>> --- >> ... >> >> Hello Viresh, >> >> Thank you very much for yours effort! I gave a quick test to this series >> and instantly found one small issue in this patch. >> >>> + /* We may reach here for devices which don't change frequency */ >>> + if (unlikely(!clk)) >> >> I replaced dev_pm_opp_set_voltage() with dev_pm_opp_set_opp() in the >> Tegra PD driver and got a crash, which happens because the above line >> should be: >> >> if (IS_ERR(clk)) > > Fixed, thanks. > Please remove unlikely() around IS_ERR(), it already has the unlikely(). https://elixir.bootlin.com/linux/v5.11-rc4/source/include/linux/err.h#L22 I'd also recommend to remove all the unlikely() from OPP code since it doesn't bring any value if not used in a very performance-critical code path. OPP core doesn't have such code paths. The [un]likely() only make code less readable and may result in a worse assembly.