Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 9 Nov 2020 10:27:40 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Dmitry Osipenko <digetx@gmail.com>
Cc:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Kevin Hilman <khilman@kernel.org>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
        Stephen Boyd <sboyd@kernel.org>, linux-pm@vger.kernel.org,
        Vincent Guittot <vincent.guittot@linaro.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] opp: Don't create an OPP table from
 dev_pm_opp_get_opp_table()
Message-ID: <20201109045740.y5kpd6tjscoqxhi5@vireshk-i7>
References: <684ff01900180c0a40ec307dacc673b24eab593b.1604643714.git.viresh.kumar@linaro.org>
 <1012a98950355bd5a52424668050a17c3430cbe0.1604643714.git.viresh.kumar@linaro.org>
 <ec9839dd-5d2d-0d6b-6563-b14da4af1a57@gmail.com>
 <20201109043457.xf55kufhjjz2fvct@vireshk-i7>
 <c5078503-ee14-0f84-85fd-9c6e55d2e897@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c5078503-ee14-0f84-85fd-9c6e55d2e897@gmail.com>
User-Agent: NeoMutt/20180716-391-311a52
Precedence: bulk

On 09-11-20, 07:41, Dmitry Osipenko wrote:
> 09.11.2020 07:34, Viresh Kumar пишет:
> > On 06-11-20, 16:18, Dmitry Osipenko wrote:
> >> 06.11.2020 09:24, Viresh Kumar пишет:
> >>> It has been found that some users (like cpufreq-dt and others on LKML)
> >>> have abused the helper dev_pm_opp_get_opp_table() to create the OPP
> >>> table instead of just finding it, which is the wrong thing to do. This
> >>> routine was meant for OPP core's internal working and exposed the whole
> >>> functionality by mistake.
> >>>
> >>> Change the scope of dev_pm_opp_get_opp_table() to only finding the
> >>> table. The internal helpers _opp_get_opp_table*() are thus renamed to
> >>> _add_opp_table*(), dev_pm_opp_get_opp_table_indexed() is removed (as we
> >>> don't need the index field for finding the OPP table) and so the only
> >>> user, genpd, is updated.
> >>>
> >>> Note that the prototype of _add_opp_table() was already left in opp.h by
> >>> mistake when it was removed earlier and so we weren't required to add it
> >>> now.
> >>
> >> Hello Viresh,
> >>
> >> It looks like this is not an entirely correct change because previously
> >> it was possible to get an empty opp_table in order to use it for the
> >> dev_pm_opp_set_rate(), which would fall back to clk_set_rate if table is
> >> empty.
> >>
> >> Now it's not possible to get an empty table and
> >> dev_pm_opp_of_add_table() would error out if OPPs are missing in a
> >> device-tree. Hence it's not possible to implement a fall back without
> >> abusing opp_set_regulators() or opp_set_supported_hw() for getting the
> >> empty table. Or am I missing something?
> > 
> > For that case you were always required to call
> > dev_pm_opp_set_clkname(), otherwise how would the OPP core know which
> > clock to set ? And the same shall work now as well.
> 
> Why _allocate_opp_table() grabs the first default clk of a device and
> assigns it to the created table?

Right, it was there so everybody isn't required to call
dev_pm_opp_set_clkname() if they don't need to pass a connection id
while getting the clock. But for the case of supporting empty OPP
tables for cases where we just want dev_pm_opp_set_rate() to act as
clk_set_rate() (in order for the drivers to avoid supporting two
different ways of doing so), we do need that call to be made.

We need to add the OPP table explicitly and what happened with
dev_pm_opp_get_opp_table() was accidental and not what I wanted.

drivers/mmc/host/sdhci-msm.c has an example of how this is done.

-- 
viresh