ALSA: Add driver for big Scarlett 4th Gen interfaces
Subsystems (2)
Geoffrey D. Bennett
maintainercover ALSA: Add driver for big Scarlett 4th Gen interfaces
Hi Takashi,
Merry Christmas!
This patch series introduces a new Focusrite Control Protocol (FCP) driver that provides equivalent functionality to the existing Scarlett2 driver, but splits the implementation between a minimal kernel driver and a user-space component. My initial user-space implementation supports the new Scarlett 4th Gen 16i16, 18i16, and 18i20 interfaces, but the new FCP kernel driver can be used with any device supported by the Scarlett2 driver (as an opt-in).
With these new interfaces, I started out by trying to extend the existing Scarlett2 driver but found that that wasn't practical. The control locations (struct scarlett2_config offset field) have previously varied between firmware versions (which is why I added the "Minimum Firmware Version" requirement for previous 4th Gen and Vocaster devices), and the situation was worse with the new 4th Gen devices, with every new firmware version moving the controls.
While the device provides a map indicating control locations, implementing the required JSON parsing in the kernel isn't feasible, necessitating a user-space implementation.
The new approach in the FCP driver significantly improves both maintainability and paves the way for future enhancements:
Firstly, the kernel component of the FCP driver is <10% the size of the Scarlett2 driver, with the obvious maintainability advantages; cloc reports:
File blank comment codesound/usb/mixer_scarlett2.c 1765 852 7152sound/usb/fcp.c 174 64 645
Secondly, changes that aren't feasible to implement in the kernel due to backward compatibility constraints will be able to be implemented in user-space, with end-users easily able to choose an older version of the controls if needed. For example:
1) The largest interface has >500 ALSA controls just for the matrix mixer (one control for each entry in the N×M matrix). This would be better done as N controls with M values each, especially since the device supports writing a row (all input gains for one mixer output) at a time.
2) Similarly, the mux table entries have a control per entry, but they're all written at once, so it would be better to have a single ALSA control for the whole mux table.
3) The interfaces have 3 independent mux tables (one per sample rate 48kHz, 96kHz, 192kHz), but supporting them isn't feasible in Scarlett2 due to backward compatibility constraints.
4) Users of the Scarlett2 driver frequently request higher-level controls like stereo-linking, pan, mute, and solo. These would be very challenging to implement in the kernel while maintaining backward compatibility to the existing controls, but again, it's easy to have configurable controls in user-space.
I did consider the alternative of a small user-space helper that just reads the map (this is the direction I was heading with my previous commit 9930c26 ALSA: scarlett2: Add support for device map retrieval) and uploads the offsets to the driver, but this would add even more complexity to the driver while gaining none of the possible benefits of a user-space implementation. Since user-space implementation is unavoidable, moving as much functionality there as possible provides the cleanest and most maintainable solution.
Hence, I present for your consideration the following patches:
1) The new FCP driver implementation, handling only the essential kernel-space components (base protocol and level meter)
2) An option enabling Scarlett2-supported devices to use the FCP driver, providing an opt-in migration path forward
The accompanying user-space implementation is available at:
https://github.com/geoffreybennett/fcp-support
Thanks,
Geoffrey.
Geoffrey D. Bennett (2):
ALSA: FCP: Add Focusrite Control Protocol driver
ALSA: scarlett2: Add device_setup option to use FCP driver
MAINTAINERS | 10 +- include/uapi/sound/fcp.h | 59 +++ sound/usb/Makefile | 1 + sound/usb/fcp.c | 883 ++++++++++++++++++++++++++++++++++++ sound/usb/fcp.h | 7 + sound/usb/mixer_quirks.c | 7 + sound/usb/mixer_scarlett2.c | 8 + 7 files changed, 971 insertions(+), 4 deletions(-) create mode 100644 include/uapi/sound/fcp.h create mode 100644 sound/usb/fcp.c create mode 100644 sound/usb/fcp.h
Signature · 1 line
2.45.0
Geoffrey D. Bennett
maintainerPATCH 1/2 ALSA: FCP: Add Focusrite Control Protocol driver
Add a new kernel driver for the Focusrite Control Protocol (FCP), which is used by Focusrite Scarlett 2nd Gen, 3rd Gen, 4th Gen, Clarett USB, Clarett+, and Vocaster series audio interfaces. This driver provides a user-space control interface via ALSA's hwdep subsystem.
Unlike the existing Scarlett2 driver which implements all ALSA controls in kernel space, this new FCP driver takes a different approach by providing a minimal kernel interface that allows a user-space driver to send FCP commands and receive notifications. The only control implemented in kernel space is the Level Meter, since it requires frequent polling of volatile data.
While this driver supports all interfaces that the Scarlett2 driver works with, it is initially enabled only for 4th Gen 16i16, 18i16, and 18i20 interfaces that are not supported by the Scarlett2 driver.
--- MAINTAINERS | 10 +- include/uapi/sound/fcp.h | 59 +++ sound/usb/Makefile | 1 + sound/usb/fcp.c | 883 +++++++++++++++++++++++++++++++++++++++ sound/usb/fcp.h | 7 + sound/usb/mixer_quirks.c | 7 + 6 files changed, 963 insertions(+), 4 deletions(-) create mode 100644 include/uapi/sound/fcp.h create mode 100644 sound/usb/fcp.c create mode 100644 sound/usb/fcp.h
MAINTAINERS +6 -4
@@ -8868,14 +8868,16 @@ L: [email protected] S: Maintained F: drivers/input/joystick/fsia6b.c -FOCUSRITE SCARLETT2 MIXER DRIVER (Scarlett Gen 2+ and Clarett) +FOCUSRITE CONTROL PROTOCOL/SCARLETT2 MIXER DRIVERS (Scarlett Gen 2+, Clarett, and Vocaster) M: Geoffrey D. Bennett <[email protected]> L: [email protected] S: Maintained -W: https://github.com/geoffreybennett/scarlett-gen2 -B: https://github.com/geoffreybennett/scarlett-gen2/issues -T: git https://github.com/geoffreybennett/scarlett-gen2.git +W: https://github.com/geoffreybennett/linux-fcp +B: https://github.com/geoffreybennett/linux-fcp/issues +T: git https://github.com/geoffreybennett/linux-fcp.git +F: include/uapi/sound/fcp.h F: include/uapi/sound/scarlett2.h +F: sound/usb/fcp.c F: sound/usb/mixer_scarlett2.c FORCEDETH GIGABIT ETHERNET DRIVER
include/uapi/sound/fcp.h new file +59 -0
59 lines not on this page · raw message
sound/usb/Makefile +1 -0
@@ -6,6 +6,7 @@ snd-usb-audio-y := card.o \ clock.o \ endpoint.o \ + fcp.o \ format.o \ helper.o \ implicit.o \
sound/usb/fcp.c new file +883 -0
883 lines not on this page · raw message
sound/usb/fcp.h new file +7 -0
@@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __USBAUDIO_FCP_H +#define __USBAUDIO_FCP_H + +int snd_fcp_init (struct usb_mixer_interface *mixer); + +#endif /* __USBAUDIO_FCP_H */
sound/usb/mixer_quirks.c +7 -0
@@ -38,6 +38,7 @@#include "mixer_us16x08.h" #include "mixer_s1810c.h" #include "helper.h" +#include "fcp.h" struct std_mono_table {unsigned int unitid, control, cmask;
@@ -4033,6 +4034,12 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) err =snd_scarlett2_init (mixer);break ; +case USB_ID (0x1235 ,0x821b ):/* Focusrite Scarlett 16i16 4th Gen */ +case USB_ID (0x1235 ,0x821c ):/* Focusrite Scarlett 18i16 4th Gen */ +case USB_ID (0x1235 ,0x821d ):/* Focusrite Scarlett 18i20 4th Gen */ + err =snd_fcp_init (mixer); +break ; +case USB_ID (0x041e ,0x323b ):/* Creative Sound Blaster E1 */ err =snd_soundblaster_e1_switch_create (mixer);break ;
Signature · 1 line
2.45.0
Re: [PATCH 1/2] ALSA: FCP: Add Focusrite Control Protocol driver
Quoted 33 lines · Geoffrey D. Bennett
include/uapi/sound/fcp.h new file +29
-0@@ -0,0 +1,59 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* +* Focusrite Control Protocol Driver for ALSA +* +* Copyright (c) 2024 by Geoffrey D. Bennett <g at b4.vu> +*/ +#ifndef __UAPI_SOUND_FCP_H +#define __UAPI_SOUND_FCP_H + +#ifdef __KERNEL__ +#include <linux/types.h> +#else +#include <stdint.h> +#endif + +#include <linux/ioctl.h> + +#define FCP_HWDEP_MAJOR 2 +#define FCP_HWDEP_MINOR 0 +#define FCP_HWDEP_SUBMINOR 0 + +#define FCP_HWDEP_VERSION \ + ((FCP_HWDEP_MAJOR << 16) | \ + (FCP_HWDEP_MINOR << 8) | \ + FCP_HWDEP_SUBMINOR) + +#define FCP_HWDEP_VERSION_MAJOR (v) (((v) >> 16) & 0xFF) +#define FCP_HWDEP_VERSION_MINOR (v) (((v) >> 8) & 0xFF) +#define FCP_HWDEP_VERSION_SUBMINOR (v) ((v) & 0xFF)
include/uapi/sound/fcp.h line 39
Quoted 10 lines
include/uapi/sound/fcp.h new file +10
-0+ +/* Get protocol version */ +#define FCP_IOCTL_PVERSION _IOR('S', 0x60, int) + +/* Do FCP step 0 */ +struct fcp_step0 { +void *data; +uint16_t size; +}; +#define FCP_IOCTL_INIT _IOWR('S', 0x64, struct fcp_step0)
An ioctl with a pointer has to be handled carefully because you must convert it for 32bit compat code. IOW, it's better to be avoided if possible.
The same is true for other ioctls.
Quoted 2 lines
sound/usb/fcp.c new file +0
-0
(snip)
sound/usb/fcp.c line 42
Quoted 13 lines
sound/usb/fcp.c new file +13
-0+struct fcp_data { +struct usb_mixer_interface *mixer; + +struct mutex mutex;/* serialise access to the device */ +struct completion cmd_done;/* wait for command completion */ +struct file *file;/* hwdep file */ + +/* notify waiting to send to *file */ +struct { +wait_queue_head_t queue; +u32 event; +spinlock_t lock; + } notify;
Might be better to define outside with an explicit type name in case of code simplification (e.g. factoring out the struct init code, etc).
sound/usb/fcp.c line 47
Quoted 4 lines
sound/usb/fcp.c new file +4
-0+__u8 bInterfaceNumber; +__u8 bEndpointAddress; +__u16 wMaxPacketSize; +__u8 bInterval;
This is an internal struct and should be types without __ prefix.
Quoted 8 lines
sound/usb/fcp.c new file +8
-0+/* Send an FCP command and get the response */ +static int fcp_usb (struct usb_mixer_interface *mixer,u32 opcode, +const void *req_data,u16 req_size, +void *resp_data,u16 resp_size) +{ +struct fcp_data *private = mixer->private_data; +struct usb_device *dev = mixer->chip->dev; +struct fcp_usb_packet *req, *resp =NULL ;
sound/usb/fcp.c line 136
Quoted 10 lines
sound/usb/fcp.c new file +10
-0+size_t req_buf_size =struct_size (req, data, req_size); +size_t resp_buf_size =struct_size (resp, data, resp_size); +int retries =0 ; +const int max_retries =5 ; +int err; + + req =kmalloc (req_buf_size,GFP_KERNEL ); +if (!req) { + err = -ENOMEM ; + goto done;
You can use cleanup.h stuff for code simplification. (Also for other possible functions.)
Quoted 23 lines
sound/usb/fcp.c new file +23
-0+static int fcp_hwdep_ioctl (struct snd_hwdep *hw,struct file *file, +unsigned int cmd,unsigned long arg) +{ +struct usb_mixer_interface *mixer = hw->private_data; +struct fcp_data *private = mixer->private_data; +int err; + +mutex_lock (&private->mutex); + +switch (cmd) { + +case FCP_IOCTL_PVERSION : + err =put_user (FCP_HWDEP_VERSION , + (int __user *)arg) ? -EFAULT :0 ; +break ; + +case FCP_IOCTL_INIT : + err =fcp_ioctl_init (mixer, arg); +break ; + +case FCP_IOCTL_CMD : + err =fcp_ioctl_cmd (mixer, arg); +break ;
sound/usb/fcp.c line 698
Quoted 10 lines
sound/usb/fcp.c new file +10
-0+ +case FCP_IOCTL_SET_METER_MAP : + err =fcp_ioctl_set_meter_map (mixer, arg); +break ; + +default : + err = -ENOIOCTLCMD ; + } + +mutex_unlock (&private->mutex);
There seems an implicit ordering of the command -- FCP_IOCTL_INIT must be called before *_CMD? If so, do we have a sanity check for that?
Also, the behavior of *_SET_METER_MAP -- it dynamically creates and deletes the meter kcontrol -- can be a bit dangerous. Can't it be without recreating the kcontrol itself?
Last but not least, any need for suspend/resume and disconnect handling?
thanks,
Takashi
Geoffrey D. Bennett
maintainerPATCH 2/2 ALSA: scarlett2: Add device_setup option to use FCP driver
In reply to Geoffrey D. BennettAdd a new device_setup option (SCARLETT2_USE_FCP_DRIVER = 0x08) that allows users to opt in to using the new FCP driver instead of the existing scarlett2 driver for their device. This provides a way to test the new FCP driver on existing supported hardware while keeping the Scarlett2 driver as the default.
When the SCARLETT2_USE_FCP_DRIVER bit is set in device_setup, the scarlett2 driver initialisation will hand off to the FCP driver instead of proceeding with its own initialisation. The FCP driver then provides access to the device via its hwdep interface.
--- sound/usb/mixer_scarlett2.c | 8 ++++++++ 1 file changed, 8 insertions(+)
sound/usb/mixer_scarlett2.c +8 -0
@@ -166,6 +166,7 @@#include "helper.h" #include "mixer_scarlett2.h" +#include "fcp.h" /* device_setup value to allow turning MSD mode back on */ #define SCARLETT2_MSD_ENABLE 0x02
@@ -173,6 +174,9 @@/* device_setup value to disable this mixer driver */ #define SCARLETT2_DISABLE 0x04 +/* device_setup value to use the FCP driver instead */ +#define SCARLETT2_USE_FCP_DRIVER 0x08 +/* some gui mixers can't handle negative ctl values */ #define SCARLETT2_VOLUME_BIAS 127
@@ -9702,6 +9706,10 @@ int snd_scarlett2_init(struct usb_mixer_interface *mixer)if (!mixer->protocol)return 0 ; +/* check if the user wants to use the FCP driver instead */ +if (chip->setup &SCARLETT2_USE_FCP_DRIVER ) +return snd_fcp_init (mixer); +/* find entry in scarlett2_devices */ entry =get_scarlett2_device_entry (mixer);if (!entry) {
Signature · 1 line
2.45.0
Quoted 72 lines · Geoffrey D. Bennett
Hi Takashi,
Merry Christmas!
This patch series introduces a new Focusrite Control Protocol (FCP) driver that provides equivalent functionality to the existing Scarlett2 driver, but splits the implementation between a minimal kernel driver and a user-space component. My initial user-space implementation supports the new Scarlett 4th Gen 16i16, 18i16, and 18i20 interfaces, but the new FCP kernel driver can be used with any device supported by the Scarlett2 driver (as an opt-in).
With these new interfaces, I started out by trying to extend the existing Scarlett2 driver but found that that wasn't practical. The control locations (struct
scarlett2_configoffset field) have previously varied between firmware versions (which is why I added the "Minimum Firmware Version" requirement for previous 4th Gen and Vocaster devices), and the situation was worse with the new 4th Gen devices, with every new firmware version moving the controls.While the device provides a map indicating control locations, implementing the required JSON parsing in the kernel isn't feasible, necessitating a user-space implementation.
The new approach in the FCP driver significantly improves both maintainability and paves the way for future enhancements:
Firstly, the kernel component of the FCP driver is <10% the size of the Scarlett2 driver, with the obvious maintainability advantages; cloc reports:
File blank comment code
sound/usb/mixer_scarlett2.c1765 852 7152sound/usb/fcp.c174 64 645Secondly, changes that aren't feasible to implement in the kernel due to backward compatibility constraints will be able to be implemented in user-space, with end-users easily able to choose an older version of the controls if needed. For example:
1) The largest interface has >500 ALSA controls just for the matrix mixer (one control for each entry in the N×M matrix). This would be better done as N controls with M values each, especially since the device supports writing a row (all input gains for one mixer output) at a time.
2) Similarly, the mux table entries have a control per entry, but they're all written at once, so it would be better to have a single ALSA control for the whole mux table.
3) The interfaces have 3 independent mux tables (one per sample rate 48kHz, 96kHz, 192kHz), but supporting them isn't feasible in Scarlett2 due to backward compatibility constraints.
4) Users of the Scarlett2 driver frequently request higher-level controls like stereo-linking, pan, mute, and solo. These would be very challenging to implement in the kernel while maintaining backward compatibility to the existing controls, but again, it's easy to have configurable controls in user-space.
I did consider the alternative of a small user-space helper that just reads the map (this is the direction I was heading with my previous commit 9930c26 ALSA: scarlett2: Add support for device map retrieval) and uploads the offsets to the driver, but this would add even more complexity to the driver while gaining none of the possible benefits of a user-space implementation. Since user-space implementation is unavoidable, moving as much functionality there as possible provides the cleanest and most maintainable solution.
Hence, I present for your consideration the following patches:
Quoted 8 lines
1) The new FCP driver implementation, handling only the essential kernel-space components (base protocol and level meter)
2) An option enabling Scarlett2-supported devices to use the FCP driver, providing an opt-in migration path forward
The accompanying user-space implementation is available at:
https://github.com/geoffreybennett/fcp-support
Thanks for the patches. The argument about kcontrol sizes sounds convincing to have a more flexible implementation with hwdep. As far as the backward compatibility is kept, I find it's OK.
But, the implementations need more consideration.
I'll comment on each patch.
thanks,
Takashi