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 4096DC04A68 for ; Tue, 26 Jul 2022 20:41:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233399AbiGZUlP (ORCPT ); Tue, 26 Jul 2022 16:41:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229835AbiGZUlL (ORCPT ); Tue, 26 Jul 2022 16:41:11 -0400 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 928A015FFF; Tue, 26 Jul 2022 13:41:10 -0700 (PDT) Received: from [192.168.87.140] (unknown [50.47.106.71]) by linux.microsoft.com (Postfix) with ESMTPSA id AC6FC20FE2E6; Tue, 26 Jul 2022 13:41:09 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AC6FC20FE2E6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1658868070; bh=VgOwC09+vnjb9WQXMFtlpt3WfTGrLfx+SsoGsAU+cFc=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=r/POcPaqbQK+X7IamgPgQgrbXsaxjYAYFv5uzXMI/O6N0IYq5ntGykO/OjrytvU9w 4wlBlS1q4t03xVRC5TSX0xM41bFel0XM/VCMmyY/a3JzAGPwLyXb1Lz4HqAQhr+Htl KvdZU/W85fSzOsCYFuK3wC7v7tEjgXmUIGoueDQM= Message-ID: <9d6beefe-9974-22f8-750c-68c9acb707ab@linux.microsoft.com> Date: Tue, 26 Jul 2022 13:41:09 -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 , Dhananjay Phadke , 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@aspeedtech.com References: <20220726113448.2964968-1-neal_liu@aspeedtech.com> <20220726113448.2964968-6-neal_liu@aspeedtech.com> From: Dhananjay Phadke In-Reply-To: <20220726113448.2964968-6-neal_liu@aspeedtech.com> 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 Hi Neal, Thanks for addressing v7 review comments, few more below. On 7/26/2022 4:34 AM, Neal Liu wrote: > Add HACE crypto driver to support symmetric-key > encryption and decryption with multiple modes of > operation. > > Signed-off-by: Neal Liu > Signed-off-by: Johnny Huang > --- > drivers/crypto/aspeed/Kconfig | 26 + > drivers/crypto/aspeed/Makefile | 7 +- > drivers/crypto/aspeed/aspeed-hace-crypto.c | 1121 ++++++++++++++++++++ > drivers/crypto/aspeed/aspeed-hace.c | 91 +- > drivers/crypto/aspeed/aspeed-hace.h | 112 ++ > 5 files changed, 1354 insertions(+), 3 deletions(-) > create mode 100644 drivers/crypto/aspeed/aspeed-hace-crypto.c > > diff --git a/drivers/crypto/aspeed/Kconfig b/drivers/crypto/aspeed/Kconfig > index 059e627efef8..f19994915a5e 100644 > --- a/drivers/crypto/aspeed/Kconfig > +++ b/drivers/crypto/aspeed/Kconfig > @@ -30,3 +30,29 @@ config CRYPTO_DEV_ASPEED_HACE_HASH_DEBUG > to ask for those messages. > Avoid enabling this option for production build to > minimize driver timing. > + > +config CRYPTO_DEV_ASPEED_HACE_CRYPTO > + bool "Enable Aspeed Hash & Crypto Engine (HACE) crypto" > + depends on CRYPTO_DEV_ASPEED > + select CRYPTO_ENGINE > + select CRYPTO_AES > + select CRYPTO_DES > + select CRYPTO_ECB > + select CRYPTO_CBC > + select CRYPTO_CFB > + select CRYPTO_OFB > + select CRYPTO_CTR > + help > + Select here to enable Aspeed Hash & Crypto Engine (HACE) > + crypto driver. > + Supports AES/DES symmetric-key encryption and decryption > + with ECB/CBC/CFB/OFB/CTR options. > + > +config CRYPTO_DEV_ASPEED_HACE_CRYPTO_DEBUG > + bool "Enable HACE crypto debug messages" > + depends on CRYPTO_DEV_ASPEED_HACE_CRYPTO > + help > + Print HACE crypto debugging messages if you use this option > + to ask for those messages. > + Avoid enabling this option for production build to > + minimize driver timing. 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? > diff --git a/drivers/crypto/aspeed/Makefile b/drivers/crypto/aspeed/Makefile > index 8bc8d4fed5a9..421e2ca9c53e 100644 > --- a/drivers/crypto/aspeed/Makefile > +++ b/drivers/crypto/aspeed/Makefile > @@ -1,6 +1,9 @@ > obj-$(CONFIG_CRYPTO_DEV_ASPEED) += aspeed_crypto.o > -aspeed_crypto-objs := aspeed-hace.o \ > - $(hace-hash-y) > +aspeed_crypto-objs := aspeed-hace.o \ > + $(hace-hash-y) \ > + $(hace-crypto-y) > > obj-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_HASH) += aspeed-hace-hash.o > hace-hash-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_HASH) := aspeed-hace-hash.o > +obj-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_CRYPTO) += aspeed-hace-crypto.o > +hace-crypto-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_CRYPTO) := aspeed-hace-crypto.o > diff --git a/drivers/crypto/aspeed/aspeed-hace-crypto.c b/drivers/crypto/aspeed/aspeed-hace-crypto.c > new file mode 100644 [...] > + > +void aspeed_register_hace_crypto_algs(struct aspeed_hace_dev *hace_dev) > +{ > + int rc, i; > + > + CIPHER_DBG(hace_dev, "\n"); > + > + for (i = 0; i < ARRAY_SIZE(aspeed_crypto_algs); i++) { > + aspeed_crypto_algs[i].hace_dev = hace_dev; > + rc = crypto_register_skcipher(&aspeed_crypto_algs[i].alg.skcipher); > + if (rc) { > + CIPHER_DBG(hace_dev, "Failed to register %s\n", > + aspeed_crypto_algs[i].alg.skcipher.base.cra_name); > + } > + } > + > + if (hace_dev->version != AST2600_VERSION) > + return; > + > + for (i = 0; i < ARRAY_SIZE(aspeed_crypto_algs_g6); i++) { > + aspeed_crypto_algs_g6[i].hace_dev = hace_dev; > + rc = crypto_register_skcipher(&aspeed_crypto_algs_g6[i].alg.skcipher); > + if (rc) { > + CIPHER_DBG(hace_dev, "Failed to register %s\n", > + aspeed_crypto_algs_g6[i].alg.skcipher.base.cra_name); > + } > + } > +} > diff --git a/drivers/crypto/aspeed/aspeed-hace.c b/drivers/crypto/aspeed/aspeed-hace.c > index 89b1585d72e2..efc0725ebf98 100644 > --- a/drivers/crypto/aspeed/aspeed-hace.c > +++ b/drivers/crypto/aspeed/aspeed-hace.c > @@ -32,10 +32,22 @@ void __weak aspeed_unregister_hace_hash_algs(struct aspeed_hace_dev *hace_dev) > dev_warn(hace_dev->dev, "%s: Not supported yet\n", __func__); > } > > +/* Weak function for HACE crypto */ > +void __weak aspeed_register_hace_crypto_algs(struct aspeed_hace_dev *hace_dev) > +{ > + dev_warn(hace_dev->dev, "%s: Not supported yet\n", __func__); > +} > + > +void __weak aspeed_unregister_hace_crypto_algs(struct aspeed_hace_dev *hace_dev) > +{ > + dev_warn(hace_dev->dev, "%s: Not supported yet\n", __func__); > +} > + aspeed_unregister_hace_crypto_algs() is not implemented in aspeed-hace-crypto.c, so those algorithms are not unregistered during unload. This was missed because of __weak function. > /* HACE interrupt service routine */ > static irqreturn_t aspeed_hace_irq(int irq, void *dev) > { > struct aspeed_hace_dev *hace_dev = (struct aspeed_hace_dev *)dev; > + struct aspeed_engine_crypto *crypto_engine = &hace_dev->crypto_engine; > struct aspeed_engine_hash *hash_engine = &hace_dev->hash_engine; > u32 sts; > > @@ -51,9 +63,24 @@ static irqreturn_t aspeed_hace_irq(int irq, void *dev) > dev_warn(hace_dev->dev, "HASH no active requests.\n"); > } > > + if (sts & HACE_CRYPTO_ISR) { > + if (crypto_engine->flags & CRYPTO_FLAGS_BUSY) > + tasklet_schedule(&crypto_engine->done_task); > + else > + dev_warn(hace_dev->dev, "CRYPTO no active requests.\n"); > + } > + > return IRQ_HANDLED; > } > > +static void aspeed_hace_crypto_done_task(unsigned long data) > +{ > + struct aspeed_hace_dev *hace_dev = (struct aspeed_hace_dev *)data; > + struct aspeed_engine_crypto *crypto_engine = &hace_dev->crypto_engine; > + > + crypto_engine->resume(hace_dev); > +} > + > static void aspeed_hace_hash_done_task(unsigned long data) > { > struct aspeed_hace_dev *hace_dev = (struct aspeed_hace_dev *)data; > @@ -65,11 +92,13 @@ static void aspeed_hace_hash_done_task(unsigned long data) > static void aspeed_hace_register(struct aspeed_hace_dev *hace_dev) > { > aspeed_register_hace_hash_algs(hace_dev); > + aspeed_register_hace_crypto_algs(hace_dev); > } > > static void aspeed_hace_unregister(struct aspeed_hace_dev *hace_dev) > { > aspeed_unregister_hace_hash_algs(hace_dev); > + aspeed_unregister_hace_crypto_algs(hace_dev); > } Could just wrap these calls instead of weak functions. static void aspeed_hace_unregister(struct aspeed_hace_dev *hace_dev) { #ifdef CONFIG_CRYPTO_DEV_ASPEED_HACE_HASH aspeed_unregister_hace_hash_algs(hace_dev); #endif #ifdef CONFIG_CRYPTO_DEV_ASPEED_HACE_CRYPTO aspeed_unregister_hace_crypto_algs(hace_dev); #endif } > > static const struct of_device_id aspeed_hace_of_matches[] = { > @@ -80,6 +109,7 @@ static const struct of_device_id aspeed_hace_of_matches[] = { > > static int aspeed_hace_probe(struct platform_device *pdev) > { > + struct aspeed_engine_crypto *crypto_engine; > const struct of_device_id *hace_dev_id; > struct aspeed_engine_hash *hash_engine; > struct aspeed_hace_dev *hace_dev; > @@ -100,6 +130,7 @@ static int aspeed_hace_probe(struct platform_device *pdev) > hace_dev->dev = &pdev->dev; > hace_dev->version = (unsigned long)hace_dev_id->data; > hash_engine = &hace_dev->hash_engine; > + crypto_engine = &hace_dev->crypto_engine; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); Thanks, 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 67018C00140 for ; Tue, 26 Jul 2022 20:42:32 +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=H2WuwkcYrNASziC3u8Q/Ui6G4IY2kT41xYqCp05R3Pg=; b=38RhipZRsSaqFy QSg3FJvncUbQQFpKPwpi1Txhxk7DMRoaZ5koC8/yyNgU+n6V5Kq/1WrxcBbElghlEtDzmhB9HJmNw 9/D2x4OTxt56KMMJuVGyeR+F8nHmRR48m+94mEjr1h/dW3i/8pR9iiDRzwCreWyxR5H3h6C4CF0ZM gn8exNV/dUnasFpPzQ/OX8DnmI83zrlUyxH9KL8ASlbgyY6cVHfLyO3lW6QEwOSBRInDaYGOyUe76 SlYrHukCh3XKmgOkrHjF7Zhgf4pHnaY6zVzr3HaZrEX9MjhtBaorANPvx7GmssMlhze9bB8Bvw4+m n6n7aVrnnU//PEDTsLxA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oGRMk-003Q1F-Uk; Tue, 26 Jul 2022 20:41:19 +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 1oGRMh-003PxN-9L for linux-arm-kernel@lists.infradead.org; Tue, 26 Jul 2022 20:41:17 +0000 Received: from [192.168.87.140] (unknown [50.47.106.71]) by linux.microsoft.com (Postfix) with ESMTPSA id AC6FC20FE2E6; Tue, 26 Jul 2022 13:41:09 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AC6FC20FE2E6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1658868070; bh=VgOwC09+vnjb9WQXMFtlpt3WfTGrLfx+SsoGsAU+cFc=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=r/POcPaqbQK+X7IamgPgQgrbXsaxjYAYFv5uzXMI/O6N0IYq5ntGykO/OjrytvU9w 4wlBlS1q4t03xVRC5TSX0xM41bFel0XM/VCMmyY/a3JzAGPwLyXb1Lz4HqAQhr+Htl KvdZU/W85fSzOsCYFuK3wC7v7tEjgXmUIGoueDQM= Message-ID: <9d6beefe-9974-22f8-750c-68c9acb707ab@linux.microsoft.com> Date: Tue, 26 Jul 2022 13:41:09 -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 , Dhananjay Phadke , 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@aspeedtech.com References: <20220726113448.2964968-1-neal_liu@aspeedtech.com> <20220726113448.2964968-6-neal_liu@aspeedtech.com> From: Dhananjay Phadke In-Reply-To: <20220726113448.2964968-6-neal_liu@aspeedtech.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220726_134115_387951_EF1656D3 X-CRM114-Status: GOOD ( 30.26 ) 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 Hi Neal, Thanks for addressing v7 review comments, few more below. On 7/26/2022 4:34 AM, Neal Liu wrote: > Add HACE crypto driver to support symmetric-key > encryption and decryption with multiple modes of > operation. > > Signed-off-by: Neal Liu > Signed-off-by: Johnny Huang > --- > drivers/crypto/aspeed/Kconfig | 26 + > drivers/crypto/aspeed/Makefile | 7 +- > drivers/crypto/aspeed/aspeed-hace-crypto.c | 1121 ++++++++++++++++++++ > drivers/crypto/aspeed/aspeed-hace.c | 91 +- > drivers/crypto/aspeed/aspeed-hace.h | 112 ++ > 5 files changed, 1354 insertions(+), 3 deletions(-) > create mode 100644 drivers/crypto/aspeed/aspeed-hace-crypto.c > > diff --git a/drivers/crypto/aspeed/Kconfig b/drivers/crypto/aspeed/Kconfig > index 059e627efef8..f19994915a5e 100644 > --- a/drivers/crypto/aspeed/Kconfig > +++ b/drivers/crypto/aspeed/Kconfig > @@ -30,3 +30,29 @@ config CRYPTO_DEV_ASPEED_HACE_HASH_DEBUG > to ask for those messages. > Avoid enabling this option for production build to > minimize driver timing. > + > +config CRYPTO_DEV_ASPEED_HACE_CRYPTO > + bool "Enable Aspeed Hash & Crypto Engine (HACE) crypto" > + depends on CRYPTO_DEV_ASPEED > + select CRYPTO_ENGINE > + select CRYPTO_AES > + select CRYPTO_DES > + select CRYPTO_ECB > + select CRYPTO_CBC > + select CRYPTO_CFB > + select CRYPTO_OFB > + select CRYPTO_CTR > + help > + Select here to enable Aspeed Hash & Crypto Engine (HACE) > + crypto driver. > + Supports AES/DES symmetric-key encryption and decryption > + with ECB/CBC/CFB/OFB/CTR options. > + > +config CRYPTO_DEV_ASPEED_HACE_CRYPTO_DEBUG > + bool "Enable HACE crypto debug messages" > + depends on CRYPTO_DEV_ASPEED_HACE_CRYPTO > + help > + Print HACE crypto debugging messages if you use this option > + to ask for those messages. > + Avoid enabling this option for production build to > + minimize driver timing. 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? > diff --git a/drivers/crypto/aspeed/Makefile b/drivers/crypto/aspeed/Makefile > index 8bc8d4fed5a9..421e2ca9c53e 100644 > --- a/drivers/crypto/aspeed/Makefile > +++ b/drivers/crypto/aspeed/Makefile > @@ -1,6 +1,9 @@ > obj-$(CONFIG_CRYPTO_DEV_ASPEED) += aspeed_crypto.o > -aspeed_crypto-objs := aspeed-hace.o \ > - $(hace-hash-y) > +aspeed_crypto-objs := aspeed-hace.o \ > + $(hace-hash-y) \ > + $(hace-crypto-y) > > obj-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_HASH) += aspeed-hace-hash.o > hace-hash-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_HASH) := aspeed-hace-hash.o > +obj-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_CRYPTO) += aspeed-hace-crypto.o > +hace-crypto-$(CONFIG_CRYPTO_DEV_ASPEED_HACE_CRYPTO) := aspeed-hace-crypto.o > diff --git a/drivers/crypto/aspeed/aspeed-hace-crypto.c b/drivers/crypto/aspeed/aspeed-hace-crypto.c > new file mode 100644 [...] > + > +void aspeed_register_hace_crypto_algs(struct aspeed_hace_dev *hace_dev) > +{ > + int rc, i; > + > + CIPHER_DBG(hace_dev, "\n"); > + > + for (i = 0; i < ARRAY_SIZE(aspeed_crypto_algs); i++) { > + aspeed_crypto_algs[i].hace_dev = hace_dev; > + rc = crypto_register_skcipher(&aspeed_crypto_algs[i].alg.skcipher); > + if (rc) { > + CIPHER_DBG(hace_dev, "Failed to register %s\n", > + aspeed_crypto_algs[i].alg.skcipher.base.cra_name); > + } > + } > + > + if (hace_dev->version != AST2600_VERSION) > + return; > + > + for (i = 0; i < ARRAY_SIZE(aspeed_crypto_algs_g6); i++) { > + aspeed_crypto_algs_g6[i].hace_dev = hace_dev; > + rc = crypto_register_skcipher(&aspeed_crypto_algs_g6[i].alg.skcipher); > + if (rc) { > + CIPHER_DBG(hace_dev, "Failed to register %s\n", > + aspeed_crypto_algs_g6[i].alg.skcipher.base.cra_name); > + } > + } > +} > diff --git a/drivers/crypto/aspeed/aspeed-hace.c b/drivers/crypto/aspeed/aspeed-hace.c > index 89b1585d72e2..efc0725ebf98 100644 > --- a/drivers/crypto/aspeed/aspeed-hace.c > +++ b/drivers/crypto/aspeed/aspeed-hace.c > @@ -32,10 +32,22 @@ void __weak aspeed_unregister_hace_hash_algs(struct aspeed_hace_dev *hace_dev) > dev_warn(hace_dev->dev, "%s: Not supported yet\n", __func__); > } > > +/* Weak function for HACE crypto */ > +void __weak aspeed_register_hace_crypto_algs(struct aspeed_hace_dev *hace_dev) > +{ > + dev_warn(hace_dev->dev, "%s: Not supported yet\n", __func__); > +} > + > +void __weak aspeed_unregister_hace_crypto_algs(struct aspeed_hace_dev *hace_dev) > +{ > + dev_warn(hace_dev->dev, "%s: Not supported yet\n", __func__); > +} > + aspeed_unregister_hace_crypto_algs() is not implemented in aspeed-hace-crypto.c, so those algorithms are not unregistered during unload. This was missed because of __weak function. > /* HACE interrupt service routine */ > static irqreturn_t aspeed_hace_irq(int irq, void *dev) > { > struct aspeed_hace_dev *hace_dev = (struct aspeed_hace_dev *)dev; > + struct aspeed_engine_crypto *crypto_engine = &hace_dev->crypto_engine; > struct aspeed_engine_hash *hash_engine = &hace_dev->hash_engine; > u32 sts; > > @@ -51,9 +63,24 @@ static irqreturn_t aspeed_hace_irq(int irq, void *dev) > dev_warn(hace_dev->dev, "HASH no active requests.\n"); > } > > + if (sts & HACE_CRYPTO_ISR) { > + if (crypto_engine->flags & CRYPTO_FLAGS_BUSY) > + tasklet_schedule(&crypto_engine->done_task); > + else > + dev_warn(hace_dev->dev, "CRYPTO no active requests.\n"); > + } > + > return IRQ_HANDLED; > } > > +static void aspeed_hace_crypto_done_task(unsigned long data) > +{ > + struct aspeed_hace_dev *hace_dev = (struct aspeed_hace_dev *)data; > + struct aspeed_engine_crypto *crypto_engine = &hace_dev->crypto_engine; > + > + crypto_engine->resume(hace_dev); > +} > + > static void aspeed_hace_hash_done_task(unsigned long data) > { > struct aspeed_hace_dev *hace_dev = (struct aspeed_hace_dev *)data; > @@ -65,11 +92,13 @@ static void aspeed_hace_hash_done_task(unsigned long data) > static void aspeed_hace_register(struct aspeed_hace_dev *hace_dev) > { > aspeed_register_hace_hash_algs(hace_dev); > + aspeed_register_hace_crypto_algs(hace_dev); > } > > static void aspeed_hace_unregister(struct aspeed_hace_dev *hace_dev) > { > aspeed_unregister_hace_hash_algs(hace_dev); > + aspeed_unregister_hace_crypto_algs(hace_dev); > } Could just wrap these calls instead of weak functions. static void aspeed_hace_unregister(struct aspeed_hace_dev *hace_dev) { #ifdef CONFIG_CRYPTO_DEV_ASPEED_HACE_HASH aspeed_unregister_hace_hash_algs(hace_dev); #endif #ifdef CONFIG_CRYPTO_DEV_ASPEED_HACE_CRYPTO aspeed_unregister_hace_crypto_algs(hace_dev); #endif } > > static const struct of_device_id aspeed_hace_of_matches[] = { > @@ -80,6 +109,7 @@ static const struct of_device_id aspeed_hace_of_matches[] = { > > static int aspeed_hace_probe(struct platform_device *pdev) > { > + struct aspeed_engine_crypto *crypto_engine; > const struct of_device_id *hace_dev_id; > struct aspeed_engine_hash *hash_engine; > struct aspeed_hace_dev *hace_dev; > @@ -100,6 +130,7 @@ static int aspeed_hace_probe(struct platform_device *pdev) > hace_dev->dev = &pdev->dev; > hace_dev->version = (unsigned long)hace_dev_id->data; > hash_engine = &hace_dev->hash_engine; > + crypto_engine = &hace_dev->crypto_engine; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); Thanks, Dhananjay _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel