Re: [RFC PATCH 5/5] ASoC: Add macaudio machine driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

On Thu, Mar 31, 2022 at 02:08:51PM +0200, Martin Povišer wrote:
> > On 31. 3. 2022, at 13:59, Mark Brown <broonie@kernel.org> wrote:

> >> +	for_each_rtd_components(rtd, i, component)
> >> +		snd_soc_component_set_jack(component, &ma->jack, NULL);

> > What is the jack configuration this is attempting to describe?  It looks
> > like you have some dedicated speaker driver devices which are going to
> > get attached to jacks here for example.

> We know the speakers will ignore the set_jack call. There’s one jack and
> this way we know the jack codec will attach to it, for speakers it’s a no-op.
> (If you prefer I will special-case it to the jack codec.)

It would be better to special case, this looks obviously wrong and will
break if someone adds error handling.

> >> +	return !strcmp(name, pattern);
> >> +}

> > This looks worryingly like use case configuration.

> I go over this in the cover letter! This is fixing the TDM slot selection
> and disabling voltage/current sensing on the speaker amp codecs, which have
> no business being exposed to userspace as options. This is not use case,
> this not letting people blow their speakers from userspace.

Your comments in the cover letter are all pretty vague too, that just
says that these controls are "ridiculous" which isn't terribly specific
about what the actual goal is.  If it's just "I can't see why anyone
would want to configure this" then that's a decision you're taking about
what people might want to do which is broadly a use case configuration
and the control should be left there in case someone comes up with an
idea.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]