[PATCH] drm/msm: Move call to PTR_ERR_OR_ZERO after reassignment

2016-04-28 Thread Vaishali Thakkar 


On Thursday 28 April 2016 06:23 PM, Eric Engestrom wrote:
> On Wed, Apr 27, 2016 at 04:51:37PM +0530, Vaishali Thakkar wrote:
>> Here, a location is reset to NULL before being passed to PTR_ERR.
>> So, PTR_ERR should be called before its argument is reassigned
>> to NULL. Further to simplify things use PTR_ERR_OR_ZERO instead
>> of PTR_ERR and IS_ERR.
>>
>> Problem found using Coccinelle.
>>
>> Signed-off-by: Vaishali Thakkar 
>> ---
>>  drivers/gpu/drm/msm/edp/edp_ctrl.c | 16 +---
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c 
>> b/drivers/gpu/drm/msm/edp/edp_ctrl.c
>> index 81200e9..7816541 100644
>> --- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
>> @@ -302,21 +302,23 @@ static void edp_clk_disable(struct edp_ctrl *ctrl, u32 
>> clk_mask)
>>  static int edp_regulator_init(struct edp_ctrl *ctrl)
>>  {
>>  struct device *dev = >pdev->dev;
>> +int ret;
>>  
>>  DBG("");
>>  ctrl->vdda_vreg = devm_regulator_get(dev, "vdda");
>> -if (IS_ERR(ctrl->vdda_vreg)) {
>> +ret = PTR_ERR_OR_ZERO(ctrl->vdda_vreg);
>> +if (ret) {
>>  pr_err("%s: Could not get vdda reg, ret = %ld\n", __func__,
>> -PTR_ERR(ctrl->vdda_vreg));
>> +ret);
>>  ctrl->vdda_vreg = NULL;
>> -return PTR_ERR(ctrl->vdda_vreg);
>> +return ret;
>>  }
>>  ctrl->lvl_vreg = devm_regulator_get(dev, "lvl-vdd");
>> -if (IS_ERR(ctrl->lvl_vreg)) {
>> -pr_err("Could not get lvl-vdd reg, %ld",
>> -PTR_ERR(ctrl->lvl_vreg));
>> +ret = PTR_ERR_OR_ZERO(ctrl->lvl_vreg);
>> +if (ret) {
> 
> While you're rewriting them, it might be worth making these two pr_err()
> print the same format, e.g.:
> 
>   pr_err("%s: Could not get lvl-vdd reg, ret = %ld\n", __func__, ret);

Done, thanks.

>> +pr_err("Could not get lvl-vdd reg, %ld", ret);
>>  ctrl->lvl_vreg = NULL;
>> -return PTR_ERR(ctrl->lvl_vreg);
>> +return ret;
>>  }
>>  
>>  return 0;
>> -- 
>> 2.1.4
> 
> Reviewed-by: Eric Engestrom 
> 

-- 
Vaishali

[PATCH] drm/msm: Move call to PTR_ERR_OR_ZERO after reassignment

2016-04-28 Thread Eric Engestrom 
On Wed, Apr 27, 2016 at 04:51:37PM +0530, Vaishali Thakkar wrote:
> Here, a location is reset to NULL before being passed to PTR_ERR.
> So, PTR_ERR should be called before its argument is reassigned
> to NULL. Further to simplify things use PTR_ERR_OR_ZERO instead
> of PTR_ERR and IS_ERR.
> 
> Problem found using Coccinelle.
> 
> Signed-off-by: Vaishali Thakkar 
> ---
>  drivers/gpu/drm/msm/edp/edp_ctrl.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c 
> b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> index 81200e9..7816541 100644
> --- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
> +++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
> @@ -302,21 +302,23 @@ static void edp_clk_disable(struct edp_ctrl *ctrl, u32 
> clk_mask)
>  static int edp_regulator_init(struct edp_ctrl *ctrl)
>  {
>   struct device *dev = >pdev->dev;
> + int ret;
>  
>   DBG("");
>   ctrl->vdda_vreg = devm_regulator_get(dev, "vdda");
> - if (IS_ERR(ctrl->vdda_vreg)) {
> + ret = PTR_ERR_OR_ZERO(ctrl->vdda_vreg);
> + if (ret) {
>   pr_err("%s: Could not get vdda reg, ret = %ld\n", __func__,
> - PTR_ERR(ctrl->vdda_vreg));
> + ret);
>   ctrl->vdda_vreg = NULL;
> - return PTR_ERR(ctrl->vdda_vreg);
> + return ret;
>   }
>   ctrl->lvl_vreg = devm_regulator_get(dev, "lvl-vdd");
> - if (IS_ERR(ctrl->lvl_vreg)) {
> - pr_err("Could not get lvl-vdd reg, %ld",
> - PTR_ERR(ctrl->lvl_vreg));
> + ret = PTR_ERR_OR_ZERO(ctrl->lvl_vreg);
> + if (ret) {

While you're rewriting them, it might be worth making these two pr_err()
print the same format, e.g.:

pr_err("%s: Could not get lvl-vdd reg, ret = %ld\n", __func__, ret);

> + pr_err("Could not get lvl-vdd reg, %ld", ret);
>   ctrl->lvl_vreg = NULL;
> - return PTR_ERR(ctrl->lvl_vreg);
> + return ret;
>   }
>  
>   return 0;
> -- 
> 2.1.4

Reviewed-by: Eric Engestrom

[PATCH] drm/msm: Move call to PTR_ERR_OR_ZERO after reassignment

2016-04-27 Thread Vaishali Thakkar 
Here, a location is reset to NULL before being passed to PTR_ERR.
So, PTR_ERR should be called before its argument is reassigned
to NULL. Further to simplify things use PTR_ERR_OR_ZERO instead
of PTR_ERR and IS_ERR.

Problem found using Coccinelle.

Signed-off-by: Vaishali Thakkar 
---
 drivers/gpu/drm/msm/edp/edp_ctrl.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c 
b/drivers/gpu/drm/msm/edp/edp_ctrl.c
index 81200e9..7816541 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -302,21 +302,23 @@ static void edp_clk_disable(struct edp_ctrl *ctrl, u32 
clk_mask)
 static int edp_regulator_init(struct edp_ctrl *ctrl)
 {
struct device *dev = >pdev->dev;
+   int ret;

DBG("");
ctrl->vdda_vreg = devm_regulator_get(dev, "vdda");
-   if (IS_ERR(ctrl->vdda_vreg)) {
+   ret = PTR_ERR_OR_ZERO(ctrl->vdda_vreg);
+   if (ret) {
pr_err("%s: Could not get vdda reg, ret = %ld\n", __func__,
-   PTR_ERR(ctrl->vdda_vreg));
+   ret);
ctrl->vdda_vreg = NULL;
-   return PTR_ERR(ctrl->vdda_vreg);
+   return ret;
}
ctrl->lvl_vreg = devm_regulator_get(dev, "lvl-vdd");
-   if (IS_ERR(ctrl->lvl_vreg)) {
-   pr_err("Could not get lvl-vdd reg, %ld",
-   PTR_ERR(ctrl->lvl_vreg));
+   ret = PTR_ERR_OR_ZERO(ctrl->lvl_vreg);
+   if (ret) {
+   pr_err("Could not get lvl-vdd reg, %ld", ret);
ctrl->lvl_vreg = NULL;
-   return PTR_ERR(ctrl->lvl_vreg);
+   return ret;
}

return 0;
-- 
2.1.4