| Message ID | 20181213093438.29621-9-josephl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tegra210 DFLL support | expand |
On 13/12/2018 09:34, 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@nvidia.com>. > > Signed-off-by: Joseph Lo <josephl@nvidia.com> > --- > *V2: > - s/align_volt/align_step/ > - s/reg_volt/reg_volt_id/ > --- > 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 72e02898006c..b3668073d9b4 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_step; > > 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_step = 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 (td->lut_uv[i] >= uv) > + if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= align_step) > return i; > } > > @@ -1532,15 +1532,17 @@ static int dfll_init(struct tegra_dfll *td) > */ > 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_id, align_step; > > + align_step = 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_id = regulator_list_voltage(td->vdd_reg, i) / > + td->soc->alignment.step_uv; > + if (reg_volt_id < 0) I don't think that this will work. If the step is say 10000 and we return an error code greater than -10000, we will end up with 0. > break; > > - if (uV == reg_uV) > + if (align_step == reg_volt_id) > 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_id, align_step; > > + align_step = 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_id = regulator_list_voltage(td->vdd_reg, i) / > + td->soc->alignment.step_uv; > + if (reg_volt_id < 0) Same here. > break; > > - if (uV <= reg_uV) > + if (align_step <= reg_volt_id) > return i; > } > >
On 12/13/18 7:46 PM, Jon Hunter wrote: > > On 13/12/2018 09:34, 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@nvidia.com>. >> >> Signed-off-by: Joseph Lo <josephl@nvidia.com> >> --- >> *V2: >> - s/align_volt/align_step/ >> - s/reg_volt/reg_volt_id/ >> --- >> 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 72e02898006c..b3668073d9b4 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_step; >> >> 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_step = 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 (td->lut_uv[i] >= uv) >> + if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= align_step) >> return i; >> } >> >> @@ -1532,15 +1532,17 @@ static int dfll_init(struct tegra_dfll *td) >> */ >> 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_id, align_step; >> >> + align_step = 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_id = regulator_list_voltage(td->vdd_reg, i) / >> + td->soc->alignment.step_uv; >> + if (reg_volt_id < 0) > > I don't think that this will work. If the step is say 10000 and we > return an error code greater than -10000, we will end up with 0. Ah,I think you mean when the error code smaller than step_uv. Yes, it will be 0 in that case. I think I will just remove the 'if' clause, then it will not match anything and leave the for loop. And return error if it couldn't find anything. Same as below. Thanks. > >> break; >> >> - if (uV == reg_uV) >> + if (align_step == reg_volt_id) >> 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_id, align_step; >> >> + align_step = 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_id = regulator_list_voltage(td->vdd_reg, i) / >> + td->soc->alignment.step_uv; >> + if (reg_volt_id < 0) > > Same here. > >> break; >> >> - if (uV <= reg_uV) >> + if (align_step <= reg_volt_id) >> return i; >> } >> >> >
On 14/12/2018 07:18, Joseph Lo wrote: > On 12/13/18 7:46 PM, Jon Hunter wrote: >> >> On 13/12/2018 09:34, 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@nvidia.com>. >>> >>> Signed-off-by: Joseph Lo <josephl@nvidia.com> >>> --- >>> *V2: >>> - s/align_volt/align_step/ >>> - s/reg_volt/reg_volt_id/ >>> --- >>> 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 72e02898006c..b3668073d9b4 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_step; >>> 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_step = 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 (td->lut_uv[i] >= uv) >>> + if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= align_step) >>> return i; >>> } >>> @@ -1532,15 +1532,17 @@ static int dfll_init(struct tegra_dfll *td) >>> */ >>> 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_id, align_step; >>> + align_step = 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_id = regulator_list_voltage(td->vdd_reg, i) / >>> + td->soc->alignment.step_uv; >>> + if (reg_volt_id < 0) >> >> I don't think that this will work. If the step is say 10000 and we >> return an error code greater than -10000, we will end up with 0. > > Ah,I think you mean when the error code smaller than step_uv. Yes, it > will be 0 in that case. I think I will just remove the 'if' clause, then > it will not match anything and leave the for loop. And return error if > it couldn't find anything. Same as below. Looks like if you remove the if clause then if will try to look up another voltage. Seems to me that we should just break out the loop on the first failure as it does today. Cheers Jon
diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c index 72e02898006c..b3668073d9b4 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_step; 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_step = 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 (td->lut_uv[i] >= uv) + if ((td->lut_uv[i] / td->soc->alignment.step_uv) >= align_step) return i; } @@ -1532,15 +1532,17 @@ static int dfll_init(struct tegra_dfll *td) */ 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_id, align_step; + align_step = 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_id = regulator_list_voltage(td->vdd_reg, i) / + td->soc->alignment.step_uv; + if (reg_volt_id < 0) break; - if (uV == reg_uV) + if (align_step == reg_volt_id) 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_id, align_step; + align_step = 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_id = regulator_list_voltage(td->vdd_reg, i) / + td->soc->alignment.step_uv; + if (reg_volt_id < 0) break; - if (uV <= reg_uV) + if (align_step <= reg_volt_id) return i; }
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@nvidia.com>. Signed-off-by: Joseph Lo <josephl@nvidia.com> --- *V2: - s/align_volt/align_step/ - s/reg_volt/reg_volt_id/ --- drivers/clk/tegra/clk-dfll.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)