ARM: dts: s5pv210: Cleanup - dtschema warnings

Add compatible for the Samsung S5Pv210 SoC PMU.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 Documentation/devicetree/bindings/arm/samsung/pmu.yaml | 2 ++

The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch screen. In such case the second interrupt is required. This second interrupt can be anyway provided, even without touch screens. This fixes dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 .../bindings/iio/adc/samsung,exynos-adc.yaml      | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +14 -1

index cc3c8ea6a894..89b4f9c252a6 100644
@@ -41,7 +41,10 @@ properties:
     maxItems: 2
 
   interrupts:
-    maxItems: 1
+    description:
+      ADC interrupt followed by optional touchscreen interrupt.
+    minItems: 1
+    maxItems: 2
 
   "#io-channel-cells":
     const: 1

@@ -107,6 +110,16 @@ allOf:
           items:
             - const: adc
 
+  - if:
+      properties:
+        has-touchscreen:
+          true
+    then:
+      properties:
+        interrupts:
+          minItems: 2
+          maxItems: 2
+
 examples:
   - |
     adc: adc@12d10000 {

Quoted 7 lines · On Mon, 7 Sep 2020 18:11:18 +0200 Krzysztof Kozlowski <[email protected]> wrote:

The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch screen. In such case the second interrupt is required. This second interrupt can be anyway provided, even without touch screens. This fixes dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long

Quoted 1 line

Signed-off-by Krzysztof Kozlowski <[email protected]>

Acked-by Jonathan Cameron <[email protected]>

Or I can pick this up through the IIO tree if that makes sense. I doubt anything else will touch this binding this cycle, so either way works for me.

The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch screen. In such case the second interrupt is required. This second interrupt can be anyway provided, even without touch screens. This fixes dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 .../bindings/iio/adc/samsung,exynos-adc.yaml      | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch screen. In such case the second interrupt is required. This second interrupt can be anyway provided, even without touch screens. This fixes dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 .../bindings/iio/adc/samsung,exynos-adc.yaml      | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +4 -1

index cc3c8ea6a894..89b4f9c252a6 100644
@@ -41,7 +41,10 @@ properties:
     maxItems: 2
 
   interrupts:
-    maxItems: 1
+    description:
+      ADC interrupt followed by optional touchscreen interrupt.
+    minItems: 1
+    maxItems: 2
 
   "#io-channel-cells":
     const: 1

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +6 -0

+    then:
+      properties:
+        interrupts:
+          minItems: 2
+          maxItems: 2
+
 examples:
   - |
     adc: adc@12d10000 {

Signature · 1 line

2.17.1

Quoted 32 lines · Krzysztof Kozlowski

The ADC in S3C/S5P/Exynos SoCs can be used also for handling touch screen. In such case the second interrupt is required. This second interrupt can be anyway provided, even without touch screens. This fixes dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-aquila.dt.yaml: adc@e1700000: interrupts: [[23], [24]] is too long

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 .../bindings/iio/adc/samsung,exynos-adc.yaml      | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +4 -1

index cc3c8ea6a894..89b4f9c252a6 100644
@@ -41,7 +41,10 @@ properties:
     maxItems: 2
 
   interrupts:
-    maxItems: 1
+    description:
+      ADC interrupt followed by optional touchscreen interrupt.
+    minItems: 1
+    maxItems: 2
 
   "#io-channel-cells":
     const: 1

@@ -107,6 +110,16 @@ allOf:
           items:
             - const: adc
 

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml allOf: · line 116

Quoted 4 lines

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +4 -0

+  - if:
+      properties:
+        has-touchscreen:
+          true

This evaluates as true if 'has-touchscreen' is not present too. You should use 'required' here.

The ADC in S5Pv210 does not have ADC phy registers in separate block for which syscon would be needed. Remove this requirement to fix dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: adc@e1700000: 'samsung,syscon-phandle' is a required property

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 -
 1 file changed, 1 deletion(-)

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +0 -1

index 89b4f9c252a6..75174af72288 100644
@@ -81,7 +81,6 @@ allOf:
               - samsung,exynos-adc-v2
               - samsung,exynos3250-adc
               - samsung,exynos4212-adc
-              - samsung,s5pv210-adc
     then:
       required:
         - samsung,syscon-phandle

The ADC in S5Pv210 does not have ADC phy registers in separate block for which syscon would be needed. Remove this requirement to fix dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: adc@e1700000: 'samsung,syscon-phandle' is a required property

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 -
 1 file changed, 1 deletion(-)

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +0 -1

index 89b4f9c252a6..75174af72288 100644
@@ -81,7 +81,6 @@ allOf:
               - samsung,exynos-adc-v2
               - samsung,exynos3250-adc
               - samsung,exynos4212-adc
-              - samsung,s5pv210-adc
     then:
       required:
         - samsung,syscon-phandle

Looking at this again, it appears that there is actually control for it at offset 0x6818 of pmu_syscon (0xe0108000) [1]. However, it defaults to enabled so it's not required for proper use of the block. Whether it should be present in the schema/DTS is up to you.

Thanks,
Jonathan

[1] https://android.googlesource.com/kernel/samsung/+/refs/heads/android-samsung-3.0-jb-mr0/arch/arm/mach-s5pv210/include/mach/regs-clock.h#325

Quoted 24 lines · Krzysztof Kozlowski

The ADC in S5Pv210 does not have ADC phy registers in separate block for which syscon would be needed. Remove this requirement to fix dtbs_check warnings like:

arch/arm/boot/dts/s5pv210-fascinate4g.dt.yaml: adc@e1700000: 'samsung,syscon-phandle' is a required property

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 -
 1 file changed, 1 deletion(-)

Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml +0 -1

index 89b4f9c252a6..75174af72288 100644
@@ -81,7 +81,6 @@ allOf:
               - samsung,exynos-adc-v2
               - samsung,exynos3250-adc
               - samsung,exynos4212-adc
-              - samsung,s5pv210-adc
     then:
       required:
         - samsung,syscon-phandle

Fix typo in pinctrl property of "vibrator-en" fixed regulator in Aries family of boards. The error caused lack of pin configuration for the GPIO used in vibrator.

Fixes 04568cb58a43 ("ARM: dts: s5pv210: Disable pull for vibrator enable GPIO on Aries boards")

Cc <[email protected]>

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

There is no need to keep DMA controller nodes under AMBA bus node. Remove the "amba" node to fix dtschema warnings like:

amba: $nodename:0: 'amba' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210.dtsi | 49 +++++++++++++++-------------------
 1 file changed, 21 insertions(+), 28 deletions(-)

arch/arm/boot/dts/s5pv210.dtsi +21 -28

index 1b0ee884e91d..84e4447931de 100644
@@ -128,35 +128,28 @@
 			};
 		};
 
-		amba {
-			#address-cells = <1>;
-			#size-cells = <1>;
-			compatible = "simple-bus";
-			ranges;
-
-			pdma0: dma@e0900000 {
-				compatible = "arm,pl330", "arm,primecell";
-				reg = <0xe0900000 0x1000>;
-				interrupt-parent = <&vic0>;
-				interrupts = <19>;
-				clocks = <&clocks CLK_PDMA0>;
-				clock-names = "apb_pclk";
-				#dma-cells = <1>;
-				#dma-channels = <8>;
-				#dma-requests = <32>;
-			};
+		pdma0: dma@e0900000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0xe0900000 0x1000>;
+			interrupt-parent = <&vic0>;
+			interrupts = <19>;
+			clocks = <&clocks CLK_PDMA0>;
+			clock-names = "apb_pclk";
+			#dma-cells = <1>;
+			#dma-channels = <8>;
+			#dma-requests = <32>;
+		};
 
-			pdma1: dma@e0a00000 {
-				compatible = "arm,pl330", "arm,primecell";
-				reg = <0xe0a00000 0x1000>;
-				interrupt-parent = <&vic0>;
-				interrupts = <20>;
-				clocks = <&clocks CLK_PDMA1>;
-				clock-names = "apb_pclk";
-				#dma-cells = <1>;
-				#dma-channels = <8>;
-				#dma-requests = <32>;
-			};
+		pdma1: dma@e0a00000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0xe0a00000 0x1000>;
+			interrupt-parent = <&vic0>;
+			interrupts = <20>;
+			clocks = <&clocks CLK_PDMA1>;
+			clock-names = "apb_pclk";
+			#dma-cells = <1>;
+			#dma-channels = <8>;
+			#dma-requests = <32>;
 		};
 
 		adc: adc@e1700000 {

The fixed clocks are kept under dedicated 'external-clocks' node, thus a fake 'reg' was added. This is not correct with dtschema as fixed-clock binding does not have a 'reg' property. Moving fixed clocks out of 'soc' to root node fixes multiple dtbs_check warnings:

external-clocks: $nodename:0: 'external-clocks' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
external-clocks: #size-cells:0:0: 0 is not one of [1, 2]
external-clocks: oscillator@0:reg:0: [0] is too short
external-clocks: oscillator@1:reg:0: [1] is too short
external-clocks: 'ranges' is a required property
oscillator@0: 'reg' does not match any of the regexes: 'pinctrl-[0-9]+'

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210.dtsi | 36 +++++++++++++---------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

arch/arm/boot/dts/s5pv210.dtsi +14 -22

index 84e4447931de..5c760a6d7955 100644
@@ -52,34 +52,26 @@
 		};
 	};
 
+	xxti: oscillator-0 {
+		compatible = "fixed-clock";
+		clock-frequency = <0>;
+		clock-output-names = "xxti";
+		#clock-cells = <0>;
+	};
+
+	xusbxti: oscillator-1 {
+		compatible = "fixed-clock";
+		clock-frequency = <0>;
+		clock-output-names = "xusbxti";
+		#clock-cells = <0>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges;
 
-		external-clocks {
-			compatible = "simple-bus";
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			xxti: oscillator@0 {
-				compatible = "fixed-clock";
-				reg = <0>;
-				clock-frequency = <0>;
-				clock-output-names = "xxti";
-				#clock-cells = <0>;
-			};
-
-			xusbxti: oscillator@1 {
-				compatible = "fixed-clock";
-				reg = <1>;
-				clock-frequency = <0>;
-				clock-output-names = "xusbxti";
-				#clock-cells = <0>;
-			};
-		};
-
 		onenand: onenand@b0600000 {
 			compatible = "samsung,s5pv210-onenand";
 			reg = <0xb0600000 0x2000>,

The Power Management Unit (PMU) is a separate device which has little common with clock controller. Moving it to one level up (from clock controller child to SoC) allows to remove fake simple-bus compatible and dtbs_check warnings like:

clock-controller@e0100000: $nodename:0:

    'clock-controller@e0100000' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210.dtsi | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

arch/arm/boot/dts/s5pv210.dtsi +5 -8

index 5c760a6d7955..46221a5c8ce5 100644
@@ -92,19 +92,16 @@
 		};
 
 		clocks: clock-controller@e0100000 {
-			compatible = "samsung,s5pv210-clock", "simple-bus";
+			compatible = "samsung,s5pv210-clock";
 			reg = <0xe0100000 0x10000>;
 			clock-names = "xxti", "xusbxti";
 			clocks = <&xxti>, <&xusbxti>;
 			#clock-cells = <1>;
-			#address-cells = <1>;
-			#size-cells = <1>;
-			ranges;
+		};
 
-			pmu_syscon: syscon@e0108000 {
-				compatible = "samsung-s5pv210-pmu", "syscon";
-				reg = <0xe0108000 0x8000>;
-			};
+		pmu_syscon: syscon@e0108000 {
+			compatible = "samsung-s5pv210-pmu", "syscon";
+			reg = <0xe0108000 0x8000>;
 		};
 
 		pinctrl0: pinctrl@e0200000 {

The 'audio-subsystem' node is an artificial creation, not representing real hardware. The hardware is described by its nodes - AUDSS clock controller and I2S0.

Remove the 'audio-subsystem' node along with its undocumented compatible to fix dtbs_check warnings like:

audio-subsystem: $nodename:0: 'audio-subsystem' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210.dtsi | 65 +++++++++++++++-------------------
 1 file changed, 29 insertions(+), 36 deletions(-)

arch/arm/boot/dts/s5pv210.dtsi +29 -36

index 46221a5c8ce5..2871351ab907 100644
@@ -223,43 +223,36 @@
 			status = "disabled";
 		};
 
-		audio-subsystem {
-			compatible = "samsung,s5pv210-audss", "simple-bus";
-			#address-cells = <1>;
-			#size-cells = <1>;
-			ranges;
-
-			clk_audss: clock-controller@eee10000 {
-				compatible = "samsung,s5pv210-audss-clock";
-				reg = <0xeee10000 0x1000>;
-				clock-names = "hclk", "xxti",
-						"fout_epll",
-						"sclk_audio0";
-				clocks = <&clocks DOUT_HCLKP>, <&xxti>,
-						<&clocks FOUT_EPLL>,
-						<&clocks SCLK_AUDIO0>;
-				#clock-cells = <1>;
-			};
+		clk_audss: clock-controller@eee10000 {
+			compatible = "samsung,s5pv210-audss-clock";
+			reg = <0xeee10000 0x1000>;
+			clock-names = "hclk", "xxti",
+				      "fout_epll",
+				      "sclk_audio0";
+			clocks = <&clocks DOUT_HCLKP>, <&xxti>,
+				 <&clocks FOUT_EPLL>,
+				 <&clocks SCLK_AUDIO0>;
+			#clock-cells = <1>;
+		};
 
-			i2s0: i2s@eee30000 {
-				compatible = "samsung,s5pv210-i2s";
-				reg = <0xeee30000 0x1000>;
-				interrupt-parent = <&vic2>;
-				interrupts = <16>;
-				dma-names = "rx", "tx", "tx-sec";
-				dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>;
-				clock-names = "iis",
-						"i2s_opclk0",
-						"i2s_opclk1";
-				clocks = <&clk_audss CLK_I2S>,
-						<&clk_audss CLK_I2S>,
-						<&clk_audss CLK_DOUT_AUD_BUS>;
-				samsung,idma-addr = <0xc0010000>;
-				pinctrl-names = "default";
-				pinctrl-0 = <&i2s0_bus>;
-				#sound-dai-cells = <0>;
-				status = "disabled";
-			};
+		i2s0: i2s@eee30000 {
+			compatible = "samsung,s5pv210-i2s";
+			reg = <0xeee30000 0x1000>;
+			interrupt-parent = <&vic2>;
+			interrupts = <16>;
+			dma-names = "rx", "tx", "tx-sec";
+			dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>;
+			clock-names = "iis",
+				      "i2s_opclk0",
+				      "i2s_opclk1";
+			clocks = <&clk_audss CLK_I2S>,
+				 <&clk_audss CLK_I2S>,
+				 <&clk_audss CLK_DOUT_AUD_BUS>;
+			samsung,idma-addr = <0xc0010000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2s0_bus>;
+			#sound-dai-cells = <0>;
+			status = "disabled";
 		};
 
 		i2s1: i2s@e2100000 {

The bindings describe I2S DAI has 1 cells. This makes especially sense for i2s0 which registers two DAIs. Adjust the cells to fix dtbs_check warnings like:

i2s@e2100000: #sound-dai-cells:0:0: 1 was expected

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-fascinate4g.dts | 2 +-
 arch/arm/boot/dts/s5pv210-galaxys.dts     | 2 +-
 arch/arm/boot/dts/s5pv210.dtsi            | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

arch/arm/boot/dts/s5pv210-fascinate4g.dts +1 -1

index ca064359dd30..a6dc8a173af1 100644
@@ -102,7 +102,7 @@
 		pinctrl-0 = <&headset_det &earpath_sel>;
 
 		cpu {
-			sound-dai = <&i2s0>, <&bt_codec>;
+			sound-dai = <&i2s0 0>, <&bt_codec>;
 		};
 
 		codec {

arch/arm/boot/dts/s5pv210-galaxys.dts +1 -1

index 560f830b6f6b..0eba06f56ac7 100644
@@ -132,7 +132,7 @@
 		pinctrl-0 = <&headset_det &earpath_sel>;
 
 		cpu {
-			sound-dai = <&i2s0>, <&bt_codec>;
+			sound-dai = <&i2s0 0>, <&bt_codec>;
 		};
 
 		codec {

arch/arm/boot/dts/s5pv210.dtsi +3 -3

index 2871351ab907..96e667ba1c3f 100644
@@ -251,7 +251,7 @@
 			samsung,idma-addr = <0xc0010000>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s0_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 

@@ -266,7 +266,7 @@
 			clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s1_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 

@@ -281,7 +281,7 @@
 			clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s2_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 

Sadly, this is causing issues for me. The machine driver is no longer probing correctly on the Galaxy S.

The failing call in sound/soc/samsung/aries_wm8994.c is

	/* Set CPU of_node for BT DAI */
	aries_dai[2].cpus->of_node = of_parse_phandle(cpu,
			"sound-dai", 1);

where cpus->of_node is not set properly. Which is definitely weird because it doesn't look like this should affect that.

Let me know if there's any specific test that you want me to do.

Thanks,
Jonathan

Quoted 71 lines · Krzysztof Kozlowski

The bindings describe I2S DAI has 1 cells. This makes especially sense for i2s0 which registers two DAIs. Adjust the cells to fix dtbs_check warnings like:

i2s@e2100000: #sound-dai-cells:0:0: 1 was expected

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-fascinate4g.dts | 2 +-
 arch/arm/boot/dts/s5pv210-galaxys.dts     | 2 +-
 arch/arm/boot/dts/s5pv210.dtsi            | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

arch/arm/boot/dts/s5pv210-fascinate4g.dts +1 -1

index ca064359dd30..a6dc8a173af1 100644
@@ -102,7 +102,7 @@
 		pinctrl-0 = <&headset_det &earpath_sel>;
 
 		cpu {
-			sound-dai = <&i2s0>, <&bt_codec>;
+			sound-dai = <&i2s0 0>, <&bt_codec>;
 		};
 
 		codec {

arch/arm/boot/dts/s5pv210-galaxys.dts +1 -1

index 560f830b6f6b..0eba06f56ac7 100644
@@ -132,7 +132,7 @@
 		pinctrl-0 = <&headset_det &earpath_sel>;
 
 		cpu {
-			sound-dai = <&i2s0>, <&bt_codec>;
+			sound-dai = <&i2s0 0>, <&bt_codec>;
 		};
 
 		codec {

arch/arm/boot/dts/s5pv210.dtsi +3 -3

index 2871351ab907..96e667ba1c3f 100644
@@ -251,7 +251,7 @@
 			samsung,idma-addr = <0xc0010000>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s0_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 

@@ -266,7 +266,7 @@
 			clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s1_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 

@@ -281,7 +281,7 @@
 			clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&i2s2_bus>;
-			#sound-dai-cells = <0>;
+			#sound-dai-cells = <1>;
 			status = "disabled";
 		};
 

Quoted 9 lines · Jonathan Bakker

Sadly, this is causing issues for me. The machine driver is no longer probing correctly on the Galaxy S.

The failing call in sound/soc/samsung/aries_wm8994.c is

	/* Set CPU of_node for BT DAI */
	aries_dai[2].cpus->of_node = of_parse_phandle(cpu,
			"sound-dai", 1);

Quoted 4 lines

where cpus->of_node is not set properly. Which is definitely weird because it doesn't look like this should affect that.

Let me know if there's any specific test that you want me to do.

Thanks for the tests. I wonder now if this was working before because really my change should not break it... I'll think more about it.

The S3C RTC requires 32768 Hz clock as input which is provided by PMIC. However there is no such clock provider but rather a regulator driver which registers the clock as a regulator. This is an old driver which will not be updated so add a workaround - a fixed-clock to fill missing clock phandle reference in S3C RTC.

This fixes dtbs_check warnings:

rtc@e2800000: clocks: [[2, 145]] is too short
rtc@e2800000: clock-names: ['rtc'] is too short

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aquila.dts | 17 +++++++++++++++++

The S3C RTC requires 32768 Hz clock as input which is provided by PMIC. However there is no such clock provider but rather a regulator driver which registers the clock as a regulator. This is an old driver which will not be updated so add a workaround - a fixed-clock to fill missing clock phandle reference in S3C RTC.

This fixes dtbs_check warnings:

rtc@e2800000: clocks: [[2, 145]] is too short
rtc@e2800000: clock-names: ['rtc'] is too short

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

arch/arm/boot/dts/s5pv210-aries.dtsi +7 -0

index 6ba23562da46..86c3b26fd21e 100644
@@ -47,6 +47,13 @@
 		};
 	};
 
+	pmic_ap_clk: clock-0 {
+		/* Workaround for missing clock on PMIC */
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+	};
+
 	bt_codec: bt_sco {
 		compatible = "linux,bt-sco";
 		#sound-dai-cells = <0>;

arch/arm/boot/dts/s5pv210-aries.dtsi +2 -0

+};
+
 &sdhci1 {
 	#address-cells = <1>;
 	#size-cells = <0>;

Hi Krzysztof,

Quoted 35 lines · Krzysztof Kozlowski

The S3C RTC requires 32768 Hz clock as input which is provided by PMIC. However there is no such clock provider but rather a regulator driver which registers the clock as a regulator. This is an old driver which will not be updated so add a workaround - a fixed-clock to fill missing clock phandle reference in S3C RTC.

This fixes dtbs_check warnings:

rtc@e2800000: clocks: [[2, 145]] is too short
rtc@e2800000: clock-names: ['rtc'] is too short

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

arch/arm/boot/dts/s5pv210-aries.dtsi +7 -0

index 6ba23562da46..86c3b26fd21e 100644
@@ -47,6 +47,13 @@
 		};
 	};
 
+	pmic_ap_clk: clock-0 {
+		/* Workaround for missing clock on PMIC */
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+	};
+
 	bt_codec: bt_sco {
 		compatible = "linux,bt-sco";
 		#sound-dai-cells = <0>;

arch/arm/boot/dts/s5pv210-aries.dtsi line 837

Quoted 6 lines

arch/arm/boot/dts/s5pv210-aries.dtsi +3 -0

@@ -825,6 +832,11 @@
 	samsung,pwm-outputs = <1>;
 };
 
+&rtc {
+	clocks = <&clocks CLK_RTC>, <&pmic_ap_clk>;
+	clock-names = "rtc", "rtc_src";

Missing a

status = "okay";

here, but with that it works fine for me. Looks like it's also missing in the patches for the other devices as well.

The fixed regulators are kept under dedicated "regulators" node but this causes multiple dtschema warnings:

regulators: $nodename:0: 'regulators' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
regulators: #size-cells:0:0: 0 is not one of [1, 2]
regulators: fixed-regulator@0:reg:0: [0] is too short
regulators: fixed-regulator@1:reg:0: [1] is too short
regulators: fixed-regulator@2:reg:0: [2] is too short

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aquila.dts | 47 +++++++++++-----------------

The fixed regulators are kept under dedicated "regulators" node but this causes multiple dtschema warnings:

regulators: $nodename:0: 'regulators' does not match '^([a-z][a-z0-9\\-]+-bus|bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$'
regulators: #size-cells:0:0: 0 is not one of [1, 2]
regulators: fixed-regulator@0:reg:0: [0] is too short
regulators: fixed-regulator@1:reg:0: [1] is too short
regulators: fixed-regulator@2:reg:0: [2] is too short
regulators: fixed-regulator@3:reg:0: [3] is too short

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-goni.dts | 64 +++++++++++++-----------------

"gpios" property is deprecated. Update the Aquila DTS to fix dtbs_checks warnings like:

i2c-pmic: 'sda-gpios' is a required property
i2c-pmic: 'scl-gpios' is a required property

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aquila.dts | 4 ++--

"gpios" property is deprecated. Update the Goni DTS to fix dtbs_checks warnings like:

i2c-pmic: 'sda-gpios' is a required property
i2c-pmic: 'scl-gpios' is a required property

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-goni.dts | 4 ++--

The device tree schema expects SPI controller to be named "spi", otherwise dtbs_check complain with a warning like:

spi-gpio-0: $nodename:0: 'spi-gpio-0' does not match '^spi(@.*|-[0-9a-f])*$'

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/boot/dts/s5pv210-aries.dtsi +1 -1

index 86c3b26fd21e..bd4450dbdcb6 100644
@@ -545,7 +545,7 @@
 		value = <0x5200>;
 	};
 
-	spi_lcd: spi-gpio-0 {
+	spi_lcd: spi-2 {
 		compatible = "spi-gpio";
 		#address-cells = <1>;
 		#size-cells = <0>;

The device tree schema expects SPI controller to be named "spi", otherwise dtbs_check complain with a warning like:

spi-gpio-0: $nodename:0: 'spi-gpio-0' does not match '^spi(@.*|-[0-9a-f])*$'

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210-aries.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

dtschema expects DMA channels in specific order (tx, rx and tx-sec). The order actually should not matter because dma-names is used however let's make it aligned with dtschema to suppress warnings like:

i2s@eee30000: dma-names: ['rx', 'tx', 'tx-sec'] is not valid under any of the given schemas

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/boot/dts/s5pv210.dtsi +6 -6

index 96e667ba1c3f..72fb9d9f7ba3 100644
@@ -240,8 +240,8 @@
 			reg = <0xeee30000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <16>;
-			dma-names = "rx", "tx", "tx-sec";
-			dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>;
+			dma-names = "tx", "rx", "tx-sec";
+			dmas = <&pdma1 10>, <&pdma1 9>, <&pdma1 11>;
 			clock-names = "iis",
 				      "i2s_opclk0",
 				      "i2s_opclk1";

@@ -260,8 +260,8 @@
 			reg = <0xe2100000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <17>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 12>, <&pdma1 13>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 13>, <&pdma1 12>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>;
 			pinctrl-names = "default";

@@ -275,8 +275,8 @@
 			reg = <0xe2a00000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <18>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 14>, <&pdma1 15>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 15>, <&pdma1 14>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>;
 			pinctrl-names = "default";

Initial testing on both an i9000 and an SGH-T959P are showing that the audio has stopped working with this. I'm not 100% convinced as I've had DMA issues in the past. However trying to play something just results in a hang after 1.5s while it works just fine without this patch.

Thanks,
Jonathan

Quoted 49 lines · Krzysztof Kozlowski

dtschema expects DMA channels in specific order (tx, rx and tx-sec). The order actually should not matter because dma-names is used however let's make it aligned with dtschema to suppress warnings like:

i2s@eee30000: dma-names: ['rx', 'tx', 'tx-sec'] is not valid under any of the given schemas

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
 arch/arm/boot/dts/s5pv210.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/boot/dts/s5pv210.dtsi +6 -6

index 96e667ba1c3f..72fb9d9f7ba3 100644
@@ -240,8 +240,8 @@
 			reg = <0xeee30000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <16>;
-			dma-names = "rx", "tx", "tx-sec";
-			dmas = <&pdma1 9>, <&pdma1 10>, <&pdma1 11>;
+			dma-names = "tx", "rx", "tx-sec";
+			dmas = <&pdma1 10>, <&pdma1 9>, <&pdma1 11>;
 			clock-names = "iis",
 				      "i2s_opclk0",
 				      "i2s_opclk1";

@@ -260,8 +260,8 @@
 			reg = <0xe2100000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <17>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 12>, <&pdma1 13>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 13>, <&pdma1 12>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S1>, <&clocks SCLK_AUDIO1>;
 			pinctrl-names = "default";

@@ -275,8 +275,8 @@
 			reg = <0xe2a00000 0x1000>;
 			interrupt-parent = <&vic2>;
 			interrupts = <18>;
-			dma-names = "rx", "tx";
-			dmas = <&pdma1 14>, <&pdma1 15>;
+			dma-names = "tx", "rx";
+			dmas = <&pdma1 15>, <&pdma1 14>;
 			clock-names = "iis", "i2s_opclk0";
 			clocks = <&clocks CLK_I2S2>, <&clocks SCLK_AUDIO2>;
 			pinctrl-names = "default";

Ah, I figured out why the dma stopped working. In s5pv210-aries.dtsi the dma's for i2s0 are overriden to use pdma0 instead of pdma1. That also needs changing for this to work properly.

Thanks,
Jonathan

Quoted 7 lines · Jonathan Bakker

Initial testing on both an i9000 and an SGH-T959P are showing that the audio has stopped working with this. I'm not 100% convinced as I've had DMA issues in the past. However trying to play something just results in a hang after 1.5s while it works just fine without this patch.

Thanks,
Jonathan