[PATCH 0/2] PM: domains: Add control for switching back and forth to HW control

[PATCH 0/2] PM: domains: Add control for switching back and forth to HW control

[Abel Vesa]

This is just a resend of [1]. This resend just adds the back story behind
the need for such a generic API as a cover letter. Also added my SoB tag
to Ulf's patch.

Some of the newer Qualcomm platforms support handing of the control of
some of the GDSCs (implemented as power domains in Linux) to some device
firmware. The idea behind such approach is that the firmware knows best
when such a power domain can be powered off or not and leads most of the
time to better power consumption.

At this point, if such GDSC provides HW control support, the current
implementation is switching to HW control right after the GDSC gets
powered on and it is left in HW control mode until right before the
request for power off is done. This needs to remain as is for now, as we
do not know for sure what each firmware expects from its related GDSCs.
For example, the venus driver expects the codec GDSCs to remain always
in HW control mode, otherwise the firmware would crush.

But in some cases, the consumer driver needs to switch back and forth.
And the explanation for such case is when a driver needs to interract
with the device (e.g. reading status bits) and the firmware doesn't
guarantee the GDSC will be enabled when in HW mode. Therefore, the
consumer would need to switch back to SW mode, do its thing, and then
switch again back to HW mode.

This is where the patch from Ulf comes in. It allows consumers that
actually need to control the HW/SW mode to do so.

The GDSC patch just implemets the set_hwmode op and sets it for each
GDSC that provides HW control mode.

[1] https://lore.kernel.org/all/20230627104033.3345659-1-abel.vesa@linaro.org/

Abel Vesa (1):
  clk: qcom: gdsc: Add support for set_hwmode_dev

Ulf Hansson (1):
  PM: domains: Allow devices attached to genpd to be managed by HW

 drivers/base/power/domain.c | 66 +++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.c     | 22 +++++++++++++
 include/linux/pm_domain.h   | 15 +++++++++
 3 files changed, 103 insertions(+)

-- 
2.34.1

[PATCH 1/2] PM: domains: Allow devices attached to genpd to be managed by HW

[Abel Vesa]

From: Ulf Hansson <ulf.hansson@linaro.org>

Some power-domains may be capable of relying on the HW to control the power
for a device that's hooked up to it. Typically, for these kinds of
configurations the device doesn't really need to be attached to a PM domain
(genpd), from Linux point of view. However, in some cases the behaviour of
the power-domain and its device can be changed in runtime.

To allow a consumer driver to change the behaviour of the PM domain for its
device, let's provide a new function, dev_pm_genpd_set_hwmode(). Moreover,
let's add a corresponding optional genpd callback, ->set_hwmode_dev(),
which the genpd provider should implement if it can support switching
between HW controlled mode and SW controlled mode.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/base/power/domain.c | 66 +++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   | 15 +++++++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 5cb2023581d4..23286853d1d0 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -541,6 +541,72 @@ void dev_pm_genpd_synced_poweroff(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_genpd_synced_poweroff);
 
+/**
+ * dev_pm_genpd_set_hwmode - Set the HW mode for the device and its PM domain.
+ *
+ * @dev: Device for which the HW-mode should be changed.
+ * @enable: Value to set or unset the HW-mode.
+ *
+ * Some PM domains can rely on HW signals to control the power for a device. To
+ * allow a consumer driver to switch the behaviour for its device in runtime,
+ * which may be beneficial from a latency or energy point of view, this function
+ * may be called.
+ *
+ * It is assumed that the users guarantee that the genpd wouldn't be detached
+ * while this routine is getting called.
+ *
+ * Returns 0 on success and negative error values on failures.
+ */
+int dev_pm_genpd_set_hwmode(struct device *dev, bool enable)
+{
+	struct generic_pm_domain *genpd;
+	int ret = 0;
+
+	genpd = dev_to_genpd_safe(dev);
+	if (!genpd)
+		return -ENODEV;
+
+	if (!genpd->set_hwmode_dev)
+		return -EOPNOTSUPP;
+
+	genpd_lock(genpd);
+
+	if (dev_gpd_data(dev)->hw_mode == enable)
+		goto out;
+
+	ret = genpd->set_hwmode_dev(genpd, dev, enable);
+	if (!ret)
+		dev_gpd_data(dev)->hw_mode = enable;
+
+out:
+	genpd_unlock(genpd);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_set_hwmode);
+
+/**
+ * dev_pm_genpd_get_hwmode - Get the HW mode setting for the device.
+ *
+ * @dev: Device for which the current HW-mode setting should be fetched.
+ *
+ * This helper function allows consumer drivers to fetch the current HW mode
+ * setting of its the device.
+ *
+ * It is assumed that the users guarantee that the genpd wouldn't be detached
+ * while this routine is getting called.
+ */
+bool dev_pm_genpd_get_hwmode(struct device *dev)
+{
+	struct generic_pm_domain *genpd;
+
+	genpd = dev_to_genpd_safe(dev);
+	if (!genpd)
+		return false;
+
+	return dev_gpd_data(dev)->hw_mode;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_get_hwmode);
+
 static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 {
 	unsigned int state_idx = genpd->state_idx;
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f776fb93eaa0..8f60c36851d5 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -146,6 +146,8 @@ struct generic_pm_domain {
 	int (*set_performance_state)(struct generic_pm_domain *genpd,
 				     unsigned int state);
 	struct gpd_dev_ops dev_ops;
+	int (*set_hwmode_dev)(struct generic_pm_domain *domain,
+			      struct device *dev, bool enable);
 	int (*attach_dev)(struct generic_pm_domain *domain,
 			  struct device *dev);
 	void (*detach_dev)(struct generic_pm_domain *domain,
@@ -208,6 +210,7 @@ struct generic_pm_domain_data {
 	unsigned int performance_state;
 	unsigned int default_pstate;
 	unsigned int rpm_pstate;
+	bool hw_mode;
 	void *data;
 };
 
@@ -237,6 +240,8 @@ int dev_pm_genpd_remove_notifier(struct device *dev);
 void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next);
 ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev);
 void dev_pm_genpd_synced_poweroff(struct device *dev);
+int dev_pm_genpd_set_hwmode(struct device *dev, bool enable);
+bool dev_pm_genpd_get_hwmode(struct device *dev);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -305,6 +310,16 @@ static inline ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev)
 static inline void dev_pm_genpd_synced_poweroff(struct device *dev)
 { }
 
+static inline int dev_pm_genpd_set_hwmode(struct device *dev, bool enable)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline bool dev_pm_genpd_get_hwmode(struct device *dev)
+{
+	return false;
+}
+
 #define simple_qos_governor		(*(struct dev_power_governor *)(NULL))
 #define pm_domain_always_on_gov		(*(struct dev_power_governor *)(NULL))
 #endif
-- 
2.34.1

[PATCH 2/2] clk: qcom: gdsc: Add support for set_hwmode_dev

[Abel Vesa]

Implement the GDSC specific genpd set_hwmode_dev callback in order to
switch the HW control on or off. For any GDSC that supports HW control
set this callback in order to allow its consumers to control it.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 5358e28122ab..9a04bf2e4379 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -314,6 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	return 0;
 }
 
+static int gdsc_set_hwmode_dev(struct generic_pm_domain *domain,
+			       struct device *dev, bool enable)
+{
+	int ret = gdsc_hwctrl(domain_to_gdsc(domain), enable);
+
+	if (ret)
+		goto out;
+
+	/*
+	 * Wait for the GDSC to go through a power down and
+	 * up cycle.  In case there is a status polling going on
+	 * before the power cycle is completed it might read an
+	 * wrong status value.
+	 */
+	udelay(1);
+
+out:
+	return ret;
+}
+
 static int gdsc_disable(struct generic_pm_domain *domain)
 {
 	struct gdsc *sc = domain_to_gdsc(domain);
@@ -451,6 +471,8 @@ 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)
+		sc->pd.set_hwmode_dev = gdsc_set_hwmode_dev;
 
 	ret = pm_genpd_init(&sc->pd, NULL, !on);
 	if (ret)
-- 
2.34.1

Re: [PATCH 0/2] PM: domains: Add control for switching back and forth to HW control

[Rafael J. Wysocki]

On Wed, Jun 28, 2023 at 12:57 PM Abel Vesa <abel.vesa@linaro.org> wrote:
>
> This is just a resend of [1]. This resend just adds the back story behind
> the need for such a generic API as a cover letter. Also added my SoB tag
> to Ulf's patch.
>
> Some of the newer Qualcomm platforms support handing of the control of
> some of the GDSCs (implemented as power domains in Linux) to some device
> firmware. The idea behind such approach is that the firmware knows best
> when such a power domain can be powered off or not and leads most of the
> time to better power consumption.
>
> At this point, if such GDSC provides HW control support, the current
> implementation is switching to HW control right after the GDSC gets
> powered on and it is left in HW control mode until right before the
> request for power off is done. This needs to remain as is for now, as we
> do not know for sure what each firmware expects from its related GDSCs.
> For example, the venus driver expects the codec GDSCs to remain always
> in HW control mode, otherwise the firmware would crush.
>
> But in some cases, the consumer driver needs to switch back and forth.
> And the explanation for such case is when a driver needs to interract
> with the device (e.g. reading status bits) and the firmware doesn't
> guarantee the GDSC will be enabled when in HW mode. Therefore, the
> consumer would need to switch back to SW mode, do its thing, and then
> switch again back to HW mode.
>
> This is where the patch from Ulf comes in. It allows consumers that
> actually need to control the HW/SW mode to do so.
>
> The GDSC patch just implemets the set_hwmode op and sets it for each
> GDSC that provides HW control mode.
>
> [1] https://lore.kernel.org/all/20230627104033.3345659-1-abel.vesa@linaro.org/
>
> Abel Vesa (1):
>   clk: qcom: gdsc: Add support for set_hwmode_dev
>
> Ulf Hansson (1):
>   PM: domains: Allow devices attached to genpd to be managed by HW
>
>  drivers/base/power/domain.c | 66 +++++++++++++++++++++++++++++++++++++
>  drivers/clk/qcom/gdsc.c     | 22 +++++++++++++
>  include/linux/pm_domain.h   | 15 +++++++++
>  3 files changed, 103 insertions(+)
>
> --

I can queue up this series if I get an ACK for the second patch.

Re: [PATCH 2/2] clk: qcom: gdsc: Add support for set_hwmode_dev

[Konrad Dybcio]

On 28.06.2023 12:56, Abel Vesa wrote:
> Implement the GDSC specific genpd set_hwmode_dev callback in order to
> switch the HW control on or off. For any GDSC that supports HW control
> set this callback in order to allow its consumers to control it.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
This still does nothing to prevent the HW_CTRL state being changed in
init, enable and disable functions.

Konrad
>  drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 5358e28122ab..9a04bf2e4379 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -314,6 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
>  	return 0;
>  }
>  
> +static int gdsc_set_hwmode_dev(struct generic_pm_domain *domain,
> +			       struct device *dev, bool enable)
> +{
> +	int ret = gdsc_hwctrl(domain_to_gdsc(domain), enable);
> +
> +	if (ret)
> +		goto out;
> +
> +	/*
> +	 * Wait for the GDSC to go through a power down and
> +	 * up cycle.  In case there is a status polling going on
> +	 * before the power cycle is completed it might read an
> +	 * wrong status value.
> +	 */
> +	udelay(1);
> +
> +out:
> +	return ret;
> +}
> +
>  static int gdsc_disable(struct generic_pm_domain *domain)
>  {
>  	struct gdsc *sc = domain_to_gdsc(domain);
> @@ -451,6 +471,8 @@ 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)
> +		sc->pd.set_hwmode_dev = gdsc_set_hwmode_dev;
>  
>  	ret = pm_genpd_init(&sc->pd, NULL, !on);
>  	if (ret)

Re: [PATCH 0/2] PM: domains: Add control for switching back and forth to HW control

[Ulf Hansson]

On Wed, 28 Jun 2023 at 19:15, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Jun 28, 2023 at 12:57 PM Abel Vesa <abel.vesa@linaro.org> wrote:
> >
> > This is just a resend of [1]. This resend just adds the back story behind
> > the need for such a generic API as a cover letter. Also added my SoB tag
> > to Ulf's patch.
> >
> > Some of the newer Qualcomm platforms support handing of the control of
> > some of the GDSCs (implemented as power domains in Linux) to some device
> > firmware. The idea behind such approach is that the firmware knows best
> > when such a power domain can be powered off or not and leads most of the
> > time to better power consumption.
> >
> > At this point, if such GDSC provides HW control support, the current
> > implementation is switching to HW control right after the GDSC gets
> > powered on and it is left in HW control mode until right before the
> > request for power off is done. This needs to remain as is for now, as we
> > do not know for sure what each firmware expects from its related GDSCs.
> > For example, the venus driver expects the codec GDSCs to remain always
> > in HW control mode, otherwise the firmware would crush.
> >
> > But in some cases, the consumer driver needs to switch back and forth.
> > And the explanation for such case is when a driver needs to interract
> > with the device (e.g. reading status bits) and the firmware doesn't
> > guarantee the GDSC will be enabled when in HW mode. Therefore, the
> > consumer would need to switch back to SW mode, do its thing, and then
> > switch again back to HW mode.
> >
> > This is where the patch from Ulf comes in. It allows consumers that
> > actually need to control the HW/SW mode to do so.
> >
> > The GDSC patch just implemets the set_hwmode op and sets it for each
> > GDSC that provides HW control mode.
> >
> > [1] https://lore.kernel.org/all/20230627104033.3345659-1-abel.vesa@linaro.org/
> >
> > Abel Vesa (1):
> >   clk: qcom: gdsc: Add support for set_hwmode_dev
> >
> > Ulf Hansson (1):
> >   PM: domains: Allow devices attached to genpd to be managed by HW
> >
> >  drivers/base/power/domain.c | 66 +++++++++++++++++++++++++++++++++++++
> >  drivers/clk/qcom/gdsc.c     | 22 +++++++++++++
> >  include/linux/pm_domain.h   | 15 +++++++++
> >  3 files changed, 103 insertions(+)
> >
> > --
>
> I can queue up this series if I get an ACK for the second patch.

Thanks, but please hold on until you get additional confirmation from me.

I would like the consumer APIs that are being added in patch1 to have
a user. At least I want to see a plan for upstreaming one user of it,
before I think we should move forward. FYI, there are discussions
around this offlist too.

Kind regards
Uffe

Re: [PATCH 2/2] clk: qcom: gdsc: Add support for set_hwmode_dev

[Taniya Das]

Hi Abel,

Thanks for the patch.

On 6/28/2023 10:48 PM, Konrad Dybcio wrote:
> On 28.06.2023 12:56, Abel Vesa wrote:
>> Implement the GDSC specific genpd set_hwmode_dev callback in order to
>> switch the HW control on or off. For any GDSC that supports HW control
>> set this callback in order to allow its consumers to control it.
>>
>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>> ---
> This still does nothing to prevent the HW_CTRL state being changed in
> init, enable and disable functions.
> 
> Konrad
>>   drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>> index 5358e28122ab..9a04bf2e4379 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -314,6 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
>>   	return 0;
>>   }
>>   
>> +static int gdsc_set_hwmode_dev(struct generic_pm_domain *domain,
>> +			       struct device *dev, bool enable)
>> +{
>> +	int ret = gdsc_hwctrl(domain_to_gdsc(domain), enable);
>> +
>> +	if (ret)
>> +		goto out;
>> +
>> +	/*
>> +	 * Wait for the GDSC to go through a power down and
>> +	 * up cycle.  In case there is a status polling going on
>> +	 * before the power cycle is completed it might read an
>> +	 * wrong status value.
>> +	 */
>> +	udelay(1);
>> +
>> +out:
>> +	return ret;
>> +}
>> +
>>   static int gdsc_disable(struct generic_pm_domain *domain)
>>   {
>>   	struct gdsc *sc = domain_to_gdsc(domain);
>> @@ -451,6 +471,8 @@ 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)
>> +		sc->pd.set_hwmode_dev = gdsc_set_hwmode_dev;
>>   
We do not want to move to SW mode without consumers wanting to move to 
this mode.

We want a new flag for the consumers wanting to move to this mode. The 
mode in which the GDSC would be enabled would be in SW mode only.
+	if (sc->flags & HW_CTRL_TRIGGER) {
+		sc->pd.set_hwmode_dev = gdsc_set_mode;
+	}
+

>>   	ret = pm_genpd_init(&sc->pd, NULL, !on);
>>   	if (ret)

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 2/2] clk: qcom: gdsc: Add support for set_hwmode_dev

[Taniya Das]

On 6/28/2023 4:26 PM, Abel Vesa wrote:
> Implement the GDSC specific genpd set_hwmode_dev callback in order to
> switch the HW control on or off. For any GDSC that supports HW control
> set this callback in order to allow its consumers to control it.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>   drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 5358e28122ab..9a04bf2e4379 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -314,6 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
>   	return 0;
>   }
>   
> +static int gdsc_set_hwmode_dev(struct generic_pm_domain *domain,
> +			       struct device *dev, bool enable)
> +{
> +	int ret = gdsc_hwctrl(domain_to_gdsc(domain), enable);
> +
> +	if (ret)
> +		goto out;
> +
> +	/*
> +	 * Wait for the GDSC to go through a power down and
> +	 * up cycle.  In case there is a status polling going on
> +	 * before the power cycle is completed it might read an
> +	 * wrong status value.
> +	 */
> +	udelay(1);
> +
> +out:
> +	return ret;
> +}
> +
>   static int gdsc_disable(struct generic_pm_domain *domain)
>   {
>   	struct gdsc *sc = domain_to_gdsc(domain);
> @@ -451,6 +471,8 @@ 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)
> +		sc->pd.set_hwmode_dev = gdsc_set_hwmode_dev;
>   

Forgot to add the get_mode.

+	if (sc->flags & HW_CTRL_TRIGGER) {
+		sc->pd.set_hwmode_dev = gdsc_set_mode;
+		sc->pd.get_hwmode_dev = gdsc_get_mode;
+	}
+
>   	ret = pm_genpd_init(&sc->pd, NULL, !on);
>   	if (ret)

-- 
Thanks & Regards,
Taniya Das.

Re: [PATCH 2/2] clk: qcom: gdsc: Add support for set_hwmode_dev

[Abel Vesa]

On 23-07-10 09:40:14, Taniya Das wrote:
> Hi Abel,
> 
> Thanks for the patch.
> 
> On 6/28/2023 10:48 PM, Konrad Dybcio wrote:
> > On 28.06.2023 12:56, Abel Vesa wrote:
> > > Implement the GDSC specific genpd set_hwmode_dev callback in order to
> > > switch the HW control on or off. For any GDSC that supports HW control
> > > set this callback in order to allow its consumers to control it.
> > > 
> > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > > ---
> > This still does nothing to prevent the HW_CTRL state being changed in
> > init, enable and disable functions.
> > 
> > Konrad
> > >   drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++++
> > >   1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> > > index 5358e28122ab..9a04bf2e4379 100644
> > > --- a/drivers/clk/qcom/gdsc.c
> > > +++ b/drivers/clk/qcom/gdsc.c
> > > @@ -314,6 +314,26 @@ static int gdsc_enable(struct generic_pm_domain *domain)
> > >   	return 0;
> > >   }
> > > +static int gdsc_set_hwmode_dev(struct generic_pm_domain *domain,
> > > +			       struct device *dev, bool enable)
> > > +{
> > > +	int ret = gdsc_hwctrl(domain_to_gdsc(domain), enable);
> > > +
> > > +	if (ret)
> > > +		goto out;
> > > +
> > > +	/*
> > > +	 * Wait for the GDSC to go through a power down and
> > > +	 * up cycle.  In case there is a status polling going on
> > > +	 * before the power cycle is completed it might read an
> > > +	 * wrong status value.
> > > +	 */
> > > +	udelay(1);
> > > +
> > > +out:
> > > +	return ret;
> > > +}
> > > +
> > >   static int gdsc_disable(struct generic_pm_domain *domain)
> > >   {
> > >   	struct gdsc *sc = domain_to_gdsc(domain);
> > > @@ -451,6 +471,8 @@ 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)
> > > +		sc->pd.set_hwmode_dev = gdsc_set_hwmode_dev;
> We do not want to move to SW mode without consumers wanting to move to this
> mode.
> 
> We want a new flag for the consumers wanting to move to this mode. The mode
> in which the GDSC would be enabled would be in SW mode only.
> +	if (sc->flags & HW_CTRL_TRIGGER) {
> +		sc->pd.set_hwmode_dev = gdsc_set_mode;
> +	}
> +

OK, maybe I'm missing something here.

Do you suggest we have GDSCs that, even though they support HW ctrl,
should not be controllable by the consumer?

Why isn't dev_pm_genpd_set_hwmode good enough? If a consumer doesn't
want to control it then the consumer can just skip calling the mentioned
function.

Or maybe you want this all hidden into the genpd provider?

> 
> > >   	ret = pm_genpd_init(&sc->pd, NULL, !on);
> > >   	if (ret)
> 
> -- 
> Thanks & Regards,
> Taniya Das.