diff mbox series

clk: tegra: clk-dfll: Verify regulator vsel values are valid

Message ID 20210127171121.322765-1-jonathanh@nvidia.com (mailing list archive)
State Awaiting Upstream, archived
Headers show
Series clk: tegra: clk-dfll: Verify regulator vsel values are valid | expand

Commit Message

Jon Hunter Jan. 27, 2021, 5:11 p.m. UTC
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(-)

Comments

Thierry Reding Jan. 27, 2021, 6:25 p.m. UTC | #1
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>
Jon Hunter Jan. 28, 2021, 11:24 a.m. UTC | #2
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 mbox series

Patch

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);