Re: [PATCH v3 00/19] Add Freescale i.MX8qxp Display Controller support

[Sui Jingfeng]

Hi,

On 7/28/24 00:39, Dmitry Baryshkov wrote:
> > Hi,
> >
> > This patch series aims to add Freescale i.MX8qxp Display Controller support.
> >
> > The controller is comprised of three main components that include a blit
> > engine for 2D graphics accelerations, display controller for display output
> > processing, as well as a command sequencer.
> >
> > Previous patch series attempts to do that can be found at:
> > https://patchwork.freedesktop.org/series/84524/
> >
> > This series addresses Maxime's comments on the previous one:
> > a. Split the display controller into multiple internal devices.
> >     1) List display engine, pixel engine, interrupt controller and more as the
> >        controller's child devices.
> >     2) List display engine and pixel engine's processing units as their child
> >        devices.
> >
> > b. Add minimal feature support.
> >     Only support two display pipelines with primary planes with XR24 fb,
> >     backed by two fetchunits.  No fetchunit dynamic allocation logic(to be done
> >     when necessary).
> >
> > c. Use drm_dev_{enter, exit}().
> >
> > Since this series changes a lot comparing to the previous one, I choose to
> > send it with a new patch series, not a new version.
> I'm sorry, I have started reviewing v2 without noticing that there is a
> v3 already.
>
> Let me summarize my comments:
>
> - You are using OF aliases. Are they documented and acked by DT
>    maintainers?
>
> - I generally feel that the use of so many small devices to declare
>    functional blocks is an abuse of the DT. Please consider creating
>    _small_ units from the driver code directly rather than going throught
>    the components. 

Well, I really don't think so. I don't agree.

I have checked the DTSpec[1] before type, the spec isn't define how
many is considered to be "many", and the spec isn't define to what
extent is think to be "small" as well.

[1] 
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.4

--
Best regards
Sui