devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] Rockchip I2S/TDM controller
@ 2021-09-03 23:15 Nicolas Frattaroli
  2021-09-03 23:15 ` [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Nicolas Frattaroli @ 2021-09-03 23:15 UTC (permalink / raw)
  Cc: Nicolas Frattaroli, linux-rockchip, alsa-devel, devicetree,
	linux-arm-kernel, linux-kernel

Hello,

this is version 4 of the I2S/TDM driver patchset. A big thanks
to everyone who has provided their valuable feedback so far.

Changes in v4:
 driver:
 - factor TDE/RDE enable/disable into their own inlined functions
 - add an RDE disable in a location where it looks like it was
   forgotten (rxctrl else), judging by corresponding TDE code
 - remove parentheses around CLK_PPM_MIN/MAX values
 - wording + titlecasing in the clock compensation control
 - use if statement in precious_reg instead
 - refactor rockchip_i2s_io_multiplex to have the switch statements
   in a function call to make the function less unwieldy
 - get rid of IS_ERR checks around clk enable/disable calls where
   already checked before by the probe
 - reworded some error message strings
 - fix potential deadlock in txrxctrl found by Sugar Zhang
   using spin_lock_irqsave
 - fix potential deadlock in trcm_mode found by Sugar Zhang
   using spin_lock_irqsave
 - use devm_platform_get_and_ioremap_resource in probe
 - only set DMA things if controller has capture/playback ability.
   Did not move this into init_dai because I'd then need to pass in
   the res and probe it earlier in the function, and it's also used
   elsewhere in the probe function
 - use _get_optional_exclusive for reset controls, as some controllers
   only have capture or playback capability
 bindings:
 - remove status = "okay" since that's the default
 - change the path configs to be an enum
 - rename "foo" to "bus"
 - make resets optional as controller may lack either playback or
   capture capability, and therefore also doesn't have a reset.
   At least one reset is still required, because a controller with
   no playback and no capture is not very useful

Changes in v3:
 driver:
 - alphabetically sort includes
 - check pm_runtime_get_sync return value, act on it
 - remove unnecessary initialisers in set_fmt
 - use udelay(15) in retry code: 10 retries * 15 = 150, so at worst
   we wait the full i2s register access delay
 - fix some weird returns to return directly
 - use __maybe_unused instead of #ifdef CONFIG_PM_SLEEP, also put
   __maybe_unused on the runtime callbacks
 - use (foo) instead of foo in header macros for precedence reasons
 - when using mclk-calibrate, also turn off/on those clocks during
   suspend and resume operations
 - remove mclk_tx and mclk_rx reenablement code in remove
 - move hclk enablement further down the probe, and disable it
   on probe failure
 - make reset controls mandatory, since the bindings state this too
 - use _exclusive for getting the reset controls
 - change reset assert/deassert delays to both be 10 usec
   (thank you Sugar Zhang!)
 - properly prepare and enable all mclks in probe, especially before
   calling clk_get_rate on them
 - if registering PCM fails, also use the cleanup error path instead of
   returning directly
 - bring back playback and capture only but in the way Sugar Zhang
   suggested it: set those modes depending on dma-names
 - rework clock enablement in general. Probe now always enables these,
   instead of relying on the pm resume thing
 - add myself to MAINTAINERS for this driver
 dt bindings:
 - fix a description still mentioning clk-trcm in the schema
 - document rockchip,io-multiplex, a property that describes the
   hardware as having multiplexed I2S GPIOs so direction needs to
   be changed dynamically
 - document rockchip,mclk-calibrate, which allows specifying
   different clocks for the two sample rate bases and switch between
   them as needed
 - dma-names now doesn't have a set order and items can be absent to
   indicate that the controller doesn't support this mode
 - add myself to MAINTAINERS for these bindings

Changes in v2:
 - remove ad-hoc writeq and needless (and broken) optimisation in
   reset assert/deassert. This wouldn't have worked on Big Endian,
   and would've been pointless on any other platform, as the
   overhead for saving one write was comparatively big
 - fix various checkpatch issues
 - get rid of leftover clk-trcm in schema
 - set status = "okay" in example in schema instead of "disabled"
 - change dma-names so rx is first, adjust device trees as necessary
 - properly reference uint32-array for rx-route and tx-route
   instead of uint32
 - replace trcm-sync with two boolean properties, adjust DT changes
   accordingly and also get rid of the header file
 - get rid of rockchip,no-dmaengine. This was only needed for
   some downstream driver and shouldn't be in the DT
 - get rid of rockchip,capture-only/playback-only. Rationale being
   that I have no way to test whether they're needed, and
   unconditionally setting channels_min to 0 breaks everything
 - change hclk description in "clocks"

Nicolas Frattaroli (4):
  ASoC: rockchip: add support for i2s-tdm controller
  dt-bindings: sound: add rockchip i2s-tdm binding
  arm64: dts: rockchip: add i2s1 on rk356x
  arm64: dts: rockchip: add analog audio on Quartz64

 .../bindings/sound/rockchip,i2s-tdm.yaml      |  218 ++
 MAINTAINERS                                   |    7 +
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   |   35 +-
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      |   26 +
 sound/soc/rockchip/Kconfig                    |   11 +
 sound/soc/rockchip/Makefile                   |    2 +
 sound/soc/rockchip/rockchip_i2s_tdm.c         | 1832 +++++++++++++++++
 sound/soc/rockchip/rockchip_i2s_tdm.h         |  398 ++++
 8 files changed, 2528 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
 create mode 100644 sound/soc/rockchip/rockchip_i2s_tdm.c
 create mode 100644 sound/soc/rockchip/rockchip_i2s_tdm.h

-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-09-03 23:15 [PATCH v4 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
@ 2021-09-03 23:15 ` Nicolas Frattaroli
  2021-09-08 12:08   ` Rob Herring
  2021-09-15 14:10   ` Mark Brown
  2021-09-03 23:15 ` [PATCH v4 3/4] arm64: dts: rockchip: add i2s1 on rk356x Nicolas Frattaroli
  2021-09-03 23:15 ` [PATCH v4 4/4] arm64: dts: rockchip: add analog audio on Quartz64 Nicolas Frattaroli
  2 siblings, 2 replies; 11+ messages in thread
From: Nicolas Frattaroli @ 2021-09-03 23:15 UTC (permalink / raw)
  To: Nicolas Frattaroli, Liam Girdwood, Mark Brown, Rob Herring,
	Heiko Stuebner
  Cc: linux-rockchip, alsa-devel, devicetree, linux-arm-kernel, linux-kernel

This adds the YAML bindings for the Rockchip I2S/TDM audio driver.

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 .../bindings/sound/rockchip,i2s-tdm.yaml      | 218 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 219 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml

diff --git a/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
new file mode 100644
index 000000000000..a04267ccb0f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
@@ -0,0 +1,218 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/rockchip,i2s-tdm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip I2S/TDM Controller
+
+description:
+  The Rockchip I2S/TDM Controller is a Time Division Multiplexed
+  audio interface found in various Rockchip SoCs, allowing up
+  to 8 channels of audio over a serial interface.
+
+maintainers:
+  - Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - rockchip,px30-i2s-tdm
+      - rockchip,rk1808-i2s-tdm
+      - rockchip,rk3308-i2s-tdm
+      - rockchip,rk3568-i2s-tdm
+      - rockchip,rv1126-i2s-tdm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  dmas:
+    minItems: 1
+    maxItems: 2
+
+  dma-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - rx
+        - tx
+
+  clocks:
+    minItems: 3
+    items:
+      - description: clock for TX
+      - description: clock for RX
+      - description: AHB clock driving the interface
+      - description:
+          Parent clock for mclk_tx (only required when using mclk-calibrate)
+      - description:
+          Parent clock for mclk_rx (only required when using mclk-calibrate)
+      - description:
+          Clock for sample rates that are an integer multiple of 8000
+          (only required when using mclk-calibrate)
+      - description:
+          Clock for sample rates that are an integer multiple of 11025
+          (only required when using mclk-calibrate)
+
+  clock-names:
+    minItems: 3
+    items:
+      - const: mclk_tx
+      - const: mclk_rx
+      - const: hclk
+      - const: mclk_tx_src
+      - const: mclk_rx_src
+      - const: mclk_root0
+      - const: mclk_root1
+
+  rockchip,frame-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 64
+    minimum: 32
+    maximum: 512
+    description:
+      Width of a frame, usually slot width multiplied by number of slots.
+      Must be even.
+
+  resets:
+    minItems: 1
+    maxItems: 2
+    description: resets for the tx and rx directions
+
+  reset-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - tx-m
+        - rx-m
+
+  rockchip,cru:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the cru.
+      Required if neither trcm-sync-tx-only nor trcm-sync-rx-only are specified.
+
+  rockchip,grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the syscon node for the GRF register.
+
+  rockchip,mclk-calibrate:
+    description:
+      Switch between two root clocks depending on the audio sample rate.
+      For integer multiples of 8000 (e.g. 48000 Hz), mclk_root0 is used.
+      For integer multiples of 11025 (e.g. 44100 Hz), mclk_root1 is used.
+    type: boolean
+
+  rockchip,trcm-sync-tx-only:
+    type: boolean
+    description: Use TX BCLK/LRCK for both TX and RX.
+
+  rockchip,trcm-sync-rx-only:
+    type: boolean
+    description: Use RX BCLK/LRCK for both TX and RX.
+
+  "#sound-dai-cells":
+    const: 0
+
+  rockchip,i2s-rx-route:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Defines the mapping of I2S RX sdis to I2S data bus lines.
+      By default, they are mapped one-to-one.
+      rockchip,i2s-rx-route = <3> would mean sdi3 is receiving from data0.
+    maxItems: 4
+    items:
+      - enum: [0, 1, 2, 3]
+
+  rockchip,i2s-tx-route:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Defines the mapping of I2S TX sdos to I2S data bus lines.
+      By default, they are mapped one-to-one.
+      rockchip,i2s-tx-route = <3> would mean sdo3 is sending to data0.
+    maxItems: 4
+    items:
+      - enum: [0, 1, 2, 3]
+
+  rockchip,tdm-fsync-half-frame:
+    description: Whether to use half frame fsync.
+    type: boolean
+
+  rockchip,io-multiplex:
+    description:
+      Specify that the GPIO lines on the I2S bus are multiplexed such that
+      the direction (input/output) needs to be dynamically adjusted.
+    type: boolean
+
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - dmas
+  - dma-names
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - rockchip,grf
+  - "#sound-dai-cells"
+
+allOf:
+  - if:
+      properties:
+        rockchip,trcm-sync-tx-only: false
+        rockchip,trcm-sync-rx-only: false
+    then:
+      required:
+        - rockchip,cru
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3568-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/pinctrl/rockchip.h>
+
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        i2s@fe410000 {
+            compatible = "rockchip,rk3568-i2s-tdm";
+            reg = <0x0 0xfe410000 0x0 0x1000>;
+            interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cru MCLK_I2S1_8CH_TX>, <&cru MCLK_I2S1_8CH_RX>,
+                     <&cru HCLK_I2S1_8CH>;
+            clock-names = "mclk_tx", "mclk_rx", "hclk";
+            dmas = <&dmac1 3>, <&dmac1 2>;
+            dma-names = "rx", "tx";
+            resets = <&cru SRST_M_I2S1_8CH_TX>, <&cru SRST_M_I2S1_8CH_RX>;
+            reset-names = "tx-m", "rx-m";
+            rockchip,trcm-sync-tx-only;
+            rockchip,cru = <&cru>;
+            rockchip,grf = <&grf>;
+            #sound-dai-cells = <0>;
+            pinctrl-names = "default";
+            pinctrl-0 =
+                <&i2s1m0_sclktx
+                &i2s1m0_sclkrx
+                &i2s1m0_lrcktx
+                &i2s1m0_lrckrx
+                &i2s1m0_sdi0
+                &i2s1m0_sdi1
+                &i2s1m0_sdi2
+                &i2s1m0_sdi3
+                &i2s1m0_sdo0
+                &i2s1m0_sdo1
+                &i2s1m0_sdo2
+                &i2s1m0_sdo3>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 106d448e660d..e2cc0357e2b7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16119,6 +16119,7 @@ ROCKCHIP I2S TDM DRIVER
 M:	Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
 L:	linux-rockchip@lists.infradead.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
 F:	sound/soc/rockchip/rockchip_i2s_tdm.*
 
 ROCKCHIP ISP V1 DRIVER
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/4] arm64: dts: rockchip: add i2s1 on rk356x
  2021-09-03 23:15 [PATCH v4 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
  2021-09-03 23:15 ` [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
@ 2021-09-03 23:15 ` Nicolas Frattaroli
  2021-09-03 23:15 ` [PATCH v4 4/4] arm64: dts: rockchip: add analog audio on Quartz64 Nicolas Frattaroli
  2 siblings, 0 replies; 11+ messages in thread
From: Nicolas Frattaroli @ 2021-09-03 23:15 UTC (permalink / raw)
  To: Rob Herring, Heiko Stuebner
  Cc: Nicolas Frattaroli, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

This adds the necessary device tree node on rk3566 and rk3568
to enable the I2S1 TDM audio controller.

I2S0 has not been added, as it is connected to HDMI and there is
no way to test that it's working without a functioning video
clock (read: VOP2 driver).

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 26 ++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index e42fbac6147b..a24f7160f6d4 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -564,6 +564,32 @@ sdhci: mmc@fe310000 {
 		status = "disabled";
 	};
 
+	i2s1_8ch: i2s@fe410000 {
+		compatible = "rockchip,rk3568-i2s-tdm";
+		reg = <0x0 0xfe410000 0x0 0x1000>;
+		interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
+		assigned-clocks = <&cru CLK_I2S1_8CH_TX_SRC>, <&cru CLK_I2S1_8CH_RX_SRC>;
+		assigned-clock-rates = <1188000000>, <1188000000>;
+		clocks = <&cru MCLK_I2S1_8CH_TX>, <&cru MCLK_I2S1_8CH_RX>,
+		<&cru HCLK_I2S1_8CH>;
+		clock-names = "mclk_tx", "mclk_rx", "hclk";
+		dmas = <&dmac1 3>, <&dmac1 2>;
+		dma-names = "rx", "tx";
+		resets = <&cru SRST_M_I2S1_8CH_TX>, <&cru SRST_M_I2S1_8CH_RX>;
+		reset-names = "tx-m", "rx-m";
+		rockchip,cru = <&cru>;
+		rockchip,grf = <&grf>;
+		#sound-dai-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_sclkrx
+			     &i2s1m0_lrcktx &i2s1m0_lrckrx
+			     &i2s1m0_sdi0   &i2s1m0_sdi1
+			     &i2s1m0_sdi2   &i2s1m0_sdi3
+			     &i2s1m0_sdo0   &i2s1m0_sdo1
+			     &i2s1m0_sdo2   &i2s1m0_sdo3>;
+		status = "disabled";
+	};
+
 	dmac0: dmac@fe530000 {
 		compatible = "arm,pl330", "arm,primecell";
 		reg = <0x0 0xfe530000 0x0 0x4000>;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 4/4] arm64: dts: rockchip: add analog audio on Quartz64
  2021-09-03 23:15 [PATCH v4 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
  2021-09-03 23:15 ` [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
  2021-09-03 23:15 ` [PATCH v4 3/4] arm64: dts: rockchip: add i2s1 on rk356x Nicolas Frattaroli
@ 2021-09-03 23:15 ` Nicolas Frattaroli
  2021-09-07 13:40   ` Chris Morgan
  2 siblings, 1 reply; 11+ messages in thread
From: Nicolas Frattaroli @ 2021-09-03 23:15 UTC (permalink / raw)
  To: Rob Herring, Heiko Stuebner
  Cc: Nicolas Frattaroli, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On the Quartz64 Model A, the I2S1 TDM controller is connected
to the rk817 codec in I2S mode. Enabling it and adding the
necessary simple-sound-card and codec nodes allows for analog
audio output on the PINE64 Quartz64 Model A SBC.

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   | 35 ++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
index a244f7b87e38..3a87c0240b30 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
@@ -58,6 +58,20 @@ led-diy {
 		};
 	};
 
+	rk817-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,name = "Analog RK817";
+		simple-audio-card,mclk-fs = <256>;
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s1_8ch>;
+		};
+		simple-audio-card,codec {
+			sound-dai = <&rk817>;
+		};
+	};
+
 	vcc12v_dcin: vcc12v_dcin {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc12v_dcin";
@@ -199,8 +213,13 @@ rk817: pmic@20 {
 		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
 		clock-output-names = "rk808-clkout1", "rk808-clkout2";
 
+		#sound-dai-cells = <0>;
+		clock-names = "mclk";
+		clocks = <&cru I2S1_MCLKOUT_TX>;
+		assigned-clocks = <&cru I2S1_MCLKOUT_TX>;
+		assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&pmic_int_l>;
+		pinctrl-0 = <&pmic_int_l>, <&i2s1m0_mclk>;
 		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
@@ -389,9 +408,23 @@ regulator-state-mem {
 				};
 			};
 		};
+
+		rk817_codec: codec {
+		};
+
 	};
 };
 
+&i2s1_8ch {
+	status = "okay";
+	rockchip,trcm-sync-tx-only;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2s1m0_sclktx
+		     &i2s1m0_lrcktx
+		     &i2s1m0_sdi0
+		     &i2s1m0_sdo0>;
+};
+
 &mdio1 {
 	rgmii_phy1: ethernet-phy@0 {
 		compatible = "ethernet-phy-ieee802.3-c22";
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 4/4] arm64: dts: rockchip: add analog audio on Quartz64
  2021-09-03 23:15 ` [PATCH v4 4/4] arm64: dts: rockchip: add analog audio on Quartz64 Nicolas Frattaroli
@ 2021-09-07 13:40   ` Chris Morgan
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Morgan @ 2021-09-07 13:40 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: Rob Herring, Heiko Stuebner, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel

On Sat, Sep 04, 2021 at 01:15:36AM +0200, Nicolas Frattaroli wrote:
> On the Quartz64 Model A, the I2S1 TDM controller is connected
> to the rk817 codec in I2S mode. Enabling it and adding the
> necessary simple-sound-card and codec nodes allows for analog
> audio output on the PINE64 Quartz64 Model A SBC.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
>  .../boot/dts/rockchip/rk3566-quartz64-a.dts   | 35 ++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
> index a244f7b87e38..3a87c0240b30 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
> @@ -58,6 +58,20 @@ led-diy {
>  		};
>  	};
>  
> +	rk817-sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,name = "Analog RK817";
> +		simple-audio-card,mclk-fs = <256>;
> +
> +		simple-audio-card,cpu {
> +			sound-dai = <&i2s1_8ch>;
> +		};
> +		simple-audio-card,codec {
> +			sound-dai = <&rk817>;
> +		};
> +	};
> +
>  	vcc12v_dcin: vcc12v_dcin {
>  		compatible = "regulator-fixed";
>  		regulator-name = "vcc12v_dcin";
> @@ -199,8 +213,13 @@ rk817: pmic@20 {
>  		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
>  		clock-output-names = "rk808-clkout1", "rk808-clkout2";
>  
> +		#sound-dai-cells = <0>;
> +		clock-names = "mclk";
> +		clocks = <&cru I2S1_MCLKOUT_TX>;
> +		assigned-clocks = <&cru I2S1_MCLKOUT_TX>;
> +		assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>;
>  		pinctrl-names = "default";
> -		pinctrl-0 = <&pmic_int_l>;
> +		pinctrl-0 = <&pmic_int_l>, <&i2s1m0_mclk>;
>  		rockchip,system-power-controller;
>  		wakeup-source;
>  		#clock-cells = <1>;
> @@ -389,9 +408,23 @@ regulator-state-mem {
>  				};
>  			};
>  		};
> +
> +		rk817_codec: codec {

This node should be optional and can probably be removed. It's only
used when there are extra parameters to pass to the codec (such as 
rockchip,mic-in-differential). Leaving it in shouldn't hurt anything
though.

> +		};
> +
>  	};
>  };
>  
> +&i2s1_8ch {
> +	status = "okay";
> +	rockchip,trcm-sync-tx-only;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2s1m0_sclktx
> +		     &i2s1m0_lrcktx
> +		     &i2s1m0_sdi0
> +		     &i2s1m0_sdo0>;
> +};
> +
>  &mdio1 {
>  	rgmii_phy1: ethernet-phy@0 {
>  		compatible = "ethernet-phy-ieee802.3-c22";
> -- 
> 2.33.0
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-09-03 23:15 ` [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
@ 2021-09-08 12:08   ` Rob Herring
  2021-09-15 14:10   ` Mark Brown
  1 sibling, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-09-08 12:08 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: alsa-devel, Mark Brown, linux-arm-kernel, linux-rockchip,
	Rob Herring, linux-kernel, Heiko Stuebner, devicetree,
	Liam Girdwood

On Sat, 04 Sep 2021 01:15:34 +0200, Nicolas Frattaroli wrote:
> This adds the YAML bindings for the Rockchip I2S/TDM audio driver.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
>  .../bindings/sound/rockchip,i2s-tdm.yaml      | 218 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 219 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/rockchip,i2s-tdm.yaml
> 

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-09-03 23:15 ` [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
  2021-09-08 12:08   ` Rob Herring
@ 2021-09-15 14:10   ` Mark Brown
  2021-09-15 17:06     ` Nicolas Frattaroli
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Brown @ 2021-09-15 14:10 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: Liam Girdwood, Rob Herring, Heiko Stuebner, linux-rockchip,
	alsa-devel, devicetree, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Sat, Sep 04, 2021 at 01:15:34AM +0200, Nicolas Frattaroli wrote:

> +  rockchip,frame-width:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 64
> +    minimum: 32
> +    maximum: 512
> +    description:
> +      Width of a frame, usually slot width multiplied by number of slots.
> +      Must be even.

Why is this in the binding?  This is normally configured by the machine
driver setting the TDM slots, not through DT.

> +  rockchip,mclk-calibrate:
> +    description:
> +      Switch between two root clocks depending on the audio sample rate.
> +      For integer multiples of 8000 (e.g. 48000 Hz), mclk_root0 is used.
> +      For integer multiples of 11025 (e.g. 44100 Hz), mclk_root1 is used.
> +    type: boolean

Why would we not want to do this, and assuming it's to do with
availability can't we detect it simply through seeing if both MCLKs are
available?

> +  rockchip,tdm-fsync-half-frame:
> +    description: Whether to use half frame fsync.
> +    type: boolean
> +

Why is this not part of the normal bus format configuration?  I don't
know what this is but it sounds a lot like I2S mode...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-09-15 14:10   ` Mark Brown
@ 2021-09-15 17:06     ` Nicolas Frattaroli
  2021-09-16 12:25       ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Nicolas Frattaroli @ 2021-09-15 17:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Rob Herring, Heiko Stuebner, linux-rockchip,
	alsa-devel, devicetree, linux-arm-kernel, linux-kernel

On Mittwoch, 15. September 2021 16:10:12 CEST Mark Brown wrote:
> On Sat, Sep 04, 2021 at 01:15:34AM +0200, Nicolas Frattaroli wrote:
> > +  rockchip,tdm-fsync-half-frame:
> > +    description: Whether to use half frame fsync.
> > +    type: boolean
> > +
> 
> Why is this not part of the normal bus format configuration?  I don't
> know what this is but it sounds a lot like I2S mode...

This affects all TDM I2S modes, i.e. TDM Normal, TDM Left Justified and TDM 
Right Justified.

Without tdm-fsync-half-frame, we purportedly get the following output in TDM 
Normal Mode (I2S Format):
(ch0l = channel 0 left, ch0r = channel 0 right)

fsync: 	_____________________________
                     	            \____________________________
sdi/sdo: ch0l, ch0r, ..., ch3l, ch3r,  ch0l, ch0r, ...

With tdm-fsync-half-frame, we purportedly get the following:

fsync: 	_____________________________
                     	            \____________________________
sdi/sdo: ch0l,  ch1l,  ch2l,  ch3l,   ch0r,  ch1r,  ch2r,  ch3r

At least, according to the TRM. I do not have an oscilloscope to verify this 
myself, and in the following paragraphs, I will elaborate why this seems 
confusing to me.

The comment block "DAI hardware signal polarity" in soc-dai.h seems to imply 
that what the TRM says the tdm-fsync-half-frame mode is (if one inverts fsync 
polarity of those waveforms), is what is expected:

> * FSYNC "normal" polarity depends on the frame format:
> * - I2S: frame consists of left then right channel data. Left channel starts
> *      with falling FSYNC edge, right channel starts with rising FSYNC edge.
> * - Left/Right Justified: frame consists of left then right channel data.
> *      Left channel starts with rising FSYNC edge, right channel starts with
> *      falling FSYNC edge.

I don't know if this is only applicable to non-TDM I2S, and whether it's 
normal to have the channels interleaved like that in TDM.

I don't see any DAIFMT that does what this does in any case.

So to answer the question, it's not part of the bus format because it applies 
to three bus formats, and I'm completely out of my depth here and wouldn't 
define three separate bus formats based on my own speculation of how this 
works.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-09-15 17:06     ` Nicolas Frattaroli
@ 2021-09-16 12:25       ` Mark Brown
  2021-09-19 17:38         ` Nicolas Frattaroli
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2021-09-16 12:25 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: Liam Girdwood, Rob Herring, Heiko Stuebner, linux-rockchip,
	alsa-devel, devicetree, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

On Wed, Sep 15, 2021 at 07:06:14PM +0200, Nicolas Frattaroli wrote:
> On Mittwoch, 15. September 2021 16:10:12 CEST Mark Brown wrote:

> > Why is this not part of the normal bus format configuration?  I don't
> > know what this is but it sounds a lot like I2S mode...

> This affects all TDM I2S modes, i.e. TDM Normal, TDM Left Justified and TDM 
> Right Justified.

> Without tdm-fsync-half-frame, we purportedly get the following output in TDM 
> Normal Mode (I2S Format):
> (ch0l = channel 0 left, ch0r = channel 0 right)

> fsync: 	_____________________________
>                      	            \____________________________
> sdi/sdo: ch0l, ch0r, ..., ch3l, ch3r,  ch0l, ch0r, ...
> 
> With tdm-fsync-half-frame, we purportedly get the following:
> 
> fsync: 	_____________________________
>                      	            \____________________________
> sdi/sdo: ch0l,  ch1l,  ch2l,  ch3l,   ch0r,  ch1r,  ch2r,  ch3r

> At least, according to the TRM. I do not have an oscilloscope to verify this 
> myself, and in the following paragraphs, I will elaborate why this seems 
> confusing to me.

fsync-half-frame is just normal TDM for I2S, the default mode is how DSP
mode normally operates.  I don't know that there's any pressing need to
support mix'n'match here, you could but it should be through the TDM
configuration API.

> So to answer the question, it's not part of the bus format because it applies 
> to three bus formats, and I'm completely out of my depth here and wouldn't 
> define three separate bus formats based on my own speculation of how this 
> works.

It is part of the bus format really.  I suspect the hardware is the kind
that only really implements DSP mode and can just fake up a LRCLK for
I2S in order to interoperate.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-09-16 12:25       ` Mark Brown
@ 2021-09-19 17:38         ` Nicolas Frattaroli
  2021-09-20 11:49           ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Nicolas Frattaroli @ 2021-09-19 17:38 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Rob Herring, Heiko Stuebner, linux-rockchip,
	alsa-devel, devicetree, linux-arm-kernel, linux-kernel

On Donnerstag, 16. September 2021 14:25:49 CEST Mark Brown wrote:
> On Wed, Sep 15, 2021 at 07:06:14PM +0200, Nicolas Frattaroli wrote:
> > On Mittwoch, 15. September 2021 16:10:12 CEST Mark Brown wrote:
> > > Why is this not part of the normal bus format configuration?  I don't
> > > know what this is but it sounds a lot like I2S mode...
> > 
> > This affects all TDM I2S modes, i.e. TDM Normal, TDM Left Justified and
> > TDM
> > Right Justified.
> > 
> > Without tdm-fsync-half-frame, we purportedly get the following output in
> > TDM Normal Mode (I2S Format):
> > (ch0l = channel 0 left, ch0r = channel 0 right)
> > 
> > fsync: 	_____________________________
> > 
> >                      	            \____________________________
> > 
> > sdi/sdo: ch0l, ch0r, ..., ch3l, ch3r,  ch0l, ch0r, ...
> > 
> > With tdm-fsync-half-frame, we purportedly get the following:
> > 
> > fsync: 	_____________________________
> > 
> >                      	            \____________________________
> > 
> > sdi/sdo: ch0l,  ch1l,  ch2l,  ch3l,   ch0r,  ch1r,  ch2r,  ch3r
> > 
> > At least, according to the TRM. I do not have an oscilloscope to verify
> > this myself, and in the following paragraphs, I will elaborate why this
> > seems confusing to me.
> 
> fsync-half-frame is just normal TDM for I2S, the default mode is how DSP
> mode normally operates.  I don't know that there's any pressing need to
> support mix'n'match here, you could but it should be through the TDM
> configuration API.
> 
> > So to answer the question, it's not part of the bus format because it
> > applies to three bus formats, and I'm completely out of my depth here and
> > wouldn't define three separate bus formats based on my own speculation of
> > how this works.
> 
> It is part of the bus format really.  I suspect the hardware is the kind
> that only really implements DSP mode and can just fake up a LRCLK for
> I2S in order to interoperate.

Thank you for your explanation!

Going forward, what would be a solution that is acceptable for upstream? As 
far as I understand, the obvious route here is to drop the rockchip,fsync-
half-frame property and just always set this mode when we're using a TDM bus 
format. Is this correct?

According to the TRM, the register bit this sets only affects TDM modes. 
Though since TDM is not standardised in any way from what I've read online, it 
is possible that there is hardware out there which expects the non-fsync-half-
frame mode, but I am completely fine with only thinking about this hardware 
when it actually surfaces.

Regards,
Nicolas Frattaroli



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding
  2021-09-19 17:38         ` Nicolas Frattaroli
@ 2021-09-20 11:49           ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2021-09-20 11:49 UTC (permalink / raw)
  To: Nicolas Frattaroli
  Cc: Liam Girdwood, Rob Herring, Heiko Stuebner, linux-rockchip,
	alsa-devel, devicetree, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

On Sun, Sep 19, 2021 at 07:38:47PM +0200, Nicolas Frattaroli wrote:

> Going forward, what would be a solution that is acceptable for upstream? As 
> far as I understand, the obvious route here is to drop the rockchip,fsync-
> half-frame property and just always set this mode when we're using a TDM bus 
> format. Is this correct?

Yes.

> According to the TRM, the register bit this sets only affects TDM modes. 
> Though since TDM is not standardised in any way from what I've read online, it 
> is possible that there is hardware out there which expects the non-fsync-half-
> frame mode, but I am completely fine with only thinking about this hardware 
> when it actually surfaces.

Right, we can figure it out later.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-09-20 11:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03 23:15 [PATCH v4 0/4] Rockchip I2S/TDM controller Nicolas Frattaroli
2021-09-03 23:15 ` [PATCH v4 2/4] dt-bindings: sound: add rockchip i2s-tdm binding Nicolas Frattaroli
2021-09-08 12:08   ` Rob Herring
2021-09-15 14:10   ` Mark Brown
2021-09-15 17:06     ` Nicolas Frattaroli
2021-09-16 12:25       ` Mark Brown
2021-09-19 17:38         ` Nicolas Frattaroli
2021-09-20 11:49           ` Mark Brown
2021-09-03 23:15 ` [PATCH v4 3/4] arm64: dts: rockchip: add i2s1 on rk356x Nicolas Frattaroli
2021-09-03 23:15 ` [PATCH v4 4/4] arm64: dts: rockchip: add analog audio on Quartz64 Nicolas Frattaroli
2021-09-07 13:40   ` Chris Morgan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).