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

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 <[email protected]>
---
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/[email protected]

 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-"))
+			return true;
+
+	return of_node_name_prefix(np, "spi");
+}
+
 static void of_gpio_flags_quirks(const struct device_node *np,
 				 const char *propname,
 				 enum of_gpio_flags *flags,
@@ -374,6 +396,11 @@ static void of_gpio_flags_quirks(const struct device_node *np,
 			if (ret)
 				continue;
 			if (cs == index) {
+				bool active_high;
+
+				if (!of_gpio_is_spi_chipselect(np, child))
+					break;
+
 				/*
 				 * SPI children have active low chip selects
 				 * by default. This can be specified negatively
@@ -386,8 +413,8 @@ static void of_gpio_flags_quirks(const struct device_node *np,
 				 * conflict and the "spi-cs-high" flag will
 				 * take precedence.
 				 */
-				bool active_high = of_property_read_bool(child,
-								"spi-cs-high");
+				active_high = of_property_read_bool(child,
+								    "spi-cs-high");
 				of_gpio_quirk_polarity(child, active_high,
 						       flags);
 				break;

base-commit: bd5f485f3f026225b86573e559af0b7254ef4184
-- 
2.43.0