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.
