> It seems like it would mostly be a programmer error if it was called twice. > > How about something like this: > > if (unlikely(WARN_ON(lsp0_dsm_in_screen_off))) > return -EINVAL; > > You could do something similar with the inverse in the other call site > too then. Indeed, under normal operation calling it twice is an error. 2 concerns here: if I make this change just here, the code on patch 2 will make the suspend sequence bail. Is this a desired outcome? Shouldn't we just keep going? And if we should keep going, does that include calling Display Off twice? I am inclined to say the desired outcome is to not call Display Off twice and to keep the suspend sequence going. In which case, I will merge your suggestion and remove the bail sequence from patch 2. 2nd concern is that I suspect it is very likely firmware vendors will want to call this sequence as part of developing devices, so I am inclined to allow calling the functions back to back. However, that could be achieved differently, e.g., with a module flag. > > + lsp0_dsm_in_screen_off = false; > > Doesn't it initialize to false already? Is this really needed? More for my peace of mind. I will remove it. Antheas
