Re: [PATCH 2/9] clk: qcom: gdsc: Add configurable poll timeout

[Abel Vesa]

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@xxxxxxxxxx>
> > ---
> >   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

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