[v2] dt-bindings: drm: rcar-du: Document optional reset properties
diff mbox

Message ID 1488817556-21410-1-git-send-email-geert+renesas@glider.be
State New
Headers show

Commit Message

Geert Uytterhoeven March 6, 2017, 4:25 p.m. UTC
Document the optional properties for describing module resets, to
support resetting display channels and LVDS encoders on R-Car Gen2 and
Gen3.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
See "[v2,1/4] dt-bindings: clock: renesas: cpg-mssr: Document reset control
support" (https://patchwork.kernel.org/patch/9536627/) for the format of
a reset specifier in the Renesas CPG/MSSR case.

E.g. "resets = <&cpg 310>;"

v2:
  - s/phandles/phandle/.
---
 Documentation/devicetree/bindings/display/renesas,du.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Rob Herring March 15, 2017, 5:01 p.m. UTC | #1
On Mon, Mar 06, 2017 at 05:25:56PM +0100, Geert Uytterhoeven wrote:
> Document the optional properties for describing module resets, to
> support resetting display channels and LVDS encoders on R-Car Gen2 and
> Gen3.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> See "[v2,1/4] dt-bindings: clock: renesas: cpg-mssr: Document reset control
> support" (https://patchwork.kernel.org/patch/9536627/) for the format of
> a reset specifier in the Renesas CPG/MSSR case.
> 
> E.g. "resets = <&cpg 310>;"
> 
> v2:
>   - s/phandles/phandle/.
> ---
>  Documentation/devicetree/bindings/display/renesas,du.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> index 1a02f099a0ff0a3a..3db418c827193e82 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> @@ -36,6 +36,16 @@ Required Properties:
>        When supplied they must be named "dclkin.x" with "x" being the input
>        clock numerical index.
>  
> +Optional properties:
> +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
> +    the reset-names property.
> +  - reset-names: Names of the resets. This property is model-dependent.
> +    - R8A779[0123456] use one reset for a group of one or more successive
> +      channels, and one reset per LVDS encoder (if available). The resets
> +      must be named "du.x" with "x" being the numerical index of the lowest
> +      channel in the group. The LVDS resets must be named "lvds.x" with "x"
> +      being the LVDS encoder numerical index.

LVDS is not a separate block?

> +
>  Required nodes:
>  
>  The connections to the DU output video ports are modeled using the OF graph
> -- 
> 2.7.4
>
Geert Uytterhoeven March 16, 2017, 8:13 a.m. UTC | #2
Hi Rob,

On Wed, Mar 15, 2017 at 6:01 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Mar 06, 2017 at 05:25:56PM +0100, Geert Uytterhoeven wrote:
>> Document the optional properties for describing module resets, to
>> support resetting display channels and LVDS encoders on R-Car Gen2 and
>> Gen3.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> See "[v2,1/4] dt-bindings: clock: renesas: cpg-mssr: Document reset control
>> support" (https://patchwork.kernel.org/patch/9536627/) for the format of
>> a reset specifier in the Renesas CPG/MSSR case.
>>
>> E.g. "resets = <&cpg 310>;"
>>
>> v2:
>>   - s/phandles/phandle/.
>> ---
>>  Documentation/devicetree/bindings/display/renesas,du.txt | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
>> index 1a02f099a0ff0a3a..3db418c827193e82 100644
>> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
>> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
>> @@ -36,6 +36,16 @@ Required Properties:
>>        When supplied they must be named "dclkin.x" with "x" being the input
>>        clock numerical index.
>>
>> +Optional properties:
>> +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
>> +    the reset-names property.
>> +  - reset-names: Names of the resets. This property is model-dependent.
>> +    - R8A779[0123456] use one reset for a group of one or more successive
>> +      channels, and one reset per LVDS encoder (if available). The resets
>> +      must be named "du.x" with "x" being the numerical index of the lowest
>> +      channel in the group. The LVDS resets must be named "lvds.x" with "x"
>> +      being the LVDS encoder numerical index.
>
> LVDS is not a separate block?

Well... from a hardware point of view, the LVDS encoders and DU channels
are all separate blocks (they have separate reg blocks, clocks, and resets).
But due to the dependencies between the blocks, they're modeled in DT as
a single device, with multiple reg, clocks, and resets properties.

The resets follow the clocks closely, as they're handled by the same block
(CPG/MSSR = Module Standby and Software Reset), except for a few quirks
(e.g. one reset for all DU channels on R-Car Gen2, one reset per pair of
 channels on R-Car Gen3).

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
Rob Herring March 16, 2017, 8:56 p.m. UTC | #3
On Thu, Mar 16, 2017 at 09:13:16AM +0100, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Wed, Mar 15, 2017 at 6:01 PM, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Mar 06, 2017 at 05:25:56PM +0100, Geert Uytterhoeven wrote:
> >> Document the optional properties for describing module resets, to
> >> support resetting display channels and LVDS encoders on R-Car Gen2 and
> >> Gen3.
> >>
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >> See "[v2,1/4] dt-bindings: clock: renesas: cpg-mssr: Document reset control
> >> support" (https://patchwork.kernel.org/patch/9536627/) for the format of
> >> a reset specifier in the Renesas CPG/MSSR case.
> >>
> >> E.g. "resets = <&cpg 310>;"
> >>
> >> v2:
> >>   - s/phandles/phandle/.
> >> ---
> >>  Documentation/devicetree/bindings/display/renesas,du.txt | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> >> index 1a02f099a0ff0a3a..3db418c827193e82 100644
> >> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> >> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> >> @@ -36,6 +36,16 @@ Required Properties:
> >>        When supplied they must be named "dclkin.x" with "x" being the input
> >>        clock numerical index.
> >>
> >> +Optional properties:
> >> +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
> >> +    the reset-names property.
> >> +  - reset-names: Names of the resets. This property is model-dependent.
> >> +    - R8A779[0123456] use one reset for a group of one or more successive
> >> +      channels, and one reset per LVDS encoder (if available). The resets
> >> +      must be named "du.x" with "x" being the numerical index of the lowest
> >> +      channel in the group. The LVDS resets must be named "lvds.x" with "x"
> >> +      being the LVDS encoder numerical index.
> >
> > LVDS is not a separate block?
> 
> Well... from a hardware point of view, the LVDS encoders and DU channels
> are all separate blocks (they have separate reg blocks, clocks, and resets).
> But due to the dependencies between the blocks, they're modeled in DT as
> a single device, with multiple reg, clocks, and resets properties.

Dependencies being DRM requirement of wanting a single device with 
sub-devices or some h/w dependencies? The former shouldn't define your 
binding.

Rob
Geert Uytterhoeven March 16, 2017, 8:59 p.m. UTC | #4
Hi Rob,

On Thu, Mar 16, 2017 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Mar 16, 2017 at 09:13:16AM +0100, Geert Uytterhoeven wrote:
>> On Wed, Mar 15, 2017 at 6:01 PM, Rob Herring <robh@kernel.org> wrote:
>> > On Mon, Mar 06, 2017 at 05:25:56PM +0100, Geert Uytterhoeven wrote:
>> >> Document the optional properties for describing module resets, to
>> >> support resetting display channels and LVDS encoders on R-Car Gen2 and
>> >> Gen3.
>> >>
>> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >> ---
>> >> See "[v2,1/4] dt-bindings: clock: renesas: cpg-mssr: Document reset control
>> >> support" (https://patchwork.kernel.org/patch/9536627/) for the format of
>> >> a reset specifier in the Renesas CPG/MSSR case.
>> >>
>> >> E.g. "resets = <&cpg 310>;"
>> >>
>> >> v2:
>> >>   - s/phandles/phandle/.
>> >> ---
>> >>  Documentation/devicetree/bindings/display/renesas,du.txt | 10 ++++++++++
>> >>  1 file changed, 10 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
>> >> index 1a02f099a0ff0a3a..3db418c827193e82 100644
>> >> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
>> >> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
>> >> @@ -36,6 +36,16 @@ Required Properties:
>> >>        When supplied they must be named "dclkin.x" with "x" being the input
>> >>        clock numerical index.
>> >>
>> >> +Optional properties:
>> >> +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
>> >> +    the reset-names property.
>> >> +  - reset-names: Names of the resets. This property is model-dependent.
>> >> +    - R8A779[0123456] use one reset for a group of one or more successive
>> >> +      channels, and one reset per LVDS encoder (if available). The resets
>> >> +      must be named "du.x" with "x" being the numerical index of the lowest
>> >> +      channel in the group. The LVDS resets must be named "lvds.x" with "x"
>> >> +      being the LVDS encoder numerical index.
>> >
>> > LVDS is not a separate block?
>>
>> Well... from a hardware point of view, the LVDS encoders and DU channels
>> are all separate blocks (they have separate reg blocks, clocks, and resets).
>> But due to the dependencies between the blocks, they're modeled in DT as
>> a single device, with multiple reg, clocks, and resets properties.
>
> Dependencies being DRM requirement of wanting a single device with
> sub-devices or some h/w dependencies? The former shouldn't define your
> binding.

Hardware dependencies. Laurent can tell you more about them (when he'll
be back).

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
Rob Herring March 20, 2017, 3:04 p.m. UTC | #5
On Thu, Mar 16, 2017 at 09:59:04PM +0100, Geert Uytterhoeven wrote:
> Hi Rob,
> 
> On Thu, Mar 16, 2017 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Mar 16, 2017 at 09:13:16AM +0100, Geert Uytterhoeven wrote:
> >> On Wed, Mar 15, 2017 at 6:01 PM, Rob Herring <robh@kernel.org> wrote:
> >> > On Mon, Mar 06, 2017 at 05:25:56PM +0100, Geert Uytterhoeven wrote:
> >> >> Document the optional properties for describing module resets, to
> >> >> support resetting display channels and LVDS encoders on R-Car Gen2 and
> >> >> Gen3.
> >> >>
> >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> >> ---
> >> >> See "[v2,1/4] dt-bindings: clock: renesas: cpg-mssr: Document reset control
> >> >> support" (https://patchwork.kernel.org/patch/9536627/) for the format of
> >> >> a reset specifier in the Renesas CPG/MSSR case.
> >> >>
> >> >> E.g. "resets = <&cpg 310>;"
> >> >>
> >> >> v2:
> >> >>   - s/phandles/phandle/.
> >> >> ---
> >> >>  Documentation/devicetree/bindings/display/renesas,du.txt | 10 ++++++++++
> >> >>  1 file changed, 10 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> >> >> index 1a02f099a0ff0a3a..3db418c827193e82 100644
> >> >> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> >> >> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> >> >> @@ -36,6 +36,16 @@ Required Properties:
> >> >>        When supplied they must be named "dclkin.x" with "x" being the input
> >> >>        clock numerical index.
> >> >>
> >> >> +Optional properties:
> >> >> +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
> >> >> +    the reset-names property.
> >> >> +  - reset-names: Names of the resets. This property is model-dependent.
> >> >> +    - R8A779[0123456] use one reset for a group of one or more successive
> >> >> +      channels, and one reset per LVDS encoder (if available). The resets
> >> >> +      must be named "du.x" with "x" being the numerical index of the lowest
> >> >> +      channel in the group. The LVDS resets must be named "lvds.x" with "x"
> >> >> +      being the LVDS encoder numerical index.
> >> >
> >> > LVDS is not a separate block?
> >>
> >> Well... from a hardware point of view, the LVDS encoders and DU channels
> >> are all separate blocks (they have separate reg blocks, clocks, and resets).
> >> But due to the dependencies between the blocks, they're modeled in DT as
> >> a single device, with multiple reg, clocks, and resets properties.
> >
> > Dependencies being DRM requirement of wanting a single device with
> > sub-devices or some h/w dependencies? The former shouldn't define your
> > binding.
> 
> Hardware dependencies. Laurent can tell you more about them (when he'll
> be back).

Okay,

Acked-by: Rob Herring <robh@kernel.org>

Rob
Laurent Pinchart April 7, 2017, 12:55 p.m. UTC | #6
Hi Geert and Rob,

On Thursday 16 Mar 2017 21:59:04 Geert Uytterhoeven wrote:
> On Thu, Mar 16, 2017 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Mar 16, 2017 at 09:13:16AM +0100, Geert Uytterhoeven wrote:
> >> On Wed, Mar 15, 2017 at 6:01 PM, Rob Herring <robh@kernel.org> wrote:
> >>> On Mon, Mar 06, 2017 at 05:25:56PM +0100, Geert Uytterhoeven wrote:
> >>>> Document the optional properties for describing module resets, to
> >>>> support resetting display channels and LVDS encoders on R-Car Gen2 and
> >>>> Gen3.
> >>>> 
> >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>> ---
> >>>> See "[v2,1/4] dt-bindings: clock: renesas: cpg-mssr: Document reset
> >>>> control support" (https://patchwork.kernel.org/patch/9536627/) for the
> >>>> format of a reset specifier in the Renesas CPG/MSSR case.
> >>>> 
> >>>> E.g. "resets = <&cpg 310>;"
> >>>> 
> >>>> v2:
> >>>>   - s/phandles/phandle/.
> >>>> 
> >>>> ---
> >>>> 
> >>>>  Documentation/devicetree/bindings/display/renesas,du.txt | 10 ++++++++
> >>>>  1 file changed, 10 insertions(+)
> >>>> 
> >>>> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt
> >>>> b/Documentation/devicetree/bindings/display/renesas,du.txt index
> >>>> 1a02f099a0ff0a3a..3db418c827193e82 100644
> >>>> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> >>>> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> >>>> @@ -36,6 +36,16 @@ Required Properties:
> >>>>        When supplied they must be named "dclkin.x" with "x" being the
> >>>>        input clock numerical index.
> >>>> 
> >>>> +Optional properties:
> >>>> +  - resets: A list of phandle + reset-specifier pairs, one for each
> >>>> entry in
> >>>> +    the reset-names property.
> >>>> +  - reset-names: Names of the resets. This property is
> >>>> model-dependent.
> >>>> +    - R8A779[0123456] use one reset for a group of one or more
> >>>> successive
> >>>> +      channels, and one reset per LVDS encoder (if available). The
> >>>> resets
> >>>> +      must be named "du.x" with "x" being the numerical index of the
> >>>> lowest
> >>>> +      channel in the group. The LVDS resets must be named "lvds.x"
> >>>> with "x"
> >>>> +      being the LVDS encoder numerical index.
> >>> 
> >>> LVDS is not a separate block?
> >> 
> >> Well... from a hardware point of view, the LVDS encoders and DU channels
> >> are all separate blocks (they have separate reg blocks, clocks, and
> >> resets). But due to the dependencies between the blocks, they're modeled
> >> in DT as a single device, with multiple reg, clocks, and resets
> >> properties.
> >
> > Dependencies being DRM requirement of wanting a single device with
> > sub-devices or some h/w dependencies? The former shouldn't define your
> > binding.
> 
> Hardware dependencies. Laurent can tell you more about them (when he'll
> be back).

I believe we could model the LVDS encoder as a separate DT node. It was 
probably a historical mistake (that would however need to be confirmed, I 
haven't double-checked all details). Obviously I can't break backward 
compatibility, so we're kind of stuck :-S

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
index 1a02f099a0ff0a3a..3db418c827193e82 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -36,6 +36,16 @@  Required Properties:
       When supplied they must be named "dclkin.x" with "x" being the input
       clock numerical index.
 
+Optional properties:
+  - resets: A list of phandle + reset-specifier pairs, one for each entry in
+    the reset-names property.
+  - reset-names: Names of the resets. This property is model-dependent.
+    - R8A779[0123456] use one reset for a group of one or more successive
+      channels, and one reset per LVDS encoder (if available). The resets
+      must be named "du.x" with "x" being the numerical index of the lowest
+      channel in the group. The LVDS resets must be named "lvds.x" with "x"
+      being the LVDS encoder numerical index.
+
 Required nodes:
 
 The connections to the DU output video ports are modeled using the OF graph