Re: [RESEND PATCH v4 3/3] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

Re: [RESEND PATCH v4 3/3] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Geert Uytterhoeven]

Hi Ricardo,

On Tue, Jun 15, 2021 at 3:15 PM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
> Convert the ADV7511/11w/13/33/35 DT bindings to json-schema. The
> original binding has been split into two files: adi,adv7511.yaml for
> ADV7511/11W/13 and adi,adv7533.yaml for ADV7533/35.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks for your patch, which is now commit cfe34bb7a770c5d8
("dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml")
in robh/for-next.

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml

> +required:
> +  - compatible
> +  - reg
> +  - ports
> +  - adi,input-depth
> +  - adi,input-colorspace
> +  - adi,input-clock
> +  - avdd-supply
> +  - dvdd-supply
> +  - pvdd-supply
> +  - dvdd-3v-supply
> +  - bgvdd-supply

This causes lots of failures like:

    arm/boot/dts/r8a7743-iwg20d-q7-dbcm-ca.dt.yaml: hdmi@39:
'avdd-supply' is a required property

Should all supplies be required?
Looking at the driver, missing supplies are automatically replaced by
dummy regulators by the regulator framework.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RESEND PATCH v4 3/3] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Laurent Pinchart]

Hi Geert,

On Fri, Jun 18, 2021 at 09:55:38AM +0200, Geert Uytterhoeven wrote:
> On Tue, Jun 15, 2021 at 3:15 PM Ricardo Cañuelo wrote:
> > Convert the ADV7511/11w/13/33/35 DT bindings to json-schema. The
> > original binding has been split into two files: adi,adv7511.yaml for
> > ADV7511/11W/13 and adi,adv7533.yaml for ADV7533/35.
> >
> > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks for your patch, which is now commit cfe34bb7a770c5d8
> ("dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml")
> in robh/for-next.
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml
> 
> > +required:
> > +  - compatible
> > +  - reg
> > +  - ports
> > +  - adi,input-depth
> > +  - adi,input-colorspace
> > +  - adi,input-clock
> > +  - avdd-supply
> > +  - dvdd-supply
> > +  - pvdd-supply
> > +  - dvdd-3v-supply
> > +  - bgvdd-supply
> 
> This causes lots of failures like:
> 
>     arm/boot/dts/r8a7743-iwg20d-q7-dbcm-ca.dt.yaml: hdmi@39:
> 'avdd-supply' is a required property
> 
> Should all supplies be required?
> Looking at the driver, missing supplies are automatically replaced by
> dummy regulators by the regulator framework.

Generally speaking, I like DT bindings to be descriptive of the
hardware, and thus require power supplies that are needed for the device
to function, even if they are fixed supplies.

This being said, I think there's also room to group some power supplies
together in the bindings, when they are not meant by the device to be
controlled separately. In this specific case, we also need to take into
account that the adv7511 and adv7533 have different supplies.

-- 
Regards,

Laurent Pinchart

Re: [RESEND PATCH v4 3/3] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml

[Ricardo Cañuelo]

Hi,
 
> On Fri, Jun 18, 2021 at 09:55:38AM +0200, Geert Uytterhoeven wrote:
> > This causes lots of failures like:
> > 
> >     arm/boot/dts/r8a7743-iwg20d-q7-dbcm-ca.dt.yaml: hdmi@39:
> > 'avdd-supply' is a required property
> > 
> > Should all supplies be required?
> > Looking at the driver, missing supplies are automatically replaced by
> > dummy regulators by the regulator framework.
> 
> Generally speaking, I like DT bindings to be descriptive of the
> hardware, and thus require power supplies that are needed for the device
> to function, even if they are fixed supplies.
> 
> This being said, I think there's also room to group some power supplies
> together in the bindings, when they are not meant by the device to be
> controlled separately. In this specific case, we also need to take into
> account that the adv7511 and adv7533 have different supplies.

Thanks for the review, guys. Yes, there were some dtbs check warnings to
be expected, the consensus in a previous version of the patch was that
that shouldn't be a blocker for a binding conversion and that the *.dts
definitions should eventually be fixed to comply with the binding, which
is, IMO, a more reasonable process to keep the binding conversion
effort progressing.

Cheers,
Ricardo