linux-kernel - Re: [PATCH] mfd: cros ec: spi: Increase wait time to 200ms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170314165525.qgecoxvne6nrgatk@dell>
Date:   Tue, 14 Mar 2017 16:55:25 +0000
From:   Lee Jones <lee.jones@...aro.org>
To:     Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc:     linux-kernel@...r.kernel.org, Doug Anderson <dianders@...omium.org>
Subject: Re: [PATCH] mfd: cros ec: spi: Increase wait time to 200ms

On Wed, 01 Mar 2017, Enric Balletbo i Serra wrote:

> From: Doug Anderson <dianders@...omium.org>
> 
> This is a sucky change to bump up the time we'll wait for the EC.  Why
> is it sucky?  If 200ms for a transfer is a common thing it will have a
> massively bad impact on keyboard responsiveness.
> 
> It still seems like a good idea to do this, though, because we have a
> gas gauge that claims that in an extreme case it could stretch the i2c
> clock for 144ms.  It's not a common case so it shouldn't affect
> responsiveness, but it can happen.  It's much better to have a single
> slow keyboard response than to start returning errors when we don't
> have to.
> 
> In newer EC designs we should probably implement a virtual battery to
> respond to the kernel to insulate the kernel from these types of
> issues.
> 
> Signed-off-by: Doug Anderson <dianders@...omium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>
> ---
>  This fixes a random BUG report in cros-ec-spi in my rk3399 gru device.
> 
>  drivers/mfd/cros_ec_spi.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index a518832..c971407 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -45,8 +45,11 @@
>   * on the other end and need to transfer ~256 bytes, then we need:
>   *  10 us/bit * ~10 bits/byte * ~256 bytes = ~25ms
>   *
> - * We'll wait 4 times that to handle clock stretching and other
> - * paranoia.
> + * We'll wait 8 times that to handle clock stretching and other
> + * paranoia.  Note that some battery gas gauge ICs claim to have a
> + * clock stretch of 144ms in rare situations.  That's incentive for
> + * not directly passing i2c through, but it's too late for that for
> + * existing hardware.
>   *
>   * It's pretty unlikely that we'll really see a 249 byte tunnel in
>   * anything other than testing.  If this was more common we might
> @@ -54,7 +57,7 @@
>   * wait loop.  The 'flash write' command would be another candidate
>   * for this, clocking in at 2-3ms.
>   */
> -#define EC_MSG_DEADLINE_MS		100
> +#define EC_MSG_DEADLINE_MS		200
>  
>  /*
>    * Time between raising the SPI chip select (for the end of a

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