From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932170AbcEMMKY (ORCPT ); Fri, 13 May 2016 08:10:24 -0400 Received: from mout.kundenserver.de ([212.227.126.133]:54191 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752058AbcEMMKW (ORCPT ); Fri, 13 May 2016 08:10:22 -0400 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Jisheng Zhang , sebastian.hesselbarth@gmail.com, davem@davemloft.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] net: mv643xx_eth: use {readl|writel}_relaxed instead of readl/writel Date: Fri, 13 May 2016 14:09:43 +0200 Message-ID: <3966837.tFk2PD8QFb@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1463140760-7128-2-git-send-email-jszhang@marvell.com> References: <1463140760-7128-1-git-send-email-jszhang@marvell.com> <1463140760-7128-2-git-send-email-jszhang@marvell.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:/H+aCT7/Cv7n0vjvqg2uLfpvSCX3Bp8B77F7GbNKo/xi9GyAEAI AXHua42xEDLFWVmc1fmFYgHZKLGeUxDBkpuJbBmVASprzRsuSZ5zUr0If/66WWcujK1WaTd zPQDUzVXSZQoTqMy4tqNUGfs2lzik16a4Vp/53p3d3zSrpUZuzQKns2Wl9ZvgQYg7arC1pa RAi7cDUSouT/j/FmbeElA== X-UI-Out-Filterresults: notjunk:1;V01:K0:eAkp2pHUb5U=:eAmPkTzAT/kgQJfBhkx+5f jfwGg+bJI84XjQOpcNF5gW69PvYNplslOnAExDpuhUo1n8t6AD9yEQBznq86cifix9sidixmh pENslD/j1qfYA2Lh5+VO0daKotolXpTaUDtubKsNGc7+N87pa7lvc+E+wrZe1AGMuf5/BlUO8 OTwF6m3GEf8xO2K56IT+mOqWzrvGY9GUoAsGKt2jst8fh8Lh5j6NXQV1jlFt85W0BobZ7ZNBw HwTQESzElscA1q4UA+GaBJ6L0M9C171db+EcqhGLEFNv2qn4kj0KPpYIKSZH2Fb5SWCXLxdvm Sau6EhRPf6pHGWcFjWgSJHhT9Xy8f8rpr7ukI/W9hCJmoyJQZq9nDIxU89B4DIY6C1Kh3eq70 nmmeQFxHm4y4dJsNsXMkFo2gYpbuCkJGMA/mZOEv91jkxmPdpABpCPvhVTa+/hhjZj8qwHazo CqqhwwC67nmZ0I0XTHpoGG86w0v0g1pEiviGIf8j7WIJE64jcZPwDQlODD5bmlrqG7z1UNems hq7S3tdAOdKAdO3IGcZ8x6xuCryINAY5yMKB+/PoY+mnEPj9mv7aTVISoxQxHx6BMyE+THnA2 LdmO3Cyx/l1l34Vxd8TsVgk/4VtvjdpxcJKWvjt9obw3+8/ka4gTpzxRKlSnlmCwt05siBpeS BlWN5CMtvODZVnYVKpxRF/uDgMKBNDSVkgh6AxsrERYLBtltycTsl1HlcIA/AkPZx51Z/4jw5 WFX1v2zLTwkJaZhx Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 13 May 2016 19:59:19 Jisheng Zhang wrote: > /* port register accessors **************************************************/ > static inline u32 rdl(struct mv643xx_eth_private *mp, int offset) > { > - return readl(mp->shared->base + offset); > + return readl_relaxed(mp->shared->base + offset); > } > > static inline u32 rdlp(struct mv643xx_eth_private *mp, int offset) > { > - return readl(mp->base + offset); > + return readl_relaxed(mp->base + offset); > } I'd recommend not changing these in general, but introducing new 'rdl_relaxed()' and 'rdlp_relaxed()' etc variants that you use in the code that actually is performance sensitive, but use the normal non-relaxed versions by default. Then add a comment to each use of the relaxed accessors on how you know that it's safe for that caller. This usually is just needed for the xmit() function and for the interrupt handler. > @@ -2642,10 +2642,10 @@ mv643xx_eth_conf_mbus_windows(struct mv643xx_eth_shared_private *msp, > int i; > > for (i = 0; i < 6; i++) { > - writel(0, base + WINDOW_BASE(i)); > - writel(0, base + WINDOW_SIZE(i)); > + writel_relaxed(0, base + WINDOW_BASE(i)); > + writel_relaxed(0, base + WINDOW_SIZE(i)); > if (i < 4) > - writel(0, base + WINDOW_REMAP_HIGH(i)); > + writel_relaxed(0, base + WINDOW_REMAP_HIGH(i)); > } > > win_enable = 0x3f; Configuring the mbus for instance is clearly not an operation in which performance matters at all, so better not touch that. > @@ -2674,8 +2675,8 @@ static void infer_hw_params(struct mv643xx_eth_shared_private *msp) > * [21:8], or a 16-bit coal limit in bits [25,21:7] of the > * SDMA config register. > */ > - writel(0x02000000, msp->base + 0x0400 + SDMA_CONFIG); > - if (readl(msp->base + 0x0400 + SDMA_CONFIG) & 0x02000000) > + writel_relaxed(0x02000000, msp->base + 0x0400 + SDMA_CONFIG); > + if (readl_relaxed(msp->base + 0x0400 + SDMA_CONFIG) & 0x02000000) > msp->extended_rx_coal_limit = 1; > else > msp->extended_rx_coal_limit = 0; This also seems to be configuration, rather than in the packet rx/tx hotpath. Arnd