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
