On 9/19/2024 15:45, Antheas Kapenekakis wrote:
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.
OK. We'll have to see Rafael's thoughts on your next version.
FYI, he's at LPC, so expect delayed response.
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.
I think vendors developing devices could rebuild the kernel too. I'm
not really sure this would be a common enough need to justify a module
parameter.
+ 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
