| Message ID | 20210127171121.322765-1-jonathanh@nvidia.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | clk: tegra: clk-dfll: Verify regulator vsel values are valid | expand |
On Wed, Jan 27, 2021 at 05:11:21PM +0000, Jon Hunter wrote: > The regulator function, regulator_list_hardware_vsel(), may return an > negative error code on failure. The Tegra DFLL driver does not check to > see if the value returned by this function is an error. Fix this by > updating the DFLL driver to check if the value returned by > regulator_list_hardware_vsel() is an error and if an error does occur > propagate the error. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > drivers/clk/tegra/clk-dfll.c | 32 ++++++++++++++++++++++++-------- > 1 file changed, 24 insertions(+), 8 deletions(-) Does this fix any particular issue? Do we want a Fixes: line for this? In either case, this looks like a correct fix, so: Acked-by: Thierry Reding <treding@nvidia.com>
On 27/01/2021 18:25, Thierry Reding wrote: > On Wed, Jan 27, 2021 at 05:11:21PM +0000, Jon Hunter wrote: >> The regulator function, regulator_list_hardware_vsel(), may return an >> negative error code on failure. The Tegra DFLL driver does not check to >> see if the value returned by this function is an error. Fix this by >> updating the DFLL driver to check if the value returned by >> regulator_list_hardware_vsel() is an error and if an error does occur >> propagate the error. >> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> drivers/clk/tegra/clk-dfll.c | 32 ++++++++++++++++++++++++-------- >> 1 file changed, 24 insertions(+), 8 deletions(-) > > Does this fix any particular issue? Do we want a Fixes: line for this? > > In either case, this looks like a correct fix, so: > > Acked-by: Thierry Reding <treding@nvidia.com> I have not seen any issue so far, but noticed that we were not checking the return value as we should. I happened to notice this while looking into this issue [0] and so thought we should make the code more robust. We could add ... Fixes: c4fe70ada40f ("clk: tegra: Add closed loop support for the DFLL") Jon [0] https://lkml.org/lkml/2020/11/24/278
diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c index a5f526bb0483..709fb1fe7073 100644 --- a/drivers/clk/tegra/clk-dfll.c +++ b/drivers/clk/tegra/clk-dfll.c @@ -672,10 +672,9 @@ static int dfll_force_output(struct tegra_dfll *td, unsigned int out_sel) * Load the voltage-to-PMIC register value lookup table into the DFLL * IP block memory. Look-up tables can be loaded at any time. */ -static void dfll_load_i2c_lut(struct tegra_dfll *td) +static int dfll_load_i2c_lut(struct tegra_dfll *td) { - int i, lut_index; - u32 val; + int i, lut_index, val; for (i = 0; i < MAX_DFLL_VOLTAGES; i++) { if (i < td->lut_min) @@ -687,10 +686,15 @@ static void dfll_load_i2c_lut(struct tegra_dfll *td) val = regulator_list_hardware_vsel(td->vdd_reg, td->lut[lut_index]); + if (val < 0) + return val; + __raw_writel(val, td->lut_base + i * 4); } dfll_i2c_wmb(td); + + return 0; } /** @@ -737,9 +741,10 @@ static void dfll_init_i2c_if(struct tegra_dfll *td) * disable the I2C command output to the PMIC, set safe voltage and * output limits, and disable and clear limit interrupts. */ -static void dfll_init_out_if(struct tegra_dfll *td) +static int dfll_init_out_if(struct tegra_dfll *td) { u32 val; + int ret; td->lut_min = td->lut_bottom; td->lut_max = td->lut_size - 1; @@ -773,9 +778,14 @@ static void dfll_init_out_if(struct tegra_dfll *td) dfll_force_output(td, vsel); } } else { - dfll_load_i2c_lut(td); + ret = dfll_load_i2c_lut(td); + if (ret < 0) + return ret; + dfll_init_i2c_if(td); } + + return 0; } /* @@ -1497,12 +1507,17 @@ static int dfll_init(struct tegra_dfll *td) dfll_set_open_loop_config(td); - dfll_init_out_if(td); + ret = dfll_init_out_if(td); pm_runtime_put_sync(td->dev); + if (ret < 0) + goto disable_rpm; + return 0; +disable_rpm: + pm_runtime_disable(td->dev); di_err2: clk_unprepare(td->soc_clk); di_err1: @@ -1547,6 +1562,7 @@ EXPORT_SYMBOL(tegra_dfll_suspend); int tegra_dfll_resume(struct device *dev) { struct tegra_dfll *td = dev_get_drvdata(dev); + int ret; reset_control_deassert(td->dvco_rst); @@ -1560,11 +1576,11 @@ int tegra_dfll_resume(struct device *dev) dfll_set_open_loop_config(td); - dfll_init_out_if(td); + ret = dfll_init_out_if(td); pm_runtime_put_sync(td->dev); - return 0; + return ret; } EXPORT_SYMBOL(tegra_dfll_resume);
The regulator function, regulator_list_hardware_vsel(), may return an negative error code on failure. The Tegra DFLL driver does not check to see if the value returned by this function is an error. Fix this by updating the DFLL driver to check if the value returned by regulator_list_hardware_vsel() is an error and if an error does occur propagate the error. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/clk/tegra/clk-dfll.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-)