[PATCH] drm/debugfs: Drop conditionals around of_node pointers

[PATCH] drm/debugfs: Drop conditionals around of_node pointers

[Sui Jingfeng]

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 debugfs.

Fixes: d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers")
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/drm_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 08fcefd804bc..28a471fe4bc8 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -597,10 +597,10 @@ static int bridges_show(struct seq_file *m, void *data)
 		drm_printf(&p, "\ttype: [%d] %s\n",
 			   bridge->type,
 			   drm_get_connector_type_name(bridge->type));
-#ifdef CONFIG_OF
+
 		if (bridge->of_node)
 			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
-#endif
+
 		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
 		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
 			drm_puts(&p, " detect");
-- 
2.34.1

Re: drm/debugfs: Drop conditionals around of_node pointers

[Sui Jingfeng]

ping

在 2024/3/22 06:22, Sui Jingfeng 写道:
> 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 debugfs.
> 
> Fixes: d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers")
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>   drivers/gpu/drm/drm_debugfs.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 08fcefd804bc..28a471fe4bc8 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -597,10 +597,10 @@ static int bridges_show(struct seq_file *m, void *data)
>   		drm_printf(&p, "\ttype: [%d] %s\n",
>   			   bridge->type,
>   			   drm_get_connector_type_name(bridge->type));
> -#ifdef CONFIG_OF
> +
>   		if (bridge->of_node)
>   			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
> -#endif
> +
>   		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
>   		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
>   			drm_puts(&p, " detect");

-- 
Best regards
Sui Jingfeng

Re: [PATCH] drm/debugfs: Drop conditionals around of_node pointers

[Dmitry Baryshkov]

On Fri, Mar 22, 2024 at 06:22:58AM +0800, Sui Jingfeng wrote:
> 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 debugfs.
> 
> Fixes: d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers")
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/drm_debugfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: drm/debugfs: Drop conditionals around of_node pointers

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Sun, Apr 28, 2024 at 04:52:13PM +0800, Sui Jingfeng wrote:
> ping
> 
> 在 2024/3/22 06:22, Sui Jingfeng 写道:
> > Having conditional around the of_node pointer of the drm_bridge structure
> > turns out to make driver code use ugly #ifdef blocks.

The code being ugly is an opinion, what problem is it causing exactly?

> Drop the conditionals to simplify debugfs.

What does it simplifies?

> > 
> > Fixes: d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers")
> > Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>

Why do we want to backport that patch to stable?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: Re: drm/debugfs: Drop conditionals around of_node pointers

[隋景峰]

Hi,


> -----原始邮件-----
> 发件人: "Maxime Ripard" <mripard@kernel.org>
> 发送时间: 2024-04-29 19:30:24 (星期一)
> 收件人: "Sui Jingfeng" <suijingfeng@bosc.ac.cn>
> 抄送: "Sui Jingfeng" <sui.jingfeng@linux.dev>, "Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>, "Thomas Zimmermann" <tzimmermann@suse.de>, "David Airlie" <airlied@gmail.com>, "Daniel Vetter" <daniel@ffwll.ch>, "Douglas Anderson" <dianders@chromium.org>, "Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>, "Biju Das" <biju.das.jz@bp.renesas.com>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
> 主题: Re: drm/debugfs: Drop conditionals around of_node pointers
> 
> On Sun, Apr 28, 2024 at 04:52:13PM +0800, Sui Jingfeng wrote:
> > ping
> > 
> > 在 2024/3/22 06:22, Sui Jingfeng 写道:
> > > Having conditional around the of_node pointer of the drm_bridge structure
> > > turns out to make driver code use ugly #ifdef blocks.
> 
> The code being ugly is an opinion, what problem is it causing exactly?
> 
> > Drop the conditionals to simplify debugfs.
> 
> What does it simplifies?
> 
> > > 
> > > Fixes: d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers")
> > > Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> 
> Why do we want to backport that patch to stable?

My commit message is written based on commit of d8dfccde2709

$ git show c9e358dfc4a8
    
    This patch is based on commit c9e358dfc4a8 ("driver-core: remove
    conditionals around devicetree pointers").
    
    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.
    
    So drop #if conditionals by adding struct device_node forward declaration.

> Maxime

I'm just start to contribute by mimic other people's tone, there seems no need
to over read.

Re: Re: drm/debugfs: Drop conditionals around of_node pointers

[Doug Anderson]

Hi,

On Mon, Apr 29, 2024 at 6:16 PM 隋景峰 <suijingfeng@bosc.ac.cn> wrote:
>
> Hi,
>
>
> > -----原始邮件-----
> > 发件人: "Maxime Ripard" <mripard@kernel.org>
> > 发送时间: 2024-04-29 19:30:24 (星期一)
> > 收件人: "Sui Jingfeng" <suijingfeng@bosc.ac.cn>
> > 抄送: "Sui Jingfeng" <sui.jingfeng@linux.dev>, "Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>, "Thomas Zimmermann" <tzimmermann@suse.de>, "David Airlie" <airlied@gmail.com>, "Daniel Vetter" <daniel@ffwll.ch>, "Douglas Anderson" <dianders@chromium.org>, "Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>, "Biju Das" <biju.das.jz@bp.renesas.com>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
> > 主题: Re: drm/debugfs: Drop conditionals around of_node pointers
> >
> > On Sun, Apr 28, 2024 at 04:52:13PM +0800, Sui Jingfeng wrote:
> > > ping
> > >
> > > 在 2024/3/22 06:22, Sui Jingfeng 写道:
> > > > Having conditional around the of_node pointer of the drm_bridge structure
> > > > turns out to make driver code use ugly #ifdef blocks.
> >
> > The code being ugly is an opinion, what problem is it causing exactly?
> >
> > > Drop the conditionals to simplify debugfs.
> >
> > What does it simplifies?
> >
> > > >
> > > > Fixes: d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers")
> > > > Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> >
> > Why do we want to backport that patch to stable?

Technically it's not CCing stable and so it's not really incorrect.
...but I agree that this is a bit of a stretch to call it a "Fix".
Maybe drop the "Fixes" line?


> My commit message is written based on commit of d8dfccde2709
>
> $ git show c9e358dfc4a8
>
>     This patch is based on commit c9e358dfc4a8 ("driver-core: remove
>     conditionals around devicetree pointers").
>
>     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.
>
>     So drop #if conditionals by adding struct device_node forward declaration.
>
> > Maxime
>
> I'm just start to contribute by mimic other people's tone, there seems no need
> to over read.

I think the fact that you skipped the reference to commit c9e358dfc4a8
("driver-core: remove conditionals around devicetree pointers") was
relevant here. Referencing that commit makes it easy for the reader to
see that you are following convention used throughout the kernel and
not just asserting your own opinion about style.

If you add that reference into your commit message and send a v2, I'm
happy to apply it.

-Doug

Re: drm/debugfs: Drop conditionals around of_node pointers

[Sui Jingfeng]

Hi,


On 2024/5/1 05:33, Doug Anderson wrote:
> Hi,
>
> On Mon, Apr 29, 2024 at 6:16 PM 隋景峰 <suijingfeng@bosc.ac.cn> wrote:
>> Hi,
>>
>>
>>> -----原始邮件-----
>>> 发件人: "Maxime Ripard" <mripard@kernel.org>
>>> 发送时间: 2024-04-29 19:30:24 (星期一)
>>> 收件人: "Sui Jingfeng" <suijingfeng@bosc.ac.cn>
>>> 抄送: "Sui Jingfeng" <sui.jingfeng@linux.dev>, "Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>, "Thomas Zimmermann" <tzimmermann@suse.de>, "David Airlie" <airlied@gmail.com>, "Daniel Vetter" <daniel@ffwll.ch>, "Douglas Anderson" <dianders@chromium.org>, "Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>, "Biju Das" <biju.das.jz@bp.renesas.com>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
>>> 主题: Re: drm/debugfs: Drop conditionals around of_node pointers
>>>
>>> On Sun, Apr 28, 2024 at 04:52:13PM +0800, Sui Jingfeng wrote:
>>>> ping
>>>>
>>>> 在 2024/3/22 06:22, Sui Jingfeng 写道:
>>>>> Having conditional around the of_node pointer of the drm_bridge structure
>>>>> turns out to make driver code use ugly #ifdef blocks.
>>> The code being ugly is an opinion, what problem is it causing exactly?
>>>
>>>> Drop the conditionals to simplify debugfs.
>>> What does it simplifies?
>>>
>>>>> Fixes: d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers")
>>>>> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
>>> Why do we want to backport that patch to stable?
> Technically it's not CCing stable and so it's not really incorrect.
> ...but I agree that this is a bit of a stretch to call it a "Fix".
> Maybe drop the "Fixes" line?


OK, good idea, acceptable.

Originally, I add the fix tag to hint that my modification belongs to
the commit d8dfccde2709 ("drm/bridge: Drop conditionals around of_node pointers") too.
But get missed.


>
>> My commit message is written based on commit of d8dfccde2709
>>
>> $ git show c9e358dfc4a8
>>
>>      This patch is based on commit c9e358dfc4a8 ("driver-core: remove
>>      conditionals around devicetree pointers").
>>
>>      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.
>>
>>      So drop #if conditionals by adding struct device_node forward declaration.
>>
>>> Maxime
>> I'm just start to contribute by mimic other people's tone, there seems no need
>> to over read.
> I think the fact that you skipped the reference to commit c9e358dfc4a8
> ("driver-core: remove conditionals around devicetree pointers") was
> relevant here. Referencing that commit makes it easy for the reader to
> see that you are following convention used throughout the kernel and
> not just asserting your own opinion about style.
>
> If you add that reference into your commit message and send a v2, I'm
> happy to apply it.


OK, thanks a lot.


> -Doug

-- 
Best regards,
Sui