Re: [PATCH 1/4] drm/msm/dpu: replace IS_ERR_OR_NULL with IS_ERR during DSC init

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-04-30 23:35:53, Dmitry Baryshkov wrote:
> Using IS_ERR_OR_NULL() together with PTR_ERR() is a typical mistake. If
> the value is NULL, then the function will return 0 instead of a proper
> return code. Moreover dpu_hw_dsc_init() can not return NULL.

More specifically, it allows the init functions to return NULL when they
do their own filtering and return NULL (as I do in [1] inside
dpu_hw_intf_init for INTF_NONE so that rm->hw_intf for that index is
assigned NULL, rather than failing the entire dpu_rm_init).

[1]: https://lore.kernel.org/linux-arm-msm/20230418-dpu-drop-useless-for-lookup-v3-3-e8d869eea455@xxxxxxxxxxxxxx/

> Replace all dpu_rm_init()'s IS_ERR_OR_NULL() calls with IS_ERR().

It's just one, but that technically counts as "all".

> This follows the commit 740828c73a36 ("drm/msm/dpu: fix error handling
> in dpu_rm_init"), which removed IS_ERR_OR_NULL() from RM init code, but
> then the commit f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in
> RM") added it back for DSC init.

Nit: it did not technically add it "back"; there was no DSC code to
begin with but I'm not sure how to concisely word that by explaining
that init was copied from downstream following downstream patterns (I
think) rather than observing local upstream context.  And not worth it
if there's no resend.

> Suggested-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

Regardless of the wording nits, the change is:

Reviewed-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index f0fc70422e56..dffd3dd0a877 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -247,7 +247,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>  		const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
>  
>  		hw = dpu_hw_dsc_init(dsc, mmio);
> -		if (IS_ERR_OR_NULL(hw)) {
> +		if (IS_ERR(hw)) {
>  			rc = PTR_ERR(hw);
>  			DPU_ERROR("failed dsc object creation: err %d\n", rc);
>  			goto fail;
> -- 
> 2.39.2
> 



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux