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 <[email protected]> > --- > 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)
