get rid of slave_id in dw_dmac (Re: [PATCH] spi/pxa2xx-pci: Enable DMA binding through device name)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux