Re: [PATCH v5 2/3] drm/bridge: Drop conditionals around of_node pointers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Biju,

Thank you for the patch.

In the commit message, s/pointers/pointer/ as you're only touching a
single one.

On Wed, Aug 30, 2023 at 06:08:18PM +0100, Biju Das wrote:
> The commit c9e358dfc4a8 ("driver-core: remove conditionals around
> devicetree pointers") supposed to remove conditionals around of_node
> pointer, but it missed out drm/bridge.h. So drop #if conditionals by
> adding struct device_node forward declaration.

You can hardly blame commit c9e358dfc4a8 for forgetting drm_bridge, as
that commit dates back from 2011 and the drm_bridge of_node field was
added in 2015 :-)

I would simply copy the rationale from the commit message of
c9e358dfc4a8 and write something like

--------
Having conditional around the of_node pointer of the drm_bridge
structure turns out to make driver code use ugly #ifdef blocks. Drop the
conditionals to simplify drivers. While this slightly increases the size
of struct drm_bridge on non-OF system, the number of bridges used today
and foreseen tomorrow on those systems is very low, so this shouldn't be
an issue.
--------

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> Suggested-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> v5:
>  * Split from patch#2
>  * Updated commit description
>  * Added struct device_node forward declaration.
> ---
>  include/drm/drm_bridge.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index c339fc85fd07..843736627f60 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -32,6 +32,8 @@
>  #include <drm/drm_mode_object.h>
>  #include <drm/drm_modes.h>
>  
> +struct device_node;
> +
>  struct drm_bridge;
>  struct drm_bridge_timings;
>  struct drm_connector;
> @@ -716,10 +718,8 @@ struct drm_bridge {
>  	struct drm_encoder *encoder;
>  	/** @chain_node: used to form a bridge chain */
>  	struct list_head chain_node;
> -#ifdef CONFIG_OF
>  	/** @of_node: device node pointer to the bridge */
>  	struct device_node *of_node;
> -#endif
>  	/** @list: to keep track of all added bridges */
>  	struct list_head list;
>  	/**

-- 
Regards,

Laurent Pinchart