[PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface

[PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface

[Yoshihiro Kaneko]

It is necessary to reset the LVDS Interface according to display on/off.
Therefore, this patch adds CPG reset properties in DU device node
for the R8A77995 SoC.

This patch was inspired by a patch in the BSP by Takeshi Kihara <takeshi.kihara.df@renesas.com>.

Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---

This patch is based on the devel branch of Simon Horman's renesas tree.

 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index e0a0149..7816fac 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -1001,6 +1001,8 @@
 			clocks = <&cpg CPG_MOD 724>,
 				 <&cpg CPG_MOD 723>;
 			clock-names = "du.0", "du.1";
+			resets = <&cpg 724>, <&cpg 724>;
+			reset-names = "du.0", "du.1";
 			vsps = <&vspd0 0 &vspd1 0>;
 			status = "disabled";
 
-- 
1.9.1

Re: [PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface

[Geert Uytterhoeven]

Hi Kaneko-san,

On Sun, Jun 9, 2019 at 1:11 PM Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote:
> It is necessary to reset the LVDS Interface according to display on/off.
> Therefore, this patch adds CPG reset properties in DU device node
> for the R8A77995 SoC.
>
> This patch was inspired by a patch in the BSP by Takeshi Kihara <takeshi.kihara.df@renesas.com>.
>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -1001,6 +1001,8 @@
>                         clocks = <&cpg CPG_MOD 724>,
>                                  <&cpg CPG_MOD 723>;
>                         clock-names = "du.0", "du.1";
> +                       resets = <&cpg 724>, <&cpg 724>;
> +                       reset-names = "du.0", "du.1";

These are not the LVDS resets, but the (shared) DU channel resets.

The LVDS interface has its own separate device node, so if you want to
be able to reset that, you need to add reset properties to the LVDS
node instead.

Note that I haven't reposted a new version of "[PATCH v2] dt-bindings:
drm: rcar-du: Document optional reset properties"[1] yet, after the
split off of the LVDS interface into its own device node. Laurent wanted
to wait until the driver gained DU reset support.
However, the above differs from my proposal, as it also adds "du.1",
pointing to the same (shared) reset.
With a fresh look (2 years later ;-), that actually makes sense, so
perhaps I should change my proposal and repost? We do have shared
resets in other places (e.g. USB).
Laurent, what do you think?

Thanks!

[1] https://lore.kernel.org/linux-renesas-soc/1488817556-21410-1-git-send-email-geert+renesas@glider.be/

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: [PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface

[Laurent Pinchart]

Hi Geert,

On Wed, Jun 12, 2019 at 09:37:14AM +0200, Geert Uytterhoeven wrote:
> On Sun, Jun 9, 2019 at 1:11 PM Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote:
> > It is necessary to reset the LVDS Interface according to display on/off.
> > Therefore, this patch adds CPG reset properties in DU device node
> > for the R8A77995 SoC.
> >
> > This patch was inspired by a patch in the BSP by Takeshi Kihara <takeshi.kihara.df@renesas.com>.
> >
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > @@ -1001,6 +1001,8 @@
> >                         clocks = <&cpg CPG_MOD 724>,
> >                                  <&cpg CPG_MOD 723>;
> >                         clock-names = "du.0", "du.1";
> > +                       resets = <&cpg 724>, <&cpg 724>;
> > +                       reset-names = "du.0", "du.1";
> 
> These are not the LVDS resets, but the (shared) DU channel resets.
> 
> The LVDS interface has its own separate device node, so if you want to
> be able to reset that, you need to add reset properties to the LVDS
> node instead.
> 
> Note that I haven't reposted a new version of "[PATCH v2] dt-bindings:
> drm: rcar-du: Document optional reset properties"[1] yet, after the
> split off of the LVDS interface into its own device node. Laurent wanted
> to wait until the driver gained DU reset support.
> However, the above differs from my proposal, as it also adds "du.1",
> pointing to the same (shared) reset.
> With a fresh look (2 years later ;-), that actually makes sense, so
> perhaps I should change my proposal and repost? We do have shared
> resets in other places (e.g. USB).
> Laurent, what do you think?

For Gen3 reset is handled at the group level, so I think specifying one
entry per group is enough. If other SoCs require per-channel reset
(which would surprise me as it would then imply a big redesign of the DU
IP core, which may lead to a separate driver) we can always extend the
bindings accordingly.

> [1] https://lore.kernel.org/linux-renesas-soc/1488817556-21410-1-git-send-email-geert+renesas@glider.be/

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface

[Simon Horman]

On Wed, Jun 12, 2019 at 03:15:56PM +0300, Laurent Pinchart wrote:
> Hi Geert,
> 
> On Wed, Jun 12, 2019 at 09:37:14AM +0200, Geert Uytterhoeven wrote:
> > On Sun, Jun 9, 2019 at 1:11 PM Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote:
> > > It is necessary to reset the LVDS Interface according to display on/off.
> > > Therefore, this patch adds CPG reset properties in DU device node
> > > for the R8A77995 SoC.
> > >
> > > This patch was inspired by a patch in the BSP by Takeshi Kihara <takeshi.kihara.df@renesas.com>.
> > >
> > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > 
> > Thanks for your patch!
> > 
> > > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > @@ -1001,6 +1001,8 @@
> > >                         clocks = <&cpg CPG_MOD 724>,
> > >                                  <&cpg CPG_MOD 723>;
> > >                         clock-names = "du.0", "du.1";
> > > +                       resets = <&cpg 724>, <&cpg 724>;
> > > +                       reset-names = "du.0", "du.1";
> > 
> > These are not the LVDS resets, but the (shared) DU channel resets.
> > 
> > The LVDS interface has its own separate device node, so if you want to
> > be able to reset that, you need to add reset properties to the LVDS
> > node instead.
> > 
> > Note that I haven't reposted a new version of "[PATCH v2] dt-bindings:
> > drm: rcar-du: Document optional reset properties"[1] yet, after the
> > split off of the LVDS interface into its own device node. Laurent wanted
> > to wait until the driver gained DU reset support.
> > However, the above differs from my proposal, as it also adds "du.1",
> > pointing to the same (shared) reset.
> > With a fresh look (2 years later ;-), that actually makes sense, so
> > perhaps I should change my proposal and repost? We do have shared
> > resets in other places (e.g. USB).
> > Laurent, what do you think?
> 
> For Gen3 reset is handled at the group level, so I think specifying one
> entry per group is enough. If other SoCs require per-channel reset
> (which would surprise me as it would then imply a big redesign of the DU
> IP core, which may lead to a separate driver) we can always extend the
> bindings accordingly.
> 
> > [1] https://lore.kernel.org/linux-renesas-soc/1488817556-21410-1-git-send-email-geert+renesas@glider.be/

Sorry, I'm a little unclear on what the suggested way forwards is here.

Is it to add a reset for du.0 but not du.1 ?

Re: [PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface

[Laurent Pinchart]

Hi Simon,

On Thu, Jun 13, 2019 at 12:02:46PM +0200, Simon Horman wrote:
> On Wed, Jun 12, 2019 at 03:15:56PM +0300, Laurent Pinchart wrote:
> > On Wed, Jun 12, 2019 at 09:37:14AM +0200, Geert Uytterhoeven wrote:
> >> On Sun, Jun 9, 2019 at 1:11 PM Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote:
> >>> It is necessary to reset the LVDS Interface according to display on/off.
> >>> Therefore, this patch adds CPG reset properties in DU device node
> >>> for the R8A77995 SoC.
> >>>
> >>> This patch was inspired by a patch in the BSP by Takeshi Kihara <takeshi.kihara.df@renesas.com>.
> >>>
> >>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> >> 
> >> Thanks for your patch!
> >> 
> >>> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> >>> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> >>> @@ -1001,6 +1001,8 @@
> >>>                         clocks = <&cpg CPG_MOD 724>,
> >>>                                  <&cpg CPG_MOD 723>;
> >>>                         clock-names = "du.0", "du.1";
> >>> +                       resets = <&cpg 724>, <&cpg 724>;
> >>> +                       reset-names = "du.0", "du.1";
> >> 
> >> These are not the LVDS resets, but the (shared) DU channel resets.
> >> 
> >> The LVDS interface has its own separate device node, so if you want to
> >> be able to reset that, you need to add reset properties to the LVDS
> >> node instead.
> >> 
> >> Note that I haven't reposted a new version of "[PATCH v2] dt-bindings:
> >> drm: rcar-du: Document optional reset properties"[1] yet, after the
> >> split off of the LVDS interface into its own device node. Laurent wanted
> >> to wait until the driver gained DU reset support.
> >> However, the above differs from my proposal, as it also adds "du.1",
> >> pointing to the same (shared) reset.
> >> With a fresh look (2 years later ;-), that actually makes sense, so
> >> perhaps I should change my proposal and repost? We do have shared
> >> resets in other places (e.g. USB).
> >> Laurent, what do you think?
> > 
> > For Gen3 reset is handled at the group level, so I think specifying one
> > entry per group is enough. If other SoCs require per-channel reset
> > (which would surprise me as it would then imply a big redesign of the DU
> > IP core, which may lead to a separate driver) we can always extend the
> > bindings accordingly.
> > 
> >> [1] https://lore.kernel.org/linux-renesas-soc/1488817556-21410-1-git-send-email-geert+renesas@glider.be/
> 
> Sorry, I'm a little unclear on what the suggested way forwards is here.
> 
> Is it to add a reset for du.0 but not du.1 ?

Correct.

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface

[Simon Horman]

On Thu, Jun 13, 2019 at 01:03:42PM +0300, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Thu, Jun 13, 2019 at 12:02:46PM +0200, Simon Horman wrote:
> > On Wed, Jun 12, 2019 at 03:15:56PM +0300, Laurent Pinchart wrote:
> > > On Wed, Jun 12, 2019 at 09:37:14AM +0200, Geert Uytterhoeven wrote:
> > >> On Sun, Jun 9, 2019 at 1:11 PM Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote:
> > >>> It is necessary to reset the LVDS Interface according to display on/off.
> > >>> Therefore, this patch adds CPG reset properties in DU device node
> > >>> for the R8A77995 SoC.
> > >>>
> > >>> This patch was inspired by a patch in the BSP by Takeshi Kihara <takeshi.kihara.df@renesas.com>.
> > >>>
> > >>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > >> 
> > >> Thanks for your patch!
> > >> 
> > >>> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > >>> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > >>> @@ -1001,6 +1001,8 @@
> > >>>                         clocks = <&cpg CPG_MOD 724>,
> > >>>                                  <&cpg CPG_MOD 723>;
> > >>>                         clock-names = "du.0", "du.1";
> > >>> +                       resets = <&cpg 724>, <&cpg 724>;
> > >>> +                       reset-names = "du.0", "du.1";
> > >> 
> > >> These are not the LVDS resets, but the (shared) DU channel resets.
> > >> 
> > >> The LVDS interface has its own separate device node, so if you want to
> > >> be able to reset that, you need to add reset properties to the LVDS
> > >> node instead.
> > >> 
> > >> Note that I haven't reposted a new version of "[PATCH v2] dt-bindings:
> > >> drm: rcar-du: Document optional reset properties"[1] yet, after the
> > >> split off of the LVDS interface into its own device node. Laurent wanted
> > >> to wait until the driver gained DU reset support.
> > >> However, the above differs from my proposal, as it also adds "du.1",
> > >> pointing to the same (shared) reset.
> > >> With a fresh look (2 years later ;-), that actually makes sense, so
> > >> perhaps I should change my proposal and repost? We do have shared
> > >> resets in other places (e.g. USB).
> > >> Laurent, what do you think?
> > > 
> > > For Gen3 reset is handled at the group level, so I think specifying one
> > > entry per group is enough. If other SoCs require per-channel reset
> > > (which would surprise me as it would then imply a big redesign of the DU
> > > IP core, which may lead to a separate driver) we can always extend the
> > > bindings accordingly.
> > > 
> > >> [1] https://lore.kernel.org/linux-renesas-soc/1488817556-21410-1-git-send-email-geert+renesas@glider.be/
> > 
> > Sorry, I'm a little unclear on what the suggested way forwards is here.
> > 
> > Is it to add a reset for du.0 but not du.1 ?
> 
> Correct.

Thanks, v2 sent.