Re: [PATCH v3 1/6] regulator: core: validate selector against linear_min_sel

Jon Hunter
2020-11-24T09:37:00Z

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