[PATCH v3 00/10] pinctrl: stm32: Support I/O synchronization

[PATCH v3 00/10] pinctrl: stm32: Support I/O synchronization

[Antonio Borneo]

This v3 is a subset of the v1, split-out to simplify the review.
The old patches concerned in v1 where 05/14, 06/14 and 07/14.

This subset:
- introduces the generic pinctrl properties "skew-delay-input" and
  "skew-delay-output", as suggested by Linus Walleij;
- applies some cleanup to STM32 pinctrl driver to simplify the following
  commits in the series;
- adds support for the I/O synchronization in STM32 pinctrl driver and
  bindings;
- updates the DT for STM32MP25 pinctrl to use the new properties.

Changes v2 -> v3:
- rebased on v6.18-rc1;
- replace "skew-delay-direction" with "skew-delay-input" and
  "skew-delay-output";
- use generic properties from pincfg-node.yaml in dt-bindings;
- add the new properties to the new node eth1 in DT.

Changes v1 -> v2 subset:
- rebased on v6.17-rc1;
- replace ST property "st,io-delay" with generic "skew-delay";
- replace ST property "st,io-delay-path" with generic "skew-delay-direction";
- collapse the other ST property in a single "st,io-sync";
- Link to v1: https://lore.kernel.org/lkml/20241022155658.1647350-1-antonio.borneo@foss.st.com/



Antonio Borneo (10):
  pinctrl: pinconf-generic: Add properties 'skew-delay-{in,out}put'
  dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'
  pinctrl: stm32: Rework stm32_pconf_parse_conf()
  pinctrl: stm32: Simplify handling of backup pin status
  pinctrl: stm32: Drop useless spinlock save and restore
  pinctrl: stm32: Avoid keeping a bool value in a u32 variable
  pinctrl: stm32: Support I/O synchronization parameters
  dt-bindings: pinctrl: stm32: Use properties from pincfg-node.yaml
  dt-bindings: pinctrl: stm32: Support I/O synchronization parameters
  arm64: dts: st: Add I/O sync to eth pinctrl in stm32mp25-pinctrl.dtsi

 .../bindings/pinctrl/pincfg-node.yaml         |  22 ++
 .../bindings/pinctrl/st,stm32-pinctrl.yaml    | 123 ++++++-
 arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi |   4 +
 drivers/pinctrl/pinconf-generic.c             |   4 +
 drivers/pinctrl/stm32/pinctrl-stm32.c         | 345 ++++++++++++++----
 drivers/pinctrl/stm32/pinctrl-stm32.h         |   1 +
 drivers/pinctrl/stm32/pinctrl-stm32mp257.c    |   2 +
 include/linux/pinctrl/pinconf-generic.h       |  12 +
 8 files changed, 421 insertions(+), 92 deletions(-)


base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
-- 
2.34.1

[PATCH v3 01/10] pinctrl: pinconf-generic: Add properties 'skew-delay-{in,out}put'

[Antonio Borneo]

Add the properties 'skew-delay-input' and 'skew-delay-output' to
the generic parameters used for parsing DT files. This allows to
specify the independent skew delay value for the two directions.
This enables drivers that use the generic pin configuration to get
the value passed through these new properties.

Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 drivers/pinctrl/pinconf-generic.c       |  4 ++++
 include/linux/pinctrl/pinconf-generic.h | 12 ++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 5de6ff62c69bd..8caec29e40a7c 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -54,6 +54,8 @@ static const struct pin_config_item conf_items[] = {
 	PCONFDUMP(PIN_CONFIG_SLEEP_HARDWARE_STATE, "sleep hardware state", NULL, false),
 	PCONFDUMP(PIN_CONFIG_SLEW_RATE, "slew rate", NULL, true),
 	PCONFDUMP(PIN_CONFIG_SKEW_DELAY, "skew delay", NULL, true),
+	PCONFDUMP(PIN_CONFIG_SKEW_DELAY_INPUT, "input skew delay", NULL, true),
+	PCONFDUMP(PIN_CONFIG_SKEW_DELAY_OUTPUT, "output skew delay", NULL, true),
 };
 
 static void pinconf_generic_dump_one(struct pinctrl_dev *pctldev,
@@ -190,6 +192,8 @@ static const struct pinconf_generic_params dt_params[] = {
 	{ "sleep-hardware-state", PIN_CONFIG_SLEEP_HARDWARE_STATE, 0 },
 	{ "slew-rate", PIN_CONFIG_SLEW_RATE, 0 },
 	{ "skew-delay", PIN_CONFIG_SKEW_DELAY, 0 },
+	{ "skew-delay-input", PIN_CONFIG_SKEW_DELAY_INPUT, 0 },
+	{ "skew-delay-output", PIN_CONFIG_SKEW_DELAY_OUTPUT, 0 },
 };
 
 /**
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index d9245ecec71dc..6ef0b343f3424 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -112,6 +112,16 @@ struct pinctrl_map;
  *	or latch delay (on outputs) this parameter (in a custom format)
  *	specifies the clock skew or latch delay. It typically controls how
  *	many double inverters are put in front of the line.
+ * @PIN_CONFIG_SKEW_DELAY_INPUT: if the pin has independent values for the
+ *	programmable skew rate (on inputs) and latch delay (on outputs), then
+ *	this parameter (in a custom format) specifies the clock skew only. It
+ *	typically controls how many double inverters are put in front of the
+ *	line.
+ * @PIN_CONFIG_SKEW_DELAY_OUPUT: if the pin has independent values for the
+ *	programmable skew rate (on inputs) and latch delay (on outputs), then
+ *	this parameter (in a custom format) specifies the latch delay only. It
+ *	typically controls how many double inverters are put in front of the
+ *	line.
  * @PIN_CONFIG_SLEEP_HARDWARE_STATE: indicate this is sleep related state.
  * @PIN_CONFIG_SLEW_RATE: if the pin can select slew rate, the argument to
  *	this parameter (on a custom format) tells the driver which alternative
@@ -147,6 +157,8 @@ enum pin_config_param {
 	PIN_CONFIG_PERSIST_STATE,
 	PIN_CONFIG_POWER_SOURCE,
 	PIN_CONFIG_SKEW_DELAY,
+	PIN_CONFIG_SKEW_DELAY_INPUT,
+	PIN_CONFIG_SKEW_DELAY_OUTPUT,
 	PIN_CONFIG_SLEEP_HARDWARE_STATE,
 	PIN_CONFIG_SLEW_RATE,
 	PIN_CONFIG_END = 0x7F,
-- 
2.34.1

[PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Antonio Borneo]

Add the properties 'skew-delay-input' and 'skew-delay-output' to
specify independent skew delay value for the two pin's directions.
Make the new properties unavailable when the existing property
'skew-delay' is selected.

Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 .../bindings/pinctrl/pincfg-node.yaml         | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
index cbfcf215e571d..c3deb103d816b 100644
--- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
@@ -153,4 +153,26 @@ properties:
       pin. Typically indicates how many double-inverters are
       used to delay the signal.
 
+  skew-delay-input:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      this affects the expected clock skew on input pins.
+      Typically indicates how many double-inverters are used to
+      delay the signal.
+
+  skew-delay-output:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      this affects the expected delay before latching a value to an
+      output pins. Typically indicates how many double-inverters are
+      used to delay the signal.
+
+if:
+  required:
+    - skew-delay
+then:
+  properties:
+    skew-delay-input: false
+    skew-delay-output: false
+
 additionalProperties: true
-- 
2.34.1

[PATCH v3 03/10] pinctrl: stm32: Rework stm32_pconf_parse_conf()

[Antonio Borneo]

Reduce the number of parameters of the function by moving inside
the decoding of the field 'config'.

While there:
- change the type of 'param' to 'unsigned int' to handle the extra
  values not in 'enum pin_config_param';
- change the type of 'arg' to 'u32' to avoid additional conversions
  and align to 'u32' the corresponding param of __stm32_gpio_set().

Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 drivers/pinctrl/stm32/pinctrl-stm32.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 3ebb468de830d..6c5f9e015e8e1 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -287,7 +287,7 @@ static void stm32_gpio_rif_release_semaphore(struct stm32_gpio_bank *bank, unsig
 /* GPIO functions */
 
 static inline void __stm32_gpio_set(struct stm32_gpio_bank *bank,
-	unsigned offset, int value)
+	unsigned int offset, u32 value)
 {
 	stm32_gpio_backup_value(bank, offset, value);
 
@@ -1195,10 +1195,11 @@ static bool stm32_pconf_get(struct stm32_gpio_bank *bank,
 }
 
 static int stm32_pconf_parse_conf(struct pinctrl_dev *pctldev,
-		unsigned int pin, enum pin_config_param param,
-		enum pin_config_param arg)
+		unsigned int pin, unsigned long config)
 {
 	struct stm32_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int param = pinconf_to_config_param(config);
+	u32 arg = pinconf_to_config_argument(config);
 	struct pinctrl_gpio_range *range;
 	struct stm32_gpio_bank *bank;
 	int offset, ret = 0;
@@ -1267,9 +1268,7 @@ static int stm32_pconf_group_set(struct pinctrl_dev *pctldev, unsigned group,
 
 	for (i = 0; i < num_configs; i++) {
 		mutex_lock(&pctldev->mutex);
-		ret = stm32_pconf_parse_conf(pctldev, g->pin,
-			pinconf_to_config_param(configs[i]),
-			pinconf_to_config_argument(configs[i]));
+		ret = stm32_pconf_parse_conf(pctldev, g->pin, configs[i]);
 		mutex_unlock(&pctldev->mutex);
 		if (ret < 0)
 			return ret;
@@ -1286,9 +1285,7 @@ static int stm32_pconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
 	int i, ret;
 
 	for (i = 0; i < num_configs; i++) {
-		ret = stm32_pconf_parse_conf(pctldev, pin,
-				pinconf_to_config_param(configs[i]),
-				pinconf_to_config_argument(configs[i]));
+		ret = stm32_pconf_parse_conf(pctldev, pin, configs[i]);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.34.1

[PATCH v3 04/10] pinctrl: stm32: Simplify handling of backup pin status

[Antonio Borneo]

Use C bit-field to keep the backup of the pin status, instead of
explicitly handling the bit-field through shift and mask of a u32
container.

Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 drivers/pinctrl/stm32/pinctrl-stm32.c | 70 +++++++++------------------
 1 file changed, 24 insertions(+), 46 deletions(-)

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 6c5f9e015e8e1..66f9783fce862 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -54,18 +54,6 @@
 #define STM32_GPIO_CIDCFGR(x)	(0x50 + (0x8 * (x)))
 #define STM32_GPIO_SEMCR(x)	(0x54 + (0x8 * (x)))
 
-/* custom bitfield to backup pin status */
-#define STM32_GPIO_BKP_MODE_SHIFT	0
-#define STM32_GPIO_BKP_MODE_MASK	GENMASK(1, 0)
-#define STM32_GPIO_BKP_ALT_SHIFT	2
-#define STM32_GPIO_BKP_ALT_MASK		GENMASK(5, 2)
-#define STM32_GPIO_BKP_SPEED_SHIFT	6
-#define STM32_GPIO_BKP_SPEED_MASK	GENMASK(7, 6)
-#define STM32_GPIO_BKP_PUPD_SHIFT	8
-#define STM32_GPIO_BKP_PUPD_MASK	GENMASK(9, 8)
-#define STM32_GPIO_BKP_TYPE		10
-#define STM32_GPIO_BKP_VAL		11
-
 #define STM32_GPIO_CIDCFGR_CFEN		BIT(0)
 #define STM32_GPIO_CIDCFGR_SEMEN	BIT(1)
 #define STM32_GPIO_CIDCFGR_SCID_MASK	GENMASK(5, 4)
@@ -100,6 +88,15 @@ struct stm32_pinctrl_group {
 	unsigned pin;
 };
 
+struct stm32_pin_backup {
+	unsigned int alt:4;
+	unsigned int mode:2;
+	unsigned int bias:2;
+	unsigned int speed:2;
+	unsigned int drive:1;
+	unsigned int value:1;
+};
+
 struct stm32_gpio_bank {
 	void __iomem *base;
 	struct reset_control *rstc;
@@ -110,7 +107,7 @@ struct stm32_gpio_bank {
 	struct irq_domain *domain;
 	u32 bank_nr;
 	u32 bank_ioport_nr;
-	u32 pin_backup[STM32_GPIO_PINS_PER_BANK];
+	struct stm32_pin_backup pin_backup[STM32_GPIO_PINS_PER_BANK];
 	u8 irq_type[STM32_GPIO_PINS_PER_BANK];
 	bool secure_control;
 	bool rif_control;
@@ -176,38 +173,32 @@ static inline u32 stm32_gpio_get_alt(u32 function)
 static void stm32_gpio_backup_value(struct stm32_gpio_bank *bank,
 				    u32 offset, u32 value)
 {
-	bank->pin_backup[offset] &= ~BIT(STM32_GPIO_BKP_VAL);
-	bank->pin_backup[offset] |= value << STM32_GPIO_BKP_VAL;
+	bank->pin_backup[offset].value = value;
 }
 
 static void stm32_gpio_backup_mode(struct stm32_gpio_bank *bank, u32 offset,
 				   u32 mode, u32 alt)
 {
-	bank->pin_backup[offset] &= ~(STM32_GPIO_BKP_MODE_MASK |
-				      STM32_GPIO_BKP_ALT_MASK);
-	bank->pin_backup[offset] |= mode << STM32_GPIO_BKP_MODE_SHIFT;
-	bank->pin_backup[offset] |= alt << STM32_GPIO_BKP_ALT_SHIFT;
+	bank->pin_backup[offset].mode = mode;
+	bank->pin_backup[offset].alt = alt;
 }
 
 static void stm32_gpio_backup_driving(struct stm32_gpio_bank *bank, u32 offset,
 				      u32 drive)
 {
-	bank->pin_backup[offset] &= ~BIT(STM32_GPIO_BKP_TYPE);
-	bank->pin_backup[offset] |= drive << STM32_GPIO_BKP_TYPE;
+	bank->pin_backup[offset].drive = drive;
 }
 
 static void stm32_gpio_backup_speed(struct stm32_gpio_bank *bank, u32 offset,
 				    u32 speed)
 {
-	bank->pin_backup[offset] &= ~STM32_GPIO_BKP_SPEED_MASK;
-	bank->pin_backup[offset] |= speed << STM32_GPIO_BKP_SPEED_SHIFT;
+	bank->pin_backup[offset].speed = speed;
 }
 
 static void stm32_gpio_backup_bias(struct stm32_gpio_bank *bank, u32 offset,
 				   u32 bias)
 {
-	bank->pin_backup[offset] &= ~STM32_GPIO_BKP_PUPD_MASK;
-	bank->pin_backup[offset] |= bias << STM32_GPIO_BKP_PUPD_SHIFT;
+	bank->pin_backup[offset].bias = bias;
 }
 
 /* RIF functions */
@@ -1798,7 +1789,7 @@ static int __maybe_unused stm32_pinctrl_restore_gpio_regs(
 					struct stm32_pinctrl *pctl, u32 pin)
 {
 	const struct pin_desc *desc = pin_desc_get(pctl->pctl_dev, pin);
-	u32 val, alt, mode, offset = stm32_gpio_pin(pin);
+	u32 mode, offset = stm32_gpio_pin(pin);
 	struct pinctrl_gpio_range *range;
 	struct stm32_gpio_bank *bank;
 	bool pin_is_irq;
@@ -1818,36 +1809,23 @@ static int __maybe_unused stm32_pinctrl_restore_gpio_regs(
 
 	bank = gpiochip_get_data(range->gc);
 
-	alt = bank->pin_backup[offset] & STM32_GPIO_BKP_ALT_MASK;
-	alt >>= STM32_GPIO_BKP_ALT_SHIFT;
-	mode = bank->pin_backup[offset] & STM32_GPIO_BKP_MODE_MASK;
-	mode >>= STM32_GPIO_BKP_MODE_SHIFT;
-
-	ret = stm32_pmx_set_mode(bank, offset, mode, alt);
+	mode = bank->pin_backup[offset].mode;
+	ret = stm32_pmx_set_mode(bank, offset, mode, bank->pin_backup[offset].alt);
 	if (ret)
 		return ret;
 
-	if (mode == 1) {
-		val = bank->pin_backup[offset] & BIT(STM32_GPIO_BKP_VAL);
-		val = val >> STM32_GPIO_BKP_VAL;
-		__stm32_gpio_set(bank, offset, val);
-	}
+	if (mode == 1)
+		__stm32_gpio_set(bank, offset, bank->pin_backup[offset].value);
 
-	val = bank->pin_backup[offset] & BIT(STM32_GPIO_BKP_TYPE);
-	val >>= STM32_GPIO_BKP_TYPE;
-	ret = stm32_pconf_set_driving(bank, offset, val);
+	ret = stm32_pconf_set_driving(bank, offset, bank->pin_backup[offset].drive);
 	if (ret)
 		return ret;
 
-	val = bank->pin_backup[offset] & STM32_GPIO_BKP_SPEED_MASK;
-	val >>= STM32_GPIO_BKP_SPEED_SHIFT;
-	ret = stm32_pconf_set_speed(bank, offset, val);
+	ret = stm32_pconf_set_speed(bank, offset, bank->pin_backup[offset].speed);
 	if (ret)
 		return ret;
 
-	val = bank->pin_backup[offset] & STM32_GPIO_BKP_PUPD_MASK;
-	val >>= STM32_GPIO_BKP_PUPD_SHIFT;
-	ret = stm32_pconf_set_bias(bank, offset, val);
+	ret = stm32_pconf_set_bias(bank, offset, bank->pin_backup[offset].bias);
 	if (ret)
 		return ret;
 
-- 
2.34.1

[PATCH v3 05/10] pinctrl: stm32: Drop useless spinlock save and restore

[Antonio Borneo]

There is no need to acquire a spinlock to only read a register for
debugfs reporting.
Drop such useless spinlock save and restore.

Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 drivers/pinctrl/stm32/pinctrl-stm32.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 66f9783fce862..7175328d0df0c 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -920,9 +920,6 @@ static void stm32_pmx_get_mode(struct stm32_gpio_bank *bank, int pin, u32 *mode,
 	u32 val;
 	int alt_shift = (pin % 8) * 4;
 	int alt_offset = STM32_GPIO_AFRL + (pin / 8) * 4;
-	unsigned long flags;
-
-	spin_lock_irqsave(&bank->lock, flags);
 
 	val = readl_relaxed(bank->base + alt_offset);
 	val &= GENMASK(alt_shift + 3, alt_shift);
@@ -931,8 +928,6 @@ static void stm32_pmx_get_mode(struct stm32_gpio_bank *bank, int pin, u32 *mode,
 	val = readl_relaxed(bank->base + STM32_GPIO_MODER);
 	val &= GENMASK(pin * 2 + 1, pin * 2);
 	*mode = val >> (pin * 2);
-
-	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
 static int stm32_pmx_set_mux(struct pinctrl_dev *pctldev,
@@ -1050,16 +1045,11 @@ static int stm32_pconf_set_driving(struct stm32_gpio_bank *bank,
 static u32 stm32_pconf_get_driving(struct stm32_gpio_bank *bank,
 	unsigned int offset)
 {
-	unsigned long flags;
 	u32 val;
 
-	spin_lock_irqsave(&bank->lock, flags);
-
 	val = readl_relaxed(bank->base + STM32_GPIO_TYPER);
 	val &= BIT(offset);
 
-	spin_unlock_irqrestore(&bank->lock, flags);
-
 	return (val >> offset);
 }
 
@@ -1101,16 +1091,11 @@ static int stm32_pconf_set_speed(struct stm32_gpio_bank *bank,
 static u32 stm32_pconf_get_speed(struct stm32_gpio_bank *bank,
 	unsigned int offset)
 {
-	unsigned long flags;
 	u32 val;
 
-	spin_lock_irqsave(&bank->lock, flags);
-
 	val = readl_relaxed(bank->base + STM32_GPIO_SPEEDR);
 	val &= GENMASK(offset * 2 + 1, offset * 2);
 
-	spin_unlock_irqrestore(&bank->lock, flags);
-
 	return (val >> (offset * 2));
 }
 
@@ -1152,27 +1137,19 @@ static int stm32_pconf_set_bias(struct stm32_gpio_bank *bank,
 static u32 stm32_pconf_get_bias(struct stm32_gpio_bank *bank,
 	unsigned int offset)
 {
-	unsigned long flags;
 	u32 val;
 
-	spin_lock_irqsave(&bank->lock, flags);
-
 	val = readl_relaxed(bank->base + STM32_GPIO_PUPDR);
 	val &= GENMASK(offset * 2 + 1, offset * 2);
 
-	spin_unlock_irqrestore(&bank->lock, flags);
-
 	return (val >> (offset * 2));
 }
 
 static bool stm32_pconf_get(struct stm32_gpio_bank *bank,
 	unsigned int offset, bool dir)
 {
-	unsigned long flags;
 	u32 val;
 
-	spin_lock_irqsave(&bank->lock, flags);
-
 	if (dir)
 		val = !!(readl_relaxed(bank->base + STM32_GPIO_IDR) &
 			 BIT(offset));
@@ -1180,8 +1157,6 @@ static bool stm32_pconf_get(struct stm32_gpio_bank *bank,
 		val = !!(readl_relaxed(bank->base + STM32_GPIO_ODR) &
 			 BIT(offset));
 
-	spin_unlock_irqrestore(&bank->lock, flags);
-
 	return val;
 }
 
-- 
2.34.1

[PATCH v3 06/10] pinctrl: stm32: Avoid keeping a bool value in a u32 variable

[Antonio Borneo]

Change type of variable to avoid keeping the bool return value in
a variable of u32 type.

Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 drivers/pinctrl/stm32/pinctrl-stm32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 7175328d0df0c..ac64cb7f86d74 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -1148,7 +1148,7 @@ static u32 stm32_pconf_get_bias(struct stm32_gpio_bank *bank,
 static bool stm32_pconf_get(struct stm32_gpio_bank *bank,
 	unsigned int offset, bool dir)
 {
-	u32 val;
+	bool val;
 
 	if (dir)
 		val = !!(readl_relaxed(bank->base + STM32_GPIO_IDR) &
-- 
2.34.1

[PATCH v3 07/10] pinctrl: stm32: Support I/O synchronization parameters

[Antonio Borneo]

Devices in the stm32mp2xx family include an I/O synchronization
block on each pin that is used to fine tune and improve the I/O
timing margins of high speed synchronous interfaces.
It can be configured to provide independently for each pin:
- skew rate on input direction or latch delay on output direction;
- inversion of clock signals or re-sampling of data signals.

Add support for the generic properties:
- skew-delay-input;
- skew-delay-output.

Add support for the property 'st,io-sync' to configure clock
inversion or data re-sampling mode.

Show the new parameters on debugfs pinconf-pins.

Enable it for the stm32mp257 pinctrl driver.

Co-developed-by: Valentin Caron <valentin.caron@foss.st.com>
Signed-off-by: Valentin Caron <valentin.caron@foss.st.com>
Co-developed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
Signed-off-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 drivers/pinctrl/stm32/pinctrl-stm32.c      | 241 +++++++++++++++++++++
 drivers/pinctrl/stm32/pinctrl-stm32.h      |   1 +
 drivers/pinctrl/stm32/pinctrl-stm32mp257.c |   2 +
 3 files changed, 244 insertions(+)

diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index ac64cb7f86d74..1f024390e0c2f 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -51,9 +51,20 @@
 #define STM32_GPIO_AFRL		0x20
 #define STM32_GPIO_AFRH		0x24
 #define STM32_GPIO_SECCFGR	0x30
+#define STM32_GPIO_DELAYRL	0x40
+#define STM32_GPIO_ADVCFGRL	0x48
 #define STM32_GPIO_CIDCFGR(x)	(0x50 + (0x8 * (x)))
 #define STM32_GPIO_SEMCR(x)	(0x54 + (0x8 * (x)))
 
+#define STM32_GPIO_ADVCFGR_DLYPATH_MASK		BIT(0)
+#define STM32_GPIO_ADVCFGR_DE_MASK		BIT(1)
+#define STM32_GPIO_ADVCFGR_INVCLK_MASK		BIT(2)
+#define STM32_GPIO_ADVCFGR_RET_MASK		BIT(3)
+#define STM32_GPIO_ADVCFGR_IO_SYNC_MASK		\
+	(STM32_GPIO_ADVCFGR_DE_MASK		\
+	 | STM32_GPIO_ADVCFGR_INVCLK_MASK	\
+	 | STM32_GPIO_ADVCFGR_RET_MASK)
+
 #define STM32_GPIO_CIDCFGR_CFEN		BIT(0)
 #define STM32_GPIO_CIDCFGR_SEMEN	BIT(1)
 #define STM32_GPIO_CIDCFGR_SCID_MASK	GENMASK(5, 4)
@@ -67,6 +78,9 @@
 
 #define SYSCFG_IRQMUX_MASK GENMASK(3, 0)
 
+/* Vendor specific pin configurations */
+#define STM32_GPIO_PIN_CONFIG_IO_SYNC	(PIN_CONFIG_END + 1)
+
 #define gpio_range_to_bank(chip) \
 		container_of(chip, struct stm32_gpio_bank, range)
 
@@ -82,6 +96,23 @@ static const char * const stm32_gpio_functions[] = {
 	"reserved",
 };
 
+static const struct pinconf_generic_params stm32_gpio_bindings[] = {
+	{"st,io-sync", STM32_GPIO_PIN_CONFIG_IO_SYNC, 0},
+};
+
+static u8 io_sync_2_advcfgr[] = {
+	/* data or clock GPIO pass-through */
+	[0] = 0,
+	/* clock GPIO inverted */
+	[1] = STM32_GPIO_ADVCFGR_INVCLK_MASK,
+	/* data GPIO re-sampled on clock rising edge */
+	[2] = STM32_GPIO_ADVCFGR_RET_MASK,
+	/* data GPIO re-sampled on clock falling edge */
+	[3] = STM32_GPIO_ADVCFGR_RET_MASK | STM32_GPIO_ADVCFGR_INVCLK_MASK,
+	/* data GPIO re-sampled on both clock edges */
+	[4] = STM32_GPIO_ADVCFGR_RET_MASK | STM32_GPIO_ADVCFGR_DE_MASK,
+};
+
 struct stm32_pinctrl_group {
 	const char *name;
 	unsigned long config;
@@ -95,6 +126,8 @@ struct stm32_pin_backup {
 	unsigned int speed:2;
 	unsigned int drive:1;
 	unsigned int value:1;
+	unsigned int advcfg:4;
+	unsigned int skew_delay:4;
 };
 
 struct stm32_gpio_bank {
@@ -110,6 +143,7 @@ struct stm32_gpio_bank {
 	struct stm32_pin_backup pin_backup[STM32_GPIO_PINS_PER_BANK];
 	u8 irq_type[STM32_GPIO_PINS_PER_BANK];
 	bool secure_control;
+	bool io_sync_control;
 	bool rif_control;
 };
 
@@ -201,6 +235,21 @@ static void stm32_gpio_backup_bias(struct stm32_gpio_bank *bank, u32 offset,
 	bank->pin_backup[offset].bias = bias;
 }
 
+static void stm32_gpio_backup_advcfg(struct stm32_gpio_bank *bank, u32 offset, u32 mask, u32 value)
+{
+	u32 val;
+
+	val = bank->pin_backup[offset].advcfg;
+	val &= ~mask;
+	val |= value & mask;
+	bank->pin_backup[offset].advcfg = val;
+}
+
+static void stm32_gpio_backup_skew_delay(struct stm32_gpio_bank *bank, u32 offset, u32 delay)
+{
+	bank->pin_backup[offset].skew_delay = delay;
+}
+
 /* RIF functions */
 
 static bool stm32_gpio_rif_valid(struct stm32_gpio_bank *bank, unsigned int gpio_nr)
@@ -1145,6 +1194,155 @@ static u32 stm32_pconf_get_bias(struct stm32_gpio_bank *bank,
 	return (val >> (offset * 2));
 }
 
+static void
+stm32_pconf_set_advcfgr_nolock(struct stm32_gpio_bank *bank, int offset, u32 mask, u32 value)
+{
+	int advcfgr_offset = STM32_GPIO_ADVCFGRL + (offset / 8) * 4;
+	int advcfgr_shift = (offset % 8) * 4;
+	u32 val;
+
+	val = readl_relaxed(bank->base + advcfgr_offset);
+	val &= ~(mask << advcfgr_shift);
+	val |= (value & mask) << advcfgr_shift;
+	writel_relaxed(val, bank->base + advcfgr_offset);
+
+	stm32_gpio_backup_advcfg(bank, offset, mask, value);
+}
+
+static int stm32_pconf_set_advcfgr(struct stm32_gpio_bank *bank, int offset, u32 mask, u32 value)
+{
+	struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
+	unsigned long flags;
+	int err = 0;
+
+	if (!bank->io_sync_control)
+		return -ENOTSUPP;
+
+	spin_lock_irqsave(&bank->lock, flags);
+
+	if (pctl->hwlock) {
+		err = hwspin_lock_timeout_in_atomic(pctl->hwlock, HWSPNLCK_TIMEOUT);
+		if (err) {
+			dev_err(pctl->dev, "Can't get hwspinlock\n");
+			goto unlock;
+		}
+	}
+
+	stm32_pconf_set_advcfgr_nolock(bank, offset, mask, value);
+
+	if (pctl->hwlock)
+		hwspin_unlock_in_atomic(pctl->hwlock);
+
+unlock:
+	spin_unlock_irqrestore(&bank->lock, flags);
+
+	return err;
+}
+
+static u32 stm32_pconf_get_advcfgr(struct stm32_gpio_bank *bank, int offset, u32 mask)
+{
+	int advcfgr_offset = STM32_GPIO_ADVCFGRL + (offset / 8) * 4;
+	int advcfgr_shift = (offset % 8) * 4;
+	u32 val;
+
+	if (!bank->io_sync_control)
+		return 0;
+
+	val = readl_relaxed(bank->base + advcfgr_offset);
+	val >>= advcfgr_shift;
+
+	return val & mask;
+}
+
+static int stm32_pconf_set_io_sync(struct stm32_gpio_bank *bank, int offset, u32 io_sync)
+{
+	if (io_sync >= ARRAY_SIZE(io_sync_2_advcfgr))
+		return -EINVAL;
+
+	return stm32_pconf_set_advcfgr(bank, offset, STM32_GPIO_ADVCFGR_IO_SYNC_MASK,
+		io_sync_2_advcfgr[io_sync]);
+}
+
+static const char *stm32_pconf_get_io_sync_str(struct stm32_gpio_bank *bank, int offset)
+{
+	u32 io_sync = stm32_pconf_get_advcfgr(bank, offset, STM32_GPIO_ADVCFGR_IO_SYNC_MASK);
+
+	if (io_sync & STM32_GPIO_ADVCFGR_RET_MASK) {
+		if (io_sync & STM32_GPIO_ADVCFGR_DE_MASK)
+			return "data GPIO re-sampled on both clock edges";
+
+		if (io_sync & STM32_GPIO_ADVCFGR_INVCLK_MASK)
+			return "data GPIO re-sampled on clock falling edge";
+
+		return "data GPIO re-sampled on clock rising edge";
+	}
+
+	if (io_sync & STM32_GPIO_ADVCFGR_INVCLK_MASK)
+		return "clock GPIO inverted";
+
+	return NULL;
+}
+
+static int
+stm32_pconf_set_skew_delay(struct stm32_gpio_bank *bank, int offset, u32 delay, bool is_dir_input)
+{
+	struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
+	int delay_offset = STM32_GPIO_DELAYRL + (offset / 8) * 4;
+	int delay_shift = (offset % 8) * 4;
+	unsigned long flags;
+	int err = 0;
+	u32 val;
+
+	if (!bank->io_sync_control)
+		return -ENOTSUPP;
+
+	spin_lock_irqsave(&bank->lock, flags);
+
+	if (pctl->hwlock) {
+		err = hwspin_lock_timeout_in_atomic(pctl->hwlock, HWSPNLCK_TIMEOUT);
+		if (err) {
+			dev_err(pctl->dev, "Can't get hwspinlock\n");
+			goto unlock;
+		}
+	}
+
+	val = readl_relaxed(bank->base + delay_offset);
+	val &= ~GENMASK(delay_shift + 3, delay_shift);
+	val |= (delay << delay_shift);
+	writel_relaxed(val, bank->base + delay_offset);
+
+	stm32_gpio_backup_skew_delay(bank, offset, delay);
+
+	stm32_pconf_set_advcfgr_nolock(bank, offset, STM32_GPIO_ADVCFGR_DLYPATH_MASK,
+				       is_dir_input ? STM32_GPIO_ADVCFGR_DLYPATH_MASK : 0);
+
+	if (pctl->hwlock)
+		hwspin_unlock_in_atomic(pctl->hwlock);
+
+unlock:
+	spin_unlock_irqrestore(&bank->lock, flags);
+
+	return err;
+}
+
+static u32 stm32_pconf_get_skew_delay_val(struct stm32_gpio_bank *bank, int offset)
+{
+	int delay_offset = STM32_GPIO_DELAYRL + (offset / 8) * 4;
+	int delay_shift = (offset % 8) * 4;
+	u32 val;
+
+	val = readl_relaxed(bank->base + delay_offset);
+	val &= GENMASK(delay_shift + 3, delay_shift);
+
+	return val >> delay_shift;
+}
+
+static const char *stm32_pconf_get_skew_dir_str(struct stm32_gpio_bank *bank, int offset)
+{
+	return stm32_pconf_get_advcfgr(bank, offset, STM32_GPIO_ADVCFGR_DLYPATH_MASK) ?
+		"input" : "output";
+}
+
 static bool stm32_pconf_get(struct stm32_gpio_bank *bank,
 	unsigned int offset, bool dir)
 {
@@ -1207,6 +1405,15 @@ static int stm32_pconf_parse_conf(struct pinctrl_dev *pctldev,
 		__stm32_gpio_set(bank, offset, arg);
 		ret = stm32_pmx_gpio_set_direction(pctldev, range, pin, false);
 		break;
+	case PIN_CONFIG_SKEW_DELAY_INPUT:
+		ret = stm32_pconf_set_skew_delay(bank, offset, arg, true);
+		break;
+	case PIN_CONFIG_SKEW_DELAY_OUTPUT:
+		ret = stm32_pconf_set_skew_delay(bank, offset, arg, false);
+		break;
+	case STM32_GPIO_PIN_CONFIG_IO_SYNC:
+		ret = stm32_pconf_set_io_sync(bank, offset, arg);
+		break;
 	default:
 		ret = -ENOTSUPP;
 	}
@@ -1349,6 +1556,22 @@ static void stm32_pconf_dbg_show(struct pinctrl_dev *pctldev,
 	case 3:
 		break;
 	}
+
+	if (bank->io_sync_control) {
+		const char *io_sync_str, *skew_dir_str;
+		u32 skew_delay;
+
+		io_sync_str = stm32_pconf_get_io_sync_str(bank, offset);
+		skew_dir_str = stm32_pconf_get_skew_dir_str(bank, offset);
+		skew_delay = stm32_pconf_get_skew_delay_val(bank, offset);
+
+		if (io_sync_str)
+			seq_printf(s, " - IO-sync: %s", io_sync_str);
+
+		if (skew_delay)
+			seq_printf(s, " - Skew-delay: %u (%u ps) %s", skew_delay, skew_delay * 250,
+				   skew_dir_str);
+	}
 }
 
 static const struct pinconf_ops stm32_pconf_ops = {
@@ -1441,6 +1664,7 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl, struct fwnode
 	bank->bank_nr = bank_nr;
 	bank->bank_ioport_nr = bank_ioport_nr;
 	bank->secure_control = pctl->match_data->secure_control;
+	bank->io_sync_control = pctl->match_data->io_sync_control;
 	bank->rif_control = pctl->match_data->rif_control;
 	spin_lock_init(&bank->lock);
 
@@ -1683,6 +1907,8 @@ int stm32_pctl_probe(struct platform_device *pdev)
 	pctl->pctl_desc.confops = &stm32_pconf_ops;
 	pctl->pctl_desc.pctlops = &stm32_pctrl_ops;
 	pctl->pctl_desc.pmxops = &stm32_pmx_ops;
+	pctl->pctl_desc.num_custom_params = ARRAY_SIZE(stm32_gpio_bindings);
+	pctl->pctl_desc.custom_params = stm32_gpio_bindings;
 	pctl->dev = &pdev->dev;
 
 	pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, &pctl->pctl_desc,
@@ -1804,6 +2030,21 @@ static int __maybe_unused stm32_pinctrl_restore_gpio_regs(
 	if (ret)
 		return ret;
 
+	if (bank->io_sync_control) {
+		bool is_input = bank->pin_backup[offset].advcfg & STM32_GPIO_ADVCFGR_DLYPATH_MASK;
+
+		ret = stm32_pconf_set_skew_delay(bank, offset,
+						 bank->pin_backup[offset].skew_delay,
+						 is_input);
+		if (ret)
+			return ret;
+
+		ret = stm32_pconf_set_advcfgr(bank, offset, STM32_GPIO_ADVCFGR_IO_SYNC_MASK,
+					      bank->pin_backup[offset].advcfg);
+		if (ret)
+			return ret;
+	}
+
 	if (pin_is_irq)
 		regmap_field_write(pctl->irqmux[offset], bank->bank_ioport_nr);
 
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.h b/drivers/pinctrl/stm32/pinctrl-stm32.h
index b98a4141bf2c0..d17cbdbba4482 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.h
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.h
@@ -64,6 +64,7 @@ struct stm32_pinctrl_match_data {
 	const struct stm32_desc_pin *pins;
 	const unsigned int npins;
 	bool secure_control;
+	bool io_sync_control;
 	bool rif_control;
 };
 
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32mp257.c b/drivers/pinctrl/stm32/pinctrl-stm32mp257.c
index d226de524bfc1..6709bddd97186 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32mp257.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32mp257.c
@@ -2543,6 +2543,7 @@ static const struct stm32_desc_pin stm32mp257_z_pins[] = {
 static struct stm32_pinctrl_match_data stm32mp257_match_data = {
 	.pins = stm32mp257_pins,
 	.npins = ARRAY_SIZE(stm32mp257_pins),
+	.io_sync_control = true,
 	.secure_control = true,
 	.rif_control = true,
 };
@@ -2550,6 +2551,7 @@ static struct stm32_pinctrl_match_data stm32mp257_match_data = {
 static struct stm32_pinctrl_match_data stm32mp257_z_match_data = {
 	.pins = stm32mp257_z_pins,
 	.npins = ARRAY_SIZE(stm32mp257_z_pins),
+	.io_sync_control = true,
 	.secure_control = true,
 	.rif_control = true,
 };
-- 
2.34.1

[PATCH v3 08/10] dt-bindings: pinctrl: stm32: Use properties from pincfg-node.yaml

[Antonio Borneo]

Don't re-declare the standard pincfg properties; take them from
the default schema.

Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 .../bindings/pinctrl/st,stm32-pinctrl.yaml    | 27 ++++++++-----------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
index 961161c2ab62b..2df141ed7222d 100644
--- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
@@ -151,6 +151,8 @@ patternProperties:
           pinctrl group available on the machine. Each subnode will list the
           pins it needs, and how they should be configured, with regard to muxer
           configuration, pullups, drive, output high/low and output speed.
+        $ref: /schemas/pinctrl/pincfg-node.yaml#
+
         properties:
           pinmux:
             $ref: /schemas/types.yaml#/definitions/uint32-array
@@ -195,26 +197,19 @@ patternProperties:
                           pinmux = <STM32_PINMUX('A', 9, RSVD)>;
                };
 
-          bias-disable:
-            type: boolean
+          bias-disable: true
 
-          bias-pull-down:
-            type: boolean
+          bias-pull-down: true
 
-          bias-pull-up:
-            type: boolean
+          bias-pull-up: true
 
-          drive-push-pull:
-            type: boolean
+          drive-push-pull: true
 
-          drive-open-drain:
-            type: boolean
+          drive-open-drain: true
 
-          output-low:
-            type: boolean
+          output-low: true
 
-          output-high:
-            type: boolean
+          output-high: true
 
           slew-rate:
             description: |
@@ -222,8 +217,8 @@ patternProperties:
               1: Medium speed
               2: Fast speed
               3: High speed
-            $ref: /schemas/types.yaml#/definitions/uint32
-            enum: [0, 1, 2, 3]
+            minimum: 0
+            maximum: 3
 
         required:
           - pinmux
-- 
2.34.1

[PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Antonio Borneo]

Document the support of the I/O synchronization parameters:
- skew-delay-input;
- skew-delay-output;
- st,io-sync.

Forbid 'skew-delay-input' and 'skew-delay-output' to be both
present on the same pin.
Allow the new properties only with compatibles that support them.
Add an example that uses the new properties.

Co-developed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
Signed-off-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 .../bindings/pinctrl/st,stm32-pinctrl.yaml    | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
index 2df141ed7222d..0010762127c05 100644
--- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
@@ -220,12 +220,89 @@ patternProperties:
             minimum: 0
             maximum: 3
 
+          skew-delay-input:
+            description: |
+              IO synchronization skew rate applied to the input path
+              0: No delay
+              1: Delay 0.30 ns
+              2: Delay 0.50 ns
+              3: Delay 0.75 ns
+              4: Delay 1.00 ns
+              5: Delay 1.25 ns
+              6: Delay 1.50 ns
+              7: Delay 1.75 ns
+              8: Delay 2.00 ns
+              9: Delay 2.25 ns
+              10: Delay 2.50 ns
+              11: Delay 2.75 ns
+              12: Delay 3.00 ns
+              13: Delay 3.25 ns
+            minimum: 0
+            maximum: 13
+
+          skew-delay-output:
+            description: |
+              IO synchronization latch delay applied to the output path
+              0: No delay
+              1: Delay 0.30 ns
+              2: Delay 0.50 ns
+              3: Delay 0.75 ns
+              4: Delay 1.00 ns
+              5: Delay 1.25 ns
+              6: Delay 1.50 ns
+              7: Delay 1.75 ns
+              8: Delay 2.00 ns
+              9: Delay 2.25 ns
+              10: Delay 2.50 ns
+              11: Delay 2.75 ns
+              12: Delay 3.00 ns
+              13: Delay 3.25 ns
+            minimum: 0
+            maximum: 13
+
+          st,io-sync:
+            description: |
+              IO synchronization through re-sampling or inversion
+              0: data or clock GPIO pass-through
+              1: clock GPIO inverted
+              2: data GPIO re-sampled on clock rising edge
+              3: data GPIO re-sampled on clock falling edge
+              4: data GPIO re-sampled on both clock edges
+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum: [0, 1, 2, 3, 4]
+
         required:
           - pinmux
 
+        # Not allowed both skew-delay-input and skew-delay-output
+        if:
+          required:
+            - skew-delay-input
+        then:
+          properties:
+            skew-delay-output: false
+
 allOf:
   - $ref: pinctrl.yaml#
 
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - st,stm32mp257-pinctrl
+                - st,stm32mp257-z-pinctrl
+    then:
+      patternProperties:
+        '-[0-9]*$':
+          patternProperties:
+            '^pins':
+              properties:
+                skew-delay-input: false
+                skew-delay-output: false
+                st,io-sync: false
+
 required:
   - compatible
   - '#address-cells'
@@ -306,4 +383,25 @@ examples:
                 pinctrl-names = "default";
     };
 
+  - |
+    #include <dt-bindings/pinctrl/stm32-pinfunc.h>
+    //Example 4 skew-delay and st,io-sync
+      pinctrl: pinctrl@44240000 {
+              compatible = "st,stm32mp257-pinctrl";
+              #address-cells = <1>;
+              #size-cells = <1>;
+              ranges = <0 0x44240000 0xa0400>;
+
+              eth3_rgmii_pins_a: eth3-rgmii-0 {
+                      pins1 {
+                              pinmux = <STM32_PINMUX('A', 6, AF14)>;
+                              st,io-sync = <4>;
+                      };
+                      pins2 {
+                              pinmux = <STM32_PINMUX('H', 2, AF14)>;
+                              skew-delay-output = <2>;
+                      };
+              };
+      };
+
 ...
-- 
2.34.1

[PATCH v3 10/10] arm64: dts: st: Add I/O sync to eth pinctrl in stm32mp25-pinctrl.dtsi

[Antonio Borneo]

On board stm32mp257f-ev1, the propagation delay between eth1/eth2
and the external PHY requires a compensation to guarantee that no
packet get lost in all the working conditions.

Add I/O synchronization properties in pinctrl on all the RGMII
data pins, activating re-sampling on both edges of the clock.

Co-developed-by: Christophe Roullier <christophe.roullier@foss.st.com>
Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
index e0d102eb61769..7906b3c585a2d 100644
--- a/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp25-pinctrl.dtsi
@@ -38,6 +38,7 @@ pins1 {
 			bias-disable;
 			drive-push-pull;
 			slew-rate = <3>;
+			st,io-sync = <4>;
 		};
 		pins2 {
 			pinmux = <STM32_PINMUX('H', 9, AF10)>, /* ETH_RGMII_CLK125 */
@@ -53,6 +54,7 @@ pins3 {
 				 <STM32_PINMUX('H', 13, AF10)>, /* ETH_RGMII_RXD3 */
 				 <STM32_PINMUX('A', 11, AF10)>; /* ETH_RGMII_RX_CTL */
 			bias-disable;
+			st,io-sync = <4>;
 		};
 		pins4 {
 			pinmux = <STM32_PINMUX('A', 14, AF10)>; /* ETH_RGMII_RX_CLK */
@@ -142,6 +144,7 @@ pins1 {
 			bias-disable;
 			drive-push-pull;
 			slew-rate = <3>;
+			st,io-sync = <4>;
 		};
 		pins2 {
 			pinmux = <STM32_PINMUX('F', 8, AF10)>, /* ETH_RGMII_CLK125 */
@@ -164,6 +167,7 @@ pins4 {
 				 <STM32_PINMUX('C', 11, AF10)>, /* ETH_RGMII_RXD3 */
 				 <STM32_PINMUX('C', 3, AF10)>; /* ETH_RGMII_RX_CTL */
 			bias-disable;
+			st,io-sync = <4>;
 		};
 		pins5 {
 			pinmux = <STM32_PINMUX('F', 6, AF10)>; /* ETH_RGMII_RX_CLK */
-- 
2.34.1

Re: [PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Conor Dooley]

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

On Tue, Oct 14, 2025 at 04:04:43PM +0200, Antonio Borneo wrote:
> Add the properties 'skew-delay-input' and 'skew-delay-output' to
> specify independent skew delay value for the two pin's directions.
> Make the new properties unavailable when the existing property
> 'skew-delay' is selected.
> 
> Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
> ---
>  .../bindings/pinctrl/pincfg-node.yaml         | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> index cbfcf215e571d..c3deb103d816b 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml
> @@ -153,4 +153,26 @@ properties:
>        pin. Typically indicates how many double-inverters are
>        used to delay the signal.
>  
> +  skew-delay-input:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      this affects the expected clock skew on input pins.
> +      Typically indicates how many double-inverters are used to
> +      delay the signal.

This property seems to be temporal, I would expect to see a unit of time
mentioned here, otherwise it'll totally inconsistent in use between
devices, and also a standard unit suffix in the property name.
pw-bot: changes-requested

> +
> +  skew-delay-output:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      this affects the expected delay before latching a value to an
> +      output pins. Typically indicates how many double-inverters are
> +      used to delay the signal.
> +
> +if:
> +  required:
> +    - skew-delay
> +then:
> +  properties:
> +    skew-delay-input: false
> +    skew-delay-output: false
> +
>  additionalProperties: true
> -- 
> 2.34.1
> 

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

Re: [PATCH v3 08/10] dt-bindings: pinctrl: stm32: Use properties from pincfg-node.yaml

[Conor Dooley]

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

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Conor Dooley]

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

On Tue, Oct 14, 2025 at 04:04:50PM +0200, Antonio Borneo wrote:
> Document the support of the I/O synchronization parameters:
> - skew-delay-input;
> - skew-delay-output;
> - st,io-sync.
> 
> Forbid 'skew-delay-input' and 'skew-delay-output' to be both
> present on the same pin.
> Allow the new properties only with compatibles that support them.
> Add an example that uses the new properties.
> 
> Co-developed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
> Signed-off-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
> Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
> ---
>  .../bindings/pinctrl/st,stm32-pinctrl.yaml    | 98 +++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> index 2df141ed7222d..0010762127c05 100644
> --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> @@ -220,12 +220,89 @@ patternProperties:
>              minimum: 0
>              maximum: 3
>  
> +          skew-delay-input:
> +            description: |
> +              IO synchronization skew rate applied to the input path
> +              0: No delay
> +              1: Delay 0.30 ns
> +              2: Delay 0.50 ns
> +              3: Delay 0.75 ns
> +              4: Delay 1.00 ns
> +              5: Delay 1.25 ns
> +              6: Delay 1.50 ns
> +              7: Delay 1.75 ns
> +              8: Delay 2.00 ns
> +              9: Delay 2.25 ns
> +              10: Delay 2.50 ns
> +              11: Delay 2.75 ns
> +              12: Delay 3.00 ns
> +              13: Delay 3.25 ns
> +            minimum: 0
> +            maximum: 13
> +
> +          skew-delay-output:
> +            description: |
> +              IO synchronization latch delay applied to the output path
> +              0: No delay
> +              1: Delay 0.30 ns
> +              2: Delay 0.50 ns
> +              3: Delay 0.75 ns
> +              4: Delay 1.00 ns
> +              5: Delay 1.25 ns
> +              6: Delay 1.50 ns
> +              7: Delay 1.75 ns
> +              8: Delay 2.00 ns
> +              9: Delay 2.25 ns
> +              10: Delay 2.50 ns
> +              11: Delay 2.75 ns
> +              12: Delay 3.00 ns
> +              13: Delay 3.25 ns
> +            minimum: 0
> +            maximum: 13

Same comments here as on the earlier patch. I would like to see times
used natively.
pw-bot: changes-requested

> +
> +          st,io-sync:
> +            description: |
> +              IO synchronization through re-sampling or inversion
> +              0: data or clock GPIO pass-through
> +              1: clock GPIO inverted
> +              2: data GPIO re-sampled on clock rising edge
> +              3: data GPIO re-sampled on clock falling edge
> +              4: data GPIO re-sampled on both clock edges
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3, 4]

I really don't like this kinds of properties that lead to "random"
numbers in devicetree. I'd much rather see a string list here.

> +
>          required:
>            - pinmux
>  
> +        # Not allowed both skew-delay-input and skew-delay-output
> +        if:
> +          required:
> +            - skew-delay-input
> +        then:
> +          properties:
> +            skew-delay-output: false
> +
>  allOf:
>    - $ref: pinctrl.yaml#
>  
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - st,stm32mp257-pinctrl
> +                - st,stm32mp257-z-pinctrl
> +    then:
> +      patternProperties:
> +        '-[0-9]*$':
> +          patternProperties:
> +            '^pins':
> +              properties:
> +                skew-delay-input: false
> +                skew-delay-output: false
> +                st,io-sync: false
> +
>  required:
>    - compatible
>    - '#address-cells'
> @@ -306,4 +383,25 @@ examples:
>                  pinctrl-names = "default";
>      };
>  
> +  - |
> +    #include <dt-bindings/pinctrl/stm32-pinfunc.h>
> +    //Example 4 skew-delay and st,io-sync
> +      pinctrl: pinctrl@44240000 {
> +              compatible = "st,stm32mp257-pinctrl";
> +              #address-cells = <1>;
> +              #size-cells = <1>;
> +              ranges = <0 0x44240000 0xa0400>;
> +
> +              eth3_rgmii_pins_a: eth3-rgmii-0 {
> +                      pins1 {
> +                              pinmux = <STM32_PINMUX('A', 6, AF14)>;
> +                              st,io-sync = <4>;
> +                      };
> +                      pins2 {
> +                              pinmux = <STM32_PINMUX('H', 2, AF14)>;
> +                              skew-delay-output = <2>;
> +                      };
> +              };
> +      };
> +
>  ...
> -- 
> 2.34.1
> 

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

Re: [PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Linus Walleij]

On Tue, Oct 14, 2025 at 8:04 PM Conor Dooley <conor@kernel.org> wrote:
> On Tue, Oct 14, 2025 at 04:04:43PM +0200, Antonio Borneo wrote:

> > +  skew-delay-input:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      this affects the expected clock skew on input pins.
> > +      Typically indicates how many double-inverters are used to
> > +      delay the signal.
>
> This property seems to be temporal, I would expect to see a unit of time
> mentioned here, otherwise it'll totally inconsistent in use between
> devices, and also a standard unit suffix in the property name.
> pw-bot: changes-requested

Don't blame the messenger, the existing property skew-delay
says:

  skew-delay:
    $ref: /schemas/types.yaml#/definitions/uint32
    description:
      this affects the expected clock skew on input pins
      and the delay before latching a value to an output
      pin. Typically indicates how many double-inverters are
      used to delay the signal.

This in turn comes from the original
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
document, which in turn comes from this commit:

commit e0e1e39de490a2d9b8a173363ccf2415ddada871
Author: Linus Walleij <linus.walleij@linaro.org>
Date:   Sat Oct 28 15:37:17 2017 +0200

    pinctrl: Add skew-delay pin config and bindings

    Some pin controllers (such as the Gemini) can control the
    expected clock skew and output delay on certain pins with a
    sub-nanosecond granularity. This is typically done by shunting
    in a number of double inverters in front of or behind the pin.
    Make it possible to configure this with a generic binding.

    Cc: devicetree@vger.kernel.org
    Acked-by: Rob Herring <robh@kernel.org>
    Acked-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

So by legacy skew-delay is a custom format, usually the number of
(double) inverters.

I don't recall the reason for this way of defining things, but one reason
could be that the skew-delay incurred by two inverters is very
dependent on the production node of the silicon, and can be
nanoseconds or picoseconds, these days mostly picoseconds.
Example: Documentation/devicetree/bindings/net/adi,adin.yaml

Antonio, what do you say? Do you have the actual skew picosecond
values for the different settings so we could define this as

skew-delay-input-ps:
skew-delay-output-ps:

and translate it to the right register values in the driver?

If we have the proper data this could be a good time to add this
ISO unit to these two props.

Yours,
Linus Walleij

Re: [PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Conor Dooley]

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

On Tue, Oct 14, 2025 at 09:33:14PM +0200, Linus Walleij wrote:
> On Tue, Oct 14, 2025 at 8:04 PM Conor Dooley <conor@kernel.org> wrote:
> > On Tue, Oct 14, 2025 at 04:04:43PM +0200, Antonio Borneo wrote:
> 
> > > +  skew-delay-input:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description:
> > > +      this affects the expected clock skew on input pins.
> > > +      Typically indicates how many double-inverters are used to
> > > +      delay the signal.
> >
> > This property seems to be temporal, I would expect to see a unit of time
> > mentioned here, otherwise it'll totally inconsistent in use between
> > devices, and also a standard unit suffix in the property name.
> > pw-bot: changes-requested
> 
> Don't blame the messenger, the existing property skew-delay
> says:
> 
>   skew-delay:
>     $ref: /schemas/types.yaml#/definitions/uint32
>     description:
>       this affects the expected clock skew on input pins
>       and the delay before latching a value to an output
>       pin. Typically indicates how many double-inverters are
>       used to delay the signal.
> 
> This in turn comes from the original
> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> document, which in turn comes from this commit:
> 
> commit e0e1e39de490a2d9b8a173363ccf2415ddada871
> Author: Linus Walleij <linus.walleij@linaro.org>
> Date:   Sat Oct 28 15:37:17 2017 +0200
> 
>     pinctrl: Add skew-delay pin config and bindings
> 
>     Some pin controllers (such as the Gemini) can control the
>     expected clock skew and output delay on certain pins with a
>     sub-nanosecond granularity. This is typically done by shunting
>     in a number of double inverters in front of or behind the pin.
>     Make it possible to configure this with a generic binding.
> 
>     Cc: devicetree@vger.kernel.org
>     Acked-by: Rob Herring <robh@kernel.org>
>     Acked-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
>     Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> So by legacy skew-delay is a custom format, usually the number of
> (double) inverters.

Yeah, I actually noticed this after sending the mail. But as you say
below, the new properties can be done with a unit etc

> 
> I don't recall the reason for this way of defining things, but one reason
> could be that the skew-delay incurred by two inverters is very
> dependent on the production node of the silicon, and can be
> nanoseconds or picoseconds, these days mostly picoseconds.
> Example: Documentation/devicetree/bindings/net/adi,adin.yaml
> 
> Antonio, what do you say? Do you have the actual skew picosecond
> values for the different settings so we could define this as
> 
> skew-delay-input-ps:
> skew-delay-output-ps:
> 
> and translate it to the right register values in the driver?

The patch for the specific binding does have values in units of seconds
assigned to each register value, so I think this should be doable.

> 
> If we have the proper data this could be a good time to add this
> ISO unit to these two props.
> 
> Yours,
> Linus Walleij

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

Re: [PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Antonio Borneo]

On Tue, 2025-10-14 at 20:39 +0100, Conor Dooley wrote:
> On Tue, Oct 14, 2025 at 09:33:14PM +0200, Linus Walleij wrote:
> > On Tue, Oct 14, 2025 at 8:04 PM Conor Dooley <conor@kernel.org> wrote:
> > > On Tue, Oct 14, 2025 at 04:04:43PM +0200, Antonio Borneo wrote:
> > 
> > > > +  skew-delay-input:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description:
> > > > +      this affects the expected clock skew on input pins.
> > > > +      Typically indicates how many double-inverters are used to
> > > > +      delay the signal.
> > > 
> > > This property seems to be temporal, I would expect to see a unit of time
> > > mentioned here, otherwise it'll totally inconsistent in use between
> > > devices, and also a standard unit suffix in the property name.
> > > pw-bot: changes-requested
> > 
> > Don't blame the messenger, the existing property skew-delay
> > says:
> > 
> >   skew-delay:
> >     $ref: /schemas/types.yaml#/definitions/uint32
> >     description:
> >       this affects the expected clock skew on input pins
> >       and the delay before latching a value to an output
> >       pin. Typically indicates how many double-inverters are
> >       used to delay the signal.
> > 
> > This in turn comes from the original
> > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > document, which in turn comes from this commit:
> > 
> > commit e0e1e39de490a2d9b8a173363ccf2415ddada871
> > Author: Linus Walleij <linus.walleij@linaro.org>
> > Date:   Sat Oct 28 15:37:17 2017 +0200
> > 
> >     pinctrl: Add skew-delay pin config and bindings
> > 
> >     Some pin controllers (such as the Gemini) can control the
> >     expected clock skew and output delay on certain pins with a
> >     sub-nanosecond granularity. This is typically done by shunting
> >     in a number of double inverters in front of or behind the pin.
> >     Make it possible to configure this with a generic binding.
> > 
> >     Cc: devicetree@vger.kernel.org
> >     Acked-by: Rob Herring <robh@kernel.org>
> >     Acked-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> >     Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> > So by legacy skew-delay is a custom format, usually the number of
> > (double) inverters.
> 
> Yeah, I actually noticed this after sending the mail. But as you say
> below, the new properties can be done with a unit etc

Quite an interesting discussion, here.

This skew delay is "implemented" inside the device through a set of
unitary delay cells (double inverters), where the value of the delay
of each cell can heavily depend on silicon process, temperature,
voltage, ...

When used to compensate skew "inside" the device, we can assume that
the skew of the signal to be compensated is also affected by process,
temperature, voltage, almost as the delay of each delay cell.
In this case it could be fine reporting the skew-delay as number of
delay cells because we don't really care about the value of delay in
time units.

But when used to compensate delay on the board, the value expressed
as time units is way more appropriate because it's what we get by the
board design.
And, this is the main use case in this series.

> 
> > 
> > I don't recall the reason for this way of defining things, but one reason
> > could be that the skew-delay incurred by two inverters is very
> > dependent on the production node of the silicon, and can be
> > nanoseconds or picoseconds, these days mostly picoseconds.
> > Example: Documentation/devicetree/bindings/net/adi,adin.yaml
> > 
> > Antonio, what do you say? Do you have the actual skew picosecond
> > values for the different settings so we could define this as
> > 
> > skew-delay-input-ps:
> > skew-delay-output-ps:
> > 
> > and translate it to the right register values in the driver?
> 
> The patch for the specific binding does have values in units of seconds
> assigned to each register value, so I think this should be doable.

While in this series I just kept the new properties uniform to
'skew-delay', I see no issue on using the '*-ps' version.
I will send a new version of the series.

What about the existing 'skew-delay'? Should it become deprecated in
favor of a new 'skew-delay-ps' ?

Thanks for the review!
Antonio

> 
> > 
> > If we have the proper data this could be a good time to add this
> > ISO unit to these two props.
> > 
> > Yours,
> > Linus Walleij

Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Antonio Borneo]

On Tue, 2025-10-14 at 19:10 +0100, Conor Dooley wrote:
> On Tue, Oct 14, 2025 at 04:04:50PM +0200, Antonio Borneo wrote:
> > Document the support of the I/O synchronization parameters:
> > - skew-delay-input;
> > - skew-delay-output;
> > - st,io-sync.
> > 
> > Forbid 'skew-delay-input' and 'skew-delay-output' to be both
> > present on the same pin.
> > Allow the new properties only with compatibles that support them.
> > Add an example that uses the new properties.
> > 
> > Co-developed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
> > Signed-off-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
> > Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
> > ---
> >  .../bindings/pinctrl/st,stm32-pinctrl.yaml    | 98 +++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > index 2df141ed7222d..0010762127c05 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > @@ -220,12 +220,89 @@ patternProperties:
> >              minimum: 0
> >              maximum: 3
> >  
> > +          skew-delay-input:
> > +            description: |
> > +              IO synchronization skew rate applied to the input path
> > +              0: No delay
> > +              1: Delay 0.30 ns
> > +              2: Delay 0.50 ns
> > +              3: Delay 0.75 ns
> > +              4: Delay 1.00 ns
> > +              5: Delay 1.25 ns
> > +              6: Delay 1.50 ns
> > +              7: Delay 1.75 ns
> > +              8: Delay 2.00 ns
> > +              9: Delay 2.25 ns
> > +              10: Delay 2.50 ns
> > +              11: Delay 2.75 ns
> > +              12: Delay 3.00 ns
> > +              13: Delay 3.25 ns
> > +            minimum: 0
> > +            maximum: 13
> > +
> > +          skew-delay-output:
> > +            description: |
> > +              IO synchronization latch delay applied to the output path
> > +              0: No delay
> > +              1: Delay 0.30 ns
> > +              2: Delay 0.50 ns
> > +              3: Delay 0.75 ns
> > +              4: Delay 1.00 ns
> > +              5: Delay 1.25 ns
> > +              6: Delay 1.50 ns
> > +              7: Delay 1.75 ns
> > +              8: Delay 2.00 ns
> > +              9: Delay 2.25 ns
> > +              10: Delay 2.50 ns
> > +              11: Delay 2.75 ns
> > +              12: Delay 3.00 ns
> > +              13: Delay 3.25 ns
> > +            minimum: 0
> > +            maximum: 13
> 
> Same comments here as on the earlier patch. I would like to see times
> used natively.

Yes, I replied to the earlier patch.

> pw-bot: changes-requested
> 
> > +
> > +          st,io-sync:
> > +            description: |
> > +              IO synchronization through re-sampling or inversion
> > +              0: data or clock GPIO pass-through
> > +              1: clock GPIO inverted
> > +              2: data GPIO re-sampled on clock rising edge
> > +              3: data GPIO re-sampled on clock falling edge
> > +              4: data GPIO re-sampled on both clock edges
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            enum: [0, 1, 2, 3, 4]
> 
> I really don't like this kinds of properties that lead to "random"
> numbers in devicetree. I'd much rather see a string list here.

Agree!
I just need to figure out some reasonably short but still meaningful
string for them.

Thanks for the review!
Antonio

> 
> > +
> >          required:
> >            - pinmux
> >  
> > +        # Not allowed both skew-delay-input and skew-delay-output
> > +        if:
> > +          required:
> > +            - skew-delay-input
> > +        then:
> > +          properties:
> > +            skew-delay-output: false
> > +
> >  allOf:
> >    - $ref: pinctrl.yaml#
> >  
> > +  - if:
> > +      not:
> > +        properties:
> > +          compatible:
> > +            contains:
> > +              enum:
> > +                - st,stm32mp257-pinctrl
> > +                - st,stm32mp257-z-pinctrl
> > +    then:
> > +      patternProperties:
> > +        '-[0-9]*$':
> > +          patternProperties:
> > +            '^pins':
> > +              properties:
> > +                skew-delay-input: false
> > +                skew-delay-output: false
> > +                st,io-sync: false
> > +
> >  required:
> >    - compatible
> >    - '#address-cells'
> > @@ -306,4 +383,25 @@ examples:
> >                  pinctrl-names = "default";
> >      };
> >  
> > +  - |
> > +    #include <dt-bindings/pinctrl/stm32-pinfunc.h>
> > +    //Example 4 skew-delay and st,io-sync
> > +      pinctrl: pinctrl@44240000 {
> > +              compatible = "st,stm32mp257-pinctrl";
> > +              #address-cells = <1>;
> > +              #size-cells = <1>;
> > +              ranges = <0 0x44240000 0xa0400>;
> > +
> > +              eth3_rgmii_pins_a: eth3-rgmii-0 {
> > +                      pins1 {
> > +                              pinmux = <STM32_PINMUX('A', 6, AF14)>;
> > +                              st,io-sync = <4>;
> > +                      };
> > +                      pins2 {
> > +                              pinmux = <STM32_PINMUX('H', 2, AF14)>;
> > +                              skew-delay-output = <2>;
> > +                      };
> > +              };
> > +      };
> > +
> >  ...
> > -- 
> > 2.34.1
> > 

Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Conor Dooley]

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

On Wed, Oct 15, 2025 at 02:56:56PM +0200, Antonio Borneo wrote:
> On Tue, 2025-10-14 at 19:10 +0100, Conor Dooley wrote:
> > On Tue, Oct 14, 2025 at 04:04:50PM +0200, Antonio Borneo wrote:

> > 
> > > +
> > > +          st,io-sync:
> > > +            description: |
> > > +              IO synchronization through re-sampling or inversion
> > > +              0: data or clock GPIO pass-through
> > > +              1: clock GPIO inverted
> > > +              2: data GPIO re-sampled on clock rising edge
> > > +              3: data GPIO re-sampled on clock falling edge
> > > +              4: data GPIO re-sampled on both clock edges
> > > +            $ref: /schemas/types.yaml#/definitions/uint32
> > > +            enum: [0, 1, 2, 3, 4]
> > 
> > I really don't like this kinds of properties that lead to "random"
> > numbers in devicetree. I'd much rather see a string list here.
> 
> Agree!
> I just need to figure out some reasonably short but still meaningful
> string for them.

pass-through
inverted
rising-edge
falling-edge
both-edges

perhaps?

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

Re: [PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Andrew Lunn]

> I don't recall the reason for this way of defining things, but one reason
> could be that the skew-delay incurred by two inverters is very
> dependent on the production node of the silicon, and can be
> nanoseconds or picoseconds, these days mostly picoseconds.
> Example: Documentation/devicetree/bindings/net/adi,adin.yaml


I'm missing the big picture here, and i don't see an example of these
properties being used. However, since you reference an old networking
example, for RGMII delays....

adi,rx-internal-delay-ps should be deprecated, we now have the generic
rx-internal-delay-ps. The point about using -ps is however still
valid.

However, i would not like to see pinctl DT properties used in place of
rx-internal-delay-ps. How the Ethernet MAC driver implements
rx-internal-delay-ps is left open, so calling a pinctl API to set the
skew is fine by me. And if the real use case has nothing to do with
networking, then i don't care.

	    Andrew

Re: [PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Linus Walleij]

On Wed, Oct 15, 2025 at 2:52 PM Antonio Borneo
<antonio.borneo@foss.st.com> wrote:

> What about the existing 'skew-delay'? Should it become deprecated in
> favor of a new 'skew-delay-ps' ?

Maybe it should be deprecated I didn't think of that.

I think there is no need for a new property to replace it,
your new properties can be used, just with input and output
skews set to the same value, so hardware that can only
control one skew knob should complain if the values
are different.

Yours,
Linus Walleij

Re: [PATCH v3 02/10] dt-bindings: pincfg-node: Add properties 'skew-delay-{in,out}put'

[Linus Walleij]

On Wed, Oct 15, 2025 at 6:37 PM Andrew Lunn <andrew@lunn.ch> wrote:

> > I don't recall the reason for this way of defining things, but one reason
> > could be that the skew-delay incurred by two inverters is very
> > dependent on the production node of the silicon, and can be
> > nanoseconds or picoseconds, these days mostly picoseconds.
> > Example: Documentation/devicetree/bindings/net/adi,adin.yaml
>
>
> I'm missing the big picture here, and i don't see an example of these
> properties being used. However, since you reference an old networking
> example, for RGMII delays....
>
> adi,rx-internal-delay-ps should be deprecated, we now have the generic
> rx-internal-delay-ps. The point about using -ps is however still
> valid.
>
> However, i would not like to see pinctl DT properties used in place of
> rx-internal-delay-ps. How the Ethernet MAC driver implements
> rx-internal-delay-ps is left open, so calling a pinctl API to set the
> skew is fine by me. And if the real use case has nothing to do with
> networking, then i don't care.

The scope here is to describe skewing the timing of any line
connected to a pin, no matter the purpose. Could be an MMC
card for example, or something else, but the point is that
the control registers are general and inside the SoC perimeter,
i.e. around the pins, not necessarily related to any specific
hardware block.

But I guess it could be used for a line used by some ethernet
interface.

But the config here happens on the pin controller, so a specific
hardware block distinct from e.g. an MMC controller or Ethernet
MAC, the latter just have their lines routed through it.

The pin controller will handle some pin named "TX", which is
just a string, a pin controller does not know what this means,
if it is a UART TX or a MAC TX but it can configure the skew
delay of the pin named like so.

Yours,
Linus Walleij

Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Antonio Borneo]

On Wed, 2025-10-15 at 15:35 +0100, Conor Dooley wrote:
> On Wed, Oct 15, 2025 at 02:56:56PM +0200, Antonio Borneo wrote:
> > On Tue, 2025-10-14 at 19:10 +0100, Conor Dooley wrote:
> > > On Tue, Oct 14, 2025 at 04:04:50PM +0200, Antonio Borneo wrote:
> 
> > > 
> > > > +
> > > > +          st,io-sync:
> > > > +            description: |
> > > > +              IO synchronization through re-sampling or inversion
> > > > +              0: data or clock GPIO pass-through
> > > > +              1: clock GPIO inverted
> > > > +              2: data GPIO re-sampled on clock rising edge
> > > > +              3: data GPIO re-sampled on clock falling edge
> > > > +              4: data GPIO re-sampled on both clock edges
> > > > +            $ref: /schemas/types.yaml#/definitions/uint32
> > > > +            enum: [0, 1, 2, 3, 4]
> > > 
> > > I really don't like this kinds of properties that lead to "random"
> > > numbers in devicetree. I'd much rather see a string list here.
> > 
> > Agree!
> > I just need to figure out some reasonably short but still meaningful
> > string for them.
> 
> pass-through
> inverted
> rising-edge
> falling-edge
> both-edges
> 
> perhaps?

Since these are strings in a custom property, I think I will use something longer and more explicit for them.

Linus,

pinconf-generic only accepts positive numeric values for both generic and custom properties in struct pinconf_generic_params.
Plus, I haven't found any existing driver that mixes pinconf-generic with custom string values.
I'm going to extend the current pinconf-generic to handle such case.
Or did I missed something?

Regards,
Antonio

Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Linus Walleij]

On Mon, Oct 20, 2025 at 5:09 PM Antonio Borneo
<antonio.borneo@foss.st.com> wrote:

> pinconf-generic only accepts positive numeric values for
> both generic and custom properties in struct pinconf_generic_params.

Do you need it to support negative values?
Patches welcome!

> Plus, I haven't found any existing driver that mixes pinconf-generic with
> custom string values.

Maybe I misunderstand, but pinconf_generic_parse_dt_config()
looks at  pctldev->desc->custom_params and
pctldev->desc->num_custom_params found in
struct pinctrl_desc in
include/linux/pinctrl/pinctrl.h

$ git grep custom_params drivers/pinctrl/
gives you a list of all drivers using this.

Yours,
Linus Walleij

Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Antonio Borneo]

On Tue, 2025-10-21 at 00:08 +0200, Linus Walleij wrote:
> On Mon, Oct 20, 2025 at 5:09 PM Antonio Borneo
> <antonio.borneo@foss.st.com> wrote:
> 
> > pinconf-generic only accepts positive numeric values for
> > both generic and custom properties in struct pinconf_generic_params.
> 
> Do you need it to support negative values?

No, the point is not the sign, but the value that has to be numeric. More details below.

> Patches welcome!
> 
> > Plus, I haven't found any existing driver that mixes pinconf-generic with
> > custom string values.
> 
> Maybe I misunderstand, but pinconf_generic_parse_dt_config()
> looks at  pctldev->desc->custom_params and
> pctldev->desc->num_custom_params found in
> struct pinctrl_desc in
> include/linux/pinctrl/pinctrl.h

The issue is that parse_dt_cfg(), called by the above mentioned pinconf_generic_parse_dt_config(), only uses of_property_read_u32() to get the value of the property.
Conor's proposal for replacing my
st,io-sync = <0>;
with
st,io-sync = "pass-through";
doesn't fit in!

For my use case I'm going to extend parse_dt_cfg() with fwnode_property_match_property_string() to extract the index from an array of strings.
Then such index would be used for pinconf_to_config_packed().
Does this approach look reasonable?

I will send out the series shortly, after some test.

Regards,
Antonio

Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

[Linus Walleij]

On Tue, Oct 21, 2025 at 1:49 PM Antonio Borneo
<antonio.borneo@foss.st.com> wrote:

> The issue is that parse_dt_cfg(), called by the above mentioned pinconf_generic_parse_dt_config(),
> only uses of_property_read_u32() to get the value of the property.
> Conor's proposal for replacing my
> st,io-sync = <0>;
> with
> st,io-sync = "pass-through";
> doesn't fit in!
>
> For my use case I'm going to extend parse_dt_cfg() with fwnode_property_match_property_string() to extract the index from an array of strings.
> Then such index would be used for pinconf_to_config_packed().
> Does this approach look reasonable?

Aha I see the issue.

Hard for me to tell how this will play out since you are the first to try this,
but it seems like this could work.

Let's see how the code looks!

Yours,
Linus Walleij