From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f54.google.com ([74.125.83.54]:36716 "EHLO mail-pg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932291AbdABJe6 (ORCPT ); Mon, 2 Jan 2017 04:34:58 -0500 Received: by mail-pg0-f54.google.com with SMTP id f188so202188511pgc.3 for ; Mon, 02 Jan 2017 01:34:57 -0800 (PST) Date: Mon, 2 Jan 2017 15:04:54 +0530 From: Viresh Kumar To: David Gstir Cc: linux-kernel@vger.kernel.org, daniel.lezcano@linaro.org, tglx@linutronix.de, richard@nod.at, stable@vger.kernel.org, Andres Salomon Subject: Re: [PATCH] clockevents/drivers/cs5535: Un-break driver with 'set-state' interface Message-ID: <20170102093454.GF13679@vireshk-i7> References: <20161228215603.6600-1-david@sigma-star.at> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161228215603.6600-1-david@sigma-star.at> Sender: stable-owner@vger.kernel.org List-ID: On 28-12-16, 22:56, David Gstir wrote: > Since migrating to the 'set-state' interface the cs5535 driver causes a > crash after loading: Right after initialization, the IRQ handler > (mfgpt_tick) is triggered with clock event device in detached state. This > state not properly handled and causes a crash through NULL pointer > dereference upon calling the clockevent's event_handler. > > This patch fixes this by handling the detached state the same way the > shutdown state is handled. > > Fixes: 8f9327cbb6e8 ("clockevents/drivers/cs5535: Migrate to new 'set-state' interface") > Cc: stable@vger.kernel.org > Cc: Andres Salomon > Cc: Viresh Kumar > Signed-off-by: David Gstir > --- > drivers/clocksource/cs5535-clockevt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/clocksource/cs5535-clockevt.c b/drivers/clocksource/cs5535-clockevt.c > index 9a7e37cf56b0..649e0cd90805 100644 > --- a/drivers/clocksource/cs5535-clockevt.c > +++ b/drivers/clocksource/cs5535-clockevt.c > @@ -117,7 +117,8 @@ static irqreturn_t mfgpt_tick(int irq, void *dev_id) > /* Turn off the clock (and clear the event) */ > disable_timer(cs5535_event_clock); > > - if (clockevent_state_shutdown(&cs5535_clockevent)) > + if (clockevent_state_shutdown(&cs5535_clockevent) || > + clockevent_state_detached(&cs5535_clockevent)) > return IRQ_HANDLED; > > /* Clear the counter */ Sorry for breaking it, but it looks we have unearthed a bug because of my patch. I wouldn't enable the IRQ unless the clockevents core has asked for it, i.e. by calling set_state_periodic() or set_state_oneshot(). The driver is currently enabling the IRQ from its init code and that's where the problem is IMHO. -- viresh