| Message ID | 20240829071208.2172825-1-a-limaye@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: ti: k3-j7200: Fix register map for main domain pmx | expand |
On 12:42-20240829, Aniket Limaye wrote: > From: Jared McArthur <j-mcarthur@ti.com> > > Commit 0d0a0b441346 ("arm64: dts: ti: k3-j7200: fix main pinmux > range") split the main_pmx0 into two nodes: main_pmx0 and main_pmx1 > due to a non-addressable region, but incorrectly represented the > ranges. As a result, the memory map for the pinctrl is incorrect. Fix > this by introducing the correct ranges. > > The ranges are taken from the J7200 TRM (Table 5-695. CTRL_MMR0 > Registers). Padconfig registers stretch from 0x11c000 to 0x11c168 > with non-addressable portions from 0x11c10c to 0x11c10f, 0x11x114 to > 0x11c11b, and 0x11c128 to 0x11c163. > > Link: https://www.ti.com/lit/ug/spruiu1c/spruiu1c.pdf (TRM) Use the canonical link that redirects to the latest document such as https://www.ti.com/lit/pdf/spruiu1 older versions of the TRM may not be retained in ti.com > Fixes: 0d0a0b441346 ("arm64: dts: ti: k3-j7200: fix main pinmux range") > Signed-off-by: Jared McArthur <j-mcarthur@ti.com> > Signed-off-by: Aniket Limaye <a-limaye@ti.com> > --- > .../dts/ti/k3-j7200-common-proc-board.dts | 2 +- > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 22 +++++++++++++++++-- > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts > index 6593c5da82c0..df39f2b1ff6b 100644 > --- a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts > +++ b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts > @@ -254,7 +254,7 @@ J721E_IOPAD(0x38, PIN_OUTPUT, 0) /* (Y21) MCAN3_TX */ > }; > }; > > -&main_pmx1 { > +&main_pmx2 { > main_usbss0_pins_default: main-usbss0-default-pins { > pinctrl-single,pins = < > J721E_IOPAD(0x04, PIN_OUTPUT, 0) /* (T4) USB0_DRVVBUS */ How is this change correct if you are changing the base from 0x1c to 0x10 (previously it was pointing to 0x20, now to 0x14? what is the correct offset of USB0_DRVBUS pin?) Did you do an audit of all offsets of main_pmx2 and 3 and resultant address split up (including overlays if applicable)? Explain that in commit message / diffstat as appropriate > diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi > index 9386bf3ef9f6..41adfa64418d 100644 > --- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi [...]
On 8/29/24 02:12, Aniket Limaye wrote: > From: Jared McArthur <j-mcarthur@ti.com> > > Commit 0d0a0b441346 ("arm64: dts: ti: k3-j7200: fix main pinmux > range") split the main_pmx0 into two nodes: main_pmx0 and main_pmx1 > due to a non-addressable region, but incorrectly represented the > ranges. As a result, the memory map for the pinctrl is incorrect. Fix > this by introducing the correct ranges. > > The ranges are taken from the J7200 TRM (Table 5-695. CTRL_MMR0 > Registers). Padconfig registers stretch from 0x11c000 to 0x11c168 > with non-addressable portions from 0x11c10c to 0x11c10f, 0x11x114 to > 0x11c11b, and 0x11c128 to 0x11c163. Still unsure whether these registers are correct. There is a conflict between the TRM [1] and the datasheet [2] on whether PADCONFIG63 exists. The datasheet doesn't think it does. I would like to confirm this before the patch is submitted/accepted, because the pinmuxing would need to change again if the datasheet is correct. [1] https://www.ti.com/lit/pdf/spruiu1 [2] https://www.ti.com/lit/gpn/dra821u > Link: https://www.ti.com/lit/ug/spruiu1c/spruiu1c.pdf (TRM) > Fixes: 0d0a0b441346 ("arm64: dts: ti: k3-j7200: fix main pinmux range") > Signed-off-by: Jared McArthur <j-mcarthur@ti.com> > Signed-off-by: Aniket Limaye <a-limaye@ti.com> [...]
On 8/29/24 13:31, Nishanth Menon wrote: > On 12:42-20240829, Aniket Limaye wrote: >> From: Jared McArthur <j-mcarthur@ti.com> >> >> Commit 0d0a0b441346 ("arm64: dts: ti: k3-j7200: fix main pinmux >> range") split the main_pmx0 into two nodes: main_pmx0 and main_pmx1 >> due to a non-addressable region, but incorrectly represented the >> ranges. As a result, the memory map for the pinctrl is incorrect. Fix >> this by introducing the correct ranges. >> >> The ranges are taken from the J7200 TRM (Table 5-695. CTRL_MMR0 >> Registers). Padconfig registers stretch from 0x11c000 to 0x11c168 >> with non-addressable portions from 0x11c10c to 0x11c10f, 0x11x114 to >> 0x11c11b, and 0x11c128 to 0x11c163. >> >> Link: https://www.ti.com/lit/ug/spruiu1c/spruiu1c.pdf (TRM) > Use the canonical link that redirects to the latest document such as https://www.ti.com/lit/pdf/spruiu1 > > older versions of the TRM may not be retained in ti.com Thank you, will do. >> Fixes: 0d0a0b441346 ("arm64: dts: ti: k3-j7200: fix main pinmux range") >> Signed-off-by: Jared McArthur <j-mcarthur@ti.com> >> Signed-off-by: Aniket Limaye <a-limaye@ti.com> >> --- >> .../dts/ti/k3-j7200-common-proc-board.dts | 2 +- >> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 22 +++++++++++++++++-- >> 2 files changed, 21 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts >> index 6593c5da82c0..df39f2b1ff6b 100644 >> --- a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts >> +++ b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts >> @@ -254,7 +254,7 @@ J721E_IOPAD(0x38, PIN_OUTPUT, 0) /* (Y21) MCAN3_TX */ >> }; >> }; >> >> -&main_pmx1 { >> +&main_pmx2 { >> main_usbss0_pins_default: main-usbss0-default-pins { >> pinctrl-single,pins = < >> J721E_IOPAD(0x04, PIN_OUTPUT, 0) /* (T4) USB0_DRVVBUS */ > How is this change correct if you are changing the base from 0x1c to > 0x10 (previously it was pointing to 0x20, now to 0x14? what is the > correct offset of USB0_DRVBUS pin?) The base hasn't changed because it's switched to main_pmx2 instead of main_pmx1. main_pmx2 is the same address range as the original main_pmx1. I agree that this should be explained in the commit message/diffstat. > Did you do an audit of all offsets of main_pmx2 and 3 and resultant > address split up (including overlays if applicable)? All current pinmuxing for J7200's dts and dtso files is within the main_pmx0, except for the USB0_DRVBUS pin. Since the address range for main_pmx0 has not changed, there are no other changes that need to be made. > Explain that in commit message / diffstat as appropriate I agree that this should be written out in a commit message / diffstat. Perhaps it should also be a separate patch within the same series. >> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi >> index 9386bf3ef9f6..41adfa64418d 100644 >> --- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi >> +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi > [...] >
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts index 6593c5da82c0..df39f2b1ff6b 100644 --- a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts +++ b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts @@ -254,7 +254,7 @@ J721E_IOPAD(0x38, PIN_OUTPUT, 0) /* (Y21) MCAN3_TX */ }; }; -&main_pmx1 { +&main_pmx2 { main_usbss0_pins_default: main-usbss0-default-pins { pinctrl-single,pins = < J721E_IOPAD(0x04, PIN_OUTPUT, 0) /* (T4) USB0_DRVVBUS */ diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi index 9386bf3ef9f6..41adfa64418d 100644 --- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi @@ -426,10 +426,28 @@ main_pmx0: pinctrl@11c000 { pinctrl-single,function-mask = <0xffffffff>; }; - main_pmx1: pinctrl@11c11c { + main_pmx1: pinctrl@11c110 { compatible = "ti,j7200-padconf", "pinctrl-single"; /* Proxy 0 addressing */ - reg = <0x00 0x11c11c 0x00 0xc>; + reg = <0x00 0x11c110 0x00 0x004>; + #pinctrl-cells = <1>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <0xffffffff>; + }; + + main_pmx2: pinctrl@11c11c { + compatible = "ti,j7200-padconf", "pinctrl-single"; + /* Proxy 0 addressing */ + reg = <0x00 0x11c11c 0x00 0x00c>; + #pinctrl-cells = <1>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <0xffffffff>; + }; + + main_pmx3: pinctrl@11c164 { + compatible = "ti,j7200-padconf", "pinctrl-single"; + /* Proxy 0 addressing */ + reg = <0x00 0x11c164 0x00 0x008>; #pinctrl-cells = <1>; pinctrl-single,register-width = <32>; pinctrl-single,function-mask = <0xffffffff>;