Re: [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Laurent,

On Wed, Feb 19, 2020 at 5:04 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Feb 14, 2020 at 09:26:23AM +0100, Geert Uytterhoeven wrote:
> > Document the optional properties for describing module resets, to
> > support resetting display channels on R-Car Gen2 and Gen3.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Acked-by: Rob Herring <robh@kernel.org>

> > --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> > @@ -50,6 +50,14 @@ Required Properties:
> >      VSP instance that serves the DU channel, and the channel index identifies
> >      the LIF instance in that VSP.
> >
> > +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.
> > +    - All but R8A7779 use one reset for a group of one or more successive
> > +      channels. The resets must be named "du.x" with "x" being the numerical
> > +      index of the lowest channel in the group.
>
> I've now reviewed the patches that add those properties to our .dtsi
> files, and I wonder how we should handle the two SoCs that have DU0, DU1
> and DU3, but not DU2. The reset resource is tied to a group of two
> channels, so we would use du.0 and du.2 respectively, but that conflicts
> with the above text.
>
> I'm trying to think about the implementation on the driver side, where
> group resources are associated with a group object, whose index is
> computed by dividing the channel number by 2. We could have a special
> case in group initialization that uses du.3 instead of du.2 for the
> second group.
>
> What do you think ? Probably overkill, and we should go for du.3 ?

The "division by 2" rule is valid for R-Car Gen3, but not for R-Car
Gen2, where there is only a single reset for all channels.

Originally we had "du.0-1" and "du.2-3" (hmm, somehow I missed adding
this to the changelog for the bindings,  but it is present in the
changelog for the DTS files), but after switching to "du.0" and "du.2",
I always envisioned implementing this by finding a "du.x" reset by
looping from the current channel index to 0.  That algorithm works for all
supported SoCs (irrespective of naming the second reset on R-Car H3-N
and M3-N "du.2" or "du.3" ;-)

As per your comment about single resets, we could drop reset-names on
R-Car Gen2, but doing so would mean another special case in the driver.

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