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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250225-delicate-tortoise-of-management-e43fa2@krzk-bin>
Date: Tue, 25 Feb 2025 09:41:17 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Francesco Dolcini <francesco@...cini.it>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Saravana Kannan <saravanak@...gle.com>, 
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, patches@...nsource.cirrus.com, 
	Ernest Van Hoecke <ernest.vanhoecke@...adex.com>, linux-sound@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Francesco Dolcini <francesco.dolcini@...adex.com>, 
	Charles Keepax <ckeepax@...nsource.cirrus.com>
Subject: Re: [PATCH v2 3/5] ASoC: dt-bindings: wm8904: Add DMIC, GPIO, MIC
 and EQ support

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