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.
