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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <er4bcixggriqp6idl6xmr7bjetf5kkhadyeplkbyxvrffuiknc@ews752x4ugh7>
Date: Thu, 27 Feb 2025 16:34:34 +0100
From: Ernest Van Hoecke <ernestvanhoecke@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Francesco Dolcini <francesco@...cini.it>, 
	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 Tue, Feb 25, 2025 at 09:41:17AM +0100, Krzysztof Kozlowski wrote:
> 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.

This is intended for integrators to be able to specify some EQ options,
mirroring the previous behaviour that was possible via platform data.

I expect most users to use the first five Retune Mobile registers and
not care about the rest, which require a proprietary tool and are not
well documented. The example in the binding shows how some simple
static EQ can be configured. Anyone interested in the extended config
can also use it (statically).

If someone requires dynamic behaviour at runtime that could be a
separate patch that should not be hindered by this static config.

> > +
> > +  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.

I will send a v3 later integrating your feedback, I have renamed this
property to wlf,retune-mobile-cfg-hz there, and dropped the "ref".

> > +
> > +            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
> 

Would the following snippet be a good way to handle wrapping this?
To me the first six registers form an "item" since they are the most
important, followed by the next 18 which belong together, but I was
not sure about the common convention to handle wrapping such a long
item.

/*
 * Config registers per name, respectively:
 * EQ_ENA,  100 Hz,  300 Hz,  875 Hz, 2400 Hz, 6900 Hz
 *      1,   +6 dB,   +3 dB,    0 dB,    0 dB,    0 dB
 *      1,   +6 dB,   +3 dB,    0 dB,    0 dB,    0 dB
 *      1,   -2 dB,   -2 dB,    0 dB,    0 dB,   +3 dB
 * Each one uses the defaults for ReTune Mobile registers 140-157
 */
wlf,retune-mobile-cfg-regs = /bits/ 16 <0x1 0x12 0xf 0xc 0xc 0xc>,
                             /bits/ 16 <0x0fca 0x0400 0x00d8 0x1eb5
                                        0xf145 0x0bd5 0x0075 0x1c58
                                        0xf3d3 0x0a54 0x0568 0x168e
                                        0xf829 0x07ad 0x1103 0x0564
                                        0x0559 0x4000>,

                             /bits/ 16 <0x1 0x12 0xf 0xc 0xc 0xc>,
                             /bits/ 16 <0x0fca 0x0400 0x00d8 0x1eb5
                                        0xf145 0x0bd5 0x0075 0x1c58
                                        0xf3d3 0x0a54 0x0568 0x168e
                                        0xf829 0x07ad 0x1103 0x0564
                                        0x0559 0x4000>,

                             /bits/ 16 <0x1 0xa 0xa 0xc 0xc 0xf>,
                             /bits/ 16 <0x0fca 0x0400 0x00d8 0x1eb5
                                        0xf145 0x0bd5 0x0075 0x1c58
                                        0xf3d3 0x0a54 0x0568 0x168e
                                        0xf829 0x07ad 0x1103 0x0564
                                        0x0559 0x4000>;

Apologies for sending the broken binding. I have integrated the rest of
your feedback into v3 which will be sent later. Thanks for taking the
time to review.

Kind regards,
Ernest