[PATCH 2/4] clk/qcom: Treat different GDSCs didderently

[Caleb Connolly]

Hi Alexey,

Thanks for the patch! Really happy to see support for this new platform.

On 08/01/2025 12:59, Alexey Minnekhanov wrote:
> Apparently not all GDSCs are the same. In Linux driver, depending on which
> GDSC flags are set, different status register is checked when powering it
> on/off. And on top of that, different bit inside that register is tested.
> 
> Port missing parts from Linux driver to U-Boot:
>  - add GDSC flags;
>  - adjust logic when checking GDSC status to match one in Linux driver.

Sorry it's a bit of a pain, but could you split this into two patch and
adjust the ordering so we don't risk breaking someones bisect if they
land here. I'd propose:

Patch 1: as-is
Patch 2: add the flags enum and adjust struct qcom_power_map
Patch 3: would be the "port to new flags" patch
Patch 4: adjust qcom_power_set() (the bulk of this patch)
Patch 5: add sdm660 clock driver

does this make sense?
> 
> This fixes e.g. SDM660's USB_30_GDSC to be forever stuck during power on,
> since unlike SDM845 one it doesn't have POLL_CFG_GDSCR flag.
> 
> Even though only POLL_CFG_GDSCR is the important one here, add all flags
> from Linux driver, to make it easier to compare which flags are needed
> for which GDSC, making porting process easier.

This is great, thanks for taking the time to add all of these.

Kind regards,
> 
> Signed-off-by: Alexey Minnekhanov <alexeymin at postmarketos.org>
> ---
>  drivers/clk/qcom/clock-qcom.c | 45 ++++++++++++++++++++++++-----------
>  drivers/clk/qcom/clock-qcom.h | 16 ++++++++++++-
>  2 files changed, 46 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clock-qcom.c b/drivers/clk/qcom/clock-qcom.c
> index 1cfc430c14a5..bf9249b55af3 100644
> --- a/drivers/clk/qcom/clock-qcom.c
> +++ b/drivers/clk/qcom/clock-qcom.c
> @@ -461,7 +461,7 @@ static int qcom_power_set(struct power_domain *pwr, bool on)
>  	struct msm_clk_data *data = (struct msm_clk_data *)dev_get_driver_data(pwr->dev);
>  	void __iomem *base = dev_get_priv(pwr->dev);
>  	const struct qcom_power_map *map;
> -	u32 value;
> +	u32 value, status_reg;
>  	int ret;
>  
>  	if (pwr->id >= data->num_power_domains)
> @@ -481,19 +481,36 @@ static int qcom_power_set(struct power_domain *pwr, bool on)
>  
>  	writel(value, base + map->reg);
>  
> -	if (on)
> -		ret = readl_poll_timeout(base + map->reg + CFG_GDSCR_OFFSET,
> -					 value,
> -					 (value & GDSC_POWER_UP_COMPLETE) ||
> -					 (value & GDSC_PWR_ON_MASK),
> -					 GDSC_STATUS_POLL_TIMEOUT_US);
> -
> -	else
> -		ret = readl_poll_timeout(base + map->reg + CFG_GDSCR_OFFSET,
> -					 value,
> -					 (value & GDSC_POWER_DOWN_COMPLETE) ||
> -					 !(value & GDSC_PWR_ON_MASK),
> -					 GDSC_STATUS_POLL_TIMEOUT_US);
> +	/* depending on the type of gdsc the status register is different */
> +	/* and we need to check different status bit */
> +	if (map->flags & POLL_CFG_GDSCR) {
> +		status_reg = map->reg + CFG_GDSCR_OFFSET;
> +
> +		if (on)
> +			ret = readl_poll_timeout(base + status_reg,
> +						 value,
> +						 (value & GDSC_POWER_UP_COMPLETE),
> +						 GDSC_STATUS_POLL_TIMEOUT_US);
> +		else
> +			ret = readl_poll_timeout(base + status_reg,
> +						 value,
> +						 (value & GDSC_POWER_DOWN_COMPLETE),
> +						 GDSC_STATUS_POLL_TIMEOUT_US);
> +	} else {
> +		status_reg = map->reg;
> +
> +		if (on)
> +			ret = readl_poll_timeout(base + status_reg,
> +						 value,
> +						 (value & GDSC_PWR_ON_MASK),
> +						 GDSC_STATUS_POLL_TIMEOUT_US);
> +
> +		else
> +			ret = readl_poll_timeout(base + status_reg,
> +						 value,
> +						 !(value & GDSC_PWR_ON_MASK),
> +						 GDSC_STATUS_POLL_TIMEOUT_US);
> +	}
>  
>  	if (ret == -ETIMEDOUT)
>  		printf("WARNING: GDSC %lu is stuck during power o%s\n",
> diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
> index 78d9b1d81ece..84965b2555c7 100644
> --- a/drivers/clk/qcom/clock-qcom.h
> +++ b/drivers/clk/qcom/clock-qcom.h
> @@ -63,8 +63,22 @@ struct qcom_reset_map {
>  	u8 bit;
>  };
>  
> +enum qcom_gdsc_flags {
> +	VOTABLE = BIT(0),
> +	CLAMP_IO = BIT(1),
> +	HW_CTRL = BIT(2),
> +	SW_RESET = BIT(3),
> +	AON_RESET = BIT(4),
> +	POLL_CFG_GDSCR = BIT(5),
> +	ALWAYS_ON = BIT(6),
> +	RETAIN_FF_ENABLE = BIT(7),
> +	NO_RET_PERIPH = BIT(8),
> +	HW_CTRL_TRIGGER = BIT(9),
> +};
> +
>  struct qcom_power_map {
> -	unsigned int reg;
> +	unsigned int reg; /* GDSCR */
> +	unsigned int flags;
>  };
>  
>  struct clk;

-- 
// Caleb (they/them)