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

> [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);
>


You are right. I overlooked the full context of the function.
I’ll add the goto parse_fail cleanup path and adjust the handling accordingly.
There is already a check for host->sdio_irq before devm_request_irq()
at the end of the function:

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

So I’ll handle the optional IRQ lookup as follows:

--------------------------------
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;
}
--------------------------------

This preserves the existing handling of -ENXIO while making sure
other errors are propagated through the proper cleanup path.

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic