On Fri, 24 Sep 2021 21:24:13 +0200, Pierre-Louis Bossart wrote: > > While reviewing the HDAudio DMA handling, I found a number of > inconsistencies in how spin_locks are used. It's not clear what the > HDaudio bus->reg_lock is supposed to protect. In most cases only the > writes to specific boolean status flags are protected, and there are > multiple cases of taking the lock after testing a status flag. > > This patchset suggests a more consistent locking pattern, but it's > entirely possible that the bus->reg_lock is only intented to protect > register read/write access on the HDaudio bus, and not the status > flags, and that this entire piece of code is completely > over-engineered. > > On the Intel side no one knows why this spinlock was used, the reasons > are lost to history. I set the 'RFC' status on purpose in the hope > that Takashi might recall what this lock is supposed to protect. The > diff format makes this patchset difficult to review, it's recommended > to apply the patches and look at entire functions with changes to get > a better idea of the suggested changes. Oh well, the missing piece was the lock inside the loop in *_stream_assign(). It was lost while factoring out and converting to the HD-audio core code. (The lock wasn't taken on the whole loop at that time because the list itself was supposed to be static.) In anyway, I applied all patches except for patch#2, as the fixes for spinlock look correct. thanks, Takashi
