[PATCH v1] mailbox: mchp-ipc-sbi: fix uninitialized symbol and other smatch warnings

Valentina Fernandez posted 1 patch 1 month, 3 weeks ago
drivers/mailbox/mailbox-mchp-ipc-sbi.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
[PATCH v1] mailbox: mchp-ipc-sbi: fix uninitialized symbol and other smatch warnings
Posted by Valentina Fernandez 1 month, 3 weeks ago
Fix uninitialized symbol 'hartid' warning in mchp_ipc_cluster_aggr_isr()
by introducing a 'found' flag to track whether the IRQ matches any
online hart. If no match is found, return IRQ_NONE.

Also fix other smatch warnings by removing dead code in
mchp_ipc_startup() and by returning -ENODEV in dev_err_probe() if the
Microchip SBI extension is not found.

Fixes below smatch warnings:
drivers/mailbox/mailbox-mchp-ipc-sbi.c:187 mchp_ipc_cluster_aggr_isr() error: uninitialized symbol 'hartid'.
drivers/mailbox/mailbox-mchp-ipc-sbi.c:324 mchp_ipc_startup() warn: ignoring unreachable code.
drivers/mailbox/mailbox-mchp-ipc-sbi.c:422 mchp_ipc_probe() warn: passing zero to 'dev_err_probe'

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202512171533.CDLdScMY-lkp@intel.com/
Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com>
---
 drivers/mailbox/mailbox-mchp-ipc-sbi.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/mailbox/mailbox-mchp-ipc-sbi.c b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
index d444491a584e..b87bf2fb4b9b 100644
--- a/drivers/mailbox/mailbox-mchp-ipc-sbi.c
+++ b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
@@ -174,17 +174,21 @@ static irqreturn_t mchp_ipc_cluster_aggr_isr(int irq, void *data)
 	struct mchp_ipc_msg ipc_msg;
 	struct mchp_ipc_status status_msg;
 	int ret;
-	unsigned long hartid;
 	u32 i, chan_index, chan_id;
+	bool found = false;
 
 	/* Find out the hart that originated the irq */
 	for_each_online_cpu(i) {
-		hartid = cpuid_to_hartid_map(i);
-		if (irq == ipc->cluster_cfg[i].irq)
+		if (irq == ipc->cluster_cfg[i].irq) {
+			found = true;
 			break;
+		}
 	}
 
-	status_msg.cluster = hartid;
+	if (unlikely(!found))
+		return IRQ_NONE;
+
+	status_msg.cluster = cpuid_to_hartid_map(i);
 	memcpy(ipc->cluster_cfg[i].buf_base, &status_msg, sizeof(struct mchp_ipc_status));
 
 	ret = mchp_ipc_sbi_send(SBI_EXT_IPC_STATUS, ipc->cluster_cfg[i].buf_base_addr);
@@ -321,13 +325,6 @@ static int mchp_ipc_startup(struct mbox_chan *chan)
 		goto fail_free_buf_msg_rx;
 	}
 
-	if (ret) {
-		dev_err(ipc->dev, "failed to register interrupt(s)\n");
-		goto fail_free_buf_msg_rx;
-	}
-
-	return ret;
-
 fail_free_buf_msg_rx:
 	kfree(chan_info->msg_buf_rx);
 fail_free_buf_msg_tx:
@@ -419,7 +416,7 @@ static int mchp_ipc_probe(struct platform_device *pdev)
 
 	ret = sbi_probe_extension(SBI_EXT_MICROCHIP_TECHNOLOGY);
 	if (ret <= 0)
-		return dev_err_probe(dev, ret, "Microchip SBI extension not detected\n");
+		return dev_err_probe(dev, -ENODEV, "Microchip SBI extension not detected\n");
 
 	ipc = devm_kzalloc(dev, sizeof(*ipc), GFP_KERNEL);
 	if (!ipc)
-- 
2.34.1
Re: [PATCH v1] mailbox: mchp-ipc-sbi: fix uninitialized symbol and other smatch warnings
Posted by Jassi Brar 2 weeks, 5 days ago
On Thu, Dec 18, 2025 at 4:35 AM Valentina Fernandez
<valentina.fernandezalanis@microchip.com> wrote:
>
> Fix uninitialized symbol 'hartid' warning in mchp_ipc_cluster_aggr_isr()
> by introducing a 'found' flag to track whether the IRQ matches any
> online hart. If no match is found, return IRQ_NONE.
>
> Also fix other smatch warnings by removing dead code in
> mchp_ipc_startup() and by returning -ENODEV in dev_err_probe() if the
> Microchip SBI extension is not found.
>
> Fixes below smatch warnings:
> drivers/mailbox/mailbox-mchp-ipc-sbi.c:187 mchp_ipc_cluster_aggr_isr() error: uninitialized symbol 'hartid'.
> drivers/mailbox/mailbox-mchp-ipc-sbi.c:324 mchp_ipc_startup() warn: ignoring unreachable code.
> drivers/mailbox/mailbox-mchp-ipc-sbi.c:422 mchp_ipc_probe() warn: passing zero to 'dev_err_probe'
>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202512171533.CDLdScMY-lkp@intel.com/
> Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com>
> ---
>  drivers/mailbox/mailbox-mchp-ipc-sbi.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mailbox/mailbox-mchp-ipc-sbi.c b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> index d444491a584e..b87bf2fb4b9b 100644
> --- a/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> +++ b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> @@ -174,17 +174,21 @@ static irqreturn_t mchp_ipc_cluster_aggr_isr(int irq, void *data)
>         struct mchp_ipc_msg ipc_msg;
>         struct mchp_ipc_status status_msg;
>         int ret;
> -       unsigned long hartid;
>         u32 i, chan_index, chan_id;
> +       bool found = false;
>
>         /* Find out the hart that originated the irq */
>         for_each_online_cpu(i) {
> -               hartid = cpuid_to_hartid_map(i);
> -               if (irq == ipc->cluster_cfg[i].irq)
> +               if (irq == ipc->cluster_cfg[i].irq) {
> +                       found = true;
>                         break;
> +               }
>         }
>
> -       status_msg.cluster = hartid;
> +       if (unlikely(!found))
> +               return IRQ_NONE;
> +
> +       status_msg.cluster = cpuid_to_hartid_map(i);
>         memcpy(ipc->cluster_cfg[i].buf_base, &status_msg, sizeof(struct mchp_ipc_status));
>
>         ret = mchp_ipc_sbi_send(SBI_EXT_IPC_STATUS, ipc->cluster_cfg[i].buf_base_addr);
> @@ -321,13 +325,6 @@ static int mchp_ipc_startup(struct mbox_chan *chan)
>                 goto fail_free_buf_msg_rx;
>         }
>
> -       if (ret) {
> -               dev_err(ipc->dev, "failed to register interrupt(s)\n");
> -               goto fail_free_buf_msg_rx;
> -       }
> -
> -       return ret;
> -
>  fail_free_buf_msg_rx:
>         kfree(chan_info->msg_buf_rx);
>  fail_free_buf_msg_tx:
> @@ -419,7 +416,7 @@ static int mchp_ipc_probe(struct platform_device *pdev)
>
>         ret = sbi_probe_extension(SBI_EXT_MICROCHIP_TECHNOLOGY);
>         if (ret <= 0)
> -               return dev_err_probe(dev, ret, "Microchip SBI extension not detected\n");
> +               return dev_err_probe(dev, -ENODEV, "Microchip SBI extension not detected\n");
>
>         ipc = devm_kzalloc(dev, sizeof(*ipc), GFP_KERNEL);
>         if (!ipc)
> --
> 2.34.1
>
Applied to mailbox/for-next.
Thanks.
Re: [PATCH v1] mailbox: mchp-ipc-sbi: fix uninitialized symbol and other smatch warnings
Posted by Dan Carpenter 1 month, 3 weeks ago
On Thu, Dec 18, 2025 at 10:33:59AM +0000, Valentina Fernandez wrote:
> Fix uninitialized symbol 'hartid' warning in mchp_ipc_cluster_aggr_isr()
> by introducing a 'found' flag to track whether the IRQ matches any
> online hart. If no match is found, return IRQ_NONE.
> 
> Also fix other smatch warnings by removing dead code in
> mchp_ipc_startup() and by returning -ENODEV in dev_err_probe() if the
> Microchip SBI extension is not found.
> 
> Fixes below smatch warnings:
> drivers/mailbox/mailbox-mchp-ipc-sbi.c:187 mchp_ipc_cluster_aggr_isr() error: uninitialized symbol 'hartid'.
> drivers/mailbox/mailbox-mchp-ipc-sbi.c:324 mchp_ipc_startup() warn: ignoring unreachable code.
> drivers/mailbox/mailbox-mchp-ipc-sbi.c:422 mchp_ipc_probe() warn: passing zero to 'dev_err_probe'
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202512171533.CDLdScMY-lkp@intel.com/
> Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com>
> ---
>  drivers/mailbox/mailbox-mchp-ipc-sbi.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mailbox/mailbox-mchp-ipc-sbi.c b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> index d444491a584e..b87bf2fb4b9b 100644
> --- a/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> +++ b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> @@ -174,17 +174,21 @@ static irqreturn_t mchp_ipc_cluster_aggr_isr(int irq, void *data)
>  	struct mchp_ipc_msg ipc_msg;
>  	struct mchp_ipc_status status_msg;
>  	int ret;
> -	unsigned long hartid;
>  	u32 i, chan_index, chan_id;
> +	bool found = false;
>  
>  	/* Find out the hart that originated the irq */
>  	for_each_online_cpu(i) {
> -		hartid = cpuid_to_hartid_map(i);
> -		if (irq == ipc->cluster_cfg[i].irq)
> +		if (irq == ipc->cluster_cfg[i].irq) {
> +			found = true;
>  			break;
> +		}
>  	}
>  
> -	status_msg.cluster = hartid;
> +	if (unlikely(!found))
> +		return IRQ_NONE;

This one is a false positive because obviously there is going to be
at least one online cpu.  I would prefer to silence this in Smatch.
Generally, you should ignore static checker false positives.

regards,
dan carpenter

From 3a2a0587208ca2f720cf170c1c9b84d155e87316 Mon Sep 17 00:00:00 2001
From: Dan Carpenter <dan.carpenter@linaro.org>
Date: Thu, 18 Dec 2025 14:17:36 +0300
Subject: [PATCH] kernel: add some more once_through macros

These macro loops always iterate at least once.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 check_kernel.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/check_kernel.c b/check_kernel.c
index 0db972637aa2..03d1982db49c 100644
--- a/check_kernel.c
+++ b/check_kernel.c
@@ -765,7 +765,7 @@ static bool delete_pci_error_returns(struct expression *expr)
 	return false;
 }
 
-static bool match_with_intel_runtime(struct statement *stmt)
+static bool match_once_through_macros(struct statement *stmt)
 {
 	char *macro;
 
@@ -774,7 +774,10 @@ static bool match_with_intel_runtime(struct statement *stmt)
 		return false;
 	if (strncmp(macro, "with_intel_runtime", 18) == 0 ||
 	    strncmp(macro, "with_intel_display", 18) == 0 ||
-	    strcmp(macro, "drm_exec_until_all_locked") == 0)
+	    strcmp(macro, "drm_exec_until_all_locked") == 0 ||
+	    strcmp(macro, "xe_validation_guard") == 0 ||
+	    strcmp(macro, "for_each_gt") == 0 ||
+	    strcmp(macro, "for_each_online_cpu") ==0)
 		return true;
 	return false;
 }
@@ -821,7 +824,7 @@ void check_kernel(int id)
 	add_function_hook("closure_call", &match_closure_call, NULL);
 	add_function_hook("put_device", &match_put_device, NULL);
 
-	add_once_through_hook(&match_with_intel_runtime);
+	add_once_through_hook(&match_once_through_macros);
 
 	add_hook(fix_msecs_to_jiffies, ASSIGNMENT_HOOK_AFTER);
 	add_hook(&match_kernel_param, BASE_HOOK);
-- 
2.51.0
Re: [PATCH v1] mailbox: mchp-ipc-sbi: fix uninitialized symbol and other smatch warnings
Posted by Dan Carpenter 1 month, 3 weeks ago
On Thu, Dec 18, 2025 at 02:23:03PM +0300, Dan Carpenter wrote:
> On Thu, Dec 18, 2025 at 10:33:59AM +0000, Valentina Fernandez wrote:
> > Fix uninitialized symbol 'hartid' warning in mchp_ipc_cluster_aggr_isr()
> > by introducing a 'found' flag to track whether the IRQ matches any
> > online hart. If no match is found, return IRQ_NONE.
> > 
> > Also fix other smatch warnings by removing dead code in
> > mchp_ipc_startup() and by returning -ENODEV in dev_err_probe() if the
> > Microchip SBI extension is not found.
> > 
> > Fixes below smatch warnings:
> > drivers/mailbox/mailbox-mchp-ipc-sbi.c:187 mchp_ipc_cluster_aggr_isr() error: uninitialized symbol 'hartid'.
> > drivers/mailbox/mailbox-mchp-ipc-sbi.c:324 mchp_ipc_startup() warn: ignoring unreachable code.
> > drivers/mailbox/mailbox-mchp-ipc-sbi.c:422 mchp_ipc_probe() warn: passing zero to 'dev_err_probe'
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Closes: https://lore.kernel.org/r/202512171533.CDLdScMY-lkp@intel.com/
> > Signed-off-by: Valentina Fernandez <valentina.fernandezalanis@microchip.com>
> > ---
> >  drivers/mailbox/mailbox-mchp-ipc-sbi.c | 21 +++++++++------------
> >  1 file changed, 9 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mailbox/mailbox-mchp-ipc-sbi.c b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> > index d444491a584e..b87bf2fb4b9b 100644
> > --- a/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> > +++ b/drivers/mailbox/mailbox-mchp-ipc-sbi.c
> > @@ -174,17 +174,21 @@ static irqreturn_t mchp_ipc_cluster_aggr_isr(int irq, void *data)
> >  	struct mchp_ipc_msg ipc_msg;
> >  	struct mchp_ipc_status status_msg;
> >  	int ret;
> > -	unsigned long hartid;
> >  	u32 i, chan_index, chan_id;
> > +	bool found = false;
> >  
> >  	/* Find out the hart that originated the irq */
> >  	for_each_online_cpu(i) {
> > -		hartid = cpuid_to_hartid_map(i);
> > -		if (irq == ipc->cluster_cfg[i].irq)
> > +		if (irq == ipc->cluster_cfg[i].irq) {
> > +			found = true;
> >  			break;
> > +		}
> >  	}
> >  
> > -	status_msg.cluster = hartid;
> > +	if (unlikely(!found))
> > +		return IRQ_NONE;
> 
> This one is a false positive because obviously there is going to be
> at least one online cpu.  I would prefer to silence this in Smatch.
> Generally, you should ignore static checker false positives.

Well...  I guess the bug is real, but the warning was bogus.  It was
a broken clock warning which works by chance twice a day.

regards,
dan carpenter