[PATCH v2 00/15] spi: Header and core clean up and refactoring

[PATCH v2 00/15] spi: Header and core clean up and refactoring

[Andy Shevchenko]

Various cleanups and refactorings of the SPI header and core parts
united in a single series. It also touches drivers under SPI subsystem
folder on the pure renaming purposes of some constants.

No functional change intended.

Changelog v2:
- added new patches 3,4,5,10,13,14
- massaged comment and kernel doc in patch 9
- split used to be patch 4 to patches 11,12
- covered a few things in SPI core in patch 15
- amended commit message for above (Mark)
- reshuffled patches in the series for better logical grouping

Andy Shevchenko (15):
  spi: Remove unneeded OF node NULL checks
  spi: Drop duplicate IDR allocation code in spi_register_controller()
  spi: Replace if-else-if by bitops and multiplications
  spi: Replace open coded spi_controller_xfer_timeout()
  spi: Remove code duplication in spi_add_device_locked()
  spi: Use sysfs_emit() to instead of s*printf()
  spi: Sort headers alphabetically
  spi: Clean up headers
  spi: Use struct_size() helper
  spi: Use predefined constants from bits.h and units.h
  spi: Get rid of old SPI_MASTER_NO_TX & SPI_MASTER_NO_RX
  spi: Get rid of old SPI_MASTER_MUST_TX & SPI_MASTER_MUST_RX
  spi: Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS
  spi: Convert to SPI_CONTROLLER_HALF_DUPLEX
  spi: Fix spelling typos and acronyms capitalization

 drivers/spi/spi-amd.c             |   2 +-
 drivers/spi/spi-at91-usart.c      |   2 +-
 drivers/spi/spi-ath79.c           |   2 +-
 drivers/spi/spi-atmel.c           |   4 +-
 drivers/spi/spi-bitbang-txrx.h    |  16 +--
 drivers/spi/spi-bitbang.c         |   8 +-
 drivers/spi/spi-cavium-thunderx.c |   2 +-
 drivers/spi/spi-davinci.c         |   2 +-
 drivers/spi/spi-dw-core.c         |   2 +-
 drivers/spi/spi-falcon.c          |   2 +-
 drivers/spi/spi-fsl-lpspi.c       |   2 +-
 drivers/spi/spi-gpio.c            |  10 +-
 drivers/spi/spi-imx.c             |   2 +-
 drivers/spi/spi-lp8841-rtc.c      |  10 +-
 drivers/spi/spi-meson-spicc.c     |   2 +-
 drivers/spi/spi-mt65xx.c          |   2 +-
 drivers/spi/spi-mxs.c             |   2 +-
 drivers/spi/spi-omap-uwire.c      |   2 +-
 drivers/spi/spi-orion.c           |   2 +-
 drivers/spi/spi-pci1xxxx.c        |   2 +-
 drivers/spi/spi-pic32-sqi.c       |   2 +-
 drivers/spi/spi-pic32.c           |   2 +-
 drivers/spi/spi-qcom-qspi.c       |   2 +-
 drivers/spi/spi-rb4xx.c           |   2 +-
 drivers/spi/spi-rockchip-sfc.c    |   2 +-
 drivers/spi/spi-rockchip.c        |   2 +-
 drivers/spi/spi-sifive.c          |   2 +-
 drivers/spi/spi-slave-mt27xx.c    |   2 +-
 drivers/spi/spi-sprd-adi.c        |   2 +-
 drivers/spi/spi-stm32.c           |   2 +-
 drivers/spi/spi-ti-qspi.c         |   2 +-
 drivers/spi/spi-xcomm.c           |   2 +-
 drivers/spi/spi-xtensa-xtfpga.c   |   2 +-
 drivers/spi/spi.c                 | 204 ++++++++++++------------------
 include/linux/spi/spi.h           | 198 +++++++++++++++++------------
 include/trace/events/spi.h        |   2 +-
 36 files changed, 247 insertions(+), 261 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 01/15] spi: Remove unneeded OF node NULL checks

[Andy Shevchenko]

In the couple of places the NULL check of OF node is implied by the call
that takes it as a parameter. Drop the respective duplicate checks.

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

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9291b2a0e887..8f3282a71c63 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2399,9 +2399,6 @@ static void of_register_spi_devices(struct spi_controller *ctlr)
 	struct spi_device *spi;
 	struct device_node *nc;
 
-	if (!ctlr->dev.of_node)
-		return;
-
 	for_each_available_child_of_node(ctlr->dev.of_node, nc) {
 		if (of_node_test_and_set_flag(nc, OF_POPULATED))
 			continue;
@@ -3134,7 +3131,7 @@ int spi_register_controller(struct spi_controller *ctlr)
 		if (WARN(id < 0, "couldn't get idr"))
 			return id == -ENOSPC ? -EBUSY : id;
 		ctlr->bus_num = id;
-	} else if (ctlr->dev.of_node) {
+	} else {
 		/* Allocate dynamic bus number using Linux idr */
 		id = of_alias_get_id(ctlr->dev.of_node, "spi");
 		if (id >= 0) {
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 01/15] spi: Remove unneeded OF node NULL checks

[AngeloGioacchino Del Regno]

Il 10/07/23 17:49, Andy Shevchenko ha scritto:
> In the couple of places the NULL check of OF node is implied by the call
> that takes it as a parameter. Drop the respective duplicate checks.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Validated against spi-mt65xx, spi-mt7621, spi-mtk-nor, spi-mtk-snfi;

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> # 
MediaTek

> ---
>   drivers/spi/spi.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 9291b2a0e887..8f3282a71c63 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2399,9 +2399,6 @@ static void of_register_spi_devices(struct spi_controller *ctlr)
>   	struct spi_device *spi;
>   	struct device_node *nc;
>   
> -	if (!ctlr->dev.of_node)
> -		return;
> -
>   	for_each_available_child_of_node(ctlr->dev.of_node, nc) {
>   		if (of_node_test_and_set_flag(nc, OF_POPULATED))
>   			continue;
> @@ -3134,7 +3131,7 @@ int spi_register_controller(struct spi_controller *ctlr)
>   		if (WARN(id < 0, "couldn't get idr"))
>   			return id == -ENOSPC ? -EBUSY : id;
>   		ctlr->bus_num = id;
> -	} else if (ctlr->dev.of_node) {
> +	} else {
>   		/* Allocate dynamic bus number using Linux idr */
>   		id = of_alias_get_id(ctlr->dev.of_node, "spi");
>   		if (id >= 0) {

Re: [PATCH v2 01/15] spi: Remove unneeded OF node NULL checks

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 10:12:55AM +0200, AngeloGioacchino Del Regno wrote:
> Il 10/07/23 17:49, Andy Shevchenko ha scritto:
> > In the couple of places the NULL check of OF node is implied by the call
> > that takes it as a parameter. Drop the respective duplicate checks.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Validated against spi-mt65xx, spi-mt7621, spi-mtk-nor, spi-mtk-snfi;
> 
> Reviewed-by: AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> # MediaTek

By some reason the tag is split and I'm not sure `b4` can cope with that.
In any case, added manually. Thank you for the review!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 02/15] spi: Drop duplicate IDR allocation code in spi_register_controller()

[Andy Shevchenko]

Refactor spi_register_controller() to drop duplicate IDR allocation.
Instead of if-else-if branching use two sequential if:s, which allows
to re-use the logic of IDR allocation in all cases.

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

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8f3282a71c63..6d74218cf38e 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3081,6 +3081,20 @@ static int spi_controller_check_ops(struct spi_controller *ctlr)
 	return 0;
 }
 
+/* Allocate dynamic bus number using Linux idr */
+static int spi_controller_id_alloc(struct spi_controller *ctlr, int start, int end)
+{
+	int id;
+
+	mutex_lock(&board_lock);
+	id = idr_alloc(&spi_master_idr, ctlr, start, end, GFP_KERNEL);
+	mutex_unlock(&board_lock);
+	if (WARN(id < 0, "couldn't get idr"))
+		return id == -ENOSPC ? -EBUSY : id;
+	ctlr->bus_num = id;
+	return 0;
+}
+
 /**
  * spi_register_controller - register SPI master or slave controller
  * @ctlr: initialized master, originally from spi_alloc_master() or
@@ -3108,8 +3122,8 @@ int spi_register_controller(struct spi_controller *ctlr)
 {
 	struct device		*dev = ctlr->dev.parent;
 	struct boardinfo	*bi;
+	int			first_dynamic;
 	int			status;
-	int			id, first_dynamic;
 
 	if (!dev)
 		return -ENODEV;
@@ -3122,27 +3136,13 @@ int spi_register_controller(struct spi_controller *ctlr)
 	if (status)
 		return status;
 
+	if (ctlr->bus_num < 0)
+		ctlr->bus_num = of_alias_get_id(ctlr->dev.of_node, "spi");
 	if (ctlr->bus_num >= 0) {
 		/* Devices with a fixed bus num must check-in with the num */
-		mutex_lock(&board_lock);
-		id = idr_alloc(&spi_master_idr, ctlr, ctlr->bus_num,
-			ctlr->bus_num + 1, GFP_KERNEL);
-		mutex_unlock(&board_lock);
-		if (WARN(id < 0, "couldn't get idr"))
-			return id == -ENOSPC ? -EBUSY : id;
-		ctlr->bus_num = id;
-	} else {
-		/* Allocate dynamic bus number using Linux idr */
-		id = of_alias_get_id(ctlr->dev.of_node, "spi");
-		if (id >= 0) {
-			ctlr->bus_num = id;
-			mutex_lock(&board_lock);
-			id = idr_alloc(&spi_master_idr, ctlr, ctlr->bus_num,
-				       ctlr->bus_num + 1, GFP_KERNEL);
-			mutex_unlock(&board_lock);
-			if (WARN(id < 0, "couldn't get idr"))
-				return id == -ENOSPC ? -EBUSY : id;
-		}
+		status = spi_controller_id_alloc(ctlr, ctlr->bus_num, ctlr->bus_num + 1);
+		if (status)
+			return status;
 	}
 	if (ctlr->bus_num < 0) {
 		first_dynamic = of_alias_get_highest_id("spi");
@@ -3151,13 +3151,9 @@ int spi_register_controller(struct spi_controller *ctlr)
 		else
 			first_dynamic++;
 
-		mutex_lock(&board_lock);
-		id = idr_alloc(&spi_master_idr, ctlr, first_dynamic,
-			       0, GFP_KERNEL);
-		mutex_unlock(&board_lock);
-		if (WARN(id < 0, "couldn't get idr"))
-			return id;
-		ctlr->bus_num = id;
+		status = spi_controller_id_alloc(ctlr, first_dynamic, 0);
+		if (status)
+			return status;
 	}
 	ctlr->bus_lock_flag = 0;
 	init_completion(&ctlr->xfer_completion);
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 02/15] spi: Drop duplicate IDR allocation code in spi_register_controller()

[Mark Brown]

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

On Mon, Jul 10, 2023 at 06:49:19PM +0300, Andy Shevchenko wrote:

> Refactor spi_register_controller() to drop duplicate IDR allocation.
> Instead of if-else-if branching use two sequential if:s, which allows
> to re-use the logic of IDR allocation in all cases.

For legibility this should have been split into a separate factoring out
of the shared code and rewriting of the logic, that'd make it trivial to
review.

> -		mutex_lock(&board_lock);
> -		id = idr_alloc(&spi_master_idr, ctlr, first_dynamic,
> -			       0, GFP_KERNEL);
> -		mutex_unlock(&board_lock);
> -		if (WARN(id < 0, "couldn't get idr"))
> -			return id;
> -		ctlr->bus_num = id;
> +		status = spi_controller_id_alloc(ctlr, first_dynamic, 0);
> +		if (status)
> +			return status;

The original does not do the remapping of return codes that the previous
two copies do...

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

Re: [PATCH v2 02/15] spi: Drop duplicate IDR allocation code in spi_register_controller()

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 06:09:00PM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2023 at 06:49:19PM +0300, Andy Shevchenko wrote:
> 
> > Refactor spi_register_controller() to drop duplicate IDR allocation.
> > Instead of if-else-if branching use two sequential if:s, which allows
> > to re-use the logic of IDR allocation in all cases.
> 
> For legibility this should have been split into a separate factoring out
> of the shared code and rewriting of the logic, that'd make it trivial to
> review.

Should I do that for v3?

> > -		mutex_lock(&board_lock);
> > -		id = idr_alloc(&spi_master_idr, ctlr, first_dynamic,
> > -			       0, GFP_KERNEL);
> > -		mutex_unlock(&board_lock);
> > -		if (WARN(id < 0, "couldn't get idr"))
> > -			return id;
> > -		ctlr->bus_num = id;
> > +		status = spi_controller_id_alloc(ctlr, first_dynamic, 0);
> > +		if (status)
> > +			return status;
> 
> The original does not do the remapping of return codes that the previous
> two copies do...

Yes, I had to mention this in the commit message that in my opinion this makes
no difference. With the dynamically allocated aliases the absence of the slot
has the same effect as in the other cases.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 03/15] spi: Replace if-else-if by bitops and multiplications

[Andy Shevchenko]

Instead of if-else-if, simply call roundup_pow_of_two(BITS_PER_BYTES()).
Note, there is no division assumed as compiler may optimize it away.

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

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 6d74218cf38e..125dea8fae00 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3643,13 +3643,7 @@ int spi_split_transfers_maxwords(struct spi_controller *ctlr,
 		size_t maxsize;
 		int ret;
 
-		if (xfer->bits_per_word <= 8)
-			maxsize = maxwords;
-		else if (xfer->bits_per_word <= 16)
-			maxsize = 2 * maxwords;
-		else
-			maxsize = 4 * maxwords;
-
+		maxsize = maxwords * roundup_pow_of_two(BITS_TO_BYTES(xfer->bits_per_word));
 		if (xfer->len > maxsize) {
 			ret = __spi_split_transfer_maxsize(ctlr, msg, &xfer,
 							   maxsize, gfp);
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 03/15] spi: Replace if-else-if by bitops and multiplications

[Mark Brown]

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

On Mon, Jul 10, 2023 at 06:49:20PM +0300, Andy Shevchenko wrote:

> -		if (xfer->bits_per_word <= 8)
> -			maxsize = maxwords;
> -		else if (xfer->bits_per_word <= 16)
> -			maxsize = 2 * maxwords;
> -		else
> -			maxsize = 4 * maxwords;
> -
> +		maxsize = maxwords * roundup_pow_of_two(BITS_TO_BYTES(xfer->bits_per_word));

This will change the behaviour if bits_per_word is more than 32.  That
is validated out elsewhere but I shouldn't have had to go around
checking the code to confirm that this is the case.  This is the sort of
thing that should be highlighted when doing this sort of edge case
stylistic change.

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

Re: [PATCH v2 03/15] spi: Replace if-else-if by bitops and multiplications

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 05:56:59PM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2023 at 06:49:20PM +0300, Andy Shevchenko wrote:
> 
> > -		if (xfer->bits_per_word <= 8)
> > -			maxsize = maxwords;
> > -		else if (xfer->bits_per_word <= 16)
> > -			maxsize = 2 * maxwords;
> > -		else
> > -			maxsize = 4 * maxwords;
> > -
> > +		maxsize = maxwords * roundup_pow_of_two(BITS_TO_BYTES(xfer->bits_per_word));
> 
> This will change the behaviour if bits_per_word is more than 32.  That
> is validated out elsewhere but I shouldn't have had to go around
> checking the code to confirm that this is the case.  This is the sort of
> thing that should be highlighted when doing this sort of edge case
> stylistic change.

Right, I have to add this into commit message of v3.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[Andy Shevchenko]

Since the new spi_controller_xfer_timeout() helper appeared,
we may replace open coded variant in spi_transfer_wait().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c       | 25 ++-----------------------
 include/linux/spi/spi.h |  6 +++++-
 2 files changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 125dea8fae00..c99ee4164f11 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1342,8 +1342,7 @@ static int spi_transfer_wait(struct spi_controller *ctlr,
 {
 	struct spi_statistics __percpu *statm = ctlr->pcpu_statistics;
 	struct spi_statistics __percpu *stats = msg->spi->pcpu_statistics;
-	u32 speed_hz = xfer->speed_hz;
-	unsigned long long ms;
+	unsigned long ms;
 
 	if (spi_controller_is_slave(ctlr)) {
 		if (wait_for_completion_interruptible(&ctlr->xfer_completion)) {
@@ -1351,29 +1350,9 @@ static int spi_transfer_wait(struct spi_controller *ctlr,
 			return -EINTR;
 		}
 	} else {
-		if (!speed_hz)
-			speed_hz = 100000;
-
-		/*
-		 * For each byte we wait for 8 cycles of the SPI clock.
-		 * Since speed is defined in Hz and we want milliseconds,
-		 * use respective multiplier, but before the division,
-		 * otherwise we may get 0 for short transfers.
-		 */
-		ms = 8LL * MSEC_PER_SEC * xfer->len;
-		do_div(ms, speed_hz);
-
-		/*
-		 * Increase it twice and add 200 ms tolerance, use
-		 * predefined maximum in case of overflow.
-		 */
-		ms += ms + 200;
-		if (ms > UINT_MAX)
-			ms = UINT_MAX;
-
+		ms = spi_controller_xfer_timeout(ctlr, xfer);
 		ms = wait_for_completion_timeout(&ctlr->xfer_completion,
 						 msecs_to_jiffies(ms));
-
 		if (ms == 0) {
 			SPI_STATISTICS_INCREMENT_FIELD(statm, timedout);
 			SPI_STATISTICS_INCREMENT_FIELD(stats, timedout);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 32c94eae8926..0ce1cb18a076 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1270,12 +1270,16 @@ static inline bool spi_is_bpw_supported(struct spi_device *spi, u32 bpw)
  * that it would take on a single data line and take twice this amount of time
  * with a minimum of 500ms to avoid false positives on loaded systems.
  *
+ * Assume speed to be 100 kHz if it's not defined at the time of invocation.
+ *
  * Returns: Transfer timeout value in milliseconds.
  */
 static inline unsigned int spi_controller_xfer_timeout(struct spi_controller *ctlr,
 						       struct spi_transfer *xfer)
 {
-	return max(xfer->len * 8 * 2 / (xfer->speed_hz / 1000), 500U);
+	u32 speed_hz = xfer->speed_hz ?: 100000;
+
+	return max(xfer->len * 8 * 2 / (speed_hz / 1000), 500U);
 }
 
 /*---------------------------------------------------------------------------*/
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[Mark Brown]

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

On Mon, Jul 10, 2023 at 06:49:21PM +0300, Andy Shevchenko wrote:

> Since the new spi_controller_xfer_timeout() helper appeared,
> we may replace open coded variant in spi_transfer_wait().

> + * Assume speed to be 100 kHz if it's not defined at the time of invocation.
> + *

You didn't mention this bit in the changelog, and I'm not 100% convinced
it was the best idea in the first place.  It's going to result in some
very big timeouts if it goes off, and we really should be doing
validation much earlier in the process.

> +	u32 speed_hz = xfer->speed_hz ?: 100000;

Not only the ternery operator, but the version without the second
argument for extra clarity!

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

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 06:30:32PM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2023 at 06:49:21PM +0300, Andy Shevchenko wrote:
> 
> > Since the new spi_controller_xfer_timeout() helper appeared,
> > we may replace open coded variant in spi_transfer_wait().
> 
> > + * Assume speed to be 100 kHz if it's not defined at the time of invocation.
> > + *
> 
> You didn't mention this bit in the changelog, and I'm not 100% convinced
> it was the best idea in the first place.  It's going to result in some
> very big timeouts if it goes off, and we really should be doing
> validation much earlier in the process.

Okay, let's drop this change.

> > +	u32 speed_hz = xfer->speed_hz ?: 100000;
> 
> Not only the ternery operator, but the version without the second
> argument for extra clarity!

Elvis can be interpreted as "A _or_ B (if A is false/0)".
Some pieces related to SPI use Elvis already IIRC.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[Mark Brown]

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

On Tue, Jul 11, 2023 at 02:01:13PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 10, 2023 at 06:30:32PM +0100, Mark Brown wrote:
> > On Mon, Jul 10, 2023 at 06:49:21PM +0300, Andy Shevchenko wrote:

> > > + * Assume speed to be 100 kHz if it's not defined at the time of invocation.

> > You didn't mention this bit in the changelog, and I'm not 100% convinced
> > it was the best idea in the first place.  It's going to result in some
> > very big timeouts if it goes off, and we really should be doing
> > validation much earlier in the process.

> Okay, let's drop this change.

Like I say we *should* be fine with the refactoring without this, or at
least if it's an issue we should improve the validation.

> > > +	u32 speed_hz = xfer->speed_hz ?: 100000;

> > Not only the ternery operator, but the version without the second
> > argument for extra clarity!

> Elvis can be interpreted as "A _or_ B (if A is false/0)".
> Some pieces related to SPI use Elvis already IIRC.

I understand what it means, I just don't find it's adding clarity most
of the times it's used (there's a few places where it is useful like
pasting in strings in formats).  There are some examples that I'd
complain about in the code, most of them predating me working on SPI too
much, but I'm not a fan.

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

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 03:14:54PM +0100, Mark Brown wrote:
> On Tue, Jul 11, 2023 at 02:01:13PM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 10, 2023 at 06:30:32PM +0100, Mark Brown wrote:
> > > On Mon, Jul 10, 2023 at 06:49:21PM +0300, Andy Shevchenko wrote:
> 
> > > > + * Assume speed to be 100 kHz if it's not defined at the time of invocation.
> 
> > > You didn't mention this bit in the changelog, and I'm not 100% convinced
> > > it was the best idea in the first place.  It's going to result in some
> > > very big timeouts if it goes off, and we really should be doing
> > > validation much earlier in the process.
> 
> > Okay, let's drop this change.
> 
> Like I say we *should* be fine with the refactoring without this, or at
> least if it's an issue we should improve the validation.

For the speeds < 1000 Hz, this change will lead to the div by 0 crash.
It seems that the current code which this one removes is better than
the spi_controller_xfer_timeout() provides.

If anything, the spi_controller_xfer_timeout() should be improved first.
So, for now I drop this for sure. Maybe in the future we can come back
to it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[Mark Brown]

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

On Tue, Jul 11, 2023 at 06:30:06PM +0300, Andy Shevchenko wrote:
> On Tue, Jul 11, 2023 at 03:14:54PM +0100, Mark Brown wrote:

> > Like I say we *should* be fine with the refactoring without this, or at
> > least if it's an issue we should improve the validation.

> For the speeds < 1000 Hz, this change will lead to the div by 0 crash.
> It seems that the current code which this one removes is better than
> the spi_controller_xfer_timeout() provides.

> If anything, the spi_controller_xfer_timeout() should be improved first.
> So, for now I drop this for sure. Maybe in the future we can come back
> to it.

I don't think this is the only thing that might fall over without a
speed, what we've generally been doing (and do try to do with speeds, we
already need to default in the controller's speed and so on) is to
sanitise input on the way into the subsystem rather than trying to
ensure that all the users are handling everything.

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

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[AngeloGioacchino Del Regno]

Il 10/07/23 17:49, Andy Shevchenko ha scritto:
> Since the new spi_controller_xfer_timeout() helper appeared,
> we may replace open coded variant in spi_transfer_wait().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/spi/spi.c       | 25 ++-----------------------
>   include/linux/spi/spi.h |  6 +++++-
>   2 files changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 125dea8fae00..c99ee4164f11 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1342,8 +1342,7 @@ static int spi_transfer_wait(struct spi_controller *ctlr,
>   {
>   	struct spi_statistics __percpu *statm = ctlr->pcpu_statistics;
>   	struct spi_statistics __percpu *stats = msg->spi->pcpu_statistics;
> -	u32 speed_hz = xfer->speed_hz;
> -	unsigned long long ms;
> +	unsigned long ms;
>   
>   	if (spi_controller_is_slave(ctlr)) {
>   		if (wait_for_completion_interruptible(&ctlr->xfer_completion)) {
> @@ -1351,29 +1350,9 @@ static int spi_transfer_wait(struct spi_controller *ctlr,
>   			return -EINTR;
>   		}
>   	} else {
> -		if (!speed_hz)
> -			speed_hz = 100000;
> -
> -		/*
> -		 * For each byte we wait for 8 cycles of the SPI clock.
> -		 * Since speed is defined in Hz and we want milliseconds,
> -		 * use respective multiplier, but before the division,
> -		 * otherwise we may get 0 for short transfers.
> -		 */
> -		ms = 8LL * MSEC_PER_SEC * xfer->len;
> -		do_div(ms, speed_hz);
> -
> -		/*
> -		 * Increase it twice and add 200 ms tolerance, use
> -		 * predefined maximum in case of overflow.
> -		 */
> -		ms += ms + 200;
> -		if (ms > UINT_MAX)
> -			ms = UINT_MAX;
> -
> +		ms = spi_controller_xfer_timeout(ctlr, xfer);

I agree on using helpers, but the logic is slightly changing here: yes it is
unlikely (and also probably useless) to get ms == UINT_MAX, but the helper is
limiting the maximum timeout value to 500mS, which may not work for some slow
controllers/devices.

This should get validated on more than a few platforms, and I'm not sure that
this kind of validation would be "fast" to get... so, probably the best thing
to do here is to add a warning in case the timeout exceeds 500mS, print the
actual value, keep it like this for a kernel version or two and check reports:
that would allow to understand what a safe maximum timeout value could be.

Aside from that, I wouldn't drop those nice comments explaining how/why the
timeout is calculated: I know how, but not everyone knows in advance.

Regards,
Angelo

>   		ms = wait_for_completion_timeout(&ctlr->xfer_completion,
>   						 msecs_to_jiffies(ms));
> -
>   		if (ms == 0) {
>   			SPI_STATISTICS_INCREMENT_FIELD(statm, timedout);
>   			SPI_STATISTICS_INCREMENT_FIELD(stats, timedout);
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 32c94eae8926..0ce1cb18a076 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -1270,12 +1270,16 @@ static inline bool spi_is_bpw_supported(struct spi_device *spi, u32 bpw)
>    * that it would take on a single data line and take twice this amount of time
>    * with a minimum of 500ms to avoid false positives on loaded systems.
>    *
> + * Assume speed to be 100 kHz if it's not defined at the time of invocation.
> + *
>    * Returns: Transfer timeout value in milliseconds.
>    */
>   static inline unsigned int spi_controller_xfer_timeout(struct spi_controller *ctlr,
>   						       struct spi_transfer *xfer)
>   {
> -	return max(xfer->len * 8 * 2 / (xfer->speed_hz / 1000), 500U);
> +	u32 speed_hz = xfer->speed_hz ?: 100000;
> +
> +	return max(xfer->len * 8 * 2 / (speed_hz / 1000), 500U);
>   }
>   
>   /*---------------------------------------------------------------------------*/

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[Mark Brown]

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

On Tue, Jul 11, 2023 at 10:28:13AM +0200, AngeloGioacchino Del Regno wrote:
> Il 10/07/23 17:49, Andy Shevchenko ha scritto:

> > +		ms = spi_controller_xfer_timeout(ctlr, xfer);

> I agree on using helpers, but the logic is slightly changing here: yes it is
> unlikely (and also probably useless) to get ms == UINT_MAX, but the helper is
> limiting the maximum timeout value to 500mS, which may not work for some slow
> controllers/devices.

The helper is limiting the *minimum* timeout value to 500ms - it's using
max() not min().  The idea is the other way around, that for a very fast
transfer we don't want to end up with such a short timeout that it false
triggers due to scheduling issues.

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

Re: [PATCH v2 04/15] spi: Replace open coded spi_controller_xfer_timeout()

[AngeloGioacchino Del Regno]

Il 11/07/23 15:05, Mark Brown ha scritto:
> On Tue, Jul 11, 2023 at 10:28:13AM +0200, AngeloGioacchino Del Regno wrote:
>> Il 10/07/23 17:49, Andy Shevchenko ha scritto:
> 
>>> +		ms = spi_controller_xfer_timeout(ctlr, xfer);
> 
>> I agree on using helpers, but the logic is slightly changing here: yes it is
>> unlikely (and also probably useless) to get ms == UINT_MAX, but the helper is
>> limiting the maximum timeout value to 500mS, which may not work for some slow
>> controllers/devices.
> 
> The helper is limiting the *minimum* timeout value to 500ms - it's using
> max() not min().  The idea is the other way around, that for a very fast
> transfer we don't want to end up with such a short timeout that it false
> triggers due to scheduling issues.

After reading the code again, yeah, I've totally misread it the first time. Argh!
Thanks! :-)

[PATCH v2 05/15] spi: Remove code duplication in spi_add_device_locked()

[Andy Shevchenko]

Seems by unknown reason, probably some kind of mis-rebase,
the commit 0c79378c0199 ("spi: add ancillary device support")
adds a dozen of duplicating lines of code. Drop them.

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

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c99ee4164f11..46cbda383228 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -712,17 +712,6 @@ EXPORT_SYMBOL_GPL(spi_add_device);
 static int spi_add_device_locked(struct spi_device *spi)
 {
 	struct spi_controller *ctlr = spi->controller;
-	struct device *dev = ctlr->dev.parent;
-
-	/* Chipselects are numbered 0..max; validate. */
-	if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
-		dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0),
-			ctlr->num_chipselect);
-		return -EINVAL;
-	}
-
-	/* Set the bus ID string */
-	spi_dev_set_name(spi);
 
 	WARN_ON(!mutex_is_locked(&ctlr->add_lock));
 	return __spi_add_device(spi);
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 05/15] spi: Remove code duplication in spi_add_device_locked()

[Mark Brown]

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

On Mon, Jul 10, 2023 at 06:49:22PM +0300, Andy Shevchenko wrote:
> Seems by unknown reason, probably some kind of mis-rebase,
> the commit 0c79378c0199 ("spi: add ancillary device support")
> adds a dozen of duplicating lines of code. Drop them.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/spi/spi.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index c99ee4164f11..46cbda383228 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -712,17 +712,6 @@ EXPORT_SYMBOL_GPL(spi_add_device);
>  static int spi_add_device_locked(struct spi_device *spi)
>  {
>  	struct spi_controller *ctlr = spi->controller;
> -	struct device *dev = ctlr->dev.parent;
> -
> -	/* Chipselects are numbered 0..max; validate. */
> -	if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> -		dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0),
> -			ctlr->num_chipselect);
> -		return -EINVAL;
> -	}
> -
> -	/* Set the bus ID string */
> -	spi_dev_set_name(spi);

I see that this is duplicating spi_add_device() (and we really could do
better with code sharing there I think) but I can't immediately see
where the duplication that's intended to be elimiated is here - where
else in the one call path that spi_add_device_locked() has would we do
the above?  Based on the changelog I was expecting to see some
duplicated code in the function itself.

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

Re: [PATCH v2 05/15] spi: Remove code duplication in spi_add_device_locked()

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 06:16:22PM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2023 at 06:49:22PM +0300, Andy Shevchenko wrote:
> > Seems by unknown reason, probably some kind of mis-rebase,
> > the commit 0c79378c0199 ("spi: add ancillary device support")
> > adds a dozen of duplicating lines of code. Drop them.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/spi/spi.c | 11 -----------
> >  1 file changed, 11 deletions(-)
> > 
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index c99ee4164f11..46cbda383228 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -712,17 +712,6 @@ EXPORT_SYMBOL_GPL(spi_add_device);
> >  static int spi_add_device_locked(struct spi_device *spi)
> >  {
> >  	struct spi_controller *ctlr = spi->controller;
> > -	struct device *dev = ctlr->dev.parent;
> > -
> > -	/* Chipselects are numbered 0..max; validate. */
> > -	if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> > -		dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0),
> > -			ctlr->num_chipselect);
> > -		return -EINVAL;
> > -	}
> > -
> > -	/* Set the bus ID string */
> > -	spi_dev_set_name(spi);
> 
> I see that this is duplicating spi_add_device() (and we really could do
> better with code sharing there I think) but I can't immediately see
> where the duplication that's intended to be elimiated is here - where
> else in the one call path that spi_add_device_locked() has would we do
> the above?  Based on the changelog I was expecting to see some
> duplicated code in the function itself.

Oh, by some reason Sebastian wasn't in this rather long Cc list.
Added him.

Reading again I don't see any useful explanation why that piece of code has to
be duplicated among these two functions. It's 100% a copy.

Sebastian, can you shed some light here?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 05/15] spi: Remove code duplication in spi_add_device_locked()

[Sebastian Reichel]

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

Hi,

On Tue, Jul 11, 2023 at 02:06:20PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 10, 2023 at 06:16:22PM +0100, Mark Brown wrote:
> > On Mon, Jul 10, 2023 at 06:49:22PM +0300, Andy Shevchenko wrote:
> > > Seems by unknown reason, probably some kind of mis-rebase,
> > > the commit 0c79378c0199 ("spi: add ancillary device support")
> > > adds a dozen of duplicating lines of code. Drop them.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  drivers/spi/spi.c | 11 -----------
> > >  1 file changed, 11 deletions(-)
> > > 
> > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > > index c99ee4164f11..46cbda383228 100644
> > > --- a/drivers/spi/spi.c
> > > +++ b/drivers/spi/spi.c
> > > @@ -712,17 +712,6 @@ EXPORT_SYMBOL_GPL(spi_add_device);
> > >  static int spi_add_device_locked(struct spi_device *spi)
> > >  {
> > >  	struct spi_controller *ctlr = spi->controller;
> > > -	struct device *dev = ctlr->dev.parent;
> > > -
> > > -	/* Chipselects are numbered 0..max; validate. */
> > > -	if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> > > -		dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0),
> > > -			ctlr->num_chipselect);
> > > -		return -EINVAL;
> > > -	}
> > > -
> > > -	/* Set the bus ID string */
> > > -	spi_dev_set_name(spi);
> > 
> > I see that this is duplicating spi_add_device() (and we really could do
> > better with code sharing there I think) but I can't immediately see
> > where the duplication that's intended to be elimiated is here - where
> > else in the one call path that spi_add_device_locked() has would we do
> > the above?  Based on the changelog I was expecting to see some
> > duplicated code in the function itself.
> 
> Oh, by some reason Sebastian wasn't in this rather long Cc list.
> Added him.
> 
> Reading again I don't see any useful explanation why that piece of code has to
> be duplicated among these two functions. It's 100% a copy.
> 
> Sebastian, can you shed some light here?

The patch in this thread is obviously wrong. It results in the
checks never beeing called for spi_add_device_locked(). The copy is
in spi_add_device() and those two are not calling into each other.

But it should be fine to move the code to the start of
__spi_add_device(), which allows removing the duplication. In that
case the code will be run with the add_lock held, which is probably
what I was worried about two years ago. Looking at it again, the
lock is held anyways in case of spi_add_device_locked().

Greetings,

-- Sebastian

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

Re: [PATCH v2 05/15] spi: Remove code duplication in spi_add_device_locked()

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 02:01:33PM +0200, Sebastian Reichel wrote:
> On Tue, Jul 11, 2023 at 02:06:20PM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 10, 2023 at 06:16:22PM +0100, Mark Brown wrote:
> > > On Mon, Jul 10, 2023 at 06:49:22PM +0300, Andy Shevchenko wrote:

...

> > > > -	struct device *dev = ctlr->dev.parent;
> > > > -
> > > > -	/* Chipselects are numbered 0..max; validate. */
> > > > -	if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> > > > -		dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0),
> > > > -			ctlr->num_chipselect);
> > > > -		return -EINVAL;
> > > > -	}
> > > > -
> > > > -	/* Set the bus ID string */
> > > > -	spi_dev_set_name(spi);
> > > 
> > > I see that this is duplicating spi_add_device() (and we really could do
> > > better with code sharing there I think) but I can't immediately see
> > > where the duplication that's intended to be elimiated is here - where
> > > else in the one call path that spi_add_device_locked() has would we do
> > > the above?  Based on the changelog I was expecting to see some
> > > duplicated code in the function itself.
> > 
> > Oh, by some reason Sebastian wasn't in this rather long Cc list.
> > Added him.
> > 
> > Reading again I don't see any useful explanation why that piece of code has to
> > be duplicated among these two functions. It's 100% a copy.
> > 
> > Sebastian, can you shed some light here?
> 
> The patch in this thread is obviously wrong. It results in the
> checks never beeing called for spi_add_device_locked(). The copy is
> in spi_add_device() and those two are not calling into each other.

Ah, now I see, I missed __ in the name.
Thank you for opening my eyes!

> But it should be fine to move the code to the start of
> __spi_add_device(), which allows removing the duplication. In that
> case the code will be run with the add_lock held, which is probably
> what I was worried about two years ago. Looking at it again, the
> lock is held anyways in case of spi_add_device_locked().

Right, I will re-do that.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 06/15] spi: Use sysfs_emit() to instead of s*printf()

[Andy Shevchenko]

Follow the advice of the Documentation/filesystems/sysfs.rst and show()
should only use sysfs_emit() or sysfs_emit_at() when formatting the
value to be returned to user space.

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

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 46cbda383228..cfb1c985d940 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -64,7 +64,7 @@ modalias_show(struct device *dev, struct device_attribute *a, char *buf)
 	if (len != -ENODEV)
 		return len;
 
-	return sprintf(buf, "%s%s\n", SPI_MODULE_PREFIX, spi->modalias);
+	return sysfs_emit(buf, "%s%s\n", SPI_MODULE_PREFIX, spi->modalias);
 }
 static DEVICE_ATTR_RO(modalias);
 
@@ -89,7 +89,7 @@ static ssize_t driver_override_show(struct device *dev,
 	ssize_t len;
 
 	device_lock(dev);
-	len = snprintf(buf, PAGE_SIZE, "%s\n", spi->driver_override ? : "");
+	len = sysfs_emit(buf, "%s\n", spi->driver_override ? : "");
 	device_unlock(dev);
 	return len;
 }
@@ -2782,8 +2782,7 @@ static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
 	struct device *child;
 
 	child = device_find_any_child(&ctlr->dev);
-	return sprintf(buf, "%s\n",
-		       child ? to_spi_device(child)->modalias : NULL);
+	return sysfs_emit(buf, "%s\n", child ? to_spi_device(child)->modalias : NULL);
 }
 
 static ssize_t slave_store(struct device *dev, struct device_attribute *attr,
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 07/15] spi: Sort headers alphabetically

[Andy Shevchenko]

Sorting headers alphabetically helps locating duplicates, and
make it easier to figure out where to insert new headers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c       | 42 ++++++++++++++++++++---------------------
 include/linux/spi/spi.h | 14 +++++++-------
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index cfb1c985d940..06a92a3a5746 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4,36 +4,36 @@
 // Copyright (C) 2005 David Brownell
 // Copyright (C) 2008 Secret Lab Technologies Ltd.
 
-#include <linux/kernel.h>
-#include <linux/device.h>
-#include <linux/init.h>
+#include <linux/acpi.h>
 #include <linux/cache.h>
-#include <linux/dma-mapping.h>
+#include <linux/clk/clk-conf.h>
+#include <linux/delay.h>
+#include <linux/device.h>
 #include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+#include <linux/export.h>
+#include <linux/gpio/consumer.h>
+#include <linux/highmem.h>
+#include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/mod_devicetable.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
-#include <linux/clk/clk-conf.h>
-#include <linux/slab.h>
-#include <linux/mod_devicetable.h>
-#include <linux/spi/spi.h>
-#include <linux/spi/spi-mem.h>
-#include <linux/gpio/consumer.h>
-#include <linux/pm_runtime.h>
+#include <linux/percpu.h>
+#include <linux/platform_data/x86/apple.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 #include <linux/property.h>
-#include <linux/export.h>
+#include <linux/ptp_clock_kernel.h>
 #include <linux/sched/rt.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 #include <uapi/linux/sched/types.h>
-#include <linux/delay.h>
-#include <linux/kthread.h>
-#include <linux/ioport.h>
-#include <linux/acpi.h>
-#include <linux/highmem.h>
-#include <linux/idr.h>
-#include <linux/platform_data/x86/apple.h>
-#include <linux/ptp_clock_kernel.h>
-#include <linux/percpu.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/spi.h>
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 0ce1cb18a076..2026eae97329 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -6,19 +6,19 @@
 #ifndef __LINUX_SPI_H
 #define __LINUX_SPI_H
 
+#include <linux/acpi.h>
 #include <linux/bits.h>
+#include <linux/completion.h>
 #include <linux/device.h>
-#include <linux/mod_devicetable.h>
-#include <linux/slab.h>
+#include <linux/gpio/consumer.h>
 #include <linux/kthread.h>
-#include <linux/completion.h>
+#include <linux/mod_devicetable.h>
 #include <linux/scatterlist.h>
-#include <linux/gpio/consumer.h>
-
-#include <uapi/linux/spi/spi.h>
-#include <linux/acpi.h>
+#include <linux/slab.h>
 #include <linux/u64_stats_sync.h>
 
+#include <uapi/linux/spi/spi.h>
+
 struct dma_chan;
 struct software_node;
 struct ptp_system_timestamp;
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 08/15] spi: Clean up headers

[Andy Shevchenko]

There is a few things done:
- include only the headers we are direct user of
- when pointer is in use, provide a forward declaration
- add missing headers
- group generic headers and subsystem headers
- sort each group alphabetically

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/spi/spi.h | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 2026eae97329..c9479badf38c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -6,27 +6,41 @@
 #ifndef __LINUX_SPI_H
 #define __LINUX_SPI_H
 
-#include <linux/acpi.h>
 #include <linux/bits.h>
 #include <linux/completion.h>
+#include <linux/container_of.h>
 #include <linux/device.h>
-#include <linux/gpio/consumer.h>
+#include <linux/export.h>
 #include <linux/kthread.h>
+#include <linux/limits.h>
+#include <linux/list.h>
+#include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
+#include <linux/smp.h>
+#include <linux/spinlock_types.h>
+#include <linux/string.h>
+#include <linux/types.h>
 #include <linux/u64_stats_sync.h>
 
+#include <asm/byteorder.h>
+
 #include <uapi/linux/spi/spi.h>
 
+struct acpi_device;
 struct dma_chan;
-struct software_node;
+struct gpio_desc;
 struct ptp_system_timestamp;
+struct software_node;
+
 struct spi_controller;
-struct spi_transfer;
 struct spi_controller_mem_ops;
 struct spi_controller_mem_caps;
+struct spi_device_id;
 struct spi_message;
+struct spi_transfer;
 
 /*
  * INTERFACES between SPI master-side drivers and SPI slave protocol handlers,
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 08/15] spi: Clean up headers

[Mark Brown]

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

On Mon, Jul 10, 2023 at 06:49:25PM +0300, Andy Shevchenko wrote:
> There is a few things done:
> - include only the headers we are direct user of
> - when pointer is in use, provide a forward declaration
> - add missing headers
> - group generic headers and subsystem headers
> - sort each group alphabetically

The previous commit was supposed to be sorting things and AFAICT did
so...

> +struct spi_device_id;

Why are we adding this given that there's also an inclusion of
mod_devicetable that you didn't remove?

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

Re: [PATCH v2 08/15] spi: Clean up headers

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 06:21:44PM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2023 at 06:49:25PM +0300, Andy Shevchenko wrote:
> > There is a few things done:
> > - include only the headers we are direct user of
> > - when pointer is in use, provide a forward declaration
> > - add missing headers
> > - group generic headers and subsystem headers
> > - sort each group alphabetically
> 
> The previous commit was supposed to be sorting things and AFAICT did
> so...

This is about forward declaration groups.

> > +struct spi_device_id;
> 
> Why are we adding this given that there's also an inclusion of
> mod_devicetable that you didn't remove?

Answered in the other email.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 08/15] spi: Clean up headers

[Mark Brown]

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

On Mon, Jul 10, 2023 at 06:49:25PM +0300, Andy Shevchenko wrote:
> There is a few things done:
> - include only the headers we are direct user of
> - when pointer is in use, provide a forward declaration
> - add missing headers
> - group generic headers and subsystem headers
> - sort each group alphabetically

This breaks an x86 allmodconfig build:

/build/stage/linux/drivers/gpio/gpio-xra1403.c: In function ‘xra1403_probe’:
/build/stage/linux/drivers/gpio/gpio-xra1403.c:161:22: error: implicit declaration of function ‘devm_gpiod_get_optional’ [-Werror=implicit-function-declaration]
  161 |         reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
      |                      ^~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/gpio/gpio-xra1403.c:161:66: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
  161 |         reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
      |                                                                  ^~~~~~~~~~~~~
/build/stage/linux/drivers/gpio/gpio-xra1403.c:161:66: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
make[5]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/gpio/gpio-xra1403.o] Error 1
make[5]: Target 'drivers/gpio/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/gpio] Error 2
/build/stage/linux/sound/soc/intel/boards/bytcr_wm5102.c: In function ‘byt_wm5102_spkvdd_power_event’:
/build/stage/linux/sound/soc/intel/boards/bytcr_wm5102.c:46:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
   46 |         gpiod_set_value_cansleep(priv->spkvdd_en_gpio,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/sound/soc/intel/boards/bytcr_wm5102.c: In function ‘snd_byt_wm5102_mc_probe’:
/build/stage/linux/sound/soc/intel/boards/bytcr_wm5102.c:421:32: error: implicit declaration of function ‘gpiod_get’; did you mean ‘bio_get’? [-Werror=implicit-function-declaration]
  421 |         priv->spkvdd_en_gpio = gpiod_get(codec_dev, "wlf,spkvdd-ena", GPIOD_OUT_LOW);
      |                                ^~~~~~~~~
      |                                bio_get
/build/stage/linux/sound/soc/intel/boards/bytcr_wm5102.c:421:71: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
  421 |         priv->spkvdd_en_gpio = gpiod_get(codec_dev, "wlf,spkvdd-ena", GPIOD_OUT_LOW);
      |                                                                       ^~~~~~~~~~~~~
/build/stage/linux/sound/soc/intel/boards/bytcr_wm5102.c:421:71: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/sound/soc/intel/boards/bytcr_wm5102.c:465:9: error: implicit declaration of function ‘gpiod_put’; did you mean ‘bio_put’? [-Werror=implicit-function-declaration]
  465 |         gpiod_put(priv->spkvdd_en_gpio);
      |         ^~~~~~~~~
      |         bio_put
cc1: all warnings being treated as errors
make[7]: *** [/build/stage/linux/scripts/Makefile.build:243: sound/soc/intel/boards/bytcr_wm5102.o] Error 1
make[7]: Target 'sound/soc/intel/boards/' not remade because of errors.
make[6]: *** [/build/stage/linux/scripts/Makefile.build:477: sound/soc/intel/boards] Error 2
make[6]: Target 'sound/soc/intel/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: sound/soc/intel] Error 2
/build/stage/linux/drivers/gpu/drm/solomon/ssd130x-spi.c: In function ‘ssd130x_spi_write’:
/build/stage/linux/drivers/gpu/drm/solomon/ssd130x-spi.c:38:17: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
   38 |                 gpiod_set_value_cansleep(t->dc, 0);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/gpu/drm/solomon/ssd130x-spi.c: In function ‘ssd130x_spi_probe’:
/build/stage/linux/drivers/gpu/drm/solomon/ssd130x-spi.c:70:14: error: implicit declaration of function ‘devm_gpiod_get’; did you mean ‘em_pd_get’? [-Werror=implicit-function-declaration]
   70 |         dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
      |              ^~~~~~~~~~~~~~
      |              em_pd_get
/build/stage/linux/drivers/gpu/drm/solomon/ssd130x-spi.c:70:40: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
   70 |         dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
      |                                        ^~~~~~~~~~~~~
/build/stage/linux/drivers/gpu/drm/solomon/ssd130x-spi.c:70:40: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
make[7]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/gpu/drm/solomon/ssd130x-spi.o] Error 1
make[7]: Target 'drivers/gpu/drm/solomon/' not remade because of errors.
make[6]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/gpu/drm/solomon] Error 2
/build/stage/linux/drivers/net/ethernet/asix/ax88796c_main.c: In function ‘ax88796c_hard_reset’:
/build/stage/linux/drivers/net/ethernet/asix/ax88796c_main.c:950:22: error: implicit declaration of function ‘gpiod_get’; did you mean ‘guid_gen’? [-Werror=implicit-function-declaration]
  950 |         reset_gpio = gpiod_get(dev, "reset", 0);
      |                      ^~~~~~~~~
      |                      guid_gen
/build/stage/linux/drivers/net/ethernet/asix/ax88796c_main.c:950:20: error: assignment to ‘struct gpio_desc *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  950 |         reset_gpio = gpiod_get(dev, "reset", 0);
      |                    ^
/build/stage/linux/drivers/net/ethernet/asix/ax88796c_main.c:957:9: error: implicit declaration of function ‘gpiod_direction_output’ [-Werror=implicit-function-declaration]
  957 |         gpiod_direction_output(reset_gpio, 1);
      |         ^~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/net/ethernet/asix/ax88796c_main.c:960:9: error: implicit declaration of function ‘gpiod_put’ [-Werror=implicit-function-declaration]
  960 |         gpiod_put(reset_gpio);
      |         ^~~~~~~~~
cc1: all warnings being treated as errors
make[7]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/net/ethernet/asix/ax88796c_main.o] Error 1
make[7]: Target 'drivers/net/ethernet/asix/' not remade because of errors.
make[6]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/net/ethernet/asix] Error 2
/build/stage/linux/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c: In function ‘td043mtea1_power_on’:
/build/stage/linux/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c:147:9: error: implicit declaration of function ‘gpiod_set_value’ [-Werror=implicit-function-declaration]
  147 |         gpiod_set_value(lcd->reset_gpio, 0);
      |         ^~~~~~~~~~~~~~~
/build/stage/linux/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c: In function ‘td043mtea1_probe’:
/build/stage/linux/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c:438:27: error: implicit declaration of function ‘devm_gpiod_get’; did you mean ‘em_pd_get’? [-Werror=implicit-function-declaration]
  438 |         lcd->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH);
      |                           ^~~~~~~~~~~~~~
      |                           em_pd_get
/build/stage/linux/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c:438:62: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
  438 |         lcd->reset_gpio = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH);
      |                                                              ^~~~~~~~~~~~~~
/build/stage/linux/drivers/gpu/drm/panel/panel-tpo-td043mtea1.c:438:62: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
make[7]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/gpu/drm/panel/panel-tpo-td043mtea1.o] Error 1
make[7]: Target 'drivers/gpu/drm/panel/' not remade because of errors.
make[6]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/gpu/drm/panel] Error 2
/build/stage/linux/drivers/spi/spi-fsl-dspi.c: In function ‘dspi_assert_cs’:
/build/stage/linux/drivers/spi/spi-fsl-dspi.c:908:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
  908 |         gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), true);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/spi/spi-fsl-dspi.c: In function ‘dspi_setup’:
/build/stage/linux/drivers/spi/spi-fsl-dspi.c:1076:9: error: implicit declaration of function ‘gpiod_direction_output’; did you mean ‘pinctrl_gpio_direction_output’? [-Werror=implicit-function-declaration]
 1076 |         gpiod_direction_output(spi_get_csgpiod(spi, 0), false);
      |         ^~~~~~~~~~~~~~~~~~~~~~
      |         pinctrl_gpio_direction_output
cc1: all warnings being treated as errors
make[5]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/spi/spi-fsl-dspi.o] Error 1
make[6]: Target 'drivers/net/ethernet/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/net/ethernet] Error 2
/build/stage/linux/drivers/media/i2c/ar0521.c: In function ‘ar0521_power_off’:
/build/stage/linux/drivers/media/i2c/ar0521.c:850:17: error: implicit declaration of function ‘gpiod_set_value’ [-Werror=implicit-function-declaration]
  850 |                 gpiod_set_value(sensor->reset_gpio, 1); /* assert RESET signal */
      |                 ^~~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ar0521.c: In function ‘ar0521_probe’:
/build/stage/linux/drivers/media/i2c/ar0521.c:1114:30: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
 1114 |         sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
      |                              ^~~~~~~~~~~~~~~~~~~~~~~
      |                              devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/ar0521.c:1115:54: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
 1115 |                                                      GPIOD_OUT_HIGH);
      |                                                      ^~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ar0521.c:1115:54: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/ar0521.o] Error 1
make[6]: Target 'drivers/gpu/drm/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/gpu/drm] Error 2
make[5]: Target 'drivers/gpu/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/gpu] Error 2
/build/stage/linux/drivers/media/i2c/imx334.c: In function ‘imx334_parse_hw_config’:
/build/stage/linux/drivers/media/i2c/imx334.c:1081:30: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
 1081 |         imx334->reset_gpio = devm_gpiod_get_optional(imx334->dev, "reset",
      |                              ^~~~~~~~~~~~~~~~~~~~~~~
      |                              devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/imx334.c:1082:54: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
 1082 |                                                      GPIOD_OUT_LOW);
      |                                                      ^~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/imx334.c:1082:54: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/media/i2c/imx334.c: In function ‘imx334_power_on’:
/build/stage/linux/drivers/media/i2c/imx334.c:1176:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
 1176 |         gpiod_set_value_cansleep(imx334->reset_gpio, 1);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/imx334.o] Error 1
/build/stage/linux/drivers/media/i2c/imx335.c: In function ‘imx335_parse_hw_config’:
/build/stage/linux/drivers/media/i2c/imx335.c:785:30: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
  785 |         imx335->reset_gpio = devm_gpiod_get_optional(imx335->dev, "reset",
      |                              ^~~~~~~~~~~~~~~~~~~~~~~
      |                              devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/imx335.c:786:54: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
  786 |                                                      GPIOD_OUT_LOW);
      |                                                      ^~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/imx335.c:786:54: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/media/i2c/imx335.c: In function ‘imx335_power_on’:
/build/stage/linux/drivers/media/i2c/imx335.c:871:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
  871 |         gpiod_set_value_cansleep(imx335->reset_gpio, 1);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/imx335.o] Error 1
/build/stage/linux/drivers/media/i2c/imx412.c: In function ‘imx412_parse_hw_config’:
/build/stage/linux/drivers/media/i2c/imx412.c:937:30: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
  937 |         imx412->reset_gpio = devm_gpiod_get_optional(imx412->dev, "reset",
      |                              ^~~~~~~~~~~~~~~~~~~~~~~
      |                              devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/imx412.c:938:54: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
  938 |                                                      GPIOD_OUT_LOW);
      |                                                      ^~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/imx412.c:938:54: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/media/i2c/imx412.c: In function ‘imx412_power_on’:
/build/stage/linux/drivers/media/i2c/imx412.c:1040:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
 1040 |         gpiod_set_value_cansleep(imx412->reset_gpio, 0);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/imx412.o] Error 1
make[5]: Target 'drivers/net/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/net] Error 2
/build/stage/linux/drivers/spi/spi-tegra114.c: In function ‘tegra_spi_setup_transfer_one’:
/build/stage/linux/drivers/spi/spi-tegra114.c:835:25: error: implicit declaration of function ‘gpiod_set_value’ [-Werror=implicit-function-declaration]
  835 |                         gpiod_set_value(spi_get_csgpiod(spi, 0), 1);
      |                         ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[5]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/spi/spi-tegra114.o] Error 1
make[5]: Target 'drivers/spi/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/spi] Error 2
make[5]: Target 'sound/soc/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: sound/soc] Error 2
make[4]: Target 'sound/' not remade because of errors.
make[3]: *** [/build/stage/linux/scripts/Makefile.build:477: sound] Error 2
/build/stage/linux/drivers/media/i2c/ov5648.c: In function ‘ov5648_sensor_power’:
/build/stage/linux/drivers/media/i2c/ov5648.c:1878:17: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
 1878 |                 gpiod_set_value_cansleep(sensor->reset, 1);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ov5648.c: In function ‘ov5648_probe’:
/build/stage/linux/drivers/media/i2c/ov5648.c:2476:29: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
 2476 |         sensor->powerdown = devm_gpiod_get_optional(dev, "powerdown",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~
      |                             devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/ov5648.c:2477:53: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
 2477 |                                                     GPIOD_OUT_HIGH);
      |                                                     ^~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ov5648.c:2477:53: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/ov5648.o] Error 1
/build/stage/linux/drivers/media/i2c/ov5693.c: In function ‘ov5693_sensor_powerdown’:
/build/stage/linux/drivers/media/i2c/ov5693.c:793:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
  793 |         gpiod_set_value_cansleep(ov5693->privacy_led, 0);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ov5693.c: In function ‘ov5693_configure_gpios’:
/build/stage/linux/drivers/media/i2c/ov5693.c:1317:25: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
 1317 |         ov5693->reset = devm_gpiod_get_optional(ov5693->dev, "reset",
      |                         ^~~~~~~~~~~~~~~~~~~~~~~
      |                         devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/ov5693.c:1318:49: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
 1318 |                                                 GPIOD_OUT_HIGH);
      |                                                 ^~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ov5693.c:1318:49: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/media/i2c/ov5693.c:1332:55: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
 1332 |                                                       GPIOD_OUT_LOW);
      |                                                       ^~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/ov5693.o] Error 1
/build/stage/linux/drivers/media/i2c/ov8865.c: In function ‘ov8865_sensor_power’:
/build/stage/linux/drivers/media/i2c/ov8865.c:2403:17: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
 2403 |                 gpiod_set_value_cansleep(sensor->reset, 1);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ov9282.c: In function ‘ov9282_parse_hw_config’:
/build/stage/linux/drivers/media/i2c/ov9282.c:1129:30: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
 1129 |         ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
      |                              ^~~~~~~~~~~~~~~~~~~~~~~
      |                              devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/ov8865.c: In function ‘ov8865_probe’:
/build/stage/linux/drivers/media/i2c/ov8865.c:2999:29: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
 2999 |         sensor->powerdown = devm_gpiod_get_optional(dev, "powerdown",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~
      |                             devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/ov9282.c:1130:54: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
 1130 |                                                      GPIOD_OUT_LOW);
      |                                                      ^~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ov9282.c:1130:54: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/media/i2c/ov8865.c:3000:53: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
 3000 |                                                     GPIOD_OUT_HIGH);
      |                                                     ^~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/ov8865.c:3000:53: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/ov8865.o] Error 1
/build/stage/linux/drivers/media/i2c/ov9282.c: In function ‘ov9282_power_on’:
/build/stage/linux/drivers/media/i2c/ov9282.c:1238:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
 1238 |         gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/ov9282.o] Error 1
/build/stage/linux/drivers/staging/media/max96712/max96712.c: In function ‘max96712_probe’:
/build/stage/linux/drivers/staging/media/max96712/max96712.c:411:28: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_regulator_get_optional’? [-Werror=implicit-function-declaration]
  411 |         priv->gpiod_pwdn = devm_gpiod_get_optional(&client->dev, "enable",
      |                            ^~~~~~~~~~~~~~~~~~~~~~~
      |                            devm_regulator_get_optional
/build/stage/linux/drivers/staging/media/max96712/max96712.c:412:52: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
  412 |                                                    GPIOD_OUT_HIGH);
      |                                                    ^~~~~~~~~~~~~~
/build/stage/linux/drivers/staging/media/max96712/max96712.c:412:52: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/staging/media/max96712/max96712.c:416:9: error: implicit declaration of function ‘gpiod_set_consumer_name’ [-Werror=implicit-function-declaration]
  416 |         gpiod_set_consumer_name(priv->gpiod_pwdn, "max96712-pwdn");
      |         ^~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/staging/media/max96712/max96712.c:417:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
  417 |         gpiod_set_value_cansleep(priv->gpiod_pwdn, 1);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[7]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/staging/media/max96712/max96712.o] Error 1
make[7]: Target 'drivers/staging/media/max96712/' not remade because of errors.
make[6]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/staging/media/max96712] Error 2
/build/stage/linux/drivers/media/i2c/tc358746.c: In function ‘tc358746_probe’:
/build/stage/linux/drivers/media/i2c/tc358746.c:1538:32: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? [-Werror=implicit-function-declaration]
 1538 |         tc358746->reset_gpio = devm_gpiod_get_optional(dev, "reset",
      |                                ^~~~~~~~~~~~~~~~~~~~~~~
      |                                devm_clk_get_optional
/build/stage/linux/drivers/media/i2c/tc358746.c:1539:56: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
 1539 |                                                        GPIOD_OUT_HIGH);
      |                                                        ^~~~~~~~~~~~~~
/build/stage/linux/drivers/media/i2c/tc358746.c:1539:56: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/media/i2c/tc358746.c: In function ‘tc358746_resume’:
/build/stage/linux/drivers/media/i2c/tc358746.c:1637:9: error: implicit declaration of function ‘gpiod_set_value’ [-Werror=implicit-function-declaration]
 1637 |         gpiod_set_value(tc358746->reset_gpio, 1);
      |         ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/media/i2c/tc358746.o] Error 1
make[6]: Target 'drivers/media/i2c/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/media/i2c] Error 2
make[6]: Target 'drivers/staging/media/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/staging/media] Error 2
make[5]: Target 'drivers/staging/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/staging] Error 2
make[5]: Target 'drivers/media/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/media] Error 2
/build/stage/linux/drivers/iio/amplifiers/ada4250.c: In function ‘ada4250_init’:
/build/stage/linux/drivers/iio/amplifiers/ada4250.c:307:25: error: implicit declaration of function ‘device_property_read_bool’ [-Werror=implicit-function-declaration]
  307 |         st->refbuf_en = device_property_read_bool(&spi->dev, "adi,refbuf-enable");
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/addac/ad74413r.c: In function ‘ad74413r_reset’:
/build/stage/linux/drivers/iio/addac/ad74413r.c:413:17: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
  413 |                 gpiod_set_value_cansleep(st->reset_gpio, 1);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/addac/ad74115.c: In function ‘ad74115_reset’:
/build/stage/linux/drivers/iio/addac/ad74115.c:1765:22: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_regulator_get_optional’? [-Werror=implicit-function-declaration]
 1765 |         reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
      |                      ^~~~~~~~~~~~~~~~~~~~~~~
      |                      devm_regulator_get_optional
cc1: all warnings being treated as errors
/build/stage/linux/drivers/iio/addac/ad74115.c:1765:60: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
 1765 |         reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
      |                                                            ^~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/addac/ad74115.c:1765:60: note: each undeclared identifier is reported only once for each function it appears in
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/amplifiers/ada4250.o] Error 1
make[6]: Target 'drivers/iio/amplifiers/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/amplifiers] Error 2
/build/stage/linux/drivers/iio/addac/ad74413r.c: In function ‘ad74413r_probe’:
/build/stage/linux/drivers/iio/addac/ad74413r.c:1375:26: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_regulator_get_optional’? [-Werror=implicit-function-declaration]
 1375 |         st->reset_gpio = devm_gpiod_get_optional(st->dev, "reset", GPIOD_OUT_LOW);
      |                          ^~~~~~~~~~~~~~~~~~~~~~~
      |                          devm_regulator_get_optional
/build/stage/linux/drivers/iio/addac/ad74413r.c:1375:68: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
 1375 |         st->reset_gpio = devm_gpiod_get_optional(st->dev, "reset", GPIOD_OUT_LOW);
      |                                                                    ^~~~~~~~~~~~~
/build/stage/linux/drivers/iio/addac/ad74413r.c:1375:68: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/addac/ad74413r.o] Error 1
/build/stage/linux/drivers/iio/addac/ad74115.c:1773:17: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
 1773 |                 gpiod_set_value_cansleep(reset_gpio, 0);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/addac/ad74115.o] Error 1
make[6]: Target 'drivers/iio/addac/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/addac] Error 2
/build/stage/linux/drivers/iio/dac/ad3552r.c: In function ‘ad3552r_trigger_hw_ldac’:
/build/stage/linux/drivers/iio/dac/ad3552r.c:521:9: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
  521 |         gpiod_set_value_cansleep(ldac, 0);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c: In function ‘ad3552r_reset’:
/build/stage/linux/drivers/iio/dac/ad3552r.c:661:27: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_regulator_get_optional’? [-Werror=implicit-function-declaration]
  661 |         dac->gpio_reset = devm_gpiod_get_optional(&dac->spi->dev, "reset",
      |                           ^~~~~~~~~~~~~~~~~~~~~~~
      |                           devm_regulator_get_optional
/build/stage/linux/drivers/iio/dac/ad3552r.c:662:51: error: ‘GPIOD_OUT_LOW’ undeclared (first use in this function)
  662 |                                                   GPIOD_OUT_LOW);
      |                                                   ^~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:662:51: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/iio/dac/ad3552r.c: In function ‘ad3552r_configure_custom_gain’:
/build/stage/linux/drivers/iio/dac/ad3552r.c:810:22: error: implicit declaration of function ‘fwnode_get_named_child_node’ [-Werror=implicit-function-declaration]
  810 |         gain_child = fwnode_get_named_child_node(child,
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:810:20: error: assignment to ‘struct fwnode_handle *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  810 |         gain_child = fwnode_get_named_child_node(child,
      |                    ^
/build/stage/linux/drivers/iio/dac/ad3552r.c:821:15: error: implicit declaration of function ‘fwnode_property_read_u32’ [-Werror=implicit-function-declaration]
  821 |         err = fwnode_property_read_u32(gain_child, "adi,gain-scaling-p", &val);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:870:9: error: implicit declaration of function ‘fwnode_handle_put’ [-Werror=implicit-function-declaration]
  870 |         fwnode_handle_put(gain_child);
      |         ^~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c: In function ‘ad3552r_configure_device’:
/build/stage/linux/drivers/iio/dac/ad3552r.c:888:63: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
  888 |         dac->gpio_ldac = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_HIGH);
      |                                                               ^~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:899:21: error: implicit declaration of function ‘device_property_read_bool’ [-Werror=implicit-function-declaration]
  899 |                 if (device_property_read_bool(dev, "adi,vref-out-en"))
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:931:15: error: implicit declaration of function ‘device_property_read_u32’ [-Werror=implicit-function-declaration]
  931 |         err = device_property_read_u32(dev, "adi,sdo-drive-strength", &val);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:946:23: error: implicit declaration of function ‘device_get_child_node_count’ [-Werror=implicit-function-declaration]
  946 |         dac->num_ch = device_get_child_node_count(dev);
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:952:9: error: implicit declaration of function ‘device_for_each_child_node’; did you mean ‘device_for_each_child’? [-Werror=implicit-function-declaration]
  952 |         device_for_each_child_node(dev, child) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |         device_for_each_child
/build/stage/linux/drivers/iio/dac/ad3552r.c:952:47: error: expected ‘;’ before ‘{’ token
  952 |         device_for_each_child_node(dev, child) {
      |                                               ^~
      |                                               ;
/build/stage/linux/drivers/iio/dac/ad3552r.c:1024:1: error: label ‘put_child’ defined but not used [-Werror=unused-label]
 1024 | put_child:
      | ^~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:886:27: error: unused variable ‘ch’ [-Werror=unused-variable]
  886 |         u32 vals[2], val, ch;
      |                           ^~
/build/stage/linux/drivers/iio/dac/ad3552r.c:886:13: error: unused variable ‘vals’ [-Werror=unused-variable]
  886 |         u32 vals[2], val, ch;
      |             ^~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:885:18: error: unused variable ‘cnt’ [-Werror=unused-variable]
  885 |         int err, cnt = 0, voltage, delta = 100000;
      |                  ^~~
/build/stage/linux/drivers/iio/dac/ad3552r.c: At top level:
/build/stage/linux/drivers/iio/dac/ad3552r.c:799:12: error: ‘ad3552r_configure_custom_gain’ defined but not used [-Werror=unused-function]
  799 | static int ad3552r_configure_custom_gain(struct ad3552r_desc *dac,
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:778:12: error: ‘ad3552r_find_range’ defined but not used [-Werror=unused-function]
  778 | static int ad3552r_find_range(u16 id, s32 *vals)
      |            ^~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/dac/ad3552r.c:738:13: error: ‘ad3552r_calc_gain_and_offset’ defined but not used [-Werror=unused-function]
  738 | static void ad3552r_calc_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/dac/ad3552r.o] Error 1
/build/stage/linux/drivers/iio/adc/ad7280a.c: In function ‘ad7280_probe’:
/build/stage/linux/drivers/iio/adc/ad7280a.c:974:17: error: implicit declaration of function ‘device_property_read_bool’ [-Werror=implicit-function-declaration]
  974 |                 device_property_read_bool(dev, "adi,thermistor-termination");
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ad7280a.c:976:13: error: implicit declaration of function ‘device_property_present’ [-Werror=implicit-function-declaration]
  976 |         if (device_property_present(dev, "adi,acquisition-time-ns")) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ad7280a.c:979:23: error: implicit declaration of function ‘device_property_read_u32’ [-Werror=implicit-function-declaration]
  979 |                 ret = device_property_read_u32(dev, "adi,acquisition-time-ns", &val);
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/ad7280a.o] Error 1
/build/stage/linux/drivers/iio/frequency/adf4371.c: In function ‘adf4371_setup’:
/build/stage/linux/drivers/iio/frequency/adf4371.c:492:13: error: implicit declaration of function ‘device_property_read_bool’ [-Werror=implicit-function-declaration]
  492 |         if (device_property_read_bool(&st->spi->dev, "adi,mute-till-lock-en")) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/frequency/adf4371.o] Error 1
/build/stage/linux/drivers/iio/adc/ad7606_spi.c: In function ‘ad7606B_sw_mode_config’:
/build/stage/linux/drivers/iio/adc/ad7606_spi.c:261:17: error: implicit declaration of function ‘gpiod_set_array_value’ [-Werror=implicit-function-declaration]
  261 |                 gpiod_set_array_value(ARRAY_SIZE(os),
      |                 ^~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ad7606_spi.c:262:50: error: invalid use of undefined type ‘struct gpio_descs’
  262 |                                       st->gpio_os->desc, st->gpio_os->info, os);
      |                                                  ^~
/build/stage/linux/drivers/iio/adc/ad7606_spi.c:262:69: error: invalid use of undefined type ‘struct gpio_descs’
  262 |                                       st->gpio_os->desc, st->gpio_os->info, os);
      |                                                                     ^~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/ad7606_spi.o] Error 1
/build/stage/linux/drivers/iio/adc/ad7768-1.c: In function ‘ad7768_set_channel_label’:
/build/stage/linux/drivers/iio/adc/ad7768-1.c:552:18: error: implicit declaration of function ‘dev_fwnode’; did you mean ‘dev_of_node’? [-Werror=implicit-function-declaration]
  552 |         fwnode = dev_fwnode(device);
      |                  ^~~~~~~~~~
      |                  dev_of_node
/build/stage/linux/drivers/iio/adc/ad7768-1.c:552:16: error: assignment to ‘struct fwnode_handle *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  552 |         fwnode = dev_fwnode(device);
      |                ^
/build/stage/linux/drivers/iio/dac/ad5766.c: In function ‘ad5766_get_output_range’:
/build/stage/linux/drivers/iio/dac/ad5766.c:500:15: error: implicit declaration of function ‘device_property_read_u32_array’ [-Werror=implicit-function-declaration]
  500 |         ret = device_property_read_u32_array(&st->spi->dev,
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ad7768-1.c:553:9: error: implicit declaration of function ‘fwnode_for_each_child_node’ [-Werror=implicit-function-declaration]
  553 |         fwnode_for_each_child_node(fwnode, child) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ad7768-1.c:553:50: error: expected ‘;’ before ‘{’ token
  553 |         fwnode_for_each_child_node(fwnode, child) {
      |                                                  ^~
      |                                                  ;
/build/stage/linux/drivers/iio/adc/ad7768-1.c:550:13: error: unused variable ‘crt_ch’ [-Werror=unused-variable]
  550 |         int crt_ch = 0;
      |             ^~~~~~
/build/stage/linux/drivers/iio/adc/ad7768-1.c:549:21: error: unused variable ‘label’ [-Werror=unused-variable]
  549 |         const char *label;
      |                     ^~~~~
/build/stage/linux/drivers/iio/adc/ad7768-1.c:545:30: error: unused variable ‘st’ [-Werror=unused-variable]
  545 |         struct ad7768_state *st = iio_priv(indio_dev);
      |                              ^~
/build/stage/linux/drivers/iio/adc/ad7768-1.c:567:1: error: no return statement in function returning non-void [-Werror=return-type]
  567 | }
      | ^
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/ad7768-1.o] Error 1
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/dac/ad5766.o] Error 1
/build/stage/linux/drivers/iio/frequency/admv4420.c: In function ‘admv4420_fw_parse’:
/build/stage/linux/drivers/iio/frequency/admv4420.c:194:15: error: implicit declaration of function ‘device_property_read_u32’ [-Werror=implicit-function-declaration]
  194 |         ret = device_property_read_u32(dev, "adi,lo-freq-khz", &tmp);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/frequency/admv4420.c:198:42: error: implicit declaration of function ‘device_property_read_bool’ [-Werror=implicit-function-declaration]
  198 |         st->ref_block.ref_single_ended = device_property_read_bool(dev,
      |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/frequency/admv4420.o] Error 1
/build/stage/linux/drivers/iio/adc/ad7949.c: In function ‘ad7949_spi_probe’:
/build/stage/linux/drivers/iio/adc/ad7949.c:353:9: error: implicit declaration of function ‘device_property_read_u32’ [-Werror=implicit-function-declaration]
  353 |         device_property_read_u32(dev, "adi,internal-ref-microvolt", &tmp);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/ad7949.o] Error 1
make[6]: Target 'drivers/iio/frequency/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/frequency] Error 2
/build/stage/linux/drivers/iio/pressure/bmp280-spi.c: In function ‘bmp280_spi_probe’:
/build/stage/linux/drivers/iio/pressure/bmp280-spi.c:61:21: error: implicit declaration of function ‘device_get_match_data’; did you mean ‘device_match_any’? [-Werror=implicit-function-declaration]
   61 |         chip_info = device_get_match_data(&spi->dev);
      |                     ^~~~~~~~~~~~~~~~~~~~~
      |                     device_match_any
/build/stage/linux/drivers/iio/pressure/bmp280-spi.c:61:19: error: assignment to ‘const struct bmp280_chip_info *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
   61 |         chip_info = device_get_match_data(&spi->dev);
      |                   ^
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/pressure/bmp280-spi.o] Error 1
/build/stage/linux/drivers/iio/proximity/as3935.c: In function ‘as3935_probe’:
/build/stage/linux/drivers/iio/proximity/as3935.c:366:15: error: implicit declaration of function ‘device_property_read_u32’ [-Werror=implicit-function-declaration]
  366 |         ret = device_property_read_u32(dev,
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/proximity/as3935.o] Error 1
make[6]: Target 'drivers/iio/proximity/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/proximity] Error 2
/build/stage/linux/drivers/iio/temperature/ltc2983.c: In function ‘ltc2983_probe’:
/build/stage/linux/drivers/iio/temperature/ltc2983.c:1639:16: error: implicit declaration of function ‘devm_gpiod_get_optional’ [-Werror=implicit-function-declaration]
 1639 |         gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH);
      |                ^~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/temperature/ltc2983.c:1639:64: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function)
 1639 |         gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH);
      |                                                                ^~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/temperature/ltc2983.c:1639:64: note: each undeclared identifier is reported only once for each function it appears in
/build/stage/linux/drivers/iio/temperature/ltc2983.c:1646:17: error: implicit declaration of function ‘gpiod_set_value_cansleep’ [-Werror=implicit-function-declaration]
 1646 |                 gpiod_set_value_cansleep(gpio, 0);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/temperature/ltc2983.o] Error 1
make[6]: Target 'drivers/iio/temperature/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/temperature] Error 2
/build/stage/linux/drivers/iio/dac/max5522.c: In function ‘max5522_spi_probe’:
/build/stage/linux/drivers/iio/dac/max5522.c:147:28: error: implicit declaration of function ‘device_get_match_data’; did you mean ‘device_match_any’? [-Werror=implicit-function-declaration]
  147 |         state->chip_info = device_get_match_data(&spi->dev);
      |                            ^~~~~~~~~~~~~~~~~~~~~
      |                            device_match_any
/build/stage/linux/drivers/iio/dac/max5522.c:147:26: error: assignment to ‘const struct max5522_chip_info *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  147 |         state->chip_info = device_get_match_data(&spi->dev);
      |                          ^
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/dac/max5522.o] Error 1
make[6]: Target 'drivers/iio/dac/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/dac] Error 2
/build/stage/linux/drivers/iio/adc/max11205.c: In function ‘max11205_probe’:
/build/stage/linux/drivers/iio/adc/max11205.c:119:25: error: implicit declaration of function ‘device_get_match_data’; did you mean ‘device_match_any’? [-Werror=implicit-function-declaration]
  119 |         st->chip_info = device_get_match_data(&spi->dev);
      |                         ^~~~~~~~~~~~~~~~~~~~~
      |                         device_match_any
/build/stage/linux/drivers/iio/adc/max11205.c:119:23: error: assignment to ‘const struct max11205_chip_info *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  119 |         st->chip_info = device_get_match_data(&spi->dev);
      |                       ^
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/max11205.o] Error 1
/build/stage/linux/drivers/iio/adc/max11410.c: In function ‘max11410_parse_channels’:
/build/stage/linux/drivers/iio/adc/max11410.c:707:18: error: implicit declaration of function ‘device_get_child_node_count’ [-Werror=implicit-function-declaration]
  707 |         num_ch = device_get_child_node_count(dev);
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/max11410.c:723:9: error: implicit declaration of function ‘device_for_each_child_node’; did you mean ‘device_for_each_child’? [-Werror=implicit-function-declaration]
  723 |         device_for_each_child_node(dev, child) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |         device_for_each_child
/build/stage/linux/drivers/iio/adc/max11410.c:723:47: error: expected ‘;’ before ‘{’ token
  723 |         device_for_each_child_node(dev, child) {
      |                                               ^~
      |                                               ;
/build/stage/linux/drivers/iio/adc/max11410.c:705:18: error: unused variable ‘i’ [-Werror=unused-variable]
  705 |         int ret, i;
      |                  ^
/build/stage/linux/drivers/iio/adc/max11410.c:705:13: error: unused variable ‘ret’ [-Werror=unused-variable]
  705 |         int ret, i;
      |             ^~~
/build/stage/linux/drivers/iio/adc/max11410.c:702:24: error: unused variable ‘scale’ [-Werror=unused-variable]
  702 |         u32 inputs[2], scale;
      |                        ^~~~~
/build/stage/linux/drivers/iio/adc/max11410.c:702:13: error: unused variable ‘inputs’ [-Werror=unused-variable]
  702 |         u32 inputs[2], scale;
      |             ^~~~~~
/build/stage/linux/drivers/iio/adc/max11410.c:701:21: error: unused variable ‘node_name’ [-Werror=unused-variable]
  701 |         const char *node_name;
      |                     ^~~~~~~~~
/build/stage/linux/drivers/iio/adc/max11410.c:700:24: error: unused variable ‘sig_path’ [-Werror=unused-variable]
  700 |         u32 reference, sig_path;
      |                        ^~~~~~~~
/build/stage/linux/drivers/iio/adc/max11410.c:700:13: error: unused variable ‘reference’ [-Werror=unused-variable]
  700 |         u32 reference, sig_path;
      |             ^~~~~~~~~
/build/stage/linux/drivers/iio/adc/max11410.c:697:41: error: unused variable ‘cfg’ [-Werror=unused-variable]
  697 |         struct max11410_channel_config *cfg;
      |                                         ^~~
/build/stage/linux/drivers/iio/adc/max11410.c:695:30: error: unused variable ‘chanspec’ [-Werror=unused-variable]
  695 |         struct iio_chan_spec chanspec = chanspec_template;
      |                              ^~~~~~~~
/build/stage/linux/drivers/iio/adc/max11410.c: In function ‘max11410_probe’:
/build/stage/linux/drivers/iio/adc/max11410.c:976:19: error: implicit declaration of function ‘fwnode_irq_get_byname’ [-Werror=implicit-function-declaration]
  976 |         irqs[0] = fwnode_irq_get_byname(dev_fwnode(dev), "gpio0");
      |                   ^~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/max11410.c:976:41: error: implicit declaration of function ‘dev_fwnode’; did you mean ‘dev_of_node’? [-Werror=implicit-function-declaration]
  976 |         irqs[0] = fwnode_irq_get_byname(dev_fwnode(dev), "gpio0");
      |                                         ^~~~~~~~~~
      |                                         dev_of_node
cc1: all warnings being treated as errors
make[6]: Target 'drivers/iio/pressure/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/pressure] Error 2
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/max11410.o] Error 1
/build/stage/linux/drivers/iio/adc/ti-lmp92064.c: In function ‘lmp92064_adc_probe’:
/build/stage/linux/drivers/iio/adc/ti-lmp92064.c:265:15: error: implicit declaration of function ‘device_property_read_u32’ [-Werror=implicit-function-declaration]
  265 |         ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c: In function ‘ads131e08_alloc_channels’:
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:701:15: error: implicit declaration of function ‘device_property_read_u32’ [-Werror=implicit-function-declaration]
  701 |         ret = device_property_read_u32(dev, "ti,vref-internal", &tmp);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ti-tsc2046.c: In function ‘tsc2046_adc_parse_fwnode’:
/build/stage/linux/drivers/iio/adc/ti-tsc2046.c:721:9: error: implicit declaration of function ‘device_for_each_child_node’; did you mean ‘device_for_each_child’? [-Werror=implicit-function-declaration]
  721 |         device_for_each_child_node(dev, child) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |         device_for_each_child
/build/stage/linux/drivers/iio/adc/ti-tsc2046.c:721:47: error: expected ‘;’ before ‘{’ token
  721 |         device_for_each_child_node(dev, child) {
      |                                               ^~
      |                                               ;
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/ti-lmp92064.o] Error 1
/build/stage/linux/drivers/iio/adc/ti-tsc2046.c: In function ‘tsc2046_adc_probe’:
/build/stage/linux/drivers/iio/adc/ti-tsc2046.c:807:16: error: implicit declaration of function ‘device_get_match_data’; did you mean ‘device_match_any’? [-Werror=implicit-function-declaration]
  807 |         dcfg = device_get_match_data(dev);
      |                ^~~~~~~~~~~~~~~~~~~~~
      |                device_match_any
/build/stage/linux/drivers/iio/adc/ti-tsc2046.c:807:14: error: assignment to ‘const struct tsc2046_adc_dcfg *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  807 |         dcfg = device_get_match_data(dev);
      |              ^
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:717:24: error: implicit declaration of function ‘device_get_child_node_count’ [-Werror=implicit-function-declaration]
  717 |         num_channels = device_get_child_node_count(dev);
      |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/ti-tsc2046.o] Error 1
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:739:9: error: implicit declaration of function ‘device_for_each_child_node’; did you mean ‘device_for_each_child’? [-Werror=implicit-function-declaration]
  739 |         device_for_each_child_node(dev, node) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |         device_for_each_child
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:739:46: error: expected ‘;’ before ‘{’ token
  739 |         device_for_each_child_node(dev, node) {
      |                                              ^~
      |                                              ;
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:789:9: error: implicit declaration of function ‘fwnode_handle_put’ [-Werror=implicit-function-declaration]
  789 |         fwnode_handle_put(node);
      |         ^~~~~~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:788:1: error: label ‘err_child_out’ defined but not used [-Werror=unused-label]
  788 | err_child_out:
      | ^~~~~~~~~~~~~
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:698:22: error: unused variable ‘channel’ [-Werror=unused-variable]
  698 |         unsigned int channel, tmp;
      |                      ^~~~~~~
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c: In function ‘ads131e08_probe’:
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:809:16: error: implicit declaration of function ‘device_get_match_data’; did you mean ‘device_match_any’? [-Werror=implicit-function-declaration]
  809 |         info = device_get_match_data(&spi->dev);
      |                ^~~~~~~~~~~~~~~~~~~~~
      |                device_match_any
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:809:14: error: assignment to ‘const struct ads131e08_info *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
  809 |         info = device_get_match_data(&spi->dev);
      |              ^
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c: At top level:
/build/stage/linux/drivers/iio/adc/ti-ads131e08.c:320:12: error: ‘ads131e08_validate_channel_mux’ defined but not used [-Werror=unused-function]
  320 | static int ads131e08_validate_channel_mux(struct ads131e08_state *st,
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/build/stage/linux/scripts/Makefile.build:243: drivers/iio/adc/ti-ads131e08.o] Error 1
make[6]: Target 'drivers/iio/adc/' not remade because of errors.
make[5]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio/adc] Error 2
make[5]: Target 'drivers/iio/' not remade because of errors.
make[4]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers/iio] Error 2
make[4]: Target 'drivers/' not remade because of errors.
make[3]: *** [/build/stage/linux/scripts/Makefile.build:477: drivers] Error 2
make[3]: Target './' not remade because of errors.
make[2]: *** [/build/stage/linux/Makefile:2020: .] Error 2
make[2]: Target '__all' not remade because of errors.
make[1]: *** [/build/stage/linux/Makefile:234: __sub-make] Error 2
make[1]: Target '__all' not remade because of errors.

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

Re: [PATCH v2 08/15] spi: Clean up headers

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 09:26:46AM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2023 at 06:49:25PM +0300, Andy Shevchenko wrote:
> > There is a few things done:
> > - include only the headers we are direct user of
> > - when pointer is in use, provide a forward declaration
> > - add missing headers
> > - group generic headers and subsystem headers
> > - sort each group alphabetically
> 
> This breaks an x86 allmodconfig build:

Yeah, it was too brave to go. That's actually the answer why I left
mod_devicetable.h included. (However the mod_devicetable.h probably should
stay to be similar to i2c.h.

Let's postpone this one, but it's good to have the build report so we know
which drivers are missing GPIO header to be included.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 09/15] spi: Use struct_size() helper

[Andy Shevchenko]

Prefer struct_size() over open-coded versions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/spi/spi.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index c9479badf38c..9fb8efb068c6 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -17,6 +17,7 @@
 #include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
 #include <linux/mutex.h>
+#include <linux/overflow.h>
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/smp.h>
@@ -1095,6 +1096,8 @@ struct spi_transfer {
  * @state: for use by whichever driver currently owns the message
  * @resources: for resource management when the spi message is processed
  * @prepared: spi_prepare_message was called for the this message
+ * @t: for use with spi_message_alloc() when message and transfers have
+ *	been allocated together
  *
  * A @spi_message is used to execute an atomic sequence of data transfers,
  * each represented by a struct spi_transfer.  The sequence is "atomic"
@@ -1147,6 +1150,9 @@ struct spi_message {
 
 	/* List of spi_res reources when the spi message is processed */
 	struct list_head        resources;
+
+	/* For embedding transfers into the memory of the message */
+	struct spi_transfer	t[];
 };
 
 static inline void spi_message_init_no_memset(struct spi_message *m)
@@ -1207,16 +1213,13 @@ static inline struct spi_message *spi_message_alloc(unsigned ntrans, gfp_t flags
 {
 	struct spi_message *m;
 
-	m = kzalloc(sizeof(struct spi_message)
-			+ ntrans * sizeof(struct spi_transfer),
-			flags);
+	m = kzalloc(struct_size(m, t, ntrans), flags);
 	if (m) {
 		unsigned i;
-		struct spi_transfer *t = (struct spi_transfer *)(m + 1);
 
 		spi_message_init_no_memset(m);
-		for (i = 0; i < ntrans; i++, t++)
-			spi_message_add_tail(t, m);
+		for (i = 0; i < ntrans; i++)
+			spi_message_add_tail(&m->t[i], m);
 	}
 	return m;
 }
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 09/15] spi: Use struct_size() helper

[Marc Kleine-Budde]

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

On 10.07.2023 18:49:26, Andy Shevchenko wrote:
> Prefer struct_size() over open-coded versions.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/spi/spi.h | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index c9479badf38c..9fb8efb068c6 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -17,6 +17,7 @@
>  #include <linux/minmax.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/mutex.h>
> +#include <linux/overflow.h>
>  #include <linux/scatterlist.h>
>  #include <linux/slab.h>
>  #include <linux/smp.h>
> @@ -1095,6 +1096,8 @@ struct spi_transfer {
>   * @state: for use by whichever driver currently owns the message
>   * @resources: for resource management when the spi message is processed
>   * @prepared: spi_prepare_message was called for the this message
> + * @t: for use with spi_message_alloc() when message and transfers have
> + *	been allocated together
>   *
>   * A @spi_message is used to execute an atomic sequence of data transfers,
>   * each represented by a struct spi_transfer.  The sequence is "atomic"
> @@ -1147,6 +1150,9 @@ struct spi_message {
>  
>  	/* List of spi_res reources when the spi message is processed */
>  	struct list_head        resources;
> +
> +	/* For embedding transfers into the memory of the message */
> +	struct spi_transfer	t[];

You might want to use the DECLARE_FLEX_ARRAY helper here.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

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

Re: [PATCH v2 09/15] spi: Use struct_size() helper

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 05:59:55PM +0200, Marc Kleine-Budde wrote:
> On 10.07.2023 18:49:26, Andy Shevchenko wrote:

...

> > +	struct spi_transfer	t[];
> 
> You might want to use the DECLARE_FLEX_ARRAY helper here.

Technically, yes, semantically documentation [1] disagrees with
you, so I leave it as is.

[1]: Documentation/process/deprecated.rst:269

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 09/15] spi: Use struct_size() helper

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 07:10:43PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 10, 2023 at 05:59:55PM +0200, Marc Kleine-Budde wrote:
> > On 10.07.2023 18:49:26, Andy Shevchenko wrote:

...

> > > +	struct spi_transfer	t[];
> > 
> > You might want to use the DECLARE_FLEX_ARRAY helper here.
> 
> Technically, yes, semantically documentation [1] disagrees with

"and [2]"

> you, so I leave it as is.
> 
> [1]: Documentation/process/deprecated.rst:269

[2]: Documentation/process/deprecated.rst:350

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 10/15] spi: Use predefined constants from bits.h and units.h

[Andy Shevchenko]

Instead of hard coded numbers, use predefined constancts,
such as BITS_PER_BYTE.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/spi/spi.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 9fb8efb068c6..10265184ea02 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -25,6 +25,7 @@
 #include <linux/string.h>
 #include <linux/types.h>
 #include <linux/u64_stats_sync.h>
+#include <linux/units.h>
 
 #include <asm/byteorder.h>
 
@@ -1294,9 +1295,9 @@ static inline bool spi_is_bpw_supported(struct spi_device *spi, u32 bpw)
 static inline unsigned int spi_controller_xfer_timeout(struct spi_controller *ctlr,
 						       struct spi_transfer *xfer)
 {
-	u32 speed_hz = xfer->speed_hz ?: 100000;
+	u32 speed_hz = xfer->speed_hz ?: 100 * HZ_PER_KHZ;
 
-	return max(xfer->len * 8 * 2 / (speed_hz / 1000), 500U);
+	return max(xfer->len * BITS_PER_BYTE * 2 / (speed_hz / MILLI), 500UL);
 }
 
 /*---------------------------------------------------------------------------*/
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 11/15] spi: Get rid of old SPI_MASTER_NO_TX & SPI_MASTER_NO_RX

[Andy Shevchenko]

Convert the users from SPI_MASTER_NO_TX and/or SPI_MASTER_NO_RX
to SPI_CONTROLLER_NO_TX and/or SPI_CONTROLLER_NO_RX respectively
and kill the not used anymore definitions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-bitbang-txrx.h  | 16 ++++++++--------
 drivers/spi/spi-bitbang.c       |  2 +-
 drivers/spi/spi-gpio.c          |  8 ++++----
 drivers/spi/spi-lp8841-rtc.c    |  8 ++++----
 drivers/spi/spi-xtensa-xtfpga.c |  2 +-
 include/linux/spi/spi.h         |  2 --
 6 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/spi-bitbang-txrx.h b/drivers/spi/spi-bitbang-txrx.h
index 2dcbe166df63..0cab48b7875b 100644
--- a/drivers/spi/spi-bitbang-txrx.h
+++ b/drivers/spi/spi-bitbang-txrx.h
@@ -57,7 +57,7 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
 	for (word <<= (32 - bits); likely(bits); bits--) {
 
 		/* setup MSB (to slave) on trailing edge */
-		if ((flags & SPI_MASTER_NO_TX) == 0) {
+		if ((flags & SPI_CONTROLLER_NO_TX) == 0) {
 			if ((word & (1 << 31)) != oldbit) {
 				setmosi(spi, word & (1 << 31));
 				oldbit = word & (1 << 31);
@@ -70,7 +70,7 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,
 
 		/* sample MSB (from slave) on leading edge */
 		word <<= 1;
-		if ((flags & SPI_MASTER_NO_RX) == 0)
+		if ((flags & SPI_CONTROLLER_NO_RX) == 0)
 			word |= getmiso(spi);
 		setsck(spi, cpol);
 	}
@@ -90,7 +90,7 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,
 
 		/* setup MSB (to slave) on leading edge */
 		setsck(spi, !cpol);
-		if ((flags & SPI_MASTER_NO_TX) == 0) {
+		if ((flags & SPI_CONTROLLER_NO_TX) == 0) {
 			if ((word & (1 << 31)) != oldbit) {
 				setmosi(spi, word & (1 << 31));
 				oldbit = word & (1 << 31);
@@ -103,7 +103,7 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,
 
 		/* sample MSB (from slave) on trailing edge */
 		word <<= 1;
-		if ((flags & SPI_MASTER_NO_RX) == 0)
+		if ((flags & SPI_CONTROLLER_NO_RX) == 0)
 			word |= getmiso(spi);
 	}
 	return word;
@@ -122,7 +122,7 @@ bitbang_txrx_le_cpha0(struct spi_device *spi,
 	for (; likely(bits); bits--) {
 
 		/* setup LSB (to slave) on trailing edge */
-		if ((flags & SPI_MASTER_NO_TX) == 0) {
+		if ((flags & SPI_CONTROLLER_NO_TX) == 0) {
 			if ((word & 1) != oldbit) {
 				setmosi(spi, word & 1);
 				oldbit = word & 1;
@@ -135,7 +135,7 @@ bitbang_txrx_le_cpha0(struct spi_device *spi,
 
 		/* sample LSB (from slave) on leading edge */
 		word >>= 1;
-		if ((flags & SPI_MASTER_NO_RX) == 0)
+		if ((flags & SPI_CONTROLLER_NO_RX) == 0)
 			word |= getmiso(spi) << rxbit;
 		setsck(spi, cpol);
 	}
@@ -156,7 +156,7 @@ bitbang_txrx_le_cpha1(struct spi_device *spi,
 
 		/* setup LSB (to slave) on leading edge */
 		setsck(spi, !cpol);
-		if ((flags & SPI_MASTER_NO_TX) == 0) {
+		if ((flags & SPI_CONTROLLER_NO_TX) == 0) {
 			if ((word & 1) != oldbit) {
 				setmosi(spi, word & 1);
 				oldbit = word & 1;
@@ -169,7 +169,7 @@ bitbang_txrx_le_cpha1(struct spi_device *spi,
 
 		/* sample LSB (from slave) on trailing edge */
 		word >>= 1;
-		if ((flags & SPI_MASTER_NO_RX) == 0)
+		if ((flags & SPI_CONTROLLER_NO_RX) == 0)
 			word |= getmiso(spi) << rxbit;
 	}
 	return word;
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index 27d0087f8688..862f209cada1 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -248,7 +248,7 @@ static int spi_bitbang_bufs(struct spi_device *spi, struct spi_transfer *t)
 	if (spi->mode & SPI_3WIRE) {
 		unsigned flags;
 
-		flags = t->tx_buf ? SPI_MASTER_NO_RX : SPI_MASTER_NO_TX;
+		flags = t->tx_buf ? SPI_CONTROLLER_NO_RX : SPI_CONTROLLER_NO_TX;
 		return cs->txrx_bufs(spi, cs->txrx_word, nsecs, t, flags);
 	}
 	return cs->txrx_bufs(spi, cs->txrx_word, nsecs, t, 0);
diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index 092afc7679d4..85e149ec2910 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -170,7 +170,7 @@ static u32 spi_gpio_txrx_word_mode3(struct spi_device *spi,
 
 /*
  * These functions do not call setmosi or getmiso if respective flag
- * (SPI_MASTER_NO_RX or SPI_MASTER_NO_TX) is set, so they are safe to
+ * (SPI_CONTROLLER_NO_RX or SPI_CONTROLLER_NO_TX) is set, so they are safe to
  * call when such pin is not present or defined in the controller.
  * A separate set of callbacks is defined to get highest possible
  * speed in the generic case (when both MISO and MOSI lines are
@@ -416,11 +416,11 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	if (!spi_gpio->mosi) {
 		/* HW configuration without MOSI pin
 		 *
-		 * No setting SPI_MASTER_NO_RX here - if there is only
+		 * No setting SPI_CONTROLLER_NO_RX here - if there is only
 		 * a MOSI pin connected the host can still do RX by
 		 * changing the direction of the line.
 		 */
-		master->flags = SPI_MASTER_NO_TX;
+		master->flags = SPI_CONTROLLER_NO_TX;
 	}
 
 	master->bus_num = pdev->id;
@@ -438,7 +438,7 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	bb->chipselect = spi_gpio_chipselect;
 	bb->set_line_direction = spi_gpio_set_direction;
 
-	if (master->flags & SPI_MASTER_NO_TX) {
+	if (master->flags & SPI_CONTROLLER_NO_TX) {
 		bb->txrx_word[SPI_MODE_0] = spi_gpio_spec_txrx_word_mode0;
 		bb->txrx_word[SPI_MODE_1] = spi_gpio_spec_txrx_word_mode1;
 		bb->txrx_word[SPI_MODE_2] = spi_gpio_spec_txrx_word_mode2;
diff --git a/drivers/spi/spi-lp8841-rtc.c b/drivers/spi/spi-lp8841-rtc.c
index 2d436541d6c2..ccaa7a946359 100644
--- a/drivers/spi/spi-lp8841-rtc.c
+++ b/drivers/spi/spi-lp8841-rtc.c
@@ -75,14 +75,14 @@ bitbang_txrx_be_cpha0_lsb(struct spi_lp8841_rtc *data,
 	for (; likely(bits); bits--) {
 
 		/* setup LSB (to slave) on leading edge */
-		if ((flags & SPI_MASTER_NO_TX) == 0)
+		if ((flags & SPI_CONTROLLER_NO_TX) == 0)
 			setmosi(data, (word & 1));
 
 		usleep_range(usecs, usecs + 1);	/* T(setup) */
 
 		/* sample LSB (from slave) on trailing edge */
 		word >>= 1;
-		if ((flags & SPI_MASTER_NO_RX) == 0)
+		if ((flags & SPI_CONTROLLER_NO_RX) == 0)
 			word |= (getmiso(data) << 31);
 
 		setsck(data, !cpol);
@@ -113,7 +113,7 @@ spi_lp8841_rtc_transfer_one(struct spi_master *master,
 		while (likely(count > 0)) {
 			word = *tx++;
 			bitbang_txrx_be_cpha0_lsb(data, 1, 0,
-					SPI_MASTER_NO_RX, word, 8);
+					SPI_CONTROLLER_NO_RX, word, 8);
 			count--;
 		}
 	} else if (rx) {
@@ -121,7 +121,7 @@ spi_lp8841_rtc_transfer_one(struct spi_master *master,
 		writeb(data->state, data->iomem);
 		while (likely(count > 0)) {
 			word = bitbang_txrx_be_cpha0_lsb(data, 1, 0,
-					SPI_MASTER_NO_TX, word, 8);
+					SPI_CONTROLLER_NO_TX, word, 8);
 			*rx++ = word;
 			count--;
 		}
diff --git a/drivers/spi/spi-xtensa-xtfpga.c b/drivers/spi/spi-xtensa-xtfpga.c
index 24dc845b940e..dbd85d7a1526 100644
--- a/drivers/spi/spi-xtensa-xtfpga.c
+++ b/drivers/spi/spi-xtensa-xtfpga.c
@@ -87,7 +87,7 @@ static int xtfpga_spi_probe(struct platform_device *pdev)
 	if (!master)
 		return -ENOMEM;
 
-	master->flags = SPI_MASTER_NO_RX;
+	master->flags = SPI_CONTROLLER_NO_RX;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 16);
 	master->bus_num = pdev->dev.id;
 	master->dev.of_node = pdev->dev.of_node;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 10265184ea02..ce77dcf47b96 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1645,8 +1645,6 @@ spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer)
 #define spi_master			spi_controller
 
 #define SPI_MASTER_HALF_DUPLEX		SPI_CONTROLLER_HALF_DUPLEX
-#define SPI_MASTER_NO_RX		SPI_CONTROLLER_NO_RX
-#define SPI_MASTER_NO_TX		SPI_CONTROLLER_NO_TX
 #define SPI_MASTER_MUST_RX		SPI_CONTROLLER_MUST_RX
 #define SPI_MASTER_MUST_TX		SPI_CONTROLLER_MUST_TX
 
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 12/15] spi: Get rid of old SPI_MASTER_MUST_TX & SPI_MASTER_MUST_RX

[Andy Shevchenko]

Convert the users from SPI_MASTER_MUST_TX and/or SPI_MASTER_MUST_RX
to SPI_CONTROLLER_MUST_TX and/or SPI_CONTROLLER_MUST_RX respectively
and kill the not used anymore definitions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-at91-usart.c   | 2 +-
 drivers/spi/spi-atmel.c        | 2 +-
 drivers/spi/spi-davinci.c      | 2 +-
 drivers/spi/spi-fsl-lpspi.c    | 2 +-
 drivers/spi/spi-meson-spicc.c  | 2 +-
 drivers/spi/spi-mt65xx.c       | 2 +-
 drivers/spi/spi-pci1xxxx.c     | 2 +-
 drivers/spi/spi-pic32.c        | 2 +-
 drivers/spi/spi-rb4xx.c        | 2 +-
 drivers/spi/spi-slave-mt27xx.c | 2 +-
 drivers/spi/spi-stm32.c        | 2 +-
 include/linux/spi/spi.h        | 2 --
 12 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-at91-usart.c b/drivers/spi/spi-at91-usart.c
index 7854d9790fe9..7dfe2b6c2990 100644
--- a/drivers/spi/spi-at91-usart.c
+++ b/drivers/spi/spi-at91-usart.c
@@ -527,7 +527,7 @@ static int at91_usart_spi_probe(struct platform_device *pdev)
 	controller->dev.of_node = pdev->dev.parent->of_node;
 	controller->bits_per_word_mask = SPI_BPW_MASK(8);
 	controller->setup = at91_usart_spi_setup;
-	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+	controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
 	controller->transfer_one = at91_usart_spi_transfer_one;
 	controller->prepare_message = at91_usart_spi_prepare_message;
 	controller->unprepare_message = at91_usart_spi_unprepare_message;
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 152cd6773403..0865993005b2 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1475,7 +1475,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
 	host->bus_num = pdev->id;
 	host->num_chipselect = 4;
 	host->setup = atmel_spi_setup;
-	host->flags = (SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX |
+	host->flags = (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX |
 			SPI_MASTER_GPIO_SS);
 	host->transfer_one = atmel_spi_one_transfer;
 	host->set_cs = atmel_spi_set_cs;
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index b04811c911e2..014392459d5e 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -939,7 +939,7 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	master->bus_num = pdev->id;
 	master->num_chipselect = pdata->num_chipselect;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16);
-	master->flags = SPI_MASTER_MUST_RX | SPI_MASTER_GPIO_SS;
+	master->flags = SPI_CONTROLLER_MUST_RX | SPI_MASTER_GPIO_SS;
 	master->setup = davinci_spi_setup;
 	master->cleanup = davinci_spi_cleanup;
 	master->can_dma = davinci_spi_can_dma;
diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index fb68c72df171..1c907d5d5bb3 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -856,7 +856,7 @@ static int fsl_lpspi_probe(struct platform_device *pdev)
 	controller->prepare_transfer_hardware = lpspi_prepare_xfer_hardware;
 	controller->unprepare_transfer_hardware = lpspi_unprepare_xfer_hardware;
 	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
-	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+	controller->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX;
 	controller->dev.of_node = pdev->dev.of_node;
 	controller->bus_num = pdev->id;
 	controller->num_chipselect = fsl_lpspi->num_cs;
diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c
index 141562c882f1..7a9eba35ac38 100644
--- a/drivers/spi/spi-meson-spicc.c
+++ b/drivers/spi/spi-meson-spicc.c
@@ -864,7 +864,7 @@ static int meson_spicc_probe(struct platform_device *pdev)
 				     SPI_BPW_MASK(24) |
 				     SPI_BPW_MASK(16) |
 				     SPI_BPW_MASK(8);
-	master->flags = (SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX);
+	master->flags = (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX);
 	master->min_speed_hz = spicc->data->min_speed_hz;
 	master->max_speed_hz = spicc->data->max_speed_hz;
 	master->setup = meson_spicc_setup;
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 39272ad6641b..0757985947dd 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -1142,7 +1142,7 @@ static int mtk_spi_probe(struct platform_device *pdev)
 		master->mode_bits |= SPI_CS_HIGH;
 
 	if (mdata->dev_comp->must_tx)
-		master->flags = SPI_MASTER_MUST_TX;
+		master->flags = SPI_CONTROLLER_MUST_TX;
 	if (mdata->dev_comp->ipm_design)
 		master->mode_bits |= SPI_LOOP | SPI_RX_DUAL | SPI_TX_DUAL |
 				     SPI_RX_QUAD | SPI_TX_QUAD;
diff --git a/drivers/spi/spi-pci1xxxx.c b/drivers/spi/spi-pci1xxxx.c
index 4445d82409d6..d23c42839da1 100644
--- a/drivers/spi/spi-pci1xxxx.c
+++ b/drivers/spi/spi-pci1xxxx.c
@@ -365,7 +365,7 @@ static int pci1xxxx_spi_probe(struct pci_dev *pdev, const struct pci_device_id *
 		spi_host->bits_per_word_mask = SPI_BPW_MASK(8);
 		spi_host->max_speed_hz = PCI1XXXX_SPI_MAX_CLOCK_HZ;
 		spi_host->min_speed_hz = PCI1XXXX_SPI_MIN_CLOCK_HZ;
-		spi_host->flags = SPI_MASTER_MUST_TX;
+		spi_host->flags = SPI_CONTROLLER_MUST_TX;
 		spi_master_set_devdata(spi_host, spi_sub_ptr);
 		ret = devm_spi_register_master(dev, spi_host);
 		if (ret)
diff --git a/drivers/spi/spi-pic32.c b/drivers/spi/spi-pic32.c
index f2af5e653f3d..e9b4c9cb97fb 100644
--- a/drivers/spi/spi-pic32.c
+++ b/drivers/spi/spi-pic32.c
@@ -773,7 +773,7 @@ static int pic32_spi_probe(struct platform_device *pdev)
 	master->max_speed_hz	= clk_get_rate(pic32s->clk);
 	master->setup		= pic32_spi_setup;
 	master->cleanup		= pic32_spi_cleanup;
-	master->flags		= SPI_MASTER_MUST_TX | SPI_MASTER_MUST_RX;
+	master->flags		= SPI_CONTROLLER_MUST_TX | SPI_CONTROLLER_MUST_RX;
 	master->bits_per_word_mask	= SPI_BPW_MASK(8) | SPI_BPW_MASK(16) |
 					  SPI_BPW_MASK(32);
 	master->transfer_one		= pic32_spi_one_transfer;
diff --git a/drivers/spi/spi-rb4xx.c b/drivers/spi/spi-rb4xx.c
index 5073736d3d1f..c817889a7797 100644
--- a/drivers/spi/spi-rb4xx.c
+++ b/drivers/spi/spi-rb4xx.c
@@ -156,7 +156,7 @@ static int rb4xx_spi_probe(struct platform_device *pdev)
 	master->num_chipselect = 3;
 	master->mode_bits = SPI_TX_DUAL;
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
-	master->flags = SPI_MASTER_MUST_TX;
+	master->flags = SPI_CONTROLLER_MUST_TX;
 	master->transfer_one = rb4xx_transfer_one;
 	master->set_cs = rb4xx_set_cs;
 
diff --git a/drivers/spi/spi-slave-mt27xx.c b/drivers/spi/spi-slave-mt27xx.c
index 4e4d426bfb43..6d6772974783 100644
--- a/drivers/spi/spi-slave-mt27xx.c
+++ b/drivers/spi/spi-slave-mt27xx.c
@@ -414,7 +414,7 @@ static int mtk_spi_slave_probe(struct platform_device *pdev)
 	mdata->dev_comp = of_id->data;
 
 	if (mdata->dev_comp->must_rx)
-		ctlr->flags = SPI_MASTER_MUST_RX;
+		ctlr->flags = SPI_CONTROLLER_MUST_RX;
 
 	platform_set_drvdata(pdev, ctlr);
 
diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 6d10fa4ab783..423212bd0675 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1750,7 +1750,7 @@ static const struct stm32_spi_cfg stm32f4_spi_cfg = {
 	.baud_rate_div_min = STM32F4_SPI_BR_DIV_MIN,
 	.baud_rate_div_max = STM32F4_SPI_BR_DIV_MAX,
 	.has_fifo = false,
-	.flags = SPI_MASTER_MUST_TX,
+	.flags = SPI_CONTROLLER_MUST_TX,
 };
 
 static const struct stm32_spi_cfg stm32h7_spi_cfg = {
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index ce77dcf47b96..cdc3addfe117 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1645,8 +1645,6 @@ spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer)
 #define spi_master			spi_controller
 
 #define SPI_MASTER_HALF_DUPLEX		SPI_CONTROLLER_HALF_DUPLEX
-#define SPI_MASTER_MUST_RX		SPI_CONTROLLER_MUST_RX
-#define SPI_MASTER_MUST_TX		SPI_CONTROLLER_MUST_TX
 
 #define spi_master_get_devdata(_ctlr)	spi_controller_get_devdata(_ctlr)
 #define spi_master_set_devdata(_ctlr, _data)	\
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 13/15] spi: Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS

[Andy Shevchenko]

Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS and
convert the users to SPI_CONTROLLER_GPIO_SS to follow
the new naming shema.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-ath79.c    | 2 +-
 drivers/spi/spi-atmel.c    | 2 +-
 drivers/spi/spi-bitbang.c  | 6 +++---
 drivers/spi/spi-davinci.c  | 2 +-
 drivers/spi/spi-dw-core.c  | 2 +-
 drivers/spi/spi-gpio.c     | 2 +-
 drivers/spi/spi-imx.c      | 2 +-
 drivers/spi/spi-orion.c    | 2 +-
 drivers/spi/spi-rockchip.c | 2 +-
 drivers/spi/spi-sifive.c   | 2 +-
 drivers/spi/spi.c          | 4 ++--
 include/linux/spi/spi.h    | 3 +--
 12 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index d3dd21386f12..1b6d977d111c 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -185,7 +185,7 @@ static int ath79_spi_probe(struct platform_device *pdev)
 
 	host->use_gpio_descriptors = true;
 	host->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 32);
-	host->flags = SPI_MASTER_GPIO_SS;
+	host->flags = SPI_CONTROLLER_GPIO_SS;
 	host->num_chipselect = 3;
 	host->mem_ops = &ath79_mem_ops;
 
diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 0865993005b2..9674499ed0a6 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1476,7 +1476,7 @@ static int atmel_spi_probe(struct platform_device *pdev)
 	host->num_chipselect = 4;
 	host->setup = atmel_spi_setup;
 	host->flags = (SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_MUST_TX |
-			SPI_MASTER_GPIO_SS);
+			SPI_CONTROLLER_GPIO_SS);
 	host->transfer_one = atmel_spi_one_transfer;
 	host->set_cs = atmel_spi_set_cs;
 	host->cleanup = atmel_spi_cleanup;
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index 862f209cada1..ecd44016c197 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -349,11 +349,11 @@ int spi_bitbang_init(struct spi_bitbang *bitbang)
 	/*
 	 * We only need the chipselect callback if we are actually using it.
 	 * If we just use GPIO descriptors, it is surplus. If the
-	 * SPI_MASTER_GPIO_SS flag is set, we always need to call the
+	 * SPI_CONTROLLER_GPIO_SS flag is set, we always need to call the
 	 * driver-specific chipselect routine.
 	 */
 	custom_cs = (!master->use_gpio_descriptors ||
-		     (master->flags & SPI_MASTER_GPIO_SS));
+		     (master->flags & SPI_CONTROLLER_GPIO_SS));
 
 	if (custom_cs && !bitbang->chipselect)
 		return -EINVAL;
@@ -371,7 +371,7 @@ int spi_bitbang_init(struct spi_bitbang *bitbang)
 	master->transfer_one = spi_bitbang_transfer_one;
 	/*
 	 * When using GPIO descriptors, the ->set_cs() callback doesn't even
-	 * get called unless SPI_MASTER_GPIO_SS is set.
+	 * get called unless SPI_CONTROLLER_GPIO_SS is set.
 	 */
 	if (custom_cs)
 		master->set_cs = spi_bitbang_set_cs;
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 014392459d5e..7ea2ac053052 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -939,7 +939,7 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	master->bus_num = pdev->id;
 	master->num_chipselect = pdata->num_chipselect;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16);
-	master->flags = SPI_CONTROLLER_MUST_RX | SPI_MASTER_GPIO_SS;
+	master->flags = SPI_CONTROLLER_MUST_RX | SPI_CONTROLLER_GPIO_SS;
 	master->setup = davinci_spi_setup;
 	master->cleanup = davinci_spi_cleanup;
 	master->can_dma = davinci_spi_can_dma;
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index a8ba41ad4541..45f5acc26b1d 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -932,7 +932,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	if (dws->mem_ops.exec_op)
 		master->mem_ops = &dws->mem_ops;
 	master->max_speed_hz = dws->max_freq;
-	master->flags = SPI_MASTER_GPIO_SS;
+	master->flags = SPI_CONTROLLER_GPIO_SS;
 	master->auto_runtime_pm = true;
 
 	/* Get default rx sample delay */
diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index 85e149ec2910..612db477ad12 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -434,7 +434,7 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	 * line, that we need to do on selection. This makes the local
 	 * callback for chipselect always get called.
 	 */
-	master->flags |= SPI_MASTER_GPIO_SS;
+	master->flags |= SPI_CONTROLLER_GPIO_SS;
 	bb->chipselect = spi_gpio_chipselect;
 	bb->set_line_direction = spi_gpio_set_direction;
 
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 528ae46c087f..8d97f2b32c2b 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1779,7 +1779,7 @@ static int spi_imx_probe(struct platform_device *pdev)
 
 	if (is_imx51_ecspi(spi_imx) || is_imx53_ecspi(spi_imx)) {
 		controller->max_native_cs = 4;
-		controller->flags |= SPI_MASTER_GPIO_SS;
+		controller->flags |= SPI_CONTROLLER_GPIO_SS;
 	}
 
 	spi_imx->spi_drctl = spi_drctl;
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index ad9e83e34297..4882aafd0505 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -677,7 +677,7 @@ static int orion_spi_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
 	master->auto_runtime_pm = true;
 	master->use_gpio_descriptors = true;
-	master->flags = SPI_MASTER_GPIO_SS;
+	master->flags = SPI_CONTROLLER_GPIO_SS;
 
 	platform_set_drvdata(pdev, master);
 
diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 143ede958ac1..a37943847e81 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -858,7 +858,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
 		ctlr->mode_bits |= SPI_NO_CS;
 		ctlr->slave_abort = rockchip_spi_slave_abort;
 	} else {
-		ctlr->flags = SPI_MASTER_GPIO_SS;
+		ctlr->flags = SPI_CONTROLLER_GPIO_SS;
 		ctlr->max_native_cs = ROCKCHIP_SPI_MAX_CS_NUM;
 		/*
 		 * rk spi0 has two native cs, spi1..5 one cs only
diff --git a/drivers/spi/spi-sifive.c b/drivers/spi/spi-sifive.c
index dae9e097c333..2f77dae85386 100644
--- a/drivers/spi/spi-sifive.c
+++ b/drivers/spi/spi-sifive.c
@@ -379,7 +379,7 @@ static int sifive_spi_probe(struct platform_device *pdev)
 	 * we need to "left-align" the bits (unless SPI_LSB_FIRST)
 	 */
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
-	master->flags = SPI_CONTROLLER_MUST_TX | SPI_MASTER_GPIO_SS;
+	master->flags = SPI_CONTROLLER_MUST_TX | SPI_CONTROLLER_GPIO_SS;
 	master->prepare_message = sifive_spi_prepare_message;
 	master->set_cs = sifive_spi_set_cs;
 	master->transfer_one = sifive_spi_transfer_one;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 06a92a3a5746..bcabae98cb7c 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -995,7 +995,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 				gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), activate);
 		}
 		/* Some SPI masters need both GPIO CS & slave_select */
-		if ((spi->controller->flags & SPI_MASTER_GPIO_SS) &&
+		if ((spi->controller->flags & SPI_CONTROLLER_GPIO_SS) &&
 		    spi->controller->set_cs)
 			spi->controller->set_cs(spi, !enable);
 	} else if (spi->controller->set_cs) {
@@ -3020,7 +3020,7 @@ static int spi_get_gpio_descs(struct spi_controller *ctlr)
 
 	ctlr->unused_native_cs = ffs(~native_cs_mask) - 1;
 
-	if ((ctlr->flags & SPI_MASTER_GPIO_SS) && num_cs_gpios &&
+	if ((ctlr->flags & SPI_CONTROLLER_GPIO_SS) && num_cs_gpios &&
 	    ctlr->max_native_cs && ctlr->unused_native_cs >= ctlr->max_native_cs) {
 		dev_err(dev, "No unused native chip select available\n");
 		return -EINVAL;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index cdc3addfe117..43f6c3f71a76 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -578,8 +578,7 @@ struct spi_controller {
 #define SPI_CONTROLLER_NO_TX		BIT(2)	/* Can't do buffer write */
 #define SPI_CONTROLLER_MUST_RX		BIT(3)	/* Requires rx */
 #define SPI_CONTROLLER_MUST_TX		BIT(4)	/* Requires tx */
-
-#define SPI_MASTER_GPIO_SS		BIT(5)	/* GPIO CS must select slave */
+#define SPI_CONTROLLER_GPIO_SS		BIT(5)	/* GPIO CS must select slave */
 
 	/* Flag indicating if the allocation of this struct is devres-managed */
 	bool			devm_allocated;
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 13/15] spi: Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS

[Serge Semin]

On Mon, Jul 10, 2023 at 06:49:30PM +0300, Andy Shevchenko wrote:
> Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS and
> convert the users to SPI_CONTROLLER_GPIO_SS to follow

* I'm not an expert in English, but imo the next would look a
* bit more readable:
* convert s/the users to SPI_CONTROLLER_GPIO_SS/the code to using SPI_CONTROLLER_GPIO_SS

> the new naming shema.

s/shema/schema

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
[...]

>  drivers/spi/spi-dw-core.c  | 2 +-

[...]

> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index a8ba41ad4541..45f5acc26b1d 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -932,7 +932,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>  	if (dws->mem_ops.exec_op)
>  		master->mem_ops = &dws->mem_ops;
>  	master->max_speed_hz = dws->max_freq;
> -	master->flags = SPI_MASTER_GPIO_SS;
> +	master->flags = SPI_CONTROLLER_GPIO_SS;
>  	master->auto_runtime_pm = true;
>  
>  	/* Get default rx sample delay */

For the DW APB/AHB SSI driver:
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

-Serge(y)

[...]

> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 06a92a3a5746..bcabae98cb7c 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -995,7 +995,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
>  				gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), activate);
>  		}
>  		/* Some SPI masters need both GPIO CS & slave_select */
> -		if ((spi->controller->flags & SPI_MASTER_GPIO_SS) &&
> +		if ((spi->controller->flags & SPI_CONTROLLER_GPIO_SS) &&
>  		    spi->controller->set_cs)
>  			spi->controller->set_cs(spi, !enable);
>  	} else if (spi->controller->set_cs) {
> @@ -3020,7 +3020,7 @@ static int spi_get_gpio_descs(struct spi_controller *ctlr)
>  
>  	ctlr->unused_native_cs = ffs(~native_cs_mask) - 1;
>  
> -	if ((ctlr->flags & SPI_MASTER_GPIO_SS) && num_cs_gpios &&
> +	if ((ctlr->flags & SPI_CONTROLLER_GPIO_SS) && num_cs_gpios &&
>  	    ctlr->max_native_cs && ctlr->unused_native_cs >= ctlr->max_native_cs) {
>  		dev_err(dev, "No unused native chip select available\n");
>  		return -EINVAL;
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index cdc3addfe117..43f6c3f71a76 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -578,8 +578,7 @@ struct spi_controller {
>  #define SPI_CONTROLLER_NO_TX		BIT(2)	/* Can't do buffer write */
>  #define SPI_CONTROLLER_MUST_RX		BIT(3)	/* Requires rx */
>  #define SPI_CONTROLLER_MUST_TX		BIT(4)	/* Requires tx */
> -
> -#define SPI_MASTER_GPIO_SS		BIT(5)	/* GPIO CS must select slave */
> +#define SPI_CONTROLLER_GPIO_SS		BIT(5)	/* GPIO CS must select slave */
>  
>  	/* Flag indicating if the allocation of this struct is devres-managed */
>  	bool			devm_allocated;
> -- 
> 2.40.0.1.gaa8946217a0b
> 

Re: [PATCH v2 13/15] spi: Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 03:30:19PM +0300, Serge Semin wrote:
> On Mon, Jul 10, 2023 at 06:49:30PM +0300, Andy Shevchenko wrote:
> > Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS and
> > convert the users to SPI_CONTROLLER_GPIO_SS to follow
> 
> * I'm not an expert in English, but imo the next would look a
> * bit more readable:
> * convert s/the users to SPI_CONTROLLER_GPIO_SS/the code to using SPI_CONTROLLER_GPIO_SS

> > the new naming shema.
> 
> s/shema/schema

Right, thank you!

...

> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

Thank you for the review!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 14/15] spi: Convert to SPI_CONTROLLER_HALF_DUPLEX

[Andy Shevchenko]

Convert the users under SPI subsystem to SPI_CONTROLLER_HALF_DUPLEX.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-amd.c             | 2 +-
 drivers/spi/spi-cavium-thunderx.c | 2 +-
 drivers/spi/spi-falcon.c          | 2 +-
 drivers/spi/spi-lp8841-rtc.c      | 2 +-
 drivers/spi/spi-mxs.c             | 2 +-
 drivers/spi/spi-omap-uwire.c      | 2 +-
 drivers/spi/spi-pic32-sqi.c       | 2 +-
 drivers/spi/spi-qcom-qspi.c       | 2 +-
 drivers/spi/spi-rockchip-sfc.c    | 2 +-
 drivers/spi/spi-sprd-adi.c        | 2 +-
 drivers/spi/spi-ti-qspi.c         | 2 +-
 drivers/spi/spi-xcomm.c           | 2 +-
 12 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index fecead757a3c..b19766571f28 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -404,7 +404,7 @@ static int amd_spi_probe(struct platform_device *pdev)
 	master->bus_num = 0;
 	master->num_chipselect = 4;
 	master->mode_bits = 0;
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->max_speed_hz = AMD_SPI_MAX_HZ;
 	master->min_speed_hz = AMD_SPI_MIN_HZ;
 	master->setup = amd_spi_master_setup;
diff --git a/drivers/spi/spi-cavium-thunderx.c b/drivers/spi/spi-cavium-thunderx.c
index 60c0d6934654..535f7eb9fa69 100644
--- a/drivers/spi/spi-cavium-thunderx.c
+++ b/drivers/spi/spi-cavium-thunderx.c
@@ -64,7 +64,7 @@ static int thunderx_spi_probe(struct pci_dev *pdev,
 		p->sys_freq = SYS_FREQ_DEFAULT;
 	dev_info(dev, "Set system clock to %u\n", p->sys_freq);
 
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->num_chipselect = 4;
 	master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_CS_HIGH |
 			    SPI_LSB_FIRST | SPI_3WIRE;
diff --git a/drivers/spi/spi-falcon.c b/drivers/spi/spi-falcon.c
index 4c103dff0d44..8a8414cbb400 100644
--- a/drivers/spi/spi-falcon.c
+++ b/drivers/spi/spi-falcon.c
@@ -401,7 +401,7 @@ static int falcon_sflash_probe(struct platform_device *pdev)
 	priv->master = master;
 
 	master->mode_bits = SPI_MODE_3;
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->setup = falcon_sflash_setup;
 	master->transfer_one_message = falcon_sflash_xfer_one;
 	master->dev.of_node = pdev->dev.of_node;
diff --git a/drivers/spi/spi-lp8841-rtc.c b/drivers/spi/spi-lp8841-rtc.c
index ccaa7a946359..c6810720b3b5 100644
--- a/drivers/spi/spi-lp8841-rtc.c
+++ b/drivers/spi/spi-lp8841-rtc.c
@@ -191,7 +191,7 @@ spi_lp8841_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, master);
 
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->mode_bits = SPI_CS_HIGH | SPI_3WIRE | SPI_LSB_FIRST;
 
 	master->bus_num = pdev->id;
diff --git a/drivers/spi/spi-mxs.c b/drivers/spi/spi-mxs.c
index 963a53dd680b..cd0e7ae07162 100644
--- a/drivers/spi/spi-mxs.c
+++ b/drivers/spi/spi-mxs.c
@@ -572,7 +572,7 @@ static int mxs_spi_probe(struct platform_device *pdev)
 	master->mode_bits = SPI_CPOL | SPI_CPHA;
 	master->num_chipselect = 3;
 	master->dev.of_node = np;
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->auto_runtime_pm = true;
 
 	spi = spi_master_get_devdata(master);
diff --git a/drivers/spi/spi-omap-uwire.c b/drivers/spi/spi-omap-uwire.c
index 902d2e0c1f2f..f89aa9e52c23 100644
--- a/drivers/spi/spi-omap-uwire.c
+++ b/drivers/spi/spi-omap-uwire.c
@@ -486,7 +486,7 @@ static int uwire_probe(struct platform_device *pdev)
 	/* the spi->mode bits understood by this driver: */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(1, 16);
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 
 	master->bus_num = 2;	/* "official" */
 	master->num_chipselect = 4;
diff --git a/drivers/spi/spi-pic32-sqi.c b/drivers/spi/spi-pic32-sqi.c
index 51dfb49523f3..5cbebcf26a2a 100644
--- a/drivers/spi/spi-pic32-sqi.c
+++ b/drivers/spi/spi-pic32-sqi.c
@@ -648,7 +648,7 @@ static int pic32_sqi_probe(struct platform_device *pdev)
 	master->dev.of_node	= pdev->dev.of_node;
 	master->mode_bits	= SPI_MODE_3 | SPI_MODE_0 | SPI_TX_DUAL |
 				  SPI_RX_DUAL | SPI_TX_QUAD | SPI_RX_QUAD;
-	master->flags		= SPI_MASTER_HALF_DUPLEX;
+	master->flags		= SPI_CONTROLLER_HALF_DUPLEX;
 	master->can_dma		= pic32_sqi_can_dma;
 	master->bits_per_word_mask	= SPI_BPW_RANGE_MASK(8, 32);
 	master->transfer_one_message	= pic32_sqi_one_message;
diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
index a8a683d6145c..5a98c52bad32 100644
--- a/drivers/spi/spi-qcom-qspi.c
+++ b/drivers/spi/spi-qcom-qspi.c
@@ -724,7 +724,7 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 	master->mode_bits = SPI_MODE_0 |
 			    SPI_TX_DUAL | SPI_RX_DUAL |
 			    SPI_TX_QUAD | SPI_RX_QUAD;
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->prepare_message = qcom_qspi_prepare_message;
 	master->transfer_one = qcom_qspi_transfer_one;
 	master->handle_err = qcom_qspi_handle_err;
diff --git a/drivers/spi/spi-rockchip-sfc.c b/drivers/spi/spi-rockchip-sfc.c
index 583f4187f030..bd550e76ab3d 100644
--- a/drivers/spi/spi-rockchip-sfc.c
+++ b/drivers/spi/spi-rockchip-sfc.c
@@ -565,7 +565,7 @@ static int rockchip_sfc_probe(struct platform_device *pdev)
 	if (!master)
 		return -ENOMEM;
 
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->mem_ops = &rockchip_sfc_mem_ops;
 	master->dev.of_node = pdev->dev.of_node;
 	master->mode_bits = SPI_TX_QUAD | SPI_TX_DUAL | SPI_RX_QUAD | SPI_RX_DUAL;
diff --git a/drivers/spi/spi-sprd-adi.c b/drivers/spi/spi-sprd-adi.c
index 22e39c4c12c4..94d16aadfa44 100644
--- a/drivers/spi/spi-sprd-adi.c
+++ b/drivers/spi/spi-sprd-adi.c
@@ -580,7 +580,7 @@ static int sprd_adi_probe(struct platform_device *pdev)
 	ctlr->dev.of_node = pdev->dev.of_node;
 	ctlr->bus_num = pdev->id;
 	ctlr->num_chipselect = num_chipselect;
-	ctlr->flags = SPI_MASTER_HALF_DUPLEX;
+	ctlr->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	ctlr->bits_per_word_mask = 0;
 	ctlr->transfer_one = sprd_adi_transfer_one;
 
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 5914335ff63d..bf4b40289bee 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -770,7 +770,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_RX_DUAL | SPI_RX_QUAD;
 
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->setup = ti_qspi_setup;
 	master->auto_runtime_pm = true;
 	master->transfer_one_message = ti_qspi_start_transfer_one;
diff --git a/drivers/spi/spi-xcomm.c b/drivers/spi/spi-xcomm.c
index ae6218bcd02a..a3d57554f5ba 100644
--- a/drivers/spi/spi-xcomm.c
+++ b/drivers/spi/spi-xcomm.c
@@ -218,7 +218,7 @@ static int spi_xcomm_probe(struct i2c_client *i2c)
 	master->num_chipselect = 16;
 	master->mode_bits = SPI_CPHA | SPI_CPOL | SPI_3WIRE;
 	master->bits_per_word_mask = SPI_BPW_MASK(8);
-	master->flags = SPI_MASTER_HALF_DUPLEX;
+	master->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	master->transfer_one_message = spi_xcomm_transfer_one;
 	master->dev.of_node = i2c->dev.of_node;
 	i2c_set_clientdata(i2c, master);
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 15/15] spi: Fix spelling typos and acronyms capitalization

[Andy Shevchenko]

Fix
  - spelling typos
  - capitalization of acronyms
in the comments.

While at it, fix the multi-line comment style.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi.c          |  54 ++++++++-------
 include/linux/spi/spi.h    | 137 ++++++++++++++++++++-----------------
 include/trace/events/spi.h |   2 +-
 3 files changed, 104 insertions(+), 89 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index bcabae98cb7c..43f7c9b18676 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -682,7 +682,7 @@ static int __spi_add_device(struct spi_device *spi)
  * @spi: spi_device to register
  *
  * Companion function to spi_alloc_device.  Devices allocated with
- * spi_alloc_device can be added onto the spi bus with this function.
+ * spi_alloc_device can be added onto the SPI bus with this function.
  *
  * Return: 0 on success; negative errno on failure
  */
@@ -878,7 +878,7 @@ int spi_register_board_info(struct spi_board_info const *info, unsigned n)
  * spi_res_alloc - allocate a spi resource that is life-cycle managed
  *                 during the processing of a spi_message while using
  *                 spi_transfer_one
- * @spi:     the spi device for which we allocate memory
+ * @spi:     the SPI device for which we allocate memory
  * @release: the release code to execute for this resource
  * @size:    size to alloc and return
  * @gfp:     GFP allocation flags
@@ -904,7 +904,7 @@ static void *spi_res_alloc(struct spi_device *spi, spi_res_release_t release,
 }
 
 /**
- * spi_res_free - free an spi resource
+ * spi_res_free - free an SPI resource
  * @res: pointer to the custom data of a resource
  */
 static void spi_res_free(void *res)
@@ -920,7 +920,7 @@ static void spi_res_free(void *res)
 
 /**
  * spi_res_add - add a spi_res to the spi_message
- * @message: the spi message
+ * @message: the SPI message
  * @res:     the spi_resource
  */
 static void spi_res_add(struct spi_message *message, void *res)
@@ -932,7 +932,7 @@ static void spi_res_add(struct spi_message *message, void *res)
 }
 
 /**
- * spi_res_release - release all spi resources for this message
+ * spi_res_release - release all SPI resources for this message
  * @ctlr:  the @spi_controller
  * @message: the @spi_message
  */
@@ -1392,7 +1392,7 @@ int spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer)
 			return -EINVAL;
 		/*
 		 * If there is unknown effective speed, approximate it
-		 * by underestimating with half of the requested hz.
+		 * by underestimating with half of the requested Hz.
 		 */
 		hz = xfer->effective_speed_hz ?: xfer->speed_hz / 2;
 		if (!hz)
@@ -1707,11 +1707,11 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr,
 }
 
 /**
- * __spi_pump_messages - function which processes spi message queue
+ * __spi_pump_messages - function which processes SPI message queue
  * @ctlr: controller to process queue for
  * @in_kthread: true if we are in the context of the message pump thread
  *
- * This function checks if there is any spi message in the queue that
+ * This function checks if there is any SPI message in the queue that
  * needs processing and if so call out to the driver to initialize hardware
  * and transfer each message.
  *
@@ -1726,7 +1726,7 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread)
 	unsigned long flags;
 	int ret;
 
-	/* Take the IO mutex */
+	/* Take the I/O mutex */
 	mutex_lock(&ctlr->io_mutex);
 
 	/* Lock queue */
@@ -2137,8 +2137,8 @@ static int __spi_queued_transfer(struct spi_device *spi,
 
 /**
  * spi_queued_transfer - transfer function for queued transfers
- * @spi: spi device which is requesting transfer
- * @msg: spi message which is to handled is queued to driver queue
+ * @spi: SPI device which is requesting transfer
+ * @msg: SPI message which is to handled is queued to driver queue
  *
  * Return: zero on success, else a negative error code.
  */
@@ -2464,7 +2464,7 @@ static int acpi_spi_count(struct acpi_resource *ares, void *data)
  * acpi_spi_count_resources - Count the number of SpiSerialBus resources
  * @adev:	ACPI device
  *
- * Returns the number of SpiSerialBus resources in the ACPI-device's
+ * Return: the number of SpiSerialBus resources in the ACPI-device's
  * resource-list; or a negative error code.
  */
 int acpi_spi_count_resources(struct acpi_device *adev)
@@ -2598,10 +2598,10 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
  * @adev: ACPI Device for the spi device
  * @index: Index of the spi resource inside the ACPI Node
  *
- * This should be used to allocate a new spi device from and ACPI Node.
- * The caller is responsible for calling spi_add_device to register the spi device.
+ * This should be used to allocate a new SPI device from and ACPI Device node.
+ * The caller is responsible for calling spi_add_device to register the SPI device.
  *
- * If ctlr is set to NULL, the Controller for the spi device will be looked up
+ * If ctlr is set to NULL, the Controller for the SPI device will be looked up
  * using the resource.
  * If index is set to -1, index is not used.
  * Note: If index is -1, ctlr must be set.
@@ -3299,7 +3299,8 @@ void spi_unregister_controller(struct spi_controller *ctlr)
 	if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
 		mutex_unlock(&ctlr->add_lock);
 
-	/* Release the last reference on the controller if its driver
+	/*
+	 * Release the last reference on the controller if its driver
 	 * has not yet been converted to devm_spi_alloc_master/slave().
 	 */
 	if (!ctlr->devm_allocated)
@@ -3512,7 +3513,7 @@ static int __spi_split_transfer_maxsize(struct spi_controller *ctlr,
 
 	/* All the others need rx_buf/tx_buf also set */
 	for (i = 1, offset = maxsize; i < count; offset += maxsize, i++) {
-		/* Update rx_buf, tx_buf and dma */
+		/* Update rx_buf, tx_buf and DMA */
 		if (xfers[i].rx_buf)
 			xfers[i].rx_buf += offset;
 		if (xfers[i].rx_dma)
@@ -3582,7 +3583,7 @@ EXPORT_SYMBOL_GPL(spi_split_transfers_maxsize);
 
 
 /**
- * spi_split_transfers_maxwords - split spi transfers into multiple transfers
+ * spi_split_transfers_maxwords - split SPI transfers into multiple transfers
  *                                when an individual transfer exceeds a
  *                                certain number of SPI words
  * @ctlr:     the @spi_controller for this transfer
@@ -3625,7 +3626,8 @@ EXPORT_SYMBOL_GPL(spi_split_transfers_maxwords);
 
 /*-------------------------------------------------------------------------*/
 
-/* Core methods for SPI controller protocol drivers.  Some of the
+/*
+ * Core methods for SPI controller protocol drivers. Some of the
  * other core methods are currently defined as inline functions.
  */
 
@@ -3685,7 +3687,7 @@ static int spi_set_cs_timing(struct spi_device *spi)
  * changes those settings, and must be called from a context that can sleep.
  * Except for SPI_CS_HIGH, which takes effect immediately, the changes take
  * effect the next time the device is selected and data is transferred to
- * or from it.  When this function returns, the spi device is deselected.
+ * or from it.  When this function returns, the SPI device is deselected.
  *
  * Note that this call will fail if the protocol driver specifies an option
  * that the underlying controller or its driver does not support.  For
@@ -4025,7 +4027,7 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message)
  * spi_async - asynchronous SPI transfer
  * @spi: device with which data will be exchanged
  * @message: describes the data transfers, including completion callback
- * Context: any (irqs may be blocked, etc)
+ * Context: any (IRQs may be blocked, etc)
  *
  * This call may be used in_irq and other contexts which can't sleep,
  * as well as from task contexts which can sleep.
@@ -4079,7 +4081,7 @@ EXPORT_SYMBOL_GPL(spi_async);
  * spi_async_locked - version of spi_async with exclusive bus usage
  * @spi: device with which data will be exchanged
  * @message: describes the data transfers, including completion callback
- * Context: any (irqs may be blocked, etc)
+ * Context: any (IRQs may be blocked, etc)
  *
  * This call may be used in_irq and other contexts which can't sleep,
  * as well as from task contexts which can sleep.
@@ -4342,9 +4344,9 @@ static u8	*buf;
 /**
  * spi_write_then_read - SPI synchronous write followed by read
  * @spi: device with which data will be exchanged
- * @txbuf: data to be written (need not be dma-safe)
+ * @txbuf: data to be written (need not be DMA-safe)
  * @n_tx: size of txbuf, in bytes
- * @rxbuf: buffer into which data will be read (need not be dma-safe)
+ * @rxbuf: buffer into which data will be read (need not be DMA-safe)
  * @n_rx: size of rxbuf, in bytes
  * Context: can sleep
  *
@@ -4355,7 +4357,7 @@ static u8	*buf;
  *
  * Parameters to this routine are always copied using a small buffer.
  * Performance-sensitive or bulk transfer code should instead use
- * spi_{async,sync}() calls with dma-safe buffers.
+ * spi_{async,sync}() calls with DMA-safe buffers.
  *
  * Return: zero on success, else a negative error code.
  */
@@ -4400,7 +4402,7 @@ int spi_write_then_read(struct spi_device *spi,
 	x[0].tx_buf = local_buf;
 	x[1].rx_buf = local_buf + n_tx;
 
-	/* Do the i/o */
+	/* Do the I/O */
 	status = spi_sync(spi, &message);
 	if (status == 0)
 		memcpy(rxbuf, x[1].rx_buf, n_rx);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 43f6c3f71a76..e40ebffbf4ea 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -52,7 +52,7 @@ extern struct bus_type spi_bus_type;
 
 /**
  * struct spi_statistics - statistics for spi transfers
- * @syncp:         seqcount to protect members in this struct for per-cpu udate
+ * @syncp:         seqcount to protect members in this struct for per-cpu update
  *                 on 32-bit systems
  *
  * @messages:      number of spi-messages handled
@@ -71,7 +71,7 @@ extern struct bus_type spi_bus_type;
  * @bytes_rx:      number of bytes received from device
  *
  * @transfer_bytes_histo:
- *                 transfer bytes histogramm
+ *                 transfer bytes histogram
  *
  * @transfers_split_maxsize:
  *                 number of transfers that have been split because of
@@ -172,7 +172,7 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg,
  *	the device will bind to the named driver and only the named driver.
  *	Do not set directly, because core frees it; use driver_set_override() to
  *	set or clear it.
- * @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when
+ * @cs_gpiod: GPIO descriptor of the chipselect line (optional, NULL when
  *	not using a GPIO line)
  * @word_delay: delay to be inserted between consecutive
  *	words of a transfer
@@ -228,7 +228,7 @@ struct spi_device {
 	void			*controller_data;
 	char			modalias[SPI_NAME_SIZE];
 	const char		*driver_override;
-	struct gpio_desc	*cs_gpiod;	/* Chip select gpio desc */
+	struct gpio_desc	*cs_gpiod;	/* Chip select GPIO descriptor */
 	struct spi_delay	word_delay; /* Inter-word delay */
 	/* CS delays */
 	struct spi_delay	cs_setup;
@@ -239,7 +239,7 @@ struct spi_device {
 	struct spi_statistics __percpu	*pcpu_statistics;
 
 	/*
-	 * likely need more hooks for more protocol options affecting how
+	 * Likely need more hooks for more protocol options affecting how
 	 * the controller talks to each chip, like:
 	 *  - memory packing (12 bit samples into low bits, others zeroed)
 	 *  - priority
@@ -315,11 +315,11 @@ static inline void spi_set_csgpiod(struct spi_device *spi, u8 idx, struct gpio_d
 /**
  * struct spi_driver - Host side "protocol" driver
  * @id_table: List of SPI devices supported by this driver
- * @probe: Binds this driver to the spi device.  Drivers can verify
+ * @probe: Binds this driver to the SPI device.  Drivers can verify
  *	that the device is actually present, and may need to configure
  *	characteristics (such as bits_per_word) which weren't needed for
  *	the initial configuration done during system setup.
- * @remove: Unbinds this driver from the spi device
+ * @remove: Unbinds this driver from the SPI device
  * @shutdown: Standard shutdown callback used during system state
  *	transitions such as powerdown/halt and kexec
  * @driver: SPI device drivers should initialize the name and owner
@@ -431,7 +431,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch
  * @queued: whether this controller is providing an internal message queue
  * @kworker: pointer to thread struct for message pump
  * @pump_messages: work struct for scheduling work to the message pump
- * @queue_lock: spinlock to syncronise access to message queue
+ * @queue_lock: spinlock to synchronise access to message queue
  * @queue: message queue
  * @cur_msg: the currently in-flight message
  * @cur_msg_completion: a completion for the current in-flight message
@@ -489,7 +489,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch
  * @unprepare_message: undo any work done by prepare_message().
  * @slave_abort: abort the ongoing transfer request on an SPI slave controller
  * @target_abort: abort the ongoing transfer request on an SPI target controller
- * @cs_gpiods: Array of GPIO descs to use as chip select lines; one per CS
+ * @cs_gpiods: Array of GPIO descriptors to use as chip select lines; one per CS
  *	number. Any individual value may be NULL for CS lines that
  *	are not GPIOs (driven by the SPI controller itself).
  * @use_gpio_descriptors: Turns on the code in the SPI core to parse and grab
@@ -516,7 +516,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch
  *	If the driver does not set this, the SPI core takes the snapshot as
  *	close to the driver hand-over as possible.
  * @irq_flags: Interrupt enable state during PTP system timestamping
- * @fallback: fallback to pio if dma transfer return failure with
+ * @fallback: fallback to PIO if DMA transfer return failure with
  *	SPI_TRANS_FAIL_NO_START.
  * @queue_empty: signal green light for opportunistically skipping the queue
  *	for spi_sync transfers.
@@ -538,15 +538,17 @@ struct spi_controller {
 
 	struct list_head list;
 
-	/* Other than negative (== assign one dynamically), bus_num is fully
-	 * board-specific.  usually that simplifies to being SOC-specific.
-	 * example:  one SOC has three SPI controllers, numbered 0..2,
-	 * and one board's schematics might show it using SPI-2.  software
+	/*
+	 * Other than negative (== assign one dynamically), bus_num is fully
+	 * board-specific. Usually that simplifies to being SoC-specific.
+	 * example: one SoC has three SPI controllers, numbered 0..2,
+	 * and one board's schematics might show it using SPI-2. Software
 	 * would normally use bus_num=2 for that controller.
 	 */
 	s16			bus_num;
 
-	/* chipselects will be integral to many controllers; some others
+	/*
+	 * Chipselects will be integral to many controllers; some others
 	 * might use board-specific GPIOs.
 	 */
 	u16			num_chipselect;
@@ -591,8 +593,8 @@ struct spi_controller {
 	};
 
 	/*
-	 * on some hardware transfer / message size may be constrained
-	 * the limit may depend on device transfer settings
+	 * On some hardware transfer / message size may be constrained
+	 * the limit may depend on device transfer settings.
 	 */
 	size_t (*max_transfer_size)(struct spi_device *spi);
 	size_t (*max_message_size)(struct spi_device *spi);
@@ -610,7 +612,8 @@ struct spi_controller {
 	/* Flag indicating that the SPI bus is locked for exclusive use */
 	bool			bus_lock_flag;
 
-	/* Setup mode and clock, etc (spi driver may call many times).
+	/*
+	 * Setup mode and clock, etc (SPI driver may call many times).
 	 *
 	 * IMPORTANT:  this may be called when transfers to another
 	 * device are active.  DO NOT UPDATE SHARED REGISTERS in ways
@@ -628,18 +631,19 @@ struct spi_controller {
 	 */
 	int (*set_cs_timing)(struct spi_device *spi);
 
-	/* Bidirectional bulk transfers
+	/*
+	 * Bidirectional bulk transfers
 	 *
 	 * + The transfer() method may not sleep; its main role is
 	 *   just to add the message to the queue.
 	 * + For now there's no remove-from-queue operation, or
 	 *   any other request management
-	 * + To a given spi_device, message queueing is pure fifo
+	 * + To a given spi_device, message queueing is pure FIFO
 	 *
 	 * + The controller's main job is to process its message queue,
 	 *   selecting a chip (for masters), then transferring data
 	 * + If there are multiple spi_device children, the i/o queue
-	 *   arbitration algorithm is unspecified (round robin, fifo,
+	 *   arbitration algorithm is unspecified (round robin, FIFO,
 	 *   priority, reservations, preemption, etc)
 	 *
 	 * + Chipselect stays active during the entire message
@@ -720,7 +724,7 @@ struct spi_controller {
 	const struct spi_controller_mem_ops *mem_ops;
 	const struct spi_controller_mem_caps *mem_caps;
 
-	/* gpio chip select */
+	/* GPIO chip select */
 	struct gpio_desc	**cs_gpiods;
 	bool			use_gpio_descriptors;
 	s8			unused_native_cs;
@@ -804,7 +808,7 @@ void spi_take_timestamp_post(struct spi_controller *ctlr,
 			     struct spi_transfer *xfer,
 			     size_t progress, bool irqs_off);
 
-/* The spi driver core manages memory for the spi_controller classdev */
+/* The SPI driver core manages memory for the spi_controller classdev */
 extern struct spi_controller *__spi_alloc_controller(struct device *host,
 						unsigned int size, bool slave);
 
@@ -893,13 +897,13 @@ typedef void (*spi_res_release_t)(struct spi_controller *ctlr,
 				  void *res);
 
 /**
- * struct spi_res - spi resource management structure
+ * struct spi_res - SPI resource management structure
  * @entry:   list entry
  * @release: release code called prior to freeing this resource
  * @data:    extra data allocated for the specific use-case
  *
- * this is based on ideas from devres, but focused on life-cycle
- * management during spi_message processing
+ * This is based on ideas from devres, but focused on life-cycle
+ * management during spi_message processing.
  */
 struct spi_res {
 	struct list_head        entry;
@@ -917,7 +921,7 @@ struct spi_res {
  *
  * The spi_messages themselves consist of a series of read+write transfer
  * segments.  Those segments always read the same number of bits as they
- * write; but one or the other is easily ignored by passing a null buffer
+ * write; but one or the other is easily ignored by passing a NULL buffer
  * pointer.  (This is unlike most types of I/O API, because SPI hardware
  * is full duplex.)
  *
@@ -928,8 +932,8 @@ struct spi_res {
 
 /**
  * struct spi_transfer - a read/write buffer pair
- * @tx_buf: data to be written (dma-safe memory), or NULL
- * @rx_buf: data to be read (dma-safe memory), or NULL
+ * @tx_buf: data to be written (DMA-safe memory), or NULL
+ * @rx_buf: data to be read (DMA-safe memory), or NULL
  * @tx_dma: DMA address of tx_buf, if @spi_message.is_dma_mapped
  * @rx_dma: DMA address of rx_buf, if @spi_message.is_dma_mapped
  * @tx_nbits: number of bits used for writing. If 0 the default
@@ -952,7 +956,7 @@ struct spi_res {
  * @word_delay: inter word delay to be introduced after each word size
  *	(set by bits_per_word) transmission.
  * @effective_speed_hz: the effective SCK-speed that was used to
- *      transfer this transfer. Set to 0 if the spi bus driver does
+ *      transfer this transfer. Set to 0 if the SPI bus driver does
  *      not support it.
  * @transfer_list: transfers are sequenced through @spi_message.transfers
  * @tx_sg: Scatterlist for transmit, currently not for client use
@@ -981,16 +985,16 @@ struct spi_res {
  *	transmitting the "pre" word, and the "post" timestamp after receiving
  *	transmit confirmation from the controller for the "post" word.
  * @timestamped: true if the transfer has been timestamped
- * @error: Error status logged by spi controller driver.
+ * @error: Error status logged by SPI controller driver.
  *
  * SPI transfers always write the same number of bytes as they read.
  * Protocol drivers should always provide @rx_buf and/or @tx_buf.
  * In some cases, they may also want to provide DMA addresses for
  * the data being transferred; that may reduce overhead, when the
- * underlying driver uses dma.
+ * underlying driver uses DMA.
  *
- * If the transmit buffer is null, zeroes will be shifted out
- * while filling @rx_buf.  If the receive buffer is null, the data
+ * If the transmit buffer is NULL, zeroes will be shifted out
+ * while filling @rx_buf.  If the receive buffer is NULL, the data
  * shifted in will be discarded.  Only "len" bytes shift out (or in).
  * It's an error to try to shift out a partial word.  (For example, by
  * shifting out three bytes with word size of sixteen or twenty bits;
@@ -1024,7 +1028,7 @@ struct spi_res {
  * Some devices need protocol transactions to be built from a series of
  * spi_message submissions, where the content of one message is determined
  * by the results of previous messages and where the whole transaction
- * ends when the chipselect goes intactive.
+ * ends when the chipselect goes inactive.
  *
  * When SPI can transfer in 1x,2x or 4x. It can get this transfer information
  * from device through @tx_nbits and @rx_nbits. In Bi-direction, these
@@ -1038,10 +1042,11 @@ struct spi_res {
  * and its transfers, ignore them until its completion callback.
  */
 struct spi_transfer {
-	/* It's ok if tx_buf == rx_buf (right?)
-	 * for MicroWire, one buffer must be null
-	 * buffers must work with dma_*map_single() calls, unless
-	 *   spi_message.is_dma_mapped reports a pre-existing mapping
+	/*
+	 * It's okay if tx_buf == rx_buf (right?).
+	 * For MicroWire, one buffer must be NULL.
+	 * Buffers must work with dma_*map_single() calls, unless
+	 * spi_message.is_dma_mapped reports a pre-existing mapping.
 	 */
 	const void	*tx_buf;
 	void		*rx_buf;
@@ -1061,9 +1066,9 @@ struct spi_transfer {
 	unsigned	tx_nbits:3;
 	unsigned	rx_nbits:3;
 	unsigned	timestamped:1;
-#define	SPI_NBITS_SINGLE	0x01 /* 1bit transfer */
-#define	SPI_NBITS_DUAL		0x02 /* 2bits transfer */
-#define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
+#define	SPI_NBITS_SINGLE	0x01 /* 1-bit transfer */
+#define	SPI_NBITS_DUAL		0x02 /* 2-bit transfer */
+#define	SPI_NBITS_QUAD		0x04 /* 4-bit transfer */
 	u8		bits_per_word;
 	struct spi_delay	delay;
 	struct spi_delay	cs_change_delay;
@@ -1084,7 +1089,7 @@ struct spi_transfer {
  * struct spi_message - one multi-segment SPI transaction
  * @transfers: list of transfer segments in this transaction
  * @spi: SPI device to which the transaction is queued
- * @is_dma_mapped: if true, the caller provided both dma and cpu virtual
+ * @is_dma_mapped: if true, the caller provided both DMA and CPU virtual
  *	addresses for each transfer buffer
  * @complete: called to report transaction completions
  * @context: the argument to complete() when it's called
@@ -1094,7 +1099,7 @@ struct spi_transfer {
  * @status: zero for success, else negative errno
  * @queue: for use by whichever driver currently owns the message
  * @state: for use by whichever driver currently owns the message
- * @resources: for resource management when the spi message is processed
+ * @resources: for resource management when the SPI message is processed
  * @prepared: spi_prepare_message was called for the this message
  * @t: for use with spi_message_alloc() when message and transfers have
  *	been allocated together
@@ -1123,7 +1128,8 @@ struct spi_message {
 	/* spi_prepare_message() was called for this message */
 	bool			prepared;
 
-	/* REVISIT:  we might want a flag affecting the behavior of the
+	/*
+	 * REVISIT: we might want a flag affecting the behavior of the
 	 * last transfer ... allowing things like "read 16 bit length L"
 	 * immediately followed by "read L bytes".  Basically imposing
 	 * a specific message scheduling algorithm.
@@ -1141,14 +1147,15 @@ struct spi_message {
 	unsigned		frame_length;
 	unsigned		actual_length;
 
-	/* For optional use by whatever driver currently owns the
+	/*
+	 * For optional use by whatever driver currently owns the
 	 * spi_message ...  between calls to spi_async and then later
 	 * complete(), that's the spi_controller controller driver.
 	 */
 	struct list_head	queue;
 	void			*state;
 
-	/* List of spi_res reources when the spi message is processed */
+	/* List of spi_res resources when the SPI message is processed */
 	struct list_head        resources;
 
 	/* For embedding transfers into the memory of the message */
@@ -1188,7 +1195,7 @@ spi_transfer_delay_exec(struct spi_transfer *t)
 /**
  * spi_message_init_with_transfers - Initialize spi_message and append transfers
  * @m: spi_message to be initialized
- * @xfers: An array of spi transfers
+ * @xfers: An array of SPI transfers
  * @num_xfers: Number of items in the xfer array
  *
  * This function initializes the given spi_message and adds each spi_transfer in
@@ -1205,10 +1212,10 @@ struct spi_transfer *xfers, unsigned int num_xfers)
 		spi_message_add_tail(&xfers[i], m);
 }
 
-/* It's fine to embed message and transaction structures in other data
+/*
+ * It's fine to embed message and transaction structures in other data
  * structures so long as you don't free them while they're in use.
  */
-
 static inline struct spi_message *spi_message_alloc(unsigned ntrans, gfp_t flags)
 {
 	struct spi_message *m;
@@ -1312,7 +1319,7 @@ typedef void (*spi_replaced_release_t)(struct spi_controller *ctlr,
  *                                 replacements that have occurred
  *                                 so that they can get reverted
  * @release:            some extra release code to get executed prior to
- *                      relasing this structure
+ *                      releasing this structure
  * @extradata:          pointer to some extra data if requested or NULL
  * @replaced_transfers: transfers that have been replaced and which need
  *                      to get restored
@@ -1322,9 +1329,9 @@ typedef void (*spi_replaced_release_t)(struct spi_controller *ctlr,
  * @inserted_transfers: array of spi_transfers of array-size @inserted,
  *                      that have been replacing replaced_transfers
  *
- * note: that @extradata will point to @inserted_transfers[@inserted]
+ * Note: that @extradata will point to @inserted_transfers[@inserted]
  * if some extra allocation is requested, so alignment will be the same
- * as for spi_transfers
+ * as for spi_transfers.
  */
 struct spi_replaced_transfers {
 	spi_replaced_release_t release;
@@ -1350,7 +1357,8 @@ extern int spi_split_transfers_maxwords(struct spi_controller *ctlr,
 
 /*---------------------------------------------------------------------------*/
 
-/* All these synchronous SPI transfer routines are utilities layered
+/*
+ * All these synchronous SPI transfer routines are utilities layered
  * over the core async transfer primitive.  Here, "synchronous" means
  * they will sleep uninterruptibly until the async transfer completes.
  */
@@ -1493,7 +1501,7 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8 cmd)
  *
  * Callable only from contexts that can sleep.
  *
- * Return: the (unsigned) sixteen bit number returned by the device in cpu
+ * Return: the (unsigned) sixteen bit number returned by the device in CPU
  * endianness, or else a negative error code.
  */
 static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
@@ -1521,7 +1529,7 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
  * As a rule, SPI devices can't be probed.  Instead, board init code
  * provides a table listing the devices which are present, with enough
  * information to bind and set up the device's driver.  There's basic
- * support for nonstatic configurations too; enough to handle adding
+ * support for non-static configurations too; enough to handle adding
  * parport adapters, or microcontrollers acting as USB-to-SPI bridges.
  */
 
@@ -1558,12 +1566,13 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
  * are active in some dynamic board configuration models.
  */
 struct spi_board_info {
-	/* The device name and module name are coupled, like platform_bus;
+	/*
+	 * The device name and module name are coupled, like platform_bus;
 	 * "modalias" is normally the driver name.
 	 *
 	 * platform_data goes to spi_device.dev.platform_data,
 	 * controller_data goes to spi_device.controller_data,
-	 * irq is copied too
+	 * IRQ is copied too.
 	 */
 	char		modalias[SPI_NAME_SIZE];
 	const void	*platform_data;
@@ -1575,7 +1584,8 @@ struct spi_board_info {
 	u32		max_speed_hz;
 
 
-	/* bus_num is board specific and matches the bus_num of some
+	/*
+	 * bus_num is board specific and matches the bus_num of some
 	 * spi_controller that will probably be registered later.
 	 *
 	 * chip_select reflects how this chip is wired to that master;
@@ -1584,12 +1594,14 @@ struct spi_board_info {
 	u16		bus_num;
 	u16		chip_select;
 
-	/* mode becomes spi_device.mode, and is essential for chips
+	/*
+	 * mode becomes spi_device.mode, and is essential for chips
 	 * where the default of SPI_CS_HIGH = 0 is wrong.
 	 */
 	u32		mode;
 
-	/* ... may need additional spi_device chip config data here.
+	/*
+	 * ... may need additional spi_device chip config data here.
 	 * avoid stuff protocol drivers can set; but include stuff
 	 * needed to behave without being bound to a driver:
 	 *  - quirks like clock rate mattering when not selected
@@ -1606,7 +1618,8 @@ spi_register_board_info(struct spi_board_info const *info, unsigned n)
 	{ return 0; }
 #endif
 
-/* If you're hotplugging an adapter with devices (parport, usb, etc)
+/*
+ * If you're hotplugging an adapter with devices (parport, USB, etc)
  * use spi_new_device() to describe each device.  You can also call
  * spi_unregister_device() to start making that device vanish, but
  * normally that would be handled by spi_unregister_controller().
diff --git a/include/trace/events/spi.h b/include/trace/events/spi.h
index c0248a8fa79c..e63d4a24d879 100644
--- a/include/trace/events/spi.h
+++ b/include/trace/events/spi.h
@@ -167,7 +167,7 @@ TRACE_EVENT(spi_message_done,
 );
 
 /*
- * consider a buffer valid if non-NULL and if it doesn't match the dummy buffer
+ * Consider a buffer valid if non-NULL and if it doesn't match the dummy buffer
  * that only exist to work with controllers that have SPI_CONTROLLER_MUST_TX or
  * SPI_CONTROLLER_MUST_RX.
  */
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 00/15] spi: Header and core clean up and refactoring

[Mark Brown]

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

On Mon, Jul 10, 2023 at 06:49:17PM +0300, Andy Shevchenko wrote:
> Various cleanups and refactorings of the SPI header and core parts
> united in a single series. It also touches drivers under SPI subsystem
> folder on the pure renaming purposes of some constants.

I've queued 1-3, 6-8 and 11- for CI thanks.

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

Re: [PATCH v2 00/15] spi: Header and core clean up and refactoring

[Andy Shevchenko]

On Mon, Jul 10, 2023 at 06:31:12PM +0100, Mark Brown wrote:
> On Mon, Jul 10, 2023 at 06:49:17PM +0300, Andy Shevchenko wrote:
> > Various cleanups and refactorings of the SPI header and core parts
> > united in a single series. It also touches drivers under SPI subsystem
> > folder on the pure renaming purposes of some constants.
> 
> I've queued 1-3, 6-8 and 11- for CI thanks.

Thank you!
Do you think patch 9 deserves to be proceeded?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 00/15] spi: Header and core clean up and refactoring

[Mark Brown]

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

On Tue, Jul 11, 2023 at 02:11:59PM +0300, Andy Shevchenko wrote:

> Do you think patch 9 deserves to be proceeded?

That one I need to think about, may as well resend it and I can think
about the resend.

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

Re: [PATCH v2 00/15] spi: Header and core clean up and refactoring

[Andy Shevchenko]

On Tue, Jul 11, 2023 at 02:38:37PM +0100, Mark Brown wrote:
> On Tue, Jul 11, 2023 at 02:11:59PM +0300, Andy Shevchenko wrote:
> 
> > Do you think patch 9 deserves to be proceeded?
> 
> That one I need to think about, may as well resend it and I can think
> about the resend.

Got it.

Probably I have to amend commit message in the patch 9 to point out why
struct_size() is better.

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v2 00/15] spi: Header and core clean up and refactoring

[Mark Brown]

On Mon, 10 Jul 2023 18:49:17 +0300, Andy Shevchenko wrote:
> Various cleanups and refactorings of the SPI header and core parts
> united in a single series. It also touches drivers under SPI subsystem
> folder on the pure renaming purposes of some constants.
> 
> No functional change intended.
> 
> Changelog v2:
> - added new patches 3,4,5,10,13,14
> - massaged comment and kernel doc in patch 9
> - split used to be patch 4 to patches 11,12
> - covered a few things in SPI core in patch 15
> - amended commit message for above (Mark)
> - reshuffled patches in the series for better logical grouping
> 
> [...]

Applied to

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

Thanks!

[01/15] spi: Remove unneeded OF node NULL checks
        commit: fbab5b2c09060e8034fee6ec2df69a62594fb7db
[02/15] spi: Drop duplicate IDR allocation code in spi_register_controller()
        commit: 440c47331bdb889e24128c75387c695ca81d9b9b
[03/15] spi: Replace if-else-if by bitops and multiplications
        commit: 2b308e7176e366a52a07a49868e3b1a295e56785
[06/15] spi: Use sysfs_emit() to instead of s*printf()
        commit: f2daa4667fda1aa951b91da0ae9675a5da9d7716
[07/15] spi: Sort headers alphabetically
        commit: edf6a864c996f9a9f5299a3b3e574a37e64000c5
[08/15] spi: Clean up headers
        (no commit info)
[11/15] spi: Get rid of old SPI_MASTER_NO_TX & SPI_MASTER_NO_RX
        commit: c397f09e5498994790503a64486213ef85e58db9
[12/15] spi: Get rid of old SPI_MASTER_MUST_TX & SPI_MASTER_MUST_RX
        commit: 90366cd60133a9f5b6a2f31360367c658585e125
[13/15] spi: Rename SPI_MASTER_GPIO_SS to SPI_CONTROLLER_GPIO_SS
        commit: 82238d2cbd99ebd09dda48fb7c1c8802097da6a2
[14/15] spi: Convert to SPI_CONTROLLER_HALF_DUPLEX
        commit: 7a2b552c8e0e5bb280558f6c120140f5f06323bc
[15/15] spi: Fix spelling typos and acronyms capitalization
        commit: 702ca0269ed56e2d8dae7874a4d8af268e2a382e

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