[PATCH 0/6] spi: spi-uclass: Fix spi_claim_bus() speed/mode setup logic

[PATCH 0/6] spi: spi-uclass: Fix spi_claim_bus() speed/mode setup logic

[Ovidiu Panait]

This patchset fixes spi_claim_bus() handling of the speed/mode settings
when multiple spi slaves claim the bus consecutively, as reported here:
https://lists.denx.de/pipermail/u-boot/2019-December/393854.html
https://lists.denx.de/pipermail/u-boot/2020-November/431554.html

It also does some minor cleanups and adds a sandbox testcase for
spi_claim_bus().


Ovidiu Panait (6):
  sandbox: spi: Drop unused sandbox_spi_parse_spec function
  sandbox: test: Add a second SPI slave on sandbox_spi bus
  spi: sandbox_spi: Implement speed/mode setup
  test: spi: Add sandbox_spi_get_{speed, mode} interface
  spi: spi-uclass: Fix spi_claim_bus() speed/mode setup logic
  test: dm: spi: Add testcase for spi_claim_bus()

 arch/sandbox/dts/test.dts       | 10 +++-
 arch/sandbox/include/asm/spi.h  | 10 ----
 arch/sandbox/include/asm/test.h | 16 +++++++
 drivers/mmc/mmc_spi.c           |  1 -
 drivers/spi/sandbox_spi.c       | 58 ++++++++++++++++-------
 drivers/spi/spi-uclass.c        | 17 +++++--
 include/spi.h                   | 18 +++++--
 test/dm/spi.c                   | 84 ++++++++++++++++++++++++++++++++-
 8 files changed, 175 insertions(+), 39 deletions(-)

-- 
2.17.1

[PATCH 1/6] sandbox: spi: Drop unused sandbox_spi_parse_spec function

[Ovidiu Panait]

Commit 1289e96797bf ("sandbox: spi: Drop command-line SPI option") dropped
support for specifying SPI devices on the command line, removing the only
user of sandbox_spi_parse_spec(). Remove the function too.

Fixes: 1289e96797bf ("sandbox: spi: Drop command-line SPI option")
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

 arch/sandbox/include/asm/spi.h | 10 ----------
 drivers/spi/sandbox_spi.c      | 16 ----------------
 2 files changed, 26 deletions(-)

diff --git a/arch/sandbox/include/asm/spi.h b/arch/sandbox/include/asm/spi.h
index 98e1826e2c..e8268bbe07 100644
--- a/arch/sandbox/include/asm/spi.h
+++ b/arch/sandbox/include/asm/spi.h
@@ -32,14 +32,4 @@ struct sandbox_spi_emu_ops {
 	int (*xfer)(void *priv, const u8 *rx, u8 *tx, uint bytes);
 };
 
-/*
- * Extract the bus/cs from the spi spec and return the start of the spi
- * client spec.  If the bus/cs are invalid for the current config, then
- * it returns NULL.
- *
- * Example: arg="0:1:foo" will set bus to 0, cs to 1, and return "foo"
- */
-const char *sandbox_spi_parse_spec(const char *arg, unsigned long *bus,
-				   unsigned long *cs);
-
 #endif
diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
index 755f176861..3640ddeeb6 100644
--- a/drivers/spi/sandbox_spi.c
+++ b/drivers/spi/sandbox_spi.c
@@ -28,22 +28,6 @@
 # define CONFIG_SPI_IDLE_VAL 0xFF
 #endif
 
-const char *sandbox_spi_parse_spec(const char *arg, unsigned long *bus,
-				   unsigned long *cs)
-{
-	char *endp;
-
-	*bus = simple_strtoul(arg, &endp, 0);
-	if (*endp != ':' || *bus >= CONFIG_SANDBOX_SPI_MAX_BUS)
-		return NULL;
-
-	*cs = simple_strtoul(endp + 1, &endp, 0);
-	if (*endp != ':' || *cs >= CONFIG_SANDBOX_SPI_MAX_CS)
-		return NULL;
-
-	return endp + 1;
-}
-
 __weak int sandbox_spi_get_emul(struct sandbox_state *state,
 				struct udevice *bus, struct udevice *slave,
 				struct udevice **emulp)
-- 
2.17.1

[PATCH 2/6] sandbox: test: Add a second SPI slave on sandbox_spi bus

[Ovidiu Panait]

Place a second spi slave on the sandbox_spi bus, to be used by the
spi_claim_bus() testcase we are about to introduce. We need to make sure
that jumping between slaves calling spi_claim_bus() sets the bus speed and
mode appropriately. Use different max-hz and mode properties for this new
slave.

Also, update sandbox_spi cs_info call to allow activity on CS0/CS1 and
adapt dm_test_spi_find() testcase for this new setup.

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

 arch/sandbox/dts/test.dts | 10 +++++++++-
 drivers/spi/sandbox_spi.c |  4 ++--
 test/dm/spi.c             |  2 +-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index f3b766271d..3eca1a73de 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -864,13 +864,21 @@
 		#size-cells = <0>;
 		reg = <0 1>;
 		compatible = "sandbox,spi";
-		cs-gpios = <0>, <&gpio_a 0>;
+		cs-gpios = <0>, <0>, <&gpio_a 0>;
 		spi.bin at 0 {
 			reg = <0>;
 			compatible = "spansion,m25p16", "jedec,spi-nor";
 			spi-max-frequency = <40000000>;
 			sandbox,filename = "spi.bin";
 		};
+		spi.bin at 1 {
+			reg = <1>;
+			compatible = "spansion,m25p16", "jedec,spi-nor";
+			spi-max-frequency = <50000000>;
+			sandbox,filename = "spi.bin";
+			spi-cpol;
+			spi-cpha;
+		};
 	};
 
 	syscon0: syscon at 0 {
diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
index 3640ddeeb6..412756aa4b 100644
--- a/drivers/spi/sandbox_spi.c
+++ b/drivers/spi/sandbox_spi.c
@@ -101,8 +101,8 @@ static int sandbox_spi_set_mode(struct udevice *bus, uint mode)
 static int sandbox_cs_info(struct udevice *bus, uint cs,
 			   struct spi_cs_info *info)
 {
-	/* Always allow activity on CS 0 */
-	if (cs >= 1)
+	/* Always allow activity on CS 0, CS 1 */
+	if (cs >= 2)
 		return -EINVAL;
 
 	return 0;
diff --git a/test/dm/spi.c b/test/dm/spi.c
index fb180aed1f..6db680edd2 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -22,7 +22,7 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 	struct sandbox_state *state = state_get_current();
 	struct spi_slave *slave;
 	struct udevice *bus, *dev;
-	const int busnum = 0, cs = 0, mode = 0, speed = 1000000, cs_b = 1;
+	const int busnum = 0, cs = 0, mode = 0, speed = 1000000, cs_b = 2;
 	struct spi_cs_info info;
 	ofnode node;
 
-- 
2.17.1

[PATCH 3/6] spi: sandbox_spi: Implement speed/mode setup

[Ovidiu Panait]

Implement sandbox_spi_set_{speed, mode} routines, to be able to keep track
of the current bus speed/mode. This will help determine whether the values
passed from dm_spi_claim_bus() are valid.

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

 drivers/spi/sandbox_spi.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
index 412756aa4b..72f22d066f 100644
--- a/drivers/spi/sandbox_spi.c
+++ b/drivers/spi/sandbox_spi.c
@@ -28,6 +28,23 @@
 # define CONFIG_SPI_IDLE_VAL 0xFF
 #endif
 
+/**
+ * struct sandbox_spi_priv - Sandbox SPI private data
+ *
+ * Helper struct to keep track of the sandbox SPI bus internal state. It is
+ * used in unit tests to verify that dm spi functions update the bus
+ * speed/mode properly (for instance, when jumping back and forth between spi
+ * slaves claiming the bus, we need to make sure that the bus speed is updated
+ * accordingly for each slave).
+ *
+ * @speed:	Current bus speed.
+ * @mode:	Current bus mode.
+ */
+struct sandbox_spi_priv {
+	uint speed;
+	uint mode;
+};
+
 __weak int sandbox_spi_get_emul(struct sandbox_state *state,
 				struct udevice *bus, struct udevice *slave,
 				struct udevice **emulp)
@@ -90,11 +107,19 @@ static int sandbox_spi_xfer(struct udevice *slave, unsigned int bitlen,
 
 static int sandbox_spi_set_speed(struct udevice *bus, uint speed)
 {
+	struct sandbox_spi_priv *priv = dev_get_priv(bus);
+
+	priv->speed = speed;
+
 	return 0;
 }
 
 static int sandbox_spi_set_mode(struct udevice *bus, uint mode)
 {
+	struct sandbox_spi_priv *priv = dev_get_priv(bus);
+
+	priv->mode = mode;
+
 	return 0;
 }
 
@@ -136,4 +161,5 @@ U_BOOT_DRIVER(sandbox_spi) = {
 	.id	= UCLASS_SPI,
 	.of_match = sandbox_spi_ids,
 	.ops	= &sandbox_spi_ops,
+	.priv_auto_alloc_size = sizeof(struct sandbox_spi_priv),
 };
-- 
2.17.1

[PATCH 4/6] test: spi: Add sandbox_spi_get_{speed, mode} interface

[Ovidiu Panait]

Introduce sandbox_spi_get_{speed, mode} public interface to retrieve the
sandbox spi bus internal state. They are meant to be used in sandbox spi
testcases.

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

 arch/sandbox/include/asm/test.h | 16 ++++++++++++++++
 drivers/spi/sandbox_spi.c       | 14 ++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index 7f99d07c47..05f66f700c 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -202,6 +202,22 @@ void sandbox_set_allow_beep(struct udevice *dev, bool allow);
  */
 int sandbox_get_beep_frequency(struct udevice *dev);
 
+/**
+ * sandbox_spi_get_speed() - Get current speed setting of a sandbox spi bus
+ *
+ * @dev: Device to check
+ * @return current bus speed
+ */
+uint sandbox_spi_get_speed(struct udevice *dev);
+
+/**
+ * sandbox_spi_get_mode() - Get current mode setting of a sandbox spi bus
+ *
+ * @dev: Device to check
+ * @return current mode
+ */
+uint sandbox_spi_get_mode(struct udevice *dev);
+
 /**
  * sandbox_get_pch_spi_protect() - Get the PCI SPI protection status
  *
diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
index 72f22d066f..cbe8d62681 100644
--- a/drivers/spi/sandbox_spi.c
+++ b/drivers/spi/sandbox_spi.c
@@ -52,6 +52,20 @@ __weak int sandbox_spi_get_emul(struct sandbox_state *state,
 	return -ENOENT;
 }
 
+uint sandbox_spi_get_speed(struct udevice *dev)
+{
+	struct sandbox_spi_priv *priv = dev_get_priv(dev);
+
+	return priv->speed;
+}
+
+uint sandbox_spi_get_mode(struct udevice *dev)
+{
+	struct sandbox_spi_priv *priv = dev_get_priv(dev);
+
+	return priv->mode;
+}
+
 static int sandbox_spi_xfer(struct udevice *slave, unsigned int bitlen,
 			    const void *dout, void *din, unsigned long flags)
 {
-- 
2.17.1

[PATCH 5/6] spi: spi-uclass: Fix spi_claim_bus() speed/mode setup logic

[Ovidiu Panait]

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")
Reported-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Reported-by: Moshe, Yaniv <yanivmo@amazon.com>
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---

 drivers/mmc/mmc_spi.c    |  1 -
 drivers/spi/spi-uclass.c | 17 ++++++++++++-----
 include/spi.h            | 18 ++++++++++++++----
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/mmc_spi.c b/drivers/mmc/mmc_spi.c
index 50fcd32674..793e3c8cfa 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 55a8eed890..4bd2adeb58 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_platdata *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 ef8c1f6692..7efced7057 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -39,9 +39,22 @@
 
 #define SPI_DEFAULT_WORDLEN	8
 
-/* TODO(sjg at 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 at 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 (bus->seq) 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;
-- 
2.17.1

[PATCH 6/6] test: dm: spi: Add testcase for spi_claim_bus()

[Ovidiu Panait]

Add testcase for spi_claim_bus(), which checks that sandbox spi bus
speed/mode settings are updated correctly when multiple slaves use
the bus consecutively. The following configurations are used for the
two spi slaves involved:
  * different max_hz / different modes
  * different max_hz / same modes
  * different modes / same max_hz

asm/test.h header is added in order to be able to retrieve the current
speed/mode of the sandbox spi bus, via sandbox_spi_get_{speed, mode}.

Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>

---

 test/dm/spi.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/test/dm/spi.c b/test/dm/spi.c
index 6db680edd2..65093f6907 100644
--- a/test/dm/spi.c
+++ b/test/dm/spi.c
@@ -9,6 +9,7 @@
 #include <spi.h>
 #include <spi_flash.h>
 #include <asm/state.h>
+#include <asm/test.h>
 #include <dm/device-internal.h>
 #include <dm/test.h>
 #include <dm/uclass-internal.h>
@@ -94,6 +95,87 @@ static int dm_test_spi_find(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_spi_find, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
 
+/* dm_test_spi_switch_slaves - Helper function to check whether spi_claim_bus
+ *                             operates correctly with two spi slaves.
+ *
+ * Check that switching back and forth between two slaves claiming the bus
+ * will update dm_spi_bus->speed and sandbox_spi bus speed/mode correctly.
+ *
+ * @uts - unit test state
+ * @slave_a - first spi slave used for testing
+ * @slave_b - second spi slave used for testing
+ */
+static int dm_test_spi_switch_slaves(struct unit_test_state *uts,
+				     struct spi_slave *slave_a,
+				     struct spi_slave *slave_b)
+{
+	struct udevice *bus;
+	struct dm_spi_bus *bus_data;
+
+	/* Check that slaves are on the same bus */
+	ut_asserteq_ptr(dev_get_parent(slave_a->dev),
+			dev_get_parent(slave_b->dev));
+
+	bus = dev_get_parent(slave_a->dev);
+	bus_data = dev_get_uclass_priv(bus);
+
+	ut_assertok(spi_claim_bus(slave_a));
+	ut_asserteq(slave_a->max_hz, bus_data->speed);
+	ut_asserteq(slave_a->max_hz, sandbox_spi_get_speed(bus));
+	ut_asserteq(slave_a->mode, sandbox_spi_get_mode(bus));
+	spi_release_bus(slave_a);
+
+	ut_assertok(spi_claim_bus(slave_b));
+	ut_asserteq(slave_b->max_hz, bus_data->speed);
+	ut_asserteq(slave_b->max_hz, sandbox_spi_get_speed(bus));
+	ut_asserteq(slave_b->mode, sandbox_spi_get_mode(bus));
+	spi_release_bus(slave_b);
+
+	ut_assertok(spi_claim_bus(slave_a));
+	ut_asserteq(slave_a->max_hz, bus_data->speed);
+	ut_asserteq(slave_a->max_hz, sandbox_spi_get_speed(bus));
+	ut_asserteq(slave_a->mode, sandbox_spi_get_mode(bus));
+	spi_release_bus(slave_a);
+
+	return 0;
+}
+
+static int dm_test_spi_claim_bus(struct unit_test_state *uts)
+{
+	struct udevice *bus;
+	struct spi_slave *slave_a, *slave_b;
+	struct dm_spi_slave_platdata *slave_plat;
+	const int busnum = 0, cs_a = 0, cs_b = 1, mode = 0;
+
+	/* Get spi slave on CS0 */
+	ut_assertok(spi_get_bus_and_cs(busnum, cs_a, 1000000, mode, NULL, 0,
+				       &bus, &slave_a));
+	/* Get spi slave on CS1 */
+	ut_assertok(spi_get_bus_and_cs(busnum, cs_b, 1000000, mode, NULL, 0,
+				       &bus, &slave_b));
+
+	/* Different max_hz, different mode. */
+	ut_assert(slave_a->max_hz != slave_b->max_hz);
+	ut_assert(slave_a->mode != slave_b->mode);
+	dm_test_spi_switch_slaves(uts, slave_a, slave_b);
+
+	/* Different max_hz, same mode. */
+	slave_a->mode = slave_b->mode;
+	dm_test_spi_switch_slaves(uts, slave_a, slave_b);
+
+	/*
+	 * Same max_hz, different mode.
+	 * Restore original mode for slave_a, from platdata.
+	 */
+	slave_plat = dev_get_parent_platdata(slave_a->dev);
+	slave_a->mode = slave_plat->max_hz;
+	slave_a->max_hz = slave_b->max_hz;
+	dm_test_spi_switch_slaves(uts, slave_a, slave_b);
+
+	return 0;
+}
+DM_TEST(dm_test_spi_claim_bus, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
 /* Test that sandbox SPI works correctly */
 static int dm_test_spi_xfer(struct unit_test_state *uts)
 {
-- 
2.17.1

[PATCH 1/6] sandbox: spi: Drop unused sandbox_spi_parse_spec function

[Simon Glass]

On Sun, 6 Dec 2020 at 14:23, Ovidiu Panait <ovidiu.panait@windriver.com> wrote:
>
> Commit 1289e96797bf ("sandbox: spi: Drop command-line SPI option") dropped
> support for specifying SPI devices on the command line, removing the only
> user of sandbox_spi_parse_spec(). Remove the function too.
>
> Fixes: 1289e96797bf ("sandbox: spi: Drop command-line SPI option")
> Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
> ---
>
>  arch/sandbox/include/asm/spi.h | 10 ----------
>  drivers/spi/sandbox_spi.c      | 16 ----------------
>  2 files changed, 26 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 2/6] sandbox: test: Add a second SPI slave on sandbox_spi bus

[Simon Glass]

On Sun, 6 Dec 2020 at 14:23, Ovidiu Panait <ovidiu.panait@windriver.com> wrote:
>
> Place a second spi slave on the sandbox_spi bus, to be used by the
> spi_claim_bus() testcase we are about to introduce. We need to make sure
> that jumping between slaves calling spi_claim_bus() sets the bus speed and
> mode appropriately. Use different max-hz and mode properties for this new
> slave.
>
> Also, update sandbox_spi cs_info call to allow activity on CS0/CS1 and
> adapt dm_test_spi_find() testcase for this new setup.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
> ---
>
>  arch/sandbox/dts/test.dts | 10 +++++++++-
>  drivers/spi/sandbox_spi.c |  4 ++--
>  test/dm/spi.c             |  2 +-
>  3 files changed, 12 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 4/6] test: spi: Add sandbox_spi_get_{speed, mode} interface

[Simon Glass]

On Sun, 6 Dec 2020 at 14:23, Ovidiu Panait <ovidiu.panait@windriver.com> wrote:
>
> Introduce sandbox_spi_get_{speed, mode} public interface to retrieve the
> sandbox spi bus internal state. They are meant to be used in sandbox spi
> testcases.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
> ---
>
>  arch/sandbox/include/asm/test.h | 16 ++++++++++++++++
>  drivers/spi/sandbox_spi.c       | 14 ++++++++++++++
>  2 files changed, 30 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 6/6] test: dm: spi: Add testcase for spi_claim_bus()

[Simon Glass]

On Sun, 6 Dec 2020 at 14:23, Ovidiu Panait <ovidiu.panait@windriver.com> wrote:
>
> Add testcase for spi_claim_bus(), which checks that sandbox spi bus
> speed/mode settings are updated correctly when multiple slaves use
> the bus consecutively. The following configurations are used for the
> two spi slaves involved:
>   * different max_hz / different modes
>   * different max_hz / same modes
>   * different modes / same max_hz
>
> asm/test.h header is added in order to be able to retrieve the current
> speed/mode of the sandbox spi bus, via sandbox_spi_get_{speed, mode}.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
>
> ---
>
>  test/dm/spi.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 5/6] spi: spi-uclass: Fix spi_claim_bus() speed/mode setup logic

[Simon Glass]

On Sun, 6 Dec 2020 at 14:24, Ovidiu Panait <ovidiu.panait@windriver.com> wrote:
>
> 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")
> Reported-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Reported-by: Moshe, Yaniv <yanivmo@amazon.com>
> Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
> ---
>
>  drivers/mmc/mmc_spi.c    |  1 -
>  drivers/spi/spi-uclass.c | 17 ++++++++++++-----
>  include/spi.h            | 18 ++++++++++++++----
>  3 files changed, 26 insertions(+), 10 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>