We need to wait *after* asserting CS and before the transfer, not before
asserting CS which isn't very useful.
Fixes: 25093bdeb6bc ("spi: implement SW control for CS times")
Signed-off-by: Hector Martin <[email protected]>
---
drivers/spi/spi.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b23e675953e1..cfb708d928b5 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -947,12 +947,9 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
spi->controller->last_cs_enable = enable;
spi->controller->last_cs_mode_high = spi->mode & SPI_CS_HIGH;
- if (spi->cs_gpiod || gpio_is_valid(spi->cs_gpio) ||
- !spi->controller->set_cs_timing) {
- if (activate)
- spi_delay_exec(&spi->cs_setup, NULL);
- else
- spi_delay_exec(&spi->cs_hold, NULL);
+ if ((spi->cs_gpiod || gpio_is_valid(spi->cs_gpio) ||
+ !spi->controller->set_cs_timing) && !activate) {
+ spi_delay_exec(&spi->cs_hold, NULL);
}
if (spi->mode & SPI_CS_HIGH)
@@ -994,7 +991,9 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
if (spi->cs_gpiod || gpio_is_valid(spi->cs_gpio) ||
!spi->controller->set_cs_timing) {
- if (!activate)
+ if (activate)
+ spi_delay_exec(&spi->cs_setup, NULL);
+ else
spi_delay_exec(&spi->cs_inactive, NULL);
}
}
--
2.33.0
On Sat, Dec 11, 2021 at 12:36:34AM +0900, Hector Martin wrote:
> We need to wait *after* asserting CS and before the transfer, not before
> asserting CS which isn't very useful.
This needs a better changelog, there's multiple delays being handled
here and it's not clear from this which are affected here or what the
problem is.
On 11/12/2021 01.21, Mark Brown wrote:
> On Sat, Dec 11, 2021 at 12:36:34AM +0900, Hector Martin wrote:
>
>> We need to wait *after* asserting CS and before the transfer, not before
>> asserting CS which isn't very useful.
>
> This needs a better changelog, there's multiple delays being handled
> here and it's not clear from this which are affected here or what the
> problem is.
cs_setup is affected, I thought at least that was clear from the patch
summary :-)
From spi.h:
* @cs_setup: delay to be introduced by the controller after CS is asserted
So clearly that delay has to happen at the end of spi_set_cs, *after*
the assertion, not at the beginning before it.
If you prefer, I can resend it with a reference to the spi.h comment.
--
Hector Martin ([email protected])
Public Key: https://mrcn.st/pub
On Sat, Dec 11, 2021 at 01:32:06AM +0900, Hector Martin wrote:
> On 11/12/2021 01.21, Mark Brown wrote:
> > This needs a better changelog, there's multiple delays being handled
> > here and it's not clear from this which are affected here or what the
> > problem is.
> cs_setup is affected, I thought at least that was clear from the patch
> summary :-)
This should be in the commit message, the subject line of the e-mail
isn't super visible when people are reviewing - basically, the body of
the commit message should make sense standalone.
> If you prefer, I can resend it with a reference to the spi.h comment.
Yes, please resend with a clear commit message.