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

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

[Gaosheng Cui]

v3:
- Update the second patch:
  1. change IS_ERR to IS_ERR_OR_NULL
  2. add Dmitry's R-b in this revision:
  link: https://patchwork.freedesktop.org/patch/511035/?series=110745&rev=1

  Thanks!

v2:
- I'm sorry I missed some emails, these patches were submitted last year,
  now let me resend it with the following changes:
  1. remove the patch: "drm/msm: Fix IS_ERR_OR_NULL() vs NULL check in msm_icc_get()"
  2. remove the patch: "drm/vc4: kms: Fix IS_ERR() vs NULL check for vc4_kms"
  3. add "Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>" to the second patch.

  Thanks!

v1:
- 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 (3):
  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/komeda: Fix IS_ERR() vs NULL check in
    komeda_component_get_avail_scaler()

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

-- 
2.25.1

[PATCH v3 1/3] 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 8b108ac80b55..4903bbf1df55 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
@@ -571,7 +571,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

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

[Sam Ravnborg]

Hi Gaosheng,
On Fri, Jul 14, 2023 at 09:48:18AM +0800, Gaosheng Cui wrote:
> The mipi_dsi_device_register_full() returns an ERR_PTR() on failure,
> we should use IS_ERR() to check the return value.

Correct - so the code is indeed wrong.

Can you update it so we print the error code as this can be very
helpful.

And then please use dev_probe_err() too.

	Sam

> 
> 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 8b108ac80b55..4903bbf1df55 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35950.c
> @@ -571,7 +571,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 v3 2/3] 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>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.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 a99310b68793..bbb1bf33f98e 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_OR_NULL(ptr)))
 				return;
 
 			for (i = 0; i < dwords; i++) {
-- 
2.25.1

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

[Abhinav Kumar]

On 7/13/2023 6:48 PM, Gaosheng Cui wrote:
> The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
> use IS_ERR() to check the return value.
> 

use IS_ERR_OR_NULL()

I can even fix this while applying

> Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.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 a99310b68793..bbb1bf33f98e 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_OR_NULL(ptr)))
>   				return;
>   
>   			for (i = 0; i < dwords; i++) {

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

[Abhinav Kumar]

On 7/14/2023 12:10 PM, Abhinav Kumar wrote:
> 
> 
> On 7/13/2023 6:48 PM, Gaosheng Cui wrote:
>> The msm_gem_get_vaddr() returns an ERR_PTR() on failure, we should
>> use IS_ERR() to check the return value.
>>
> 
> use IS_ERR_OR_NULL()
> 
> I can even fix this while applying
> 

Actually even subject line needs correction, so better you push another 
rev.

>> Fixes: 6a8bd08d0465 ("drm/msm: add sudo flag to submit ioctl")
>> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.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 a99310b68793..bbb1bf33f98e 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_OR_NULL(ptr)))
>>                   return;
>>               for (i = 0; i < dwords; i++) {

[PATCH v3 3/3] 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>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.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

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

[Liviu Dudau]

Hi Gaosheng,

On Fri, Jul 14, 2023 at 09:48:20AM +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>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.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))

If you're going to update the other patches to use IS_ERR_OR_NULL() please do so
here too. You can keep my R-b for that change.

Best regards,
Liviu

>  		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 v3 3/3] drm/komeda: Fix IS_ERR() vs NULL check in komeda_component_get_avail_scaler()

[cuigaosheng]

> If you're going to update the other patches to use IS_ERR_OR_NULL() please do so
> here too. You can keep my R-b for that change.

Thanks for taking time to review the patch.

I have update the patch set and add this change to v5.

On 2023/7/21 18:24, Liviu Dudau wrote:
> Hi Gaosheng,
>
> On Fri, Jul 14, 2023 at 09:48:20AM +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>
>> Reviewed-by: Liviu Dudau <liviu.dudau@arm.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))
> If you're going to update the other patches to use IS_ERR_OR_NULL() please do so
> here too. You can keep my R-b for that change.
>
> Best regards,
> Liviu
>
>>   		return NULL;
>>   
>>   	avail_scalers = (pipe_st->active_comps & KOMEDA_PIPELINE_SCALERS) ^
>> -- 
>> 2.25.1
>>