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

[Rob Herring]

On Wed, Sep 2, 2026 at 4:28 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>
> 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.

Do you have an actual suggestion on how to fix it instead? It is
needed for every SPI controller that uses cs-gpios which is
potentially all of them. There's 522 cases of cs-gpios and only a few
are NAND. Even if it is 10 boards per SoC, that's still 52 cases which
would be more than I want to list in the kernel.

Perhaps the SPI core when requesting "cs-gpios" could state "apply the
polarity quirk to to this GPIO" rather than the GPIO core having any
specific knowledge about "cs-gpios".

However, IMO, for bus nodes, the node names are ABI. That's the only
way we can identify what type of bus for some buses. The DT core looks
for "isa", "pci" or "pcie" node names for ISA and PCI buses for
example. So I don't have much issue with the current approach.

Rob