Re: [PATCH 1/2] arm64: dts: ti: k3-am67a-beagley-ai: Add gpio ranges

[David Lechner <dlechner@baylibre.com>]

Hi Tony,

Mostly a question for you since you are the pinctrl-single dt-bindings
maintainer.


On 7/28/26 2:57 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Adding `pinctrl-single,gpio-range` on K3 platforms causes dynamic GPIO requests to silently overwrite the entire pad configuration register, destroying previously applied board-specific settings like pull-ups and drive strengths.
> --
> 
> --- Patch [1]: [PATCH 1/2] arm64: dts: ti: k3-am67a-beagley-ai: Add gpio ranges ---
> commit c5d1a1cd5d0bb91d0c680ae90328952c44500103
> Author: David Lechner (TI) <dlechner@baylibre.com>
> 
> arm64: dts: ti: k3-am67a-beagley-ai: Add gpio ranges
> 
> Add pinctrl-single,gpio-range properties to the BeagleY-AI .dts so that
> GPIOs get the correct pin mux when requested.
> 
>> diff --git a/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts b/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts
>> index 5255e04b9ac76..599d576f694f6 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am67a-beagley-ai.dts
>> @@ -149,6 +149,19 @@ led-1 {
>>  };
>>  
>>  &main_pmx0 {
>> +	pinctrl-single,gpio-range =
>> +		<&main_pmx0_range 0 32 PIN_GPIO_RANGE_IOPAD>,
>> +		<&main_pmx0_range 33 38 PIN_GPIO_RANGE_IOPAD>,
> 
> [Severity: High]
> Does adding pinctrl-single,gpio-range here cause dynamic GPIO requests to
> silently overwrite the entire pad configuration register?
> 
> On K3 platforms, the pinctrl-single,function-mask is typically <0xffffffff>,

Is pinctrl-single,function-mask supposed to just contain the mask for the
mux bits (function) rather than all config bits?

The DT binding description is vague:

	Mask of the allowed register bits

Does it mean all bits in the register even not related to function or all
bits related only to function?

Looking at the code, it seems like it should just be the mux bits (function)
since pinctrl-single,function-mask is stored in pcs->fmask and pcs->fmask is
only used by pcs_set_mux() and pcs_request_gpio().


> meaning the pad electrical properties and mux mode share the same register.
> When a GPIO in these ranges is requested via gpiod_get(), the gpiolib path
> calls the pinctrl driver's pcs_request_gpio().
> 
> Since the function mask is 0xffffffff, pcs_request_gpio() will execute:
> 
>     data &= ~pcs->fmask;
>     data |= frange->gpiofunc;
> 
> Could this completely erase any previously applied board-specific settings
> like pull-ups or drive strengths from a device's pinctrl-0 property, and
> replace them with PIN_GPIO_RANGE_IOPAD (which includes PULL_DISABLE)?
> 
> [ ... ]
> 
>> @@ -267,6 +280,15 @@ &main_uart0 {
>>  };
>>  
>>  &mcu_pmx0 {
>> +	pinctrl-single,gpio-range =
>> +		<&mcu_pmx_range 0 21 PIN_GPIO_RANGE_IOPAD>,
>> +		<&mcu_pmx_range 23 1 PIN_GPIO_RANGE_IOPAD>,
> 
> [Severity: High]
> Will this have the same pad configuration overwrite issue for the MCU domain
> GPIOs when requested?
> 

So in order for pinctrl-single,gpio-range to actually work correctly, we would
need to make this fix first.

---
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi
index f130c7cb998d..cae21cf92ca7 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi
@@ -280,7 +280,7 @@ main_pmx0: pinctrl@f4000 {
 		reg = <0x00 0xf4000 0x00 0x2b0>;
 		#pinctrl-cells = <1>;
 		pinctrl-single,register-width = <32>;
-		pinctrl-single,function-mask = <0xffffffff>;
+		pinctrl-single,function-mask = <0xf>;
 		bootph-all;
 	};
 
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
index 5288c959f3c1..1aa73526bba4 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
@@ -11,7 +11,7 @@ mcu_pmx0: pinctrl@4084000 {
 		reg = <0x00 0x04084000 0x00 0x88>;
 		#pinctrl-cells = <1>;
 		pinctrl-single,register-width = <32>;
-		pinctrl-single,function-mask = <0xffffffff>;
+		pinctrl-single,function-mask = <0xf>;
 		bootph-all;
 	};
---

And then we would want this change as well...

---
diff --git a/arch/arm64/boot/dts/ti/k3-pinctrl.h b/arch/arm64/boot/dts/ti/k3-pinctrl.h
index 4491898d8294..29e401f16bc9 100644
--- a/arch/arm64/boot/dts/ti/k3-pinctrl.h
+++ b/arch/arm64/boot/dts/ti/k3-pinctrl.h
@@ -112,7 +112,7 @@
 #define PIN_WKUP_EN			(WKUP_ENABLE | WKUP_ON_EDGE)
 
 /* Default mux configuration for gpio-ranges to use with pinctrl */
-#define PIN_GPIO_RANGE_IOPAD	(PIN_INPUT | 7)
+#define PIN_GPIO_RANGE_IOPAD	(7)
 
 #define AM62AX_IOPAD(pa, val, muxmode)		(((pa) & 0x1fff)) ((val) | (muxmode))
 #define AM62AX_MCU_IOPAD(pa, val, muxmode)	(((pa) & 0x1fff)) ((val) | (muxmode))
---