Re: [PATCH v2 3/3] drm: rcar-du: lvds: Refactor LVDS startup

[Laurent Pinchart]

Hi Sergei,

On Wednesday, 14 February 2018 20:39:59 EET Sergei Shtylyov wrote:
> On 02/14/2018 09:13 PM, Laurent Pinchart wrote:
> > From: Sergei Shtylyov 
> > 
> > After the recent corrections to the R-Car gen2/3 LVDS startup code,
> > already similar enough at their ends rcar_lvds_enable_gen{2|3}() started
> > asking for a merge and it's becoming actually necessary with the addition
> > of the R-Car V3M (R8A77970) support -- this gen3 SoC has gen2-like
> > LVDPLLCR layout.
> > 
> > Signed-off-by: Sergei Shtylyov 
> > Reviewed-by: Laurent Pinchart 
> > Tested-by: Laurent Pinchart 
> 
> Well, your role wasn't limited to reviewning/testing, you'd clearly did
> some editing too... thus I was expecting to see some changelog.

My bad, it was an oversight. I'll add the following.

[Set the LVDS mode and input before turning channels on]
[Rebased, coding style changes]

> > Signed-off-by: Laurent Pinchart
> > 
> 
> Your variant of my patch looks good otherwise. :-)

Thank you :-) I've queued it in my tree.

-- 
Regards,

Laurent Pinchart

[PATCH v3 10/12] ARM: dts: r8a7794: Convert to new DU DT bindings

[Laurent Pinchart]

The DU DT bindings have been updated to drop the reg-names property.
Update the r8a7792 device tree accordingly.

Signed-off-by: Laurent Pinchart 
---
 arch/arm/boot/dts/r8a7794.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi
index 5643976c1356..cccdada9e4a4 100644
--- a/arch/arm/boot/dts/r8a7794.dtsi
+++ b/arch/arm/boot/dts/r8a7794.dtsi
@@ -992,7 +992,6 @@
du: display@feb0 {
compatible = "renesas,du-r8a7794";
reg = <0 0xfeb0 0 0x4>;
-   reg-names = "du";
interrupts = ,
 ;
clocks = < CPG_MOD 724>, < CPG_MOD 723>;
-- 
Regards,

Laurent Pinchart

[PATCH v3 12/12] arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings

[Laurent Pinchart]

The internal LVDS encoder now has DT bindings separate from the DU. Port
the device tree over to the new model.

Signed-off-by: Laurent Pinchart 
---
Changes since v2:

- Fixed LVDS compatible string

Changes since v1:

- Remove the DU reg-names property
---
 arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts |  3 +-
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |  3 +-
 arch/arm64/boot/dts/renesas/r8a7796.dtsi   | 34 ++
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  7 -
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  3 --
 drivers/gpu/drm/rcar-du/rcar_du_group.c| 13 -
 6 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts 
b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
index daee1f1a3f68..21bd679062f3 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
@@ -33,10 +33,9 @@
clocks = < CPG_MOD 724>,
 < CPG_MOD 723>,
 < CPG_MOD 722>,
-< CPG_MOD 727>,
 < 1>,
 < 3>,
 < 2>;
-   clock-names = "du.0", "du.1", "du.2", "lvds.0",
+   clock-names = "du.0", "du.1", "du.2",
  "dclkin.0", "dclkin.1", "dclkin.2";
 };
diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts 
b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index 4088bea8d62c..ecbfbcb65df3 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -32,11 +32,10 @@
clocks = < CPG_MOD 724>,
 < CPG_MOD 723>,
 < CPG_MOD 722>,
-< CPG_MOD 727>,
 < 1>,
 <_clk>,
 < 2>;
-   clock-names = "du.0", "du.1", "du.2", "lvds.0",
+   clock-names = "du.0", "du.1", "du.2",
  "dclkin.0", "dclkin.1", "dclkin.2";
 };
 
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index f2b2e40c655e..86f4bead3c0b 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -1826,17 +1826,14 @@
 
du: display@feb0 {
compatible = "renesas,du-r8a7796";
-   reg = <0 0xfeb0 0 0x7>,
- <0 0xfeb9 0 0x14>;
-   reg-names = "du", "lvds.0";
+   reg = <0 0xfeb0 0 0x7>;
interrupts = ,
 ,
 ;
clocks = < CPG_MOD 724>,
 < CPG_MOD 723>,
-< CPG_MOD 722>,
-< CPG_MOD 727>;
-   clock-names = "du.0", "du.1", "du.2", "lvds.0";
+< CPG_MOD 722>;
+   clock-names = "du.0", "du.1", "du.2";
status = "disabled";
 
vsps = <  >;
@@ -1859,6 +1856,31 @@
port@2 {
reg = <2>;
du_out_lvds0: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,r8a7796-lvds";
+   reg = <0 0xfeb9 0 0x14>;
+   clocks = < CPG_MOD 727>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_in: endpoint {
+   remote-endpoint = 
<_out_lvds0>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+   lvds0_out: endpoint {
};
};
};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index c4420538ec85..c296db68eddb 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -321,12 +321,6 @@ void rcar_du_crtc_route_output(struct drm_crtc *crtc,
struct rcar_du_device *rcdu = rcrtc->group->dev;
 
/*
-* Store the route from the CRTC output to the DU output. The DU will be
-* 

[PATCH v3 08/12] ARM: dts: r8a7792: Convert to new DU DT bindings

[Laurent Pinchart]

The DU DT bindings have been updated to drop the reg-names property.
Update the r8a7792 device tree accordingly.

Signed-off-by: Laurent Pinchart 
---
 arch/arm/boot/dts/r8a7792.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/r8a7792.dtsi b/arch/arm/boot/dts/r8a7792.dtsi
index 3d080e07374c..d40325466aa8 100644
--- a/arch/arm/boot/dts/r8a7792.dtsi
+++ b/arch/arm/boot/dts/r8a7792.dtsi
@@ -678,7 +678,6 @@
du: display@feb0 {
compatible = "renesas,du-r8a7792";
reg = <0 0xfeb0 0 0x4>;
-   reg-names = "du";
interrupts = ,
 ;
clocks = < CPG_MOD 724>,
-- 
Regards,

Laurent Pinchart

[PATCH v3 11/12] arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings

[Laurent Pinchart]

The internal LVDS encoder now has DT bindings separate from the DU. Port
the device tree over to the new model.

Signed-off-by: Laurent Pinchart 
---
Changes since v2:

- Fixed LVDS compatible string

Changes since v1:

- Remove the DU reg-names property
---
 .../boot/dts/renesas/r8a7795-es1-salvator-x.dts|  3 +-
 arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts |  3 +-
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |  3 +-
 .../arm64/boot/dts/renesas/r8a7795-salvator-xs.dts |  3 +-
 arch/arm64/boot/dts/renesas/r8a7795.dtsi   | 34 ++
 5 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-salvator-x.dts 
b/arch/arm64/boot/dts/renesas/r8a7795-es1-salvator-x.dts
index 9ef9a4e5f376..0dda1c0ed1ce 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-es1-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-salvator-x.dts
@@ -43,12 +43,11 @@
 < CPG_MOD 723>,
 < CPG_MOD 722>,
 < CPG_MOD 721>,
-< CPG_MOD 727>,
 < 1>,
 <_clk>,
 <_clk>,
 < 2>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0",
+   clock-names = "du.0", "du.1", "du.2", "du.3",
  "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
 };
 
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts 
b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
index 0afe777973de..f0d6528c05eb 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
@@ -44,11 +44,10 @@
 < CPG_MOD 723>,
 < CPG_MOD 722>,
 < CPG_MOD 721>,
-< CPG_MOD 727>,
 < 1>,
 < 3>,
 < 4>,
 < 2>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0",
+   clock-names = "du.0", "du.1", "du.2", "du.3",
  "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
 };
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts 
b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
index 00ad1ab53a7d..e34b986185d4 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -43,12 +43,11 @@
 < CPG_MOD 723>,
 < CPG_MOD 722>,
 < CPG_MOD 721>,
-< CPG_MOD 727>,
 < 1>,
 <_clk>,
 <_clk>,
 < 2>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0",
+   clock-names = "du.0", "du.1", "du.2", "du.3",
  "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
 };
 
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts 
b/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts
index 79e8b65622db..31f2abaa90aa 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-xs.dts
@@ -43,12 +43,11 @@
 < CPG_MOD 723>,
 < CPG_MOD 722>,
 < CPG_MOD 721>,
-< CPG_MOD 727>,
 < 1>,
 <_clk>,
 <_clk>,
 < 2>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0",
+   clock-names = "du.0", "du.1", "du.2", "du.3",
  "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
 };
 
diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 15ef292a8d9f..d67ad6d3d67f 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -2077,9 +2077,7 @@
 
du: display@feb0 {
compatible = "renesas,du-r8a7795";
-   reg = <0 0xfeb0 0 0x8>,
- <0 0xfeb9 0 0x14>;
-   reg-names = "du", "lvds.0";
+   reg = <0 0xfeb0 0 0x8>;
interrupts = ,
 ,
 ,
@@ -2087,9 +2085,8 @@
clocks = < CPG_MOD 724>,
 < CPG_MOD 723>,
 < CPG_MOD 722>,
-< CPG_MOD 721>,
-< CPG_MOD 727>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0";
+< CPG_MOD 721>;
+   clock-names = "du.0", "du.1", "du.2", "du.3";
vsps = < 0  0  0  1>;
status = "disabled";
 
@@ -2117,6 +2114,31 @@
port@3 {
reg = <3>;
du_out_lvds0: endpoint {
+  

[PATCH v3 09/12] ARM: dts: r8a7793: Convert to new LVDS DT bindings

[Laurent Pinchart]

The internal LVDS encoder now has DT bindings separate from the DU. Port
the device tree over to the new model.

Signed-off-by: Laurent Pinchart 
---
Changes since v2:

- Fixed LVDS compatible string

Changes since v1:

- Remove the DU reg-names property
---
 arch/arm/boot/dts/r8a7793-gose.dts | 10 +++---
 arch/arm/boot/dts/r8a7793.dtsi | 34 --
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7793-gose.dts 
b/arch/arm/boot/dts/r8a7793-gose.dts
index 51b3ffac8efa..c6121f9b72a8 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -303,10 +303,9 @@
pinctrl-names = "default";
status = "okay";
 
-   clocks = < CPG_MOD 724>, < CPG_MOD 723>, < CPG_MOD 726>,
+   clocks = < CPG_MOD 724>, < CPG_MOD 723>,
 <_clk>, <_clk>;
-   clock-names = "du.0", "du.1", "lvds.0",
- "dclkin.0", "dclkin.1";
+   clock-names = "du.0", "du.1", "dclkin.0", "dclkin.1";
 
ports {
port@0 {
@@ -314,6 +313,11 @@
remote-endpoint = <_in>;
};
};
+   };
+};
+
+ {
+   ports {
port@1 {
lvds_connector: endpoint {
};
diff --git a/arch/arm/boot/dts/r8a7793.dtsi b/arch/arm/boot/dts/r8a7793.dtsi
index 0cd1035de1a4..3a4918dfc1d9 100644
--- a/arch/arm/boot/dts/r8a7793.dtsi
+++ b/arch/arm/boot/dts/r8a7793.dtsi
@@ -976,15 +976,12 @@
 
du: display@feb0 {
compatible = "renesas,du-r8a7793";
-   reg = <0 0xfeb0 0 0x4>,
- <0 0xfeb9 0 0x1c>;
-   reg-names = "du", "lvds.0";
+   reg = <0 0xfeb0 0 0x4>;
interrupts = ,
 ;
clocks = < CPG_MOD 724>,
-< CPG_MOD 723>,
-< CPG_MOD 726>;
-   clock-names = "du.0", "du.1", "lvds.0";
+< CPG_MOD 723>;
+   clock-names = "du.0", "du.1";
status = "disabled";
 
ports {
@@ -999,6 +996,31 @@
port@1 {
reg = <1>;
du_out_lvds0: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,r8a7793-lvds";
+   reg = <0 0xfeb9 0 0x1c>;
+   clocks = < CPG_MOD 726>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_in: endpoint {
+   remote-endpoint = <_out_lvds0>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+   lvds0_out: endpoint {
};
};
};
-- 
Regards,

Laurent Pinchart

[PATCH v3 07/12] ARM: dts: r8a7791: Convert to new LVDS DT bindings

[Laurent Pinchart]

The internal LVDS encoder now has DT bindings separate from the DU. Port
the device tree over to the new model.

Signed-off-by: Laurent Pinchart 
---
Changes since v2:

- Fixed LVDS compatible string

Changes since v1:

- Remove the DU reg-names property
---
 arch/arm/boot/dts/r8a7791-koelsch.dts | 10 --
 arch/arm/boot/dts/r8a7791-porter.dts  | 16 +---
 arch/arm/boot/dts/r8a7791.dtsi| 34 --
 3 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts 
b/arch/arm/boot/dts/r8a7791-koelsch.dts
index e164eda69baf..5a0f87ed7f16 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -332,8 +332,7 @@
 
clocks = < CPG_MOD 724>, < CPG_MOD 723>, < CPG_MOD 726>,
 <_clk>, <_clk>;
-   clock-names = "du.0", "du.1", "lvds.0",
- "dclkin.0", "dclkin.1";
+   clock-names = "du.0", "du.1", "dclkin.0", "dclkin.1";
 
ports {
port@0 {
@@ -341,6 +340,13 @@
remote-endpoint = <_in>;
};
};
+   };
+};
+
+ {
+   status = "okay";
+
+   ports {
port@1 {
lvds_connector: endpoint {
};
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts 
b/arch/arm/boot/dts/r8a7791-porter.dts
index 9a02d03b23c2..e6d02839d636 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -419,10 +419,9 @@
pinctrl-names = "default";
status = "okay";
 
-   clocks = < CPG_MOD 724>, < CPG_MOD 723>, < CPG_MOD 726>,
+   clocks = < CPG_MOD 724>, < CPG_MOD 723>,
 <_clk>, <_clk>;
-   clock-names = "du.0", "du.1", "lvds.0",
- "dclkin.0", "dclkin.1";
+   clock-names = "du.0", "du.1", "dclkin.0", "dclkin.1";
 
ports {
port@0 {
@@ -433,6 +432,17 @@
};
 };
 
+ {
+   status = "okay";
+
+   ports {
+   port@1 {
+   lvds_connector: endpoint {
+   };
+   };
+   };
+};
+
 _sound {
pinctrl-0 = <_pins _clk_pins>;
pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 67831d0405f3..aef34c80c584 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -1103,15 +1103,12 @@
 
du: display@feb0 {
compatible = "renesas,du-r8a7791";
-   reg = <0 0xfeb0 0 0x4>,
- <0 0xfeb9 0 0x1c>;
-   reg-names = "du", "lvds.0";
+   reg = <0 0xfeb0 0 0x4>;
interrupts = ,
 ;
clocks = < CPG_MOD 724>,
-< CPG_MOD 723>,
-< CPG_MOD 726>;
-   clock-names = "du.0", "du.1", "lvds.0";
+< CPG_MOD 723>;
+   clock-names = "du.0", "du.1";
status = "disabled";
 
ports {
@@ -1126,6 +1123,31 @@
port@1 {
reg = <1>;
du_out_lvds0: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,r8a7791-lvds";
+   reg = <0 0xfeb9 0 0x1c>;
+   clocks = < CPG_MOD 726>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_in: endpoint {
+   remote-endpoint = <_out_lvds0>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+   lvds0_out: endpoint {
};
};
};
-- 
Regards,

Laurent Pinchart

[PATCH v3 03/12] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes

[Laurent Pinchart]

The internal LVDS encoders now have their own DT bindings. Before
switching the driver infrastructure to those new bindings, implement
backward-compatibility through live DT patching.

Patching is disabled and will be enabled along with support for the new
DT bindings in the DU driver.

Signed-off-by: Laurent Pinchart 
---
Changes since v2:

- Update the SPDX headers to use C-style comments in header files
- Removed the manually created __local_fixups__ node
- Perform manual fixups on live DT instead of overlay

Changes since v1:

- Select OF_FLATTREE
- Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected
- Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
- Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char
- Pass void begin and end pointers to rcar_du_of_get_overlay()
- Use of_get_parent() instead of accessing the parent pointer directly
- Find the LVDS endpoints nodes based on the LVDS node instead of the
  root of the overlay
- Update to the -lvds compatible string format
---
 drivers/gpu/drm/rcar-du/Kconfig|   2 +
 drivers/gpu/drm/rcar-du/Makefile   |   7 +-
 drivers/gpu/drm/rcar-du/rcar_du_of.c   | 374 +
 drivers/gpu/drm/rcar-du/rcar_du_of.h   |  20 ++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts|  81 +
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts|  55 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts|  55 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts|  55 +++
 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts|  55 +++
 9 files changed, 703 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 5d0b4b7119af..3f83352a7313 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -22,6 +22,8 @@ config DRM_RCAR_LVDS
bool "R-Car DU LVDS Encoder Support"
depends on DRM_RCAR_DU
select DRM_PANEL
+   select OF_FLATTREE
+   select OF_OVERLAY
help
  Enable support for the R-Car Display Unit embedded LVDS encoders.
 
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 0cf5c11030e8..86b337b4be5d 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -8,7 +8,12 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_plane.o
 
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
-
+rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of.o \
+  rcar_du_of_lvds_r8a7790.dtb.o \
+  rcar_du_of_lvds_r8a7791.dtb.o \
+  rcar_du_of_lvds_r8a7793.dtb.o \
+  rcar_du_of_lvds_r8a7795.dtb.o \
+  rcar_du_of_lvds_r8a7796.dtb.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
 
 obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c 
b/drivers/gpu/drm/rcar-du/rcar_du_of.c
new file mode 100644
index ..141f6eda6e98
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -0,0 +1,374 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * rcar_du_of.c - Legacy DT bindings compatibility
+ *
+ * Copyright (C) 2018 Laurent Pinchart 
+ *
+ * Based on work from Jyri Sarha 
+ * Copyright (C) 2015 Texas Instruments
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rcar_du_crtc.h"
+#include "rcar_du_drv.h"
+
+/* 
-
+ * Generic Overlay Handling
+ */
+
+struct rcar_du_of_overlay {
+   const char *compatible;
+   void *begin;
+   void *end;
+};
+
+#define RCAR_DU_OF_DTB(type, soc)  \
+   extern char __dtb_rcar_du_of_##type##_##soc##_begin[];  \
+   extern char __dtb_rcar_du_of_##type##_##soc##_end[]
+
+#define RCAR_DU_OF_OVERLAY(type, soc)  \
+   {   \
+   .compatible = "renesas,du-" #soc,   \
+   .begin = __dtb_rcar_du_of_##type##_##soc##_begin,   \
+   .end = __dtb_rcar_du_of_##type##_##soc##_end,   \
+   }
+
+static 

[PATCH v3 01/12] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings

[Laurent Pinchart]

The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add
corresponding device tree bindings.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
Changes since v1:

- Move the SoC name before the IP name in compatible strings
- Rename parallel input to parallel RGB input
- Fixed "renesas,r8a7743-lvds" description
- Document the resets property
- Fixed typo
---
 .../bindings/display/bridge/renesas,lvds.txt   | 56 ++
 MAINTAINERS|  1 +
 2 files changed, 57 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
new file mode 100644
index ..2b19ce51ec07
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -0,0 +1,56 @@
+Renesas R-Car LVDS Encoder
+==
+
+These DT bindings describe the LVDS encoder embedded in the Renesas R-Car
+Gen2, R-Car Gen3 and RZ/G SoCs.
+
+Required properties:
+
+- compatible : Shall contain one of
+  - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders
+  - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders
+  - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders
+  - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS encoders
+  - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS encoders
+  - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders
+
+- reg: Base address and length for the memory-mapped registers
+- clocks: A phandle + clock-specifier pair for the functional clock
+- resets: A phandle + reset specifier for the module reset
+
+Required nodes:
+
+The LVDS encoder has two video ports. Their connections are modelled using the
+OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 corresponds to the parallel RGB input
+- Video port 1 corresponds to the LVDS output
+
+Each port shall have a single endpoint.
+
+
+Example:
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,r8a7790-lvds";
+   reg = <0 0xfeb9 0 0x1c>;
+   clocks = < CPG_MOD 726>;
+   resets = < 726>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_in: endpoint {
+   remote-endpoint = <_out_lvds0>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+   lvds0_out: endpoint {
+   };
+   };
+   };
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index 5bc088f27c83..5b0c14f4209b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4723,6 +4723,7 @@ F:drivers/gpu/drm/rcar-du/
 F: drivers/gpu/drm/shmobile/
 F: include/linux/platform_data/shmob_drm.h
 F: Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
+F: Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
 F: Documentation/devicetree/bindings/display/renesas,du.txt
 
 DRM DRIVERS FOR ROCKCHIP
-- 
Regards,

Laurent Pinchart

[PATCH v3 06/12] ARM: dts: r8a7790: Convert to new LVDS DT bindings

[Laurent Pinchart]

The internal LVDS encoder now has DT bindings separate from the DU. Port
the device tree over to the new model.

Signed-off-by: Laurent Pinchart 
---
Changes since v2:

- Fixed LVDS compatible string

Changes since v1:

- Remove the DU reg-names property
---
 arch/arm/boot/dts/r8a7790-lager.dts | 22 ++
 arch/arm/boot/dts/r8a7790.dtsi  | 60 -
 2 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts 
b/arch/arm/boot/dts/r8a7790-lager.dts
index 7d5cd01069a3..f84963cb6785 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -317,10 +317,8 @@
status = "okay";
 
clocks = < CPG_MOD 724>, < CPG_MOD 723>, < CPG_MOD 722>,
-< CPG_MOD 726>, < CPG_MOD 725>,
 <_clk>, <_clk>;
-   clock-names = "du.0", "du.1", "du.2", "lvds.0", "lvds.1",
- "dclkin.0", "dclkin.1";
+   clock-names = "du.0", "du.1", "du.2", "dclkin.0", "dclkin.1";
 
ports {
port@0 {
@@ -328,12 +326,26 @@
remote-endpoint = <_in>;
};
};
+   };
+};
+
+ {
+   status = "okay";
+
+   ports {
port@1 {
endpoint {
remote-endpoint = <_in>;
};
};
-   port@2 {
+   };
+};
+
+ {
+   status = "okay";
+
+   ports {
+   port@1 {
lvds_connector: endpoint {
};
};
@@ -689,7 +701,7 @@
port@0 {
reg = <0>;
adv7511_in: endpoint {
-   remote-endpoint = <_out_lvds0>;
+   remote-endpoint = <_out>;
};
};
 
diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 62baabd757b6..450951ff25eb 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -1067,17 +1067,13 @@
 
du: display@feb0 {
compatible = "renesas,du-r8a7790";
-   reg = <0 0xfeb0 0 0x7>,
- <0 0xfeb9 0 0x1c>,
- <0 0xfeb94000 0 0x1c>;
-   reg-names = "du", "lvds.0", "lvds.1";
+   reg = <0 0xfeb0 0 0x7>;
interrupts = ,
 ,
 ;
clocks = < CPG_MOD 724>, < CPG_MOD 723>,
-< CPG_MOD 722>, < CPG_MOD 726>,
-< CPG_MOD 725>;
-   clock-names = "du.0", "du.1", "du.2", "lvds.0", "lvds.1";
+< CPG_MOD 722>;
+   clock-names = "du.0", "du.1", "du.2";
status = "disabled";
 
ports {
@@ -1092,11 +1088,61 @@
port@1 {
reg = <1>;
du_out_lvds0: endpoint {
+   remote-endpoint = <_in>;
};
};
port@2 {
reg = <2>;
du_out_lvds1: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+   };
+
+   lvds0: lvds@feb9 {
+   compatible = "renesas,r8a7790-lvds";
+   reg = <0 0xfeb9 0 0x1c>;
+   clocks = < CPG_MOD 726>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_in: endpoint {
+   remote-endpoint = <_out_lvds0>;
+   };
+   };
+   port@1 {
+   reg = <1>;
+   lvds0_out: endpoint {
+   };
+   };
+   };
+   };
+
+   lvds1: lvds@feb94000 {
+   compatible = "renesas,r8a7790-lvds";
+   reg = <0 0xfeb94000 0 0x1c>;
+   clocks = < CPG_MOD 725>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds1_in: endpoint {
+   remote-endpoint = <_out_lvds1>;
+   

[PATCH v3 04/12] drm: rcar-du: Convert LVDS encoder code to bridge driver

[Laurent Pinchart]

The LVDS encoders used to be described in DT as part of the DU. They now
have their own DT node, linked to the DU using the OF graph bindings.
This allows moving internal LVDS encoder support to a separate driver
modelled as a DRM bridge. Backward compatibility is retained as legacy
DT is patched live to move to the new bindings.

Signed-off-by: Laurent Pinchart 
---
Changes since v1:

- Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only
- Update to the -lvds compatible string format
---
 drivers/gpu/drm/rcar-du/Kconfig   |   4 +-
 drivers/gpu/drm/rcar-du/Makefile  |   3 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c |  21 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h |   5 -
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 175 +-
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h |  12 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c |  14 +-
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c |  93 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h |  24 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 238 --
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h |  64 
 drivers/gpu/drm/rcar-du/rcar_lvds.c   | 524 ++
 12 files changed, 561 insertions(+), 616 deletions(-)
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
 delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 3f83352a7313..edde8d4b87a3 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -19,8 +19,8 @@ config DRM_RCAR_DW_HDMI
  Enable support for R-Car Gen3 internal HDMI encoder.
 
 config DRM_RCAR_LVDS
-   bool "R-Car DU LVDS Encoder Support"
-   depends on DRM_RCAR_DU
+   tristate "R-Car DU LVDS Encoder Support"
+   depends on DRM && DRM_BRIDGE && OF
select DRM_PANEL
select OF_FLATTREE
select OF_OVERLAY
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 86b337b4be5d..3e58ed93d5b1 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -4,10 +4,8 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar_du_encoder.o \
 rcar_du_group.o \
 rcar_du_kms.o \
-rcar_du_lvdscon.o \
 rcar_du_plane.o
 
-rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of.o \
   rcar_du_of_lvds_r8a7790.dtb.o \
   rcar_du_of_lvds_r8a7791.dtb.o \
@@ -18,3 +16,4 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)+= rcar_du_vsp.o
 
 obj-$(CONFIG_DRM_RCAR_DU)  += rcar-du-drm.o
 obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o
+obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 6e02c762a557..06a3fbdd728a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -29,6 +29,7 @@
 
 #include "rcar_du_drv.h"
 #include "rcar_du_kms.h"
+#include "rcar_du_of.h"
 #include "rcar_du_regs.h"
 
 /* 
-
@@ -74,7 +75,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info 
= {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7779_info = {
@@ -95,14 +95,13 @@ static const struct rcar_du_device_info 
rcar_du_r8a7779_info = {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7790_info = {
.gen = 2,
.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-   .quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
+   .quirks = RCAR_DU_QUIRK_ALIGN_128B,
.num_crtcs = 3,
.routes = {
/*
@@ -164,7 +163,6 @@ static const struct rcar_du_device_info 
rcar_du_r8a7792_info = {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7794_info = {
@@ -186,7 +184,6 @@ static const struct rcar_du_device_info 
rcar_du_r8a7794_info = {
.port = 1,
},
},
-   .num_lvds = 0,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7795_info = {
@@ -434,7 +431,19 @@ static struct platform_driver rcar_du_platform_driver = {
},
 };
 
-module_platform_driver(rcar_du_platform_driver);
+static int __init rcar_du_init(void)
+{
+   

[PATCH v3 05/12] ARM: dts: porter: Fix HDMI output routing

[Laurent Pinchart]

The HDMI encoder is connected to the RGB output of the DU, which is
port@0, not port@1. Fix the incorrect DT description.

Signed-off-by: Laurent Pinchart 
---
 arch/arm/boot/dts/r8a7791-porter.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/r8a7791-porter.dts 
b/arch/arm/boot/dts/r8a7791-porter.dts
index eb374956294f..9a02d03b23c2 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -425,7 +425,7 @@
  "dclkin.0", "dclkin.1";
 
ports {
-   port@1 {
+   port@0 {
endpoint {
remote-endpoint = <_in>;
};
-- 
Regards,

Laurent Pinchart

[PATCH v3 00/12] R-Car DU: Convert LVDS code to bridge driver

[Laurent Pinchart]

Hello,

This patch series addresses a design mistake that dates back from the initial
DU support. Support for the LVDS encoders, which are IP cores separate from
the DU, was bundled in the DU driver. Worse, both the DU and LVDS were
described through a single DT node.

To fix the, patches 01/12 and 02/12 define new DT bindings for the LVDS
encoders, and deprecate their description inside the DU bindings. To retain
backward compatibility with existing DT, patch 03/12 then patches the device
tree at runtime to convert the legacy bindings to the new ones.

With the DT side addressed, patch 04/12 then converts the LVDS support code to
a separate bridge driver. After a small fix to the Porter board device tree in
patch 05/12, patches 06/12 to 12/12 then update all the device tree sources to
the new DU and LVDS encoders bindings.

I decided to go for live DT patching in patch 03/12 because implementing
support for both the legacy and new bindings in the driver would have been
very intrusive, and prevented further cleanups. This version relies more
heavily on overlays to avoid touching the internals of the OF core compared to
v2, even if manual fixes to the device tree are still needed.

There were a few shortcomings in the OF API that I worked around with local
code in the driver. If anyone is interested in performing similar live DT
patching I think we could move some of the code to the OF core. I'm thinking
in particular about the rcar_du_of_find_node_by_path() function that resembles
of_find_node_by_path() but with a configurable base node, and about the
rcar_du_of_add_property() function that adds a property to an existing DT
node. Rob, Frank, should I submit patches ? Any advice ?

Compared to v2, the biggest change is in patch 03/12. Following Rob's and
Frank's reviews it was clear that modifying the unflattened DT structure of
the overlay before applying it wasn't popular. I have thus decided to use one
overlay source per SoC to move as much of the DT changes to the overlay as
possible, and only perform manual modifications (that are still needed as some
of the information is board-specific) on the system DT after applying the
overlay. As a result the overlay is parsed and applied without being modified.

Compared to v1, this series update the r8a7792 and r8a7794 device tree sources
and incorporate review feedback as described by the changelogs of individual
patches.

Laurent Pinchart (12):
  dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
  dt-bindings: display: renesas: Deprecate LVDS support in the DU
bindings
  drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
  drm: rcar-du: Convert LVDS encoder code to bridge driver
  ARM: dts: porter: Fix HDMI output routing
  ARM: dts: r8a7790: Convert to new LVDS DT bindings
  ARM: dts: r8a7791: Convert to new LVDS DT bindings
  ARM: dts: r8a7792: Convert to new DU DT bindings
  ARM: dts: r8a7793: Convert to new LVDS DT bindings
  ARM: dts: r8a7794: Convert to new DU DT bindings
  arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings
  arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings

 .../bindings/display/bridge/renesas,lvds.txt   |  56 +++
 .../devicetree/bindings/display/renesas,du.txt |  31 +-
 MAINTAINERS|   1 +
 arch/arm/boot/dts/r8a7790-lager.dts|  22 +-
 arch/arm/boot/dts/r8a7790.dtsi |  60 ++-
 arch/arm/boot/dts/r8a7791-koelsch.dts  |  10 +-
 arch/arm/boot/dts/r8a7791-porter.dts   |  18 +-
 arch/arm/boot/dts/r8a7791.dtsi |  34 +-
 arch/arm/boot/dts/r8a7792.dtsi |   1 -
 arch/arm/boot/dts/r8a7793-gose.dts |  10 +-
 arch/arm/boot/dts/r8a7793.dtsi |  34 +-
 arch/arm/boot/dts/r8a7794.dtsi |   1 -
 .../boot/dts/renesas/r8a7795-es1-salvator-x.dts|   3 +-
 arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts |   3 +-
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts |   3 +-
 .../arm64/boot/dts/renesas/r8a7795-salvator-xs.dts |   3 +-
 arch/arm64/boot/dts/renesas/r8a7795.dtsi   |  34 +-
 arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts |   3 +-
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |   3 +-
 arch/arm64/boot/dts/renesas/r8a7796.dtsi   |  34 +-
 drivers/gpu/drm/rcar-du/Kconfig|   6 +-
 drivers/gpu/drm/rcar-du/Makefile   |  10 +-
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c |   7 -
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h |   3 -
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  21 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h  |   5 -
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c  | 175 +--
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h  |  12 -
 drivers/gpu/drm/rcar-du/rcar_du_group.c|  13 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c  |  14 +-
 

[PATCH v3 02/12] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings

[Laurent Pinchart]

The internal LVDS encoders now have their own DT bindings, representing
them as part of the DU is deprecated.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Rob Herring 
---
Changes since v1:

- Remove the LVDS reg range from the example
- Remove the reg-names property
---
 .../devicetree/bindings/display/renesas,du.txt | 31 --
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt 
b/Documentation/devicetree/bindings/display/renesas,du.txt
index cd48aba3bc8c..e79cf9b0ad38 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -14,12 +14,7 @@ Required Properties:
 - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
 - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
 
-  - reg: A list of base address and length of each memory resource, one for
-each entry in the reg-names property.
-  - reg-names: Name of the memory resources. The DU requires one memory
-resource for the DU core (named "du") and one memory resource for each
-LVDS encoder (named "lvds.x" with "x" being the LVDS controller numerical
-index).
+  - reg: the memory-mapped I/O registers base address and length
 
   - interrupt-parent: phandle of the parent interrupt controller.
   - interrupts: Interrupt specifiers for the DU interrupts.
@@ -29,14 +24,13 @@ Required Properties:
   - clock-names: Name of the clocks. This property is model-dependent.
 - R8A7779 uses a single functional clock. The clock doesn't need to be
   named.
-- All other DU instances use one functional clock per channel and one
-  clock per LVDS encoder (if available). The functional clocks must be
-  named "du.x" with "x" being the channel numerical index. The LVDS clocks
-  must be named "lvds.x" with "x" being the LVDS encoder numerical index.
-- In addition to the functional and encoder clocks, all DU versions also
-  support externally supplied pixel clocks. Those clocks are optional.
-  When supplied they must be named "dclkin.x" with "x" being the input
-  clock numerical index.
+- All other DU instances use one functional clock per channel The
+  functional clocks must be named "du.x" with "x" being the channel
+  numerical index.
+- In addition to the functional clocks, all DU versions also support
+  externally supplied pixel clocks. Those clocks are optional. When
+  supplied they must be named "dclkin.x" with "x" being the input clock
+  numerical index.
 
   - vsps: A list of phandle and channel index tuples to the VSPs that handle
 the memory interfaces for the DU channels. The phandle identifies the VSP
@@ -69,9 +63,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
 
du: display@feb0 {
compatible = "renesas,du-r8a7795";
-   reg = <0 0xfeb0 0 0x8>,
- <0 0xfeb9 0 0x14>;
-   reg-names = "du", "lvds.0";
+   reg = <0 0xfeb0 0 0x8>;
interrupts = ,
 ,
 ,
@@ -79,9 +71,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
clocks = < CPG_MOD 724>,
 < CPG_MOD 723>,
 < CPG_MOD 722>,
-< CPG_MOD 721>,
-< CPG_MOD 727>;
-   clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0";
+< CPG_MOD 721>;
+   clock-names = "du.0", "du.1", "du.2", "du.3";
vsps = < 0>, < 0>, < 0>, < 1>;
 
ports {
-- 
Regards,

Laurent Pinchart

Re: [PATCH 05/15] ARM64: dts: Add R-Car Salvator-x M3-N support

[Philippe Ombredanne]

Jacopo,

On Wed, Feb 14, 2018 at 2:58 PM, Geert Uytterhoeven
 wrote:

>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
>> @@ -0,0 +1,30 @@
>> +// SPDX-License-Identifier: GPL-2.

This should be GPL-2.0



>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> @@ -0,0 +1,495 @@
>> +// SPDX-License-Identifier: GPL-2.

This should be GPL-2.0 too.
-- 
Cordially
Philippe Ombredanne

Re: [PATCH v2 3/3] drm: rcar-du: lvds: Refactor LVDS startup

[Sergei Shtylyov]

On 02/14/2018 09:13 PM, Laurent Pinchart wrote:

> From: Sergei Shtylyov 
> 
> After the recent corrections to the R-Car gen2/3 LVDS startup code, already
> similar enough at their ends rcar_lvds_enable_gen{2|3}() started asking for
> a merge and it's becoming actually necessary with the addition of the R-Car
> V3M (R8A77970) support -- this gen3 SoC has gen2-like LVDPLLCR layout.
> 
> Signed-off-by: Sergei Shtylyov 
> Reviewed-by: Laurent Pinchart 
> Tested-by: Laurent Pinchart 

   Well, your role wasn't limited to reviewning/testing, you'd clearly did some 
editing too... thus I was expecting to see some changelog.

> Signed-off-by: Laurent Pinchart 

   Your variant of my patch looks good otherwise. :-)

[...]

MBR, Sergei

[PATCH v2 1/3] drm: rcar-du: lvds: Fix LVDS startup on R-Car Gen2

[Laurent Pinchart]

From: Sergei Shtylyov 

According to the latest revision 2.00 of the R-Car Gen2 manual, the LVDS
and the bias circuit must be enabled after the LVDS I/O pins are
enabled, not before. Fix the Gen2 LVDS startup sequence accordingly.

While at it, also fix the comment preceding the first LVDCR0 write that
still talks about hardcoding the LVDS mode 0.

Fixes: 90374b5c25c9 ("drm/rcar-du: Add internal LVDS encoder support")
Signed-off-by: Sergei Shtylyov 
Reviewed-by: Laurent Pinchart 
Tested-by: Laurent Pinchart 
Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
index abbb7b25129a..dcffd3b59b69 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -59,11 +59,8 @@ static void rcar_du_lvdsenc_start_gen2(struct 
rcar_du_lvdsenc *lvds,
 
rcar_lvds_write(lvds, LVDPLLCR, pllcr);
 
-   /*
-* Select the input, hardcode mode 0, enable LVDS operation and turn
-* bias circuitry on.
-*/
-   lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_BEN | LVDCR0_LVEN;
+   /* Select the input and set the LVDS mode. */
+   lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
if (rcrtc->index == 2)
lvdcr0 |= LVDCR0_DUSEL;
rcar_lvds_write(lvds, LVDCR0, lvdcr0);
@@ -73,6 +70,10 @@ static void rcar_du_lvdsenc_start_gen2(struct 
rcar_du_lvdsenc *lvds,
LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
 
+   /* Enable LVDS operation and turn bias circuitry on. */
+   lvdcr0 |= LVDCR0_BEN | LVDCR0_LVEN;
+   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+
/*
 * Turn the PLL on, wait for the startup delay, and turn the output
 * on.
-- 
Regards,

Laurent Pinchart

[PATCH v2 0/3] R-Car DU: Fix and refactor LVDS to prepare for V3M support

[Laurent Pinchart]

Hello, 

This patch series fixes the LVDS startup sequence for Gen2 and Gen3
SoCs, and then proceeds to refactoring the code to merge the Gen2 and
Gen3 implementations in preparation for V3M support.

The patches have been previously posted as part of the following series.

[PATCH 0/2] Fix LVDS startup sequences in the R-Car DU driver...
[PATCH 0/3] Add R-Car V3M (R8A77970) support to the R-Car LVDS driver

They have since been rebased on top of the latest DU driver and the
current DRM -next branch.

For convenience the patches can be found at

git://linuxtv.org/pinchartl/media.git drm/next/du

They have been tested on H2 (Lager), H3 ES1.1 (Salvator-X) and H3 ES2.0
(Salvator-XS) without any issue.

The patches that add V3M support have been left out as they depend on
the LVDS encoder move to the DRM bridge API that is not ready for
upstream yet.

Sergei Shtylyov (3):
  drm: rcar-du: lvds: Fix LVDS startup on R-Car Gen2
  drm: rcar-du: lvds: Fix LVDS startup on R-Car Gen3
  drm: rcar-du: lvds: Refactor LVDS startup

 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 128 --
 1 file changed, 51 insertions(+), 77 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/3] drm: rcar-du: lvds: Refactor LVDS startup

[Laurent Pinchart]

From: Sergei Shtylyov 

After the recent corrections to the R-Car gen2/3 LVDS startup code, already
similar enough at their ends rcar_lvds_enable_gen{2|3}() started asking for
a merge and it's becoming actually necessary with the addition of the R-Car
V3M (R8A77970) support -- this gen3 SoC has gen2-like LVDPLLCR layout.

Signed-off-by: Sergei Shtylyov 
Reviewed-by: Laurent Pinchart 
Tested-by: Laurent Pinchart 
Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 132 --
 1 file changed, 51 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
index 01ef0f728e94..4defa8123eb2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -39,102 +39,37 @@ static void rcar_lvds_write(struct rcar_du_lvdsenc *lvds, 
u32 reg, u32 data)
iowrite32(data, lvds->mmio + reg);
 }
 
-static void rcar_du_lvdsenc_start_gen2(struct rcar_du_lvdsenc *lvds,
-  struct rcar_du_crtc *rcrtc)
+static u32 rcar_lvds_lvdpllcr_gen2(unsigned int freq)
 {
-   const struct drm_display_mode *mode = >crtc.mode;
-   unsigned int freq = mode->clock;
-   u32 lvdcr0;
-   u32 pllcr;
-
-   /* PLL clock configuration */
if (freq < 39000)
-   pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_38M;
+   return LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_38M;
else if (freq < 61000)
-   pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_60M;
+   return LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_60M;
else if (freq < 121000)
-   pllcr = LVDPLLCR_CEEN | LVDPLLCR_COSEL | 
LVDPLLCR_PLLDLYCNT_121M;
+   return LVDPLLCR_CEEN | LVDPLLCR_COSEL | LVDPLLCR_PLLDLYCNT_121M;
else
-   pllcr = LVDPLLCR_PLLDLYCNT_150M;
-
-   rcar_lvds_write(lvds, LVDPLLCR, pllcr);
-
-   /* Select the input and set the LVDS mode. */
-   lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
-   if (rcrtc->index == 2)
-   lvdcr0 |= LVDCR0_DUSEL;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
-   /* Turn all the channels on. */
-   rcar_lvds_write(lvds, LVDCR1,
-   LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
-   LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
-
-   /* Enable LVDS operation and turn bias circuitry on. */
-   lvdcr0 |= LVDCR0_BEN | LVDCR0_LVEN;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
-   /*
-* Turn the PLL on, wait for the startup delay, and turn the output
-* on.
-*/
-   lvdcr0 |= LVDCR0_PLLON;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
-   usleep_range(100, 150);
-
-   lvdcr0 |= LVDCR0_LVRES;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+   return LVDPLLCR_PLLDLYCNT_150M;
 }
 
-static void rcar_du_lvdsenc_start_gen3(struct rcar_du_lvdsenc *lvds,
-  struct rcar_du_crtc *rcrtc)
+static u32 rcar_lvds_lvdpllcr_gen3(unsigned int freq)
 {
-   const struct drm_display_mode *mode = >crtc.mode;
-   unsigned int freq = mode->clock;
-   u32 lvdcr0;
-   u32 pllcr;
-
-   /* Set the PLL clock configuration and LVDS mode. */
if (freq < 42000)
-   pllcr = LVDPLLCR_PLLDIVCNT_42M;
+   return LVDPLLCR_PLLDIVCNT_42M;
else if (freq < 85000)
-   pllcr = LVDPLLCR_PLLDIVCNT_85M;
+   return LVDPLLCR_PLLDIVCNT_85M;
else if (freq < 128000)
-   pllcr = LVDPLLCR_PLLDIVCNT_128M;
+   return LVDPLLCR_PLLDIVCNT_128M;
else
-   pllcr = LVDPLLCR_PLLDIVCNT_148M;
-
-   rcar_lvds_write(lvds, LVDPLLCR, pllcr);
-
-   lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
-   /* Turn all the channels on. */
-   rcar_lvds_write(lvds, LVDCR1,
-   LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
-   LVDCR1_CHSTBY(1) | LVDCR1_CHSTBY(0) | LVDCR1_CLKSTBY);
-
-   /*
-* Turn the PLL on, set it to LVDS normal mode, wait for the startup
-* delay and turn the output on.
-*/
-   lvdcr0 |= LVDCR0_PLLON;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
-   lvdcr0 |= LVDCR0_PWD;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
-
-   usleep_range(100, 150);
-
-   lvdcr0 |= LVDCR0_LVRES;
-   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+   return LVDPLLCR_PLLDIVCNT_148M;
 }
 
 static int rcar_du_lvdsenc_start(struct rcar_du_lvdsenc *lvds,
 struct rcar_du_crtc *rcrtc)
 {
+   

[PATCH v2 2/3] drm: rcar-du: lvds: Fix LVDS startup on R-Car Gen3

[Laurent Pinchart]

From: Sergei Shtylyov 

According to the latest revisions of the R-Car Gen3 manual, the LVDS mode
must be set before the LVDS I/O pins are enabled, not after -- fix the
Gen3 LVDS startup sequence accordingly.

Fixes: e947eccbeba4 ("drm: rcar-du: Add support for LVDS mode selection")
Signed-off-by: Sergei Shtylyov 
Reviewed-by: Laurent Pinchart 
[Updated comment in rcar_du_lvdsenc_start_gen3()]
[Moved Gen2 startup comment update to separate commit]
[Fixed =| typo]
Tested-by: Laurent Pinchart 
Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
index dcffd3b59b69..01ef0f728e94 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
@@ -95,7 +95,7 @@ static void rcar_du_lvdsenc_start_gen3(struct rcar_du_lvdsenc 
*lvds,
u32 lvdcr0;
u32 pllcr;
 
-   /* PLL clock configuration */
+   /* Set the PLL clock configuration and LVDS mode. */
if (freq < 42000)
pllcr = LVDPLLCR_PLLDIVCNT_42M;
else if (freq < 85000)
@@ -107,6 +107,9 @@ static void rcar_du_lvdsenc_start_gen3(struct 
rcar_du_lvdsenc *lvds,
 
rcar_lvds_write(lvds, LVDPLLCR, pllcr);
 
+   lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
+   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+
/* Turn all the channels on. */
rcar_lvds_write(lvds, LVDCR1,
LVDCR1_CHSTBY(3) | LVDCR1_CHSTBY(2) |
@@ -116,7 +119,7 @@ static void rcar_du_lvdsenc_start_gen3(struct 
rcar_du_lvdsenc *lvds,
 * Turn the PLL on, set it to LVDS normal mode, wait for the startup
 * delay and turn the output on.
 */
-   lvdcr0 = (lvds->mode << LVDCR0_LVMD_SHIFT) | LVDCR0_PLLON;
+   lvdcr0 |= LVDCR0_PLLON;
rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 
lvdcr0 |= LVDCR0_PWD;
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] drm: rcar-du: lvds: add R8A77970 support

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Friday, 19 January 2018 20:29:21 EET Sergei Shtylyov wrote:
> Add support for the R-Car V3M (R8A77970) SoC to the LVDS encoder driver.
> Note that there are some differences with the other R-Car gen3 SoCs, e.g.
> LVDPLLCR has the same layout as in the R-Car gen2 SoCs...
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c |   21 +++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> Index: linux/drivers/gpu/drm/rcar-du/rcar_lvds.c
> ===
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -32,6 +32,10 @@ enum rcar_lvds_mode {
>  };
> 
>  #define RCAR_LVDS_QUIRK_LANES(1 << 0)/* LVDS lanes 1 and 3 
> inverted */
> +#define RCAR_LVDS_QUIRK_GEN2_PLLCR (1 << 1)  /* LVDPLLCR has gen2-like */
> + /* layout on R8A77970 */
> +#define RCAR_LVDS_QUIRK_PHY  (1 << 2)/* LVDS has PHY on R8A77970 */
> + /* and  R8A7799{0|5} */

I'm not sure if that's the right explanation for this quirk. I assume the LVDS 
encoder to always have a PHY. The difference here is that it needs to be 
explicitly enabled. Note that the LVEN bit also exists in Gen2.

>  struct rcar_lvds_device_info {
>   unsigned int gen;
> @@ -162,6 +166,7 @@ static void rcar_lvds_enable(struct drm_
>*/
>   struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
>   const struct drm_display_mode *mode = >display_mode;
> + unsigned int quirks = lvds->info->quirks;
>   unsigned int gen = lvds->info->gen;
>   u32 lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
>   u32 lvdpllcr;
> @@ -186,7 +191,7 @@ static void rcar_lvds_enable(struct drm_
>   LVDCTRCR_CTR2SEL_DISP | LVDCTRCR_CTR1SEL_VSYNC |
>   LVDCTRCR_CTR0SEL_HSYNC);
> 
> - if (lvds->info->quirks & RCAR_LVDS_QUIRK_LANES)
> + if (quirks & RCAR_LVDS_QUIRK_LANES)
>   lvdhcr = LVDCHCR_CHSEL_CH(0, 0) | LVDCHCR_CHSEL_CH(1, 3)
> 
>  | LVDCHCR_CHSEL_CH(2, 2) | LVDCHCR_CHSEL_CH(3, 1);
> 
>   else
> @@ -195,7 +200,7 @@ static void rcar_lvds_enable(struct drm_
>   rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> 
>   /* PLL clock configuration */
> - if (gen < 3)
> + if (gen < 3 || quirks & RCAR_LVDS_QUIRK_GEN2_PLLCR)

I'd set the RCAR_LVDS_QUIRK_GEN2_PLLCR flag in all the Gen2 
rcar_lvds_device_info instances, and remove the gen check here.

>   lvdpllcr = rcar_lvds_lvdpllcr_gen2(mode->clock);
>   else
>   lvdpllcr = rcar_lvds_lvdpllcr_gen3(mode->clock);
> @@ -227,6 +232,12 @@ static void rcar_lvds_enable(struct drm_
>   rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>   }
> 
> + if (quirks & RCAR_LVDS_QUIRK_PHY) {
> + /* Turn on the LVDS PHY. */
> + lvdcr0 |= LVDCR0_LVEN;
> + rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> + }
> +
>   /* Wait for the startup delay. */
>   usleep_range(100, 150);
> 
> @@ -499,6 +510,11 @@ static const struct rcar_lvds_device_inf
>   .gen = 3,
>  };
> 
> +static const struct rcar_lvds_device_info rcar_lvds_r8a77970_info = {
> + .gen = 3,
> + .quirks = RCAR_LVDS_QUIRK_GEN2_PLLCR | RCAR_LVDS_QUIRK_PHY,
> +};
> +
>  static const struct of_device_id rcar_lvds_of_table[] = {
>   { .compatible = "renesas,r8a7743-lvds", .data = _lvds_gen2_info },
>   { .compatible = "renesas,r8a7790-lvds", .data = _lvds_r8a7790_info 
> },
> @@ -506,6 +522,7 @@ static const struct of_device_id rcar_lv
>   { .compatible = "renesas,r8a7793-lvds", .data = _lvds_gen2_info },
>   { .compatible = "renesas,r8a7795-lvds", .data = _lvds_gen3_info },
>   { .compatible = "renesas,r8a7796-lvds", .data = _lvds_gen3_info },
> + { .compatible = "renesas,r8a77970-lvds", .data = 
> _lvds_r8a77970_info
> }, { }
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: lvds: fix LVDS startup on R-Car gen3

[Laurent Pinchart]

Hi Sergei,

On Tuesday, 16 January 2018 17:42:41 EET Laurent Pinchart wrote:
> On Saturday, 13 January 2018 11:25:31 EET Sergei Shtylyov wrote:
> > On 1/13/2018 1:15 AM, Laurent Pinchart wrote:
>  According to the latest revisions of the R-Car gen3 manual, the LVDS
>  mode must be set before the LVDS I/O pins are enabled, not after -- 
>  fix the gen3 LVDS startup sequence accordingly...
>  
>  While  at it,  also fix the comment  preceding the first LVDCR0 write
>  in the R-Car gen2 startup code that still talks about hardcoding the
>  LVDS mode 0...
> >>> 
> >>> How about fixing that in patch 2/2 that touches the Gen2 initialization
> >>> sequence ? I think I'd even go as far as squashing both patches, I
> >>> don't think there's a need to split them.
> >>> 
>  Fixes: e947eccbeba4 ("drm: rcar-du: Add support for LVDS mode
>  selection")
>  Signed-off-by: Sergei Shtylyov 
> >>> 
> >>> Is this really needed ? Does it fix a problem you've experienced, or is
> >>> it theoretical only ? The mode shouldn't matter before the LVDS
> >>> internal logic is turned on. Unless there's a real issue I'm not sure we
> >>> should make the code more complex.
> >> 
> >> Furthermore the datasheet states
> >> 
> >> "3. This refers to settings other than those that are concerned with
> >> LVDS-IF startup. These items may be set while waiting for the conditions
> >> of step 6 to be met."
> > 
> > Ah, I hadn't paid much attention to this note. Howeve, it seems quite
> > vague to me... and there's no condition in step 6. ;-)
> 
> Lots of bits and pieces are lost in translation yes :-)
> 
> >> Doesn't this mean that the mode and input selector don't have to be set
> >> as the very first step, but can be programmed at any point before
> >> starting the LVDS encoder through the PWD bit (on Gen3) or the PLLON bit
> >> (on Gen2) ?
> > 
> > Frankly speaking, I don't know how to interpret that note...
> 
> My understanding is that the parameters can be programmed at any time before
> step 6. The fact that the current code works seems to confirm that
> interpretation. We could ask Renesas for a confirmation if you want.

I've received feedback, and while it wasn't clear what the not really means, 
Renesas recommends following the documented startup sequence (I'm still not 
sure it's really needed, but that's a different story). I'll thus rebase this 
patch and repost it, and take it in my tree if you're fine with the result.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: lvds: Fix LVDS clock frequency range

[Sergei Shtylyov]

On 01/13/2018 12:40 AM, Laurent Pinchart wrote:

> According to the latest versions of both the Gen2 and Gen3 datasheets,
> the operating range for the LVDS clock is 31 MHz to 148.5 MHz on all

   Not quite so with R-Car D3/E3 (which have the low boundary of 5 MHz) But we 
don't
support them yet anyway...

> SoCs. Update the driver accordingly.
> 
> Signed-off-by: Laurent Pinchart 
[...]

Acked-by: Sergei Shtylyov 

MBR, Sergei

Re: [PATCH 2/3] DT: display: renesas,lvds: document R8A77970 bindings

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Friday, 19 January 2018 20:29:20 EET Sergei Shtylyov wrote:
> Document the R-Car V3M (R8A77970) SoC in the R-Car LVDS bindings.
> 
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Laurent Pinchart 

> ---
>  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt |1 +
>  1 file changed, 1 insertion(+)
> 
> Index:
> linux/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> === ---
> linux.orig/Documentation/devicetree/bindings/display/bridge/renesas,lvds.tx
> t +++
> linux/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@
> -13,6 +13,7 @@ Required properties:
>- "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS
> encoders - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS
> encoders - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS
> encoders +  - "renesas,r8a77970-lvds" for R8A77970 (R-Car V3M) compatible
> LVDS encoders
> 
>  - reg: Base address and length for the memory-mapped registers
>  - clocks: A phandle + clock-specifier pair for the functional clock

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: Enable VSP compositor by default on Gen3

[Kieran Bingham]

Hi Laurent,

Thankyou for the patch.

On 12/01/18 03:20, Laurent Pinchart wrote:
> On Gen3 hardware the VSP compositor is required for display. Enable it
> by default in the kernel configuration. The option is kept
> user-configurable for testing purpose on Gen2 platforms.
> 
> Signed-off-by: Laurent Pinchart 

Reviewed-by: Kieran Bingham 


> ---
>  drivers/gpu/drm/rcar-du/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 1d6f1b849287..2b104daf56eb 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -27,7 +27,8 @@ config DRM_RCAR_LVDS
> Enable support for the R-Car Display Unit embedded LVDS encoders.
>  
>  config DRM_RCAR_VSP
> - bool "R-Car DU VSP Compositor Support"
> + bool "R-Car DU VSP Compositor Support" if ARM
> + default y if ARM64
>   depends on DRM_RCAR_DU
>   depends on VIDEO_RENESAS_VSP1=y || (VIDEO_RENESAS_VSP1 && DRM_RCAR_DU=m)
>   help
> 

Re: [PATCH/RFC 00/11] ARM: dts: rcar-gen2: Enable watchdog support

[Geert Uytterhoeven]

On Fri, Feb 9, 2018 at 11:11 AM, Geert Uytterhoeven
 wrote:
> On Thu, Feb 8, 2018 at 11:34 AM, Geert Uytterhoeven
>  wrote:
>> This patch series enables the builtin watchdog timer on R-Car Gen2 SoCs
>> on all supported boards, and builds on top of Fabrizio's "[RFC v4 00/26]
>> Fix watchdog on Renesas R-Car Gen2 and RZ/G1".  It is marked RFC as it
>> is known not to work on all SoCs and SoC revisions.
>>
>> Based on my experiments, there are 3 success/failure modes:
>>
>>   1. It works!
>>
>>  This is the case on R-Car M2-N ES1.0 and E2 ES1.0 (tested on gose
>>  and alt).
>
> It also succeeds on R-Car M2-W ES3.0 (tested on porter).

I have received a success report for R-Car H2 ES2.0 on lager, too.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wednesday, 14 February 2018 12:36:43 EET Niklas Söderlund wrote:
> There is no way for drivers to validate a colorspace value, which could
> be provided by user-space by VIDIOC_S_FMT for example. Add a helper to
> validate that the colorspace value is part of enum v4l2_colorspace.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  include/uapi/linux/videodev2.h | 4 
>  1 file changed, 4 insertions(+)
> 
> Hi,
> 
> I hope this is the correct header to add this helper to. I think it's
> since if it's in uapi not only can v4l2 drivers use it but tools like
> v4l-compliance gets access to it and can be updated to use this instead
> of the hard-coded check of just < 0xff as it was last time I checked.
> 
> * Changes since v1
> - Cast colorspace to u32 as suggested by Sakari and only check the upper
>   boundary to address a potential issue brought up by Laurent if the
>   data type tested is u32 which is not uncommon:
> 
> enum.c:30:16: warning: comparison of unsigned expression >= 0 is always
> true [-Wtype-limits]
>   return V4L2_COLORSPACE_IS_VALID(colorspace);
> 
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 9827189651801e12..1f27c0f4187cbded 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -238,6 +238,10 @@ enum v4l2_colorspace {
>   V4L2_COLORSPACE_DCI_P3= 12,
>  };
> 
> +/* Determine if a colorspace is defined in enum v4l2_colorspace */
> +#define V4L2_COLORSPACE_IS_VALID(colorspace) \
> + ((u32)(colorspace) <= V4L2_COLORSPACE_DCI_P3)
> +

Casting to u32 has the added benefit that the colorspace expression is 
evaluated once only, I like that.

Reviewed-by: Laurent Pinchart 

>  /*
>   * Determine how COLORSPACE_DEFAULT should map to a proper colorspace.
>   * This depends on whether this is a SDTV image (use SMPTE 170M), an


-- 
Regards,

Laurent Pinchart

Re: [PATCH 13/15] Documentation: devicetree: ravb: Add r8a77965

[Sergei Shtylyov]

Hello!

   You need to send this patch to netdev and Cc me as well...

On 02/13/2018 12:46 PM, Jacopo Mondi wrote:

> Add documentation for r8a77965 compatible string to renesas ravb device
> tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi 
[...]

Acked-by: Sergei Shtylyov 

MBR, Sergei

Re: [PATCH 15/15] ARM64: dts: r8a77965: Add EtherAVB device node

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:46 AM, Jacopo Mondi
 wrote:
> Populate the ethernet@e680 device node to enable Ethernet interface
> for R-Car M3-N (r8a77965) SoC.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 14/15] pinctrl: sh-pfc: r8a77965: Add EtherAVB groups/functions

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:46 AM, Jacopo Mondi
 wrote:
> Add EtherAVB groups and functions definitions for R-Car M3-N.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 09/15] pinctrl: sh-pfc: r8a77965: Add SCIFs groups/functions

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add SCIF[0-5] groups and pin function definitions for R-Car M3-N.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

Minor nit below...

> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> @@ -1577,10 +1577,306 @@ static const struct sh_pfc_pin pinmux_pins[] = {
> SH_PFC_PIN_NAMED_CFG(ROW_GROUP_A('T'), 30, ASEBRK, CFG_FLAGS),
>  };
>
> +/* - SCIF0 
> -- */
> +static const unsigned int scif0_data_pins[] = {
> +   /* RX, TX */
> +   RCAR_GP_PIN(5, 1), RCAR_GP_PIN(5, 2),
> +};
> +static const unsigned int scif0_data_mux[] = {
> +   RX0_MARK, TX0_MARK,
> +};
> +static const unsigned int scif0_clk_pins[] = {
> +   /* SCK */
> +   RCAR_GP_PIN(5, 0),
> +};
> +static const unsigned int scif0_clk_mux[] = {
> +   SCK0_MARK,
> +};
> +static const unsigned int scif0_ctrl_pins[] = {
> +   /* RTS, CTS */
> +   RCAR_GP_PIN(5, 4), RCAR_GP_PIN(5, 3),
> +};
> +static const unsigned int scif0_ctrl_mux[] = {
> +   RTS0_N_TANS_MARK, CTS0_N_MARK,

Without TANS please (cfr. recent fixes to pfc-r8a7796.c).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 04/15] pinctrl: sh-pfc: Initial R-Car M3-N support

[Geert Uytterhoeven]

Hi Jacopo,

On Wed, Feb 14, 2018 at 2:53 PM, jacopo mondi  wrote:
> On Wed, Feb 14, 2018 at 02:37:08PM +0100, Geert Uytterhoeven wrote:
>> On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
>>  wrote:
>> > Add initial PFC support for R-Car M3-N (r8a77965) SoC.
>> > No groups or functions defined, just pin and registers enumeration.
>> >
>> > Signed-off-by: Jacopo Mondi 
>>
>> Thanks for your patch!
>>
>> Looks mostly OK to me.
>> You do want to compare with pfc-r8a7796.c: all differences not related to
>> SATA_DEVSL, FSCLK, DU_DOTCLKIN2/3, and PRESET are issues that were fixed
>> recently in pfc-r8a7796.c, and should apply to pfc-r8a77965.c, too.
>>
>
> I have used the M3-W tables with the exception of the pins/groups you
> mentioned, that are clearly marked as different in the datasheet. At
> least, this was my intention :)
>
> I used v4.15 M3-W PFC tables, should I look in v4.16-rc1 or in
> renesas-drivers for updates?

Always look at the latest version in my sh-pfc branch ;-)

>> That leaves us with very few differences only, but it won't be trivial to 
>> have
>> a combined M3-W/N PFC driver, I'm afraid.
>
> Takes a certain degree of grep-foo to clearly highlight differences
> between the two version. Do you have any script/tools you use to
> compare PFC tables a bit more easily?

The "--no-index" option of git diff allows to compare files against each
other, instead of against some other version.
Can be combined with wdiff:

$ git help wdiff
`git wdiff' is aliased to `diff --color-words'

soc-dts-diff (https://www.spinics.net/lists/linux-renesas-soc/msg22630.html)
also helps, even for drivers.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 10/15] ARM64: dts: r8a77965: Add SCIF device nodes

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add SCIF[0-5] device nodes for M3-N (r8a77965) SoC.
>
> Signed-off-by: Jacopo Mondi 

> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi

> scif3: serial@e6c5 {
> -   /* placeholder */
> +   compatible = "renesas,scif-r8a7796",

renesas,scif-r8a77965

> +"renesas,rcar-gen3-scif", "renesas,scif";
> +   reg = <0 0xe6c5 0 64>;
> +   interrupts = ;
> +   clocks = < CPG_MOD 204>,
> +< CPG_CORE R8A77965_CLK_S3D1>,
> +<_clk>;
> +   clock-names = "fck", "brg_int", "scif_clk";
> +   dmas = < 0x57>, < 0x56>;
> +   dma-names = "tx", "rx";
> +   power-domains = < R8A77965_PD_ALWAYS_ON>;
> +   resets = < 204>;
> +   status = "disabled";
> };
>
> scif4: serial@e6c4 {
> -   /* placeholder */
> +   compatible = "renesas,scif-r8a7796",

renesas,scif-r8a77965

> +"renesas,rcar-gen3-scif", "renesas,scif";
> +   reg = <0 0xe6c4 0 64>;
> +   interrupts = ;
> +   clocks = < CPG_MOD 203>,
> +< CPG_CORE R8A77965_CLK_S3D1>,
> +<_clk>;
> +   clock-names = "fck", "brg_int", "scif_clk";
> +   dmas = < 0x59>, < 0x58>;
> +   dma-names = "tx", "rx";
> +   power-domains = < R8A77965_PD_ALWAYS_ON>;
> +   resets = < 203>;
> +   status = "disabled";
> };
>
> scif5: serial@e6f3 {
> -   /* placeholder */
> +   compatible = "renesas,scif-r8a7796",

renesas,scif-r8a77965

> +"renesas,rcar-gen3-scif", "renesas,scif";
> +   reg = <0 0xe6f3 0 64>;
> +   interrupts = ;
> +   clocks = < CPG_MOD 202>,
> +< CPG_CORE R8A77965_CLK_S3D1>,
> +<_clk>;
> +   clock-names = "fck", "brg_int", "scif_clk";
> +   dmas = < 0x5b>, < 0x5a>,
> +  < 0x5b>, < 0x5a>;
> +   dma-names = "tx", "rx", "tx", "rx";
> +   power-domains = < R8A77965_PD_ALWAYS_ON>;
> +   resets = < 202>;
> +   status = "disabled";

With the above fixed:
Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 12/15] ARM64: dts: r8a77965: Add GPIO nodes

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add GPIO nodes to r8a77965 SoC device tree file.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -201,38 +201,6 @@
> #power-domain-cells = <1>;
> };
>
> -   gpio0: gpio@e605 {
> -   /* placeholder */
> -   };
> -
> -   gpio1: gpio@e6051000 {
> -   /* placeholder */
> -   };
> -
> -   gpio2: gpio@e6052000 {
> -   /* placeholder */
> -   };
> -
> -   gpio3: gpio@e6053000 {
> -   /* placeholder */
> -   };
> -
> -   gpio4: gpio@e6054000 {
> -   /* placeholder */
> -   };
> -
> -   gpio5: gpio@e6055000 {
> -   /* placeholder */
> -   };
> -
> -   gpio6: gpio@e6055400 {
> -   /* placeholder */
> -   };
> -
> -   gpio7: gpio@e6055800 {
> -   /* placeholder */
> -   };
> -
> intc_ex: interrupt-controller@e61c {
> /* placeholder */
> };
> @@ -339,6 +307,126 @@
> dma-channels = <16>;
> };
>
> +   gpio0: gpio@e605 {

Why have you moved them?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 07/15] ARM64: dts: r8a77965: Add dmac device nods

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add dmac[0-2] device nodes for R-Car M3-N (r8a77965) SoC.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ravb: add support for changing MTU

[Niklas Söderlund]

Hi Sergei,

Thanks for your feedback.

On 2018-02-14 14:34:09 +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 02/13/2018 04:12 PM, Niklas Söderlund wrote:
> 
> >> On 02/12/2018 11:00 PM, Niklas Söderlund wrote:
> >>
> >>> Allow for chancing the MTU within the limit of the maximum size of a
> >>
> >>Changing. :-)
> > 
> > Yes :-)
> > 
> >>> descriptor (2048 bytes). Add the callback to change MTU from user-space
> >>> and take the configurable MTU into account when configuring the
> >>> hardware.
> >>>
> >>> Signed-off-by: Niklas Söderlund 
> >> [...]
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c 
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index c87f57ca44371586..a4870c9e42195802 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >>> @@ -300,9 +300,9 @@ static void ravb_ring_format(struct net_device *ndev, 
> >>> int q)
> >>>   for (i = 0; i < priv->num_rx_ring[q]; i++) {
> >>>   /* RX descriptor */
> >>>   rx_desc = >rx_ring[q][i];
> >>> - rx_desc->ds_cc = cpu_to_le16(PKT_BUF_SZ);
> >>> + rx_desc->ds_cc = cpu_to_le16(priv->rx_buf_sz);
> >>>   dma_addr = dma_map_single(ndev->dev.parent, 
> >>> priv->rx_skb[q][i]->data,
> >>> -   PKT_BUF_SZ,
> >>> +   le16_to_cpu(rx_desc->ds_cc),
> >>
> >>   Why not 'priv->rx_buf_sz'?
> > 
> > To align the arguments used with the one in ravb_rx() which uses 
> > le16_to_cpu(rx_desc->ds_cc) already before this patch.
> 
>Why?
> 
> > static bool ravb_rx(struct net_device *ndev, int *quota, int q)
> > {
> > ...
> > /* Refill the RX ring buffers. */
> > for (; priv->cur_rx[q] - priv->dirty_rx[q] > 0; 
> > priv->dirty_rx[q]++) {
> > ...
> > desc->ds_cc = cpu_to_le16(priv->rx_buf_sz);
> > 
> > if (!priv->rx_skb[q][entry]) {
> > ...
> > dma_addr = dma_map_single(ndev->dev.parent, 
> > skb->data,
> >   le16_to_cpu(desc->ds_cc),
> >   DMA_FROM_DEVICE);
> > ...
> > }
> > ...
> > }
> > ...
> > }
> > 
> > I have no preference one way or the other but I think both call sites 
> > should look the same :-)
> 
>Why? I don't like this idea at all...

OK, I will use 'priv->rx_buf_sz' in next version. But I still think it's 
confusing to not align the call sites :-)

> 
> >> [...]
> >>> @@ -346,6 +346,10 @@ static int ravb_ring_init(struct net_device *ndev, 
> >>> int q)
> >>>   int ring_size;
> >>>   int i;
> >>>  
> >>> + /* +16 gets room from the status from the card. */
> >>> + priv->rx_buf_sz = (ndev->mtu <= 1492 ? PKT_BUF_SZ : ndev->mtu) +
> >>> + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN + 16;
> >>
> >>Mhm, I don't think FCS gets added to the frame buffer...
> 
>It certainly isn't included, judging by the manuals... Instead 2-byte 
> checksum is
> included after the frame data (if checksumming is enabled).

OK, I will drop ETH_FCS_LEN from v2. Would you like a similar patch for 
sh_eth ?

> 
> > And why add 16?
> > 
> > And +16 is added as the comment above states, to leave from the 
> > descriptor status appended by the hardware.
> 
>I don't see any appended status in the manuals, do you?

You are correct, looks like I misunderstood the docs, I was thinking of 
the descriptor described in 50.4.4 (7) but I now see that is handled 
differently, will drop the +16 for v2. Thanks for spotting this!

> 
> > This is already the case 
> > with PKT_BUF_SZ which for ravb is is set to 1538. MTU (1500) + ETH_HLEN 
> > (14) + VLAN_HLEN(4) + ETH_FCS_LEN(4) + ravb status (16) == 1538.
> 
> > This is also what the sh_eth driver do and I think it's value to keep 
> > these to driver as similar as possible in this regard, would you not 
> 
>   The DMA hardware is totally different, so I don't see any value in 
> mirroring what sh_eth does...
> 
> > agree? If in deed the FSC is not added I think we should fix this for 
> > both drivers in a follow up commit.
> 
>Probably a good idea... :-)
> 
> [...]
> 
> MBR, Sergei

-- 
Regards,
Niklas Söderlund

Re: [PATCH 13/15] Documentation: devicetree: ravb: Add r8a77965

[Geert Uytterhoeven]

Subject prefix should be "dt-bindings: net: ravb:"

On Tue, Feb 13, 2018 at 10:46 AM, Jacopo Mondi
 wrote:
> Add documentation for r8a77965 compatible string to renesas ravb device
> tree bindings documentation.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 11/15] gpio: rcar: Add R-Car M3-N compatible string

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add compatible string for R-Car M3-N (r8a77965) in gpio-rcar.
>
> Signed-off-by: Jacopo Mondi 

> --- a/drivers/gpio/gpio-rcar.c
> +++ b/drivers/gpio/gpio-rcar.c
> @@ -360,6 +360,10 @@ static const struct of_device_id gpio_rcar_of_table[] = {
> /* Gen3 GPIO is identical to Gen2. */
> .data = _rcar_info_gen2,
> }, {
> +   .compatible = "renesas,gpio-r8a77965",
> +   /* Gen3 GPIO is identical to Gen2. */
> +   .data = _rcar_info_gen2,
> +   }, {

This part is not needed, as the driver already matches agains the generic
"renesas,rcar-gen3-gpio".

> .compatible = "renesas,rcar-gen1-gpio",
> .data = _rcar_info_gen1,
> }, {

With the above fixed:
Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 08/15] Documentation: devicetree: renesas,sci: Add r8a77965

[Geert Uytterhoeven]

Subject prefix should be "dt-bindings: serial: sh-sci: "

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add documentation for r8a77965 compatible string to reneass sci-serial

Renesas

> device tree bindings documentation.
>
> Signed-off-by: Jacopo Mondi 
> ---
>  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt 
> b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> index cf504d0..cbb418a 100644
> --- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> @@ -40,7 +40,9 @@ Required properties:
>  - "renesas,scif-r8a7795" for R8A7795 (R-Car H3) SCIF compatible UART.
>  - "renesas,hscif-r8a7795" for R8A7795 (R-Car H3) HSCIF compatible UART.
>  - "renesas,scif-r8a7796" for R8A7796 (R-Car M3-W) SCIF compatible UART.
> +- "renesas,scif-r8a77965" for R8A77965 (R-Car M3-N) SCIF compatible UART.
>  - "renesas,hscif-r8a7796" for R8A7796 (R-Car M3-W) HSCIF compatible UART.
> +- "renesas,hscif-r8a77965" for R8A77965 (R-Car M3-N) HSCIF compatible 
> UART.

Please keep both r8a77965 entries together, like is done for other SoCs.

>  - "renesas,scif-r8a77970" for R8A77970 (R-Car V3M) SCIF compatible UART.
>  - "renesas,hscif-r8a77970" for R8A77970 (R-Car V3M) HSCIF compatible 
> UART.
>  - "renesas,scif-r8a77995" for R8A77995 (R-Car D3) SCIF compatible UART.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] regulator: Fix resume from suspend to idle

[Sudeep Holla]

On Tue, Feb 13, 2018 at 9:37 AM, Geert Uytterhoeven
 wrote:
> When resuming from idle with the new suspend mode configuration support
> we go through the resume callbacks with a state of PM_SUSPEND_TO_IDLE
> which we don't have regulator constraints for, causing an error:
>
> dpm_run_callback(): regulator_resume_early+0x0/0x64 returns -22
> PM: Device regulator.0 failed to resume early: error -22
>
> Avoid this and similar errors by treating missing constraints as a noop.
>
> See also commit 57a0dd187956ea04 ("regulator: Fix suspend to idle"),
> which fixed the suspend part.
>
> Signed-off-by: Geert Uytterhoeven 

Tested-by: Sudeep Holla 

Re: [PATCH 06/15] Documentation: devicetree: dma: Add r8a77965 dmac

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add documentation for r8a77965 compatible string to rcar-dmac device
> tree bindings documentation.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 05/15] ARM64: dts: Add R-Car Salvator-x M3-N support

[Geert Uytterhoeven]

Hi Jacopo,

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add initial support for R-Car M3-N Salvator-x and r8a77965 SoC in
> device tree with cpg-mssr, reset and clock nodes.
>
> Add place-holder device nodes for all nodes referred by
> "salvator-common.dtsi"
>
> Signed-off-by: Jacopo Mondi 

Thanks for your patch!

Looks mostly fine to me.

P.S. scripts/dtc/dtx_diff arch/arm64/boot/dts/renesas/r8a7796{,5}-salvator-x.dtb
 is your friend.

>  arch/arm64/Kconfig.platforms   |   6 +
>  arch/arm64/boot/dts/renesas/Makefile   |   1 +
>  .../arm64/boot/dts/renesas/r8a77965-salvator-x.dts |  30 ++
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi  | 495 
> +
>  4 files changed, 532 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a77965.dtsi

The maintainer will probably ask you to split this in three parts:
  - ARCH_R8A77965
  - r8a77965.dtsi
  - r8a77965-salvator-x.dts

> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.
> +/*
> + * Device Tree Source for the Salvator-X board

with R-Car M3-N

> + *
> + * Copyright (C) 2018 Jacopo Mondi 
> + */
> +
> +/dts-v1/;
> +#include "r8a77965.dtsi"
> +#include "salvator-x.dtsi"
> +
> +/ {
> +   model = "Renesas Salvator-X board based on r8a77965";
> +   compatible = "renesas,salvator-x", "renesas,r8a77965";
> +
> +   aliases {
> +   serial0 = 
> +   };
> +
> +   chosen {
> +   bootargs = "ignore_loglevel rw root=/dev/nfs ip=dhcp";
> +   stdout-path = "serial0:115200n8";
> +   };

Both aliases and chosen are already defined in salvator-common.dtsi,
included via salvator-x.dtsi.

> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -0,0 +1,495 @@
> +// SPDX-License-Identifier: GPL-2.
> +/*
> + * Device Tree Source for the r8a77965 SoC
> + *
> + * Copyright (C) 2018 Jacopo Mondi 
> + *
> + * Based on r8a7796.dtsi
> + * Copyright (C) 2016 Renesas Electronics Corp.
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#define CPG_AUDIO_CLK_IR8A77965_CLK_S0D4
> +
> +/ {

> +   soc {
> +   compatible = "simple-bus";
> +   interrupt-parent = <>;
> +   #address-cells = <2>;
> +   #size-cells = <2>;
> +   ranges;

> +   timer {
> +   compatible = "arm,armv8-timer";
> +   interrupts =  +   (GIC_CPU_MASK_SIMPLE(2) | 
> IRQ_TYPE_LEVEL_LOW)>,
> + +   (GIC_CPU_MASK_SIMPLE(2) | 
> IRQ_TYPE_LEVEL_LOW)>,
> + +   (GIC_CPU_MASK_SIMPLE(2) | 
> IRQ_TYPE_LEVEL_LOW)>,
> + +   (GIC_CPU_MASK_SIMPLE(2) | 
> IRQ_TYPE_LEVEL_LOW)>;
> +   };

Please move the timer out of the soc node, as it does't have a unit address
and a reg property.

> +   pmu_a57 {
> +   compatible = "arm,cortex-a57-pmu";
> +   interrupts = ,
> +;
> +   interrupt-affinity = <_0>,
> +<_1>;
> +   };

Please move the pmu out of the soc node, as it does't have a unit address
and a reg property.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 04/15] pinctrl: sh-pfc: Initial R-Car M3-N support

[jacopo mondi]

Hi Geert,
   thanks for review

On Wed, Feb 14, 2018 at 02:37:08PM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
>  wrote:
> > Add initial PFC support for R-Car M3-N (r8a77965) SoC.
> > No groups or functions defined, just pin and registers enumeration.
> >
> > Signed-off-by: Jacopo Mondi 
>
> Thanks for your patch!
>
> Looks mostly OK to me.
> You do want to compare with pfc-r8a7796.c: all differences not related to
> SATA_DEVSL, FSCLK, DU_DOTCLKIN2/3, and PRESET are issues that were fixed
> recently in pfc-r8a7796.c, and should apply to pfc-r8a77965.c, too.
>

I have used the M3-W tables with the exception of the pins/groups you
mentioned, that are clearly marked as different in the datasheet. At
least, this was my intention :)

I used v4.15 M3-W PFC tables, should I look in v4.16-rc1 or in
renesas-drivers for updates?

> That leaves us with very few differences only, but it won't be trivial to have
> a combined M3-W/N PFC driver, I'm afraid.
>

Takes a certain degree of grep-foo to clearly highlight differences
between the two version. Do you have any script/tools you use to
compare PFC tables a bit more easily?

Thanks
  j

> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

[PATCH v2] device_tree: Increase FDT_MAX_SIZE to 1 MiB

[Geert Uytterhoeven]

It is not uncommon for a contemporary FDT to be larger than 64 KiB,
leading to failures loading the device tree from sysfs:

qemu-system-aarch64: qemu_fdt_setprop: Couldn't set ...: FDT_ERR_NOSPACE

Hence increase the limit to 1 MiB, like on PPC.

For reference, the largest arm64 DTB created from the Linux sources is
70 KiB large (93 KiB when built with symbols/fixup support).

Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Enlarge from 128 KiB to 1 MiB, as suggested by Peter Maydell.
---
 device_tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/device_tree.c b/device_tree.c
index a24ddff02bdd857c..9eb5fae7381fe885 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -29,7 +29,7 @@
 
 #include 
 
-#define FDT_MAX_SIZE  0x1
+#define FDT_MAX_SIZE  0x10
 
 void *create_device_tree(int *sizep)
 {
-- 
2.7.4

Re: [PATCH 03/15] soc: renesas: Add R-Car M3-N support

[Geert Uytterhoeven]

Hi Jacopo,

Thanks for your patch!

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add support for R-Car M3-N (r8a77965) power areas and reset.
> M3-N power areas are identical to M3-W ones, so just copy and rename
> them.

They are not identical:
  - M3-N does not have the CA53-related areas,
  - M3-W does not have A3VP,
  - M3-N does not have A2VC0 (M3-W also doesn't, according to latest
datasheet?).

The datasheet also mentions A3SH, without further info about the register
block. I think we need to bring this up with Renesas.

>  .../bindings/power/renesas,rcar-sysc.txt   |  1 +
>  .../devicetree/bindings/reset/renesas,rst.txt  |  1 +
>  drivers/soc/renesas/Kconfig|  9 --
>  drivers/soc/renesas/Makefile   |  1 +
>  drivers/soc/renesas/r8a77965-sysc.c| 37 
> ++
>  drivers/soc/renesas/rcar-rst.c |  1 +
>  drivers/soc/renesas/rcar-sysc.c|  3 ++
>  drivers/soc/renesas/rcar-sysc.h|  1 +
>  drivers/soc/renesas/renesas-soc.c  |  8 +
>  include/dt-bindings/power/r8a77965-sysc.h  | 31 ++
>  10 files changed, 91 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/soc/renesas/r8a77965-sysc.c
>  create mode 100644 include/dt-bindings/power/r8a77965-sysc.h

The maintainer may ask you to split this patch by functionality...

> --- /dev/null
> +++ b/drivers/soc/renesas/r8a77965-sysc.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas R-Car M3-N System Controller
> + * Copyright (C) 2018 Jacopo Mondi 
> + *
> + * Based on Renesas R-Car M3-W System Controller
> + * Copyright (C) 2016 Glider bvba
> + */
> +
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "rcar-sysc.h"
> +
> +static const struct rcar_sysc_area r8a77965_areas[] __initconst = {
> +   { "always-on",  0, 0, R8A77965_PD_ALWAYS_ON, -1, PD_ALWAYS_ON },
> +   { "ca57-scu",   0x1c0, 0, R8A77965_PD_CA57_SCU, R8A77965_PD_ALWAYS_ON,
> + PD_SCU },
> +   { "ca57-cpu0",   0x80, 0, R8A77965_PD_CA57_CPU0, R8A77965_PD_CA57_SCU,
> + PD_CPU_NOCR },
> +   { "ca57-cpu1",   0x80, 1, R8A77965_PD_CA57_CPU1, R8A77965_PD_CA57_SCU,
> + PD_CPU_NOCR },
> +   { "cr7",0x240, 0, R8A77965_PD_CR7,  R8A77965_PD_ALWAYS_ON 
> },
> +   { "a3vc",   0x380, 0, R8A77965_PD_A3VC, R8A77965_PD_ALWAYS_ON 
> },
> +   { "a2vc0",  0x3c0, 0, R8A77965_PD_A2VC0,R8A77965_PD_A3VC },

M3-N (and M3-W) does not have A2VC0?

> +   { "a2vc1",  0x3c0, 1, R8A77965_PD_A2VC1,R8A77965_PD_A3VC },
> +   { "3dg-a",  0x100, 0, R8A77965_PD_3DG_A,R8A77965_PD_ALWAYS_ON 
> },
> +   { "3dg-b",  0x100, 1, R8A77965_PD_3DG_B,R8A77965_PD_3DG_A },
> +   { "a3ir",   0x180, 0, R8A77965_PD_A3IR, R8A77965_PD_ALWAYS_ON 
> },

A3VP is missing?

> +};
> +
> +const struct rcar_sysc_info r8a77965_sysc_info __initconst = {
> +   .areas = r8a77965_areas,
> +   .num_areas = ARRAY_SIZE(r8a77965_areas),
> +};

> --- /dev/null
> +++ b/include/dt-bindings/power/r8a77965-sysc.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Jacopo Mondi 
> + * Copyright (C) 2016 Glider bvba
> + */
> +
> +#ifndef __DT_BINDINGS_POWER_R8A77965_SYSC_H__
> +#define __DT_BINDINGS_POWER_R8A77965_SYSC_H__
> +
> +/*
> + * These power domain indices match the numbers of the interrupt bits
> + * representing the power areas in the various Interrupt Registers
> + * (e.g. SYSCISR, Interrupt Status Register)
> + */
> +
> +#define R8A77965_PD_CA57_CPU0   0
> +#define R8A77965_PD_CA57_CPU1   1
> +#define R8A77965_PD_A3VP9
> +#define R8A77965_PD_CA57_SCU   12
> +#define R8A77965_PD_CR713
> +#define R8A77965_PD_A3VC   14
> +#define R8A77965_PD_3DG_A  17
> +#define R8A77965_PD_3DG_B  18
> +#define R8A77965_PD_A3IR   24
> +#define R8A77965_PD_A2VC0  25

M3-N (and M3-W) does not have A2VC0?

> +#define R8A77965_PD_A2VC1  26
> +
> +/* Always-on power area */
> +#define R8A77965_PD_ALWAYS_ON  32
> +
> +#endif /* __DT_BINDINGS_POWER_R8A77965_SYSC_H__ */

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ravb: add support for changing MTU

[Sergei Shtylyov]

Hello!

On 02/13/2018 04:12 PM, Niklas Söderlund wrote:

>> On 02/12/2018 11:00 PM, Niklas Söderlund wrote:
>>
>>> Allow for chancing the MTU within the limit of the maximum size of a
>>
>>Changing. :-)
> 
> Yes :-)
> 
>>> descriptor (2048 bytes). Add the callback to change MTU from user-space
>>> and take the configurable MTU into account when configuring the
>>> hardware.
>>>
>>> Signed-off-by: Niklas Söderlund 
>> [...]
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c 
>>> b/drivers/net/ethernet/renesas/ravb_main.c
>>> index c87f57ca44371586..a4870c9e42195802 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>> @@ -300,9 +300,9 @@ static void ravb_ring_format(struct net_device *ndev, 
>>> int q)
>>> for (i = 0; i < priv->num_rx_ring[q]; i++) {
>>> /* RX descriptor */
>>> rx_desc = >rx_ring[q][i];
>>> -   rx_desc->ds_cc = cpu_to_le16(PKT_BUF_SZ);
>>> +   rx_desc->ds_cc = cpu_to_le16(priv->rx_buf_sz);
>>> dma_addr = dma_map_single(ndev->dev.parent, 
>>> priv->rx_skb[q][i]->data,
>>> - PKT_BUF_SZ,
>>> + le16_to_cpu(rx_desc->ds_cc),
>>
>>   Why not 'priv->rx_buf_sz'?
> 
> To align the arguments used with the one in ravb_rx() which uses 
> le16_to_cpu(rx_desc->ds_cc) already before this patch.

   Why?

>   static bool ravb_rx(struct net_device *ndev, int *quota, int q)
>   {
>   ...
>   /* Refill the RX ring buffers. */
>   for (; priv->cur_rx[q] - priv->dirty_rx[q] > 0; 
> priv->dirty_rx[q]++) {
>   ...
>   desc->ds_cc = cpu_to_le16(priv->rx_buf_sz);
> 
>   if (!priv->rx_skb[q][entry]) {
>   ...
>   dma_addr = dma_map_single(ndev->dev.parent, 
> skb->data,
> le16_to_cpu(desc->ds_cc),
> DMA_FROM_DEVICE);
>   ...
>   }
>   ...
>   }
>   ...
>   }
> 
> I have no preference one way or the other but I think both call sites 
> should look the same :-)

   Why? I don't like this idea at all...

>> [...]
>>> @@ -346,6 +346,10 @@ static int ravb_ring_init(struct net_device *ndev, int 
>>> q)
>>> int ring_size;
>>> int i;
>>>  
>>> +   /* +16 gets room from the status from the card. */
>>> +   priv->rx_buf_sz = (ndev->mtu <= 1492 ? PKT_BUF_SZ : ndev->mtu) +
>>> +   ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN + 16;
>>
>>Mhm, I don't think FCS gets added to the frame buffer...

   It certainly isn't included, judging by the manuals... Instead 2-byte 
checksum is
included after the frame data (if checksumming is enabled).

> And why add 16?
> 
> And +16 is added as the comment above states, to leave from the 
> descriptor status appended by the hardware.

   I don't see any appended status in the manuals, do you?

> This is already the case 
> with PKT_BUF_SZ which for ravb is is set to 1538. MTU (1500) + ETH_HLEN 
> (14) + VLAN_HLEN(4) + ETH_FCS_LEN(4) + ravb status (16) == 1538.

> This is also what the sh_eth driver do and I think it's value to keep 
> these to driver as similar as possible in this regard, would you not 

  The DMA hardware is totally different, so I don't see any value in mirroring 
what sh_eth does...

> agree? If in deed the FSC is not added I think we should fix this for 
> both drivers in a follow up commit.

   Probably a good idea... :-)

[...]

MBR, Sergei

Re: [PATCH 02/15] clk: renesas: cpg-msr: Add support for R-Car M3-N

[Geert Uytterhoeven]

Hi Jacopo,

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Initial support for R-Car M3-N (r8a77965), including core and module
> clocks.
>
> Signed-off-by: Jacopo Mondi 

Thanks for your patch!

Please refer to Table 8.2d of R-Car Series, 3rd Generation User's Manual:
Hardware (Rev. 0.80, Oct 31, 2017), so we know which exact version of
the datasheet
was used for the core clock definitions.

> --- /dev/null
> +++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
> @@ -0,0 +1,333 @@

> +static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = {
> +   DEF_MOD("scif5",202,R8A77965_CLK_S3D4),
> +   DEF_MOD("scif4",203,R8A77965_CLK_S3D4),
> +   DEF_MOD("scif3",204,R8A77965_CLK_S3D4),
> +   DEF_MOD("scif1",206,R8A77965_CLK_S3D4),
> +   DEF_MOD("scif0",207,R8A77965_CLK_S3D4),
> +   DEF_MOD("sys-dmac2",217,R8A77965_CLK_S0D3),
> +   DEF_MOD("sys-dmac1",218,R8A77965_CLK_S0D3),
> +   DEF_MOD("sys-dmac0",219,R8A77965_CLK_S0D3),
> +
> +   DEF_MOD("cmt3", 300,R8A77965_CLK_R),
> +   DEF_MOD("cmt2", 301,R8A77965_CLK_R),
> +   DEF_MOD("cmt1", 302,R8A77965_CLK_R),
> +   DEF_MOD("cmt0", 303,R8A77965_CLK_R),
> +   DEF_MOD("scif2",310,R8A77965_CLK_S3D4),
> +   DEF_MOD("sdif3",311,R8A77965_CLK_SD3),
> +   DEF_MOD("sdif2",312,R8A77965_CLK_SD2),
> +   DEF_MOD("sdif1",313,R8A77965_CLK_SD1),
> +   DEF_MOD("sdif0",314,R8A77965_CLK_SD0),
> +   DEF_MOD("pcie1",318,R8A77965_CLK_S3D1),
> +   DEF_MOD("pcie0",319,R8A77965_CLK_S3D1),
> +   DEF_MOD("usb3-if0", 328,R8A77965_CLK_S3D1),
> +   DEF_MOD("usb-dmac0",330,R8A77965_CLK_S3D1),
> +   DEF_MOD("usb-dmac1",331,R8A77965_CLK_S3D1),
> +
> +   DEF_MOD("rwdt", 402,R8A77965_CLK_R),
> +   DEF_MOD("intc-ex",  407,R8A77965_CLK_CP),
> +   DEF_MOD("intc-ap",  408,R8A77965_CLK_S3D1),

According to Figure 12A.1 the parent clock is S0D3. See also commit
6e7ddf89d67c2b0c ("clk: renesas: r8a7796: Correct parent clock of INTC-AP").

> +static int __init r8a77965_cpg_mssr_init(struct device *dev)
> +{

[...]

> +
> +   return rcar_gen3_cpg_init(cpg_pll_config, CLK_EXTALR, cpg_mode);
> +};

Stray semicolon.

With the above fixed:
Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH/RFC 5/5] hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic instantiation

[Auger Eric]

Hi Geert,

On 09/02/18 16:17, Geert Uytterhoeven wrote:
> Allow the instantiation of a Renesas R-Car Gen3 GPIO controller device
> from the QEMU command line:
> 
> -device vfio-platform,host=,manufacturer=renesas,model=rcar-gen3-gpio
> -device 
> vfio-platform,sysfsdev=,manufacturer=renesas,model=rcar-gen3-gpio
> 
> A specialized device tree node is created for the guest, containing
> compatible, reg, gpio-controller, and #gpio-cells properties.
> 
> Not-Yet-Signed-off-by: Geert Uytterhoeven 
> ---
> Question:
>   - Why do we need the manufacturer=foo,model=bar syntax? Can't this
> just be extracted from the host DT?
I think this could be achieved that way too. We just need to pay
attention to the fact the dt node creation function matches the exact
same compatible property value.

> 
> TODO:
>   - Copy properties from the host DT, as add_amd_xgbe_fdt_node() does,
>   - Make this more generic?

Yes I think devising helpers to generate regs/interrupts properties
could help reducing the amount of code to be written

Thanks

Eric
> ---
>  hw/arm/sysbus-fdt.c | 47 +++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index c5d4fd5604c28118..428175f343d9f3b9 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -416,6 +416,52 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, 
> void *opaque)
>  return 0;
>  }
>  
> +/**
> + * add_rcar_gpio_fdt_node
> + *
> + * Generates a simple node with following properties:
> + * compatible string, regs, #gpio-cells, gpio-controller
> + */
> +static int add_rcar_gpio_fdt_node(SysBusDevice *sbdev, void *opaque)
> +{
> +PlatformBusFDTData *data = opaque;
> +PlatformBusDevice *pbus = data->pbus;
> +void *fdt = data->fdt;
> +const char *parent_node = data->pbus_node_name;
> +int compat_str_len, i;
> +char *nodename;
> +uint32_t *reg_attr;
> +uint64_t mmio_base;
> +VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
> +VFIODevice *vbasedev = >vbasedev;
> +
> +mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> +nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
> +   vbasedev->name, mmio_base);
> +qemu_fdt_add_subnode(fdt, nodename);
> +
> +compat_str_len = strlen(vdev->compat) + 1;
> +qemu_fdt_setprop(fdt, nodename, "compatible",
> +  vdev->compat, compat_str_len);
> +
> +qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
> +qemu_fdt_setprop_cells(fdt, nodename, "#gpio-cells", 2);
> +
> +reg_attr = g_new(uint32_t, vbasedev->num_regions * 2);
> +for (i = 0; i < vbasedev->num_regions; i++) {
> +mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i);
> +reg_attr[2 * i] = cpu_to_be32(mmio_base);
> +reg_attr[2 * i + 1] = cpu_to_be32(
> +memory_region_size(vdev->regions[i]->mem));
> +}
> +qemu_fdt_setprop(fdt, nodename, "reg", reg_attr,
> + vbasedev->num_regions * 2 * sizeof(uint32_t));
> +
> +g_free(reg_attr);
> +g_free(nodename);
> +return 0;
> +}
> +
>  /* manufacturer/model matching */
>  static bool vfio_platform_match(SysBusDevice *sbdev,
>  const BindingEntry *entry)
> @@ -454,6 +500,7 @@ static const BindingEntry bindings[] = {
>  TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
>  TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
>  VFIO_PLATFORM_BINDING("amd", "xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
> +VFIO_PLATFORM_BINDING("renesas", "rcar-gen3-gpio", 
> add_rcar_gpio_fdt_node),
>  #endif
>  TYPE_BINDING("", NULL), /* last element */
>  };
> 

Re: [PATCH v2] videodev2.h: add helper to validate colorspace

[Sakari Ailus]

On Wed, Feb 14, 2018 at 11:36:43AM +0100, Niklas Söderlund wrote:
> There is no way for drivers to validate a colorspace value, which could
> be provided by user-space by VIDIOC_S_FMT for example. Add a helper to
> validate that the colorspace value is part of enum v4l2_colorspace.
> 
> Signed-off-by: Niklas Söderlund 

Acked-by: Sakari Ailus 

> ---
>  include/uapi/linux/videodev2.h | 4 
>  1 file changed, 4 insertions(+)
> 
> Hi,
> 
> I hope this is the correct header to add this helper to. I think it's
> since if it's in uapi not only can v4l2 drivers use it but tools like
> v4l-compliance gets access to it and can be updated to use this instead
> of the hard-coded check of just < 0xff as it was last time I checked.
> 
> * Changes since v1
> - Cast colorspace to u32 as suggested by Sakari and only check the upper 
>   boundary to address a potential issue brought up by Laurent if the 
>   data type tested is u32 which is not uncommon:
> 
> enum.c:30:16: warning: comparison of unsigned expression >= 0 is always 
> true
> [-Wtype-limits]
>   return V4L2_COLORSPACE_IS_VALID(colorspace);
> 
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 9827189651801e12..1f27c0f4187cbded 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -238,6 +238,10 @@ enum v4l2_colorspace {
>   V4L2_COLORSPACE_DCI_P3= 12,
>  };
>  
> +/* Determine if a colorspace is defined in enum v4l2_colorspace */
> +#define V4L2_COLORSPACE_IS_VALID(colorspace) \
> + ((u32)(colorspace) <= V4L2_COLORSPACE_DCI_P3)
> +
>  /*
>   * Determine how COLORSPACE_DEFAULT should map to a proper colorspace.
>   * This depends on whether this is a SDTV image (use SMPTE 170M), an
> -- 
> 2.16.1
> 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v3 00/16] mmc: tmio: another batch of TMIO MMC fixes and cleanups

[Ulf Hansson]

On 14 February 2018 at 11:23, Wolfram Sang  wrote:
>
>> You need to check again. :-)
>
> Seems like it. Sorry for the noise.
>
>> I am eager to apply those patches you already have reviewed (to
>> patch14), as those mostly seemed rather trivial and should be nice
>> cleanups.
>>
>> Do you mind me going ahead, then you can continue and review/test the rest?
>
> Yes, we can work incremently from there. I'll send patches if I discover
> something unexpected.
>

Great! So, applied patches 7->14.

Thanks and kind regards
Uffe

Re: [PATCH] mmc: sh_mmcif: remove some cruft

[Ulf Hansson]

On 5 February 2018 at 14:28, Wolfram Sang
 wrote:
> The TODO section from 2010 is obsolete. We have DMA and PM meanwhile and
> we always want to handle errors better, if possible. Also DRIVER_VERSION
> is not used anymore these days.
>
> Signed-off-by: Wolfram Sang 

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sh_mmcif.c | 8 
>  1 file changed, 8 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 7bb00c68a7561a..4c2a1f8ddbf324 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -7,13 +7,6 @@
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
>   * the Free Software Foundation; either version 2 of the License.
> - *
> - *
> - * TODO
> - *  1. DMA
> - *  2. Power management
> - *  3. Handle MMC errors better
> - *
>   */
>
>  /*
> @@ -67,7 +60,6 @@
>  #include 
>
>  #define DRIVER_NAME"sh_mmcif"
> -#define DRIVER_VERSION "2010-04-28"
>
>  /* CE_CMD_SET */
>  #define CMD_MASK   0x3f00
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again

[Auger Eric]

Hi Geert,
On 09/02/18 16:17, Geert Uytterhoeven wrote:
> Allow the instantation of generic dynamic sysbus devices again, without
> the need to create a new device-specific vfio type.
> 
> This is a partial revert of commit  6f2062b9758ebc64 ("hw/arm/virt:
> Allow only supported dynamic sysbus devices").
> 
> Not-Yet-Signed-off-by: Geert Uytterhoeven 
> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b334c82edae9fb1f..fa83784fc08ed076 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->max_cpus = 255;
>  machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>  machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
> +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
Couldn't you limit that to TYPE_VFIO_PLATFORM instead?

Thanks

Eric
>  mc->block_default_type = IF_VIRTIO;
>  mc->no_cdrom = 1;
>  mc->pci_allow_0_address = true;
> 

[PATCH v2] videodev2.h: add helper to validate colorspace

[Niklas Söderlund]

There is no way for drivers to validate a colorspace value, which could
be provided by user-space by VIDIOC_S_FMT for example. Add a helper to
validate that the colorspace value is part of enum v4l2_colorspace.

Signed-off-by: Niklas Söderlund 
---
 include/uapi/linux/videodev2.h | 4 
 1 file changed, 4 insertions(+)

Hi,

I hope this is the correct header to add this helper to. I think it's
since if it's in uapi not only can v4l2 drivers use it but tools like
v4l-compliance gets access to it and can be updated to use this instead
of the hard-coded check of just < 0xff as it was last time I checked.

* Changes since v1
- Cast colorspace to u32 as suggested by Sakari and only check the upper 
  boundary to address a potential issue brought up by Laurent if the 
  data type tested is u32 which is not uncommon:

enum.c:30:16: warning: comparison of unsigned expression >= 0 is always true
[-Wtype-limits]
  return V4L2_COLORSPACE_IS_VALID(colorspace);

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 9827189651801e12..1f27c0f4187cbded 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -238,6 +238,10 @@ enum v4l2_colorspace {
V4L2_COLORSPACE_DCI_P3= 12,
 };
 
+/* Determine if a colorspace is defined in enum v4l2_colorspace */
+#define V4L2_COLORSPACE_IS_VALID(colorspace)   \
+   ((u32)(colorspace) <= V4L2_COLORSPACE_DCI_P3)
+
 /*
  * Determine how COLORSPACE_DEFAULT should map to a proper colorspace.
  * This depends on whether this is a SDTV image (use SMPTE 170M), an
-- 
2.16.1

Re: [PATCH 01/15] Documentation: devicetree: R-Car M3-N SoC DT bindings

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 10:45 AM, Jacopo Mondi
 wrote:
> Add device tree bindings documentation for Renesas R-Car M3-N (r8a77965)
> SoC.
>
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 1/5] soc: renesas: Add symbols for R-Car Gen3 register offsets

[Geert Uytterhoeven]

Hi Simon,

On Mon, Feb 12, 2018 at 4:44 PM, Simon Horman
 wrote:
> Add symbols for Gen3 register offsets.
> These may be used to improve readability of users of these offsets.
>
> This does not introduce any functional change.
>
> Signed-off-by: Simon Horman 

Thanks for your patch!

> --- a/drivers/soc/renesas/rcar-sysc.h
> +++ b/drivers/soc/renesas/rcar-sysc.h
> @@ -24,6 +24,25 @@
>  #define PD_CPU_NOCRPD_CPU | PD_NO_CR /* CPU area lacks CR (R-Car Gen2/3) 
> */
>  #define PD_ALWAYS_ON   PD_NO_CR  /* Always-on area */
>
> +/*
> + * R-Car Gen3 register offsets
> + */
> +
> +#define RCAR_GEN3_SYSCSR  0

Please drop this. The "always-on" domains use 0 to indicate "no register
block", not as a reference to the SYSCSR register.

> +#define RCAR_GEN3_PWRSR0  0x80
> +#define RCAR_GEN3_PWRSR2  0x100
> +#define RCAR_GEN3_PWRSR3  0x140
> +#define RCAR_GEN3_PWRSR4  0x180
> +#define RCAR_GEN3_PWRSR5  0x1c0
> +#define RCAR_GEN3_PWRSR6  0x200
> +#define RCAR_GEN3_PWRSR7  0x240
> +#define RCAR_GEN3_PWRSR8  0x340
> +#define RCAR_GEN3_PWRSR9  0x380
> +#define RCAR_GEN3_PWRSR10 0x3c0
> +#define RCAR_GEN3_PWRSR11 0x400
> +#define RCAR_GEN3_PWRSR12 0x2c0
> +#define RCAR_GEN3_PWRSR13 0x300
> +#define RCAR_GEN3_PWRSR14 0x280

What about

#define RCAR_GEN3_PWRSR(n) (0x80 + (n) * 0x40)

instead?
Bummer, they have a hole between 0x240 and 0x340, which they filled later :-(
So I don't know how stable the PWRSRx indices are...

For R-Car H1 and Gen2 you can use the following, though:

#define RCAR_H1_PWRSR(n) (0x40 + (n) * 0x40)
#define RCAR_GEN2_PWRSR(n) (0x40 + (n) * 0x40)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 00/16] mmc: tmio: another batch of TMIO MMC fixes and cleanups

[Wolfram Sang]

> You need to check again. :-)

Seems like it. Sorry for the noise.

> I am eager to apply those patches you already have reviewed (to
> patch14), as those mostly seemed rather trivial and should be nice
> cleanups.
> 
> Do you mind me going ahead, then you can continue and review/test the rest?

Yes, we can work incremently from there. I'll send patches if I discover
something unexpected.



signature.asc
Description: PGP signature

Re: [PATCH] ARM: dts: stout: Initial r8a7790 Stout board support

[Marek Vasut]

On 02/14/2018 07:05 AM, Wolfram Sang wrote:
>> +chosen {
>> +bootargs = "ignore_loglevel rw root=/dev/nfs ip=dhcp";
>> +stdout-path = "serial0:38400n8";
> 
> Hmm, that would be our first board to not have 115200, or? We already
> have some other boards upstream where UBoot has 38400 and Linux 115200.
> I'd prefer consistency here a little, but no strong opinion and asking
> for other opinions.

The vendoruboot is wired to 38400 (sigh), I'm planning to set mainline
uboot to 115200 though.

>> +vcc_sdhi0: regulator-vcc-sdhi0 {
>> +compatible = "regulator-fixed";
>> +
>> +regulator-name = "SDHI0 Vcc";
>> +regulator-min-microvolt = <330>;
>> +regulator-max-microvolt = <330>;
>> +
>> +gpio = < 24 GPIO_ACTIVE_HIGH>;
>> +enable-active-high;
>> +};
> 
> Just double checking: No 1.8V for SDR50/104?

Nope, the SD card slot is hard-wired to 3V3.

-- 
Best regards,
Marek Vasut

Re: [PATCH 2/2] vfio: platform: Add generic DT reset support

[Auger Eric]

Hi Geert,

On 14/02/18 10:43, Geert Uytterhoeven wrote:
> Hi Eric,
> 
> On Wed, Feb 14, 2018 at 10:09 AM, Auger Eric  wrote:
>> On 13/02/18 17:36, Geert Uytterhoeven wrote:
>>> Vfio-platform requires reset support, provided either by ACPI, or, on DT
>>> platforms, by a device-specific reset driver matching against the
>>> device's compatible value.
>>>
>>> On many SoCs, devices are connected to an SoC-internal reset controller,
>>> and can be reset in a generic way.  Hence add support to reset such
>>> devices using the reset controller subsystem, provided the reset
>>> hierarchy is described correctly in DT using the "resets" property.
>>
>> I first acknowledge I am not familiar with what those reset controllers
>> do in practice. My fear is that we may rely on generic SW pieces that
>> may not be adapted to passthrough constraints. We must guarantee that
>> any DMA access attempted by the devices are stopped and any interrupts
>> gets stopped. Can we guarantee that the reset controller always induce
>> that? Otherwise we may leave the door opened to badly reset assigned
>> devices.
> 
> An on-SoC reset controller is basically a block controlling signals to the
> reset inputs of the individual on-SoC devices.
> On Renesas ARM SoCs, this allows to do a full reset of the attached device.
> 
> Of course the exact semantics depend on the actual SoC.
that's the issue actually.
> If e.g. DMA and interrupts are not stopped for a specific device on a
> specific SoC, it still needs a device-specific reset driver, matching against
> the appropriate compatible value, cfr. the quoted paragraph below.
yes but by default we still accept the reset controller solution.
> 
> You could add a whitelist for of_machine_is_compatible() or
> of_device_is_compatible(), but that will grow large fast.
Could be the kind of solution needed. At the moment the list of assigned
platform devices is pretty reduced.

Couldn't we imagine additional dt properties to emphasize the fact a
platform device is passthrough friendly in terms of reset, either
through a reset controller or exposing a single reg that need to be
reset for full reset to be achieved, in accordance with assignment
constraints. That way, the driver writer would somehow certify the
device is eligible for passthrough. One of the issue today is that the
vfio platform reset driver is not maintained by the native driver
maintainer.

I think if people want to do platform passthrough, they need to devise
their HW IPs so that this reset modality is simplified by exposing this
kind of single reg and then dt description may expose this. Also if
possible, the dt node must be as simple/generic as possible to avoid
writing a huge dt node creation function on QEMU side and avoid
dependencies on other nodes.


> 
>>> Devices that require a more complex reset procedure can still
>>> provide a device-specific reset driver, as that takes precedence.
> 
>>> @@ -138,6 +152,8 @@ static void vfio_platform_put_reset(struct 
>>> vfio_platform_device *vdev)
>>>
>>>   if (vdev->of_reset)
>>>   module_put(vdev->reset_module);
>>> +
>> if (vdev->reset_control) ?
>> reset_control_put seems to only check IS_ERR()
> 
> void reset_control_put(struct reset_control *rstc)
> {
> if (IS_ERR_OR_NULL(rstc))
> return;
> 
> So it does handle NULL.
Sorry I checked an older 4.3 kernel version.

Thanks

Eric
> 
>>> + reset_control_put(vdev->reset_control);
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> 

Re: [PATCH v3 3/5] arm64: dts: renesas: r8a7795-es1: Fix register mappings on VSPs

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wednesday, 14 February 2018 11:55:06 EET Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> The VSPD includes a CLUT on RPF2. Ensure that the register space is
> mapped correctly to support this.
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

> ---
>  arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi
> b/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi index
> ed553338b4d4..1adfe6cad268 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi
> @@ -80,7 +80,7 @@
> 
>   vspd3: vsp@fea38000 {
>   compatible = "renesas,vsp2";
> - reg = <0 0xfea38000 0 0x4000>;
> + reg = <0 0xfea38000 0 0x8000>;
>   interrupts = ;
>   clocks = < CPG_MOD 620>;
>   power-domains = < R8A7795_PD_ALWAYS_ON>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] arm64: defconfig: Enable USB 3.0 PHY for R-Car

[Geert Uytterhoeven]

On Wed, Feb 14, 2018 at 7:23 AM, Yoshihiro Shimoda
 wrote:
> Enable the R-Car's USB 3.0 PHY.
>
> Signed-off-by: Yoshihiro Shimoda 

Acked-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 2/4] arm64: defconfig: Enable USB-DMAC for R-Car

[Geert Uytterhoeven]

On Wed, Feb 14, 2018 at 7:23 AM, Yoshihiro Shimoda
 wrote:
> Enable the R-Car's USB-DMAC controller. This controller will be used
> by HS-USB (renesas_usbhs driver). Since the renesas_usbhs driver
> (CONFIG_USB_RENESAS_USBHS_UDC) is enabled as module, the USB-DMAC
> driver is also enabled as module.
>
> Signed-off-by: Yoshihiro Shimoda 

Acked-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 1/4] arm64: defconfig: Enable PWM for R-Car

[Geert Uytterhoeven]

On Wed, Feb 14, 2018 at 7:23 AM, Yoshihiro Shimoda
 wrote:
> Enable the Renesas R-Car's PWM controller.
>
> Signed-off-by: Yoshihiro Shimoda 

Acked-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 2/2] arm64: dts: renesas: eagle: specify EtherAVB PHY IRQ

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 12:24 PM, Sergei Shtylyov
 wrote:
> Specify  EtherAVB PHY IRQ  in the Eagle board's device tree, now that we
> have the GPIO support (previously phylib had to resort to polling).
>
> Signed-off-by: Sergei Shtylyov 

You've dropped my:
Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 01/15] Documentation: devicetree: R-Car M3-N SoC DT bindings

[Simon Horman]

On Tue, Feb 13, 2018 at 10:45:48AM +0100, Jacopo Mondi wrote:
> Add device tree bindings documentation for Renesas R-Car M3-N (r8a77965)
> SoC.
> 
> Signed-off-by: Jacopo Mondi 

Thanks, this looks fine to me but I think the subject should be updated to

dt-bindings: arm: document R8A77965 SoC bindings

I'll let this sit for a few days to see if any other review is forthcoming.

> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt 
> b/Documentation/devicetree/bindings/arm/shmobile.txt
> index 5c3af7e..7eb4830 100644
> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -39,6 +39,8 @@ SoCs:
>  compatible = "renesas,r8a7795"
>- R-Car M3-W (R8A77960)
>  compatible = "renesas,r8a7796"
> +  - R-Car M3-N (R8A77965)
> +compatible = "renesas,r8a77965"
>- R-Car V3M (R8A77970)
>  compatible = "renesas,r8a77970"
>- R-Car D3 (R8A77995)
> -- 
> 2.7.4
> 

Re: [PATCH v2 1/2] arm64: dts: renesas: r8a77970: add GPIO support

[Geert Uytterhoeven]

On Tue, Feb 13, 2018 at 12:22 PM, Sergei Shtylyov
 wrote:
> Describe all 6 GPIO controllers in the R8A77970 device tree.
>
> Based on the original (and large) patch by Daisuke Matsushita
> .
>
> Signed-off-by: Vladimir Barinov 
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH v3 2/5] arm64: dts: renesas: r8a77995: add VSP instances

[Kieran Bingham]

From: Kieran Bingham 

The r8a77995 has a VSPBS to support image processing such as blending of
two input images, and has two VSPDs to handle display pipelines with a
DU.

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 

---
v2:
 - Fix VSPD register map size
 - Squash VSPBS and VSPD patches together

v3:
 - Fix VSPBS register map size too :-)

 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 196a917afea6..0db242114bc5 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -692,6 +692,16 @@
status = "disabled";
};
 
+   vspbs: vsp@fe96 {
+   compatible = "renesas,vsp2";
+   reg = <0 0xfe96 0 0x8000>;
+   interrupts = ;
+   clocks = < CPG_MOD 627>;
+   power-domains = < R8A77995_PD_ALWAYS_ON>;
+   resets = < 627>;
+   renesas,fcp = <>;
+   };
+
fcpvb0: fcp@fe96f000 {
compatible = "renesas,fcpv";
reg = <0 0xfe96f000 0 0x200>;
@@ -701,6 +711,16 @@
iommus = <_vp0 5>;
};
 
+   vspd0: vsp@fea2 {
+   compatible = "renesas,vsp2";
+   reg = <0 0xfea2 0 0x8000>;
+   interrupts = ;
+   clocks = < CPG_MOD 623>;
+   power-domains = < R8A77995_PD_ALWAYS_ON>;
+   resets = < 623>;
+   renesas,fcp = <>;
+   };
+
fcpvd0: fcp@fea27000 {
compatible = "renesas,fcpv";
reg = <0 0xfea27000 0 0x200>;
@@ -710,6 +730,16 @@
iommus = <_vi0 8>;
};
 
+   vspd1: vsp@fea8 {
+   compatible = "renesas,vsp2";
+   reg = <0 0xfea28000 0 0x8000>;
+   interrupts = ;
+   clocks = < CPG_MOD 622>;
+   power-domains = < R8A77995_PD_ALWAYS_ON>;
+   resets = < 622>;
+   renesas,fcp = <>;
+   };
+
fcpvd1: fcp@fea2f000 {
compatible = "renesas,fcpv";
reg = <0 0xfea2f000 0 0x200>;
-- 
2.7.4

[PATCH v3 0/5] arm64: dts: renesas: r8a77995: Add VSP support

[Kieran Bingham]

From: Kieran Bingham 

The r8a77995-d3 platform supports 3 VSP instances. One VSPBS can be used
as a dual-input image blender, while two VSPD instances can be utilised as
part of a display (DU) pipeline.

Add support for these, along with their required FCPV nodes.

During review, Laurent noticed that the r8a7795 and r8a7796 were not mapping
enough register space to handle RPFs with CLUT modules.

The last three patches fix this issue.

v2:
 - Merge FCPV nodes to a single patch
 - Merge VSP nodes to a single patch
 - Fix VSP register map size
 - Add fixes for r8a7795 and r8a7796

v3:
 - Fix VSBB register map size
 - Add r8a7795-es1 vspd3

Kieran Bingham (5):
  arm64: dts: renesas: r8a77995: add FCPV nodes
  arm64: dts: renesas: r8a77995: add VSP instances
  arm64: dts: renesas: r8a7795-es1: Fix register mappings on VSPs
  arm64: dts: renesas: r8a7795: Fix register mappings on VSPs
  arm64: dts: renesas: r8a7796: Fix register mappings on VSPs

 arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi |  2 +-
 arch/arm64/boot/dts/renesas/r8a7795.dtsi |  6 +--
 arch/arm64/boot/dts/renesas/r8a7796.dtsi |  6 +--
 arch/arm64/boot/dts/renesas/r8a77995.dtsi| 57 
 4 files changed, 64 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH v3 1/5] arm64: dts: renesas: r8a77995: add FCPV nodes

[Kieran Bingham]

From: Kieran Bingham 

The FCPVB handles the interface between the VSPB and memory, while the
FCPVD handles the interface between the VSPD and memory.

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
---
 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index cd3c6a30fc47..196a917afea6 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -691,6 +691,33 @@
#phy-cells = <0>;
status = "disabled";
};
+
+   fcpvb0: fcp@fe96f000 {
+   compatible = "renesas,fcpv";
+   reg = <0 0xfe96f000 0 0x200>;
+   clocks = < CPG_MOD 607>;
+   power-domains = < R8A77995_PD_ALWAYS_ON>;
+   resets = < 607>;
+   iommus = <_vp0 5>;
+   };
+
+   fcpvd0: fcp@fea27000 {
+   compatible = "renesas,fcpv";
+   reg = <0 0xfea27000 0 0x200>;
+   clocks = < CPG_MOD 603>;
+   power-domains = < R8A77995_PD_ALWAYS_ON>;
+   resets = < 603>;
+   iommus = <_vi0 8>;
+   };
+
+   fcpvd1: fcp@fea2f000 {
+   compatible = "renesas,fcpv";
+   reg = <0 0xfea2f000 0 0x200>;
+   clocks = < CPG_MOD 602>;
+   power-domains = < R8A77995_PD_ALWAYS_ON>;
+   resets = < 602>;
+   iommus = <_vi0 9>;
+   };
};
 
timer {
-- 
2.7.4

[PATCH v3 4/5] arm64: dts: renesas: r8a7795: Fix register mappings on VSPs

[Kieran Bingham]

From: Kieran Bingham 

The VSPD includes a CLUT on RPF2. Ensure that the register space is
mapped correctly to support this.

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 1f32340af2d1..772991db8820 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -2607,7 +2607,7 @@
 
vspd0: vsp@fea2 {
compatible = "renesas,vsp2";
-   reg = <0 0xfea2 0 0x4000>;
+   reg = <0 0xfea2 0 0x8000>;
interrupts = ;
clocks = < CPG_MOD 623>;
power-domains = < R8A7795_PD_ALWAYS_ON>;
@@ -2627,7 +2627,7 @@
 
vspd1: vsp@fea28000 {
compatible = "renesas,vsp2";
-   reg = <0 0xfea28000 0 0x4000>;
+   reg = <0 0xfea28000 0 0x8000>;
interrupts = ;
clocks = < CPG_MOD 622>;
power-domains = < R8A7795_PD_ALWAYS_ON>;
@@ -2647,7 +2647,7 @@
 
vspd2: vsp@fea3 {
compatible = "renesas,vsp2";
-   reg = <0 0xfea3 0 0x4000>;
+   reg = <0 0xfea3 0 0x8000>;
interrupts = ;
clocks = < CPG_MOD 621>;
power-domains = < R8A7795_PD_ALWAYS_ON>;
-- 
2.7.4

[PATCH v3 5/5] arm64: dts: renesas: r8a7796: Fix register mappings on VSPs

[Kieran Bingham]

From: Kieran Bingham 

The VSPD includes a CLUT on RPF2. Ensure that the register space is
mapped correctly to support this.

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 60755117cba5..3fe5566e0630 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -2285,7 +2285,7 @@
 
vspd0: vsp@fea2 {
compatible = "renesas,vsp2";
-   reg = <0 0xfea2 0 0x4000>;
+   reg = <0 0xfea2 0 0x8000>;
interrupts = ;
clocks = < CPG_MOD 623>;
power-domains = < R8A7796_PD_ALWAYS_ON>;
@@ -2305,7 +2305,7 @@
 
vspd1: vsp@fea28000 {
compatible = "renesas,vsp2";
-   reg = <0 0xfea28000 0 0x4000>;
+   reg = <0 0xfea28000 0 0x8000>;
interrupts = ;
clocks = < CPG_MOD 622>;
power-domains = < R8A7796_PD_ALWAYS_ON>;
@@ -2325,7 +2325,7 @@
 
vspd2: vsp@fea3 {
compatible = "renesas,vsp2";
-   reg = <0 0xfea3 0 0x4000>;
+   reg = <0 0xfea3 0 0x8000>;
interrupts = ;
clocks = < CPG_MOD 621>;
power-domains = < R8A7796_PD_ALWAYS_ON>;
-- 
2.7.4

[PATCH v3 3/5] arm64: dts: renesas: r8a7795-es1: Fix register mappings on VSPs

[Kieran Bingham]

From: Kieran Bingham 

The VSPD includes a CLUT on RPF2. Ensure that the register space is
mapped correctly to support this.

Signed-off-by: Kieran Bingham 
---
 arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi
index ed553338b4d4..1adfe6cad268 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795-es1.dtsi
@@ -80,7 +80,7 @@
 
vspd3: vsp@fea38000 {
compatible = "renesas,vsp2";
-   reg = <0 0xfea38000 0 0x4000>;
+   reg = <0 0xfea38000 0 0x8000>;
interrupts = ;
clocks = < CPG_MOD 620>;
power-domains = < R8A7795_PD_ALWAYS_ON>;
-- 
2.7.4

Re: [PATCH v3 00/16] mmc: tmio: another batch of TMIO MMC fixes and cleanups

[Ulf Hansson]

On 14 February 2018 at 10:43, Masahiro Yamada
 wrote:
> Hi Ulf,
>
>
> 2018-02-14 18:36 GMT+09:00 Ulf Hansson :
>> On 7 February 2018 at 20:11, Wolfram Sang  wrote:
>>>
 I picked patch5 and patch6, as those seemed trivial. Unless there is a
 rc9, let's aim for 4.17 for the rest.
>>>
>>> I can't find the branch where you picked these patches? I wanted to use
>>> it to apply the rest of the patches. They don't seem to fit on v4.15
>>> or current linus/master. Can you share this branch?
>>
>> You need to check again. :-)
>>
>> Patches up and to patch6 is already in Linus' tree v.16rc1, those I
>> carried for quite a while on my next branch before sending them in the
>> PR to Linus.
>>
>>>
>>> I will do reviewing now "visually". But please wait until I tested them
>>> before applying.
>>
>> I am eager to apply those patches you already have reviewed (to
>> patch14), as those mostly seemed rather trivial and should be nice
>> cleanups.
>>
>> Do you mind me going ahead, then you can continue and review/test the rest?
>>
>
>
> If you move forward, please fix-up the commit log of the patch 11.
> (https://patchwork.kernel.org/patch/10170007/)
>
>
> Documentation/devicetree/bindings/mmc/tmio_mmc.txt
>
>   ->
>
> Documentation/devicetree/bindings/mmc/mmc.txt

Thanks for the reminder. Yes I can amend the patch, no worries.

Kind regards
Uffe

Re: [PATCH v3 00/16] mmc: tmio: another batch of TMIO MMC fixes and cleanups

[Masahiro Yamada]

Hi Ulf,


2018-02-14 18:36 GMT+09:00 Ulf Hansson :
> On 7 February 2018 at 20:11, Wolfram Sang  wrote:
>>
>>> I picked patch5 and patch6, as those seemed trivial. Unless there is a
>>> rc9, let's aim for 4.17 for the rest.
>>
>> I can't find the branch where you picked these patches? I wanted to use
>> it to apply the rest of the patches. They don't seem to fit on v4.15
>> or current linus/master. Can you share this branch?
>
> You need to check again. :-)
>
> Patches up and to patch6 is already in Linus' tree v.16rc1, those I
> carried for quite a while on my next branch before sending them in the
> PR to Linus.
>
>>
>> I will do reviewing now "visually". But please wait until I tested them
>> before applying.
>
> I am eager to apply those patches you already have reviewed (to
> patch14), as those mostly seemed rather trivial and should be nice
> cleanups.
>
> Do you mind me going ahead, then you can continue and review/test the rest?
>


If you move forward, please fix-up the commit log of the patch 11.
(https://patchwork.kernel.org/patch/10170007/)


Documentation/devicetree/bindings/mmc/tmio_mmc.txt

  ->

Documentation/devicetree/bindings/mmc/mmc.txt



Thanks.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] vfio: platform: Add generic DT reset support

[Geert Uytterhoeven]

Hi Eric,

On Wed, Feb 14, 2018 at 10:09 AM, Auger Eric  wrote:
> On 13/02/18 17:36, Geert Uytterhoeven wrote:
>> Vfio-platform requires reset support, provided either by ACPI, or, on DT
>> platforms, by a device-specific reset driver matching against the
>> device's compatible value.
>>
>> On many SoCs, devices are connected to an SoC-internal reset controller,
>> and can be reset in a generic way.  Hence add support to reset such
>> devices using the reset controller subsystem, provided the reset
>> hierarchy is described correctly in DT using the "resets" property.
>
> I first acknowledge I am not familiar with what those reset controllers
> do in practice. My fear is that we may rely on generic SW pieces that
> may not be adapted to passthrough constraints. We must guarantee that
> any DMA access attempted by the devices are stopped and any interrupts
> gets stopped. Can we guarantee that the reset controller always induce
> that? Otherwise we may leave the door opened to badly reset assigned
> devices.

An on-SoC reset controller is basically a block controlling signals to the
reset inputs of the individual on-SoC devices.
On Renesas ARM SoCs, this allows to do a full reset of the attached device.

Of course the exact semantics depend on the actual SoC.
If e.g. DMA and interrupts are not stopped for a specific device on a
specific SoC, it still needs a device-specific reset driver, matching against
the appropriate compatible value, cfr. the quoted paragraph below.

You could add a whitelist for of_machine_is_compatible() or
of_device_is_compatible(), but that will grow large fast.

>> Devices that require a more complex reset procedure can still
>> provide a device-specific reset driver, as that takes precedence.

>> @@ -138,6 +152,8 @@ static void vfio_platform_put_reset(struct 
>> vfio_platform_device *vdev)
>>
>>   if (vdev->of_reset)
>>   module_put(vdev->reset_module);
>> +
> if (vdev->reset_control) ?
> reset_control_put seems to only check IS_ERR()

void reset_control_put(struct reset_control *rstc)
{
if (IS_ERR_OR_NULL(rstc))
return;

So it does handle NULL.

>> + reset_control_put(vdev->reset_control);

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 00/16] mmc: tmio: another batch of TMIO MMC fixes and cleanups

[Ulf Hansson]

On 7 February 2018 at 20:11, Wolfram Sang  wrote:
>
>> I picked patch5 and patch6, as those seemed trivial. Unless there is a
>> rc9, let's aim for 4.17 for the rest.
>
> I can't find the branch where you picked these patches? I wanted to use
> it to apply the rest of the patches. They don't seem to fit on v4.15
> or current linus/master. Can you share this branch?

You need to check again. :-)

Patches up and to patch6 is already in Linus' tree v.16rc1, those I
carried for quite a while on my next branch before sending them in the
PR to Linus.

>
> I will do reviewing now "visually". But please wait until I tested them
> before applying.

I am eager to apply those patches you already have reviewed (to
patch14), as those mostly seemed rather trivial and should be nice
cleanups.

Do you mind me going ahead, then you can continue and review/test the rest?

Kind regards
Uffe

Re: [PATCH 1/2] vfio: platform: Fix reset module leak in error path

[Geert Uytterhoeven]

Hi Eric,

On Wed, Feb 14, 2018 at 9:36 AM, Auger Eric  wrote:
> If I am not wrong we also leak the reset_module if
> vfio_platform_get_reset() fails to find the reset function (of_reset ==
> NULL), in which case we should do the module_put() in
> vfio_platform_get_reset().

Correct. Will look into fixing it...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ARM: shmobile: stout: enable R-Car Gen2 regulator quirk

[Geert Uytterhoeven]

Hi Marek,

On Wed, Feb 14, 2018 at 12:28 AM, Marek Vasut  wrote:
> Regulator setup is suboptimal on H2 Stout too.

Worse, Stout has 2 DA9210 regulators, so you have to add a check for a
DA9210 at address 0x70.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ARM: dts: stout: Initial r8a7790 Stout board support

[Geert Uytterhoeven]

Hi Marek,

On Wed, Feb 14, 2018 at 12:29 AM, Marek Vasut  wrote:
> Stout base board support making use of 1 GiB of memory,
> the Renesas H2 r8a7790 SoC with the SCIFA0 serial port
> and CA15 with ARM architected timer.
>
> Furthermore, this device tree contains entries for:
>   - 4x LEDs
>   - SDHI SD/MMC controller
>   - Display unit with HDMI output
>   - SH fast ethernet controller
>   - QSPI controller with S25FL512S attached to it
>   - I2C controller with DA9210 and DA 9063 PMICs
>
> Signed-off-by: Marek Vasut 

Thanks fro your patch!

> --- /dev/null
> +++ b/arch/arm/boot/dts/r8a7790-stout.dts
> @@ -0,0 +1,360 @@
> +/*
> + * Device Tree Source for the Stout board
> + *
> + * Copyright (C) 2018 Marek Vasut 
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.

SPDX for new files?

> +/ {

> +   memory@4000 {
> +   device_type = "memory";
> +   reg = <0 0x4000 0 0x4000>;

According to the schematics, Stout has 2 GiB of RAM.
The extra 1 GiB can be added later, though.

> +   };
> +
> +   leds {
> +   compatible = "gpio-leds";
> +   led1 {
> +   gpios = < 22 GPIO_ACTIVE_HIGH>;
> +   };
> +   led2 {
> +   gpios = < 23 GPIO_ACTIVE_HIGH>;
> +   };
> +   led3 {
> +   gpios = < 17 GPIO_ACTIVE_HIGH>;
> +   };
> +   led5 {
> +   gpios = < 24 GPIO_ACTIVE_HIGH>;

GPIO_ACTIVE_LOW, for all 4 LEDs, as there are no inverting NPN drivers.

> +   };
> +   };

> +   x2_clk: x2-clock {
> +   compatible = "fixed-clock";
> +   #clock-cells = <0>;
> +   clock-frequency = <14850>;
> +   };

There's no X2 clock (copied from Lager?).

> +
> +   x13_clk: x13-clock {

This clock is called "OSC1", not "X13".

> +   compatible = "fixed-clock";
> +   #clock-cells = <0>;
> +   clock-frequency = <14850>;
> +   };
> +};
> +
> + {
> +   pinctrl-0 = <_pins>;
> +   pinctrl-names = "default";
> +   status = "okay";
> +
> +   clocks = < CPG_MOD 724>, < CPG_MOD 723>, < CPG_MOD 722>,
> +< CPG_MOD 726>, < CPG_MOD 725>,
> +<_clk>, <_clk>;

"<_clk>" instead of "<_clk>".
Please drop the reference to the non-existent "<_clk>".

> +   clock-names = "du.0", "du.1", "du.2", "lvds.0", "lvds.1",
> + "dclkin.0", "dclkin.1";

Please drop "dclkin.1", as it is not wired on Stout.

> + {
> +   pinctrl-0 = <_pins>;
> +   pinctrl-names = "default";
> +
> +   status = "okay";
> +
> +   flash: flash@0 {
> +   compatible = "spansion,s25fl512s", "jedec,spi-nor";
> +   reg = <0>;
> +   spi-max-frequency = <3000>;
> +   spi-tx-bus-width = <4>;
> +   spi-rx-bus-width = <4>;
> +   spi-cpha;
> +   spi-cpol;
> +   m25p,fast-read;
> +
> +   partitions {
> +   compatible = "fixed-partitions";
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +
> +   partition@0 {
> +   label = "loader";
> +   reg = <0x 0x0004>;
> +   read-only;
> +   };
> +   partition@4 {
> +   label = "user";
> +   reg = <0x0004 0x0040>;
> +   read-only;
> +   };
> +   partition@44 {
> +   label = "flash";
> +   reg = <0x0044 0x03bc>;
> +   };

I can't verify the partition layout.

> +   };
> +   };
> +};


> +  {
> +   status = "okay";
> +   pinctrl-0 = <_pins>;
> +   pinctrl-names = "default";
> +
> +   clock-frequency = <10>;
> +
> +   hdmi@39 {
> +   compatible = "adi,adv7511w";
> +   reg = <0x39>;
> +   interrupt-parent = <>;
> +   interrupts = <15 IRQ_TYPE_LEVEL_LOW>;

Missing cec clock (OSC4).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 2/2] vfio: platform: Add generic DT reset support

[Auger Eric]

Hi Geert,

On 13/02/18 17:36, Geert Uytterhoeven wrote:
> Vfio-platform requires reset support, provided either by ACPI, or, on DT
> platforms, by a device-specific reset driver matching against the
> device's compatible value.
> 
> On many SoCs, devices are connected to an SoC-internal reset controller,
> and can be reset in a generic way.  Hence add support to reset such
> devices using the reset controller subsystem, provided the reset
> hierarchy is described correctly in DT using the "resets" property.

I first acknowledge I am not familiar with what those reset controllers
do in practice. My fear is that we may rely on generic SW pieces that
may not be adapted to passthrough constraints. We must guarantee that
any DMA access attempted by the devices are stopped and any interrupts
gets stopped. Can we guarantee that the reset controller always induce
that? Otherwise we may leave the door opened to badly reset assigned
devices.
> 
> Devices that require a more complex reset procedure can still
> provide a device-specific reset driver, as that takes precedence.
> 
> Note that this functionality depends on CONFIG_RESET_CONTROLLER=y, and
> becomes a no-op if reset controller support is disabled.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/vfio/platform/vfio_platform_common.c  | 23 +--
>  drivers/vfio/platform/vfio_platform_private.h |  1 +
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/platform/vfio_platform_common.c 
> b/drivers/vfio/platform/vfio_platform_common.c
> index b60bb5326668498c..5d1e48f96e423508 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -112,7 +113,13 @@ static bool vfio_platform_has_reset(struct 
> vfio_platform_device *vdev)
>   if (VFIO_PLATFORM_IS_ACPI(vdev))
>   return vfio_platform_acpi_has_reset(vdev);
>  
> - return vdev->of_reset ? true : false;
> + if (vdev->of_reset)
> + return true;
> +
> + if (!IS_ERR_OR_NULL(vdev->reset_control))
> + return true;
> +
> + return false;
>  }
>  
>  static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
> @@ -127,8 +134,15 @@ static int vfio_platform_get_reset(struct 
> vfio_platform_device *vdev)
>   vdev->of_reset = vfio_platform_lookup_reset(vdev->compat,
>   >reset_module);
>   }
> + if (vdev->of_reset)
> + return 0;
> +
> + vdev->reset_control = __of_reset_control_get(vdev->device->of_node,
> +  NULL, 0, false, false);
> + if (!IS_ERR(vdev->reset_control))
> + return 0;
>  
> - return vdev->of_reset ? 0 : -ENOENT;
> + return PTR_ERR(vdev->reset_control);
>  }
>  
>  static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
> @@ -138,6 +152,8 @@ static void vfio_platform_put_reset(struct 
> vfio_platform_device *vdev)
>  
>   if (vdev->of_reset)
>   module_put(vdev->reset_module);
> +
if (vdev->reset_control) ?
reset_control_put seems to only check IS_ERR()

Thanks

Eric
> + reset_control_put(vdev->reset_control);
>  }
>  
>  static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
> @@ -217,6 +233,9 @@ static int vfio_platform_call_reset(struct 
> vfio_platform_device *vdev,
>   } else if (vdev->of_reset) {
>   dev_info(vdev->device, "reset\n");
>   return vdev->of_reset(vdev);
> + } else if (!IS_ERR_OR_NULL(vdev->reset_control)) {
> + dev_info(vdev->device, "reset\n");
> + return reset_control_reset(vdev->reset_control);
>   }
>  
>   dev_warn(vdev->device, "no reset function found!\n");
> diff --git a/drivers/vfio/platform/vfio_platform_private.h 
> b/drivers/vfio/platform/vfio_platform_private.h
> index 85ffe5d9d1abd94e..a56e80ae5986540b 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -60,6 +60,7 @@ struct vfio_platform_device {
>   const char  *compat;
>   const char  *acpihid;
>   struct module   *reset_module;
> + struct reset_control*reset_control;
>   struct device   *device;
>  
>   /*
> 

Re: [PATCH] ARM: dts: stout: Initial r8a7790 Stout board support

[Wolfram Sang]

Hi Geert,

> The other R-Car Gen2 boards use 115200 bps for U-Boot and Linux, but some
> other value (38400?) for the SPI loader that loads U-Boot.

Yes, mostly correct. I am quite sure I have one board which has non-115200
for U-Boot. I am not at home currently, so I can't check now.

> What about U-Boot commit f40574e2d78c96a3 ("Kconfig: Migrate 
> CONFIG_BAUDRATE"),
> which says all R-Car Gen2 boards use 38400, before and after?

Don't like, too slow. Can we change it upstream with the reasoning that
it is a) faster and b) BSP-compatible?

Regards,

   Wolfram



signature.asc
Description: PGP signature

Re: [PATCH v2 2/4] arm64: dts: renesas: r8a77995: add VSP instances

[Kieran Bingham]

Hi Laurent,

Thanks for the review,

On 13/02/18 22:03, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tuesday, 13 February 2018 21:30:35 EET Kieran Bingham wrote:
>> From: Kieran Bingham 
>>
>> The r8a77995 has a VSPBS to support image processing such as blending of
>> two input images, and has two VSPDs to handle display pipelines with a
>> DU.
>>
>> Signed-off-by: Kieran Bingham 
>>
>> ---
>> v2:
>>  - Fix VSPD register map size
>>  - Squash VSPBS and VSPD patches together
>>
>>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 30 
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index
>> 196a917afea6..19bd8be9926a 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
>> @@ -692,6 +692,16 @@
>>  status = "disabled";
>>  };
>>
>> +vspbs: vsp@fe96 {
>> +compatible = "renesas,vsp2";
>> +reg = <0 0xfe96 0 0x4000>;
> 
> The VSPBS also has OSD-CLUT support in its RPFs, so you need to extend the 
> registers range too.
> 

Wait, but I already changed this ...



Yup - this change is already in my tree ... which means I must have made the
change *after* calling git format-patch. Not helpful.

Orz




> Apart from that,
> 
> Reviewed-by: Laurent Pinchart 

I'll collect the tag and repost, this time with the correctly updated version.

(And I'll add in that missing vspd3 from the es1 tree too)


>> +interrupts = ;
>> +clocks = < CPG_MOD 627>;
>> +power-domains = < R8A77995_PD_ALWAYS_ON>;
>> +resets = < 627>;
>> +renesas,fcp = <>;
>> +};
>> +
>>  fcpvb0: fcp@fe96f000 {
>>  compatible = "renesas,fcpv";
>>  reg = <0 0xfe96f000 0 0x200>;
>> @@ -701,6 +711,16 @@
>>  iommus = <_vp0 5>;
>>  };
>>
>> +vspd0: vsp@fea2 {
>> +compatible = "renesas,vsp2";
>> +reg = <0 0xfea2 0 0x8000>;
>> +interrupts = ;
>> +clocks = < CPG_MOD 623>;
>> +power-domains = < R8A77995_PD_ALWAYS_ON>;
>> +resets = < 623>;
>> +renesas,fcp = <>;
>> +};
>> +
>>  fcpvd0: fcp@fea27000 {
>>  compatible = "renesas,fcpv";
>>  reg = <0 0xfea27000 0 0x200>;
>> @@ -710,6 +730,16 @@
>>  iommus = <_vi0 8>;
>>  };
>>
>> +vspd1: vsp@fea8 {
>> +compatible = "renesas,vsp2";
>> +reg = <0 0xfea28000 0 0x8000>;
>> +interrupts = ;
>> +clocks = < CPG_MOD 622>;
>> +power-domains = < R8A77995_PD_ALWAYS_ON>;
>> +resets = < 622>;
>> +renesas,fcp = <>;
>> +};
>> +
>>  fcpvd1: fcp@fea2f000 {
>>  compatible = "renesas,fcpv";
>>  reg = <0 0xfea2f000 0 0x200>;
> 

Re: [PATCH v2 0/4] arm64: dts: renesas: r8a77995: Add VSP support

[Kieran Bingham]

Hi Laurent,

On 13/02/18 22:08, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patches.
> 
>> Kieran Bingham (4):
>>   arm64: dts: renesas: r8a77995: add FCPV nodes
>>   arm64: dts: renesas: r8a77995: add VSP instances
>>   arm64: dts: renesas: r8a7795: Fix register mappings on VSPs
> 
> Do you plan to also address r8a7795-es1.dtsi ?

Yes :-) - That pesky little extra vspd3 ...


>>   arm64: dts: renesas: r8a7796: Fix register mappings on VSP
--
Kieran

Re: [PATCH 1/2] vfio: platform: Fix reset module leak in error path

[Auger Eric]

Hi Geert,

On 13/02/18 17:36, Geert Uytterhoeven wrote:
> If the IOMMU group setup fails, the reset module is not released.
> 
> Fixes: b5add544d677d363 ("vfio, platform: make reset driver a requirement by 
> default")
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/vfio/platform/vfio_platform_common.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vfio/platform/vfio_platform_common.c 
> b/drivers/vfio/platform/vfio_platform_common.c
> index 35469af87f88678e..b60bb5326668498c 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -680,18 +680,23 @@ int vfio_platform_probe_common(struct 
> vfio_platform_device *vdev,

Thanks for fixing this.

If I am not wrong we also leak the reset_module if
vfio_platform_get_reset() fails to find the reset function (of_reset ==
NULL), in which case we should do the module_put() in
vfio_platform_get_reset().

Thanks

Eric
>   group = vfio_iommu_group_get(dev);
>   if (!group) {
>   pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto put_reset;
>   }
>  
>   ret = vfio_add_group_dev(dev, _platform_ops, vdev);
> - if (ret) {
> - vfio_iommu_group_put(group, dev);
> - return ret;
> - }
> + if (ret)
> + goto put_iommu;
>  
>   mutex_init(>igate);
>  
>   return 0;
> +
> +put_iommu:
> + vfio_iommu_group_put(group, dev);
> +put_reset:
> + vfio_platform_put_reset(vdev);
> + return ret;
>  }
>  EXPORT_SYMBOL_GPL(vfio_platform_probe_common);
>  
> 

Re: [PATCH] ARM: dts: stout: Initial r8a7790 Stout board support

[Geert Uytterhoeven]

Hi Wolfram,

On Wed, Feb 14, 2018 at 7:05 AM, Wolfram Sang  wrote:
>> + chosen {
>> + bootargs = "ignore_loglevel rw root=/dev/nfs ip=dhcp";
>> + stdout-path = "serial0:38400n8";
>
> Hmm, that would be our first board to not have 115200, or? We already
> have some other boards upstream where UBoot has 38400 and Linux 115200.
> I'd prefer consistency here a little, but no strong opinion and asking
> for other opinions.

The other R-Car Gen2 boards use 115200 bps for U-Boot and Linux, but some
other value (38400?) for the SPI loader that loads U-Boot.

What about U-Boot commit f40574e2d78c96a3 ("Kconfig: Migrate CONFIG_BAUDRATE"),
which says all R-Car Gen2 boards use 38400, before and after?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ARM: shmobile: Document Renesas H2-based Stout DT bindings

[Geert Uytterhoeven]

Hi Marek,

On Wed, Feb 14, 2018 at 12:29 AM, Marek Vasut  wrote:
> Document the Renesas H2 Stout (ADAS Starter Kit) device tree bindings,

Thanks for your patch!

According to https://elinux.org/images/1/10/StoutTopWithLabels.png,
it's called "ADAS Starterkit".

> listing it as a supported board.
>
> This allows to use checkpatch.pl to validate .dts files referring to
> the Stout board.
>
> Signed-off-by: Marek Vasut 


> Cc: Geert Uytterhoeven 
> Cc: Kuninori Morimoto 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt 
> b/Documentation/devicetree/bindings/arm/shmobile.txt
> index 41d3920aaa5e..bf9bb6eda4a6 100644
> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -94,6 +94,8 @@ Boards:
>  compatible = "renesas,kzm9g", "renesas,sh73a0"
>- Lager (RTP0RC7790SEB00010S)
>  compatible = "renesas,lager", "renesas,r8a7790"
> +  - Stout (Y-R-CAR-ADAS-SKH2-BOARD)

Please keep the list alphabetically sorted.

I'd add "ADAS Starterkit" here, too, cfr. M3ULCB below.

> +compatible = "renesas,stout", "renesas,r8a7790"
>- M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
>  compatible = "renesas,m3ulcb", "renesas,r8a7796"
>- Marzen (R0P7779A00010S)

With the above fixed:
Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ARM: shmobile: stout: enable R-Car Gen2 regulator quirk

[Geert Uytterhoeven]

On Wed, Feb 14, 2018 at 6:58 AM, Wolfram Sang  wrote:
>> - * The r8a7790/lager and r8a7791/koelsch development boards have da9063 and
>> - * da9210 regulators.  Both regulators have their interrupt request lines 
>> tied
>> - * to the same interrupt pin (IRQ2) on the SoC.
>> + * The r8a7790/lager,stout and r8a7791/koelsch development boards have 
>> da9063
>> + * and da9210 regulators.  Both regulators have their interrupt request 
>> lines
>> + * tied to the same interrupt pin (IRQ2) on the SoC.
>
> I think listing the boards here doesn't scale well. Gose is already
> missing. How about rephrasing the paragraph to something like "Some Gen2
> development boards have..."?

+1

>>   *
>>   * After cold boot or da9063-induced restart, both the da9063 and da9210 
>> seem
>>   * to assert their interrupt request lines.  Hence as soon as one driver
>> @@ -118,6 +118,7 @@ static int __init rcar_gen2_regulator_quirk(void)
>>
>>   if (!of_machine_is_compatible("renesas,koelsch") &&
>>   !of_machine_is_compatible("renesas,lager") &&
>> + !of_machine_is_compatible("renesas,stout") &&
>>   !of_machine_is_compatible("renesas,gose"))
>>   return -ENODEV;

Have we reached critical mass to start using array-based matching with
of_device_compatible_match()?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds