Re: [PATCH 1/2] power: bq24190_charger: Check the interrupt status on resume

From: Sebastian Reichel <sre@kernel.org>
To: Liam Breck <liam@networkimprov.net>
Cc: Tony Lindgren <tony@atomide.com>,
	Liam Breck <kernel@networkimprov.net>,
	"Mark A . Greer" <mgreer@animalcreek.com>,
	linux-pm@vger.kernel.org, linux-omap@vger.kernel.org,
	Matt Ranostay <matt@ranostay.consulting>
Subject: Re: [PATCH 1/2] power: bq24190_charger: Check the interrupt status on resume
Date: Sun, 22 Jan 2017 02:51:34 +0100	[thread overview]
Message-ID: <20170122015134.rhzckezpkxy5odla@earth> (raw)
In-Reply-To: <CAKvHMgRipKyVAF6Y6Wr3t=McPouqfC2B9KuAnM+RFUtE-LRtuw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

Hi,

On Sat, Jan 21, 2017 at 12:08:02AM -0800, Liam Breck wrote:
> > +static irqreturn_t bq24190_irq_handler_thread(int irq, void *data)
> > +{
> > +       struct bq24190_dev_info *bdi = data;
> > +
> > +       bdi->irq_event = true;
> > +       pm_runtime_get_sync(bdi->dev);
> 
> Do we need to call pm_runtime_irq_safe() prior to the above?

It's not needed here, since the irq handler does not run in
irq context (it's a threaded irq handler).

Btw. calling pm_runtime_irq_safe() basically means, that your
driver's power-management is broken:

http://lxr.free-electrons.com/source/drivers/base/power/runtime.c#L1288

(The relevant part is: »It also causes the parent's usage counter to
be permanently incremented«)

> > +       bq24190_check_status(bdi);
> > +       pm_runtime_put_sync(bdi->dev);
> > +       bdi->irq_event = false;
> >
> >         return IRQ_HANDLED;
> >  }

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]