* [PATCH 01/10] dt-bindings: mfd: Add documentation for ad242x
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-19 19:29 ` Rob Herring
2019-12-09 18:35 ` [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers Daniel Mack
` (10 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This patch adds documentation on the top-level MFD support for AD242x
devices. The bindings implemented by drivers for sub-devices of the
MFD are documented in other files in their respective subsystems.
The example in this file is referred to by other documents.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
.../bindings/mfd/adi,ad242x-bus.yaml | 29 +++
.../bindings/mfd/adi,ad242x-master.yaml | 235 ++++++++++++++++++
.../bindings/mfd/adi,ad242x-slave.yaml | 108 ++++++++
3 files changed, 372 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml
diff --git a/Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml b/Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml
new file mode 100644
index 000000000000..89ca8d009bb9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mfd/adi,ad242x-bus.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x A²B bus node
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+description: |
+ AD242x slave nodes represent the secondary I²C address a master node
+ transceiver exposes on the bus.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-bus
+
+ reg:
+ maxItems: 1
+ description: |
+ The secondary I²C address of the master node
+ (called 'BUS' in the datasheet)
+
+required:
+ - compatible
+ - reg
diff --git a/Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml b/Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml
new file mode 100644
index 000000000000..649510575a79
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml
@@ -0,0 +1,235 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mfd/adi,ad242x-master.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x A²B master node transceiver
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+description: |
+ AD242x devices are A²B (Automotive Audio Bus) transceivers that are connected
+ to each other in a daisy-chain. The payload transported on that bus includes
+ multi-channel audio, I²C, GPIOs and others.
+
+ The datasheet is located here:
+
+ https://www.analog.com/media/en/technical-documentation/user-guides/AD242x_TRM_Rev1.1.pdf
+
+ The primary node in the chain is called the master node, and the nodes in the
+ chain are called slave nodes. A master can address up to 15 slave nodes. The
+ master node exposes two I²C addresses, one for accessing the registers on the
+ node itself, and one for registers on one of the slave nodes.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-master
+
+ reg:
+ maxItems: 1
+ description: |
+ The primary I²C address of the master node
+ (called 'BASE' in the datasheet)
+
+ clocks:
+ minItems: 1
+
+ clock-names:
+ $ref: /schemas/types.yaml#/definitions/string-array
+ const: sync
+
+ clock-frequency:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ enum: [44100, 48000]
+ description: |
+ Specifies the clock frequency in Hz to configure on the given sync clock.
+ If not specified, the clock is expected to already be configured to either
+ 44100 or 48000 Hz.
+
+ interrupts:
+ maxItems: 1
+
+ adi,a2b-bus:
+ $ref: '/schemas/types.yaml#/definitions/phandle'
+ description: Specifies the bus handle node
+
+ adi,upstream-slot-size:
+ description: The size for upstream slots
+ allOf:
+ - $ref: '/schemas/types.yaml#/definitions/uint32'
+ - enum: [8, 12, 16, 20, 24, 28, 32]
+
+ adi,downstream-slot-size:
+ description: The size for downstream slots
+ allOf:
+ - $ref: '/schemas/types.yaml#/definitions/uint32'
+ - enum: [8, 12, 16, 20, 24, 28, 32]
+
+ adi,tdm-mode:
+ description: The TDM mode to use
+ allOf:
+ - $ref: '/schemas/types.yaml#/definitions/uint32'
+ - enum: [2, 4, 8, 12, 16, 20, 24, 32]
+
+ adi,tdm-slot-size:
+ description: The TDM slot size to use
+ allOf:
+ - $ref: '/schemas/types.yaml#/definitions/uint32'
+ - enum: [16, 32]
+
+ adi,alternate-upstream-slot-format:
+ description: Selects the alternate format for upstream slots
+ type: boolean
+
+ adi,alternate-downstream-slot-format:
+ description: Selects the alternate format for downstream slots
+ type: boolean
+
+ adi,invert-xcvr-b:
+ description: Inverts the LVDS XCVR B data line
+ type: boolean
+
+ adi,alternating-sync:
+ description: Drives the SYNC pin for I²S operation
+ type: boolean
+
+ adi,invert-sync:
+ description: Invert the SYNC pin
+ type: boolean
+
+ adi,early-sync:
+ description: |
+ Make the SYNC pin change one cycle before the first slot is transmitted
+ type: boolean
+
+ adi,spread-a2b-clock:
+ description: Enables spread spectrum mode for A²B bus clocks
+ type: boolean
+
+ adi,spread-a2b-i2s-clock:
+ description: Enables spread spectrum mode for both A²B and I²S clocks
+ type: boolean
+
+ adi,spread-spectrum-high:
+ description: Selects high spectrum spreading mode
+ type: boolean
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - adi,a2b-bus
+ - adi,upstream-slot-size
+ - adi,downstream-slot-size
+ - adi,tdm-mode
+ - adi,tdm-slot-size
+
+examples:
+ - |
+ sync_clock: clock {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <48000>;
+ };
+
+ i2c-bus {
+ ad2428w-master@68 {
+ reg = <0x68>;
+ compatible = "adi,ad2428w-master";
+ adi,a2b-bus = <&a2b_bus>;
+ clocks = <&sync_clock>;
+ clock-names = "sync";
+
+ adi,upstream-slot-size = <24>;
+ adi,downstream-slot-size = <24>;
+ adi,tdm-mode = <2>;
+ adi,tdm-slot-size = <32>;
+ adi,alternating-sync;
+ adi,early-sync;
+
+ codec {
+ compatible = "adi,ad2428w-codec";
+ #sound-dai-cells = <1>;
+ };
+
+ clock {
+ compatible = "adi,ad2428w-clk";
+ #clock-cells = <1>;
+ clock-output-names = "master-clk1", "master-clk2";
+ };
+
+ nodes {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ node@0 {
+ compatible = "adi,ad2428w-slave";
+ reg = <0>;
+
+ adi,alternating-sync;
+ adi,early-sync;
+ adi,invert-sync;
+ adi,tdm-mode = <8>;
+ adi,tdm-slot-size = <32>;
+
+ downstream {
+ rx-slots = <2 3 6 7 8 9>;
+ #tx-slots = <4>;
+ #forward-slots = <6>;
+ };
+
+ upstream {
+ rx-slots = <0 1 6 7 8 9>;
+ #tx-slots = <4>;
+ #forward-slots = <6>;
+ };
+
+ a2bgpio: gpio {
+ compatible = "adi,ad2428w-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ gpio-over-distance {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pin@0 {
+ reg = <0>;
+ adi,virtual-port-mask = <0x01>;
+ };
+ };
+ };
+
+ i2c {
+ compatible = "adi,ad2428w-i2c";
+ clock-frequency = <400000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ // I²C client devices located on the remote side
+ eeprom-top@52 {
+ reg = <0x52>;
+ compatible = "atmel,24c02";
+ read-only;
+ };
+ };
+
+ a2bclk: clock {
+ compatible = "adi,ad2428w-clk";
+ #clock-cells = <1>;
+ clock-output-names = "node0-clk1", "node0-clk2";
+ };
+
+ codec {
+ compatible = "adi,ad2428w-codec";
+ #sound-dai-cells = <1>;
+ adi,pdm-highpass-filter;
+ };
+ };
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml b/Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml
new file mode 100644
index 000000000000..3bea04dff267
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mfd/adi,ad242x-slave.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x A²B slave node transceiver
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+description: |
+ AD242x slave nodes are connected to the master node through a daisy-chain.
+ Modules of this type must be listed under the 'nodes' property of the master
+ DT schema.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-slave
+
+ adi,tdm-mode:
+ description: The TDM mode to use
+ allOf:
+ - $ref: '/schemas/types.yaml#/definitions/uint32'
+ - enum: [2, 4, 8, 12, 16, 20, 24, 32]
+
+ adi,tdm-slot-size:
+ description: The TDM slot size to use
+ allOf:
+ - $ref: '/schemas/types.yaml#/definitions/uint32'
+ - enum: [16, 32]
+
+ adi,alternating-sync:
+ description: Drives the SYNC pin for I²S operation
+ type: boolean
+
+ adi,invert-sync:
+ description: Invert the SYNC pin
+ type: boolean
+
+ adi,early-sync:
+ description: |
+ Make the SYNC pin change one cycle before the first slot is transmitted
+ type: boolean
+
+ adi,spread-a2b-clock:
+ description: Enables spread spectrum mode for A²B bus clocks
+ type: boolean
+
+ adi,spread-a2b-i2s-clock:
+ description: Enables spread spectrum mode for both A²B and I²S clocks
+ type: boolean
+
+ adi,spread-spectrum-high:
+ description: Selects high spectrum spreading mode
+ type: boolean
+
+ upstream:
+ type: object
+ properties:
+ rx-slots:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ A bitmask that describes the slots that are received by the
+ transceiver from the upstream (A) side and put into its TX output
+ framebuffers. If not specified, an empty bitmask is assumed.
+
+ '#tx-slots':
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The number of slots this transceiver contributes to the upstream
+ traffic from its RX input frame buffer
+
+ '#forward-slots':
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The number of slots this transceiver forwards from the upstream side
+ to the downstream side.
+
+ downstream:
+ type: object
+ properties:
+ rx-slots:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ A bitmask that describes the slots that are received by the
+ transceiver from the downstream (B) side and put into its TX output
+ framebuffers. If not specified, an empty bitmask is assumed.
+
+ '#tx-slots':
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The number of slots this transceiver contributes to the downstream
+ traffic from its RX input frame buffer
+
+ '#forward-slots':
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The number of slots this transceiver forwards from the downstream side
+ to the upstream side.
+
+required:
+ - compatible
+ - adi,tdm-mode
+ - adi,tdm-slot-size
+ - upstream
+ - downstream
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 01/10] dt-bindings: mfd: Add documentation for ad242x
2019-12-09 18:35 ` [PATCH 01/10] dt-bindings: mfd: Add documentation for ad242x Daniel Mack
@ 2019-12-19 19:29 ` Rob Herring
0 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2019-12-19 19:29 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, broonie, lee.jones, lars,
pascal.huerst
On Mon, Dec 09, 2019 at 07:35:01PM +0100, Daniel Mack wrote:
> This patch adds documentation on the top-level MFD support for AD242x
> devices. The bindings implemented by drivers for sub-devices of the
> MFD are documented in other files in their respective subsystems.
>
> The example in this file is referred to by other documents.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
> .../bindings/mfd/adi,ad242x-bus.yaml | 29 +++
> .../bindings/mfd/adi,ad242x-master.yaml | 235 ++++++++++++++++++
> .../bindings/mfd/adi,ad242x-slave.yaml | 108 ++++++++
> 3 files changed, 372 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml
> create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml
> create mode 100644 Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml b/Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml
> new file mode 100644
> index 000000000000..89ca8d009bb9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,ad242x-bus.yaml
> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mfd/adi,ad242x-bus.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Analog Devices AD242x A²B bus node
> +
> +maintainers:
> + - Daniel Mack <daniel@zonque.org>
> +
> +description: |
> + AD242x slave nodes represent the secondary I²C address a master node
> + transceiver exposes on the bus.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad2428w-bus
Where is this in the example?
Is A2B a standard thing? If so, then shouldn't some of this be split
into a A2B binding and then AD242x specific binding?
> +
> + reg:
> + maxItems: 1
> + description: |
> + The secondary I²C address of the master node
> + (called 'BUS' in the datasheet)
> +
> +required:
> + - compatible
> + - reg
> diff --git a/Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml b/Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml
> new file mode 100644
> index 000000000000..649510575a79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,ad242x-master.yaml
> @@ -0,0 +1,235 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mfd/adi,ad242x-master.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Analog Devices AD242x A²B master node transceiver
> +
> +maintainers:
> + - Daniel Mack <daniel@zonque.org>
> +
> +description: |
> + AD242x devices are A²B (Automotive Audio Bus) transceivers that are connected
> + to each other in a daisy-chain. The payload transported on that bus includes
> + multi-channel audio, I²C, GPIOs and others.
> +
> + The datasheet is located here:
> +
> + https://www.analog.com/media/en/technical-documentation/user-guides/AD242x_TRM_Rev1.1.pdf
> +
> + The primary node in the chain is called the master node, and the nodes in the
> + chain are called slave nodes. A master can address up to 15 slave nodes. The
> + master node exposes two I²C addresses, one for accessing the registers on the
> + node itself, and one for registers on one of the slave nodes.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad2428w-master
> +
> + reg:
> + maxItems: 1
> + description: |
> + The primary I²C address of the master node
> + (called 'BASE' in the datasheet)
> +
> + clocks:
> + minItems: 1
> +
> + clock-names:
> + $ref: /schemas/types.yaml#/definitions/string-array
> + const: sync
> +
> + clock-frequency:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + enum: [44100, 48000]
> + description: |
> + Specifies the clock frequency in Hz to configure on the given sync clock.
> + If not specified, the clock is expected to already be configured to either
> + 44100 or 48000 Hz.
> +
> + interrupts:
> + maxItems: 1
> +
> + adi,a2b-bus:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + description: Specifies the bus handle node
> +
> + adi,upstream-slot-size:
> + description: The size for upstream slots
> + allOf:
> + - $ref: '/schemas/types.yaml#/definitions/uint32'
> + - enum: [8, 12, 16, 20, 24, 28, 32]
> +
> + adi,downstream-slot-size:
> + description: The size for downstream slots
> + allOf:
> + - $ref: '/schemas/types.yaml#/definitions/uint32'
> + - enum: [8, 12, 16, 20, 24, 28, 32]
> +
> + adi,tdm-mode:
> + description: The TDM mode to use
> + allOf:
> + - $ref: '/schemas/types.yaml#/definitions/uint32'
> + - enum: [2, 4, 8, 12, 16, 20, 24, 32]
> +
> + adi,tdm-slot-size:
> + description: The TDM slot size to use
> + allOf:
> + - $ref: '/schemas/types.yaml#/definitions/uint32'
> + - enum: [16, 32]
> +
> + adi,alternate-upstream-slot-format:
> + description: Selects the alternate format for upstream slots
> + type: boolean
> +
> + adi,alternate-downstream-slot-format:
> + description: Selects the alternate format for downstream slots
> + type: boolean
> +
> + adi,invert-xcvr-b:
> + description: Inverts the LVDS XCVR B data line
> + type: boolean
> +
> + adi,alternating-sync:
> + description: Drives the SYNC pin for I²S operation
> + type: boolean
> +
> + adi,invert-sync:
> + description: Invert the SYNC pin
> + type: boolean
> +
> + adi,early-sync:
> + description: |
> + Make the SYNC pin change one cycle before the first slot is transmitted
> + type: boolean
> +
> + adi,spread-a2b-clock:
> + description: Enables spread spectrum mode for A²B bus clocks
> + type: boolean
> +
> + adi,spread-a2b-i2s-clock:
> + description: Enables spread spectrum mode for both A²B and I²S clocks
> + type: boolean
> +
> + adi,spread-spectrum-high:
> + description: Selects high spectrum spreading mode
> + type: boolean
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - adi,a2b-bus
> + - adi,upstream-slot-size
> + - adi,downstream-slot-size
> + - adi,tdm-mode
> + - adi,tdm-slot-size
> +
> +examples:
> + - |
> + sync_clock: clock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <48000>;
> + };
> +
> + i2c-bus {
> + ad2428w-master@68 {
> + reg = <0x68>;
> + compatible = "adi,ad2428w-master";
> + adi,a2b-bus = <&a2b_bus>;
> + clocks = <&sync_clock>;
> + clock-names = "sync";
> +
> + adi,upstream-slot-size = <24>;
> + adi,downstream-slot-size = <24>;
> + adi,tdm-mode = <2>;
> + adi,tdm-slot-size = <32>;
> + adi,alternating-sync;
> + adi,early-sync;
> +
> + codec {
> + compatible = "adi,ad2428w-codec";
> + #sound-dai-cells = <1>;
> + };
> +
> + clock {
> + compatible = "adi,ad2428w-clk";
> + #clock-cells = <1>;
> + clock-output-names = "master-clk1", "master-clk2";
> + };
> +
> + nodes {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + node@0 {
> + compatible = "adi,ad2428w-slave";
> + reg = <0>;
> +
> + adi,alternating-sync;
> + adi,early-sync;
> + adi,invert-sync;
> + adi,tdm-mode = <8>;
> + adi,tdm-slot-size = <32>;
> +
> + downstream {
> + rx-slots = <2 3 6 7 8 9>;
> + #tx-slots = <4>;
> + #forward-slots = <6>;
> + };
> +
> + upstream {
> + rx-slots = <0 1 6 7 8 9>;
> + #tx-slots = <4>;
> + #forward-slots = <6>;
> + };
> +
> + a2bgpio: gpio {
> + compatible = "adi,ad2428w-gpio";
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> + gpio-over-distance {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pin@0 {
> + reg = <0>;
> + adi,virtual-port-mask = <0x01>;
> + };
> + };
> + };
> +
> + i2c {
> + compatible = "adi,ad2428w-i2c";
> + clock-frequency = <400000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + // I²C client devices located on the remote side
> + eeprom-top@52 {
> + reg = <0x52>;
> + compatible = "atmel,24c02";
> + read-only;
> + };
> + };
> +
> + a2bclk: clock {
> + compatible = "adi,ad2428w-clk";
> + #clock-cells = <1>;
> + clock-output-names = "node0-clk1", "node0-clk2";
> + };
> +
> + codec {
> + compatible = "adi,ad2428w-codec";
> + #sound-dai-cells = <1>;
> + adi,pdm-highpass-filter;
> + };
> + };
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml b/Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml
> new file mode 100644
> index 000000000000..3bea04dff267
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,ad242x-slave.yaml
> @@ -0,0 +1,108 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mfd/adi,ad242x-slave.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Analog Devices AD242x A²B slave node transceiver
> +
> +maintainers:
> + - Daniel Mack <daniel@zonque.org>
> +
> +description: |
> + AD242x slave nodes are connected to the master node through a daisy-chain.
> + Modules of this type must be listed under the 'nodes' property of the master
> + DT schema.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad2428w-slave
> +
> + adi,tdm-mode:
> + description: The TDM mode to use
> + allOf:
> + - $ref: '/schemas/types.yaml#/definitions/uint32'
> + - enum: [2, 4, 8, 12, 16, 20, 24, 32]
> +
> + adi,tdm-slot-size:
> + description: The TDM slot size to use
> + allOf:
> + - $ref: '/schemas/types.yaml#/definitions/uint32'
> + - enum: [16, 32]
> +
> + adi,alternating-sync:
> + description: Drives the SYNC pin for I²S operation
> + type: boolean
> +
> + adi,invert-sync:
> + description: Invert the SYNC pin
> + type: boolean
> +
> + adi,early-sync:
> + description: |
> + Make the SYNC pin change one cycle before the first slot is transmitted
> + type: boolean
> +
> + adi,spread-a2b-clock:
> + description: Enables spread spectrum mode for A²B bus clocks
> + type: boolean
> +
> + adi,spread-a2b-i2s-clock:
> + description: Enables spread spectrum mode for both A²B and I²S clocks
> + type: boolean
> +
> + adi,spread-spectrum-high:
> + description: Selects high spectrum spreading mode
> + type: boolean
> +
> + upstream:
> + type: object
> + properties:
> + rx-slots:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + A bitmask that describes the slots that are received by the
> + transceiver from the upstream (A) side and put into its TX output
> + framebuffers. If not specified, an empty bitmask is assumed.
> +
> + '#tx-slots':
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + The number of slots this transceiver contributes to the upstream
> + traffic from its RX input frame buffer
> +
> + '#forward-slots':
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + The number of slots this transceiver forwards from the upstream side
> + to the downstream side.
> +
> + downstream:
> + type: object
> + properties:
> + rx-slots:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + A bitmask that describes the slots that are received by the
> + transceiver from the downstream (B) side and put into its TX output
> + framebuffers. If not specified, an empty bitmask is assumed.
> +
> + '#tx-slots':
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + The number of slots this transceiver contributes to the downstream
> + traffic from its RX input frame buffer
> +
> + '#forward-slots':
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + The number of slots this transceiver forwards from the downstream side
> + to the upstream side.
> +
> +required:
> + - compatible
> + - adi,tdm-mode
> + - adi,tdm-slot-size
> + - upstream
> + - downstream
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
2019-12-09 18:35 ` [PATCH 01/10] dt-bindings: mfd: Add documentation for ad242x Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2020-01-08 3:45 ` Rob Herring
2019-12-09 18:35 ` [PATCH 03/10] dt-bindings: gpio: Add documentation for ad242x GPIO controllers Daniel Mack
` (9 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This device must be placed as a sub-device of an AD242x MFD node.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
.../bindings/i2c/adi,ad242x-i2c.yaml | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
diff --git a/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
new file mode 100644
index 000000000000..ded92f8a791b
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/i2c/adi,ad242x-i2c.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x I2C controller
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-i2c
+
+ clock-frequency:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ default: 100000
+ enum: [100000, 400000]
+ description: Specifies the I²C clock frequency in Hz.
+
+required:
+ - compatible
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers
2019-12-09 18:35 ` [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers Daniel Mack
@ 2020-01-08 3:45 ` Rob Herring
0 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2020-01-08 3:45 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, broonie, lee.jones, lars,
pascal.huerst
On Mon, Dec 09, 2019 at 07:35:02PM +0100, Daniel Mack wrote:
> This device must be placed as a sub-device of an AD242x MFD node.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
> .../bindings/i2c/adi,ad242x-i2c.yaml | 31 +++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
>
> diff --git a/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
> new file mode 100644
> index 000000000000..ded92f8a791b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/adi,ad242x-i2c.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/i2c/adi,ad242x-i2c.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Analog Devices AD242x I2C controller
> +
> +maintainers:
> + - Daniel Mack <daniel@zonque.org>
> +
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +description: |
> + This module is part of the AD242x MFD device. For more details and an example
> + refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad2428w-i2c
> +
> + clock-frequency:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
Can drop as it already has a type.
> + default: 100000
> + enum: [100000, 400000]
> + description: Specifies the I²C clock frequency in Hz.
> +
> +required:
> + - compatible
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 03/10] dt-bindings: gpio: Add documentation for ad242x GPIO controllers
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
2019-12-09 18:35 ` [PATCH 01/10] dt-bindings: mfd: Add documentation for ad242x Daniel Mack
2019-12-09 18:35 ` [PATCH 02/10] dt-bindings: i2c: Add documentation for ad242x i2c controllers Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-09 18:35 ` [PATCH 03/10] dt-bindings: gpio: Add documentation for AD242x " Daniel Mack
` (8 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This device must be place as a sub-device of an AD242x MFD node.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
.../bindings/gpio/adi,ad242x-gpio.yaml | 65 +++++++++++++++++++
1 file changed, 65 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml
diff --git a/Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml b/Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml
new file mode 100644
index 000000000000..0a5a339fc84e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/gpio/adi,ad242x-gpio.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x GPIO controller
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-gpio
+
+ "#gpio-cells":
+ const: 2
+
+ gpio-controller: true
+
+ gpio-over-distance:
+ type: object
+ description: |
+ Sub-node to configure pins as 'GPIO over distance'.
+ Pins in this mode are not accessible as regular GPIOs; instead, their
+ state is transparantly mirrored between one or multiple nodes.
+
+ Each child node of the 'gpio-over-distance' node describes one pin
+ that is to be configured in 'over distance' mode
+
+ properties:
+ pin:
+ type: object
+ properties:
+ reg:
+ maxItems: 1
+
+ adi,virtual-port-mask:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The virtual port mask to assign this GPIO to.
+ Multiple GPIOs can use the same virtual port to link them
+ together. Refer to the datasheet for the details.
+
+ adi,gpio-output:
+ type: boolean
+ description: Configures this GPIO as output. Defaults to input mode.
+
+ adi,gpio-inverted:
+ type: boolean
+ description: Inverts the GPIO value
+
+ required:
+ - reg
+ - adi,virtual-port-mask
+
+required:
+ - compatible
+ - '#gpio-cells'
+ - gpio-controller
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 03/10] dt-bindings: gpio: Add documentation for AD242x GPIO controllers
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (2 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 03/10] dt-bindings: gpio: Add documentation for ad242x GPIO controllers Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-09 18:35 ` [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers Daniel Mack
` (7 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This device must be place as a sub-device of an AD242x MFD node.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
.../bindings/gpio/adi,ad242x-gpio.yaml | 65 +++++++++++++++++++
1 file changed, 65 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml
diff --git a/Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml b/Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml
new file mode 100644
index 000000000000..0a5a339fc84e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/adi,ad242x-gpio.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/gpio/adi,ad242x-gpio.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x GPIO controller
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-gpio
+
+ "#gpio-cells":
+ const: 2
+
+ gpio-controller: true
+
+ gpio-over-distance:
+ type: object
+ description: |
+ Sub-node to configure pins as 'GPIO over distance'.
+ Pins in this mode are not accessible as regular GPIOs; instead, their
+ state is transparantly mirrored between one or multiple nodes.
+
+ Each child node of the 'gpio-over-distance' node describes one pin
+ that is to be configured in 'over distance' mode
+
+ properties:
+ pin:
+ type: object
+ properties:
+ reg:
+ maxItems: 1
+
+ adi,virtual-port-mask:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The virtual port mask to assign this GPIO to.
+ Multiple GPIOs can use the same virtual port to link them
+ together. Refer to the datasheet for the details.
+
+ adi,gpio-output:
+ type: boolean
+ description: Configures this GPIO as output. Defaults to input mode.
+
+ adi,gpio-inverted:
+ type: boolean
+ description: Inverts the GPIO value
+
+ required:
+ - reg
+ - adi,virtual-port-mask
+
+required:
+ - compatible
+ - '#gpio-cells'
+ - gpio-controller
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (3 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 03/10] dt-bindings: gpio: Add documentation for AD242x " Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-24 7:32 ` Stephen Boyd
2019-12-09 18:35 ` [PATCH 05/10] dt-bindings: sound: Add documentation for AD242x codecs Daniel Mack
` (6 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This device must be placed as a sub-device of an AD242x MFD node.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
.../bindings/clock/adi,ad242x-clk.yaml | 32 +++++++++++++++++++
include/dt-bindings/clock/adi,ad242x.h | 9 ++++++
2 files changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
create mode 100644 include/dt-bindings/clock/adi,ad242x.h
diff --git a/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
new file mode 100644
index 000000000000..f434b3e4928e
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/clock/adi,ad242x-clk.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x clock provider
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-clk
+
+ '#clock-cells':
+ const: 1
+
+ clock-output-names:
+ minItems: 2
+ maxItems: 2
+ description: |
+ Array of two strings to use as names for the generated output clocks
+
+required:
+ - compatible
+ - '#clock-cells'
\ No newline at end of file
diff --git a/include/dt-bindings/clock/adi,ad242x.h b/include/dt-bindings/clock/adi,ad242x.h
new file mode 100644
index 000000000000..307a6cd1f5a6
--- /dev/null
+++ b/include/dt-bindings/clock/adi,ad242x.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __DT_BINDINGS_AD242X_CLK_H
+#define __DT_BINDINGS_AD242X_CLK_H
+
+#define MAX9485_CLKOUT1 0
+#define MAX9485_CLKOUT2 1
+
+#endif /* __DT_BINDINGS_AD242X_CLK_H */
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers
2019-12-09 18:35 ` [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers Daniel Mack
@ 2019-12-24 7:32 ` Stephen Boyd
0 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-12-24 7:32 UTC (permalink / raw)
To: Daniel Mack, alsa-devel, devicetree, linux-clk, linux-gpio,
linux-i2c, linux-kernel
Cc: mturquette, robh+dt, broonie, lee.jones, lars, pascal.huerst,
Daniel Mack
Quoting Daniel Mack (2019-12-09 10:35:05)
> diff --git a/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
> new file mode 100644
> index 000000000000..f434b3e4928e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/adi,ad242x-clk.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/clock/adi,ad242x-clk.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Analog Devices AD242x clock provider
> +
> +maintainers:
> + - Daniel Mack <daniel@zonque.org>
> +
> +description: |
> + This module is part of the AD242x MFD device. For more details and an example
> + refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
I think we usually leave off Documentation/devicetree/ from paths when
they're inside the bindings directory.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad2428w-clk
> +
> + '#clock-cells':
> + const: 1
> +
> + clock-output-names:
> + minItems: 2
> + maxItems: 2
> + description: |
> + Array of two strings to use as names for the generated output clocks
> +
> +required:
> + - compatible
> + - '#clock-cells'
> \ No newline at end of file
Why no newline at end of file? Is there an example?
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 05/10] dt-bindings: sound: Add documentation for AD242x codecs
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (4 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 04/10] dt-bindings: clock: Add documentation for AD242x clock providers Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-09 18:35 ` [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers Daniel Mack
` (5 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This device must be placed as a sub-device of an AD242x MFD node.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
.../bindings/sound/adi,ad242x-codec.yaml | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml
diff --git a/Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml b/Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml
new file mode 100644
index 000000000000..2cfb6f9fc548
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/adi,ad242x-codec.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/sound/adi,ad242x-codec.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Analog Devices AD242x clock provider
+
+maintainers:
+ - Daniel Mack <daniel@zonque.org>
+
+description: |
+ This module is part of the AD242x MFD device. For more details and an example
+ refer to Documentation/devicetree/bindings/mfd/ad242x.yaml.
+
+properties:
+ compatible:
+ enum:
+ - adi,ad2428w-codec
+
+ '#sound-dai-cells':
+ const: 1
+
+ adi,pdm-highpass-filter:
+ type: boolean
+ description: |
+ Enables highpass filtering for data received through the PDM ports
+
+required:
+ - compatible
+ - '#sound-dai-cells'
\ No newline at end of file
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (5 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 05/10] dt-bindings: sound: Add documentation for AD242x codecs Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-17 13:39 ` Lee Jones
2019-12-17 19:16 ` [alsa-devel] " Pierre-Louis Bossart
2019-12-09 18:35 ` [PATCH 07/10] i2c: Add driver for AD242x bus controller Daniel Mack
` (4 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
The core driver for these devices is split into several parts.
The master node driver is an I2C client. It is responsible for
bringing up the bus topology and discovering the slave nodes.
This process requries some knowlegde of the slave node configuration
to program the bus timings correctly, so the master drivers walks
the tree of nodes in the devicetree. The slave driver handles platform
devices that are instantiated by the master node driver after
discovery has finished.
Master nodes expose two addresses on the I2C bus, one (referred to as
'BASE' in the datasheet) for accessing registers on the transceiver
node itself, and one (referred to as 'BUS') for accessing remote
registers, either on the remote transceiver itself, or on I2C hardware
connected to that remote transceiver, which then acts as a remote I2C
bus master.
In order to allow MFD sub-devices to be registered as children of
either the master or any slave node, the details on how to access the
registers are hidden behind a regmap config. A pointer to the regmap
is then exposed in the struct shared with the sub-devices.
The ad242x-bus driver is a simple proxy that occupies the BUS I2C
address and which is referred to through a devicetree handle by the
master driver.
For the discovery process, the driver has to wait for an interrupt
to occur. In case no interrupt is configured in DT, the driver falls
back to interrupt polling. After the discovery phase is completed,
interrupts are only needed for error handling and GPIO handling,
both of which is not currenty implemented.
Code common to both the master and the slave driver lives in
'ad242x-node.c'.
Signed-off-by: Daniel Mack <daniel@zonque.org>
mfd
---
drivers/mfd/Kconfig | 11 +
drivers/mfd/Makefile | 1 +
drivers/mfd/ad242x-bus.c | 42 +++
drivers/mfd/ad242x-master.c | 611 ++++++++++++++++++++++++++++++++++++
drivers/mfd/ad242x-node.c | 262 ++++++++++++++++
drivers/mfd/ad242x-slave.c | 234 ++++++++++++++
include/linux/mfd/ad242x.h | 400 +++++++++++++++++++++++
7 files changed, 1561 insertions(+)
create mode 100644 drivers/mfd/ad242x-bus.c
create mode 100644 drivers/mfd/ad242x-master.c
create mode 100644 drivers/mfd/ad242x-node.c
create mode 100644 drivers/mfd/ad242x-slave.c
create mode 100644 include/linux/mfd/ad242x.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 420900852166..727a35053d8c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -99,6 +99,17 @@ config PMIC_ADP5520
individual components like LCD backlight, LEDs, GPIOs and Kepad
under the corresponding menus.
+config MFD_AD242X
+ bool "Analog Devices AD242x A2B support"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=y && OF
+ help
+ If you say yes here, you get support for devices from the AD242x
+ familiy. This driver provides common support for accessing the
+ devices, additional drivers must be enabled in order to use the
+ functionality of the devices.
+
config MFD_AAT2870_CORE
bool "AnalogicTech AAT2870"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index aed99f08739f..2361c676f6c8 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -203,6 +203,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC) += qcom-spmi-pmic.o
obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o
obj-$(CONFIG_MFD_TPS65090) += tps65090.o
obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o
+obj-$(CONFIG_MFD_AD242X) += ad242x-master.o ad242x-slave.o ad242x-bus.o ad242x-node.o
obj-$(CONFIG_MFD_AT91_USART) += at91-usart.o
obj-$(CONFIG_MFD_ATMEL_FLEXCOM) += atmel-flexcom.o
obj-$(CONFIG_MFD_ATMEL_HLCDC) += atmel-hlcdc.o
diff --git a/drivers/mfd/ad242x-bus.c b/drivers/mfd/ad242x-bus.c
new file mode 100644
index 000000000000..6660e13ce43d
--- /dev/null
+++ b/drivers/mfd/ad242x-bus.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+static int ad242x_bus_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ dev_set_drvdata(&i2c->dev, i2c);
+ i2c_set_clientdata(i2c, &i2c->dev);
+ return 0;
+}
+
+static const struct of_device_id ad242x_bus_of_match[] = {
+ { .compatible = "adi,ad2428w-bus" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ad242x_bus_of_match);
+
+static const struct i2c_device_id ad242x_bus_i2c_id[] = {
+ { "ad242x_bus", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, ad242x_bus_i2c_id);
+
+static struct i2c_driver ad242x_bus_i2c_driver = {
+ .driver = {
+ .name = "ad242x-bus",
+ .of_match_table = ad242x_bus_of_match,
+ },
+ .probe = ad242x_bus_i2c_probe,
+ .id_table = ad242x_bus_i2c_id,
+};
+
+module_i2c_driver(ad242x_bus_i2c_driver);
+
+MODULE_DESCRIPTION("AD242x bus driver");
+MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mfd/ad242x-master.c b/drivers/mfd/ad242x-master.c
new file mode 100644
index 000000000000..1b0bf90442a2
--- /dev/null
+++ b/drivers/mfd/ad242x-master.c
@@ -0,0 +1,611 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+struct ad242x_master {
+ struct ad242x_node node;
+ struct clk *sync_clk;
+ struct completion run_completion;
+ struct completion discover_completion;
+ struct ad242x_i2c_bus bus;
+ unsigned int up_slot_size;
+ unsigned int dn_slot_size;
+ bool up_slot_alt_fmt;
+ bool dn_slot_alt_fmt;
+ unsigned int sync_clk_rate;
+ int irq;
+ u8 response_cycles;
+};
+
+struct ad242x_node *ad242x_master_get_node(struct ad242x_master *master)
+{
+ return &master->node;
+}
+EXPORT_SYMBOL_GPL(ad242x_master_get_node);
+
+struct ad242x_i2c_bus *ad242x_master_get_bus(struct ad242x_master *master)
+{
+ return &master->bus;
+}
+EXPORT_SYMBOL_GPL(ad242x_master_get_bus);
+
+const char *ad242x_master_get_clk_name(struct ad242x_master *master)
+{
+ return __clk_get_name(master->sync_clk);
+}
+EXPORT_SYMBOL_GPL(ad242x_master_get_clk_name);
+
+unsigned int ad242x_master_get_clk_rate(struct ad242x_master *master)
+{
+ return master->sync_clk_rate;
+}
+EXPORT_SYMBOL_GPL(ad242x_master_get_clk_rate);
+
+static int ad242x_read_one_irq(struct ad242x_master *master)
+{
+ struct regmap *regmap = master->node.regmap;
+ struct device *dev = master->node.dev;
+ unsigned int val, inttype;
+ int ret;
+
+ ret = regmap_read(regmap, AD242X_INTSTAT, &val);
+ if (ret < 0) {
+ dev_err(dev, "unable to read INTSTAT register: %d\n", ret);
+ return ret;
+ }
+
+ if (!(val & AD242X_INTSTAT_IRQ))
+ return -ENOENT;
+
+ ret = regmap_read(regmap, AD242X_INTTYPE, &inttype);
+ if (ret < 0) {
+ dev_err(dev, "unable to read INTTYPE register: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_read(regmap, AD242X_INTSRC, &val);
+ if (ret < 0) {
+ dev_err(dev, "unable to read INTSRC register: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_read(regmap, AD242X_INTPND0, &val);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_INTPND0, val);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(regmap, AD242X_INTPND1, &val);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_INTPND1, val);
+ if (ret < 0)
+ return ret;
+
+ if (val & AD242X_INTSRC_MSTINT) {
+ ret = regmap_read(regmap, AD242X_INTPND2, &val);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_INTPND2, val);
+ if (ret < 0)
+ return ret;
+ }
+
+ dev_err(dev, "%s() inttype: 0x%02x\n", __func__, inttype);
+
+ switch (inttype) {
+ case AD242X_INTTYPE_DSCDONE:
+ complete(&master->discover_completion);
+ break;
+ case AD242X_INTTYPE_MSTR_RUNNING:
+ complete(&master->run_completion);
+ break;
+ default:
+ dev_info(dev, "Unhandled interrupt type 0x%02x\n", inttype);
+ }
+
+ return 0;
+}
+
+static int ad242x_read_irqs(struct ad242x_master *master)
+{
+ int ret;
+ bool first = true;
+
+ while (true) {
+ ret = ad242x_read_one_irq(master);
+ if (ret < 0)
+ return ret;
+ if (ret == -ENOENT)
+ return first ? ret : 0;
+
+ first = false;
+ }
+}
+
+static irqreturn_t ad242x_handle_irq(int irq, void *devid)
+{
+ struct ad242x_master *master = devid;
+ int ret;
+
+ ret = ad242x_read_irqs(master);
+ if (ret == -ENOENT)
+ return IRQ_NONE;
+
+ return IRQ_HANDLED;
+}
+
+static int ad242x_wait_for_irq(struct ad242x_master *master,
+ struct completion *completion,
+ unsigned int timeout)
+{
+ int ret;
+
+ if (master->irq > 0) {
+ ret = wait_for_completion_timeout(completion,
+ msecs_to_jiffies(timeout));
+ } else {
+ usleep_range(timeout * 1000, timeout * 1500);
+ ad242x_read_irqs(master);
+ ret = completion_done(completion);
+ }
+
+ return ret == 0 ? -ETIMEDOUT : 0;
+}
+
+/* See Table 3-2 in the datasheet */
+static unsigned int ad242x_bus_bits(unsigned int slot_size, bool alt_fmt)
+{
+ int alt_bits[8] = { 0, 13, 17, 21, 30, 0, 39, 0 };
+ int idx = AD242X_SLOTFMT_DNSIZE(slot_size);
+
+ return alt_fmt ? alt_bits[idx] : slot_size + 1;
+}
+
+/* See Table 9-1 in the datasheet */
+static unsigned int ad242x_master_respoffs(struct ad242x_node *node)
+{
+ if (node->tdm_mode == 2 && node->tdm_slot_size == 16)
+ return 238;
+
+ if ((node->tdm_mode == 2 && node->tdm_slot_size == 32) ||
+ (node->tdm_mode == 4 && node->tdm_slot_size == 16))
+ return 245;
+
+ return 248;
+}
+
+static int ad242x_discover(struct ad242x_master *master,
+ struct device_node *nodes_np)
+{
+ struct regmap *regmap = master->node.regmap;
+ struct device *dev = master->node.dev;
+ struct device_node *child_np;
+ unsigned int val, n = 0, i, respoffs, respcycs;
+ unsigned int respcycs_up_min = UINT_MAX;
+ unsigned int respcycs_dn_max = 0;
+ unsigned int master_up_slots = 0;
+ unsigned int master_dn_slots = 0;
+ bool up_enabled = false, dn_enabled = false;
+ uint8_t slave_control = 0;
+ int ret;
+
+ respoffs = ad242x_master_respoffs(&master->node);
+
+ for_each_available_child_of_node(nodes_np, child_np) {
+ unsigned int dnslot_activity, upslot_activity;
+ unsigned int slave_dn_slots, slave_up_slots;
+ unsigned int respcycs_dn, respcycs_up;
+ struct ad242x_slot_config slot_config;
+
+ ret = ad242x_read_slot_config(dev, child_np, &slot_config);
+ if (ret < 0) {
+ dev_err(dev, "slot config of slave %d is invalid\n", n);
+ return ret;
+ }
+
+ /* See section 3-18 in the datasheet */
+ slave_dn_slots = max_t(int, slot_config.dn_n_forward_slots,
+ fls(slot_config.dn_rx_slots));
+ slave_up_slots = max_t(int, slot_config.up_n_forward_slots,
+ fls(slot_config.up_rx_slots));
+
+ if (n == 0) {
+ master_up_slots = slave_up_slots;
+ master_dn_slots = slave_dn_slots;
+ }
+
+ /* See Appendix B in the datasheet */
+ dnslot_activity = slave_dn_slots *
+ ad242x_bus_bits(master->dn_slot_size,
+ master->dn_slot_alt_fmt);
+ upslot_activity = slave_up_slots *
+ ad242x_bus_bits(master->up_slot_size,
+ master->up_slot_alt_fmt);
+
+ respcycs_dn = DIV_ROUND_UP(64 + dnslot_activity, 4) + 4*n + 2;
+ respcycs_up = respoffs -
+ (DIV_ROUND_UP(64 + upslot_activity, 4) + 1);
+
+ if (respcycs_dn > respcycs_dn_max)
+ respcycs_dn_max = respcycs_dn;
+
+ if (respcycs_up < respcycs_up_min)
+ respcycs_up_min = respcycs_up;
+
+ if (slave_dn_slots > 0)
+ dn_enabled = true;
+
+ if (slave_up_slots > 0)
+ up_enabled = true;
+
+ n++;
+ }
+
+ if (n == 0) {
+ dev_err(dev, "No child nodes specified\n");
+ return -EINVAL;
+ }
+
+ if (of_property_read_bool(dev->of_node, "adi,invert-xcvr-b")) {
+ ret = regmap_update_bits(regmap, AD242X_CONTROL,
+ AD242X_CONTROL_XCVRBINV,
+ AD242X_CONTROL_XCVRBINV);
+ if (ret < 0)
+ return ret;
+
+ slave_control = AD242X_CONTROL_XCVRBINV;
+ }
+
+ if (respcycs_dn_max > respcycs_up_min) {
+ dev_err(dev, "Unsupported bus topology\n");
+ return -EINVAL;
+ }
+
+ respcycs = (respcycs_dn_max + respcycs_up_min) / 2;
+ ret = regmap_write(regmap, AD242X_RESPCYCS, respcycs);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_CONTROL,
+ AD242X_CONTROL_NEWSTRCT,
+ AD242X_CONTROL_NEWSTRCT);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_SWCTL, AD242X_SWCTL_ENSW);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < n; i++) {
+ ret = regmap_write(regmap, AD242X_DISCVRY, respcycs - (4*i));
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_wait_for_irq(master,
+ &master->discover_completion, 35);
+ if (ret < 0) {
+ dev_err(dev, "Discovery of node %d timed out\n", i);
+ return ret;
+ }
+
+ val = AD242X_SWCTL_MODE(2) | AD242X_SWCTL_ENSW;
+
+ if (i == 0)
+ ret = regmap_write(regmap, AD242X_SWCTL, val);
+ else
+ ret = ad242x_slave_write(&master->bus, regmap, i,
+ AD242X_SWCTL, val);
+
+ if (ret < 0)
+ return ret;
+
+ dev_info(dev, "Node %d discovered\n", i);
+
+ /* Last node? */
+ if (i == n - 1)
+ break;
+
+ ret = ad242x_slave_write(&master->bus, regmap, i,
+ AD242X_INTMSK2,
+ AD242X_INTMSK2_DSCDIEN);
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_slave_write(&master->bus, regmap, i,
+ AD242X_CONTROL, slave_control);
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_slave_write(&master->bus, regmap, i,
+ AD242X_SWCTL, AD242X_SWCTL_ENSW);
+ if (ret < 0)
+ return ret;
+
+ reinit_completion(&master->discover_completion);
+ }
+
+ ret = regmap_write(regmap, AD242X_DNSLOTS, master_dn_slots);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_UPSLOTS, master_up_slots);
+ if (ret < 0)
+ return ret;
+
+ val = 0;
+ if (dn_enabled)
+ val |= AD242X_DATCTL_DNS;
+
+ if (up_enabled)
+ val |= AD242X_DATCTL_UPS;
+
+ ret = regmap_write(regmap, AD242X_DATCTL, val);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int ad242x_init_irq(struct ad242x_master *master)
+{
+ struct regmap *regmap = master->node.regmap;
+ struct device *dev = master->node.dev;
+ int ret;
+
+ if (master->irq > 0) {
+ ret = devm_request_threaded_irq(dev, master->irq, NULL,
+ ad242x_handle_irq, IRQF_ONESHOT,
+ dev_name(dev), master);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = regmap_write(regmap, AD242X_INTMSK0,
+ AD242X_INTMSK0_SRFEIEN | AD242X_INTMSK0_BECIEN |
+ AD242X_INTMSK0_PWREIEN | AD242X_INTMSK0_CRCEIEN |
+ AD242X_INTMSK0_DDEIEN | AD242X_INTMSK0_HCEIEN);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_INTMSK2,
+ AD242X_INTMSK2_DSCDIEN | AD242X_INTMSK2_SLVIRQEN);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static const struct regmap_config ad242x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_reg = ad242x_is_volatile_reg,
+ .writeable_reg = ad242x_is_writeable_reg,
+ .max_register = AD242X_MAX_REG,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int ad242x_master_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct device_node *bus_np, *nodes_np, *np;
+ struct device *busdev, *dev = &i2c->dev;
+ struct ad242x_master *master;
+ struct regmap *regmap;
+ unsigned int val;
+ int ret;
+
+ nodes_np = of_get_child_by_name(dev->of_node, "nodes");
+ if (!nodes_np) {
+ dev_err(dev, "no 'nodes' property given\n");
+ return -EINVAL;
+ }
+
+ bus_np = of_parse_phandle(dev->of_node, "adi,a2b-bus", 0);
+ if (!bus_np) {
+ dev_err(dev, "no 'adi,a2b-bus' handle specified for master node\n");
+ return -EINVAL;
+ }
+
+ busdev = bus_find_device_by_of_node(&i2c_bus_type, bus_np);
+ if (!busdev) {
+ dev_err(dev, "'adi,a2b-bus' handle invalid\n");
+ return -EINVAL;
+ }
+
+ master = devm_kzalloc(dev, sizeof(struct ad242x_master), GFP_KERNEL);
+ if (!master)
+ return -ENOMEM;
+
+ mutex_init(&master->bus.mutex);
+ init_completion(&master->run_completion);
+ init_completion(&master->discover_completion);
+ dev_set_drvdata(dev, &master->node);
+ i2c_set_clientdata(i2c, master);
+
+ regmap = devm_regmap_init_i2c(i2c, &ad242x_regmap_config);
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ dev_err(dev, "regmap init failed: %d\n", ret);
+ return ret;
+ }
+
+ master->bus.client = to_i2c_client(busdev);
+ master->node.regmap = regmap;
+ master->node.dev = dev;
+ master->node.master = master;
+ master->node.id = AD242X_MASTER_ID;
+ master->irq = i2c->irq;
+
+ master->sync_clk = devm_clk_get(dev, "sync");
+ if (IS_ERR(master->sync_clk)) {
+ ret = PTR_ERR(master->sync_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to get sync clk: %d\n", ret);
+
+ return ret;
+ }
+
+ if (of_property_read_u32(dev->of_node, "clock-frequency",
+ &master->sync_clk_rate)) {
+ ret = clk_set_rate(master->sync_clk, master->sync_clk_rate);
+ if (ret < 0) {
+ dev_err(dev, "Cannot set sync clock rate: %d\n", ret);
+ return ret;
+ }
+ }
+
+ master->sync_clk_rate = clk_get_rate(master->sync_clk);
+ if (master->sync_clk_rate != 44100 && master->sync_clk_rate != 48000) {
+ dev_err(dev, "SYNC clock rate %d is invalid\n",
+ master->sync_clk_rate);
+ return -EINVAL;
+ }
+
+ ret = clk_prepare_enable(master->sync_clk);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable sync clk: %d\n", ret);
+ return ret;
+ }
+
+ /* Master node setup */
+
+ ret = regmap_write(regmap, AD242X_CONTROL,
+ AD242X_CONTROL_MSTR | AD242X_CONTROL_SOFTRST);
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_wait_for_irq(master, &master->run_completion, 10);
+ if (ret < 0) {
+ dev_err(dev, "timeout waiting for PLL sync: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_update_bits(regmap, AD242X_CONTROL,
+ AD242X_CONTROL_SOFTRST, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_node_probe(&master->node);
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_init_irq(master);
+ if (ret < 0) {
+ dev_err(dev, "Unable to set up IRQ: %d", ret);
+ return ret;
+ }
+
+ /* Slot format setup */
+
+ of_property_read_u32(dev->of_node, "adi,upstream-slot-size", &val);
+ if (val < 8 || val > 32 || (val % 4 != 0)) {
+ dev_err(dev, "invalid upstream-slot-size %d\n", val);
+ return -EINVAL;
+ }
+ master->up_slot_size = val;
+
+ of_property_read_u32(dev->of_node, "adi,downstream-slot-size", &val);
+ if (val < 8 || val > 32 || (val % 4 != 0)) {
+ dev_err(dev, "invalid downstream-slot-size %d\n", val);
+ return -EINVAL;
+ }
+ master->dn_slot_size = val;
+
+ master->dn_slot_alt_fmt =
+ of_property_read_bool(dev->of_node,
+ "adi,alternate-downstream-slot-format");
+ master->up_slot_alt_fmt =
+ of_property_read_bool(dev->of_node,
+ "adi,alternate-upstream-slot-format");
+
+ val = AD242X_SLOTFMT_DNSIZE(master->dn_slot_size) |
+ AD242X_SLOTFMT_UPSIZE(master->up_slot_size);
+
+ if (master->dn_slot_alt_fmt)
+ val |= AD242X_SLOTFMT_DNFMT;
+
+ if (master->up_slot_alt_fmt)
+ val |= AD242X_SLOTFMT_UPFMT;
+
+ ret = regmap_write(regmap, AD242X_SLOTFMT, val);
+ if (ret < 0)
+ return ret;
+
+ /* Node discovery and MFD setup */
+
+ ret = ad242x_discover(master, nodes_np);
+ if (ret < 0) {
+ dev_err(dev, "error discovering nodes: %d\n", ret);
+ return ret;
+ }
+
+ ret = ad242x_node_add_mfd_cells(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to add MFD devices %d\n", ret);
+ return ret;
+ }
+
+ /* Register platform devices for nodes */
+
+ for_each_available_child_of_node(nodes_np, np)
+ of_platform_device_create(np, NULL, dev);
+
+ of_node_put(nodes_np);
+
+ return 0;
+}
+
+static int ad242x_master_remove(struct i2c_client *i2c)
+{
+ struct ad242x_master *master = i2c_get_clientdata(i2c);
+
+ if (master->sync_clk)
+ clk_disable_unprepare(master->sync_clk);
+
+ return 0;
+}
+
+static const struct of_device_id ad242x_master_of_match[] = {
+ { .compatible = "adi,ad2428w-master" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad242x_master_of_match);
+
+static const struct i2c_device_id ad242x_master_i2c_id[] = {
+ {"ad242x-master", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, ad242x_master_i2c_id);
+
+static struct i2c_driver ad242x_master_i2c_driver = {
+ .driver = {
+ .name = "ad242x-master",
+ .of_match_table = ad242x_master_of_match,
+ },
+ .probe = ad242x_master_probe,
+ .remove = ad242x_master_remove,
+ .id_table = ad242x_master_i2c_id,
+};
+
+module_i2c_driver(ad242x_master_i2c_driver);
+
+MODULE_DESCRIPTION("AD242x master master driver");
+MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mfd/ad242x-node.c b/drivers/mfd/ad242x-node.c
new file mode 100644
index 000000000000..f9db689380a7
--- /dev/null
+++ b/drivers/mfd/ad242x-node.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+/* See Table 7-43 in the datasheet */
+static int ad242x_tdmmode_index(unsigned int mode, bool slave)
+{
+ switch (mode) {
+ case 2:
+ return 0;
+ case 4:
+ return 1;
+ case 8:
+ return 2;
+ case 12:
+ return slave ? -EINVAL : 3;
+ case 16:
+ return 4;
+ case 20:
+ return slave ? -EINVAL : 5;
+ case 24:
+ return slave ? -EINVAL : 6;
+ case 32:
+ return 7;
+ default:
+ return -EINVAL;
+ }
+}
+
+int ad242x_node_probe(struct ad242x_node *node)
+{
+ struct device_node *np = node->dev->of_node;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(node->regmap, AD242X_VENDOR, &val);
+ if (ret < 0) {
+ dev_err(node->dev, "failed to read VENDOR register %d\n", ret);
+ return ret;
+ }
+
+ if (val != 0xad) {
+ dev_err(node->dev, "bogus value 0x%02x in VENDOR register\n",
+ val);
+ return -ENODEV;
+ }
+
+ ret = regmap_read(node->regmap, AD242X_PRODUCT, &val);
+ if (ret < 0) {
+ dev_err(node->dev, "failed to read PRODUCT register %d\n",
+ ret);
+ return ret;
+ }
+
+ if (val != 0x28) {
+ dev_err(node->dev, "bogus value 0x%02x in PRODUCT register\n",
+ val);
+ return -ENODEV;
+ }
+
+ ret = regmap_read(node->regmap, AD242X_VERSION, &val);
+ if (ret < 0) {
+ dev_err(node->dev, "failed to read VERSION register %d\n", ret);
+ return ret;
+ }
+
+ if (node->id == AD242X_MASTER_ID)
+ dev_info(node->dev,
+ "Detected AD242x master node, version %d.%d\n",
+ val >> 4, val & 0xf);
+ else
+ dev_info(node->dev,
+ "Detected AD242x slave node, version %d.%d, id %d\n",
+ val >> 4, val & 0xf, node->id);
+
+ ret = regmap_read(node->regmap, AD242X_CAPABILITY, &val);
+ if (ret < 0) {
+ dev_err(node->dev, "failed to read CAPABILITY register %d\n",
+ ret);
+ return ret;
+ }
+
+ node->caps = val;
+
+ val = 0;
+
+ if (of_property_read_bool(np, "adi,spread-a2b-clock"))
+ val |= AD242X_PLLCTL_SSMODE_AB;
+ else if (of_property_read_bool(np, "adi,spread-a2b-i2s-clock"))
+ val |= AD242X_PLLCTL_SSMODE_AB_I2S;
+
+ if (of_property_read_bool(np, "adi,spread-spectrum-high"))
+ val |= AD242X_PLLCTL_SSDEPTH;
+
+ ret = regmap_write(node->regmap, AD242X_PLLCTL, val);
+ if (ret < 0) {
+ dev_err(node->dev, "failed to write PLLCTL register %d\n", ret);
+ return ret;
+ }
+
+ /* I2S global setup */
+
+ of_property_read_u32(np, "adi,tdm-mode", &node->tdm_mode);
+ of_property_read_u32(np, "adi,tdm-slot-size", &node->tdm_slot_size);
+
+ ret = ad242x_tdmmode_index(node->tdm_mode, false);
+ if (ret < 0) {
+ dev_err(node->dev, "invalid TDM mode %d\n", node->tdm_mode);
+ return -EINVAL;
+ }
+
+ val = AD242X_I2SGCTL_TDMMODE(ret);
+
+ if (node->tdm_slot_size == 16) {
+ val |= AD242X_I2SGCTL_TDMSS;
+ } else if (node->tdm_slot_size != 32) {
+ dev_err(node->dev, "invalid TDM slot size %d\n",
+ node->tdm_slot_size);
+ return -EINVAL;
+ }
+
+ if (of_property_read_bool(np, "adi,alternating-sync"))
+ val |= AD242X_I2SGCTL_ALT;
+
+ if (of_property_read_bool(np, "adi,early-sync"))
+ val |= AD242X_I2SGCTL_EARLY;
+
+ if (of_property_read_bool(np, "adi,invert-sync"))
+ val |= AD242X_I2SGCTL_INV;
+
+ ret = regmap_write(node->regmap, AD242X_I2SGCTL, val);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static const struct mfd_cell ad242x_mfd_cells[] = {
+ {
+ .of_compatible = "adi,ad2428w-i2c",
+ .name = "ad242x-i2c",
+ },
+ {
+ .of_compatible = "adi,ad2428w-gpio",
+ .name = "ad242x-gpio",
+ },
+ {
+ .of_compatible = "adi,ad2428w-clk",
+ .name = "ad242x-clk",
+ },
+ {
+ .of_compatible = "adi,ad2428w-codec",
+ .name = "ad242x-codec",
+ },
+};
+
+int ad242x_node_add_mfd_cells(struct device *dev)
+{
+ return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+ ad242x_mfd_cells,
+ ARRAY_SIZE(ad242x_mfd_cells),
+ NULL, 0, NULL);
+}
+
+static int ad242x_get_slot_mask(const struct device_node *np,
+ const char *propname, u32 *mask)
+{
+ unsigned int i, num;
+ int ret, proplen;
+ u32 slots[32];
+
+ if (!of_get_property(np, propname, &proplen))
+ return -ENOENT;
+
+ num = proplen / sizeof(u32);
+
+ if (num > ARRAY_SIZE(slots))
+ return -EOVERFLOW;
+
+ ret = of_property_read_u32_array(np, propname, slots, num);
+ if (ret < 0)
+ return ret;
+
+ *mask = 0;
+
+ for (i = 0; i < num; i++) {
+ if (slots[i] >= 32)
+ return -EINVAL;
+
+ *mask |= BIT(slots[i]);
+ }
+
+ return 0;
+}
+
+int ad242x_read_slot_config(struct device *dev,
+ struct device_node *np,
+ struct ad242x_slot_config *config)
+{
+ struct device_node *dn_np, *up_np;
+ int ret;
+
+ dn_np = of_get_child_by_name(np, "downstream");
+ if (!dn_np) {
+ dev_err(dev, "no downstream node\n");
+ return -EINVAL;
+ }
+
+ up_np = of_get_child_by_name(np, "upstream");
+ if (!dn_np) {
+ dev_err(dev, "no upstream node\n");
+ ret = -EINVAL;
+ goto err_put_dn_node;
+ }
+
+ ret = ad242x_get_slot_mask(dn_np, "rx-slots", &config->dn_rx_slots);
+ if (ret < 0 && ret != -ENOENT) {
+ dev_err(dev, "invalid downstream rx-slots property\n");
+ goto err_put_nodes;
+ }
+
+ of_property_read_u32(dn_np, "#tx-slots", &config->dn_n_tx_slots);
+ of_property_read_u32(dn_np, "#forward-slots",
+ &config->dn_n_forward_slots);
+ if (config->dn_n_tx_slots + config->dn_n_forward_slots >= 32) {
+ dev_err(dev, "invalid downstream tx-slots property\n");
+ goto err_put_nodes;
+ }
+
+
+ ret = ad242x_get_slot_mask(up_np, "rx-slots", &config->up_rx_slots);
+ if (ret < 0) {
+ dev_err(dev, "invalid upstream rx-slots property\n");
+ goto err_put_nodes;
+ }
+
+ of_property_read_u32(up_np, "#tx-slots", &config->up_n_tx_slots);
+ of_property_read_u32(up_np, "#forward-slots",
+ &config->up_n_forward_slots);
+ if (config->up_n_tx_slots + config->up_n_forward_slots >= 32) {
+ dev_err(dev, "invalid downstream tx-slots property\n");
+ goto err_put_nodes;
+ }
+
+err_put_nodes:
+ of_node_put(up_np);
+err_put_dn_node:
+ of_node_put(dn_np);
+
+ return ret;
+}
diff --git a/drivers/mfd/ad242x-slave.c b/drivers/mfd/ad242x-slave.c
new file mode 100644
index 000000000000..ad255d67a5b6
--- /dev/null
+++ b/drivers/mfd/ad242x-slave.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/mfd/core.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct ad242x_slave {
+ struct ad242x_node node;
+ struct ad242x_node *master;
+ struct ad242x_slot_config slot_config;
+ unsigned int sync_offset;
+};
+
+int ad242x_slave_read(struct ad242x_i2c_bus *bus,
+ struct regmap *master_regmap,
+ uint8_t node_id, uint8_t reg, unsigned int *val)
+{
+ int ret;
+
+ mutex_lock(&bus->mutex);
+
+ ret = regmap_write(master_regmap, AD242X_NODEADR, node_id);
+ if (ret < 0)
+ goto err_unlock;
+
+ ret = i2c_smbus_read_byte_data(bus->client, reg);
+ if (ret < 0)
+ goto err_unlock;
+
+ *val = ret;
+ ret = 0;
+
+err_unlock:
+ mutex_unlock(&bus->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ad242x_slave_read);
+
+int ad242x_slave_write(struct ad242x_i2c_bus *bus,
+ struct regmap *master_regmap,
+ uint8_t node_id, uint8_t reg, unsigned int val)
+{
+ int ret;
+
+ mutex_lock(&bus->mutex);
+
+ ret = regmap_write(master_regmap, AD242X_NODEADR, node_id);
+ if (ret < 0)
+ goto err_unlock;
+
+ ret = i2c_smbus_write_byte_data(bus->client, reg, val);
+ if (ret < 0)
+ goto err_unlock;
+
+ ret = 0;
+
+err_unlock:
+ mutex_unlock(&bus->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ad242x_slave_write);
+
+static int ad242x_slave_regmap_read(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ struct ad242x_slave *slave = context;
+ struct ad242x_i2c_bus *bus = ad242x_master_get_bus(slave->node.master);
+ struct ad242x_node *mnode = ad242x_master_get_node(slave->node.master);
+
+ if (reg > 0xff)
+ return -EINVAL;
+
+ return ad242x_slave_read(bus, mnode->regmap, slave->node.id, reg, val);
+}
+
+static int ad242x_slave_regmap_write(void *context, unsigned int reg,
+ unsigned int val)
+{
+ struct ad242x_slave *slave = context;
+ struct ad242x_i2c_bus *bus = ad242x_master_get_bus(slave->node.master);
+ struct ad242x_node *mnode = ad242x_master_get_node(slave->node.master);
+
+ if (val > 0xff || reg > 0xff)
+ return -EINVAL;
+
+ return ad242x_slave_write(bus, mnode->regmap, slave->node.id, reg, val);
+}
+
+static const struct regmap_config ad242x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_reg = ad242x_is_volatile_reg,
+ .writeable_reg = ad242x_is_writeable_reg,
+ .reg_read = ad242x_slave_regmap_read,
+ .reg_write = ad242x_slave_regmap_write,
+ .max_register = AD242X_MAX_REG,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int ad242x_calc_sync_offset(unsigned int val)
+{
+ if (val == 0)
+ return 0;
+
+ if (val > 127)
+ return -EINVAL;
+
+ return 256 - val;
+}
+
+static int ad242x_slave_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ad242x_slave *slave;
+ struct ad242x_node *mnode;
+ struct regmap *regmap;
+ unsigned int val;
+ int i, ret;
+
+ slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
+ if (!slave)
+ return -ENOMEM;
+
+ regmap = devm_regmap_init(dev, NULL, slave, &ad242x_regmap_config);
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ dev_err(dev, "regmap init failed: %d\n", ret);
+ return ret;
+ }
+
+ of_property_read_u32(dev->of_node, "reg", &val);
+ slave->node.id = val;
+ slave->node.dev = dev;
+ slave->node.regmap = regmap;
+
+ mnode = dev_get_drvdata(dev->parent);
+ slave->node.master = mnode->master;
+
+ dev_set_name(dev, "%s-a2b-%d", dev_name(dev->parent), slave->node.id);
+ dev_set_drvdata(dev, &slave->node);
+
+ ret = ad242x_node_probe(&slave->node);
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_read_slot_config(dev, dev->of_node, &slave->slot_config);
+ if (ret < 0) {
+ dev_err(dev, "slot config is invalid: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_write(regmap, AD242X_UPSLOTS,
+ slave->slot_config.up_n_forward_slots);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_DNSLOTS,
+ slave->slot_config.dn_n_forward_slots);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_LUPSLOTS,
+ slave->slot_config.up_n_tx_slots);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_LDNSLOTS,
+ slave->slot_config.dn_n_tx_slots |
+ AD242X_LDNSLOTS_DNMASKEN);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < 4; i++) {
+ ret = regmap_write(regmap, AD242X_UPMASK(i),
+ (slave->slot_config.up_rx_slots >> (i * 8)) & 0xff);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_DNMASK(i),
+ (slave->slot_config.dn_rx_slots >> (i * 8)) & 0xff);
+ if (ret < 0)
+ return ret;
+ }
+
+ of_property_read_u32(dev->of_node, "adi,sync-offset",
+ &slave->sync_offset);
+
+ ret = ad242x_calc_sync_offset(slave->sync_offset);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(regmap, AD242X_SYNCOFFSET, ret);
+ if (ret < 0)
+ return ret;
+
+ ret = ad242x_node_add_mfd_cells(dev);
+ if (ret < 0) {
+ dev_err(dev, "failed to add MFD devices %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id ad242x_slave_of_match[] = {
+ { .compatible = "adi,ad2428w-slave" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad242x_slave_of_match);
+
+static struct platform_driver ad242x_slave_driver = {
+ .driver = {
+ .name = "ad242x-slave",
+ .of_match_table = ad242x_slave_of_match,
+ },
+ .probe = ad242x_slave_probe,
+};
+
+module_platform_driver(ad242x_slave_driver);
+
+MODULE_DESCRIPTION("AD242x slave node driver");
+MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/ad242x.h b/include/linux/mfd/ad242x.h
new file mode 100644
index 000000000000..02a174824f85
--- /dev/null
+++ b/include/linux/mfd/ad242x.h
@@ -0,0 +1,400 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __LINUX_MFD_AD242X_H
+#define __LINUX_MFD_AD242X_H
+
+#define AD242X_CHIP 0x00
+#define AD242X_NODEADR 0x01
+#define AD242X_NODEADR_MASK 0x0f
+#define AD242X_NODEADR_PERI BIT(5)
+#define AD242X_NODEADR_BRCST BIT(7)
+
+#define AD242X_VENDOR 0x02
+#define AD242X_PRODUCT 0x03
+#define AD242X_VERSION 0x04
+
+#define AD242X_CAPABILITY 0x05
+#define AD242X_CAPABILITY_I2C BIT(0)
+
+#define AD242X_SWCTL 0x09
+#define AD242X_SWCTL_ENSW BIT(0)
+#define AD242X_SWCTL_DIAGMODE BIT(3)
+#define AD242X_SWCTL_MODE(X) (((X) & 3) << 4)
+#define AD242X_SWCTL_MODE_MASK (3 << 4)
+#define AD242X_SWCTL_DISNXT BIT(6)
+
+#define AD242X_BCDNSLOTS 0x0a
+#define AD242X_BCDNSLOTS_MASK 0x3f
+
+#define AD242X_LDNSLOTS 0x0b
+#define AD242X_LDNSLOTS_MASK 0x3f
+#define AD242X_LDNSLOTS_DNMASKEN BIT(7)
+
+#define AD242X_LUPSLOTS 0x0c
+#define AD242X_LUPSLOTS_MASK 0x3f
+
+#define AD242X_DNSLOTS 0x0d
+#define AD242X_DNSLOTS_MASK 0x3f
+
+#define AD242X_UPSLOTS 0x0e
+#define AD242X_UPSLOTS_MASK 0x3f
+
+#define AD242X_RESPCYCS 0x0f
+
+#define AD242X_SLOTFMT 0x10
+#define AD242X_SLOTFMT_DNSIZE(X) ((((X) - 8) >> 2) & 7)
+#define AD242X_SLOTFMT_DNFMT BIT(3)
+#define AD242X_SLOTFMT_UPSIZE(X) (((((X) - 8) >> 2) & 7) << 4)
+#define AD242X_SLOTFMT_UPFMT BIT(7)
+
+#define AD242X_DATCTL 0x11
+#define AD242X_DATCTL_DNS BIT(0)
+#define AD242X_DATCTL_UPS BIT(1)
+#define AD242X_DATCTL_ENDSNIFF BIT(5)
+#define AD242X_DATCTL_STANDBY BIT(7)
+
+#define AD242X_CONTROL 0x12
+#define AD242X_CONTROL_NEWSTRCT BIT(0)
+#define AD242X_CONTROL_ENDDSC BIT(1)
+#define AD242X_CONTROL_SOFTRST BIT(2)
+#define AD242X_CONTROL_SWBYP BIT(3)
+#define AD242X_CONTROL_XCVRBINV BIT(4)
+#define AD242X_CONTROL_MSTR BIT(7)
+
+#define AD242X_DISCVRY 0x13
+
+#define AD242X_SWSTAT 0x14
+#define AD242X_SWSTAT_FIN BIT(0)
+#define AD242X_SWSTAT_FAULT BIT(1)
+#define AD242X_SWSTAT_FAULTCODE(X) (((X) & 0x7) >> 4)
+#define AD242X_SWSTAT_FAULT_NLOC BIT(7)
+
+#define AD242X_INTSTAT 0x15
+#define AD242X_INTSTAT_IRQ BIT(0)
+
+#define AD242X_INTSRC 0x16
+#define AD242X_INTSRC_INODE 0x0f
+#define AD242X_INTSRC_SLVINT BIT(6)
+#define AD242X_INTSRC_MSTINT BIT(7)
+
+#define AD242X_INTTYPE 0x17
+
+#define AD242X_INTTYPE_DSCDONE 24
+#define AD242X_INTTYPE_MSTR_RUNNING 255
+
+#define AD242X_INTPND0 0x18
+#define AD242X_INTPDN0_HDCNTERR BIT(0)
+#define AD242X_INTPDN0_DDERR BIT(1)
+#define AD242X_INTPDN0_CRCERR BIT(2)
+#define AD242X_INTPDN0_DPERR BIT(3)
+#define AD242X_INTPDN0_PWRERR BIT(4)
+#define AD242X_INTPDN0_BECOVF BIT(5)
+#define AD242X_INTPDN0_SRFERR BIT(6)
+#define AD242X_INTPDN0_SRFCRCERR BIT(7)
+
+#define AD242X_INTPND1 0x19
+#define AD242X_INTPND1_IOPND(X) BIT(X)
+
+#define AD242X_INTPND2 0x1a
+#define AD242X_INTPND2_DSCDONE BIT(0)
+#define AD242X_INTPND2_I2CERR BIT(1)
+#define AD242X_INTPND2_ICRCERR BIT(2)
+#define AD242X_INTPND2_SLVIRQ BIT(3)
+
+#define AD242X_INTMSK0 0x1b
+#define AD242X_INTMSK0_HCEIEN BIT(0)
+#define AD242X_INTMSK0_DDEIEN BIT(1)
+#define AD242X_INTMSK0_CRCEIEN BIT(2)
+#define AD242X_INTMSK0_DPEIEN BIT(3)
+#define AD242X_INTMSK0_PWREIEN BIT(4)
+#define AD242X_INTMSK0_BECIEN BIT(5)
+#define AD242X_INTMSK0_SRFEIEN BIT(6)
+#define AD242X_INTMSK0_SRFCRCEIEN BIT(7)
+
+#define AD242X_INTMSK1 0x1c
+#define AD242X_INTMSK1_IOIRQEN(X) BIT(X)
+
+#define AD242X_INTMSK2 0x1d
+#define AD242X_INTMSK2_DSCDIEN BIT(0)
+#define AD242X_INTMSK2_I2CEIEN BIT(1)
+#define AD242X_INTMSK2_ICRCEIEN BIT(2)
+#define AD242X_INTMSK2_SLVIRQEN BIT(3)
+
+#define AD242X_BECCTL 0x1e
+#define AD242X_BECCTL_ENHDCNT BIT(0)
+#define AD242X_BECCTL_ENDD BIT(1)
+#define AD242X_BECCTL_ENCRC BIT(2)
+#define AD242X_BECCTL_ENDP BIT(3)
+#define AD242X_BECCTL_ENICRC BIT(4)
+#define AD242X_BECCTL_THRESHLD(X) ((X) >> 5)
+
+#define AD242X_BECNT 0x1f
+
+#define AD242X_TESTMODE 0x20
+#define AD242X_TESTMODE_PRBSUP BIT(0)
+#define AD242X_TESTMODE_PRBSDN BIT(1)
+#define AD242X_TESTMODE_PRBSN2N BIT(2)
+#define AD242X_TESTMODE_RXDPTH(X) ((X) >> 4)
+
+#define AD242X_ERRCNT0 0x21
+#define AD242X_ERRCNT1 0x22
+#define AD242X_ERRCNT2 0x23
+#define AD242X_ERRCNT3 0x24
+
+#define AD242X_NODE 0x29
+#define AD242X_NODE_MASK 0xf
+#define AD242X_NODE_DISCVD BIT(5)
+#define AD242X_NODE_NLAST BIT(6)
+#define AD242X_NODE_LAST BIT(7)
+
+#define AD242X_DISCSTAT 0x2b
+#define AD242X_DISCSTAT_DNODE(X) ((X) & 0xf)
+#define AD242X_DISCSTAT_DSCACT BIT(7)
+
+#define AD242X_TXACTL 0x2e
+#define AD242X_TXACTL_LEVEL_HIGH 0
+#define AD242X_TXACTL_LEVEL_MEDIUM 2
+#define AD242X_TXACTL_LEVEL_LOW 3
+
+#define AD242X_TXBCTL 0x30
+#define AD242X_TXBCTL_LEVEL_HIGH 0
+#define AD242X_TXBCTL_LEVEL_MEDIUM 2
+#define AD242X_TXBCTL_LEVEL_LOW 3
+
+#define AD242X_LINTTYPE 0x3e
+
+#define AD242X_I2CCFG 0x3f
+#define AD242X_I2CCFG_DATARATE BIT(0)
+#define AD242X_I2CCFG_EACK BIT(1)
+#define AD242X_I2CCFG_FRAMERATE BIT(2)
+
+#define AD242X_PLLCTL 0x40
+#define AD242X_PLLCTL_SSFREQ(X) ((X) & 3)
+#define AD242X_PLLCTL_SSDEPTH BIT(2)
+#define AD242X_PLLCTL_SSMODE_AB (1 << 6)
+#define AD242X_PLLCTL_SSMODE_AB_I2S (2 << 6)
+
+#define AD242X_I2SGCTL 0x41
+#define AD242X_I2SGCTL_TDMMODE(X) ((X) & 3)
+#define AD242X_I2SGCTL_RXONDTX1 BIT(3)
+#define AD242X_I2SGCTL_TDMSS BIT(4)
+#define AD242X_I2SGCTL_ALT BIT(5)
+#define AD242X_I2SGCTL_EARLY BIT(6)
+#define AD242X_I2SGCTL_INV BIT(7)
+
+#define AD242X_I2SCTL 0x42
+#define AD242X_I2SCTL_TX0EN BIT(0)
+#define AD242X_I2SCTL_TX1EN BIT(1)
+#define AD242X_I2SCTL_TX2PINTL BIT(2)
+#define AD242X_I2SCTL_TXBCLKINV BIT(3)
+#define AD242X_I2SCTL_RX0EN BIT(4)
+#define AD242X_I2SCTL_RX1EN BIT(5)
+#define AD242X_I2SCTL_RX2PINTL BIT(6)
+#define AD242X_I2SCTL_RXBCLKINV BIT(7)
+
+#define AD242X_I2SRATE 0x43
+#define AD242X_I2SRATE_I2SRATE(X) ((X) & 3)
+#define AD242X_I2SRATE_BCLKRATE(X) (((X) << 3) & 3)
+#define AD242X_I2SRATE_REDUCE BIT(6)
+#define AD242X_I2SRATE_SHARE BIT(7)
+
+#define AD242X_I2STXOFFSET 0x44
+#define AD242X_I2STXOFFSET_VAR(X) ((X) & 0x3f)
+#define AD242X_I2STXOFFSET_TSAFTER BIT(6)
+#define AD242X_I2STXOFFSET_TSBEFORE BIT(7)
+
+#define AD242X_2SRXOFFSET 0x45
+#define AD242X_I2SRXOFFSET_VAR(X) ((X) & 0x3f)
+
+#define AD242X_SYNCOFFSET 0x46
+
+#define AD242X_PDMCTL 0x47
+#define AD242X_PDMCTL_PDM0EN BIT(0)
+#define AD242X_PDMCTL_PDM0SLOTS BIT(1)
+#define AD242X_PDMCTL_PDM1EN BIT(2)
+#define AD242X_PDMCTL_PDM1SLOTS BIT(3)
+#define AD242X_PDMCTL_HPFEN BIT(4)
+#define AD242X_PDMCTL_PDMRATE(X) (((X) & 3) << 5)
+
+#define AD242X_ERRMGMT 0x48
+#define AD242X_ERRMGMT_ERRLSB BIT(0)
+#define AD242X_ERRMGMT_ERRSIG BIT(1)
+#define AD242X_ERRMGMT_ERRSLOT BIT(2)
+
+#define AD242X_GPIODAT 0x4a
+#define AD242X_GPIODAT_SET 0x4b
+#define AD242X_GPIODAT_CLR 0x4c
+#define AD242X_GPIOOEN 0x4d
+#define AD242X_GPIOIEN 0x4e
+#define AD242X_GPIODAT_IN 0x4f
+#define AD242X_PINTEN 0x50
+#define AD242X_PINTINV 0x51
+
+#define AD242X_PINCFG 0x52
+#define AD242X_PINCFG_DRVSTR BIT(0)
+#define AD242X_PINCFG_IRQINV BIT(4)
+#define AD242X_PINCFG_IRQTS BIT(5)
+
+#define AD242X_I2STEST 0x53
+#define AD242X_I2STEST_PATTRN2TX BIT(0)
+#define AD242X_I2STEST_LOOPBK2TX BIT(1)
+#define AD242X_I2STEST_RX2LOOPBK BIT(2)
+#define AD242X_I2STEST_SELRX1 BIT(3)
+#define AD242X_I2STEST_BUSLOOPBK BIT(4)
+
+#define AD242X_RAISE 0x54
+
+#define AD242X_GENERR 0x55
+#define AD242X_GENERR_GENHCERR BIT(0)
+#define AD242X_GENERR_GENDDERR BIT(1)
+#define AD242X_GENERR_GENCRCERR BIT(2)
+#define AD242X_GENERR_GENDPERR BIT(3)
+#define AD242X_GENERR_GENICRCERR BIT(4)
+
+#define AD242X_I2SRRATE 0x56
+#define AD242X_I2SRRATE_RRDIV(X) ((X) & 0x3f)
+#define AD242X_I2SRRATE_RBUS BIT(7)
+
+#define AD242X_I2SRRCTL 0x57
+#define AD242X_I2SRRCTL_ENVLSB BIT(0)
+#define AD242X_I2SRRCTL_ENXBIT BIT(1)
+#define AD242X_I2SRRCTL_ENSTRB BIT(4)
+#define AD242X_I2SRRCTL_STRBDIR BIT(5)
+
+#define AD242X_I2SRRSOFFS 0x58
+
+#define AD242X_CLK1CFG 0x59
+#define AD242X_CLK2CFG 0x5a
+#define AD242X_CLKCFG_DIV(X) ((X) & 0xf)
+#define AD242X_CLKCFG_DIVMSK 0xf
+#define AD242X_CLKCFG_PDIV32 BIT(5)
+#define AD242X_CLKCFG_INV BIT(6)
+#define AD242X_CLKCFG_EN BIT(7)
+
+#define AD242X_BMMCFG 0x5b
+#define AD242X_BMMCFG_BMMEN BIT(0)
+#define AD242X_BMMCFG_BMMRXEN BIT(1)
+#define AD242X_BMMCFG_BMMNDSC BIT(2)
+
+#define AD242X_PDMCTL2 0x5d
+#define AD242X_PDMCTL2_DEST_A2B 0
+#define AD242X_PDMCTL2_DEST_DTX 1
+#define AD242X_PDMCTL2_DEST_A2B_DTX 2
+
+#define AD242X_UPMASK(X) (0x60 + ((X) & 3))
+
+#define AD242X_UPOFFSET 0x64
+#define AD242X_UPOFFSET_VAL(X) ((X) & 0x1f)
+
+#define AD242X_DNMASK(X) (0x65 + ((X) % 3))
+
+#define AD242X_DNOFFSET 0x69
+#define AD242X_DNOFFSET_VAL(X) ((X) & 0x1f)
+
+#define AD242X_CHIPID(X) ((X) + 0x6a)
+
+#define AD242X_GPIODEN 0x80
+#define AD242X_GPIOD_MSK(X) ((X) + 0x81)
+
+#define AD242X_GPIODDAT 0x89
+#define AD242X_GPIODINV 0x8a
+
+#define AD242X_MAX_REG 0x9b
+
+static inline bool ad242x_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case AD242X_VENDOR:
+ case AD242X_PRODUCT:
+ case AD242X_VERSION:
+ case AD242X_CAPABILITY:
+ case AD242X_SWSTAT:
+ case AD242X_INTSTAT:
+ case AD242X_INTSRC:
+ case AD242X_INTTYPE:
+ case AD242X_INTPND0:
+ case AD242X_INTPND1:
+ case AD242X_INTPND2:
+ case AD242X_BECNT:
+ case AD242X_ERRCNT0:
+ case AD242X_ERRCNT1:
+ case AD242X_ERRCNT2:
+ case AD242X_ERRCNT3:
+ case AD242X_NODE:
+ case AD242X_DISCSTAT:
+ case AD242X_LINTTYPE:
+ case AD242X_GPIODAT:
+ case AD242X_GPIODAT_IN:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static inline bool ad242x_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+ /* Write-to-clean registers */
+ switch (reg) {
+ case AD242X_INTPND0:
+ case AD242X_INTPND1:
+ case AD242X_INTPND2:
+ case AD242X_BECNT:
+ return true;
+ default:
+ return !ad242x_is_volatile_reg(dev, reg);
+ }
+}
+
+#define AD242X_MASTER_ID 0xff
+
+struct ad242x_master;
+
+struct ad242x_i2c_bus {
+ struct i2c_client *client;
+ struct mutex mutex;
+};
+
+struct ad242x_node {
+ struct device *dev;
+ struct regmap *regmap;
+ struct ad242x_master *master;
+ unsigned int tdm_mode;
+ unsigned int tdm_slot_size;
+ uint8_t id;
+ uint8_t caps;
+};
+
+struct ad242x_slot_config {
+ unsigned int dn_rx_slots;
+ unsigned int dn_n_tx_slots;
+ unsigned int dn_n_forward_slots;
+ unsigned int up_rx_slots;
+ unsigned int up_n_tx_slots;
+ unsigned int up_n_forward_slots;
+};
+
+int ad242x_read_slot_config(struct device *dev,
+ struct device_node *np,
+ struct ad242x_slot_config *config);
+
+static inline bool ad242x_node_is_master(struct ad242x_node *node)
+{
+ return node->id == AD242X_MASTER_ID;
+}
+
+int ad242x_node_probe(struct ad242x_node *node);
+int ad242x_node_add_mfd_cells(struct device *dev);
+
+struct ad242x_node *ad242x_master_get_node(struct ad242x_master *master);
+struct ad242x_i2c_bus *ad242x_master_get_bus(struct ad242x_master *master);
+const char *ad242x_master_get_clk_name(struct ad242x_master *master);
+unsigned int ad242x_master_get_clk_rate(struct ad242x_master *master);
+
+int ad242x_slave_read(struct ad242x_i2c_bus *bus,
+ struct regmap *master_regmap,
+ uint8_t node_id, uint8_t reg, unsigned int *val);
+int ad242x_slave_write(struct ad242x_i2c_bus *bus,
+ struct regmap *master_regmap,
+ uint8_t node_id, uint8_t reg, unsigned int val);
+
+#endif
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-09 18:35 ` [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers Daniel Mack
@ 2019-12-17 13:39 ` Lee Jones
2019-12-17 13:46 ` Lee Jones
2019-12-17 19:24 ` Daniel Mack
2019-12-17 19:16 ` [alsa-devel] " Pierre-Louis Bossart
1 sibling, 2 replies; 34+ messages in thread
From: Lee Jones @ 2019-12-17 13:39 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lars,
pascal.huerst
On Mon, 09 Dec 2019, Daniel Mack wrote:
> The core driver for these devices is split into several parts.
>
> The master node driver is an I2C client. It is responsible for
> bringing up the bus topology and discovering the slave nodes.
> This process requries some knowlegde of the slave node configuration
> to program the bus timings correctly, so the master drivers walks
> the tree of nodes in the devicetree. The slave driver handles platform
> devices that are instantiated by the master node driver after
> discovery has finished.
>
> Master nodes expose two addresses on the I2C bus, one (referred to as
> 'BASE' in the datasheet) for accessing registers on the transceiver
> node itself, and one (referred to as 'BUS') for accessing remote
> registers, either on the remote transceiver itself, or on I2C hardware
> connected to that remote transceiver, which then acts as a remote I2C
> bus master.
>
> In order to allow MFD sub-devices to be registered as children of
> either the master or any slave node, the details on how to access the
> registers are hidden behind a regmap config. A pointer to the regmap
> is then exposed in the struct shared with the sub-devices.
>
> The ad242x-bus driver is a simple proxy that occupies the BUS I2C
> address and which is referred to through a devicetree handle by the
> master driver.
>
> For the discovery process, the driver has to wait for an interrupt
> to occur. In case no interrupt is configured in DT, the driver falls
> back to interrupt polling. After the discovery phase is completed,
> interrupts are only needed for error handling and GPIO handling,
> both of which is not currenty implemented.
>
> Code common to both the master and the slave driver lives in
> 'ad242x-node.c'.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
>
> mfd
?
> ---
> drivers/mfd/Kconfig | 11 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/ad242x-bus.c | 42 +++
> drivers/mfd/ad242x-master.c | 611 ++++++++++++++++++++++++++++++++++++
> drivers/mfd/ad242x-node.c | 262 ++++++++++++++++
> drivers/mfd/ad242x-slave.c | 234 ++++++++++++++
> include/linux/mfd/ad242x.h | 400 +++++++++++++++++++++++
This device, or at least the way it's been coded is batty!
It's going to need a lot of massaging before being accepted.
Let's start with a quick (it's taken 2 hours already!) glance.
See below ...
> 7 files changed, 1561 insertions(+)
> create mode 100644 drivers/mfd/ad242x-bus.c
> create mode 100644 drivers/mfd/ad242x-master.c
> create mode 100644 drivers/mfd/ad242x-node.c
> create mode 100644 drivers/mfd/ad242x-slave.c
> create mode 100644 include/linux/mfd/ad242x.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 420900852166..727a35053d8c 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -99,6 +99,17 @@ config PMIC_ADP5520
> individual components like LCD backlight, LEDs, GPIOs and Kepad
> under the corresponding menus.
>
> +config MFD_AD242X
> + bool "Analog Devices AD242x A2B support"
> + select MFD_CORE
> + select REGMAP_I2C
> + depends on I2C=y && OF
> + help
> + If you say yes here, you get support for devices from the AD242x
> + familiy. This driver provides common support for accessing the
> + devices, additional drivers must be enabled in order to use the
> + functionality of the devices.
> +
> config MFD_AAT2870_CORE
> bool "AnalogicTech AAT2870"
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index aed99f08739f..2361c676f6c8 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -203,6 +203,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC) += qcom-spmi-pmic.o
> obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o
> obj-$(CONFIG_MFD_TPS65090) += tps65090.o
> obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o
> +obj-$(CONFIG_MFD_AD242X) += ad242x-master.o ad242x-slave.o ad242x-bus.o ad242x-node.o
> obj-$(CONFIG_MFD_AT91_USART) += at91-usart.o
> obj-$(CONFIG_MFD_ATMEL_FLEXCOM) += atmel-flexcom.o
> obj-$(CONFIG_MFD_ATMEL_HLCDC) += atmel-hlcdc.o
> diff --git a/drivers/mfd/ad242x-bus.c b/drivers/mfd/ad242x-bus.c
> new file mode 100644
> index 000000000000..6660e13ce43d
> --- /dev/null
> +++ b/drivers/mfd/ad242x-bus.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/mfd/ad242x.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +static int ad242x_bus_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + dev_set_drvdata(&i2c->dev, i2c);
> + i2c_set_clientdata(i2c, &i2c->dev);
Please explain to me what you think is happening here.
> + return 0;
> +}
What does this driver do? Seems kinda pointless?
> +static const struct of_device_id ad242x_bus_of_match[] = {
> + { .compatible = "adi,ad2428w-bus" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ad242x_bus_of_match);
> +
> +static const struct i2c_device_id ad242x_bus_i2c_id[] = {
> + { "ad242x_bus", 0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, ad242x_bus_i2c_id);
This table cam be removed if you use probe_new.
> +static struct i2c_driver ad242x_bus_i2c_driver = {
> + .driver = {
> + .name = "ad242x-bus",
> + .of_match_table = ad242x_bus_of_match,
> + },
> + .probe = ad242x_bus_i2c_probe,
> + .id_table = ad242x_bus_i2c_id,
> +};
> +
> +module_i2c_driver(ad242x_bus_i2c_driver);
> +
> +MODULE_DESCRIPTION("AD242x bus driver");
> +MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/ad242x-master.c b/drivers/mfd/ad242x-master.c
> new file mode 100644
> index 000000000000..1b0bf90442a2
> --- /dev/null
> +++ b/drivers/mfd/ad242x-master.c
> @@ -0,0 +1,611 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/mfd/ad242x.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +struct ad242x_master {
> + struct ad242x_node node;
> + struct clk *sync_clk;
> + struct completion run_completion;
> + struct completion discover_completion;
> + struct ad242x_i2c_bus bus;
> + unsigned int up_slot_size;
> + unsigned int dn_slot_size;
> + bool up_slot_alt_fmt;
> + bool dn_slot_alt_fmt;
> + unsigned int sync_clk_rate;
> + int irq;
> + u8 response_cycles;
> +};
> +struct ad242x_node *ad242x_master_get_node(struct ad242x_master *master)
> +{
> + return &master->node;
> +}
> +EXPORT_SYMBOL_GPL(ad242x_master_get_node);
> +
> +struct ad242x_i2c_bus *ad242x_master_get_bus(struct ad242x_master *master)
> +{
> + return &master->bus;
> +}
> +EXPORT_SYMBOL_GPL(ad242x_master_get_bus);
> +
> +const char *ad242x_master_get_clk_name(struct ad242x_master *master)
> +{
> + return __clk_get_name(master->sync_clk);
> +}
> +EXPORT_SYMBOL_GPL(ad242x_master_get_clk_name);
> +
> +unsigned int ad242x_master_get_clk_rate(struct ad242x_master *master)
> +{
> + return master->sync_clk_rate;
> +}
> +EXPORT_SYMBOL_GPL(ad242x_master_get_clk_rate);
All of these functions provide abstraction for the sake of
abstraction. They should be removed and replaced with the code
contained within them.
> +static int ad242x_read_one_irq(struct ad242x_master *master)
> +{
> + struct regmap *regmap = master->node.regmap;
> + struct device *dev = master->node.dev;
> + unsigned int val, inttype;
> + int ret;
> +
> + ret = regmap_read(regmap, AD242X_INTSTAT, &val);
> + if (ret < 0) {
> + dev_err(dev, "unable to read INTSTAT register: %d\n", ret);
Users do not care about registers.
"Failed to obtain interrupt status"
> + return ret;
> + }
> +
> + if (!(val & AD242X_INTSTAT_IRQ))
> + return -ENOENT;
What happened here? No interrupts fired?
IRQ_NONE would be better than "No such file or directory".
> + ret = regmap_read(regmap, AD242X_INTTYPE, &inttype);
> + if (ret < 0) {
> + dev_err(dev, "unable to read INTTYPE register: %d\n", ret);
Same for all log messages throughout this patch-set.
> + return ret;
> + }
> +
> + ret = regmap_read(regmap, AD242X_INTSRC, &val);
> + if (ret < 0) {
> + dev_err(dev, "unable to read INTSRC register: %d\n", ret);
> + return ret;
> + }
What does this prove? Why aren't you doing anything with the value?
> + ret = regmap_read(regmap, AD242X_INTPND0, &val);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_INTPND0, val);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(regmap, AD242X_INTPND1, &val);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_INTPND1, val);
> + if (ret < 0)
> + return ret;
What does writing back the value do? Comments please.
> + if (val & AD242X_INTSRC_MSTINT) {
> + ret = regmap_read(regmap, AD242X_INTPND2, &val);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_INTPND2, val);
> + if (ret < 0)
> + return ret;
> + }
> +
> + dev_err(dev, "%s() inttype: 0x%02x\n", __func__, inttype);
No debugging type 'func's please.
What makes this an error?
> + switch (inttype) {
> + case AD242X_INTTYPE_DSCDONE:
> + complete(&master->discover_completion);
> + break;
> + case AD242X_INTTYPE_MSTR_RUNNING:
> + complete(&master->run_completion);
> + break;
> + default:
> + dev_info(dev, "Unhandled interrupt type 0x%02x\n", inttype);
> + }
> +
> + return 0;
> +}
> +
> +static int ad242x_read_irqs(struct ad242x_master *master)
> +{
> + int ret;
> + bool first = true;
> +
> + while (true) {
> + ret = ad242x_read_one_irq(master);
> + if (ret < 0)
> + return ret;
> + if (ret == -ENOENT)
> + return first ? ret : 0;
> +
> + first = false;
> + }
> +}
> +
> +static irqreturn_t ad242x_handle_irq(int irq, void *devid)
> +{
> + struct ad242x_master *master = devid;
> + int ret;
> +
> + ret = ad242x_read_irqs(master);
> + if (ret == -ENOENT)
> + return IRQ_NONE;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ad242x_wait_for_irq(struct ad242x_master *master,
> + struct completion *completion,
> + unsigned int timeout)
> +{
> + int ret;
> +
> + if (master->irq > 0) {
> + ret = wait_for_completion_timeout(completion,
> + msecs_to_jiffies(timeout));
> + } else {
> + usleep_range(timeout * 1000, timeout * 1500);
> + ad242x_read_irqs(master);
> + ret = completion_done(completion);
> + }
What are the semantics of this function. Comments please.
> + return ret == 0 ? -ETIMEDOUT : 0;
> +}
> +
> +/* See Table 3-2 in the datasheet */
Do you provide a link to the datasheet anywhere?
All I can find is a 1 page overview.
Please provide a description to what you're doing *here*.
> +static unsigned int ad242x_bus_bits(unsigned int slot_size, bool alt_fmt)
> +{
> + int alt_bits[8] = { 0, 13, 17, 21, 30, 0, 39, 0 };
> + int idx = AD242X_SLOTFMT_DNSIZE(slot_size);
> +
> + return alt_fmt ? alt_bits[idx] : slot_size + 1;
> +}
> +
> +/* See Table 9-1 in the datasheet */
It's okay to reference the datasheet, but tell us what you're doing
here as well.
> +static unsigned int ad242x_master_respoffs(struct ad242x_node *node)
> +{
> + if (node->tdm_mode == 2 && node->tdm_slot_size == 16)
> + return 238;
> +
> + if ((node->tdm_mode == 2 && node->tdm_slot_size == 32) ||
> + (node->tdm_mode == 4 && node->tdm_slot_size == 16))
> + return 245;
> +
> + return 248;
No magic numbers please. You need to define them.
> +}
> +
> +static int ad242x_discover(struct ad242x_master *master,
> + struct device_node *nodes_np)
> +{
> + struct regmap *regmap = master->node.regmap;
> + struct device *dev = master->node.dev;
> + struct device_node *child_np;
> + unsigned int val, n = 0, i, respoffs, respcycs;
> + unsigned int respcycs_up_min = UINT_MAX;
> + unsigned int respcycs_dn_max = 0;
What are these?
> + unsigned int master_up_slots = 0;
> + unsigned int master_dn_slots = 0;
> + bool up_enabled = false, dn_enabled = false;
> + uint8_t slave_control = 0;
> + int ret;
> +
> + respoffs = ad242x_master_respoffs(&master->node);
> +
> + for_each_available_child_of_node(nodes_np, child_np) {
What are we discovering here? Child devices, or something else?
> + unsigned int dnslot_activity, upslot_activity;
> + unsigned int slave_dn_slots, slave_up_slots;
> + unsigned int respcycs_dn, respcycs_up;
> + struct ad242x_slot_config slot_config;
> +
> + ret = ad242x_read_slot_config(dev, child_np, &slot_config);
> + if (ret < 0) {
> + dev_err(dev, "slot config of slave %d is invalid\n", n);
> + return ret;
> + }
What is a 'slot' defined as?
> + /* See section 3-18 in the datasheet */
Give us a quick explanation.
> + slave_dn_slots = max_t(int, slot_config.dn_n_forward_slots,
> + fls(slot_config.dn_rx_slots));
> + slave_up_slots = max_t(int, slot_config.up_n_forward_slots,
> + fls(slot_config.up_rx_slots));
> +
> + if (n == 0) {
> + master_up_slots = slave_up_slots;
> + master_dn_slots = slave_dn_slots;
> + }
> +
> + /* See Appendix B in the datasheet */
Give us a quick explanation.
> + dnslot_activity = slave_dn_slots *
> + ad242x_bus_bits(master->dn_slot_size,
> + master->dn_slot_alt_fmt);
> + upslot_activity = slave_up_slots *
> + ad242x_bus_bits(master->up_slot_size,
> + master->up_slot_alt_fmt);
> +
> + respcycs_dn = DIV_ROUND_UP(64 + dnslot_activity, 4) + 4*n + 2;
Spaces around the '*'. If it's not clear, use brackets.
> + respcycs_up = respoffs -
> + (DIV_ROUND_UP(64 + upslot_activity, 4) + 1);
No idea what's going on here.
You need to define these magic numbers to make it clear.
> + if (respcycs_dn > respcycs_dn_max)
> + respcycs_dn_max = respcycs_dn;
> +
> + if (respcycs_up < respcycs_up_min)
> + respcycs_up_min = respcycs_up;
> +
> + if (slave_dn_slots > 0)
> + dn_enabled = true;
> +
> + if (slave_up_slots > 0)
> + up_enabled = true;
> +
> + n++;
> + }
> +
> + if (n == 0) {
> + dev_err(dev, "No child nodes specified\n");
> + return -EINVAL;
> + }
> +
> + if (of_property_read_bool(dev->of_node, "adi,invert-xcvr-b")) {
> + ret = regmap_update_bits(regmap, AD242X_CONTROL,
> + AD242X_CONTROL_XCVRBINV,
> + AD242X_CONTROL_XCVRBINV);
> + if (ret < 0)
> + return ret;
> +
> + slave_control = AD242X_CONTROL_XCVRBINV;
> + }
> +
> + if (respcycs_dn_max > respcycs_up_min) {
> + dev_err(dev, "Unsupported bus topology\n");
> + return -EINVAL;
> + }
> +
> + respcycs = (respcycs_dn_max + respcycs_up_min) / 2;
> + ret = regmap_write(regmap, AD242X_RESPCYCS, respcycs);
> + if (ret < 0)
> + return ret;
Comments please.
In fact, comments throughout please.
Anything that isn't absolutely crystal clear should have at least a
little one liner to clarify what is being calculated/set.
> + ret = regmap_update_bits(regmap, AD242X_CONTROL,
> + AD242X_CONTROL_NEWSTRCT,
> + AD242X_CONTROL_NEWSTRCT);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_SWCTL, AD242X_SWCTL_ENSW);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < n; i++) {
> + ret = regmap_write(regmap, AD242X_DISCVRY, respcycs - (4*i));
Spaces.
What is 4?
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_wait_for_irq(master,
> + &master->discover_completion, 35);
Define magic numbers throughout.
> + if (ret < 0) {
> + dev_err(dev, "Discovery of node %d timed out\n", i);
> + return ret;
> + }
> +
> + val = AD242X_SWCTL_MODE(2) | AD242X_SWCTL_ENSW;
> +
> + if (i == 0)
> + ret = regmap_write(regmap, AD242X_SWCTL, val);
> + else
> + ret = ad242x_slave_write(&master->bus, regmap, i,
> + AD242X_SWCTL, val);
> +
> + if (ret < 0)
> + return ret;
> +
> + dev_info(dev, "Node %d discovered\n", i);
> +
> + /* Last node? */
> + if (i == n - 1)
> + break;
> +
> + ret = ad242x_slave_write(&master->bus, regmap, i,
> + AD242X_INTMSK2,
> + AD242X_INTMSK2_DSCDIEN);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_slave_write(&master->bus, regmap, i,
> + AD242X_CONTROL, slave_control);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_slave_write(&master->bus, regmap, i,
> + AD242X_SWCTL, AD242X_SWCTL_ENSW);
> + if (ret < 0)
> + return ret;
> +
> + reinit_completion(&master->discover_completion);
> + }
> +
> + ret = regmap_write(regmap, AD242X_DNSLOTS, master_dn_slots);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_UPSLOTS, master_up_slots);
> + if (ret < 0)
> + return ret;
> +
> + val = 0;
> + if (dn_enabled)
> + val |= AD242X_DATCTL_DNS;
> +
> + if (up_enabled)
> + val |= AD242X_DATCTL_UPS;
> +
> + ret = regmap_write(regmap, AD242X_DATCTL, val);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int ad242x_init_irq(struct ad242x_master *master)
> +{
> + struct regmap *regmap = master->node.regmap;
> + struct device *dev = master->node.dev;
> + int ret;
> +
> + if (master->irq > 0) {
> + ret = devm_request_threaded_irq(dev, master->irq, NULL,
> + ad242x_handle_irq, IRQF_ONESHOT,
> + dev_name(dev), master);
> + if (ret < 0)
> + return ret;
> + }
> +
> + ret = regmap_write(regmap, AD242X_INTMSK0,
> + AD242X_INTMSK0_SRFEIEN | AD242X_INTMSK0_BECIEN |
> + AD242X_INTMSK0_PWREIEN | AD242X_INTMSK0_CRCEIEN |
> + AD242X_INTMSK0_DDEIEN | AD242X_INTMSK0_HCEIEN);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_INTMSK2,
> + AD242X_INTMSK2_DSCDIEN | AD242X_INTMSK2_SLVIRQEN);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct regmap_config ad242x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_reg = ad242x_is_volatile_reg,
> + .writeable_reg = ad242x_is_writeable_reg,
> + .max_register = AD242X_MAX_REG,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int ad242x_master_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + struct device_node *bus_np, *nodes_np, *np;
> + struct device *busdev, *dev = &i2c->dev;
> + struct ad242x_master *master;
> + struct regmap *regmap;
> + unsigned int val;
> + int ret;
> +
> + nodes_np = of_get_child_by_name(dev->of_node, "nodes");
> + if (!nodes_np) {
> + dev_err(dev, "no 'nodes' property given\n");
> + return -EINVAL;
> + }
> +
> + bus_np = of_parse_phandle(dev->of_node, "adi,a2b-bus", 0);
> + if (!bus_np) {
> + dev_err(dev, "no 'adi,a2b-bus' handle specified for master node\n");
> + return -EINVAL;
> + }
> +
> + busdev = bus_find_device_by_of_node(&i2c_bus_type, bus_np);
> + if (!busdev) {
> + dev_err(dev, "'adi,a2b-bus' handle invalid\n");
> + return -EINVAL;
> + }
> +
> + master = devm_kzalloc(dev, sizeof(struct ad242x_master), GFP_KERNEL);
sizeof(*master)
> + if (!master)
> + return -ENOMEM;
> +
> + mutex_init(&master->bus.mutex);
> + init_completion(&master->run_completion);
> + init_completion(&master->discover_completion);
> + dev_set_drvdata(dev, &master->node);
> + i2c_set_clientdata(i2c, master);
What do you think is happening here?
> + regmap = devm_regmap_init_i2c(i2c, &ad242x_regmap_config);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + dev_err(dev, "regmap init failed: %d\n", ret);
"initialisation"
Or even better "Failed to initialise I2C Regmap"
> + return ret;
> + }
> +
> + master->bus.client = to_i2c_client(busdev);
What does 'bus' do in this context?
> + master->node.regmap = regmap;
> + master->node.dev = dev;
> + master->node.master = master;
> + master->node.id = AD242X_MASTER_ID;
> + master->irq = i2c->irq;
> +
> + master->sync_clk = devm_clk_get(dev, "sync");
> + if (IS_ERR(master->sync_clk)) {
> + ret = PTR_ERR(master->sync_clk);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "failed to get sync clk: %d\n", ret);
> +
> + return ret;
> + }
> +
> + if (of_property_read_u32(dev->of_node, "clock-frequency",
> + &master->sync_clk_rate)) {
> + ret = clk_set_rate(master->sync_clk, master->sync_clk_rate);
> + if (ret < 0) {
> + dev_err(dev, "Cannot set sync clock rate: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + master->sync_clk_rate = clk_get_rate(master->sync_clk);
> + if (master->sync_clk_rate != 44100 && master->sync_clk_rate != 48000) {
Please define these magic numbers.
Something descriptive that tells us what the different clock speeds
do.
> + dev_err(dev, "SYNC clock rate %d is invalid\n",
> + master->sync_clk_rate);
> + return -EINVAL;
> + }
> +
> + ret = clk_prepare_enable(master->sync_clk);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable sync clk: %d\n", ret);
> + return ret;
> + }
> +
> + /* Master node setup */
> +
> + ret = regmap_write(regmap, AD242X_CONTROL,
> + AD242X_CONTROL_MSTR | AD242X_CONTROL_SOFTRST);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_wait_for_irq(master, &master->run_completion, 10);
> + if (ret < 0) {
> + dev_err(dev, "timeout waiting for PLL sync: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regmap_update_bits(regmap, AD242X_CONTROL,
> + AD242X_CONTROL_SOFTRST, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_node_probe(&master->node);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_init_irq(master);
> + if (ret < 0) {
> + dev_err(dev, "Unable to set up IRQ: %d", ret);
> + return ret;
> + }
> +
> + /* Slot format setup */
> +
> + of_property_read_u32(dev->of_node, "adi,upstream-slot-size", &val);
> + if (val < 8 || val > 32 || (val % 4 != 0)) {
> + dev_err(dev, "invalid upstream-slot-size %d\n", val);
> + return -EINVAL;
> + }
> + master->up_slot_size = val;
> +
> + of_property_read_u32(dev->of_node, "adi,downstream-slot-size", &val);
> + if (val < 8 || val > 32 || (val % 4 != 0)) {
> + dev_err(dev, "invalid downstream-slot-size %d\n", val);
> + return -EINVAL;
> + }
> + master->dn_slot_size = val;
> +
> + master->dn_slot_alt_fmt =
> + of_property_read_bool(dev->of_node,
> + "adi,alternate-downstream-slot-format");
> + master->up_slot_alt_fmt =
> + of_property_read_bool(dev->of_node,
> + "adi,alternate-upstream-slot-format");
Obviously this all needs to be run past the DT maintainer(s).
> + val = AD242X_SLOTFMT_DNSIZE(master->dn_slot_size) |
> + AD242X_SLOTFMT_UPSIZE(master->up_slot_size);
> +
> + if (master->dn_slot_alt_fmt)
> + val |= AD242X_SLOTFMT_DNFMT;
> +
> + if (master->up_slot_alt_fmt)
> + val |= AD242X_SLOTFMT_UPFMT;
> +
> + ret = regmap_write(regmap, AD242X_SLOTFMT, val);
> + if (ret < 0)
> + return ret;
> +
> + /* Node discovery and MFD setup */
> +
> + ret = ad242x_discover(master, nodes_np);
> + if (ret < 0) {
> + dev_err(dev, "error discovering nodes: %d\n", ret);
> + return ret;
> + }
> +
> + ret = ad242x_node_add_mfd_cells(dev);
Why is this called twice with the same children?
> + if (ret < 0) {
> + dev_err(dev, "failed to add MFD devices %d\n", ret);
> + return ret;
> + }
> +
> + /* Register platform devices for nodes */
> +
> + for_each_available_child_of_node(nodes_np, np)
> + of_platform_device_create(np, NULL, dev);
What are you doing here?
Either use OF to register all child devices OR use MFD, not a mixture
of both.
> + of_node_put(nodes_np);
> +
> + return 0;
> +}
> +
> +static int ad242x_master_remove(struct i2c_client *i2c)
> +{
> + struct ad242x_master *master = i2c_get_clientdata(i2c);
> +
> + if (master->sync_clk)
> + clk_disable_unprepare(master->sync_clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ad242x_master_of_match[] = {
> + { .compatible = "adi,ad2428w-master" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad242x_master_of_match);
> +
> +static const struct i2c_device_id ad242x_master_i2c_id[] = {
> + {"ad242x-master", 0},
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, ad242x_master_i2c_id);
If you one the OF match table, you don't need this empty I2C table.
Grep for probe_new.
> +static struct i2c_driver ad242x_master_i2c_driver = {
> + .driver = {
> + .name = "ad242x-master",
> + .of_match_table = ad242x_master_of_match,
> + },
> + .probe = ad242x_master_probe,
> + .remove = ad242x_master_remove,
> + .id_table = ad242x_master_i2c_id,
> +};
> +
Remove this line.
> +module_i2c_driver(ad242x_master_i2c_driver);
> +
> +MODULE_DESCRIPTION("AD242x master master driver");
Typo.
> +MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mfd/ad242x-node.c b/drivers/mfd/ad242x-node.c
> new file mode 100644
> index 000000000000..f9db689380a7
> --- /dev/null
> +++ b/drivers/mfd/ad242x-node.c
> @@ -0,0 +1,262 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/ad242x.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +/* See Table 7-43 in the datasheet */
More information please.
> +static int ad242x_tdmmode_index(unsigned int mode, bool slave)
> +{
> + switch (mode) {
> + case 2:
> + return 0;
> + case 4:
> + return 1;
> + case 8:
> + return 2;
> + case 12:
> + return slave ? -EINVAL : 3;
> + case 16:
> + return 4;
> + case 20:
> + return slave ? -EINVAL : 5;
> + case 24:
> + return slave ? -EINVAL : 6;
> + case 32:
> + return 7;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +int ad242x_node_probe(struct ad242x_node *node)
> +{
> + struct device_node *np = node->dev->of_node;
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(node->regmap, AD242X_VENDOR, &val);
> + if (ret < 0) {
> + dev_err(node->dev, "failed to read VENDOR register %d\n", ret);
Please re-write all of your kernel log messages to be user friendly.
> + return ret;
> + }
> +
> + if (val != 0xad) {
No magic numbers - please define them all.
> + dev_err(node->dev, "bogus value 0x%02x in VENDOR register\n",
> + val);
> + return -ENODEV;
> + }
> +
> + ret = regmap_read(node->regmap, AD242X_PRODUCT, &val);
> + if (ret < 0) {
> + dev_err(node->dev, "failed to read PRODUCT register %d\n",
> + ret);
> + return ret;
> + }
> +
> + if (val != 0x28) {
> + dev_err(node->dev, "bogus value 0x%02x in PRODUCT register\n",
> + val);
> + return -ENODEV;
> + }
> +
> + ret = regmap_read(node->regmap, AD242X_VERSION, &val);
> + if (ret < 0) {
> + dev_err(node->dev, "failed to read VERSION register %d\n", ret);
> + return ret;
> + }
> +
> + if (node->id == AD242X_MASTER_ID)
> + dev_info(node->dev,
> + "Detected AD242x master node, version %d.%d\n",
> + val >> 4, val & 0xf);
> + else
> + dev_info(node->dev,
> + "Detected AD242x slave node, version %d.%d, id %d\n",
> + val >> 4, val & 0xf, node->id);
> +
> + ret = regmap_read(node->regmap, AD242X_CAPABILITY, &val);
> + if (ret < 0) {
> + dev_err(node->dev, "failed to read CAPABILITY register %d\n",
> + ret);
> + return ret;
> + }
> +
> + node->caps = val;
> +
> + val = 0;
> +
> + if (of_property_read_bool(np, "adi,spread-a2b-clock"))
> + val |= AD242X_PLLCTL_SSMODE_AB;
> + else if (of_property_read_bool(np, "adi,spread-a2b-i2s-clock"))
> + val |= AD242X_PLLCTL_SSMODE_AB_I2S;
> +
> + if (of_property_read_bool(np, "adi,spread-spectrum-high"))
> + val |= AD242X_PLLCTL_SSDEPTH;
> +
> + ret = regmap_write(node->regmap, AD242X_PLLCTL, val);
> + if (ret < 0) {
> + dev_err(node->dev, "failed to write PLLCTL register %d\n", ret);
> + return ret;
> + }
> +
> + /* I2S global setup */
> +
> + of_property_read_u32(np, "adi,tdm-mode", &node->tdm_mode);
> + of_property_read_u32(np, "adi,tdm-slot-size", &node->tdm_slot_size);
> +
> + ret = ad242x_tdmmode_index(node->tdm_mode, false);
> + if (ret < 0) {
> + dev_err(node->dev, "invalid TDM mode %d\n", node->tdm_mode);
> + return -EINVAL;
> + }
> +
> + val = AD242X_I2SGCTL_TDMMODE(ret);
> +
> + if (node->tdm_slot_size == 16) {
> + val |= AD242X_I2SGCTL_TDMSS;
> + } else if (node->tdm_slot_size != 32) {
> + dev_err(node->dev, "invalid TDM slot size %d\n",
> + node->tdm_slot_size);
> + return -EINVAL;
> + }
> +
> + if (of_property_read_bool(np, "adi,alternating-sync"))
> + val |= AD242X_I2SGCTL_ALT;
> +
> + if (of_property_read_bool(np, "adi,early-sync"))
> + val |= AD242X_I2SGCTL_EARLY;
> +
> + if (of_property_read_bool(np, "adi,invert-sync"))
> + val |= AD242X_I2SGCTL_INV;
> +
> + ret = regmap_write(node->regmap, AD242X_I2SGCTL, val);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct mfd_cell ad242x_mfd_cells[] = {
> + {
> + .of_compatible = "adi,ad2428w-i2c",
> + .name = "ad242x-i2c",
Swap these around. Or better still, use the macros found in:
include/linux/mfd/core.h
> + },
> + {
> + .of_compatible = "adi,ad2428w-gpio",
> + .name = "ad242x-gpio",
> + },
> + {
> + .of_compatible = "adi,ad2428w-clk",
> + .name = "ad242x-clk",
> + },
> + {
> + .of_compatible = "adi,ad2428w-codec",
> + .name = "ad242x-codec",
> + },
> +};
> +
> +int ad242x_node_add_mfd_cells(struct device *dev)
> +{
> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
> + ad242x_mfd_cells,
> + ARRAY_SIZE(ad242x_mfd_cells),
> + NULL, 0, NULL);
> +}
> +
> +static int ad242x_get_slot_mask(const struct device_node *np,
> + const char *propname, u32 *mask)
> +{
> + unsigned int i, num;
> + int ret, proplen;
> + u32 slots[32];
You should define 32 as the maximum number of slots available, then
use it again for most of the random '32's below.
> + if (!of_get_property(np, propname, &proplen))
> + return -ENOENT;
This whole piece becomes simpler if you use:
of_property_read_variable_u32_array()
> + num = proplen / sizeof(u32);
> +
> + if (num > ARRAY_SIZE(slots))
> + return -EOVERFLOW;
> +
> + ret = of_property_read_u32_array(np, propname, slots, num);
> + if (ret < 0)
> + return ret;
> +
> + *mask = 0;
> +
> + for (i = 0; i < num; i++) {
> + if (slots[i] >= 32)
> + return -EINVAL;
> +
> + *mask |= BIT(slots[i]);
> + }
> +
> + return 0;
> +}
> +
> +int ad242x_read_slot_config(struct device *dev,
> + struct device_node *np,
> + struct ad242x_slot_config *config)
> +{
> + struct device_node *dn_np, *up_np;
> + int ret;
> +
> + dn_np = of_get_child_by_name(np, "downstream");
> + if (!dn_np) {
> + dev_err(dev, "no downstream node\n");
> + return -EINVAL;
> + }
> +
> + up_np = of_get_child_by_name(np, "upstream");
> + if (!dn_np) {
> + dev_err(dev, "no upstream node\n");
> + ret = -EINVAL;
> + goto err_put_dn_node;
> + }
> +
> + ret = ad242x_get_slot_mask(dn_np, "rx-slots", &config->dn_rx_slots);
> + if (ret < 0 && ret != -ENOENT) {
If you're going to ignore -ENOENT, why not just return 0?
> + dev_err(dev, "invalid downstream rx-slots property\n");
> + goto err_put_nodes;
> + }
> +
> + of_property_read_u32(dn_np, "#tx-slots", &config->dn_n_tx_slots);
> + of_property_read_u32(dn_np, "#forward-slots",
> + &config->dn_n_forward_slots);
> + if (config->dn_n_tx_slots + config->dn_n_forward_slots >= 32) {
> + dev_err(dev, "invalid downstream tx-slots property\n");
> + goto err_put_nodes;
> + }
> +
> +
Superfluous '\n'.
> + ret = ad242x_get_slot_mask(up_np, "rx-slots", &config->up_rx_slots);
> + if (ret < 0) {
> + dev_err(dev, "invalid upstream rx-slots property\n");
> + goto err_put_nodes;
> + }
> +
> + of_property_read_u32(up_np, "#tx-slots", &config->up_n_tx_slots);
> + of_property_read_u32(up_np, "#forward-slots",
> + &config->up_n_forward_slots);
> + if (config->up_n_tx_slots + config->up_n_forward_slots >= 32) {
> + dev_err(dev, "invalid downstream tx-slots property\n");
> + goto err_put_nodes;
> + }
> +
> +err_put_nodes:
> + of_node_put(up_np);
> +err_put_dn_node:
> + of_node_put(dn_np);
> +
> + return ret;
> +}
> diff --git a/drivers/mfd/ad242x-slave.c b/drivers/mfd/ad242x-slave.c
> new file mode 100644
> index 000000000000..ad255d67a5b6
> --- /dev/null
> +++ b/drivers/mfd/ad242x-slave.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/mfd/ad242x.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +struct ad242x_slave {
> + struct ad242x_node node;
> + struct ad242x_node *master;
> + struct ad242x_slot_config slot_config;
> + unsigned int sync_offset;
> +};
> +
> +int ad242x_slave_read(struct ad242x_i2c_bus *bus,
> + struct regmap *master_regmap,
> + uint8_t node_id, uint8_t reg, unsigned int *val)
> +{
> + int ret;
> +
> + mutex_lock(&bus->mutex);
> +
> + ret = regmap_write(master_regmap, AD242X_NODEADR, node_id);
> + if (ret < 0)
> + goto err_unlock;
> +
> + ret = i2c_smbus_read_byte_data(bus->client, reg);
> + if (ret < 0)
> + goto err_unlock;
> +
> + *val = ret;
> + ret = 0;
> +
> +err_unlock:
> + mutex_unlock(&bus->mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ad242x_slave_read);
> +
> +int ad242x_slave_write(struct ad242x_i2c_bus *bus,
> + struct regmap *master_regmap,
> + uint8_t node_id, uint8_t reg, unsigned int val)
> +{
> + int ret;
> +
> + mutex_lock(&bus->mutex);
> +
> + ret = regmap_write(master_regmap, AD242X_NODEADR, node_id);
> + if (ret < 0)
> + goto err_unlock;
> +
> + ret = i2c_smbus_write_byte_data(bus->client, reg, val);
> + if (ret < 0)
> + goto err_unlock;
> +
> + ret = 0;
> +
> +err_unlock:
> + mutex_unlock(&bus->mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ad242x_slave_write);
> +
> +static int ad242x_slave_regmap_read(void *context, unsigned int reg,
> + unsigned int *val)
> +{
> + struct ad242x_slave *slave = context;
> + struct ad242x_i2c_bus *bus = ad242x_master_get_bus(slave->node.master);
> + struct ad242x_node *mnode = ad242x_master_get_node(slave->node.master);
> +
> + if (reg > 0xff)
> + return -EINVAL;
> +
> + return ad242x_slave_read(bus, mnode->regmap, slave->node.id, reg, val);
> +}
> +
> +static int ad242x_slave_regmap_write(void *context, unsigned int reg,
> + unsigned int val)
> +{
> + struct ad242x_slave *slave = context;
> + struct ad242x_i2c_bus *bus = ad242x_master_get_bus(slave->node.master);
> + struct ad242x_node *mnode = ad242x_master_get_node(slave->node.master);
> +
> + if (val > 0xff || reg > 0xff)
> + return -EINVAL;
> +
> + return ad242x_slave_write(bus, mnode->regmap, slave->node.id, reg, val);
> +}
> +
> +static const struct regmap_config ad242x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_reg = ad242x_is_volatile_reg,
> + .writeable_reg = ad242x_is_writeable_reg,
> + .reg_read = ad242x_slave_regmap_read,
> + .reg_write = ad242x_slave_regmap_write,
> + .max_register = AD242X_MAX_REG,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int ad242x_calc_sync_offset(unsigned int val)
> +{
> + if (val == 0)
> + return 0;
> +
> + if (val > 127)
> + return -EINVAL;
> +
> + return 256 - val;
More magic numbers to define.
> +}
> +
> +static int ad242x_slave_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ad242x_slave *slave;
> + struct ad242x_node *mnode;
> + struct regmap *regmap;
> + unsigned int val;
> + int i, ret;
> +
> + slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
> + if (!slave)
> + return -ENOMEM;
> +
> + regmap = devm_regmap_init(dev, NULL, slave, &ad242x_regmap_config);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + dev_err(dev, "regmap init failed: %d\n", ret);
> + return ret;
> + }
> +
> + of_property_read_u32(dev->of_node, "reg", &val);
> + slave->node.id = val;
This looks like an abuse of the 'reg' property.
> + slave->node.dev = dev;
> + slave->node.regmap = regmap;
> +
> + mnode = dev_get_drvdata(dev->parent);
What is the parent of the slave?
(it's not clear without looking at the DT I guess)
> + slave->node.master = mnode->master;
> +
> + dev_set_name(dev, "%s-a2b-%d", dev_name(dev->parent), slave->node.id);
> + dev_set_drvdata(dev, &slave->node);
> +
> + ret = ad242x_node_probe(&slave->node);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_read_slot_config(dev, dev->of_node, &slave->slot_config);
> + if (ret < 0) {
> + dev_err(dev, "slot config is invalid: %d\n", ret);
> + return ret;
> + }
> +
> + ret = regmap_write(regmap, AD242X_UPSLOTS,
> + slave->slot_config.up_n_forward_slots);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_DNSLOTS,
> + slave->slot_config.dn_n_forward_slots);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_LUPSLOTS,
> + slave->slot_config.up_n_tx_slots);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_LDNSLOTS,
> + slave->slot_config.dn_n_tx_slots |
> + AD242X_LDNSLOTS_DNMASKEN);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < 4; i++) {
Why 4? Please define it.
> + ret = regmap_write(regmap, AD242X_UPMASK(i),
> + (slave->slot_config.up_rx_slots >> (i * 8)) & 0xff);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_DNMASK(i),
> + (slave->slot_config.dn_rx_slots >> (i * 8)) & 0xff);
> + if (ret < 0)
> + return ret;
> + }
> +
> + of_property_read_u32(dev->of_node, "adi,sync-offset",
> + &slave->sync_offset);
> +
> + ret = ad242x_calc_sync_offset(slave->sync_offset);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_write(regmap, AD242X_SYNCOFFSET, ret);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_node_add_mfd_cells(dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to add MFD devices %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ad242x_slave_of_match[] = {
> + { .compatible = "adi,ad2428w-slave" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad242x_slave_of_match);
> +
> +static struct platform_driver ad242x_slave_driver = {
> + .driver = {
> + .name = "ad242x-slave",
> + .of_match_table = ad242x_slave_of_match,
> + },
> + .probe = ad242x_slave_probe,
> +};
> +
> +module_platform_driver(ad242x_slave_driver);
> +
> +MODULE_DESCRIPTION("AD242x slave node driver");
> +MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/ad242x.h b/include/linux/mfd/ad242x.h
> new file mode 100644
> index 000000000000..02a174824f85
> --- /dev/null
> +++ b/include/linux/mfd/ad242x.h
> @@ -0,0 +1,400 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __LINUX_MFD_AD242X_H
> +#define __LINUX_MFD_AD242X_H
> +
> +#define AD242X_CHIP 0x00
> +#define AD242X_NODEADR 0x01
> +#define AD242X_NODEADR_MASK 0x0f
> +#define AD242X_NODEADR_PERI BIT(5)
> +#define AD242X_NODEADR_BRCST BIT(7)
> +
> +#define AD242X_VENDOR 0x02
> +#define AD242X_PRODUCT 0x03
> +#define AD242X_VERSION 0x04
> +
> +#define AD242X_CAPABILITY 0x05
> +#define AD242X_CAPABILITY_I2C BIT(0)
> +
> +#define AD242X_SWCTL 0x09
> +#define AD242X_SWCTL_ENSW BIT(0)
> +#define AD242X_SWCTL_DIAGMODE BIT(3)
> +#define AD242X_SWCTL_MODE(X) (((X) & 3) << 4)
> +#define AD242X_SWCTL_MODE_MASK (3 << 4)
> +#define AD242X_SWCTL_DISNXT BIT(6)
> +
> +#define AD242X_BCDNSLOTS 0x0a
> +#define AD242X_BCDNSLOTS_MASK 0x3f
> +
> +#define AD242X_LDNSLOTS 0x0b
> +#define AD242X_LDNSLOTS_MASK 0x3f
> +#define AD242X_LDNSLOTS_DNMASKEN BIT(7)
> +
> +#define AD242X_LUPSLOTS 0x0c
> +#define AD242X_LUPSLOTS_MASK 0x3f
> +
> +#define AD242X_DNSLOTS 0x0d
> +#define AD242X_DNSLOTS_MASK 0x3f
> +
> +#define AD242X_UPSLOTS 0x0e
> +#define AD242X_UPSLOTS_MASK 0x3f
> +
> +#define AD242X_RESPCYCS 0x0f
> +
> +#define AD242X_SLOTFMT 0x10
> +#define AD242X_SLOTFMT_DNSIZE(X) ((((X) - 8) >> 2) & 7)
> +#define AD242X_SLOTFMT_DNFMT BIT(3)
> +#define AD242X_SLOTFMT_UPSIZE(X) (((((X) - 8) >> 2) & 7) << 4)
> +#define AD242X_SLOTFMT_UPFMT BIT(7)
> +
> +#define AD242X_DATCTL 0x11
> +#define AD242X_DATCTL_DNS BIT(0)
> +#define AD242X_DATCTL_UPS BIT(1)
> +#define AD242X_DATCTL_ENDSNIFF BIT(5)
> +#define AD242X_DATCTL_STANDBY BIT(7)
> +
> +#define AD242X_CONTROL 0x12
> +#define AD242X_CONTROL_NEWSTRCT BIT(0)
> +#define AD242X_CONTROL_ENDDSC BIT(1)
> +#define AD242X_CONTROL_SOFTRST BIT(2)
> +#define AD242X_CONTROL_SWBYP BIT(3)
> +#define AD242X_CONTROL_XCVRBINV BIT(4)
> +#define AD242X_CONTROL_MSTR BIT(7)
> +
> +#define AD242X_DISCVRY 0x13
> +
> +#define AD242X_SWSTAT 0x14
> +#define AD242X_SWSTAT_FIN BIT(0)
> +#define AD242X_SWSTAT_FAULT BIT(1)
> +#define AD242X_SWSTAT_FAULTCODE(X) (((X) & 0x7) >> 4)
> +#define AD242X_SWSTAT_FAULT_NLOC BIT(7)
> +
> +#define AD242X_INTSTAT 0x15
> +#define AD242X_INTSTAT_IRQ BIT(0)
> +
> +#define AD242X_INTSRC 0x16
> +#define AD242X_INTSRC_INODE 0x0f
> +#define AD242X_INTSRC_SLVINT BIT(6)
> +#define AD242X_INTSRC_MSTINT BIT(7)
> +
> +#define AD242X_INTTYPE 0x17
> +
> +#define AD242X_INTTYPE_DSCDONE 24
> +#define AD242X_INTTYPE_MSTR_RUNNING 255
> +
> +#define AD242X_INTPND0 0x18
> +#define AD242X_INTPDN0_HDCNTERR BIT(0)
> +#define AD242X_INTPDN0_DDERR BIT(1)
> +#define AD242X_INTPDN0_CRCERR BIT(2)
> +#define AD242X_INTPDN0_DPERR BIT(3)
> +#define AD242X_INTPDN0_PWRERR BIT(4)
> +#define AD242X_INTPDN0_BECOVF BIT(5)
> +#define AD242X_INTPDN0_SRFERR BIT(6)
> +#define AD242X_INTPDN0_SRFCRCERR BIT(7)
> +
> +#define AD242X_INTPND1 0x19
> +#define AD242X_INTPND1_IOPND(X) BIT(X)
> +
> +#define AD242X_INTPND2 0x1a
> +#define AD242X_INTPND2_DSCDONE BIT(0)
> +#define AD242X_INTPND2_I2CERR BIT(1)
> +#define AD242X_INTPND2_ICRCERR BIT(2)
> +#define AD242X_INTPND2_SLVIRQ BIT(3)
> +
> +#define AD242X_INTMSK0 0x1b
> +#define AD242X_INTMSK0_HCEIEN BIT(0)
> +#define AD242X_INTMSK0_DDEIEN BIT(1)
> +#define AD242X_INTMSK0_CRCEIEN BIT(2)
> +#define AD242X_INTMSK0_DPEIEN BIT(3)
> +#define AD242X_INTMSK0_PWREIEN BIT(4)
> +#define AD242X_INTMSK0_BECIEN BIT(5)
> +#define AD242X_INTMSK0_SRFEIEN BIT(6)
> +#define AD242X_INTMSK0_SRFCRCEIEN BIT(7)
> +
> +#define AD242X_INTMSK1 0x1c
> +#define AD242X_INTMSK1_IOIRQEN(X) BIT(X)
> +
> +#define AD242X_INTMSK2 0x1d
> +#define AD242X_INTMSK2_DSCDIEN BIT(0)
> +#define AD242X_INTMSK2_I2CEIEN BIT(1)
> +#define AD242X_INTMSK2_ICRCEIEN BIT(2)
> +#define AD242X_INTMSK2_SLVIRQEN BIT(3)
> +
> +#define AD242X_BECCTL 0x1e
> +#define AD242X_BECCTL_ENHDCNT BIT(0)
> +#define AD242X_BECCTL_ENDD BIT(1)
> +#define AD242X_BECCTL_ENCRC BIT(2)
> +#define AD242X_BECCTL_ENDP BIT(3)
> +#define AD242X_BECCTL_ENICRC BIT(4)
> +#define AD242X_BECCTL_THRESHLD(X) ((X) >> 5)
> +
> +#define AD242X_BECNT 0x1f
> +
> +#define AD242X_TESTMODE 0x20
> +#define AD242X_TESTMODE_PRBSUP BIT(0)
> +#define AD242X_TESTMODE_PRBSDN BIT(1)
> +#define AD242X_TESTMODE_PRBSN2N BIT(2)
> +#define AD242X_TESTMODE_RXDPTH(X) ((X) >> 4)
> +
> +#define AD242X_ERRCNT0 0x21
> +#define AD242X_ERRCNT1 0x22
> +#define AD242X_ERRCNT2 0x23
> +#define AD242X_ERRCNT3 0x24
> +
> +#define AD242X_NODE 0x29
> +#define AD242X_NODE_MASK 0xf
> +#define AD242X_NODE_DISCVD BIT(5)
> +#define AD242X_NODE_NLAST BIT(6)
> +#define AD242X_NODE_LAST BIT(7)
> +
> +#define AD242X_DISCSTAT 0x2b
> +#define AD242X_DISCSTAT_DNODE(X) ((X) & 0xf)
> +#define AD242X_DISCSTAT_DSCACT BIT(7)
> +
> +#define AD242X_TXACTL 0x2e
> +#define AD242X_TXACTL_LEVEL_HIGH 0
> +#define AD242X_TXACTL_LEVEL_MEDIUM 2
> +#define AD242X_TXACTL_LEVEL_LOW 3
> +
> +#define AD242X_TXBCTL 0x30
> +#define AD242X_TXBCTL_LEVEL_HIGH 0
> +#define AD242X_TXBCTL_LEVEL_MEDIUM 2
> +#define AD242X_TXBCTL_LEVEL_LOW 3
> +
> +#define AD242X_LINTTYPE 0x3e
> +
> +#define AD242X_I2CCFG 0x3f
> +#define AD242X_I2CCFG_DATARATE BIT(0)
> +#define AD242X_I2CCFG_EACK BIT(1)
> +#define AD242X_I2CCFG_FRAMERATE BIT(2)
> +
> +#define AD242X_PLLCTL 0x40
> +#define AD242X_PLLCTL_SSFREQ(X) ((X) & 3)
> +#define AD242X_PLLCTL_SSDEPTH BIT(2)
> +#define AD242X_PLLCTL_SSMODE_AB (1 << 6)
> +#define AD242X_PLLCTL_SSMODE_AB_I2S (2 << 6)
> +
> +#define AD242X_I2SGCTL 0x41
> +#define AD242X_I2SGCTL_TDMMODE(X) ((X) & 3)
> +#define AD242X_I2SGCTL_RXONDTX1 BIT(3)
> +#define AD242X_I2SGCTL_TDMSS BIT(4)
> +#define AD242X_I2SGCTL_ALT BIT(5)
> +#define AD242X_I2SGCTL_EARLY BIT(6)
> +#define AD242X_I2SGCTL_INV BIT(7)
> +
> +#define AD242X_I2SCTL 0x42
> +#define AD242X_I2SCTL_TX0EN BIT(0)
> +#define AD242X_I2SCTL_TX1EN BIT(1)
> +#define AD242X_I2SCTL_TX2PINTL BIT(2)
> +#define AD242X_I2SCTL_TXBCLKINV BIT(3)
> +#define AD242X_I2SCTL_RX0EN BIT(4)
> +#define AD242X_I2SCTL_RX1EN BIT(5)
> +#define AD242X_I2SCTL_RX2PINTL BIT(6)
> +#define AD242X_I2SCTL_RXBCLKINV BIT(7)
> +
> +#define AD242X_I2SRATE 0x43
> +#define AD242X_I2SRATE_I2SRATE(X) ((X) & 3)
> +#define AD242X_I2SRATE_BCLKRATE(X) (((X) << 3) & 3)
> +#define AD242X_I2SRATE_REDUCE BIT(6)
> +#define AD242X_I2SRATE_SHARE BIT(7)
> +
> +#define AD242X_I2STXOFFSET 0x44
> +#define AD242X_I2STXOFFSET_VAR(X) ((X) & 0x3f)
> +#define AD242X_I2STXOFFSET_TSAFTER BIT(6)
> +#define AD242X_I2STXOFFSET_TSBEFORE BIT(7)
> +
> +#define AD242X_2SRXOFFSET 0x45
> +#define AD242X_I2SRXOFFSET_VAR(X) ((X) & 0x3f)
> +
> +#define AD242X_SYNCOFFSET 0x46
> +
> +#define AD242X_PDMCTL 0x47
> +#define AD242X_PDMCTL_PDM0EN BIT(0)
> +#define AD242X_PDMCTL_PDM0SLOTS BIT(1)
> +#define AD242X_PDMCTL_PDM1EN BIT(2)
> +#define AD242X_PDMCTL_PDM1SLOTS BIT(3)
> +#define AD242X_PDMCTL_HPFEN BIT(4)
> +#define AD242X_PDMCTL_PDMRATE(X) (((X) & 3) << 5)
> +
> +#define AD242X_ERRMGMT 0x48
> +#define AD242X_ERRMGMT_ERRLSB BIT(0)
> +#define AD242X_ERRMGMT_ERRSIG BIT(1)
> +#define AD242X_ERRMGMT_ERRSLOT BIT(2)
> +
> +#define AD242X_GPIODAT 0x4a
> +#define AD242X_GPIODAT_SET 0x4b
> +#define AD242X_GPIODAT_CLR 0x4c
> +#define AD242X_GPIOOEN 0x4d
> +#define AD242X_GPIOIEN 0x4e
> +#define AD242X_GPIODAT_IN 0x4f
> +#define AD242X_PINTEN 0x50
> +#define AD242X_PINTINV 0x51
> +
> +#define AD242X_PINCFG 0x52
> +#define AD242X_PINCFG_DRVSTR BIT(0)
> +#define AD242X_PINCFG_IRQINV BIT(4)
> +#define AD242X_PINCFG_IRQTS BIT(5)
> +
> +#define AD242X_I2STEST 0x53
> +#define AD242X_I2STEST_PATTRN2TX BIT(0)
> +#define AD242X_I2STEST_LOOPBK2TX BIT(1)
> +#define AD242X_I2STEST_RX2LOOPBK BIT(2)
> +#define AD242X_I2STEST_SELRX1 BIT(3)
> +#define AD242X_I2STEST_BUSLOOPBK BIT(4)
> +
> +#define AD242X_RAISE 0x54
> +
> +#define AD242X_GENERR 0x55
> +#define AD242X_GENERR_GENHCERR BIT(0)
> +#define AD242X_GENERR_GENDDERR BIT(1)
> +#define AD242X_GENERR_GENCRCERR BIT(2)
> +#define AD242X_GENERR_GENDPERR BIT(3)
> +#define AD242X_GENERR_GENICRCERR BIT(4)
> +
> +#define AD242X_I2SRRATE 0x56
> +#define AD242X_I2SRRATE_RRDIV(X) ((X) & 0x3f)
> +#define AD242X_I2SRRATE_RBUS BIT(7)
> +
> +#define AD242X_I2SRRCTL 0x57
> +#define AD242X_I2SRRCTL_ENVLSB BIT(0)
> +#define AD242X_I2SRRCTL_ENXBIT BIT(1)
> +#define AD242X_I2SRRCTL_ENSTRB BIT(4)
> +#define AD242X_I2SRRCTL_STRBDIR BIT(5)
> +
> +#define AD242X_I2SRRSOFFS 0x58
> +
> +#define AD242X_CLK1CFG 0x59
> +#define AD242X_CLK2CFG 0x5a
> +#define AD242X_CLKCFG_DIV(X) ((X) & 0xf)
> +#define AD242X_CLKCFG_DIVMSK 0xf
> +#define AD242X_CLKCFG_PDIV32 BIT(5)
> +#define AD242X_CLKCFG_INV BIT(6)
> +#define AD242X_CLKCFG_EN BIT(7)
> +
> +#define AD242X_BMMCFG 0x5b
> +#define AD242X_BMMCFG_BMMEN BIT(0)
> +#define AD242X_BMMCFG_BMMRXEN BIT(1)
> +#define AD242X_BMMCFG_BMMNDSC BIT(2)
> +
> +#define AD242X_PDMCTL2 0x5d
> +#define AD242X_PDMCTL2_DEST_A2B 0
> +#define AD242X_PDMCTL2_DEST_DTX 1
> +#define AD242X_PDMCTL2_DEST_A2B_DTX 2
> +
> +#define AD242X_UPMASK(X) (0x60 + ((X) & 3))
> +
> +#define AD242X_UPOFFSET 0x64
> +#define AD242X_UPOFFSET_VAL(X) ((X) & 0x1f)
> +
> +#define AD242X_DNMASK(X) (0x65 + ((X) % 3))
> +
> +#define AD242X_DNOFFSET 0x69
> +#define AD242X_DNOFFSET_VAL(X) ((X) & 0x1f)
> +
> +#define AD242X_CHIPID(X) ((X) + 0x6a)
> +
> +#define AD242X_GPIODEN 0x80
> +#define AD242X_GPIOD_MSK(X) ((X) + 0x81)
> +
> +#define AD242X_GPIODDAT 0x89
> +#define AD242X_GPIODINV 0x8a
> +
> +#define AD242X_MAX_REG 0x9b
> +
> +static inline bool ad242x_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case AD242X_VENDOR:
> + case AD242X_PRODUCT:
> + case AD242X_VERSION:
> + case AD242X_CAPABILITY:
> + case AD242X_SWSTAT:
> + case AD242X_INTSTAT:
> + case AD242X_INTSRC:
> + case AD242X_INTTYPE:
> + case AD242X_INTPND0:
> + case AD242X_INTPND1:
> + case AD242X_INTPND2:
> + case AD242X_BECNT:
> + case AD242X_ERRCNT0:
> + case AD242X_ERRCNT1:
> + case AD242X_ERRCNT2:
> + case AD242X_ERRCNT3:
> + case AD242X_NODE:
> + case AD242X_DISCSTAT:
> + case AD242X_LINTTYPE:
> + case AD242X_GPIODAT:
> + case AD242X_GPIODAT_IN:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static inline bool ad242x_is_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + /* Write-to-clean registers */
> + switch (reg) {
> + case AD242X_INTPND0:
> + case AD242X_INTPND1:
> + case AD242X_INTPND2:
> + case AD242X_BECNT:
> + return true;
> + default:
> + return !ad242x_is_volatile_reg(dev, reg);
> + }
> +}
> +
> +#define AD242X_MASTER_ID 0xff
> +
> +struct ad242x_master;
> +
> +struct ad242x_i2c_bus {
> + struct i2c_client *client;
> + struct mutex mutex;
> +};
> +
> +struct ad242x_node {
> + struct device *dev;
> + struct regmap *regmap;
> + struct ad242x_master *master;
> + unsigned int tdm_mode;
> + unsigned int tdm_slot_size;
> + uint8_t id;
> + uint8_t caps;
> +};
> +
> +struct ad242x_slot_config {
> + unsigned int dn_rx_slots;
> + unsigned int dn_n_tx_slots;
> + unsigned int dn_n_forward_slots;
> + unsigned int up_rx_slots;
> + unsigned int up_n_tx_slots;
> + unsigned int up_n_forward_slots;
> +};
> +
> +int ad242x_read_slot_config(struct device *dev,
> + struct device_node *np,
> + struct ad242x_slot_config *config);
> +
> +static inline bool ad242x_node_is_master(struct ad242x_node *node)
> +{
> + return node->id == AD242X_MASTER_ID;
> +}
> +
> +int ad242x_node_probe(struct ad242x_node *node);
> +int ad242x_node_add_mfd_cells(struct device *dev);
> +
> +struct ad242x_node *ad242x_master_get_node(struct ad242x_master *master);
> +struct ad242x_i2c_bus *ad242x_master_get_bus(struct ad242x_master *master);
> +const char *ad242x_master_get_clk_name(struct ad242x_master *master);
> +unsigned int ad242x_master_get_clk_rate(struct ad242x_master *master);
> +
> +int ad242x_slave_read(struct ad242x_i2c_bus *bus,
> + struct regmap *master_regmap,
> + uint8_t node_id, uint8_t reg, unsigned int *val);
> +int ad242x_slave_write(struct ad242x_i2c_bus *bus,
> + struct regmap *master_regmap,
> + uint8_t node_id, uint8_t reg, unsigned int val);
> +
> +#endif
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-17 13:39 ` Lee Jones
@ 2019-12-17 13:46 ` Lee Jones
2019-12-17 19:36 ` Daniel Mack
2019-12-17 19:24 ` Daniel Mack
1 sibling, 1 reply; 34+ messages in thread
From: Lee Jones @ 2019-12-17 13:46 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lars,
pascal.huerst
On Tue, 17 Dec 2019, Lee Jones wrote:
> On Mon, 09 Dec 2019, Daniel Mack wrote:
>
> > The core driver for these devices is split into several parts.
> >
> > The master node driver is an I2C client. It is responsible for
> > bringing up the bus topology and discovering the slave nodes.
> > This process requries some knowlegde of the slave node configuration
> > to program the bus timings correctly, so the master drivers walks
> > the tree of nodes in the devicetree. The slave driver handles platform
> > devices that are instantiated by the master node driver after
> > discovery has finished.
> >
> > Master nodes expose two addresses on the I2C bus, one (referred to as
> > 'BASE' in the datasheet) for accessing registers on the transceiver
> > node itself, and one (referred to as 'BUS') for accessing remote
> > registers, either on the remote transceiver itself, or on I2C hardware
> > connected to that remote transceiver, which then acts as a remote I2C
> > bus master.
> >
> > In order to allow MFD sub-devices to be registered as children of
> > either the master or any slave node, the details on how to access the
> > registers are hidden behind a regmap config. A pointer to the regmap
> > is then exposed in the struct shared with the sub-devices.
> >
> > The ad242x-bus driver is a simple proxy that occupies the BUS I2C
> > address and which is referred to through a devicetree handle by the
> > master driver.
> >
> > For the discovery process, the driver has to wait for an interrupt
> > to occur. In case no interrupt is configured in DT, the driver falls
> > back to interrupt polling. After the discovery phase is completed,
> > interrupts are only needed for error handling and GPIO handling,
> > both of which is not currenty implemented.
> >
> > Code common to both the master and the slave driver lives in
> > 'ad242x-node.c'.
> >
> > Signed-off-by: Daniel Mack <daniel@zonque.org>
> >
> > mfd
>
> ?
>
> > ---
> > drivers/mfd/Kconfig | 11 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/ad242x-bus.c | 42 +++
> > drivers/mfd/ad242x-master.c | 611 ++++++++++++++++++++++++++++++++++++
> > drivers/mfd/ad242x-node.c | 262 ++++++++++++++++
> > drivers/mfd/ad242x-slave.c | 234 ++++++++++++++
> > include/linux/mfd/ad242x.h | 400 +++++++++++++++++++++++
>
> This device, or at least the way it's been coded is batty!
>
> It's going to need a lot of massaging before being accepted.
One thing I should mention upfront; there is too much code "doing
things" in here for it to be an MFD. MFDs don't care about; syncs,
slots, TDM, inverting lines, upstreams, downstreams, etc etc etc.
Anything remotely technical or functional, the code that "does things"
should be moved out to the relevant areas. In the case of this
device, that's looking like one of the Audio related subsystems.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-17 13:46 ` Lee Jones
@ 2019-12-17 19:36 ` Daniel Mack
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-17 19:36 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lars,
pascal.huerst
Hi Lee,
On 12/17/19 2:46 PM, Lee Jones wrote:
> One thing I should mention upfront; there is too much code "doing
> things" in here for it to be an MFD. MFDs don't care about; syncs,
> slots, TDM, inverting lines, upstreams, downstreams, etc etc etc.
> Anything remotely technical or functional, the code that "does things"
> should be moved out to the relevant areas. In the case of this
> device, that's looking like one of the Audio related subsystems.
Okay, that's good to know.
I in fact considered that when I started working on it; after all, A2B
stands for "automotive audio bus". The reason why I didn't do it was the
fact that these devices certainly do have multiple functions, where
audio is just one of them, and there needs to be a 'top-level' layer
that enables all these functions and does the node discovery etc. Hence
I thought it's cleaner to separate things that way.
I can move things over to the ASoC layer for the next iteration, and
then maybe also merge the codec driver with the baseline drivers. Let's
see how this looks like then.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-17 13:39 ` Lee Jones
2019-12-17 13:46 ` Lee Jones
@ 2019-12-17 19:24 ` Daniel Mack
2019-12-18 11:20 ` Luca Ceresoli
1 sibling, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-17 19:24 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lars,
pascal.huerst
Hi Lee,
Thanks a lot for your review, much appreciated.
I'll leave out the trivial things from your reply and address those in a v2.
I'm well aware of the fact that there are some details in this driver
stack that deserve discussion. I wanted to title this set 'RFC', but I
forgot to do so when I sent it.
On 12/17/19 2:39 PM, Lee Jones wrote:
> On Mon, 09 Dec 2019, Daniel Mack wrote:
>
>> The core driver for these devices is split into several parts.
>>
>> The master node driver is an I2C client. It is responsible for
>> bringing up the bus topology and discovering the slave nodes.
>> This process requries some knowlegde of the slave node configuration
>> to program the bus timings correctly, so the master drivers walks
>> the tree of nodes in the devicetree. The slave driver handles platform
>> devices that are instantiated by the master node driver after
>> discovery has finished.
>>
>> Master nodes expose two addresses on the I2C bus, one (referred to as
>> 'BASE' in the datasheet) for accessing registers on the transceiver
>> node itself, and one (referred to as 'BUS') for accessing remote
>> registers, either on the remote transceiver itself, or on I2C hardware
>> connected to that remote transceiver, which then acts as a remote I2C
>> bus master.
>>
>> In order to allow MFD sub-devices to be registered as children of
>> either the master or any slave node, the details on how to access the
>> registers are hidden behind a regmap config. A pointer to the regmap
>> is then exposed in the struct shared with the sub-devices.
>>
>> The ad242x-bus driver is a simple proxy that occupies the BUS I2C
>> address and which is referred to through a devicetree handle by the
>> master driver.
>>
>> For the discovery process, the driver has to wait for an interrupt
>> to occur. In case no interrupt is configured in DT, the driver falls
>> back to interrupt polling. After the discovery phase is completed,
>> interrupts are only needed for error handling and GPIO handling,
>> both of which is not currenty implemented.
>>
>> Code common to both the master and the slave driver lives in
>> 'ad242x-node.c'.
>> +++ b/drivers/mfd/ad242x-bus.c
>> @@ -0,0 +1,42 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/mfd/ad242x.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +
>> +static int ad242x_bus_i2c_probe(struct i2c_client *i2c,
>> + const struct i2c_device_id *id)
>> +{
>> + dev_set_drvdata(&i2c->dev, i2c);
>> + i2c_set_clientdata(i2c, &i2c->dev);
>
> Please explain to me what you think is happening here.
>
>> + return 0;
>> +}
>
> What does this driver do? Seems kinda pointless?
As explained in the commit log, these devices expose two addresses on
the i2c bus, and each of which exists for a distinct purpose. The
primary one is used to access registers on the master node itself, the
second one is proxying traffic to remote nodes.
Now, the question is how to support that, and the approach chosen here
is to have a dummy driver sitting on the 2nd address, and to reach out
to it via a DT phandle from the master node. I don't like that much
either, but I'm not aware of a cleaner way to bind two addresses with
one driver. If there is any, I'd be happy to change that.
>> +struct ad242x_node *ad242x_master_get_node(struct ad242x_master *master)
>> +{
>> + return &master->node;
>> +}
>> +EXPORT_SYMBOL_GPL(ad242x_master_get_node);
>> +
>> +struct ad242x_i2c_bus *ad242x_master_get_bus(struct ad242x_master *master)
>> +{
>> + return &master->bus;
>> +}
>> +EXPORT_SYMBOL_GPL(ad242x_master_get_bus);
>> +
>> +const char *ad242x_master_get_clk_name(struct ad242x_master *master)
>> +{
>> + return __clk_get_name(master->sync_clk);
>> +}
>> +EXPORT_SYMBOL_GPL(ad242x_master_get_clk_name);
>> +
>> +unsigned int ad242x_master_get_clk_rate(struct ad242x_master *master)
>> +{
>> + return master->sync_clk_rate;
>> +}
>> +EXPORT_SYMBOL_GPL(ad242x_master_get_clk_rate);
>
> All of these functions provide abstraction for the sake of
> abstraction. They should be removed and replaced with the code
> contained within them.
That would mean to expose the internals of the struct, which is what I
wanted to avoid. But okay, I'll see how the respin of this driver looks
like and reconsider.
>> + return ret == 0 ? -ETIMEDOUT : 0;
>> +}
>> +
>> +/* See Table 3-2 in the datasheet */
>
> Do you provide a link to the datasheet anywhere?
Yes, in the cover letter. But you're right, I can add that to the code
as well. Will do.
>> +static unsigned int ad242x_master_respoffs(struct ad242x_node *node)
>> +{
>> + if (node->tdm_mode == 2 && node->tdm_slot_size == 16)
>> + return 238;
>> +
>> + if ((node->tdm_mode == 2 && node->tdm_slot_size == 32) ||
>> + (node->tdm_mode == 4 && node->tdm_slot_size == 16))
>> + return 245;
>> +
>> + return 248;
>
> No magic numbers please. You need to define them.
I generally agree, but these are just magic numbers in the datasheet.
You're thinking of something like this, next to a comment?
#define AD242X_RESPOFFS_248 248
>> + master->sync_clk_rate = clk_get_rate(master->sync_clk);
>> + if (master->sync_clk_rate != 44100 && master->sync_clk_rate != 48000) {
>
> Please define these magic numbers.
>
> Something descriptive that tells us what the different clock speeds
> do.
The device can only operate on one of the two clock speeds. I can add a
comment on that, but do you think defines for these two particular
constants would make the code more readable?
>> + master->dn_slot_alt_fmt =
>> + of_property_read_bool(dev->of_node,
>> + "adi,alternate-downstream-slot-format");
>> + master->up_slot_alt_fmt =
>> + of_property_read_bool(dev->of_node,
>> + "adi,alternate-upstream-slot-format");
>
> Obviously this all needs to be run past the DT maintainer(s).
Yes, absolutely. I believe I copied them all in the thread.
>> + /* Register platform devices for nodes */
>> +
>> + for_each_available_child_of_node(nodes_np, np)
>> + of_platform_device_create(np, NULL, dev);
>
> What are you doing here?
>
> Either use OF to register all child devices OR use MFD, not a mixture
> of both.
Okay, this one is interesting, and I'd really appreciate some guidance here.
What the master node driver does here is register a number of slave node
devices which are then handled by the driver in ad242x-slave.c. Because
the master is not able to auto-probe slave nodes, users need to define
them manually in DT. The driver will try to discover them at probe time,
and then create a platform device for each of them.
Both the master driver and the slave driver are then registering
sub-devices for functions (such as GPIO, clk, audio-codec etc) via MFD,
as they can be enabled on both device types. And the function-drivers
are agnostic about whether the device type (master or slave) they
communicate with.
What would be a good way to support this scheme if not by a mixture of
child devices and MFD?
>> +static int ad242x_slave_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct ad242x_slave *slave;
>> + struct ad242x_node *mnode;
>> + struct regmap *regmap;
>> + unsigned int val;
>> + int i, ret;
>> +
>> + slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
>> + if (!slave)
>> + return -ENOMEM;
>> +
>> + regmap = devm_regmap_init(dev, NULL, slave, &ad242x_regmap_config);
>> + if (IS_ERR(regmap)) {
>> + ret = PTR_ERR(regmap);
>> + dev_err(dev, "regmap init failed: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + of_property_read_u32(dev->of_node, "reg", &val);
>> + slave->node.id = val;
>
> This looks like an abuse of the 'reg' property.
I had my doubts about that as well, but I've found my places that do
similar things. Not sure, but of course that can be renamed.
Again, thanks for your review.
Daniel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-17 19:24 ` Daniel Mack
@ 2019-12-18 11:20 ` Luca Ceresoli
0 siblings, 0 replies; 34+ messages in thread
From: Luca Ceresoli @ 2019-12-18 11:20 UTC (permalink / raw)
To: Daniel Mack, Lee Jones
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lars,
pascal.huerst
Hi Daniel,
On 17/12/19 20:24, Daniel Mack wrote:
>>> +++ b/drivers/mfd/ad242x-bus.c
>>> @@ -0,0 +1,42 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +#include <linux/i2c.h>
>>> +#include <linux/init.h>
>>> +#include <linux/mfd/ad242x.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +
>>> +static int ad242x_bus_i2c_probe(struct i2c_client *i2c,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + dev_set_drvdata(&i2c->dev, i2c);
>>> + i2c_set_clientdata(i2c, &i2c->dev);
>>
>> Please explain to me what you think is happening here.
>>
>>> + return 0;
>>> +}
>>
>> What does this driver do? Seems kinda pointless?
>
> As explained in the commit log, these devices expose two addresses on
> the i2c bus, and each of which exists for a distinct purpose. The
> primary one is used to access registers on the master node itself, the
> second one is proxying traffic to remote nodes.
>
> Now, the question is how to support that, and the approach chosen here
> is to have a dummy driver sitting on the 2nd address, and to reach out
> to it via a DT phandle from the master node. I don't like that much
> either, but I'm not aware of a cleaner way to bind two addresses with
> one driver. If there is any, I'd be happy to change that.
Have a look at i2c_new_dummy_device(), perhaps it is what you need here.
--
Luca
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-09 18:35 ` [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers Daniel Mack
2019-12-17 13:39 ` Lee Jones
@ 2019-12-17 19:16 ` Pierre-Louis Bossart
2019-12-18 9:40 ` Daniel Mack
1 sibling, 1 reply; 34+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-17 19:16 UTC (permalink / raw)
To: Daniel Mack, linux-kernel, linux-gpio, linux-i2c, alsa-devel,
devicetree, linux-clk
Cc: lars, sboyd, mturquette, robh+dt, broonie, pascal.huerst, lee.jones
> +config MFD_AD242X
> + bool "Analog Devices AD242x A2B support"
> + select MFD_CORE
> + select REGMAP_I2C
> + depends on I2C=y && OF
is there a specific reason why I2C needs to be built-in (as opposed to 'm')?
> +/* See Table 3-2 in the datasheet */
is the datasheet public? I thought it was only available under NDA.
> + master->sync_clk = devm_clk_get(dev, "sync");
> + if (IS_ERR(master->sync_clk)) {
> + ret = PTR_ERR(master->sync_clk);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "failed to get sync clk: %d\n", ret);
> +
> + return ret;
> + }
> +
> + if (of_property_read_u32(dev->of_node, "clock-frequency",
> + &master->sync_clk_rate)) {
> + ret = clk_set_rate(master->sync_clk, master->sync_clk_rate);
shouldn't you check the rate before setting it?
> + if (ret < 0) {
> + dev_err(dev, "Cannot set sync clock rate: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + master->sync_clk_rate = clk_get_rate(master->sync_clk);
> + if (master->sync_clk_rate != 44100 && master->sync_clk_rate != 48000) {
> + dev_err(dev, "SYNC clock rate %d is invalid\n",
> + master->sync_clk_rate);
> + return -EINVAL;
> + }
this is a bit odd, you set the rate in case there is a property but get
it anyways. the last block could be an else?
> +
> + ret = clk_prepare_enable(master->sync_clk);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable sync clk: %d\n", ret);
> + return ret;
> + }
> +
> + /* Master node setup */
> +
> + ret = regmap_write(regmap, AD242X_CONTROL,
> + AD242X_CONTROL_MSTR | AD242X_CONTROL_SOFTRST);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad242x_wait_for_irq(master, &master->run_completion, 10);
what is 10?
> +static int ad242x_master_remove(struct i2c_client *i2c)
> +{
> + struct ad242x_master *master = i2c_get_clientdata(i2c);
> +
> + if (master->sync_clk)
> + clk_disable_unprepare(master->sync_clk);
earlier you tested for IS_ERR(master->sync_clk)?
> + for (i = 0; i < 4; i++) {
what is 4? 4 hops?
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [alsa-devel] [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers
2019-12-17 19:16 ` [alsa-devel] " Pierre-Louis Bossart
@ 2019-12-18 9:40 ` Daniel Mack
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-18 9:40 UTC (permalink / raw)
To: Pierre-Louis Bossart, linux-kernel, linux-gpio, linux-i2c,
alsa-devel, devicetree, linux-clk
Cc: lars, sboyd, mturquette, robh+dt, broonie, pascal.huerst, lee.jones
Hi Pierre,
Thanks for looking into this!
On 12/17/19 8:16 PM, Pierre-Louis Bossart wrote:
> is the datasheet public? I thought it was only available under NDA.
It was until recently, but it is now public:
https://www.analog.com/media/en/technical-documentation/user-guides/AD242x_TRM_Rev1.1.pdf
>> + master->sync_clk = devm_clk_get(dev, "sync");
>> + if (IS_ERR(master->sync_clk)) {
>> + ret = PTR_ERR(master->sync_clk);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "failed to get sync clk: %d\n", ret);
>> +
>> + return ret;
>> + }
>> +
>> + if (of_property_read_u32(dev->of_node, "clock-frequency",
>> + &master->sync_clk_rate)) {
>> + ret = clk_set_rate(master->sync_clk, master->sync_clk_rate);
>
> shouldn't you check the rate before setting it?
>
>> + if (ret < 0) {
>> + dev_err(dev, "Cannot set sync clock rate: %d\n", ret);
>> + return ret;
>> + }
>> + }
>> +
>> + master->sync_clk_rate = clk_get_rate(master->sync_clk);
>> + if (master->sync_clk_rate != 44100 && master->sync_clk_rate !=
>> 48000) {
>> + dev_err(dev, "SYNC clock rate %d is invalid\n",
>> + master->sync_clk_rate);
>> + return -EINVAL;
>> + }
>
> this is a bit odd, you set the rate in case there is a property but get
> it anyways. the last block could be an else?
The idea is: if 'clock-frequency' is given, we use it to set the clock,
otherwise we rely on the clock having one of the two allowed rates. This
way, we also catch setups where the clock provider cannot generated the
desired frequency, or where the value of 'clock-frequency' is illegal.
>> + ret = clk_prepare_enable(master->sync_clk);
>> + if (ret < 0) {
>> + dev_err(dev, "failed to enable sync clk: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + /* Master node setup */
>> +
>> + ret = regmap_write(regmap, AD242X_CONTROL,
>> + AD242X_CONTROL_MSTR | AD242X_CONTROL_SOFTRST);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = ad242x_wait_for_irq(master, &master->run_completion, 10);
>
> what is 10?
Milliseconds. The parameter needs to get a better name I figure.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 07/10] i2c: Add driver for AD242x bus controller
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (6 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 06/10] mfd: Add core driver for AD242x A2B transceivers Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-12 16:11 ` Luca Ceresoli
2019-12-09 18:35 ` [PATCH 08/10] gpio: Add driver for AD242x GPIO controllers Daniel Mack
` (3 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This device must be instantiated as a sub-device of the AD242x MFD
device.
In order to access remote I2C peripherals, the master node is configured
to the slave node number and the remote I2C client address on the remote
side, and then the payload is sent to the BUS client of the master node,
which transparently proxies the traffic through.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/i2c/busses/Kconfig | 10 ++
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ad242x.c | 178 ++++++++++++++++++++++++++++++++
3 files changed, 189 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-ad242x.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 6a0aa76859f3..b9cf049bedb0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -365,6 +365,16 @@ config I2C_POWERMAC
comment "I2C system bus drivers (mostly embedded / system-on-chip)"
+config I2C_AD242X
+ tristate "Analog Devices AD242x"
+ depends on MFD_AD242X
+ help
+ If you say yes to this option, support will be included for the
+ I2C bus controller function of AD242x slave nodes.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-ad242x.
+
config I2C_ALTERA
tristate "Altera Soft IP I2C"
depends on (ARCH_SOCFPGA || NIOS2) && OF
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 3ab8aebc39c9..57c31ea8a477 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o
# Embedded system I2C/SMBus host controller drivers
+obj-$(CONFIG_I2C_AD242X) += i2c-ad242x.o
obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
obj-$(CONFIG_I2C_AMD_MP2) += i2c-amd-mp2-pci.o i2c-amd-mp2-plat.o
obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
diff --git a/drivers/i2c/busses/i2c-ad242x.c b/drivers/i2c/busses/i2c-ad242x.c
new file mode 100644
index 000000000000..b94056653898
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ad242x.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct ad242x_i2c {
+ struct device *dev;
+ struct ad242x_node *node;
+ struct i2c_adapter adap;
+ u32 node_index;
+};
+
+static int ad242x_set_addr(struct ad242x_node *mnode,
+ struct ad242x_i2c_bus *bus,
+ uint8_t node_id, uint8_t addr)
+{
+ int ret;
+ uint8_t buf[2] = { AD242X_CHIP, addr };
+
+ ret = regmap_update_bits(mnode->regmap, AD242X_NODEADR,
+ AD242X_NODEADR_PERI | AD242X_NODEADR_MASK,
+ node_id);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * We can't use the slave's regmap here as it holds the same
+ * lock we also need to guard this context.
+ */
+ ret = i2c_transfer_buffer_flags(bus->client,
+ buf, sizeof(buf), 0);
+ if (ret < 0)
+ return ret;
+
+ return regmap_update_bits(mnode->regmap, AD242X_NODEADR,
+ AD242X_NODEADR_PERI, AD242X_NODEADR_PERI);
+}
+
+static int ad242x_i2c_xfer(struct i2c_adapter *adap,
+ struct i2c_msg msgs[], int num)
+{
+ struct ad242x_i2c *i2c = adap->algo_data;
+ struct ad242x_i2c_bus *bus = ad242x_master_get_bus(i2c->node->master);
+ struct ad242x_node *mnode = ad242x_master_get_node(i2c->node->master);
+ int ret, i, current_addr = -1;
+
+ mutex_lock(&bus->mutex);
+
+ for (i = 0; i < num; i++) {
+ struct i2c_msg *msg = msgs + i;
+
+ if (msg->addr != current_addr) {
+ ret = ad242x_set_addr(mnode, bus,
+ i2c->node->id, msg->addr);
+ if (ret < 0) {
+ dev_err(i2c->node->dev,
+ "Cannot set address: %d\n", ret);
+ break;
+ }
+
+ current_addr = msg->addr;
+ }
+
+ ret = i2c_transfer_buffer_flags(bus->client,
+ msg->buf, msg->len, msg->flags);
+ if (ret < 0)
+ break;
+ }
+
+ mutex_unlock(&bus->mutex);
+
+ return ret < 0 ? ret : num;
+}
+
+static u32 ad242x_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm ad242x_i2c_algorithm = {
+ .master_xfer = ad242x_i2c_xfer,
+ .functionality = ad242x_i2c_functionality,
+};
+
+static int ad242x_i2c_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ad242x_node *node;
+ struct ad242x_i2c *i2c;
+ u32 freq, val = 0;
+ int ret;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ node = dev_get_drvdata(dev->parent);
+ if ((node->caps & AD242X_CAPABILITY_I2C) == 0) {
+ dev_err(dev, "Node %d has no I2C capability", node->id);
+ return -ENOTSUPP;
+ }
+
+ if (ad242x_node_is_master(node))
+ return -EINVAL;
+
+ freq = ad242x_master_get_clk_rate(node->master);
+ if (freq == 44100)
+ val |= AD242X_I2CCFG_FRAMERATE;
+
+ if (!of_property_read_u32(dev->of_node, "clock-frequency", &freq)) {
+ if (freq == 400000)
+ val |= AD242X_I2CCFG_DATARATE;
+ else if (freq != 100000)
+ dev_warn(dev, "Unsupported frequency %d\n", freq);
+ }
+
+ ret = regmap_write(node->regmap, AD242X_I2CCFG, val);
+ if (ret < 0)
+ return ret;
+
+ i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
+ if (!i2c)
+ return -ENOMEM;
+
+ i2c->node = node;
+ i2c->adap.algo = &ad242x_i2c_algorithm;
+ i2c->adap.algo_data = i2c;
+ i2c->adap.dev.parent = dev;
+ i2c->adap.dev.of_node = dev->of_node;
+ i2c_set_adapdata(&i2c->adap, i2c);
+ strlcpy(i2c->adap.name, "ad242x remote I2C bus",
+ sizeof(i2c->adap.name));
+
+ ret = i2c_add_adapter(&i2c->adap);
+ if (ret < 0) {
+ dev_err(dev, "error registering adapter: %d\n", ret);
+ return ret;
+ }
+
+ dev_info(dev, "ad242x i2c driver, node ID %d\n", node->id);
+ platform_set_drvdata(pdev, i2c);
+
+ return 0;
+}
+
+static int ad242x_i2c_remove(struct platform_device *dev)
+{
+ struct ad242x_i2c *i2c = platform_get_drvdata(dev);
+
+ i2c_del_adapter(&i2c->adap);
+
+ return 0;
+}
+
+static const struct of_device_id ad242x_i2c_of_match[] = {
+ { .compatible = "adi,ad2428w-i2c" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad242x_i2c_of_match);
+
+static struct platform_driver ad242x_i2c_driver = {
+ .driver = {
+ .name = "ad242x-i2c",
+ .of_match_table = ad242x_i2c_of_match,
+ },
+ .probe = ad242x_i2c_probe,
+ .remove = ad242x_i2c_remove,
+};
+
+module_platform_driver(ad242x_i2c_driver);
+MODULE_LICENSE("GPL");
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller
2019-12-09 18:35 ` [PATCH 07/10] i2c: Add driver for AD242x bus controller Daniel Mack
@ 2019-12-12 16:11 ` Luca Ceresoli
2019-12-12 16:33 ` Wolfram Sang
0 siblings, 1 reply; 34+ messages in thread
From: Luca Ceresoli @ 2019-12-12 16:11 UTC (permalink / raw)
To: Daniel Mack, linux-kernel, linux-gpio, linux-i2c, alsa-devel,
devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars, pascal.huerst
Hi Daniel,
On 09/12/19 19:35, Daniel Mack wrote:
> This device must be instantiated as a sub-device of the AD242x MFD
> device.
>
> In order to access remote I2C peripherals, the master node is configured
> to the slave node number and the remote I2C client address on the remote
> side, and then the payload is sent to the BUS client of the master node,
> which transparently proxies the traffic through.
This remote I2C feature in these chips is interesting. It looks somewhat
similar to remote I2C in the video serdes chip by TI and Maxim, but it's
different from both of them. So now we have 3 vendors implementing the
same feature in 3 different ways.
Cool.
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
> drivers/i2c/busses/Kconfig | 10 ++
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-ad242x.c | 178 ++++++++++++++++++++++++++++++++
> 3 files changed, 189 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-ad242x.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 6a0aa76859f3..b9cf049bedb0 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -365,6 +365,16 @@ config I2C_POWERMAC
>
> comment "I2C system bus drivers (mostly embedded / system-on-chip)"
>
> +config I2C_AD242X
> + tristate "Analog Devices AD242x"
> + depends on MFD_AD242X
> + help
> + If you say yes to this option, support will be included for the
> + I2C bus controller function of AD242x slave nodes.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-ad242x.
> +
> config I2C_ALTERA
> tristate "Altera Soft IP I2C"
> depends on (ARCH_SOCFPGA || NIOS2) && OF
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 3ab8aebc39c9..57c31ea8a477 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
> obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o
>
> # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_AD242X) += i2c-ad242x.o
> obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
> obj-$(CONFIG_I2C_AMD_MP2) += i2c-amd-mp2-pci.o i2c-amd-mp2-plat.o
> obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
> diff --git a/drivers/i2c/busses/i2c-ad242x.c b/drivers/i2c/busses/i2c-ad242x.c
> new file mode 100644
> index 000000000000..b94056653898
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-ad242x.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/ad242x.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +struct ad242x_i2c {
> + struct device *dev;
> + struct ad242x_node *node;
> + struct i2c_adapter adap;
> + u32 node_index;
> +};
> +
> +static int ad242x_set_addr(struct ad242x_node *mnode,
> + struct ad242x_i2c_bus *bus,
> + uint8_t node_id, uint8_t addr)
> +{
> + int ret;
> + uint8_t buf[2] = { AD242X_CHIP, addr };
> +
> + ret = regmap_update_bits(mnode->regmap, AD242X_NODEADR,
> + AD242X_NODEADR_PERI | AD242X_NODEADR_MASK,
> + node_id);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * We can't use the slave's regmap here as it holds the same
> + * lock we also need to guard this context.
> + */
> + ret = i2c_transfer_buffer_flags(bus->client,
> + buf, sizeof(buf), 0);
> + if (ret < 0)
> + return ret;
> +
> + return regmap_update_bits(mnode->regmap, AD242X_NODEADR,
> + AD242X_NODEADR_PERI, AD242X_NODEADR_PERI);
> +}
> +
> +static int ad242x_i2c_xfer(struct i2c_adapter *adap,
> + struct i2c_msg msgs[], int num)
> +{
> + struct ad242x_i2c *i2c = adap->algo_data;
> + struct ad242x_i2c_bus *bus = ad242x_master_get_bus(i2c->node->master);
> + struct ad242x_node *mnode = ad242x_master_get_node(i2c->node->master);
> + int ret, i, current_addr = -1;
> +
> + mutex_lock(&bus->mutex);
> +
> + for (i = 0; i < num; i++) {
> + struct i2c_msg *msg = msgs + i;
> +
> + if (msg->addr != current_addr) {
> + ret = ad242x_set_addr(mnode, bus,
> + i2c->node->id, msg->addr);
> + if (ret < 0) {
> + dev_err(i2c->node->dev,
> + "Cannot set address: %d\n", ret);
> + break;
> + }
> +
> + current_addr = msg->addr;
> + }
> +
> + ret = i2c_transfer_buffer_flags(bus->client,
> + msg->buf, msg->len, msg->flags);
> + if (ret < 0)
> + break;
> + }
> +
> + mutex_unlock(&bus->mutex);
> +
> + return ret < 0 ? ret : num;
> +}
Your implementation here looks quite clean and simple, and simple is
good, but I think there's a problem in this function. A "normal"
master_xfer function issues a repeated start between one msg and the
next one, at least in the typical case where all msgs have the same
slave address. Your implementation breaks repeated start. At first sight
we might need more complex code here to coalesce all consecutive msgs
with the same address into a single i2c_transfer() call.
--
Luca
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller
2019-12-12 16:11 ` Luca Ceresoli
@ 2019-12-12 16:33 ` Wolfram Sang
2019-12-15 20:27 ` Daniel Mack
0 siblings, 1 reply; 34+ messages in thread
From: Wolfram Sang @ 2019-12-12 16:33 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Daniel Mack, linux-kernel, linux-gpio, linux-i2c, alsa-devel,
devicetree, linux-clk, mturquette, sboyd, robh+dt, broonie,
lee.jones, lars, pascal.huerst
[-- Attachment #1: Type: text/plain, Size: 780 bytes --]
Hi Luca,
thanks for the review!
> good, but I think there's a problem in this function. A "normal"
> master_xfer function issues a repeated start between one msg and the
> next one, at least in the typical case where all msgs have the same
> slave address. Your implementation breaks repeated start. At first sight
> we might need more complex code here to coalesce all consecutive msgs
> with the same address into a single i2c_transfer() call.
Note that it is by far the standard case that all messages in a transfer
have the same client address (99,999%?). But technically, this is not a
requirement and the repeated start on the bus is totally independent of
the addresses used. It is just a master wanting to send without being
interrupted by another master.
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller
2019-12-12 16:33 ` Wolfram Sang
@ 2019-12-15 20:27 ` Daniel Mack
2019-12-17 8:35 ` Luca Ceresoli
0 siblings, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-15 20:27 UTC (permalink / raw)
To: Wolfram Sang, Luca Ceresoli
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst
Hi,
Thanks for the review!
On 12/12/2019 5:33 pm, Wolfram Sang wrote:
> Hi Luca,
>
> thanks for the review!
>
>> good, but I think there's a problem in this function. A "normal"
>> master_xfer function issues a repeated start between one msg and the
>> next one, at least in the typical case where all msgs have the same
>> slave address. Your implementation breaks repeated start. At first sight
>> we might need more complex code here to coalesce all consecutive msgs
>> with the same address into a single i2c_transfer() call.
>
> Note that it is by far the standard case that all messages in a transfer
> have the same client address (99,999%?). But technically, this is not a
> requirement and the repeated start on the bus is totally independent of
> the addresses used. It is just a master wanting to send without being
> interrupted by another master.
I'm not quite sure I understand.
Let's assume the following setup. An i2c client (some driver code) is
sending a list of messages to the a2b xfer function, which in turn is
logically connected to a 'real' i2c bus master that'll put the data on
the wire.
The a2b code has to tell the 'master node' the final destination of the
payload by programming registers on its primary i2c address, and then
forwards the messages to its secondary i2c address. The layout of the
messages don't change, and neither do the flags; i2c messages are being
sent as i2c messages, except their addresses are changed, a bit like NAT
in networking. That procedure is described on page 3-4 of the TRM,
"Remote Peripheral I2C Accesses".
The 'real' i2c master that handles the hardware bus is responsible for
adding start conditions, and as the messages as such are untouched, I
believe it should do the right thing. The code in my xfer functions
merely suppresses reprogramming remote addresses by remembering the last
one that was used, but that is independent of the start conditions on
the wire.
Maybe I'm missing anything. Could you provide an example that explains
in which case this approach would leads to issues?
Thanks,
Daniel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller
2019-12-15 20:27 ` Daniel Mack
@ 2019-12-17 8:35 ` Luca Ceresoli
2019-12-17 18:17 ` Daniel Mack
0 siblings, 1 reply; 34+ messages in thread
From: Luca Ceresoli @ 2019-12-17 8:35 UTC (permalink / raw)
To: Daniel Mack, Wolfram Sang
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst
Hi Daniel,
On 15/12/19 21:27, Daniel Mack wrote:
> Hi,
>
> Thanks for the review!
>
> On 12/12/2019 5:33 pm, Wolfram Sang wrote:
>> Hi Luca,
>>
>> thanks for the review!
>>
>>> good, but I think there's a problem in this function. A "normal"
>>> master_xfer function issues a repeated start between one msg and the
>>> next one, at least in the typical case where all msgs have the same
>>> slave address. Your implementation breaks repeated start. At first sight
>>> we might need more complex code here to coalesce all consecutive msgs
>>> with the same address into a single i2c_transfer() call.
>>
>> Note that it is by far the standard case that all messages in a transfer
>> have the same client address (99,999%?). But technically, this is not a
>> requirement and the repeated start on the bus is totally independent of
>> the addresses used. It is just a master wanting to send without being
>> interrupted by another master.
>
> I'm not quite sure I understand.
>
> Let's assume the following setup. An i2c client (some driver code) is
> sending a list of messages to the a2b xfer function, which in turn is
> logically connected to a 'real' i2c bus master that'll put the data on
> the wire.
>
> The a2b code has to tell the 'master node' the final destination of the
> payload by programming registers on its primary i2c address, and then
> forwards the messages to its secondary i2c address. The layout of the
> messages don't change, and neither do the flags; i2c messages are being
> sent as i2c messages, except their addresses are changed, a bit like NAT
> in networking. That procedure is described on page 3-4 of the TRM,
> "Remote Peripheral I2C Accesses".
>
> The 'real' i2c master that handles the hardware bus is responsible for
> adding start conditions, and as the messages as such are untouched, I
> believe it should do the right thing. The code in my xfer functions
> merely suppresses reprogramming remote addresses by remembering the last
> one that was used, but that is independent of the start conditions on
> the wire.
My concern is not about the start condition, it's about the *repeated*
start condition.
The first question is whether the A2B chips can do it. What if the host
processor sets a slave chip address and then issues two messages
separated by a repeated start condition? Will the slave transceiver emit
a repeated start condition too?
If the answer is "yes", then the issue moves to the driver code. A
master xfer function receives a set of messages that are normally
emitted with a repeated start between each other. But ad242x_i2c_xfer()
splits the msgs and calls i2c_transfer_buffer_flags() with one msg at a
time. i2c_transfer_buffer_flags() then will emit a stop condition.
This is not necessarily a problem, unless multi-master is used, but if
there are limitations or deviations from the standard they should at
least be well known and documented.
--
Luca
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 07/10] i2c: Add driver for AD242x bus controller
2019-12-17 8:35 ` Luca Ceresoli
@ 2019-12-17 18:17 ` Daniel Mack
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-17 18:17 UTC (permalink / raw)
To: Luca Ceresoli, Wolfram Sang
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst
Hi Luca,
On 12/17/19 9:35 AM, Luca Ceresoli wrote:
> On 15/12/19 21:27, Daniel Mack wrote:
>> The a2b code has to tell the 'master node' the final destination of the
>> payload by programming registers on its primary i2c address, and then
>> forwards the messages to its secondary i2c address. The layout of the
>> messages don't change, and neither do the flags; i2c messages are being
>> sent as i2c messages, except their addresses are changed, a bit like NAT
>> in networking. That procedure is described on page 3-4 of the TRM,
>> "Remote Peripheral I2C Accesses".
>>
>> The 'real' i2c master that handles the hardware bus is responsible for
>> adding start conditions, and as the messages as such are untouched, I
>> believe it should do the right thing. The code in my xfer functions
>> merely suppresses reprogramming remote addresses by remembering the last
>> one that was used, but that is independent of the start conditions on
>> the wire.
>
> My concern is not about the start condition, it's about the *repeated*
> start condition.
>
> The first question is whether the A2B chips can do it. What if the host
> processor sets a slave chip address and then issues two messages
> separated by a repeated start condition? Will the slave transceiver emit
> a repeated start condition too?
Ah, alright. Thanks for taking the time to explain. I'll have to do some
measurements with a hardware analyzer. Will revisit this then, and
either provide an implementation that handles such cases correctly, or a
comment to explain that the hardware can't do it.
Best regards,
Daniel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 08/10] gpio: Add driver for AD242x GPIO controllers
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (7 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 07/10] i2c: Add driver for AD242x bus controller Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-09 18:35 ` [PATCH 09/10] clk: Add support for AD242x clock output providers Daniel Mack
` (2 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This driver makes the 8 GPIOs on AD242x nodes available to consumers.
Apart from that, it also allows putting the GPIO lines in a 'gpio over
distance' mode. This mirrors the state of several GPIOs in the topology
without further interaction by any driver. For instance, when a GPIO pin
on the master node is put in input mode, and another one on a slave node
is in output mode, they can be linked together through virtual ports.
Then, the pin on the slave node will reflect the logical level on
whatever is applied to the respective pin on the master node.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/gpio/Kconfig | 6 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-ad242x.c | 229 +++++++++++++++++++++++++++++++++++++
3 files changed, 236 insertions(+)
create mode 100644 drivers/gpio/gpio-ad242x.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8adffd42f8cb..c8af1159a585 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -845,6 +845,12 @@ endmenu
menu "I2C GPIO expanders"
depends on I2C
+config GPIO_AD242X
+ tristate "AD242x A2B GPIO controller"
+ depends on MFD_AD242X
+ help
+ This option enables support for GPIOs on AD242x A2B nodes.
+
config GPIO_ADP5588
tristate "ADP5588 I2C GPIO expander"
help
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 34eb8b2b12dd..2490ce6e6905 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_GPIO_104_IDI_48) += gpio-104-idi-48.o
obj-$(CONFIG_GPIO_104_IDIO_16) += gpio-104-idio-16.o
obj-$(CONFIG_GPIO_74X164) += gpio-74x164.o
obj-$(CONFIG_GPIO_74XX_MMIO) += gpio-74xx-mmio.o
+obj-$(CONFIG_GPIO_AD242X) += gpio-ad242x.o
obj-$(CONFIG_GPIO_ADNP) += gpio-adnp.o
obj-$(CONFIG_GPIO_ADP5520) += gpio-adp5520.o
obj-$(CONFIG_GPIO_ADP5588) += gpio-adp5588.o
diff --git a/drivers/gpio/gpio-ad242x.c b/drivers/gpio/gpio-ad242x.c
new file mode 100644
index 000000000000..8970e434b56a
--- /dev/null
+++ b/drivers/gpio/gpio-ad242x.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+struct ad242x_gpio {
+ struct gpio_chip chip;
+ struct ad242x_node *node;
+ u32 gpio_od_mask;
+};
+
+static int ad242x_gpio_request(struct gpio_chip *chip, unsigned int gpio)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+
+ if (gpio == 0 && ad242x_node_is_master(ad242x_gpio->node))
+ return -EBUSY;
+
+ if (ad242x_gpio->gpio_od_mask & BIT(gpio))
+ return -EBUSY;
+
+ return 0;
+}
+
+static int ad242x_gpio_get_value(struct gpio_chip *chip, unsigned int gpio)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(regmap, AD242X_GPIODAT_IN, &val);
+ if (ret < 0)
+ return ret;
+
+ return !!(val & BIT(gpio));
+}
+
+static void ad242x_gpio_set_value(struct gpio_chip *chip,
+ unsigned int gpio, int value)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ uint8_t bit = BIT(gpio);
+ int ret;
+
+ if (value)
+ ret = regmap_write(regmap, AD242X_GPIODAT_SET, bit);
+ else
+ ret = regmap_write(regmap, AD242X_GPIODAT_CLR, bit);
+
+ if (ret < 0)
+ dev_err(ad242x_gpio->node->dev,
+ "Unable to set GPIO #%d: %d\n", gpio, ret);
+}
+
+static int ad242x_gpio_direction_input(struct gpio_chip *chip,
+ unsigned int gpio)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ uint8_t bit = BIT(gpio);
+ int ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOOEN, bit, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOIEN, bit, bit);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_INTMSK1, bit, bit);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int ad242x_gpio_direction_output(struct gpio_chip *chip,
+ unsigned int gpio, int value)
+{
+ struct ad242x_gpio *ad242x_gpio = gpiochip_get_data(chip);
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ uint8_t bit = BIT(gpio);
+ int ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOIEN, bit, 0);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_GPIOOEN, bit, bit);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_update_bits(regmap, AD242X_INTMSK1, bit, 0);
+ if (ret < 0)
+ return ret;
+
+ ad242x_gpio_set_value(chip, gpio, value);
+
+ return 0;
+}
+
+static int ad242x_gpio_over_distance_init(struct device *dev,
+ struct ad242x_gpio *ad242x_gpio)
+{
+ struct regmap *regmap = ad242x_gpio->node->regmap;
+ struct device_node *np, *child_np;
+ int ret = 0;
+
+ np = of_get_child_by_name(dev->of_node, "gpio-over-distance");
+ if (!np)
+ return 0;
+
+ for_each_available_child_of_node(np, child_np) {
+ u32 reg, port_mask, bit;
+ bool output, inv;
+
+ ret = of_property_read_u32(child_np, "reg", ®);
+ if (ret < 0)
+ continue;
+
+ ret = of_property_read_u32(child_np, "adi,virtual-port-mask",
+ &port_mask);
+ if (ret < 0)
+ continue;
+
+ if (reg > 7) {
+ ret = -EINVAL;
+ break;
+ }
+
+ bit = BIT(reg);
+
+ ret = regmap_update_bits(regmap, AD242X_GPIODEN, bit, bit);
+ if (ret < 0)
+ break;
+
+ ret = regmap_write(regmap, AD242X_GPIOD_MSK(reg), port_mask);
+ if (ret < 0)
+ break;
+
+ output = of_property_read_bool(child_np, "adi,gpio-output");
+ ret = regmap_update_bits(regmap, AD242X_GPIOOEN,
+ bit, output ? bit : 0);
+ if (ret < 0)
+ break;
+
+ inv = of_property_read_bool(child_np, "adi,gpio-inverted");
+ ret = regmap_update_bits(regmap, AD242X_GPIODINV,
+ bit, inv ? bit : 0);
+ if (ret < 0)
+ break;
+
+ ad242x_gpio->gpio_od_mask |= bit;
+ dev_info(dev,
+ "pin %d set up as gpio-over-distance, port mask 0x%02x\n",
+ reg, port_mask);
+ }
+
+ of_node_put(np);
+
+ return ret;
+}
+
+static int ad242x_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ad242x_gpio *ad242x_gpio;
+ int ret;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ ad242x_gpio = devm_kzalloc(dev, sizeof(*ad242x_gpio), GFP_KERNEL);
+ if (!ad242x_gpio)
+ return -ENOMEM;
+
+ ad242x_gpio->node = dev_get_drvdata(dev->parent);
+
+ ad242x_gpio->chip.request = ad242x_gpio_request;
+ ad242x_gpio->chip.direction_input = ad242x_gpio_direction_input;
+ ad242x_gpio->chip.direction_output = ad242x_gpio_direction_output;
+ ad242x_gpio->chip.get = ad242x_gpio_get_value;
+ ad242x_gpio->chip.set = ad242x_gpio_set_value;
+ ad242x_gpio->chip.can_sleep = 1;
+ ad242x_gpio->chip.base = -1;
+ ad242x_gpio->chip.ngpio = 8;
+ ad242x_gpio->chip.label = "ad242x-gpio";
+ ad242x_gpio->chip.owner = THIS_MODULE;
+ ad242x_gpio->chip.parent = dev;
+
+ dev_info(dev, "A2B node ID %d\n", ad242x_gpio->node->id);
+
+ ret = ad242x_gpio_over_distance_init(dev, ad242x_gpio);
+ if (ret < 0) {
+ dev_err(dev, "GPIO over distance init failed: %d\n", ret);
+ return ret;
+ }
+
+ return devm_gpiochip_add_data(dev, &ad242x_gpio->chip, ad242x_gpio);
+}
+
+static const struct of_device_id ad242x_gpio_of_match[] = {
+ { .compatible = "adi,ad2428w-gpio", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad242x_gpio_of_match);
+
+static struct platform_driver ad242x_gpio_driver = {
+ .driver = {
+ .name = "ad242x-gpio",
+ .of_match_table = ad242x_gpio_of_match,
+ },
+ .probe = ad242x_gpio_probe,
+};
+module_platform_driver(ad242x_gpio_driver);
+
+MODULE_DESCRIPTION("AD242x GPIO driver");
+MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
+MODULE_LICENSE("GPL v2");
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 09/10] clk: Add support for AD242x clock output providers
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (8 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 08/10] gpio: Add driver for AD242x GPIO controllers Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-24 7:46 ` Stephen Boyd
2019-12-09 18:35 ` [PATCH 10/10] ASoC: Add codec component for AD242x nodes Daniel Mack
2019-12-17 19:29 ` [alsa-devel] [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Pierre-Louis Bossart
11 siblings, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
AD242x have two pins that can be used as clock outputs. This driver makes
that functionality available through the common clock framework.
Apart from gating the clocks and setting their rates, the hardware also
allows for a phase shift of 180 degrees.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/clk/Kconfig | 6 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-ad242x.c | 231 +++++++++++++++++++++++++++++++++++++++
3 files changed, 238 insertions(+)
create mode 100644 drivers/clk/clk-ad242x.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 45653a0e6ecd..28b700d068fe 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -39,6 +39,12 @@ config CLK_HSDK
This driver supports the HSDK core, system, ddr, tunnel and hdmi PLLs
control.
+config COMMON_CLK_AD242X
+ tristate "Clock driver for AD242x A2B nodes"
+ depends on MFD_AD242X
+ ---help---
+ This driver supports clock outputs on AD242x A2B nodes.
+
config COMMON_CLK_MAX77686
tristate "Clock driver for Maxim 77620/77686/77802 MFD"
depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0696a0c1ab58..3f8cbddb48c7 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -18,6 +18,7 @@ endif
# hardware specific clock types
# please keep this section sorted lexicographically by file path name
+obj-$(CONFIG_COMMON_CLK_AD242X) += clk-ad242x.o
obj-$(CONFIG_MACH_ASM9260) += clk-asm9260.o
obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN) += clk-axi-clkgen.o
obj-$(CONFIG_ARCH_AXXIA) += clk-axm5516.o
diff --git a/drivers/clk/clk-ad242x.c b/drivers/clk/clk-ad242x.c
new file mode 100644
index 000000000000..201789d8f174
--- /dev/null
+++ b/drivers/clk/clk-ad242x.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/adi,ad242x.h>
+
+#define AD242X_NUM_CLKS 2
+
+struct ad242x_clk_hw {
+ struct clk_hw hw;
+ struct clk_init_data init;
+ struct ad242x_node *node;
+ u8 reg;
+};
+
+struct ad242x_clk_driver_data {
+ struct ad242x_clk_hw hw[AD242X_NUM_CLKS];
+};
+
+static inline struct ad242x_clk_hw *to_ad242x_clk(struct clk_hw *hw)
+{
+ return container_of(hw, struct ad242x_clk_hw, hw);
+}
+
+static int ad242x_clk_prepare(struct clk_hw *hw)
+{
+ struct ad242x_clk_hw *clk_hw = to_ad242x_clk(hw);
+
+ return regmap_update_bits(clk_hw->node->regmap, clk_hw->reg,
+ AD242X_CLKCFG_EN, AD242X_CLKCFG_EN);
+}
+
+static void ad242x_clk_unprepare(struct clk_hw *hw)
+{
+ struct ad242x_clk_hw *clk_hw = to_ad242x_clk(hw);
+
+ regmap_update_bits(clk_hw->node->regmap, clk_hw->reg,
+ AD242X_CLKCFG_EN, 0);
+}
+
+static void ad242x_do_div(unsigned long rate, unsigned long parent_rate,
+ unsigned long *prediv, unsigned long *div)
+{
+ if (rate < parent_rate / 32UL)
+ *prediv = 32UL;
+ else
+ *prediv = 2UL;
+
+ parent_rate /= *prediv;
+ *div = parent_rate / rate;
+}
+
+static int ad242x_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct ad242x_clk_hw *clk_hw = to_ad242x_clk(hw);
+ unsigned long pll_rate = parent_rate * 2048UL;
+ unsigned long prediv, div;
+ unsigned int val = 0;
+
+ if (rate > pll_rate / 4 || rate < pll_rate / 1024UL)
+ return -EINVAL;
+
+ ad242x_do_div(rate, pll_rate, &prediv, &div);
+
+ if (prediv == 32UL)
+ val |= AD242X_CLKCFG_PDIV32;
+
+ val |= AD242X_CLKCFG_DIV((div / 2UL) - 1UL);
+
+ return regmap_update_bits(clk_hw->node->regmap, clk_hw->reg,
+ AD242X_CLKCFG_DIVMSK | AD242X_CLKCFG_PDIV32,
+ val);
+}
+
+static unsigned long ad242x_clk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct ad242x_clk_hw *clk_hw = to_ad242x_clk(hw);
+ unsigned long pll_rate = parent_rate * 2048UL;
+ unsigned long prediv, div;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(clk_hw->node->regmap, clk_hw->reg, &val);
+ if (ret < 0)
+ return ret;
+
+ prediv = (val & AD242X_CLKCFG_PDIV32) ? 32UL : 2UL;
+ div = 2UL * ((val & AD242X_CLKCFG_DIVMSK) + 1UL);
+
+ return pll_rate / (prediv * div);
+}
+
+static long ad242x_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ unsigned long pll_rate = *parent_rate * 2048UL;
+ unsigned long prediv, div;
+
+ if (rate > pll_rate / 4 || rate < pll_rate / 1024UL)
+ return -EINVAL;
+
+ ad242x_do_div(rate, pll_rate, &prediv, &div);
+
+ return pll_rate / (prediv * div);
+}
+
+static int ad242x_clk_get_phase(struct clk_hw *hw)
+{
+ struct ad242x_clk_hw *clk_hw = to_ad242x_clk(hw);
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(clk_hw->node->regmap, clk_hw->reg, &val);
+ if (ret < 0)
+ return ret;
+
+ return (val & AD242X_CLKCFG_INV) ? 180 : 0;
+}
+
+static int ad242x_clk_set_phase(struct clk_hw *hw, int phase)
+{
+ struct ad242x_clk_hw *clk_hw = to_ad242x_clk(hw);
+ unsigned int val;
+
+ switch (phase) {
+ case 0:
+ val = 0;
+ break;
+ case 180:
+ val = AD242X_CLKCFG_INV;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(clk_hw->node->regmap, clk_hw->reg,
+ AD242X_CLKCFG_INV, val);
+}
+
+static const struct clk_ops ad242x_clk_ops = {
+ .prepare = ad242x_clk_prepare,
+ .unprepare = ad242x_clk_unprepare,
+ .get_phase = ad242x_clk_get_phase,
+ .set_phase = ad242x_clk_set_phase,
+ .recalc_rate = ad242x_clk_recalc_rate,
+ .round_rate = ad242x_clk_round_rate,
+ .set_rate = ad242x_clk_set_rate,
+};
+
+static struct clk_hw *
+ad242x_of_clk_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct ad242x_clk_driver_data *drvdata = data;
+ unsigned int idx = clkspec->args[0];
+
+ return &drvdata->hw[idx].hw;
+}
+
+static int ad242x_clk_probe(struct platform_device *pdev)
+{
+ const char *clk_names[AD242X_NUM_CLKS] = { "clkout1", "clkout2" };
+ u8 regs[AD242X_NUM_CLKS] = { AD242X_CLK1CFG, AD242X_CLK2CFG };
+ struct ad242x_clk_driver_data *drvdata;
+ struct device *dev = &pdev->dev;
+ const char *sync_clk_name;
+ struct ad242x_node *node;
+ int i, ret;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ node = dev_get_drvdata(dev->parent);
+ sync_clk_name = ad242x_master_get_clk_name(node->master);
+
+ for (i = 0; i < AD242X_NUM_CLKS; i++) {
+ const char *name;
+
+ if (of_property_read_string_index(dev->of_node,
+ "clock-output-names",
+ i, &name) == 0)
+ drvdata->hw[i].init.name = name;
+ else
+ drvdata->hw[i].init.name = clk_names[i];
+
+ drvdata->hw[i].reg = regs[i];
+ drvdata->hw[i].init.ops = &ad242x_clk_ops;
+ drvdata->hw[i].init.num_parents = 1;
+ drvdata->hw[i].init.parent_names = &sync_clk_name;
+ drvdata->hw[i].hw.init = &drvdata->hw[i].init;
+ drvdata->hw[i].node = node;
+
+ ret = devm_clk_hw_register(dev, &drvdata->hw[i].hw);
+ if (ret < 0)
+ return ret;
+ }
+
+ return devm_of_clk_add_hw_provider(dev, ad242x_of_clk_get, drvdata);
+}
+
+static const struct of_device_id ad242x_dt_ids[] = {
+ { .compatible = "adi,ad2428w-clk", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad242x_dt_ids);
+
+static struct platform_driver ad242x_clk_driver = {
+ .probe = ad242x_clk_probe,
+ .driver = {
+ .name = "ad242x-clk",
+ .of_match_table = ad242x_dt_ids,
+ },
+};
+module_platform_driver(ad242x_clk_driver);
+
+MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
+MODULE_DESCRIPTION("AD242x clock driver");
+MODULE_LICENSE("GPL v2");
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 09/10] clk: Add support for AD242x clock output providers
2019-12-09 18:35 ` [PATCH 09/10] clk: Add support for AD242x clock output providers Daniel Mack
@ 2019-12-24 7:46 ` Stephen Boyd
0 siblings, 0 replies; 34+ messages in thread
From: Stephen Boyd @ 2019-12-24 7:46 UTC (permalink / raw)
To: Daniel Mack, alsa-devel, devicetree, linux-clk, linux-gpio,
linux-i2c, linux-kernel
Cc: mturquette, robh+dt, broonie, lee.jones, lars, pascal.huerst,
Daniel Mack
Quoting Daniel Mack (2019-12-09 10:35:10)
> diff --git a/drivers/clk/clk-ad242x.c b/drivers/clk/clk-ad242x.c
> new file mode 100644
> index 000000000000..201789d8f174
> --- /dev/null
> +++ b/drivers/clk/clk-ad242x.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/clk.h>
Is this include used?
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
Is this include used?
> +#include <linux/mfd/ad242x.h>
Any way we can avoid this build dependency? Maybe just put defines in
this driver that deals with the clk bits of the device?
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/adi,ad242x.h>
> +
> +#define AD242X_NUM_CLKS 2
> +
> +struct ad242x_clk_hw {
> + struct clk_hw hw;
> + struct clk_init_data init;
Do we need to keep around this init data after probe? I'd rather leave
this out.
> + struct ad242x_node *node;
What's the point of this structure? Can we use dev->parent->regmap and
just store the struct regmap pointer here instead of using this custom
struct?
> + u8 reg;
> +};
> +
> +struct ad242x_clk_driver_data {
> + struct ad242x_clk_hw hw[AD242X_NUM_CLKS];
If this is the only drvdata, then I'd prefer just the array and not
another struct so we can have clarity.
> +};
> +
> +static inline struct ad242x_clk_hw *to_ad242x_clk(struct clk_hw *hw)
> +{
> + return container_of(hw, struct ad242x_clk_hw, hw);
> +}
> +
[...]
> +
> +static long ad242x_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + unsigned long pll_rate = *parent_rate * 2048UL;
> + unsigned long prediv, div;
> +
> + if (rate > pll_rate / 4 || rate < pll_rate / 1024UL)
> + return -EINVAL;
This callback should round the rate to something valid. If the rate is
larger than pll_rate / 4 then it should clamp to be the highest rate
supported. Likewise for something slow.
> +
> + ad242x_do_div(rate, pll_rate, &prediv, &div);
> +
> + return pll_rate / (prediv * div);
> +}
> +
[...]
> +
> +static struct clk_hw *
> +ad242x_of_clk_get(struct of_phandle_args *clkspec, void *data)
> +{
> + struct ad242x_clk_driver_data *drvdata = data;
> + unsigned int idx = clkspec->args[0];
> +
> + return &drvdata->hw[idx].hw;
It looks quite a bit like of_clk_hw_onecell_get(). Can that be used? Or
at least check for out of bounds and return failure?
> +}
> +
> +static int ad242x_clk_probe(struct platform_device *pdev)
> +{
> + const char *clk_names[AD242X_NUM_CLKS] = { "clkout1", "clkout2" };
> + u8 regs[AD242X_NUM_CLKS] = { AD242X_CLK1CFG, AD242X_CLK2CFG };
> + struct ad242x_clk_driver_data *drvdata;
> + struct device *dev = &pdev->dev;
> + const char *sync_clk_name;
> + struct ad242x_node *node;
> + int i, ret;
> +
> + if (!dev->of_node)
> + return -ENODEV;
Please drop this. It's not useful.
> +
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + node = dev_get_drvdata(dev->parent);
Add a NULL check on node?
> + sync_clk_name = ad242x_master_get_clk_name(node->master);
> +
> + for (i = 0; i < AD242X_NUM_CLKS; i++) {
> + const char *name;
> +
> + if (of_property_read_string_index(dev->of_node,
> + "clock-output-names",
> + i, &name) == 0)
> + drvdata->hw[i].init.name = name;
> + else
> + drvdata->hw[i].init.name = clk_names[i];
Do you need unique names? Or can you generate psuedo unique names based
on the device name and clk number?
> +
> + drvdata->hw[i].reg = regs[i];
> + drvdata->hw[i].init.ops = &ad242x_clk_ops;
> + drvdata->hw[i].init.num_parents = 1;
> + drvdata->hw[i].init.parent_names = &sync_clk_name;
> + drvdata->hw[i].hw.init = &drvdata->hw[i].init;
> + drvdata->hw[i].node = node;
> +
> + ret = devm_clk_hw_register(dev, &drvdata->hw[i].hw);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return devm_of_clk_add_hw_provider(dev, ad242x_of_clk_get, drvdata);
> +}
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 10/10] ASoC: Add codec component for AD242x nodes
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (9 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 09/10] clk: Add support for AD242x clock output providers Daniel Mack
@ 2019-12-09 18:35 ` Daniel Mack
2019-12-16 14:23 ` Mark Brown
2019-12-17 19:28 ` [alsa-devel] " Pierre-Louis Bossart
2019-12-17 19:29 ` [alsa-devel] [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Pierre-Louis Bossart
11 siblings, 2 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-09 18:35 UTC (permalink / raw)
To: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree, linux-clk
Cc: mturquette, sboyd, robh+dt, broonie, lee.jones, lars,
pascal.huerst, Daniel Mack
This driver makes AD242x nodes available as DAIs in ASoC topologies.
The hardware allows multiple TDM channel modes and bitdepths, but
as these modes have influence in the timing calculations at discovery
time, the mode in that the will be used in needs to be configured
statically in the devicetree.
The configuration applied at runtime through hwparams() is then
required to match the pre-configured settings.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
sound/soc/codecs/Kconfig | 5 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/ad242x.c | 338 ++++++++++++++++++++++++++++++++++++++
3 files changed, 345 insertions(+)
create mode 100644 sound/soc/codecs/ad242x.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 4abf37b5083f..75365abc277f 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -22,6 +22,7 @@ config SND_SOC_ALL_CODECS
select SND_SOC_AD193X_SPI if SPI_MASTER
select SND_SOC_AD193X_I2C if I2C
select SND_SOC_AD1980 if SND_SOC_AC97_BUS
+ select SND_SOC_AD242X if MFD_AD242X
select SND_SOC_AD73311
select SND_SOC_ADAU1373 if I2C
select SND_SOC_ADAU1761_I2C if I2C
@@ -333,6 +334,10 @@ config SND_SOC_AD1980
select REGMAP_AC97
tristate
+config SND_SOC_AD242X
+ tristate "Analog Devices AD242x CODEC"
+ depends on MFD_AD242X
+
config SND_SOC_AD73311
tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index ddfd07071925..ec76448fc1da 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -7,6 +7,7 @@ snd-soc-ad193x-objs := ad193x.o
snd-soc-ad193x-spi-objs := ad193x-spi.o
snd-soc-ad193x-i2c-objs := ad193x-i2c.o
snd-soc-ad1980-objs := ad1980.o
+snd-soc-ad242x-objs := ad242x.o
snd-soc-ad73311-objs := ad73311.o
snd-soc-adau-utils-objs := adau-utils.o
snd-soc-adau1373-objs := adau1373.o
@@ -294,6 +295,7 @@ obj-$(CONFIG_SND_SOC_AD193X) += snd-soc-ad193x.o
obj-$(CONFIG_SND_SOC_AD193X_SPI) += snd-soc-ad193x-spi.o
obj-$(CONFIG_SND_SOC_AD193X_I2C) += snd-soc-ad193x-i2c.o
obj-$(CONFIG_SND_SOC_AD1980) += snd-soc-ad1980.o
+obj-$(CONFIG_SND_SOC_AD242X) += snd-soc-ad242x.o
obj-$(CONFIG_SND_SOC_AD73311) += snd-soc-ad73311.o
obj-$(CONFIG_SND_SOC_ADAU_UTILS) += snd-soc-adau-utils.o
obj-$(CONFIG_SND_SOC_ADAU1373) += snd-soc-adau1373.o
diff --git a/sound/soc/codecs/ad242x.c b/sound/soc/codecs/ad242x.c
new file mode 100644
index 000000000000..76189a7c3c92
--- /dev/null
+++ b/sound/soc/codecs/ad242x.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mfd/ad242x.h>
+#include <linux/of_device.h>
+#include <linux/slab.h>
+#include <sound/asoundef.h>
+#include <sound/core.h>
+#include <sound/initval.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+struct ad242x_private {
+ struct ad242x_node *node;
+ bool pdm[2];
+ bool pdm_highpass;
+};
+
+static const struct snd_soc_dapm_widget ad242x_dapm_widgets[] = {
+ SND_SOC_DAPM_AIF_IN("RX0", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_AIF_IN("RX1", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_AIF_OUT("TX0", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_AIF_OUT("TX1", NULL, 0, SND_SOC_NOPM, 0, 0),
+};
+
+static const struct snd_soc_dapm_route ad242x_dapm_routes[] = {
+ { "DAI0 Playback", NULL, "RX0" },
+ { "TX0", NULL, "DAI0 Capture" },
+ { "DAI1 Playback", NULL, "RX1" },
+ { "TX1", NULL, "DAI1 Capture" },
+};
+
+static int ad242x_set_dai_fmt(struct snd_soc_dai *codec_dai,
+ unsigned int format,
+ unsigned int index)
+{
+ struct snd_soc_component *component = codec_dai->component;
+ struct ad242x_private *priv = snd_soc_component_get_drvdata(component);
+ int ret, val = 0;
+
+ /* set DAI format */
+ switch (format & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S:
+ priv->pdm[index] = false;
+ break;
+ case SND_SOC_DAIFMT_PDM:
+ priv->pdm[index] = true;
+ break;
+ default:
+ dev_err(component->dev, "unsupported dai format\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Setting clock inversion is only supported globally for both DAIs,
+ * so we ignore the settings made for DAI1 here.
+ */
+ if (index == 0) {
+ switch (format & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_NB_NF:
+ val = 0;
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ val = AD242X_I2SCTL_RXBCLKINV;
+ break;
+ case SND_SOC_DAIFMT_NB_IF:
+ case SND_SOC_DAIFMT_IB_IF:
+ dev_err(component->dev, "unsupported inversion mask\n");
+ return -EINVAL;
+ }
+
+ ret = regmap_update_bits(priv->node->regmap, AD242X_I2SCTL,
+ AD242X_I2SCTL_RXBCLKINV, val);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (ad242x_node_is_master(priv->node) &&
+ ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBS_CFS)) {
+ dev_err(component->dev, "master node must be clock slave\n");
+ return -EINVAL;
+ }
+
+ if (!ad242x_node_is_master(priv->node) &&
+ ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)) {
+ dev_err(component->dev, "slave node must be clock master\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int ad242x_set_dai_fmt_dai0(struct snd_soc_dai *codec_dai,
+ unsigned int format)
+{
+ return ad242x_set_dai_fmt(codec_dai, format, 0);
+}
+
+static int ad242x_set_dai_fmt_dai1(struct snd_soc_dai *codec_dai,
+ unsigned int format)
+{
+ return ad242x_set_dai_fmt(codec_dai, format, 1);
+}
+
+static int ad242x_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai,
+ int index)
+{
+ struct snd_soc_component *component = dai->component;
+ struct ad242x_private *priv = snd_soc_component_get_drvdata(component);
+ unsigned int sff_rate = ad242x_master_get_clk_rate(priv->node->master);
+ unsigned int rate = params_rate(params);
+ unsigned int val, mask;
+ int ret;
+
+ switch (params_format(params)) {
+ case SNDRV_PCM_FORMAT_S16_LE:
+ if (priv->node->tdm_slot_size != 16)
+ return -EINVAL;
+ break;
+ case SNDRV_PCM_FORMAT_S32_LE:
+ if (priv->node->tdm_slot_size != 32)
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (priv->pdm[index]) {
+ if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
+ return -EINVAL;
+
+ if (index == 0) {
+ val = AD242X_PDMCTL_PDM0EN;
+ mask = AD242X_PDMCTL_PDM0EN | AD242X_PDMCTL_PDM0SLOTS;
+ } else {
+ val = AD242X_PDMCTL_PDM1EN;
+ mask = AD242X_PDMCTL_PDM1EN | AD242X_PDMCTL_PDM1SLOTS;
+ }
+
+ switch (params_channels(params)) {
+ case 1:
+ break;
+ case 2:
+ val = mask;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ mask |= AD242X_PDMCTL_HPFEN;
+ if (priv->pdm_highpass)
+ val |= AD242X_PDMCTL_HPFEN;
+
+ ret = regmap_update_bits(priv->node->regmap, AD242X_PDMCTL,
+ mask, val);
+ if (ret < 0)
+ return ret;
+ } else {
+ if (params_channels(params) != priv->node->tdm_mode)
+ return -EINVAL;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ if (index == 0)
+ mask = AD242X_I2SCTL_RX0EN;
+ else
+ mask = AD242X_I2SCTL_RX1EN;
+ } else {
+ if (index == 0)
+ mask = AD242X_I2SCTL_TX0EN;
+ else
+ mask = AD242X_I2SCTL_TX1EN;
+ }
+
+ ret = regmap_update_bits(priv->node->regmap, AD242X_I2SCTL,
+ mask, mask);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (!ad242x_node_is_master(priv->node)) {
+ val = 0;
+
+ if (rate == sff_rate / 2)
+ val = AD242X_I2SRATE_I2SRATE(1);
+ else if (rate == sff_rate / 4)
+ val = AD242X_I2SRATE_I2SRATE(2);
+ else if (rate == sff_rate * 2)
+ val = AD242X_I2SRATE_I2SRATE(5);
+ else if (rate == sff_rate * 4)
+ val = AD242X_I2SRATE_I2SRATE(6);
+ else if (rate != sff_rate)
+ return -EINVAL;
+
+ ret = regmap_write(priv->node->regmap, AD242X_I2SRATE, val);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ad242x_hw_params_dai0(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ return ad242x_hw_params(substream, params, dai, 0);
+}
+
+static int ad242x_hw_params_dai1(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ return ad242x_hw_params(substream, params, dai, 1);
+}
+
+static const struct snd_soc_dai_ops ad242x_dai0_ops = {
+ .hw_params = ad242x_hw_params_dai0,
+ .set_fmt = ad242x_set_dai_fmt_dai0,
+};
+
+static const struct snd_soc_dai_ops ad242x_dai1_ops = {
+ .hw_params = ad242x_hw_params_dai1,
+ .set_fmt = ad242x_set_dai_fmt_dai1,
+};
+
+#define AD242X_RATES ( \
+ SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | \
+ SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 | \
+ SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \
+ SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000)
+#define AD242X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_driver ad242x_dai[] = {
+ {
+ .name = "ad242x-dai0",
+ .playback = {
+ .stream_name = "DAI0 Playback",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .capture = {
+ .stream_name = "DAI0 Capture",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .ops = &ad242x_dai0_ops,
+ },
+ {
+ .name = "ad242x-dai1",
+ .playback = {
+ .stream_name = "DAI1 Playback",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .capture = {
+ .stream_name = "DAI1 Capture",
+ .channels_min = 1,
+ .channels_max = 32,
+ .rates = AD242X_RATES,
+ .formats = AD242X_FORMATS,
+ },
+ .ops = &ad242x_dai1_ops,
+ },
+};
+
+static int ad242x_soc_probe(struct snd_soc_component *component)
+{
+ struct ad242x_private *priv = snd_soc_component_get_drvdata(component);
+
+ component->regmap = priv->node->regmap;
+
+ return 0;
+}
+
+static const struct snd_soc_component_driver soc_component_device_ad242x = {
+ .probe = ad242x_soc_probe,
+ .dapm_widgets = ad242x_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(ad242x_dapm_widgets),
+ .dapm_routes = ad242x_dapm_routes,
+ .num_dapm_routes = ARRAY_SIZE(ad242x_dapm_routes),
+ .idle_bias_on = 1,
+ .use_pmdown_time = 1,
+ .endianness = 1,
+ .non_legacy_dai_naming = 1,
+};
+
+static int ad242x_codec_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ad242x_private *priv;
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->node = dev_get_drvdata(dev->parent);
+ platform_set_drvdata(pdev, priv);
+
+ priv->pdm_highpass = of_property_read_bool(dev->of_node,
+ "adi,pdm-highpass-filter");
+
+ return devm_snd_soc_register_component(dev,
+ &soc_component_device_ad242x,
+ ad242x_dai,
+ ARRAY_SIZE(ad242x_dai));
+}
+
+static const struct of_device_id ad242x_of_match[] = {
+ { .compatible = "adi,ad2428w-codec", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad242x_of_match);
+
+static struct platform_driver ad242x_platform_driver = {
+ .driver = {
+ .name = "ad242x-codec",
+ .of_match_table = ad242x_of_match,
+ },
+ .probe = ad242x_codec_platform_probe,
+};
+
+module_platform_driver(ad242x_platform_driver);
+
+MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
+MODULE_DESCRIPTION("AD242X ALSA SoC driver");
+MODULE_LICENSE("GPL");
--
2.23.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 10/10] ASoC: Add codec component for AD242x nodes
2019-12-09 18:35 ` [PATCH 10/10] ASoC: Add codec component for AD242x nodes Daniel Mack
@ 2019-12-16 14:23 ` Mark Brown
2019-12-17 19:28 ` [alsa-devel] " Pierre-Louis Bossart
1 sibling, 0 replies; 34+ messages in thread
From: Mark Brown @ 2019-12-16 14:23 UTC (permalink / raw)
To: Daniel Mack
Cc: linux-kernel, linux-gpio, linux-i2c, alsa-devel, devicetree,
linux-clk, mturquette, sboyd, robh+dt, lee.jones, lars,
pascal.huerst
[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]
On Mon, Dec 09, 2019 at 07:35:11PM +0100, Daniel Mack wrote:
> + /*
> + * Setting clock inversion is only supported globally for both DAIs,
> + * so we ignore the settings made for DAI1 here.
> + */
> + if (index == 0) {
> + switch (format & SND_SOC_DAIFMT_INV_MASK) {
> + case SND_SOC_DAIFMT_NB_NF:
I dunno if it's a blocker but it'd feel nicer to try to verify that the
settings are the same and warn if not.
> +static int ad242x_set_dai_fmt_dai0(struct snd_soc_dai *codec_dai,
> + unsigned int format)
> +{
> + return ad242x_set_dai_fmt(codec_dai, format, 0);
> +}
> +
> +static int ad242x_set_dai_fmt_dai1(struct snd_soc_dai *codec_dai,
> + unsigned int format)
> +{
> + return ad242x_set_dai_fmt(codec_dai, format, 1);
> +}
You don't need separate ops, just look at dai->id.
> +module_platform_driver(ad242x_platform_driver);
> +
> +MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
> +MODULE_DESCRIPTION("AD242X ALSA SoC driver");
> +MODULE_LICENSE("GPL");
Should have a MODULE_ALIAS() as well for autoloading.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [alsa-devel] [PATCH 10/10] ASoC: Add codec component for AD242x nodes
2019-12-09 18:35 ` [PATCH 10/10] ASoC: Add codec component for AD242x nodes Daniel Mack
2019-12-16 14:23 ` Mark Brown
@ 2019-12-17 19:28 ` Pierre-Louis Bossart
2019-12-18 9:49 ` Daniel Mack
1 sibling, 1 reply; 34+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-17 19:28 UTC (permalink / raw)
To: Daniel Mack, linux-kernel, linux-gpio, linux-i2c, alsa-devel,
devicetree, linux-clk
Cc: lars, sboyd, mturquette, robh+dt, broonie, pascal.huerst, lee.jones
On 12/9/19 12:35 PM, Daniel Mack wrote:
> This driver makes AD242x nodes available as DAIs in ASoC topologies.
>
> The hardware allows multiple TDM channel modes and bitdepths, but
> as these modes have influence in the timing calculations at discovery
> time, the mode in that the will be used in needs to be configured
the mode in that the <what> will be used in?
You should probably reword this for clarity.
> statically in the devicetree.
> + if (ad242x_node_is_master(priv->node) &&
> + ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBS_CFS)) {
> + dev_err(component->dev, "master node must be clock slave\n");
> + return -EINVAL;
> + }
> +
> + if (!ad242x_node_is_master(priv->node) &&
> + ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)) {
> + dev_err(component->dev, "slave node must be clock master\n");
> + return -EINVAL;
> + }
It was my understanding that the master node provides the clock to the
bus, so not sure how it could be a clock slave, and conversely how a
slave node could provide a clock to the bus?
> + switch (params_format(params)) {
> + case SNDRV_PCM_FORMAT_S16_LE:
> + if (priv->node->tdm_slot_size != 16)
> + return -EINVAL;
> + break;
> + case SNDRV_PCM_FORMAT_S32_LE:
> + if (priv->node->tdm_slot_size != 32)
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
how does this work for PDM data?
is the PDM data packed into a regular TDM slot?
> +
> + if (priv->pdm[index]) {
> + if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> + return -EINVAL;
> +
> + if (index == 0) {
> + val = AD242X_PDMCTL_PDM0EN;
> + mask = AD242X_PDMCTL_PDM0EN | AD242X_PDMCTL_PDM0SLOTS;
> + } else {
> + val = AD242X_PDMCTL_PDM1EN;
> + mask = AD242X_PDMCTL_PDM1EN | AD242X_PDMCTL_PDM1SLOTS;
> + }
> +
> + switch (params_channels(params)) {
> + case 1:
> + break;
> + case 2:
> + val = mask;
> + break;
A comment wouldn't hurt here...
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [alsa-devel] [PATCH 10/10] ASoC: Add codec component for AD242x nodes
2019-12-17 19:28 ` [alsa-devel] " Pierre-Louis Bossart
@ 2019-12-18 9:49 ` Daniel Mack
2019-12-18 15:32 ` Pierre-Louis Bossart
0 siblings, 1 reply; 34+ messages in thread
From: Daniel Mack @ 2019-12-18 9:49 UTC (permalink / raw)
To: Pierre-Louis Bossart, linux-kernel, linux-gpio, linux-i2c,
alsa-devel, devicetree, linux-clk
Cc: lars, sboyd, mturquette, robh+dt, broonie, pascal.huerst, lee.jones
Hi,
On 12/17/19 8:28 PM, Pierre-Louis Bossart wrote:
> On 12/9/19 12:35 PM, Daniel Mack wrote:
>> + if (!ad242x_node_is_master(priv->node) &&
>> + ((format & SND_SOC_DAIFMT_MASTER_MASK) !=
>> SND_SOC_DAIFMT_CBM_CFM)) {
>> + dev_err(component->dev, "slave node must be clock master\n");
>> + return -EINVAL;
>> + }
>
> It was my understanding that the master node provides the clock to the
> bus, so not sure how it could be a clock slave, and conversely how a
> slave node could provide a clock to the bus?
The slave nodes receive the A2B clock from the master node and then
produce digital audio output that is sent to other components such as
codecs. Hence, in ASoC terms, they are the clock master.
Likewise, as the master node is receiving its clock from other
components, it has to be a clock slave in the audio network.
Does that make sense?
>> + switch (params_format(params)) {
>> + case SNDRV_PCM_FORMAT_S16_LE:
>> + if (priv->node->tdm_slot_size != 16)
>> + return -EINVAL;
>> + break;
>> + case SNDRV_PCM_FORMAT_S32_LE:
>> + if (priv->node->tdm_slot_size != 32)
>> + return -EINVAL;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>
> how does this work for PDM data?
>
> is the PDM data packed into a regular TDM slot?
Yes. But I admit this needs some more testing. We're still working on
the hardware that uses this. I'll revisit this.
And I'll also add a lot more comments all over the place, as also
requested by Lee.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [alsa-devel] [PATCH 10/10] ASoC: Add codec component for AD242x nodes
2019-12-18 9:49 ` Daniel Mack
@ 2019-12-18 15:32 ` Pierre-Louis Bossart
0 siblings, 0 replies; 34+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-18 15:32 UTC (permalink / raw)
To: Daniel Mack, linux-kernel, linux-gpio, linux-i2c, alsa-devel,
devicetree, linux-clk
Cc: lars, sboyd, mturquette, robh+dt, broonie, pascal.huerst, lee.jones
On 12/18/19 3:49 AM, Daniel Mack wrote:
> Hi,
>
> On 12/17/19 8:28 PM, Pierre-Louis Bossart wrote:
>> On 12/9/19 12:35 PM, Daniel Mack wrote:
>
>>> + if (!ad242x_node_is_master(priv->node) &&
>>> + ((format & SND_SOC_DAIFMT_MASTER_MASK) !=
>>> SND_SOC_DAIFMT_CBM_CFM)) {
>>> + dev_err(component->dev, "slave node must be clock master\n");
>>> + return -EINVAL;
>>> + }
>>
>> It was my understanding that the master node provides the clock to the
>> bus, so not sure how it could be a clock slave, and conversely how a
>> slave node could provide a clock to the bus?
>
> The slave nodes receive the A2B clock from the master node and then
> produce digital audio output that is sent to other components such as
> codecs. Hence, in ASoC terms, they are the clock master.
>
> Likewise, as the master node is receiving its clock from other
> components, it has to be a clock slave in the audio network.
>
> Does that make sense?
Your slave node acts as a bridge then, but it seems you don't model the
bus-facing interface, which has to follow the master clock. Or do you?
Likewise the master has an 'SOC-facing' interface and a bus-facing
interface. it *could* be master on both if ASRC was supported. The point
is that the bus-facing interface is not clock slave.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [alsa-devel] [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver
2019-12-09 18:35 [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Daniel Mack
` (10 preceding siblings ...)
2019-12-09 18:35 ` [PATCH 10/10] ASoC: Add codec component for AD242x nodes Daniel Mack
@ 2019-12-17 19:29 ` Pierre-Louis Bossart
2019-12-18 9:53 ` Daniel Mack
11 siblings, 1 reply; 34+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-17 19:29 UTC (permalink / raw)
To: Daniel Mack, linux-kernel, linux-gpio, linux-i2c, alsa-devel,
devicetree, linux-clk
Cc: lars, sboyd, mturquette, robh+dt, broonie, pascal.huerst, lee.jones
> Transceivers can both receive and provide audio, and streams can be
> routed from one node to any other, including many others. The tricky
> bit is how to expose the audio routing in DT in a sane way.
> The way it is implemented here, the slave nodes specify the number of
> slots they each consume and generate, and which thereof they forward
> from one side to the other. This mimics the internal register
> structure and should allow for even exotic setups.
It was my understanding that the A2B bus is bidirectional but with
separate time windows allocated for host->device and device->host
transmission. The wording seems to hint at device-to-device
communication but I wonder if this is really what you meant.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [alsa-devel] [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver
2019-12-17 19:29 ` [alsa-devel] [PATCH 00/10] mfd: Add support for Analog Devices A2B transceiver Pierre-Louis Bossart
@ 2019-12-18 9:53 ` Daniel Mack
0 siblings, 0 replies; 34+ messages in thread
From: Daniel Mack @ 2019-12-18 9:53 UTC (permalink / raw)
To: Pierre-Louis Bossart, linux-kernel, linux-gpio, linux-i2c,
alsa-devel, devicetree, linux-clk
Cc: lars, sboyd, mturquette, robh+dt, broonie, pascal.huerst, lee.jones
Hi,
On 12/17/19 8:29 PM, Pierre-Louis Bossart wrote:
>> Transceivers can both receive and provide audio, and streams can be
>> routed from one node to any other, including many others. The tricky
>> bit is how to expose the audio routing in DT in a sane way.
>> The way it is implemented here, the slave nodes specify the number of
>> slots they each consume and generate, and which thereof they forward
>> from one side to the other. This mimics the internal register
>> structure and should allow for even exotic setups.
>
> It was my understanding that the A2B bus is bidirectional but with
> separate time windows allocated for host->device and device->host
> transmission. The wording seems to hint at device-to-device
> communication but I wonder if this is really what you meant.
Yes, audio frames can be exchanged between two slave nodes without
interaction by the master node. I'm not sure what the best way is to
describe that in DT really, but as the hardware is capable of doing it,
there must be a way to enable such setups.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 34+ messages in thread