Hi Udit, On 19:00-20240830, Kumar, Udit wrote: > Hi Manorit > > Overall series looks ok but few comments below > > On 8/28/2024 4:44 PM, Manorit Chawdhry wrote: > > Refactor J784s4 SoC files to a common file which uses the > > superset device to allow reuse in j742s2-evm which uses the subset part. > > > > Signed-off-by: Manorit Chawdhry <m-chawdhry@xxxxxx> > > Reviewed-by: Beleswar Padhi <b-padhi@xxxxxx> > > --- > > .../arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi | 150 ++ > > .../boot/dts/ti/k3-j784s4-j742s2-main-common.dtsi | 2667 ++++++++++++++++++ > > ...tsi => k3-j784s4-j742s2-mcu-wakeup-common.dtsi} | 2 +- > > ...l.dtsi => k3-j784s4-j742s2-thermal-common.dtsi} | 0 > > arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 2847 +------------------- > > arch/arm64/boot/dts/ti/k3-j784s4.dtsi | 135 +- > > 6 files changed, 2914 insertions(+), 2887 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi > > new file mode 100644 > > index 000000000000..43fee57f0926 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi > > @@ -0,0 +1,150 @@ > > +// SPDX-License-Identifier: GPL-2.0-only OR MIT > > +/* > > + * Device Tree Source for J784S4 and J742S2 SoC Family > > + * > > + * TRM (j784s4) (SPRUJ43 JULY 2022): https://www.ti.com/lit/zip/spruj52 > > + * TRM (j742s2): https://www.ti.com/lit/pdf/spruje3 > > + * > > [..] <0x00 0x01000000 0x00 0x01000000 0x00 0x0d000000>, /* Most peripherals */ > > + <0x00 0x04210000 0x00 0x04210000 0x00 0x00010000>, /* VPU0 */ > > + <0x00 0x04220000 0x00 0x04220000 0x00 0x00010000>, /* VPU1 */ > > + <0x00 0x0d000000 0x00 0x0d000000 0x00 0x00800000>, /* PCIe0 Core*/ > > + <0x00 0x0d800000 0x00 0x0d800000 0x00 0x00800000>, /* PCIe1 Core*/ > > + <0x00 0x0e000000 0x00 0x0e000000 0x00 0x00800000>, /* PCIe2 Core*/ > > + <0x00 0x0e800000 0x00 0x0e800000 0x00 0x00800000>, /* PCIe3 Core*/ > > > PCie2 and PCIe3 ranges are not common across these devices, > > Do you want to move this into J784s4 specific file > > Same comment for PCIe region DAT below This was already discussed in the previous revision and my stance is not to change it due to maintainance reasons [0]. > > > [..] > > > <0x42 0x00000000 0x42 0x00000000 0x01 0x00000000>, /* PCIe2 DAT1 */ > > + <0x43 0x00000000 0x43 0x00000000 0x01 0x00000000>, /* PCIe3 DAT1 */ > > [..] > > +#include "k3-j784s4-j742s2-main-common.dtsi" > > +#include "k3-j784s4-j742s2-mcu-wakeup-common.dtsi" > > diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-main-common.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-main-common.dtsi > > [...] > > + > > +&cbass_main { > > + msmc_ram: sram@70000000 { > > + compatible = "mmio-sram"; > > + reg = <0x00 0x70000000 0x00 0x800000>; > > Table 2-1 of J742S2 TRM says msmc RAM is 4MB and on J784S4 this is 8MB > > Please see, if you can address that > I think this was thought through before. So from my understanding, this memory map is just a dummy node that the bootloaders is supposed to be fixing up based on it's usecase [1]. Though ig it's not very intuitive looking at the DT, let me add a comment in the corresponding node to clarify this. "MSMC is configured by bootloaders and a runtime fixup is done in the DT for this node" Would you be okay with the following comment in the DT node for MSMC but keeping the following node in common file only? Regards, Manorit [0]: https://lore.kernel.org/linux-arm-kernel/20240827082445.bfx2r7z4iry4fdax@uda0497581/ [1]: https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=query#tisci-msg-query-msmc > > > [...] > >
