From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161365AbcA1Qyb (ORCPT ); Thu, 28 Jan 2016 11:54:31 -0500 Received: from mout.kundenserver.de ([212.227.126.135]:49747 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966629AbcA1Qy0 (ORCPT ); Thu, 28 Jan 2016 11:54:26 -0500 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: David Laight , "David S. Miller" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , =?ISO-8859-1?Q?fran=E7ois?= romieu Subject: Re: [PATCH 4/9] net: moxart: use correct accessors for DMA memory Date: Thu, 28 Jan 2016 17:53:43 +0100 Message-ID: <3468358.kXWNPLZEG9@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CCCEDCE@AcuExch.aculab.com> References: <1453903507-3427225-1-git-send-email-arnd@arndb.de> <1453903507-3427225-5-git-send-email-arnd@arndb.de> <063D6719AE5E284EB5DD2968C1650D6D1CCCEDCE@AcuExch.aculab.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:hQHtyM8ydc5iqeSX+pSj2r6YzPDZ3E83UN7bUWG3/QZhFUz1BSz +lc6GgEiEa9k+O4yNbCwGcDmnfr1sJLGbZB9sH8sq+xdf86GykEA7sUn+Kcx9YyFi6FYUmK 1Kaw8aFRtdgAIp+8ckDEoClP9dbZBW3WHCQgw/Ll1UfHqvGJUd/uNYlHEuz02HKNHT/SK9y j9b5EQt+f8HthlQ6qUxXg== X-UI-Out-Filterresults: notjunk:1;V01:K0:5W+/0Qo0tS4=:x2/oRBxpuW91fz+kGjfLgZ UwpTT50EcpXO97pY0aj+9r5lQdmLpKlT+bfjFZJt9jgH3IkcnZ9xsWZT3KpV3YepoOVOIEAgy mEVGjNgsIA1NDWbf5SQEx+uKqlxWKddSL0wkeJofKHt1Jw8Rs54IWnib1hExKKyPBzil/ToaT HX3mmeIjXQtUP5HHeq/mdxn8dzV9+WWfpRNcFBM8C03xmKmCSoMaxWH8UcHDQ0EHtjgQTiQgE ILFpe9EEDhn4pgPUmTHkPeDqF9OTZjvr/FpFGCmxKjw6f7FP/WlheVMZ3RAcXPQQKrE3Zp8ZH m/j2mH2aWxD4fPow7o6jKpYPnWTKVt9nXgjQIBD6DKyqeSFtdHxsTUBB+a3z2UI4vGUpu8Ko8 Y9yJSYA02DfBE/Xk0V4KLf3bENAnZUUEaIccW0sjuMKSzDaochwwhz+ckrj5XAMW3fnw9ofU/ tgdL08OkUG2CsOAnLTKDvq8pcQ6e2PrDPOeJx3tmRhQjYwSJEw75iWIhxYcUHijn7yQuII1Vb X9JqFGgl8zeKdaSmrKqaijISH+mBznKUFaq0zc+9pFG59ob2Cxv6LMT9Iac/4qfFkp7aGnclP NtnMuwnHw6iteIgZaQ/gWgn1k7881FRBrlya5DCxXSWYP7+CQm+3dBte83nDE4nwJvGZjTq0h /CespVgxMwVo0ximv4EU5YYBc4cDn5CKiZCSJdXhUGWQAdT00qi97OjNB9J2B7mOVJEHF4eIC cQgYRp/wgB7HWnXio1KwohaGR4GWph9N1Wr4AROZrwWLllBAgVC37Pr4sFjvZZxHymeACrHMF nZ50he+tdw4srQVpyCLqu+dWV+GaQ== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday 28 January 2016 12:36:19 David Laight wrote: > From: Arnd Bergmann > > Sent: 27 January 2016 14:05 > > The moxart ethernet driver confuses coherent DMA buffers with > > MMIO registers. > > > > moxart_ether.c: In function 'moxart_mac_setup_desc_ring': > > moxart_ether.c:146:428: error: passing argument 1 of '__fswab32' makes integer from pointer without a > > cast [-Werror=int-conversion] > > moxart_ether.c:74:39: warning: incorrect type in argument 3 (different address spaces) > > moxart_ether.c:74:39: expected void *cpu_addr > > moxart_ether.c:74:39: got void [noderef] *tx_desc_base > > > > This leaves the basic logic alone and uses normal pointers for > > the virtual address of the descriptor. As we cannot use readl/writel > > to access them, we also introduce our own moxart_desc_read > > moxart_desc_write helpers that perform the same endianess swap > > as the original code, but without the extra barriers and address > > space conversion. > > I'm pretty sure you need to add some explicit barriers: > > > @@ -354,8 +364,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev) > > txdes1 = TX_DESC1_LTS | TX_DESC1_FTS | (len & TX_DESC1_BUF_SIZE_MASK); > > if (tx_head == TX_DESC_NUM_MASK) > > txdes1 |= TX_DESC1_END; > > - writel(txdes1, desc + TX_REG_OFFSET_DESC1); > > - writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0); > > + moxart_desc_write(txdes1, desc + TX_REG_OFFSET_DESC1); > > + moxart_desc_write(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0); > > Those last two writes must happen in that order. > There may be others. Makes sense. I looked at the ftmac100 driver, which is another driver for the same hardware, and it's also missing barriers. We should probably add them for both then. I think for the SoC that uses this, a barrier() would be sufficient because of the page flags that dma_alloc_coherent() uses on ARM for non-coherent platforms, but to be on the safe side we need a full rmb()/wmb(). Sending a version 2 now. Arnd