From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752035AbbCJATc (ORCPT ); Mon, 9 Mar 2015 20:19:32 -0400 Received: from mail-by2on0136.outbound.protection.outlook.com ([207.46.100.136]:5730 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751769AbbCJATZ (ORCPT ); Mon, 9 Mar 2015 20:19:25 -0400 Date: Mon, 9 Mar 2015 19:14:05 -0500 From: Kim Phillips To: Christophe Leroy CC: Herbert Xu , "David S. Miller" , , , Subject: Re: [PATCH v2 15/17] crypto: talitos - Implementation of SEC1 Message-ID: <20150309191405.d5494a29565ebd72d9bf33f0@freescale.com> In-Reply-To: <20150306164226.331B51A241C@localhost.localdomain> References: <20150306164226.331B51A241C@localhost.localdomain> Organization: Freescale Semiconductor, Inc. X-Mailer: Sylpheed 3.2.0 (GTK+ 2.24.13; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-EOPAttributedMessage: 0 Authentication-Results: spf=fail (sender IP is 192.88.158.2) smtp.mailfrom=kim.phillips@freescale.com; vger.kernel.org; dkim=none (message not signed) header.d=none; X-Forefront-Antispam-Report: CIP:192.88.158.2;CTRY:US;IPV:NLI;EFV:NLI;BMV:0;SFV:NSPM;SFS:(10019020)(6009001)(339900001)(52604005)(51704005)(199003)(189002)(24454002)(164054003)(50466002)(46102003)(77156002)(100306002)(76176999)(106466001)(77096005)(104016003)(19580405001)(23726002)(62966003)(19580395003)(105606002)(110136001)(87936001)(92566002)(85426001)(2950100001)(47776003)(86362001)(46406003)(33646002)(6806004)(36756003)(50986999)(50226001);DIR:OUT;SFP:1102;SCL:1;SRVR:BY1PR03MB1436;H:az84smr01.freescale.net;FPR:;SPF:Fail;MLV:sfv;A:1;MX:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR03MB1436; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006)(5001009);SRVR:BY1PR03MB1436;BCL:0;PCL:0;RULEID:;SRVR:BY1PR03MB1436; X-Forefront-PRVS: 051158ECBB X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Mar 2015 00:19:21.8671 (UTC) X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.158.2];Helo=[az84smr01.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR03MB1436 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 6 Mar 2015 17:42:26 +0100 Christophe Leroy wrote: > This patch adds talitos1.c and talitos1.h with all specificities needed > to handle the SEC1 security engine found in MPC885 and MPC8272. > > The SEC1 has several differences with its younger brother SEC2: > * Several bits in registers have different locations > * Many bits are missing > * Some bits are in addition > * SEC1 doesn't support scatter/gather > * SEC1 has a different descriptor structure > > We add a helper function for clearing the desc field in the descriptor as that field needs to be cleared on SEC1 and doesn't exist on SEC2. > > Signed-off-by: Christophe Leroy Hi Christophe, Thanks for rebasing on cryptodev-2.6, now I can easily see the differences for review. > +++ b/drivers/crypto/Kconfig > @@ -211,7 +211,8 @@ config CRYPTO_DEV_TALITOS > select CRYPTO_ALGAPI > select CRYPTO_AUTHENC > select HW_RANDOM > - select CRYPTO_DEV_TALITOS2 > + select CRYPTO_DEV_TALITOS1 if PPC_8xx || PPC_82xx > + select CRYPTO_DEV_TALITOS2 if !CRYPTO_DEV_TALITOS1 This will set CONFIG_CRYPTO_DEV_TALITOS1=y on a kernel with both PPC_82xx and PPC_83xx set, and will therefore break talitos when run on an 83xx part. The driver needs to work on both v1 and v2/3 SECs dynamically, and behave accordingly depending on what version h/w is described in the device tree ("fsl,secX"). So, barring making a completely new driver, a couple of points: - A h/w version 1 vs 2 compatible priv variable can be allocated for parts of the driver that need quicker access than calling of_device_is_compatible(). Other parts that do better as complete function rewrites (the scatter-gather mapping fns?) can be abstracted away using pointers to the v1 vs. v2+ base functions. The pointers would live in the device's priv struct, and be assigned at module initialization time. - instead of rewriting the structs talitos_ptr, talitos_desc, either use the v2-named member as-is, or make unions. E.g., instead of having a new struct talitos_ptr, either use the v2 h/w version as-is and shift the length into place, or, add a v1 len via a union, and make to_talitos_ptr a pointer to a function, with the v1 version of the function updating only the v1 len. For struct talitos_desc, how about: struct talitos_desc { __be32 hdr; struct talitos_ptr[8]; } and have the desc->ptr[X] assigning code add 1 via a new macro iff on a SEC2 compatible (the hdr_lo code would have to be hardcoded to refer to ptr[0] instead of hdr_lo directly, and next_desc would be ptr[7]). talitos_edesc shouldn't need the extra ptr_{in,out}, rather allocate the extra space when they're needed, and refer to them via aliases to defines that cast and use link_tbl[0] as a base reference. For IO register accesses, either a lookup table can be used, or, since they're not frequent, define v1-specific defines in addition to the v2 ones, and reference using the compatible variable. Other than that, please make sure to use checkpatch.pl --strict, sparse -D__CHECK_ENDIAN__, and that building as a module doesn't break. Thanks, Kim