2022-05-31 07:04:39

by Rahul T R

[permalink] [raw]
Subject: [PATCH v4 0/3] Enable RPi header on j721e sk

The following series of patches enables RPi header
on j721e sk. It is a 40 pin io expasion header which
brings out i2c5, ehrpwm 2,3 and some pins of gpio 0,1

v4:
- Correct the node name in dt binding example

v3:
- Change node name from clock to clock-controller
- Add correct description for clock-controller node

v2:
- Add full path for clock property $ref
- Remove the discription for clock pattern property,
since $ref is added
- Remove the label in the example
- Fix the indentation in the example

Rahul T R (1):
dt-bindings: mfd: ti,j721e-system-controller: Add clock property

Sinthu Raja (1):
arm64: dts: ti: k3-j721e-sk: Add pinmux for RPi Header

Vijay Pothukuchi (1):
arm64: dts: ti: k3-j721e-*: Add dts nodes for EHRPWMs

.../mfd/ti,j721e-system-controller.yaml | 12 +++
.../dts/ti/k3-j721e-common-proc-board.dts | 24 +++++
arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 62 ++++++++++-
arch/arm64/boot/dts/ti/k3-j721e-sk.dts | 101 +++++++++++++++++-
4 files changed, 193 insertions(+), 6 deletions(-)

--
2.17.1



2022-05-31 12:26:55

by Rahul T R

[permalink] [raw]
Subject: [PATCH v4 3/3] arm64: dts: ti: k3-j721e-sk: Add pinmux for RPi Header

From: Sinthu Raja <[email protected]>

Add pinmux required to bring out
i2c5, ehrpwm 2 and 3 and gpios on
40 pin RPi header on sk board

Signed-off-by: Sinthu Raja <[email protected]>
Signed-off-by: Rahul T R <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j721e-sk.dts | 89 ++++++++++++++++++++++----
1 file changed, 78 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
index 98a55778f3fe..b913b18ae133 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
@@ -400,6 +400,57 @@
J721E_IOPAD(0x124, PIN_INPUT, 7) /* (Y24) PRG0_PRU1_GPO9.GPIO0_72 */
>;
};
+
+ main_i2c5_pins_default: main-i2c5-pins-default {
+ pinctrl-single,pins = <
+ J721E_IOPAD(0x150, PIN_INPUT_PULLUP, 2) /* (Y26) PRG0_MDIO0_MDIO.I2C5_SCL */
+ J721E_IOPAD(0x154, PIN_INPUT_PULLUP, 2) /* (AA27) PRG0_MDIO0_MDC.I2C5_SDA */
+ >;
+ };
+
+ rpi_header_gpio0_pins_default: rpi-header-gpio0-pins-default {
+ pinctrl-single,pins = <
+ J721E_IOPAD(0x01c, PIN_INPUT, 7) /* (AD22) PRG1_PRU0_GPO6.GPIO0_7 */
+ J721E_IOPAD(0x120, PIN_INPUT, 7) /* (AA28) PRG0_PRU1_GPO8.GPIO0_71 */
+ J721E_IOPAD(0x14c, PIN_INPUT, 7) /* (AA29) PRG0_PRU1_GPO19.GPIO0_82 */
+ J721E_IOPAD(0x02c, PIN_INPUT, 7) /* (AD21) PRG1_PRU0_GPO10.GPIO0_11 */
+ J721E_IOPAD(0x198, PIN_INPUT, 7) /* (V25) RGMII6_TD1.GPIO0_101 */
+ J721E_IOPAD(0x1b0, PIN_INPUT, 7) /* (W24) RGMII6_RD1.GPIO0_107 */
+ J721E_IOPAD(0x1a0, PIN_INPUT, 7) /* (W29) RGMII6_TXC.GPIO0_103 */
+ J721E_IOPAD(0x008, PIN_INPUT, 7) /* (AG22) PRG1_PRU0_GPO1.GPIO0_2 */
+ J721E_IOPAD(0x1d0, PIN_INPUT, 7) /* (AA3) SPI0_D1.GPIO0_115 */
+ J721E_IOPAD(0x11c, PIN_INPUT, 7) /* (AA24) PRG0_PRU1_GPO7.GPIO0_70 */
+ J721E_IOPAD(0x148, PIN_INPUT, 7) /* (AA26) PRG0_PRU1_GPO18.GPIO0_81 */
+ J721E_IOPAD(0x004, PIN_INPUT, 7) /* (AC23) PRG1_PRU0_GPO0.GPIO0_1 */
+ J721E_IOPAD(0x014, PIN_INPUT, 7) /* (AH23) PRG1_PRU0_GPO4.GPIO0_5 */
+ J721E_IOPAD(0x020, PIN_INPUT, 7) /* (AE20) PRG1_PRU0_GPO7.GPIO0_8 */
+ J721E_IOPAD(0x19c, PIN_INPUT, 7) /* (W27) RGMII6_TD0.GPIO0_102 */
+ J721E_IOPAD(0x1b4, PIN_INPUT, 7) /* (W25) RGMII6_RD0.GPIO0_108 */
+ J721E_IOPAD(0x188, PIN_INPUT, 7) /* (Y28) RGMII6_TX_CTL.GPIO0_97 */
+ J721E_IOPAD(0x00c, PIN_INPUT, 7) /* (AF22) PRG1_PRU0_GPO2.GPIO0_3 */
+ J721E_IOPAD(0x010, PIN_INPUT, 7) /* (AJ23) PRG1_PRU0_GPO3.GPIO0_4 */
+ >;
+ };
+
+ rpi_header_gpio1_pins_default: rpi-header-gpio1-pins-default {
+ pinctrl-single,pins = <
+ J721E_IOPAD(0x234, PIN_INPUT, 7) /* (U3) EXT_REFCLK1.GPIO1_12 */
+ >;
+ };
+
+ rpi_header_ehrpwm2_pins_default: rpi-header-ehrpwm2-pins-default {
+ pinctrl-single,pins = <
+ J721E_IOPAD(0x178, PIN_INPUT, 6) /* (U27) RGMII5_RD3.EHRPWM2_A */
+ J721E_IOPAD(0x17c, PIN_INPUT, 6) /* (U24) RGMII5_RD2.EHRPWM2_B */
+ >;
+ };
+
+ rpi_header_ehrpwm3_pins_default: rpi-header-ehrpwm3-pins-default {
+ pinctrl-single,pins = <
+ J721E_IOPAD(0x18c, PIN_INPUT, 6) /* (V23) RGMII6_RX_CTL.EHRPWM3_A */
+ J721E_IOPAD(0x190, PIN_INPUT, 6) /* (W23) RGMII6_TD3.EHRPWM3_B */
+ >;
+ };
};

&wkup_pmx0 {
@@ -631,11 +682,6 @@
status = "disabled";
};

-&main_i2c5 {
- /* Brought out on RPi Header */
- status = "disabled";
-};
-
&main_i2c6 {
/* Unused */
status = "disabled";
@@ -1138,18 +1184,39 @@
status = "disabled";
};

-&main_ehrpwm2 {
+&main_ehrpwm4 {
status = "disabled";
};

-&main_ehrpwm3 {
+&main_ehrpwm5 {
status = "disabled";
};

-&main_ehrpwm4 {
- status = "disabled";
+&main_gpio0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rpi_header_gpio0_pins_default>;
};

-&main_ehrpwm5 {
- status = "disabled";
+&main_gpio1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rpi_header_gpio1_pins_default>;
+};
+
+&main_i2c5 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c5_pins_default>;
+ clock-frequency = <400000>;
+ status = "okay";
+};
+
+&main_ehrpwm2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rpi_header_ehrpwm2_pins_default>;
+ status = "okay";
+};
+
+&main_ehrpwm3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rpi_header_ehrpwm3_pins_default>;
+ status = "okay";
};
--
2.17.1


2022-06-18 02:32:42

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-j721e-sk: Add pinmux for RPi Header

On 15:40-20220530, Rahul T R wrote:
> From: Sinthu Raja <[email protected]>
>
> Add pinmux required to bring out
> i2c5, ehrpwm 2 and 3 and gpios on
> 40 pin RPi header on sk board
>
> Signed-off-by: Sinthu Raja <[email protected]>
> Signed-off-by: Rahul T R <[email protected]>


I was digging deeper at https://www.ti.com/lit/zip/sprr438
(PROC112E2(001)_SCH.pdf - J3, Also looking at
https://github.com/beagleboard/beaglebone-ai-64/blob/master/BeagleBone%20AI%20-64_SCH_V1.02_211119.pdf
(P8, P9)

And comparing it to https://www.raspberrypi-spy.co.uk/2012/06/simple-guide-to-the-rpi-gpio-header-and-pins/
And considering potential use such as https://pypi.org/project/RPi.GPIO/
variation,

Here is my suggestion (applies to other TI Boards that attempt to
emulate RPI header)
a) Default mux in board.dts should be GPIO except for the i2c used for
ID detection.
b) Secondary functions should be a dt overlay. (These can easily enable
the pwms and other functions as needed)
c) Maintain node names consistent to allow reuse of overlays across
platforms.

Usage: you can either use extlinux.conf OR uEnv.txt to apply the
overlays as desired (ID detection from hats might help automate it based
on the bootloader you'd want to use)

Else, you have created a custom configuration here for 1 specific
application, various hats that expect GPIO will end up croaking.


I am open to discussions here.

> ---
> arch/arm64/boot/dts/ti/k3-j721e-sk.dts | 89 ++++++++++++++++++++++----
> 1 file changed, 78 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> index 98a55778f3fe..b913b18ae133 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> @@ -400,6 +400,57 @@
> J721E_IOPAD(0x124, PIN_INPUT, 7) /* (Y24) PRG0_PRU1_GPO9.GPIO0_72 */
> >;
> };
> +
> + main_i2c5_pins_default: main-i2c5-pins-default {
> + pinctrl-single,pins = <
> + J721E_IOPAD(0x150, PIN_INPUT_PULLUP, 2) /* (Y26) PRG0_MDIO0_MDIO.I2C5_SCL */
> + J721E_IOPAD(0x154, PIN_INPUT_PULLUP, 2) /* (AA27) PRG0_MDIO0_MDC.I2C5_SDA */
> + >;
> + };
> +
> + rpi_header_gpio0_pins_default: rpi-header-gpio0-pins-default {
> + pinctrl-single,pins = <
> + J721E_IOPAD(0x01c, PIN_INPUT, 7) /* (AD22) PRG1_PRU0_GPO6.GPIO0_7 */
> + J721E_IOPAD(0x120, PIN_INPUT, 7) /* (AA28) PRG0_PRU1_GPO8.GPIO0_71 */
> + J721E_IOPAD(0x14c, PIN_INPUT, 7) /* (AA29) PRG0_PRU1_GPO19.GPIO0_82 */
> + J721E_IOPAD(0x02c, PIN_INPUT, 7) /* (AD21) PRG1_PRU0_GPO10.GPIO0_11 */
> + J721E_IOPAD(0x198, PIN_INPUT, 7) /* (V25) RGMII6_TD1.GPIO0_101 */
> + J721E_IOPAD(0x1b0, PIN_INPUT, 7) /* (W24) RGMII6_RD1.GPIO0_107 */
> + J721E_IOPAD(0x1a0, PIN_INPUT, 7) /* (W29) RGMII6_TXC.GPIO0_103 */
> + J721E_IOPAD(0x008, PIN_INPUT, 7) /* (AG22) PRG1_PRU0_GPO1.GPIO0_2 */
> + J721E_IOPAD(0x1d0, PIN_INPUT, 7) /* (AA3) SPI0_D1.GPIO0_115 */
> + J721E_IOPAD(0x11c, PIN_INPUT, 7) /* (AA24) PRG0_PRU1_GPO7.GPIO0_70 */
> + J721E_IOPAD(0x148, PIN_INPUT, 7) /* (AA26) PRG0_PRU1_GPO18.GPIO0_81 */
> + J721E_IOPAD(0x004, PIN_INPUT, 7) /* (AC23) PRG1_PRU0_GPO0.GPIO0_1 */
> + J721E_IOPAD(0x014, PIN_INPUT, 7) /* (AH23) PRG1_PRU0_GPO4.GPIO0_5 */
> + J721E_IOPAD(0x020, PIN_INPUT, 7) /* (AE20) PRG1_PRU0_GPO7.GPIO0_8 */
> + J721E_IOPAD(0x19c, PIN_INPUT, 7) /* (W27) RGMII6_TD0.GPIO0_102 */
> + J721E_IOPAD(0x1b4, PIN_INPUT, 7) /* (W25) RGMII6_RD0.GPIO0_108 */
> + J721E_IOPAD(0x188, PIN_INPUT, 7) /* (Y28) RGMII6_TX_CTL.GPIO0_97 */
> + J721E_IOPAD(0x00c, PIN_INPUT, 7) /* (AF22) PRG1_PRU0_GPO2.GPIO0_3 */
> + J721E_IOPAD(0x010, PIN_INPUT, 7) /* (AJ23) PRG1_PRU0_GPO3.GPIO0_4 */
> + >;
> + };
> +
> + rpi_header_gpio1_pins_default: rpi-header-gpio1-pins-default {
> + pinctrl-single,pins = <
> + J721E_IOPAD(0x234, PIN_INPUT, 7) /* (U3) EXT_REFCLK1.GPIO1_12 */
> + >;
> + };
> +
> + rpi_header_ehrpwm2_pins_default: rpi-header-ehrpwm2-pins-default {
> + pinctrl-single,pins = <
> + J721E_IOPAD(0x178, PIN_INPUT, 6) /* (U27) RGMII5_RD3.EHRPWM2_A */
> + J721E_IOPAD(0x17c, PIN_INPUT, 6) /* (U24) RGMII5_RD2.EHRPWM2_B */
> + >;
> + };
> +
> + rpi_header_ehrpwm3_pins_default: rpi-header-ehrpwm3-pins-default {
> + pinctrl-single,pins = <
> + J721E_IOPAD(0x18c, PIN_INPUT, 6) /* (V23) RGMII6_RX_CTL.EHRPWM3_A */
> + J721E_IOPAD(0x190, PIN_INPUT, 6) /* (W23) RGMII6_TD3.EHRPWM3_B */
> + >;
> + };
> };
>
> &wkup_pmx0 {
> @@ -631,11 +682,6 @@
> status = "disabled";
> };
>
> -&main_i2c5 {
> - /* Brought out on RPi Header */
> - status = "disabled";
> -};
> -

Please don't relocate nodes in the same patch - kinda messes up the
diffstat and makes review a bit harder.

> &main_i2c6 {
> /* Unused */
> status = "disabled";
> @@ -1138,18 +1184,39 @@
> status = "disabled";
> };
>
> -&main_ehrpwm2 {
> +&main_ehrpwm4 {
> status = "disabled";
> };
>
> -&main_ehrpwm3 {
> +&main_ehrpwm5 {
> status = "disabled";
> };
>
> -&main_ehrpwm4 {
> - status = "disabled";
> +&main_gpio0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&rpi_header_gpio0_pins_default>;
> };
>
> -&main_ehrpwm5 {
> - status = "disabled";
> +&main_gpio1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&rpi_header_gpio1_pins_default>;
> +};
> +
> +&main_i2c5 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&main_i2c5_pins_default>;
> + clock-frequency = <400000>;
> + status = "okay";

Defaults in SoC.dtsi so far are "okay" - so adding that again is
superfluous This happens when you are relocating nodes etc

> +};
> +
> +&main_ehrpwm2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&rpi_header_ehrpwm2_pins_default>;
> + status = "okay";
> +};
> +
> +&main_ehrpwm3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&rpi_header_ehrpwm3_pins_default>;
> + status = "okay";
> };

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2022-06-18 12:14:40

by Jason Kridner

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-j721e-sk: Add pinmux for RPi Header

For those seeing this message multiple times, my apologies for not
properly setting up my mailer.

On Fri, Jun 17, 2022 at 10:16 PM Nishanth Menon <[email protected]> wrote:
>
> On 15:40-20220530, Rahul T R wrote:
> > From: Sinthu Raja <[email protected]>
> >
> > Add pinmux required to bring out
> > i2c5, ehrpwm 2 and 3 and gpios on
> > 40 pin RPi header on sk board
> >
> > Signed-off-by: Sinthu Raja <[email protected]>
> > Signed-off-by: Rahul T R <[email protected]>
>
>
> I was digging deeper at https://www.ti.com/lit/zip/sprr438
> (PROC112E2(001)_SCH.pdf - J3, Also looking at
> https://github.com/beagleboard/beaglebone-ai-64/blob/master/BeagleBone%20AI%20-64_SCH_V1.02_211119.pdf
> (P8, P9)
>
> And comparing it to https://www.raspberrypi-spy.co.uk/2012/06/simple-guide-to-the-rpi-gpio-header-and-pins/
> And considering potential use such as https://pypi.org/project/RPi.GPIO/
> variation,
>
> Here is my suggestion (applies to other TI Boards that attempt to
> emulate RPI header)
> a) Default mux in board.dts should be GPIO except for the i2c used for
> ID detection.
> b) Secondary functions should be a dt overlay. (These can easily enable
> the pwms and other functions as needed)
> c) Maintain node names consistent to allow reuse of overlays across
> platforms.
>
> Usage: you can either use extlinux.conf OR uEnv.txt to apply the
> overlays as desired (ID detection from hats might help automate it based
> on the bootloader you'd want to use)
>
> Else, you have created a custom configuration here for 1 specific
> application, various hats that expect GPIO will end up croaking.
>
>
> I am open to discussions here.



A read of our recent blog series done with Bootlin is worth a read on
this topic. https://bbb.io/@2804

My suggestion is to put the entries for the common functions in the
base tree, but not enable them (except for the I2C used for ID
detection, as you suggested). This can remove SoC-specific
requirements on the overlays such that the overlays are written
against a header specification as defined by the common symbols
provided. So, make the entries in the base tree and leave them
"disabled" such that overlays only need to set the symbol to "okay",
at least for the common stuff to all implementations of the Pi header.


>
>
> > ---
> > arch/arm64/boot/dts/ti/k3-j721e-sk.dts | 89 ++++++++++++++++++++++----
> > 1 file changed, 78 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> > index 98a55778f3fe..b913b18ae133 100644
> > --- a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> > @@ -400,6 +400,57 @@
> > J721E_IOPAD(0x124, PIN_INPUT, 7) /* (Y24) PRG0_PRU1_GPO9.GPIO0_72 */
> > >;
> > };
> > +
> > + main_i2c5_pins_default: main-i2c5-pins-default {
> > + pinctrl-single,pins = <
> > + J721E_IOPAD(0x150, PIN_INPUT_PULLUP, 2) /* (Y26) PRG0_MDIO0_MDIO.I2C5_SCL */
> > + J721E_IOPAD(0x154, PIN_INPUT_PULLUP, 2) /* (AA27) PRG0_MDIO0_MDC.I2C5_SDA */
> > + >;
> > + };
> > +
> > + rpi_header_gpio0_pins_default: rpi-header-gpio0-pins-default {
> > + pinctrl-single,pins = <
> > + J721E_IOPAD(0x01c, PIN_INPUT, 7) /* (AD22) PRG1_PRU0_GPO6.GPIO0_7 */
> > + J721E_IOPAD(0x120, PIN_INPUT, 7) /* (AA28) PRG0_PRU1_GPO8.GPIO0_71 */
> > + J721E_IOPAD(0x14c, PIN_INPUT, 7) /* (AA29) PRG0_PRU1_GPO19.GPIO0_82 */
> > + J721E_IOPAD(0x02c, PIN_INPUT, 7) /* (AD21) PRG1_PRU0_GPO10.GPIO0_11 */
> > + J721E_IOPAD(0x198, PIN_INPUT, 7) /* (V25) RGMII6_TD1.GPIO0_101 */
> > + J721E_IOPAD(0x1b0, PIN_INPUT, 7) /* (W24) RGMII6_RD1.GPIO0_107 */
> > + J721E_IOPAD(0x1a0, PIN_INPUT, 7) /* (W29) RGMII6_TXC.GPIO0_103 */
> > + J721E_IOPAD(0x008, PIN_INPUT, 7) /* (AG22) PRG1_PRU0_GPO1.GPIO0_2 */
> > + J721E_IOPAD(0x1d0, PIN_INPUT, 7) /* (AA3) SPI0_D1.GPIO0_115 */
> > + J721E_IOPAD(0x11c, PIN_INPUT, 7) /* (AA24) PRG0_PRU1_GPO7.GPIO0_70 */
> > + J721E_IOPAD(0x148, PIN_INPUT, 7) /* (AA26) PRG0_PRU1_GPO18.GPIO0_81 */
> > + J721E_IOPAD(0x004, PIN_INPUT, 7) /* (AC23) PRG1_PRU0_GPO0.GPIO0_1 */
> > + J721E_IOPAD(0x014, PIN_INPUT, 7) /* (AH23) PRG1_PRU0_GPO4.GPIO0_5 */
> > + J721E_IOPAD(0x020, PIN_INPUT, 7) /* (AE20) PRG1_PRU0_GPO7.GPIO0_8 */
> > + J721E_IOPAD(0x19c, PIN_INPUT, 7) /* (W27) RGMII6_TD0.GPIO0_102 */
> > + J721E_IOPAD(0x1b4, PIN_INPUT, 7) /* (W25) RGMII6_RD0.GPIO0_108 */
> > + J721E_IOPAD(0x188, PIN_INPUT, 7) /* (Y28) RGMII6_TX_CTL.GPIO0_97 */
> > + J721E_IOPAD(0x00c, PIN_INPUT, 7) /* (AF22) PRG1_PRU0_GPO2.GPIO0_3 */
> > + J721E_IOPAD(0x010, PIN_INPUT, 7) /* (AJ23) PRG1_PRU0_GPO3.GPIO0_4 */
> > + >;
> > + };
> > +
> > + rpi_header_gpio1_pins_default: rpi-header-gpio1-pins-default {
> > + pinctrl-single,pins = <
> > + J721E_IOPAD(0x234, PIN_INPUT, 7) /* (U3) EXT_REFCLK1.GPIO1_12 */
> > + >;
> > + };
> > +
> > + rpi_header_ehrpwm2_pins_default: rpi-header-ehrpwm2-pins-default {
> > + pinctrl-single,pins = <
> > + J721E_IOPAD(0x178, PIN_INPUT, 6) /* (U27) RGMII5_RD3.EHRPWM2_A */
> > + J721E_IOPAD(0x17c, PIN_INPUT, 6) /* (U24) RGMII5_RD2.EHRPWM2_B */
> > + >;
> > + };
> > +
> > + rpi_header_ehrpwm3_pins_default: rpi-header-ehrpwm3-pins-default {
> > + pinctrl-single,pins = <
> > + J721E_IOPAD(0x18c, PIN_INPUT, 6) /* (V23) RGMII6_RX_CTL.EHRPWM3_A */
> > + J721E_IOPAD(0x190, PIN_INPUT, 6) /* (W23) RGMII6_TD3.EHRPWM3_B */
> > + >;
> > + };
> > };
> >
> > &wkup_pmx0 {
> > @@ -631,11 +682,6 @@
> > status = "disabled";
> > };
> >
> > -&main_i2c5 {
> > - /* Brought out on RPi Header */
> > - status = "disabled";
> > -};
> > -
>
> Please don't relocate nodes in the same patch - kinda messes up the
> diffstat and makes review a bit harder.
>
> > &main_i2c6 {
> > /* Unused */
> > status = "disabled";
> > @@ -1138,18 +1184,39 @@
> > status = "disabled";
> > };
> >
> > -&main_ehrpwm2 {
> > +&main_ehrpwm4 {
> > status = "disabled";
> > };
> >
> > -&main_ehrpwm3 {
> > +&main_ehrpwm5 {
> > status = "disabled";
> > };
> >
> > -&main_ehrpwm4 {
> > - status = "disabled";
> > +&main_gpio0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&rpi_header_gpio0_pins_default>;
> > };
> >
> > -&main_ehrpwm5 {
> > - status = "disabled";
> > +&main_gpio1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&rpi_header_gpio1_pins_default>;
> > +};
> > +
> > +&main_i2c5 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&main_i2c5_pins_default>;
> > + clock-frequency = <400000>;
> > + status = "okay";
>
> Defaults in SoC.dtsi so far are "okay" - so adding that again is
> superfluous This happens when you are relocating nodes etc
>
> > +};
> > +
> > +&main_ehrpwm2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&rpi_header_ehrpwm2_pins_default>;
> > + status = "okay";
> > +};
> > +
> > +&main_ehrpwm3 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&rpi_header_ehrpwm3_pins_default>;
> > + status = "okay";
> > };
>
> --
> Regards,
> Nishanth Menon
> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D



--
https://beagleboard.org/about/jkridner - a 501c3 non-profit educating
around open hardware computing

2022-06-20 15:44:43

by Rahul T R

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-j721e-sk: Add pinmux for RPi Header

On 08:11-20220618, Jason Kridner wrote:
> For those seeing this message multiple times, my apologies for not
> properly setting up my mailer.
>
> On Fri, Jun 17, 2022 at 10:16 PM Nishanth Menon <[email protected]> wrote:
> >
> > On 15:40-20220530, Rahul T R wrote:
> > > From: Sinthu Raja <[email protected]>
> > >
> > > Add pinmux required to bring out
> > > i2c5, ehrpwm 2 and 3 and gpios on
> > > 40 pin RPi header on sk board
> > >
> > > Signed-off-by: Sinthu Raja <[email protected]>
> > > Signed-off-by: Rahul T R <[email protected]>
> >
> >
> > I was digging deeper at https://www.ti.com/lit/zip/sprr438
> > (PROC112E2(001)_SCH.pdf - J3, Also looking at
> > https://github.com/beagleboard/beaglebone-ai-64/blob/master/BeagleBone%20AI%20-64_SCH_V1.02_211119.pdf
> > (P8, P9)
> >
> > And comparing it to https://www.raspberrypi-spy.co.uk/2012/06/simple-guide-to-the-rpi-gpio-header-and-pins/
> > And considering potential use such as https://pypi.org/project/RPi.GPIO/
> > variation,
> >
> > Here is my suggestion (applies to other TI Boards that attempt to
> > emulate RPI header)
> > a) Default mux in board.dts should be GPIO except for the i2c used for
> > ID detection.
> > b) Secondary functions should be a dt overlay. (These can easily enable
> > the pwms and other functions as needed)
> > c) Maintain node names consistent to allow reuse of overlays across
> > platforms.
> >
> > Usage: you can either use extlinux.conf OR uEnv.txt to apply the
> > overlays as desired (ID detection from hats might help automate it based
> > on the bootloader you'd want to use)
> >
> > Else, you have created a custom configuration here for 1 specific
> > application, various hats that expect GPIO will end up croaking.
> >
> >
> > I am open to discussions here.
>
>
>
> A read of our recent blog series done with Bootlin is worth a read on
> this topic. https://bbb.io/@2804
>
> My suggestion is to put the entries for the common functions in the
> base tree, but not enable them (except for the I2C used for ID
> detection, as you suggested). This can remove SoC-specific
> requirements on the overlays such that the overlays are written
> against a header specification as defined by the common symbols
> provided. So, make the entries in the base tree and leave them
> "disabled" such that overlays only need to set the symbol to "okay",
> at least for the common stuff to all implementations of the Pi header.
>

Hi Nishanth, Jason,

I agree for enabling only gpio's in the base dts
and create overlays based on the specific use case

We cannot create separate overlays for each functionality
since the pimux varies based on the combination of
functions enabled

Ex: pinmux for gpio will be different between
enabling only pwm and enabling pwm + spi

When we create a overlay, along with setting
the status to okay, we need to take care of
configuring right pinmux as well

I will send a respin with enabling only gpio

>
> >
> >
> > > ---
> > > arch/arm64/boot/dts/ti/k3-j721e-sk.dts | 89 ++++++++++++++++++++++----
> > > 1 file changed, 78 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> > > index 98a55778f3fe..b913b18ae133 100644
> > > --- a/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> > > +++ b/arch/arm64/boot/dts/ti/k3-j721e-sk.dts
> > > @@ -400,6 +400,57 @@
> > > J721E_IOPAD(0x124, PIN_INPUT, 7) /* (Y24) PRG0_PRU1_GPO9.GPIO0_72 */
> > > >;
> > > };
> > > +
> > > + main_i2c5_pins_default: main-i2c5-pins-default {
> > > + pinctrl-single,pins = <
> > > + J721E_IOPAD(0x150, PIN_INPUT_PULLUP, 2) /* (Y26) PRG0_MDIO0_MDIO.I2C5_SCL */
> > > + J721E_IOPAD(0x154, PIN_INPUT_PULLUP, 2) /* (AA27) PRG0_MDIO0_MDC.I2C5_SDA */
> > > + >;
> > > + };
> > > +
> > > + rpi_header_gpio0_pins_default: rpi-header-gpio0-pins-default {
> > > + pinctrl-single,pins = <
> > > + J721E_IOPAD(0x01c, PIN_INPUT, 7) /* (AD22) PRG1_PRU0_GPO6.GPIO0_7 */
> > > + J721E_IOPAD(0x120, PIN_INPUT, 7) /* (AA28) PRG0_PRU1_GPO8.GPIO0_71 */
> > > + J721E_IOPAD(0x14c, PIN_INPUT, 7) /* (AA29) PRG0_PRU1_GPO19.GPIO0_82 */
> > > + J721E_IOPAD(0x02c, PIN_INPUT, 7) /* (AD21) PRG1_PRU0_GPO10.GPIO0_11 */
> > > + J721E_IOPAD(0x198, PIN_INPUT, 7) /* (V25) RGMII6_TD1.GPIO0_101 */
> > > + J721E_IOPAD(0x1b0, PIN_INPUT, 7) /* (W24) RGMII6_RD1.GPIO0_107 */
> > > + J721E_IOPAD(0x1a0, PIN_INPUT, 7) /* (W29) RGMII6_TXC.GPIO0_103 */
> > > + J721E_IOPAD(0x008, PIN_INPUT, 7) /* (AG22) PRG1_PRU0_GPO1.GPIO0_2 */
> > > + J721E_IOPAD(0x1d0, PIN_INPUT, 7) /* (AA3) SPI0_D1.GPIO0_115 */
> > > + J721E_IOPAD(0x11c, PIN_INPUT, 7) /* (AA24) PRG0_PRU1_GPO7.GPIO0_70 */
> > > + J721E_IOPAD(0x148, PIN_INPUT, 7) /* (AA26) PRG0_PRU1_GPO18.GPIO0_81 */
> > > + J721E_IOPAD(0x004, PIN_INPUT, 7) /* (AC23) PRG1_PRU0_GPO0.GPIO0_1 */
> > > + J721E_IOPAD(0x014, PIN_INPUT, 7) /* (AH23) PRG1_PRU0_GPO4.GPIO0_5 */
> > > + J721E_IOPAD(0x020, PIN_INPUT, 7) /* (AE20) PRG1_PRU0_GPO7.GPIO0_8 */
> > > + J721E_IOPAD(0x19c, PIN_INPUT, 7) /* (W27) RGMII6_TD0.GPIO0_102 */
> > > + J721E_IOPAD(0x1b4, PIN_INPUT, 7) /* (W25) RGMII6_RD0.GPIO0_108 */
> > > + J721E_IOPAD(0x188, PIN_INPUT, 7) /* (Y28) RGMII6_TX_CTL.GPIO0_97 */
> > > + J721E_IOPAD(0x00c, PIN_INPUT, 7) /* (AF22) PRG1_PRU0_GPO2.GPIO0_3 */
> > > + J721E_IOPAD(0x010, PIN_INPUT, 7) /* (AJ23) PRG1_PRU0_GPO3.GPIO0_4 */
> > > + >;
> > > + };
> > > +
> > > + rpi_header_gpio1_pins_default: rpi-header-gpio1-pins-default {
> > > + pinctrl-single,pins = <
> > > + J721E_IOPAD(0x234, PIN_INPUT, 7) /* (U3) EXT_REFCLK1.GPIO1_12 */
> > > + >;
> > > + };
> > > +
> > > + rpi_header_ehrpwm2_pins_default: rpi-header-ehrpwm2-pins-default {
> > > + pinctrl-single,pins = <
> > > + J721E_IOPAD(0x178, PIN_INPUT, 6) /* (U27) RGMII5_RD3.EHRPWM2_A */
> > > + J721E_IOPAD(0x17c, PIN_INPUT, 6) /* (U24) RGMII5_RD2.EHRPWM2_B */
> > > + >;
> > > + };
> > > +
> > > + rpi_header_ehrpwm3_pins_default: rpi-header-ehrpwm3-pins-default {
> > > + pinctrl-single,pins = <
> > > + J721E_IOPAD(0x18c, PIN_INPUT, 6) /* (V23) RGMII6_RX_CTL.EHRPWM3_A */
> > > + J721E_IOPAD(0x190, PIN_INPUT, 6) /* (W23) RGMII6_TD3.EHRPWM3_B */
> > > + >;
> > > + };
> > > };
> > >
> > > &wkup_pmx0 {
> > > @@ -631,11 +682,6 @@
> > > status = "disabled";
> > > };
> > >
> > > -&main_i2c5 {
> > > - /* Brought out on RPi Header */
> > > - status = "disabled";
> > > -};
> > > -
> >
> > Please don't relocate nodes in the same patch - kinda messes up the
> > diffstat and makes review a bit harder.
> >

will fix this in the respin

Regards
Rahul T R

> > > &main_i2c6 {
> > > /* Unused */
> > > status = "disabled";
> > > @@ -1138,18 +1184,39 @@
> > > status = "disabled";
> > > };
> > >
> > > -&main_ehrpwm2 {
> > > +&main_ehrpwm4 {
> > > status = "disabled";
> > > };
> > >
> > > -&main_ehrpwm3 {
> > > +&main_ehrpwm5 {
> > > status = "disabled";
> > > };
> > >
> > > -&main_ehrpwm4 {
> > > - status = "disabled";
> > > +&main_gpio0 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&rpi_header_gpio0_pins_default>;
> > > };
> > >
> > > -&main_ehrpwm5 {
> > > - status = "disabled";
> > > +&main_gpio1 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&rpi_header_gpio1_pins_default>;
> > > +};
> > > +
> > > +&main_i2c5 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&main_i2c5_pins_default>;
> > > + clock-frequency = <400000>;
> > > + status = "okay";
> >
> > Defaults in SoC.dtsi so far are "okay" - so adding that again is
> > superfluous This happens when you are relocating nodes etc
> >
> > > +};
> > > +
> > > +&main_ehrpwm2 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&rpi_header_ehrpwm2_pins_default>;
> > > + status = "okay";
> > > +};
> > > +
> > > +&main_ehrpwm3 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&rpi_header_ehrpwm3_pins_default>;
> > > + status = "okay";
> > > };
> >
> > --
> > Regards,
> > Nishanth Menon
> > Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
>
>
>
> --
> https://beagleboard.org/about/jkridner - a 501c3 non-profit educating
> around open hardware computing