Re: [PATCH] drm/rcar-du: Convert drm_atomic_helper_suspend/resume()

2018-07-28 Thread Vaishali Thakkar
On Sat, Jul 28, 2018 at 9:10 PM, Souptick Joarder  wrote:
> convert drm_atomic_helper_suspend/resume() to use
> drm_mode_config_helper_suspend/resume().

Hi Souptick,

Thanks for your patch.

> Signed-off-by: Souptick Joarder 
> Signed-off-by: Ajit Negi 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 ++---
>  1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 02aee6c..288220f 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -357,32 +357,15 @@ static void rcar_du_lastclose(struct drm_device *dev)
>  static int rcar_du_pm_suspend(struct device *dev)
>  {
> struct rcar_du_device *rcdu = dev_get_drvdata(dev);
> -   struct drm_atomic_state *state;
>
> -   drm_kms_helper_poll_disable(rcdu->ddev);
> -   drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, true);
> -
> -   state = drm_atomic_helper_suspend(rcdu->ddev);
> -   if (IS_ERR(state)) {
> -   drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);

I don't think we can use drm_mode_config_helper_(suspend/resume)
API here as this file uses CMA functions. And from git grep it seems that
there are very few drivers using it at the moment, so not sure if introducing
new API functions similar to  drm_mode_config will make sense or not.

Thanks.

> -   drm_kms_helper_poll_enable(rcdu->ddev);
> -   return PTR_ERR(state);
> -   }
> -
> -   rcdu->suspend_state = state;
> -
> -   return 0;
> +   return drm_mode_config_helper_suspend(rcdu->ddev);
>  }
>
>  static int rcar_du_pm_resume(struct device *dev)
>  {
> struct rcar_du_device *rcdu = dev_get_drvdata(dev);
>
> -   drm_atomic_helper_resume(rcdu->ddev, rcdu->suspend_state);
> -   drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);
> -   drm_kms_helper_poll_enable(rcdu->ddev);
> -
> -   return 0;
> +   return drm_mode_config_helper_resume(rcdu->ddev);
>  }
>  #endif
>
> --
> 1.9.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH] drm/core: Remove drm_dev_unref() and it's uses

2018-04-26 Thread Vaishali Thakkar
On Thu, Apr 26, 2018 at 6:15 PM, Laurent Pinchart
 wrote:
> Hi Daniel,
>
> On Thursday, 26 April 2018 15:36:15 EEST Daniel Vetter wrote:
>> On Thu, Apr 26, 2018 at 03:58:19PM +0530, Vaishali Thakkar wrote:
>> > It's been a while since we introduced drm_dev{get/put} functions
>> > to replace reference/unreference in drm subsystem for the
>> > consistency purpose. So, with this patch, let's just replace
>> > all current use cases of drm_dev_unref() with drm_dev_put and remove
>> > the function itself.
>> >
>> > Coccinelle was used for mass-patching.
>> >
>> > Signed-off-by: Vaishali Thakkar 
>>
>> Thanks for doing this. Unfortunately drm moves pretty fast, so already a
>> conflict when I tried to apply this. Some drivers are also in their own
>> trees, so this might lead to more fun :-/
>>
>> Can you pls split it up per-driver (just the directories under
>> drivers/gpu/drm/ is enough)? Final patch to remove the function might then
>> get stalled a bit ofc.
>
> I requested a single patch instead of splitting it per driver, you might want
> to blame me for that.
>
>> Also can you pls update ./scripts/coccinelle/api/drm-get-put.cocci and
>> remove that spatch hunk in the final patch, since we no longer need it?
>
> How about just rerunning the coccinelle patch when it's time to apply this ?
> There's precedent for performing such automated changes, and it would ensure
> that no driver is left out.

I was planning to send patches to remove all remaining reference/unreference
functions by the weekend [as there aren't much remaining now and I see that
new drivers keeps adding them instead of new API]. So, wanted to delete whole
cocci file after that. I thought of dividing a patch per function because
Laurent requested to send a single patch for all files.

But if we are going to split it per driver under gpu/drm, would it work if per
driver patch contains all function cases? Also, would you be fine with taking a
patch for removal of coccinelle file via your tree? Then I can include that in
the same patchset as well.

Thanks!

>> > ---
>> >
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 ++--
>> >  drivers/gpu/drm/arc/arcpgu_drv.c   |  4 ++--
>> >  drivers/gpu/drm/armada/armada_drv.c|  6 +++---
>> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   |  4 ++--
>> >  drivers/gpu/drm/drm_drv.c  | 13 -
>> >  drivers/gpu/drm/etnaviv/etnaviv_drv.c  |  4 ++--
>> >  drivers/gpu/drm/exynos/exynos_drm_drv.c|  4 ++--
>> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c  |  4 ++--
>> >  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c|  4 ++--
>> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c|  8 
>> >  drivers/gpu/drm/i915/selftests/huge_pages.c|  2 +-
>> >  drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c   |  2 +-
>> >  drivers/gpu/drm/i915/selftests/i915_gem_evict.c|  2 +-
>> >  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c  |  2 +-
>> >  drivers/gpu/drm/i915/selftests/i915_gem_object.c   |  2 +-
>> >  drivers/gpu/drm/i915/selftests/i915_request.c  |  2 +-
>> >  drivers/gpu/drm/i915/selftests/i915_vma.c  |  2 +-
>> >  drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c |  2 +-
>> >  drivers/gpu/drm/imx/imx-drm-core.c |  4 ++--
>> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c |  6 +++---
>> >  drivers/gpu/drm/msm/msm_drv.c  |  8 
>> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c  |  4 ++--
>> >  drivers/gpu/drm/nouveau/nouveau_platform.c |  2 +-
>> >  drivers/gpu/drm/omapdrm/omap_drv.c |  4 ++--
>> >  drivers/gpu/drm/pl111/pl111_drv.c  |  4 ++--
>> >  drivers/gpu/drm/qxl/qxl_drv.c  |  2 +-
>> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  2 +-
>> >  drivers/gpu/drm/rockchip/rockchip_drm_drv.c|  4 ++--
>> >  drivers/gpu/drm/shmobile/shmob_drm_drv.c   |  4 ++--
>> >  drivers/gpu/drm/sti/sti_drv.c  |  8 
>> >  drivers/gpu/drm/stm/drv.c  |  4 ++--
>> >  drivers/gpu/drm/sun4i/sun4i_drv.c  |  4 ++--
>> >  drivers/gpu/drm/tegra/drm.c|  4 ++--
>> >  drivers/gpu/drm/tinydrm/core/tinydrm-core.c|  6 +++---
>> >  drivers/gpu/drm/tve200/tve200_drv.c|  4 ++--

[PATCH] drm/core: Remove drm_dev_unref() and it's uses

2018-04-26 Thread Vaishali Thakkar
It's been a while since we introduced drm_dev{get/put} functions
to replace reference/unreference in drm subsystem for the
consistency purpose. So, with this patch, let's just replace
all current use cases of drm_dev_unref() with drm_dev_put and remove
the function itself.

Coccinelle was used for mass-patching.

Signed-off-by: Vaishali Thakkar 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|  4 ++--
 drivers/gpu/drm/arc/arcpgu_drv.c   |  4 ++--
 drivers/gpu/drm/armada/armada_drv.c|  6 +++---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   |  4 ++--
 drivers/gpu/drm/drm_drv.c  | 13 -
 drivers/gpu/drm/etnaviv/etnaviv_drv.c  |  4 ++--
 drivers/gpu/drm/exynos/exynos_drm_drv.c|  4 ++--
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c  |  4 ++--
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c|  4 ++--
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c|  8 
 drivers/gpu/drm/i915/selftests/huge_pages.c|  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c   |  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_evict.c|  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c  |  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_object.c   |  2 +-
 drivers/gpu/drm/i915/selftests/i915_request.c  |  2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c  |  2 +-
 drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c |  2 +-
 drivers/gpu/drm/imx/imx-drm-core.c |  4 ++--
 drivers/gpu/drm/mediatek/mtk_drm_drv.c |  6 +++---
 drivers/gpu/drm/msm/msm_drv.c  |  8 
 drivers/gpu/drm/mxsfb/mxsfb_drv.c  |  4 ++--
 drivers/gpu/drm/nouveau/nouveau_platform.c |  2 +-
 drivers/gpu/drm/omapdrm/omap_drv.c |  4 ++--
 drivers/gpu/drm/pl111/pl111_drv.c  |  4 ++--
 drivers/gpu/drm/qxl/qxl_drv.c  |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c|  4 ++--
 drivers/gpu/drm/shmobile/shmob_drm_drv.c   |  4 ++--
 drivers/gpu/drm/sti/sti_drv.c  |  8 
 drivers/gpu/drm/stm/drv.c  |  4 ++--
 drivers/gpu/drm/sun4i/sun4i_drv.c  |  4 ++--
 drivers/gpu/drm/tegra/drm.c|  4 ++--
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c|  6 +++---
 drivers/gpu/drm/tve200/tve200_drv.c|  4 ++--
 drivers/gpu/drm/udl/udl_drv.c  |  2 +-
 drivers/gpu/drm/vc4/vc4_drv.c  |  4 ++--
 drivers/gpu/drm/vgem/vgem_drv.c|  2 +-
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c   |  2 +-
 drivers/gpu/drm/zte/zx_drm_drv.c   |  4 ++--
 include/drm/drm_drv.h  |  1 -
 41 files changed, 73 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0b19482b36b8..5fb455febeba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -647,7 +647,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
 err_pci:
pci_disable_device(pdev);
 err_free:
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
return ret;
 }
 
@@ -657,7 +657,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
 
drm_dev_unregister(dev);
-   drm_dev_unref(dev);
+   drm_dev_put(dev);
pci_disable_device(pdev);
pci_set_drvdata(pdev, NULL);
 }
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index f067de4e1e82..dcb06d4e9135 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -216,7 +216,7 @@ static int arcpgu_probe(struct platform_device *pdev)
arcpgu_unload(drm);
 
 err_unref:
-   drm_dev_unref(drm);
+   drm_dev_put(drm);
 
return ret;
 }
@@ -227,7 +227,7 @@ static int arcpgu_remove(struct platform_device *pdev)
 
drm_dev_unregister(drm);
arcpgu_unload(drm);
-   drm_dev_unref(drm);
+   drm_dev_put(drm);
 
return 0;
 }
diff --git a/drivers/gpu/drm/armada/armada_drv.c 
b/drivers/gpu/drm/armada/armada_drv.c
index 4b11b6b52f1d..d1705d298a39 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -109,7 +109,7 @@ static int armada_drm_bind(struct device *dev)
 
/*
 * The drm_device structure must be at the start of
-* armada_private for drm_dev_unref() to work correctly.
+* armada_private for drm_dev_put() to work correctly.
 */
BUILD_BUG_ON(offsetof(struct armada_private, drm) != 0);
 
@@ -180,7 +180,7 @@ static int armada_drm_bind(struct device *dev)
drm_mode_config_cleanup(&priv->drm);
drm_mm_takedown(&priv->lin

Re: [PATCH] drm: rcar-du: Use drm_dev_put

2018-04-25 Thread Vaishali Thakkar
On Wed, Apr 25, 2018 at 7:02 PM, Laurent Pinchart
 wrote:
> Hi Vaishali,
>
> Thank you for the patch.
>
> On Wednesday, 25 April 2018 15:10:36 EEST Vaishali Thakkar wrote:
>> As specified in drm_drv.c, drm_dev_unref is a compatibility alias
>> for drm_dev_put and shouldn't be used in new code. So, use
>> drm_dev_put instead.
>
> This looks good to me. However, how about taking it one step further and mass-
> patching all drivers to use drm_dev_put() ? We could then remove
> drm_dev_unref() completely.

Sure thing! Do you want me to fold this patch in that patchset or will you be
fine with applying this separately?

>> Signed-off-by: Vaishali Thakkar 
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 05745e86d73e..418a80e6b2f4
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> @@ -378,7 +378,7 @@ static int rcar_du_remove(struct platform_device *pdev)
>>   drm_kms_helper_poll_fini(ddev);
>>   drm_mode_config_cleanup(ddev);
>>
>> - drm_dev_unref(ddev);
>> + drm_dev_put(ddev);
>>
>>   return 0;
>>  }
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>



-- 
Vaishali
http://vaishalithakkar.in/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm: rcar-du: Use drm_dev_put

2018-04-25 Thread Vaishali Thakkar
As specified in drm_drv.c, drm_dev_unref is a compatibility alias
for drm_dev_put and shouldn't be used in new code. So, use
drm_dev_put instead.

Signed-off-by: Vaishali Thakkar 
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 05745e86d73e..418a80e6b2f4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -378,7 +378,7 @@ static int rcar_du_remove(struct platform_device *pdev)
drm_kms_helper_poll_fini(ddev);
drm_mode_config_cleanup(ddev);
 
-   drm_dev_unref(ddev);
+   drm_dev_put(ddev);
 
return 0;
 }
-- 
2.14.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm/vc4: make function vc4_allocate_bin_bo static

2018-04-25 Thread Vaishali Thakkar
Sparse complains with following warning:
drivers/gpu/drm/vc4/vc4_v3d.c:222:1: warning: symbol
'vc4_allocate_bin_bo' was not declared. Should it be static?

Make vc4_allocate_bin static as it is not used outside of
vc4_v3d.c.

Signed-off-by: Vaishali Thakkar 
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index bfc2fa73d2ae..e47e29426078 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -218,8 +218,7 @@ int vc4_v3d_get_bin_slot(struct vc4_dev *vc4)
  * overall CMA pool before they make scenes complicated enough to run
  * out of bin space.
  */
-int
-vc4_allocate_bin_bo(struct drm_device *drm)
+static int vc4_allocate_bin_bo(struct drm_device *drm)
 {
struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_v3d *v3d = vc4->v3d;
-- 
2.14.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm: rcar-du: of: Include header to define prototypes

2018-04-25 Thread Vaishali Thakkar
On Tue, Apr 24, 2018 at 9:09 PM, Kieran Bingham
 wrote:
> The symbol 'rcar_du_of_init' is defined by the rcar_du_of module header,
> but it is not included by the C implementation.
>
> Include the header to correctly define the function prototypes.
>
> Fixes the following warning:
>
> linux/drivers/gpu/drm/rcar-du/rcar_du_of.c:319:13:
>warning: symbol 'rcar_du_of_init' was not declared. Should it be static?
> CC  drivers/gpu/drm/rcar-du/rcar_du_of.o
>
> Fixes: 81c0e3dd8292 ("drm: rcar-du: Fix legacy DT to create LVDS encoder 
> nodes")
> Signed-off-by: Kieran Bingham 

Reviewed-by: Vaishali Thakkar 

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_of.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> index 68a0b82cb17e..afef69669bb4 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> @@ -18,6 +18,7 @@
>
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_drv.h"
> +#include "rcar_du_of.h"
>
>  /* 
> -
>   * Generic Overlay Handling
> --
> 2.17.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm: rcar-du: Use NULL for table initialisation

2018-04-25 Thread Vaishali Thakkar
On Tue, Apr 24, 2018 at 9:10 PM, Kieran Bingham
 wrote:
> Replace the initialisation of the vsps table with a NULL specifier.
>
> Fixes the following warning:
>  linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c:483:40:
> warning: Using plain integer as NULL pointer
>   CC  drivers/gpu/drm/rcar-du/rcar_du_kms.o

Hi Kieran,

Change looks ok to me.

> Fixes: 3e81374e2014 ("drm: rcar-du: Support multiple sources from the same 
> VSP")
> Signed-off-by: Kieran Bingham 

Reviewed-by: Vaishali Thakkar 

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index ef72dff00763..cf5b422fc753 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -480,7 +480,7 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> struct {
> struct device_node *np;
> unsigned int crtcs_mask;
> -   } vsps[RCAR_DU_MAX_VSPS] = { { 0, }, };
> +   } vsps[RCAR_DU_MAX_VSPS] = { { NULL, }, };
> unsigned int vsps_count = 0;
> unsigned int cells;
> unsigned int i;
> --
> 2.17.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Use of copy_from_user in msm_gem_submit.c while holding a spin_lock

2016-08-17 Thread Vaishali Thakkar
Hello,

I was wondering about the call to copy_from_user in function 
submit_lookup_objects for drive
/gpu/drm/msm/msm_gem_submit.c  It calls copy_from_user[1] in a spin_lock, which 
is not normally
allowed, due to the possibility of a deadlock.  

Is there some reason that I am overlooking why it is OK in this case? Is there 
some code in the
same file which ensures that page fault will not occur when we are calling the 
function holding
spin_lock?

Thank you.

[1] 
http://lxr.free-electrons.com/source/drivers/gpu/drm/msm/msm_gem_submit.c#L85

-- 
Vaishali


[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 = &ctrl->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 v2] drm/msm: Move call to PTR_ERR_OR_ZERO after reassignment

2016-04-28 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 
Reviewed-by: Eric Engestrom 
---
Changes since v1:
- No functional changes, just make two pr_err() to print
  the same format
---
 drivers/gpu/drm/msm/edp/edp_ctrl.c | 17 ++---
 1 file changed, 10 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..e10fa810 100644
--- a/drivers/gpu/drm/msm/edp/edp_ctrl.c
+++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c
@@ -302,21 +302,24 @@ static void edp_clk_disable(struct edp_ctrl *ctrl, u32 
clk_mask)
 static int edp_regulator_init(struct edp_ctrl *ctrl)
 {
struct device *dev = &ctrl->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("%s: Could not get lvl-vdd reg, ret = %ld\n", __func__,
+   ret);
ctrl->lvl_vreg = NULL;
-   return PTR_ERR(ctrl->lvl_vreg);
+   return ret;
}

return 0;
-- 
2.1.4



[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 = &ctrl->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