[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240827082445.bfx2r7z4iry4fdax@uda0497581>
Date: Tue, 27 Aug 2024 13:54:45 +0530
From: Manorit Chawdhry <m-chawdhry@...com>
To: Siddharth Vadapalli <s-vadapalli@...com>
CC: Nishanth Menon <nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
Tero
Kristo <kristo@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof
Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
<linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Udit Kumar <u-kumar1@...com>,
Neha Malcom
Francis <n-francis@...com>,
Aniket Limaye <a-limaye@...com>, Beleswar Padhi
<b-padhi@...com>
Subject: Re: [PATCH v4 1/5] arm64: dts: ti: Refactor J784s4 SoC files to a
common file
Hi Siddharth,
On 13:36-20240827, Siddharth Vadapalli wrote:
> On Mon, Aug 19, 2024 at 03:09:35PM +0530, 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.
> >
> > Reviewed-by: Beleswar Padhi <b-padhi@...com>
> > Signed-off-by: Manorit Chawdhry <m-chawdhry@...com>
> > ---
> > .../arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi | 149 +
> > .../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, 2913 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..958054ab1018
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-common.dtsi
> > @@ -0,0 +1,149 @@
> > +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> > +/*
> > + * Device Tree Source for J784S4 SoC Family
>
> Since the file is already named "...-j784s4-j742s2-...", wouldn't it be
> better to add J742S2 and the link to its TRM here itself rather than
> adding it in patch 4/5?
I would align this, thanks!
>
> > + *
> > + * TRM (SPRUJ43 JULY 2022): https://www.ti.com/lit/zip/spruj52
> > + *
> > + * Copyright (C) 2022-2024 Texas Instruments Incorporated - https://www.ti.com/
>
> Since this is a new file and not a moved file, should it simply be "2024"?
The content of this file is copyright since 2022 so it seemed
counter-intuitive to me to remove that copyright, if the flow is to
remove older copyright whenever file is moved then I can do it but need
to know what is followed here.
>
> > + *
> > + */
>
> [...]
>
> > +
> > +
> > + cbass_main: bus@...000 {
> > + bootph-all;
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges = <0x00 0x00100000 0x00 0x00100000 0x00 0x00020000>, /* ctrl mmr */
> > + <0x00 0x00600000 0x00 0x00600000 0x00 0x00031100>, /* GPIO */
> > + <0x00 0x00700000 0x00 0x00700000 0x00 0x00001000>, /* ESM */
> > + <0x00 0x01000000 0x00 0x01000000 0x00 0x0d000000>, /* Most peripherals */
>
> Since SERDES2 lies in the above range, to be techincally correct, the
> above range should be split up to move SERDES2 out of the common file.
It's a maintainance overhead as whatever support is being added to
j784s4 would have to be explicitely ported to j742s2 by adding ranges, I
know it's technically correct to add j742s2 ranges in a separate file
but it's not worth the maintainance that future development on both the
devices would be bringing so I think keeping it the same would be the
way to go.
>
> > + <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 should be dropped from the common file.
>
> > + <0x00 0x10000000 0x00 0x10000000 0x00 0x08000000>, /* PCIe0 DAT0 */
> > + <0x00 0x18000000 0x00 0x18000000 0x00 0x08000000>, /* PCIe1 DAT0 */
> > + <0x00 0x64800000 0x00 0x64800000 0x00 0x0070c000>, /* C71_1 */
> > + <0x00 0x65800000 0x00 0x65800000 0x00 0x0070c000>, /* C71_2 */
> > + <0x00 0x66800000 0x00 0x66800000 0x00 0x0070c000>, /* C71_3 */
> > + <0x00 0x67800000 0x00 0x67800000 0x00 0x0070c000>, /* C71_4 */
> > + <0x00 0x6f000000 0x00 0x6f000000 0x00 0x00310000>, /* A72 PERIPHBASE */
> > + <0x00 0x70000000 0x00 0x70000000 0x00 0x00400000>, /* MSMC RAM */
> > + <0x00 0x30000000 0x00 0x30000000 0x00 0x0c400000>, /* MAIN NAVSS */
> > + <0x40 0x00000000 0x40 0x00000000 0x01 0x00000000>, /* PCIe0 DAT1 */
> > + <0x41 0x00000000 0x41 0x00000000 0x01 0x00000000>, /* PCIe1 DAT1 */
> > + <0x42 0x00000000 0x42 0x00000000 0x01 0x00000000>, /* PCIe2 DAT1 */
> > + <0x43 0x00000000 0x43 0x00000000 0x01 0x00000000>, /* PCIe3 DAT1 */
> > + <0x44 0x00000000 0x44 0x00000000 0x00 0x08000000>, /* PCIe2 DAT0 */
> > + <0x44 0x10000000 0x44 0x10000000 0x00 0x08000000>, /* PCIe3 DAT0 */
>
> PCIe2 and PCIe3 should be dropped from the common file.
>
> > + <0x4e 0x20000000 0x4e 0x20000000 0x00 0x00080000>, /* GPU */
> > +
> > + /* MCUSS_WKUP Range */
> > + <0x00 0x28380000 0x00 0x28380000 0x00 0x03880000>,
> > + <0x00 0x40200000 0x00 0x40200000 0x00 0x00998400>,
> > + <0x00 0x40f00000 0x00 0x40f00000 0x00 0x00020000>,
> > + <0x00 0x41000000 0x00 0x41000000 0x00 0x00020000>,
>
> [...]
>
> > 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
> > new file mode 100644
> > index 000000000000..04d77c42442d
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-main-common.dtsi
> > @@ -0,0 +1,2667 @@
> > +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> > +/*
> > + * Device Tree Source for J784S4 and J742S2 SoC Family Main Domain peripherals
>
> Here, you have mentioned J742S2 as well, so to keep it consistent,
> J742S2 should be mentioned in the "k3-j784s4-j742s2-common.dtsi" file as
> well as I have indicated above.
>
> > + *
> > + * Copyright (C) 2022-2024 Texas Instruments Incorporated - https://www.ti.com/
>
> Since this is a new file and not a renamed file, shouldn't it be "2024"?
>
> > + */
> > +
> > +#include <dt-bindings/mux/mux.h>
> > +#include <dt-bindings/phy/phy.h>
> > +#include <dt-bindings/phy/phy-ti.h>
> > +
>
> [...]
>
> Regards,
> Siddharth.
Regards,
Manorit
Powered by blists - more mailing lists