From patchwork Thu Aug 25 22:37:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 9300099 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B457F607F0 for ; Thu, 25 Aug 2016 22:39:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A39F4284B6 for ; Thu, 25 Aug 2016 22:39:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 988E4293FC; Thu, 25 Aug 2016 22:39:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D8886293FD for ; Thu, 25 Aug 2016 22:39:56 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bd3Hu-0008Bw-LU; Thu, 25 Aug 2016 22:38:18 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bd3Hp-00089A-Kb for linux-arm-kernel@lists.infradead.org; Thu, 25 Aug 2016 22:38:15 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=uo9xk02v0kmyxKUV6uf11fV/q8a/Z9TNJqb2nSRo020=; b=QtmtGlzc6aW64lRgkURE609EAxJOohLfxmyUljV2qCNWeGVg5i+rRT6xCsLnSmjtVypeMqcleZ4HZkpqXbPCMyJBHJmSA7jHErac/RWR4h5vTBaupdmoO94h18xIg4B7juQJj8vZ279Okap0DkLTkNJBbHfAQ784fonp4X/9zrA=; Received: from n2100.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:4f86]:53341) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1bd3HP-0007dt-BX; Thu, 25 Aug 2016 23:37:47 +0100 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1bd3HM-0007QD-9Y; Thu, 25 Aug 2016 23:37:44 +0100 Date: Thu, 25 Aug 2016 23:37:43 +0100 From: Russell King - ARM Linux To: Robert Jarzmik Subject: Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes Message-ID: <20160825223743.GK1041@n2100.armlinux.org.uk> References: <20160825144314.1850730-1-arnd@arndb.de> <3062310.z9h3CgCZ7p@wuerfel> <87inuopw04.fsf@belgarion.home> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87inuopw04.fsf@belgarion.home> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160825_153814_146990_797614F7 X-CRM114-Status: GOOD ( 26.19 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Arnd Bergmann , Yoshinori Sato , Nicolas Pitre , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "David S. Miller" , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > Arnd Bergmann writes: > > > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: > >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- > >> 1 file changed, 30 insertions(+), 20 deletions(-) > >> > >> While this patch fixes one bug on Neponset, it probably doesn't address > >> the one that Russell ran into first, so this is for review only for now, > >> until the remaining problem(s) have been worked out. > >> > > > > The comment should have been on another patch, my mistake. please > > see v2. > > > > Arnd > > Hi Arnd, > > I didn't review the patch thoroughly, but I launched your 2 patches in my pxa > little farm. > > The result is that lubbock and mainstone are all right, but zylonite is broken > (ie. networkless). I removed then these 2 patches and zylonite worked again. > > I have also an error message on the console on a "broken" zylonite : > Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: > Device or resource busy > > I reran the test twice (2 times with your patches, 2 times without), the result > looks consistent, ie. zylonite doesn't really like them. Please try the patch below. I sent this to Will a few days ago, as he said (on irc) that he was also seeing problems on a platform he had, but I've yet to hear back. I've not posted it yet because I haven't got around to writing a commit description for it. It does require that at least one of 8-bit or 16-bit accesses are supported, but I think that's already true. 8<======== From: Russell King Subject: [PATCH] net: smc91x: fix SMC accesses Signed-off-by: Russell King Tested-by: Robert Jarzmik --- drivers/net/ethernet/smsc/smc91x.h | 65 ++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h index 1a55c7976df0..e17671c9d1b0 100644 --- a/drivers/net/ethernet/smsc/smc91x.h +++ b/drivers/net/ethernet/smsc/smc91x.h @@ -37,6 +37,27 @@ #include /* + * Any 16-bit access is performed with two 8-bit accesses if the hardware + * can't do it directly. Most registers are 16-bit so those are mandatory. + */ +#define SMC_outw_b(x, a, r) \ + do { \ + unsigned int __val16 = (x); \ + unsigned int __reg = (r); \ + SMC_outb(__val16, a, __reg); \ + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ + } while (0) + +#define SMC_inw_b(a, r) \ + ({ \ + unsigned int __val16; \ + unsigned int __reg = r; \ + __val16 = SMC_inb(a, __reg); \ + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ + __val16; \ + }) + +/* * Define your architecture specific bus configuration parameters here. */ @@ -55,10 +76,30 @@ #define SMC_IO_SHIFT (lp->io_shift) #define SMC_inb(a, r) readb((a) + (r)) -#define SMC_inw(a, r) readw((a) + (r)) +#define SMC_inw(a, r) \ + ({ \ + unsigned int __smc_r = r; \ + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ + ({ BUG(); 0; }); \ + }) + #define SMC_inl(a, r) readl((a) + (r)) #define SMC_outb(v, a, r) writeb(v, (a) + (r)) +#define SMC_outw(v, a, r) \ + do { \ + unsigned int __v = v, __smc_r = r; \ + if (SMC_16BIT(lp)) \ + __SMC_outw(__v, a, __smc_r); \ + else if (SMC_8BIT(lp)) \ + SMC_outw_b(__v, a, __smc_r); \ + else \ + BUG(); \ + } while (0) + #define SMC_outl(v, a, r) writel(v, (a) + (r)) +#define SMC_insb(a, r, p, l) readsb((a) + (r), p, l) +#define SMC_outsb(a, r, p, l) writesb((a) + (r), p, l) #define SMC_insw(a, r, p, l) readsw((a) + (r), p, l) #define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l) #define SMC_insl(a, r, p, l) readsl((a) + (r), p, l) @@ -66,7 +107,7 @@ #define SMC_IRQ_FLAGS (-1) /* from resource */ /* We actually can't write halfwords properly if not word aligned */ -static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg) +static inline void __SMC_outw(u16 val, void __iomem *ioaddr, int reg) { if ((machine_is_mainstone() || machine_is_stargate2() || machine_is_pxa_idp()) && reg & 2) { @@ -416,24 +457,8 @@ smc_pxa_dma_insw(void __iomem *ioaddr, struct smc_local *lp, int reg, int dma, #if ! SMC_CAN_USE_16BIT -/* - * Any 16-bit access is performed with two 8-bit accesses if the hardware - * can't do it directly. Most registers are 16-bit so those are mandatory. - */ -#define SMC_outw(x, ioaddr, reg) \ - do { \ - unsigned int __val16 = (x); \ - SMC_outb( __val16, ioaddr, reg ); \ - SMC_outb( __val16 >> 8, ioaddr, reg + (1 << SMC_IO_SHIFT));\ - } while (0) -#define SMC_inw(ioaddr, reg) \ - ({ \ - unsigned int __val16; \ - __val16 = SMC_inb( ioaddr, reg ); \ - __val16 |= SMC_inb( ioaddr, reg + (1 << SMC_IO_SHIFT)) << 8; \ - __val16; \ - }) - +#define SMC_outw(x, ioaddr, reg) SMC_outw_b(x, ioaddr, reg) +#define SMC_inw(ioaddr, reg) SMC_inw_b(ioaddr, reg) #define SMC_insw(a, r, p, l) BUG() #define SMC_outsw(a, r, p, l) BUG()