| Message ID | 1560078659-19236-1-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCH/RFT] arm64: dts: renesas: r8a77995: Add cpg reset for LVDS Interface | expand |
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
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/
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 ?
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.
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 --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";
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(+)