Re: [PATCH v2 10/14] mmc: jz4740: Use dma_request_chan()

[Ezequiel Garcia <ezequiel@collabora.co.uk>]

On Thu, 2018-03-15 at 17:59 -0300, Paul Cercueil wrote:
> Hi,
> 
> Le lun. 12 mars 2018 à 18:55, Ezequiel Garcia 
> <ezequiel@vanguardiasur.com.ar> a écrit :
> > From: Ezequiel Garcia <ezequiel@collabora.co.uk>
> > 
> > Replace dma_request_channel() with dma_request_chan(),
> > which also supports probing from the devicetree.
> > 
> > Tested-by: Mathieu Malaterre <malat@debian.org>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.co.uk>
> > ---
> >  drivers/mmc/host/jz4740_mmc.c | 22 +++++++---------------
> >  1 file changed, 7 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/jz4740_mmc.c 
> > b/drivers/mmc/host/jz4740_mmc.c
> > index c3ec8e662706..37183fe32ef8 100644
> > --- a/drivers/mmc/host/jz4740_mmc.c
> > +++ b/drivers/mmc/host/jz4740_mmc.c
> > @@ -225,31 +225,23 @@ static void 
> > jz4740_mmc_release_dma_channels(struct jz4740_mmc_host *host)
> > 
> >  static int jz4740_mmc_acquire_dma_channels(struct jz4740_mmc_host 
> > *host)
> >  {
> > -	dma_cap_mask_t mask;
> > -
> > -	dma_cap_zero(mask);
> > -	dma_cap_set(DMA_SLAVE, mask);
> > -
> > -	host->dma_tx = dma_request_channel(mask, NULL, host);
> > -	if (!host->dma_tx) {
> > +	host->dma_tx = dma_request_chan(mmc_dev(host->mmc), "tx");
> > +	if (IS_ERR(host->dma_tx)) {
> >  		dev_err(mmc_dev(host->mmc), "Failed to get dma_tx
> > channel\n");
> > -		return -ENODEV;
> > +		return PTR_ERR(host->dma_tx);
> >  	}
> > 
> > -	host->dma_rx = dma_request_channel(mask, NULL, host);
> > -	if (!host->dma_rx) {
> > +	host->dma_rx = dma_request_chan(mmc_dev(host->mmc), "rx");
> 
> I suspect this breaks on jz4740... Did you test?
> 

No, but code inspecting I was expecting it wouldn't break anything.
dma_request_channel() searches for a slave channel, via the DMA_SLAVE
mask that the driver sets.

dma_request_chan() seems to fallback to do the same.

struct dma_chan *dma_request_chan(struct device *dev, const char *name)
{
        struct dma_device *d, *_d;
        struct dma_chan *chan = NULL;

        /* If device-tree is present get slave info from here */
        if (dev->of_node)
                chan = of_dma_request_slave_channel(dev->of_node,
name);

        /* ... */

        if (chan) {
                /* Valid channel found or requester need to be deferred
*/
                if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
                        return chan;
        }

        /* Try to find the channel via the DMA filter map(s) */
        mutex_lock(&dma_list_mutex);
        list_for_each_entry_safe(d, _d, &dma_device_list, global_node)
{
                dma_cap_mask_t mask;
                const struct dma_slave_map *map = dma_filter_match(d,
name, dev);

                if (!map)
                        continue;

                dma_cap_zero(mask);
                dma_cap_set(DMA_SLAVE, mask);

                chan = find_candidate(d, &mask, d->filter.fn, map-
>param);
                if (!IS_ERR(chan))
                        break;
        }
        mutex_unlock(&dma_list_mutex);

        return chan ? chan : ERR_PTR(-EPROBE_DEFER);
}

Unfortunately, I don't have anything but a jz4780 Ci20, so can't really
test this.

Thanks,
Eze