Hi Rob,
On Tue, Mar 16, 2021 at 03:09:10PM -0600, Rob Herring wrote:
> On Tue, Mar 16, 2021 at 2:48 PM Laurent Pinchart wrote:
> > On Tue, Mar 16, 2021 at 01:51:00PM -0600, Rob Herring wrote:
> > > The example in video-interfaces.yaml managed to use a bunch of undocumented
> > > bindings. Update the example to use real bindings (and ones with a schema).
> > >
> > > Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
> > > Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > Cc: linux-media@xxxxxxxxxxxxxxx
> > > Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> > > ---
> > > .../bindings/media/video-interfaces.yaml | 75 ++++++++-----------
> > > 1 file changed, 33 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.yaml b/Documentation/devicetree/bindings/media/video-interfaces.yaml
> > > index 0a7a73fd59f2..f30b9b91717b 100644
> > > --- a/Documentation/devicetree/bindings/media/video-interfaces.yaml
> > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.yaml
> > > @@ -227,17 +227,12 @@ examples:
> > > # only one of the following data pipelines can be active:
> > > # ov772x -> ceu0 or imx074 -> csi2 -> ceu0.
> > > - |
> > > + #include <dt-bindings/clock/r8a7796-cpg-mssr.h>
> > > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > + #include <dt-bindings/power/r8a7796-sysc.h>
> > > +
> > > ceu@fe910000 {
> > > - compatible = "renesas,sh-mobile-ceu";
> > > reg = <0xfe910000 0xa0>;
> > > - interrupts = <0x880>;
> > > -
> > > - mclk: master_clock {
> > > - compatible = "renesas,ceu-clock";
> > > - #clock-cells = <1>;
> > > - clock-frequency = <50000000>; /* Max clock frequency */
> > > - clock-output-names = "mclk";
> > > - };
> > >
> > > port {
> > > #address-cells = <1>;
> > > @@ -271,18 +266,14 @@ examples:
> > > #size-cells = <0>;
> > >
> > > camera@21 {
> > > - compatible = "ovti,ov772x";
> > > + compatible = "ovti,ov7720";
> > > reg = <0x21>;
> > > - vddio-supply = <®ulator1>;
> > > - vddcore-supply = <®ulator2>;
> > > -
> > > - clock-frequency = <20000000>;
> > > clocks = <&mclk 0>;
> > > - clock-names = "xclk";
> > >
> > > port {
> > > /* With 1 endpoint per port no need for addresses. */
> > > ov772x_1_1: endpoint {
> > > + bus-type = <5>;
> > > bus-width = <8>;
> > > remote-endpoint = <&ceu0_1>;
> > > hsync-active = <1>;
> > > @@ -295,48 +286,48 @@ examples:
> > > };
> > >
> > > camera@1a {
> > > - compatible = "sony,imx074";
> > > + compatible = "sony,imx334";
> > > reg = <0x1a>;
> > > - vddio-supply = <®ulator1>;
> > > - vddcore-supply = <®ulator2>;
> > >
> > > - clock-frequency = <30000000>; /* Shared clock with ov772x_1 */
> > > clocks = <&mclk 0>;
> > > - clock-names = "sysclk"; /* Assuming this is the
> > > - name in the datasheet */
> > > +
> > > port {
> > > - imx074_1: endpoint {
> > > + imx334_1: endpoint {
> > > clock-lanes = <0>;
> > > data-lanes = <1 2>;
> > > + link-frequencies = /bits/ 64 <891000000>;
> > > remote-endpoint = <&csi2_1>;
> > > };
> > > };
> > > };
> > > };
> > >
> > > - csi2: csi2@ffc90000 {
> > > - compatible = "renesas,sh-mobile-csi2";
> > > - reg = <0xffc90000 0x1000>;
> > > - interrupts = <0x17a0>;
> > > - #address-cells = <1>;
> > > - #size-cells = <0>;
> > > + csi2@fea80000 {
> > > + compatible = "renesas,r8a7796-csi2";
> >
> > That's certainly better, but the r8a7796 doesn't have a CEU :-) It has a
> > VIN. Maybe we could copy the last example from renesas,vin.yaml to
> > replace the CEU ?
>
> What about just removing the example here? It bothers me to have 2
> copies (maybe 3 with sensor schemas) of an example and we should have
> plenty of examples.
I'd be fine with that.
> On the flip side, it's nice to have this stand on its own. Another
> option would be just remove compatibles and make the example barebones
> with only what's defined in video-interfaces.yaml.
Abstract examples seem good in this context.
> But then it's not validated at all.
But this part isn't nice :-(
If we keep examples that use real bindings, they should match the real
hardware platforms. Other than that, I have no strong preference, it's
up to you.
--
Regards,
Laurent Pinchart
