[PATCH 1/2] PM / clk: Add stubs for pm_clk_suspend/resume

[PATCH 1/2] PM / clk: Add stubs for pm_clk_suspend/resume

[Jon Hunter]

The functions pm_clk_suspend() and pm_clk_resume() cannot be used
directly within the main body of a function, because when CONFIG_PM_CLK
is not defined the functions are defined as NULL and this will lead to
a compilation error.

Add stubs functions for pm_clk_suspend() and pm_clk_resume() so that
these functions may be called directly.

Please note that these functions are currently assigned to function
pointers in the PM domain core and so to avoid have a valid function
pointer defined when CONFIG_PM_CLK is not defined, define
GENPD_FLAG_PM_CLK as 0 when CONFIG_PM_CLK is not defined to ensure
the stubs are not populated as valid function pointers.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 include/linux/pm_clock.h  | 10 ++++++++--
 include/linux/pm_domain.h |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
index 25266c600021..ffefbf2df1aa 100644
--- a/include/linux/pm_clock.h
+++ b/include/linux/pm_clock.h
@@ -72,8 +72,14 @@ static inline int pm_clk_add_clk(struct device *dev, struct clk *clk)
 static inline void pm_clk_remove(struct device *dev, const char *con_id)
 {
 }
-#define pm_clk_suspend	NULL
-#define pm_clk_resume	NULL
+static inline int pm_clk_suspend(struct device *dev)
+{
+	return -EINVAL;
+}
+static inline int pm_clk_resume(struct device *dev)
+{
+	return -EINVAL;
+}
 #endif
 
 #ifdef CONFIG_HAVE_CLK
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 49cd8890b873..1074c2073df8 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -17,7 +17,11 @@
 #include <linux/notifier.h>
 
 /* Defines used for the flags field in the struct generic_pm_domain */
+#ifdef CONFIG_PM_CLK
 #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
+#else
+#define GENPD_FLAG_PM_CLK	0
+#endif
 
 #define GENPD_MAX_NUM_STATES	8 /* Number of possible low power states */
 
-- 
2.1.4

[PATCH 2/2] PM / clk: Add support for obtaining clocks from device-tree

[Jon Hunter]

The PM clocks framework requires clients to pass either a con-id or a
valid clk pointer in order to add a clock to a device. Add a new
function pm_clk_add_clks_of() to allows device clocks to be retrieved
from device-tree and populated for a given device. Note that there
should be no need to add a stub function for this new function when
CONFIG_OF is not selected because the OF functions used already have
stubs functions.

In order to handle errors encountered when adding clocks from
device-tree, add a function pm_clk_remove_clk() to remove any clocks
(using a pointer to the clk structure) that have been added
successfully before the error occurred.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/base/power/clock_ops.c | 85 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_clock.h       |  9 +++++
 2 files changed, 94 insertions(+)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 272a52ebafc0..5aa7365699c1 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -138,6 +138,58 @@ int pm_clk_add_clk(struct device *dev, struct clk *clk)
 }
 
 /**
+ * pm_clk_add_clks_of - Start using device clock(s) for power management.
+ * @dev: Device whose clock(s) is going to be used for power management.
+ *
+ * Add a series of clocks described in the 'clocks' device-tree node for
+ * a device to the list of clocks used for the power management of @dev.
+ */
+int pm_clk_add_clks_of(struct device *dev)
+{
+	struct clk **clks;
+	unsigned int i, count;
+	int ret;
+
+	if (!dev || !dev->of_node)
+		return -EINVAL;
+
+	count = of_count_phandle_with_args(dev->of_node, "clocks",
+					   "#clock-cells");
+	if (count == 0)
+		return -ENODEV;
+
+	clks = kcalloc(count, sizeof(*clks), GFP_KERNEL);
+	if (!clks)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		clks[i] = of_clk_get(dev->of_node, i);
+		if (IS_ERR(clks[i])) {
+			ret = PTR_ERR(clks[i]);
+			goto error;
+		}
+
+		ret = pm_clk_add_clk(dev, clks[i]);
+		if (ret) {
+			clk_put(clks[i]);
+			goto error;
+		}
+	}
+
+	kfree(clks);
+
+	return 0;
+
+error:
+	while (i--)
+		pm_clk_remove_clk(dev, clks[i]);
+
+	kfree(clks);
+
+	return ret;
+}
+
+/**
  * __pm_clk_remove - Destroy PM clock entry.
  * @ce: PM clock entry to destroy.
  */
@@ -198,6 +250,39 @@ void pm_clk_remove(struct device *dev, const char *con_id)
 }
 
 /**
+ * pm_clk_remove_clk - Stop using a device clock for power management.
+ * @dev: Device whose clock should not be used for PM any more.
+ * @clk: Clock pointer
+ *
+ * Remove the clock pointed to by @clk from the list of clocks used for
+ * the power management of @dev.
+ */
+void pm_clk_remove_clk(struct device *dev, struct clk *clk)
+{
+	struct pm_subsys_data *psd = dev_to_psd(dev);
+	struct pm_clock_entry *ce;
+
+	if (!psd || !clk)
+		return;
+
+	spin_lock_irq(&psd->lock);
+
+	list_for_each_entry(ce, &psd->clock_list, node) {
+		if (clk == ce->clk)
+			goto remove;
+	}
+
+	spin_unlock_irq(&psd->lock);
+	return;
+
+ remove:
+	list_del(&ce->node);
+	spin_unlock_irq(&psd->lock);
+
+	__pm_clk_remove(ce);
+}
+
+/**
  * pm_clk_init - Initialize a device's list of power management clocks.
  * @dev: Device to initialize the list of PM clocks for.
  *
diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
index ffefbf2df1aa..6f609226bd85 100644
--- a/include/linux/pm_clock.h
+++ b/include/linux/pm_clock.h
@@ -42,7 +42,9 @@ extern int pm_clk_create(struct device *dev);
 extern void pm_clk_destroy(struct device *dev);
 extern int pm_clk_add(struct device *dev, const char *con_id);
 extern int pm_clk_add_clk(struct device *dev, struct clk *clk);
+extern int pm_clk_add_clks_of(struct device *dev);
 extern void pm_clk_remove(struct device *dev, const char *con_id);
+extern void pm_clk_remove_clk(struct device *dev, struct clk *clk);
 extern int pm_clk_suspend(struct device *dev);
 extern int pm_clk_resume(struct device *dev);
 #else
@@ -69,9 +71,16 @@ static inline int pm_clk_add_clk(struct device *dev, struct clk *clk)
 {
 	return -EINVAL;
 }
+static inline int pm_clk_add_clks_of(struct device *dev)
+{
+	return -EINVAL;
+}
 static inline void pm_clk_remove(struct device *dev, const char *con_id)
 {
 }
+static inline void pm_clk_remove_clk(struct device *dev, struct clk *clk)
+{
+}
 static inline int pm_clk_suspend(struct device *dev)
 {
 	return -EINVAL;
-- 
2.1.4

Re: [PATCH 2/2] PM / clk: Add support for obtaining clocks from device-tree

[Geert Uytterhoeven]

Hi Jon,

On Fri, Mar 4, 2016 at 4:33 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> The PM clocks framework requires clients to pass either a con-id or a
> valid clk pointer in order to add a clock to a device. Add a new
> function pm_clk_add_clks_of() to allows device clocks to be retrieved
> from device-tree and populated for a given device. Note that there
> should be no need to add a stub function for this new function when
> CONFIG_OF is not selected because the OF functions used already have
> stubs functions.
>
> In order to handle errors encountered when adding clocks from
> device-tree, add a function pm_clk_remove_clk() to remove any clocks
> (using a pointer to the clk structure) that have been added
> successfully before the error occurred.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/base/power/clock_ops.c | 85 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm_clock.h       |  9 +++++
>  2 files changed, 94 insertions(+)
>
> diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
> index 272a52ebafc0..5aa7365699c1 100644
> --- a/drivers/base/power/clock_ops.c
> +++ b/drivers/base/power/clock_ops.c
> @@ -138,6 +138,58 @@ int pm_clk_add_clk(struct device *dev, struct clk *clk)
>  }
>
>  /**
> + * pm_clk_add_clks_of - Start using device clock(s) for power management.
> + * @dev: Device whose clock(s) is going to be used for power management.
> + *
> + * Add a series of clocks described in the 'clocks' device-tree node for
> + * a device to the list of clocks used for the power management of @dev.

This adds all clocks referenced by the device node, while not all clocks may
be suitable for power management, and some of them may be under explicit
driver control.
Cfr. drivers/clk/shmobile/renesas-cpg-mssr.c:cpg_mssr_attach_dev(), which
looks for module clocks, or core clocks explicitly listed in core_pm_clks[].

What about adding an optional filter function to filter for suitable clocks?

> + */
> +int pm_clk_add_clks_of(struct device *dev)
> +{
> +       struct clk **clks;
> +       unsigned int i, count;
> +       int ret;
> +
> +       if (!dev || !dev->of_node)
> +               return -EINVAL;
> +
> +       count = of_count_phandle_with_args(dev->of_node, "clocks",
> +                                          "#clock-cells");
> +       if (count == 0)
> +               return -ENODEV;
> +
> +       clks = kcalloc(count, sizeof(*clks), GFP_KERNEL);
> +       if (!clks)
> +               return -ENOMEM;
> +

Don't you have to call pm_clk_create() here, to allocate a pm_subsys_data
structure that pm_clk_add_clk() can populate?

> +       for (i = 0; i < count; i++) {
> +               clks[i] = of_clk_get(dev->of_node, i);
> +               if (IS_ERR(clks[i])) {
> +                       ret = PTR_ERR(clks[i]);
> +                       goto error;
> +               }
> +
> +               ret = pm_clk_add_clk(dev, clks[i]);
> +               if (ret) {
> +                       clk_put(clks[i]);
> +                       goto error;
> +               }
> +       }
> +
> +       kfree(clks);
> +
> +       return 0;
> +
> +error:
> +       while (i--)
> +               pm_clk_remove_clk(dev, clks[i]);

Just pm_clk_destroy(dev) to destroy all at once?

> +
> +       kfree(clks);
> +
> +       return ret;
> +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] PM / clk: Add support for obtaining clocks from device-tree

[Jon Hunter]

On 07/03/16 08:48, Geert Uytterhoeven wrote:
> Hi Jon,
> 
> On Fri, Mar 4, 2016 at 4:33 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> The PM clocks framework requires clients to pass either a con-id or a
>> valid clk pointer in order to add a clock to a device. Add a new
>> function pm_clk_add_clks_of() to allows device clocks to be retrieved
>> from device-tree and populated for a given device. Note that there
>> should be no need to add a stub function for this new function when
>> CONFIG_OF is not selected because the OF functions used already have
>> stubs functions.
>>
>> In order to handle errors encountered when adding clocks from
>> device-tree, add a function pm_clk_remove_clk() to remove any clocks
>> (using a pointer to the clk structure) that have been added
>> successfully before the error occurred.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/base/power/clock_ops.c | 85 ++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/pm_clock.h       |  9 +++++
>>  2 files changed, 94 insertions(+)
>>
>> diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
>> index 272a52ebafc0..5aa7365699c1 100644
>> --- a/drivers/base/power/clock_ops.c
>> +++ b/drivers/base/power/clock_ops.c
>> @@ -138,6 +138,58 @@ int pm_clk_add_clk(struct device *dev, struct clk *clk)
>>  }
>>
>>  /**
>> + * pm_clk_add_clks_of - Start using device clock(s) for power management.
>> + * @dev: Device whose clock(s) is going to be used for power management.
>> + *
>> + * Add a series of clocks described in the 'clocks' device-tree node for
>> + * a device to the list of clocks used for the power management of @dev.
> 
> This adds all clocks referenced by the device node, while not all clocks may
> be suitable for power management, and some of them may be under explicit
> driver control.
> Cfr. drivers/clk/shmobile/renesas-cpg-mssr.c:cpg_mssr_attach_dev(), which
> looks for module clocks, or core clocks explicitly listed in core_pm_clks[].
> 
> What about adding an optional filter function to filter for suitable clocks?

Yes that sounds reasonable. Are you filtering on clock ID? Would it work
to allow the caller to pass a filter function which takes a struct *clk
as it's only argument?  Or do you prefer we call
of_parse_phandle_with_args(np, "clocks", "#clock-cells", i, &clkspec)
and return the clkspec info?

>> + */
>> +int pm_clk_add_clks_of(struct device *dev)
>> +{
>> +       struct clk **clks;
>> +       unsigned int i, count;
>> +       int ret;
>> +
>> +       if (!dev || !dev->of_node)
>> +               return -EINVAL;
>> +
>> +       count = of_count_phandle_with_args(dev->of_node, "clocks",
>> +                                          "#clock-cells");
>> +       if (count == 0)
>> +               return -ENODEV;
>> +
>> +       clks = kcalloc(count, sizeof(*clks), GFP_KERNEL);
>> +       if (!clks)
>> +               return -ENOMEM;
>> +
> 
> Don't you have to call pm_clk_create() here, to allocate a pm_subsys_data
> structure that pm_clk_add_clk() can populate?

This function will return an error when pm_clk_add_clk() is called if
the user has not called pm_clk_create() first. It seems more natural to
me that the user should call pm_clk_create() before calling
pm_clk_add_clks_of() which is the same convention used for adding clocks
by the other APIs to add clocks.

>> +       for (i = 0; i < count; i++) {
>> +               clks[i] = of_clk_get(dev->of_node, i);
>> +               if (IS_ERR(clks[i])) {
>> +                       ret = PTR_ERR(clks[i]);
>> +                       goto error;
>> +               }
>> +
>> +               ret = pm_clk_add_clk(dev, clks[i]);
>> +               if (ret) {
>> +                       clk_put(clks[i]);
>> +                       goto error;
>> +               }
>> +       }
>> +
>> +       kfree(clks);
>> +
>> +       return 0;
>> +
>> +error:
>> +       while (i--)
>> +               pm_clk_remove_clk(dev, clks[i]);
> 
> Just pm_clk_destroy(dev) to destroy all at once?

Right, however, this will remove all clocks associated with the device
and not just those we have added. It is probably very unlikely that
someone would call pm_clk_add_clks_of() and had previously called
pm_clk_add(), but it seems best to not make any assumptions and only
remove the clocks that were actually added. I guess that it would be
possible to check to see if there were any clocks already added when
this function is called and return an error in that case.

All of that said, I do agree that it may be simpler, if we do call
pm_clk_create() from within pm_clk_add_clks_of() and this would allow us
to pmc_clk_destroy() in the error path. So if this is preferred I could
do that, but I think that I would not allow clocks to be added if
clock_list is not empty when this function is called.

Cheers
Jon

Re: [PATCH 2/2] PM / clk: Add support for obtaining clocks from device-tree

[Geert Uytterhoeven]

Hi Jon,

On Mon, Mar 7, 2016 at 11:45 AM, Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On 07/03/16 08:48, Geert Uytterhoeven wrote:
>> On Fri, Mar 4, 2016 at 4:33 PM, Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>> The PM clocks framework requires clients to pass either a con-id or a
>>> valid clk pointer in order to add a clock to a device. Add a new
>>> function pm_clk_add_clks_of() to allows device clocks to be retrieved
>>> from device-tree and populated for a given device. Note that there
>>> should be no need to add a stub function for this new function when
>>> CONFIG_OF is not selected because the OF functions used already have
>>> stubs functions.
>>>
>>> In order to handle errors encountered when adding clocks from
>>> device-tree, add a function pm_clk_remove_clk() to remove any clocks
>>> (using a pointer to the clk structure) that have been added
>>> successfully before the error occurred.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  drivers/base/power/clock_ops.c | 85 ++++++++++++++++++++++++++++++++++++++++++
>>>  include/linux/pm_clock.h       |  9 +++++
>>>  2 files changed, 94 insertions(+)
>>>
>>> diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
>>> index 272a52ebafc0..5aa7365699c1 100644
>>> --- a/drivers/base/power/clock_ops.c
>>> +++ b/drivers/base/power/clock_ops.c
>>> @@ -138,6 +138,58 @@ int pm_clk_add_clk(struct device *dev, struct clk *clk)
>>>  }
>>>
>>>  /**
>>> + * pm_clk_add_clks_of - Start using device clock(s) for power management.
>>> + * @dev: Device whose clock(s) is going to be used for power management.
>>> + *
>>> + * Add a series of clocks described in the 'clocks' device-tree node for
>>> + * a device to the list of clocks used for the power management of @dev.
>>
>> This adds all clocks referenced by the device node, while not all clocks may
>> be suitable for power management, and some of them may be under explicit
>> driver control.
>> Cfr. drivers/clk/shmobile/renesas-cpg-mssr.c:cpg_mssr_attach_dev(), which
>> looks for module clocks, or core clocks explicitly listed in core_pm_clks[].
>>
>> What about adding an optional filter function to filter for suitable clocks?
>
> Yes that sounds reasonable. Are you filtering on clock ID? Would it work
> to allow the caller to pass a filter function which takes a struct *clk
> as it's only argument?  Or do you prefer we call
> of_parse_phandle_with_args(np, "clocks", "#clock-cells", i, &clkspec)
> and return the clkspec info?

cpg_mssr_attach_dev() filters on clkspec.

As pm_clk_add_clks_of() has "_of" in the name (BTW, should it be called
of_pm_clk_add_clks()), I think passing the clkspec makes more sense than
passing a struct clk *.

>>> +int pm_clk_add_clks_of(struct device *dev)
>>> +{
>>> +       struct clk **clks;
>>> +       unsigned int i, count;
>>> +       int ret;
>>> +
>>> +       if (!dev || !dev->of_node)
>>> +               return -EINVAL;
>>> +
>>> +       count = of_count_phandle_with_args(dev->of_node, "clocks",
>>> +                                          "#clock-cells");
>>> +       if (count == 0)
>>> +               return -ENODEV;
>>> +
>>> +       clks = kcalloc(count, sizeof(*clks), GFP_KERNEL);
>>> +       if (!clks)
>>> +               return -ENOMEM;
>>> +
>>
>> Don't you have to call pm_clk_create() here, to allocate a pm_subsys_data
>> structure that pm_clk_add_clk() can populate?
>
> This function will return an error when pm_clk_add_clk() is called if
> the user has not called pm_clk_create() first. It seems more natural to
> me that the user should call pm_clk_create() before calling
> pm_clk_add_clks_of() which is the same convention used for adding clocks
> by the other APIs to add clocks.

OK. That way the caller can add more clocks if needed. I missed that case.

>>> +       for (i = 0; i < count; i++) {
>>> +               clks[i] = of_clk_get(dev->of_node, i);
>>> +               if (IS_ERR(clks[i])) {
>>> +                       ret = PTR_ERR(clks[i]);
>>> +                       goto error;
>>> +               }
>>> +
>>> +               ret = pm_clk_add_clk(dev, clks[i]);
>>> +               if (ret) {
>>> +                       clk_put(clks[i]);
>>> +                       goto error;
>>> +               }
>>> +       }
>>> +
>>> +       kfree(clks);
>>> +
>>> +       return 0;
>>> +
>>> +error:
>>> +       while (i--)
>>> +               pm_clk_remove_clk(dev, clks[i]);
>>
>> Just pm_clk_destroy(dev) to destroy all at once?
>
> Right, however, this will remove all clocks associated with the device
> and not just those we have added. It is probably very unlikely that
> someone would call pm_clk_add_clks_of() and had previously called
> pm_clk_add(), but it seems best to not make any assumptions and only
> remove the clocks that were actually added. I guess that it would be
> possible to check to see if there were any clocks already added when
> this function is called and return an error in that case.
>
> All of that said, I do agree that it may be simpler, if we do call
> pm_clk_create() from within pm_clk_add_clks_of() and this would allow us
> to pmc_clk_destroy() in the error path. So if this is preferred I could
> do that, but I think that I would not allow clocks to be added if
> clock_list is not empty when this function is called.

Given the above, your code is fine.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] PM / clk: Add stubs for pm_clk_suspend/resume

[Jon Hunter]

On 04/03/16 15:33, Jon Hunter wrote:
> The functions pm_clk_suspend() and pm_clk_resume() cannot be used
> directly within the main body of a function, because when CONFIG_PM_CLK
> is not defined the functions are defined as NULL and this will lead to
> a compilation error.
> 
> Add stubs functions for pm_clk_suspend() and pm_clk_resume() so that
> these functions may be called directly.
> 
> Please note that these functions are currently assigned to function
> pointers in the PM domain core and so to avoid have a valid function
> pointer defined when CONFIG_PM_CLK is not defined, define
> GENPD_FLAG_PM_CLK as 0 when CONFIG_PM_CLK is not defined to ensure
> the stubs are not populated as valid function pointers.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  include/linux/pm_clock.h  | 10 ++++++++--
>  include/linux/pm_domain.h |  4 ++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
> index 25266c600021..ffefbf2df1aa 100644
> --- a/include/linux/pm_clock.h
> +++ b/include/linux/pm_clock.h
> @@ -72,8 +72,14 @@ static inline int pm_clk_add_clk(struct device *dev, struct clk *clk)
>  static inline void pm_clk_remove(struct device *dev, const char *con_id)
>  {
>  }
> -#define pm_clk_suspend	NULL
> -#define pm_clk_resume	NULL
> +static inline int pm_clk_suspend(struct device *dev)
> +{
> +	return -EINVAL;
> +}
> +static inline int pm_clk_resume(struct device *dev)
> +{
> +	return -EINVAL;
> +}
>  #endif
>  
>  #ifdef CONFIG_HAVE_CLK
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 49cd8890b873..1074c2073df8 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -17,7 +17,11 @@
>  #include <linux/notifier.h>
>  
>  /* Defines used for the flags field in the struct generic_pm_domain */
> +#ifdef CONFIG_PM_CLK
>  #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
> +#else
> +#define GENPD_FLAG_PM_CLK	0
> +#endif
>  
>  #define GENPD_MAX_NUM_STATES	8 /* Number of possible low power states */

Thinking about this some more, the alternative is for drivers using
PM_CLK to select it. Would that be appropriate? If so, we would not need
these stubs.

Cheers
Jon

Re: [PATCH 1/2] PM / clk: Add stubs for pm_clk_suspend/resume

[Rafael J. Wysocki]

On Tuesday, March 08, 2016 05:18:06 PM Jon Hunter wrote:
> 
> On 04/03/16 15:33, Jon Hunter wrote:
> > The functions pm_clk_suspend() and pm_clk_resume() cannot be used
> > directly within the main body of a function, because when CONFIG_PM_CLK
> > is not defined the functions are defined as NULL and this will lead to
> > a compilation error.
> > 
> > Add stubs functions for pm_clk_suspend() and pm_clk_resume() so that
> > these functions may be called directly.
> > 
> > Please note that these functions are currently assigned to function
> > pointers in the PM domain core and so to avoid have a valid function
> > pointer defined when CONFIG_PM_CLK is not defined, define
> > GENPD_FLAG_PM_CLK as 0 when CONFIG_PM_CLK is not defined to ensure
> > the stubs are not populated as valid function pointers.
> > 
> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> > ---
> >  include/linux/pm_clock.h  | 10 ++++++++--
> >  include/linux/pm_domain.h |  4 ++++
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
> > index 25266c600021..ffefbf2df1aa 100644
> > --- a/include/linux/pm_clock.h
> > +++ b/include/linux/pm_clock.h
> > @@ -72,8 +72,14 @@ static inline int pm_clk_add_clk(struct device *dev, struct clk *clk)
> >  static inline void pm_clk_remove(struct device *dev, const char *con_id)
> >  {
> >  }
> > -#define pm_clk_suspend	NULL
> > -#define pm_clk_resume	NULL
> > +static inline int pm_clk_suspend(struct device *dev)
> > +{
> > +	return -EINVAL;
> > +}
> > +static inline int pm_clk_resume(struct device *dev)
> > +{
> > +	return -EINVAL;
> > +}
> >  #endif
> >  
> >  #ifdef CONFIG_HAVE_CLK
> > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> > index 49cd8890b873..1074c2073df8 100644
> > --- a/include/linux/pm_domain.h
> > +++ b/include/linux/pm_domain.h
> > @@ -17,7 +17,11 @@
> >  #include <linux/notifier.h>
> >  
> >  /* Defines used for the flags field in the struct generic_pm_domain */
> > +#ifdef CONFIG_PM_CLK
> >  #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
> > +#else
> > +#define GENPD_FLAG_PM_CLK	0
> > +#endif
> >  
> >  #define GENPD_MAX_NUM_STATES	8 /* Number of possible low power states */
> 
> Thinking about this some more, the alternative is for drivers using
> PM_CLK to select it. Would that be appropriate? If so, we would not need
> these stubs.

Well, I don't see why it might not be appropriate.

Thanks,
Rafael

Re: [PATCH 1/2] PM / clk: Add stubs for pm_clk_suspend/resume

[Jon Hunter]

On 08/03/16 22:58, Rafael J. Wysocki wrote:
> On Tuesday, March 08, 2016 05:18:06 PM Jon Hunter wrote:
>>
>> On 04/03/16 15:33, Jon Hunter wrote:
>>> The functions pm_clk_suspend() and pm_clk_resume() cannot be used
>>> directly within the main body of a function, because when CONFIG_PM_CLK
>>> is not defined the functions are defined as NULL and this will lead to
>>> a compilation error.
>>>
>>> Add stubs functions for pm_clk_suspend() and pm_clk_resume() so that
>>> these functions may be called directly.
>>>
>>> Please note that these functions are currently assigned to function
>>> pointers in the PM domain core and so to avoid have a valid function
>>> pointer defined when CONFIG_PM_CLK is not defined, define
>>> GENPD_FLAG_PM_CLK as 0 when CONFIG_PM_CLK is not defined to ensure
>>> the stubs are not populated as valid function pointers.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>>  include/linux/pm_clock.h  | 10 ++++++++--
>>>  include/linux/pm_domain.h |  4 ++++
>>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
>>> index 25266c600021..ffefbf2df1aa 100644
>>> --- a/include/linux/pm_clock.h
>>> +++ b/include/linux/pm_clock.h
>>> @@ -72,8 +72,14 @@ static inline int pm_clk_add_clk(struct device *dev, struct clk *clk)
>>>  static inline void pm_clk_remove(struct device *dev, const char *con_id)
>>>  {
>>>  }
>>> -#define pm_clk_suspend	NULL
>>> -#define pm_clk_resume	NULL
>>> +static inline int pm_clk_suspend(struct device *dev)
>>> +{
>>> +	return -EINVAL;
>>> +}
>>> +static inline int pm_clk_resume(struct device *dev)
>>> +{
>>> +	return -EINVAL;
>>> +}
>>>  #endif
>>>  
>>>  #ifdef CONFIG_HAVE_CLK
>>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>>> index 49cd8890b873..1074c2073df8 100644
>>> --- a/include/linux/pm_domain.h
>>> +++ b/include/linux/pm_domain.h
>>> @@ -17,7 +17,11 @@
>>>  #include <linux/notifier.h>
>>>  
>>>  /* Defines used for the flags field in the struct generic_pm_domain */
>>> +#ifdef CONFIG_PM_CLK
>>>  #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
>>> +#else
>>> +#define GENPD_FLAG_PM_CLK	0
>>> +#endif
>>>  
>>>  #define GENPD_MAX_NUM_STATES	8 /* Number of possible low power states */
>>
>> Thinking about this some more, the alternative is for drivers using
>> PM_CLK to select it. Would that be appropriate? If so, we would not need
>> these stubs.
> 
> Well, I don't see why it might not be appropriate.

Ok, thanks. I will drop this patch for now and update the 2nd patch of
this series to address the comments from Geert.

Cheers
Jon