Hi Pavel, On Mon, May 10, 2021 at 10:56:50PM +0200, Pavel Machek wrote: > Hi! > > > From: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx> > > > > commit 47705c08465931923e2f2b506986ca0bdf80380d upstream. > > > > When clearing up the channel context after client drivers are > > done using channels, the configuration is currently not being > > reset entirely. Ensure this is done to appropriately handle > > issues where clients unaware of the context state end up calling > > functions which expect a context. > > > +++ b/drivers/bus/mhi/core/init.c > > @@ -544,6 +544,7 @@ void mhi_deinit_chan_ctxt(struct mhi_con > > + u32 tmp; > > @@ -554,7 +555,19 @@ void mhi_deinit_chan_ctxt(struct mhi_con > ... > > + tmp = chan_ctxt->chcfg; > > + tmp &= ~CHAN_CTX_CHSTATE_MASK; > > + tmp |= (MHI_CH_STATE_DISABLED << CHAN_CTX_CHSTATE_SHIFT); > > + chan_ctxt->chcfg = tmp; > > + > > + /* Update to all cores */ > > + smp_wmb(); > > } > > This is really interesting code; author was careful to make sure chcfg > is updated atomically, but C compiler is free to undo that. Plus, this > will make all kinds of checkers angry. > > Does the file need to use READ_ONCE and WRITE_ONCE? > Thanks for looking into this. I agree that the order could be mangled between chcfg read & write and using READ_ONCE & WRITE_ONCE seems to be a good option. Bhaumik, can you please submit a patch and tag stable? Thanks, Mani > Best regards, > Pavel > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
