Re: [PATCH v2 10/13] arm: dts: introduce am62p5 uboot dts files

Bryan Brattlof
2024-02-02T17:06:55Z

On February 2, 2024 thus sayeth Andrew Davis:
> On 2/2/24 10:28 AM, Bryan Brattlof wrote:
> > Hi Andrew!
> >
> > On February 2, 2024 thus sayeth Andrew Davis:
> > > On 1/31/24 9:06 PM, Bryan Brattlof wrote:
> > > > Include the uboot device tree files needed to boot the board.
> > > >
> > > > Signed-off-by: Bryan Brattlof <bb@ti.com>
> > > > ---
> > > > arch/arm/dts/Makefile | 2 +
> > > > arch/arm/dts/k3-am62p-ddr-lp4-50-1600.dtsi | 2800 ++++++++++++++++++++
> > > > arch/arm/dts/k3-am62p-sk-binman.dtsi | 173 ++
> > > > arch/arm/dts/k3-am62p5-r5-sk.dts | 101 +
> > > > arch/arm/dts/k3-am62p5-sk-u-boot.dtsi | 49 +
> > > > arch/arm/mach-k3/Makefile | 1 +
> > > > arch/arm/mach-k3/am62p5_init.c | 4 +-
> > > > dts/arch/arm64/ti | 1 +
> > > > 8 files changed, 3129 insertions(+), 2 deletions(-)
> > > > create mode 100644 arch/arm/dts/k3-am62p-ddr-lp4-50-1600.dtsi
> > > > create mode 100644 arch/arm/dts/k3-am62p-sk-binman.dtsi
> > > > create mode 100644 arch/arm/dts/k3-am62p5-r5-sk.dts
> > > > create mode 100644 arch/arm/dts/k3-am62p5-sk-u-boot.dtsi
> > > > create mode 120000 dts/arch/arm64/ti
> > > >
> >
> > ...
> >
> > > > diff --git a/arch/arm/dts/k3-am62p-ddr-lp4-50-1600.dtsi
> > > > b/arch/arm/dts/k3-am62p-ddr-lp4-50-1600.dtsi
> > > > new file mode 100644
> > > > index 0000000000000..f66435201530f
> > > > --- /dev/null
> > > > +++ b/arch/arm/dts/k3-am62p-ddr-lp4-50-1600.dtsi
> > > > @@ -0,0 +1,2800 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * This file was generated with the
> > > > + * AM62Px SysConfig DDR Subsystem Register Configuration Tool v0.10.02
> > > > + * Thu Jan 25 2024 10:43:46 GMT-0600 (Central Standard Time)
> > > > + * DDR Type: LPDDR4
> > > > + * F0 = 50MHz F1 = NA F2 = 1600MHz
> > > > + * Density (per channel): 16Gb
> > > > + * Number of Ranks: 2
> > > > + */
> > > > +
> > > > +#define DDRSS_PLL_FHS_CNT 5
> > > > +#define DDRSS_PLL_FREQUENCY_1 800000000
> > > > +#define DDRSS_PLL_FREQUENCY_2 800000000
> > > > +#define DDRSS_SDRAM_IDX 17
> > > > +#define DDRSS_REGION_IDX 17
> > > > +
> > > > +#define DDRSS_CTL_0_DATA 0x00000B00
> > > > +#define DDRSS_CTL_1_DATA 0x00000000
> > > > +#define DDRSS_CTL_2_DATA 0x00000000
> > > > +#define DDRSS_CTL_3_DATA 0x00000000
> > > > +#define DDRSS_CTL_4_DATA 0x00000000
> > > > +#define DDRSS_CTL_5_DATA 0x00000000
> > >
> > > I wonder if it would be better to generate this file
> > > during build..
> > >
> >
> > Yeah we can save a lot of space by removing tons of these registers
> > writing 0x0 to the reserved ranges... The section for the different PHYs
> > is particularly aggravating.
> >
>
> Might be a silly question, what happens if you don't write these 0x0
> at all, is there a sane default of 0x0, or is it random on powerup?
>

The vast majority (especially the spaces between the PHYs) are just
holes in the bus that find their way to a black hole when we write to
it. Nothing bad will happen. There are some shadow registers we need to
be careful about but those aren't hard to work around before we start
the controller.

We'd just need to refactor how these registers are copied into the
controller, right now it's something dumb like

for(offset=0, offset<num_regs, offset++)
writel(rev_val, base + offset);

for the entire controller reg range.

I wanted to see if I could get this down to a memcopy plus a readl to
flush the bus writes to see how fast I could make this :)

Carving out each sub reg range will really save space and time as well.

> > I told Simon and Marex last year I'd get to it once I find some free
> > cycles and then I went and had a son ;) but it's still in the backlog
> >
>
> Know the feeling :)
>
> Andrew
>
> > >
> > > > +&main_bcdma {
> > > > + reg = <0x00 0x485c0100 0x00 0x100>,
> > > > + <0x00 0x4c000000 0x00 0x20000>,
> > > > + <0x00 0x4a820000 0x00 0x20000>,
> > > > + <0x00 0x4aa40000 0x00 0x20000>,
> > > > + <0x00 0x4bc00000 0x00 0x100000>,
> > > > + <0x00 0x48600000 0x00 0x8000>,
> > > > + <0x00 0x484a4000 0x00 0x2000>,
> > > > + <0x00 0x484c2000 0x00 0x2000>;
> > > > + reg-names = "gcfg", "bchanrt", "rchanrt", "tchanrt",
> > > > + "ringrt", "cfg", "tchan", "rchan";
> > > > +};
> > > > +
> > >
> > > Do we still need these? Thought they got fixed kernel side already.
> >
> > Ah you're right I missed that i'll remove this in v2
> >
> > >
> > > > +&main_pktdma {
> > > > + reg = <0x00 0x485c0000 0x00 0x100>,
> > > > + <0x00 0x4a800000 0x00 0x20000>,
> > > > + <0x00 0x4aa00000 0x00 0x40000>,
> > > > + <0x00 0x4b800000 0x00 0x400000>,
> > > > + <0x00 0x485e0000 0x00 0x20000>,
> > > > + <0x00 0x484a0000 0x00 0x4000>,
> > > > + <0x00 0x484c0000 0x00 0x2000>,
> > > > + <0x00 0x48430000 0x00 0x4000>;
> > > > + reg-names = "gcfg", "rchanrt", "tchanrt", "ringrt",
> > > > + "cfg", "tchan", "rchan", "rflow";
> > > > +};
> > > > diff --git a/arch/arm/mach-k3/Makefile b/arch/arm/mach-k3/Makefile
> > > > index 42161376469e2..820b313a83c23 100644
> > > > --- a/arch/arm/mach-k3/Makefile
> > > > +++ b/arch/arm/mach-k3/Makefile
> > > > @@ -20,5 +20,6 @@ obj-$(CONFIG_SOC_K3_J721S2) += j721s2_init.o
> > > > obj-$(CONFIG_SOC_K3_AM642) += am642_init.o
> > > > obj-$(CONFIG_SOC_K3_AM625) += am625_init.o
> > > > obj-$(CONFIG_SOC_K3_AM62A7) += am62a7_init.o
> > > > +obj-$(CONFIG_SOC_K3_AM62P5) += am62p5_init.o
> > > > endif
> > > > obj-y += common.o security.o
> > > > diff --git a/arch/arm/mach-k3/am62p5_init.c b/arch/arm/mach-k3/am62p5_init.c
> > > > index 5b6795cc7d246..9ff877d5d26e8 100644
> > > > --- a/arch/arm/mach-k3/am62p5_init.c
> > > > +++ b/arch/arm/mach-k3/am62p5_init.c
> > > > @@ -110,7 +110,7 @@ void board_init_f(ulong dummy)
> > > > * through a SYSFW PM-init step and will need a re-init in some way
> > > > * due to changing module clock frequencies.
> > > > */
> > > > - if (IS_ENABLED(CONFIG_K3_EARLY_CONS))
> > > > + if (IS_ENABLED(CONFIG_K3_EARLY_CONS)) {
> > >
> > > Looks like fixes for a previous patch that didn't get squashed.
> > >
> >
> > haha Oops good catch.. I'll fixup this properly
> >
> >
> > Thanks for the review Andrew
> > ~Bryan