On Mon, Apr 03, 2023 at 07:29:36AM +0300, Dan Carpenter wrote:
This checks that we are not in WD_STATE_COLDBOOT_START state. How are we going to be in COLDBOOT if we are in INIT? Is this changing in the background? Can this check be removed?
It turned out the answer was yes, it can be removed.
Also if stuff is changing in the background and there is no way the locking is correct.
The locking is correct. Take a look at it.
We are holding the &arche_pdata->wake_lock in arche_platform_wd_irq() and every place that calls arche_platform_set_wake_detect_state() takes that lock first. So it can't change in the background.
Delete the check like so.
regards, dan carpenter
diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c index fcbd5f71eff2..4cca45ee70b3 100644 --- a/drivers/staging/greybus/arche-platform.c +++ b/drivers/staging/greybus/arche-platform.c @@ -172,18 +172,11 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid) arche_platform_set_wake_detect_state(arche_pdata, WD_STATE_IDLE); } else { - /* - * Check we are not in middle of irq thread - * already - */ - if (arche_pdata->wake_detect_state != - WD_STATE_COLDBOOT_START) { - arche_platform_set_wake_detect_state(arche_pdata, - WD_STATE_COLDBOOT_TRIG); - spin_unlock_irqrestore(&arche_pdata->wake_lock, - flags); - return IRQ_WAKE_THREAD; - } + arche_platform_set_wake_detect_state(arche_pdata, + WD_STATE_COLDBOOT_TRIG); + spin_unlock_irqrestore(&arche_pdata->wake_lock, + flags); + return IRQ_WAKE_THREAD; } } } else {