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 >
