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