Re: [PATCH v2 2/6] dt-bindings: sound: cirrus,cs42l43: Add initial DT binding

On 30/05/2023 14:21, 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

Thank you for your patch. There is something to discuss/improve.

• clocks:
• items:

•  - description: Synchronous audio clock provided on mclk_in.
  

• clock-names:
• const: mclk
• cirrus,bias-low:
• type: boolean
• description:

•  Select a 1.8V headset micbias rather than 2.8V.
  

• cirrus,bias-sense-ua:

"ua" looks like microamp. If so, microamp instead: https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/prope...

• $ref: /schemas/types.yaml#/definitions/uint32
• description:

•  Current at which the headset micbias sense clamp will engage, 0 to
  

•  disable.
  

• enum: [ 0, 14, 23, 41, 50, 60, 68, 86, 95 ]
• default: 0
• cirrus,bias-ramp-ms:
• description:

•  Time in milliseconds the hardware allows for the headset micbias to
  

•  ramp up.
  

• enum: [ 10, 40, 90, 170 ]
• default: 170
• cirrus,detect-us:
• description:

•  Time in microseconds the type detection will run for. Long values will
  

•  cause more audible effects, but give more accurate detection.
  

• enum: [ 20, 100, 1000, 10000, 50000, 75000, 100000, 200000 ]
• default: 10000
• cirrus,button-automute:
• type: boolean
• description:

•  Enable the hardware automuting of decimator 1 when a headset button is
  

•  pressed.
  

• cirrus,buttons-ohms:
• description:

•  Impedance in Ohms for each headset button, these should be listed in
  

•  ascending order.
  

• minItems: 1
• maxItems: 6
• cirrus,tip-debounce-ms:
• description:

•  Software debounce on tip sense triggering in milliseconds.
  

• default: 0
• cirrus,tip-invert:
• type: boolean
• description:

•  Indicates tip detect polarity, inverted implies open-circuit whilst the
  

•  jack is inserted.
  

• cirrus,tip-disable-pullup:
• type: boolean
• description:

•  Indicates if the internal pullup on the tip detect should be disabled.
  

• cirrus,tip-fall-db-ms:
• description:

•  Time in milliseconds a falling edge on the tip detect should be hardware
  

•  debounced for. Note the falling edge is considered after the invert.
  

• enum: [ 0, 125, 250, 500, 750, 1000, 1250, 1500 ]
• default: 500
• cirrus,tip-rise-db-ms:
• description:

•  Time in milliseconds a rising edge on the tip detect should be hardware
  

•  debounced for. Note the rising edge is considered after the invert.
  

• enum: [ 0, 125, 250, 500, 750, 1000, 1250, 1500 ]
• default: 500
• cirrus,use-ring-sense:
• type: boolean
• description:

•  Indicates if the ring sense should be used.
  

• cirrus,ring-invert:
• type: boolean
• description:

•  Indicates ring detect polarity, inverted implies open-circuit whilst the
  

•  jack is inserted.
  

• cirrus,ring-disable-pullup:
• type: boolean
• description:

•  Indicates if the internal pullup on the ring detect should be disabled.
  

• cirrus,ring-fall-db-ms:
• description:

•  Time in milliseconds a falling edge on the ring detect should be hardware
  

•  debounced for. Note the falling edge is considered after the invert.
  

• enum: [ 0, 125, 250, 500, 750, 1000, 1250, 1500 ]
• default: 500
• cirrus,ring-rise-db-ms:
• description:

•  Time in milliseconds a rising edge on the ring detect should be hardware
  

•  debounced for. Note the rising edge is considered after the invert.
  

• enum: [ 0, 125, 250, 500, 750, 1000, 1250, 1500 ]
• default: 500
• pinctrl:
• type: object
• allOf:

Drop allOf, just "$ref: ......"

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

• additionalProperties: false

Also drop blank lines between these three above.

• properties:

•  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
  

• patternProperties:

•  "-state$":
  

Use consistent quotes, either " or ' everywhere

•    oneOf:
  

•      - $ref: "#/$defs/cirrus-cs42l43-state"
  

•      - patternProperties:
  

•          "-pins$":
  

•            $ref: "#/$defs/cirrus-cs42l43-state"
  

•        additionalProperties: false
  

• spi:
• type: object
• allOf:

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

• unevaluatedProperties: false

Same comments here.

+$defs:

• cirrus-cs42l43-state:
• type: object

Best regards, Krzysztof