[PATCH 0/5] Fix IS_ERR() vs NULL check for drm

[PATCH 0/5] Fix IS_ERR() vs NULL check for drm

[Gaosheng Cui]

This series contains a few fixup patches, to fix IS_ERR() vs NULL check
for drm, and avoid a potential null-ptr-defer issue, too. Thanks!

Gaosheng Cui (5):
  drm/panel: Fix IS_ERR() vs NULL check in nt35950_probe()
  drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()
  drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()
  drm/komeda: Fix IS_ERR() vs NULL check in
    komeda_component_get_avail_scaler()
  drm/vc4: kms: Fix IS_ERR() vs NULL check for vc4_kms

 .../gpu/drm/arm/display/komeda/komeda_pipeline_state.c    | 2 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c                     | 2 +-
 drivers/gpu/drm/msm/msm_io_utils.c                        | 2 +-
 drivers/gpu/drm/panel/panel-novatek-nt35950.c             | 2 +-
 drivers/gpu/drm/vc4/vc4_kms.c                             | 8 ++++----
 5 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 0/5] Fix IS_ERR() vs NULL check for drm

[Gaosheng Cui]

This series contains a few fixup patches, to fix IS_ERR() vs NULL check
for drm, and avoid a potential null-ptr-defer issue, too. Thanks!

Gaosheng Cui (5):
  drm/panel: Fix IS_ERR() vs NULL check in nt35950_probe()
  drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()
  drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()
  drm/komeda: Fix IS_ERR() vs NULL check in
    komeda_component_get_avail_scaler()
  drm/vc4: kms: Fix IS_ERR() vs NULL check for vc4_kms

 .../gpu/drm/arm/display/komeda/komeda_pipeline_state.c    | 2 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c                     | 2 +-
 drivers/gpu/drm/msm/msm_io_utils.c                        | 2 +-
 drivers/gpu/drm/panel/panel-novatek-nt35950.c             | 2 +-
 drivers/gpu/drm/vc4/vc4_kms.c                             | 8 ++++----
 5 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/5] drm/panel: Fix IS_ERR() vs NULL check in nt35950_probe()

[Gaosheng Cui]

The mipi_dsi_device_register_full() returns an ERR_PTR() on failure,
we should use IS_ERR() to check the return value.

Fixes: 623a3531e9cf ("drm/panel: Add driver for Novatek NT35950 DSI DriverIC panels")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/panel/panel-novatek-nt35950.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
index 3a844917da07..6304fe5b9038 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
@@ -579,7 +579,7 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
 		}
 
 		nt->dsi[1] = mipi_dsi_device_register_full(dsi_r_host, info);
-		if (!nt->dsi[1]) {
+		if (IS_ERR(nt->dsi[1])) {
 			dev_err(dev, "Cannot get secondary DSI node\n");
 			return -ENODEV;
 		}
-- 
2.25.1

[PATCH 1/5] drm/panel: Fix IS_ERR() vs NULL check in nt35950_probe()

[Gaosheng Cui]

The mipi_dsi_device_register_full() returns an ERR_PTR() on failure,
we should use IS_ERR() to check the return value.

Fixes: 623a3531e9cf ("drm/panel: Add driver for Novatek NT35950 DSI DriverIC panels")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/panel/panel-novatek-nt35950.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35950.c b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
index 3a844917da07..6304fe5b9038 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
@@ -579,7 +579,7 @@ static int nt35950_probe(struct mipi_dsi_device *dsi)
 		}
 
 		nt->dsi[1] = mipi_dsi_device_register_full(dsi_r_host, info);
-		if (!nt->dsi[1]) {
+		if (IS_ERR(nt->dsi[1])) {
 			dev_err(dev, "Cannot get secondary DSI node\n");
 			return -ENODEV;
 		}
-- 
2.25.1

[PATCH 2/5] drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()

[Gaosheng Cui]

The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
use IS_ERR() to check the return value.

Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 3c537c0016fa..0abc802e8d5f 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -89,7 +89,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit
 			 * since we've already mapped it once in
 			 * submit_reloc()
 			 */
-			if (WARN_ON(!ptr))
+			if (WARN_ON(IS_ERR(ptr)))
 				return;
 
 			for (i = 0; i < dwords; i++) {
-- 
2.25.1

[PATCH 2/5] drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()

[Gaosheng Cui]

The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
use IS_ERR() to check the return value.

Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 3c537c0016fa..0abc802e8d5f 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -89,7 +89,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit
 			 * since we've already mapped it once in
 			 * submit_reloc()
 			 */
-			if (WARN_ON(!ptr))
+			if (WARN_ON(IS_ERR(ptr)))
 				return;
 
 			for (i = 0; i < dwords; i++) {
-- 
2.25.1

[PATCH 3/5] drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()

[Gaosheng Cui]

The of_icc_get() function returns NULL or error pointers on error path,
so we should use IS_ERR_OR_NULL() to check the return value.

Fixes: 5ccdcecaf8f7 ("drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/msm/msm_io_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
index d02cd29ce829..950083b2f092 100644
--- a/drivers/gpu/drm/msm/msm_io_utils.c
+++ b/drivers/gpu/drm/msm/msm_io_utils.c
@@ -133,7 +133,7 @@ struct icc_path *msm_icc_get(struct device *dev, const char *name)
 	struct icc_path *path;
 
 	path = of_icc_get(dev, name);
-	if (path)
+	if (IS_ERR_OR_NULL(path))
 		return path;
 
 	/*
-- 
2.25.1

[PATCH 3/5] drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()

[Gaosheng Cui]

The of_icc_get() function returns NULL or error pointers on error path,
so we should use IS_ERR_OR_NULL() to check the return value.

Fixes: 5ccdcecaf8f7 ("drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/msm/msm_io_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
index d02cd29ce829..950083b2f092 100644
--- a/drivers/gpu/drm/msm/msm_io_utils.c
+++ b/drivers/gpu/drm/msm/msm_io_utils.c
@@ -133,7 +133,7 @@ struct icc_path *msm_icc_get(struct device *dev, const char *name)
 	struct icc_path *path;
 
 	path = of_icc_get(dev, name);
-	if (path)
+	if (IS_ERR_OR_NULL(path))
 		return path;
 
 	/*
-- 
2.25.1

[PATCH 4/5] drm/komeda: Fix IS_ERR() vs NULL check in komeda_component_get_avail_scaler()

[Gaosheng Cui]

The komeda_pipeline_get_state() returns an ERR_PTR() on failure, we should
use IS_ERR() to check the return value.

Fixes: 502932a03fce ("drm/komeda: Add the initial scaler support for CORE")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 3276a3e82c62..e9c92439398d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -259,7 +259,7 @@ komeda_component_get_avail_scaler(struct komeda_component *c,
 	u32 avail_scalers;
 
 	pipe_st = komeda_pipeline_get_state(c->pipeline, state);
-	if (!pipe_st)
+	if (IS_ERR(pipe_st))
 		return NULL;
 
 	avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^
-- 
2.25.1

[PATCH 4/5] drm/komeda: Fix IS_ERR() vs NULL check in komeda_component_get_avail_scaler()

[Gaosheng Cui]

The komeda_pipeline_get_state() returns an ERR_PTR() on failure, we should
use IS_ERR() to check the return value.

Fixes: 502932a03fce ("drm/komeda: Add the initial scaler support for CORE")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 3276a3e82c62..e9c92439398d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -259,7 +259,7 @@ komeda_component_get_avail_scaler(struct komeda_component *c,
 	u32 avail_scalers;
 
 	pipe_st = komeda_pipeline_get_state(c->pipeline, state);
-	if (!pipe_st)
+	if (IS_ERR(pipe_st))
 		return NULL;
 
 	avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^
-- 
2.25.1

[PATCH 5/5] drm/vc4: kms: Fix IS_ERR() vs NULL check for vc4_kms

[Gaosheng Cui]

The drm_atomic_get_new_private_obj_state() function returns NULL
on error path, drm_atomic_get_old_private_obj_state() function
returns NULL on error path, too, they does not return error pointers.

By the way, vc4_hvs_get_new/old_global_state() should return
ERR_PTR(-EINVAL), otherwise there will be null-ptr-defer issue,
such as follows:

In function vc4_atomic_commit_tail():
  |-- old_hvs_state = vc4_hvs_get_old_global_state(state); <-- return NULL
  |-- if (WARN_ON(IS_ERR(old_hvs_state))) <-- no return
  |-- unsigned long state_rate = max(old_hvs_state->core_clock_rate,
	new_hvs_state->core_clock_rate); <-- null-ptr-defer

Fixes: 9ec03d7f1ed3 ("drm/vc4: kms: Wait on previous FIFO users before a commit")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/vc4/vc4_kms.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 5c97642ed66a..8fbeecdf2ec4 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -197,8 +197,8 @@ vc4_hvs_get_new_global_state(struct drm_atomic_state *state)
 	struct drm_private_state *priv_state;
 
 	priv_state = drm_atomic_get_new_private_obj_state(state, &vc4->hvs_channels);
-	if (IS_ERR(priv_state))
-		return ERR_CAST(priv_state);
+	if (!priv_state)
+		return ERR_PTR(-EINVAL);
 
 	return to_vc4_hvs_state(priv_state);
 }
@@ -210,8 +210,8 @@ vc4_hvs_get_old_global_state(struct drm_atomic_state *state)
 	struct drm_private_state *priv_state;
 
 	priv_state = drm_atomic_get_old_private_obj_state(state, &vc4->hvs_channels);
-	if (IS_ERR(priv_state))
-		return ERR_CAST(priv_state);
+	if (!priv_state)
+		return ERR_PTR(-EINVAL);
 
 	return to_vc4_hvs_state(priv_state);
 }
-- 
2.25.1

[PATCH 5/5] drm/vc4: kms: Fix IS_ERR() vs NULL check for vc4_kms

[Gaosheng Cui]

The drm_atomic_get_new_private_obj_state() function returns NULL
on error path, drm_atomic_get_old_private_obj_state() function
returns NULL on error path, too, they does not return error pointers.

By the way, vc4_hvs_get_new/old_global_state() should return
ERR_PTR(-EINVAL), otherwise there will be null-ptr-defer issue,
such as follows:

In function vc4_atomic_commit_tail():
  |-- old_hvs_state = vc4_hvs_get_old_global_state(state); <-- return NULL
  |-- if (WARN_ON(IS_ERR(old_hvs_state))) <-- no return
  |-- unsigned long state_rate = max(old_hvs_state->core_clock_rate,
	new_hvs_state->core_clock_rate); <-- null-ptr-defer

Fixes: 9ec03d7f1ed3 ("drm/vc4: kms: Wait on previous FIFO users before a commit")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
---
 drivers/gpu/drm/vc4/vc4_kms.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 5c97642ed66a..8fbeecdf2ec4 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -197,8 +197,8 @@ vc4_hvs_get_new_global_state(struct drm_atomic_state *state)
 	struct drm_private_state *priv_state;
 
 	priv_state = drm_atomic_get_new_private_obj_state(state, &vc4->hvs_channels);
-	if (IS_ERR(priv_state))
-		return ERR_CAST(priv_state);
+	if (!priv_state)
+		return ERR_PTR(-EINVAL);
 
 	return to_vc4_hvs_state(priv_state);
 }
@@ -210,8 +210,8 @@ vc4_hvs_get_old_global_state(struct drm_atomic_state *state)
 	struct drm_private_state *priv_state;
 
 	priv_state = drm_atomic_get_old_private_obj_state(state, &vc4->hvs_channels);
-	if (IS_ERR(priv_state))
-		return ERR_CAST(priv_state);
+	if (!priv_state)
+		return ERR_PTR(-EINVAL);
 
 	return to_vc4_hvs_state(priv_state);
 }
-- 
2.25.1

Re: (subset) [PATCH 5/5] drm/vc4: kms: Fix IS_ERR() vs NULL check for vc4_kms

[Maxime Ripard]

On Thu, 10 Nov 2022 17:44:45 +0800, Gaosheng Cui wrote:
> The drm_atomic_get_new_private_obj_state() function returns NULL
> on error path, drm_atomic_get_old_private_obj_state() function
> returns NULL on error path, too, they does not return error pointers.
> 
> By the way, vc4_hvs_get_new/old_global_state() should return
> ERR_PTR(-EINVAL), otherwise there will be null-ptr-defer issue,
> such as follows:
> 
> [...]

Applied to drm/drm-misc (drm-misc-fixes).

Thanks!
Maxime

Re: (subset) [PATCH 5/5] drm/vc4: kms: Fix IS_ERR() vs NULL check for vc4_kms

[Maxime Ripard]

On Thu, 10 Nov 2022 17:44:45 +0800, Gaosheng Cui wrote:
> The drm_atomic_get_new_private_obj_state() function returns NULL
> on error path, drm_atomic_get_old_private_obj_state() function
> returns NULL on error path, too, they does not return error pointers.
> 
> By the way, vc4_hvs_get_new/old_global_state() should return
> ERR_PTR(-EINVAL), otherwise there will be null-ptr-defer issue,
> such as follows:
> 
> [...]

Applied to drm/drm-misc (drm-misc-fixes).

Thanks!
Maxime

Re: [PATCH 4/5] drm/komeda: Fix IS_ERR() vs NULL check in komeda_component_get_avail_scaler()

[Liviu Dudau]

On Thu, Nov 10, 2022 at 05:44:44PM +0800, Gaosheng Cui wrote:
> The komeda_pipeline_get_state() returns an ERR_PTR() on failure, we should
> use IS_ERR() to check the return value.
> 
> Fixes: 502932a03fce ("drm/komeda: Add the initial scaler support for CORE")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks for the fix!

Best regards,
Liviu

> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 3276a3e82c62..e9c92439398d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -259,7 +259,7 @@ komeda_component_get_avail_scaler(struct komeda_component *c,
>  	u32 avail_scalers;
>  
>  	pipe_st = komeda_pipeline_get_state(c->pipeline, state);
> -	if (!pipe_st)
> +	if (IS_ERR(pipe_st))
>  		return NULL;
>  
>  	avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^
> -- 
> 2.25.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 4/5] drm/komeda: Fix IS_ERR() vs NULL check in komeda_component_get_avail_scaler()

[Liviu Dudau]

On Thu, Nov 10, 2022 at 05:44:44PM +0800, Gaosheng Cui wrote:
> The komeda_pipeline_get_state() returns an ERR_PTR() on failure, we should
> use IS_ERR() to check the return value.
> 
> Fixes: 502932a03fce ("drm/komeda: Add the initial scaler support for CORE")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks for the fix!

Best regards,
Liviu

> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 3276a3e82c62..e9c92439398d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -259,7 +259,7 @@ komeda_component_get_avail_scaler(struct komeda_component *c,
>  	u32 avail_scalers;
>  
>  	pipe_st = komeda_pipeline_get_state(c->pipeline, state);
> -	if (!pipe_st)
> +	if (IS_ERR(pipe_st))
>  		return NULL;
>  
>  	avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^
> -- 
> 2.25.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 3/5] drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()

[Marijn Suijten]

On 2022-11-10 17:44:43, Gaosheng Cui wrote:
> The of_icc_get() function returns NULL or error pointers on error path,
> so we should use IS_ERR_OR_NULL() to check the return value.
> 
> Fixes: 5ccdcecaf8f7 ("drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
>  drivers/gpu/drm/msm/msm_io_utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
> index d02cd29ce829..950083b2f092 100644
> --- a/drivers/gpu/drm/msm/msm_io_utils.c
> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> @@ -133,7 +133,7 @@ struct icc_path *msm_icc_get(struct device *dev, const char *name)
>  	struct icc_path *path;
>  
>  	path = of_icc_get(dev, name);
> -	if (path)
> +	if (IS_ERR_OR_NULL(path))

NAK. This path should be returned if it is NON-NULL, otherwise we defer
to of_icc_get() on the parent device.  See the code-comment below.

- Marijn

>  		return path;
>  
>  	/*
> -- 
> 2.25.1
> 

Re: [PATCH 3/5] drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()

[Marijn Suijten]

On 2022-11-10 17:44:43, Gaosheng Cui wrote:
> The of_icc_get() function returns NULL or error pointers on error path,
> so we should use IS_ERR_OR_NULL() to check the return value.
> 
> Fixes: 5ccdcecaf8f7 ("drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
>  drivers/gpu/drm/msm/msm_io_utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
> index d02cd29ce829..950083b2f092 100644
> --- a/drivers/gpu/drm/msm/msm_io_utils.c
> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> @@ -133,7 +133,7 @@ struct icc_path *msm_icc_get(struct device *dev, const char *name)
>  	struct icc_path *path;
>  
>  	path = of_icc_get(dev, name);
> -	if (path)
> +	if (IS_ERR_OR_NULL(path))

NAK. This path should be returned if it is NON-NULL, otherwise we defer
to of_icc_get() on the parent device.  See the code-comment below.

- Marijn

>  		return path;
>  
>  	/*
> -- 
> 2.25.1
> 

Re: [PATCH 3/5] drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()

[cuigaosheng]

> NAK. This path should be returned if it is NON-NULL, otherwise we defer
> to of_icc_get() on the parent device.  See the code-comment below.

Thanks for taking time to review this patch, how do you think of the following changes:
  
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c 
> b/drivers/gpu/drm/msm/msm_io_utils.c index d02cd29ce829..a112d8c74d59 
> 100644 --- a/drivers/gpu/drm/msm/msm_io_utils.c +++ 
> b/drivers/gpu/drm/msm/msm_io_utils.c @@ -133,7 +133,7 @@ struct 
> icc_path *msm_icc_get(struct device *dev, const char *name) struct 
> icc_path *path; path = of_icc_get(dev, name); - if (path) + if 
> (!IS_ERR_OR_NULL(path)) return path; 

Looking forward to your reply, thanks again!

On 2022/11/11 18:02, Marijn Suijten wrote:
> On 2022-11-10 17:44:43, Gaosheng Cui wrote:
>> The of_icc_get() function returns NULL or error pointers on error path,
>> so we should use IS_ERR_OR_NULL() to check the return value.
>>
>> Fixes: 5ccdcecaf8f7 ("drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices")
>> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
>> ---
>>   drivers/gpu/drm/msm/msm_io_utils.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
>> index d02cd29ce829..950083b2f092 100644
>> --- a/drivers/gpu/drm/msm/msm_io_utils.c
>> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
>> @@ -133,7 +133,7 @@ struct icc_path *msm_icc_get(struct device *dev, const char *name)
>>   	struct icc_path *path;
>>   
>>   	path = of_icc_get(dev, name);
>> -	if (path)
>> +	if (IS_ERR_OR_NULL(path))
> NAK. This path should be returned if it is NON-NULL, otherwise we defer
> to of_icc_get() on the parent device.  See the code-comment below.
>
> - Marijn
>
>>   		return path;
>>   
>>   	/*
>> -- 
>> 2.25.1
>>
> .

Re: [PATCH 3/5] drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()

[cuigaosheng]

> NAK. This path should be returned if it is NON-NULL, otherwise we defer
> to of_icc_get() on the parent device.  See the code-comment below.

Thanks for taking time to review this patch, how do you think of the following changes:
  
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c 
> b/drivers/gpu/drm/msm/msm_io_utils.c index d02cd29ce829..a112d8c74d59 
> 100644 --- a/drivers/gpu/drm/msm/msm_io_utils.c +++ 
> b/drivers/gpu/drm/msm/msm_io_utils.c @@ -133,7 +133,7 @@ struct 
> icc_path *msm_icc_get(struct device *dev, const char *name) struct 
> icc_path *path; path = of_icc_get(dev, name); - if (path) + if 
> (!IS_ERR_OR_NULL(path)) return path; 

Looking forward to your reply, thanks again!

On 2022/11/11 18:02, Marijn Suijten wrote:
> On 2022-11-10 17:44:43, Gaosheng Cui wrote:
>> The of_icc_get() function returns NULL or error pointers on error path,
>> so we should use IS_ERR_OR_NULL() to check the return value.
>>
>> Fixes: 5ccdcecaf8f7 ("drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices")
>> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
>> ---
>>   drivers/gpu/drm/msm/msm_io_utils.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
>> index d02cd29ce829..950083b2f092 100644
>> --- a/drivers/gpu/drm/msm/msm_io_utils.c
>> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
>> @@ -133,7 +133,7 @@ struct icc_path *msm_icc_get(struct device *dev, const char *name)
>>   	struct icc_path *path;
>>   
>>   	path = of_icc_get(dev, name);
>> -	if (path)
>> +	if (IS_ERR_OR_NULL(path))
> NAK. This path should be returned if it is NON-NULL, otherwise we defer
> to of_icc_get() on the parent device.  See the code-comment below.
>
> - Marijn
>
>>   		return path;
>>   
>>   	/*
>> -- 
>> 2.25.1
>>
> .

Re: [PATCH 2/5] drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()

[Dmitry Baryshkov]

On 10/11/2022 11:44, Gaosheng Cui wrote:
> The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
> use IS_ERR() to check the return value.
> 
> Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
>   drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 3c537c0016fa..0abc802e8d5f 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -89,7 +89,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit
>   			 * since we've already mapped it once in
>   			 * submit_reloc()
>   			 */
> -			if (WARN_ON(!ptr))
> +			if (WARN_ON(IS_ERR(ptr)))
>   				return;
>   
>   			for (i = 0; i < dwords; i++) {

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 2/5] drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()

[Dmitry Baryshkov]

On 10/11/2022 11:44, Gaosheng Cui wrote:
> The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
> use IS_ERR() to check the return value.
> 
> Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
>   drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 3c537c0016fa..0abc802e8d5f 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -89,7 +89,7 @@ static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit
>   			 * since we've already mapped it once in
>   			 * submit_reloc()
>   			 */
> -			if (WARN_ON(!ptr))
> +			if (WARN_ON(IS_ERR(ptr)))
>   				return;
>   
>   			for (i = 0; i < dwords; i++) {

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 2/5] drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()

[Abhinav Kumar]

On 11/10/2022 1:44 AM, Gaosheng Cui wrote:
> The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
> use IS_ERR() to check the return value.
> 
> Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
>   drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

Re: [PATCH 2/5] drm/msm: Fix IS_ERR() vs NULL check in a5xx_submit_in_rb()

[Abhinav Kumar]

On 11/10/2022 1:44 AM, Gaosheng Cui wrote:
> The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
> use IS_ERR() to check the return value.
> 
> Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> ---
>   drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>