Re: [PATCH v2 3/5] ASoC: dt-bindings: wm8904: Add DMIC, GPIO, MIC and EQ support

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On Mon, Feb 24, 2025 at 04:54:58PM +0100, Francesco Dolcini wrote:
> +  wlf,drc-cfg-regs:
> +    $ref: /schemas/types.yaml#/definitions/uint16-array
> +    description:
> +      Default register values for R40/41/42/43 (DRC).
> +      The list must be 4 times the length of wlf,drc-cfg-names.
> +      If absent, DRC is disabled.
> +
> +  wlf,retune-mobile-cfg-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    description:
> +      List of strings for the available retune modes.
> +      If absent, retune is disabled.

How is this retune supposed to be used? If by user-space I can easily
imagine that static DTS configuration won't be enough, because you need
to factor for example temperature or some other minor differences
between same boards.

> +
> +  wlf,retune-mobile-cfg-rates:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

Drop

> +    description:
> +      List of rates for the available retune modes.

Use standard property suffixes - hz or whatever is matching here.

> +      The list must be the same length as wlf,retune-mobile-cfg-names.
> +      If absent, retune is disabled.
> +
> +  wlf,retune-mobile-cfg-regs:
> +    $ref: /schemas/types.yaml#/definitions/uint16-array
> +    description:
> +      Default register values for R134/.../157 (EQ).
> +      The list must be 24 times the length of wlf,retune-mobile-cfg-names.
> +      If absent, retune is disabled.
> +
> +dependencies:
> +  wlf,drc-cfg-names: [ 'wlf,drc-cfg-regs' ]
> +  wlf,drc-cfg-regs: [ 'wlf,drc-cfg-names' ]
> +
> +  wlf,retune-mobile-cfg-names: [ 'wlf,retune-mobile-cfg-rates', 'wlf,retune-mobile-cfg-regs' ]
> +  wlf,retune-mobile-cfg-regs: [ 'wlf,retune-mobile-cfg-names', 'wlf,retune-mobile-cfg-rates' ]
> +  wlf,retune-mobile-cfg-rates: [ 'wlf,retune-mobile-cfg-names', 'wlf,retune-mobile-cfg-regs' ]
> +
>  required:
>    - compatible
>    - reg
> @@ -70,5 +138,43 @@ examples:
>              DBVDD-supply = <&reg_1p8v>;
>              DCVDD-supply = <&reg_1p8v>;
>              MICVDD-supply = <&reg_1p8v>;
> +
> +            wlf,drc-cfg-names = "default", "peaklimiter", "tradition", "soft", "music";
> +            wlf,drc-cfg-regs =
> +                /* coded default: KNEE_IP = KNEE_OP = 0, HI_COMP = LO_COMP = 1  */

Please follow DTS coding style - how you wrap, where to break lines.

> +                /bits/ 16 <0x01af 0x3248 0x0000 0x0000>,
> +                /* coded default: KNEE_IP = -24, KNEE_OP = -6, HI_COMP = 1/4, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0010 0x0408>,
> +                /* coded default: KNEE_IP = -42, KNEE_OP = -3, HI_COMP = 0, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0028 0x0704>,
> +                /* coded default: KNEE_IP = -45, KNEE_OP = -9, HI_COMP = 1/8, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0018 0x078c>,
> +                /* coded default: KNEE_IP = -30, KNEE_OP = -10.5, HI_COMP = 1/4, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0010 0x050e>;
> +
> +            wlf,gpio-cfg = <

There is almost never empty line after <

> +                0x0018 /* GPIO1 => DMIC_CLK */
> +                0xffff /* GPIO2 => don't touch */
> +                0xffff /* GPIO3 => don't touch */
> +                0xffff /* GPIO4 => don't touch */

Not mentioning this looks really incorrect or incomplete. No ending of
property.

> +
> +            wlf,retune-mobile-cfg-names = "bassboost", "bassboost", "treble";
> +            wlf,retune-mobile-cfg-rates = <48000 44100 48000>;
> +            wlf,retune-mobile-cfg-regs =
> +                /* bassboost: EQ_ENA = 1, +6 dB @ 100 Hz, +3 dB @ 300 Hz, 0 dB @ 875, 2400, 6900 Hz */
> +                /bits/ 16 <0x1 0x12 0xf 0xc 0xc 0xc>,
> +                /* default values for ReTune Mobile registers 140-157 */
> +                /bits/ 16 <0x0fca 0x0400 0x00d8 0x1eb5 0xf145 0x0bd5 0x0075 0x1c58 0xf3d3 0x0a54 0x0568 0x168e 0xf829 0x07ad 0x1103 0x0564 0x0559 0x4000>,

See DTS coding style.

Best regards,
Krzysztof