Re: Splitting out controls

[Richard Fitzgerald <rf@opensource.wolfsonmicro.com>]

On Fri, 2015-10-16 at 18:00 +0200, Takashi Iwai wrote:
> On Fri, 16 Oct 2015 17:35:30 +0200,
> Richard Fitzgerald wrote:
> > 
> > On Tue, 2015-10-13 at 09:07 +0200, David Henningsson wrote:
> > > 
> > > On 2015-10-12 22:59, James Cameron wrote:
> > > > On Mon, Oct 12, 2015 at 02:49:46PM +0100, Liam Girdwood wrote:
> > > >> I've written up the minutes here below
> > > >
> > > > Thanks!
> > > >
> > > >> Splitting out controls: Takashi
> > > >> ===============================
> > > >>
> > > >>   - Restricted access.  Consensus to restrict access to some controls due
> > > >> to possibility of breaking HW at kernel level. i.e. prevent feeding
> > > >> digital Mic into HP amp to prevent speaker over heating.
> > > >
> > > > I'd like that.  rt5631.  Avoiding at the moment by removing the controls.
> > > 
> > > IIRC, the debate was over "do not expose dangerous controls to userspace 
> > > at all" vs "expose dangerous controls controls only to root".
> > > 
> > > I'm strongly voting for "do not expose to userspace at all".
> > > 
> > > I personally believe that if the physical hardware can be set to state 
> > > where it's bricked, the hardware itself is buggy.
> > > 
> > > If the hardware is buggy, this should be worked around in BIOS or 
> > > whatever firmware is present on the machine. Otherwise there is a bug in 
> > > BIOS.
> > > 
> > > If BIOS is buggy and cannot protect the machine from being physically 
> > > damaged, then we need to work around that in the kernel. Otherwise there 
> > > is a bug in the kernel.
> > > 
> > > And if the kernel is buggy, we should fix the kernel. Period. :-)
> > > 
> > I agree with you in principle that if it can break the hardware then
> > either it shouldn't be exposed to user-side at all, or it should be
> > checked by the kernel/driver to prevent bad settings.
> > 
> > However, what about this sort of scenario: some codec has a speaker
> > volume range of 0..100, all of which are valid and safe. Manufacturer X
> > makes a device with an inadequate speaker that can be damaged with
> > volume settings above 80. How is that protected? There's nothing wrong
> > with the codec driver. There's no software at all for a speaker - it's
> > just a speaker. Where do we put a hard limit of 80 on a codec control
> > for one specific device? If it was my codec driver I don't want to have
> > to put a workaround for one specific device because manufacturer X chose
> > the wrong type of speaker. Or do we not care about the "stupid
> > manufacturer" cases and we're only interested in protecting the device
> > the control directly applies to - in this example it's a codec control
> > so it mustn't damage the codec but we don't care if poor hardware design
> > means it could damage other hardware connected to the codec.
> 
> There is snd_soc_limit_volume() function to override the volume range
> from a machine driver for such a purpose.  This was what was suggested
> in the meeting.
> 
> 
> Takashi

OK, I didn't know that but I do now, so that wasn't a good example. But
how about something more complex. Let's say it was a set of coefficient
values for a filter. That's not a simple range check, it would need
specialized code to understand whether the coefficients were safe.

Really my point was that if all hardware was completely isolated from
other hardware you can error-check controls. But when you start hooking
up bits of hardware to other bits of hardware, it becomes more complex
defining what is safe, and who is responsible for checking that it is
safe, and where the knowledge about how to check it's safe should live.

That said, I'm not a fan of the "unless we can fix everything we
shouldn't fix anything" attitude. Fixing something is always better than
fixing nothing. So the fact that combining real hardware can introduce
new types of unsafe settings isn't an argument against error-checking
control values.