[v4,3/3] platform/chrome: cros_ec_spi: Request the SPI thread be realtime
diff mbox series

Message ID 20190515164814.258898-4-dianders@chromium.org
State New
Headers show
Series
  • spi: A better solution for cros_ec_spi reliability
Related show

Commit Message

Doug Anderson May 15, 2019, 4:48 p.m. UTC
All currently known ECs in the wild are very sensitive to timing.
Specifically the ECs are known to drop a transfer if more than 8 ms
passes from the assertion of the chip select until the transfer
finishes.

Let's use the new feature introduced in the patch (spi: Allow SPI
devices to request the pumping thread be realtime") to request the SPI
pumping thread be realtime.  This means that if we get shunted off to
the SPI thread for whatever reason we won't get downgraded to low
priority.

NOTES:
- We still need to keep ourselves as high priority since the SPI core
  doesn't guarantee that all transfers end up on the pumping thread
  (in fact, it tries pretty hard to do them in the calling context).
- If future Chrome OS ECs ever fix themselves to be less sensitive
  then we could consider adding a property (or compatible string) to
  not set this property.  For now we need it across the board.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
---

Changes in v4: None
Changes in v3:
- Updated description and variable name since we no longer force.

Changes in v2:
- Renamed variable to "force_rt_transfers".

 drivers/platform/chrome/cros_ec_spi.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Enric Balletbo i Serra May 21, 2019, 7:50 a.m. UTC | #1
Hi,

On 15/5/19 18:48, Douglas Anderson wrote:
> All currently known ECs in the wild are very sensitive to timing.
> Specifically the ECs are known to drop a transfer if more than 8 ms
> passes from the assertion of the chip select until the transfer
> finishes.
> 
> Let's use the new feature introduced in the patch (spi: Allow SPI
> devices to request the pumping thread be realtime") to request the SPI
> pumping thread be realtime.  This means that if we get shunted off to
> the SPI thread for whatever reason we won't get downgraded to low
> priority.
> 
> NOTES:
> - We still need to keep ourselves as high priority since the SPI core
>   doesn't guarantee that all transfers end up on the pumping thread
>   (in fact, it tries pretty hard to do them in the calling context).
> - If future Chrome OS ECs ever fix themselves to be less sensitive
>   then we could consider adding a property (or compatible string) to
>   not set this property.  For now we need it across the board.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>

For my own reference:

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks,
 Enric

> ---
> 
> Changes in v4: None
> Changes in v3:
> - Updated description and variable name since we no longer force.
> 
> Changes in v2:
> - Renamed variable to "force_rt_transfers".
> 
>  drivers/platform/chrome/cros_ec_spi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
> index 1e38a885c539..daf3119191c8 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -740,6 +740,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
>  
>  	spi->bits_per_word = 8;
>  	spi->mode = SPI_MODE_0;
> +	spi->rt = true;
>  	err = spi_setup(spi);
>  	if (err < 0)
>  		return err;
>
Enric Balletbo i Serra May 24, 2019, 10:26 a.m. UTC | #2
Hi,

On 15/5/19 18:48, Douglas Anderson wrote:
> All currently known ECs in the wild are very sensitive to timing.
> Specifically the ECs are known to drop a transfer if more than 8 ms
> passes from the assertion of the chip select until the transfer
> finishes.
> 
> Let's use the new feature introduced in the patch (spi: Allow SPI
> devices to request the pumping thread be realtime") to request the SPI
> pumping thread be realtime.  This means that if we get shunted off to
> the SPI thread for whatever reason we won't get downgraded to low
> priority.
> 
> NOTES:
> - We still need to keep ourselves as high priority since the SPI core
>   doesn't guarantee that all transfers end up on the pumping thread
>   (in fact, it tries pretty hard to do them in the calling context).
> - If future Chrome OS ECs ever fix themselves to be less sensitive
>   then we could consider adding a property (or compatible string) to
>   not set this property.  For now we need it across the board.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---

queued for 5.3

Thanks,
 Enric

> 
> Changes in v4: None
> Changes in v3:
> - Updated description and variable name since we no longer force.
> 
> Changes in v2:
> - Renamed variable to "force_rt_transfers".
> 
>  drivers/platform/chrome/cros_ec_spi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
> index 1e38a885c539..daf3119191c8 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -740,6 +740,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
>  
>  	spi->bits_per_word = 8;
>  	spi->mode = SPI_MODE_0;
> +	spi->rt = true;
>  	err = spi_setup(spi);
>  	if (err < 0)
>  		return err;
>

Patch
diff mbox series

diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 1e38a885c539..daf3119191c8 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -740,6 +740,7 @@  static int cros_ec_spi_probe(struct spi_device *spi)
 
 	spi->bits_per_word = 8;
 	spi->mode = SPI_MODE_0;
+	spi->rt = true;
 	err = spi_setup(spi);
 	if (err < 0)
 		return err;