[PATCH 0/3] mmc: Handle errors from optional IRQ lookup

[PATCH 0/3] mmc: Handle errors from optional IRQ lookup

[phucduc.bui]

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

Hi all,

This series improves error handling for optional IRQ lookups in several
MMC drivers.

I noticed that mmc: litex_mmc handles the return value from
platform_get_irq_optional() correctly, distinguishing the case where an
optional IRQ is not available from other errors. This series follows the
same approach in the affected drivers.

Best regards,
Phuc

bui duc phuc (3):
  mmc: meson-gx: Handle errors from optional IRQ lookup
  mmc: davinci: Handle errors from optional IRQ lookup
  mmc: davinci: Handle optional IRQ return value correctly

 drivers/mmc/host/davinci_mmc.c  | 8 ++++++--
 drivers/mmc/host/meson-gx-mmc.c | 6 +++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 1/3] mmc: meson-gx: Handle errors from optional IRQ lookup

[phucduc.bui]

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

platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no optional IRQ is available. Other errors, such as -EPROBE_DEFER
and -EINVAL, should be propagated so that the caller can handle them
appropriately.

However, the driver currently stores the return value directly in
cd_irq and continues probing.

Propagate negative errors other than -ENXIO, and only assign the IRQ to
cd_irq when a valid IRQ number is returned.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/mmc/host/meson-gx-mmc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 694bb443d5f3..af94c8138fcf 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -1185,7 +1185,11 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (host->irq < 0)
 		return host->irq;
 
-	cd_irq = platform_get_irq_optional(pdev, 1);
+	ret = platform_get_irq_optional(pdev, 1);
+	if (ret < 0 && ret != -ENXIO)
+		return ret;
+	if (ret > 0)
+		cd_irq = ret;
 	mmc_gpio_set_cd_irq(mmc, cd_irq);
 
 	host->pinctrl = devm_pinctrl_get(&pdev->dev);
-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 2/3] mmc: davinci: Handle errors from optional IRQ lookup

[phucduc.bui]

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

platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no optional IRQ is available. Other errors, such as -EPROBE_DEFER
and -EINVAL, should be propagated so that the caller can handle them
appropriately.

However, the driver currently stores the return value directly in
host->sdio_irq and continues probing.

Propagate negative errors other than -ENXIO, and only assign the IRQ to
host->sdio_irq when a valid IRQ number is returned.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/mmc/host/davinci_mmc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index cdb9fa94b56d..4765d2dee599 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -1248,7 +1248,11 @@ static int davinci_mmcsd_probe(struct platform_device *pdev)
 
 	host->use_dma = use_dma;
 	host->mmc_irq = irq;
-	host->sdio_irq = platform_get_irq_optional(pdev, 1);
+	ret = platform_get_irq_optional(pdev, 1);
+	if (ret < 0 && ret != -ENXIO)
+		return ret;
+	if (ret > 0)
+		host->sdio_irq = ret;
 
 	if (host->use_dma) {
 		ret = davinci_acquire_dma_channels(host);
-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 3/3] mmc: davinci: Handle optional IRQ return value correctly

[phucduc.bui]

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

host->sdio_irq is assigned from platform_get_irq_optional(), which
returns a positive IRQ number on success or a negative error code on
failure. Therefore, 0 is not a possible return value from this API.

Check for a positive IRQ number before requesting the SDIO IRQ instead
of treating zero as a valid IRQ.

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

diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 4765d2dee599..dddd152d2ec4 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -1303,7 +1303,7 @@ static int davinci_mmcsd_probe(struct platform_device *pdev)
 	if (ret)
 		goto mmc_add_host_fail;
 
-	if (host->sdio_irq >= 0) {
+	if (host->sdio_irq > 0) {
 		ret = devm_request_irq(&pdev->dev, host->sdio_irq,
 				       mmc_davinci_sdio_irq, 0,
 				       mmc_hostname(mmc), host);
-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/3] mmc: meson-gx: Handle errors from optional IRQ lookup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized variable usage in `meson_mmc_probe()`: `cd_irq` is passed to `mmc_gpio_set_cd_irq()` without being initialized when the optional IRQ is not present.
--

--- Patch [1]: [PATCH 1/3] mmc: meson-gx: Handle errors from optional IRQ lookup ---
commit 050320309b9f222bb3f134cc08b3a48c3a62f6c8
Author: bui duc phuc <phucduc.bui@gmail.com>

mmc: meson-gx: Handle errors from optional IRQ lookup

This commit updates the optional IRQ lookup in meson_mmc_probe() to
properly propagate errors other than -ENXIO. It also assigns the fetched
IRQ to cd_irq only if a valid positive number is returned.

> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -1185,7 +1185,11 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  	if (host->irq < 0)
>  		return host->irq;
>  
> -	cd_irq = platform_get_irq_optional(pdev, 1);
> +	ret = platform_get_irq_optional(pdev, 1);
> +	if (ret < 0 && ret != -ENXIO)
> +		return ret;
> +	if (ret > 0)
> +		cd_irq = ret;
>  	mmc_gpio_set_cd_irq(mmc, cd_irq);

[Severity: High]
Does this code pass an uninitialized variable to mmc_gpio_set_cd_irq()
when the optional IRQ is not present?

If platform_get_irq_optional() returns -ENXIO, the assignment to cd_irq
is skipped. The uninitialized cd_irq variable is then passed to
mmc_gpio_set_cd_irq().

If the uninitialized stack value happens to be >= 0, mmc_gpio_set_cd_irq()
will accept it and overwrite the card-detect IRQ configuration with an
invalid or random IRQ number.

Could cd_irq be initialized to a negative value when it is declared?

>  
>  	host->pinctrl = devm_pinctrl_get(&pdev->dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812112023.42254-1-phucduc.bui@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 2/3] mmc: davinci: Handle errors from optional IRQ lookup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Direct return on error from `platform_get_irq_optional()` leaks the clock prepare/enable reference.
--

--- Patch [2]: [PATCH 2/3] mmc: davinci: Handle errors from optional IRQ lookup ---
commit 3b3d706a95688ff2d8229702c82d2a0cd95e3f5d
Author: bui duc phuc <phucduc.bui@gmail.com>

mmc: davinci: Handle errors from optional IRQ lookup

This commit updates the error handling for platform_get_irq_optional to
propagate negative error codes, such as -EPROBE_DEFER, rather than storing
them directly as the sdio_irq number. This allows the caller to handle these
errors appropriately instead of blindly continuing the probe.

> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index cdb9fa94b56d0..4765d2dee5993 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -1248,7 +1248,11 @@ static int davinci_mmcsd_probe(struct platform_device *pdev)
>  
>  	host->use_dma = use_dma;
>  	host->mmc_irq = irq;
> -	host->sdio_irq = platform_get_irq_optional(pdev, 1);
> +	ret = platform_get_irq_optional(pdev, 1);
> +	if (ret < 0 && ret != -ENXIO)
> +		return ret;

[Severity: High]
Does this direct return leak the clock reference?

Earlier in davinci_mmcsd_probe(), the clock is prepared and enabled:

	ret = clk_prepare_enable(host->clk);
	if (ret)
		return ret;

If platform_get_irq_optional() returns an error like -EPROBE_DEFER, returning
directly here bypasses the cleanup labels at the end of the function that
call clk_disable_unprepare(host->clk). This can endlessly increase the
clock reference count on probe retries, permanently preventing the clock from
being disabled for power management.

Should this use a goto label (like parse_fail or dma_probe_defer) instead?

> +	if (ret > 0)
> +		host->sdio_irq = ret;
>  
>  	if (host->use_dma) {
>  		ret = davinci_acquire_dma_channels(host);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812112023.42254-1-phucduc.bui@gmail.com?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 3/3] mmc: davinci: Handle optional IRQ return value correctly

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Direct return on `platform_get_irq_optional` error bypasses clock cleanup, leaking the clock enable reference.

Pre-existing issues:
- [High] The driver manually disables the hardware clock in the remove and probe error paths while devm-managed IRQs are still active, risking unclocked register accesses.
- [High] If DMA descriptor allocation fails, the driver attempts PIO fallback but leaves `host->do_dma` set and leaks the mapped scatterlist.
- [High] The system suspend routine disables the MMC controller clock without masking the SDIO interrupt, risking unclocked register access.
--

--- Patch [3]: [PATCH 3/3] mmc: davinci: Handle optional IRQ return value correctly ---
commit 6f8e3291a8bba48fcd7880ece2584b86bc2ca506
Author: bui duc phuc <phucduc.bui@gmail.com>

mmc: davinci: Handle optional IRQ return value correctly
    
The commit checks for a positive IRQ number before requesting the SDIO
IRQ, fixing an issue where 0 was treated as a valid IRQ.

> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
> index 4765d2dee5993..dddd152d2ec45 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -1303,7 +1303,7 @@ static int davinci_mmcsd_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto mmc_add_host_fail;
>  
> -	if (host->sdio_irq >= 0) {
> +	if (host->sdio_irq > 0) {

[Severity: Medium]
In davinci_mmcsd_probe(), if platform_get_irq_optional() returns an error:

    ret = platform_get_irq_optional(pdev, 1);
    if (ret < 0 && ret != -ENXIO)
        return ret;

Does this direct return bypass the error handling sequence (e.g., goto
parse_fail) and leak the clock enable reference?


[Severity: High]
This isn't a bug introduced by this patch, but I noticed the driver
manually disables the hardware clock in the remove and probe error paths
while devm-managed IRQs are still active.

In the davinci_mmcsd_probe() error paths and davinci_mmcsd_remove(),
clk_disable_unprepare(host->clk) is called before devres frees the IRQs.

If a spurious or SDIO interrupt fires in this window,
mmc_davinci_sdio_irq() will execute:

    status = readl(host->base + DAVINCI_SDIOIST);

Could this cause a fatal bus fault by reading from memory-mapped
registers while the clock is disabled?


[Severity: High]
This is a pre-existing issue, but I found a potential problem in the DMA
fallback path.

In mmc_davinci_start_dma_transfer():

    host->do_dma = 1;
    ret = mmc_davinci_send_dma_request(host, data);

If dmaengine_prep_slave_sg() fails inside mmc_davinci_send_dma_request(),
it returns an error but leaves host->do_dma set to 1, and does not unmap
the scatterlist mapped via dma_map_sg().

Does this break the PIO fallback since mmc_davinci_start_command() will
still configure the hardware for DMA?


[Severity: High]
This is a pre-existing issue, but the system suspend routine might have a
clock management problem.

In davinci_mmcsd_suspend():

    writel(0, host->base + DAVINCI_MMCIM);
    mmc_davinci_reset_ctrl(host, 1);
    clk_disable(host->clk);

The MMC controller clock is disabled without masking the SDIO interrupt
via DAVINCI_SDIOIEN.

If an SDIO interrupt fires between the device suspend callback and the IRQ
core's suspend_device_irqs() phase, won't mmc_davinci_sdio_irq() attempt
to read DAVINCI_SDIOIST while the clock domain is off?

>  		ret = devm_request_irq(&pdev->dev, host->sdio_irq,
>  				       mmc_davinci_sdio_irq, 0,
>  				       mmc_hostname(mmc), host);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812112023.42254-1-phucduc.bui@gmail.com?part=3

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic