On Wed, Dec 05, 2018 at 02:51:07PM +0800, Joseph Lo wrote:
> On 12/5/18 2:20 PM, Joseph Lo wrote:
> > On 12/4/18 11:46 PM, Peter De Schrijver wrote:
> > > On Tue, Dec 04, 2018 at 05:25:37PM +0800, Joseph Lo wrote:
> > > > When generating the OPP table, the voltages are round down with the
> > > > alignment from the regulator. The alignment should be applied for
> > > > voltages look up as well.
> > > >
> > > > Based on the work of Penny Chiu <pchiu@xxxxxxxxxx>.
> > > >
> > > > Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx>
> > > > ---
> > > > drivers/clk/tegra/clk-dfll.c | 26 +++++++++++++++-----------
> > > > 1 file changed, 15 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> > > > index c294a2989f31..4a943c136d4d 100644
> > > > --- a/drivers/clk/tegra/clk-dfll.c
> > > > +++ b/drivers/clk/tegra/clk-dfll.c
> > > > @@ -804,17 +804,17 @@ static void dfll_init_out_if(struct
> > > > tegra_dfll *td)
> > > > static int find_lut_index_for_rate(struct tegra_dfll *td,
> > > > unsigned long rate)
> > > > {
> > > > struct dev_pm_opp *opp;
> > > > - int i, uv;
> > > > + int i, align_volt;
> > > > opp = dev_pm_opp_find_freq_ceil(td->soc->dev, &rate);
> > > > if (IS_ERR(opp))
> > > > return PTR_ERR(opp);
> > > > - uv = dev_pm_opp_get_voltage(opp);
> > > > + align_volt = dev_pm_opp_get_voltage(opp) /
> > > > td->soc->alignment.step_uv;
> > > > dev_pm_opp_put(opp);
> > > > for (i = td->lut_bottom; i < td->lut_size; i++) {
> > > > - if (regulator_list_voltage(td->vdd_reg, td->lut[i]) == uv)
> > > > + if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= align_volt)
> > > > return i;
> > > > }
> > > > @@ -1532,15 +1532,17 @@ static int dfll_init(struct tegra_dfll *td)
> > > > */
> > >
> > > These 2 functions are only valid for I2C mode. We should probably add a
> > > WARN_ON() in case they are called when PWM mode is used and return
> > > -EINVAL.
> > >
> >
> > Okay, will add that.
> >
> Peter,
>
> Sorry, just double check again. These 2 functions are used for generating
> LUT table for DFLL-I2C mode. They are only used in "dfll_build_i2c_lut"
> function. So I think it's fine. The WARN_ON for protection from PWM mode is
> not necessary.
>
They are indeed not used today, but to prevent them from being used in
the future I was thinking it makes sense to add some form of protection
here.
Peter.
>
> >
> > > > static int find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
> > > > {
> > > > - int i, n_voltages, reg_uV;
> > > > + int i, n_voltages, reg_volt, align_volt;
> > > > + align_volt = uV / td->soc->alignment.step_uv;
> > > > n_voltages = regulator_count_voltages(td->vdd_reg);
> > > > for (i = 0; i < n_voltages; i++) {
> > > > - reg_uV = regulator_list_voltage(td->vdd_reg, i);
> > > > - if (reg_uV < 0)
> > > > + reg_volt = regulator_list_voltage(td->vdd_reg, i) /
> > > > + td->soc->alignment.step_uv;
> > > > + if (reg_volt < 0)
> > > > break;
> > > > - if (uV == reg_uV)
> > > > + if (align_volt == reg_volt)
> > > > return i;
> > > > }
> > > > @@ -1554,15 +1556,17 @@ static int
> > > > find_vdd_map_entry_exact(struct tegra_dfll *td, int uV)
> > > > * */
> > > > static int find_vdd_map_entry_min(struct tegra_dfll *td, int uV)
> > > > {
> > > > - int i, n_voltages, reg_uV;
> > > > + int i, n_voltages, reg_volt, align_volt;
> > > > + align_volt = uV / td->soc->alignment.step_uv;
> > > > n_voltages = regulator_count_voltages(td->vdd_reg);
> > > > for (i = 0; i < n_voltages; i++) {
> > > > - reg_uV = regulator_list_voltage(td->vdd_reg, i);
> > > > - if (reg_uV < 0)
> > > > + reg_volt = regulator_list_voltage(td->vdd_reg, i) /
> > > > + td->soc->alignment.step_uv;
> > > > + if (reg_volt < 0)
> > > > break;
> > > > - if (uV <= reg_uV)
> > > > + if (align_volt <= reg_volt)
> > > > return i;
> > > > }
> > > > --
> > > > 2.19.2
> > > >
