Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

clk: qcom: gdsc: Fix handling of hw control enable/disable

Once a gdsc is brought in and out of HW control, there is a
power down and up cycle which can take upto 1us. Polling on
the gdsc status immediately after the hw control enable/disable
can mislead software/firmware to belive the gdsc is already either on
or off, while its yet to complete the power cycle.
To avoid this add a 1us delay post a enable/disable of HW control
mode.

Also after the HW control mode is disabled, poll on the status to
check gdsc status reflects its 'on' before force disabling it
in software.

Reported-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Reviewed-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Tested-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Fixes: 904bb4f5c7de ("clk: qcom: gdsc: Add support for gdscs with HW control")
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

authored by

Rajendra Nayak and committed by
Stephen Boyd
843be1e7 340a84ce

+45 -13
+45 -13
drivers/clk/qcom/gdsc.c
··· 63 63 return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val); 64 64 } 65 65 66 + static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en) 67 + { 68 + ktime_t start; 69 + 70 + start = ktime_get(); 71 + do { 72 + if (gdsc_is_enabled(sc, reg) == en) 73 + return 0; 74 + } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US); 75 + 76 + if (gdsc_is_enabled(sc, reg) == en) 77 + return 0; 78 + 79 + return -ETIMEDOUT; 80 + } 81 + 66 82 static int gdsc_toggle_logic(struct gdsc *sc, bool en) 67 83 { 68 84 int ret; 69 85 u32 val = en ? 0 : SW_COLLAPSE_MASK; 70 - ktime_t start; 71 86 unsigned int status_reg = sc->gdscr; 72 87 73 88 ret = regmap_update_bits(sc->regmap, sc->gdscr, SW_COLLAPSE_MASK, val); ··· 115 100 udelay(1); 116 101 } 117 102 118 - start = ktime_get(); 119 - do { 120 - if (gdsc_is_enabled(sc, status_reg) == en) 121 - return 0; 122 - } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US); 123 - 124 - if (gdsc_is_enabled(sc, status_reg) == en) 125 - return 0; 126 - 127 - return -ETIMEDOUT; 103 + return gdsc_poll_status(sc, status_reg, en); 128 104 } 129 105 130 106 static inline int gdsc_deassert_reset(struct gdsc *sc) ··· 194 188 udelay(1); 195 189 196 190 /* Turn on HW trigger mode if supported */ 197 - if (sc->flags & HW_CTRL) 198 - return gdsc_hwctrl(sc, true); 191 + if (sc->flags & HW_CTRL) { 192 + ret = gdsc_hwctrl(sc, true); 193 + if (ret) 194 + return ret; 195 + /* 196 + * Wait for the GDSC to go through a power down and 197 + * up cycle. In case a firmware ends up polling status 198 + * bits for the gdsc, it might read an 'on' status before 199 + * the GDSC can finish the power cycle. 200 + * We wait 1us before returning to ensure the firmware 201 + * can't immediately poll the status bits. 202 + */ 203 + udelay(1); 204 + } 199 205 200 206 return 0; 201 207 } ··· 222 204 223 205 /* Turn off HW trigger mode if supported */ 224 206 if (sc->flags & HW_CTRL) { 207 + unsigned int reg; 208 + 225 209 ret = gdsc_hwctrl(sc, false); 226 210 if (ret < 0) 211 + return ret; 212 + /* 213 + * Wait for the GDSC to go through a power down and 214 + * up cycle. In case we end up polling status 215 + * bits for the gdsc before the power cycle is completed 216 + * it might read an 'on' status wrongly. 217 + */ 218 + udelay(1); 219 + 220 + reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr; 221 + ret = gdsc_poll_status(sc, reg, true); 222 + if (ret) 227 223 return ret; 228 224 } 229 225