From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DE0F2C04A68 for ; Thu, 28 Jul 2022 06:18:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234049AbiG1GS1 (ORCPT ); Thu, 28 Jul 2022 02:18:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58710 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233781AbiG1GSY (ORCPT ); Thu, 28 Jul 2022 02:18:24 -0400 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id DCA115D0F2; Wed, 27 Jul 2022 23:18:22 -0700 (PDT) Received: from [192.168.87.140] (unknown [50.47.106.71]) by linux.microsoft.com (Postfix) with ESMTPSA id 18D4620FE86F; Wed, 27 Jul 2022 23:18:22 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 18D4620FE86F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1658989102; bh=dOWYO9Tc9My88lpKym2V9t4KyEwrpWAjOIRXvs4C5XQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Ak0uQ40xJOJRfAIrFUYHIr9TFMyUDjJbJTZm6CGUkKy+4qOq8U56I8c+XrKWgMQ8B csdkyl9vpQCSv5JuJFTpnLAuSRIlKNFG4wOc1us79zruuwJNmDK1gCUlFhLZtlZyG7 wGUvBV/uHTBcB+UTdH5KjjtwmuxD2IG+4K95CEvI= Message-ID: <450dee2f-63bf-51a7-730e-b780b594c1c5@linux.microsoft.com> Date: Wed, 27 Jul 2022 23:18:22 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v8 5/5] crypto: aspeed: add HACE crypto driver Content-Language: en-US To: Neal Liu , Corentin Labbe , Christophe JAILLET , Randy Dunlap , Herbert Xu , "David S . Miller" , Rob Herring , Krzysztof Kozlowski , Joel Stanley , Andrew Jeffery , Johnny Huang Cc: "linux-aspeed@lists.ozlabs.org" , "linux-crypto@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , BMC-SW References: <20220726113448.2964968-1-neal_liu@aspeedtech.com> <20220726113448.2964968-6-neal_liu@aspeedtech.com> <9d6beefe-9974-22f8-750c-68c9acb707ab@linux.microsoft.com> From: Dhananjay Phadke In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/26/2022 10:31 PM, Neal Liu wrote: >> Why are separate options required for hash and crypto algorithms, if hace is >> only hw crypto on the SoCs? >> >> Looks like that's requiring unnecessary __weak register / unregister functions >> [see below]. >> >> Couldn't just two options CONFIG_CRYPTO_DEV_ASPEED and >> CONFIG_CRYPTO_DEV_ASPEED_DEBUG be simpler to set for downstream >> defconfigs? > I would like to separate different algorithms by different options for more convenient for further use and debug. > We also have RSA engine named ACRY, and would upstream once hash & crypto being accepted. > So combined them into one option seems not a good choice for multiple hw crypto, do you agree? Not sure what's the use case of just enabling crypto or hash separately out of same HW engine and esp. when there's no alternative accel available, but that's fine. If ARCY is different HW engine (interface) then having separate config sounds logical. Multiplying DEBUG configs seems unnecessary though. With dynamic debug any of the dev_dbg could be turned on. Suggest using single one for the module, if not drop it altogether. Following code is still not covered by Kconfig, it in common code. > +#ifdef ASPEED_HACE_DEBUG > +#define HACE_DBG(d, fmt, ...) \ > + dev_info((d)->dev, "%s() " fmt, __func__, ##__VA_ARGS__) > +#else > +#define HACE_DBG(d, fmt, ...) \ > + dev_dbg((d)->dev, "%s() " fmt, __func__, ##__VA_ARGS__) > +#endif Regards, Dhananjay From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D0715C04A68 for ; Thu, 28 Jul 2022 06:19:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=IQKrfj42U4a1x+6GVFCN1sEBSnFTCKTeqJsH8aZq1WI=; b=fCyIv4AsOnnzO1 CjmBxK7ImWYimmpZKIHAfmxotTas34AuHbtgjdAq6L0b19OplnnokD45oNOv3I/5uIdXu2di/b+E7 /3u2eZNVMt5hAHbX1GGwU4f/NIBeeyhFAVuUfaRSCY3u3orm5siJYZq5dj8pmSUZ9JRQgbb0R4LX9 Kd4uaSs7w6kmnTM/xEGYodUcQn4Km2mSulwnrmiV9T2xe8To/yB0VSBZUc9lFIoaGIhPhq8dJ5Bnv 718rpoDf+kMRt1PTd8w5LY6DT/0bbIDW8dc665+0iC+DijM/XR8ScCpyiQMUIqcHhW+B0KP95HYKz uEcL5KQvKVLk1lyuUIdw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oGwqs-004x23-KP; Thu, 28 Jul 2022 06:18:30 +0000 Received: from linux.microsoft.com ([13.77.154.182]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oGwqp-004wyY-47 for linux-arm-kernel@lists.infradead.org; Thu, 28 Jul 2022 06:18:28 +0000 Received: from [192.168.87.140] (unknown [50.47.106.71]) by linux.microsoft.com (Postfix) with ESMTPSA id 18D4620FE86F; Wed, 27 Jul 2022 23:18:22 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 18D4620FE86F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1658989102; bh=dOWYO9Tc9My88lpKym2V9t4KyEwrpWAjOIRXvs4C5XQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Ak0uQ40xJOJRfAIrFUYHIr9TFMyUDjJbJTZm6CGUkKy+4qOq8U56I8c+XrKWgMQ8B csdkyl9vpQCSv5JuJFTpnLAuSRIlKNFG4wOc1us79zruuwJNmDK1gCUlFhLZtlZyG7 wGUvBV/uHTBcB+UTdH5KjjtwmuxD2IG+4K95CEvI= Message-ID: <450dee2f-63bf-51a7-730e-b780b594c1c5@linux.microsoft.com> Date: Wed, 27 Jul 2022 23:18:22 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v8 5/5] crypto: aspeed: add HACE crypto driver Content-Language: en-US To: Neal Liu , Corentin Labbe , Christophe JAILLET , Randy Dunlap , Herbert Xu , "David S . Miller" , Rob Herring , Krzysztof Kozlowski , Joel Stanley , Andrew Jeffery , Johnny Huang Cc: "linux-aspeed@lists.ozlabs.org" , "linux-crypto@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , BMC-SW References: <20220726113448.2964968-1-neal_liu@aspeedtech.com> <20220726113448.2964968-6-neal_liu@aspeedtech.com> <9d6beefe-9974-22f8-750c-68c9acb707ab@linux.microsoft.com> From: Dhananjay Phadke In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220727_231827_287868_A74FB818 X-CRM114-Status: GOOD ( 10.75 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 7/26/2022 10:31 PM, Neal Liu wrote: >> Why are separate options required for hash and crypto algorithms, if hace is >> only hw crypto on the SoCs? >> >> Looks like that's requiring unnecessary __weak register / unregister functions >> [see below]. >> >> Couldn't just two options CONFIG_CRYPTO_DEV_ASPEED and >> CONFIG_CRYPTO_DEV_ASPEED_DEBUG be simpler to set for downstream >> defconfigs? > I would like to separate different algorithms by different options for more convenient for further use and debug. > We also have RSA engine named ACRY, and would upstream once hash & crypto being accepted. > So combined them into one option seems not a good choice for multiple hw crypto, do you agree? Not sure what's the use case of just enabling crypto or hash separately out of same HW engine and esp. when there's no alternative accel available, but that's fine. If ARCY is different HW engine (interface) then having separate config sounds logical. Multiplying DEBUG configs seems unnecessary though. With dynamic debug any of the dev_dbg could be turned on. Suggest using single one for the module, if not drop it altogether. Following code is still not covered by Kconfig, it in common code. > +#ifdef ASPEED_HACE_DEBUG > +#define HACE_DBG(d, fmt, ...) \ > + dev_info((d)->dev, "%s() " fmt, __func__, ##__VA_ARGS__) > +#else > +#define HACE_DBG(d, fmt, ...) \ > + dev_dbg((d)->dev, "%s() " fmt, __func__, ##__VA_ARGS__) > +#endif Regards, Dhananjay _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel