linux-kernel - Re: [PATCH v4 3/9] spi: support controllers with multiple data lanes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251227171850.0c93c1c9@jic23-huawei>
Date: Sat, 27 Dec 2025 17:18:50 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Marcelo Schmitt <marcelo.schmitt@...log.com>,
 Michael Hennerich <michael.hennerich@...log.com>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>, Sean Anderson
 <sean.anderson@...ux.dev>, linux-spi@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org
Subject: Re: [PATCH v4 3/9] spi: support controllers with multiple data
 lanes

On Fri, 19 Dec 2025 15:32:11 -0600
David Lechner <dlechner@...libre.com> wrote:

> Add support for SPI controllers with multiple physical SPI data lanes.
> (A data lane in this context means lines connected to a serializer, so a
> controller with two data lanes would have two serializers in a single
> controller).
> 
> This is common in the type of controller that can be used with parallel
> flash memories, but can be used for general purpose SPI as well.
> 
> To indicate support, a controller just needs to set ctlr->num_data_lanes
> to something greater than 1. Peripherals indicate which lane they are
> connected to via device tree (ACPI support can be added if needed).
> 
> The spi-{tx,rx}-bus-width DT properties can now be arrays. The length of
> the array indicates the number of data lanes, and each element indicates
> the bus width of that lane. For now, we restrict all lanes to have the
> same bus width to keep things simple. Support for an optional controller
> lane mapping property is also implemented.
> 
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
> 
> v4 changes:
> - Update for changes in devicetree bindings.
> - Don't put new fields in the middle of CS fields.
> - Dropped acks since this was a significant rework.
> 
> v3 changes:
> * Renamed "buses" to "lanes" to reflect devicetree property name change.
> 
> This patch has been seen in a different series [1] by Sean before:
> 
> [1]: https://lore.kernel.org/linux-spi/20250616220054.3968946-4-sean.anderson@linux.dev/
> 
> Changes:
> * Use u8 array instead of bitfield so that the order of the mapping is
>   preserved. (Now looks very much like chip select mapping.)
> * Added doc strings for added fields.
> * Tweaked the comments.
> ---
>  drivers/spi/spi.c       | 114 +++++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/spi/spi.h |  22 ++++++++++
>  2 files changed, 134 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index e25df9990f82..9caa22583b8f 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2370,7 +2370,52 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>  		spi->mode |= SPI_CS_HIGH;
>  
>  	/* Device DUAL/QUAD mode */
> -	if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) {
> +
> +	rc = of_property_read_u32_array(nc, "spi-tx-lane-map", spi->tx_lane_map,
> +					ARRAY_SIZE(spi->tx_lane_map));

Why must there always be a fixed number of these?  Isn't it meant to be
one per lane in use? Or at least to me that seems reasonable assumption?
Maybe use of_property_read_variable_u32_array() which takes min and max sizes
and returns (on success) how many were there.



> +	if (rc == -EINVAL) {
> +		/* Default lane map */
> +		for (idx = 0; idx < ARRAY_SIZE(spi->tx_lane_map); idx++)
> +			spi->tx_lane_map[idx] = idx;

Having this fixed in size is fine even if we only use first few elements.

> +	} else if (rc < 0) {
> +		dev_err(&ctlr->dev,
> +			"failed to read spi-tx-lane-map property: %d\n", rc);
> +		return rc;
> +	}
> +
> +	rc = of_property_count_u32_elems(nc, "spi-tx-bus-width");
> +	if (rc < 0 && rc != -EINVAL) {
> +		dev_err(&ctlr->dev,

...

> @@ -2394,7 +2439,61 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>  		}
>  	}
>  
> -	if (!of_property_read_u32(nc, "spi-rx-bus-width", &value)) {
> +	for (idx = 0; idx < spi->num_tx_lanes; idx++) {
> +		if (spi->tx_lane_map[idx] >= spi->controller->num_data_lanes) {
> +			dev_err(&ctlr->dev,
> +				"spi-tx-lane-map has invalid value %d (num_data_lanes=%d)\n",
> +				spi->tx_lane_map[idx],
> +				spi->controller->num_data_lanes);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	rc = of_property_read_u32_array(nc, "spi-rx-lane-map", spi->rx_lane_map,
> +					ARRAY_SIZE(spi->rx_lane_map));

Similar to above. Not obvious to me why this is fixed size read.


> +	if (rc == -EINVAL) {
> +		/* Default lane map */
> +		for (idx = 0; idx < ARRAY_SIZE(spi->rx_lane_map); idx++)
> +			spi->rx_lane_map[idx] = idx;
...
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index cb2c2df31089..7aff60ab257e 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
...

> +/* Max no. of data lanes supported per spi device */