Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
On 08/22/2018 12:55 PM, Simon Horman wrote: >>> Define the Condor/V3HSK board dependent parts of the DU and LVDS device >>> nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and >>> Analog Devices ADV7511W HDMI transmitter... >>> >>> Based on the original (and large) patch by Vladimir Barinov. >>> >>> Signed-off-by: Vladimir Barinov >>> Signed-off-by: Sergei Shtylyov >>> >>> --- >>> Changes in version 2: >>> - added the V3HSK DT update, reworded the description, renamed the patch; >>> - added a space between the HDMI node name and a brace. >>> >>> arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106 >>> arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120 >>> 2 files changed, 226 insertions(+) >> >> I would have split that in two patchees. > > I take your point but I think one is fine. Yeah, one patch here fits Arnd's criterion. :-) > Sergei, will you address the other review items? Done, about to repost... MBR, Sergei
Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
Hello!
On 08/13/2018 06:19 PM, Laurent Pinchart wrote:
>> Define the Condor/V3HSK board dependent parts of the DU and LVDS device
>> nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and
>> Analog Devices ADV7511W HDMI transmitter...
>>
>> Based on the original (and large) patch by Vladimir Barinov.
>>
>> Signed-off-by: Vladimir Barinov
>> Signed-off-by: Sergei Shtylyov
>>
>> ---
>> Changes in version 2:
>> - added the V3HSK DT update, reworded the description, renamed the patch;
>> - added a space between the HDMI node name and a brace.
>>
>> arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106
>> arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120
>> 2 files changed, 226 insertions(+)
>
> I would have split that in two patchees.
>
>>
>> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
>> ===
>> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
>> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
[...]
>> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts
>> ===
>> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts
>> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts
>> @@ -27,6 +27,63 @@
>> /* first 128MB is reserved for secure area. */
>> reg = <0 0x4800 0 0x7800>;
>> };
>> +
>> +hdmi-out {
>> +compatible = "hdmi-connector";
>> +type = "a";
>> +
>> +port {
>> +hdmi_con: endpoint {
>> +remote-endpoint = <_out>;
>> +};
>> +};
>> +};
>> +
>> +lvds-decoder {
>> +compatible = "thine,thc63lvd1024";
>> +vcc-supply = <_d5>;
>> +
>> +ports {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +port@0 {
>> +reg = <0>;
>> +thc63lvd1024_in: endpoint {
>> +remote-endpoint = <_out>;
>> +};
>> +};
>> +
>> +port@2 {
>> +reg = <2>;
>> +thc63lvd1024_out: endpoint {
>> +remote-endpoint = <_in>;
>> +};
>> +};
>> +};
>> +};
>> +
>> +vcc1v8_d4: regulator-0 {
>> +compatible = "regulator-fixed";
>> +regulator-name = "VCC1V8_D4";
>> +regulator-min-microvolt = <180>;
>> +regulator-max-microvolt = <180>;
>> +regulator-boot-on;
>> +regulator-always-on;
>> +};
>> +
>> +vcc3v3_d5: regulator-1 {
>> +compatible = "regulator-fixed";
>> +regulator-name = "VCC3V3_D5";
>> +regulator-min-microvolt = <330>;
>> +regulator-max-microvolt = <330>;
>> +regulator-boot-on;
>> +regulator-always-on;
>> +};
>> +};
>> +
>> + {
>> +status = "okay";
>
> No dot clock for the DU ?
You're right, there's OSC1 providing 148.5 MHz. Fixed.
> Apart from that,
>
> Reviewed-by: Laurent Pinchart
Thanks.
MBR, Sergei
Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
On Mon, Aug 13, 2018 at 06:19:20PM +0300, Laurent Pinchart wrote:
> Hi Sergei,
>
> Thank you for the patch.
>
> On Wednesday, 13 June 2018 23:12:40 EEST Sergei Shtylyov wrote:
> > Define the Condor/V3HSK board dependent parts of the DU and LVDS device
> > nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and
> > Analog Devices ADV7511W HDMI transmitter...
> >
> > Based on the original (and large) patch by Vladimir Barinov.
> >
> > Signed-off-by: Vladimir Barinov
> > Signed-off-by: Sergei Shtylyov
> >
> > ---
> > Changes in version 2:
> > - added the V3HSK DT update, reworded the description, renamed the patch;
> > - added a space between the HDMI node name and a brace.
> >
> > arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106
> > arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120
> > 2 files changed, 226 insertions(+)
>
> I would have split that in two patchees.
I take your point but I think one is fine.
Sergei, will you address the other review items?
>
> >
> > Index: renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> > ===
> > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> > +++ renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> > @@ -45,6 +45,56 @@
> > regulator-boot-on;
> > regulator-always-on;
> > };
> > +
> > + d1_8v: regulator-2 {
>
> Nitpicking, the naming for the regulator and clock nodes seems a bit weird.
> That shouldn't block this patch, but the issue should still be discussed with
> DT maintainers. A clear rule should be enacted on how to name top level nodes
> for which no register value makes sense.
>
> > + compatible = "regulator-fixed";
> > + regulator-name = "D1.8V";
> > + regulator-min-microvolt = <180>;
> > + regulator-max-microvolt = <180>;
> > + regulator-boot-on;
> > + regulator-always-on;
> > + };
> > +
> > + hdmi-out {
> > + compatible = "hdmi-connector";
> > + type = "a";
> > +
> > + port {
> > + hdmi_con: endpoint {
> > + remote-endpoint = <_out>;
> > + };
> > + };
> > + };
> > +
> > + lvds-decoder {
> > + compatible = "thine,thc63lvd1024";
> > + vcc-supply = <_3v>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + thc63lvd1024_in: endpoint {
> > + remote-endpoint = <_out>;
> > + };
> > + };
> > +
> > + port@2 {
> > + reg = <2>;
> > + thc63lvd1024_out: endpoint {
> > + remote-endpoint = <_in>;
> > + };
> > + };
> > + };
> > + };
> > +
> > + x1_clk: x1-clock {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <14850>;
> > + };
> > };
> >
> > {
> > @@ -74,6 +124,13 @@
> > };
> > };
> >
> > + {
> > + clocks = < CPG_MOD 724>,
> > +<_clk>;
> > + clock-names = "du.0", "dclkin.0";
> > + status = "okay";
> > +};
> > +
> > _clk {
> > clock-frequency = <1666>;
> > };
> > @@ -102,6 +159,55 @@
> > gpio-controller;
> > #gpio-cells = <2>;
> > };
> > +
> > + hdmi@39 {
> > + compatible = "adi,adv7511w";
> > + reg = <0x39>;
> > + interrupt-parent = <>;
> > + interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> > + avdd-supply = <_8v>;
> > + dvdd-supply = <_8v>;
> > + pvdd-supply = <_8v>;
> > + bgvdd-supply = <_8v>;
> > + dvdd-3v-supply = <_3v>;
> > +
> > + adi,input-depth = <8>;
> > + adi,input-colorspace = "rgb";
> > + adi,input-clock = "1x";
> > + adi,input-style = <1>;
> > + adi,input-justification = "evenly";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + adv7511_in: endpoint {
> > + remote-endpoint = <_out>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + adv7511_out: endpoint {
> > + remote-endpoint = <_con>;
> > + };
> > + };
> > + };
> > + };
> > +};
> > +
> > + {
> > +
Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
Hi Sergei,
Thank you for the patch.
On Wednesday, 13 June 2018 23:12:40 EEST Sergei Shtylyov wrote:
> Define the Condor/V3HSK board dependent parts of the DU and LVDS device
> nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and
> Analog Devices ADV7511W HDMI transmitter...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov
> Signed-off-by: Sergei Shtylyov
>
> ---
> Changes in version 2:
> - added the V3HSK DT update, reworded the description, renamed the patch;
> - added a space between the HDMI node name and a brace.
>
> arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106
> arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120
> 2 files changed, 226 insertions(+)
I would have split that in two patchees.
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> ===
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> @@ -45,6 +45,56 @@
> regulator-boot-on;
> regulator-always-on;
> };
> +
> + d1_8v: regulator-2 {
Nitpicking, the naming for the regulator and clock nodes seems a bit weird.
That shouldn't block this patch, but the issue should still be discussed with
DT maintainers. A clear rule should be enacted on how to name top level nodes
for which no register value makes sense.
> + compatible = "regulator-fixed";
> + regulator-name = "D1.8V";
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + hdmi-out {
> + compatible = "hdmi-connector";
> + type = "a";
> +
> + port {
> + hdmi_con: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> + lvds-decoder {
> + compatible = "thine,thc63lvd1024";
> + vcc-supply = <_3v>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + thc63lvd1024_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + thc63lvd1024_out: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> + };
> +
> + x1_clk: x1-clock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <14850>;
> + };
> };
>
> {
> @@ -74,6 +124,13 @@
> };
> };
>
> + {
> + clocks = < CPG_MOD 724>,
> + <_clk>;
> + clock-names = "du.0", "dclkin.0";
> + status = "okay";
> +};
> +
> _clk {
> clock-frequency = <1666>;
> };
> @@ -102,6 +159,55 @@
> gpio-controller;
> #gpio-cells = <2>;
> };
> +
> + hdmi@39 {
> + compatible = "adi,adv7511w";
> + reg = <0x39>;
> + interrupt-parent = <>;
> + interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> + avdd-supply = <_8v>;
> + dvdd-supply = <_8v>;
> + pvdd-supply = <_8v>;
> + bgvdd-supply = <_8v>;
> + dvdd-3v-supply = <_3v>;
> +
> + adi,input-depth = <8>;
> + adi,input-colorspace = "rgb";
> + adi,input-clock = "1x";
> + adi,input-style = <1>;
> + adi,input-justification = "evenly";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + adv7511_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + adv7511_out: endpoint {
> + remote-endpoint = <_con>;
> + };
> + };
> + };
> + };
> +};
> +
> + {
> + status = "okay";
> +
> + ports {
> + port@1 {
> + lvds0_out: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> };
>
> {
> Index:
Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
Hello! On 06/14/2018 10:24 AM, Simon Horman wrote: >> Define the Condor/V3HSK board dependent parts of the DU and LVDS device >> nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and >> Analog Devices ADV7511W HDMI transmitter... >> >> Based on the original (and large) patch by Vladimir Barinov. >> >> Signed-off-by: Vladimir Barinov >> Signed-off-by: Sergei Shtylyov >> >> --- >> Changes in version 2: >> - added the V3HSK DT update, reworded the description, renamed the patch; >> - added a space between the HDMI node name and a brace. >> >> arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106 + >> arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120 >> > > Laurent, could you review this? 2 months have passed and no review so far... MBR, Sergei
Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
On Mon, Jul 16, 2018 at 9:53 AM, Simon Horman wrote: > On Thu, Jun 14, 2018 at 09:24:27AM +0200, Simon Horman wrote: >> On Wed, Jun 13, 2018 at 11:12:40PM +0300, Sergei Shtylyov wrote: >> > Define the Condor/V3HSK board dependent parts of the DU and LVDS device >> > nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and >> > Analog Devices ADV7511W HDMI transmitter... >> > >> > Based on the original (and large) patch by Vladimir Barinov. >> > >> > Signed-off-by: Vladimir Barinov >> > Signed-off-by: Sergei Shtylyov >> > >> > --- >> > Changes in version 2: >> > - added the V3HSK DT update, reworded the description, renamed the patch; >> > - added a space between the HDMI node name and a brace. >> > >> > arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106 >> > + >> > arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120 >> > >> >> Laurent, could you review this? > > Ping For the Condor part: Reviewed-by: Ulrich Hecht Unfortunately, I do not have documentation for the other board. CU Uli
Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
On Thu, Jun 14, 2018 at 09:24:27AM +0200, Simon Horman wrote: > On Wed, Jun 13, 2018 at 11:12:40PM +0300, Sergei Shtylyov wrote: > > Define the Condor/V3HSK board dependent parts of the DU and LVDS device > > nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and > > Analog Devices ADV7511W HDMI transmitter... > > > > Based on the original (and large) patch by Vladimir Barinov. > > > > Signed-off-by: Vladimir Barinov > > Signed-off-by: Sergei Shtylyov > > > > --- > > Changes in version 2: > > - added the V3HSK DT update, reworded the description, renamed the patch; > > - added a space between the HDMI node name and a brace. > > > > arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106 > > + > > arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120 > > > > Laurent, could you review this? Ping
Re: [PATCH v3 2/2] arm64: dts: renesas: condor/v3hsk: add DU/LVDS/HDMI support
On Wed, Jun 13, 2018 at 11:12:40PM +0300, Sergei Shtylyov wrote:
> Define the Condor/V3HSK board dependent parts of the DU and LVDS device
> nodes. Also add the device nodes for Thine THC63LVD1024 LVDS decoder and
> Analog Devices ADV7511W HDMI transmitter...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov
> Signed-off-by: Sergei Shtylyov
>
> ---
> Changes in version 2:
> - added the V3HSK DT update, reworded the description, renamed the patch;
> - added a space between the HDMI node name and a brace.
>
> arch/arm64/boot/dts/renesas/r8a77980-condor.dts | 106 +
> arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts | 120
>
Laurent, could you review this?
> 2 files changed, 226 insertions(+)
>
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> ===
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980-condor.dts
> @@ -45,6 +45,56 @@
> regulator-boot-on;
> regulator-always-on;
> };
> +
> + d1_8v: regulator-2 {
> + compatible = "regulator-fixed";
> + regulator-name = "D1.8V";
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + hdmi-out {
> + compatible = "hdmi-connector";
> + type = "a";
> +
> + port {
> + hdmi_con: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +
> + lvds-decoder {
> + compatible = "thine,thc63lvd1024";
> + vcc-supply = <_3v>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + thc63lvd1024_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + thc63lvd1024_out: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> + };
> +
> + x1_clk: x1-clock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <14850>;
> + };
> };
>
> {
> @@ -74,6 +124,13 @@
> };
> };
>
> + {
> + clocks = < CPG_MOD 724>,
> + <_clk>;
> + clock-names = "du.0", "dclkin.0";
> + status = "okay";
> +};
> +
> _clk {
> clock-frequency = <1666>;
> };
> @@ -102,6 +159,55 @@
> gpio-controller;
> #gpio-cells = <2>;
> };
> +
> + hdmi@39 {
> + compatible = "adi,adv7511w";
> + reg = <0x39>;
> + interrupt-parent = <>;
> + interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> + avdd-supply = <_8v>;
> + dvdd-supply = <_8v>;
> + pvdd-supply = <_8v>;
> + bgvdd-supply = <_8v>;
> + dvdd-3v-supply = <_3v>;
> +
> + adi,input-depth = <8>;
> + adi,input-colorspace = "rgb";
> + adi,input-clock = "1x";
> + adi,input-style = <1>;
> + adi,input-justification = "evenly";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + adv7511_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + adv7511_out: endpoint {
> + remote-endpoint = <_con>;
> + };
> + };
> + };
> + };
> +};
> +
> + {
> + status = "okay";
> +
> + ports {
> + port@1 {
> + lvds0_out: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> };
>
> {
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts
> ===
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980-v3hsk.dts
> @@ -27,6 +27,63 @@
> /* first 128MB is reserved for secure area. */
> reg =
