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

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: <YkWk5vvBlC/Orpdr@sirena.org.uk>
Date:   Thu, 31 Mar 2022 13:56:06 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Martin Povišer <povik@...ebit.org>
Cc:     Martin Povišer <povik+lin@...ebit.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Mark Kettenis <kettenis@...nbsd.org>,
        Hector Martin <marcan@...can.st>,
        Sven Peter <sven@...npeter.dev>
Subject: Re: [RFC PATCH 5/5] ASoC: Add macaudio machine driver

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

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)