On 11/03/2024 21:33, Volodymyr Babchuk wrote:
> Now sub-drivers for particular SoCs can register them as power domain
> drivers. This is needed for upcoming SM8150 support, because it needs
> to power up the Ethernet module.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
> 
> ---
> Caleb suggested to use "imply POWER_DOMAIN", not "depends
> POWER_DOMAIN" in the Kconfig, but this does not work:
> $ make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm
> scripts/kconfig/conf  --syncconfig Kconfig
> drivers/clk/Kconfig:3:error: recursive dependency detected!
> drivers/clk/Kconfig:3:  symbol CLK is selected by IMX8M_POWER_DOMAIN
> drivers/power/domain/Kconfig:35:        symbol IMX8M_POWER_DOMAIN depends on 
> POWER_DOMAIN
> drivers/power/domain/Kconfig:3: symbol POWER_DOMAIN is implied by CLK_QCOM
> drivers/clk/qcom/Kconfig:3:     symbol CLK_QCOM depends on CLK
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> 
> 
> Changes in v3:
>  - Added "depends POWER_DOMAIN" to Kconfig (see note)
>  - Use readl_poll_timeout() instead of open coded wait loop
>  - Print warning if power domain can't be enabled/disabled
> 
> Changes in v2:
>  - Reworked qcom_cc_bind() function
>  - Added timeout to qcom_power_set()
>  - Minor fixes in register names and formatting
> 
>  drivers/clk/qcom/Kconfig      |   2 +-
>  drivers/clk/qcom/clock-qcom.c | 132 ++++++++++++++++++++++++++++++----
>  drivers/clk/qcom/clock-qcom.h |   6 ++
>  3 files changed, 126 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 0df0d1881a..8dae635ac2 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -2,7 +2,7 @@ if ARCH_SNAPDRAGON || ARCH_IPQ40XX
>  
>  config CLK_QCOM
>       bool
> -     depends on CLK && DM_RESET
> +     depends on CLK && DM_RESET && POWER_DOMAIN

This results in a similar dependency error to the one above unless
CONFIG_POWER_DOMAIN is enabled in the defconfig. I think we'll just add
"select POWER_DOMAIN" to ARCH_SNAPDRAGON, same as we have for DM_RESET.

I'll do then when picking this series up, just a note to self.
>       def_bool n
>  
>  menu "Qualcomm clock drivers"
> diff --git a/drivers/clk/qcom/clock-qcom.c b/drivers/clk/qcom/clock-qcom.c
> index 729d190c54..7a5938a06a 100644
> --- a/drivers/clk/qcom/clock-qcom.c
> +++ b/drivers/clk/qcom/clock-qcom.c
> @@ -22,7 +22,9 @@
>  #include <linux/bug.h>
>  #include <linux/delay.h>
>  #include <linux/bitops.h>
> +#include <linux/iopoll.h>
>  #include <reset-uclass.h>
> +#include <power-domain-uclass.h>
>  
>  #include "clock-qcom.h"
>  
> @@ -30,6 +32,13 @@
>  #define CBCR_BRANCH_ENABLE_BIT  BIT(0)
>  #define CBCR_BRANCH_OFF_BIT     BIT(31)
>  
> +#define GDSC_SW_COLLAPSE_MASK                BIT(0)
> +#define GDSC_POWER_DOWN_COMPLETE     BIT(15)
> +#define GDSC_POWER_UP_COMPLETE               BIT(16)
> +#define GDSC_PWR_ON_MASK             BIT(31)
> +#define CFG_GDSCR_OFFSET             0x4
> +#define GDSC_STATUS_POLL_TIMEOUT_US  1500
> +
>  /* Enable clock controlled by CBC soft macro */
>  void clk_enable_cbc(phys_addr_t cbcr)
>  {
> @@ -223,7 +232,7 @@ U_BOOT_DRIVER(qcom_clk) = {
>  int qcom_cc_bind(struct udevice *parent)
>  {
>       struct msm_clk_data *data = (struct msm_clk_data 
> *)dev_get_driver_data(parent);
> -     struct udevice *clkdev, *rstdev;
> +     struct udevice *clkdev = NULL, *rstdev = NULL, *pwrdev;
>       struct driver *drv;
>       int ret;
>  
> @@ -238,20 +247,41 @@ int qcom_cc_bind(struct udevice *parent)
>       if (ret)
>               return ret;
>  
> -     /* Bail out early if resets are not specified for this platform */
> -     if (!data->resets)
> -             return ret;
> +     if (data->resets) {
> +             /* Get a handle to the common reset handler */
> +             drv = lists_driver_lookup_name("qcom_reset");
> +             if (!drv) {
> +                     ret = -ENOENT;
> +                     goto unbind_clkdev;
> +             }
> +
> +             /* Register the reset controller */
> +             ret = device_bind_with_driver_data(parent, drv, "qcom_reset", 
> (ulong)data,
> +                                                dev_ofnode(parent), &rstdev);
> +             if (ret)
> +                     goto unbind_clkdev;
> +     }
>  
> -     /* Get a handle to the common reset handler */
> -     drv = lists_driver_lookup_name("qcom_reset");
> -     if (!drv)
> -             return -ENOENT;
> +     if (data->power_domains) {
> +             /* Get a handle to the common power domain handler */
> +             drv = lists_driver_lookup_name("qcom_power");
> +             if (!drv) {
> +                     ret = -ENOENT;
> +                     goto unbind_rstdev;
> +             }
> +             /* Register the power domain controller */
> +             ret = device_bind_with_driver_data(parent, drv, "qcom_power", 
> (ulong)data,
> +                                                dev_ofnode(parent), &pwrdev);
> +             if (ret)
> +                     goto unbind_rstdev;
> +     }
>  
> -     /* Register the reset controller */
> -     ret = device_bind_with_driver_data(parent, drv, "qcom_reset", 
> (ulong)data,
> -                                        dev_ofnode(parent), &rstdev);
> -     if (ret)
> -             device_unbind(clkdev);
> +     return 0;
> +
> +unbind_rstdev:
> +     device_unbind(rstdev);
> +unbind_clkdev:
> +     device_unbind(clkdev);
>  
>       return ret;
>  }
> @@ -306,3 +336,79 @@ U_BOOT_DRIVER(qcom_reset) = {
>       .ops = &qcom_reset_ops,
>       .probe = qcom_reset_probe,
>  };
> +
> +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;
> +     int ret;
> +
> +     if (pwr->id >= data->num_power_domains)
> +             return -ENODEV;
> +
> +     map = &data->power_domains[pwr->id];
> +
> +     if (!map->reg)
> +             return -ENODEV;
> +
> +     value = readl(base + map->reg);
> +
> +     if (on)
> +             value &= ~GDSC_SW_COLLAPSE_MASK;
> +     else
> +             value |= GDSC_SW_COLLAPSE_MASK;
> +
> +     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);
> +
> +
> +     if (ret == -ETIMEDOUT)
> +             printf("WARNING: GDSC %lu is stuck during power on/off\n",
> +                    pwr->id);
> +     return ret;
> +}
> +
> +static int qcom_power_on(struct power_domain *pwr)
> +{
> +     return qcom_power_set(pwr, true);
> +}
> +
> +static int qcom_power_off(struct power_domain *pwr)
> +{
> +     return qcom_power_set(pwr, false);
> +}
> +
> +static const struct power_domain_ops qcom_power_ops = {
> +     .on = qcom_power_on,
> +     .off = qcom_power_off,
> +};
> +
> +static int qcom_power_probe(struct udevice *dev)
> +{
> +     /* Set our priv pointer to the base address */
> +     dev_set_priv(dev, (void *)dev_read_addr(dev));
> +
> +     return 0;
> +}
> +
> +U_BOOT_DRIVER(qcom_power) = {
> +     .name = "qcom_power",
> +     .id = UCLASS_POWER_DOMAIN,
> +     .ops = &qcom_power_ops,
> +     .probe = qcom_power_probe,
> +};
> diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
> index 01088c1901..12a1eaec2b 100644
> --- a/drivers/clk/qcom/clock-qcom.h
> +++ b/drivers/clk/qcom/clock-qcom.h
> @@ -59,9 +59,15 @@ struct qcom_reset_map {
>       u8 bit;
>  };
>  
> +struct qcom_power_map {
> +     unsigned int reg;
> +};
> +
>  struct clk;
>  
>  struct msm_clk_data {
> +     const struct qcom_power_map     *power_domains;
> +     unsigned long                   num_power_domains;
>       const struct qcom_reset_map     *resets;
>       unsigned long                   num_resets;
>       const struct gate_clk           *clks;

-- 
// Caleb (they/them)

Reply via email to