linux-kernel - Re: [PATCH v2 28/48] soc/tegra: Introduce core power domain driver

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]

Message-ID: <3a5c00e5-2cdd-35ce-2714-d4ffbf9d516a@gmail.com>
Date:   Tue, 22 Dec 2020 22:39:23 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Mark Brown <broonie@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Peter Geis <pgwipeout@...il.com>,
        Nicolas Chauvet <kwizart@...il.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Kevin Hilman <khilman@...nel.org>,
        Peter De Schrijver <pdeschrijver@...dia.com>,
        Viresh Kumar <vireshk@...nel.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-media@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH v2 28/48] soc/tegra: Introduce core power domain driver

22.12.2020 22:21, Dmitry Osipenko пишет:
>>> +	if (IS_ERR(opp)) {
>>> +		dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n",
>>> +			level, opp);
>>> +		return PTR_ERR(opp);
>>> +	}
>>> +
>>> +	err = dev_pm_opp_set_voltage(&genpd->dev, opp);
>> IIUC, you implemented this callback because you want to use the voltage triplet
>> present in the OPP table ?
>>
>> And so you are setting the regulator ("power") later in this patch ?
> yes
> 
>> I am not in favor of implementing this routine, as it just adds a wrapper above
>> the regulator API. What you should be doing rather is get the regulator by
>> yourself here (instead of depending on the OPP core). And then you can do
>> dev_pm_opp_get_voltage() here and set the voltage yourself. You may want to
>> implement a version supporting triplet here though for the same.
>>
>> And you won't require the sync version of the API as well then.
>>
> That's what I initially did for this driver. I don't mind to revert back
> to the initial variant in v3, it appeared to me that it will be nicer
> and cleaner to have OPP API managing everything here.

I forgot one important detail (why the initial variant wasn't good)..
OPP entries that have unsupportable voltages should be filtered out and
OPP core performs the filtering only if regulator is assigned to the OPP
table.

If regulator is assigned to the OPP table, then we need to use OPP API
for driving the regulator, hence that's why I added
dev_pm_opp_sync_regulators() and dev_pm_opp_set_voltage().

Perhaps it should be possible to add dev_pm_opp_get_regulator() that
will return the OPP table regulator in order to allow driver to use the
regulator directly. But I'm not sure whether this is a much better
option than the opp_sync_regulators() and opp_set_voltage() APIs.