From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757060AbbJVI6p (ORCPT ); Thu, 22 Oct 2015 04:58:45 -0400 Received: from mout.kundenserver.de ([212.227.17.13]:61463 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756257AbbJVI6k (ORCPT ); Thu, 22 Oct 2015 04:58:40 -0400 From: Arnd Bergmann To: Marc Kleine-Budde Cc: linux-arm-kernel@lists.infradead.org, Kedareswara rao Appana , anirudh@xilinx.com, wg@grandegger.com, michal.simek@xilinx.com, soren.brinkmann@xilinx.com, appanad@xilinx.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-can@vger.kernel.org Subject: Re: [PATCH 1/2] can: xilinx: use readl/writel instead of ioread/iowrite Date: Thu, 22 Oct 2015 10:58:33 +0200 Message-ID: <8460953.p47oezaZnR@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <56289CA6.1060809@pengutronix.de> References: <1445489163-11045-1-git-send-email-appanad@xilinx.com> <4061693.oBld7AKBIp@wuerfel> <56289CA6.1060809@pengutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:me43F97oTDyYwwLyi5xc1UiJDH1y7j2vJ0rkzebERzil43wkeq+ 0Bv31nbYfQX5n2llkIYI8t8rZ6HMHNdzPJzOQrcl2bzEAeEH0u5JGjLFH/TEFn8ZfVAy9wj EnmIloL46Q6sbLPHujcHiPdptp7vAOc8a8wkCINH5U+ApBsImxFKcPQPuDXFPHgIXd5m9TP kHZX1i4O8yUJaGe+Z5h8Q== X-UI-Out-Filterresults: notjunk:1;V01:K0:W6Upm53lMMA=:c8KtGMlwSRmXtr6YYkdnbp byzAFZUkW6h11OBvTl/tjFI7qrgUH+bay4whQmGsQQ/xZkqFV3FWzDbnc3AA5zazsai+gTZ6P l7Sgj5XLRhYrL1bYVCLYmOuudJJc8v0qqcDEDm75i1bIKUxjqrQJs9LZ9EflLBn/+CZPKfxM8 J2DPKUCyTG1uFFCN0QAkOZgyYVC2Hi82VbZVKhi66q9Y0D2iBulQ8zpYrRy/p/QLanEe8hSww nFjlv08/EQowyNZPvpa/laUhyvCPue+yXh1uTGdreWi8RgZFeeLhsBeXaaZFYE/QeuF2pPt8z SlmsvCsVvLqKY6A3AZJlsPnoMzhKQNcsRrukTb2uT+/Igg0ICTnEvkTN/uZ5XgisC5hhGeTVI DjYdvfeCp0JcdPZouuD7rzgRWR3bQUMf8LpiI0rmCA3z3Ve7fon6Tu259leWuuKxTJkVi+ITb WYYj4+pydj/YRYt+1lK3c068M48XpmbzRZcQzWSNpI1FyDJGU7hNceBLNCZW1zbSdiKnW/Pjg JbEor/gd0Ge5IZBiTKJNNQgG5r9yyw7bwKyrk3VqFWsmIVZzH/WQBEPPJihcIbhCu+YtUCv9p wyPpImWc4hVXzxw3JwACP7VQvEe0qm/YMCaJAxVFX9dB8A1NUQ9K6QwYMO3n9GOW/h6aVfgLg txD8+M6CfCKEumW06FHsKCJlzaOUYgH0mbAa+cbpdUEixbbubGQ2LsGouxW11OLLtH9PJ7Kl+ KwsKCANtkZOTZZ1v Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday 22 October 2015 10:21:58 Marc Kleine-Budde wrote: > On 10/22/2015 10:14 AM, Arnd Bergmann wrote: > > On Thursday 22 October 2015 10:16:02 Kedareswara rao Appana wrote: > >> The driver only supports memory-mapped I/O [by ioremap()], > >> so readl/writel is actually the right thing to do, IMO. > >> During the validation of this driver or IP on ARM 64-bit processor > >> while sending lot of packets observed that the tx packet drop with iowrite > >> Putting the barriers for each tx fifo register write fixes this issue > >> Instead of barriers using writel also fixed this issue. > >> > >> Signed-off-by: Kedareswara rao Appana > > > > The two should really do the same thing: iowrite32() is just a static inline > > calling writel() on both ARM32 and ARM64. On which kernel version did you > > observe the difference? It's possible that an older version used > > CONFIG_GENERIC_IOMAP, which made this slightly more expensive. > > > > If there are barriers that you want to get rid of for performance reasons, > > you should use writel_relaxed(), but be careful to synchronize them correctly > > with regard to DMA. It should be fine in this driver, as it does not > > perform any DMA, but be aware that there is no big-endian version of > > writel_relaxed() at the moment. > > We don't have DMA in CAN drivers, but usually a certain write triggers > sending. Do we need a barrier before triggering the sending? No, the relaxed writes are not well-defined across architectures. On ARM, the CPU guarantees that stores to an MMIO area are still in order with respect to one another, the barrier is only needed for actual DMA, so you are fine. I would expect the same to be true everywhere, otherwise a lot of other drivers would be broken too. To be on the safe side, that last write() could remain a writel() instead of writel_relaxed(), and that would be guaranteed to work on all architectures even if they end relax the ordering between MMIO writes. If there is a measurable performance difference, just use writel_relaxed() and add a comment. Arnd