The change in the patch done to address the issue Geunter mentioned is that da7219_aad_handle_gnd_switch_time() is now called before interrupts are enabled. To address the msleep() issue, the delay is now added as a delayed work of its own workqueue.
On Tue, Apr 11, 2023 at 10:28 PM Guenter Roeck groeck@google.com wrote:
On Tue, Apr 11, 2023 at 4:32 AM Mark Brown broonie@kernel.org wrote:
On Mon, Apr 10, 2023 at 09:26:34AM +0000, David Rau wrote:
This commit improves the control of ground switches in AAD IRQ
In what way does it do this - what was previously unrelabile and how does this change address that?
One very specific problem is that da7219_aad_handle_gnd_switch_time() is currently called after interrupts were enabled. As a result, the delay time is not initialized if there is an interrupt before the initialization. This results in a negative value passed to msleep(). Since the parameter to msleep() is unsigned, this causes it to sleep forever which in turn causes a substantial number of hung task crashes in ChromeOS. Plus, of course, the code doesn't really do anything in this situation.
A secondary problem may be that calling msleep() with a potentially large sleep time on a system worker isn't really a good idea, but I didn't explore the impact further.
Guenter