Re: [char-misc stable 3.16] mei: me: wait for power gating exit confirmation

[Luis Henriques <luis.henriques@canonical.com>]

On Sun, Jul 05, 2015 at 05:06:40PM +0300, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> 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: <stable@vger.kernel.org> #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 <alexander.usyskin@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  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