| Message ID | 20200526225022.20405-6-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine: dw: Take Baikal-T1 SoC DW DMAC peculiarities into account | expand |
On Wed, May 27, 2020 at 01:50:16AM +0300, Serge Semin wrote: > There are DMA devices (like ours version of Synopsys DW DMAC) which have > DMA capabilities non-uniformly redistributed amongst the device channels. > In order to provide a way of exposing the channel-specific parameters to > the DMA engine consumers, we introduce a new DMA-device callback. In case > if provided it gets called from the dma_get_slave_caps() method and is > able to override the generic DMA-device capabilities. > + if (device->device_caps) > + device->device_caps(chan, caps); > + > return 0; I dunno why this returns int, but either we get rid of this returned value (perhaps in the future, b/c it's not directly related to this series), or something like if (device->device_caps) return device->device_caps(chan, caps);
On Thu, May 28, 2020 at 05:42:57PM +0300, Andy Shevchenko wrote: > On Wed, May 27, 2020 at 01:50:16AM +0300, Serge Semin wrote: > > There are DMA devices (like ours version of Synopsys DW DMAC) which have > > DMA capabilities non-uniformly redistributed amongst the device channels. > > In order to provide a way of exposing the channel-specific parameters to > > the DMA engine consumers, we introduce a new DMA-device callback. In case > > if provided it gets called from the dma_get_slave_caps() method and is > > able to override the generic DMA-device capabilities. > > > + if (device->device_caps) > > + device->device_caps(chan, caps); > > + > > return 0; > > I dunno why this returns int, but either we get rid of this returned value > (perhaps in the future, b/c it's not directly related to this series), or > something like > > if (device->device_caps) > return device->device_caps(chan, caps); It returns int because dma_get_slave_caps() check parameters and some other stuff. Regarding device_caps() callback having a return value. IMO it's redundant. The only thing what the callback should do is to update the caps and device is supposed to know it' capabilities, otherwise who else should know? So I don't see why device_caps would be needed. -Sergey > > -- > With Best Regards, > Andy Shevchenko > >
On Thu, May 28, 2020 at 6:23 PM Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > On Thu, May 28, 2020 at 05:42:57PM +0300, Andy Shevchenko wrote: > > On Wed, May 27, 2020 at 01:50:16AM +0300, Serge Semin wrote: ... > > > + if (device->device_caps) > > > + device->device_caps(chan, caps); > > > + > > > return 0; > > > > I dunno why this returns int, but either we get rid of this returned value > > (perhaps in the future, b/c it's not directly related to this series), or > > something like > > > > if (device->device_caps) > > return device->device_caps(chan, caps); > > It returns int because dma_get_slave_caps() check parameters and some other > stuff. > > Regarding device_caps() callback having a return value. IMO it's redundant. > The only thing what the callback should do is to update the caps and device > is supposed to know it' capabilities, otherwise who else should know? So I > don't see why device_caps would be needed. It might be useful in some (weird?) cases, when you would like to override a parameter which device provides to relax it (my common sense tells me that device on global level should not be restrictive, rather permissive), which might be considered as an error (we would like to set return capability out of the boundaries of global ones which provided on device level). But okay, up to you and Vinod.
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index ad56ad58932c..edbb11d56cde 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -599,6 +599,9 @@ int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps) caps->cmd_resume = !!device->device_resume; caps->cmd_terminate = !!device->device_terminate_all; + if (device->device_caps) + device->device_caps(chan, caps); + return 0; } EXPORT_SYMBOL_GPL(dma_get_slave_caps); diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index 6801200c76b6..429eef3a702b 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -899,6 +899,8 @@ struct dma_device { struct dma_chan *chan, dma_addr_t dst, u64 data, unsigned long flags); + void (*device_caps)(struct dma_chan *chan, + struct dma_slave_caps *caps); int (*device_config)(struct dma_chan *chan, struct dma_slave_config *config); int (*device_pause)(struct dma_chan *chan);
There are DMA devices (like ours version of Synopsys DW DMAC) which have DMA capabilities non-uniformly redistributed amongst the device channels. In order to provide a way of exposing the channel-specific parameters to the DMA engine consumers, we introduce a new DMA-device callback. In case if provided it gets called from the dma_get_slave_caps() method and is able to override the generic DMA-device capabilities. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: linux-mips@vger.kernel.org Cc: devicetree@vger.kernel.org --- Changelog v3: - This is a new patch created as a result of the discussion with Vinod and Andy in the framework of DW DMA burst and LLP capabilities. --- drivers/dma/dmaengine.c | 3 +++ include/linux/dmaengine.h | 2 ++ 2 files changed, 5 insertions(+)