| Message ID | 20221116104716.2583320-3-abel.vesa@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk: qcom: Add support for SM8550 | expand |
On 17/11/2022 09:05, Abel Vesa wrote: > On 22-11-16 12:19:09, Konrad Dybcio wrote: >> >> >> On 16/11/2022 11:47, Abel Vesa wrote: >>> Depending on the platform, the poll timeout delay might be different, >>> so allow the platform specific drivers to specify their own values. >>> >>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> >>> --- >>> drivers/clk/qcom/gdsc.c | 5 ++++- >>> drivers/clk/qcom/gdsc.h | 1 + >>> 2 files changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c >>> index 0f21a8a767ac..3753f3ef7241 100644 >>> --- a/drivers/clk/qcom/gdsc.c >>> +++ b/drivers/clk/qcom/gdsc.c >>> @@ -107,7 +107,7 @@ static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status) >>> do { >>> if (gdsc_check_status(sc, status)) >>> return 0; >>> - } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US); >>> + } while (ktime_us_delta(ktime_get(), start) < sc->poll_timeout); >> What about the second usage of TIMEOUT_US (in gdsc_toggle_logic)? Is it fine >> for that to be the default value? > > The usleep you mention is not really for polling the state. > So I think it should stay as is. Who knows, maybe in the future we will > need to have the configurable as well, but as a toggle delay rather than > a status poll timeout. > > I added this configurable poll timeout just because I saw that > downstream, each driver has different values. And it kind of makes sense, > because the state machine inside the GDSC might be different between > platforms, and so, it might take different time to reach a certain on/off > state. > > Thanks, > Abel Okay, thanks for explaining Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > >> >> >> Konrad >>> if (gdsc_check_status(sc, status)) >>> return 0; >>> @@ -454,6 +454,9 @@ static int gdsc_init(struct gdsc *sc) >>> if (ret) >>> goto err_disable_supply; >>> + if (!sc->poll_timeout) >>> + sc->poll_timeout = 500; >>> + >>> return 0; >>> err_disable_supply: >>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h >>> index 803512688336..9a1e1fb3d12f 100644 >>> --- a/drivers/clk/qcom/gdsc.h >>> +++ b/drivers/clk/qcom/gdsc.h >>> @@ -36,6 +36,7 @@ struct gdsc { >>> struct generic_pm_domain *parent; >>> struct regmap *regmap; >>> unsigned int gdscr; >>> + unsigned int poll_timeout; >>> unsigned int collapse_ctrl; >>> unsigned int collapse_mask; >>> unsigned int gds_hw_ctrl;
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c index 0f21a8a767ac..3753f3ef7241 100644 --- a/drivers/clk/qcom/gdsc.c +++ b/drivers/clk/qcom/gdsc.c @@ -107,7 +107,7 @@ static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status) do { if (gdsc_check_status(sc, status)) return 0; - } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US); + } while (ktime_us_delta(ktime_get(), start) < sc->poll_timeout); if (gdsc_check_status(sc, status)) return 0; @@ -454,6 +454,9 @@ static int gdsc_init(struct gdsc *sc) if (ret) goto err_disable_supply; + if (!sc->poll_timeout) + sc->poll_timeout = 500; + return 0; err_disable_supply: diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h index 803512688336..9a1e1fb3d12f 100644 --- a/drivers/clk/qcom/gdsc.h +++ b/drivers/clk/qcom/gdsc.h @@ -36,6 +36,7 @@ struct gdsc { struct generic_pm_domain *parent; struct regmap *regmap; unsigned int gdscr; + unsigned int poll_timeout; unsigned int collapse_ctrl; unsigned int collapse_mask; unsigned int gds_hw_ctrl;
Depending on the platform, the poll timeout delay might be different, so allow the platform specific drivers to specify their own values. Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- drivers/clk/qcom/gdsc.c | 5 ++++- drivers/clk/qcom/gdsc.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)