Re: [PATCH] arm64: dts: ti: k3-j7200: Fix register map for main domain pmx

[Jared McArthur <j-mcarthur@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
> [...]
>
-- 
Best,
Jared McArthur