| Message ID | 1526386840-15368-4-git-send-email-ulrich.hecht+renesas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
On Tue, May 15, 2018 at 02:20:38PM +0200, Ulrich Hecht wrote: > From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > The r8a77995 D3 platform has 2 LVDS channels connected to the DU. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > [uli: moved lvds* into the soc node, added PM domains, resets] > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> This looks fine but I will wait to see if there are other reviews before applying. Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Hi Ulrich, Thank you for the patch. On Tuesday, 15 May 2018 15:20:38 EEST Ulrich Hecht wrote: > From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > The r8a77995 D3 platform has 2 LVDS channels connected to the DU. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > [uli: moved lvds* into the soc node, added PM domains, resets] > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > --- > arch/arm64/boot/dts/renesas/r8a77995.dtsi | 56 ++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi > b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index ba98865..8e78110d 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi > @@ -757,12 +757,68 @@ > port@1 { > reg = <1>; > du_out_lvds0: endpoint { > + remote-endpoint = <&lvds0_in>; > }; > }; > > port@2 { > reg = <2>; > du_out_lvds1: endpoint { > + remote-endpoint = <&lvds1_in>; > + }; > + }; > + }; > + }; > + > + lvds0: lvds-encoder@feb90000 { > + compatible = "renesas,r8a77995-lvds"; > + reg = <0 0xfeb90000 0 0x20>; > + clocks = <&cpg CPG_MOD 727>; > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; > + resets = <&cpg 727>; > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds0_in: endpoint { > + remote-endpoint = <&du_out_lvds0>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + lvds0_out: endpoint { > + }; > + }; > + }; > + }; > + > + lvds1: lvds-encoder@feb90100 { > + compatible = "renesas,r8a77995-lvds"; > + reg = <0 0xfeb90100 0 0x20>; > + clocks = <&cpg CPG_MOD 727>; > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; > + resets = <&cpg 727>; While there seems to be a single clock for both LVDS encoders, it appears that two separate reset lines are used. Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Given that the LVDS encoder driver isn't functional yet I wouldn't rule out a need to update the LVDS DT bindings in order to properly support D3. I don't mind if this patch gets merged already (provided the reset problem gets fixed of course), as long as it won't be considered a blocker for DT bindings rework. Otherwise I'd prefer delaying upstreaming until the whole series can be tested. > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds1_in: endpoint { > + remote-endpoint = <&du_out_lvds1>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + lvds1_out: endpoint { > }; > }; > };
Hi Laurent, On Sun, May 20, 2018 at 10:50 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Tuesday, 15 May 2018 15:20:38 EEST Ulrich Hecht wrote: >> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> The r8a77995 D3 platform has 2 LVDS channels connected to the DU. >> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> [uli: moved lvds* into the soc node, added PM domains, resets] >> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> >> --- >> arch/arm64/boot/dts/renesas/r8a77995.dtsi | 56 ++++++++++++++++++++++++++++ >> 1 file changed, 56 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index ba98865..8e78110d 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> @@ -757,12 +757,68 @@ >> port@1 { >> reg = <1>; >> du_out_lvds0: endpoint { >> + remote-endpoint = <&lvds0_in>; >> }; >> }; >> >> port@2 { >> reg = <2>; >> du_out_lvds1: endpoint { >> + remote-endpoint = <&lvds1_in>; >> + }; >> + }; >> + }; >> + }; >> + >> + lvds0: lvds-encoder@feb90000 { >> + compatible = "renesas,r8a77995-lvds"; >> + reg = <0 0xfeb90000 0 0x20>; >> + clocks = <&cpg CPG_MOD 727>; >> + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; >> + resets = <&cpg 727>; >> + status = "disabled"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + lvds0_in: endpoint { >> + remote-endpoint = <&du_out_lvds0>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + lvds0_out: endpoint { >> + }; >> + }; >> + }; >> + }; >> + >> + lvds1: lvds-encoder@feb90100 { >> + compatible = "renesas,r8a77995-lvds"; >> + reg = <0 0xfeb90100 0 0x20>; >> + clocks = <&cpg CPG_MOD 727>; >> + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; >> + resets = <&cpg 727>; > > While there seems to be a single clock for both LVDS encoders, it appears that > two separate reset lines are used. Nice catch! So you can reset the individual LVDS instances, but not the individual DU instances. Doh... Gr{oetje,eeting}s, Geert
Hi Ulrich, I know this series needs to be re-spin when the D3/E3 LVDS PLL support will be added, but since I need it for testing on D3 the LVDS interface reset support, I noticed a small issue which I thought it is worth reporting. On Tue, May 15, 2018 at 02:20:38PM +0200, Ulrich Hecht wrote: > From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > The r8a77995 D3 platform has 2 LVDS channels connected to the DU. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > [uli: moved lvds* into the soc node, added PM domains, resets] > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > --- > arch/arm64/boot/dts/renesas/r8a77995.dtsi | 56 +++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi > index ba98865..8e78110d 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi > @@ -757,12 +757,68 @@ > port@1 { > reg = <1>; > du_out_lvds0: endpoint { > + remote-endpoint = <&lvds0_in>; > }; > }; > > port@2 { > reg = <2>; > du_out_lvds1: endpoint { > + remote-endpoint = <&lvds1_in>; > + }; > + }; > + }; > + }; > + > + lvds0: lvds-encoder@feb90000 { > + compatible = "renesas,r8a77995-lvds"; > + reg = <0 0xfeb90000 0 0x20>; > + clocks = <&cpg CPG_MOD 727>; > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; > + resets = <&cpg 727>; > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds0_in: endpoint { > + remote-endpoint = <&du_out_lvds0>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + lvds0_out: endpoint { > + }; > + }; > + }; > + }; > + > + lvds1: lvds-encoder@feb90100 { > + compatible = "renesas,r8a77995-lvds"; > + reg = <0 0xfeb90100 0 0x20>; > + clocks = <&cpg CPG_MOD 727>; > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; > + resets = <&cpg 727>; The LVDS-IF1 CPG reset is SRCR7[26], this I think this should be <&cpg 726> Please note that the most recent chip manual release has probably an error at page 3180, section 37-32 5. When display on in the LVDS0-IF, it is necessary to reset (SRCR7[27]) and reset clearing (SRSTCLR7[27]) LVDS0-IF module. When display on in the LVDS1-IF, it is necessary to reset (SRCR7[26]) and reset clearing (SRSTCLR7[26]) LVDS0-IF module ^- This should probablt be LVDS1-IF Thanks j > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds1_in: endpoint { > + remote-endpoint = <&du_out_lvds1>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + lvds1_out: endpoint { > }; > }; > }; > -- > 2.7.4 >
> On August 1, 2018 at 12:35 PM jacopo mondi <jacopo@jmondi.org> wrote: > > > Hi Ulrich, > I know this series needs to be re-spin when the D3/E3 LVDS PLL > support will be added, but since I need it for testing on D3 the LVDS > interface reset support, I noticed a small issue which I thought it is > worth reporting. > > On Tue, May 15, 2018 at 02:20:38PM +0200, Ulrich Hecht wrote: > > From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > The r8a77995 D3 platform has 2 LVDS channels connected to the DU. > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > [uli: moved lvds* into the soc node, added PM domains, resets] > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > > --- > > arch/arm64/boot/dts/renesas/r8a77995.dtsi | 56 +++++++++++++++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi > > index ba98865..8e78110d 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi > > @@ -757,12 +757,68 @@ > > port@1 { > > reg = <1>; > > du_out_lvds0: endpoint { > > + remote-endpoint = <&lvds0_in>; > > }; > > }; > > > > port@2 { > > reg = <2>; > > du_out_lvds1: endpoint { > > + remote-endpoint = <&lvds1_in>; > > + }; > > + }; > > + }; > > + }; > > + > > + lvds0: lvds-encoder@feb90000 { > > + compatible = "renesas,r8a77995-lvds"; > > + reg = <0 0xfeb90000 0 0x20>; > > + clocks = <&cpg CPG_MOD 727>; > > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; > > + resets = <&cpg 727>; > > + status = "disabled"; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + lvds0_in: endpoint { > > + remote-endpoint = <&du_out_lvds0>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + lvds0_out: endpoint { > > + }; > > + }; > > + }; > > + }; > > + > > + lvds1: lvds-encoder@feb90100 { > > + compatible = "renesas,r8a77995-lvds"; > > + reg = <0 0xfeb90100 0 0x20>; > > + clocks = <&cpg CPG_MOD 727>; > > + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; > > + resets = <&cpg 727>; > > The LVDS-IF1 CPG reset is SRCR7[26], this I think this should be > <&cpg 726> > > Please note that the most recent chip manual release has probably an > error at page 3180, section 37-32 > > 5. When display on in the LVDS0-IF, it is necessary to reset (SRCR7[27]) and reset clearing (SRSTCLR7[27]) > LVDS0-IF module. > When display on in the LVDS1-IF, it is necessary to reset (SRCR7[26]) and reset clearing (SRSTCLR7[26]) > LVDS0-IF module > ^- This should probablt be LVDS1-IF > Thanks for the hint, I will keep this in mind. CU Uli
diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index ba98865..8e78110d 100644 --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi @@ -757,12 +757,68 @@ port@1 { reg = <1>; du_out_lvds0: endpoint { + remote-endpoint = <&lvds0_in>; }; }; port@2 { reg = <2>; du_out_lvds1: endpoint { + remote-endpoint = <&lvds1_in>; + }; + }; + }; + }; + + lvds0: lvds-encoder@feb90000 { + compatible = "renesas,r8a77995-lvds"; + reg = <0 0xfeb90000 0 0x20>; + clocks = <&cpg CPG_MOD 727>; + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; + resets = <&cpg 727>; + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lvds0_in: endpoint { + remote-endpoint = <&du_out_lvds0>; + }; + }; + + port@1 { + reg = <1>; + lvds0_out: endpoint { + }; + }; + }; + }; + + lvds1: lvds-encoder@feb90100 { + compatible = "renesas,r8a77995-lvds"; + reg = <0 0xfeb90100 0 0x20>; + clocks = <&cpg CPG_MOD 727>; + power-domains = <&sysc R8A77995_PD_ALWAYS_ON>; + resets = <&cpg 727>; + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lvds1_in: endpoint { + remote-endpoint = <&du_out_lvds1>; + }; + }; + + port@1 { + reg = <1>; + lvds1_out: endpoint { }; }; };