linux-kernel - Re: [PATCH v8 01/34] opp: Add dev_pm_opp

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: <20210819061617.r4kuqxafjstrv3kt@vireshk-i7>
Date:   Thu, 19 Aug 2021 11:46:17 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Dmitry Osipenko <digetx@...il.com>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Viresh Kumar <vireshk@...nel.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Peter De Schrijver <pdeschrijver@...dia.com>,
        Mikko Perttunen <mperttunen@...dia.com>,
        Peter Chen <peter.chen@...nel.org>,
        Mark Brown <broonie@...nel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, Nishanth Menon <nm@...com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Richard Weinberger <richard@....at>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Lucas Stach <dev@...xeye.de>, Stefan Agner <stefan@...er.ch>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-tegra <linux-tegra@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux USB List <linux-usb@...r.kernel.org>,
        linux-staging@...ts.linux.dev, linux-spi@...r.kernel.org,
        linux-pwm@...r.kernel.org, linux-mtd@...ts.infradead.org,
        linux-mmc <linux-mmc@...r.kernel.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        DTML <devicetree@...r.kernel.org>,
        linux-clk <linux-clk@...r.kernel.org>
Subject: Re: [PATCH v8 01/34] opp: Add dev_pm_opp_sync() helper

On 18-08-21, 18:55, Dmitry Osipenko wrote:
> 18.08.2021 12:41, Ulf Hansson пишет:
> 
> Either way gives the equal result. The new callback allows to remove the
> boilerplate dev_pm_opp_set_rate(clk_get_rate() code from the rpm-resume
> of consumer devices, that's it.

It may not be equal, as dev_pm_opp_set_rate() may do additional stuff,
now or in a later implementation. Currently it only does
regulator_enable() as a special case, but it can be clk_enable() as
well. Also, this tries to solve the problem in a tricky/hacky way,
while all you wanted was to make the genpd aware of what the
performance state should be.

Your driver can break tomorrow if we started to do more stuff from
this API at another time.

> > dev_pm_opp_set_rate() is best called from consumer drivers, as they
> > need to be in control.
> >> What we need here is just configure. So something like this then:
> The intent wasn't to use dev_pm_opp_set_rate() from
> __genpd_dev_pm_attach(), but to set genpd->rpm_pstate in accordance to
> the h/w configuration.

Right.

> On Tegra we have a chain of PDs and it's not trivial to convert the
> device's OPP into pstate because only the parent domain can translate
> the required OPP.

The driver should just be required to make a call, and OPP/genpd core
should return it a value. This is already done today while setting the
pstate for a device. The same frameworks must be able to supply a
value to be used for the device.

> Viresh, please take a look at what I did in [1]. Maybe it could be done
> in another way.

I looked into this and looked like too much trouble. The
implementation needs to be simple. I am not sure I understand all the
problems you faced while doing that, would be better to start with a
simpler implementation of get_performance_state() kind of API for
genpd, after the domain is attached and its OPP table is initialized.

Note, that the OPP table isn't required to be fully initialized for
the device at this point, we can parse the DT as well if needed be.

-- 
viresh