On 17:01-20230828, Neha Malcom Francis wrote:
Sync k3-j721e DTS with kernel.org v6.5-rc1.
* pcie_epx nodes have been deleted, they are not needed [1]
* use mcu_timer0 instead of the redundant timer1. Also delete
clock and power domain properties since J721E follows legacy
boot flow where system firmware is not yet available to modify
clocks
* remove mcu_i2c0 as it is used for tps659413 PMIC in j721e-sk
for which support is not yet added.
* remove main_i2c2 pinmux for j721e-sk as it is connected to the
test automation header for which support is not there
* change mcu_secproxy to secure_proxy_mcu since linux has
defined these nodes
* hbmc nodes have been disabled as their support in kernel
itself is unfinished (dependent series not merged) [2]
* Drop mcu_cpsw node in U-Boot as it is no longer needed here
thanks to [3]
Just use spaces.
You do not need to provide changelog to all the u-boot changes in the commit
message - use the diffstat for that. if you are referring to topics such
as those use commit id style.
[...]
diff --git a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
index 540c847eb3..6a95ed2a96 100644
--- a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
@@ -8,12 +8,10 @@
/ {
chosen {
- stdout-path = "serial2:115200n8";
- tick-timer = &timer1;
+ tick-timer = &mcu_timer0;
Looking at Manorit's series, I realized that tick-timer property is only
needed for R5 dts.
};
aliases {
- ethernet0 = &cpsw_port1;
spi0 = &ospi0;
spi1 = &ospi1;
You don't need any of these. nor do you need i2c aliases etc. all those
come from the board.dts.
remoteproc0 = &mcu_r5fss0_core0;
remoteproc aliases is probably needed to remain sane - upstream kernel
maintainer hasn't agreed[1] to pick up aliases so far.
@@ -34,52 +32,44 @@
&cbass_main{
watch for that space after nodename &cbass_main { instead of &cbass_main{
bootph-pre-ram;
[...]
assigned-clock-parents = <&k3_clks 295 0>, <&k3_clks 295 9>;
Why do we still have this for &wiz3_pll1_refclk and &wiz3_pll1_refclk
&serdes0_pcie_link ?
Also why are we dropping assigned-clocks and assigned-clock-parents in
&serdes0? And these dont even have a bootph or u-boot, property - so
what is the point in invoking them if they are'nt getting set in early
stages of boot. if this is a valid clocking bug, fix should have gone to
kernel - and document it as such.
[...]
I will comment about dr_mode and usb
There is a bit of a debate ongoing on this.
https://lore.kernel.org/u-boot/20230706-handle-otg-as-periph-v3-0-27e24fa17...@baylibre.com/
But I don't see why we need to hold off for that resolution.
&main_mmc1_pins_default {
bootph-pre-ram;
};
@@ -169,8 +158,14 @@
bootph-pre-ram;
};
+&wkup_uart0 {
+ bootph-pre-ram;
+ status = "okay";
+};
+
&wkup_i2c0 {
bootph-pre-ram;
+ status = "okay";
};
&main_i2c0 {
@@ -181,6 +176,10 @@
bootph-pre-ram;
};
+&main_esm{
Watch out for that space before {.
+ bootph-pre-ram;
+};
+
&exp2 {
bootph-pre-ram;
};
@@ -194,6 +193,7 @@
};
&hbmc {
+ status = "disabled";
bootph-pre-ram;
flash@0,0 {
Just drop the flash information etc in the node and mark the node as disabled.
@@ -268,7 +268,38 @@
assigned-clock-parents = <&wiz0_pll1_refclk>;
};
-&serdes0_qsgmii_link {
- assigned-clocks = <&serdes0 CDNS_SIERRA_PLL_CMNLC1>;
- assigned-clock-parents = <&wiz0_pll1_refclk>;
+&main_crypto {
+ dma-coherent;
+};
Why?
+
+&rng {
+ clocks = <&k3_clks 264 1>;
+};
We don't even use rng in u-boot.
+
+&main_i2c2 {
+ status = "okay";
+};
+
+&main_i2c4 {
+ status = "okay";
+};
+
+&main_i2c5 {
+ status = "okay";
+};
You need all of these in u-boot? They dont even have bootph properties.
nothing without bootph OR u-boot, properties should even be present in
u-boot.dtsi
+
+&wkup_uart0 {
+ status = "okay";
+};
+
+&mcu_i2c0 {
+ status = "okay";
+};
+
+&mcu_i2c1 {
+ status = "okay";
+};
+
+&dss {
+ status = "disabled";
};
Why not just leave things as is from kernel dts?
[...]
diff --git a/arch/arm/dts/k3-j721e-r5-common-proc-board.dts
b/arch/arm/dts/k3-j721e-r5-common-proc-board.dts
index 7bb5ce775c..da8cf7f37b 100644
--- a/arch/arm/dts/k3-j721e-r5-common-proc-board.dts
+++ b/arch/arm/dts/k3-j721e-r5-common-proc-board.dts
[...]
&mcu_uart0 {
/delete-property/ power-domains;
/delete-property/ clocks;
/delete-property/ clock-names;
NAK. no explanation why we have to delete these properties. and why not
we add data to handle it if it is mandatory.
- pinctrl-names = "default";
- pinctrl-0 = <&mcu_uart0_pins_default>;
- status = "okay";
clock-frequency = <48000000>;
};
-&main_uart0 {
- pinctrl-names = "default";
- pinctrl-0 = <&main_uart0_pins_default>;
- status = "okay";
- power-domains = <&k3_pds 146 TI_SCI_PD_SHARED>;
-};
-
&main_sdhci0 {
/delete-property/ power-domains;
/delete-property/ assigned-clocks;
/delete-property/ assigned-clock-parents;
NAK. no explanation why we have to delete these properties. and why not
we add data to handle it if it is mandatory.
clock-names = "clk_xin";
clocks = <&clk_200mhz>;
NAK again. please explain why.
- ti,driver-strength-ohm = <50>;
Will this even work?
- non-removable;
bus-width = <8>;
};
[.. skipping the rest.. since pattern of comments seem to be repeated..]
[1] https://lore.kernel.org/lkml/20230822215042.yjaqtwhuhls57pbu@glamour/T/
