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
