From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754959AbcGEM6r (ORCPT ); Tue, 5 Jul 2016 08:58:47 -0400 Received: from mout.kundenserver.de ([212.227.17.13]:65305 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754401AbcGEM5Y (ORCPT ); Tue, 5 Jul 2016 08:57:24 -0400 From: Arnd Bergmann To: Philipp Zabel Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Chen-Yu Tsai , Dinh Nguyen , Steffen Trumtrar , Maxime Ripard Subject: Re: [PATCH 3/3] reset: socfpga: use readl/writel_relaxed Date: Tue, 05 Jul 2016 14:59:49 +0200 Message-ID: <3713699.k1gRikAJNK@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-22-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <1467718816.2978.41.camel@pengutronix.de> References: <1467713872-4051-1-git-send-email-p.zabel@pengutronix.de> <10266400.9nrl73J4sT@wuerfel> <1467718816.2978.41.camel@pengutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:m6hw6J6MxTM1yBSraoBNlKRh9FJQ+pDAJZynCjO1ZSvBSbY9Uw+ Db9K+gjQYvhtf1zonOYaZIdO6gZBtI0iG2EzV8QbsdaHEFdJg7X/6w7lOxElMhm3ovuGNcW 403TFQfFKrCRGKx2zZgTxFk5ouK1I3eAQ09heyxud3UMSzZzWZmo0vv30Dorz1g/cHHKPF8 gGg+BEVgjFuYECgdN0tuQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:sMAJLuYVf6k=:vFakbXD04UCDI57v6P6mTl isSsReP/rYCmn4nJCza8EFmna/rZMIXq3U/9upb5FvDjye2cJ5Etn888YCTpG+44/HGwKBBgC jIY0Ftp58M29+dxee/7ouwxnTfads/yNQJh+VS0cmNMwxX12yikbhaa9b3IKtMWQmorlmc3Fs aKq9NGKIMAv9DQJrV4DNeXoPX9MLhTa61arQzeLCHpjvJ+18yvHIFZDxWe1tkBzjLoXMkx0rC C2pJ39zF3G54qdsZYFGSwOJoqcjXzdU1YO7EGQONV9H/IySvR3DabxMS5K/CUdU7Nu0j0Eskd 9GymyK1mXIvLYY73rR7gGRC3B9J89x7tn448Kkhgcg7FU1hl7s6IgLYKayy8C3F4eo+TIRZDP /qDNTuyeENZCgBofEoqF9e42X/QJ/zW55PAi/vJFjGhGI+Hia9H+tNhvbWHarJq3KIy5SW8O5 93bK8LMX7vdq44WCAboGJ2yWr72pbHgeNRWuGLw6yXlBuEiMpiCZcTvBvD9ofu632yFbeprtk W+D9rl8GJlj40/7bQD7KVdBeouOzElHPv9ow2muYD6X+vbKghccWR4ABBIAfGe3MboeoZbjgB W1Zdl86DCfBx8i40L+Vtf/ZNjX/dqo1KPm0AsyFpbczrIG4TXaZhcJdsYg7AChxTCAjqS47QW Elqks/hjo6Gjg4kdiw1RLrdEmVkW3ZPPBFah7vd2D9TdwNEIRxQWwj61s52bpK+Nax3ch0ZTs ySmnzK6Bi7F/cBj6 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday, July 5, 2016 1:40:16 PM CEST Philipp Zabel wrote: > Am Dienstag, den 05.07.2016, 13:20 +0200 schrieb Arnd Bergmann: > > On Tuesday, July 5, 2016 12:17:52 PM CEST Philipp Zabel wrote: > > > This just removes the rmb()/wmb() pair between register read and > > > write. Since no relevant reads follow the rmb and no relevant writes > > > precede the wmb, they should be safe to remove. > > > > > > Signed-off-by: Philipp Zabel > > > > We should only do this if you are fixing a bug (which you don't mention > > in the changelog), or if you can show a relevant performance > > improvement. Is this code ever used in a fast path? If it is, > > wouldn't that indicate a problem in some driver? > > It does not fix a bug, and it's not about performance either. I'd like > to align code with the recently posted stm32 driver, to unify them in a > future patch. > Of course we can change the stm32 driver to use readl/writel instead of > the relaxed variants, it just seemed useless to have those barriers > between the read and write. On stm32, there is no barrier because ARM_DMA_MEM_BUFFERABLE is not set. I'd really prefer to just have readl/writel everywhere except in the few places that are performance critical and have a comment explaining why it's safe there, mainly to avoid having new developers blindly add the relaxed accessors in drivers because they think it's the normal coding style. > If anything, we'd need to try to make sure that the writel in assert > hits the hardware before the function returns, so that a > assert-delay-deassert doesn't accidentally spend half its delay with the > writel still in the store buffer, and we'd need a full barrier after the > writel in deassert so that there can be no successive reads from still > disabled IP cores. In general, I think you need a readl() following the writel() to guarantee that it has actually hit the hardware. On ARM you often have just the CPU write buffer that needs to be flushed, but if you have a PCI device or a more complex SoC, then a barriers doesn't wait for a write to arrive at the device, it only ensures that a subsequent write cannot arrive any earlier. Arnd