[PATCH 1/3] powercap/drivers/dtpm: Fix a double shift bug

[PATCH 1/3] powercap/drivers/dtpm: Fix a double shift bug

[Dan Carpenter]

The DTPM_POWER_LIMIT_FLAG is used for test_bit() etc which take a bit
number so it should be bit 0.  But currently it's set to BIT(0) then
that is double shifted equivalent to BIT(BIT(0)).  This doesn't cause a
run time problem because it's done consistently.

Fixes: a20d0ef97abf ("powercap/drivers/dtpm: Add API for dynamic thermal power management")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/powercap/dtpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
index 0abcc439d728..d49df0569cd4 100644
--- a/drivers/powercap/dtpm.c
+++ b/drivers/powercap/dtpm.c
@@ -24,7 +24,7 @@
 #include <linux/slab.h>
 #include <linux/mutex.h>
 
-#define DTPM_POWER_LIMIT_FLAG BIT(0)
+#define DTPM_POWER_LIMIT_FLAG 0
 
 static const char *constraint_name[] = {
 	"Instantaneous",
-- 
2.29.2

[PATCH 2/3] powercap/drivers/dtpm: Fix some missing unlock bugs

[Dan Carpenter]

We need to unlock on these paths before returning.

Fixes: a20d0ef97abf ("powercap/drivers/dtpm: Add API for dynamic thermal power management")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/powercap/dtpm.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
index d49df0569cd4..470a1182b868 100644
--- a/drivers/powercap/dtpm.c
+++ b/drivers/powercap/dtpm.c
@@ -147,13 +147,17 @@ static void __dtpm_add_power(struct dtpm *dtpm)
  */
 int dtpm_update_power(struct dtpm *dtpm, u64 power_min, u64 power_max)
 {
+	int ret = 0;
+
 	mutex_lock(&dtpm_lock);
 
 	if (power_min == dtpm->power_min && power_max == dtpm->power_max)
-		return 0;
+		goto unlock;
 
-	if (power_max < power_min)
-		return -EINVAL;
+	if (power_max < power_min) {
+		ret = -EINVAL;
+		goto unlock;
+	}
 
 	__dtpm_sub_power(dtpm);
 
@@ -164,9 +168,10 @@ int dtpm_update_power(struct dtpm *dtpm, u64 power_min, u64 power_max)
 
 	__dtpm_add_power(dtpm);
 
+unlock:
 	mutex_unlock(&dtpm_lock);
 
-	return 0;
+	return ret;
 }
 
 /**
@@ -187,8 +192,10 @@ int dtpm_release_zone(struct powercap_zone *pcz)
 
 	mutex_lock(&dtpm_lock);
 
-	if (!list_empty(&dtpm->children))
+	if (!list_empty(&dtpm->children)) {
+		mutex_unlock(&dtpm_lock);
 		return -EBUSY;
+	}
 
 	if (parent)
 		list_del(&dtpm->sibling);
-- 
2.29.2

Re: [PATCH 2/3] powercap/drivers/dtpm: Fix some missing unlock bugs

[Daniel Lezcano]

On 06/01/2021 09:41, Dan Carpenter wrote:
> We need to unlock on these paths before returning.
> 
> Fixes: a20d0ef97abf ("powercap/drivers/dtpm: Add API for dynamic thermal power management")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


> ---
>  drivers/powercap/dtpm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
> index d49df0569cd4..470a1182b868 100644
> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -147,13 +147,17 @@ static void __dtpm_add_power(struct dtpm *dtpm)
>   */
>  int dtpm_update_power(struct dtpm *dtpm, u64 power_min, u64 power_max)
>  {
> +	int ret = 0;
> +
>  	mutex_lock(&dtpm_lock);
>  
>  	if (power_min == dtpm->power_min && power_max == dtpm->power_max)
> -		return 0;
> +		goto unlock;
>  
> -	if (power_max < power_min)
> -		return -EINVAL;
> +	if (power_max < power_min) {
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
>  
>  	__dtpm_sub_power(dtpm);
>  
> @@ -164,9 +168,10 @@ int dtpm_update_power(struct dtpm *dtpm, u64 power_min, u64 power_max)
>  
>  	__dtpm_add_power(dtpm);
>  
> +unlock:
>  	mutex_unlock(&dtpm_lock);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  /**
> @@ -187,8 +192,10 @@ int dtpm_release_zone(struct powercap_zone *pcz)
>  
>  	mutex_lock(&dtpm_lock);
>  
> -	if (!list_empty(&dtpm->children))
> +	if (!list_empty(&dtpm->children)) {
> +		mutex_unlock(&dtpm_lock);
>  		return -EBUSY;
> +	}
>  
>  	if (parent)
>  		list_del(&dtpm->sibling);
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 3/3] powercap/drivers/dtpm: Fix an IS_ERR() vs NULL check

[Dan Carpenter]

The powercap_register_control_type() function never returns NULL, it
returns error pointers on error so update this check.

Fixes: a20d0ef97abf ("powercap/drivers/dtpm: Add API for dynamic thermal power management")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/powercap/dtpm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
index 470a1182b868..5a51cd34a7e8 100644
--- a/drivers/powercap/dtpm.c
+++ b/drivers/powercap/dtpm.c
@@ -467,9 +467,9 @@ static int __init dtpm_init(void)
 	struct dtpm_descr **dtpm_descr;
 
 	pct = powercap_register_control_type(NULL, "dtpm", NULL);
-	if (!pct) {
+	if (IS_ERR(pct)) {
 		pr_err("Failed to register control type\n");
-		return -EINVAL;
+		return PTR_ERR(pct);
 	}
 
 	for_each_dtpm_table(dtpm_descr)
-- 
2.29.2

Re: [PATCH 3/3] powercap/drivers/dtpm: Fix an IS_ERR() vs NULL check

[Daniel Lezcano]

On 06/01/2021 10:03, Dan Carpenter wrote:
> The powercap_register_control_type() function never returns NULL, it
> returns error pointers on error so update this check.
> 
> Fixes: a20d0ef97abf ("powercap/drivers/dtpm: Add API for dynamic thermal power management")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Thanks for these fixes.

  --Daniel

> ---
>  drivers/powercap/dtpm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
> index 470a1182b868..5a51cd34a7e8 100644
> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -467,9 +467,9 @@ static int __init dtpm_init(void)
>  	struct dtpm_descr **dtpm_descr;
>  
>  	pct = powercap_register_control_type(NULL, "dtpm", NULL);
> -	if (!pct) {
> +	if (IS_ERR(pct)) {
>  		pr_err("Failed to register control type\n");
> -		return -EINVAL;
> +		return PTR_ERR(pct);
>  	}
>  
>  	for_each_dtpm_table(dtpm_descr)
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 3/3] powercap/drivers/dtpm: Fix an IS_ERR() vs NULL check

[Rafael J. Wysocki]

On Thu, Jan 7, 2021 at 6:04 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 06/01/2021 10:03, Dan Carpenter wrote:
> > The powercap_register_control_type() function never returns NULL, it
> > returns error pointers on error so update this check.
> >
> > Fixes: a20d0ef97abf ("powercap/drivers/dtpm: Add API for dynamic thermal power management")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> Thanks for these fixes.

All 3 patches applied, but as the original DTPM changes have not made
it into 5.11-rc, all of this is 5.12 material now.

Re: [PATCH 1/3] powercap/drivers/dtpm: Fix a double shift bug

[Daniel Lezcano]

On 06/01/2021 09:36, Dan Carpenter wrote:
> The DTPM_POWER_LIMIT_FLAG is used for test_bit() etc which take a bit
> number so it should be bit 0.  But currently it's set to BIT(0) then
> that is double shifted equivalent to BIT(BIT(0)).  This doesn't cause a
> run time problem because it's done consistently.
> 
> Fixes: a20d0ef97abf ("powercap/drivers/dtpm: Add API for dynamic thermal power management")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  drivers/powercap/dtpm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
> index 0abcc439d728..d49df0569cd4 100644
> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -24,7 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
>  
> -#define DTPM_POWER_LIMIT_FLAG BIT(0)
> +#define DTPM_POWER_LIMIT_FLAG 0
>  
>  static const char *constraint_name[] = {
>  	"Instantaneous",
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog