AM654: Add pinmux support
Vignesh R
maintainercover AM654: Add pinmux support
Add pinmux definitions and pinctrl regions for AM654 SoCs
Tero Kristo (2):
dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions
arm64: dts: ti: k3-am65: Add pinctrl regions
MAINTAINERS | 1 + arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 16 +++++++ arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 8 ++++ arch/arm64/boot/dts/ti/k3-am65.dtsi | 1 + include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++ 5 files changed, 75 insertions(+) create mode 100644 include/dt-bindings/pinctrl/k3-am6.h
Signature · 1 line
2.19.1
Vignesh R
maintainerPATCH 1/2 dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions
From: Tero Kristo <[email protected]>
The dt-bindings header for TI K3-AM6 SoCs define a set of macros for defining pinmux configs in human readable form, instead of raw-coded hex values.
--- MAINTAINERS | 1 + include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 include/dt-bindings/pinctrl/k3-am6.h
MAINTAINERS +1 -0
@@ -2204,6 +2204,7 @@ S: Supported F: Documentation/devicetree/bindings/arm/ti/k3.txt F: arch/arm64/boot/dts/ti/Makefile F: arch/arm64/boot/dts/ti/k3-* +F: include/dt-bindings/pinctrl/k3-am6.h ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE M: Santosh Shilimkar <[email protected]>
include/dt-bindings/pinctrl/k3-am6.h new file +49 -0
49 lines not on this page · raw message
Signature · 1 line
2.19.1
Nishanth Menon
maintainerPATCH 1/2 dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions
include/dt-bindings/pinctrl/k3-am6.h
Quoted 32 lines · Vignesh R
From: Tero Kristo <[email protected]>
The dt-bindings header for TI K3-AM6 SoCs define a set of macros for defining pinmux configs in human readable form, instead of raw-coded hex values.
Signed-off-by Tero Kristo <[email protected]>Signed-off-by Lokesh Vutla <[email protected]>Signed-off-by Vignesh R <[email protected]>--- MAINTAINERS | 1 + include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 include/dt-bindings/pinctrl/k3-am6.hMAINTAINERS +1
-0@@ -2204,6 +2204,7 @@ S: Supported F: Documentation/devicetree/bindings/arm/ti/k3.txt F: arch/arm64/boot/dts/ti/Makefile F: arch/arm64/boot/dts/ti/k3-* +F: include/dt-bindings/pinctrl/k3-am6.h ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE M: Santosh Shilimkar <[email protected]>include/dt-bindings/pinctrl/k3-am6.h new file +0
-0
Are we thinking of creating headers for every single SoC?
Is it possible for us to reduce the number of headers we'd need?
include/dt-bindings/pinctrl/k3-am6.h line 5
Quoted 6 lines
include/dt-bindings/pinctrl/k3-am6.h new file +5
-0@@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* This header provides constants for TI K3-AM6 pinctrl bindings. +* +* Copyright (C) 2018 Texas Instruments
Please fix this:
Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
Quoted 5 lines
include/dt-bindings/pinctrl/k3-am6.h new file +5
-0+*/ +#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H +#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H + +/* K3 mux mode options for each pin. See TRM for options */
include/dt-bindings/pinctrl/k3-am6.h line 20
Quoted 10 lines
include/dt-bindings/pinctrl/k3-am6.h new file +10
-0+#define MUX_MODE0 0 +#define MUX_MODE1 1 +#define MUX_MODE2 2 +#define MUX_MODE3 3 +#define MUX_MODE4 4 +#define MUX_MODE5 5 +#define MUX_MODE6 6 +#define MUX_MODE7 7 +#define MUX_MODE15 15 +
bit 15?
include/dt-bindings/pinctrl/k3-am6.h line 21
Quoted 1 line
include/dt-bindings/pinctrl/k3-am6.h new file +1
-0+#define PULL_DISABLE (1 << 16)
PULLUDEN -> this bit enables the Pull
include/dt-bindings/pinctrl/k3-am6.h line 22
Quoted 1 line
include/dt-bindings/pinctrl/k3-am6.h new file +1
-0+#define PULL_UP (1 << 17)
This is a pull direction selection bit.
so, should these be:
#define PULLUDEN_SHIFT (16)
#define PULLTYPESEL_SHIFT (17)
#define RXACTIVE_SHIFT (18)
#define PULL_DISABLE (1 << PULLUDEN_SHIFT)
#define PULL_ENABLE (0 << PULLUDEN_SHIFT)
#define PULL_UP (1 << PULLTYPESEL_SHIFT | PULL_ENABLE) #define PULL_DOWN (0 << PULLTYPESEL_SHIFT | PULL_ENABLE)
include/dt-bindings/pinctrl/k3-am6.h line 23
Quoted 1 line
include/dt-bindings/pinctrl/k3-am6.h new file +1
-0+#define INPUT_EN (1 << 18)
#define INPUT_EN (1 << RXACTIVE_SHIFT)
These names dont seem to match up to the http://www.ti.com/lit/pdf/spruid7
This might be readable when people look up trm and try to generate the pinmux setup.
include/dt-bindings/pinctrl/k3-am6.h line 28
Quoted 6 lines
include/dt-bindings/pinctrl/k3-am6.h new file +5
-0+#define SLEWCTRL_200MHZ 0 +#define SLEWCTRL_150MHZ (1 << 19) +#define SLEWCTRL_100MHZ (2 << 19) +#define SLEWCTRL_50MHZ (3 << 19) +#define TX_DIS (1 << 21)
What does this mean? Transmit disable? or output disable?
include/dt-bindings/pinctrl/k3-am6.h line 32
Quoted 4 lines
include/dt-bindings/pinctrl/k3-am6.h new file +4
-0+#define ISO_OVR (1 << 22) +#define ISO_BYPASS (1 << 23) +#define DS_EN (1 << 24) +#define DS_INPUT (1 << 25)
Seems to be DSOUT_DIS not the same as input -> in deepsleep, there is no input - this bit simply means that you are driving an output or not during deep sleep?
Quoted 1 line
+#define
DS_FORCE_OUT_HIGH(1 << 26)
you need a FORCE_OUT_LOW as well to be readable.
include/dt-bindings/pinctrl/k3-am6.h line 37
Quoted 4 lines
include/dt-bindings/pinctrl/k3-am6.h new file +4
-0+#define DS_PULL_UP_DOWN_EN 0 +#define DS_PULL_UP_DOWN_DIS (1 << 27) +#define DS_PULL_UP_SEL (1 << 28) +#define WAKEUP_ENABLE (1 << 29)
I do have a similar set of confusion here as well.
include/dt-bindings/pinctrl/k3-am6.h line 44
Quoted 7 lines
include/dt-bindings/pinctrl/k3-am6.h new file +7
-0+ +#define PIN_OUTPUT (PULL_DISABLE) +#define PIN_OUTPUT_PULLUP (PULL_UP) +#define PIN_OUTPUT_PULLDOWN 0 +#define PIN_INPUT (INPUT_EN | PULL_DISABLE) +#define PIN_INPUT_PULLUP (INPUT_EN | PULL_UP) +#define PIN_INPUT_PULLDOWN (INPUT_EN)
Not exactly sure of the approach taken here -> it seems to be a mix of implicit macro -> there needs to be some level of consistency and guidance to dts folks as to which macros are to be used and what not.
will be nice if the number of macros are minimal and is clearly documented as to what macros are expected to be used in dts.
Quoted 8 lines
include/dt-bindings/pinctrl/k3-am6.h new file +5
-0+ +#define AM65X_IOPAD (pa, val) (((pa) & 0x1fff)) (val) +#define AM65X_WKUP_IOPAD (pa, val) (((pa) & 0x1fff)) (val) + +#endif Signature · 1 line
2.19.1
Signature · 2 lines
Regards, Nishanth Menon
Vignesh R
maintainerPATCH 1/2 dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions
Quoted 26 lines · Nishanth Menon
Quoted 25 lines · Vignesh R
From: Tero Kristo <[email protected]>
The dt-bindings header for TI K3-AM6 SoCs define a set of macros for defining pinmux configs in human readable form, instead of raw-coded hex values.
Signed-off-by Tero Kristo <[email protected]>Signed-off-by Lokesh Vutla <[email protected]>Signed-off-by Vignesh R <[email protected]>--- MAINTAINERS | 1 + include/dt-bindings/pinctrl/k3-am6.h | 49 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 include/dt-bindings/pinctrl/k3-am6.hMAINTAINERS +1
-0@@ -2204,6 +2204,7 @@ S: Supported F: Documentation/devicetree/bindings/arm/ti/k3.txt F: arch/arm64/boot/dts/ti/Makefile F: arch/arm64/boot/dts/ti/k3-* +F: include/dt-bindings/pinctrl/k3-am6.h
Quoted 9 lines
include/dt-bindings/pinctrl/k3-am6.h
Quoted 7 lines
MAINTAINERS +0
-0ARM/TEXAS INSTRUMENT KEYSTONE ARCHITECTURE M: Santosh Shilimkar <[email protected]>include/dt-bindings/pinctrl/k3-am6.h new file +0
-0Are we thinking of creating headers for every single SoC?
We would need one file per SoC family (i.e atleast one for K3 family as a whole) as pinctrl register layout is significantly different than other TI SoCs. I can rename the file to include/dt-bindings/pinctrl/k3.h to indicate its intended to be common for all K3 SoCs, if you prefer.
Quoted 13 lines
Is it possible for us to reduce the number of headers we'd need?
include/dt-bindings/pinctrl/k3-am6.h line 5
Quoted 6 lines
include/dt-bindings/pinctrl/k3-am6.h new file +5
-0@@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* This header provides constants for TI K3-AM6 pinctrl bindings. +* +* Copyright (C) 2018 Texas Instruments Please fix this:
Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
Done.
Quoted 8 lines
Quoted 8 lines
include/dt-bindings/pinctrl/k3-am6.h new file +8
-0+*/ +#ifndef _DT_BINDINGS_PINCTRL_TI_K3_AM6_H +#define _DT_BINDINGS_PINCTRL_TI_K3_AM6_H + +/* K3 mux mode options for each pin. See TRM for options */ +#define MUX_MODE0 0 +#define MUX_MODE1 1 +#define MUX_MODE2 2
Quoted 9 lines
include/dt-bindings/pinctrl/k3-am6.h line 20
Quoted 7 lines
include/dt-bindings/pinctrl/k3-am6.h new file +7
-0+#define MUX_MODE3 3 +#define MUX_MODE4 4 +#define MUX_MODE5 5 +#define MUX_MODE6 6 +#define MUX_MODE7 7 +#define MUX_MODE15 15 +bit 15?
Nope its 0xF. Anyway, dropping these macros as suggested by Tony on Patch 2/2
Quoted 16 lines
include/dt-bindings/pinctrl/k3-am6.h line 21
Quoted 1 line
include/dt-bindings/pinctrl/k3-am6.h new file +1
-0+#define PULL_DISABLE (1 << 16)PULLUDEN -> this bit enables the Pull
include/dt-bindings/pinctrl/k3-am6.h line 22
Quoted 1 line
include/dt-bindings/pinctrl/k3-am6.h new file +1
-0+#define PULL_UP (1 << 17)This is a pull direction selection bit.
so, should these be:
#definePULLUDEN_SHIFT(16)
#definePULLTYPESEL_SHIFT(17)
#defineRXACTIVE_SHIFT(18)#define
PULL_DISABLE(1 <<PULLUDEN_SHIFT)
#definePULL_ENABLE(0 <<PULLUDEN_SHIFT)#define
PULL_UP(1 <<PULLTYPESEL_SHIFT|PULL_ENABLE) #definePULL_DOWN(0 <<PULLTYPESEL_SHIFT|PULL_ENABLE)
Quoted 8 lines
include/dt-bindings/pinctrl/k3-am6.h line 23
Quoted 1 line
include/dt-bindings/pinctrl/k3-am6.h new file +1
-0+#define INPUT_EN (1 << 18)#define
INPUT_EN(1 <<RXACTIVE_SHIFT)These names dont seem to match up to the http://www.ti.com/lit/pdf/spruid7
This might be readable when people look up trm and try to generate the pinmux setup.
Ok, will update as you suggested above.
Quoted 16 lines
include/dt-bindings/pinctrl/k3-am6.h line 28
Quoted 6 lines
include/dt-bindings/pinctrl/k3-am6.h new file +5
-0+#define SLEWCTRL_200MHZ 0 +#define SLEWCTRL_150MHZ (1 << 19) +#define SLEWCTRL_100MHZ (2 << 19) +#define SLEWCTRL_50MHZ (3 << 19) +#define TX_DIS (1 << 21)What does this mean? Transmit disable? or output disable?
include/dt-bindings/pinctrl/k3-am6.h line 32
Quoted 4 lines
include/dt-bindings/pinctrl/k3-am6.h new file +4
-0+#define ISO_OVR (1 << 22) +#define ISO_BYPASS (1 << 23) +#define DS_EN (1 << 24) +#define DS_INPUT (1 << 25)Seems to be
DSOUT_DISnot the same as input -> in deepsleep, there is no input - this bit simply means that you are driving an output or not during deep sleep?
Quoted 8 lines
Quoted 1 line
+#define
DS_FORCE_OUT_HIGH(1 << 26)you need a
FORCE_OUT_LOWas well to be readable.include/dt-bindings/pinctrl/k3-am6.h line 37
Quoted 4 lines
include/dt-bindings/pinctrl/k3-am6.h new file +4
-0+#define DS_PULL_UP_DOWN_EN 0 +#define DS_PULL_UP_DOWN_DIS (1 << 27) +#define DS_PULL_UP_SEL (1 << 28) +#define WAKEUP_ENABLE (1 << 29)I do have a similar set of confusion here as well.
Dropping all the above from v2 as we wont be needing them ATM.
Quoted 4 lines
Quoted 4 lines
include/dt-bindings/pinctrl/k3-am6.h new file +4
-0+ +#define PIN_OUTPUT (PULL_DISABLE) +#define PIN_OUTPUT_PULLUP (PULL_UP) +#define PIN_OUTPUT_PULLDOWN 0
Quoted 9 lines
include/dt-bindings/pinctrl/k3-am6.h line 44
Quoted 3 lines
include/dt-bindings/pinctrl/k3-am6.h new file +3
-0+#define PIN_INPUT (INPUT_EN | PULL_DISABLE) +#define PIN_INPUT_PULLUP (INPUT_EN | PULL_UP) +#define PIN_INPUT_PULLDOWN (INPUT_EN)Not exactly sure of the approach taken here -> it seems to be a mix of implicit macro -> there needs to be some level of consistency and guidance to dts folks as to which macros are to be used and what not.
Will fix that.
Quoted 3 lines
will be nice if the number of macros are minimal and is clearly documented as to what macros are expected to be used in dts.
I will come up with minimal set needed for now and add a comment as to what macros are intended to be used in DT.
Quoted 9 lines
Quoted 8 lines
include/dt-bindings/pinctrl/k3-am6.h new file +5
-0+ +#define AM65X_IOPAD (pa, val) (((pa) & 0x1fff)) (val) +#define AM65X_WKUP_IOPAD (pa, val) (((pa) & 0x1fff)) (val) + +#endif Signature · 1 line
2.19.1
Signature · 2 lines
Regards Vignesh
Nishanth Menon
maintainerPATCH 1/2 dt-bindings: pinctrl: k3-am6: Introduce pinmux definitions
Quoted 6 lines · Vignesh R
Quoted 3 lines
Quoted 1 line
+++
b/include/dt-bindings/pinctrl/k3-am6.hAre we thinking of creating headers for every single SoC?
We would need one file per SoC family (i.e atleast one for K3 family as a whole) as pinctrl register layout is significantly different than
Yes, I am aware of the same.
Quoted 2 lines
other TI SoCs. I can rename the file to
include/dt-bindings/pinctrl/k3.hto indicate its intended to be common for all K3 SoCs, if you prefer.
Yes, that would be the preference. There are two types of IO muxing IPs (the equivalent of padconf from omap days) that I
am aware of. However these are all within the same 32bit definitions and I know there has been significant effort spend by designers based on software team feedbacks to maintain consistency as much as possible within technological constraints.
So macros could be used to differentiate within the same header. The deltas do exist, however they can be differentiated easily.
You should have access to the alternative solution, and even if you cannot publish it, you should plan the header to scale easily when the time is appropriate as a trivial additional patch.
For the rest.. Will wait for v2.
Signature · 2 lines
Regards, Nishanth Menon
Vignesh R
maintainerPATCH 2/2 arm64: dts: ti: k3-am65: Add pinctrl regions
In reply to Vignesh RFrom: Tero Kristo <[email protected]>
Add pinctrl regions for the main and wkup mmr.
The range for main pinctrl region contains a gap at offset 0x2e4, and because of this, the pinctrl range is split into two sections.
--- arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 16 ++++++++++++++++ arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 8 ++++++++ arch/arm64/boot/dts/ti/k3-am65.dtsi | 1 + 3 files changed, 25 insertions(+)
arch/arm64/boot/dts/ti/k3-am65-main.dtsi +16 -0
@@ -69,4 +69,20 @@ clock-frequency = <48000000>; current-speed = <115200>; }; + + main_pmx0: pinmux at 11c000 { + compatible = "pinctrl-single"; + reg = <0x0 0x11c000 0x0 0x2e4>; + #pinctrl-cells = <1>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <0xffffffff>; + }; + + main_pmx1: pinmux at 11c2e8 { + compatible = "pinctrl-single"; + reg = <0x0 0x11c2e8 0x0 0x24>; + #pinctrl-cells = <1>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <0xffffffff>; + }; };
arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi +8 -0
@@ -34,6 +34,14 @@ }; }; + wkup_pmx0: pinmux at 4301c000 { + compatible = "pinctrl-single"; + reg = <0x4301c000 0x118>; + #pinctrl-cells = <1>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <0xffffffff>; + }; + wkup_uart0: serial at 42300000 { compatible = "ti,am654-uart"; reg = <0x42300000 0x100>;
arch/arm64/boot/dts/ti/k3-am65.dtsi +1 -0
@@ -8,6 +8,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/pinctrl/k3-am65.h> / { model = "Texas Instruments K3 AM654 SoC";
Signature · 1 line
2.19.1
Nishanth Menon
maintainerPATCH 2/2 arm64: dts: ti: k3-am65: Add pinctrl regions
Quoted 15 lines · Vignesh R
arch/arm64/boot/dts/ti/k3-am65.dtsi +1
-0@@ -8,6 +8,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/pinctrl/k3-am65.h> / { model = "Texas Instruments K3 AM654 SoC";Signature · 1 line
2.19.1
Where is this used? Could we mux the uart console that we are using? will be good to see an example of usage.
Signature · 2 lines
Regards, Nishanth Menon
PATCH 2/2 arm64: dts: ti: k3-am65: Add pinctrl regions
Quoted 11 lines · Nishanth Menon
Quoted 10 lines · Vignesh R
arch/arm64/boot/dts/ti/k3-am65.dtsi +1
-0@@ -8,6 +8,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/pinctrl/k3-am65.h>
Quoted 8 lines
Quoted 5 lines
arch/arm64/boot/dts/ti/k3-am65.dtsi +0
-0/ { model = "Texas Instruments K3 AM654 SoC";Signature · 1 line
2.19.1Where is this used? Could we mux the uart console that we are using? will be good to see an example of usage.
Yes please.
And please add the SoC specific macro for the pins in a format that makes the pinconf and pinmux options separate parameters for the macro where the usage becomes something like:
AM65X_IOPAD(0x1234, PIN_INPUT_PULLUP, 0)
The above 0 is just the mux mode number in the example. So no more need to use MUX_MODE0 define to redefine a number when set up that way :)
This will allow us to eventually take advantage of the #pinctrl-cells = <2> and not have to change all the dts files and just change the macro.
I also added Linus Walleij to Cc as this might interest him also.
Regards,
Tony