On Mon, Mar 18, 2019 at 08:20:07PM -0600, George Hilliard wrote: > The module was initializing completions whenever it was going to wait on > them, and not when the completion was allocated. This is incorrect > according to the completion docs: > > Calling init_completion() on the same completion object twice is > most likely a bug [...] > > Re-initialization is also unnecessary because the module never uses > complete_all(). Fix this by only ever initializing the completion a > single time. > > Signed-off-by: George Hilliard <[email protected]> > --- > drivers/staging/mt7621-mmc/sd.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c > index 89fbc0a1dec7..c272aa780719 100644 > --- a/drivers/staging/mt7621-mmc/sd.c > +++ b/drivers/staging/mt7621-mmc/sd.c > @@ -467,7 +467,9 @@ static unsigned int msdc_command_start(struct msdc_host > *host, > host->cmd = cmd; > host->cmd_rsp = resp; > > - init_completion(&host->cmd_done); > + // The completion should have been consumed by the previous command > + // response handler, because the mmc requests should be serialized > + BUG_ON(completion_done(&host->cmd_done));
Adding new BUG_ON() calls is not ok. That crashes the machine and will break everyone. If this really is a potential error that could happen, then handle it. If not, then do not even put this there, it's not needed. All of the BUG_ON() entries need to be removed in the end from this code anyway. thanks, greg k-h
