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.