Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752308AbbGILFs (ORCPT ); Thu, 9 Jul 2015 07:05:48 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:57677 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750799AbbGILFl (ORCPT ); Thu, 9 Jul 2015 07:05:41 -0400 Date: Thu, 9 Jul 2015 12:05:34 +0100 From: Luis Henriques To: Tomas Winkler Cc: Ben Hutchings , gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, Alexander Usyskin , stable@vger.kernel.org Subject: Re: [char-misc stable 3.16] mei: me: wait for power gating exit confirmation Message-ID: <20150709110534.GA2190@ares> References: <1436105200-6297-1-git-send-email-tomas.winkler@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1436105200-6297-1-git-send-email-tomas.winkler@intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8195 Lines: 242 On Sun, Jul 05, 2015 at 05:06:40PM +0300, Tomas Winkler wrote: > From: Alexander Usyskin > > commit 3dc196eae1db548f05e53e5875ff87b8ff79f249 upstream > > Fix the hbm power gating state machine so it will wait till it receives > confirmation interrupt for PG_ISOLATION_EXIT message. > > In process of the suspend flow the devices first have to exit from the > power gating state (runtime pm resume). > If we do not handle the confirmation interrupt after sending > PG_ISOLATION_EXIT message, we may receive it already after the suspend > flow has changed the device state and interrupt will be interpreted as a > spurious event, consequently link reset will be invoked which will > prevent the device from completing the suspend flow > > kernel: [6603] mei_reset:136: mei_me 0000:00:16.0: powering down: end of reset > kernel: [476] mei_me_irq_thread_handler:643: mei_me 0000:00:16.0: function called after ISR to handle the interrupt processing. > kernel: mei_me 0000:00:16.0: FW not ready: resetting > > Cc: #3.16-3.17 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=86241 > Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=770397 > Signed-off-by: Alexander Usyskin > Signed-off-by: Tomas Winkler > --- > drivers/misc/mei/hw-me.c | 59 ++++++++++++++++++++++++++++++++++++++++++---- > drivers/misc/mei/hw-txe.c | 13 ++++++++++ > drivers/misc/mei/mei_dev.h | 11 +++++++++ Thank you for this backport, Tomas. I didn't queue this commit for the 3.16 kernel because it was tagged for stable kernels >= 3.18. But looking at it again, I fail to understand why you dropped the changes to drivers/misc/mei/client.c. Was that done by mistake? Cheers, -- Lu?s > 3 files changed, 79 insertions(+), 4 deletions(-) > > diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c > index 721824bcade6..67b766d9f8ec 100644 > --- a/drivers/misc/mei/hw-me.c > +++ b/drivers/misc/mei/hw-me.c > @@ -565,11 +565,27 @@ int mei_me_pg_unset_sync(struct mei_device *dev) > mutex_lock(&dev->device_lock); > > reply: > - if (dev->pg_event == MEI_PG_EVENT_RECEIVED) > - ret = mei_hbm_pg(dev, MEI_PG_ISOLATION_EXIT_RES_CMD); > + if (dev->pg_event != MEI_PG_EVENT_RECEIVED) { > + ret = -ETIME; > + goto out; > + } > + > + dev->pg_event = MEI_PG_EVENT_INTR_WAIT; > + ret = mei_hbm_pg(dev, MEI_PG_ISOLATION_EXIT_RES_CMD); > + if (ret) > + return ret; > + > + mutex_unlock(&dev->device_lock); > + wait_event_timeout(dev->wait_pg, > + dev->pg_event == MEI_PG_EVENT_INTR_RECEIVED, timeout); > + mutex_lock(&dev->device_lock); > + > + if (dev->pg_event == MEI_PG_EVENT_INTR_RECEIVED) > + ret = 0; > else > ret = -ETIME; > > +out: > dev->pg_event = MEI_PG_EVENT_IDLE; > hw->pg_state = MEI_PG_OFF; > > @@ -577,6 +593,19 @@ reply: > } > > /** > + * mei_me_pg_in_transition - is device now in pg transition > + * > + * @dev: the device structure > + * > + * Return: true if in pg transition, false otherwise > + */ > +static bool mei_me_pg_in_transition(struct mei_device *dev) > +{ > + return dev->pg_event >= MEI_PG_EVENT_WAIT && > + dev->pg_event <= MEI_PG_EVENT_INTR_WAIT; > +} > + > +/** > * mei_me_pg_is_enabled - detect if PG is supported by HW > * > * @dev: the device structure > @@ -612,6 +641,24 @@ notsupported: > } > > /** > + * mei_me_pg_intr - perform pg processing in interrupt thread handler > + * > + * @dev: the device structure > + */ > +static void mei_me_pg_intr(struct mei_device *dev) > +{ > + struct mei_me_hw *hw = to_me_hw(dev); > + > + if (dev->pg_event != MEI_PG_EVENT_INTR_WAIT) > + return; > + > + dev->pg_event = MEI_PG_EVENT_INTR_RECEIVED; > + hw->pg_state = MEI_PG_OFF; > + if (waitqueue_active(&dev->wait_pg)) > + wake_up(&dev->wait_pg); > +} > + > +/** > * mei_me_irq_quick_handler - The ISR of the MEI device > * > * @irq: The irq number > @@ -669,6 +716,8 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id) > goto end; > } > > + mei_me_pg_intr(dev); > + > /* check if we need to start the dev */ > if (!mei_host_is_ready(dev)) { > if (mei_hw_is_ready(dev)) { > @@ -707,9 +756,10 @@ irqreturn_t mei_me_irq_thread_handler(int irq, void *dev_id) > /* > * During PG handshake only allowed write is the replay to the > * PG exit message, so block calling write function > - * if the pg state is not idle > + * if the pg event is in PG handshake > */ > - if (dev->pg_event == MEI_PG_EVENT_IDLE) { > + if (dev->pg_event != MEI_PG_EVENT_WAIT && > + dev->pg_event != MEI_PG_EVENT_RECEIVED) { > rets = mei_irq_write_handler(dev, &complete_list); > dev->hbuf_is_ready = mei_hbuf_is_ready(dev); > } > @@ -733,6 +783,7 @@ static const struct mei_hw_ops mei_me_hw_ops = { > .hw_config = mei_me_hw_config, > .hw_start = mei_me_hw_start, > > + .pg_in_transition = mei_me_pg_in_transition, > .pg_is_enabled = mei_me_pg_is_enabled, > > .intr_clear = mei_me_intr_clear, > diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c > index f1cd166094f2..d0736e127c2a 100644 > --- a/drivers/misc/mei/hw-txe.c > +++ b/drivers/misc/mei/hw-txe.c > @@ -285,6 +285,18 @@ int mei_txe_aliveness_set_sync(struct mei_device *dev, u32 req) > } > > /** > + * mei_txe_pg_in_transition - is device now in pg transition > + * > + * @dev: the device structure > + * > + * Return: true if in pg transition, false otherwise > + */ > +static bool mei_txe_pg_in_transition(struct mei_device *dev) > +{ > + return dev->pg_event == MEI_PG_EVENT_WAIT; > +} > + > +/** > * mei_txe_pg_is_enabled - detect if PG is supported by HW > * > * @dev: the device structure > @@ -1053,6 +1065,7 @@ static const struct mei_hw_ops mei_txe_hw_ops = { > .hw_config = mei_txe_hw_config, > .hw_start = mei_txe_hw_start, > > + .pg_in_transition = mei_txe_pg_in_transition, > .pg_is_enabled = mei_txe_pg_is_enabled, > > .intr_clear = mei_txe_intr_clear, > diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h > index 0b0d6135543b..dd30e604938d 100644 > --- a/drivers/misc/mei/mei_dev.h > +++ b/drivers/misc/mei/mei_dev.h > @@ -235,6 +235,7 @@ struct mei_cl { > * @hw_config - configure hw > > * @pg_state - power gating state of the device > + * @pg_in_transition - is device now in pg transition > * @pg_is_enabled - is power gating enabled > > * @intr_clear - clear pending interrupts > @@ -262,6 +263,7 @@ struct mei_hw_ops { > void (*hw_config)(struct mei_device *dev); > > enum mei_pg_state (*pg_state)(struct mei_device *dev); > + bool (*pg_in_transition)(struct mei_device *dev); > bool (*pg_is_enabled)(struct mei_device *dev); > > void (*intr_clear)(struct mei_device *dev); > @@ -358,11 +360,15 @@ struct mei_cl_device { > * @MEI_PG_EVENT_IDLE: the driver is not in power gating transition > * @MEI_PG_EVENT_WAIT: the driver is waiting for a pg event to complete > * @MEI_PG_EVENT_RECEIVED: the driver received pg event > + * @MEI_PG_EVENT_INTR_WAIT: the driver is waiting for a pg event interrupt > + * @MEI_PG_EVENT_INTR_RECEIVED: the driver received pg event interrupt > */ > enum mei_pg_event { > MEI_PG_EVENT_IDLE, > MEI_PG_EVENT_WAIT, > MEI_PG_EVENT_RECEIVED, > + MEI_PG_EVENT_INTR_WAIT, > + MEI_PG_EVENT_INTR_RECEIVED, > }; > > /** > @@ -646,6 +652,11 @@ static inline enum mei_pg_state mei_pg_state(struct mei_device *dev) > return dev->ops->pg_state(dev); > } > > +static inline bool mei_pg_in_transition(struct mei_device *dev) > +{ > + return dev->ops->pg_in_transition(dev); > +} > + > static inline bool mei_pg_is_enabled(struct mei_device *dev) > { > return dev->ops->pg_is_enabled(dev); > -- > 2.4.3 > > -- > To unsubscribe from this list: send the line "unsubscribe stable" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/