On 30/07/2019 16:50, Mark Brown wrote: > On Tue, Jul 30, 2019 at 04:25:56PM +0100, Thomas Preston wrote: >> On 30/07/2019 15:19, Mark Brown wrote: > >>> It is unclear what this mutex usefully protects, it only gets taken when >>> writing to the debugfs file to trigger this diagnostic mode but doesn't >>> do anything to control interactions with any other code path in the >>> driver. > >> If another process reads the debugfs node "diagnostic" while the turn-on >> diagnostic mode is running, this mutex prevents the second process >> restarting the diagnostics. > >> This is redundant if debugfs reads are atomic, but I don't think they are. > > Like I say it's not just debugfs though, there's the standard driver > interface too. > Ah right, I understand. So if we run the turn-on diagnostics routine, there's nothing stopping anyone from interacting with the device in other ways. I guess there's no way to share that mutex with ALSA? In that case, it doesn't matter if this mutex is there or not - this feature is incompatible. How compatible do debugfs interfaces have to be? I was under the impression anything goes. I would argue that the debugfs is better off for having the mutex so that no one re-reads "diagnostic" within the 5s poll timeout. Alternatively, this diagnostic feature could be handled with an external-handler kcontrol SOC_SINGLE_EXT? I'm not sure if this is an atomic interface either. What would be acceptable?
