Re: [PATCH V2 24/30] serial: qcom: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

On 01-07-22, 12:18, Greg Kroah-Hartman wrote:
> On Fri, Jul 01, 2022 at 03:31:00PM +0530, Viresh Kumar wrote:
> Still crazy, but a bit better.

:)

> Why do you need the clk_count?  A null terminated list is better,

Because I am not a big fan of the null terminated lists :)

I had to chase a bug once where someone removed that NULL at the end
and it was a nightmare to understand what's going on.

> as the
> compiler can do it for you and you do not have to keep things in sync
> like you are expecting people to be forced to do now.

I am not sure I understand what the compiler can do for us here.

The users will be required to do this here, isn't it ?

        const char *clks[] = { "core", NULL };
        struct dev_pm_opp_config opp_config = {
               .clk_names = clks,
        };


> The above is much more complex than a simple function call to make.
> Remember to make it very simple for driver authors, and more
> importantly, reviewers.

Hmm.

> Thanks, and drop the count field please.

There is one case at least [1] where we actually have to pass NULL in
the clk name. This is basically to allow the same code to run on
different devices, one where an OPP table is present and one where it
isn't. We don't want to do clk_set_rate() for the second case but just
use dev_pm_opp_set_rate() (which does a lot of stuff apart from just
clk).

-- 
viresh

[1] https://lore.kernel.org/lkml/b19a02422cae2408f953b92ae3c46a37fba688a3.1656660185.git.viresh.kumar@xxxxxxxxxx/