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

From: Luis Henriques
Date: Thu Jul 09 2015 - 07:05:50 EST

On Sun, Jul 05, 2015 at 05:06:40PM +0300, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx> #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@xxxxxxxxx>
> Signed-off-by: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/