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

["Geoffrey D. Bennett" <g@b4.vu>]

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.