| Message ID | 20260930092020.1047221-2-y-abhilashchandra@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add pin controller support for TI TDA54 SoC | expand |
Hi Yemike, thanks for your patch! On Wed, Sep 30, 2026 at 11:21 AM Yemike Abhilash Chandra <y-abhilashchandra@ti.com> wrote: > + ti,debounce-select: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3, 4, 5, 6] > + description: > + Selects which DBOUNCE_CFGn period register in the control module > + drives the debounce filter for this pad. 0 disables debouncing, > + 1 to 6 select DBOUNCE_CFG1 to DBOUNCE_CFG6. What's wrong with the existing input-debounce property? input-debounce: $ref: /schemas/types.yaml#/definitions/uint32-array description: Takes the debounce time in usec as argument or 0 to disable debouncing Just translate usec:s into your custom format in the code, problem solved. > + ti,virt-gpio-instance: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3, 4, 5, 6, 7] > + description: > + Selects which virtual GPIO instance controls this pad, allowing > + protection between multiple virtual views of the GPIO control > + registers. Has no effect unless the pad is muxed to GPIO mode > + (muxmode 7). Wow crazy stuff. OK keep it :D > + ti,wakeup: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2, 3] > + description: | > + Wakeup event configuration for this pad. > + 0 - wakeup disabled > + 1 - wakeup triggered by any change of the pin input value > + 2 - wakeup triggered by a low value on the pin > + 3 - wakeup triggered by a high value on the pin I just have the feeling this should be a generic property. It seems so useful. Can you just add this as wakeup-mode = <custom value> in Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml > + ti,retention-bias: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2] > + description: | > + Enables the internal I/O pullup/pulldown resistor when pin enters > + TDA54 I/O retention mode, and configures the I/O pull resistor > + direction. > + 0 - OFF mode pad pull resistor disable > + 1 - Select OFF mode pulldown resistor > + 2 - Select OFF mode pullup resistor Use names inspired by the generic config types and flags instead of enums. ti,retention-bias-disable; ti,retention-bias-pull-down; ti,retention-bias-pull-up; > + ti,retention-offmode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1] > + description: | > + I/O behaviour when retention mode is active. > + 0 - I/O maintains its previous state > + 1 - I/O state is forced to the OFF mode value Behaviour of *what*? The driver stage? I suspect you should make two bool flags ti,retention-output-hold; ti,retention-output-disable; > + ti,retention-output: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2] > + description: | > + Output driver behaviour while the pad is in retention mode. > + 0 - output driver disabled > + 1 - output driver enabled, pad driven low > + 2 - output driver enabled, pad driven high Make three flags: ti,retention-output-disable; ti,retention-output-low; ti,retention-output-high; > + ti,retention-force: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1] > + description: | > + I/O retention controls. > + 0 - gated by the Device Manager logic > + 1 - forced active, overriding the Device Manager gating logic This is clearly a bool flag. It should contain device-manager as that magic entity is involved. ti,retention-device-manager-enable; ti,retention-device-manager-forced-active; Both seems to be related to the device manager whatever that is. > + ti,isolation-bypass: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1] > + description: | > + I/O isolation for this pad. > + 0 - isolation preserved > + 1 - isolation bypassed What does this even mean electronically speaking? Explain in a description: statement. ti,isolation-preserve; ti.isolation-bypass-enable; perhaps? Some of the retention settings seem *very* generic, c.f. this from include/dt-bindings/pinctrl/nomadik.h that has been around forever: #define SLPM_DISABLED 0 #define SLPM_ENABLED 1 #define SLPM_INPUT_NOPULL 0 #define SLPM_INPUT_PULLUP 1 #define SLPM_INPUT_PULLDOWN 2 #define SLPM_DIR_INPUT 3 #define SLPM_OUTPUT_LOW 0 #define SLPM_OUTPUT_HIGH 1 #define SLPM_DIR_OUTPUT 2 #define SLPM_WAKEUP_DISABLE 0 #define SLPM_WAKEUP_ENABLE 1 SLPM means "sleep mode", yeah pretty much retention... I think the corresponding retention settings for things that are really quite generic should just be added to the generic pin config bindings. Yours, Linus Walleij
On Thu, Oct 01, 2026 at 11:10:34PM +0200, Linus Walleij wrote: > Hi Yemike, > > thanks for your patch! > > On Wed, Sep 30, 2026 at 11:21 AM Yemike Abhilash Chandra > <y-abhilashchandra@ti.com> wrote: > > > + ti,debounce-select: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2, 3, 4, 5, 6] > > + description: > > + Selects which DBOUNCE_CFGn period register in the control module > > + drives the debounce filter for this pad. 0 disables debouncing, > > + 1 to 6 select DBOUNCE_CFG1 to DBOUNCE_CFG6. > > What's wrong with the existing input-debounce property? > > input-debounce: > $ref: /schemas/types.yaml#/definitions/uint32-array > description: Takes the debounce time in usec as argument or 0 to disable > debouncing > > Just translate usec:s into your custom format in the code, problem solved. > > > + ti,virt-gpio-instance: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2, 3, 4, 5, 6, 7] > > + description: > > + Selects which virtual GPIO instance controls this pad, allowing > > + protection between multiple virtual views of the GPIO control > > + registers. Has no effect unless the pad is muxed to GPIO mode > > + (muxmode 7). > > Wow crazy stuff. OK keep it :D > > > + ti,wakeup: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2, 3] > > + description: | > > + Wakeup event configuration for this pad. > > + 0 - wakeup disabled > > + 1 - wakeup triggered by any change of the pin input value > > + 2 - wakeup triggered by a low value on the pin > > + 3 - wakeup triggered by a high value on the pin > > I just have the feeling this should be a generic property. It seems so useful. > > Can you just add this as wakeup-mode = <custom value> in > Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml And I think it should probably be strings, so that there's some hope of commonality, since obviously any user is going to invent different mappings. > > > + ti,retention-bias: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2] > > + description: | > > + Enables the internal I/O pullup/pulldown resistor when pin enters > > + TDA54 I/O retention mode, and configures the I/O pull resistor > > + direction. > > + 0 - OFF mode pad pull resistor disable > > + 1 - Select OFF mode pulldown resistor > > + 2 - Select OFF mode pullup resistor > > Use names inspired by the generic config types and flags instead > of enums. > > ti,retention-bias-disable; > ti,retention-bias-pull-down; > ti,retention-bias-pull-up; > > > + ti,retention-offmode: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > + description: | > > + I/O behaviour when retention mode is active. > > + 0 - I/O maintains its previous state > > + 1 - I/O state is forced to the OFF mode value > > Behaviour of *what*? > > The driver stage? > > I suspect you should make two bool flags > ti,retention-output-hold; > ti,retention-output-disable; > > > + ti,retention-output: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2] > > + description: | > > + Output driver behaviour while the pad is in retention mode. > > + 0 - output driver disabled > > + 1 - output driver enabled, pad driven low > > + 2 - output driver enabled, pad driven high > > Make three flags: > ti,retention-output-disable; > ti,retention-output-low; > ti,retention-output-high; > > > + ti,retention-force: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > + description: | > > + I/O retention controls. > > + 0 - gated by the Device Manager logic > > + 1 - forced active, overriding the Device Manager gating logic > > This is clearly a bool flag. It should contain device-manager as that > magic entity is involved. > > ti,retention-device-manager-enable; > ti,retention-device-manager-forced-active; > > Both seems to be related to the device manager whatever that is. > > > + ti,isolation-bypass: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > + description: | > > + I/O isolation for this pad. > > + 0 - isolation preserved > > + 1 - isolation bypassed > > What does this even mean electronically speaking? Explain in a description: > statement. > > ti,isolation-preserve; > ti.isolation-bypass-enable; Is this not just an either or, so there should only be one property here. > > perhaps? > > Some of the retention settings seem *very* generic, c.f. this > from include/dt-bindings/pinctrl/nomadik.h that has been > around forever: > > #define SLPM_DISABLED 0 > #define SLPM_ENABLED 1 > #define SLPM_INPUT_NOPULL 0 > #define SLPM_INPUT_PULLUP 1 > #define SLPM_INPUT_PULLDOWN 2 > #define SLPM_DIR_INPUT 3 > #define SLPM_OUTPUT_LOW 0 > #define SLPM_OUTPUT_HIGH 1 > #define SLPM_DIR_OUTPUT 2 > #define SLPM_WAKEUP_DISABLE 0 > #define SLPM_WAKEUP_ENABLE 1 > > SLPM means "sleep mode", yeah pretty much retention... > > I think the corresponding retention settings for things that are really > quite generic should just be added to the generic pin config bindings. Additionally, if any of the custom properties are gonna be kept as numerical values, they're all missing defaults. In the case where you're advocating for multiple boolean properties, one of them is gonna have to be required as otherwise we're looking at indeterminate states based on reset values or what a prior stage may or may not have done? The reset values should probably be considered default, but the cleanest way to make this work with separate booleans is have a oneOf: - required: - a - required: - b - required: - c and you've also got to deal with people potentially making mistakes and adding multiple properties for the same feature or trying to overwrite a $som.dtsi/$soc-common.dtsi config and not realising that they've got to do a /delete-property/ on the other property for the feature.
diff --git a/Documentation/devicetree/bindings/pinctrl/ti,tda54-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/ti,tda54-pinctrl.yaml new file mode 100644 index 000000000000..4660470b7613 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/ti,tda54-pinctrl.yaml @@ -0,0 +1,209 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/ti,tda54-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI TDA54 SoC pin controller + +maintainers: + - Yemike Abhilash Chandra <y-abhilashchandra@ti.com> + +description: | + Pinmux and pinconf controller in the TI TDA54 SoC. There are two instances, + one in the MAIN domain and one in the MCU domain. + + The configuration of each pad is split across two registers: + + PADCFGA mux mode and functional pin configuration + PADCFGB wakeup and retention configuration + +properties: + compatible: + const: ti,tda54-pinctrl + + reg: + maxItems: 1 + description: + The register space is divided into two proxy addressing regions, each + covering the same registers. This covers the entire register space, + including both proxy addressing regions. + + ti,npins: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + description: + Number of pins on this instance. + + ti,proxy-id: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + default: 0 + description: + Proxy 0 is the default access path and can write any register + that proxy 1 has not claimed. A register claimed through proxy 1 + becomes read-only at proxy 0 and can only be written through proxy 1. + This selects the proxy region to use. + +allOf: + - $ref: pinctrl.yaml# + +required: + - compatible + - reg + - ti,npins + +patternProperties: + '-pins$': + type: object + allOf: + - $ref: pinmux-node.yaml# + - $ref: pincfg-node.yaml# + + additionalProperties: false + + description: + Pin configuration node describing one or more pads, their mux mode and + the configuration to apply. + + properties: + pinmux: + description: + Values are constructed from pad number and mux mode, pad + number is left shifted by 8 bits, then ORed with mux mode + + bias-disable: true + bias-pull-up: true + bias-pull-down: true + input-enable: true + input-disable: true + output-enable: true + output-disable: true + input-schmitt-enable: true + input-schmitt-disable: true + + slew-rate: + description: + I/O slew rate. 0 = slew control disabled, 1 = mild, 2 = moderate, + 3 = high. + enum: [0, 1, 2, 3] + + ti,debounce-select: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3, 4, 5, 6] + description: + Selects which DBOUNCE_CFGn period register in the control module + drives the debounce filter for this pad. 0 disables debouncing, + 1 to 6 select DBOUNCE_CFG1 to DBOUNCE_CFG6. + + ti,virt-gpio-instance: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3, 4, 5, 6, 7] + description: + Selects which virtual GPIO instance controls this pad, allowing + protection between multiple virtual views of the GPIO control + registers. Has no effect unless the pad is muxed to GPIO mode + (muxmode 7). + + ti,wakeup: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + description: | + Wakeup event configuration for this pad. + 0 - wakeup disabled + 1 - wakeup triggered by any change of the pin input value + 2 - wakeup triggered by a low value on the pin + 3 - wakeup triggered by a high value on the pin + + ti,retention-bias: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2] + description: | + Enables the internal I/O pullup/pulldown resistor when pin enters + TDA54 I/O retention mode, and configures the I/O pull resistor + direction. + 0 - OFF mode pad pull resistor disable + 1 - Select OFF mode pulldown resistor + 2 - Select OFF mode pullup resistor + + ti,retention-offmode: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + description: | + I/O behaviour when retention mode is active. + 0 - I/O maintains its previous state + 1 - I/O state is forced to the OFF mode value + + ti,retention-output: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2] + description: | + Output driver behaviour while the pad is in retention mode. + 0 - output driver disabled + 1 - output driver enabled, pad driven low + 2 - output driver enabled, pad driven high + + ti,retention-force: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + description: | + I/O retention controls. + 0 - gated by the Device Manager logic + 1 - forced active, overriding the Device Manager gating logic + + ti,isolation-bypass: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + description: | + I/O isolation for this pad. + 0 - isolation preserved + 1 - isolation bypassed + + required: + - pinmux + +additionalProperties: false + +examples: + - | + #define TDA54_PINMUX(pad, mux) (((pad) << 8) | ((mux) & 0xf)) + #define TDA54_MUX_MODE0 0 + #define TDA54_MUX_MODE7 7 + + pinctrl@5e200000 { + compatible = "ti,tda54-pinctrl"; + reg = <0x5e200000 0x8000>; + ti,npins = <161>; + ti,proxy-id = <1>; + + mcu-uart0-rx-pins { + pinmux = <TDA54_PINMUX(58, TDA54_MUX_MODE0)>, + <TDA54_PINMUX(60, TDA54_MUX_MODE0)>; + input-enable; + bias-disable; + }; + + mcu-uart0-tx-pins { + pinmux = <TDA54_PINMUX(59, TDA54_MUX_MODE0)>, + <TDA54_PINMUX(61, TDA54_MUX_MODE0)>; + output-enable; + bias-disable; + }; + + mcu-gpio0-button-pins { + pinmux = <TDA54_PINMUX(42, TDA54_MUX_MODE7)>; + input-enable; + bias-pull-up; + ti,debounce-select = <2>; + ti,virt-gpio-instance = <1>; + ti,wakeup = <2>; + }; + + mcu-i2c0-pins { + pinmux = <TDA54_PINMUX(0, TDA54_MUX_MODE0)>, + <TDA54_PINMUX(1, TDA54_MUX_MODE0)>; + input-enable; + bias-pull-up; + ti,retention-bias = <2>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 1c02b919d6ae..60aa8483301e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -27865,6 +27865,12 @@ L: linux-sound@vger.kernel.org S: Odd Fixes F: sound/soc/codecs/tas571x* +TI TDA54 PIN CONTROLLER +M: Yemike Abhilash Chandra <y-abhilashchandra@ti.com> +L: linux-gpio@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/pinctrl/ti,tda54-pinctrl.yaml + TI TMAG5273 MAGNETOMETER DRIVER M: Gerald Loacker <gerald.loacker@wolfvision.net> L: linux-iio@vger.kernel.org
Document the device tree bindings for the pin controller found in the TI TDA54 SoC. The TDA54 pin controller splits the pad configuration across two registers: PADCFGA holds the mux mode and the functional pin configuration, PADCFGB holds the wakeup and retention configuration. The controller also supports two proxy addressing regions and claiming of registers. A register claimed through proxy 1 becomes read-only at proxy 0 and can only be written through proxy 1. Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com> --- .../bindings/pinctrl/ti,tda54-pinctrl.yaml | 209 ++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 215 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,tda54-pinctrl.yaml