[PATCH v2 0/2] mailbox: cix: clean up channel validation and error reporting

[PATCH v2 0/2] mailbox: cix: clean up channel validation and error reporting

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

Per maintainer review feedback from Jassi Brar, this patch series splits
the CIX mailbox driver fixes into two separate patches:

Patch 1 validates fast channel index bounds before request_irq() in
cix_mbox_startup() to avoid unnecessary IRQ registration and teardown churn.

Patch 2 fixes the DT property name string typo ("cix,mbox_dir" -> "cix,mbox-dir")
and converts cix_mbox_probe() error paths to dev_err_probe().

v2:
 - Split into 2 separate patches per Jassi Brar review.
 - Drop bounced address peter.chen@cixtech.com.

Surendra Singh Chouhan (2):
  mailbox: cix: validate fast channel index before request_irq()
  mailbox: cix: fix DT property name string typo and use dev_err_probe()

 drivers/mailbox/cix-mailbox.c | 36 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

-- 
2.55.0

[PATCH v2 1/2] mailbox: cix: validate fast channel index before request_irq()

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

cix_mbox_startup() checked fast channel index constraints (index < 0 ||
index > CIX_MBOX_FAST_IDX) inside the channel switch block after
calling request_irq(). If validation failed, it triggered a free_irq()
cleanup path.

Validating channel parameters prior to request_irq() avoids unnecessary
IRQ registration and teardown churn.

Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/mailbox/cix-mailbox.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/mailbox/cix-mailbox.c b/drivers/mailbox/cix-mailbox.c
index 43c76cdab24a..615218c69eeb 100644
--- a/drivers/mailbox/cix-mailbox.c
+++ b/drivers/mailbox/cix-mailbox.c
@@ -403,6 +403,13 @@ static int cix_mbox_startup(struct mbox_chan *chan)
 	int index = cp->index, ret;
 	u32 val;
 
+	if (cp->type == CIX_MBOX_TYPE_FAST && priv->dir == CIX_MBOX_RX) {
+		if (index < 0 || index > CIX_MBOX_FAST_IDX) {
+			dev_err(priv->dev, "Invalid index %d\n", index);
+			return -EINVAL;
+		}
+	}
+
 	ret = request_irq(priv->irq, cix_mbox_isr, IRQF_NO_SUSPEND,
 			  dev_name(priv->dev), chan);
 	if (ret) {
@@ -448,11 +455,6 @@ static int cix_mbox_startup(struct mbox_chan *chan)
 	case CIX_MBOX_TYPE_FAST:
 		/* Only RX channel has intterupt */
 		if (priv->dir == CIX_MBOX_RX) {
-			if (index < 0 || index > CIX_MBOX_FAST_IDX) {
-				dev_err(priv->dev, "Invalid index %d\n", index);
-				ret = -EINVAL;
-				goto failed;
-			}
 			/* enable fast channel interrupt */
 			val = cix_mbox_read(priv, CIX_INT_ENABLE_SIDE_B);
 			val |= CIX_FAST_CH_INT(index);
@@ -461,14 +463,10 @@ static int cix_mbox_startup(struct mbox_chan *chan)
 		break;
 	default:
 		dev_err(priv->dev, "Invalid channel type: %d\n", cp->type);
-		ret = -EINVAL;
-		goto failed;
+		free_irq(priv->irq, chan);
+		return -EINVAL;
 	}
 	return 0;
-
-failed:
-	free_irq(priv->irq, chan);
-	return ret;
 }
 
 static void cix_mbox_shutdown(struct mbox_chan *chan)
-- 
2.55.0

[PATCH v2 2/2] mailbox: cix: fix DT property name string typo and use dev_err_probe()

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

cix_mbox_probe() logged property error messages referencing
"cix,mbox_dir" (with an underscore) instead of the actual DT property
string "cix,mbox-dir".

Fix the DT property string in error log messages and convert probe error
paths to dev_err_probe().

Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/mailbox/cix-mailbox.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/mailbox/cix-mailbox.c b/drivers/mailbox/cix-mailbox.c
index 615218c69eeb..fb5d7641ba90 100644
--- a/drivers/mailbox/cix-mailbox.c
+++ b/drivers/mailbox/cix-mailbox.c
@@ -585,19 +585,15 @@ static int cix_mbox_probe(struct platform_device *pdev)
 	if (priv->irq < 0)
 		return priv->irq;
 
-	if (device_property_read_string(dev, "cix,mbox-dir", &dir_str)) {
-		dev_err(priv->dev, "cix,mbox_dir property not found\n");
-		return -EINVAL;
-	}
+	if (device_property_read_string(dev, "cix,mbox-dir", &dir_str))
+		return dev_err_probe(dev, -EINVAL, "cix,mbox-dir property not found\n");
 
 	if (!strcmp(dir_str, "tx"))
 		priv->dir = 0;
 	else if (!strcmp(dir_str, "rx"))
 		priv->dir = 1;
-	else {
-		dev_err(priv->dev, "cix,mbox_dir=%s is not expected\n", dir_str);
-		return -EINVAL;
-	}
+	else
+		return dev_err_probe(dev, -EINVAL, "cix,mbox-dir=%s is not expected\n", dir_str);
 
 	cix_mbox_init(priv);
 
@@ -611,9 +607,9 @@ static int cix_mbox_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, priv);
 	ret = devm_mbox_controller_register(dev, &priv->mbox);
 	if (ret)
-		dev_err(dev, "Failed to register mailbox %d\n", ret);
+		return dev_err_probe(dev, ret, "Failed to register mailbox\n");
 
-	return ret;
+	return 0;
 }
 
 static const struct of_device_id cix_mbox_dt_ids[] = {
-- 
2.55.0

Re: [PATCH v2 1/2] mailbox: cix: validate fast channel index before request_irq()

[Guomin chen]

On Wed, Aug 12, 2026 at 03:36:22PM +0530, kr494167@gmail.com wrote:
> [Some people who received this message don't often get email from kr494167@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL
> 
> From: Surendra Singh Chouhan <kr494167@gmail.com>
> 
> cix_mbox_startup() checked fast channel index constraints (index < 0 ||
> index > CIX_MBOX_FAST_IDX) inside the channel switch block after
> calling request_irq(). If validation failed, it triggered a free_irq()
> cleanup path.
> 
> Validating channel parameters prior to request_irq() avoids unnecessary
> IRQ registration and teardown churn.
> 
> Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
> ---
>  drivers/mailbox/cix-mailbox.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mailbox/cix-mailbox.c b/drivers/mailbox/cix-mailbox.c
> index 43c76cdab24a..615218c69eeb 100644
> --- a/drivers/mailbox/cix-mailbox.c
> +++ b/drivers/mailbox/cix-mailbox.c
> @@ -403,6 +403,13 @@ static int cix_mbox_startup(struct mbox_chan *chan)
>         int index = cp->index, ret;
>         u32 val;
> 
> +       if (cp->type == CIX_MBOX_TYPE_FAST && priv->dir == CIX_MBOX_RX) {
> +               if (index < 0 || index > CIX_MBOX_FAST_IDX) {
> +                       dev_err(priv->dev, "Invalid index %d\n", index);
> +                       return -EINVAL;
> +               }
> +       }
> +
>         ret = request_irq(priv->irq, cix_mbox_isr, IRQF_NO_SUSPEND,
>                           dev_name(priv->dev), chan);
>         if (ret) {
> @@ -448,11 +455,6 @@ static int cix_mbox_startup(struct mbox_chan *chan)
>         case CIX_MBOX_TYPE_FAST:
>                 /* Only RX channel has intterupt */
>                 if (priv->dir == CIX_MBOX_RX) {
> -                       if (index < 0 || index > CIX_MBOX_FAST_IDX) {
> -                               dev_err(priv->dev, "Invalid index %d\n", index);
> -                               ret = -EINVAL;
> -                               goto failed;
> -                       }
>                         /* enable fast channel interrupt */
>                         val = cix_mbox_read(priv, CIX_INT_ENABLE_SIDE_B);
>                         val |= CIX_FAST_CH_INT(index);
> @@ -461,14 +463,10 @@ static int cix_mbox_startup(struct mbox_chan *chan)
>                 break;
>         default:
>                 dev_err(priv->dev, "Invalid channel type: %d\n", cp->type);
> -               ret = -EINVAL;
> -               goto failed;
> +               free_irq(priv->irq, chan);
> +               return -EINVAL;
>         }
>         return 0;
> -
> -failed:
> -       free_irq(priv->irq, chan);
> -       return ret;
>  }
The premise of the commit message doesn't hold, so I don't think this
patch should be applied as-is.

The check being moved,index < 0 || index > CIX_MBOX_FAST_IDX, is
unreachable for a CIX_MBOX_TYPE_FAST channel: cp->index and cp->type
are only ever assigned in cix_mbox_init(), where cp->index = i (so >= 0)
and CIX_MBOX_TYPE_FAST is only set when cp->index <= CIX_MBOX_FAST_IDX.
There is no DT or probe path that overrides either field. So for any
FAST channel the condition is always false, and the "unnecessary IRQ
registration and teardown churn" the message describes cannot actually
occur at runtime.

Given that, the patch is reshuffling dead code: it duplicates the
(type == FAST && dir == RX) condition outside the switch, adding a
second spot that has to stay in sync with the FAST case, in exchange
for optimizing a path that never executes. That's net negative as-is.

Best regards,
Guomin.Chen
>
>  static void cix_mbox_shutdown(struct mbox_chan *chan)
> --
> 2.55.0
> 

Re: [PATCH v2 2/2] mailbox: cix: fix DT property name string typo and use dev_err_probe()

[Guomin chen]

On Wed, Aug 12, 2026 at 03:36:23PM +0530, kr494167@gmail.com wrote:
> [Some people who received this message don't often get email from kr494167@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL
> 
> From: Surendra Singh Chouhan <kr494167@gmail.com>
> 
> cix_mbox_probe() logged property error messages referencing
> "cix,mbox_dir" (with an underscore) instead of the actual DT property
> string "cix,mbox-dir".
> 
> Fix the DT property string in error log messages and convert probe error
> paths to dev_err_probe().
> 
> Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
> ---
>  drivers/mailbox/cix-mailbox.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mailbox/cix-mailbox.c b/drivers/mailbox/cix-mailbox.c
> index 615218c69eeb..fb5d7641ba90 100644
> --- a/drivers/mailbox/cix-mailbox.c
> +++ b/drivers/mailbox/cix-mailbox.c
> @@ -585,19 +585,15 @@ static int cix_mbox_probe(struct platform_device *pdev)
>         if (priv->irq < 0)
>                 return priv->irq;
> 
> -       if (device_property_read_string(dev, "cix,mbox-dir", &dir_str)) {
> -               dev_err(priv->dev, "cix,mbox_dir property not found\n");
> -               return -EINVAL;
> -       }
> +       if (device_property_read_string(dev, "cix,mbox-dir", &dir_str))
> +               return dev_err_probe(dev, -EINVAL, "cix,mbox-dir property not found\n");
> 
>         if (!strcmp(dir_str, "tx"))
>                 priv->dir = 0;
>         else if (!strcmp(dir_str, "rx"))
>                 priv->dir = 1;
> -       else {
> -               dev_err(priv->dev, "cix,mbox_dir=%s is not expected\n", dir_str);
> -               return -EINVAL;
> -       }
> +       else
> +               return dev_err_probe(dev, -EINVAL, "cix,mbox-dir=%s is not expected\n", dir_str);
> 
>         cix_mbox_init(priv);
> 
> @@ -611,9 +607,9 @@ static int cix_mbox_probe(struct platform_device *pdev)
>         platform_set_drvdata(pdev, priv);
>         ret = devm_mbox_controller_register(dev, &priv->mbox);
>         if (ret)
> -               dev_err(dev, "Failed to register mailbox %d\n", ret);
> +               return dev_err_probe(dev, ret, "Failed to register mailbox\n");
> 
> -       return ret;
> +       return 0;
>  }
>
Reviewed-by: Guomin Chen <Guomin.Chen@cixtech.com>

Best regards,
Guomin Chen