RE: [PATCH v2 2/3] media: dt-bindings: media: renesas,drif: Convert to json-schema
Hi Geert,
Thank you for getting back to me.
> From: Geert Uytterhoeven
> Sent: 23 September 2020 14:33
>
> Hi Fabrizio,
>
> On Wed, Sep 23, 2020 at 1:55 PM Fabrizio Castro
> wrote:
> > > From: Geert Uytterhoeven
> > > On Wed, Sep 16, 2020 at 1:00 PM Fabrizio Castro
> > > wrote:
> > > > Convert the Renesas DRIF bindings to DT schema and update
> > > > MAINTAINERS accordingly.
> > > >
> > > > Signed-off-by: Fabrizio Castro
>
> > > > +if:
> > > > + required:
> > > > +- renesas,primary-bond
> > > > +then:
> > > > + required:
> > > > +- pinctrl-0
> > > > +- pinctrl-names
> > > > +- port
> > >
> > > The last 3 properties must not be present for a secondary interface,
> > > right?
> >
> > If you have both channels enabled, then one of the two has to define
> properties:
> > * renesas,primary-bond, pinctrl-0, pinctrl-names, and port.
> >
> > If only one channel is enabled, then the primary bond concept loses its
> value,
> > whether renesas,primary-bond is specified or not doesn’t matter anymore,
> but the
> > enabled node has to specify the pinctrl related properties and the port.
> >
> > > To express that, I think you need to add:
> > >
> > > else:
> > > properties:
> > > - pinctrl-0: false
> > > - pinctrl-names: false
> > > - port: false
> >
> > If I went with this, we would not be able to allow those properties to be
> specified in the
> > only enabled node for single channels configurations.
> >
> > Is there a better way to approach this?
> >
> > I'll wait for this point to get sorted before sending v3 out.
>
> The old binding said:
>
> -Required properties of an internal channel when:
> - - It is the only enabled channel of the bond (or)
> - - If it acts as primary among enabled bonds
>
> -- renesas,primary-bond: empty property indicating the channel
> acts as primary
> - among the bonded channels.
>
> so renesas,primary-bond is required for the only enabled node for
> single channels configurations anyway?
The old binding also said:
"
When only one of the bonded channels need to be enabled, the property
"renesas,bonding" or "renesas,primary-bond" will have no effect. That
enabled channel can act alone as any other independent device.
"
If you then look at the driver implementation, the renesas,primary-bond
property doesn’t get evaluated if you have only one channel enabled.
Thanks,
Fab
>
> 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
Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch,
Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472
Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB
3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg.
no.: DE 14978647
Re: [PATCH v2 2/3] media: dt-bindings: media: renesas,drif: Convert to json-schema
Hi Fabrizio,
On Wed, Sep 23, 2020 at 1:55 PM Fabrizio Castro
wrote:
> > From: Geert Uytterhoeven
> > On Wed, Sep 16, 2020 at 1:00 PM Fabrizio Castro
> > wrote:
> > > Convert the Renesas DRIF bindings to DT schema and update
> > > MAINTAINERS accordingly.
> > >
> > > Signed-off-by: Fabrizio Castro
> > > +if:
> > > + required:
> > > +- renesas,primary-bond
> > > +then:
> > > + required:
> > > +- pinctrl-0
> > > +- pinctrl-names
> > > +- port
> >
> > The last 3 properties must not be present for a secondary interface,
> > right?
>
> If you have both channels enabled, then one of the two has to define
> properties:
> * renesas,primary-bond, pinctrl-0, pinctrl-names, and port.
>
> If only one channel is enabled, then the primary bond concept loses its value,
> whether renesas,primary-bond is specified or not doesn’t matter anymore, but
> the
> enabled node has to specify the pinctrl related properties and the port.
>
> > To express that, I think you need to add:
> >
> > else:
> > properties:
> > - pinctrl-0: false
> > - pinctrl-names: false
> > - port: false
>
> If I went with this, we would not be able to allow those properties to be
> specified in the
> only enabled node for single channels configurations.
>
> Is there a better way to approach this?
>
> I'll wait for this point to get sorted before sending v3 out.
The old binding said:
-Required properties of an internal channel when:
- - It is the only enabled channel of the bond (or)
- - If it acts as primary among enabled bonds
-- renesas,primary-bond: empty property indicating the channel
acts as primary
- among the bonded channels.
so renesas,primary-bond is required for the only enabled node for
single channels configurations anyway?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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: [PATCH v2 2/3] media: dt-bindings: media: renesas,drif: Convert to json-schema
Hi Geert,
Thank you for your feedback!
> From: Geert Uytterhoeven
> Sent: 23 September 2020 11:07
>
> Hi Fabrizio,
>
> On Wed, Sep 16, 2020 at 1:00 PM Fabrizio Castro
> wrote:
> > Convert the Renesas DRIF bindings to DT schema and update
> > MAINTAINERS accordingly.
> >
> > Signed-off-by: Fabrizio Castro
> > Reviewed-by: Lad Prabhakar lad...@bp.renesas.com>
> > Reviewed-by: Laurent Pinchart
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,drif.yaml
>
> > +properties:
>
> > + pinctrl-0:
> > +maxItems: 1
> > +
> > + pinctrl-names:
> > +maxItems: 1
> > +items:
> > + - const: default
>
> I think these are added automatically by the tooling, so there is no
> need to list them explicitly.
> Or do you need to list them here because of the required logic below?
I have now tried taking pinctrl-0 and pinctrl-names properties out, and things
seem to be okay, I'll take them out in v3.
>
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - resets
> > + - dmas
> > + - dma-names
> > + - renesas,bonding
>
> Missing "power-domains".
I'll add it in v3
>
> > +
> > +if:
> > + required:
> > +- renesas,primary-bond
> > +then:
> > + required:
> > +- pinctrl-0
> > +- pinctrl-names
> > +- port
>
> The last 3 properties must not be present for a secondary interface,
> right?
If you have both channels enabled, then one of the two has to define properties:
* renesas,primary-bond, pinctrl-0, pinctrl-names, and port.
If only one channel is enabled, then the primary bond concept loses its value,
whether renesas,primary-bond is specified or not doesn’t matter anymore, but the
enabled node has to specify the pinctrl related properties and the port.
> To express that, I think you need to add:
>
> else:
> properties:
> - pinctrl-0: false
> - pinctrl-names: false
> - port: false
If I went with this, we would not be able to allow those properties to be
specified in the
only enabled node for single channels configurations.
Is there a better way to approach this?
I'll wait for this point to get sorted before sending v3 out.
Thanks,
Fab
>
> 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
Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch,
Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472
Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB
3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg.
no.: DE 14978647
Re: [PATCH v2 2/3] media: dt-bindings: media: renesas,drif: Convert to json-schema
Hi Fabrizio,
On Wed, Sep 16, 2020 at 1:00 PM Fabrizio Castro
wrote:
> Convert the Renesas DRIF bindings to DT schema and update
> MAINTAINERS accordingly.
>
> Signed-off-by: Fabrizio Castro
> Reviewed-by: Lad Prabhakar
> Reviewed-by: Laurent Pinchart
Thanks for your patch!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,drif.yaml
> +properties:
> + pinctrl-0:
> +maxItems: 1
> +
> + pinctrl-names:
> +maxItems: 1
> +items:
> + - const: default
I think these are added automatically by the tooling, so there is no
need to list them explicitly.
Or do you need to list them here because of the required logic below?
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - resets
> + - dmas
> + - dma-names
> + - renesas,bonding
Missing "power-domains".
> +
> +if:
> + required:
> +- renesas,primary-bond
> +then:
> + required:
> +- pinctrl-0
> +- pinctrl-names
> +- port
The last 3 properties must not be present for a secondary interface,
right? To express that, I think you need to add:
else:
properties:
- pinctrl-0: false
- pinctrl-names: false
- port: false
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
[PATCH v2 2/3] media: dt-bindings: media: renesas,drif: Convert to json-schema
Convert the Renesas DRIF bindings to DT schema and update MAINTAINERS accordingly. Signed-off-by: Fabrizio Castro Reviewed-by: Lad Prabhakar Reviewed-by: Laurent Pinchart --- v1->v2: * s/controller/Controller/ in the title of renesas,drif.yaml as suggested by Laurent. .../bindings/media/renesas,drif.txt | 177 .../bindings/media/renesas,drif.yaml | 270 ++ MAINTAINERS | 2 +- 3 files changed, 271 insertions(+), 178 deletions(-) delete mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.yaml diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt b/Documentation/devicetree/bindings/media/renesas,drif.txt deleted file mode 100644 index 0d8974aa8b38.. --- a/Documentation/devicetree/bindings/media/renesas,drif.txt +++ /dev/null @@ -1,177 +0,0 @@ -Renesas R-Car Gen3 Digital Radio Interface controller (DRIF) - - -R-Car Gen3 DRIF is a SPI like receive only slave device. A general -representation of DRIF interfacing with a master device is shown below. - -+-++-+ -| |-SCK--->|CLK | -| Master|-SS>|SYNC DRIFn (slave) | -| |-SD0--->|D0 | -| |-SD1--->|D1 | -+-++-+ - -As per datasheet, each DRIF channel (drifn) is made up of two internal -channels (drifn0 & drifn1). These two internal channels share the common -CLK & SYNC. Each internal channel has its own dedicated resources like -irq, dma channels, address space & clock. This internal split is not -visible to the external master device. - -The device tree model represents each internal channel as a separate node. -The internal channels sharing the CLK & SYNC are tied together by their -phandles using a property called "renesas,bonding". For the rest of -the documentation, unless explicitly stated, the word channel implies an -internal channel. - -When both internal channels are enabled they need to be managed together -as one (i.e.) they cannot operate alone as independent devices. Out of the -two, one of them needs to act as a primary device that accepts common -properties of both the internal channels. This channel is identified by a -property called "renesas,primary-bond". - -To summarize, - - When both the internal channels that are bonded together are enabled, - the zeroth channel is selected as primary-bond. This channels accepts - properties common to all the members of the bond. - - When only one of the bonded channels need to be enabled, the property - "renesas,bonding" or "renesas,primary-bond" will have no effect. That - enabled channel can act alone as any other independent device. - -Required properties of an internal channel: -- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of R8A7795 SoC. - "renesas,r8a7796-drif" if DRIF controller is a part of R8A7796 SoC. - "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible device. - - When compatible with the generic version, nodes must list the - SoC-specific version corresponding to the platform first - followed by the generic version. - -- reg: offset and length of that channel. -- interrupts: associated with that channel. -- clocks: phandle and clock specifier of that channel. -- clock-names: clock input name string: "fck". -- dmas: phandles to the DMA channels. -- dma-names: names of the DMA channel: "rx". -- renesas,bonding: phandle to the other channel. - -Optional properties of an internal channel: -- power-domains: phandle to the respective power domain. - -Required properties of an internal channel when: - - It is the only enabled channel of the bond (or) - - If it acts as primary among enabled bonds - -- pinctrl-0: pin control group to be used for this channel. -- pinctrl-names: must be "default". -- renesas,primary-bond: empty property indicating the channel acts as primary - among the bonded channels. -- port: child port node corresponding to the data input, in accordance with - the video interface bindings defined in - Documentation/devicetree/bindings/media/video-interfaces.txt. The port - node must contain at least one endpoint. - -Optional endpoint property: -- sync-active: Indicates sync signal polarity, 0/1 for low/high respectively. - This property maps to SYNCAC bit in the hardware manual. The -
