[PATCH 1/4] ipmi: bt-bmc: Propagate errors from IRQ configuration

[PATCH 1/4] ipmi: bt-bmc: Propagate errors from IRQ configuration

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Check and propagate the return value of bt_bmc_config_irq() instead of
ignoring errors during probe.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/char/ipmi/bt-bmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index a179d4797011..7d3944bda8db 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -436,7 +436,9 @@ static int bt_bmc_probe(struct platform_device *pdev)
 		return rc;
 	}
 
-	bt_bmc_config_irq(bt_bmc, pdev);
+	rc = bt_bmc_config_irq(bt_bmc, pdev);
+	if (rc)
+		return rc;
 
 	if (bt_bmc->irq >= 0) {
 		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
-- 
2.43.0

[PATCH 2/4] ipmi: bt-bmc: Handle -ENXIO from optional IRQ lookup

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

platform_get_irq_optional() can return -ENXIO when no IRQ resource is
available, as well as other negative error codes.

The probe path supports running without an IRQ by falling back to
the timer. Treat -ENXIO as the no-IRQ case while propagating other
errors to the caller.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/char/ipmi/bt-bmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index 7d3944bda8db..4c5457c88503 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -380,7 +380,7 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
 	u32 reg;
 
 	bt_bmc->irq = platform_get_irq_optional(pdev, 0);
-	if (bt_bmc->irq < 0)
+	if (bt_bmc->irq < 0 && bt_bmc->irq != -ENXIO)
 		return bt_bmc->irq;
 
 	rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
-- 
2.43.0

Re: [PATCH 2/4] ipmi: bt-bmc: Handle -ENXIO from optional IRQ lookup

[Corey Minyard]

On Mon, Aug 17, 2026 at 05:50:39PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> platform_get_irq_optional() can return -ENXIO when no IRQ resource is
> available, as well as other negative error codes.
> 
> The probe path supports running without an IRQ by falling back to
> the timer. Treat -ENXIO as the no-IRQ case while propagating other
> errors to the caller.

This is obviously wrong, it will pass -ENXIO into devm_request_irq()
if returned.

-corey

> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>  drivers/char/ipmi/bt-bmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
> index 7d3944bda8db..4c5457c88503 100644
> --- a/drivers/char/ipmi/bt-bmc.c
> +++ b/drivers/char/ipmi/bt-bmc.c
> @@ -380,7 +380,7 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
>  	u32 reg;
>  
>  	bt_bmc->irq = platform_get_irq_optional(pdev, 0);
> -	if (bt_bmc->irq < 0)
> +	if (bt_bmc->irq < 0 && bt_bmc->irq != -ENXIO)
>  		return bt_bmc->irq;
>  
>  	rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
> -- 
> 2.43.0
> 

Re: [PATCH 2/4] ipmi: bt-bmc: Handle -ENXIO from optional IRQ lookup

[Bui Duc Phuc]

On Mon, Aug 17, 2026 at 6:40 PM Corey Minyard <corey@minyard.net> wrote:
>
> On Mon, Aug 17, 2026 at 05:50:39PM +0700, phucduc.bui@gmail.com wrote:
> > From: bui duc phuc <phucduc.bui@gmail.com>
> >
> > platform_get_irq_optional() can return -ENXIO when no IRQ resource is
> > available, as well as other negative error codes.
> >
> > The probe path supports running without an IRQ by falling back to
> > the timer. Treat -ENXIO as the no-IRQ case while propagating other
> > errors to the caller.
>
> This is obviously wrong, it will pass -ENXIO into devm_request_irq()
> if returned.
>


No. This is addressed in the following patch:
[PATCH 3/4] ipmi: bt-bmc: Request IRQ only when available

https://lore.kernel.org/all/20260817105041.63224-3-phucduc.bui@gmail.com/

Please take a look at the subsequent patches in the series as well.

Best regards,
Phuc

Re: [PATCH 2/4] ipmi: bt-bmc: Handle -ENXIO from optional IRQ lookup

[Corey Minyard]

On Mon, Aug 17, 2026 at 09:28:50PM +0700, Bui Duc Phuc wrote:
> On Mon, Aug 17, 2026 at 6:40 PM Corey Minyard <corey@minyard.net> wrote:
> >
> > On Mon, Aug 17, 2026 at 05:50:39PM +0700, phucduc.bui@gmail.com wrote:
> > > From: bui duc phuc <phucduc.bui@gmail.com>
> > >
> > > platform_get_irq_optional() can return -ENXIO when no IRQ resource is
> > > available, as well as other negative error codes.
> > >
> > > The probe path supports running without an IRQ by falling back to
> > > the timer. Treat -ENXIO as the no-IRQ case while propagating other
> > > errors to the caller.
> >
> > This is obviously wrong, it will pass -ENXIO into devm_request_irq()
> > if returned.
> >
> 
> 
> No. This is addressed in the following patch:
> [PATCH 3/4] ipmi: bt-bmc: Request IRQ only when available
> 
> https://lore.kernel.org/all/20260817105041.63224-3-phucduc.bui@gmail.com/
> 
> Please take a look at the subsequent patches in the series as well.

You cannot add patches that introduce bugs then fix them later.
Occassionally it might be necessary, but it should be avoided if
possible, and it's certainly possible here.

And as I said earlier, this function should return an error/interrupt,
not set the value inside the function.  The function has issues,
and you are right to work on it, but it needs to be consistent with
everything else in the kernel.

This would also be better as a single patch.  There's no reason to split
it up to this fine a level of detail, it makes it hard to follow.

And it must work if no interrupt is available for any reason.

-corey

> 
> Best regards,
> Phuc

Re: [PATCH 2/4] ipmi: bt-bmc: Handle -ENXIO from optional IRQ lookup

[Bui Duc Phuc]

Hi Corey,

Thanks for your feedback.


> > No. This is addressed in the following patch:
> > [PATCH 3/4] ipmi: bt-bmc: Request IRQ only when available
> >
> > https://lore.kernel.org/all/20260817105041.63224-3-phucduc.bui@gmail.com/
> >
> > Please take a look at the subsequent patches in the series as well.
>
> You cannot add patches that introduce bugs then fix them later.
> Occassionally it might be necessary, but it should be avoided if
> possible, and it's certainly possible here.
>

This patch does not introduce a new bug. It fixes the incorrect handling of
platform_get_irq_optional() .
The only issue is that, as you pointed out, the patches are not
ordered appropriately.
Perhaps it would be better to make this patch look like this:

-       if (bt_bmc->irq < 0)
+       if (bt_bmc->irq < 0 && bt_bmc->irq != -ENXIO)
                return bt_bmc->irq;

-       rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
-                             DEVICE_NAME, bt_bmc);
-       if (rc < 0) {
-               dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
-               bt_bmc->irq = rc;
-               return rc;
+       if (bt_bmc->irq > 0) {
+               rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq,
+                                     IRQF_SHARED, DEVICE_NAME, bt_bmc);
+               if (rc < 0) {
+                       dev_warn(dev, "Unable to request IRQ %d\n",
bt_bmc->irq);
+                       bt_bmc->irq = rc;
+                       return rc;
        }

Then the next patch would remove the error log:
       if (rc < 0) {
-               dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
-               bt_bmc->irq = rc;
               return rc;



> And as I said earlier, this function should return an error/interrupt,
> not set the value inside the function.

I agree. This is reasonable and can be refactored to make the code clearer.

>The function has issues,
> and you are right to work on it, but it needs to be consistent with
> everything else in the kernel.
>

Yes, I agree.
I found that quite a few places in the kernel handle the error values
from platform_get_irq_optional()
correctly. However, there are still some places where they are not
handled correctly.

Regarding the incorrect error handling of platform_get_irq_optional(),
I have submitted several patches to address such cases. Some places have
agreed with my approach of propagating all error values except -ENXIO,
while some other places have so far only agreed with propagating -EPROBE_DEFER.

I think that when we do not fully understand the semantics of an API,
we may end up with incorrect error handling. But once the issue has
been identified,
I think we should fix it rather than continue to maintain the
incorrect behavior.

If, with other APIs, callers also unintentionally or intentionally
hide meaningful errors
instead of propagating them upwards, just imagine what the kernel would become.

> This would also be better as a single patch.  There's no reason to split
> it up to this fine a level of detail, it makes it hard to follow.
>

If the issue is fixed, having it as a single patch would not be a problem.

> And it must work if no interrupt is available for any reason.
>

As I understand it, you mean keeping the current behavior of ignoring
all errors and
continuing to run, in order to ensure that existing systems can still work?
Since the current code already ignores all errors, is that correct?

Best regards,
Phuc

[PATCH 3/4] ipmi: bt-bmc: Request IRQ only when available

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Only call devm_request_irq() when a valid positive IRQ number is
available.

Remove the redundant warning since devm_request_irq() already reports
the error. There is also no need to store the error in bt_bmc->irq,
as errors other than -ENXIO are returned to the caller instead of
being handled as the no-IRQ case.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/char/ipmi/bt-bmc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index 4c5457c88503..99b38300f9e1 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -383,12 +383,11 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc,
 	if (bt_bmc->irq < 0 && bt_bmc->irq != -ENXIO)
 		return bt_bmc->irq;
 
-	rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq, IRQF_SHARED,
-			      DEVICE_NAME, bt_bmc);
-	if (rc < 0) {
-		dev_warn(dev, "Unable to request IRQ %d\n", bt_bmc->irq);
-		bt_bmc->irq = rc;
-		return rc;
+	if (bt_bmc->irq > 0) {
+		rc = devm_request_irq(dev, bt_bmc->irq, bt_bmc_irq,
+				      IRQF_SHARED, DEVICE_NAME, bt_bmc);
+		if (rc < 0)
+			return rc;
 	}
 
 	/*
-- 
2.43.0

[PATCH 4/4] ipmi: bt-bmc: Check IRQ number correctly

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

bt_bmc->irq does not have a value of zero, so check for a positive
IRQ number when selecting between IRQ and timer-based handling.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/char/ipmi/bt-bmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index 99b38300f9e1..68bf34715387 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -439,7 +439,7 @@ static int bt_bmc_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
-	if (bt_bmc->irq >= 0) {
+	if (bt_bmc->irq > 0) {
 		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
 	} else {
 		dev_info(dev, "No IRQ; using timer\n");
-- 
2.43.0

Re: [PATCH 1/4] ipmi: bt-bmc: Propagate errors from IRQ configuration

[Corey Minyard]

On Mon, Aug 17, 2026 at 05:50:38PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Check and propagate the return value of bt_bmc_config_irq() instead of
> ignoring errors during probe.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>  drivers/char/ipmi/bt-bmc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
> index a179d4797011..7d3944bda8db 100644
> --- a/drivers/char/ipmi/bt-bmc.c
> +++ b/drivers/char/ipmi/bt-bmc.c
> @@ -436,7 +436,9 @@ static int bt_bmc_probe(struct platform_device *pdev)
>  		return rc;
>  	}
>  
> -	bt_bmc_config_irq(bt_bmc, pdev);
> +	rc = bt_bmc_config_irq(bt_bmc, pdev);
> +	if (rc)
> +		return rc;

No, this will break the driver if the interrupt is not available.  That
function is badly written (it should return the irq and the irq should
be set here) but if it encounters an error, it should use the timer.

-corey

>  
>  	if (bt_bmc->irq >= 0) {
>  		dev_info(dev, "Using IRQ %d\n", bt_bmc->irq);
> -- 
> 2.43.0
> 

Re: [PATCH 1/4] ipmi: bt-bmc: Propagate errors from IRQ configuration

[Bui Duc Phuc]

Hi Corey,

Thank you for your review .

>
> No, this will break the driver if the interrupt is not available.  That
> function is badly written (it should return the irq and the irq should
> be set here) but if it encounters an error, it should use the timer.
>

Yes, that's why I created the follow-up patches.

Best regards,
Phuc