From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: Paul Kocialkowski <paulk@sys-base.io>
Cc: u-boot@lists.denx.de, Andre Przywara <andre.przywara@arm.com>,
Chen-Yu Tsai <wens@kernel.org>,
linux-sunxi@lists.linux.dev
Subject: Re: [PATCH v2 1/2] sunxi: A523: Move NSI init routine into generic function
Date: Sat, 09 May 2026 10:53:55 +0200 [thread overview]
Message-ID: <Bl4zQkuXTByJdq97nqmfwg@gmail.com> (raw)
In-Reply-To: <af3Xpkw8_Ux2czAh@shepard>
Dne petek, 8. maj 2026 ob 14:31:34 Srednjeevropski poletni čas je Paul Kocialkowski napisal(a):
> Hi,
>
> Thanks again for picking this up!
>
> On Sun 03 May 26, 10:07, Jernej Škrabec wrote:
> > Dne četrtek, 30. april 2026 ob 15:58:37 Srednjeevropski poletni čas je Andre Przywara napisal(a):
> > > In previous generations of Allwinner SoCs, the memory bus (MBUS) access
> > > arbitration was configured as part of the DRAM top registers. This is no
> > > longer the case with for instance the A133 or A523, which have a dedicated
> > > base address for the bus arbiter that is now called NSI instead of MBUS.
> > >
> > > NSI appears to be a later iteration of MBUS design, with new dedicated
> > > registers that resemble the previous MBUS ones. Despite NSI not being
> > > documented in the manual, the A133 BSP includes a nsi driver with some
> > > description of the registers. Like previous generations, it implements
> > > port arbitration priority for DRAM access and also supports an optional
> > > QoS mode based on bandwidth limits.
> > >
> > > In preparation for re-using code for other SoCs, factor out the existing
> > > NSI init routine from the A523 DRAM code, which was a bit ad-hoc and A523
> > > specific, into a separate function, and abstract the settings a bit.
> > > No functional change.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > Co-develeoped-by: Paul Kocialkowski <paulk@sys-base.io>
> > > Suggested-by: Jernej Škrabec <jernej.skrabec@gmail.com>
> > > Sponsored-by: MEC Electronics GmbH <https://www.mec.at/>
> > > ---
> > > .../include/asm/arch-sunxi/cpu_sunxi_ncat2.h | 1 +
> > > .../include/asm/arch-sunxi/dram_sun55i_a523.h | 29 +++++++++++
> > > arch/arm/include/asm/arch-sunxi/sunxi_nsi.h | 25 ++++++++++
> > > arch/arm/mach-sunxi/Makefile | 2 +-
> > > arch/arm/mach-sunxi/dram_sun55i_a523.c | 49 +++++++++----------
> > > arch/arm/mach-sunxi/sunxi_nsi.c | 31 ++++++++++++
> > > 6 files changed, 110 insertions(+), 27 deletions(-)
> > > create mode 100644 arch/arm/include/asm/arch-sunxi/sunxi_nsi.h
> > > create mode 100644 arch/arm/mach-sunxi/sunxi_nsi.c
> > >
> > > diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h b/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h
> > > index 7cee7efe8b4..fd7d9b22058 100644
> > > --- a/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h
> > > +++ b/arch/arm/include/asm/arch-sunxi/cpu_sunxi_ncat2.h
> > > @@ -9,6 +9,7 @@
> > >
> > > #define SUNXI_TZPC_BASE 0x02000800
> > > #define SUNXI_CCM_BASE 0x02001000
> > > +#define SUNXI_NSI_BASE 0x02020000
> > > #define SUNXI_TIMER_BASE 0x02050000
> > >
> > > #define SUNXI_TWI0_BASE 0x02502000
> > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun55i_a523.h b/arch/arm/include/asm/arch-sunxi/dram_sun55i_a523.h
> > > index 08bfe462856..462d4726a21 100644
> > > --- a/arch/arm/include/asm/arch-sunxi/dram_sun55i_a523.h
> > > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun55i_a523.h
> > > @@ -20,6 +20,35 @@ enum sunxi_dram_type {
> > > #define MCTL_COM_UNK_008 0x008
> > > #define MCTL_COM_MAER0 0x020
> > >
> > > +enum sunxi_nsi_port {
> > > + SUNXI_NSI_PORT_GPU = 0,
> > > + SUNXI_NSI_PORT_GIC, = 1,
> > > + SUNXI_NSI_PORT_USB3, = 2,
> > > + SUNXI_NSI_PORT_PCIE, = 3,
> > > + SUNXI_NSI_PORT_CE, = 4,
> > > + SUNXI_NSI_PORT_NPU, = 5,
> > > + SUNXI_NSI_PORT_ISP, = 6,
> > > + SUNXI_NSI_PORT_DSP, = 7,
> > > + SUNXI_NSI_PORT_G2D, = 8,
> > > + SUNXI_NSI_PORT_DI, = 9,
> > > + SUNXI_NSI_PORT_IOMMU, = 10,
> > > + SUNXI_NSI_PORT_VE_R, = 11,
> > > + SUNXI_NSI_PORT_VE_RW, = 12,
> > > + SUNXI_NSI_PORT_DE, = 13,
> > > + SUNXI_NSI_PORT_CSI, = 14,
> > > + SUNXI_NSI_PORT_GMAC0, = 18,
> > > + SUNXI_NSI_PORT_GMAC1, = 19,
> > > + SUNXI_NSI_PORT_MMC0, = 20,
> > > + SUNXI_NSI_PORT_MMC1, = 21,
> > > + SUNXI_NSI_PORT_MMC2, = 22,
> > > + SUNXI_NSI_PORT_USB0, = 23,
> > > + SUNXI_NSI_PORT_USB1, = 24,
> > > + SUNXI_NSI_PORT_USB2, = 25,
> > > + SUNXI_NSI_PORT_NPD, = 26,
> > > + SUNXI_NSI_PORT_DMAC, = 27,
> > > + SUNXI_NSI_PORT_DMA, = 28,
> > > +};
> > > +
> > > /*
> > > * Controller registers seems to be the same or at least very similar
> > > * to those in H6.
> > > diff --git a/arch/arm/include/asm/arch-sunxi/sunxi_nsi.h b/arch/arm/include/asm/arch-sunxi/sunxi_nsi.h
> > > new file mode 100644
> > > index 00000000000..7d41f9318b5
> > > --- /dev/null
> > > +++ b/arch/arm/include/asm/arch-sunxi/sunxi_nsi.h
> > > @@ -0,0 +1,25 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * A133/A523 NSI interconnect register and constant defines
> > > + *
> > > + * (C) Copyright 2026 Arm Ltd.
> > > + */
> > > +
> > > +#ifndef SUNXI_NSI_H__
> > > +#define SUNXI_NSI_H__
> > > +
> > > +#define SUNXI_NSI_PRI_CFG_LOWEST 0
> > > +#define SUNXI_NSI_PRI_CFG_LOW 1
> > > +#define SUNXI_NSI_PRI_CFG_HIGH 2
> > > +#define SUNXI_NSI_PRI_CFG_HIGHEST 3
> > > +
> > > +#define SUNXI_NSI_IO_CFG_QOS_SEL_OUTPUT 0
> > > +#define SUNXI_NSI_IO_CFG_QOS_SEL_INPUT 1
> > > +
> > > +#define NSI_CONF(port, pri, qos_sel) \
> > > + { SUNXI_NSI_PORT_ ## port, SUNXI_NSI_PRI_CFG_ ## pri, \
> > > + SUNXI_NSI_IO_CFG_QOS_SEL_ ## qos_sel }
> > > +
> > > +void nsi_configure_port(unsigned int port, u8 pri, u8 qos_sel);
> > > +
> > > +#endif /* SUNXI_NSI_H__ */
> > > diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
> > > index 0bee04d660f..3ef0113ea43 100644
> > > --- a/arch/arm/mach-sunxi/Makefile
> > > +++ b/arch/arm/mach-sunxi/Makefile
> > > @@ -50,6 +50,6 @@ obj-$(CONFIG_DRAM_SUN50I_H616) += dram_sun50i_h616.o dram_dw_helpers.o
> > > obj-$(CONFIG_DRAM_SUN50I_H616) += dram_timings/
> > > obj-$(CONFIG_DRAM_SUN50I_A133) += dram_sun50i_a133.o
> > > obj-$(CONFIG_DRAM_SUN50I_A133) += dram_timings/
> > > -obj-$(CONFIG_MACH_SUN55I_A523) += dram_sun55i_a523.o dram_dw_helpers.o
> > > +obj-$(CONFIG_MACH_SUN55I_A523) += dram_sun55i_a523.o dram_dw_helpers.o sunxi_nsi.o
> > > obj-$(CONFIG_DRAM_SUN55I_A523) += dram_timings/
> > > endif
> > > diff --git a/arch/arm/mach-sunxi/dram_sun55i_a523.c b/arch/arm/mach-sunxi/dram_sun55i_a523.c
> > > index 1ffb62863e2..9fb054cea84 100644
> > > --- a/arch/arm/mach-sunxi/dram_sun55i_a523.c
> > > +++ b/arch/arm/mach-sunxi/dram_sun55i_a523.c
> > > @@ -15,6 +15,7 @@
> > > #include <asm/arch/dram_dw_helpers.h>
> > > #include <asm/arch/cpu.h>
> > > #include <asm/arch/prcm.h>
> > > +#include <asm/arch/sunxi_nsi.h>
> > > #include <linux/bitops.h>
> > > #include <linux/delay.h>
> > >
> > > @@ -1412,40 +1413,36 @@ static const struct dram_para para = {
> > > .tpr10 = CONFIG_DRAM_SUNXI_TPR10,
> > > };
> > >
> > > -static void sunxi_nsi_init(void)
> > > +static void nsi_set_master_priority(void)
> > > {
> > > - /* IOMMU prio 3 */
> > > - writel(0x1, 0x02021418);
> > > - writel(0xf, 0x02021414);
> > > - /* DE prio 2 */
> > > - writel(0x1, 0x02021a18);
> > > - writel(0xa, 0x02021a14);
> > > - /* VE R prio 2 */
> > > - writel(0x1, 0x02021618);
> > > - writel(0xa, 0x02021614);
> > > - /* VE RW prio 2 */
> > > - writel(0x1, 0x02021818);
> > > - writel(0xa, 0x02021814);
> > > - /* ISP prio 2 */
> > > - writel(0x1, 0x02020c18);
> > > - writel(0xa, 0x02020c14);
> > > - /* CSI prio 2 */
> > > - writel(0x1, 0x02021c18);
> > > - writel(0xa, 0x02021c14);
> > > - /* NPU prio 2 */
> > > - writel(0x1, 0x02020a18);
> > > - writel(0xa, 0x02020a14);
> > > + struct {
> > > + unsigned int port;
> > > + u8 pri;
> > > + u8 qos_sel;
> >
> > What about introducing above struct in sunxi_nsi.h? I imagine same pattern
> > will be used in future code.
>
> Sounds like a good idea!
>
> > > + } ports[] = {
> > > + NSI_CONF(NPU, HIGH, INPUT),
> > > + NSI_CONF(ISP, HIGH, INPUT),
> > > + NSI_CONF(IOMMU, HIGHEST, INPUT),
> > > + NSI_CONF(VE_R, HIGH, INPUT),
> > > + NSI_CONF(VE_RW, HIGH, INPUT),
> > > + NSI_CONF(DE, HIGH, INPUT),
> > > + NSI_CONF(CSI, HIGH, INPUT),
> > > + };
> > > + unsigned int i;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(ports); i++)
> > > + nsi_configure_port(ports[i].port, ports[i].pri,
> > > + ports[i].qos_sel);
> > >
> > > /* close ra0 autogating */
> > > - writel(0x0, 0x02023c00);
> > > + writel(0x0, 0x02023c00); /* port 30 */
> > > /* close ta autogating */
> > > - writel(0x0, 0x02023e00);
> > > + writel(0x0, 0x02023e00); /* port 31 */
> > > /* close pcie autogating */
> > > writel(0x0, 0x02020600);
> >
> > Use SUNXI_NSI_BASE + ... above
>
> We should probably introduce proper defines for this. From what I understand
> the RA and TA are special ports but they share the same layout for the
> autogating register. So something like:
>
> SUNXI_NSI_PORT_RA, = 30,
> SUNXI_NSI_PORT_TA, = 31,
>
> Then a definition for:
>
> #define SUNXI_NSI_AUTOGATING_REG(i) ((i) * 0x200)
>
> And a helper like:
> void nsi_configure_port_autogating(unsigned int port)
>
> To set the reg to zero and "close" autogating.
>
> What do you think?
Yep, that looks the cleanest.
Best regards,
Jernej
> > > }
> > >
> > > static void init_something(void)
> > > -
> > > {
> > > u32 *ptr = (u32 *)0x02000804;
> > >
> > > @@ -1507,7 +1504,7 @@ unsigned long sunxi_dram_init(void)
> > >
> > > size = mctl_calc_size(&config);
> > >
> > > - sunxi_nsi_init();
> > > + nsi_set_master_priority();
> > > init_something();
> > >
> > > return size;
> > > diff --git a/arch/arm/mach-sunxi/sunxi_nsi.c b/arch/arm/mach-sunxi/sunxi_nsi.c
> > > new file mode 100644
> > > index 00000000000..3d7eb43df46
> > > --- /dev/null
> > > +++ b/arch/arm/mach-sunxi/sunxi_nsi.c
> > > @@ -0,0 +1,31 @@
> > > +#include <asm/io.h>
> > > +#include <asm/arch/cpu.h>
> > > +#include <asm/arch/sunxi_nsi.h>
> > > +
> > > +#define SUNXI_NSI_MODE_REG(i) ((i) * 0x200 + 0x10)
> > > +#define SUNXI_NSI_PRI_CFG_REG(i) ((i) * 0x200 + 0x14)
> > > +#define SUNXI_NSI_PRI_CFG_RD(v) (((v) & 0x3) << 2)
> > > +#define SUNXI_NSI_PRI_CFG_WR(v) ((v) & 0x3)
> > > +#define SUNXI_NSI_IO_CFG_REG(i) ((i) * 0x200 + 0x18)
> > > +#define SUNXI_NSI_ENABLE_REG(i) ((i) * 0x200 + 0xc0)
>
> I'd move this to the header unless there's a particular reason to not
> include it there.
>
> All the best,
>
> Paul
>
> > > +
> > > +void nsi_configure_port(unsigned int port, u8 pri, u8 qos_sel)
> > > +{
> > > + void *base = (void *)SUNXI_NSI_BASE;
> > > + u32 pri_cfg;
> > > +
> > > + /* QoS with bandwidth limits is not supported, disable it. */
> > > + writel(0, base + SUNXI_NSI_MODE_REG(port));
> > > + writel(0, base + SUNXI_NSI_ENABLE_REG(port));
> > > +
> > > + /*
> > > + * QoS direction selection should not be in use, but set it nevertheless
> > > + * to match the BSP behavior (in case it has some other meaning).
> > > + */
> > > + writel(qos_sel, base + SUNXI_NSI_IO_CFG_REG(port));
> > > +
> > > + /* Port priority is always active. */
> > > + pri_cfg = SUNXI_NSI_PRI_CFG_RD(pri) | SUNXI_NSI_PRI_CFG_WR(pri);
> > > +
> > > + writel(pri_cfg, base + SUNXI_NSI_PRI_CFG_REG(port));
> > > +}
> > >
> >
> >
> >
> >
>
>
next prev parent reply other threads:[~2026-05-09 8:53 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-30 13:58 [PATCH v2 0/2] sunxi: DRAM: rework NSI priority settings Andre Przywara
2026-04-30 13:58 ` [PATCH v2 1/2] sunxi: A523: Move NSI init routine into generic function Andre Przywara
2026-05-03 8:07 ` Jernej Škrabec
2026-05-08 12:31 ` Paul Kocialkowski
2026-05-09 8:53 ` Jernej Škrabec [this message]
2026-05-06 22:36 ` Andre Przywara
2026-04-30 13:58 ` [PATCH v2 2/2] sunxi: A133: dram: Add NSI arbiter configuration support Andre Przywara
2026-05-08 12:25 ` Paul Kocialkowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Bl4zQkuXTByJdq97nqmfwg@gmail.com \
--to=jernej.skrabec@gmail.com \
--cc=andre.przywara@arm.com \
--cc=linux-sunxi@lists.linux.dev \
--cc=paulk@sys-base.io \
--cc=u-boot@lists.denx.de \
--cc=wens@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox