Re: [PATCH RESEND v3 3/5] clk: qcom: gdsc: Add set and get hwmode callbacks to switch GDSC mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/2/2023 2:58 AM, Dmitry Baryshkov wrote:
On Wed, 1 Nov 2023 at 11:06, Abel Vesa <abel.vesa@xxxxxxxxxx> wrote:

From: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>

Add support for set and get hwmode callbacks to switch the GDSC between
SW and HW modes. Currently, the GDSC is moved to HW control mode
using HW_CTRL flag and if this flag is present, GDSC is moved to HW
mode as part of GDSC enable itself. The intention is to keep the
HW_CTRL flag functionality as is, since many older chipsets still use
this flag.

Introduce a new HW_CTRL_TRIGGER flag to switch the GDSC back and forth
between HW/SW modes dynamically at runtime. If HW_CTRL_TRIGGER flag is
present, register set_hwmode_dev callback to switch the GDSC mode which
can be invoked from consumer drivers using dev_pm_genpd_set_hwmode
function. Unlike HW_CTRL flag, HW_CTRL_TRIGGER won't move the GDSC to HW
control mode as part of GDSC enable itself, GDSC will be moved to HW
control mode only when consumer driver explicity calls
dev_pm_genpd_set_hwmode to switch to HW mode. Also add the
dev_pm_genpd_get_hwmode to allow the consumers to read the actual
HW/SW mode from hardware.

Can we add two new flags:
- HW_CTRL_TRIGGER
- DEFAULT_HW_TRIGGER

And then define HW_CTRL as HW_CTRL_TRIGGER | DEFAULT_HW_TRIGGER ?

This way older platforms will keep existing behaviour, but can
gradually migrate to the new callbacks?


Thanks Dmitry for your review,

The current usecases we have for GDSC's are either use the existing HW_CTRL flag to switch the GDSC to HW mode in gdsc_enable() and back to SW mode in gdsc_disable(). The second usecase is don't switch GDSC mode in gdsc_enable() & gdsc_disable() at all, and switch the GDSC mode only in new callbacks when consumers explicitly request it using dev_pm_genpd_set_hwmode(), this can be achieved using the new HW_CTRL_TRIGGER flag.

By defining HW_CTRL as HW_CTRL_TRIGGER | DEFAULT_HW_TRIGGER, it switches the GDSC mode both in enable & disable callbacks and in new callbacks as well. But we currently don't have any usecase that requires this behaviour.

Thanks,
Jagadeesh


Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
---
  drivers/clk/qcom/gdsc.c | 32 ++++++++++++++++++++++++++++++++
  drivers/clk/qcom/gdsc.h |  1 +
  2 files changed, 33 insertions(+)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 5358e28122ab..c763524cd5da 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -363,6 +363,34 @@ static int gdsc_disable(struct generic_pm_domain *domain)
         return 0;
  }

+static int gdsc_set_hwmode(struct generic_pm_domain *domain, struct device *dev, bool mode)
+{
+       struct gdsc *sc = domain_to_gdsc(domain);
+
+       if (sc->rsupply && !regulator_is_enabled(sc->rsupply)) {
+               pr_err("Cannot set mode while parent is disabled\n");
+               return -EIO;
+       }
+
+       return gdsc_hwctrl(sc, mode);
+}
+
+static bool gdsc_get_hwmode(struct generic_pm_domain *domain, struct device *dev)
+{
+       struct gdsc *sc = domain_to_gdsc(domain);
+       u32 val;
+       int ret;
+
+       ret = regmap_read(sc->regmap, sc->gdscr, &val);
+       if (ret)
+               return ret;
+
+       if (val & HW_CONTROL_MASK)
+               return true;
+
+       return false;
+}
+
  static int gdsc_init(struct gdsc *sc)
  {
         u32 mask, val;
@@ -451,6 +479,10 @@ static int gdsc_init(struct gdsc *sc)
                 sc->pd.power_off = gdsc_disable;
         if (!sc->pd.power_on)
                 sc->pd.power_on = gdsc_enable;
+       if (sc->flags & HW_CTRL_TRIGGER) {
+               sc->pd.set_hwmode_dev = gdsc_set_hwmode;
+               sc->pd.get_hwmode_dev = gdsc_get_hwmode;
+       }

         ret = pm_genpd_init(&sc->pd, NULL, !on);
         if (ret)
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 803512688336..1e2779b823d1 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -67,6 +67,7 @@ struct gdsc {
  #define ALWAYS_ON      BIT(6)
  #define RETAIN_FF_ENABLE       BIT(7)
  #define NO_RET_PERIPH  BIT(8)
+#define HW_CTRL_TRIGGER        BIT(9)
         struct reset_controller_dev     *rcdev;
         unsigned int                    *resets;
         unsigned int                    reset_count;

--
2.34.1







[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux