2023-08-11 15:20:05

by Apurva Nandan

[permalink] [raw]
Subject: [PATCH v4 3/3] arm64: dts: ti: k3-am69-sk: Add phase tags marking

bootph-all as phase tag was added to dt-schema
(dtschema/schemas/bootph.yaml) to cover U-Boot challenges with DT.
That's why add it also to Linux to be aligned with bootloader requirement.

wkup_uart0, wkup_i2c0, mcu_uart0, main_uart8, main_sdhci0 and main_sdhci1
are required for bootloader operation on TI K3 AM69-SK EVM. These IPs
along with pinmuxes need to be marked for all bootloader phases, hence add
bootph-all to these nodes in kernel dts.

Signed-off-by: Apurva Nandan <[email protected]>
---
arch/arm64/boot/dts/ti/k3-am69-sk.dts | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am69-sk.dts b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
index d282c2c633c1..2302d55c3fe7 100644
--- a/arch/arm64/boot/dts/ti/k3-am69-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am69-sk.dts
@@ -110,7 +110,9 @@ vdd_sd_dv: regulator-tlv71033 {
};

&main_pmx0 {
+ bootph-all;
main_uart8_pins_default: main-uart8-default-pins {
+ bootph-all;
pinctrl-single,pins = <
J784S4_IOPAD(0x0d0, PIN_INPUT, 11) /* (AP38) SPI0_CS1.UART8_RXD */
J784S4_IOPAD(0x0d4, PIN_OUTPUT, 11) /* (AN38) SPI0_CLK.UART8_TXD */
@@ -125,6 +127,7 @@ J784S4_IOPAD(0x0e4, PIN_INPUT_PULLUP, 0) /* (AP37) I2C0_SDA */
};

main_mmc1_pins_default: main-mmc1-default-pins {
+ bootph-all;
pinctrl-single,pins = <
J784S4_IOPAD(0x104, PIN_INPUT, 0) /* (AB38) MMC1_CLK */
J784S4_IOPAD(0x108, PIN_INPUT, 0) /* (AB36) MMC1_CMD */
@@ -164,7 +167,9 @@ J784S4_IOPAD(0x004, PIN_INPUT, 7) /* (AG36) MCAN12_TX.GPIO0_1 */
};

&wkup_pmx2 {
+ bootph-all;
wkup_uart0_pins_default: wkup-uart0-default-pins {
+ bootph-all;
pinctrl-single,pins = <
J721S2_WKUP_IOPAD(0x070, PIN_INPUT, 0) /* (L37) WKUP_GPIO0_6.WKUP_UART0_CTSn */
J721S2_WKUP_IOPAD(0x074, PIN_INPUT, 0) /* (L36) WKUP_GPIO0_7.WKUP_UART0_RTSn */
@@ -174,6 +179,7 @@ J721S2_WKUP_IOPAD(0x04c, PIN_INPUT, 0) /* (K34) WKUP_UART0_TXD */
};

wkup_i2c0_pins_default: wkup-i2c0-default-pins {
+ bootph-all;
pinctrl-single,pins = <
J721S2_WKUP_IOPAD(0x98, PIN_INPUT, 0) /* (N33) WKUP_I2C0_SCL */
J721S2_WKUP_IOPAD(0x9c, PIN_INPUT, 0) /* (N35) WKUP_I2C0_SDA */
@@ -181,6 +187,7 @@ J721S2_WKUP_IOPAD(0x9c, PIN_INPUT, 0) /* (N35) WKUP_I2C0_SDA */
};

mcu_uart0_pins_default: mcu-uart0-default-pins {
+ bootph-all;
pinctrl-single,pins = <
J784S4_WKUP_IOPAD(0x08c, PIN_INPUT, 0) /* (K38) WKUP_GPIO0_13.MCU_UART0_RXD */
J784S4_WKUP_IOPAD(0x088, PIN_OUTPUT, 0) /* (J37) WKUP_GPIO0_12.MCU_UART0_TXD */
@@ -242,6 +249,7 @@ J784S4_WKUP_IOPAD(0x0, PIN_INPUT, 7) /* (M33) WKUP_GPIO0_49 */
};

&wkup_uart0 {
+ bootph-all;
/* Firmware usage */
status = "reserved";
pinctrl-names = "default";
@@ -249,6 +257,7 @@ &wkup_uart0 {
};

&wkup_i2c0 {
+ bootph-all;
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&wkup_i2c0_pins_default>;
@@ -268,6 +277,7 @@ &wkup_gpio0 {
};

&mcu_uart0 {
+ bootph-all;
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&mcu_uart0_pins_default>;
@@ -281,6 +291,7 @@ &mcu_i2c0 {
};

&main_uart8 {
+ bootph-all;
status = "okay";
pinctrl-names = "default";
pinctrl-0 = <&main_uart8_pins_default>;
@@ -307,6 +318,7 @@ exp1: gpio@21 {
};

&main_sdhci0 {
+ bootph-all;
/* eMMC */
status = "okay";
non-removable;
@@ -315,6 +327,7 @@ &main_sdhci0 {
};

&main_sdhci1 {
+ bootph-all;
/* SD card */
status = "okay";
pinctrl-0 = <&main_mmc1_pins_default>;
--
2.34.1



2023-08-11 18:36:06

by Kumar, Udit

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-am69-sk: Add phase tags marking

Hi Apurva

On 8/11/2023 8:46 PM, Apurva Nandan wrote:
> bootph-all as phase tag was added to dt-schema
> (dtschema/schemas/bootph.yaml) to cover U-Boot challenges with DT.
> That's why add it also to Linux to be aligned with bootloader requirement.
>
> wkup_uart0, wkup_i2c0, mcu_uart0, main_uart8, main_sdhci0 and main_sdhci1
> are required for bootloader operation on TI K3 AM69-SK EVM. These IPs
> along with pinmuxes need to be marked for all bootloader phases, hence add
> bootph-all to these nodes in kernel dts.
>
> Signed-off-by: Apurva Nandan <[email protected]>
> ---
> [...]
> &wkup_uart0 {
> + bootph-all;
> /* Firmware usage */
> status = "reserved";
> pinctrl-names = "default";

I am not sure, if you want to treat wkup_uart in same way as you are
treating secure_proxy_mcu in patch 1 of this series.

IMO, where we are making this node status is okay, mark booth-all at
that place only.

Otherwise for rest of series

LGTM


> @@ -249,6 +257,7 @@ &wkup_uart0 {
> };
>
> &wkup_i2c0 {
> + bootph-all;
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&wkup_i2c0_pins_default>;
> @@ -268,6 +277,7 @@ &wkup_gpio0 {
> };
>
> &mcu_uart0 {
> + bootph-all;
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&mcu_uart0_pins_default>;
> @@ -281,6 +291,7 @@ &mcu_i2c0 {
> };
>
> &main_uart8 {
> + bootph-all;
> status = "okay";
> pinctrl-names = "default";
> pinctrl-0 = <&main_uart8_pins_default>;
> @@ -307,6 +318,7 @@ exp1: gpio@21 {
> };
>
> &main_sdhci0 {
> + bootph-all;
> /* eMMC */
> status = "okay";
> non-removable;
> @@ -315,6 +327,7 @@ &main_sdhci0 {
> };
>
> &main_sdhci1 {
> + bootph-all;
> /* SD card */
> status = "okay";
> pinctrl-0 = <&main_mmc1_pins_default>;

2023-08-11 19:07:20

by Kumar, Udit

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-am69-sk: Add phase tags marking


On 8/11/2023 11:24 PM, Nishanth Menon wrote:
> On 23:05-20230811, Kumar, Udit wrote:
>> Hi Apurva
>>
>> On 8/11/2023 8:46 PM, Apurva Nandan wrote:
>>> bootph-all as phase tag was added to dt-schema
>>> (dtschema/schemas/bootph.yaml) to cover U-Boot challenges with DT.
>>> That's why add it also to Linux to be aligned with bootloader requirement.
>>>
>>> wkup_uart0, wkup_i2c0, mcu_uart0, main_uart8, main_sdhci0 and main_sdhci1
>>> are required for bootloader operation on TI K3 AM69-SK EVM. These IPs
>>> along with pinmuxes need to be marked for all bootloader phases, hence add
>>> bootph-all to these nodes in kernel dts.
>>>
>>> Signed-off-by: Apurva Nandan <[email protected]>
>>> ---
>>> [...]
>>> &wkup_uart0 {
>>> + bootph-all;
>>> /* Firmware usage */
>>> status = "reserved";
>>> pinctrl-names = "default";
>> I am not sure, if you want to treat wkup_uart in same way as you are
>> treating secure_proxy_mcu in patch 1 of this series.
> You should'nt. wkup_uart0 or what ever peripherals are specifically
> board dependent. This patch does it the right way. I do have other
> platforms on other K3 SoCs where the TIFS uart logs are actually
> disabled.

Sorry, if i was not clear in my previous response.

This node is marked as reserved, adding bootph is not adding any value.

We can drop bootph from this node here.

>> IMO, where we are making this node status is okay, mark booth-all at that
>> place only.
>>
>> Otherwise for rest of series
>>
>> LGTM
> Do i take that as a Reviewed-by: for the series?


With above change,

Reviewed-by: Udit Kumar <[email protected]>

>>
> [...]
>

2023-08-11 20:15:36

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-am69-sk: Add phase tags marking

On 23:35-20230811, Kumar, Udit wrote:
>
> On 8/11/2023 11:24 PM, Nishanth Menon wrote:
> > On 23:05-20230811, Kumar, Udit wrote:
> > > Hi Apurva
> > >
> > > On 8/11/2023 8:46 PM, Apurva Nandan wrote:
> > > > bootph-all as phase tag was added to dt-schema
> > > > (dtschema/schemas/bootph.yaml) to cover U-Boot challenges with DT.
> > > > That's why add it also to Linux to be aligned with bootloader requirement.
> > > >
> > > > wkup_uart0, wkup_i2c0, mcu_uart0, main_uart8, main_sdhci0 and main_sdhci1
> > > > are required for bootloader operation on TI K3 AM69-SK EVM. These IPs
> > > > along with pinmuxes need to be marked for all bootloader phases, hence add
> > > > bootph-all to these nodes in kernel dts.
> > > >
> > > > Signed-off-by: Apurva Nandan <[email protected]>
> > > > ---
> > > > [...]
> > > > &wkup_uart0 {
> > > > + bootph-all;
> > > > /* Firmware usage */
> > > > status = "reserved";
> > > > pinctrl-names = "default";
> > > I am not sure, if you want to treat wkup_uart in same way as you are
> > > treating secure_proxy_mcu in patch 1 of this series.
> > You should'nt. wkup_uart0 or what ever peripherals are specifically
> > board dependent. This patch does it the right way. I do have other
> > platforms on other K3 SoCs where the TIFS uart logs are actually
> > disabled.
>
> Sorry, if i was not clear in my previous response.
>
> This node is marked as reserved, adding bootph is not adding any value.
>
> We can drop bootph from this node here.


Aah - yes. makes sense. Thanks for catching this. reserved nodes dont
make sense to have bootph in kernel dts. zephyr or u-boot r5 view will
probably enable them, and they should call it out.

>
> > > IMO, where we are making this node status is okay, mark booth-all at that
> > > place only.
> > >
> > > Otherwise for rest of series
> > >
> > > LGTM
> > Do i take that as a Reviewed-by: for the series?
>
>
> With above change,
>
> Reviewed-by: Udit Kumar <[email protected]>
>
> > >
> > [...]
> >

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

2023-08-11 20:59:27

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: k3-am69-sk: Add phase tags marking

On 23:05-20230811, Kumar, Udit wrote:
> Hi Apurva
>
> On 8/11/2023 8:46 PM, Apurva Nandan wrote:
> > bootph-all as phase tag was added to dt-schema
> > (dtschema/schemas/bootph.yaml) to cover U-Boot challenges with DT.
> > That's why add it also to Linux to be aligned with bootloader requirement.
> >
> > wkup_uart0, wkup_i2c0, mcu_uart0, main_uart8, main_sdhci0 and main_sdhci1
> > are required for bootloader operation on TI K3 AM69-SK EVM. These IPs
> > along with pinmuxes need to be marked for all bootloader phases, hence add
> > bootph-all to these nodes in kernel dts.
> >
> > Signed-off-by: Apurva Nandan <[email protected]>
> > ---
> > [...]
> > &wkup_uart0 {
> > + bootph-all;
> > /* Firmware usage */
> > status = "reserved";
> > pinctrl-names = "default";
>
> I am not sure, if you want to treat wkup_uart in same way as you are
> treating secure_proxy_mcu in patch 1 of this series.

You should'nt. wkup_uart0 or what ever peripherals are specifically
board dependent. This patch does it the right way. I do have other
platforms on other K3 SoCs where the TIFS uart logs are actually
disabled.

>
> IMO, where we are making this node status is okay, mark booth-all at that
> place only.
>
> Otherwise for rest of series
>
> LGTM

Do i take that as a Reviewed-by: for the series?
>
>
[...]

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