On Tue, Jul 09, 2019 at 07:14:15PM +0200, Takashi Iwai wrote:
On Tue, 09 Jul 2019 18:24:30 +0200, Geoffrey D. Bennett wrote:
[...]
+static int scarlett_gen2_mixer_enable; +module_param(scarlett_gen2_mixer_enable, int, 0444); +MODULE_PARM_DESC(scarlett_gen2_mixer_enable,
"Focusrite Scarlett Gen 2 Mixer Driver Enable");
Do we need this? If disabling the quirk is really required, it should be implemented rather in a generic option, instead.
Actually it would be best to have it disabled by default as I have had two reports from people who tried this mixer driver and it broke audio for them.
Hm, and these have the same USB device ID?
Yes, same device ID. I couldn't see any significant difference between mine and theirs usbmon trace and lsusb -v output.
[...]
I guess you can use chip->setup value for the quirk-specific configuration as a start. It's a bit hackish but the parameter is already there for long time, hence you don't need to add any extra stuff for that.
Got it. Have implemented and will resend soon.
In your other email you wrote:
Canceling the pending work is the right thing for suspend, but we need reconsidering the side effect. For example, the device might be removed while the machine is in sleep. And the purpose of the scheduled work is to sync the config to NVRAM. That is, it should have been done before going to suspend. Then we don't have to resume the task at resume, too.
That said, the best would be:
- cancel the pending work, check the return value
- re-execute the work immediately (delay=0) if the work was canceled in the above, and do flush -- or just call the function directly
I agree and have implemented that as well.
Thanks, Geoffrey.