ARM Sunxi Platform Development
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@arm.com>
To: Jagan Teki <jagan@amarulasolutions.com>,
	Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Gunjan Gupta <viraniac@gmail.com>,
	u-boot@lists.denx.de, linux-sunxi@lists.linux.dev
Subject: [PATCH 4/4] sunxi: DRAM: H6: use proper MMIO accessors in mctl_set_addrmap()
Date: Sat, 21 Oct 2023 02:10:25 +0100	[thread overview]
Message-ID: <20231021011025.568-5-andre.przywara@arm.com> (raw)
In-Reply-To: <20231021011025.568-1-andre.przywara@arm.com>

For accessing MMIO registers, we must not rely on the compiler to
realise every access to a struct which we made point to some MMIO base
address. From a compiler's point of view, those writes could be
considered pointless, since no code consumes them later on: the compiler
would actually be free to optimise them away.

So we need at least the "volatile" attribute in the pointer declaration,
but a better fix is of course to use the proper MMIO accessors (writel),
as we do everywhere else.
Since MMIO writes are already ordered within themselves (courtesy of the
"device nGnRnE" memory attribures), and we don't do any DMA operation
which would requrire synchronising with normal memory accesses, we can
use the cheaper writel_relaxed() accessor, which have the additional
advantange of saving one instruction, for each call.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/mach-sunxi/dram_sun50i_h6.c | 69 ++++++++++++++++------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
index 8e959f4c600..89e855c1a7d 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
@@ -192,77 +192,86 @@ static void mctl_set_addrmap(const struct dram_config *config)
 
 	/* Ranks */
 	if (ranks == 2)
-		mctl_ctl->addrmap[0] = rows + cols - 3;
+		writel_relaxed(rows + cols - 3, &mctl_ctl->addrmap[0]);
 	else
-		mctl_ctl->addrmap[0] = 0x1F;
+		writel_relaxed(0x1f, &mctl_ctl->addrmap[0]);
 
 	/* Banks, hardcoded to 8 banks now */
-	mctl_ctl->addrmap[1] = (cols - 2) | (cols - 2) << 8 | (cols - 2) << 16;
+	writel_relaxed((cols - 2) | (cols - 2) << 8 | (cols - 2) << 16,
+		       &mctl_ctl->addrmap[1]);
 
 	/* Columns */
-	mctl_ctl->addrmap[2] = 0;
+	writel_relaxed(0, &mctl_ctl->addrmap[2]);
 	switch (cols) {
 	case 7:
-		mctl_ctl->addrmap[3] = 0x1F1F1F00;
-		mctl_ctl->addrmap[4] = 0x1F1F;
+		writel_relaxed(0x1f1f1f00, &mctl_ctl->addrmap[3]);
+		writel_relaxed(0x1f1f, &mctl_ctl->addrmap[4]);
 		break;
 	case 8:
-		mctl_ctl->addrmap[3] = 0x1F1F0000;
-		mctl_ctl->addrmap[4] = 0x1F1F;
+		writel_relaxed(0x1f1f0000, &mctl_ctl->addrmap[3]);
+		writel_relaxed(0x1f1f, &mctl_ctl->addrmap[4]);
 		break;
 	case 9:
-		mctl_ctl->addrmap[3] = 0x1F000000;
-		mctl_ctl->addrmap[4] = 0x1F1F;
+		writel_relaxed(0x1f000000, &mctl_ctl->addrmap[3]);
+		writel_relaxed(0x1f1f, &mctl_ctl->addrmap[4]);
 		break;
 	case 10:
-		mctl_ctl->addrmap[3] = 0;
-		mctl_ctl->addrmap[4] = 0x1F1F;
+		writel_relaxed(0, &mctl_ctl->addrmap[3]);
+		writel_relaxed(0x1f1f, &mctl_ctl->addrmap[4]);
 		break;
 	case 11:
-		mctl_ctl->addrmap[3] = 0;
-		mctl_ctl->addrmap[4] = 0x1F00;
+		writel_relaxed(0, &mctl_ctl->addrmap[3]);
+		writel_relaxed(0x1f00, &mctl_ctl->addrmap[4]);
 		break;
 	case 12:
-		mctl_ctl->addrmap[3] = 0;
-		mctl_ctl->addrmap[4] = 0;
+		writel_relaxed(0, &mctl_ctl->addrmap[3]);
+		writel_relaxed(0, &mctl_ctl->addrmap[4]);
 		break;
 	default:
 		panic("Unsupported DRAM configuration: column number invalid\n");
 	}
 
 	/* Rows */
-	mctl_ctl->addrmap[5] = (cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24);
+	writel_relaxed((cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24),
+		       &mctl_ctl->addrmap[5]);
 	switch (rows) {
 	case 13:
-		mctl_ctl->addrmap[6] = (cols - 3) | 0x0F0F0F00;
-		mctl_ctl->addrmap[7] = 0x0F0F;
+		writel_relaxed((cols - 3) | 0x0f0f0f00, &mctl_ctl->addrmap[6]);
+		writel_relaxed(0x0f0f, &mctl_ctl->addrmap[7]);
 		break;
 	case 14:
-		mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 3) << 8) | 0x0F0F0000;
-		mctl_ctl->addrmap[7] = 0x0F0F;
+		writel_relaxed((cols - 3) | ((cols - 3) << 8) | 0x0f0f0000,
+			       &mctl_ctl->addrmap[6]);
+		writel_relaxed(0x0f0f, &mctl_ctl->addrmap[7]);
 		break;
 	case 15:
-		mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | 0x0F000000;
-		mctl_ctl->addrmap[7] = 0x0F0F;
+		writel_relaxed((cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | 0x0f000000,
+				&mctl_ctl->addrmap[6]);
+		writel_relaxed(0x0f0f, &mctl_ctl->addrmap[7]);
 		break;
 	case 16:
-		mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24);
-		mctl_ctl->addrmap[7] = 0x0F0F;
+		writel_relaxed((cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24),
+			       &mctl_ctl->addrmap[6]);
+		writel_relaxed(0x0f0f, &mctl_ctl->addrmap[7]);
 		break;
 	case 17:
-		mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24);
-		mctl_ctl->addrmap[7] = (cols - 3) | 0x0F00;
+		writel_relaxed((cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24),
+			       &mctl_ctl->addrmap[6]);
+		writel_relaxed((cols - 3) | 0x0f00, &mctl_ctl->addrmap[7]);
 		break;
 	case 18:
-		mctl_ctl->addrmap[6] = (cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24);
-		mctl_ctl->addrmap[7] = (cols - 3) | ((cols - 3) << 8);
+		writel_relaxed((cols - 3) | ((cols - 3) << 8) | ((cols - 3) << 16) | ((cols - 3) << 24),
+			       &mctl_ctl->addrmap[6]);
+		writel_relaxed((cols - 3) | ((cols - 3) << 8),
+			       &mctl_ctl->addrmap[7]);
 		break;
 	default:
 		panic("Unsupported DRAM configuration: row number invalid\n");
 	}
 
 	/* Bank groups, DDR4 only */
-	mctl_ctl->addrmap[8] = 0x3F3F;
+	writel_relaxed(0x3f3f, &mctl_ctl->addrmap[8]);
+	dsb();
 }
 
 static void mctl_com_init(const struct dram_para *para,
-- 
2.35.8


  parent reply	other threads:[~2023-10-21  1:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-21  1:10 [PATCH 0/4] sunxi: DRAM: H6: fixes and size reduction Andre Przywara
2023-10-21  1:10 ` [PATCH 1/4] sunxi: DRAM: H6: add barrier after finishing DRAM setup Andre Przywara
2023-10-28 10:29   ` Gunjan Gupta
2023-10-21  1:10 ` [PATCH 2/4] sunxi: DRAM: H6: const-ify DRAM function parameters Andre Przywara
2023-10-21  5:52   ` Jernej Škrabec
2023-10-21  1:10 ` [PATCH 3/4] sunxi: DRAM: H6: split struct dram_para Andre Przywara
2023-10-21  5:56   ` Jernej Škrabec
2023-10-21  1:10 ` Andre Przywara [this message]
2023-10-21  5:57   ` [PATCH 4/4] sunxi: DRAM: H6: use proper MMIO accessors in mctl_set_addrmap() Jernej Škrabec

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=20231021011025.568-5-andre.przywara@arm.com \
    --to=andre.przywara@arm.com \
    --cc=jagan@amarulasolutions.com \
    --cc=jernej.skrabec@gmail.com \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=u-boot@lists.denx.de \
    --cc=viraniac@gmail.com \
    /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