Re: [PATCH] memory: samsung: exynos5422-dmc: Do not ignore return code of regmap_read()

[Lukasz Luba]

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 <l...@intel.com>
Signed-off-by: Krzysztof Kozlowski <k...@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, &reg);

+       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.l...@arm.com>

Regards,
Lukasz