[PATCH] dmaengine: dma_request_chan_by_mask() defer probing unconditionally

[Enric Balletbo i Serra]

Hi,

On Tue, Dec 3, 2024 at 5:35 AM Vignesh Raghavendra <vigneshr at ti.com> wrote:
>
> Hi Enric,
>
> On 29/11/24 22:14, Robin Murphy wrote:
> > On 2024-11-27 8:23 am, Enric Balletbo i Serra wrote:
> >> Having no DMA devices registered is not a guarantee that the device
> >> doesn't exist, it could be that is not registered yet, so return
> >> EPROBE_DEFER unconditionally so the caller can wait for the required
> >> DMA device registered.
> >>
> >> Signed-off-by: Enric Balletbo i Serra <eballetb at redhat.com>
> >> ---
> >> This patch fixes the following error on TI AM69-SK
> >>
> >> [    2.854501] cadence-qspi 47040000.spi: error -ENODEV: No Rx DMA
> >> available
> >>
> >> The DMA device is probed after cadence-qspi driver, so deferring it
> >> solves the problem.
> >
> > Conversely, though, it does carry some risk that if there really is no
> > DMA device/driver, other callers (e.g. spi-ti-qspi) may now get stuck
> > deferring forever where the -ENODEV would have let them proceed with a
> > fallback to non-DMA operation. driver_deferred_probe_check_state() is
> > typically a good tool for these situations, but I guess it's a bit
> > tricky in a context where we don't actually have the dependent device to
> > hand :/
>
>
> +1. There is no explicit dependency that can be modeled (via DT or
> otherwise) for memcpy DMA channels. And the IP (cadence-qspi) is not
> specific to TI platforms. Its very much possible that a non TI platform
> may not have memcpy DMA channel at all. Wont this end up breaking such
> platforms wrt using SPI flash using CPU bound IO?
>

To be honest, I didn't take into account this, so yes, this patch will
break such platforms. But isn't it already broken in such cases? If I
understand correctly, if there are no dma devices at all, we're
already deferring the probe [1].

        if (list_empty(&dma_device_list))
             chan = ERR_PTR(-EPROBE_DEFER);

[1] https://elixir.bootlin.com/linux/v6.13-rc1/source/drivers/dma/dmaengine.c#L892

Thanks,
   Enric

> >
> > Thanks,
> > Robin.
> >
> >> ---
> >>   drivers/dma/dmaengine.c | 8 ++++----
> >>   1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> >> index
> >> c1357d7f3dc6ca7899c4d68a039567e73b0f089d..57f07b477a5d9ad8f2656584b8c0d6dffb2ab469 100644
> >> --- a/drivers/dma/dmaengine.c
> >> +++ b/drivers/dma/dmaengine.c
> >> @@ -889,10 +889,10 @@ struct dma_chan *dma_request_chan_by_mask(const
> >> dma_cap_mask_t *mask)
> >>       chan = __dma_request_channel(mask, NULL, NULL, NULL);
> >>       if (!chan) {
> >>           mutex_lock(&dma_list_mutex);
> >> -        if (list_empty(&dma_device_list))
> >> -            chan = ERR_PTR(-EPROBE_DEFER);
> >> -        else
> >> -            chan = ERR_PTR(-ENODEV);
> >> +        /* If the required DMA device is not registered yet,
> >> +         * return EPROBE_DEFER
> >> +         */
> >> +        chan = ERR_PTR(-EPROBE_DEFER);
> >>           mutex_unlock(&dma_list_mutex);
> >>       }
> >>
> >> ---
> >> base-commit: 43fb83c17ba2d63dfb798f0be7453ed55ca3f9c2
> >> change-id: 20241127-defer-dma-request-chan-4f26c62c8691
> >>
> >> Best regards,
> >
>
> --
> Regards
> Vignesh
> https://ti.com/opensource
>