Re: [PATCH v2 01/15] dt-bindings: display: mediatek: ethdr: Add compatible for MT8188

[AngeloGioacchino Del Regno]

Il 15/06/23 07:51, Shawn Sung (宋孝謙) ha scritto:
> Hi Angelo,
>   
> Yes, MT8188 ETHDR is fully compatible with MT8195, so we didn't add its
> compatible name to the driver but just listed it in dt-bindings.
>   
> May I double check with you that
> I see there is a short description regarding "items" object:
> > # items is a list of possible values for the property. The number of
> > # values is determined by the number of elements in the list.
> > # Order in lists is significant, order in dicts is not
> in https://docs.kernel.org/devicetree/bindings/writing-schema.html
>   
> So does the compatible has to be:
> "mediatek,mt8188-disp-ethdr", "mediatek,mt8195-disp-
> ethdr" rather than in reversed order?
>   
> However, I couldn't find any description mentions the order in
> Documentation/devicetree/bindings/writing-schema.rst
> Not sure if the order in "items" object does not matter after then.
>   

No the order doesn't matter in that sense. If you check the other bindings,
you'll see that the rule is to order by name, which your change as well does.

It's fine.

P.S.: In case you didn't know, `make dt_binding_check` and `make dtbs_check`
      will validate your bindings and your devicetrees against the bindings.

Regards,
Angelo

> Thanks,
> Hsiao Chien Sung
>
>
> On Wed, 2023-06-14 at 13:41 +0200, AngeloGioacchino Del Regno wrote:
> >   	
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >   Il 14/06/23 09:31, Hsiao Chien Sung ha scritto:
> > > Add compatible name for MediaTek MT8188 ETHDR.
> > >
> > > Signed-off-by: Hsiao Chien Sung <shawn.sung@xxxxxxxxxxxx>
> > > ---
> > >    .../bindings/display/mediatek/mediatek,ethdr.yaml           | 6
> > +++++-
> > >    1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> > aml
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> > aml
> > > index 801fa66ae615..677882348ede 100644
> > > ---
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> > aml
> > > +++
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> > aml
> > > @@ -23,7 +23,11 @@ description:
> > >
> > >    properties:
> > >      compatible:
> > > -    const: mediatek,mt8195-disp-ethdr
> > > +    oneOf:
> > > +      - const: mediatek,mt8195-disp-ethdr
> > > +      - items:
> > > +          - const: mediatek,mt8188-disp-ethdr
> > > +          - const: mediatek,mt8195-disp-ethdr
> >
> > Is MT8188's ETHDR fully compatible with MT8195's ETHDR?
> >
> > If it is, you're not adding a mt8188 specific compatible string in
> > the driver and
> > this means that the devicetree will look like:
> >
> > compatible = "mediatek,mt8195-disp-ethdr", "mediatek,mt8188-disp-
> > ethdr"
> >
> > ...so the proposed doc change works.
> >
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@xxxxxxxxxxxxx>
> >
> > >      reg:
> > >        maxItems: 7
> > > --
> > > 2.18.0