linux-kernel - Re: [PATCH 5/8] OPP: Allow multiple clocks for a device

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, 5 Jul 2022 10:18:47 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org,
        Vincent Guittot <vincent.guittot@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/8] OPP: Allow multiple clocks for a device

On 05/07/2022 08:59, Viresh Kumar wrote:
> On 30-06-22, 14:39, Krzysztof Kozlowski wrote:
>> On 30/06/2022 14:32, Krzysztof Kozlowski wrote:
>>> On 10/06/2022 10:20, Viresh Kumar wrote:
>>>> +	ret = _read_rate(new_opp, opp_table, np);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +	else if (opp_table->clk_count == 1)
>>>
>>> Shouldn't this be >=1? I got several clocks and this one fails.
>>
>> Actually this might be correct, but you need to update the bindings. Now
>> you require opp-level for case with multiple clocks.
> 
> I have thought about this again and adding such "fake" property in DT
> doesn't look right, specially in binding document. It maybe fine to
> have a "level" property in your case of UFS, where we want something
> to represent gears. But others may not want it.

I would say it is not different than existing opp-level property. To me
it sounded fine, so at least one DT bindings maintainer would accept it. :)

> 
> So, in the new version I am sending now, we still consider opp-hz
> property as the property that uniquely identifies an OPP. Just that we
> compare all the rates now, and not just the first one. I have updated
> _opp_compare_keys() for this as well.
> 
> The drivers, for multiple clock case, are expected to call
> dev_pm_opp_set_opp() to set the specific OPP. Though how they find the
> target OPP is left for the users to handle. For some, we may have
> another unique OPP property, like level, which can be used to find the
> OPP. While in case of others, we may want to implement freq-based OPP
> finder APIs for multiple clock rates. I have decided not to implement
> them in advance, and add them only someone wants to use them.

Thanks! Let me take a look at v2.


Best regards,
Krzysztof