From: Ovidiu Panait <ovidiu.panait@windriver.com>
Date: Mon, 14 Dec 2020 17:06:50 +0000 (+0200)
Subject: spi: spi-uclass: Fix spi_claim_bus() speed/mode setup logic
X-Git-Url: http://git.dujemihanovic.xyz/?a=commitdiff_plain;h=741280e9accd3da20650a04f716538944d878482;p=u-boot.git

spi: spi-uclass: Fix spi_claim_bus() speed/mode setup logic

Currently, when different spi slaves claim the bus consecutively using
spi_claim_bus(), spi_set_speed_mode() will only be executed on the first
two calls, leaving the bus in a bad state starting with the third call.

This patch drops spi_slave->speed member and adds caching of bus
speed/mode in dm_spi_bus struct. It also updates spi_claim_bus() to call
spi_set_speed_mode() if either speed or mode is different from what the
bus is currently configured for. Current behavior is to only take into
account the speed, but not the mode, which seems wrong.

Fixes: 60e2809a848 ("dm: spi: Avoid setting the speed with every transfer")
Reviewed-by: Simon Glass <sjg@chromium.org>
Reported-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Reported-by: Moshe, Yaniv <yanivmo@amazon.com>
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index 51b1aa4372..46800bbed2 100644
--- a/drivers/mmc/mmc_spi.c
+++ b/drivers/mmc/mmc_spi.c
@@ -418,7 +418,6 @@ static int mmc_spi_probe(struct udevice *dev)
 	priv->spi = dev_get_parent_priv(dev);
 	if (!priv->spi->max_hz)
 		priv->spi->max_hz = MMC_SPI_MAX_CLOCK;
-	priv->spi->speed = 0;
 	priv->spi->mode = SPI_MODE_0;
 	priv->spi->wordlen = 8;
 
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index f3b8ffad42..acef09d6f4 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -51,23 +51,28 @@ int dm_spi_claim_bus(struct udevice *dev)
 	struct dm_spi_ops *ops = spi_get_ops(bus);
 	struct dm_spi_bus *spi = dev_get_uclass_priv(bus);
 	struct spi_slave *slave = dev_get_parent_priv(dev);
-	int speed;
+	uint speed, mode;
 
 	speed = slave->max_hz;
+	mode = slave->mode;
+
 	if (spi->max_hz) {
 		if (speed)
-			speed = min(speed, (int)spi->max_hz);
+			speed = min(speed, spi->max_hz);
 		else
 			speed = spi->max_hz;
 	}
 	if (!speed)
 		speed = SPI_DEFAULT_SPEED_HZ;
-	if (speed != slave->speed) {
+
+	if (speed != spi->speed || mode != spi->mode) {
 		int ret = spi_set_speed_mode(bus, speed, slave->mode);
 
 		if (ret)
 			return log_ret(ret);
-		slave->speed = speed;
+
+		spi->speed = speed;
+		spi->mode = mode;
 	}
 
 	return log_ret(ops->claim_bus ? ops->claim_bus(dev) : 0);
@@ -324,6 +329,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 {
 	struct udevice *bus, *dev;
 	struct dm_spi_slave_plat *plat;
+	struct dm_spi_bus *bus_data;
 	struct spi_slave *slave;
 	bool created = false;
 	int ret;
@@ -381,12 +387,13 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
 	}
 
 	slave = dev_get_parent_priv(dev);
+	bus_data = dev_get_uclass_priv(bus);
 
 	/*
 	 * In case the operation speed is not yet established by
 	 * dm_spi_claim_bus() ensure the bus is configured properly.
 	 */
-	if (!slave->speed) {
+	if (!bus_data->speed) {
 		ret = spi_claim_bus(slave);
 		if (ret)
 			goto err;
diff --git a/include/spi.h b/include/spi.h
index a0342e3169..e81f799650 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -39,9 +39,22 @@
 
 #define SPI_DEFAULT_WORDLEN	8
 
-/* TODO(sjg@chromium.org): Remove this and use max_hz from struct spi_slave */
+/**
+ * struct dm_spi_bus - SPI bus info
+ *
+ * This contains information about a SPI bus. To obtain this structure, use
+ * dev_get_uclass_priv(bus) where bus is the SPI bus udevice.
+ *
+ * @max_hz:	Maximum speed that the bus can tolerate.
+ * @speed:	Current bus speed. This is 0 until the bus is first claimed.
+ * @mode:	Current bus mode. This is 0 until the bus is first claimed.
+ *
+ * TODO(sjg@chromium.org): Remove this and use max_hz from struct spi_slave.
+ */
 struct dm_spi_bus {
 	uint max_hz;
+	uint speed;
+	uint mode;
 };
 
 /**
@@ -112,8 +125,6 @@ enum spi_polarity {
  *
  * @dev:		SPI slave device
  * @max_hz:		Maximum speed for this slave
- * @speed:		Current bus speed. This is 0 until the bus is first
- *			claimed.
  * @bus:		ID of the bus that the slave is attached to. For
  *			driver model this is the sequence number of the SPI
  *			bus (dev_seq(bus)) so does not need to be stored
@@ -131,7 +142,6 @@ struct spi_slave {
 #if CONFIG_IS_ENABLED(DM_SPI)
 	struct udevice *dev;	/* struct spi_slave is dev->parentdata */
 	uint max_hz;
-	uint speed;
 #else
 	unsigned int bus;
 	unsigned int cs;