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@xxxxxxxxxxxxxxxx>
> Suggested-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> 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
