Let's move this one here as well, since it's not related to SPI anymore. On Mon, 2014-07-28 at 16:02 +0200, Arnd Bergmann wrote: > On Monday 28 July 2014 16:34:04 Andy Shevchenko wrote: > > On Mon, 2014-07-28 at 14:47 +0200, Arnd Bergmann wrote: > > > On Monday 28 July 2014 14:06:20 Andy Shevchenko wrote: > > > static bool atmci_filter(struct dma_chan *chan, void *pdata) > > > { > > > struct mci_platform_data *sl_pdata = pdata; > > > struct mci_dma_data *sl; > > > > > > if (!sl_pdata) > > > return false; > > > > > > sl = sl_pdata->dma_slave; > > > if (sl && find_slave_dev(sl) == chan->device->dev) { > > > chan->private = slave_data_ptr(sl); > > > return true; > > > } else { > > > return false; > > > } > > > } > > > > > > It sets the dw_dma_slave information from slave_data_ptr(sl) here, > > > and does not attempt to set a slave_id at all, the slave_config > > > call only sets the required fields. > > > > > > Do you still see a problem here? > > > > Sorry, I was talking about master defaults regarding to your patch that > > proposes to remove that part from dw_dmac. I mean it should be set in > > function mentioned early. > > Ok. So atmci_filter is still correct because chan->private contains > a pointer to a structure with both request and master settings, right? Right, but we can't set them unconditionally in set_runtime_config(). Currently I have implemented "generic" filter function for dw_dmac users (under linux/dw_dmac.h of course). It filters channel and assigns necessary data (request line, masters) to the structure. We converted SPI and HSUART to use it. Besides it has dependency to an actual DMA driver (which is dependent on hardware), it looks more or less okay and it works. But for the Atmel drivers we have to reuse it or leave that code you suggested to remove in the dw/core.c. Or we can use private dw_dma_slave structure for the SPI/UART cases with the additional field of request line. In such case filter function could be really simple and we may unconditionally set data in set_runtime_config(). Also we have to require users to provide that structure. Latter looks better for me. > > > d) request line and masters are all configured from Bay Trail > > > PCI ID (Implemented by Mika's patch) > > > > Actually here I just recall the case when we might have different number > > of masters and use the same IP block [of SPI controller], how could you > > distinguish that case? How to provide proper masters? > > > > It's luckily not a problem right now, but still potential one in some > > cases. > > Do you mean case d? Not exactly, just generic PCI case, where DMA controller can be separate IP from the IP that uses it (not even different function of PCI mfd). I recalled Medfield device, where PCI is actually fake one and devices are independent to each other. As I said, luckily there is no problem *now* by several reasons. > I think for each PCI ID, you will have to add the > correct settings to some table that is used in the filter function. > > Are there any cases you know where we can't hardcode them? I can't see how it helps. Case 1: - PCI ID for DMA is AAA (one AHB master) - PCI ID for DMA user is BBB Case 2: - PCI ID for DMA is CCC (two AHB masters) - PCI ID for DMA user is BBB DMA driver knows this info, but actual user doesn't. If it provides wrong master it wouldn't work I suppose. -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html