[alsa-devel] [PATCH RFC V2] ALSA: usb-audio: Scarlett Gen 2 mixer interface

[Takashi Iwai]

On Mon, 29 Apr 2019 15:56:02 +0200,
Geoffrey D. Bennett wrote:
> 
> Add mixer quirk for the Focusrite Scarlett 18i8 and 18i20 Gen 2 audio
> interfaces. Although the interfaces are USB compliant, additional
> input/output level controls and hardware routing/mixing functionality
> are available using proprietary USB requests.
> 
> Signed-off-by: Geoffrey D. Bennett <g at b4.vu>
> ---
> Hi all,
> 
> This patch adds the following controls for the Scarlett 18i8 and 18i20
> Gen 2:
> - Master volume knob indicator (18i20 only)
> - Volume controls for the analogue HW outputs
> - HW/SW volume switches for the 10 analogue HW outputs (18i20 only)
> - Output mux (where the sound for the HW outputs comes from; defaults
>   to PCM outputs)
> - Capture mux (where the sound for PCM recording comes from; defaults
>   to HW inputs)
> - Matrix mux (where the sound going into the mixer comes from; 18
>   inputs default off)
> - Mixer matrix (18 inputs * 10 outputs = 180 controls)
> - Mute and dim indicators (18i20 only)
> - Line Level/Instrument Level and Pad controls (18i8 only)
> - Level meters
> 
> Changes since v1:
> - Add support for the Scarlett 18i8 Gen 2
> - Save configuration parameters to NVRAM
> - Implemented feedback from Takashi's email 24/Apr/2019
> - Moved private field from struct snd_usb_audio to struct
>   usb_mixer_interface
> - Added timer and buffer fields to struct usb_mixer_interface
> - Other small code fixes/cleanups/improvements
> 
> Outstanding issues/questions:
> 
> - Adding fields to struct usb_mixer_interface seemed a better place
>   than struct snd_usb_audio as in version 1 of this patch (I think
>   these new fields are used in the same sort of context as the "Sound
>   Blaster remote control stuff" fields, and I am reusing rc_urb and
>   rc_setup_packet). I wasn't sure of the naming; should they be named
>   generically ("timer", "private") or specifically
>   ("scarlett2_buffer")? Should the Sound Blaster RC fields that I'm
>   reusing be renamed to have generic names?

The best would be completely without it, but it's a better place,
indeed.  However, the addition should be only one pointer field
private_data and one function pointer private_free.  Put the all
device specific stuff to private_data and release via the own
private_free callback instead.

> - There are software controllable pads on the 18i8 line-inputs. I
>   called the controls "Line In %d Pad Capture Switch". They work
>   correctly in alsamixer (shown in the capture view as e.g. "Line In 1
>   Pad") but the "on" state is displayed as "L R CAPTURE" which looks
>   unusual. I could use an Enum with Off/On values, but I thought this
>   is what "Switch" is for (as per control-names.rst). Should I leave
>   this as-is, or is there a better name that will make this show up as
>   a regular on/off switch?
> 
>   amixer contents shows:
> 
>   numid=15,iface=MIXER,name='Line In 1 Pad Capture Switch'
>     ; type=BOOLEAN,access=rw------,values=1
>     : values=off
> 
>   amixer scontents shows:
> 
>   Simple mixer control 'Line In 1 Pad',0
>     Capabilities: cswitch cswitch-joined
>     Capture channels: Mono
>     Mono: Capture [off]

It's possible to override per device-specific stuff via a
shared-object plugin in alsa-lib, but this is far too complex.
Rather leave it as is.  alsamixer isn't the best tool for a
complicated mixer UI, after all.

About the other changes:
a timer is no suitable choice for such a purpose.  The easier option
would be the delayed work.  Then you can use GFP_KERNEL instead of
GFP_ATOMIC.

Also, the delayed work needs to be canceled or flushed at PM suspend
callback as well as disconnecting.


thanks,

Takashi