linux-kernel - Re: [PATCH v5 2/3] drm/bridge: Drop conditionals around of

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 30 Aug 2023 23:38:25 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Biju Das <biju.das.jz@...renesas.com>
Cc:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH v5 2/3] drm/bridge: Drop conditionals around of_node
 pointers

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@...asonboard.com>

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

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@...asonboard.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