diff mbox series

[07/12] crypto: caam - use devres to de-initialize the RNG

Message ID 20190904023515.7107-8-andrew.smirnov@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series CAAM bugfixes, small improvements | expand

Commit Message

Andrey Smirnov Sept. 4, 2019, 2:35 a.m. UTC
Use devres to de-initialize the RNG and drop explicit de-initialization
code in caam_remove().

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Iuliana Prodan <iuliana.prodan@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c | 129 ++++++++++++++++++++-----------------
 1 file changed, 70 insertions(+), 59 deletions(-)

Comments

Horia Geanta Sept. 9, 2019, 3:39 p.m. UTC | #1
On 9/4/2019 5:35 AM, Andrey Smirnov wrote:
> Use devres to de-initialize the RNG and drop explicit de-initialization
> code in caam_remove().
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Horia Geantă <horia.geanta@nxp.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Iuliana Prodan <iuliana.prodan@nxp.com>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/crypto/caam/ctrl.c | 129 ++++++++++++++++++++-----------------
>  1 file changed, 70 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> index 254963498abc..25f8f76551a5 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -175,6 +175,73 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
>  	return 0;
>  }
>  
> +/*
> + * deinstantiate_rng - builds and executes a descriptor on DECO0,
> + *		       which deinitializes the RNG block.
> + * @ctrldev - pointer to device
> + * @state_handle_mask - bitmask containing the instantiation status
> + *			for the RNG4 state handles which exist in
> + *			the RNG4 block: 1 if it's been instantiated
> + *
> + * Return: - 0 if no error occurred
> + *	   - -ENOMEM if there isn't enough memory to allocate the descriptor
> + *	   - -ENODEV if DECO0 couldn't be acquired
> + *	   - -EAGAIN if an error occurred when executing the descriptor
> + */
> +static int deinstantiate_rng(struct device *ctrldev, int state_handle_mask)
I assume this function is not modified, only moved further up
to avoid forward declaration.

> +	if (!ret) {
> +		ret = devm_add_action_or_reset(ctrldev, devm_deinstantiate_rng,
> +					       ctrldev);
>  	}
Braces not needed.

Is there any guidance wrt. when *not* to use devres?

Horia
Andrey Smirnov Sept. 18, 2019, 6:06 a.m. UTC | #2
On Mon, Sep 9, 2019 at 8:39 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>
> On 9/4/2019 5:35 AM, Andrey Smirnov wrote:
> > Use devres to de-initialize the RNG and drop explicit de-initialization
> > code in caam_remove().
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Horia Geantă <horia.geanta@nxp.com>
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: Iuliana Prodan <iuliana.prodan@nxp.com>
> > Cc: linux-crypto@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  drivers/crypto/caam/ctrl.c | 129 ++++++++++++++++++++-----------------
> >  1 file changed, 70 insertions(+), 59 deletions(-)
> >
> > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> > index 254963498abc..25f8f76551a5 100644
> > --- a/drivers/crypto/caam/ctrl.c
> > +++ b/drivers/crypto/caam/ctrl.c
> > @@ -175,6 +175,73 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
> >       return 0;
> >  }
> >
> > +/*
> > + * deinstantiate_rng - builds and executes a descriptor on DECO0,
> > + *                  which deinitializes the RNG block.
> > + * @ctrldev - pointer to device
> > + * @state_handle_mask - bitmask containing the instantiation status
> > + *                   for the RNG4 state handles which exist in
> > + *                   the RNG4 block: 1 if it's been instantiated
> > + *
> > + * Return: - 0 if no error occurred
> > + *      - -ENOMEM if there isn't enough memory to allocate the descriptor
> > + *      - -ENODEV if DECO0 couldn't be acquired
> > + *      - -EAGAIN if an error occurred when executing the descriptor
> > + */
> > +static int deinstantiate_rng(struct device *ctrldev, int state_handle_mask)
> I assume this function is not modified, only moved further up
> to avoid forward declaration.
>

Correct.

> > +     if (!ret) {
> > +             ret = devm_add_action_or_reset(ctrldev, devm_deinstantiate_rng,
> > +                                            ctrldev);
> >       }
> Braces not needed.
>

OK, will remove in next version.

> Is there any guidance wrt. when *not* to use devres?
>

Not that I now of.

Thanks,
Andrey Smirnov
diff mbox series

Patch

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 254963498abc..25f8f76551a5 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -175,6 +175,73 @@  static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
 	return 0;
 }
 
+/*
+ * deinstantiate_rng - builds and executes a descriptor on DECO0,
+ *		       which deinitializes the RNG block.
+ * @ctrldev - pointer to device
+ * @state_handle_mask - bitmask containing the instantiation status
+ *			for the RNG4 state handles which exist in
+ *			the RNG4 block: 1 if it's been instantiated
+ *
+ * Return: - 0 if no error occurred
+ *	   - -ENOMEM if there isn't enough memory to allocate the descriptor
+ *	   - -ENODEV if DECO0 couldn't be acquired
+ *	   - -EAGAIN if an error occurred when executing the descriptor
+ */
+static int deinstantiate_rng(struct device *ctrldev, int state_handle_mask)
+{
+	u32 *desc, status;
+	int sh_idx, ret = 0;
+
+	desc = kmalloc(CAAM_CMD_SZ * 3, GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+
+	for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
+		/*
+		 * If the corresponding bit is set, then it means the state
+		 * handle was initialized by us, and thus it needs to be
+		 * deinitialized as well
+		 */
+		if ((1 << sh_idx) & state_handle_mask) {
+			/*
+			 * Create the descriptor for deinstantating this state
+			 * handle
+			 */
+			build_deinstantiation_desc(desc, sh_idx);
+
+			/* Try to run it through DECO0 */
+			ret = run_descriptor_deco0(ctrldev, desc, &status);
+
+			if (ret ||
+			    (status && status != JRSTA_SSRC_JUMP_HALT_CC)) {
+				dev_err(ctrldev,
+					"Failed to deinstantiate RNG4 SH%d\n",
+					sh_idx);
+				break;
+			}
+			dev_info(ctrldev, "Deinstantiated RNG4 SH%d\n", sh_idx);
+		}
+	}
+
+	kfree(desc);
+
+	return ret;
+}
+
+static void devm_deinstantiate_rng(void *data)
+{
+	struct device *ctrldev = data;
+	struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev);
+
+	/*
+	 * De-initialize RNG state handles initialized by this driver.
+	 * In case of SoCs with Management Complex, RNG is managed by MC f/w.
+	 */
+	if (ctrlpriv->rng4_sh_init)
+		deinstantiate_rng(ctrldev, ctrlpriv->rng4_sh_init);
+}
+
 /*
  * instantiate_rng - builds and executes a descriptor on DECO0,
  *		     which initializes the RNG block.
@@ -247,60 +314,11 @@  static int instantiate_rng(struct device *ctrldev, int state_handle_mask,
 
 	kfree(desc);
 
-	return ret;
-}
-
-/*
- * deinstantiate_rng - builds and executes a descriptor on DECO0,
- *		       which deinitializes the RNG block.
- * @ctrldev - pointer to device
- * @state_handle_mask - bitmask containing the instantiation status
- *			for the RNG4 state handles which exist in
- *			the RNG4 block: 1 if it's been instantiated
- *
- * Return: - 0 if no error occurred
- *	   - -ENOMEM if there isn't enough memory to allocate the descriptor
- *	   - -ENODEV if DECO0 couldn't be acquired
- *	   - -EAGAIN if an error occurred when executing the descriptor
- */
-static int deinstantiate_rng(struct device *ctrldev, int state_handle_mask)
-{
-	u32 *desc, status;
-	int sh_idx, ret = 0;
-
-	desc = kmalloc(CAAM_CMD_SZ * 3, GFP_KERNEL);
-	if (!desc)
-		return -ENOMEM;
-
-	for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
-		/*
-		 * If the corresponding bit is set, then it means the state
-		 * handle was initialized by us, and thus it needs to be
-		 * deinitialized as well
-		 */
-		if ((1 << sh_idx) & state_handle_mask) {
-			/*
-			 * Create the descriptor for deinstantating this state
-			 * handle
-			 */
-			build_deinstantiation_desc(desc, sh_idx);
-
-			/* Try to run it through DECO0 */
-			ret = run_descriptor_deco0(ctrldev, desc, &status);
-
-			if (ret ||
-			    (status && status != JRSTA_SSRC_JUMP_HALT_CC)) {
-				dev_err(ctrldev,
-					"Failed to deinstantiate RNG4 SH%d\n",
-					sh_idx);
-				break;
-			}
-			dev_info(ctrldev, "Deinstantiated RNG4 SH%d\n", sh_idx);
-		}
+	if (!ret) {
+		ret = devm_add_action_or_reset(ctrldev, devm_deinstantiate_rng,
+					       ctrldev);
 	}
 
-	kfree(desc);
-
 	return ret;
 }
 
@@ -320,13 +338,6 @@  static int caam_remove(struct platform_device *pdev)
 		caam_qi_shutdown(ctrldev);
 #endif
 
-	/*
-	 * De-initialize RNG state handles initialized by this driver.
-	 * In case of SoCs with Management Complex, RNG is managed by MC f/w.
-	 */
-	if (!ctrlpriv->mc_en && ctrlpriv->rng4_sh_init)
-		deinstantiate_rng(ctrldev, ctrlpriv->rng4_sh_init);
-
 	return 0;
 }