Re: [PATCH v2 1/4] drm/bridge: Add fwnode based helpers to get the next bridge

[Sui Jingfeng]

Hi,


On 2024/3/8 02:43, Dmitry Baryshkov wrote:
> > +
> >   MODULE_AUTHOR("Ajay Kumar<ajaykumar.rs@xxxxxxxxxxx>");
> >   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