On Wednesday 07 January 2015 02:28:43 Kuninori Morimoto wrote:
>
> Hi Arnd
>
> Thank you for your DMAEngine fixup patch.
> I tried this patch, and have comment.
Thanks for looking at the changes
> >
> > - 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)
Right, I broke it in a last-minute change. Originally I had
schan->real_slave_id = slave_id;
ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
if (ret < 0) {
schan->real_slave_id = 0;
return ret;
}
and then meant to shorten it to
ret = sdev->ops->set_slave(schan, slave_id, 0, true);
if (ret < 0)
return ret;
schan->real_slave_id = slave_id;
Either of those should be fine, but what I wrote was instead was
completely wrong.
> > 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
Either that or undo the change to the type. I originally planned to change the
sh_mmcif_plat_data to use a void* type already, but then didn't do that because
it conflicts with your other patch, and I failed to revert my earlier change
correctly.
Arnd
--
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
