On Thu, Mar 31, 2022 at 02:08:51PM +0200, Martin Povišer wrote: > > On 31. 3. 2022, at 13:59, Mark Brown <broonie@xxxxxxxxxx> 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:
signature.asc
Description: PGP signature
