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@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);elseret = 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);elseret = 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;