On Wednesday 12 August 2015 17:13:31 Geert Uytterhoeven wrote: > Hi Arnd, > > On Wed, Aug 12, 2015 at 4:50 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Tuesday 11 August 2015 14:48:34 Geert Uytterhoeven wrote: > >> dma_cap_mask_t mask; > >> @@ -413,12 +413,11 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host, > >> dma_cap_set(DMA_SLAVE, mask); > >> > >> if (pdata) > >> - slave_data = direction == DMA_MEM_TO_DEV ? > >> - (void *)pdata->slave_id_tx : > >> - (void *)pdata->slave_id_rx; > >> + slave_id = direction == DMA_MEM_TO_DEV ? > >> + pdata->slave_id_tx : pdata->slave_id_rx; > >> > >> chan = dma_request_slave_channel_compat(mask, shdma_chan_filter, > >> - slave_data, dev, > >> + (void *)(uintptr_t)slave_id, dev, > >> direction == DMA_MEM_TO_DEV ? "tx" : "rx"); > >> > >> dev_dbg(dev, "%s: %s: got channel %p\n", __func__, > > > > How about changing the type of the slave_id_rx/slave_id_tx fields > > to void*? That way, the hack can be moved to arch/sh/boards/board-sh7757lcr.c, > > which is now the only file passing data this way. Ideally, we'd also > > And to the other SH boards... This is not limited to the mmcif driver. > > Ah, I see tmio_mmc_data already does it this way. I probably made a similar comment there, now I just did 'git grep slave_id_.x' > This will break out-of-tree boards at compile-time though. Given the limited > number of wired up DMA channels under arch/sh/, they may actually be > more in use out-of-tree than in-tree? > > Alternatively, perhaps we should just drop pdata DMA configuration from > some drivers? I think that would be helpful, but it would not work for this driver as long as we keep arch/sh/boards/board-sh7757lcr.c in the kernel. > > pass the shdma_chan_filter function pointer in pdata to avoid the link > > time dependency on a particular dmaengine driver. > > That's actually more tricky, as a NULL filter function means something > completely different, and causes funny failures if the DTS lacks "dmas" > properties, cfr. "dmaengine: shdma: Make dummy shdma_chan_filter() always > return false". > https://git.kernel.org/cgit/linux/kernel/git/vkoul/slave-dma.git/commit/?h=next&id=056f6c87028544de934f27caf95aa1545d585767 Ah, this is nasty, I had not thought of that case. This makes the entire dma_request_slave_channel_compat() interface (which I never liked, fwiw) dangerous to use, because a lot of other drivers will get the filter function pointer from pdata already (any driver that works with more than one dmaengine master has to). I wonder if we should enforce this in __dma_request_slave_channel_compat() by checking both the fn and fn_param arguments to ensure they are not NULL before calling the legacy fallback using __dma_request_channel. 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
