Skip to contentSkip to conversation

crypto: talitos: init tasklets before requesting IRQs

PATCH In review Rosen Penev 2 participants 2 messages Oct 4 – Oct 8 Crypto API

lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 crypto: talitos: init tasklets before requesting IRQs

The talitos interrupt handlers schedule priv->done_task[] via tasklet_schedule(). Since the second IRQ support, probe has requested the IRQs before calling tasklet_init(), so an interrupt already pending when a line is requested, e.g. engine state left by the bootloader or a kexec'd kernel, would schedule a tasklet with a NULL callback.

Look up the IRQ numbers and map the registers up front, so that -EPROBE_DEFER returns before anything is allocated and the done_task[] selection and reset_device() can see the secondary IRQ. Only request the IRQs once the channel fifos are allocated and init_device() has reset the engine. This matches remove(), which frees the IRQs before killing the tasklets.

remove() frees every IRQ number that is > 0, so make sure that only requested lines are set when it runs from the probe error path: clear both numbers when probe fails before the request, and clear any line talitos_probe_irq() did not request. Previously a failed primary request in the two-IRQ case freed the never-requested secondary line.

Fixes c3e337f88a5b ("crypto: talitos - support for channel remap and 2nd IRQ")
Assisted-by LLM

---
v4: actually fix IRQs this time. Add a Fixes: tag.
v3: reshuffle again to fix IRQs.
v2: reshuffle code to avoid NULL derefs

 drivers/crypto/talitos.c | 54 ++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 21 deletions(-)
drivers/crypto/talitos.c +33 -21
index 41a87d7c30a0..c7d9ce2905be 100644
@@ -3353,21 +3353,13 @@ static int talitos_probe_irq(struct platform_device *ofdev)
 	int err;
 	bool is_sec1 = has_ftr_sec1(priv);
 
-	priv->irq[0] = platform_get_irq(ofdev, 0);
-	if (priv->irq[0] < 0)
-		return priv->irq[0];
-
 	if (is_sec1) {
 		err = request_irq(priv->irq[0], talitos1_interrupt_4ch, 0,
 				  dev_driver_string(dev), priv);
 		goto primary_out;
 	}
 
-	priv->irq[1] = platform_get_irq_optional(ofdev, 1);
-	if (priv->irq[1] == -EPROBE_DEFER)
-		return priv->irq[1];
-
-	/* get the primary irq line */
+	/* single (or primary) irq line */
 	if (priv->irq[1] < 0) {
 		err = request_irq(priv->irq[0], talitos2_interrupt_4ch, 0,
 				  dev_driver_string(dev), priv);
@@ -3379,7 +3371,7 @@ static int talitos_probe_irq(struct platform_device *ofdev)
 	if (err)
 		goto primary_out;
 
-	/* get the secondary irq line */
+	/* secondary irq line */
 	err = request_irq(priv->irq[1], talitos2_interrupt_ch1_3, 0,
 			  dev_driver_string(dev), priv);
 	if (err) {
@@ -3392,7 +3384,9 @@ static int talitos_probe_irq(struct platform_device *ofdev)
 primary_out:
 	if (err) {
 		dev_err(dev, "failed to request primary irq\n");
+		/* neither line is requested, keep remove() from freeing them */
 		priv->irq[0] = 0;
+		priv->irq[1] = 0;
 	}
 
 	return err;
@@ -3404,12 +3398,27 @@ static int talitos_probe(struct platform_device *ofdev)
 	struct device_node *np = ofdev->dev.of_node;
 	struct talitos_private *priv;
 	unsigned int num_channels;
+	void __iomem *reg;
 	int i, err;
 	int stride;
+	int irq0;
+	int irq1;
 
 	if (of_property_read_u32(np, "fsl,num-channels", &num_channels))
 		return -EINVAL;
 
+	irq0 = platform_get_irq(ofdev, 0);
+	if (irq0 < 0)
+		return irq0;
+
+	irq1 = platform_get_irq_optional(ofdev, 1);
+	if (irq1 == -EPROBE_DEFER)
+		return irq1;
+
+	reg = devm_platform_ioremap_resource(ofdev, 0);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
 	priv = devm_kzalloc(dev, struct_size(priv, chan, num_channels), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -3425,12 +3434,7 @@ static int talitos_probe(struct platform_device *ofdev)
 
 	spin_lock_init(&priv->reg_lock);
 
-	priv->reg = devm_platform_ioremap_resource(ofdev, 0);
-	if (IS_ERR(priv->reg)) {
-		dev_err(dev, "failed to of_iomap\n");
-		err = PTR_ERR(priv->reg);
-		goto err_out;
-	}
+	priv->reg = reg;
 
 	/* get SEC version capabilities from device tree */
 	of_property_read_u32(np, "fsl,channel-fifo-len", &priv->chfifo_len);
@@ -3481,9 +3485,7 @@ static int talitos_probe(struct platform_device *ofdev)
 		stride = TALITOS2_CH_STRIDE;
 	}
 
-	err = talitos_probe_irq(ofdev);
-	if (err)
-		goto err_out;
+	priv->irq[0] = irq0;
 
 	if (has_ftr_sec1(priv)) {
 		if (priv->num_channels == 1)
@@ -3493,6 +3495,7 @@ static int talitos_probe(struct platform_device *ofdev)
 			tasklet_init(&priv->done_task[0], talitos1_done_4ch,
 				     (unsigned long)dev);
 	} else {
+		priv->irq[1] = irq1;
 		if (priv->irq[1] > 0) {
 			tasklet_init(&priv->done_task[0], talitos2_done_ch0_2,
 				     (unsigned long)dev);
@@ -3524,7 +3527,7 @@ static int talitos_probe(struct platform_device *ofdev)
 		if (!priv->chan[i].fifo) {
 			dev_err(dev, "failed to allocate request fifo %d\n", i);
 			err = -ENOMEM;
-			goto err_out;
+			goto err_noirq;
 		}
 
 		atomic_set(&priv->chan[i].submit_count,
@@ -3537,9 +3540,14 @@ static int talitos_probe(struct platform_device *ofdev)
 	err = init_device(dev);
 	if (err) {
 		dev_err(dev, "failed to initialize device\n");
-		goto err_out;
+		goto err_noirq;
 	}
 
+	/* enable interrupts once the channel fifos and tasklets are set up */
+	err = talitos_probe_irq(ofdev);
+	if (err)
+		goto err_out;
+
 	/* register the RNG, if available */
 	if (hw_supports(dev, DESC_HDR_SEL0_RNG)) {
 		err = talitos_register_rng(dev);
@@ -3597,6 +3605,10 @@ static int talitos_probe(struct platform_device *ofdev)
 
 	return 0;
 
+err_noirq:
+	/* the irq numbers are known, but the lines are not requested yet */
+	priv->irq[0] = 0;
+	priv->irq[1] = 0;
 err_out:
 	talitos_remove(ofdev);
 
Signature · 1 line
2.56.0
maintainer
#1 lore raw
Quoted 22 lines · Rosen Penev

The talitos interrupt handlers schedule priv->done_task[] via tasklet_schedule(). Since the second IRQ support, probe has requested the IRQs before calling tasklet_init(), so an interrupt already pending when a line is requested, e.g. engine state left by the bootloader or a kexec'd kernel, would schedule a tasklet with a NULL callback.

Look up the IRQ numbers and map the registers up front, so that -EPROBE_DEFER returns before anything is allocated and the done_task[] selection and reset_device() can see the secondary IRQ. Only request the IRQs once the channel fifos are allocated and init_device() has reset the engine. This matches remove(), which frees the IRQs before killing the tasklets.

remove() frees every IRQ number that is > 0, so make sure that only requested lines are set when it runs from the probe error path: clear both numbers when probe fails before the request, and clear any line talitos_probe_irq() did not request. Previously a failed primary request in the two-IRQ case freed the never-requested secondary line.

Fixes c3e337f88a5b ("crypto: talitos - support for channel remap and 2nd IRQ")
Assisted-by LLM
Quoted 6 lines

---
v4: actually fix IRQs this time. Add a Fixes: tag.
v3: reshuffle again to fix IRQs.
v2: reshuffle code to avoid NULL derefs

 drivers/crypto/talitos.c | 54 ++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 21 deletions(-)

Patch applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt