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;
Subject: Re: [PATCH v2 19/48] opp: Fix adding OPP entries in a wrong order if
 rate is unavailable
To:     Viresh Kumar <viresh.kumar@linaro.org>
Cc:     Thierry Reding <thierry.reding@gmail.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Mark Brown <broonie@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Peter Geis <pgwipeout@gmail.com>,
        Nicolas Chauvet <kwizart@gmail.com>,
        Krzysztof Kozlowski <krzk@kernel.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Kevin Hilman <khilman@kernel.org>,
        Peter De Schrijver <pdeschrijver@nvidia.com>,
        Viresh Kumar <vireshk@kernel.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-media@vger.kernel.org, linux-tegra@vger.kernel.org,
        linux-clk@vger.kernel.org
References: <20201217180638.22748-1-digetx@gmail.com>
 <20201217180638.22748-20-digetx@gmail.com>
 <20201222091255.wentz5hyt726qezg@vireshk-i7>
 <db6273e6-8406-b7ae-b51d-48ceb6d21962@gmail.com>
 <20201223043443.rklw5er6hck3gl4y@vireshk-i7>
 <7688d6b9-52a2-d30f-123f-43c01e03b968@gmail.com>
 <20201224062826.frppxddfinjomfui@vireshk-i7>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <fa54a097-7edd-89af-6233-1e71c131668c@gmail.com>
Date:   Thu, 24 Dec 2020 15:14:01 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.2
MIME-Version: 1.0
In-Reply-To: <20201224062826.frppxddfinjomfui@vireshk-i7>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

24.12.2020 09:28, Viresh Kumar пишет:
> On 23-12-20, 23:36, Dmitry Osipenko wrote:
>> 23.12.2020 07:34, Viresh Kumar пишет:
>>> On 22-12-20, 22:19, Dmitry Osipenko wrote:
>>>> 22.12.2020 12:12, Viresh Kumar пишет:
>>>>> rate will be 0 for both the OPPs here if rate_not_available is true and so this
>>>>> change shouldn't be required.
>>>>
>>>> The rate_not_available is negated in the condition. This change is
>>>> required because both rates are 0 and then we should proceed to the
>>>> levels comparison.
>>>
>>> Won't that happen without this patch ?
>>
>> No
> 
> This is how the code looks like currently:
> 
> int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2)
> {
> 	if (opp1->rate != opp2->rate)
> 		return opp1->rate < opp2->rate ? -1 : 1;
> 	if (opp1->bandwidth && opp2->bandwidth &&
> 	    opp1->bandwidth[0].peak != opp2->bandwidth[0].peak)
> 		return opp1->bandwidth[0].peak < opp2->bandwidth[0].peak ? -1 : 1;
> 	if (opp1->level != opp2->level)
> 		return opp1->level < opp2->level ? -1 : 1;
> 	return 0;
> }
> 
> Lets consider the case you are focussing on, where rate is 0 for both the OPPs,
> bandwidth isn't there and we want to run the level comparison here.
> 
> Since both the rates are 0, (opp1->rate != opp2->rate) will fail and so we will
> move to bandwidth check which will fail too. And so we will get to the level
> comparison.
> 
> What am I missing here ? I am sure there is something for sure as you won't have
> missed this..
> 

Ah, you're right. It was me who was missing something as I see now,
after taking a closer look and trying to implement yours suggestion, my
bad. I'll improve this patch in the next revision, thanks!