[PATCH v1 0/2] spi: More refacroings after multi-CS support

[PATCH v1 0/2] spi: More refacroings after multi-CS support

[Andy Shevchenko]

A couple of additional refactorings on top of the multi-CS support.
One is to make sure that the comment and the code are not disrupted
if additional changes come in the future and second one is f or the
sake of deduplication. In both cases it also makes indentation level
smaller in the affected pieces of the code.

No functional changes intended.

Andy Shevchenko (2):
  spi: Extract spi_toggle_csgpiod() helper for better maintanance
  spi: Introduce spi_for_each_valid_cs() in order of deduplication

 drivers/spi/spi.c | 63 +++++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 1/2] spi: Extract spi_toggle_csgpiod() helper for better maintanance

[Andy Shevchenko]

The multi-CS support splits the comment and the code in the spi_set_cs().
To avoid this in the future extract spi_toggle_csgpiod() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c | 49 ++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ff75838c1b5d..4e40efd25aec 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1036,6 +1036,29 @@ static inline bool spi_is_last_cs(struct spi_device *spi)
 	return last;
 }
 
+static void spi_toggle_csgpiod(struct spi_device *spi, u8 idx, bool enable, bool activate)
+{
+	/*
+	 * Historically ACPI has no means of the GPIO polarity and
+	 * thus the SPISerialBus() resource defines it on the per-chip
+	 * basis. In order to avoid a chain of negations, the GPIO
+	 * polarity is considered being Active High. Even for the cases
+	 * when _DSD() is involved (in the updated versions of ACPI)
+	 * the GPIO CS polarity must be defined Active High to avoid
+	 * ambiguity. That's why we use enable, that takes SPI_CS_HIGH
+	 * into account.
+	 */
+	if (has_acpi_companion(&spi->dev))
+		gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx), !enable);
+	else
+		/* Polarity handled by GPIO library */
+		gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx), activate);
+
+	if (activate)
+		spi_delay_exec(&spi->cs_setup, NULL);
+	else
+		spi_delay_exec(&spi->cs_inactive, NULL);
+}
 
 static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 {
@@ -1072,31 +1095,9 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 
 	if (spi_is_csgpiod(spi)) {
 		if (!(spi->mode & SPI_NO_CS)) {
-			/*
-			 * Historically ACPI has no means of the GPIO polarity and
-			 * thus the SPISerialBus() resource defines it on the per-chip
-			 * basis. In order to avoid a chain of negations, the GPIO
-			 * polarity is considered being Active High. Even for the cases
-			 * when _DSD() is involved (in the updated versions of ACPI)
-			 * the GPIO CS polarity must be defined Active High to avoid
-			 * ambiguity. That's why we use enable, that takes SPI_CS_HIGH
-			 * into account.
-			 */
 			for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
-				if ((spi->cs_index_mask & BIT(idx)) && spi_get_csgpiod(spi, idx)) {
-					if (has_acpi_companion(&spi->dev))
-						gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx),
-									 !enable);
-					else
-						/* Polarity handled by GPIO library */
-						gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx),
-									 activate);
-
-					if (activate)
-						spi_delay_exec(&spi->cs_setup, NULL);
-					else
-						spi_delay_exec(&spi->cs_inactive, NULL);
-				}
+				if ((spi->cs_index_mask & BIT(idx)) && spi_get_csgpiod(spi, idx))
+					spi_toggle_csgpiod(spi, idx, enable, activate);
 			}
 		}
 		/* Some SPI masters need both GPIO CS & slave_select */
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v1 2/2] spi: Introduce spi_for_each_valid_cs() in order of deduplication

[Andy Shevchenko]

In order of deduplication and better maintenance introduce a new
spi_for_each_valid_cs() helper macro.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 4e40efd25aec..ffaa9dce8304 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1022,16 +1022,18 @@ 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++)		\
+		if (!(spi->cs_index_mask & BIT(idx))) {} else
+
 static inline bool spi_is_last_cs(struct spi_device *spi)
 {
 	u8 idx;
 	bool last = false;
 
-	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
-		if (spi->cs_index_mask & BIT(idx)) {
-			if (spi->controller->last_cs[idx] == spi_get_chipselect(spi, idx))
-				last = true;
-		}
+	spi_for_each_valid_cs(spi, idx) {
+		if (spi->controller->last_cs[idx] == spi_get_chipselect(spi, idx))
+			last = true;
 	}
 	return last;
 }
@@ -1095,8 +1097,8 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 
 	if (spi_is_csgpiod(spi)) {
 		if (!(spi->mode & SPI_NO_CS)) {
-			for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
-				if ((spi->cs_index_mask & BIT(idx)) && spi_get_csgpiod(spi, idx))
+			spi_for_each_valid_cs(spi, idx) {
+				if (spi_get_csgpiod(spi, idx))
 					spi_toggle_csgpiod(spi, idx, enable, activate);
 			}
 		}
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 0/2] spi: More refacroings after multi-CS support

[Mark Brown]

On Mon, 15 Apr 2024 22:31:18 +0300, Andy Shevchenko wrote:
> A couple of additional refactorings on top of the multi-CS support.
> One is to make sure that the comment and the code are not disrupted
> if additional changes come in the future and second one is f or the
> sake of deduplication. In both cases it also makes indentation level
> smaller in the affected pieces of the code.
> 
> No functional changes intended.
> 
> [...]

Applied to

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

Thanks!

[1/2] spi: Extract spi_toggle_csgpiod() helper for better maintanance
      commit: e81582c080ddec3359bc6726291e62a1ba8b7350
[2/2] spi: Introduce spi_for_each_valid_cs() in order of deduplication
      commit: d707530b1ea518e23c7aa7b50ee79231f2964da0

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