| Message ID | 20230811151644.3216621-4-a-nandan@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: ti: k3-j784s4: Add phase tags marking | expand |
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 <a-nandan@ti.com> > --- > [...] > &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>;
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 <a-nandan@ti.com> > > --- > > [...] > > &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? > > [...]
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 <a-nandan@ti.com> >>> --- >>> [...] >>> &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 <u-kumar1@ti.com> >> > [...] >
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 <a-nandan@ti.com> > > > > --- > > > > [...] > > > > &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 <u-kumar1@ti.com> > > > > > > [...] > >
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>;
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 <a-nandan@ti.com> --- arch/arm64/boot/dts/ti/k3-am69-sk.dts | 13 +++++++++++++ 1 file changed, 13 insertions(+)