[PATCH RFC] ALSA: scarlett2: Add ioctls for user-space access
In order to support functions such as firmware upgrade from user-space, add ioctls for submitting arbitrary proprietary requests through scarlett2_usb() and requesting/releasing exclusive access. ---
Hi Takashi,
I recently figured how to update the firmware on Scarlett Gen 2+ devices. I think the best way to implement this is with an ioctl giving access to the scarlett2_usb() function from user-space, plus two ioctls to request/release exclusive access.
Does something like this seem reasonable?
What ioctl magic/command values should I use?
Should lock/unlock be separate, or one ioctl takes an argument for which operation to do?
Thanks, Geoffrey.
MAINTAINERS | 1 + include/uapi/sound/scarlett_gen2.h | 28 +++++++++ sound/usb/mixer_scarlett_gen2.c | 94 +++++++++++++++++++++++++++++- 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 include/uapi/sound/scarlett_gen2.h
diff --git a/MAINTAINERS b/MAINTAINERS index 4cc6bf79fdd8..25e5e40f7118 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8066,6 +8066,7 @@ M: Geoffrey D. Bennett g@b4.vu L: alsa-devel@alsa-project.org (moderated for non-subscribers) S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git +F: include/uapi/sound/scarlett_gen2.h F: sound/usb/mixer_scarlett_gen2.c
FORCEDETH GIGABIT ETHERNET DRIVER diff --git a/include/uapi/sound/scarlett_gen2.h b/include/uapi/sound/scarlett_gen2.h new file mode 100644 index 000000000000..0b51a9754ba2 --- /dev/null +++ b/include/uapi/sound/scarlett_gen2.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* + * Focusrite Scarlett Gen 2/3 and Clarett USB/Clarett+ Driver for ALSA + * + * Copyright (c) 2023 by Geoffrey D. Bennett <g at b4.vu> + */ +#ifndef __UAPI_SOUND_SCARLETT_GEN2_H +#define __UAPI_SOUND_SCARLETT_GEN2_H + +#include <linux/types.h> +#include <linux/ioctl.h> + +/* Send a proprietary format request to the interface */ +struct snd_scarlett_gen2_usb_cmd { + u32 cmd; + void *req_data; + u16 req_size; + void *resp_data; + u16 resp_size; +}; + +#define SCARLETT2_IOCTL_USB_CMD _IOWR('S', 0x60, struct snd_scarlett_gen2_usb_cmd) + +/* Request/release exclusive access */ +#define SCARLETT2_IOCTL_LOCK _IO('S', 0x61) +#define SCARLETT2_IOCTL_UNLOCK _IO('S', 0x62) + +#endif /* __UAPI_SOUND_SCARLETT_GEN2_H */ diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c index ffd398f26d2c..f1ef87b54813 100644 --- a/sound/usb/mixer_scarlett_gen2.c +++ b/sound/usb/mixer_scarlett_gen2.c @@ -144,6 +144,9 @@
#include <sound/control.h> #include <sound/tlv.h> +#include <sound/hwdep.h> + +#include <uapi/sound/scarlett_gen2.h>
#include "usbaudio.h" #include "mixer.h" @@ -4262,10 +4265,84 @@ static int snd_scarlett_gen2_controls_create( return 0; }
+static int scarlett_gen2_ioctl_usb_cmd(struct snd_hwdep *hw, unsigned long arg) +{ + struct scarlett2_data *private = hw->private_data; + struct snd_scarlett_gen2_usb_cmd usb_cmd; + int err; + void *req_data = NULL; + void *resp_data = NULL; + + // get cmd & req/resp buffers + if (copy_from_user(&usb_cmd, (void __user *)arg, sizeof(usb_cmd))) + return -EFAULT; + + // allocate request buffer, copy data from user + if (usb_cmd.req_size > 0) { + req_data = kmalloc(usb_cmd.req_size, GFP_KERNEL); + if (!req_data) { + err = -ENOMEM; + goto exit; + } + if (copy_from_user(req_data, usb_cmd.req_data, + usb_cmd.req_size)) { + err = -EFAULT; + goto exit; + } + } + + // allocate response buffer + if (usb_cmd.resp_size > 0) { + resp_data = kmalloc(usb_cmd.resp_size, GFP_KERNEL); + if (!resp_data) { + err = -ENOMEM; + goto exit; + } + } + + // send request, get response + err = scarlett2_usb(private->mixer, usb_cmd.cmd, + req_data, usb_cmd.req_size, + resp_data, usb_cmd.resp_size); + if (err < 0) + goto exit; + + // copy response to user + if (usb_cmd.resp_size > 0) + if (copy_to_user(usb_cmd.resp_data, resp_data, + usb_cmd.resp_size)) + err = -EFAULT; + +exit: + kfree(req_data); + kfree(resp_data); + + return err; +} + +static int scarlett_gen2_hwdep_ioctl(struct snd_hwdep *hw, struct file *file, + unsigned int cmd, unsigned long arg) +{ + switch (cmd) { + + case SCARLETT2_IOCTL_USB_CMD: + return scarlett_gen2_ioctl_usb_cmd(hw, arg); + + // TODO + case SCARLETT2_IOCTL_LOCK: + case SCARLETT2_IOCTL_UNLOCK: + return -EINVAL; + + default: + return -ENOIOCTLCMD; + } +} + int snd_scarlett_gen2_init(struct usb_mixer_interface *mixer) { struct snd_usb_audio *chip = mixer->chip; const struct scarlett2_device_entry *entry; + struct snd_hwdep *hw; int err;
/* only use UAC_VERSION_2 */ @@ -4302,11 +4379,24 @@ int snd_scarlett_gen2_init(struct usb_mixer_interface *mixer) USB_ID_PRODUCT(chip->usb_id));
err = snd_scarlett_gen2_controls_create(mixer, entry); - if (err < 0) + if (err < 0) { usb_audio_err(mixer->chip, "Error initialising %s Mixer Driver: %d", entry->series_name, err); + return err; + } + + err = snd_hwdep_new(mixer->chip->card, "Focusrite Control", 0, &hw); + if (err < 0) { + usb_audio_err(mixer->chip, + "Error creating hwdep device: %d", + err); + return err; + }
- return err; + hw->private_data = mixer->private_data; + hw->ops.ioctl = scarlett_gen2_hwdep_ioctl; + + return 0; }
On 14. 10. 23 15:58, Geoffrey D. Bennett wrote:
In order to support functions such as firmware upgrade from user-space, add ioctls for submitting arbitrary proprietary requests through scarlett2_usb() and requesting/releasing exclusive access.
Hi Takashi,
I recently figured how to update the firmware on Scarlett Gen 2+ devices. I think the best way to implement this is with an ioctl giving access to the scarlett2_usb() function from user-space, plus two ioctls to request/release exclusive access.
Does something like this seem reasonable?
Maybe you can use libusb for this job without an additional kernel interface. It allows to detach the USB kernel driver and attach it again when the job is complete.
Jaroslav
On Mon, Oct 16, 2023 at 09:04:21AM +0200, Jaroslav Kysela wrote:
On 14. 10. 23 15:58, Geoffrey D. Bennett wrote:
In order to support functions such as firmware upgrade from user-space, add ioctls for submitting arbitrary proprietary requests through scarlett2_usb() and requesting/releasing exclusive access.
Hi Takashi,
I recently figured how to update the firmware on Scarlett Gen 2+ devices. I think the best way to implement this is with an ioctl giving access to the scarlett2_usb() function from user-space, plus two ioctls to request/release exclusive access.
Does something like this seem reasonable?
Maybe you can use libusb for this job without an additional kernel interface. It allows to detach the USB kernel driver and attach it again when the job is complete.
Hi Jaroslav,
I considered using libusb (I used it during initial development of the driver), and if the only purpose of the ioctl would be for firmware updates then it would be reasonable to detach the kernel driver for that. However...
Beyond just being able to do firmware operations, that ioctl would also allow access to all of the configuration space using cmd = SCARLETT2_USB_GET_DATA and SCARLETT2_USB_SET_DATA. I think this would be the cleanest way to allow implementing non-mixer related functionality in user-space, such as reading the current firmware version, reading/updating the device name and channel names, and updating the software configuration space for Focusrite Control compatibility to name a few. These sorts of applications need to be able to make these proprietary requests through the scarlett2 driver to avoid disrupting it (or disrupting audio).
Regards, Geoffrey.
On 16. 10. 23 14:32, Geoffrey D. Bennett wrote:
On Mon, Oct 16, 2023 at 09:04:21AM +0200, Jaroslav Kysela wrote:
On 14. 10. 23 15:58, Geoffrey D. Bennett wrote:
In order to support functions such as firmware upgrade from user-space, add ioctls for submitting arbitrary proprietary requests through scarlett2_usb() and requesting/releasing exclusive access.
Hi Takashi,
I recently figured how to update the firmware on Scarlett Gen 2+ devices. I think the best way to implement this is with an ioctl giving access to the scarlett2_usb() function from user-space, plus two ioctls to request/release exclusive access.
Does something like this seem reasonable?
Maybe you can use libusb for this job without an additional kernel interface. It allows to detach the USB kernel driver and attach it again when the job is complete.
Hi Jaroslav,
I considered using libusb (I used it during initial development of the driver), and if the only purpose of the ioctl would be for firmware updates then it would be reasonable to detach the kernel driver for that. However...
Beyond just being able to do firmware operations, that ioctl would also allow access to all of the configuration space using cmd = SCARLETT2_USB_GET_DATA and SCARLETT2_USB_SET_DATA. I think this would be the cleanest way to allow implementing non-mixer related functionality in user-space, such as reading the current firmware version, reading/updating the device name and channel names, and updating the software configuration space for Focusrite Control compatibility to name a few. These sorts of applications need to be able to make these proprietary requests through the scarlett2 driver to avoid disrupting it (or disrupting audio).
Thank you for this bigger picture. But except the firmware upgrade, all those functions seem to be implementable in a more abstract way using standard control API. Note that we can assign the controls also to card (e.g. SNDRV_CTL_ELEM_IFACE_CARD) to classify them as non-mixer.
Jaroslav
On Mon, Oct 16, 2023 at 03:28:11PM +0200, Jaroslav Kysela wrote:
On 16. 10. 23 14:32, Geoffrey D. Bennett wrote:
On Mon, Oct 16, 2023 at 09:04:21AM +0200, Jaroslav Kysela wrote:
On 14. 10. 23 15:58, Geoffrey D. Bennett wrote:
In order to support functions such as firmware upgrade from user-space, add ioctls for submitting arbitrary proprietary requests through scarlett2_usb() and requesting/releasing exclusive access.
Hi Takashi,
I recently figured how to update the firmware on Scarlett Gen 2+ devices. I think the best way to implement this is with an ioctl giving access to the scarlett2_usb() function from user-space, plus two ioctls to request/release exclusive access.
Does something like this seem reasonable?
Maybe you can use libusb for this job without an additional kernel interface. It allows to detach the USB kernel driver and attach it again when the job is complete.
Hi Jaroslav,
I considered using libusb (I used it during initial development of the driver), and if the only purpose of the ioctl would be for firmware updates then it would be reasonable to detach the kernel driver for that. However...
Beyond just being able to do firmware operations, that ioctl would also allow access to all of the configuration space using cmd = SCARLETT2_USB_GET_DATA and SCARLETT2_USB_SET_DATA. I think this would be the cleanest way to allow implementing non-mixer related functionality in user-space, such as reading the current firmware version, reading/updating the device name and channel names, and updating the software configuration space for Focusrite Control compatibility to name a few. These sorts of applications need to be able to make these proprietary requests through the scarlett2 driver to avoid disrupting it (or disrupting audio).
Thank you for this bigger picture. But except the firmware upgrade, all those functions seem to be implementable in a more abstract way using standard control API. Note that we can assign the controls also to card (e.g. SNDRV_CTL_ELEM_IFACE_CARD) to classify them as non-mixer.
Hi Jaroslav,
By "more abstract way", you mean to have a control for every parameter which could be read or written? I can see that working for things like firmware version, device name, and channel name, but I think it would be pretty awful for the software configuration space that Focusrite Control uses, and bloat the driver quite a bit for what seems to me to be something more suited to user-land implementation.
(an aside, will alsactl store/restore SNDRV_CTL_ELEM_TYPE_BYTES?)
Or am I misunderstanding and you mean there is already a way (like SNDRV_CTL_IOCTL_TLV_COMMAND?) to send commands & get responses?
By "pretty awful"/bloat, a bit more explanation:
(1) Part of the NVRAM that can be accessed refers to the hardware controls which the driver allows the user to read/write (such as dim/mute/volume/level/pad/air/phantom/etc.)
(2) Part of the NVRAM is used by the Focusrite Control software to store the state of the interface in a higher-level way. It does not support all features of the hardware (e.g. routing is quite restricted).
It's not possible to represent all functions of (1) inside (2), so when I developed the driver I ignored (2) and implemented all features of (1). It doesn't make sense to implement (2) in the kernel as that would double the number of controls, but what would make sense would be a user-space application that implements read/write of (2) with a UI that restricts the user to what can be represented in (2).
For example: in (1), routing is all mono vs. in (2) routing is stereo-only, channels can be paired together, and there are balance/pan controls. I feel strongly this is the sort of thing where the kernel provides the low-level (1) hardware interface, and a user-space app provides a higher-level (2) interface.
It'd be nice if the app could store this data on the device itself like Focusrite Control does. And perhaps it could even do this in a non-Focusrite Control compatible way (for additional functionality when compatibility is not desired). But those options are not feasible if there is no access to read/write NVRAM arbitrarily.
That's why I came up with the proposed ioctl interface to the scarlett2_usb() function. That will allow user-space to access all applicable functions:
- reboot - get flash info - get flash segment info - erase flash segment - get erase segment progress - write flash segment - read NVRAM - write NVRAM
through a common interface, without disconnecting the kernel driver, and without adding specific support for a bunch of things that are not applicable to the hardware controls (1).
Thanks, Geoffrey.
On Mon, 16 Oct 2023 17:45:54 +0200, Geoffrey D. Bennett wrote:
On Mon, Oct 16, 2023 at 03:28:11PM +0200, Jaroslav Kysela wrote:
On 16. 10. 23 14:32, Geoffrey D. Bennett wrote:
On Mon, Oct 16, 2023 at 09:04:21AM +0200, Jaroslav Kysela wrote:
On 14. 10. 23 15:58, Geoffrey D. Bennett wrote:
In order to support functions such as firmware upgrade from user-space, add ioctls for submitting arbitrary proprietary requests through scarlett2_usb() and requesting/releasing exclusive access.
Hi Takashi,
I recently figured how to update the firmware on Scarlett Gen 2+ devices. I think the best way to implement this is with an ioctl giving access to the scarlett2_usb() function from user-space, plus two ioctls to request/release exclusive access.
Does something like this seem reasonable?
Maybe you can use libusb for this job without an additional kernel interface. It allows to detach the USB kernel driver and attach it again when the job is complete.
Hi Jaroslav,
I considered using libusb (I used it during initial development of the driver), and if the only purpose of the ioctl would be for firmware updates then it would be reasonable to detach the kernel driver for that. However...
Beyond just being able to do firmware operations, that ioctl would also allow access to all of the configuration space using cmd = SCARLETT2_USB_GET_DATA and SCARLETT2_USB_SET_DATA. I think this would be the cleanest way to allow implementing non-mixer related functionality in user-space, such as reading the current firmware version, reading/updating the device name and channel names, and updating the software configuration space for Focusrite Control compatibility to name a few. These sorts of applications need to be able to make these proprietary requests through the scarlett2 driver to avoid disrupting it (or disrupting audio).
Thank you for this bigger picture. But except the firmware upgrade, all those functions seem to be implementable in a more abstract way using standard control API. Note that we can assign the controls also to card (e.g. SNDRV_CTL_ELEM_IFACE_CARD) to classify them as non-mixer.
Hi Jaroslav,
By "more abstract way", you mean to have a control for every parameter which could be read or written? I can see that working for things like firmware version, device name, and channel name, but I think it would be pretty awful for the software configuration space that Focusrite Control uses, and bloat the driver quite a bit for what seems to me to be something more suited to user-land implementation.
(an aside, will alsactl store/restore SNDRV_CTL_ELEM_TYPE_BYTES?)
Or am I misunderstanding and you mean there is already a way (like SNDRV_CTL_IOCTL_TLV_COMMAND?) to send commands & get responses?
By "pretty awful"/bloat, a bit more explanation:
(1) Part of the NVRAM that can be accessed refers to the hardware controls which the driver allows the user to read/write (such as dim/mute/volume/level/pad/air/phantom/etc.)
(2) Part of the NVRAM is used by the Focusrite Control software to store the state of the interface in a higher-level way. It does not support all features of the hardware (e.g. routing is quite restricted).
It's not possible to represent all functions of (1) inside (2), so when I developed the driver I ignored (2) and implemented all features of (1). It doesn't make sense to implement (2) in the kernel as that would double the number of controls, but what would make sense would be a user-space application that implements read/write of (2) with a UI that restricts the user to what can be represented in (2).
For example: in (1), routing is all mono vs. in (2) routing is stereo-only, channels can be paired together, and there are balance/pan controls. I feel strongly this is the sort of thing where the kernel provides the low-level (1) hardware interface, and a user-space app provides a higher-level (2) interface.
It'd be nice if the app could store this data on the device itself like Focusrite Control does. And perhaps it could even do this in a non-Focusrite Control compatible way (for additional functionality when compatibility is not desired). But those options are not feasible if there is no access to read/write NVRAM arbitrarily.
That's why I came up with the proposed ioctl interface to the scarlett2_usb() function. That will allow user-space to access all applicable functions:
- reboot
- get flash info
- get flash segment info
- erase flash segment
- get erase segment progress
- write flash segment
- read NVRAM
- write NVRAM
through a common interface, without disconnecting the kernel driver, and without adding specific support for a bunch of things that are not applicable to the hardware controls (1).
I caught a flu and am still in sick leave since the last week, so this is just a short followup from my side.
First off, I don't think it appropriate to expose a generic register access via (hwdep) ioctl as your RFC patch. Then it allows to screw up the hardware too easily by sending random bytes. It may result in a severe defect, too.
If you have some proper "features" to be implemented as the driver functionality, they can be provided via ioctl, though; but then each ioctl must serve for only a single purpose.
OTOH, if you need a handy register access for debugging, providing a debugfs interface could be a solution, too. It's safer than ioctl (as it's not allowed for every user unlike ioctl), and it can be disabled on a production system.
thanks,
Takashi
Hi Jaroslav, Takashi,
I took your feedback onboard about not providing generic access to the scarlett2_usb() function from user-space.
After a few iterations, I've come up with this hwdep interface to support reset-to-factory-defaults, reset-to-factory-firmware, and firmware-update in a safe way:
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
/* Get protocol version */ #define SCARLETT2_IOCTL_PVERSION _IOR('S', 0x60, int)
/* Reboot */ #define SCARLETT2_IOCTL_REBOOT _IO('S', 0x61)
/* Select flash segment */ #define SCARLETT2_SEGMENT_ID_SETTINGS 0 #define SCARLETT2_SEGMENT_ID_FIRMWARE 1 #define SCARLETT2_SEGMENT_ID_COUNT 2
#define SCARLETT2_IOCTL_SELECT_FLASH_SEGMENT _IOW('S', 0x62, int)
/* Erase selected flash segment */ #define SCARLETT2_IOCTL_ERASE_FLASH_SEGMENT _IO('S', 0x63)
/* Get selected flash segment erase progress * 1 through to num_blocks, or 255 for complete */ struct scarlett2_flash_segment_erase_progress { unsigned char progress; unsigned char num_blocks; }; #define SCARLETT2_IOCTL_GET_ERASE_PROGRESS \ _IOR('S', 0x64, struct scarlett2_flash_segment_erase_progress)
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
Does that look reasonable to you?
Broadly, it's used like this:
Reset to factory default configuration:
- ioctl select_flash_segment SCARLETT2_SEGMENT_ID_SETTINGS - ioctl erase_flash_segment - ioctl get_erase_progress (optional)
Erase firmware (reverts to factory firmware which is stored in a different flash segment, inaccessible from these ioctls):
- ioctl select_flash_segment SCARLETT2_SEGMENT_ID_FIRMWARE - ioctl erase_flash_segment - ioctl get_erase_progress (optional)
Upload new firmware:
- write() <- a bunch of these, only permitted after the previous erase step was completed
On completion:
- ioctl reboot
To confirm that this interface is sufficient, I have implemented it in the scarlett2 driver and written a user-space utility which can perform all the above operations.
I will clean up the implementation a bit and then submit for review; just wanted to share the interface first in case you have any comments at this point.
Thanks, Geoffrey.
On Sun, 19 Nov 2023 18:35:27 +0100, Geoffrey D. Bennett wrote:
Hi Jaroslav, Takashi,
I took your feedback onboard about not providing generic access to the scarlett2_usb() function from user-space.
After a few iterations, I've come up with this hwdep interface to support reset-to-factory-defaults, reset-to-factory-firmware, and firmware-update in a safe way:
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
/* Get protocol version */ #define SCARLETT2_IOCTL_PVERSION _IOR('S', 0x60, int)
/* Reboot */ #define SCARLETT2_IOCTL_REBOOT _IO('S', 0x61)
/* Select flash segment */ #define SCARLETT2_SEGMENT_ID_SETTINGS 0 #define SCARLETT2_SEGMENT_ID_FIRMWARE 1 #define SCARLETT2_SEGMENT_ID_COUNT 2
#define SCARLETT2_IOCTL_SELECT_FLASH_SEGMENT _IOW('S', 0x62, int)
/* Erase selected flash segment */ #define SCARLETT2_IOCTL_ERASE_FLASH_SEGMENT _IO('S', 0x63)
/* Get selected flash segment erase progress
- 1 through to num_blocks, or 255 for complete
*/ struct scarlett2_flash_segment_erase_progress { unsigned char progress; unsigned char num_blocks; }; #define SCARLETT2_IOCTL_GET_ERASE_PROGRESS \ _IOR('S', 0x64, struct scarlett2_flash_segment_erase_progress)
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
Does that look reasonable to you?
Broadly, it's used like this:
Reset to factory default configuration:
- ioctl select_flash_segment SCARLETT2_SEGMENT_ID_SETTINGS
- ioctl erase_flash_segment
- ioctl get_erase_progress (optional)
So the erase operation is asynchronous? This sounds a bit dangerous. Will the driver block further conflicting operations until the erase finishes?
Erase firmware (reverts to factory firmware which is stored in a different flash segment, inaccessible from these ioctls):
- ioctl select_flash_segment SCARLETT2_SEGMENT_ID_FIRMWARE
- ioctl erase_flash_segment
- ioctl get_erase_progress (optional)
Upload new firmware:
- write() <- a bunch of these, only permitted after the previous erase step was completed
The write op must accept partial writes, and it becomes cumbersome. Can it be a one-shot ioctl, too?
On completion:
- ioctl reboot
To confirm that this interface is sufficient, I have implemented it in the scarlett2 driver and written a user-space utility which can perform all the above operations.
I will clean up the implementation a bit and then submit for review; just wanted to share the interface first in case you have any comments at this point.
IMO, from the user POV, it's easier to have per-purpose ioctls, instead of combining multiple ioctl sequences. Of course, it won't scale too much, but for the limited number of operations, it's clearer.
That is, we can provide just a few ioctls for reset-to-factory, reset-to-something-else, and update.
But, if you need asynchronous operations inevitably by some reason, it's a different story, though.
thanks,
Takashi
Hi Takashi,
On Fri, Nov 24, 2023 at 02:39:26PM +0100, Takashi Iwai wrote:
On Sun, 19 Nov 2023 18:35:27 +0100, Geoffrey D. Bennett wrote:
Hi Jaroslav, Takashi,
I took your feedback onboard about not providing generic access to the scarlett2_usb() function from user-space.
After a few iterations, I've come up with this hwdep interface to support reset-to-factory-defaults, reset-to-factory-firmware, and firmware-update in a safe way:
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
/* Get protocol version */ #define SCARLETT2_IOCTL_PVERSION _IOR('S', 0x60, int)
/* Reboot */ #define SCARLETT2_IOCTL_REBOOT _IO('S', 0x61)
/* Select flash segment */ #define SCARLETT2_SEGMENT_ID_SETTINGS 0 #define SCARLETT2_SEGMENT_ID_FIRMWARE 1 #define SCARLETT2_SEGMENT_ID_COUNT 2
#define SCARLETT2_IOCTL_SELECT_FLASH_SEGMENT _IOW('S', 0x62, int)
/* Erase selected flash segment */ #define SCARLETT2_IOCTL_ERASE_FLASH_SEGMENT _IO('S', 0x63)
/* Get selected flash segment erase progress
- 1 through to num_blocks, or 255 for complete
*/ struct scarlett2_flash_segment_erase_progress { unsigned char progress; unsigned char num_blocks; }; #define SCARLETT2_IOCTL_GET_ERASE_PROGRESS \ _IOR('S', 0x64, struct scarlett2_flash_segment_erase_progress)
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
Does that look reasonable to you?
Broadly, it's used like this:
Reset to factory default configuration:
- ioctl select_flash_segment SCARLETT2_SEGMENT_ID_SETTINGS
- ioctl erase_flash_segment
- ioctl get_erase_progress (optional)
So the erase operation is asynchronous? This sounds a bit dangerous. Will the driver block further conflicting operations until the erase finishes?
Yes it is asynchronous. I've made it so that it's not dangerous by locking out any conflicting operations: - Mixer operations that require device access return EBUSY - The hwdep is marked as exclusive so other processes can't use it - Subsequent hwdep operations (if get_erase_progress wasn't called) will block until the erase is complete
Erase firmware (reverts to factory firmware which is stored in a different flash segment, inaccessible from these ioctls):
- ioctl select_flash_segment SCARLETT2_SEGMENT_ID_FIRMWARE
- ioctl erase_flash_segment
- ioctl get_erase_progress (optional)
Upload new firmware:
- write() <- a bunch of these, only permitted after the previous erase step was completed
The write op must accept partial writes, and it becomes cumbersome. Can it be a one-shot ioctl, too?
I considered one-shot ioctls, but as the erase & write operations take some seconds, then it is not possible to provide feedback to the end-user while the erase & write operations happen.
On completion:
- ioctl reboot
To confirm that this interface is sufficient, I have implemented it in the scarlett2 driver and written a user-space utility which can perform all the above operations.
I will clean up the implementation a bit and then submit for review; just wanted to share the interface first in case you have any comments at this point.
IMO, from the user POV, it's easier to have per-purpose ioctls, instead of combining multiple ioctl sequences. Of course, it won't scale too much, but for the limited number of operations, it's clearer.
That is, we can provide just a few ioctls for reset-to-factory, reset-to-something-else, and update.
But, if you need asynchronous operations inevitably by some reason, it's a different story, though.
Just to provide progress feedback to the end-user.
I've written the CLI tool using the proposed ioctl interface, and it works nicely:
https://github.com/geoffreybennett/scarlett2
[g@fedora ~]$ time scarlett2 update Selected device Scarlett 4th Gen Solo Found firmware version 2115 for Scarlett 4th Gen Solo: /usr/lib/firmware/scarlett2/scarlett2-1235-8218-2115.bin Updating Scarlett 4th Gen Solo from firmware version 1974 to 2115 Resetting configuration to factory default... Erase progress: Done! Erasing upgrade firmware... Erase progress: Done! Firmware write progress: Done! Rebooting interface...
real 0m5.919s user 0m0.007s sys 0m0.034s
The user experience would not be as nice with one-shot ioctls. And using ioctls which block for a long time would make using them from the GUI https://github.com/geoffreybennett/alsa-scarlett-gui/ rather awkward. None of the other operations on the interface block for an appreciable amount of time.
I've got a first draft of firmware update and Scarlett 4th Gen support that I am sharing with others to test now. It's 48 commits, divided into: - 5 commits to add extra checks that are missing - 5 commits for firmware management - 20 commits refactoring the existing driver to allow Scarlett 4th Gen support to be added - 18 commits adding the support (although the underlying Gen 4 protocol is the same as the other series, there are many new different types of controls)
I've put those commits on this branch: https://github.com/geoffreybennett/scarlett-gen2/tree/scarlett-gen4
Do you want me to share all 48 commits on the mailing list at once? Or maybe just the first 5+5 commits for now and the rest after I get some feedback from others?
Thanks, Geoffrey.
participants (3)
-
Geoffrey D. Bennett -
Jaroslav Kysela -
Takashi Iwai