[rfc 0/2] pinctrl property checks

[rfc 0/2] pinctrl property checks

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Hey Linus,

Finally got around to sending the property stuff that we were talking
about. It's definitely not the best thing I have ever written, but I
think it does an okay job of warning about setups that don't make sense
while adding fairly little complexity wise.

Cheers,
Conor.

CC: Linus Walleij <linusw@kernel.org>
CC: Rob Herring <robh@kernel.org>
CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
CC: Conor Dooley <conor+dt@kernel.org>
CC: linux-gpio@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org

Conor Dooley (2):
  pinctrl: pinconf-generic: perform basic checks on pincfg properties
  dt-bindings: pinctrl: pincfg-node: add restrictions on conflicting
    properties

 .../bindings/pinctrl/pincfg-node.yaml         | 105 ++++++++++++++++--
 drivers/pinctrl/pinconf-generic.c             |  41 ++++++-
 2 files changed, 138 insertions(+), 8 deletions(-)

-- 
2.51.0

[rfc 1/2] pinctrl: pinconf-generic: perform basic checks on pincfg properties

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Some pinconf properties are mutually exclusive, either because they
convey the same information in different units or represent incompatible
configurations of the same pin. Attempt, in two ways, to prevent these
situations.

Firstly, for enable/disable properties, produce an error if both are
set. Since enable/disable properties share the same enum value, they can
be trivially checked via the newly added bitmap. Having both enable and
disable for the same config makes no sense at all, so produce an error
in this case.

For interactions between properties, doing them outside the loop makes
more sense as it can be evaluated once. In case there are some edge
cases that would be broken by producing an error, only warn for now.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
ngl, this is a bit of a lazy approach, and I just didn't bother checking
the more complex things that a simple bitmap scheme could not address.
Chief among the more complexes thing that I didn't do anything about is
that properties that I don't feel make sense when something is disabled
don't complain when they are present but the disabled property also is.
That's because enable and disable are the same bit in the bitmap, so
more info than just the bitmap would be required to perform that check.

Part of me says that that's okay and should be handled by dt tools, since
it's mostly harmless and that denying things that are effectively
undefined behaviour (well really, they're driver/implementation defined
behaviour) is what's actually valuable here.

I dunno about the tests being done as a sum, but I dunno if there's a
neater way to do that test without creating bitmaps to and with the
created one.
---
 drivers/pinctrl/pinconf-generic.c | 41 ++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 366775841c63..d182ec84e2df 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -222,7 +222,10 @@ static int parse_dt_cfg(struct device_node *np,
 			unsigned int count, unsigned long *cfg,
 			unsigned int *ncfg)
 {
-	int i;
+	unsigned long *properties;
+	int i, test;
+
+	properties = bitmap_zalloc(count, GFP_KERNEL);
 
 	for (i = 0; i < count; i++) {
 		u32 val;
@@ -251,11 +254,45 @@ static int parse_dt_cfg(struct device_node *np,
 		if (ret)
 			val = par->default_value;
 
+		/* if param is greater than count, these are custom properties */
+		if (par->param <= count) {
+			ret = test_and_set_bit(par->param, properties);
+			if (ret) {
+				pr_err("%s: conflicting setting detected for %s\n",
+				       np->name, par->property);
+				bitmap_free(properties);
+				return -EINVAL;
+			}
+		}
+
 		pr_debug("found %s with value %u\n", par->property, val);
 		cfg[*ncfg] = pinconf_to_config_packed(par->param, val);
 		(*ncfg)++;
 	}
 
+	if (test_bit(PIN_CONFIG_DRIVE_STRENGTH, properties) &&
+			test_bit(PIN_CONFIG_DRIVE_STRENGTH_UA, properties))
+		pr_err("%s: cannot have multiple drive strength properties\n",
+		       np->name);
+
+	test = test_bit(PIN_CONFIG_BIAS_BUS_HOLD, properties) +
+		test_bit(PIN_CONFIG_BIAS_DISABLE, properties) +
+		test_bit(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, properties) +
+		test_bit(PIN_CONFIG_BIAS_PULL_UP, properties) +
+		test_bit(PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, properties) +
+		test_bit(PIN_CONFIG_BIAS_PULL_DOWN, properties);
+	if (test > 1)
+		pr_err("%s: cannot have multiple bias configurations\n",
+		       np->name);
+
+	test = test_bit(PIN_CONFIG_DRIVE_OPEN_DRAIN, properties) +
+		test_bit(PIN_CONFIG_DRIVE_OPEN_SOURCE, properties) +
+		test_bit(PIN_CONFIG_DRIVE_PUSH_PULL, properties);
+	if (test > 1)
+		pr_err("%s: cannot have multiple drive configurations\n",
+		       np->name);
+
+	bitmap_free(properties);
 	return 0;
 }
 
@@ -352,6 +389,7 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
 	ret = parse_dt_cfg(np, dt_params, ARRAY_SIZE(dt_params), cfg, &ncfg);
 	if (ret)
 		return ret;
+
 	if (pctldev && pctldev->desc->num_custom_params &&
 		pctldev->desc->custom_params) {
 		ret = parse_dt_cfg(np, pctldev->desc->custom_params,
@@ -360,6 +398,7 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
 			return ret;
 	}
 
+
 	/* no configs found at all */
 	if (ncfg == 0) {
 		*configs = NULL;
-- 
2.51.0

[rfc 2/2] dt-bindings: pinctrl: pincfg-node: add restrictions on conflicting properties

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Many of the possible pincfg properties are not compatible with one
another, either because they represent mutually exclusive states for a
pin or because they provide the same information in different units.

Add some simple restrictions to prevent invalid configurations.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/pinctrl/pincfg-node.yaml         | 105 ++++++++++++++++--
 1 file changed, 98 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
index d1bc389e0a6d..874f0781d2fc 100644
--- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
@@ -162,12 +162,103 @@ properties:
       this affects the expected delay in ps before latching a value to
       an output pin.
 
-if:
-  required:
-    - skew-delay
-then:
-  properties:
-    skew-delay-input-ps: false
-    skew-delay-output-ps: false
+allOf:
+  - if:
+      required:
+        - skew-delay
+    then:
+      properties:
+        skew-delay-input-ps: false
+        skew-delay-output-ps: false
+
+  - if:
+      required:
+        - input-disable
+    then:
+      properties:
+        input-enable: false
+
+  - if:
+      required:
+        - output-disable
+    then:
+      properties:
+        output-enable: false
+        output-impedance-ohms: false
+
+  - if:
+      required:
+        - output-low
+    then:
+      properties:
+        output-high: false
+
+  - if:
+      required:
+        - low-power-enable
+    then:
+      properties:
+        low-power-disable: false
+
+  - if:
+      required:
+        - input-schmitt-disable
+    then:
+      properties:
+        input-schmitt-enable: false
+        input-schmitt-microvolt: false
+
+  - if:
+      required:
+        - drive-strength
+    then:
+      properties:
+        drive-strength-microamp: false
+
+  - if:
+      anyOf:
+        - required:
+            - drive-open-source
+        - required:
+            - drive-open-drain
+        - required:
+            - drive-push-pull
+    then:
+      oneOf:
+        - required:
+            - drive-open-source
+        - required:
+            - drive-open-drain
+        - required:
+            - drive-push-pull
+
+  - if:
+      anyOf:
+        - required:
+            - bias-disable
+        - required:
+            - bias-high-impedance
+        - required:
+            - bias-bus-hold
+        - required:
+            - bias-pull-up
+        - required:
+            - bias-pull-down
+        - required:
+            - bias-pull-pin-default
+    then:
+      oneOf:
+        - required:
+            - bias-disable
+        - required:
+            - bias-high-impedance
+        - required:
+            - bias-bus-hold
+        - required:
+            - bias-pull-up
+        - required:
+            - bias-pull-down
+        - required:
+            - bias-pull-pin-default
 
 additionalProperties: true
-- 
2.51.0

Re: [rfc 0/2] pinctrl property checks

[Linus Walleij]

On Tue, Feb 3, 2026 at 6:30 PM Conor Dooley <conor@kernel.org> wrote:

> Finally got around to sending the property stuff that we were talking
> about. It's definitely not the best thing I have ever written, but I
> think it does an okay job of warning about setups that don't make sense
> while adding fairly little complexity wise.

I like what I see and I think we should apply it for the v7.1 kernel cycle
post v7.0-rc1.

Are you ready to send a non-RFC version by then?

Yours,
Linus Walleij

Re: [rfc 0/2] pinctrl property checks

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

On Wed, Feb 04, 2026 at 12:30:30AM +0100, Linus Walleij wrote:
> On Tue, Feb 3, 2026 at 6:30 PM Conor Dooley <conor@kernel.org> wrote:
> 
> > Finally got around to sending the property stuff that we were talking
> > about. It's definitely not the best thing I have ever written, but I
> > think it does an okay job of warning about setups that don't make sense
> > while adding fairly little complexity wise.
> 
> I like what I see and I think we should apply it for the v7.1 kernel cycle
> post v7.0-rc1.
> 
> Are you ready to send a non-RFC version by then?

The rfc tag is was more because of the point in the cycle we are at than
the question on the driver patch. Sending a non-rtc then should be no
problem.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]