mmc: Handle errors from optional IRQ lookup
Subsystems (2)
v2 cover mmc: Handle errors from optional IRQ lookup
From: bui duc phuc <[email protected]>
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.
Changes in v2:
- Use parse_fail for proper cleanup on IRQ lookup errors.
- Initialize cd_irq to -ENXIO.
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 | 6 +++++- drivers/mmc/host/meson-gx-mmc.c | 9 +++++++-- 2 files changed, 12 insertions(+), 3 deletions(-)
Signature · 1 line
2.43.0
PATCH v2 1/3 mmc: meson-gx: Handle errors from optional IRQ lookup
From: bui duc phuc <[email protected]>
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.
---
Changes in v2:
- Initialize cd_irq to -ENXIO.
drivers/mmc/host/meson-gx-mmc.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/mmc/host/meson-gx-mmc.c +7 -2
@@ -1141,7 +1141,8 @@ static int meson_mmc_probe(struct platform_device *pdev)struct meson_host *host;struct mmc_host *mmc;struct clk *core_clk; -int cd_irq, ret; +int cd_irq = -ENXIO ; +int ret; mmc =devm_mmc_alloc_host (&pdev->dev,sizeof (struct meson_host ));if (!mmc)
@@ -1185,7 +1186,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);
Signature · 1 line
2.43.0
PATCH v2 2/3 mmc: davinci: Handle errors from optional IRQ lookup
In reply to [email protected]From: bui duc phuc <[email protected]>
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.
---
Changes in v2:
- Use parse_fail for proper cleanup on IRQ lookup errors.
- Keep the value returned by platform_get_irq_optional() in host->sdio_irq, since it is already checked before calling devm_request_irq().
drivers/mmc/host/davinci_mmc.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/mmc/host/davinci_mmc.c +4 -0
@@ -1249,6 +1249,10 @@ 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 ); +if (host->sdio_irq <0 && host->sdio_irq != -ENXIO ) { + ret = host->sdio_irq; + goto parse_fail; + }if (host->use_dma) { ret =davinci_acquire_dma_channels (host);
Signature · 1 line
2.43.0
PATCH v2 3/3 mmc: davinci: Handle optional IRQ return value correctly
In reply to [email protected]From: bui duc phuc <[email protected]>
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.
--- drivers/mmc/host/davinci_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/davinci_mmc.c +1 -1
@@ -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);
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v2 3/3] mmc: davinci: Handle optional IRQ return value correctly
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Hardware clock is disabled before devm-managed IRQ handlers are unregistered, creating a window for fatal unclocked register accesses.
- [High] The probe function overwrites pdev->id_entry with NULL for legacy devices, breaking hardware version detection.
- [High] The device's memory region name points to a dynamically allocated string that is freed prior to the region itself during teardown.
- [High] Failure to check the dma_map_sg return value leads to an invalid dma_unmap_sg call during PIO fallback.
--
--- Patch [3]: [PATCH v2 3/3] mmc: davinci: Handle optional IRQ return value correctly --- commit 27e81af9e24b190ffa17b59c0cbac338793c903c
Author: bui duc phuc <[email protected]>
mmc: davinci: Handle optional IRQ return value correctly
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.
drivers/mmc/host/davinci_mmc.c static int davinci_mmcsd_probe(struct platform_device *pdev) · line 1309
Quoted 13 lines
drivers/mmc/host/davinci_mmc.c +1
-1@@ -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);
[Severity: High]
This is a pre-existing issue, but does the error handling in probe and the remove function create a window where unclocked register accesses can occur?
In the probe error path and the remove function, the hardware clock is disabled before the devres-managed IRQ handlers are unregistered:
drivers/mmc/host/davinci_mmc.c:davinci_mmcsd_probe() {
...
dma_probe_defer:
clk_disable_unprepare(host->clk);
return ret;
}Since the IRQ is managed by devm_request_irq(), it remains active while the clock is explicitly disabled. Could an interrupt fire during this window and cause a synchronous external abort when the handler reads unclocked registers?
[Severity: High]
This isn't a bug introduced by this patch, but does the probe function break hardware version detection for legacy non-OF devices?
The probe function unconditionally overwrites the platform device's id_entry:
drivers/mmc/host/davinci_mmc.c:davinci_mmcsd_probe() {
...
pdev->id_entry = device_get_match_data(&pdev->dev);
...
}If the device is instantiated via legacy platform data rather than device tree, device_get_match_data() will return NULL. This overwrites the previously valid pdev->id_entry, which corrupts subsequent platform_get_device_id() lookups later in the probe sequence.
[Severity: High]
This is a pre-existing issue, but could the memory region rename cause a use-after-free during device teardown?
In the probe function, the memory region's name is set to a dynamically allocated string from the mmc host:
drivers/mmc/host/davinci_mmc.c:davinci_mmcsd_probe() {
...
rename_region(mem, mmc_hostname(mmc));
...
}During teardown in davinci_mmcsd_remove(), devres executes in reverse order. The MMC host (which contains the mmc_hostname string) will be freed before the devres-managed memory region is released. If /proc/iomem is read during this window, would it dereference a dangling pointer?
[Severity: High]
This isn't a bug introduced by this patch, but could a failure in DMA mapping lead to an invalid unmap call later?
In mmc_davinci_start_dma_transfer(), the return value of dma_map_sg() is not checked for failure (which returns 0):
drivers/mmc/host/davinci_mmc.c:mmc_davinci_start_dma_transfer() {
...
host->sg_len = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
mmc_get_dma_dir(data));
...
host->do_dma = 1;
...
}If dma_map_sg() fails and returns 0, host->do_dma is still set to 1. This engages the PIO fallback but retains the DMA state, potentially causing invalid dma_unmap_sg() calls when the transfer completes.
Signature · 6 lines
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3 _______________________________________________ linux-amlogic mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-amlogic
Quoted 5 lines · On Thu, Aug 13, 2026 at 7:34 AM <[email protected]> wrote:
From: bui duc phuc <[email protected]>
Hi all,
Quoted 7 lines
This series improves error handling for optional IRQ lookups in several MMC drivers.
I noticed that mmc:
litex_mmchandles the return value fromplatform_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.
The series looks correct to me, but someone should really look into improving/replacing the platform_get_irq_optional() API. Callers should not have to check for specific error codes as that is just prone to problems.
Quoted 13 lines
Changes in v2:
- Useparse_failfor proper cleanup on IRQ lookup errors.
- Initializecd_irqto-ENXIO.Best regards,
Phucbui 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
Quoted 7 lines
drivers/mmc/host/davinci_mmc.c | 6 +++++- drivers/mmc/host/meson-gx-mmc.c | 9 +++++++-- 2 files changed, 12 insertions(+), 3 deletions(-)--
2.43.0
Applied for next, thanks!
Kind regards
Uffe