Re: [PATCH 05/10] dt-bindings: mfd: cirrus,cs42l43: Add initial DT binding

On 12/05/2023 14:28, Charles Keepax wrote:

The CS42L43 is an audio CODEC with integrated MIPI SoundWire interface (Version 1.2.1 compliant), I2C, SPI, and I2S/TDM interfaces designed for portable applications. It provides a high dynamic range, stereo DAC for headphone output, two integrated Class D amplifiers for loudspeakers, and two ADCs for wired headset microphone input or stereo line input. PDM inputs are provided for digital microphones.

Add a YAML DT binding document for this device.

Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com

.../bindings/mfd/cirrus,cs42l43.yaml | 212 ++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 213 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/cirrus,cs42l43.yaml

diff --git a/Documentation/devicetree/bindings/mfd/cirrus,cs42l43.yaml b/Documentation/devicetree/bindings/mfd/cirrus,cs42l43.yaml new file mode 100644 index 0000000000000..e1fd70e0a3467 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/cirrus,cs42l43.yaml @@ -0,0 +1,212 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/cirrus,cs42l43.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#

+title: Cirrus Logic CS42L43 Audio CODEC

That's audio codec, so it should be in sound, not MFD.

+maintainers:

• • patches@opensource.cirrus.com

+description: |

• The CS42L43 is an audio CODEC with integrated MIPI SoundWire interface
• (Version 1.2.1 compliant), I2C, SPI, and I2S/TDM interfaces designed
• for portable applications. It provides a high dynamic range, stereo
• DAC for headphone output, two integrated Class D amplifiers for
• loudspeakers, and two ADCs for wired headset microphone input or
• stereo line input. PDM inputs are provided for digital microphones.

+required:

• • compatible
• • reg
• • VDD_P-supply
• • VDD_A-supply
• • VDD_D-supply
• • VDD_IO-supply
• • VDD_CP-supply

lowercase, no underscores in all property names.

+additionalProperties: false

This order is quite unexpected... please do not invent your own layout. Use example-schema as your starting point. I suspect there will be many things to fix, so limited review follows (not complete).

Missing ref to dai-common

+properties:

• compatible:
• enum:

•  - cirrus,cs42l43
  

• reg:
• maxItems: 1
• VDD_P-supply:
• description:

•  Power supply for the high voltage interface.
  

• VDD_A-supply:
• description:

•  Power supply for internal analog circuits.
  

• VDD_D-supply:
• description:

•  Power supply for internal digital circuits.
  

• VDD_IO-supply:
• description:

•  Power supply for external interface and internal digital logic.
  

• VDD_CP-supply:
• description:

•  Power supply for the amplifier 3 and 4 charge pump.
  

• VDD_AMP-supply:
• description:

•  Power supply for amplifier 1 and 2.
  

• reset-gpios:
• maxItems: 1
• gpio-controller: true
• '#gpio-cells':
• const: 2
• gpio-ranges:
• items:

•  - description: A phandle to the CODEC pinctrl node
  

•    minimum: 0
  

•  - const: 0
  

•  - const: 0
  

•  - const: 3
  

• interrupt-controller: true
• '#interrupt-cells':
• const: 2
• interrupts:
• maxItems: 1
• '#sound-dai-cells':
• const: 1
• clocks:
• items:

•  - description: Synchronous audio clock provided on mclk_in.
  

• clock-names:
• const: mclk
• pinctrl:
• type: object

additionalProperties: false

• allOf:

•  - $ref: "../pinctrl/pinctrl.yaml#"
  

No quotes, absolute path, so /schemas/pinctrl/....

• properties:

•  pin-settings:
  

What's this node about? pinctrl/pinctrl/pins? One level too much.

•    type: object
  

•    additionalProperties: false
  

•    patternProperties:
  

•      '-pins$':
  

•        type: object
  

•        allOf:
  

•          - $ref: "../pinctrl/pincfg-node.yaml#"
  

•          - $ref: "../pinctrl/pinmux-node.yaml#"
  

Same comments.

•        oneOf:
  

•          - required: [ groups ]
  

•          - required: [ pins ]
  

•        unevaluatedProperties: false
  

•        properties:
  

•          groups:
  

•            enum: [ gpio1, gpio2, gpio3, asp, pdmout2, pdmout1, i2c, spi ]
  

•          pins:
  

•            enum: [ gpio1, gpio2, gpio3,
  

•                    asp_dout, asp_fsync, asp_bclk,
  

•                    pdmout2_clk, pdmout2_data, pdmout1_clk, pdmout1_data,
  

•                    i2c_sda, i2c_scl,
  

•                    spi_miso, spi_sck, spi_ssb ]
  

•          function:
  

•            enum: [ gpio, spdif, irq, mic-shutter, spk-shutter ]
  

•          drive-strength:
  

•            description: Set drive strength in mA
  

•            enum: [ 1, 2, 4, 8, 9, 10, 12, 16 ]
  

•          input-debounce:
  

•            description: Set input debounce in uS
  

•            enum: [ 0, 85 ]
  

• spi:
• type: object
• allOf:

•  - $ref: "../spi/spi-controller.yaml#"
  

Same comments.

• unevaluatedProperties: false

+examples:

• • |
• i2c@e0004000 {

•    #address-cells = <1>;
  

•    #size-cells = <0>;
  

•    reg = <0xe0004000 0x1000>;
  

Drop, just i2c

•    cs42l43: codec@1a {
  

•        compatible = "cirrus,cs42l43";
  

•        reg = <0x1a>;
  

•        VDD_P-supply = <&vdd5v0>;
  

•        VDD_D-supply = <&vdd1v8>;
  

•        VDD_A-supply = <&vdd1v8>;
  

•        VDD_IO-supply = <&vdd1v8>;
  

•        VDD_CP-supply = <&vdd1v8>;
  

•        VDD_AMP-supply = <&vdd5v0>;
  

•        reset-gpios = <&gpio 0>;
  

Use proper defines for flags.

Best regards, Krzysztof