Received-SPF: pass (google.com: domain of linux-kernel+bounces-96092-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <45f59f31-1f03-4a96-adb6-25c7cdd5e8a1@linux.dev>
Date: Fri, 8 Mar 2024 03:20:07 +0800
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v2 1/4] drm/bridge: Add fwnode based helpers to get the
 next bridge
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>,
 Neil Armstrong <neil.armstrong@linaro.org>, Robert Foss <rfoss@kernel.org>,
 Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
 Jonas Karlman <jonas@kwiboo.se>, Jernej Skrabec <jernej.skrabec@gmail.com>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>,
 David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
 Phong LE <ple@baylibre.com>, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org
References: <20240307172334.1753343-1-sui.jingfeng@linux.dev>
 <20240307172334.1753343-2-sui.jingfeng@linux.dev>
 <CAA8EJpp8tsHi0RhsJXG+r6nOsV3AUC_n6jNHL0Cr6Ku2h3NMog@mail.gmail.com>
Content-Language: en-US
From: Sui Jingfeng <sui.jingfeng@linux.dev>
In-Reply-To: <CAA8EJpp8tsHi0RhsJXG+r6nOsV3AUC_n6jNHL0Cr6Ku2h3NMog@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi,


On 2024/3/8 02:43, Dmitry Baryshkov wrote:
>> +
>>   MODULE_AUTHOR("Ajay Kumar<ajaykumar.rs@samsung.com>");
>>   MODULE_DESCRIPTION("DRM bridge infrastructure");
>>   MODULE_LICENSE("GPL and additional rights");
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index 3606e1a7f965..d4c95afdd662 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -26,6 +26,7 @@
>>   #include <linux/ctype.h>
>>   #include <linux/list.h>
>>   #include <linux/mutex.h>
>> +#include <linux/of.h>
>>
>>   #include <drm/drm_atomic.h>
>>   #include <drm/drm_encoder.h>
>> @@ -721,6 +722,8 @@ struct drm_bridge {
>>          struct list_head chain_node;
>>          /** @of_node: device node pointer to the bridge */
>>          struct device_node *of_node;
> In my opinion, if you are adding fwnode, we can drop of_node
> completely. There is no need to keep both of them.


But the 'struct device' have both of them contained, we should *follow the core*, right?
They are two major firmware subsystems (DT and ACPT), both are great and large, right?
Personally, I think the drm_bridge should embeds 'struct device', after all, drm bridge
are mainly stand for display bridges device. And also to reflect what you said: "to
reflect the hardware perfectly" and remove some boilerplate.

I think I'm not good enough to do such a big refactor, sorry. I believe that Maxime
and Laurent are the advanced programmers who is good enough to do such things, maybe
you can ask them for help?

Beside this, other reviews are acceptable and will be fixed at the next version,
thanks a lot for review.


Best Regards,
Sui