mbox series

[v8,0/2] ALSA: Add driver for big Scarlett 4th Gen interfaces

Message ID cover.1737048528.git.g@b4.vu (mailing list archive)
Headers show
Series ALSA: Add driver for big Scarlett 4th Gen interfaces | expand

Message

Geoffrey D. Bennett Jan. 16, 2025, 5:47 p.m. UTC
Hi Takashi,

Thanks again! All changes from your feedback on v7 incorporated in v8.

Regards,
Geoffrey.

---
Changes in v8:
- Rename SNDRV_CTL_TLVT_LABELS to SNDRV_CTL_TLVT_FCP_CHANNEL_LABELS
- Make meter levels buffer persistent rather than allocate/free each
  time
- Add data protection to fcp_meter_tlv_callback()
- Set private->step* and co after allocation of the buffer
- Use ALIGN() rather than open-coding it

---
Changes in v7:
- Add #define SNDRV_CTL_TLVT_LABELS for channel labels
- Add ioctl for setting meter labels and remove TLV write support
- Simplify reinit handling
- Remove more unnecessary casts
- Add #define FCP_SEGMENT_APP_GOLD to remove hardcoded value

---
Changes in v6:
- Move re-initialisation out of fcp_usb() so it's clear there's no
  infinite recursion
- Update theory of operation and clarify the use of meter TLV metadata
- Rename meter_labels to meter_metadata for clarity
- Remove unnecessary casts in fcp_ioctl_cmd()

---
Changes in v5:
- Remove version/union complexity from init ioctl
- Add documentation to clarify ioctl usage and big picture

---
Changes in v4:
- Use variable-length data arrays in ioctl structs instead of pointers
- Add CAP_SYS_RAWIO requirement to hwdep interface
- Add validation of flash commands to prevent accidental bricking due
  to erasing/writing the App_Gold segment
- Refactor URB cleanup

---
Changes in v3:
- Update ioctl structs and add ioctl_compat op to work with 32-bit
  userspace on 64-bit kernels
- Update driver to do all init steps so it can re-init after
  suspend/resume
- Add version field to init ioctl for future compatibility
- Improve error messages when unexpected response data is received

---
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    |  120 ++++
 include/uapi/sound/tlv.h    |    2 +
 sound/usb/Makefile          |    1 +
 sound/usb/fcp.c             | 1134 +++++++++++++++++++++++++++++++++++
 sound/usb/fcp.h             |    7 +
 sound/usb/mixer_quirks.c    |    7 +
 sound/usb/mixer_scarlett2.c |    8 +
 8 files changed, 1285 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

Comments

Takashi Iwai Jan. 18, 2025, 11:07 a.m. UTC | #1
On Thu, 16 Jan 2025 18:47:07 +0100,
Geoffrey D. Bennett wrote:
> 
> Hi Takashi,
> 
> Thanks again! All changes from your feedback on v7 incorporated in v8.

Now applied to for-next branch.


thanks,

Takashi

> 
> Regards,
> Geoffrey.
> 
> ---
> Changes in v8:
> - Rename SNDRV_CTL_TLVT_LABELS to SNDRV_CTL_TLVT_FCP_CHANNEL_LABELS
> - Make meter levels buffer persistent rather than allocate/free each
>   time
> - Add data protection to fcp_meter_tlv_callback()
> - Set private->step* and co after allocation of the buffer
> - Use ALIGN() rather than open-coding it
> 
> ---
> Changes in v7:
> - Add #define SNDRV_CTL_TLVT_LABELS for channel labels
> - Add ioctl for setting meter labels and remove TLV write support
> - Simplify reinit handling
> - Remove more unnecessary casts
> - Add #define FCP_SEGMENT_APP_GOLD to remove hardcoded value
> 
> ---
> Changes in v6:
> - Move re-initialisation out of fcp_usb() so it's clear there's no
>   infinite recursion
> - Update theory of operation and clarify the use of meter TLV metadata
> - Rename meter_labels to meter_metadata for clarity
> - Remove unnecessary casts in fcp_ioctl_cmd()
> 
> ---
> Changes in v5:
> - Remove version/union complexity from init ioctl
> - Add documentation to clarify ioctl usage and big picture
> 
> ---
> Changes in v4:
> - Use variable-length data arrays in ioctl structs instead of pointers
> - Add CAP_SYS_RAWIO requirement to hwdep interface
> - Add validation of flash commands to prevent accidental bricking due
>   to erasing/writing the App_Gold segment
> - Refactor URB cleanup
> 
> ---
> Changes in v3:
> - Update ioctl structs and add ioctl_compat op to work with 32-bit
>   userspace on 64-bit kernels
> - Update driver to do all init steps so it can re-init after
>   suspend/resume
> - Add version field to init ioctl for future compatibility
> - Improve error messages when unexpected response data is received
> 
> ---
> 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    |  120 ++++
>  include/uapi/sound/tlv.h    |    2 +
>  sound/usb/Makefile          |    1 +
>  sound/usb/fcp.c             | 1134 +++++++++++++++++++++++++++++++++++
>  sound/usb/fcp.h             |    7 +
>  sound/usb/mixer_quirks.c    |    7 +
>  sound/usb/mixer_scarlett2.c |    8 +
>  8 files changed, 1285 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
> 
> -- 
> 2.45.0
>
Geoffrey D. Bennett Jan. 18, 2025, 6:39 p.m. UTC | #2
On Sat, Jan 18, 2025 at 12:07:25PM +0100, Takashi Iwai wrote:
> On Thu, 16 Jan 2025 18:47:07 +0100,
> Geoffrey D. Bennett wrote:
> > 
> > Hi Takashi,
> > 
> > Thanks again! All changes from your feedback on v7 incorporated in v8.
> 
> Now applied to for-next branch.

Hi Takashi,

Thank you for all your time and patience in reviewing the patches
through eight versions!

For the pull request notes, would you consider mentioning the specific
new hardware support? Something like "Add support for Focusrite
Scarlett 4th Gen 16i16, 18i16, and 18i20 interfaces via new FCP
driver" would help users discover this functionality.

Thanks again,
Geoffrey.
Takashi Iwai Jan. 19, 2025, 8:55 a.m. UTC | #3
On Sat, 18 Jan 2025 19:39:47 +0100,
Geoffrey D. Bennett wrote:
> 
> On Sat, Jan 18, 2025 at 12:07:25PM +0100, Takashi Iwai wrote:
> > On Thu, 16 Jan 2025 18:47:07 +0100,
> > Geoffrey D. Bennett wrote:
> > > 
> > > Hi Takashi,
> > > 
> > > Thanks again! All changes from your feedback on v7 incorporated in v8.
> > 
> > Now applied to for-next branch.
> 
> Hi Takashi,
> 
> Thank you for all your time and patience in reviewing the patches
> through eight versions!
> 
> For the pull request notes, would you consider mentioning the specific
> new hardware support? Something like "Add support for Focusrite
> Scarlett 4th Gen 16i16, 18i16, and 18i20 interfaces via new FCP
> driver" would help users discover this functionality.

Yes, will put something like that.


thanks,

Takashi