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 v3 1/6] regulator: core: validate selector against
 linear_min_sel
To:     Claudiu Beznea <claudiu.beznea@microchip.com>,
        <lgirdwood@gmail.com>, <broonie@kernel.org>
CC:     <s.hauer@pengutronix.de>, <ttynkkynen@nvidia.com>,
        <linus.walleij@linaro.org>, <axel.lin@ingics.com>,
        <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        linux-tegra <linux-tegra@vger.kernel.org>
References: <1605280870-32432-1-git-send-email-claudiu.beznea@microchip.com>
 <1605280870-32432-2-git-send-email-claudiu.beznea@microchip.com>
From:   Jon Hunter <jonathanh@nvidia.com>
Message-ID: <cb096dc5-3757-d72f-41a9-c99007c84b40@nvidia.com>
Date:   Tue, 24 Nov 2020 09:36:52 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <1605280870-32432-2-git-send-email-claudiu.beznea@microchip.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 13/11/2020 15:21, Claudiu Beznea wrote:
> There are regulators who's min selector is not zero. Selectors loops
> (looping b/w zero and regulator::desc::n_voltages) might throw errors
> because invalid selectors are used (lower than
> regulator::desc::linear_min_sel). For this situations validate selectors
> against regulator::desc::linear_min_sel.


After this commit was merged, I noticed a regression in the DFLL (CPU
clock source) on Tegra124. The DFLL driver
(drivers/clk/tegra/clk-dfll.c) calls regulator_list_voltage() in a loop
to determine the selector for a given voltage (see function
find_vdd_map_entry_exact()).

Currently, the DFLL driver queries the number of voltages provided by
the regulator by calling regulator_count_voltages() and then starting
from 0, iterates through the number of voltages to find the selector
value for the voltage it is looking for by calling
regulator_list_voltage(). It assumes that any negative value returned by
calling regulator_list_voltage() is an error and this will cause the
loop up to terminate.

In this case the regulator in question is the as3722 and the
linear_min_sel for this regulator is 1 and so when the DFLL driver calls
regulator_list_voltage() with a selector value of 0 it now returns a
negative error code, as expected by this change, and this terminates the
loop up in the DFLL driver. So I can clearly see why this is happening
and I could fix up the DFLL driver to avoid this.

Before doing so, I wanted to ask if that is the correct fix here,
because it seems a bit odd that regulator_count_voltages() returns N
voltages, but if the min selector value is greater than 0, then actually
there are less than N. However, changing the number of voltages
supported by the regulator to be N - linear_min_sel does not make sense
either because then we need to know the linear_min_sel in order to
determine the first valid voltage.

In case of the as3722, the value 0 means that the regulator is powered
down. So it is a valid setting and equates to 0 volts at the output AFAICT.

Please let me know your thoughts are the correct way to fix this up.

Thanks
Jon

-- 
nvpublic