[PATCH 0/3] mailbox: zynqmp-ipi: Improve resource cleanup on driver unbind

[PATCH 0/3] mailbox: zynqmp-ipi: Improve resource cleanup on driver unbind

[Harini T]

1. Remove redundant mbox_controller_unregister() call, as
device_unregister() already handles controller cleanup.
2. Fix the mailbox cleanup loop to prevent out-of-bounds array access by
starting at num_mboxes-1.
3. Fix SGI resource cleanup by using an explicit irq_type field for
reliable SGI detection, replacing the unreliable IRQ number check.


Harini T (3):
  mailbox: zynqmp-ipi: Remove redundant mbox_controller_unregister()
    call
  mailbox: zynqmp-ipi: Fix out-of-bounds access in mailbox cleanup loop
  mailbox: zynqmp-ipi: Fix SGI cleanup on unbind

 drivers/mailbox/zynqmp-ipi-mailbox.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH 1/3] mailbox: zynqmp-ipi: Remove redundant mbox_controller_unregister() call

[Harini T]

Remove redundant mbox_controller_unregister() call as
device_unregister() handles controller cleanup.

Signed-off-by: Harini T <harini.t@amd.com>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
index 0c143beaafda..bdcc6937ee30 100644
--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -893,11 +893,8 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
 	i = pdata->num_mboxes;
 	for (; i >= 0; i--) {
 		ipi_mbox = &pdata->ipi_mboxes[i];
-		if (ipi_mbox->dev.parent) {
-			mbox_controller_unregister(&ipi_mbox->mbox);
-			if (device_is_registered(&ipi_mbox->dev))
-				device_unregister(&ipi_mbox->dev);
-		}
+		if (device_is_registered(&ipi_mbox->dev))
+			device_unregister(&ipi_mbox->dev);
 	}
 }
 
-- 
2.43.0

Re: [PATCH 1/3] mailbox: zynqmp-ipi: Remove redundant mbox_controller_unregister() call

[Peng Fan]

On Fri, Aug 22, 2025 at 10:27:30AM +0530, Harini T wrote:
>Remove redundant mbox_controller_unregister() call as
>device_unregister() handles controller cleanup.
>

Should Fixes tag be added here?

Not sure whether device_unregister to trigger devres managed
mbox_controller_unregister would have any issue.

>Signed-off-by: Harini T <harini.t@amd.com>
>---
> drivers/mailbox/zynqmp-ipi-mailbox.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
>index 0c143beaafda..bdcc6937ee30 100644
>--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
>+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
>@@ -893,11 +893,8 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
> 	i = pdata->num_mboxes;
> 	for (; i >= 0; i--) {
> 		ipi_mbox = &pdata->ipi_mboxes[i];
>-		if (ipi_mbox->dev.parent) {
>-			mbox_controller_unregister(&ipi_mbox->mbox);
>-			if (device_is_registered(&ipi_mbox->dev))
>-				device_unregister(&ipi_mbox->dev);
>-		}
>+		if (device_is_registered(&ipi_mbox->dev))
>+			device_unregister(&ipi_mbox->dev);

The commit does not explain removing the check of ipi_mbox->dev.parent.
But I think this change is valid.

Regards
Peng

> 	}
> }
> 
>-- 
>2.43.0
>

[PATCH 2/3] mailbox: zynqmp-ipi: Fix out-of-bounds access in mailbox cleanup loop

[Harini T]

Fix mailbox cleanup loop that accesses array out-of-bounds by starting
at num_boxes instead of numb_boxes-1 for 0-indexed arrays.

Signed-off-by: Harini T <harini.t@amd.com>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
index bdcc6937ee30..3b806d1f89bb 100644
--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -891,7 +891,7 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
 		xlnx_mbox_cleanup_sgi(pdata);
 
 	i = pdata->num_mboxes;
-	for (; i >= 0; i--) {
+	for (i--; i >= 0; i--) {
 		ipi_mbox = &pdata->ipi_mboxes[i];
 		if (device_is_registered(&ipi_mbox->dev))
 			device_unregister(&ipi_mbox->dev);
-- 
2.43.0

Re: [PATCH 2/3] mailbox: zynqmp-ipi: Fix out-of-bounds access in mailbox cleanup loop

[Peng Fan]

On Fri, Aug 22, 2025 at 10:27:31AM +0530, Harini T wrote:
>Fix mailbox cleanup loop that accesses array out-of-bounds by starting
>at num_boxes instead of numb_boxes-1 for 0-indexed arrays.
>

Fix tag?

>Signed-off-by: Harini T <harini.t@amd.com>
>---
> drivers/mailbox/zynqmp-ipi-mailbox.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
>index bdcc6937ee30..3b806d1f89bb 100644
>--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
>+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
>@@ -891,7 +891,7 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
> 		xlnx_mbox_cleanup_sgi(pdata);
> 
> 	i = pdata->num_mboxes;
>-	for (; i >= 0; i--) {
>+	for (i--; i >= 0; i--) {

I would avoid i-- as the 1st param in for loop.

i = pdata->num_mboxes - 1;
or
for (i = 0; i < pdata->num_mboxes; i++)

Thanks,
Peng

> 		ipi_mbox = &pdata->ipi_mboxes[i];
> 		if (device_is_registered(&ipi_mbox->dev))
> 			device_unregister(&ipi_mbox->dev);
>-- 
>2.43.0
>

[PATCH 3/3] mailbox: zynqmp-ipi: Fix SGI cleanup on unbind

[Harini T]

The driver incorrectly determines SGI vs SPI interrupts by checking IRQ
number < 16, which fails with dynamic IRQ allocation. During unbind,
this causes improper SGI cleanup leading to kernel crash.

Add explicit irq_type field to pdata for reliable identification of SGI
interrupts (type-2) and only clean up SGI resources when appropriate.

Signed-off-by: Harini T <harini.t@amd.com>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
index 3b806d1f89bb..1af758a406e2 100644
--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -62,7 +62,8 @@
 #define DST_BIT_POS	9U
 #define SRC_BITMASK	GENMASK(11, 8)
 
-#define MAX_SGI 16
+/* Macro to represent SGI type for IPI IRQs */
+#define IPI_IRQ_TYPE_SGI	2
 
 /*
  * Module parameters
@@ -121,6 +122,7 @@ struct zynqmp_ipi_mbox {
  * @dev:                  device pointer corresponding to the Xilinx ZynqMP
  *                        IPI agent
  * @irq:                  IPI agent interrupt ID
+ * @irq_type:             IPI SGI or SPI IRQ type
  * @method:               IPI SMC or HVC is going to be used
  * @local_id:             local IPI agent ID
  * @virq_sgi:             IRQ number mapped to SGI
@@ -130,6 +132,7 @@ struct zynqmp_ipi_mbox {
 struct zynqmp_ipi_pdata {
 	struct device *dev;
 	int irq;
+	unsigned int irq_type;
 	unsigned int method;
 	u32 local_id;
 	int virq_sgi;
@@ -887,7 +890,7 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
 	struct zynqmp_ipi_mbox *ipi_mbox;
 	int i;
 
-	if (pdata->irq < MAX_SGI)
+	if (pdata->irq_type == IPI_IRQ_TYPE_SGI)
 		xlnx_mbox_cleanup_sgi(pdata);
 
 	i = pdata->num_mboxes;
@@ -956,14 +959,16 @@ static int zynqmp_ipi_probe(struct platform_device *pdev)
 		dev_err(dev, "failed to parse interrupts\n");
 		goto free_mbox_dev;
 	}
-	ret = out_irq.args[1];
+
+	/* Use interrupt type to distinguish SGI and SPI interrupts */
+	pdata->irq_type = out_irq.args[0];
 
 	/*
 	 * If Interrupt number is in SGI range, then request SGI else request
 	 * IPI system IRQ.
 	 */
-	if (ret < MAX_SGI) {
-		pdata->irq = ret;
+	if (pdata->irq_type == IPI_IRQ_TYPE_SGI) {
+		pdata->irq = out_irq.args[1];
 		ret = xlnx_mbox_init_sgi(pdev, pdata->irq, pdata);
 		if (ret)
 			goto free_mbox_dev;
-- 
2.43.0

Re: [PATCH 3/3] mailbox: zynqmp-ipi: Fix SGI cleanup on unbind

[Peng Fan]

On Fri, Aug 22, 2025 at 10:27:32AM +0530, Harini T wrote:
>The driver incorrectly determines SGI vs SPI interrupts by checking IRQ
>number < 16, which fails with dynamic IRQ allocation. During unbind,
>this causes improper SGI cleanup leading to kernel crash.
>
>Add explicit irq_type field to pdata for reliable identification of SGI
>interrupts (type-2) and only clean up SGI resources when appropriate.
>
>Signed-off-by: Harini T <harini.t@amd.com>

Fixes tag?

Then Reviewed-by: Peng Fan <peng.fan@nxp.com>