From patchwork Mon Jun 15 16:28:16 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 6610661 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: X-Original-To: patchwork-linux-crypto@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8AF56C0020 for ; Mon, 15 Jun 2015 16:28:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8BCA120776 for ; Mon, 15 Jun 2015 16:28:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5947820774 for ; Mon, 15 Jun 2015 16:28:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755423AbbFOQ21 (ORCPT ); Mon, 15 Jun 2015 12:28:27 -0400 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:32906 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753288AbbFOQ20 (ORCPT ); Mon, 15 Jun 2015 12:28:26 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=/7yEMJ4dTS10DbbWU3qsjbYrZ9EH6IqcHRInyWrzAq0=; b=VXQb4YSq1rybWEH/ICJ2q42TknQtvH6fLCcSDLb1sGB/BV8QXG+lEn3PEE0LntSaQim4RlgWHHN4ESmpb7Br3JKkFOwlGb5NDZzq2WHvxn8FCuTd8qhZ06VWt6f/WQ5gAJv2QPKV6JDPiOKRljU+OmXb245VDt9a8okqLg8XqW8=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:49161) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1Z4XFE-0001Zh-5V; Mon, 15 Jun 2015 17:28:20 +0100 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1Z4XFB-0003QQ-1I; Mon, 15 Jun 2015 17:28:17 +0100 Date: Mon, 15 Jun 2015 17:28:16 +0100 From: Russell King - ARM Linux To: Steffen Trumtrar Cc: linux-kernel@vger.kernel.org, kernel@pengutronix.de, Ruchika Gupta , linux-crypto@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Herbert Xu Subject: Re: [BUG?] crypto: caam: little/big endianness on ARM vs PPC Message-ID: <20150615162816.GO7557@n2100.arm.linux.org.uk> References: <20150615155907.GC7947@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150615155907.GC7947@pengutronix.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Jun 15, 2015 at 05:59:07PM +0200, Steffen Trumtrar wrote: > I'm working on CAAM support for the ARM-based i.MX6 SoCs. The current > drivers/crypto/caam driver only works for PowerPC AFAIK. > Actually, there isn't that much to do, to get support for the i.MX6 but > one patch breaks the driver severely: > > commit ef94b1d834aace7101de77c3a7c2631b9ae9c5f6 > crypto: caam - Add definition of rd/wr_reg64 for little endian platform You're not the only one who's hitting problems with this - Jon Nettleton has been working on it recently. The way this has been done is fairly yucky to start with: several things about it are particularly horrid. The first is the repetitive code for the BE and LE cases, when all that's actually different is the register order between the two code cases. The second thing is the excessive use of masking - I'm pretty sure the compiler won't complain with the below. The second issue is that apparently, the register order doesn't actually change for LE devices - in other words, the byte order within each register does change, but they aren't a 64-bit register, they're two separate 32-bit registers. So, they should always be written as such. So, I'd get rid of the #if defined(__BIG_ENDIAN) stuff and instead have: +/* + * The DMA address register is a pair of 32-bit registers, and should + * always be accessed as such. + */ +#define REG64_HI32(reg) ((u32 __iomem *)(reg)) +#define REG64_LO32(reg) ((u32 __iomem *)(reg) + 1) diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h index 378ddc17f60e..ba0fa630a25d 100644 --- a/drivers/crypto/caam/regs.h +++ b/drivers/crypto/caam/regs.h @@ -84,34 +84,28 @@ #endif #ifndef CONFIG_64BIT -#ifdef __BIG_ENDIAN -static inline void wr_reg64(u64 __iomem *reg, u64 data) -{ - wr_reg32((u32 __iomem *)reg, (data & 0xffffffff00000000ull) >> 32); - wr_reg32((u32 __iomem *)reg + 1, data & 0x00000000ffffffffull); -} - -static inline u64 rd_reg64(u64 __iomem *reg) -{ - return (((u64)rd_reg32((u32 __iomem *)reg)) << 32) | - ((u64)rd_reg32((u32 __iomem *)reg + 1)); -} +#if defined(__BIG_ENDIAN) +#define REG64_HI32(reg) ((u32 __iomem *)(reg)) +#define REG64_LO32(reg) ((u32 __iomem *)(reg) + 1) +#elif defined(__LITTLE_ENDIAN) +#define REG64_HI32(reg) ((u32 __iomem *)(reg) + 1) +#define REG64_LO32(reg) ((u32 __iomem *)(reg)) #else -#ifdef __LITTLE_ENDIAN +#error Broken endian? +#endif + static inline void wr_reg64(u64 __iomem *reg, u64 data) { - wr_reg32((u32 __iomem *)reg + 1, (data & 0xffffffff00000000ull) >> 32); - wr_reg32((u32 __iomem *)reg, data & 0x00000000ffffffffull); + wr_reg32(REG64_HI32(reg), data >> 32); + wr_reg32(REG64_LO32(reg), data); } static inline u64 rd_reg64(u64 __iomem *reg) { - return (((u64)rd_reg32((u32 __iomem *)reg + 1)) << 32) | - ((u64)rd_reg32((u32 __iomem *)reg)); + return ((u64)rd_reg32(REG64_HI32(reg))) << 32 | + rd_reg32(REG64_LO32(reg)); } #endif -#endif -#endif /* * jr_outentry