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

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Wed, Mar 05, 2025 at 07:45:50AM +0100, Krzysztof Kozlowski wrote:
> On 27/02/2025 16:34, Ernest Van Hoecke wrote:
> > 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.
> 
> 
> No, if this is suitable for dynamic configuration then it's a proof it
> is not suitable for DT.
> 

Whilst I can see the argument that this could be exposed as an
ALSA control. I would also suggest that this is not adding some new
feature, these values have been filled in from pdata for 16 years
now. Changing the way such a vintage part works at this point feels
more problematic to me than a slightly iffy DT property.

On the flip side of the argument, the parameters that are filled
into this are almost certainly specific tuning for the hardware,
so in many ways this is hardware description and there is a
certain appeal to shipping the tuning with the hardware (ie. in
DT).

Thanks,
Charles