Re: [PATCH 06/11] drm/msm/dpu: Cleanup callers of dpu_hw_blk_init

[Sean Paul <sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]

On Mon, Nov 05, 2018 at 04:30:58PM -0700, Jordan Crouse wrote:
> Outside of superfluous parameter checks the dpu_hw_blk_init()
> doesn't have any failure paths. Switch it over to be a void
> function and we can remove error handling paths in all the functions
> that call it. While we're in those functions remove unneeded
> initialization for a static variable.
> 
> v2: Removed a cleanup intended for a different patch
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c      | 10 +---------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h      |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c      | 17 ++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c     | 17 ++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c       | 17 ++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 17 ++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c     | 17 ++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c      | 17 ++---------------
>  8 files changed, 14 insertions(+), 100 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
> index 58d29e43faef..92f1c4241b9a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c
> @@ -30,16 +30,10 @@ static LIST_HEAD(dpu_hw_blk_list);
>   * @type: hw block type - enum dpu_hw_blk_type
>   * @id: instance id of the hw block
>   * @ops: Pointer to block operations
> - * return: 0 if success; error code otherwise
>   */
> -int dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
> +void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
>  		struct dpu_hw_blk_ops *ops)
>  {
> -	if (!hw_blk) {
> -		pr_err("invalid parameters\n");
> -		return -EINVAL;
> -	}
> -
>  	INIT_LIST_HEAD(&hw_blk->list);
>  	hw_blk->type = type;
>  	hw_blk->id = id;
> @@ -51,8 +45,6 @@ int dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
>  	mutex_lock(&dpu_hw_blk_lock);
>  	list_add(&hw_blk->list, &dpu_hw_blk_list);
>  	mutex_unlock(&dpu_hw_blk_lock);
> -
> -	return 0;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h
> index 0f4ca8af1ec5..1934c2f7e8fa 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.h
> @@ -44,7 +44,7 @@ struct dpu_hw_blk {
>  	struct dpu_hw_blk_ops ops;
>  };
>  
> -int dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
> +void dpu_hw_blk_init(struct dpu_hw_blk *hw_blk, u32 type, int id,
>  		struct dpu_hw_blk_ops *ops);
>  void dpu_hw_blk_destroy(struct dpu_hw_blk *hw_blk);
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 4aab04335c6d..1068b4b7940f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -483,10 +483,7 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
>  	ops->get_bitmask_intf = dpu_hw_ctl_get_bitmask_intf;
>  };
>  
> -static struct dpu_hw_blk_ops dpu_hw_ops = {
> -	.start = NULL,
> -	.stop = NULL,
> -};
> +static struct dpu_hw_blk_ops dpu_hw_ops;
>  
>  struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx,
>  		void __iomem *addr,
> @@ -494,7 +491,6 @@ struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx,
>  {
>  	struct dpu_hw_ctl *c;
>  	struct dpu_ctl_cfg *cfg;
> -	int rc;
>  
>  	c = kzalloc(sizeof(*c), GFP_KERNEL);
>  	if (!c)
> @@ -513,18 +509,9 @@ struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx,
>  	c->mixer_count = m->mixer_count;
>  	c->mixer_hw_caps = m->mixer;
>  
> -	rc = dpu_hw_blk_init(&c->base, DPU_HW_BLK_CTL, idx, &dpu_hw_ops);
> -	if (rc) {
> -		DPU_ERROR("failed to init hw blk %d\n", rc);
> -		goto blk_init_error;
> -	}
> +	dpu_hw_blk_init(&c->base, DPU_HW_BLK_CTL, idx, &dpu_hw_ops);
>  
>  	return c;
> -
> -blk_init_error:
> -	kzfree(c);
> -
> -	return ERR_PTR(rc);
>  }
>  
>  void dpu_hw_ctl_destroy(struct dpu_hw_ctl *ctx)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 695d27a730e8..f6a83daa385b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -264,10 +264,7 @@ static void _setup_intf_ops(struct dpu_hw_intf_ops *ops,
>  	ops->get_line_count = dpu_hw_intf_get_line_count;
>  }
>  
> -static struct dpu_hw_blk_ops dpu_hw_ops = {
> -	.start = NULL,
> -	.stop = NULL,
> -};
> +static struct dpu_hw_blk_ops dpu_hw_ops;
>  
>  struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
>  		void __iomem *addr,
> @@ -275,7 +272,6 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
>  {
>  	struct dpu_hw_intf *c;
>  	struct dpu_intf_cfg *cfg;
> -	int rc;
>  
>  	c = kzalloc(sizeof(*c), GFP_KERNEL);
>  	if (!c)
> @@ -296,18 +292,9 @@ struct dpu_hw_intf *dpu_hw_intf_init(enum dpu_intf idx,
>  	c->mdss = m;
>  	_setup_intf_ops(&c->ops, c->cap->features);
>  
> -	rc = dpu_hw_blk_init(&c->base, DPU_HW_BLK_INTF, idx, &dpu_hw_ops);
> -	if (rc) {
> -		DPU_ERROR("failed to init hw blk %d\n", rc);
> -		goto blk_init_error;
> -	}
> +	dpu_hw_blk_init(&c->base, DPU_HW_BLK_INTF, idx, &dpu_hw_ops);
>  
>  	return c;
> -
> -blk_init_error:
> -	kzfree(c);
> -
> -	return ERR_PTR(rc);
>  }
>  
>  void dpu_hw_intf_destroy(struct dpu_hw_intf *intf)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c
> index c76b5cc949fb..9f342af2aba7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c
> @@ -175,10 +175,7 @@ static void _setup_mixer_ops(struct dpu_mdss_cfg *m,
>  	ops->setup_gc = dpu_hw_lm_gc;
>  };
>  
> -static struct dpu_hw_blk_ops dpu_hw_ops = {
> -	.start = NULL,
> -	.stop = NULL,
> -};
> +static struct dpu_hw_blk_ops dpu_hw_ops;
>  
>  struct dpu_hw_mixer *dpu_hw_lm_init(enum dpu_lm idx,
>  		void __iomem *addr,
> @@ -186,7 +183,6 @@ struct dpu_hw_mixer *dpu_hw_lm_init(enum dpu_lm idx,
>  {
>  	struct dpu_hw_mixer *c;
>  	struct dpu_lm_cfg *cfg;
> -	int rc;
>  
>  	c = kzalloc(sizeof(*c), GFP_KERNEL);
>  	if (!c)
> @@ -203,18 +199,9 @@ struct dpu_hw_mixer *dpu_hw_lm_init(enum dpu_lm idx,
>  	c->cap = cfg;
>  	_setup_mixer_ops(m, &c->ops, c->cap->features);
>  
> -	rc = dpu_hw_blk_init(&c->base, DPU_HW_BLK_LM, idx, &dpu_hw_ops);
> -	if (rc) {
> -		DPU_ERROR("failed to init hw blk %d\n", rc);
> -		goto blk_init_error;
> -	}
> +	dpu_hw_blk_init(&c->base, DPU_HW_BLK_LM, idx, &dpu_hw_ops);
>  
>  	return c;
> -
> -blk_init_error:
> -	kzfree(c);
> -
> -	return ERR_PTR(rc);
>  }
>  
>  void dpu_hw_lm_destroy(struct dpu_hw_mixer *lm)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> index cfc93816078f..3bdf47ed1845 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> @@ -199,10 +199,7 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong_ops *ops,
>  	ops->get_line_count = dpu_hw_pp_get_line_count;
>  };
>  
> -static struct dpu_hw_blk_ops dpu_hw_ops = {
> -	.start = NULL,
> -	.stop = NULL,
> -};
> +static struct dpu_hw_blk_ops dpu_hw_ops;
>  
>  struct dpu_hw_pingpong *dpu_hw_pingpong_init(enum dpu_pingpong idx,
>  		void __iomem *addr,
> @@ -210,7 +207,6 @@ struct dpu_hw_pingpong *dpu_hw_pingpong_init(enum dpu_pingpong idx,
>  {
>  	struct dpu_hw_pingpong *c;
>  	struct dpu_pingpong_cfg *cfg;
> -	int rc;
>  
>  	c = kzalloc(sizeof(*c), GFP_KERNEL);
>  	if (!c)
> @@ -226,18 +222,9 @@ struct dpu_hw_pingpong *dpu_hw_pingpong_init(enum dpu_pingpong idx,
>  	c->caps = cfg;
>  	_setup_pingpong_ops(&c->ops, c->caps);
>  
> -	rc = dpu_hw_blk_init(&c->base, DPU_HW_BLK_PINGPONG, idx, &dpu_hw_ops);
> -	if (rc) {
> -		DPU_ERROR("failed to init hw blk %d\n", rc);
> -		goto blk_init_error;
> -	}
> +	dpu_hw_blk_init(&c->base, DPU_HW_BLK_PINGPONG, idx, &dpu_hw_ops);
>  
>  	return c;
> -
> -blk_init_error:
> -	kzfree(c);
> -
> -	return ERR_PTR(rc);
>  }
>  
>  void dpu_hw_pingpong_destroy(struct dpu_hw_pingpong *pp)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> index 8297963d2c4c..3ebdf292d8f2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -696,10 +696,7 @@ static struct dpu_sspp_cfg *_sspp_offset(enum dpu_sspp sspp,
>  	return ERR_PTR(-ENOMEM);
>  }
>  
> -static struct dpu_hw_blk_ops dpu_hw_ops = {
> -	.start = NULL,
> -	.stop = NULL,
> -};
> +static struct dpu_hw_blk_ops dpu_hw_ops;
>  
>  struct dpu_hw_pipe *dpu_hw_sspp_init(enum dpu_sspp idx,
>  		void __iomem *addr, struct dpu_mdss_cfg *catalog,
> @@ -707,7 +704,6 @@ struct dpu_hw_pipe *dpu_hw_sspp_init(enum dpu_sspp idx,
>  {
>  	struct dpu_hw_pipe *hw_pipe;
>  	struct dpu_sspp_cfg *cfg;
> -	int rc;
>  
>  	if (!addr || !catalog)
>  		return ERR_PTR(-EINVAL);
> @@ -729,18 +725,9 @@ struct dpu_hw_pipe *dpu_hw_sspp_init(enum dpu_sspp idx,
>  	hw_pipe->cap = cfg;
>  	_setup_layer_ops(hw_pipe, hw_pipe->cap->features);
>  
> -	rc = dpu_hw_blk_init(&hw_pipe->base, DPU_HW_BLK_SSPP, idx, &dpu_hw_ops);
> -	if (rc) {
> -		DPU_ERROR("failed to init hw blk %d\n", rc);
> -		goto blk_init_error;
> -	}
> +	dpu_hw_blk_init(&hw_pipe->base, DPU_HW_BLK_SSPP, idx, &dpu_hw_ops);
>  
>  	return hw_pipe;
> -
> -blk_init_error:
> -	kzfree(hw_pipe);
> -
> -	return ERR_PTR(rc);
>  }
>  
>  void dpu_hw_sspp_destroy(struct dpu_hw_pipe *ctx)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> index f7e0f661a8bb..a041597bb849 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
> @@ -321,10 +321,7 @@ static const struct dpu_mdp_cfg *_top_offset(enum dpu_mdp mdp,
>  	return ERR_PTR(-EINVAL);
>  }
>  
> -static struct dpu_hw_blk_ops dpu_hw_ops = {
> -	.start = NULL,
> -	.stop = NULL,
> -};
> +static struct dpu_hw_blk_ops dpu_hw_ops;
>  
>  struct dpu_hw_mdp *dpu_hw_mdptop_init(enum dpu_mdp idx,
>  		void __iomem *addr,
> @@ -332,7 +329,6 @@ struct dpu_hw_mdp *dpu_hw_mdptop_init(enum dpu_mdp idx,
>  {
>  	struct dpu_hw_mdp *mdp;
>  	const struct dpu_mdp_cfg *cfg;
> -	int rc;
>  
>  	if (!addr || !m)
>  		return ERR_PTR(-EINVAL);
> @@ -354,18 +350,9 @@ struct dpu_hw_mdp *dpu_hw_mdptop_init(enum dpu_mdp idx,
>  	mdp->caps = cfg;
>  	_setup_mdp_ops(&mdp->ops, mdp->caps->features);
>  
> -	rc = dpu_hw_blk_init(&mdp->base, DPU_HW_BLK_TOP, idx, &dpu_hw_ops);
> -	if (rc) {
> -		DPU_ERROR("failed to init hw blk %d\n", rc);
> -		goto blk_init_error;
> -	}
> +	dpu_hw_blk_init(&mdp->base, DPU_HW_BLK_TOP, idx, &dpu_hw_ops);
>  
>  	return mdp;
> -
> -blk_init_error:
> -	kzfree(mdp);
> -
> -	return ERR_PTR(rc);
>  }
>  
>  void dpu_hw_mdp_destroy(struct dpu_hw_mdp *mdp)
> -- 
> 2.18.0
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno