Hi Rohit, On 20-06-18, 13:07, Rohit Kumar wrote: > > On 19-06-18, 19:20, Rohit Kumar wrote: > > > On 6/19/2018 10:35 AM, Vinod wrote: > > > > On 18-06-18, 16:46, Rohit kumar wrote: > > > > > > > > > +struct sdm845_snd_data { > > > > > + struct snd_soc_card *card; > > > > > + struct regulator *vdd_supply; > > > > > + struct snd_soc_dai_link dai_link[]; > > > > > +}; > > > > > + > > > > > +static struct mutex pri_mi2s_res_lock; > > > > > +static struct mutex quat_tdm_res_lock; > > > > any reason why the locks can't be part of sdm845_snd_data? > > > > Also why do we need two locks ? > > > No specific reason, I will move it to sdm845_snd_data. > > > These locks are used to protect enable/disable of bit clocks. We have > > > Primary MI2S RX/TX > > > and Quaternary TDM RX/TX interfaces. For primary mi2s rx/tx, we have single > > > clock which is > > > synchronized with pri_mi2s_res_lock. For Quat TDM RX/TX, we are using > > > quat_tdm_res_lock. > > > We need two locks as we are protecting two different resources. > > I think bigger question is why do you need any locks? What is the race > > scenario you envision which needs protection > > > > Below is one of the race condition: > > Thread1 | Thread2 > ---------------------------------------------------------- > startup() | > count++; | startup() > read count (count = 1) | > enable_clock() | count++; //count = 2 > shutdown() | > count--;// count = 1 | > | read count (count = 1) > | enable_clock() > > Here clock will be enabled twice but disable will be called only once when > count = 0. > > This will make the clock always enabled. So, I think we should keep either > mutex lock or atomic variable to synchronize this. we are using DPCM here right? -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html