Re: [PATCH v5 3/9] spi: support controllers with multiple data lanes

Jonathan Cameron
2026-01-14T09:05:18Z

On Mon, 12 Jan 2026 11:45:21 -0600
David Lechner <dlechner@baylibre.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@baylibre.com>
> ---
>
> v5 changes:
> - Use of_property_read_variable_u32_array() for lane maps.
For this, I think you need to check for short maps.
>
> 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 | 116 +++++++++++++++++++++++++++++++++++++++++++++++-
> include/linux/spi/spi.h | 22 +++++++++
> 2 files changed, 136 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index e25df9990f82..5c3f9ba3f606 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2370,7 +2370,53 @@ 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_variable_u32_array(nc, "spi-tx-lane-map",
> + spi->tx_lane_map, 1,
> + ARRAY_SIZE(spi->tx_lane_map));

This reads 'up to' the ARRAY_SIZE(spi->tx_lane_map)
If it is short, what is the right thing to do? I'd either expect a check
for that or for rc to be stashed somewhere if positive for later use.
If the intent is for short the default of 0 is fine, then if it's a lot
short we'll end up with repeated mappings to 0 which makes little sense.

> + if (rc == -EINVAL) {
> + /* Default lane map */
> + for (idx = 0; idx < ARRAY_SIZE(spi->tx_lane_map); idx++)
> + spi->tx_lane_map[idx] = idx;
> + } 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,
> + "failed to read spi-tx-bus-width property: %d\n", rc);
> + return rc;
> + }
> +
> + if (rc == -EINVAL) {
> + /* Default when property is not present. */
> + spi->num_tx_lanes = 1;
> + } else {
> + u32 first_value;
> +
> + spi->num_tx_lanes = rc;
> +
> + for (idx = 0; idx < spi->num_tx_lanes; idx++) {
> + of_property_read_u32_index(nc, "spi-tx-bus-width", idx,
> + &value);

Probably want a sanity check on return value of that even though we are fairly sure
it won't fail.

> +
> + /*
> + * For now, we only support all lanes having the same
> + * width so we can keep using the existing mode flags.
> + */
> + if (!idx)
> + first_value = value;
> + else if (first_value != value) {
> + dev_err(&ctlr->dev,
> + "spi-tx-bus-width has inconsistent values: first %d vs later %d\n",
> + first_value, value);
> + return -EINVAL;
> + }
> + }