Re: [PATCH v2] gpiolib: of: Only apply the SPI CS quirk to SPI buses

[Krzysztof Kozlowski]

On 19/08/2026 17:55, Maciej Andrzejewski ICEYE wrote:
> The legacy SPI chip-select polarity quirk in of_gpio_flags_quirks() is
> keyed on nothing but the property name "cs-gpios". That name is not
> exclusive to SPI: nand-controller.yaml documents the very same property
> for NAND controllers, and rawnand_dt_parse_gpio_cs() requests those
> lines with gpiod_count(dev, "cs"), which gpiolib expands to "cs-gpios".
> A NAND controller therefore has SPI chip-select semantics forced onto
> its chip selects, and any chip node whose first reg cell matches a GPIO
> index is silently flipped to active low. The NAND core requests the
> descriptors GPIOD_OUT_HIGH and drivers assert with a logical 0, so the
> inversion leaves the die permanently deselected.
> 
> The example in nand-controller.yaml is itself affected: it pairs a
> native chip select with a GPIO one and gives the second chip a reg of 1,
> which trips the quirk whenever CONFIG_SPI_MASTER is enabled. In-tree the
> collision is real but latent. 36 board trees, all Atmel/Microchip at91,
> give their nand@3 controller a cs-gpios line, and they escape only
> because the sole child of those nodes is a partitions container with no
> reg for the quirk to match against.
> 
> Device tree carries no bus type marker, so identify the bus from two
> hints. Properties of an SPI peripheral are namespaced with "spi-"
> (spi-max-frequency, spi-cpol, spi-cs-high and the rest of
> spi-peripheral-props.yaml), whereas a NAND chip node carries only reg,
> nand-* and its partition table. That is a convention rather than a
> guarantee, since compatible and reg are the only properties
> spi-controller.yaml makes mandatory for a peripheral, so let the
> controller settle the remaining cases: spi-controller.yaml constrains
> the controller nodename to ^spi(@.*|-[0-9]+)?$, which makes the name the
> one bus marker every conforming controller has to carry. A peripheral
> that carries nothing but compatible and reg therefore keeps its active
> low default through its parent. Skip the quirk only when neither test
> matches.
> 
> Both tests were scored over every board device tree in the kernel,
> expanded with the same cpp and scripts/dtc pipeline the build uses, at
> v7.2-rc7. All 3620 trees under arch/*/boot/dts expand; 503 of them hold
> at least one GPIO chip select that can reach the quirk, 765 such chip
> selects in total. 761 are matched by both tests, and none are matched by
> neither, so no in-tree board changes behaviour.
> 
> One needs the property scan on its own: psc@11400 on ac14xx, a
> fsl,mpc5121-psc-spi named after the hardware block rather than the bus,
> whose m25p128@0 child carries spi-max-frequency.
> 
> Three need the nodename on its own, all peripherals with no "spi-"
> property of any kind:
> 
>   - panel@0 under the spi-gpio controller on rk3566-anbernic-rg503
>   - can@0, an mcp251xfd, under ecspi3 on imx8mn-vhip4-evalboard-v1
>   - spi@1 under ecspi1 on imx53-ppd
> 
> The two tests are complementary, so both are needed to keep every
> in-tree board working. A tree that names its controller after the
> hardware block and gives it a peripheral with no "spi-" property would
> still lose the quirk. No in-tree board does, and such a controller is
> already outside the nodename pattern spi-controller.yaml requires.
> 
> of_gpio_spi_cs_get_count() in this file identifies SPI controllers with
> of_device_is_compatible() instead, but it only has to name three legacy
> controllers whose bindings are closed. An allow-list here would have to
> name every SPI controller binding in the tree, 71 distinct compatible
> strings among the candidates alone, and grow with every new one.
> 
> Signed-off-by: Maciej Andrzejewski ICEYE <maciej.andrzejewski@xxxxxxxxxxx>
> ---
> Changes in v2:
> - Squashed the two patches into one. The property scan and the nodename
>   test are complementary, so patch 1 alone stopped applying the quirk to
>   the three peripherals that carry no "spi-" property, which would have
>   broken those boards for anyone bisecting through the series. Reported
>   by an automated review of v1.
> - Folded the two helpers into a single of_gpio_is_spi_chipselect().
> - Said outright that a peripheral with nothing but compatible and reg
>   keeps its active low default through the controller nodename.
> - Rebased; drivers/gpio/gpiolib-of.c is unchanged since v7.2-rc7, so the
>   diff itself is the v1 pair verbatim.
> 
> v1: https://lore.kernel.org/r/20260810141629.81650-1-maciej.andrzejewski@xxxxxxxxxxx
> 
>  drivers/gpio/gpiolib-of.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 940b566946ce..d73114166f6b 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -340,6 +340,28 @@ static void of_gpio_set_polarity_by_property(const struct device_node *np,
>  	}
>  }
>  
> +/*
> + * The legacy SPI chip select binding below is keyed on a property name that
> + * other subsystems reuse for the same purpose, notably NAND controllers (see
> + * Documentation/devicetree/bindings/mtd/nand-controller.yaml), whose chip
> + * selects carry no SPI polarity semantics. Device tree has no bus type
> + * marker, so take two hints. Properties of an SPI peripheral are namespaced
> + * with "spi-", and a peripheral that declares none is covered by the
> + * controller, whose nodename spi-controller.yaml constrains to
> + * ^spi(@.*|-[0-9]+)?$.
> + */
> +static bool of_gpio_is_spi_chipselect(const struct device_node *np,
> +				      const struct device_node *child)
> +{
> +	struct property *pp;
> +
> +	for_each_property_of_node(child, pp)
> +		if (str_has_prefix(pp->name, "spi-"))

You should not rely on prefixes of properties. Node can have no such
properties at all.


> +			return true;
> +
> +	return of_node_name_prefix(np, "spi");

And neither this. You just added node name based ABI.

Nope.

Each peripheral exactly knows that it is a SPI device - it 100% specific
knowledge based on the bus, thus nothing of above is needed.

Best regards,
Krzysztof