| Message ID | 20200720110301.22996-1-krzk@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | c4f16e96d8fdd62ef12898fc0965c42093bed237 |
| Headers | show |
| Series | memory: samsung: exynos5422-dmc: Do not ignore return code of regmap_read() | expand |
Hi Krzysztof, On 7/20/20 12:03 PM, Krzysztof Kozlowski wrote: > Check for regmap_read() return code before using the read value in > following write in exynos5_switch_timing_regs(). Pass reading error > code to the callers. > > This does not introduce proper error handling for such failed reads (and > obviously regmap_write() error is still ignored) because the driver > ignored this in all places. Therefor it only fixes reported issue while > matching current driver coding style: > > drivers/memory/samsung/exynos5422-dmc.c: In function 'exynos5_switch_timing_regs': > >> drivers/memory/samsung/exynos5422-dmc.c:216:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > drivers/memory/samsung/exynos5422-dmc.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c > index 93e9c2429c0d..2c326998cb1c 100644 > --- a/drivers/memory/samsung/exynos5422-dmc.c > +++ b/drivers/memory/samsung/exynos5422-dmc.c > @@ -270,12 +270,14 @@ static int find_target_freq_idx(struct exynos5_dmc *dmc, > * This function switches between these banks according to the > * currently used clock source. > */ > -static void exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set) > +static int exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set) > { > unsigned int reg; > int ret; > > ret = regmap_read(dmc->clk_regmap, CDREX_LPDDR3PHY_CON3, ®); > + if (ret) > + return ret; > > if (set) > reg |= EXYNOS5_TIMING_SET_SWI; > @@ -283,6 +285,8 @@ static void exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set) > reg &= ~EXYNOS5_TIMING_SET_SWI; > > regmap_write(dmc->clk_regmap, CDREX_LPDDR3PHY_CON3, reg); > + > + return 0; > } > > /** > @@ -516,7 +520,7 @@ exynos5_dmc_switch_to_bypass_configuration(struct exynos5_dmc *dmc, > /* > * Delays are long enough, so use them for the new coming clock. > */ > - exynos5_switch_timing_regs(dmc, USE_MX_MSPLL_TIMINGS); > + ret = exynos5_switch_timing_regs(dmc, USE_MX_MSPLL_TIMINGS); > > return ret; > } > @@ -577,7 +581,9 @@ exynos5_dmc_change_freq_and_volt(struct exynos5_dmc *dmc, > > clk_set_rate(dmc->fout_bpll, target_rate); > > - exynos5_switch_timing_regs(dmc, USE_BPLL_TIMINGS); > + ret = exynos5_switch_timing_regs(dmc, USE_BPLL_TIMINGS); > + if (ret) > + goto disable_clocks; > > ret = clk_set_parent(dmc->mout_mclk_cdrex, dmc->mout_bpll); > if (ret) > LGTM Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> Regards, Lukasz
diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c index 93e9c2429c0d..2c326998cb1c 100644 --- a/drivers/memory/samsung/exynos5422-dmc.c +++ b/drivers/memory/samsung/exynos5422-dmc.c @@ -270,12 +270,14 @@ static int find_target_freq_idx(struct exynos5_dmc *dmc, * This function switches between these banks according to the * currently used clock source. */ -static void exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set) +static int exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set) { unsigned int reg; int ret; ret = regmap_read(dmc->clk_regmap, CDREX_LPDDR3PHY_CON3, ®); + if (ret) + return ret; if (set) reg |= EXYNOS5_TIMING_SET_SWI; @@ -283,6 +285,8 @@ static void exynos5_switch_timing_regs(struct exynos5_dmc *dmc, bool set) reg &= ~EXYNOS5_TIMING_SET_SWI; regmap_write(dmc->clk_regmap, CDREX_LPDDR3PHY_CON3, reg); + + return 0; } /** @@ -516,7 +520,7 @@ exynos5_dmc_switch_to_bypass_configuration(struct exynos5_dmc *dmc, /* * Delays are long enough, so use them for the new coming clock. */ - exynos5_switch_timing_regs(dmc, USE_MX_MSPLL_TIMINGS); + ret = exynos5_switch_timing_regs(dmc, USE_MX_MSPLL_TIMINGS); return ret; } @@ -577,7 +581,9 @@ exynos5_dmc_change_freq_and_volt(struct exynos5_dmc *dmc, clk_set_rate(dmc->fout_bpll, target_rate); - exynos5_switch_timing_regs(dmc, USE_BPLL_TIMINGS); + ret = exynos5_switch_timing_regs(dmc, USE_BPLL_TIMINGS); + if (ret) + goto disable_clocks; ret = clk_set_parent(dmc->mout_mclk_cdrex, dmc->mout_bpll); if (ret)
Check for regmap_read() return code before using the read value in following write in exynos5_switch_timing_regs(). Pass reading error code to the callers. This does not introduce proper error handling for such failed reads (and obviously regmap_write() error is still ignored) because the driver ignored this in all places. Therefor it only fixes reported issue while matching current driver coding style: drivers/memory/samsung/exynos5422-dmc.c: In function 'exynos5_switch_timing_regs': >> drivers/memory/samsung/exynos5422-dmc.c:216:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable] Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- drivers/memory/samsung/exynos5422-dmc.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)