linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal
@ 2024-10-12  9:53 Jonas Gorski
  2024-10-12  9:53 ` [PATCH 1/6] spi: fix return code when spi device has too many chipselects Jonas Gorski
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Jonas Gorski @ 2024-10-12  9:53 UTC (permalink / raw)
  To: Mark Brown, Amit Kumar Mahapatra; +Cc: linux-spi, linux-kernel

This series aims at cleaning up the current multi CS parts and removing
the CS limit per controller that was introduced with the multi CS
support.

To do this, store the assigned chip selects per device in
spi_device::num_chipselects, which allows us to use that instead of
SPI_CS_CNT_MAX for most loops, as well as remove the initialization to
and the check for SPI_INVALID_CS.

This should hopefully make it obvious that SPI_CS_CNT_MAX only limits
accesses to arrays indexed by the number of chip selects of a device, not
the controller, and we can remove the check for
spi_controller::num_chipselects being less than SPI_CS_CNT_MAX in device
registration (which was the wrong place to do that anyway).

After having done that, we can reduce SPI_CS_CNT_MAX again to 4 without
breaking devices on higher CS lines.

Finally, rename SPI_CS_CNT_MAX to SPI_DEVICE_CNT_MAX to make it more
clear that this limit only applies to devices, not controllers.

There are still more issues left, but these can be addressed in future
submissions:

* The code allows multi-cs devices for any controller, as long as the
  device does not set parallel-memories.
* No current spi controller driver handles logical chip selects other
  than the first one, and always use it, regardless what cs_index_mask
  says.
* While most spi controllers should be able to handle devices that have
  multiple cs that just get enables selectively, but not at the same
  time, there is no way to tell that to the core (ties into the above).
* There is no parallel memories/multi cs flag for devices, so any
  implementing driver needs to check the device tree node, making it
  impossible to register these kind of devices via code.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
Jonas Gorski (6):
      spi: fix return code when spi device has too many chipselects
      spi: keep track of number of chipselects in spi_device
      spi: do not initialize device chipselects to SPI_INVALID_CS
      spi: don't check spi_controller::num_chipselect when parsing a dt device
      Revert "spi: Raise limit on number of chip selects"
      spi: rename SPI_CS_CNT_MAX => SPI_DEVICE_CS_CNT_MAX

 drivers/spi/spi-cadence-quadspi.c |  2 +-
 drivers/spi/spi.c                 | 71 +++++++++++++++------------------------
 include/linux/spi/spi.h           | 17 ++++++----
 3 files changed, 39 insertions(+), 51 deletions(-)
---
base-commit: c2a59c892f20379a3e48124a83491a12374cd7e0
change-id: 20241010-spi_multi_cs_cleanup-28b1a1516933

Best regards,
-- 
Jonas Gorski <jonas.gorski@gmail.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] spi: fix return code when spi device has too many chipselects
  2024-10-12  9:53 [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal Jonas Gorski
@ 2024-10-12  9:53 ` Jonas Gorski
  2024-10-12  9:53 ` [PATCH 2/6] spi: keep track of number of chipselects in spi_device Jonas Gorski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Jonas Gorski @ 2024-10-12  9:53 UTC (permalink / raw)
  To: Mark Brown, Amit Kumar Mahapatra; +Cc: linux-spi, linux-kernel

Don't return a positive value when there are too many chipselects.

Fixes: 4d8ff6b0991d ("spi: Add multi-cs memories support in SPI core")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/spi/spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 7c5e76b154212585af1c3893005cb353a0eaeafc..ab38978e9c58517bb671d7bda017c8550cc82d58 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2452,7 +2452,7 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 	if (rc > ctlr->num_chipselect) {
 		dev_err(&ctlr->dev, "%pOF has number of CS > ctlr->num_chipselect (%d)\n",
 			nc, rc);
-		return rc;
+		return -EINVAL;
 	}
 	if ((of_property_read_bool(nc, "parallel-memories")) &&
 	    (!(ctlr->flags & SPI_CONTROLLER_MULTI_CS))) {

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/6] spi: keep track of number of chipselects in spi_device
  2024-10-12  9:53 [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal Jonas Gorski
  2024-10-12  9:53 ` [PATCH 1/6] spi: fix return code when spi device has too many chipselects Jonas Gorski
@ 2024-10-12  9:53 ` Jonas Gorski
  2024-10-12  9:53 ` [PATCH 3/6] spi: do not initialize device chipselects to SPI_INVALID_CS Jonas Gorski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Jonas Gorski @ 2024-10-12  9:53 UTC (permalink / raw)
  To: Mark Brown, Amit Kumar Mahapatra; +Cc: linux-spi, linux-kernel

There are several places where we need to iterate over a device's
chipselect. To be able to do it efficiently, store the number of
chipselects in spi_device, like we do for controllers.

Since we now use a device supplied value, add a check to make sure it
isn't more than we can support.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/spi/spi.c       | 29 +++++++++++++++++++++--------
 include/linux/spi/spi.h |  4 +++-
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ab38978e9c58517bb671d7bda017c8550cc82d58..15b2f4e02a685b2c778b27473289197ab08987d8 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -587,6 +587,7 @@ struct spi_device *spi_alloc_device(struct spi_controller *ctlr)
 	spi->dev.bus = &spi_bus_type;
 	spi->dev.release = spidev_release;
 	spi->mode = ctlr->buswidth_override_bits;
+	spi->num_chipselect = 1;
 
 	device_initialize(&spi->dev);
 	return spi;
@@ -636,7 +637,7 @@ static inline int spi_dev_check_cs(struct device *dev,
 	u8 idx_new;
 
 	cs = spi_get_chipselect(spi, idx);
-	for (idx_new = new_idx; idx_new < SPI_CS_CNT_MAX; idx_new++) {
+	for (idx_new = new_idx; idx_new < new_spi->num_chipselect; idx_new++) {
 		cs_new = spi_get_chipselect(new_spi, idx_new);
 		if (is_valid_cs(cs) && is_valid_cs(cs_new) && cs == cs_new) {
 			dev_err(dev, "chipselect %u already in use\n", cs_new);
@@ -653,7 +654,7 @@ static int spi_dev_check(struct device *dev, void *data)
 	int status, idx;
 
 	if (spi->controller == new_spi->controller) {
-		for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
+		for (idx = 0; idx < spi->num_chipselect; idx++) {
 			status = spi_dev_check_cs(dev, spi, idx, new_spi, 0);
 			if (status)
 				return status;
@@ -675,7 +676,13 @@ static int __spi_add_device(struct spi_device *spi)
 	int status, idx;
 	u8 cs;
 
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
+	if (spi->num_chipselect > SPI_CS_CNT_MAX) {
+		dev_err(dev, "num_cs %d > max %d\n", spi->num_chipselect,
+			SPI_CS_CNT_MAX);
+		return -EOVERFLOW;
+	}
+
+	for (idx = 0; idx < spi->num_chipselect; idx++) {
 		/* Chipselects are numbered 0..max; validate. */
 		cs = spi_get_chipselect(spi, idx);
 		if (is_valid_cs(cs) && cs >= ctlr->num_chipselect) {
@@ -690,7 +697,7 @@ static int __spi_add_device(struct spi_device *spi)
 	 * For example, spi->chip_select[0] != spi->chip_select[1] and so on.
 	 */
 	if (!spi_controller_is_target(ctlr)) {
-		for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
+		for (idx = 0; idx < spi->num_chipselect; idx++) {
 			status = spi_dev_check_cs(dev, spi, idx, spi, idx + 1);
 			if (status)
 				return status;
@@ -718,7 +725,7 @@ static int __spi_add_device(struct spi_device *spi)
 	if (ctlr->cs_gpiods) {
 		u8 cs;
 
-		for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
+		for (idx = 0; idx < spi->num_chipselect; idx++) {
 			cs = spi_get_chipselect(spi, idx);
 			if (is_valid_cs(cs))
 				spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[cs]);
@@ -1025,7 +1032,7 @@ static void spi_res_release(struct spi_controller *ctlr, struct spi_message *mes
 
 /*-------------------------------------------------------------------------*/
 #define spi_for_each_valid_cs(spi, idx)				\
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)		\
+	for (idx = 0; idx < spi->num_chipselect; idx++)		\
 		if (!(spi->cs_index_mask & BIT(idx))) {} else
 
 static inline bool spi_is_last_cs(struct spi_device *spi)
@@ -1083,8 +1090,12 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 	trace_spi_set_cs(spi, activate);
 
 	spi->controller->last_cs_index_mask = spi->cs_index_mask;
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)
-		spi->controller->last_cs[idx] = enable ? spi_get_chipselect(spi, 0) : SPI_INVALID_CS;
+	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
+		if (enable && idx < spi->num_chipselect)
+			spi->controller->last_cs[idx] = spi_get_chipselect(spi, 0);
+		else
+			spi->controller->last_cs[idx] = SPI_INVALID_CS;
+	}
 	spi->controller->last_cs_mode_high = spi->mode & SPI_CS_HIGH;
 
 	if (spi->mode & SPI_CS_HIGH)
@@ -2459,6 +2470,8 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 		dev_err(&ctlr->dev, "SPI controller doesn't support multi CS\n");
 		return -EINVAL;
 	}
+
+	spi->num_chipselect = rc;
 	for (idx = 0; idx < rc; idx++)
 		spi_set_chipselect(spi, idx, cs[idx]);
 
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 8497f4747e24d4ecd85b74f49609ac1c82c73535..77511c7d40df7085644cecaae325c982fb306afa 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -136,6 +136,7 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg,
  *	The spi_transfer.speed_hz can override this for each transfer.
  * @chip_select: Array of physical chipselect, spi->chipselect[i] gives
  *	the corresponding physical CS for logical CS i.
+ * @num_chipselect: Number of physical chipselects used.
  * @mode: The spi mode defines how data is clocked out and in.
  *	This may be changed by the device's driver.
  *	The "active low" default for chipselect mode can be overridden
@@ -186,6 +187,7 @@ struct spi_device {
 	struct spi_controller	*controller;
 	u32			max_speed_hz;
 	u8			chip_select[SPI_CS_CNT_MAX];
+	u8			num_chipselect;
 	u8			bits_per_word;
 	bool			rt;
 #define SPI_NO_TX		BIT(31)		/* No transmit wire */
@@ -311,7 +313,7 @@ static inline bool spi_is_csgpiod(struct spi_device *spi)
 {
 	u8 idx;
 
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
+	for (idx = 0; idx < spi->num_chipselect; idx++) {
 		if (spi_get_csgpiod(spi, idx))
 			return true;
 	}

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/6] spi: do not initialize device chipselects to SPI_INVALID_CS
  2024-10-12  9:53 [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal Jonas Gorski
  2024-10-12  9:53 ` [PATCH 1/6] spi: fix return code when spi device has too many chipselects Jonas Gorski
  2024-10-12  9:53 ` [PATCH 2/6] spi: keep track of number of chipselects in spi_device Jonas Gorski
@ 2024-10-12  9:53 ` Jonas Gorski
  2024-10-12 10:27   ` Mark Brown
  2024-10-12  9:53 ` [PATCH 4/6] spi: don't check spi_controller::num_chipselect when parsing a dt device Jonas Gorski
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Jonas Gorski @ 2024-10-12  9:53 UTC (permalink / raw)
  To: Mark Brown, Amit Kumar Mahapatra; +Cc: linux-spi, linux-kernel

Now that we know the number of valid chipselects, we don't need to
initialize them to SPI_INVALID_CS to be able to know if they are valid,
so we can drop both the initialization to SPI_INVALID_CS, as well as the
check for it.

We cannot drop the define itself though, since it is still used for
spi_controller::last_cs[]'s state.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/spi/spi.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 15b2f4e02a685b2c778b27473289197ab08987d8..495b391710d69a46beaa56c1a4332eb6677d2f45 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -624,11 +624,6 @@ static void spi_dev_set_name(struct spi_device *spi)
  */
 #define SPI_INVALID_CS		((s8)-1)
 
-static inline bool is_valid_cs(s8 chip_select)
-{
-	return chip_select != SPI_INVALID_CS;
-}
-
 static inline int spi_dev_check_cs(struct device *dev,
 				   struct spi_device *spi, u8 idx,
 				   struct spi_device *new_spi, u8 new_idx)
@@ -639,7 +634,7 @@ static inline int spi_dev_check_cs(struct device *dev,
 	cs = spi_get_chipselect(spi, idx);
 	for (idx_new = new_idx; idx_new < new_spi->num_chipselect; idx_new++) {
 		cs_new = spi_get_chipselect(new_spi, idx_new);
-		if (is_valid_cs(cs) && is_valid_cs(cs_new) && cs == cs_new) {
+		if (cs == cs_new) {
 			dev_err(dev, "chipselect %u already in use\n", cs_new);
 			return -EBUSY;
 		}
@@ -685,7 +680,7 @@ static int __spi_add_device(struct spi_device *spi)
 	for (idx = 0; idx < spi->num_chipselect; idx++) {
 		/* Chipselects are numbered 0..max; validate. */
 		cs = spi_get_chipselect(spi, idx);
-		if (is_valid_cs(cs) && cs >= ctlr->num_chipselect) {
+		if (cs >= ctlr->num_chipselect) {
 			dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, idx),
 				ctlr->num_chipselect);
 			return -EINVAL;
@@ -727,8 +722,7 @@ static int __spi_add_device(struct spi_device *spi)
 
 		for (idx = 0; idx < spi->num_chipselect; idx++) {
 			cs = spi_get_chipselect(spi, idx);
-			if (is_valid_cs(cs))
-				spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[cs]);
+			spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[cs]);
 		}
 	}
 
@@ -781,14 +775,6 @@ int spi_add_device(struct spi_device *spi)
 }
 EXPORT_SYMBOL_GPL(spi_add_device);
 
-static void spi_set_all_cs_unused(struct spi_device *spi)
-{
-	u8 idx;
-
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)
-		spi_set_chipselect(spi, idx, SPI_INVALID_CS);
-}
-
 /**
  * spi_new_device - instantiate one new SPI device
  * @ctlr: Controller to which device is connected
@@ -824,7 +810,6 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr,
 	WARN_ON(strlen(chip->modalias) >= sizeof(proxy->modalias));
 
 	/* Use provided chip-select for proxy device */
-	spi_set_all_cs_unused(proxy);
 	spi_set_chipselect(proxy, 0, chip->chip_select);
 
 	proxy->max_speed_hz = chip->max_speed_hz;
@@ -2450,8 +2435,6 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 		return -EINVAL;
 	}
 
-	spi_set_all_cs_unused(spi);
-
 	/* Device address */
 	rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1,
 						 SPI_CS_CNT_MAX);
@@ -2596,7 +2579,6 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi,
 	strscpy(ancillary->modalias, "dummy", sizeof(ancillary->modalias));
 
 	/* Use provided chip-select for ancillary device */
-	spi_set_all_cs_unused(ancillary);
 	spi_set_chipselect(ancillary, 0, chip_select);
 
 	/* Take over SPI mode/speed from SPI main device */
@@ -2844,7 +2826,6 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	spi_set_all_cs_unused(spi);
 	spi_set_chipselect(spi, 0, lookup.chip_select);
 
 	ACPI_COMPANION_SET(&spi->dev, adev);

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/6] spi: don't check spi_controller::num_chipselect when parsing a dt device
  2024-10-12  9:53 [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal Jonas Gorski
                   ` (2 preceding siblings ...)
  2024-10-12  9:53 ` [PATCH 3/6] spi: do not initialize device chipselects to SPI_INVALID_CS Jonas Gorski
@ 2024-10-12  9:53 ` Jonas Gorski
  2024-10-12  9:53 ` [PATCH 5/6] Revert "spi: Raise limit on number of chip selects" Jonas Gorski
  2024-10-12  9:53 ` [PATCH 6/6] spi: rename SPI_CS_CNT_MAX => SPI_DEVICE_CS_CNT_MAX Jonas Gorski
  5 siblings, 0 replies; 9+ messages in thread
From: Jonas Gorski @ 2024-10-12  9:53 UTC (permalink / raw)
  To: Mark Brown, Amit Kumar Mahapatra; +Cc: linux-spi, linux-kernel

Do not validate spi_controller::num_chipselect against SPI_CS_CNT_MAX
when parsing an spi device firmware node.

Firstly this is the wrong place, and this should be done while
registering/validating the controller. Secondly, there is no reason for
that check, as SPI_CS_CNT_MAX controls the amount of chipselects a
device may have, not a controller may have.

So drop that check as it needlessly limits controllers to SPI_CS_CNT_MAX
number of chipselects.

Likewise, drop the check for number of device chipselects larger than
controller's number of chipselects, as __spi_add_device() will already
catch that as either one of the chip selects will be out of range, or
there is a duplicate one.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/spi/spi.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 495b391710d69a46beaa56c1a4332eb6677d2f45..ebf8bc9c2276a6f50ba6e9fded48c870c4bd5ff0 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2430,11 +2430,6 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 		return 0;
 	}
 
-	if (ctlr->num_chipselect > SPI_CS_CNT_MAX) {
-		dev_err(&ctlr->dev, "No. of CS is more than max. no. of supported CS\n");
-		return -EINVAL;
-	}
-
 	/* Device address */
 	rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1,
 						 SPI_CS_CNT_MAX);
@@ -2443,11 +2438,7 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 			nc, rc);
 		return rc;
 	}
-	if (rc > ctlr->num_chipselect) {
-		dev_err(&ctlr->dev, "%pOF has number of CS > ctlr->num_chipselect (%d)\n",
-			nc, rc);
-		return -EINVAL;
-	}
+
 	if ((of_property_read_bool(nc, "parallel-memories")) &&
 	    (!(ctlr->flags & SPI_CONTROLLER_MULTI_CS))) {
 		dev_err(&ctlr->dev, "SPI controller doesn't support multi CS\n");

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/6] Revert "spi: Raise limit on number of chip selects"
  2024-10-12  9:53 [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal Jonas Gorski
                   ` (3 preceding siblings ...)
  2024-10-12  9:53 ` [PATCH 4/6] spi: don't check spi_controller::num_chipselect when parsing a dt device Jonas Gorski
@ 2024-10-12  9:53 ` Jonas Gorski
  2024-10-12 10:28   ` Mark Brown
  2024-10-12  9:53 ` [PATCH 6/6] spi: rename SPI_CS_CNT_MAX => SPI_DEVICE_CS_CNT_MAX Jonas Gorski
  5 siblings, 1 reply; 9+ messages in thread
From: Jonas Gorski @ 2024-10-12  9:53 UTC (permalink / raw)
  To: Mark Brown, Amit Kumar Mahapatra; +Cc: linux-spi, linux-kernel

Now that we removed the limit of chip selects for the controller, we can
reduce the amount of chip selects per device again to its original
value.

This reverts commit 2f8c7c3715f2c6fb51a4ecc0905c04dd78a3da29.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 include/linux/spi/spi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 77511c7d40df7085644cecaae325c982fb306afa..fe99f46c7d926eeb75398f4dddc5ef64d8f7736e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -21,7 +21,7 @@
 #include <uapi/linux/spi/spi.h>
 
 /* Max no. of CS supported per spi device */
-#define SPI_CS_CNT_MAX 16
+#define SPI_CS_CNT_MAX 4
 
 struct dma_chan;
 struct software_node;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/6] spi: rename SPI_CS_CNT_MAX => SPI_DEVICE_CS_CNT_MAX
  2024-10-12  9:53 [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal Jonas Gorski
                   ` (4 preceding siblings ...)
  2024-10-12  9:53 ` [PATCH 5/6] Revert "spi: Raise limit on number of chip selects" Jonas Gorski
@ 2024-10-12  9:53 ` Jonas Gorski
  5 siblings, 0 replies; 9+ messages in thread
From: Jonas Gorski @ 2024-10-12  9:53 UTC (permalink / raw)
  To: Mark Brown, Amit Kumar Mahapatra; +Cc: linux-spi, linux-kernel

Rename SPI_CS_CNT_MAX to SPI_DEVICE_CS_CNT_MAX to make it more obvious
that this is the max number of CS per device supported, not per
controller.

Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/spi/spi-cadence-quadspi.c |  2 +-
 drivers/spi/spi.c                 | 12 ++++++------
 include/linux/spi/spi.h           | 13 +++++++------
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 0b45b7b2b3ab30951d94ea2ce57dcba3a2600847..3b5776c80c6adfdb4ff67f03b065f0bb81fb735b 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -33,7 +33,7 @@
 #define CQSPI_NAME			"cadence-qspi"
 #define CQSPI_MAX_CHIPSELECT		4
 
-static_assert(CQSPI_MAX_CHIPSELECT <= SPI_CS_CNT_MAX);
+static_assert(CQSPI_MAX_CHIPSELECT <= SPI_DEVICE_CS_CNT_MAX);
 
 /* Quirks */
 #define CQSPI_NEEDS_WR_DELAY		BIT(0)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ebf8bc9c2276a6f50ba6e9fded48c870c4bd5ff0..a147975cec452bc857f836ce9742da0b6ddfcff2 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -671,9 +671,9 @@ static int __spi_add_device(struct spi_device *spi)
 	int status, idx;
 	u8 cs;
 
-	if (spi->num_chipselect > SPI_CS_CNT_MAX) {
+	if (spi->num_chipselect > SPI_DEVICE_CS_CNT_MAX) {
 		dev_err(dev, "num_cs %d > max %d\n", spi->num_chipselect,
-			SPI_CS_CNT_MAX);
+			SPI_DEVICE_CS_CNT_MAX);
 		return -EOVERFLOW;
 	}
 
@@ -1075,7 +1075,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 	trace_spi_set_cs(spi, activate);
 
 	spi->controller->last_cs_index_mask = spi->cs_index_mask;
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
+	for (idx = 0; idx < SPI_DEVICE_CS_CNT_MAX; idx++) {
 		if (enable && idx < spi->num_chipselect)
 			spi->controller->last_cs[idx] = spi_get_chipselect(spi, 0);
 		else
@@ -2357,7 +2357,7 @@ static void of_spi_parse_dt_cs_delay(struct device_node *nc,
 static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 			   struct device_node *nc)
 {
-	u32 value, cs[SPI_CS_CNT_MAX];
+	u32 value, cs[SPI_DEVICE_CS_CNT_MAX];
 	int rc, idx;
 
 	/* Mode (clock phase/polarity/etc.) */
@@ -2432,7 +2432,7 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 
 	/* Device address */
 	rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1,
-						 SPI_CS_CNT_MAX);
+						 SPI_DEVICE_CS_CNT_MAX);
 	if (rc < 0) {
 		dev_err(&ctlr->dev, "%pOF has no valid 'reg' property (%d)\n",
 			nc, rc);
@@ -3316,7 +3316,7 @@ int spi_register_controller(struct spi_controller *ctlr)
 	}
 
 	/* Setting last_cs to SPI_INVALID_CS means no chip selected */
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)
+	for (idx = 0; idx < SPI_DEVICE_CS_CNT_MAX; idx++)
 		ctlr->last_cs[idx] = SPI_INVALID_CS;
 
 	status = device_add(&ctlr->dev);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index fe99f46c7d926eeb75398f4dddc5ef64d8f7736e..cdf8e4338f3850075123eec76db75a94033ccaaa 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -21,7 +21,7 @@
 #include <uapi/linux/spi/spi.h>
 
 /* Max no. of CS supported per spi device */
-#define SPI_CS_CNT_MAX 4
+#define SPI_DEVICE_CS_CNT_MAX 4
 
 struct dma_chan;
 struct software_node;
@@ -186,7 +186,7 @@ struct spi_device {
 	struct device		dev;
 	struct spi_controller	*controller;
 	u32			max_speed_hz;
-	u8			chip_select[SPI_CS_CNT_MAX];
+	u8			chip_select[SPI_DEVICE_CS_CNT_MAX];
 	u8			num_chipselect;
 	u8			bits_per_word;
 	bool			rt;
@@ -218,7 +218,8 @@ struct spi_device {
 	void			*controller_data;
 	char			modalias[SPI_NAME_SIZE];
 	const char		*driver_override;
-	struct gpio_desc	*cs_gpiod[SPI_CS_CNT_MAX];	/* Chip select gpio desc */
+	/* Chip select gpio desc */
+	struct gpio_desc	*cs_gpiod[SPI_DEVICE_CS_CNT_MAX];
 	struct spi_delay	word_delay; /* Inter-word delay */
 	/* CS delays */
 	struct spi_delay	cs_setup;
@@ -233,7 +234,7 @@ struct spi_device {
 	 * multiple chip selects & memories are connected in parallel
 	 * then more than one bit need to be set in cs_index_mask.
 	 */
-	u32			cs_index_mask : SPI_CS_CNT_MAX;
+	u32			cs_index_mask : SPI_DEVICE_CS_CNT_MAX;
 
 	/*
 	 * Likely need more hooks for more protocol options affecting how
@@ -711,8 +712,8 @@ struct spi_controller {
 	bool				auto_runtime_pm;
 	bool                            fallback;
 	bool				last_cs_mode_high;
-	s8				last_cs[SPI_CS_CNT_MAX];
-	u32				last_cs_index_mask : SPI_CS_CNT_MAX;
+	s8				last_cs[SPI_DEVICE_CS_CNT_MAX];
+	u32				last_cs_index_mask : SPI_DEVICE_CS_CNT_MAX;
 	struct completion               xfer_completion;
 	size_t				max_dma_len;
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/6] spi: do not initialize device chipselects to SPI_INVALID_CS
  2024-10-12  9:53 ` [PATCH 3/6] spi: do not initialize device chipselects to SPI_INVALID_CS Jonas Gorski
@ 2024-10-12 10:27   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2024-10-12 10:27 UTC (permalink / raw)
  To: Jonas Gorski; +Cc: Amit Kumar Mahapatra, linux-spi, linux-kernel

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

On Sat, Oct 12, 2024 at 11:53:37AM +0200, Jonas Gorski wrote:
> Now that we know the number of valid chipselects, we don't need to
> initialize them to SPI_INVALID_CS to be able to know if they are valid,
> so we can drop both the initialization to SPI_INVALID_CS, as well as the
> check for it.

Dropping the check is fine but it seems useful to set a poison value to
unused chip selects for robustness.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/6] Revert "spi: Raise limit on number of chip selects"
  2024-10-12  9:53 ` [PATCH 5/6] Revert "spi: Raise limit on number of chip selects" Jonas Gorski
@ 2024-10-12 10:28   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2024-10-12 10:28 UTC (permalink / raw)
  To: Jonas Gorski; +Cc: Amit Kumar Mahapatra, linux-spi, linux-kernel

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

On Sat, Oct 12, 2024 at 11:53:39AM +0200, Jonas Gorski wrote:
> Now that we removed the limit of chip selects for the controller, we can
> reduce the amount of chip selects per device again to its original
> value.
> 
> This reverts commit 2f8c7c3715f2c6fb51a4ecc0905c04dd78a3da29.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-10-12 10:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-12  9:53 [PATCH 0/6] spi: multi CS cleanup and controller CS limit removal Jonas Gorski
2024-10-12  9:53 ` [PATCH 1/6] spi: fix return code when spi device has too many chipselects Jonas Gorski
2024-10-12  9:53 ` [PATCH 2/6] spi: keep track of number of chipselects in spi_device Jonas Gorski
2024-10-12  9:53 ` [PATCH 3/6] spi: do not initialize device chipselects to SPI_INVALID_CS Jonas Gorski
2024-10-12 10:27   ` Mark Brown
2024-10-12  9:53 ` [PATCH 4/6] spi: don't check spi_controller::num_chipselect when parsing a dt device Jonas Gorski
2024-10-12  9:53 ` [PATCH 5/6] Revert "spi: Raise limit on number of chip selects" Jonas Gorski
2024-10-12 10:28   ` Mark Brown
2024-10-12  9:53 ` [PATCH 6/6] spi: rename SPI_CS_CNT_MAX => SPI_DEVICE_CS_CNT_MAX Jonas Gorski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).