* [rfc 1/2] pinctrl: pinconf-generic: perform basic checks on pincfg properties
2026-02-03 17:29 [rfc 0/2] pinctrl property checks Conor Dooley
@ 2026-02-03 17:29 ` Conor Dooley
2026-02-03 17:29 ` [rfc 2/2] dt-bindings: pinctrl: pincfg-node: add restrictions on conflicting properties Conor Dooley
2026-02-03 23:30 ` [rfc 0/2] pinctrl property checks Linus Walleij
2 siblings, 0 replies; 5+ messages in thread
From: Conor Dooley @ 2026-02-03 17:29 UTC (permalink / raw)
To: linusw
Cc: conor, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-gpio, devicetree, linux-kernel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [rfc 2/2] dt-bindings: pinctrl: pincfg-node: add restrictions on conflicting properties
2026-02-03 17:29 [rfc 0/2] pinctrl property checks Conor Dooley
2026-02-03 17:29 ` [rfc 1/2] pinctrl: pinconf-generic: perform basic checks on pincfg properties Conor Dooley
@ 2026-02-03 17:29 ` Conor Dooley
2026-02-03 23:30 ` [rfc 0/2] pinctrl property checks Linus Walleij
2 siblings, 0 replies; 5+ messages in thread
From: Conor Dooley @ 2026-02-03 17:29 UTC (permalink / raw)
To: linusw
Cc: conor, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-gpio, devicetree, linux-kernel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread