ALSA: Add driver for big Scarlett 4th Gen interfaces

RFC PATCH Superseded Geoffrey D. Bennett 1 participant 3 messages Dec 29, 2024

Subsystems (2)

Sound Focusrite control protocol/SCARLETT2 mixer drivers (Scarlett Gen 2+, Clarett, and Vocaster)

lore

Share & tools

lore

Geoffrey D. Bennett

author maintainer

2024-12-29 22:42 UTC #0 lore raw

v2 cover ALSA: Add driver for big Scarlett 4th Gen interfaces

Hi Takashi,

Thanks for your feedback on the first version. Appreciate your comments. I implemented fixes for almost all; I've got a couple of questions, and I found one thing to fix.

Quoted 10 lines

Review
Quoted 6 lines
(quoted diff) +6 -0
+/* 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.

For v2, I have changed the structs to have fixed-size data arrays instead of a pointer, e.g.:

/* Do FCP step 0 */
struct fcp_step0 {

-       void     *data;
-       uint16_t  size;
+       uint16_t size;
+       uint8_t  data[FCP_MAX_DATA_SIZE];

};

However, this approach makes struct fcp_cmd over 2KB which seems excessive/a lot of copying for each ioctl call?

https://www.kernel.org/doc/html/v6.12/driver-api/ioctl.html suggests:

Quoted 11 lines

32-bit compat mode
[...]
As long as all the rules for data structures are followed, this is as easy as setting the .compat_ioctl pointer to a helper function such as compat_ptr_ioctl() or blkdev_compat_ptr_ioctl(). [...]
Pointers have the same problem, in addition to requiring the use of compat_ptr(). The best workaround is to use __u64 in place of pointers, which requires a cast to uintptr_t in user space, and the use of u64_to_user_ptr() in the kernel to convert it back into a user pointer.

Between these two options (fixed arrays vs u64_to_user_ptr), which would you recommend? Or is there a third approach you had in mind?

Quoted 4 lines

[...other comments noted and addressed...]
Last but not least, any need for suspend/resume and disconnect handling?

I have tested on a laptop, playing audio, getting meter data 20x per second:

1. suspend then resume
2. suspend, unplug, replug, resume
3. suspend, unplug, power off interface, power on interface, replug, resume

Results from testing:
- Cases 1 and 2: Driver remained connected and functioned correctly
- Case 3: Driver remained connected (to my surprise) but the sequence numbers were out of sync; I will work on a fix for that.

For disconnect handling, I've disconnected the device (software reset, USB disconnect, and power off) during testing countless times, and have not seen any issues. Is there something in particular I should be looking for?

Thanks again for your feedback.

Regards,
Geoffrey.

---
Changes in v2 as per Takashi's feedback:
- Use fixed-size data arrays instead of pointers in ioctl structs
- Define notify struct outside of struct fcp_dev
- Use u8/u16 types without __ prefix
- Use cleanup.h for code simplification
- Add init flag to ensure FCP_IOCTL_INIT is called before FCP_IOCTL_CMD and FCP_IOCTL_SET_METER_MAP
- Do not destroy/recreate the meter control (the number of channels is now fixed when it is created)

---
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    |  65 +++
 sound/usb/Makefile          |   1 +
 sound/usb/fcp.c             | 837 ++++++++++++++++++++++++++++++++++++
 sound/usb/fcp.h             |   7 +
 sound/usb/mixer_quirks.c    |   7 +
 sound/usb/mixer_scarlett2.c |   8 +
 7 files changed, 931 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

author maintainer

2024-12-29 22:43 UTC #1 lore raw

RFC PATCH v2 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.

Signed-off-by Geoffrey D. Bennett <[email protected]>

---
 MAINTAINERS              |  10 +-
 include/uapi/sound/fcp.h |  65 +++
 sound/usb/Makefile       |   1 +
 sound/usb/fcp.c          | 837 +++++++++++++++++++++++++++++++++++++++
 sound/usb/fcp.h          |   7 +
 sound/usb/mixer_quirks.c |   7 +
 6 files changed, 923 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

index 6bb4ec0c162a..2fd0945cb738 100644
@@ -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 +65 -0

65 lines not on this page · raw message

sound/usb/Makefile +1 -0

index 0532499dbc6d..30bd5348477b 100644
@@ -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 +837 -0

837 lines not on this page · raw message

sound/usb/fcp.h new file +7 -0

new file mode 100644
index 000000000000..d58cc7db75b1
@@ -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

index a95ebcf4e46e..150fbb5ac7ed 100644
@@ -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

Geoffrey D. Bennett

author maintainer

2024-12-29 22:43 UTC #2 lore raw

RFC PATCH v2 2/2 ALSA: scarlett2: Add device_setup option to use FCP driver

In reply to Geoffrey D. Bennett

Add 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.

Signed-off-by Geoffrey D. Bennett <[email protected]>

---
 sound/usb/mixer_scarlett2.c | 8 ++++++++
 1 file changed, 8 insertions(+)

sound/usb/mixer_scarlett2.c +8 -0

index 7f595c1752a5..288d22e6a0b2 100644
@@ -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