Re: [PATCH 05/11] drm/msm/dpu: Cleanup some container_of helper functions

[Sam Ravnborg <sam@ravnborg.org>]

Hi Jordan.

> -/**
> - * to_dpu_hw_mixer - convert base object dpu_hw_base to container
> - * @hw: Pointer to base hardware block
> - * return: Pointer to hardware block container
> - */
> -static inline struct dpu_hw_mixer *to_dpu_hw_mixer(struct dpu_hw_blk *hw)
> -{
> -	return container_of(hw, struct dpu_hw_mixer, base);
> -}
This is a nicely documented static inline that when used provides type checks.

>  
> +#define to_dpu_hw_mixer(hw) container_of((hw), struct dpu_hw_mixer, base)
> +
Replaced by an undocumented define without the same obvious typecheck.

This looks like a step backward.


>  /**
>   * _dpu_rm_check_lm_and_get_connected_blks - check if proposed layer mixer meets
>   *	proposed use case requirements, incl. hardwired dependent blocks like
> @@ -566,7 +568,8 @@ static int _dpu_rm_reserve_ctls(
>  
>  	dpu_rm_init_hw_iter(&iter, 0, DPU_HW_BLK_CTL);
>  	while (_dpu_rm_get_hw_locked(rm, &iter)) {
> -		const struct dpu_hw_ctl *ctl = to_dpu_hw_ctl(iter.blk->hw);
> +		const struct dpu_hw_ctl *ctl =
> +			container_of(iter.blk->hw, struct dpu_hw_ctl, base);
And here you have an open coded container_of where there before
was a nice little properly named helper function.

This does not help readability.

Drop the unused stuff, but sometimes small single line helper functions
makes code more readable even if only used once.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel