[PATCH 00/18] arm64: dts: renesas: Cleanup Beacon Kit and support more SoC's

[PATCH 00/18] arm64: dts: renesas: Cleanup Beacon Kit and support more SoC's

[Adam Ford]

This series contains minor fixes and some reorganization to better
support the hardware and add two new kits to which reuse the same
SOM and baseboard files.

Adam Ford (18):
  arm64: dts: renesas: beacon kit: Configure programmable clocks
  arm64: dts: renesas: beacon kit: Fix choppy Bluetooth Audio
  arm64: dts: renesas: beacon kit: Remove unnecessary nodes
  arm64: dts: renesas: beacon kit: Fix Audio Clock sources
  arm64: dts: renesas: beacon: Fix audio-1.8V pin enable
  arm64: dts: renesas: beacon: Configure Audio CODEC clocks
  arm64: dts: renesas: beacon: Fix LVDS PWM Backlight
  arm64: dts: renesas: beacon: Enable SCIF4
  arm64: dts: renesas: beacon: Fix RGB Display PWM Backlight
  arm64: dts: renesas: Don't make vccq_sdhi0 always on
  arm64: dts: renesas: beacon: Remove redundant audio code
  arm64: dts: renesas: beacon: Better describe keys
  arm64: dts: renesas: beacon: Enable SPI
  arm64: dts: renesas: beacon: Correct I2C bus speeds
  arm64: dts: renesas: beacon-rzg2m-kit: Rearange SoC unique functions
  dt-bindings: arm: renesas: Add Beacon RZ/G2N and RZ/G2H boards
  arm64: dts: renesas: Introduce r8a774b1-beacon-rzg2n-kit
  arm64: dts: renesas: Introduce r8a774e1-beacon-rzg2h-kit

 .../devicetree/bindings/arm/renesas.yaml      |   2 +
 arch/arm64/boot/dts/renesas/Makefile          |   1 +
 .../dts/renesas/beacon-renesom-baseboard.dtsi | 150 ++++++++++--------
 .../boot/dts/renesas/beacon-renesom-som.dtsi  |  43 +++--
 .../dts/renesas/r8a774a1-beacon-rzg2m-kit.dts |  16 ++
 .../dts/renesas/r8a774b1-beacon-rzg2n-kit.dts |  43 +++++
 .../dts/renesas/r8a774e1-beacon-rzg2h-kit.dts |  48 ++++++
 7 files changed, 224 insertions(+), 79 deletions(-)
 create mode 100644 arch/arm64/boot/dts/renesas/r8a774b1-beacon-rzg2n-kit.dts
 create mode 100644 arch/arm64/boot/dts/renesas/r8a774e1-beacon-rzg2h-kit.dts

-- 
2.25.1

[PATCH 01/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Adam Ford]

When the board was added, clock drivers were being updated done at
the same time to allow the versaclock driver to properly configure
the modes.  Unforutnately, the updates were not applied to the board
files at the time they should have been, so do it now.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../dts/renesas/beacon-renesom-baseboard.dtsi | 35 +++++++++++++++++--
 .../boot/dts/renesas/beacon-renesom-som.dtsi  | 26 ++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index e66b5b36e489..3c84e060c69f 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -5,6 +5,7 @@
 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/clk/versaclock.h>
 
 / {
 	backlight_lvds: backlight-lvds {
@@ -294,12 +295,12 @@ &du_out_rgb {
 &ehci0 {
 	dr_mode = "otg";
 	status = "okay";
-	clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
+	clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
 };
 
 &ehci1 {
 	status = "okay";
-	clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
+	clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
 };
 
 &hdmi0 {
@@ -373,12 +374,40 @@ versaclock6_bb: clock-controller@6a {
 		#clock-cells = <1>;
 		clocks = <&x304_clk>;
 		clock-names = "xin";
-		/* CSI0_MCLK, CSI1_MCLK, AUDIO_CLKIN, USB_HUB_MCLK_BB */
+		clock-output-names = "versaclock6_bb.out0_sel_i2cb",
+				      "versaclock6_bb.out1",
+				      "versaclock6_bb.out2",
+				      "versaclock6_bb.out3",
+				      "versaclock6_bb.out4";
 		assigned-clocks = <&versaclock6_bb 1>,
 				   <&versaclock6_bb 2>,
 				   <&versaclock6_bb 3>,
 				   <&versaclock6_bb 4>;
 		assigned-clock-rates =	<24000000>, <24000000>, <24000000>, <24576000>;
+
+		OUT1 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <1800000>;
+			idt,slew-percent = <100>;
+		};
+
+		OUT2 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <1800000>;
+			idt,slew-percent = <100>;
+		};
+
+		OUT3 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <3300000>;
+			idt,slew-percent = <100>;
+		};
+
+		OUT4 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <3300000>;
+			idt,slew-percent = <100>;
+		};
 	};
 };
 
diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index 8ac167aa18f0..449ff5937fc6 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -4,6 +4,7 @@
  */
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clk/versaclock.h>
 
 / {
 	memory@48000000 {
@@ -170,7 +171,32 @@ versaclock5: versaclock_som@6a {
 				   <&versaclock5 2>,
 				   <&versaclock5 3>,
 				   <&versaclock5 4>;
+
 		assigned-clock-rates = <33333333>, <33333333>, <50000000>, <125000000>;
+
+		OUT1 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <1800000>;
+			idt,slew-percent = <100>;
+		};
+
+		OUT2 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <1800000>;
+			idt,slew-percent = <100>;
+		};
+
+		OUT3 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <1800000>;
+			idt,slew-percent = <100>;
+		};
+
+		OUT4 {
+			idt,mode = <VC5_CMOS>;
+			idt,voltage-microvolts = <3300000>;
+			idt,slew-percent = <100>;
+		};
 	};
 };
 
-- 
2.25.1

[PATCH 02/18] arm64: dts: renesas: beacon kit: Fix choppy Bluetooth Audio

[Adam Ford]

The Bluetooth chip is capable of operating at 4Mbps, but the
max-speed setting was on the UART node instead of the Bluetooth
node, so the chip didn't operate at the correct speed resulting
in choppy audio.  Fix this by setting the max-speed in the proper
node.

Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index 449ff5937fc6..c50f8e63c80f 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -90,7 +90,6 @@ &hscif0 {
 	pinctrl-names = "default";
 	uart-has-rtscts;
 	status = "okay";
-	max-speed = <4000000>;
 
 	bluetooth {
 		compatible = "brcm,bcm43438-bt";
@@ -99,6 +98,7 @@ bluetooth {
 		device-wakeup-gpios = <&pca9654 5 GPIO_ACTIVE_HIGH>;
 		clocks = <&osc_32k>;
 		clock-names = "extclk";
+		max-speed = <4000000>;
 	};
 };
 
-- 
2.25.1

[PATCH 03/18] arm64: dts: renesas: beacon kit: Remove unnecessary nodes

[Adam Ford]

VSPI0 and VSPB are already enabled by default.  There is no need
to add extra nodes to enable them.  Remove the redundant nodes.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index c50f8e63c80f..b093a34b0fa0 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -329,11 +329,3 @@ &usb_extal_clk {
 &usb3s0_clk {
 	clock-frequency = <100000000>;
 };
-
-&vspb {
-	status = "okay";
-};
-
-&vspi0 {
-	status = "okay";
-};
-- 
2.25.1

[PATCH 04/18] arm64: dts: renesas: beacon kit: Fix Audio Clock sources

[Adam Ford]

The SoC was expecting two clock sources with different frequencies.
One to support 44.1KHz and one to support 48KHz.  With the newly added
ability to configure the programmably clock, configure both clocks.

Beacause the SoC is expecting a fixed clock/oscillator, it doesn't
attempt to get and enable the clock for audio_clk_a. The choice to
use a fixed-factor-clock was due to the fact that it will automatically
enable the programmable clock frequency without change any code.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../boot/dts/renesas/beacon-renesom-baseboard.dtsi    | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index 3c84e060c69f..5c09e64001cc 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -250,9 +250,12 @@ ss_ep: endpoint {
 };
 
 &audio_clk_a {
-	clock-frequency = <24576000>;
-	assigned-clocks = <&versaclock6_bb 4>;
-	assigned-clock-rates = <24576000>;
+	/delete-property/ clock-frequency;
+	#clock-cells = <0>;
+	compatible = "fixed-factor-clock";
+	clock-mult = <1>;
+	clock-div = <1>;
+	clocks = <&versaclock6_bb 4>;
 };
 
 &audio_clk_b {
@@ -591,7 +594,7 @@ sound_pins: sound {
 	};
 
 	sound_clk_pins: sound_clk {
-		groups = "audio_clk_a_a";
+		groups = "audio_clk_a_a", "audio_clk_b_a";
 		function = "audio_clk";
 	};
 
-- 
2.25.1

[PATCH 05/18] arm64: dts: renesas: beacon: Fix audio-1.8V pin enable

[Adam Ford]

The fact the audio worked at all was a coindicence because the wrong
gpio enable was used.  Use the correct GPIO pin to ensure its operation.

Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index 5c09e64001cc..ee7809e8db07 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -151,7 +151,7 @@ reg_audio: regulator_audio {
 		regulator-name = "audio-1.8V";
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
-		gpio = <&gpio_exp2 7 GPIO_ACTIVE_HIGH>;
+		gpio = <&gpio_exp4 1 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 	};
 
-- 
2.25.1

[PATCH 06/18] arm64: dts: renesas: beacon: Configure Audio CODEC clocks

[Adam Ford]

With the newly added configurable clock options, the audio CODEC can
configure the mclk automatically.  Add the reference to the versaclock.
Since the devices on I2C5 can communicate at 400KHz, let's also increase
that too

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index ee7809e8db07..130993b1b20a 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -424,13 +424,15 @@ &i2c0 {
 
 &i2c5 {
 	status = "okay";
-	clock-frequency = <100000>;
+	clock-frequency = <400000>;
 	pinctrl-0 = <&i2c5_pins>;
 	pinctrl-names = "default";
 
 	codec: wm8962@1a {
 		compatible = "wlf,wm8962";
 		reg = <0x1a>;
+		clocks = <&versaclock6_bb 3>;
+		clock-names = "mclk";
 		DCVDD-supply = <&reg_audio>;
 		DBVDD-supply = <&reg_audio>;
 		AVDD-supply = <&reg_audio>;
-- 
2.25.1

[PATCH 07/18] arm64: dts: renesas: beacon: Fix LVDS PWM Backlight

[Adam Ford]

The backlight didn't really work correctly due to some updates that were
made in hardware.  It should be safe to apply these, because the older
hardware was never shipped to anyone, so it shouldn't break anything.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../boot/dts/renesas/beacon-renesom-baseboard.dtsi     | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index 130993b1b20a..aab39aae5ccb 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -11,8 +11,8 @@ / {
 	backlight_lvds: backlight-lvds {
 		compatible = "pwm-backlight";
 		power-supply = <&reg_lcd>;
-		enable-gpios = <&gpio_exp1 3 GPIO_ACTIVE_LOW>;
-		pwms = <&pwm2 0 50000>;
+		enable-gpios = <&gpio_exp1 3 GPIO_ACTIVE_HIGH>;
+		pwms = <&pwm2 0 25000>;
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
 	};
@@ -119,9 +119,9 @@ panel-timing {
 			hback-porch = <40>;
 			vfront-porch = <13>;
 			vback-porch = <29>;
-			vsync-len = <3>;
+			vsync-len = <1>;
 			hsync-active = <1>;
-			vsync-active = <1>;
+			vsync-active = <3>;
 			de-active = <1>;
 			pixelclk-active = <0>;
 		};
@@ -575,7 +575,7 @@ pwm0_pins: pwm0 {
 
 	pwm2_pins: pwm2 {
 		groups = "pwm2_a";
-		function = "pwm2_a";
+		function = "pwm2";
 	};
 
 	sdhi0_pins: sd0 {
-- 
2.25.1

[PATCH 08/18] arm64: dts: renesas: beacon: Enable SCIF4

[Adam Ford]

The baseboard supports SCIF4, enable the pins and the node for it.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../boot/dts/renesas/beacon-renesom-baseboard.dtsi    | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index aab39aae5ccb..bf047a9836ed 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -578,6 +578,11 @@ pwm2_pins: pwm2 {
 		function = "pwm2";
 	};
 
+	scif4_pins: scif4 {
+		groups = "scif4_data_c";
+		function = "scif4";
+	};
+
 	sdhi0_pins: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
@@ -706,6 +711,12 @@ &scif0 {
 	status = "okay";
 };
 
+&scif4 {
+	pinctrl-0 = <&scif4_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
 &scif5 {
 	pinctrl-0 = <&scif5_pins>;
 	pinctrl-names = "default";
-- 
2.25.1

[PATCH 09/18] arm64: dts: renesas: beacon: Fix RGB Display PWM Backlight

[Adam Ford]

The backlight didn't really work correctly due to some updates that were
made in hardware.  It should be safe to apply these, because the older
hardware was never shipped to anyone, so it shouldn't break anything.
Because the display driver refers to the display as DPI, this also
renames the backlight to use DPI for consistency.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index bf047a9836ed..87a0f556cd2f 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -17,12 +17,12 @@ backlight_lvds: backlight-lvds {
 		default-brightness-level = <6>;
 	};
 
-	backlight_rgb: backlight-rgb {
+	backlight_dpi: backlight-dpi {
 		compatible = "pwm-backlight";
 		power-supply = <&reg_lcd>;
 		enable-gpios = <&gpio_exp1 7 GPIO_ACTIVE_LOW>;
-		pwms = <&pwm0 0 50000>;
-		brightness-levels = <0 4 8 16 32 64 128 255>;
+		pwms = <&pwm0 0 25000>;
+		brightness-levels = <0 25 33 50 63 75 88 100>;
 		default-brightness-level = <6>;
 	};
 
@@ -136,7 +136,7 @@ panel_in: endpoint {
 	rgb {
 		/* Different LCD with compatible timings */
 		compatible = "rocktech,rk070er9427";
-		backlight = <&backlight_rgb>;
+		backlight = <&backlight_dpi>;
 		enable-gpios = <&gpio1 21 GPIO_ACTIVE_HIGH>;
 		power-supply = <&reg_lcd>;
 		port {
-- 
2.25.1

[PATCH 10/18] arm64: dts: renesas: Don't make vccq_sdhi0 always on

[Adam Ford]

vccq_sdhi0 is referenced from sdhi0, so there is no need to force
this regualtor to be always-on.  In theory it could help with
low power modes in the future.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index 87a0f556cd2f..f951bae2a1c6 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -201,15 +201,12 @@ sound_card {
 
 	vccq_sdhi0: regulator-vccq-sdhi0 {
 		compatible = "regulator-gpio";
-
 		regulator-name = "SDHI0 VccQ";
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <3300000>;
-
 		gpios = <&gpio6 30 GPIO_ACTIVE_HIGH>;
 		gpios-states = <1>;
 		states = <3300000 1>, <1800000 0>;
-		regulator-always-on;
 	};
 
 	/* External DU dot clocks */
-- 
2.25.1

[PATCH 11/18] arm64: dts: renesas: beacon: Remove redundant audio code

[Adam Ford]

The SoC's DTSI file already configures this.  There is no need to do
it again.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../dts/renesas/beacon-renesom-baseboard.dtsi   | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index f951bae2a1c6..beaea6de547b 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -649,23 +649,6 @@ &rcar_sound {
 
 	status = "okay";
 
-	clocks = <&cpg CPG_MOD 1005>,
-		 <&cpg CPG_MOD 1006>, <&cpg CPG_MOD 1007>,
-		 <&cpg CPG_MOD 1008>, <&cpg CPG_MOD 1009>,
-		 <&cpg CPG_MOD 1010>, <&cpg CPG_MOD 1011>,
-		 <&cpg CPG_MOD 1012>, <&cpg CPG_MOD 1013>,
-		 <&cpg CPG_MOD 1014>, <&cpg CPG_MOD 1015>,
-		 <&cpg CPG_MOD 1022>, <&cpg CPG_MOD 1023>,
-		 <&cpg CPG_MOD 1024>, <&cpg CPG_MOD 1025>,
-		 <&cpg CPG_MOD 1026>, <&cpg CPG_MOD 1027>,
-		 <&cpg CPG_MOD 1028>, <&cpg CPG_MOD 1029>,
-		 <&cpg CPG_MOD 1030>, <&cpg CPG_MOD 1031>,
-		 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
-		 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
-		 <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
-		 <&audio_clk_a>, <&audio_clk_b>, <&audio_clk_c>,
-		 <&cpg CPG_CORE R8A774A1_CLK_S0D4>;
-
 	ports {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.25.1

[PATCH 12/18] arm64: dts: renesas: beacon: Better describe keys

[Adam Ford]

The keys on the baseboard are laid out in an diamond pattern, up, down,
left, right and center.  Update the descriptions to make it easier to
read.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../dts/renesas/beacon-renesom-baseboard.dtsi | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index beaea6de547b..db3ef33faac5 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -40,38 +40,38 @@ hdmi0_con: endpoint {
 	keys {
 		compatible = "gpio-keys";
 
-		key-1 {
+		key-1 { /* S19 */
 			gpios = <&gpio4 6 GPIO_ACTIVE_LOW>;
 			linux,code = <KEY_1>;
-			label = "Switch-1";
+			label = "Up";
 			wakeup-source;
 			debounce-interval = <20>;
 		};
-		key-2 {
+		key-2 { /*S20 */
 			gpios = <&gpio3 13 GPIO_ACTIVE_LOW>;
 			linux,code = <KEY_2>;
-			label = "Switch-2";
+			label = "Left";
 			wakeup-source;
 			debounce-interval = <20>;
 		};
-		key-3 {
+		key-3 { /* S21 */
 			gpios = <&gpio5 17 GPIO_ACTIVE_LOW>;
 			linux,code = <KEY_3>;
-			label = "Switch-3";
+			label = "Down";
 			wakeup-source;
 			debounce-interval = <20>;
 		};
-		key-4 {
+		key-4 { /* S22 */
 			gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
 			linux,code = <KEY_4>;
-			label = "Switch-4";
+			label = "Right";
 			wakeup-source;
 			debounce-interval = <20>;
 		};
-		key-5 {
+		key-5 { /* S23 */
 			gpios = <&gpio5 22 GPIO_ACTIVE_LOW>;
 			linux,code = <KEY_5>;
-			label = "Switch-4";
+			label = "Center";
 			wakeup-source;
 			debounce-interval = <20>;
 		};
-- 
2.25.1

[PATCH 13/18] arm64: dts: renesas: beacon: Enable SPI

[Adam Ford]

The baseboard routes the SPI to a header which can/will be configured at
either the kit level or using device tree overlays.  Because the baseboard
be supporting more than one kit, enable at the baseboard level rather
than a bunch of duplicates later.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../boot/dts/renesas/beacon-renesom-baseboard.dtsi   | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index db3ef33faac5..200236b6e0ef 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -512,6 +512,13 @@ lvds0_out: endpoint {
 	};
 };
 
+&msiof1 {
+	pinctrl-0 = <&msiof1_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+	cs-gpios = <&gpio3 10 GPIO_ACTIVE_LOW>;
+};
+
 &ohci0 {
 	dr_mode = "otg";
 	status = "okay";
@@ -565,6 +572,11 @@ led_pins: leds {
 		bias-pull-down;
 	};
 
+	msiof1_pins: msiof1 {
+		groups = "msiof1_clk_g", "msiof1_rxd_g", "msiof1_txd_g";
+		function = "msiof1";
+	};
+
 	pwm0_pins: pwm0 {
 		groups = "pwm0";
 		function = "pwm0";
-- 
2.25.1

[PATCH 14/18] arm64: dts: renesas: beacon: Correct I2C bus speeds

[Adam Ford]

For greater compatibility with upcoming kits that will reuse the baseboard
and SOM-level files, adjust the I2C speeds to make it the most compatible
with all devices.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi | 2 +-
 arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index 200236b6e0ef..7f499282f851 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -343,7 +343,7 @@ &hsusb {
 
 &i2c2 {
 	status = "okay";
-	clock-frequency = <100000>;
+	clock-frequency = <400000>;
 	pinctrl-0 = <&i2c2_pins>;
 	pinctrl-names = "default";
 
diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index b093a34b0fa0..b5ba45261c0b 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -110,7 +110,7 @@ &hscif2 {
 
 &i2c4 {
 	status = "okay";
-	clock-frequency = <400000>;
+	clock-frequency = <100000>;
 
 	pca9654: gpio@20 {
 		compatible = "onnn,pca9654";
-- 
2.25.1

[PATCH 15/18] arm64: dts: renesas: beacon-rzg2m-kit: Rearange SoC unique functions

[Adam Ford]

In preparation for adding new dev kits, move anything specific to the
RZ/G2M from the SOM-level and baseboard-levels and move them to the
kit-level.  This allows the SOM and baseboard to be reused with
other SoC's.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../dts/renesas/beacon-renesom-baseboard.dtsi    | 15 ---------------
 .../boot/dts/renesas/beacon-renesom-som.dtsi     |  5 -----
 .../dts/renesas/r8a774a1-beacon-rzg2m-kit.dts    | 16 ++++++++++++++++
 3 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
index 7f499282f851..facb3e6d8010 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
@@ -273,21 +273,6 @@ &can1 {
 	status = "okay";
 };
 
-&du {
-	pinctrl-0 = <&du_pins>;
-	pinctrl-names = "default";
-	status = "okay";
-
-	clocks = <&cpg CPG_MOD 724>,
-		<&cpg CPG_MOD 723>,
-		<&cpg CPG_MOD 722>,
-		<&versaclock5 1>,
-		<&x302_clk>,
-		<&versaclock5 2>;
-	clock-names = "du.0", "du.1", "du.2",
-		"dclkin.0", "dclkin.1", "dclkin.2";
-};
-
 &du_out_rgb {
 	remote-endpoint = <&rgb_panel>;
 };
diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
index b5ba45261c0b..d68e9f5b8b38 100644
--- a/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi
@@ -13,11 +13,6 @@ memory@48000000 {
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
 
-	memory@600000000 {
-		device_type = "memory";
-		reg = <0x6 0x00000000 0x0 0x80000000>;
-	};
-
 	osc_32k: osc_32k {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
diff --git a/arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dts b/arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dts
index 2c5b057c30c6..581e4ec36bcb 100644
--- a/arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dts
@@ -26,4 +26,20 @@ aliases {
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
+
+	memory@600000000 {
+		device_type = "memory";
+		reg = <0x6 0x00000000 0x0 0x80000000>;
+	};
+};
+
+&du {
+	clocks = <&cpg CPG_MOD 724>,
+		 <&cpg CPG_MOD 723>,
+		 <&cpg CPG_MOD 722>,
+		 <&versaclock5 1>,
+		 <&x302_clk>,
+		 <&versaclock5 2>;
+	clock-names = "du.0", "du.1", "du.2",
+		      "dclkin.0", "dclkin.1", "dclkin.2";
 };
-- 
2.25.1

[PATCH 16/18] dt-bindings: arm: renesas: Add Beacon RZ/G2N and RZ/G2H boards

[Adam Ford]

Add beacon,beacon-rzg2n and beacon,beacon-rzg2h to the bindings
list.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 Documentation/devicetree/bindings/arm/renesas.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml b/Documentation/devicetree/bindings/arm/renesas.yaml
index fe11be65039a..5fd0696a9f91 100644
--- a/Documentation/devicetree/bindings/arm/renesas.yaml
+++ b/Documentation/devicetree/bindings/arm/renesas.yaml
@@ -130,6 +130,7 @@ properties:
       - description: RZ/G2N (R8A774B1)
         items:
           - enum:
+              - beacon,beacon-rzg2n # Beacon EmbeddedWorks RZ/G2N Kit
               - hoperun,hihope-rzg2n # HopeRun HiHope RZ/G2N platform
           - const: renesas,r8a774b1
 
@@ -154,6 +155,7 @@ properties:
       - description: RZ/G2H (R8A774E1)
         items:
           - enum:
+              - beacon,beacon-rzg2h # Beacon EmbeddedWorks RZ/G2H Kit
               - hoperun,hihope-rzg2h # HopeRun HiHope RZ/G2H platform
           - const: renesas,r8a774e1
 
-- 
2.25.1

[PATCH 17/18] arm64: dts: renesas: Introduce r8a774b1-beacon-rzg2n-kit

[Adam Ford]

Beacon EmebeddedWorks is introducing a new kit based on the
RZ/G2N SoC from Renesas.

The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1
cellular radio.

The Baseboard has Ethernet, USB, HDMI, stereo audio in and out,
along with a variety of push buttons and LED's, and support for
a parallel RGB and an LVDS display.  It uses the same baseboard
and SOM as the RZ/G2M.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/renesas/Makefile          |  1 +
 .../dts/renesas/r8a774b1-beacon-rzg2n-kit.dts | 43 +++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
index 3b8b03705917..ea3d7d9bc52e 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-rev2.dtb
 dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-rev2-ex.dtb
 dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-rev2-ex-idk-1110wr.dtb
 
+dtb-$(CONFIG_ARCH_R8A774B1) += r8a774b1-beacon-rzg2n-kit.dtb
 dtb-$(CONFIG_ARCH_R8A774B1) += r8a774b1-hihope-rzg2n.dtb
 dtb-$(CONFIG_ARCH_R8A774B1) += r8a774b1-hihope-rzg2n-ex.dtb
 dtb-$(CONFIG_ARCH_R8A774B1) += r8a774b1-hihope-rzg2n-ex-idk-1110wr.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a774b1-beacon-rzg2n-kit.dts b/arch/arm64/boot/dts/renesas/r8a774b1-beacon-rzg2n-kit.dts
new file mode 100644
index 000000000000..01a233102976
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a774b1-beacon-rzg2n-kit.dts
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020, Compass Electronics Group, LLC
+ */
+
+/dts-v1/;
+
+#include "r8a774b1.dtsi"
+#include "beacon-renesom-som.dtsi"
+#include "beacon-renesom-baseboard.dtsi"
+
+/ {
+	model = "Beacon Embedded Works RZ/G2N Development Kit";
+	compatible =	"beacon,beacon-rzg2n", "renesas,r8a774b1";
+
+	aliases {
+		serial0 = &scif2;
+		serial1 = &hscif0;
+		serial2 = &hscif1;
+		serial3 = &scif0;
+		serial4 = &hscif2;
+		serial5 = &scif5;
+		serial6 = &scif4;
+		ethernet0 = &avb;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+&du {
+	status = "okay";
+
+	clocks = <&cpg CPG_MOD 724>,
+		<&cpg CPG_MOD 723>,
+		<&cpg CPG_MOD 721>,
+		<&versaclock5 1>,
+		<&x302_clk>,
+		<&versaclock5 2>;
+	clock-names = "du.0", "du.1", "du.3",
+		"dclkin.0", "dclkin.1", "dclkin.3";
+};
-- 
2.25.1

[PATCH 18/18] arm64: dts: renesas: Introduce r8a774e1-beacon-rzg2h-kit

[Adam Ford]

Beacon EmebeddedWorks is introducing a new kit based on the
RZ/G2H SoC from Renesas.

The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1
cellular radio.

The Baseboard has Ethernet, USB, HDMI, stereo audio in and out,
along with a variety of push buttons and LED's, and support for
a parallel RGB and an LVDS display.  It uses the same baseboard
and SOM files as the RZ/G2M and RZ/G2N kits.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 .../dts/renesas/r8a774e1-beacon-rzg2h-kit.dts | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774e1-beacon-rzg2h-kit.dts b/arch/arm64/boot/dts/renesas/r8a774e1-beacon-rzg2h-kit.dts
new file mode 100644
index 000000000000..8ff5856ac727
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a774e1-beacon-rzg2h-kit.dts
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020, Compass Electronics Group, LLC
+ */
+
+/dts-v1/;
+
+#include "r8a774e1.dtsi"
+#include "beacon-renesom-som.dtsi"
+#include "beacon-renesom-baseboard.dtsi"
+
+/ {
+	model = "Beacon Embedded Works RZ/G2H Development Kit";
+	compatible =	"beacon,beacon-rzg2h", "renesas,r8a774e1";
+
+	aliases {
+		serial0 = &scif2;
+		serial1 = &hscif0;
+		serial2 = &hscif1;
+		serial3 = &scif0;
+		serial4 = &hscif2;
+		serial5 = &scif5;
+		serial6 = &scif4;
+		ethernet0 = &avb;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory@500000000 {
+		device_type = "memory";
+		reg = <0x5 0x00000000 0x0 0x80000000>;
+	};
+};
+
+&du {
+	status = "okay";
+
+	clocks = <&cpg CPG_MOD 724>,
+		<&cpg CPG_MOD 723>,
+		<&cpg CPG_MOD 721>,
+		<&versaclock5 1>,
+		<&x302_clk>,
+		<&versaclock5 2>;
+	clock-names = "du.0", "du.1", "du.3",
+		"dclkin.0", "dclkin.1", "dclkin.3";
+};
-- 
2.25.1

Re: [PATCH 16/18] dt-bindings: arm: renesas: Add Beacon RZ/G2N and RZ/G2H boards

[Rob Herring]

On Sun, 13 Dec 2020 12:37:56 -0600, Adam Ford wrote:
> Add beacon,beacon-rzg2n and beacon,beacon-rzg2h to the bindings
> list.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/renesas.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 01/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Geert Uytterhoeven]

Hi Adam,

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> When the board was added, clock drivers were being updated done at
> the same time to allow the versaclock driver to properly configure
> the modes.  Unforutnately, the updates were not applied to the board

Unfortunately

> files at the time they should have been, so do it now.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -5,6 +5,7 @@
>
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
> +#include <dt-bindings/clk/versaclock.h>
>
>  / {
>         backlight_lvds: backlight-lvds {
> @@ -294,12 +295,12 @@ &du_out_rgb {
>  &ehci0 {
>         dr_mode = "otg";
>         status = "okay";
> -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;

Why this change? You said before you don't need this
https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/

BTW, something I missed in the earlier review: is there an override
needed at all?

>  };
>
>  &ehci1 {
>         status = "okay";
> -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;

Same here.

BTW, something I missed in the earlier review: why did you override

    clocks = <&cpg CPG_MOD 702>;

by

    clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;

?

>  };
>
>  &hdmi0 {
> @@ -373,12 +374,40 @@ versaclock6_bb: clock-controller@6a {
>                 #clock-cells = <1>;
>                 clocks = <&x304_clk>;
>                 clock-names = "xin";
> -               /* CSI0_MCLK, CSI1_MCLK, AUDIO_CLKIN, USB_HUB_MCLK_BB */
> +               clock-output-names = "versaclock6_bb.out0_sel_i2cb",
> +                                     "versaclock6_bb.out1",
> +                                     "versaclock6_bb.out2",
> +                                     "versaclock6_bb.out3",
> +                                     "versaclock6_bb.out4";

Why? IIUIC, the driver doesn't parse clock-output-names
(and it shouldn't).

>                 assigned-clocks = <&versaclock6_bb 1>,
>                                    <&versaclock6_bb 2>,
>                                    <&versaclock6_bb 3>,
>                                    <&versaclock6_bb 4>;
>                 assigned-clock-rates =  <24000000>, <24000000>, <24000000>, <24576000>;
> +
> +               OUT1 {
> +                       idt,mode = <VC5_CMOS>;
> +                       idt,voltage-microvolts = <1800000>;

Oops. The DT bindings say "idt,voltage-microvolt", the example in the DT
bindings says "idt,voltage-microvolts", and the driver parses
"idt,voltage-microvolts".

According to Documentation/devicetree/bindings/property-units.txt, the
property name should end in "microvolt".

Patch sent.
https://lore.kernel.org/linux-clk/20201216145231.1344317-1-geert+renesas@glider.be/

> +                       idt,slew-percent = <100>;
> +               };

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Adam Ford]

On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > When the board was added, clock drivers were being updated done at
> > the same time to allow the versaclock driver to properly configure
> > the modes.  Unforutnately, the updates were not applied to the board
>
> Unfortunately

Sorry, I can fix that.

>
> > files at the time they should have been, so do it now.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > @@ -5,6 +5,7 @@
> >
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/input/input.h>
> > +#include <dt-bindings/clk/versaclock.h>
> >
> >  / {
> >         backlight_lvds: backlight-lvds {
> > @@ -294,12 +295,12 @@ &du_out_rgb {
> >  &ehci0 {
> >         dr_mode = "otg";
> >         status = "okay";
> > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
>
> Why this change? You said before you don't need this
> https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
>

I had talked with the hardware guys about buy pre-programmed
versaclock chips which would have been pre-configured and pre-enabled.
I thought it was going to happen, but it didn't, so we need the
versaclock driver to enable the reference clock for the USB
controllers, ethernet controller and audio clocks.  Previously we were
manually configuring it or it was coincidentally working. Ideally,
we'd have the clock system intentionally enable/disable the clocks
when drivers are loaded/unloaded for for power management reasons.

> BTW, something I missed in the earlier review: is there an override
> needed at all?

We need the versaclock for sure.  I'll do some more testing and try to
clean this up in the next revision.

>
> >  };
> >
> >  &ehci1 {
> >         status = "okay";
> > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
>
> Same here.
>
> BTW, something I missed in the earlier review: why did you override
>
>     clocks = <&cpg CPG_MOD 702>;
>
> by
>
>     clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;

Might be an accidental copy-paste error.  I need to review all three
SoC's and adjust the device trees accordingly.

>
> ?
>
> >  };
> >
> >  &hdmi0 {
> > @@ -373,12 +374,40 @@ versaclock6_bb: clock-controller@6a {
> >                 #clock-cells = <1>;
> >                 clocks = <&x304_clk>;
> >                 clock-names = "xin";
> > -               /* CSI0_MCLK, CSI1_MCLK, AUDIO_CLKIN, USB_HUB_MCLK_BB */
> > +               clock-output-names = "versaclock6_bb.out0_sel_i2cb",
> > +                                     "versaclock6_bb.out1",
> > +                                     "versaclock6_bb.out2",
> > +                                     "versaclock6_bb.out3",
> > +                                     "versaclock6_bb.out4";
>
> Why? IIUIC, the driver doesn't parse clock-output-names
> (and it shouldn't).

This was probably copy-paste from an internal repo we have using an
older, customized kernel due to clashing of names with more than one
versaclock was available.  I'll remove it during the next revision.

>
> >                 assigned-clocks = <&versaclock6_bb 1>,
> >                                    <&versaclock6_bb 2>,
> >                                    <&versaclock6_bb 3>,
> >                                    <&versaclock6_bb 4>;
> >                 assigned-clock-rates =  <24000000>, <24000000>, <24000000>, <24576000>;
> > +
> > +               OUT1 {
> > +                       idt,mode = <VC5_CMOS>;
> > +                       idt,voltage-microvolts = <1800000>;
>
> Oops. The DT bindings say "idt,voltage-microvolt", the example in the DT
> bindings says "idt,voltage-microvolts", and the driver parses
> "idt,voltage-microvolts".
>
> According to Documentation/devicetree/bindings/property-units.txt, the
> property name should end in "microvolt".
>
> Patch sent.
> https://lore.kernel.org/linux-clk/20201216145231.1344317-1-geert+renesas@glider.be/
>

Thanks for that.  I'll submit an update based on the patch you sent.

adam
> > +                       idt,slew-percent = <100>;
> > +               };
>

Thank you for the review.  Is that the only patch in the series with
concerns?  I probably won't get to V2 until this weekend.

adam
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

>
> > files at the time they should have been, so do it now.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > @@ -5,6 +5,7 @@
> >
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/input/input.h>
> > +#include <dt-bindings/clk/versaclock.h>
> >
> >  / {
> >         backlight_lvds: backlight-lvds {
> > @@ -294,12 +295,12 @@ &du_out_rgb {
> >  &ehci0 {
> >         dr_mode = "otg";
> >         status = "okay";
> > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
>
> Why this change? You said before you don't need this
> https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
>
> BTW, something I missed in the earlier review: is there an override
> needed at all?
>
> >  };
> >
> >  &ehci1 {
> >         status = "okay";
> > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
>
> Same here.
>
> BTW, something I missed in the earlier review: why did you override
>
>     clocks = <&cpg CPG_MOD 702>;
>
> by
>
>     clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
>
> ?
>
> >  };
> >
> >  &hdmi0 {
> > @@ -373,12 +374,40 @@ versaclock6_bb: clock-controller@6a {
> >                 #clock-cells = <1>;
> >                 clocks = <&x304_clk>;
> >                 clock-names = "xin";
> > -               /* CSI0_MCLK, CSI1_MCLK, AUDIO_CLKIN, USB_HUB_MCLK_BB */
> > +               clock-output-names = "versaclock6_bb.out0_sel_i2cb",
> > +                                     "versaclock6_bb.out1",
> > +                                     "versaclock6_bb.out2",
> > +                                     "versaclock6_bb.out3",
> > +                                     "versaclock6_bb.out4";
>
> Why? IIUIC, the driver doesn't parse clock-output-names
> (and it shouldn't).
>
> >                 assigned-clocks = <&versaclock6_bb 1>,
> >                                    <&versaclock6_bb 2>,
> >                                    <&versaclock6_bb 3>,
> >                                    <&versaclock6_bb 4>;
> >                 assigned-clock-rates =  <24000000>, <24000000>, <24000000>, <24576000>;
> > +
> > +               OUT1 {
> > +                       idt,mode = <VC5_CMOS>;
> > +                       idt,voltage-microvolts = <1800000>;
>
> Oops. The DT bindings say "idt,voltage-microvolt", the example in the DT
> bindings says "idt,voltage-microvolts", and the driver parses
> "idt,voltage-microvolts".
>
> According to Documentation/devicetree/bindings/property-units.txt, the
> property name should end in "microvolt".
>
> Patch sent.
> https://lore.kernel.org/linux-clk/20201216145231.1344317-1-geert+renesas@glider.be/
>
> > +                       idt,slew-percent = <100>;
> > +               };
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Geert Uytterhoeven]

Hi Adam,

On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > When the board was added, clock drivers were being updated done at
> > > the same time to allow the versaclock driver to properly configure
> > > the modes.  Unforutnately, the updates were not applied to the board

> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > @@ -5,6 +5,7 @@
> > >
> > >  #include <dt-bindings/gpio/gpio.h>
> > >  #include <dt-bindings/input/input.h>
> > > +#include <dt-bindings/clk/versaclock.h>
> > >
> > >  / {
> > >         backlight_lvds: backlight-lvds {
> > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > >  &ehci0 {
> > >         dr_mode = "otg";
> > >         status = "okay";
> > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> >
> > Why this change? You said before you don't need this
> > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> >
>
> I had talked with the hardware guys about buy pre-programmed
> versaclock chips which would have been pre-configured and pre-enabled.
> I thought it was going to happen, but it didn't, so we need the
> versaclock driver to enable the reference clock for the USB
> controllers, ethernet controller and audio clocks.  Previously we were
> manually configuring it or it was coincidentally working. Ideally,
> we'd have the clock system intentionally enable/disable the clocks
> when drivers are loaded/unloaded for for power management reasons.

Can you tell me how exactly the Versaclock outputs are wired?
E.g. for USB, the bindings don't say anything about a third clock input,
so I'd like to know where that clock is fed into USB.

> Thank you for the review.  Is that the only patch in the series with
> concerns?  I probably won't get to V2 until this weekend.

Sorry, I still have to review the other patches in your series.
Anyway, we have time until the end of January to queue DT patches for
v5.12...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 02/18] arm64: dts: renesas: beacon kit: Fix choppy Bluetooth Audio

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The Bluetooth chip is capable of operating at 4Mbps, but the
> max-speed setting was on the UART node instead of the Bluetooth
> node, so the chip didn't operate at the correct speed resulting
> in choppy audio.  Fix this by setting the max-speed in the proper
> node.
>
> Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.12.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 03/18] arm64: dts: renesas: beacon kit: Remove unnecessary nodes

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> VSPI0 and VSPB are already enabled by default.  There is no need
> to add extra nodes to enable them.  Remove the redundant nodes.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.12.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Fix Audio Clock sources

[Geert Uytterhoeven]

Hi Adam,

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The SoC was expecting two clock sources with different frequencies.
> One to support 44.1KHz and one to support 48KHz.  With the newly added
> ability to configure the programmably clock, configure both clocks.
>
> Beacause the SoC is expecting a fixed clock/oscillator, it doesn't
> attempt to get and enable the clock for audio_clk_a. The choice to
> use a fixed-factor-clock was due to the fact that it will automatically
> enable the programmable clock frequency without change any code.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -250,9 +250,12 @@ ss_ep: endpoint {
>  };
>
>  &audio_clk_a {
> -       clock-frequency = <24576000>;
> -       assigned-clocks = <&versaclock6_bb 4>;
> -       assigned-clock-rates = <24576000>;
> +       /delete-property/ clock-frequency;
> +       #clock-cells = <0>;
> +       compatible = "fixed-factor-clock";
> +       clock-mult = <1>;
> +       clock-div = <1>;
> +       clocks = <&versaclock6_bb 4>;
>  };

Shouldn't you override the clocks property in the rcar_sound node
instead, like is done in several other board DTS files (with cs2000)?

>
>  &audio_clk_b {
> @@ -591,7 +594,7 @@ sound_pins: sound {
>         };
>
>         sound_clk_pins: sound_clk {
> -               groups = "audio_clk_a_a";
> +               groups = "audio_clk_a_a", "audio_clk_b_a";
>                 function = "audio_clk";
>         };

Yes, this part was definitely missing.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon: Fix audio-1.8V pin enable

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The fact the audio worked at all was a coindicence because the wrong

coincidence.

> gpio enable was used.  Use the correct GPIO pin to ensure its operation.
>
> Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks, I have to trust you on this one, i.e. will queue in
renesas-devel for v5.12
(with the above fixed).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 06/18] arm64: dts: renesas: beacon: Configure Audio CODEC clocks

[Geert Uytterhoeven]

Hi Adam,

CC alsa-devel

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> With the newly added configurable clock options, the audio CODEC can
> configure the mclk automatically.  Add the reference to the versaclock.
> Since the devices on I2C5 can communicate at 400KHz, let's also increase
> that too
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -424,13 +424,15 @@ &i2c0 {
>
>  &i2c5 {
>         status = "okay";
> -       clock-frequency = <100000>;
> +       clock-frequency = <400000>;
>         pinctrl-0 = <&i2c5_pins>;
>         pinctrl-names = "default";
>
>         codec: wm8962@1a {
>                 compatible = "wlf,wm8962";
>                 reg = <0x1a>;
> +               clocks = <&versaclock6_bb 3>;
> +               clock-names = "mclk";

While the driver does get the (nameless) clock, the DT bindings lack any
mention of a clocks property.  It would be good to update the bindings.

Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
hand) use "xclk" instead of "mclk"?

>                 DCVDD-supply = <&reg_audio>;
>                 DBVDD-supply = <&reg_audio>;
>                 AVDD-supply = <&reg_audio>;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon: Fix LVDS PWM Backlight

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The backlight didn't really work correctly due to some updates that were
> made in hardware.  It should be safe to apply these, because the older
> hardware was never shipped to anyone, so it shouldn't break anything.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks, I have to trust you on this one, i.e. will queue in
renesas-devel for v5.12.

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -575,7 +575,7 @@ pwm0_pins: pwm0 {
>
>         pwm2_pins: pwm2 {
>                 groups = "pwm2_a";
> -               function = "pwm2_a";
> +               function = "pwm2";
>         };

This part is definitely correct.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon: Enable SCIF4

[Geert Uytterhoeven]

Hi Adam,

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The baseboard supports SCIF4, enable the pins and the node for it.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -578,6 +578,11 @@ pwm2_pins: pwm2 {
>                 function = "pwm2";
>         };
>
> +       scif4_pins: scif4 {
> +               groups = "scif4_data_c";
> +               function = "scif4";
> +       };
> +
>         sdhi0_pins: sd0 {
>                 groups = "sdhi0_data4", "sdhi0_ctrl";
>                 function = "sdhi0";
> @@ -706,6 +711,12 @@ &scif0 {
>         status = "okay";
>  };
>
> +&scif4 {
> +       pinctrl-0 = <&scif4_pins>;
> +       pinctrl-names = "default";
> +       status = "okay";
> +};
> +
>  &scif5 {
>         pinctrl-0 = <&scif5_pins>;
>         pinctrl-names = "default";

As mixing SCIF ports with and without aliases may lead to failures,
depending on probe order, you want to add an aliases for scif4 to
arch/arm64/boot/dts/renesas/r8a774a1-beacon-rzg2m-kit.dts.
I see you did that for the rzg2h and rzg2n kits, but rzh2m lacks it.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: Don't make vccq_sdhi0 always on

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> vccq_sdhi0 is referenced from sdhi0, so there is no need to force
> this regualtor to be always-on.  In theory it could help with

regulator

> low power modes in the future.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.12 (with the typo fixed, and "beacon"
added to the one-line summary).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon: Fix RGB Display PWM Backlight

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The backlight didn't really work correctly due to some updates that were
> made in hardware.  It should be safe to apply these, because the older
> hardware was never shipped to anyone, so it shouldn't break anything.
> Because the display driver refers to the display as DPI, this also
> renames the backlight to use DPI for consistency.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks, I have to trust you on this one, i.e. will queue in
renesas-devel for v5.12.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon: Better describe keys

[Geert Uytterhoeven]

Hi Adam,

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The keys on the baseboard are laid out in an diamond pattern, up, down,
> left, right and center.  Update the descriptions to make it easier to
> read.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -40,38 +40,38 @@ hdmi0_con: endpoint {
>         keys {
>                 compatible = "gpio-keys";
>
> -               key-1 {
> +               key-1 { /* S19 */
>                         gpios = <&gpio4 6 GPIO_ACTIVE_LOW>;
>                         linux,code = <KEY_1>;
> -                       label = "Switch-1";
> +                       label = "Up";
>                         wakeup-source;
>                         debounce-interval = <20>;
>                 };
> -               key-2 {
> +               key-2 { /*S20 */
>                         gpios = <&gpio3 13 GPIO_ACTIVE_LOW>;
>                         linux,code = <KEY_2>;
> -                       label = "Switch-2";
> +                       label = "Left";
>                         wakeup-source;
>                         debounce-interval = <20>;
>                 };
> -               key-3 {
> +               key-3 { /* S21 */
>                         gpios = <&gpio5 17 GPIO_ACTIVE_LOW>;
>                         linux,code = <KEY_3>;
> -                       label = "Switch-3";
> +                       label = "Down";
>                         wakeup-source;
>                         debounce-interval = <20>;
>                 };
> -               key-4 {
> +               key-4 { /* S22 */
>                         gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
>                         linux,code = <KEY_4>;
> -                       label = "Switch-4";
> +                       label = "Right";
>                         wakeup-source;
>                         debounce-interval = <20>;
>                 };
> -               key-5 {
> +               key-5 { /* S23 */
>                         gpios = <&gpio5 22 GPIO_ACTIVE_LOW>;
>                         linux,code = <KEY_5>;
> -                       label = "Switch-4";
> +                       label = "Center";
>                         wakeup-source;
>                         debounce-interval = <20>;
>                 };

Wouldn't it make sense for the linux,code properties to reflect this, and thus
change them to KEY_{UP,LEFT,DOWN,RIGHT,ENTER} (or SELECT, or OK)?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 13/18] arm64: dts: renesas: beacon: Enable SPI

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> The baseboard routes the SPI to a header which can/will be configured at
> either the kit level or using device tree overlays.  Because the baseboard
> be supporting more than one kit, enable at the baseboard level rather
> than a bunch of duplicates later.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks, I have to trust you on this one, i.e. will queue in
renesas-devel for v5.12.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon: Correct I2C bus speeds

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> For greater compatibility with upcoming kits that will reuse the baseboard
> and SOM-level files, adjust the I2C speeds to make it the most compatible
> with all devices.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks, all i2c devices on the bus support 400 kHz.
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.12.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 15/18] arm64: dts: renesas: beacon-rzg2m-kit: Rearange SoC unique functions

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> In preparation for adding new dev kits, move anything specific to the
> RZ/G2M from the SOM-level and baseboard-levels and move them to the
> kit-level.  This allows the SOM and baseboard to be reused with
> other SoC's.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.12.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 16/18] dt-bindings: arm: renesas: Add Beacon RZ/G2N and RZ/G2H boards

[Geert Uytterhoeven]

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> Add beacon,beacon-rzg2n and beacon,beacon-rzg2h to the bindings
> list.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.12.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 15/18] arm64: dts: renesas: beacon-rzg2m-kit: Rearange SoC unique functions

[Geert Uytterhoeven]

Hi Adam,

On Thu, Dec 17, 2020 at 12:41 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > In preparation for adding new dev kits, move anything specific to the
> > RZ/G2M from the SOM-level and baseboard-levels and move them to the
> > kit-level.  This allows the SOM and baseboard to be reused with
> > other SoC's.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel for v5.12.

Sorry, spoke too soon. What happened to:

-       pinctrl-0 = <&du_pins>;
-       pinctrl-names = "default";
-       status = "okay";

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 17/18] arm64: dts: renesas: Introduce r8a774b1-beacon-rzg2n-kit

[Geert Uytterhoeven]

Hi Adam,

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> Beacon EmebeddedWorks is introducing a new kit based on the
> RZ/G2N SoC from Renesas.
>
> The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1
> cellular radio.
>
> The Baseboard has Ethernet, USB, HDMI, stereo audio in and out,
> along with a variety of push buttons and LED's, and support for
> a parallel RGB and an LVDS display.  It uses the same baseboard
> and SOM as the RZ/G2M.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a774b1-beacon-rzg2n-kit.dts
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020, Compass Electronics Group, LLC
> + */
> +
> +/dts-v1/;
> +
> +#include "r8a774b1.dtsi"
> +#include "beacon-renesom-som.dtsi"
> +#include "beacon-renesom-baseboard.dtsi"
> +
> +/ {
> +       model = "Beacon Embedded Works RZ/G2N Development Kit";
> +       compatible =    "beacon,beacon-rzg2n", "renesas,r8a774b1";
> +
> +       aliases {
> +               serial0 = &scif2;
> +               serial1 = &hscif0;
> +               serial2 = &hscif1;
> +               serial3 = &scif0;
> +               serial4 = &hscif2;
> +               serial5 = &scif5;
> +               serial6 = &scif4;
> +               ethernet0 = &avb;
> +       };
> +
> +       chosen {
> +               stdout-path = "serial0:115200n8";
> +       };

No memory nodes? Are you relying on U-Boot to fill them in?
If yes, why do you have them in the other board DTS files?

> +};
> +
> +&du {
> +       status = "okay";

Missing pinctrl properties?

> +
> +       clocks = <&cpg CPG_MOD 724>,
> +               <&cpg CPG_MOD 723>,
> +               <&cpg CPG_MOD 721>,
> +               <&versaclock5 1>,
> +               <&x302_clk>,
> +               <&versaclock5 2>;
> +       clock-names = "du.0", "du.1", "du.3",
> +               "dclkin.0", "dclkin.1", "dclkin.3";
> +};

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 18/18] arm64: dts: renesas: Introduce r8a774e1-beacon-rzg2h-kit

[Geert Uytterhoeven]

Hi Adam,

On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> Beacon EmebeddedWorks is introducing a new kit based on the

Embedded Works (suggested by Google ;-)

> RZ/G2H SoC from Renesas.
>
> The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1
> cellular radio.
>
> The Baseboard has Ethernet, USB, HDMI, stereo audio in and out,
> along with a variety of push buttons and LED's, and support for
> a parallel RGB and an LVDS display.  It uses the same baseboard
> and SOM files as the RZ/G2M and RZ/G2N kits.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a774e1-beacon-rzg2h-kit.dts
> @@ -0,0 +1,48 @@

> +&du {
> +       status = "okay";

Missing pinctrl properties?

> +
> +       clocks = <&cpg CPG_MOD 724>,
> +               <&cpg CPG_MOD 723>,
> +               <&cpg CPG_MOD 721>,
> +               <&versaclock5 1>,
> +               <&x302_clk>,
> +               <&versaclock5 2>;
> +       clock-names = "du.0", "du.1", "du.3",
> +               "dclkin.0", "dclkin.1", "dclkin.3";
> +};

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Adam Ford]

On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > When the board was added, clock drivers were being updated done at
> > > > the same time to allow the versaclock driver to properly configure
> > > > the modes.  Unforutnately, the updates were not applied to the board
>
> > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > @@ -5,6 +5,7 @@
> > > >
> > > >  #include <dt-bindings/gpio/gpio.h>
> > > >  #include <dt-bindings/input/input.h>
> > > > +#include <dt-bindings/clk/versaclock.h>
> > > >
> > > >  / {
> > > >         backlight_lvds: backlight-lvds {
> > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > >  &ehci0 {
> > > >         dr_mode = "otg";
> > > >         status = "okay";
> > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > >
> > > Why this change? You said before you don't need this
> > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > >
> >
> > I had talked with the hardware guys about buy pre-programmed
> > versaclock chips which would have been pre-configured and pre-enabled.
> > I thought it was going to happen, but it didn't, so we need the
> > versaclock driver to enable the reference clock for the USB
> > controllers, ethernet controller and audio clocks.  Previously we were
> > manually configuring it or it was coincidentally working. Ideally,
> > we'd have the clock system intentionally enable/disable the clocks
> > when drivers are loaded/unloaded for for power management reasons.
>
> Can you tell me how exactly the Versaclock outputs are wired?

The SoC is expecting a fixed external 50 MHz clock connected to
USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
We're also using the Versaclock to drive the AVB TXCRefClk,
du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
RZ/G2N) instead of fixed clocks.

> E.g. for USB, the bindings don't say anything about a third clock input,
> so I'd like to know where that clock is fed into USB.

The way the driver is crafted, it can take in multiple clocks and it
goes through a list to enable them all, so I added the versaclock to
the array.  Without the versaclock reference, the clock doesn't get
turned on and the USB fails to operate.

The DU clocks are also expecting an array, so I added the versaclock
to that array as well.

It's similar to the rationale that I'm trying to add the option clock
for the AVB TXC_Ref clock on the other path.  We're using the
versaclock there as well.  The difference is that in the case of the
AVB_TXCRefClk, the driver isn't expecting an array of clocks, it's
only expecting a single clock.  In order to enable the additional
clock,  I started the patch to accept the optional clock for the
TXCRefClk in order to get the clock system to enable the clock.

Because the Versaclock isn't programmed to automatically start, they
need the consumers of the clock to request and enable them.

I admit that I'll probably need to update the bindings to add the
extra clocks as optional, so if you want, I can submit additional
patches to add these optional clocks to their respective bindings.

>
> > Thank you for the review.  Is that the only patch in the series with
> > concerns?  I probably won't get to V2 until this weekend.
>
> Sorry, I still have to review the other patches in your series.
> Anyway, we have time until the end of January to queue DT patches for
> v5.12...

Great.  Thank you,

adam
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Fix Audio Clock sources

[Adam Ford]

On Thu, Dec 17, 2020 at 4:54 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > The SoC was expecting two clock sources with different frequencies.
> > One to support 44.1KHz and one to support 48KHz.  With the newly added
> > ability to configure the programmably clock, configure both clocks.
> >
> > Beacause the SoC is expecting a fixed clock/oscillator, it doesn't
> > attempt to get and enable the clock for audio_clk_a. The choice to
> > use a fixed-factor-clock was due to the fact that it will automatically
> > enable the programmable clock frequency without change any code.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > @@ -250,9 +250,12 @@ ss_ep: endpoint {
> >  };
> >
> >  &audio_clk_a {
> > -       clock-frequency = <24576000>;
> > -       assigned-clocks = <&versaclock6_bb 4>;
> > -       assigned-clock-rates = <24576000>;
> > +       /delete-property/ clock-frequency;
> > +       #clock-cells = <0>;
> > +       compatible = "fixed-factor-clock";
> > +       clock-mult = <1>;
> > +       clock-div = <1>;
> > +       clocks = <&versaclock6_bb 4>;
> >  };
>
> Shouldn't you override the clocks property in the rcar_sound node
> instead, like is done in several other board DTS files (with cs2000)?
>

I guess there are multiple ways to do this.  Because the rcar_sound
was already expecting a reference to audio_clk_a, it seemed less
intrusive this way. The way I proposed, we can use the default
rcar_sound clocking and just change the audio_clk node to enable the
versaclock output.  The versaclock is driving the audio_clk_a
reference clock, so it seemed appropriate to put it there.

If you want me to change, I will.

> >
> >  &audio_clk_b {
> > @@ -591,7 +594,7 @@ sound_pins: sound {
> >         };
> >
> >         sound_clk_pins: sound_clk {
> > -               groups = "audio_clk_a_a";
> > +               groups = "audio_clk_a_a", "audio_clk_b_a";
> >                 function = "audio_clk";
> >         };
>
> Yes, this part was definitely missing.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 02/18] arm64: dts: renesas: beacon kit: Fix choppy Bluetooth Audio

[Adam Ford]

On Thu, Dec 17, 2020 at 4:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > The Bluetooth chip is capable of operating at 4Mbps, but the
> > max-speed setting was on the UART node instead of the Bluetooth
> > node, so the chip didn't operate at the correct speed resulting
> > in choppy audio.  Fix this by setting the max-speed in the proper
> > node.
> >
> > Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel for v5.12.

Since various other patches in the series need a V2, should this be
included in the V2 as no-change, or should I skip this and others that
have been queued?  If/when they appear in your branch, I can rebase
the series against that branch and just submit V2's on what's missing.

I want to do whatever creates less work for you.

adam
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 06/18] arm64: dts: renesas: beacon: Configure Audio CODEC clocks

[Adam Ford]

On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> CC alsa-devel
>
> On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > With the newly added configurable clock options, the audio CODEC can
> > configure the mclk automatically.  Add the reference to the versaclock.
> > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > that too
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > @@ -424,13 +424,15 @@ &i2c0 {
> >
> >  &i2c5 {
> >         status = "okay";
> > -       clock-frequency = <100000>;
> > +       clock-frequency = <400000>;
> >         pinctrl-0 = <&i2c5_pins>;
> >         pinctrl-names = "default";
> >
> >         codec: wm8962@1a {
> >                 compatible = "wlf,wm8962";
> >                 reg = <0x1a>;
> > +               clocks = <&versaclock6_bb 3>;
> > +               clock-names = "mclk";
>
> While the driver does get the (nameless) clock, the DT bindings lack any
> mention of a clocks property.  It would be good to update the bindings.

Agreed.  I'll push an update to add the clocks property.

>
> Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> hand) use "xclk" instead of "mclk"?

On the schematics for the two imx boards, it's labeled as xclk, so it
was named as such.  For this board, the schematic names it mclk. The
driver doesn't care about the clock-names property, so I'll just
remove them.

adam
>
> >                 DCVDD-supply = <&reg_audio>;
> >                 DBVDD-supply = <&reg_audio>;
> >                 AVDD-supply = <&reg_audio>;
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 06/18] arm64: dts: renesas: beacon: Configure Audio CODEC clocks

[Adam Ford]

On Thu, Dec 17, 2020 at 7:33 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Adam,
> >
> > CC alsa-devel
> >
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > With the newly added configurable clock options, the audio CODEC can
> > > configure the mclk automatically.  Add the reference to the versaclock.
> > > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > > that too
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > @@ -424,13 +424,15 @@ &i2c0 {
> > >
> > >  &i2c5 {
> > >         status = "okay";
> > > -       clock-frequency = <100000>;
> > > +       clock-frequency = <400000>;
> > >         pinctrl-0 = <&i2c5_pins>;
> > >         pinctrl-names = "default";
> > >
> > >         codec: wm8962@1a {
> > >                 compatible = "wlf,wm8962";
> > >                 reg = <0x1a>;
> > > +               clocks = <&versaclock6_bb 3>;
> > > +               clock-names = "mclk";
> >
> > While the driver does get the (nameless) clock, the DT bindings lack any
> > mention of a clocks property.  It would be good to update the bindings.
>
> Agreed.  I'll push an update to add the clocks property.
>

I pushed a change to add the optional clock information to the
bindings txt file [1].
> >
> > Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> > arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> > hand) use "xclk" instead of "mclk"?
>
> On the schematics for the two imx boards, it's labeled as xclk, so it
> was named as such.  For this board, the schematic names it mclk. The
> driver doesn't care about the clock-names property, so I'll just
> remove them.

I pushed patches to remove these nodes from the other boards [2].
I'll remove them if V2 of the patch series for the Renesas board.

adam
[1] - https://patchwork.kernel.org/project/alsa-devel/patch/20201217162740.1452000-1-aford173@gmail.com/
[2] - https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=403739

>
> adam
> >
> > >                 DCVDD-supply = <&reg_audio>;
> > >                 DBVDD-supply = <&reg_audio>;
> > >                 AVDD-supply = <&reg_audio>;
> >
> > Gr{oetje,eeting}s,
> >
> >                         Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 06/18] arm64: dts: renesas: beacon: Configure Audio CODEC clocks

[Geert Uytterhoeven]

Hi Adam,

On Thu, Dec 17, 2020 at 2:33 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > CC alsa-devel
> >
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > With the newly added configurable clock options, the audio CODEC can
> > > configure the mclk automatically.  Add the reference to the versaclock.
> > > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > > that too
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > @@ -424,13 +424,15 @@ &i2c0 {
> > >
> > >  &i2c5 {
> > >         status = "okay";
> > > -       clock-frequency = <100000>;
> > > +       clock-frequency = <400000>;
> > >         pinctrl-0 = <&i2c5_pins>;
> > >         pinctrl-names = "default";
> > >
> > >         codec: wm8962@1a {
> > >                 compatible = "wlf,wm8962";
> > >                 reg = <0x1a>;
> > > +               clocks = <&versaclock6_bb 3>;
> > > +               clock-names = "mclk";
> >
> > While the driver does get the (nameless) clock, the DT bindings lack any
> > mention of a clocks property.  It would be good to update the bindings.
>
> Agreed.  I'll push an update to add the clocks property.

Thanks!

> > Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> > arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> > hand) use "xclk" instead of "mclk"?
>
> On the schematics for the two imx boards, it's labeled as xclk, so it
> was named as such.  For this board, the schematic names it mclk. The
> driver doesn't care about the clock-names property, so I'll just
> remove them.

If there's a single clock, not using clock-names is fine.
If you do use clock-names, the names should be clock-centric, not
board-centric.

BTW, looking at the WM8962 datasheet, it's called "MCLK".

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 02/18] arm64: dts: renesas: beacon kit: Fix choppy Bluetooth Audio

[Geert Uytterhoeven]

Hi Adam,

On Thu, Dec 17, 2020 at 1:16 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Dec 17, 2020 at 4:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > The Bluetooth chip is capable of operating at 4Mbps, but the
> > > max-speed setting was on the UART node instead of the Bluetooth
> > > node, so the chip didn't operate at the correct speed resulting
> > > in choppy audio.  Fix this by setting the max-speed in the proper
> > > node.
> > >
> > > Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > i.e. will queue in renesas-devel for v5.12.
>
> Since various other patches in the series need a V2, should this be
> included in the V2 as no-change, or should I skip this and others that
> have been queued?  If/when they appear in your branch, I can rebase
> the series against that branch and just submit V2's on what's missing.
>
> I want to do whatever creates less work for you.

I think it's best to postpone v2 until I have queued up the accepted patches
in renesas-devel.  Probably that will happen on Monday.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Fix Audio Clock sources

[Geert Uytterhoeven]

Hi Adam,

On Thu, Dec 17, 2020 at 1:01 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Dec 17, 2020 at 4:54 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > The SoC was expecting two clock sources with different frequencies.
> > > One to support 44.1KHz and one to support 48KHz.  With the newly added
> > > ability to configure the programmably clock, configure both clocks.
> > >
> > > Beacause the SoC is expecting a fixed clock/oscillator, it doesn't
> > > attempt to get and enable the clock for audio_clk_a. The choice to
> > > use a fixed-factor-clock was due to the fact that it will automatically
> > > enable the programmable clock frequency without change any code.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > @@ -250,9 +250,12 @@ ss_ep: endpoint {
> > >  };
> > >
> > >  &audio_clk_a {
> > > -       clock-frequency = <24576000>;
> > > -       assigned-clocks = <&versaclock6_bb 4>;
> > > -       assigned-clock-rates = <24576000>;
> > > +       /delete-property/ clock-frequency;
> > > +       #clock-cells = <0>;
> > > +       compatible = "fixed-factor-clock";
> > > +       clock-mult = <1>;
> > > +       clock-div = <1>;
> > > +       clocks = <&versaclock6_bb 4>;
> > >  };
> >
> > Shouldn't you override the clocks property in the rcar_sound node
> > instead, like is done in several other board DTS files (with cs2000)?
> >
>
> I guess there are multiple ways to do this.  Because the rcar_sound
> was already expecting a reference to audio_clk_a, it seemed less
> intrusive this way. The way I proposed, we can use the default
> rcar_sound clocking and just change the audio_clk node to enable the
> versaclock output.  The versaclock is driving the audio_clk_a
> reference clock, so it seemed appropriate to put it there.
>
> If you want me to change, I will.

Taking a fresh look at this, I start to like it.
What do other people think?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Geert Uytterhoeven]

Hi Adam,

CC Shimoda-san

On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > When the board was added, clock drivers were being updated done at
> > > > > the same time to allow the versaclock driver to properly configure
> > > > > the modes.  Unforutnately, the updates were not applied to the board
> >
> > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > @@ -5,6 +5,7 @@
> > > > >
> > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > >  #include <dt-bindings/input/input.h>
> > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > >
> > > > >  / {
> > > > >         backlight_lvds: backlight-lvds {
> > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > >  &ehci0 {
> > > > >         dr_mode = "otg";
> > > > >         status = "okay";
> > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > >
> > > > Why this change? You said before you don't need this
> > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > >
> > >
> > > I had talked with the hardware guys about buy pre-programmed
> > > versaclock chips which would have been pre-configured and pre-enabled.
> > > I thought it was going to happen, but it didn't, so we need the
> > > versaclock driver to enable the reference clock for the USB
> > > controllers, ethernet controller and audio clocks.  Previously we were
> > > manually configuring it or it was coincidentally working. Ideally,
> > > we'd have the clock system intentionally enable/disable the clocks
> > > when drivers are loaded/unloaded for for power management reasons.
> >
> > Can you tell me how exactly the Versaclock outputs are wired?
>
> The SoC is expecting a fixed external 50 MHz clock connected to
> USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> We're also using the Versaclock to drive the AVB TXCRefClk,
> du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> RZ/G2N) instead of fixed clocks.
>
> > E.g. for USB, the bindings don't say anything about a third clock input,
> > so I'd like to know where that clock is fed into USB.
>
> The way the driver is crafted, it can take in multiple clocks and it
> goes through a list to enable them all, so I added the versaclock to
> the array.  Without the versaclock reference, the clock doesn't get
> turned on and the USB fails to operate.

According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
while you added the clock references to the EHCI nodes.
Are you sure EHCI is failing without this?

Still, it means we need to extend the bindings/driver for
renesas,rcar-gen3-xhci to handle USB_EXTAL.

> The DU clocks are also expecting an array, so I added the versaclock
> to that array as well.

For DU, the clock inputs are clearly defined in the bindings.

> It's similar to the rationale that I'm trying to add the option clock
> for the AVB TXC_Ref clock on the other path.  We're using the
> versaclock there as well.  The difference is that in the case of the
> AVB_TXCRefClk, the driver isn't expecting an array of clocks, it's
> only expecting a single clock.  In order to enable the additional
> clock,  I started the patch to accept the optional clock for the
> TXCRefClk in order to get the clock system to enable the clock.

Sure.

> Because the Versaclock isn't programmed to automatically start, they
> need the consumers of the clock to request and enable them.
>
> I admit that I'll probably need to update the bindings to add the
> extra clocks as optional, so if you want, I can submit additional
> patches to add these optional clocks to their respective bindings.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 06/18] arm64: dts: renesas: beacon: Configure Audio CODEC clocks

[Adam Ford]

On Fri, Dec 18, 2020 at 6:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Thu, Dec 17, 2020 at 2:33 PM Adam Ford <aford173@gmail.com> wrote:
> > On Thu, Dec 17, 2020 at 5:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > CC alsa-devel
> > >
> > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > With the newly added configurable clock options, the audio CODEC can
> > > > configure the mclk automatically.  Add the reference to the versaclock.
> > > > Since the devices on I2C5 can communicate at 400KHz, let's also increase
> > > > that too
> > > >
> > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > @@ -424,13 +424,15 @@ &i2c0 {
> > > >
> > > >  &i2c5 {
> > > >         status = "okay";
> > > > -       clock-frequency = <100000>;
> > > > +       clock-frequency = <400000>;
> > > >         pinctrl-0 = <&i2c5_pins>;
> > > >         pinctrl-names = "default";
> > > >
> > > >         codec: wm8962@1a {
> > > >                 compatible = "wlf,wm8962";
> > > >                 reg = <0x1a>;
> > > > +               clocks = <&versaclock6_bb 3>;
> > > > +               clock-names = "mclk";
> > >
> > > While the driver does get the (nameless) clock, the DT bindings lack any
> > > mention of a clocks property.  It would be good to update the bindings.
> >
> > Agreed.  I'll push an update to add the clocks property.
>
> Thanks!
>
> > > Note that arch/arm/boot/dts/imx6-logicpd-baseboard.dtsi and
> > > arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi (both by your
> > > hand) use "xclk" instead of "mclk"?
> >
> > On the schematics for the two imx boards, it's labeled as xclk, so it
> > was named as such.  For this board, the schematic names it mclk. The
> > driver doesn't care about the clock-names property, so I'll just
> > remove them.
>
> If there's a single clock, not using clock-names is fine.
> If you do use clock-names, the names should be clock-centric, not
> board-centric.

I already submitted patches to remove the clock-names reference from
the other two boards you noted [1].  I agree it should match  the
driver and not the schematic.  That line was a left-over from our
internal git repo where the decision was used to follow the schematic
and not the driver.

Thanks for bringing that to my attention.

adam

[1] - https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=403739

>
> BTW, looking at the WM8962 datasheet, it's called "MCLK".
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Adam Ford]

On Fri, Dec 18, 2020 at 7:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> CC Shimoda-san
>
> On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > When the board was added, clock drivers were being updated done at
> > > > > > the same time to allow the versaclock driver to properly configure
> > > > > > the modes.  Unforutnately, the updates were not applied to the board
> > >
> > > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > @@ -5,6 +5,7 @@
> > > > > >
> > > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > > >  #include <dt-bindings/input/input.h>
> > > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > > >
> > > > > >  / {
> > > > > >         backlight_lvds: backlight-lvds {
> > > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > > >  &ehci0 {
> > > > > >         dr_mode = "otg";
> > > > > >         status = "okay";
> > > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > > >
> > > > > Why this change? You said before you don't need this
> > > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > > >
> > > >
> > > > I had talked with the hardware guys about buy pre-programmed
> > > > versaclock chips which would have been pre-configured and pre-enabled.
> > > > I thought it was going to happen, but it didn't, so we need the
> > > > versaclock driver to enable the reference clock for the USB
> > > > controllers, ethernet controller and audio clocks.  Previously we were
> > > > manually configuring it or it was coincidentally working. Ideally,
> > > > we'd have the clock system intentionally enable/disable the clocks
> > > > when drivers are loaded/unloaded for for power management reasons.
> > >
> > > Can you tell me how exactly the Versaclock outputs are wired?
> >
> > The SoC is expecting a fixed external 50 MHz clock connected to
> > USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> > We're also using the Versaclock to drive the AVB TXCRefClk,
> > du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> > RZ/G2N) instead of fixed clocks.
> >
> > > E.g. for USB, the bindings don't say anything about a third clock input,
> > > so I'd like to know where that clock is fed into USB.
> >
> > The way the driver is crafted, it can take in multiple clocks and it
> > goes through a list to enable them all, so I added the versaclock to
> > the array.  Without the versaclock reference, the clock doesn't get
> > turned on and the USB fails to operate.
>
> According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
> while you added the clock references to the EHCI nodes.
> Are you sure EHCI is failing without this?
>
> Still, it means we need to extend the bindings/driver for
> renesas,rcar-gen3-xhci to handle USB_EXTAL.

After investigating this, it looks like the USB_EXTAL is already
referenced from the device tree and it's referenced by the USB3 Phy.
The SoC calls it usb_extal_clk.  Since the phy driver is calling
devm_clk_get() it looks like i could just redefine the clocks of
usb3_phy0 to point to the versaclock instead of usb_extal_clk.

The other option is to use a similar method I proposed for the audio
reference clock and redefine the usb_extal_clk as a fixed
fixed-factor-clock.

Do you have a preference as to which direction I go?

>
> > The DU clocks are also expecting an array, so I added the versaclock
> > to that array as well.
>
> For DU, the clock inputs are clearly defined in the bindings.
>
> > It's similar to the rationale that I'm trying to add the option clock
> > for the AVB TXC_Ref clock on the other path.  We're using the
> > versaclock there as well.  The difference is that in the case of the
> > AVB_TXCRefClk, the driver isn't expecting an array of clocks, it's
> > only expecting a single clock.  In order to enable the additional
> > clock,  I started the patch to accept the optional clock for the
> > TXCRefClk in order to get the clock system to enable the clock.
>
> Sure.
>
> > Because the Versaclock isn't programmed to automatically start, they
> > need the consumers of the clock to request and enable them.
> >
> > I admit that I'll probably need to update the bindings to add the
> > extra clocks as optional, so if you want, I can submit additional
> > patches to add these optional clocks to their respective bindings.
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Geert Uytterhoeven]

Hi Adam,

On Tue, Dec 22, 2020 at 2:39 AM Adam Ford <aford173@gmail.com> wrote:
> On Fri, Dec 18, 2020 at 7:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > When the board was added, clock drivers were being updated done at
> > > > > > > the same time to allow the versaclock driver to properly configure
> > > > > > > the modes.  Unforutnately, the updates were not applied to the board
> > > >
> > > > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > @@ -5,6 +5,7 @@
> > > > > > >
> > > > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > > > >  #include <dt-bindings/input/input.h>
> > > > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > > > >
> > > > > > >  / {
> > > > > > >         backlight_lvds: backlight-lvds {
> > > > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > > > >  &ehci0 {
> > > > > > >         dr_mode = "otg";
> > > > > > >         status = "okay";
> > > > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > > > >
> > > > > > Why this change? You said before you don't need this
> > > > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > > > >
> > > > >
> > > > > I had talked with the hardware guys about buy pre-programmed
> > > > > versaclock chips which would have been pre-configured and pre-enabled.
> > > > > I thought it was going to happen, but it didn't, so we need the
> > > > > versaclock driver to enable the reference clock for the USB
> > > > > controllers, ethernet controller and audio clocks.  Previously we were
> > > > > manually configuring it or it was coincidentally working. Ideally,
> > > > > we'd have the clock system intentionally enable/disable the clocks
> > > > > when drivers are loaded/unloaded for for power management reasons.
> > > >
> > > > Can you tell me how exactly the Versaclock outputs are wired?
> > >
> > > The SoC is expecting a fixed external 50 MHz clock connected to
> > > USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> > > We're also using the Versaclock to drive the AVB TXCRefClk,
> > > du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> > > RZ/G2N) instead of fixed clocks.
> > >
> > > > E.g. for USB, the bindings don't say anything about a third clock input,
> > > > so I'd like to know where that clock is fed into USB.
> > >
> > > The way the driver is crafted, it can take in multiple clocks and it
> > > goes through a list to enable them all, so I added the versaclock to
> > > the array.  Without the versaclock reference, the clock doesn't get
> > > turned on and the USB fails to operate.
> >
> > According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
> > while you added the clock references to the EHCI nodes.
> > Are you sure EHCI is failing without this?
> >
> > Still, it means we need to extend the bindings/driver for
> > renesas,rcar-gen3-xhci to handle USB_EXTAL.
>
> After investigating this, it looks like the USB_EXTAL is already
> referenced from the device tree and it's referenced by the USB3 Phy.
> The SoC calls it usb_extal_clk.  Since the phy driver is calling
> devm_clk_get() it looks like i could just redefine the clocks of
> usb3_phy0 to point to the versaclock instead of usb_extal_clk.
>
> The other option is to use a similar method I proposed for the audio
> reference clock and redefine the usb_extal_clk as a fixed
> fixed-factor-clock.
>
> Do you have a preference as to which direction I go?

I'd go for the classical solution: override the clocks property of the
usb3_phy0 node.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 17/18] arm64: dts: renesas: Introduce r8a774b1-beacon-rzg2n-kit

[Adam Ford]

On Thu, Dec 17, 2020 at 5:49 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > Beacon EmebeddedWorks is introducing a new kit based on the
> > RZ/G2N SoC from Renesas.
> >
> > The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1
> > cellular radio.
> >
> > The Baseboard has Ethernet, USB, HDMI, stereo audio in and out,
> > along with a variety of push buttons and LED's, and support for
> > a parallel RGB and an LVDS display.  It uses the same baseboard
> > and SOM as the RZ/G2M.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!

Thank you for the review!

>
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r8a774b1-beacon-rzg2n-kit.dts
> > @@ -0,0 +1,43 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2020, Compass Electronics Group, LLC
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "r8a774b1.dtsi"
> > +#include "beacon-renesom-som.dtsi"
> > +#include "beacon-renesom-baseboard.dtsi"
> > +
> > +/ {
> > +       model = "Beacon Embedded Works RZ/G2N Development Kit";
> > +       compatible =    "beacon,beacon-rzg2n", "renesas,r8a774b1";
> > +
> > +       aliases {
> > +               serial0 = &scif2;
> > +               serial1 = &hscif0;
> > +               serial2 = &hscif1;
> > +               serial3 = &scif0;
> > +               serial4 = &hscif2;
> > +               serial5 = &scif5;
> > +               serial6 = &scif4;
> > +               ethernet0 = &avb;
> > +       };
> > +
> > +       chosen {
> > +               stdout-path = "serial0:115200n8";
> > +       };
>
> No memory nodes? Are you relying on U-Boot to fill them in?
> If yes, why do you have them in the other board DTS files?

There is already a memory node included in the beacon-renesom.dtsi
file which is defining memory@48000000 node which is common to the M,
N and H.   If I understand it correctly, the extra memory nodes
defined in the higher-level dts for M and H variation are not
applicable on the N.

>
> > +};
> > +
> > +&du {
> > +       status = "okay";
>
> Missing pinctrl properties?
>
oops.  I'll fix.
> > +
> > +       clocks = <&cpg CPG_MOD 724>,
> > +               <&cpg CPG_MOD 723>,
> > +               <&cpg CPG_MOD 721>,
> > +               <&versaclock5 1>,
> > +               <&x302_clk>,
> > +               <&versaclock5 2>;
> > +       clock-names = "du.0", "du.1", "du.3",
> > +               "dclkin.0", "dclkin.1", "dclkin.3";
> > +};
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 17/18] arm64: dts: renesas: Introduce r8a774b1-beacon-rzg2n-kit

[Biju Das]

Hi Adam,

> Subject: Re: [PATCH 17/18] arm64: dts: renesas: Introduce r8a774b1-beacon-
> rzg2n-kit
> 
> On Thu, Dec 17, 2020 at 5:49 AM Geert Uytterhoeven <geert@linux-m68k.org>
> wrote:
> >
> > Hi Adam,
> >
> > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > Beacon EmebeddedWorks is introducing a new kit based on the RZ/G2N
> > > SoC from Renesas.
> > >
> > > The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1
> > > cellular radio.
> > >
> > > The Baseboard has Ethernet, USB, HDMI, stereo audio in and out,
> > > along with a variety of push buttons and LED's, and support for a
> > > parallel RGB and an LVDS display.  It uses the same baseboard and
> > > SOM as the RZ/G2M.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> 
> Thank you for the review!
> 
> >
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/renesas/r8a774b1-beacon-rzg2n-kit.dts
> > > @@ -0,0 +1,43 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright 2020, Compass Electronics Group, LLC  */
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include "r8a774b1.dtsi"
> > > +#include "beacon-renesom-som.dtsi"
> > > +#include "beacon-renesom-baseboard.dtsi"
> > > +
> > > +/ {
> > > +       model = "Beacon Embedded Works RZ/G2N Development Kit";
> > > +       compatible =    "beacon,beacon-rzg2n", "renesas,r8a774b1";
> > > +
> > > +       aliases {
> > > +               serial0 = &scif2;
> > > +               serial1 = &hscif0;
> > > +               serial2 = &hscif1;
> > > +               serial3 = &scif0;
> > > +               serial4 = &hscif2;
> > > +               serial5 = &scif5;
> > > +               serial6 = &scif4;
> > > +               ethernet0 = &avb;
> > > +       };
> > > +
> > > +       chosen {
> > > +               stdout-path = "serial0:115200n8";
> > > +       };
> >
> > No memory nodes? Are you relying on U-Boot to fill them in?
> > If yes, why do you have them in the other board DTS files?
> 
> There is already a memory node included in the beacon-renesom.dtsi file
> which is defining memory@48000000 node which is common to the M,
> N and H.   If I understand it correctly, the extra memory nodes
> defined in the higher-level dts for M and H variation are not applicable
> on the N.

If I am correct, It is not applicable, if it has only 2GB memory for the N variant.

But If it has 4GB memory(1ch x4GB), then the memory is split between 32bit space(first 2GB) and 64 bit space(remaining 2GB). 
In this case you need to define extra DT node for memory in 64bit space.

Cheers,
Biju

Re: [PATCH 01/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Adam Ford]

On Tue, Dec 22, 2020 at 2:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Tue, Dec 22, 2020 at 2:39 AM Adam Ford <aford173@gmail.com> wrote:
> > On Fri, Dec 18, 2020 at 7:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > > On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > When the board was added, clock drivers were being updated done at
> > > > > > > > the same time to allow the versaclock driver to properly configure
> > > > > > > > the modes.  Unforutnately, the updates were not applied to the board
> > > > >
> > > > > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > @@ -5,6 +5,7 @@
> > > > > > > >
> > > > > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > > > > >  #include <dt-bindings/input/input.h>
> > > > > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > > > > >
> > > > > > > >  / {
> > > > > > > >         backlight_lvds: backlight-lvds {
> > > > > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > > > > >  &ehci0 {
> > > > > > > >         dr_mode = "otg";
> > > > > > > >         status = "okay";
> > > > > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > > > > >
> > > > > > > Why this change? You said before you don't need this
> > > > > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > > > > >
> > > > > >
> > > > > > I had talked with the hardware guys about buy pre-programmed
> > > > > > versaclock chips which would have been pre-configured and pre-enabled.
> > > > > > I thought it was going to happen, but it didn't, so we need the
> > > > > > versaclock driver to enable the reference clock for the USB
> > > > > > controllers, ethernet controller and audio clocks.  Previously we were
> > > > > > manually configuring it or it was coincidentally working. Ideally,
> > > > > > we'd have the clock system intentionally enable/disable the clocks
> > > > > > when drivers are loaded/unloaded for for power management reasons.
> > > > >
> > > > > Can you tell me how exactly the Versaclock outputs are wired?
> > > >
> > > > The SoC is expecting a fixed external 50 MHz clock connected to
> > > > USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> > > > We're also using the Versaclock to drive the AVB TXCRefClk,
> > > > du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> > > > RZ/G2N) instead of fixed clocks.
> > > >
> > > > > E.g. for USB, the bindings don't say anything about a third clock input,
> > > > > so I'd like to know where that clock is fed into USB.
> > > >
> > > > The way the driver is crafted, it can take in multiple clocks and it
> > > > goes through a list to enable them all, so I added the versaclock to
> > > > the array.  Without the versaclock reference, the clock doesn't get
> > > > turned on and the USB fails to operate.
> > >
> > > According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
> > > while you added the clock references to the EHCI nodes.
> > > Are you sure EHCI is failing without this?

Geert,

I talked to a colleague about the USB_EXTAL.  He pointed me to table
60.1 of the RZ/2 Series, 2nd Generate reference manual
(R01UH0808EJ0100 Rev.1.00),  which shows the USB EHCI needing the
50MHz.  When I clear out the references from ehci0 and echi1, the USB
stops working, so it does appear that using the versaclock as the 3rd
clock is needed for operating.  The device tree bindings for the
generic-ehci provide for up to 4 clocks, so it seems like referencing
clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3> are
not a violation of the bindings.

I can add a better description when I do the V2 update for this if you like.


> > > Still, it means we need to extend the bindings/driver for
> > > renesas,rcar-gen3-xhci to handle USB_EXTAL.
> >
> > After investigating this, it looks like the USB_EXTAL is already
> > referenced from the device tree and it's referenced by the USB3 Phy.
> > The SoC calls it usb_extal_clk.  Since the phy driver is calling
> > devm_clk_get() it looks like i could just redefine the clocks of
> > usb3_phy0 to point to the versaclock instead of usb_extal_clk.
> >
> > The other option is to use a similar method I proposed for the audio
> > reference clock and redefine the usb_extal_clk as a fixed
> > fixed-factor-clock.
> >
> > Do you have a preference as to which direction I go?
>
> I'd go for the classical solution: override the clocks property of the
> usb3_phy0 node.

I dug into the USB3_phy and it enables and immediately disables the
clocks for the simple purpose of determining which clock reference to
use between usb3s0_clk or usb_extal_clk.  I was hoping that simply
referencing the versaclock here would be sufficient, but the Beacon
board has a usb3s0_clk at 100MHz, and the driver appears to use it
instead of the versaclock so adding the versaclock reference here
isn't sufficient to make it work for the ehci, nor do I think it's
appropriate.

It seems like the driver shouldn't immediately disable the clocks, but
they're expecting external fixed clocks.  Since we meet that criteria
with the usb3s0_clk, the USB3 works without the versaclock reference.

adam
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Geert Uytterhoeven]

Hi Adam,

On Thu, Dec 24, 2020 at 2:53 PM Adam Ford <aford173@gmail.com> wrote:
> On Tue, Dec 22, 2020 at 2:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Dec 22, 2020 at 2:39 AM Adam Ford <aford173@gmail.com> wrote:
> > > On Fri, Dec 18, 2020 at 7:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > > When the board was added, clock drivers were being updated done at
> > > > > > > > > the same time to allow the versaclock driver to properly configure
> > > > > > > > > the modes.  Unforutnately, the updates were not applied to the board
> > > > > >
> > > > > > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > @@ -5,6 +5,7 @@
> > > > > > > > >
> > > > > > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > > > > > >  #include <dt-bindings/input/input.h>
> > > > > > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > > > > > >
> > > > > > > > >  / {
> > > > > > > > >         backlight_lvds: backlight-lvds {
> > > > > > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > > > > > >  &ehci0 {
> > > > > > > > >         dr_mode = "otg";
> > > > > > > > >         status = "okay";
> > > > > > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > > > > > >
> > > > > > > > Why this change? You said before you don't need this
> > > > > > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > > > > > >
> > > > > > >
> > > > > > > I had talked with the hardware guys about buy pre-programmed
> > > > > > > versaclock chips which would have been pre-configured and pre-enabled.
> > > > > > > I thought it was going to happen, but it didn't, so we need the
> > > > > > > versaclock driver to enable the reference clock for the USB
> > > > > > > controllers, ethernet controller and audio clocks.  Previously we were
> > > > > > > manually configuring it or it was coincidentally working. Ideally,
> > > > > > > we'd have the clock system intentionally enable/disable the clocks
> > > > > > > when drivers are loaded/unloaded for for power management reasons.
> > > > > >
> > > > > > Can you tell me how exactly the Versaclock outputs are wired?
> > > > >
> > > > > The SoC is expecting a fixed external 50 MHz clock connected to
> > > > > USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> > > > > We're also using the Versaclock to drive the AVB TXCRefClk,
> > > > > du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> > > > > RZ/G2N) instead of fixed clocks.
> > > > >
> > > > > > E.g. for USB, the bindings don't say anything about a third clock input,
> > > > > > so I'd like to know where that clock is fed into USB.
> > > > >
> > > > > The way the driver is crafted, it can take in multiple clocks and it
> > > > > goes through a list to enable them all, so I added the versaclock to
> > > > > the array.  Without the versaclock reference, the clock doesn't get
> > > > > turned on and the USB fails to operate.
> > > >
> > > > According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
> > > > while you added the clock references to the EHCI nodes.
> > > > Are you sure EHCI is failing without this?
>
> I talked to a colleague about the USB_EXTAL.  He pointed me to table
> 60.1 of the RZ/2 Series, 2nd Generate reference manual
> (R01UH0808EJ0100 Rev.1.00),  which shows the USB EHCI needing the
> 50MHz.  When I clear out the references from ehci0 and echi1, the USB
> stops working, so it does appear that using the versaclock as the 3rd
> clock is needed for operating.  The device tree bindings for the
> generic-ehci provide for up to 4 clocks, so it seems like referencing
> clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3> are
> not a violation of the bindings.

Perhaps you need to use renesas,rcar-usb2-clock-sel?
Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.yaml

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Adam Ford]

On Mon, Dec 28, 2020 at 6:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Thu, Dec 24, 2020 at 2:53 PM Adam Ford <aford173@gmail.com> wrote:
> > On Tue, Dec 22, 2020 at 2:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, Dec 22, 2020 at 2:39 AM Adam Ford <aford173@gmail.com> wrote:
> > > > On Fri, Dec 18, 2020 at 7:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > > > When the board was added, clock drivers were being updated done at
> > > > > > > > > > the same time to allow the versaclock driver to properly configure
> > > > > > > > > > the modes.  Unforutnately, the updates were not applied to the board
> > > > > > >
> > > > > > > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > > @@ -5,6 +5,7 @@
> > > > > > > > > >
> > > > > > > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > >  #include <dt-bindings/input/input.h>
> > > > > > > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > > > > > > >
> > > > > > > > > >  / {
> > > > > > > > > >         backlight_lvds: backlight-lvds {
> > > > > > > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > > > > > > >  &ehci0 {
> > > > > > > > > >         dr_mode = "otg";
> > > > > > > > > >         status = "okay";
> > > > > > > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > > > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > > > > > > >
> > > > > > > > > Why this change? You said before you don't need this
> > > > > > > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > > > > > > >
> > > > > > > >
> > > > > > > > I had talked with the hardware guys about buy pre-programmed
> > > > > > > > versaclock chips which would have been pre-configured and pre-enabled.
> > > > > > > > I thought it was going to happen, but it didn't, so we need the
> > > > > > > > versaclock driver to enable the reference clock for the USB
> > > > > > > > controllers, ethernet controller and audio clocks.  Previously we were
> > > > > > > > manually configuring it or it was coincidentally working. Ideally,
> > > > > > > > we'd have the clock system intentionally enable/disable the clocks
> > > > > > > > when drivers are loaded/unloaded for for power management reasons.
> > > > > > >
> > > > > > > Can you tell me how exactly the Versaclock outputs are wired?
> > > > > >
> > > > > > The SoC is expecting a fixed external 50 MHz clock connected to
> > > > > > USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> > > > > > We're also using the Versaclock to drive the AVB TXCRefClk,
> > > > > > du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> > > > > > RZ/G2N) instead of fixed clocks.
> > > > > >
> > > > > > > E.g. for USB, the bindings don't say anything about a third clock input,
> > > > > > > so I'd like to know where that clock is fed into USB.
> > > > > >
> > > > > > The way the driver is crafted, it can take in multiple clocks and it
> > > > > > goes through a list to enable them all, so I added the versaclock to
> > > > > > the array.  Without the versaclock reference, the clock doesn't get
> > > > > > turned on and the USB fails to operate.
> > > > >
> > > > > According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
> > > > > while you added the clock references to the EHCI nodes.
> > > > > Are you sure EHCI is failing without this?
> >
> > I talked to a colleague about the USB_EXTAL.  He pointed me to table
> > 60.1 of the RZ/2 Series, 2nd Generate reference manual
> > (R01UH0808EJ0100 Rev.1.00),  which shows the USB EHCI needing the
> > 50MHz.  When I clear out the references from ehci0 and echi1, the USB
> > stops working, so it does appear that using the versaclock as the 3rd
> > clock is needed for operating.  The device tree bindings for the
> > generic-ehci provide for up to 4 clocks, so it seems like referencing
> > clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3> are
> > not a violation of the bindings.
>
> Perhaps you need to use renesas,rcar-usb2-clock-sel?
> Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.yaml

Thanks for the pointer. I didn't know this existed.  It looks like the
right thing to do.  With that node, it appears to enable the
versaclock and USB works.
I'll submit a V3 at some point with this node added to each of the
kit-level files since they use slightly different power-domains.

Do I need to add updates to the bindings for
renesas,r8a774a1-rcar-usb2-clock-sel; r8a774b1-rcar-usb2-clock-sel;
and renesas,r8a774e1-rcar-usb2-clock-sel; or I can I just use the
generic reference to renesas,rcar-gen3-usb2-clock-sel?

adam
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Geert Uytterhoeven]

Hi Adam,

On Mon, Dec 28, 2020 at 3:39 PM Adam Ford <aford173@gmail.com> wrote:
> On Mon, Dec 28, 2020 at 6:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Dec 24, 2020 at 2:53 PM Adam Ford <aford173@gmail.com> wrote:
> > > On Tue, Dec 22, 2020 at 2:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Tue, Dec 22, 2020 at 2:39 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > On Fri, Dec 18, 2020 at 7:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > > > > When the board was added, clock drivers were being updated done at
> > > > > > > > > > > the same time to allow the versaclock driver to properly configure
> > > > > > > > > > > the modes.  Unforutnately, the updates were not applied to the board
> > > > > > > >
> > > > > > > > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > > > @@ -5,6 +5,7 @@
> > > > > > > > > > >
> > > > > > > > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > >  #include <dt-bindings/input/input.h>
> > > > > > > > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > > > > > > > >
> > > > > > > > > > >  / {
> > > > > > > > > > >         backlight_lvds: backlight-lvds {
> > > > > > > > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > > > > > > > >  &ehci0 {
> > > > > > > > > > >         dr_mode = "otg";
> > > > > > > > > > >         status = "okay";
> > > > > > > > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > > > > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > > > > > > > >
> > > > > > > > > > Why this change? You said before you don't need this
> > > > > > > > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I had talked with the hardware guys about buy pre-programmed
> > > > > > > > > versaclock chips which would have been pre-configured and pre-enabled.
> > > > > > > > > I thought it was going to happen, but it didn't, so we need the
> > > > > > > > > versaclock driver to enable the reference clock for the USB
> > > > > > > > > controllers, ethernet controller and audio clocks.  Previously we were
> > > > > > > > > manually configuring it or it was coincidentally working. Ideally,
> > > > > > > > > we'd have the clock system intentionally enable/disable the clocks
> > > > > > > > > when drivers are loaded/unloaded for for power management reasons.
> > > > > > > >
> > > > > > > > Can you tell me how exactly the Versaclock outputs are wired?
> > > > > > >
> > > > > > > The SoC is expecting a fixed external 50 MHz clock connected to
> > > > > > > USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> > > > > > > We're also using the Versaclock to drive the AVB TXCRefClk,
> > > > > > > du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> > > > > > > RZ/G2N) instead of fixed clocks.
> > > > > > >
> > > > > > > > E.g. for USB, the bindings don't say anything about a third clock input,
> > > > > > > > so I'd like to know where that clock is fed into USB.
> > > > > > >
> > > > > > > The way the driver is crafted, it can take in multiple clocks and it
> > > > > > > goes through a list to enable them all, so I added the versaclock to
> > > > > > > the array.  Without the versaclock reference, the clock doesn't get
> > > > > > > turned on and the USB fails to operate.
> > > > > >
> > > > > > According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
> > > > > > while you added the clock references to the EHCI nodes.
> > > > > > Are you sure EHCI is failing without this?
> > >
> > > I talked to a colleague about the USB_EXTAL.  He pointed me to table
> > > 60.1 of the RZ/2 Series, 2nd Generate reference manual
> > > (R01UH0808EJ0100 Rev.1.00),  which shows the USB EHCI needing the
> > > 50MHz.  When I clear out the references from ehci0 and echi1, the USB
> > > stops working, so it does appear that using the versaclock as the 3rd
> > > clock is needed for operating.  The device tree bindings for the
> > > generic-ehci provide for up to 4 clocks, so it seems like referencing
> > > clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3> are
> > > not a violation of the bindings.
> >
> > Perhaps you need to use renesas,rcar-usb2-clock-sel?
> > Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.yaml
>
> Thanks for the pointer. I didn't know this existed.  It looks like the
> right thing to do.  With that node, it appears to enable the
> versaclock and USB works.
> I'll submit a V3 at some point with this node added to each of the
> kit-level files since they use slightly different power-domains.
>
> Do I need to add updates to the bindings for
> renesas,r8a774a1-rcar-usb2-clock-sel; r8a774b1-rcar-usb2-clock-sel;
> and renesas,r8a774e1-rcar-usb2-clock-sel; or I can I just use the
> generic reference to renesas,rcar-gen3-usb2-clock-sel?

Please update the bindings to add support for RZ/G1[MNH].
Note that without doing so, checkpatch will complain when adding the
device nodes to the .dtsi files.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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/18] arm64: dts: renesas: beacon kit: Configure programmable clocks

[Adam Ford]

On Mon, Dec 28, 2020 at 6:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Thu, Dec 24, 2020 at 2:53 PM Adam Ford <aford173@gmail.com> wrote:
> > On Tue, Dec 22, 2020 at 2:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, Dec 22, 2020 at 2:39 AM Adam Ford <aford173@gmail.com> wrote:
> > > > On Fri, Dec 18, 2020 at 7:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Thu, Dec 17, 2020 at 12:52 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > On Thu, Dec 17, 2020 at 2:16 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > On Wed, Dec 16, 2020 at 6:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > On Wed, Dec 16, 2020 at 8:55 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > > > > > On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > > > When the board was added, clock drivers were being updated done at
> > > > > > > > > > the same time to allow the versaclock driver to properly configure
> > > > > > > > > > the modes.  Unforutnately, the updates were not applied to the board
> > > > > > >
> > > > > > > > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > > > > > > > @@ -5,6 +5,7 @@
> > > > > > > > > >
> > > > > > > > > >  #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > >  #include <dt-bindings/input/input.h>
> > > > > > > > > > +#include <dt-bindings/clk/versaclock.h>
> > > > > > > > > >
> > > > > > > > > >  / {
> > > > > > > > > >         backlight_lvds: backlight-lvds {
> > > > > > > > > > @@ -294,12 +295,12 @@ &du_out_rgb {
> > > > > > > > > >  &ehci0 {
> > > > > > > > > >         dr_mode = "otg";
> > > > > > > > > >         status = "okay";
> > > > > > > > > > -       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
> > > > > > > > > > +       clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
> > > > > > > > >
> > > > > > > > > Why this change? You said before you don't need this
> > > > > > > > > https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
> > > > > > > > >
> > > > > > > >
> > > > > > > > I had talked with the hardware guys about buy pre-programmed
> > > > > > > > versaclock chips which would have been pre-configured and pre-enabled.
> > > > > > > > I thought it was going to happen, but it didn't, so we need the
> > > > > > > > versaclock driver to enable the reference clock for the USB
> > > > > > > > controllers, ethernet controller and audio clocks.  Previously we were
> > > > > > > > manually configuring it or it was coincidentally working. Ideally,
> > > > > > > > we'd have the clock system intentionally enable/disable the clocks
> > > > > > > > when drivers are loaded/unloaded for for power management reasons.
> > > > > > >
> > > > > > > Can you tell me how exactly the Versaclock outputs are wired?
> > > > > >
> > > > > > The SoC is expecting a fixed external 50 MHz clock connected to
> > > > > > USB_EXTAL.  Instead of a fixed clock, we're using the Versaclock.
> > > > > > We're also using the Versaclock to drive the AVB TXCRefClk,
> > > > > > du_dotclkiun0 and du_dotclkin2 (also also called du_dotclkin3 on
> > > > > > RZ/G2N) instead of fixed clocks.
> > > > > >
> > > > > > > E.g. for USB, the bindings don't say anything about a third clock input,
> > > > > > > so I'd like to know where that clock is fed into USB.
> > > > > >
> > > > > > The way the driver is crafted, it can take in multiple clocks and it
> > > > > > goes through a list to enable them all, so I added the versaclock to
> > > > > > the array.  Without the versaclock reference, the clock doesn't get
> > > > > > turned on and the USB fails to operate.
> > > > >
> > > > > According to the Hardware User's Manual, USBL_EXTAL is used for USB3.0,
> > > > > while you added the clock references to the EHCI nodes.
> > > > > Are you sure EHCI is failing without this?
> >
> > I talked to a colleague about the USB_EXTAL.  He pointed me to table
> > 60.1 of the RZ/2 Series, 2nd Generate reference manual
> > (R01UH0808EJ0100 Rev.1.00),  which shows the USB EHCI needing the
> > 50MHz.  When I clear out the references from ehci0 and echi1, the USB
> > stops working, so it does appear that using the versaclock as the 3rd
> > clock is needed for operating.  The device tree bindings for the
> > generic-ehci provide for up to 4 clocks, so it seems like referencing
> > clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3> are
> > not a violation of the bindings.
>
> Perhaps you need to use renesas,rcar-usb2-clock-sel?
> Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.yaml
>

Geert,

Sorry to resurrect an old thread, but I've been working with a
colleague on this, but we've had a lot of interruptions, and we're
just now getting back to this.

Based on our previous conversations, you didn’t want me to add a
reference clock to the EHCI node, because you wanted us to use the
rcar-usb2-clock-sel driver.
If I just add the node for the rcar-usb2-clock-sel  that references
the versaclock, the clock tree shows it’s present, but neither the
rcar-usb2-clock-sel nor the versaclock are actually enabled.  From
what we’re seeing, the rcar-usb2-clock-sel driver needs a consumer in
order for it to activate.

It seems like it makes sense to optionally associate the
rcar-usb2-clock-sel to all USB nodes, including the USB3 node. The
EHCI controller section in the UG calls out USB_XTAL/USB_EXTAL as
external pins as well as the USBHS module calling out the same pins in
its overview section.  The USB3 Phy section mentions
USB_XTAL/USB_EXTAL, but for some reason the USB3 controller overview
does not mention them as “external pins”

I’d like to propose that we add an optional reference clock for the
USB3 which can point to the rcar-usb2-clock-sel.
On the USB EHCI nodes where I previously wanted to reference the
versaclock, I’d like to reference the rcar-usb2-clock-sel.

The clock tree would look something like:
     Versaclock-> rcar-usb2-clock-sel->USB

The EHCI clocks would look like:
 clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&usb2_clksel>

If we do it this way, we’d need to modify the rcar-usb2-clock-sel to
enable the external versaclock and keep it enabled.  Currently, it
enables the clock, reads the clock speed and shuts down after
determining the clock speed.

An alternative to modifying the rcar-usb2-clock-sel code would be to
add both usb2_clksel and the versaclock reference to the EHCI nodes,
but I know you were not completely satisfied with that idea, but it
would likely not require any code changes.

Versaclock-> rcar-usb2-clock-sel->USB<-versaclock

The ECHI clocks would like:
clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>,
<&usb2_clksel>

Before I move forward on writing this code, I'd like to make sure
you're OK with one of those options , since there are a few ways to do
it.  If you have another suggestion, I'm willing to do that instead.

adam
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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