diff mbox series

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

Message ID 1560078659-19236-1-git-send-email-ykaneko0929@gmail.com (mailing list archive)
State Superseded
Delegated to: Simon Horman
Headers show
Series [PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface | expand

Commit Message

Yoshihiro Kaneko June 9, 2019, 11:10 a.m. UTC
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(+)

Comments

Geert Uytterhoeven June 12, 2019, 7:37 a.m. UTC | #1
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
Laurent Pinchart June 12, 2019, 12:15 p.m. UTC | #2
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/
Simon Horman June 13, 2019, 10:02 a.m. UTC | #3
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 ?
Laurent Pinchart June 13, 2019, 10:03 a.m. UTC | #4
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.
Simon Horman June 17, 2019, 8:37 a.m. UTC | #5
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.
diff mbox series

Patch

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";