Re: [PATCH 1/3] clk: qcom: gdsc: propagate gdsc_check_status() errors from gdsc_poll_status

[Dmitry Baryshkov]

On Tue, Jun 02, 2026 at 04:09:32PM +0200, Herman van Hazendonk wrote:
> gdsc_check_status() returns negative errno when the underlying
> regmap_read() fails -- e.g. when a parent regmap dies during system
> suspend, a CSR is removed by an HW debug tool, or the bus controller
> goes into protection.

Well... For MMIO bus this is mostly theoretical...

> gdsc_poll_status() treats the result as a plain
> boolean ("is the GDSC in the requested state?"), so any negative error
> return is truncated to "true" and the poll exits with success even
> though the rail's real state is unknown:
> 
> 	do {
> 		if (gdsc_check_status(sc, status))
> 			return 0;
> 	} while (ktime_us_delta(ktime_get(), start) < STATUS_POLL_TIMEOUT_US);
> 
> 	if (gdsc_check_status(sc, status))
> 		return 0;
> 
> 	return -ETIMEDOUT;
> 
> This silently misleads gdsc_toggle_logic() (which writes/un-writes
> SW_COLLAPSE on the strength of the poll succeeding) and the gdsc_init()
> sync path (which assumes the readback represents real silicon state).
> 
> Latch the return value, propagate negative errno immediately, and only
> treat a strictly-positive value as "reached the target state". Make the
> same change in the post-timeout final check so a regmap that comes back
> after the deadline does not silently degrade to -ETIMEDOUT.
> 
> Signed-off-by: Herman van Hazendonk <github.com@xxxxxxxxxx>

Fixes: 77b1067a19b4 ("clk: qcom: gdsc: Add support for gdscs with gds hw controller")

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>



> ---
>  drivers/clk/qcom/gdsc.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 95aa07120245..b9b47f584f6d 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -103,14 +103,21 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
>  static int gdsc_poll_status(struct gdsc *sc, enum gdsc_status status)
>  {
>  	ktime_t start;
> +	int ret;
>  
>  	start = ktime_get();
>  	do {
> -		if (gdsc_check_status(sc, status))
> +		ret = gdsc_check_status(sc, status);
> +		if (ret < 0)
> +			return ret;
> +		if (ret)
>  			return 0;
>  	} while (ktime_us_delta(ktime_get(), start) < STATUS_POLL_TIMEOUT_US);
>  
> -	if (gdsc_check_status(sc, status))
> +	ret = gdsc_check_status(sc, status);
> +	if (ret < 0)
> +		return ret;
> +	if (ret)
>  		return 0;
>  
>  	return -ETIMEDOUT;
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry