[RFC v2 1/4] dt-bindings: drm/mediatek: Add Mediatek display subsystem dts binding

[Rob Herring]

On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel at pengutronix.de> wrote:
> Hi Rob,
>
> thank you for the comments.
>
> Am Freitag, den 18.09.2015, 15:33 -0500 schrieb Rob Herring:
> [...]
>> > +The display-subsystem node binds together all individual device nodes that
>> > +comprise the DISP subsystem.
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: "mediatek,<chip>-disp"
>>
>> > +- components: Should contain a list of phandles pointing to the DISP function
>> > +  block device nodes.
>> > +- component-names: Should contain the name of the function block pointed to
>> > +  by the components phandle of the same index.
>>
>> NAK. Group these nodes under a parent node, use of-graph or just don't
>> put this into DT. Don't invent a new way.
>
> Ok. The reason I haven't grouped all the display nodes together in the
> first place is that they aren't contiguous in the register space. So
> instead of:
>
>         ovl at 1400c000 {
>                 compatible = "mediatek,mt8173-disp-ovl";
>         };
>         ...
>         ufoe at 1401a000 {
>                 compatible = "mediatek,mt8173-disp-ufoe";
>         };
>
>         pwm at 1401e000 {
>                 compatible = "mediatek,mt8173-disp-pwm";
>         };
>
>         larb at 14021000 {
>                 compatible = "mediatek,mt8173-smi-larb";
>         };
>
>         od at 14023000 {
>                 compatible = "mediatek,mt8173-disp-od";
>         };
>
> We'd have:
>
>         display-subsystem at 1400c00 {
>                 compatible = "mediatek,mt8173-disp", "simple-bus";
>
>                 ovl at 1400c000 {
>                         compatible = "mediatek,mt8173-disp-ovl";
>                 };
>                 ...
>                 ufoe at 1401a000 {
>                         compatible = "mediatek,mt8173-disp-ufoe";
>                 };
>
>                 od at 14023000 {
>                         compatible = "mediatek,mt8173-disp-od";
>                 };
>         };
>
>         pwm at 1401e000 {
>                 compatible = "mediatek,mt8173-disp-pwm";
>         };
>
>         larb at 14021000 {
>                 compatible = "mediatek,mt8173-smi-larb";
>         };
>
> Note how the display-subsystem node overlaps the larb node. Is that
> acceptable?

Given what the graph looks like, perhaps. However, do you really need
a container node? It only serves to provide a list of nodes (e.g. all
the children) to include as components. There are other ways to
determine this list. You could find all nodes just searching
compatible strings for each component. You just need to bind the drm
driver to some other DT node. Is there no node you can pick as the
master component?

Rob