Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1717002 > unrolled thread

Re: [PATCH] mfd: intel-lpss: Put I2C and SPI controllers into reset state on suspend

Started byFurquan Shaikh <furquan@google.com>
First post2017-08-22 05:40 +0200
Last post2017-08-22 09:50 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] mfd: intel-lpss: Put I2C and SPI controllers into reset  state on suspend Furquan Shaikh <furquan@google.com> - 2017-08-22 05:40 +0200
    Re: [PATCH] mfd: intel-lpss: Put I2C and SPI controllers into reset  state on suspend Lee Jones <lee.jones@linaro.org> - 2017-08-22 09:50 +0200

#1717002 — Re: [PATCH] mfd: intel-lpss: Put I2C and SPI controllers into reset state on suspend

FromFurquan Shaikh <furquan@google.com>
Date2017-08-22 05:40 +0200
SubjectRe: [PATCH] mfd: intel-lpss: Put I2C and SPI controllers into reset state on suspend
Message-ID<uh8cO-51I-1@gated-at.bofh.it>
Hello Lee,

Gentle ping. Do you see any issues with the following change?

Thanks,
Furquan

On Sun, Jul 23, 2017 at 11:02 PM, Furquan Shaikh <furquan@google.com> wrote:
>
> Commit 274e43edcda6f ("mfd: intel-lpss: Do not put device in reset
> state on suspend") changed the behavior on suspend by not putting LPSS
> controllers into reset. This was done because S3/S0ix fail if UART
> device is put into reset and no_console_suspend flag is enabled.
>
> Because of the above change, I2C controller gets into a bad state if
> it observes that the I2C lines are pulled low when power to I2C device
> is cut off during suspend (generally, I2C lines are pulled to power
> rail of the I2C device in order to ensure that there is no leakage
> because of the pulls when device is turned off). This results in the
> controller timing out for all future I2C operations after resume. It
> is primarily because of the following sequence of operations:
>
> During suspend:
> 1. I2C controller is disabled, but it is not put into reset.
> 2. Power to I2C device is cut off.
> 3. #2 results in the I2C lines being pulled low.
>
> ==> At this point the I2C controller gets into a bad state
>
> On resume:
> 1. Power to I2C device is enabled.
> 2. #2 results in the I2C lines being pulled high.
> 3. I2C controller is enabled.
>
> However, even after enabling the I2C controller, all future I2C xfers
> fail since the controller is in a bad state and does not attempt to
> make any transactions and hence times out.
>
> In order to ensure that the controller does not get into a bad state,
> this change puts it into reset if the controller type is not
> UART. With this change, the order of operations is:
>
> During suspend:
> 1. I2C controller is disabled and put into reset.
> 2. Power to I2C device is cut off.
> 3. #2 results in the I2C lines being pulled low.
>
> On resume:
> 1. Power to I2C device is enabled.
> 2. #2 results in the I2C lines being pulled high.
> 3. I2C controller is enabled and taken out of reset.
>
> Signed-off-by: Furquan Shaikh <furquan@google.com>
> ---
>  drivers/mfd/intel-lpss.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 70c646b0097d..0e0ab9bb1530 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -502,6 +502,14 @@ int intel_lpss_suspend(struct device *dev)
>         for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
>                 lpss->priv_ctx[i] = readl(lpss->priv + i * 4);
>
> +       /*
> +        * If the device type is not UART, then put the controller into
> +        * reset. UART cannot be put into reset since S3/S0ix fail when
> +        * no_console_suspend flag is enabled.
> +        */
> +       if (lpss->type != LPSS_DEV_UART)
> +               writel(0, lpss->priv + LPSS_PRIV_RESETS);
> +
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(intel_lpss_suspend);
> --
> 2.14.0.rc0.284.gd933b75aa4-goog
>

[toc] | [next] | [standalone]


#1717100

FromLee Jones <lee.jones@linaro.org>
Date2017-08-22 09:50 +0200
Message-ID<uhc6J-7GF-1@gated-at.bofh.it>
In reply to#1717002
On Mon, 21 Aug 2017, Furquan Shaikh wrote:

> Hello Lee,
> 
> Gentle ping. Do you see any issues with the following change?

Couple of things:

1. Please do not top post.
2. Please do not send contentless pings.

If you think a patch had fallen through the cracks (which this one
had), please resend the patch with [RESEND] in the subject line.
[RESEND]s will normally be prioritised over [PATCH]es.

No need to do that this time, but please remember for next.

I'll deal with this patch now, since it looks pretty trivial.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web