[PATCH] spi: sync up initial chipselect state

[PATCH] spi: sync up initial chipselect state

[David Bauer]

When initially probing the SPI slave device, the call for disabling an
SPI device without the SPI_CS_HIGH flag is not applied, as the
condition for checking whether or not the state to be applied equals the
one currently set evaluates to true.

This however might not necessarily be the case, as the chipselect might
be active.

Add a force flag to spi_set_cs which allows to override this
early exit condition. Set it to false everywhere except when called
from spi_setup to sync up the initial CS state.

Fixes commit d40f0b6f2e21 ("spi: Avoid setting the chip select if we don't
need to")

Signed-off-by: David Bauer <mail@david-bauer.net>
---
 drivers/spi/spi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b08efe88ccd6..927c2a28011f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -795,7 +795,7 @@ int spi_register_board_info(struct spi_board_info const *info, unsigned n)
 
 /*-------------------------------------------------------------------------*/
 
-static void spi_set_cs(struct spi_device *spi, bool enable)
+static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 {
 	bool enable1 = enable;
 
@@ -803,7 +803,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable)
 	 * Avoid calling into the driver (or doing delays) if the chip select
 	 * isn't actually changing from the last time this was called.
 	 */
-	if ((spi->controller->last_cs_enable == enable) &&
+	if (!force && (spi->controller->last_cs_enable == enable) &&
 	    (spi->controller->last_cs_mode_high == (spi->mode & SPI_CS_HIGH)))
 		return;
 
@@ -1253,7 +1253,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 	struct spi_statistics *statm = &ctlr->statistics;
 	struct spi_statistics *stats = &msg->spi->statistics;
 
-	spi_set_cs(msg->spi, true);
+	spi_set_cs(msg->spi, true, false);
 
 	SPI_STATISTICS_INCREMENT_FIELD(statm, messages);
 	SPI_STATISTICS_INCREMENT_FIELD(stats, messages);
@@ -1321,9 +1321,9 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 					 &msg->transfers)) {
 				keep_cs = true;
 			} else {
-				spi_set_cs(msg->spi, false);
+				spi_set_cs(msg->spi, false, false);
 				_spi_transfer_cs_change_delay(msg, xfer);
-				spi_set_cs(msg->spi, true);
+				spi_set_cs(msg->spi, true, false);
 			}
 		}
 
@@ -1332,7 +1332,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 
 out:
 	if (ret != 0 || !keep_cs)
-		spi_set_cs(msg->spi, false);
+		spi_set_cs(msg->spi, false, false);
 
 	if (msg->status == -EINPROGRESS)
 		msg->status = ret;
@@ -3423,11 +3423,11 @@ int spi_setup(struct spi_device *spi)
 		 */
 		status = 0;
 
-		spi_set_cs(spi, false);
+		spi_set_cs(spi, false, true);
 		pm_runtime_mark_last_busy(spi->controller->dev.parent);
 		pm_runtime_put_autosuspend(spi->controller->dev.parent);
 	} else {
-		spi_set_cs(spi, false);
+		spi_set_cs(spi, false, true);
 	}
 
 	mutex_unlock(&spi->controller->io_mutex);
-- 
2.31.1

Re: [PATCH] spi: sync up initial chipselect state

[Mark Brown]

On Fri, 16 Apr 2021 21:59:56 +0200, David Bauer wrote:
> When initially probing the SPI slave device, the call for disabling an
> SPI device without the SPI_CS_HIGH flag is not applied, as the
> condition for checking whether or not the state to be applied equals the
> one currently set evaluates to true.
> 
> This however might not necessarily be the case, as the chipselect might
> be active.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: sync up initial chipselect state
      commit: d347b4aaa1a042ea528e385d9070b74c77a14321

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH] spi: sync up initial chipselect state

[Andy Shevchenko]

On Fri, Apr 16, 2021 at 11:11 PM David Bauer <mail@david-bauer.net> wrote:
>
> When initially probing the SPI slave device, the call for disabling an
> SPI device without the SPI_CS_HIGH flag is not applied, as the
> condition for checking whether or not the state to be applied equals the
> one currently set evaluates to true.
>
> This however might not necessarily be the case, as the chipselect might
> be active.
>
> Add a force flag to spi_set_cs which allows to override this
> early exit condition. Set it to false everywhere except when called
> from spi_setup to sync up the initial CS state.

> Fixes commit d40f0b6f2e21 ("spi: Avoid setting the chip select if we don't
> need to")

It should be on one line and no blank lines in the tag block.
Otherwise bots will not see it as a fix.

Mark, I guess you need to amend your scripts to check that.

> Signed-off-by: David Bauer <mail@david-bauer.net>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: sync up initial chipselect state

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

On Tue, Apr 20, 2021 at 08:31:25PM +0300, Andy Shevchenko wrote:

> Mark, I guess you need to amend your scripts to check that.

I don't check anything, the scripts that were going around were just
adding lots of noise over meaningless things and causing lots of noise.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]