[PATCH V2 00/14] Cleanup before adding Scarlett Gen 3 support

[PATCH V2 00/14] Cleanup before adding Scarlett Gen 3 support

[Geoffrey D. Bennett]

Hi Takashi,

Here is version 2 of a set of patches which is some cleanup of the
Scarlett Gen 2 mixer driver in preparation for adding Scarlett Gen 3
support.

One review comment I got (from Hin-Tak) was:

> 40+ patches is a lot, for modifying just one file. I would collapse
> it all into one and break it up again to under 10, maybe, broadly
> into "functionally-equivalent re-org", "small isolated bug fixes",
> "additional functions, not yet used", "hooking up those new
> functions", etc?

I'm not sure that I agree with that comment -- I tried to follow the
Documentation/process/submitting-patches.rst advice of "Separate each
logical change into a separate patch" for easy review of the
individual pieces, but perhaps I went too far in that direction?

Please let me know if I should combine some of these patches together.

After this set of patches I have a second set of patches almost ready.
They add a few features to the Gen 2 support and update the code ready
to add Gen 3 support:

  ALSA: usb-audio: scarlett2: Add usb_tx/rx functions
  ALSA: usb-audio: scarlett2: Update initialisation sequence
  ALSA: usb-audio: scarlett2: Fix 6i6 Gen 2 line out descriptions
  ALSA: usb-audio: scarlett2: Always enable interrupt polling
  ALSA: usb-audio: scarlett2: Add "Sync Status" control
  ALSA: usb-audio: scarlett2: Merge common line in capture strings
  ALSA: usb-audio: scarlett2: Reformat scarlett2_config_items[]
  ALSA: usb-audio: scarlett2: Improve device info lookup
  ALSA: usb-audio: scarlett2: Move info lookup out of init function
  ALSA: usb-audio: scarlett2: Remove repeated device info comments
  ALSA: usb-audio: scarlett2: Add scarlett2_vol_ctl_write() helper
  ALSA: usb-audio: scarlett2: Add mute support
  ALSA: usb-audio: scarlett2: Allow arbitrary ordering of mux entries
  ALSA: usb-audio: scarlett2: Split struct scarlett2_ports
  ALSA: usb-audio: scarlett2: Fix Level Meter control

The final set of patches adds Gen 3 mixer support and then adds
support for the new Gen 3 features:

  ALSA: usb-audio: scarlett2: Add Gen 3 mixer support
  ALSA: usb-audio: scarlett2: Add support for "input-other" notify
  ALSA: usb-audio: scarlett2: Add Gen 3 MSD mode switch
  ALSA: usb-audio: scarlett2: Move get config above set config
  ALSA: usb-audio: scarlett2: Allow bit-level access to config
  ALSA: usb-audio: scarlett2: Add support for Solo and 2i2 Gen 3
  ALSA: usb-audio: scarlett2: Add "air" switch support
  ALSA: usb-audio: scarlett2: Add phantom power switch support
  ALSA: usb-audio: scarlett2: Add direct monitor support
  ALSA: usb-audio: scarlett2: Label 18i8 Gen 3 line outputs correctly
  ALSA: usb-audio: scarlett2: Split up sw_hw_enum_ctl_put()
  ALSA: usb-audio: scarlett2: Add sw_hw_ctls and mux_ctls
  ALSA: usb-audio: scarlett2: Update mux controls to allow updates
  ALSA: usb-audio: scarlett2: Add speaker switching support
  ALSA: usb-audio: scarlett2: Update get_config to do endian conversion
  ALSA: usb-audio: scarlett2: Add support for the talkback feature

If you'd like to see the commits so far all at once, please check this
branch:

https://github.com/geoffreybennett/scarlett-gen2/commits/scarlett-gen3-for-next

Thanks,
Geoffrey.

Geoffrey D. Bennett (14):
  ALSA: usb-audio: scarlett2: Remove incorrect S/PDIF comment
  ALSA: usb-audio: scarlett2: Fix 18i8 Gen 2 PCM Input count
  ALSA: usb-audio: scarlett2: Coding style improvements
  ALSA: usb-audio: scarlett2: Remove unused/useless code
  ALSA: usb-audio: scarlett2: Remove interrupt debug message
  ALSA: usb-audio: scarlett2: Remove redundant info->button_count
  ALSA: usb-audio: scarlett2: Rename buttons/interrupts/vol
  ALSA: usb-audio: scarlett2: Rename struct scarlett2_mixer_data
  ALSA: usb-audio: scarlett2: Add temp variable for consistency
  ALSA: usb-audio: scarlett2: Fix data_mutex lock
  ALSA: usb-audio: scarlett2: Fix scarlett2_*_ctl_put() return values
  ALSA: usb-audio: scarlett2: Fix union usage in mixer control callbacks
  ALSA: usb-audio: scarlett2: Don't copy struct scarlett2_config
  ALSA: usb-audio: scarlett2: Remove hard-coded USB #defines

 sound/usb/mixer_scarlett_gen2.c | 425 +++++++++++++++++---------------
 1 file changed, 224 insertions(+), 201 deletions(-)


base-commit: f8fbcdfb0665de60997d9746809e1704ed782bbc
-- 
2.31.1

Re: [PATCH V2 00/14] Cleanup before adding Scarlett Gen 3 support

[Takashi Iwai]

On Sun, 20 Jun 2021 18:46:15 +0200,
Geoffrey D. Bennett wrote:
> 
> Hi Takashi,
> 
> Here is version 2 of a set of patches which is some cleanup of the
> Scarlett Gen 2 mixer driver in preparation for adding Scarlett Gen 3
> support.
> 
> One review comment I got (from Hin-Tak) was:
> 
> > 40+ patches is a lot, for modifying just one file. I would collapse
> > it all into one and break it up again to under 10, maybe, broadly
> > into "functionally-equivalent re-org", "small isolated bug fixes",
> > "additional functions, not yet used", "hooking up those new
> > functions", etc?
> 
> I'm not sure that I agree with that comment -- I tried to follow the
> Documentation/process/submitting-patches.rst advice of "Separate each
> logical change into a separate patch" for easy review of the
> individual pieces, but perhaps I went too far in that direction?
> 
> Please let me know if I should combine some of these patches together.

The split is fine as long as it's done logically, so I took as is.

But, one thing that can be improved at the next time is to sort out
fix patches.  e.g. you had patches for fixing the mixer field type
(int vs enum) and a patch to correct the locking; those are rather
independent from the cleanup series and should be applied for the
stable backports, too.  I didn't add stable at this time because I
wasn't sure whether applicable and that's no severe issue, but the
process can be better.

Note that the merge window may be closed in this week, so if you want
the stuff to be merged, please submit now.


thanks,

Takashi

Re: [PATCH V2 00/14] Cleanup before adding Scarlett Gen 3 support

[Takashi Iwai]

On Mon, 21 Jun 2021 08:43:51 +0200,
Takashi Iwai wrote:
> 
> On Sun, 20 Jun 2021 18:46:15 +0200,
> Geoffrey D. Bennett wrote:
> > 
> > Hi Takashi,
> > 
> > Here is version 2 of a set of patches which is some cleanup of the
> > Scarlett Gen 2 mixer driver in preparation for adding Scarlett Gen 3
> > support.
> > 
> > One review comment I got (from Hin-Tak) was:
> > 
> > > 40+ patches is a lot, for modifying just one file. I would collapse
> > > it all into one and break it up again to under 10, maybe, broadly
> > > into "functionally-equivalent re-org", "small isolated bug fixes",
> > > "additional functions, not yet used", "hooking up those new
> > > functions", etc?
> > 
> > I'm not sure that I agree with that comment -- I tried to follow the
> > Documentation/process/submitting-patches.rst advice of "Separate each
> > logical change into a separate patch" for easy review of the
> > individual pieces, but perhaps I went too far in that direction?
> > 
> > Please let me know if I should combine some of these patches together.
> 
> The split is fine as long as it's done logically, so I took as is.
> 
> But, one thing that can be improved at the next time is to sort out
> fix patches.  e.g. you had patches for fixing the mixer field type
> (int vs enum) and a patch to correct the locking; those are rather
> independent from the cleanup series and should be applied for the
> stable backports, too.  I didn't add stable at this time because I
> wasn't sure whether applicable and that's no severe issue, but the
> process can be better.
> 
> Note that the merge window may be closed in this week, so if you want
> the stuff to be merged, please submit now.

Oh, one more thing: please use the mail thread for a patch set at the
next time!


Takashi

Re: [PATCH V2 00/14] Cleanup before adding Scarlett Gen 3 support

[Geoffrey D. Bennett]

On Mon, Jun 21, 2021 at 08:43:51AM +0200, Takashi Iwai wrote:
> On Sun, 20 Jun 2021 18:46:15 +0200,
> Geoffrey D. Bennett wrote:
> > 
> > Hi Takashi,
> > 
> > Here is version 2 of a set of patches which is some cleanup of the
> > Scarlett Gen 2 mixer driver in preparation for adding Scarlett Gen 3
> > support.
> > 
> > One review comment I got (from Hin-Tak) was:
> > 
> > > 40+ patches is a lot, for modifying just one file. I would collapse
> > > it all into one and break it up again to under 10, maybe, broadly
> > > into "functionally-equivalent re-org", "small isolated bug fixes",
> > > "additional functions, not yet used", "hooking up those new
> > > functions", etc?
> > 
> > I'm not sure that I agree with that comment -- I tried to follow the
> > Documentation/process/submitting-patches.rst advice of "Separate each
> > logical change into a separate patch" for easy review of the
> > individual pieces, but perhaps I went too far in that direction?
> > 
> > Please let me know if I should combine some of these patches together.
> 
> The split is fine as long as it's done logically, so I took as is.
> 
> But, one thing that can be improved at the next time is to sort out
> fix patches.  e.g. you had patches for fixing the mixer field type
> (int vs enum) and a patch to correct the locking; those are rather
> independent from the cleanup series and should be applied for the
> stable backports, too.  I didn't add stable at this time because I
> wasn't sure whether applicable and that's no severe issue, but the
> process can be better.

I did not notice any actual bug from using the wrong field type, and
nobody reported the locking problem with Gen 2, so I think those are
low priority.

The two patches I submitted before for "Read all configuration at init
time" are a higher priority for stable as they fix an actual problem
that users are encountering: since our last discussion I had another
report from a user; they were wondering why their headphones stopped
working after changing an unrelated control.

> Note that the merge window may be closed in this week, so if you want
> the stuff to be merged, please submit now.

Thanks, I will submit very soon.

> Oh, one more thing: please use the mail thread for a patch set at the
> next time!

Sorry about that. Right after I sent I noticed that I forgot
--thread=shallow.

Thanks,
Geoffrey.