| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3381435.LMORAGhIb5@avalon>
Date: Tue, 27 Jun 2017 16:57:10 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: kieran.bingham@...asonboard.com
Cc: Kieran Bingham <kbingham@...nel.org>,
Niklas Söderlund
<niklas.soderlund@...natech.se>, linux-renesas-soc@...r.kernel.org,
linux-kernel@...r.kernel.org, Simon Horman <horms@...ge.net.au>,
Magnus Damm <magnus.damm@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
"moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v5 2/2] arm64: dts: renesas: salvator-x: Add ADV7482 support
Hi Kieran,
On Tuesday 27 Jun 2017 10:25:39 Kieran Bingham wrote:
> On 23/06/17 08:43, Laurent Pinchart wrote:
> > On Wednesday 14 Jun 2017 20:58:13 Kieran Bingham wrote:
> >> From: Kieran Bingham <kieran.bingham+renesas@...asonboard.com>
> >>
> >> The Salvator boards use an ADV7482 receiver for HDMI and CVBS inputs.
> >>
> >> Provide ADV7482 node on the i2c4 bus, along with connectors for the
> >> hdmi and cvbs inputs, and link to the csi20 and csi40 nodes as outputs.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@...asonboard.com>
> >>
> >> v4:
> >> - dt: Rebase to dts/renesas/salvator-x.dtsi
> >> - dt: Use AIN0-7 rather than AIN1-8
> >>
> >> v5:
> >> - dt: Move to salvator-common.dtsi
> >>
> >> ---
> >>
> >> arch/arm64/boot/dts/renesas/salvator-common.dtsi | 123 ++++++++++++++++-
> >> 1 file changed, 123 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >> b/arch/arm64/boot/dts/renesas/salvator-common.dtsi index
> >> aef35e0b685a..d74fb55c38bc 100644
> >> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> >> @@ -65,6 +65,27 @@
> >> enable-gpios = <&gpio6 7 GPIO_ACTIVE_HIGH>;
> >> };
> >>
> >> + cvbs-in {
> >> + compatible = "composite-video-connector";
> >> + label = "CVBS IN";
> >> +
> >> + port {
> >> + cvbs_con: endpoint {
> >> + };
> >> + };
> >> + };
> >> +
> >> + hdmi-in {
> >> + compatible = "hdmi-connector";
> >> + label = "HDMI IN";
> >> + type = "a";
> >> +
> >> + port {
> >> + hdmi_in_con: endpoint {
> >> + };
> >> + };
> >> + };
> >> +
> >> reg_1p8v: regulator0 {
> >> compatible = "regulator-fixed";
> >> regulator-name = "fixed-1.8V";
> >> @@ -257,6 +278,59 @@
> >> };
> >> };
> >>
> >> +&csi20 {
> >> + status = "okay";
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + port@0 {
> >> + reg = <0>;
> >
> > Aren't #address-cells, #size-cells and reg already present in the SoC
> > .dtsi ? And if they're not, shouldn't they be ? :-)
>
> Hrm - I think I added them because I was getting warnings from the DTC.
>
> I've removed them from the csi nodes and no warning returns :)
> So they're gone.
You need the #address-cells and #size-cells properties in the ports node, but
as they're already added by the SoC .dtsi, you don't need to duplicate them
here.
> >> + csi20_in: endpoint {
> >> + clock-lanes = <0>;
> >> + data-lanes = <1>;
> >> + remote-endpoint = <&adv7482_txb>;
> >> + };
> >> + };
> >> + };
> >> +};
> >> +
> >> +&csi40 {
> >> + status = "okay";
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
>
> # Removed
>
> >> +
> >> + port@0 {
> >> + reg = <0>;
> >> +
> >> + csi40_in: endpoint {
> >> + clock-lanes = <0>;
> >> + data-lanes = <1 2 3 4>;
> >> + remote-endpoint = <&adv7482_txa>;
> >> + };
> >> + };
> >> + };
> >> +};
> >> +
> >> +&cvbs_con {
> >> + port {
> >> + cvbs_in: endpoint {
> >> + remote-endpoint = <&adv7482_ain7>;
> >> + };
> >> + };
> >> +};
> >
> > You could merge this with the cvbs_con node above.
>
> What is the distinction that means cvbs_con can be merged where hdmi_in_con
> would not ?
No reason, the same comment applies to HDMI too :-)
> Similarly, I see that VGA out is 'merged' where hdmi outs are not ... is it
> just determined by whether the connector is on all boards that inherit
> salvator-common.dtsi ?
My point was that it seems a bit pointless to define a node and extend it with
additional properties in the same file, when you could merge those properties
in the node definition. You only need to extend nodes with additional
properties when they're defined in one file and extended in another one.
> > Apart from that the patch looks good to me.
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> >
> >> +&hdmi_in_con {
> >> + port {
> >> + hdmi_in: endpoint {
> >> + remote-endpoint = <&adv7482_hdmi>;
> >> + };
> >> + };
> >> +};
> >> +
> >> &du {
> >> pinctrl-0 = <&du_pins>;
> >> pinctrl-names = "default";
> >> @@ -343,6 +417,55 @@
> >>
> >> shunt-resistor-micro-ohms = <5000>;
> >> };
> >> +
> >> + video-receiver@70 {
> >> + compatible = "adi,adv7482";
> >> + reg = <0x70>;
> >> +
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
>
> But I can't remove these # without generating a warning message ...
The properties are needed, why should you remove them ? :-)
> >> +
> >> + interrupt-parent = <&gpio6>;
> >> + interrupt-names = "intrq1", "intrq2";
> >> + interrupts = <30 IRQ_TYPE_LEVEL_LOW>,
> >> + <31 IRQ_TYPE_LEVEL_LOW>;
> >> +
> >> + port@7 {
> >> + reg = <7>;
> >> +
> >> + adv7482_ain7: endpoint {
> >> + remote-endpoint = <&cvbs_in>;
> >> + };
> >> + };
> >> +
> >> + port@8 {
> >> + reg = <8>;
> >> +
> >> + adv7482_hdmi: endpoint {
> >> + remote-endpoint = <&hdmi_in>;
> >> + };
> >> + };
> >> +
> >> + port@10 {
> >> + reg = <10>;
> >> +
> >> + adv7482_txa: endpoint {
> >> + clock-lanes = <0>;
> >> + data-lanes = <1 2 3 4>;
> >> + remote-endpoint = <&csi40_in>;
> >> + };
> >> + };
> >> +
> >> + port@11 {
> >> + reg = <11>;
> >> +
> >> + adv7482_txb: endpoint {
> >> + clock-lanes = <0>;
> >> + data-lanes = <1>;
> >> + remote-endpoint = <&csi20_in>;
> >> + };
> >> + };
> >> + };
> >> };
> >>
> >> &i2c_dvfs {
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists