Hi Arnd
Thank you for your DMAEngine fixup patch.
I tried this patch, and have comment.
> dmaengine: shdma: use normal interface for passing slave id
>
> The shmobile platform is one of only two users of the slave_id field
> in dma_slave_config, which is incompatible with the way that the
> dmaengine API normally works.
>
> I've had a closer look at the existing code now and found that all
> slave drivers that pass a slave_id in dma_slave_config for SH do that
> right after passing the same ID into shdma_chan_filter, so we can just
> rely on that. However, the various shdma drivers currently do not
> remember the slave ID that was passed into the filter function when
> used in non-DT mode and only check the value to find a matching channel,
> unlike all other drivers.
>
> There might still be drivers that are not part of the kernel that rely
> on setting the slave_id to some other value, so to be on the safe side,
> this adds another 'real_slave_id' field to shdma_chan that remembers
> the ID and uses it when a driver passes a zero slave_id in dma_slave_config,
> like most drivers do.
>
> Eventually, the real_slave_id and slave_id fields should just get merged
> into one field, but that requires other changes.
>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ----
> drivers/dma/sh/shdma-base.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------------------
> drivers/mmc/host/sh_mmcif.c | 4 +---
> drivers/mmc/host/tmio_mmc_dma.c | 4 ----
> drivers/mtd/nand/sh_flctl.c | 2 --
> drivers/spi/spi-rspi.c | 1 -
> drivers/spi/spi-sh-msiof.c | 1 -
> include/linux/shdma-base.h | 1 +
> 7 files changed, 52 insertions(+), 31 deletions(-)
(snip)
> @@ -284,16 +286,35 @@ bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> shdma_alloc_chan_resources)
> return false;
>
> - if (match < 0)
> + schan = to_shdma_chan(chan);
> + sdev = to_shdma_dev(chan->device);
> +
> + /*
> + * For DT, the schan->slave_id field is generated by the
> + * set_slave function from the slave ID that is passed in
> + * from xlate. For the non-DT case, the slave ID is
> + * directly passed into the filter function by the driver
> + */
> + if (schan->dev->of_node) {
> + ret = sdev->ops->set_slave(schan, slave_id, 0, true);
> + if (ret < 0)
> + return false;
> +
> + schan->real_slave_id = schan->slave_id;
> + return true;
> + }
> +
> + if (slave_id < 0) {
> /* No slave requested - arbitrary channel */
> + dev_warn(sdev->dma_dev.dev, "invalid slave ID passed to dma_request_slave\n");
> return true;
> + }
>
> - schan = to_shdma_chan(chan);
> - if (!schan->dev->of_node && match >= slave_num)
> + if (slave_id >= slave_num)
> return false;
>
> - sdev = to_shdma_dev(schan->dma_chan.device);
> - ret = sdev->ops->set_slave(schan, match, 0, true);
> + ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
> + schan->real_slave_id = slave_id;
> if (ret < 0)
> return false;
Here, your patch is
ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
schan->real_slave_id = slave_id;
if (ret < 0)
But, it doesn't work. Maybe, you want this
schan->real_slave_id = slave_id;
ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
if (ret < 0)
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 7d9d6a321521..df3a537f5a83 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -388,7 +388,7 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
> {
> struct dma_slave_config cfg = { 0, };
> struct dma_chan *chan;
> - unsigned int slave_id;
> + void *slave_data;
> struct resource *res;
> dma_cap_mask_t mask;
> int ret;
> @@ -414,8 +414,6 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
>
> res = platform_get_resource(host->pd, IORESOURCE_MEM, 0);
>
> - /* In the OF case the driver will get the slave ID from the DT */
> - cfg.slave_id = slave_id;
> cfg.direction = direction;
>
> if (direction == DMA_DEV_TO_MEM) {
I got error here
/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c: In function ‘sh_mmcif_request_dma_one’:
/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:400:3: error: ‘slave_id’ undeclared (first use in this function)
slave_id = direction == DMA_MEM_TO_DEV
^
/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:400:3: note: each undeclared identifier is reported only once for each function it appears in
/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:391:8: warning: unused variable ‘slave_data’ [-Wunused-variable]
void *slave_data;
^
Maybe you are missing this
if (pdata)
- slave_id = direction == DMA_MEM_TO_DEV
- ? pdata->slave_id_tx : pdata->slave_id_rx;
+ slave_data = direction == DMA_MEM_TO_DEV
+ ? (void *)pdata->slave_id_tx : (void *)pdata->slave_id_rx;
else
- slave_id = 0;
+ slave_data = 0;
chan = dma_request_slave_channel_compat(mask, shdma_chan_filter,
- (void *)(unsigned long)slave_id, &host->pd->dev,
+ slave_data, &host->pd->dev,
direction == DMA_MEM_TO_DEV ? "tx" : "rx");
dev_dbg(&host->pd->dev, "%s: %s: got channel %p\n", __func__,
sh_mobile_sdhi DMA works if I fixuped above
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
