From: Maxime Ripard Date: Thu, 6 Dec 2018 08:42:30 +0100 Subject: [U-Boot] [PATCH] sunxi: H6: DRAM: avoid memcpy() on MMIO registers In-Reply-To: <20181206000220.2302-1-andre.przywara@arm.com> References: <20181206000220.2302-1-andre.przywara@arm.com> Message-ID: <20181206074230.rcurmal3454kjucf@flea> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thu, Dec 06, 2018 at 12:02:20AM +0000, Andre Przywara wrote: > Using memcpy() for MMIO operations is, however tempting, not a good idea: > It depends on the specific implementation of memcpy, also lacks barriers. > In this particular case the first registers were written using 64-bit > writes, and the last register using four separate single-byte writes. > Neither is what we actually want. > We get away with it in this case because of the particular details of > the bus implementation, the DRAM controller IP and the values that we > actually write, but we should not leave a bad example around. > > Replace the memcpy with a proper loop using the writel() accessor. > > Signed-off-by: Andre Przywara > --- > arch/arm/mach-sunxi/dram_sun50i_h6.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c > index 5da90a2835..e2f141eb9b 100644 > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c > @@ -182,6 +182,7 @@ static void mctl_set_timing_lpddr3(struct dram_para *para) > (struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE; > struct sunxi_mctl_phy_reg * const mctl_phy = > (struct sunxi_mctl_phy_reg *)SUNXI_DRAM_PHY0_BASE; > + int i; > > u8 tccd = 2; > u8 tfaw = max(ns_to_t(50), 4); > @@ -237,8 +238,9 @@ static void mctl_set_timing_lpddr3(struct dram_para *para) > u8 twr2rd = tcwl + 4 + 1 + twtr; > u8 trd2wr = tcl + 4 + (tcksrea >> 1) - tcwl + 1; > > - /* set mode register */ > - memcpy(mctl_phy->mr, mr_lpddr3, sizeof(mr_lpddr3)); > + /* set mode registers */ > + for (i = 0; i < ARRAY_SIZE(mr_lpddr3); i++) > + writel(mr_lpddr3[i], &mctl_phy->mr[i]); memcpy_toio is meant to do just that. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 228 bytes Desc: not available URL: