Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Dan Carpenter
2025-10-14T12:27:17Z

On Mon, Oct 13, 2025 at 04:54:13PM -0600, Khalid Aziz wrote:
> > @@ -703,20 +705,15 @@ static int pxamci_probe(struct platform_device *pdev)
> > platform_set_drvdata(pdev, mmc);
> > - host->dma_chan_rx = dma_request_chan(dev, "rx");
> > - if (IS_ERR(host->dma_chan_rx)) {
> > - host->dma_chan_rx = NULL;
> > + host->dma_chan_rx = devm_dma_request_chan(dev, "rx");
> > + if (IS_ERR(host->dma_chan_rx))
> > return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
> > "unable to request rx dma channel\n");
> > - }
> > - host->dma_chan_tx = dma_request_chan(dev, "tx");
> > - if (IS_ERR(host->dma_chan_tx)) {
> > - dev_err(dev, "unable to request tx dma channel\n");
> > - ret = PTR_ERR(host->dma_chan_tx);
> > - host->dma_chan_tx = NULL;
> > - goto out;
> > - }
> > + host->dma_chan_tx = devm_dma_request_chan(dev, "tx");
> > + if (IS_ERR(host->dma_chan_tx))
> > + return dev_err_probe(dev, PTR_ERR(host->dma_chan_tx),
> > + "unable to request tx dma channel\n");
>
> We should still release DMA rx channel before returning here.
>
> > if (host->pdata) {
> > host->detect_delay_ms = host->pdata->detect_delay_ms;
> > @@ -724,25 +721,21 @@ static int pxamci_probe(struct platform_device *pdev)
> > host->power = devm_gpiod_get_optional(dev, "power", GPIOD_OUT_LOW);
> > if (IS_ERR(host->power)) {
> > ret = PTR_ERR(host->power);
> > - dev_err(dev, "Failed requesting gpio_power\n");
> > - goto out;
> > + return dev_err_probe(dev, ret, "Failed requesting gpio_power\n");
>
> Don't we need to release DMA Rx and Tx channels before we return from here?
>
> > }
> > /* FIXME: should we pass detection delay to debounce? */
> > ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0);
> > - if (ret && ret != -ENOENT) {
> > - dev_err(dev, "Failed requesting gpio_cd\n");
> > - goto out;
> > - }
> > + if (ret && ret != -ENOENT)
> > + return dev_err_probe(dev, ret, "Failed requesting gpio_cd\n");
>
> Same here
>
> > if (!host->pdata->gpio_card_ro_invert)
> > mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> > ret = mmc_gpiod_request_ro(mmc, "wp", 0, 0);
> > - if (ret && ret != -ENOENT) {
> > - dev_err(dev, "Failed requesting gpio_ro\n");
> > - goto out;
> > - }
> > + if (ret && ret != -ENOENT)
> > + return dev_err_probe(dev, ret, "Failed requesting gpio_ro\n");
>
> and here.
>
> Looking at Documentation/driver-api/driver-model/devres.rst,
> dma_request_chan() is not devres managed interface and thus will not be
> released automatically. Do you agree?
>

The patch changes dma_request_chan() to devm_dma_request_chan().

regards,
dan carpenter