Re: [RFC PATCH 1/2] RapidIO: Add DMA Engine support for RIO data transfers

2011-10-01 Thread Vinod Koul
On Fri, 2011-09-30 at 17:38 -0400, Alexandre Bounine wrote:
Please CC *maintainers* on your patches, get_maintainers.pl will tell
you who. Adding Dan here
> Adds DMA Engine framework support into RapidIO subsystem.
> Uses DMA Engine DMA_SLAVE interface to generate data transfers to/from remote
> RapidIO target devices. Uses scatterlist to describe local data buffer and
> dma_chan.private member to pass target specific information. Supports flat
> data buffer only for a remote side.
The way dmaengine works today is that it doesn't know anything about
client subsystem. But this brings in a subsystem details to dmaengine
which I don't agree with yet.
Why can't we abstract this out??

After going thru the patch, I do not believe that this this is case of
SLAVE transfers, Dan can you please take a look at this patch


> Signed-off-by: Alexandre Bounine 
> Cc: Vinod Koul 
> Cc: Kumar Gala 
> Cc: Matt Porter 
> Cc: Li Yang 
> ---
>  drivers/dma/dmaengine.c   |4 ++
>  drivers/rapidio/Kconfig   |6 +++
>  drivers/rapidio/rio.c |   79 
> +
>  include/linux/dmaengine.h |1 +
>  include/linux/rio.h   |   47 ++
>  include/linux/rio_drv.h   |9 +
>  6 files changed, 146 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index b48967b..11fdc2c 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -699,6 +699,10 @@ int dma_async_device_register(struct dma_device *device)
>   !device->device_prep_dma_cyclic);
>   BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>   !device->device_control);
> + BUG_ON(dma_has_cap(DMA_RAPIDIO, device->cap_mask) &&
> + !device->device_prep_slave_sg);
> + BUG_ON(dma_has_cap(DMA_RAPIDIO, device->cap_mask) &&
> + !device->device_control);
>  
>   BUG_ON(!device->device_alloc_chan_resources);
>   BUG_ON(!device->device_free_chan_resources);
> diff --git a/drivers/rapidio/Kconfig b/drivers/rapidio/Kconfig
> index bc87192..c4aa279 100644
> --- a/drivers/rapidio/Kconfig
> +++ b/drivers/rapidio/Kconfig
> @@ -34,3 +34,9 @@ config RAPIDIO_DEBUG
> If you are unsure about this, say N here.
>  
>  source "drivers/rapidio/switches/Kconfig"
> +
> +# This option to be turned on by a device selection
> +config RAPIDIO_DMA_ENGINE
> + bool
> + select DMADEVICES
> + select DMA_ENGINE
> diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
> index 86c9a09..e5905fc 100644
> --- a/drivers/rapidio/rio.c
> +++ b/drivers/rapidio/rio.c
> @@ -1121,6 +1121,85 @@ int rio_std_route_clr_table(struct rio_mport *mport, 
> u16 destid, u8 hopcount,
>   return 0;
>  }
>  
> +#ifdef CONFIG_RAPIDIO_DMA_ENGINE
> +
> +#include 
> +
> +static bool rio_chan_filter(struct dma_chan *chan, void *arg)
> +{
> + struct rio_dev *rdev = arg;
> +
> + /* Check that DMA device belongs to the right MPORT */
> + return (rdev->net->hport ==
> + container_of(chan->device, struct rio_mport, dma));
> +}
> +
> +/**
> + * rio_request_dma - request RapidIO capable DMA channel that supports
> + *   specified target RapidIO device.
> + * @rdev: RIO device control structure
> + *
> + * Returns pointer to allocated DMA channel or NULL if failed.
> + */
> +struct dma_chan *rio_request_dma(struct rio_dev *rdev)
> +{
> + dma_cap_mask_t mask;
> + struct dma_chan *dchan;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_RAPIDIO, mask);
> + dchan = dma_request_channel(mask, rio_chan_filter, rdev);
> +
> + return dchan;
> +}
> +EXPORT_SYMBOL_GPL(rio_request_dma);
> +
> +/**
> + * rio_release_dma - release specified DMA channel
> + * @dchan: DMA channel to release
> + */
> +void rio_release_dma(struct dma_chan *dchan)
> +{
> + dma_release_channel(dchan);
> +}
> +EXPORT_SYMBOL_GPL(rio_release_dma);
> +
> +/**
> + * rio_dma_prep_slave_sg - RapidIO specific wrapper
> + *   for device_prep_slave_sg callback defined by DMAENGINE.
> + * @rdev: RIO device control structure
> + * @dchan: DMA channel to configure
> + * @data: RIO specific data descriptor
> + * @direction: DMA data transfer direction (TO or FROM the device)
> + * @flags: dmaengine defined flags
> + *
> + * Initializes RapidIO capable DMA channel for the specified data transfer.
> + * Uses DMA channel private extension to pass information related to remote
> + * target RIO device.
> + * Returns pointer to DMA transaction descriptor or NULL if failed.
> + */
> +struct dma_asyn

Re: [RFC PATCH 2/2 -mm] RapidIO: TSI721 Add DMA Engine support

2011-10-01 Thread Vinod Koul
On Fri, 2011-09-30 at 17:38 -0400, Alexandre Bounine wrote:
> Adds support for DMA Engine API.
> 
> Includes following changes:
> - Modifies BDMA register offset definitions to support per-channel handling
> - Separates BDMA channel reserved for RIO Maintenance requests
> - Adds DMA Engine callback routines
Dan please review this, I donot agree with approach here
> 
> Signed-off-by: Alexandre Bounine 
> Cc: Vinod Koul 
> Cc: Kumar Gala 
> Cc: Matt Porter 
> Cc: Li Yang 
> ---
>  drivers/rapidio/devices/Kconfig  |8 +
>  drivers/rapidio/devices/Makefile |1 +
>  drivers/rapidio/devices/tsi721.c |  201 ++
>  drivers/rapidio/devices/tsi721.h |  107 -
>  drivers/rapidio/devices/tsi721_dma.c |  802 
> ++
>  5 files changed, 1029 insertions(+), 90 deletions(-)
>  create mode 100644 drivers/rapidio/devices/tsi721_dma.c
> 
> diff --git a/drivers/rapidio/devices/Kconfig b/drivers/rapidio/devices/Kconfig
> index 12a9d7f..3a2db3d 100644
> --- a/drivers/rapidio/devices/Kconfig
> +++ b/drivers/rapidio/devices/Kconfig
> @@ -8,3 +8,11 @@ config RAPIDIO_TSI721
>   default "n"
>   ---help---
> Include support for IDT Tsi721 PCI Express Serial RapidIO controller.
> +
> +config TSI721_DMA
> + bool "IDT Tsi721 RapidIO DMA support"
> + depends on RAPIDIO_TSI721
> + default "n"
> + select RAPIDIO_DMA_ENGINE
> + help
> +   Enable DMA support for IDT Tsi721 PCIe-to-SRIO controller.
> diff --git a/drivers/rapidio/devices/Makefile 
> b/drivers/rapidio/devices/Makefile
> index 3b7b4e2..8cbce45 100644
> --- a/drivers/rapidio/devices/Makefile
> +++ b/drivers/rapidio/devices/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_RAPIDIO_TSI721) += tsi721.o
> +obj-$(CONFIG_TSI721_DMA) += tsi721_dma.o
> diff --git a/drivers/rapidio/devices/tsi721.c 
> b/drivers/rapidio/devices/tsi721.c
> index 5225930..5e893a6 100644
> --- a/drivers/rapidio/devices/tsi721.c
> +++ b/drivers/rapidio/devices/tsi721.c
> @@ -108,6 +108,7 @@ static int tsi721_maint_dma(struct tsi721_device *priv, 
> u32 sys_size,
>   u16 destid, u8 hopcount, u32 offset, int len,
>   u32 *data, int do_wr)
>  {
> + void __iomem *regs = priv->regs + TSI721_DMAC_BASE(priv->mdma.ch_id);
>   struct tsi721_dma_desc *bd_ptr;
>   u32 rd_count, swr_ptr, ch_stat;
>   int i, err = 0;
> @@ -116,10 +117,9 @@ static int tsi721_maint_dma(struct tsi721_device *priv, 
> u32 sys_size,
>   if (offset > (RIO_MAINT_SPACE_SZ - len) || (len != sizeof(u32)))
>   return -EINVAL;
>  
> - bd_ptr = priv->bdma[TSI721_DMACH_MAINT].bd_base;
> + bd_ptr = priv->mdma.bd_base;
>  
> - rd_count = ioread32(
> - priv->regs + TSI721_DMAC_DRDCNT(TSI721_DMACH_MAINT));
> + rd_count = ioread32(regs + TSI721_DMAC_DRDCNT);
>  
>   /* Initialize DMA descriptor */
>   bd_ptr[0].type_id = cpu_to_le32((DTYPE2 << 29) | (op << 19) | destid);
> @@ -134,19 +134,18 @@ static int tsi721_maint_dma(struct tsi721_device *priv, 
> u32 sys_size,
>   mb();
>  
>   /* Start DMA operation */
> - iowrite32(rd_count + 2,
> - priv->regs + TSI721_DMAC_DWRCNT(TSI721_DMACH_MAINT));
> - ioread32(priv->regs + TSI721_DMAC_DWRCNT(TSI721_DMACH_MAINT));
> + iowrite32(rd_count + 2, regs + TSI721_DMAC_DWRCNT);
> + ioread32(regs + TSI721_DMAC_DWRCNT);
>   i = 0;
>  
>   /* Wait until DMA transfer is finished */
> - while ((ch_stat = ioread32(priv->regs +
> - TSI721_DMAC_STS(TSI721_DMACH_MAINT))) & TSI721_DMAC_STS_RUN) {
> + while ((ch_stat = ioread32(regs + TSI721_DMAC_STS))
> + & TSI721_DMAC_STS_RUN) {
>   udelay(1);
>   if (++i >= 500) {
>   dev_dbg(&priv->pdev->dev,
>   "%s : DMA[%d] read timeout ch_status=%x\n",
> - __func__, TSI721_DMACH_MAINT, ch_stat);
> + __func__, priv->mdma.ch_id, ch_stat);
>   if (!do_wr)
>   *data = 0x;
>   err = -EIO;
> @@ -162,13 +161,10 @@ static int tsi721_maint_dma(struct tsi721_device *priv, 
> u32 sys_size,
>   __func__, ch_stat);
>   dev_dbg(&priv->pdev->dev, "OP=%d : destid=%x hc=%x off=%x\n",
>   do_wr ? MAINT_WR : MAINT_RD, destid, hopcount, offset);
> -   

Re: [PATCH v6] dmaengine: Driver support for FSL RaidEngine device.

2015-04-02 Thread Vinod Koul
On Tue, Mar 03, 2015 at 02:26:22PM +0800, xuelin@freescale.com wrote:
> From: Xuelin Shi 
> 
> The RaidEngine is a new FSL hardware used for Raid5/6 acceration.
> This patch enables the RaidEngine functionality and provides
> hardware offloading capability for memcpy, xor and pq computation.
> It works with async_tx.
Applied, with below patch added on top

diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
index 12778bd6446b..4d9470f16552 100644
--- a/drivers/dma/fsl_raid.c
+++ b/drivers/dma/fsl_raid.c
@@ -629,7 +629,7 @@ static void fsl_re_free_chan_resources(struct dma_chan
*chan)
dev_err(re_chan->dev, "chan resource cannot be cleaned!\n");
 }

-int fsl_re_chan_probe(struct platform_device *ofdev,
+static int fsl_re_chan_probe(struct platform_device *ofdev,
  struct device_node *np, u8 q, u32 off)
 {
struct device *dev, *chandev;

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [RFC PATCH 1/2] RapidIO: Add DMA Engine support for RIO data transfers

2011-10-06 Thread Vinod Koul
On Mon, 2011-10-03 at 09:52 -0700, Bounine, Alexandre wrote:
> Vinod Koul wrote:
> > 
> > On Fri, 2011-09-30 at 17:38 -0400, Alexandre Bounine wrote:
> > Please CC *maintainers* on your patches, get_maintainers.pl will tell
> > you who. Adding Dan here
> 
> Based on https://lkml.org/lkml/2011/2/14/67 and use of DMA_SLAVE in this
> patch I decided that you are the best match among two and there is no reason
> to disturb Dan ;) 
I don't think he minds :) and we can all benefit from his wisdom

> 
> > > Adds DMA Engine framework support into RapidIO subsystem.
> > > Uses DMA Engine DMA_SLAVE interface to generate data transfers to/from 
> > > remote
> > > RapidIO target devices. Uses scatterlist to describe local data buffer and
> > > dma_chan.private member to pass target specific information. Supports flat
> > > data buffer only for a remote side.
> > The way dmaengine works today is that it doesn't know anything about
> > client subsystem. But this brings in a subsystem details to dmaengine
> > which I don't agree with yet.
> > Why can't we abstract this out??
> 
> The only thing that brings subsystem knowledge into dmaengine is DMA_RAPIDIO 
> flag.
> I am actually on the fence about this. From RapidIO side point of view I do 
> not
> need that flag at all.
> RapidIO uses a filter routine that is sufficient to identify dmaengine 
> channels
> associated with specific RapidIO mport. Use of DMA_SLAVE flag is safe here.
> Use of private member of dma_chan is "private" business of RapidIO and does
> not break anything. 
> 
> My concern here is that other subsystems may use/request DMA_SLAVE channel(s) 
> as well
> and wrongfully acquire one that belongs to RapidIO. In this case separation 
> with another
> flag may have a sense - it is possible to have a system that uses RapidIO
> and other "traditional" DMA slave channel.
Nope that will never happen in current form.
Every controller driver today "magically" ensures that it doesn't get
any other dma controllers channel. We use filter function for that.
Although it is not clean yet and we are working to fix that but that's
another discussion.
Even specifying plain DMA_SLAVE should work if you code your filter
function properly :)

> 
> This is why I put that proposed interface for discussion instead of keeping 
> everything
> inside of RapidIO.
> If you think that situation above will not happen I will be happy to remove
> that subsystem knowledge from dmaengine files.
> 
> My most recent test implementation runs without DMA_RAPIDIO flag though.
> 
> > 
> > After going thru the patch, I do not believe that this this is case of
> > SLAVE transfers, Dan can you please take a look at this patch
> 
> I agree, this is not a case of "pure" slave transfers but existing DMA_SLAVE
> interface fits well into the RapidIO operations.
> 
> First, we have only one memory mapped location on the host side. We transfer
> data to/from location that is not mapped into memory on the same side.  
> 
> Second, having ability to pass private target information allows me to pass
> information about remote target device on per-transfer basis.
Okay, then why not pass the dma address and make your dma driver
transparent (i saw you passed RIO address, IIRC 64+2 bits)
Currently using dma_slave_config we pass channel specific information,
things like peripheral address and config don't change typically between
transfers and if you have some controller specific properties you can
pass them by embedding dma_slave_config in your specific structure.
Worst case, you can configure slave before every prepare


-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [RFC PATCH 1/2] RapidIO: Add DMA Engine support for RIO data transfers

2011-10-15 Thread Vinod Koul
On Fri, 2011-10-07 at 12:08 -0700, Bounine, Alexandre wrote:
> Vinod Koul wrote:
> > 
> > On Mon, 2011-10-03 at 09:52 -0700, Bounine, Alexandre wrote:
Adding Jassi to this and sorry for late reply...

> ... skip ...
> > >
> > > Second, having ability to pass private target information allows me to 
> > > pass
> > > information about remote target device on per-transfer basis.
> > Okay, then why not pass the dma address and make your dma driver
> > transparent (i saw you passed RIO address, IIRC 64+2 bits)
> > Currently using dma_slave_config we pass channel specific information,
> > things like peripheral address and config don't change typically
> > between
> > transfers and if you have some controller specific properties you can
> > pass them by embedding dma_slave_config in your specific structure.
> > Worst case, you can configure slave before every prepare
> 
> In addition to address on target RIO device I need to pass corresponding
> device destination ID. With single channel capable to transfer data between
> local memory and different RapidIO devices I have to pass device specific
> information on per transfer basis (destID + 66-bit address + type of write 
> ops, etc.).
> 
> Even having 8 channels (each set for specific target) will not help me with
> full support of RapidIO network where I have 8- or 16-bit destID (256 or 64K
> devices respectively).
> 
> RapidIO controller device (and its DMA component) may provide services to
> multiple device drivers which service individual devices on RapidIO network
> (similar to PCIe having multiple peripherals, but not using memory mapped
> model - destID defines route to a device). 
> 
> Generic RapidIO controller may have only one DMA channel which services all
> target devices forming the network. We may have multiple concurrent data
> transfer requests for different devices.
> 
> Parallel discussion with Dan touches the same post-config approach and
> another option. I like Dan's idea of having RIO-specific version of prep_sg().
> At the same time my current implementation of rio_dma_prep_slave_sg() with
> added appropriate locking may do job as well and keeps DMA part within
> existing API (DMA_RAPIDIO removed). 
Thanks this helps to understand your model.

Per above you need destID, 66bit address and type of write to be passed
for each transfer. Looking at your rio_dma_data, i cant see the destID.

Nevertheless we have few ways to solve this, pass this using
dma_slave_config _every-time_ before doing a prep_slave_sg. Or as Dan
pointed out create RIO specific version. Principally I am not for adding
a subsystem specific stuff, even more when we are talking about
converging and cutting down on dmaengine API :)

Another alternate approach could be to add one more argument to
prep_slave_sg API which allows us to pass additional runtime specific
parameters. This can be NULL and unused for existing drivers and used in
RIO and any future subsystems which want to use dmaengine.
Thoughts...?

-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC PATCH 1/2] RapidIO: Add DMA Engine support for RIO data transfers

2011-10-17 Thread Vinod Koul
On Mon, 2011-10-17 at 21:22 +0530, Jassi Brar wrote:
> On 15 October 2011 23:05, Vinod Koul  wrote:
> 
> > Another alternate approach could be to add one more argument to
> > prep_slave_sg API which allows us to pass additional runtime specific
> > parameters. This can be NULL and unused for existing drivers and used in
> > RIO and any future subsystems which want to use dmaengine.
> > Thoughts...?
> >
> That doesn't sound much different than passing the data via
> dma_chan.private during prep_slave_sg. Only now we add to
> the number of arguments.
Yes agreed, but we already decided and marked it as depreciated.

> And then either this argument would be RapidIO specific (unfair
> to other users) or generic. In latter case what would it look like ?
My initial thoughts were to add an argument which only the specific dmac
knows howto decode and is filled by its client. As i said for existing
users and people who don't require dynamic information wouldn't bother.
The pros
 - allows us to support RIO kind of subsystems where one needs to pass
subsystem specific information for programing the dmac
 - doesn't require us to add subsystem specific stuff in dmaengine,
today its RIO tomorrow some other folks may want to add. We want to
maintain dmaengine as a generic framework, while also trying to support
multiple audiences.
Cons:
 - there is no guarantee;  dmac expects foo and clients pass bar

I am okay if we have alternate way to support this with above goal :)

-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback

2012-01-30 Thread Vinod Koul
On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> As we agreed during our discussion about adding DMA Engine support for RapidIO
> subsystem, RapidIO and similar clients may benefit from adding an extra 
> context
> parameter to device_prep_slave_sg() callback.
> See https://lkml.org/lkml/2011/10/24/275 for more details.
> 
> Adding the context parameter will allow to pass client/target specific
> information associated with an individual data transfer request.
> 
> In the case of RapidIO support this additional information consists of target
> destination ID and its buffer address (which is not mapped into the local CPU
> memory space). Because a single RapidIO-capable DMA channel may queue data
> transfer requests to different target devices, the per-request configuration
> is required.
> 
> The proposed change eliminates need for new subsystem-specific API.
> Existing DMA_SLAVE clients will ignore the new parameter.
> 
> This RFC only demonstrates the API change and does not include corresponding
> changes to existing DMA_SLAVE clients. Complete set of patches will be 
> provided
> after (if) this API change is accepted.
This looks good to me. But was thinking if we need to add this new
parameter for other slave calls (circular, interleaved, memcpy...)

> 
> Signed-off-by: Alexandre Bounine 
> Cc: Jassi Brar 
> Cc: Russell King  
> Cc: Kumar Gala 
> Cc: Matt Porter 
> Cc: Li Yang 
> ---
>  include/linux/dmaengine.h |7 ---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 679b349..79d71bb 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -575,7 +575,7 @@ struct dma_device {
>   struct dma_async_tx_descriptor *(*device_prep_slave_sg)(
>   struct dma_chan *chan, struct scatterlist *sgl,
>   unsigned int sg_len, enum dma_transfer_direction direction,
> - unsigned long flags);
> + unsigned long flags, void *context);
>   struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
>   struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
>   size_t period_len, enum dma_transfer_direction direction);
> @@ -607,12 +607,13 @@ static inline int dmaengine_slave_config(struct 
> dma_chan *chan,
>  
>  static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
>   struct dma_chan *chan, void *buf, size_t len,
> - enum dma_transfer_direction dir, unsigned long flags)
> + enum dma_transfer_direction dir, unsigned long flags, void *context)
>  {
>   struct scatterlist sg;
>   sg_init_one(&sg, buf, len);
>  
> - return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags);
> + return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags,
> +   context);
>  }
>  
>  static inline int dmaengine_terminate_all(struct dma_chan *chan)


-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback

2012-01-30 Thread Vinod Koul
On Mon, 2012-01-30 at 08:55 -0800, Bounine, Alexandre wrote:
> On Monday, January 30, 2012 at 4:31 AM, Vinod Koul wrote:
> > 
> > On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> > > As we agreed during our discussion about adding DMA Engine support for 
> > > RapidIO
> > > subsystem, RapidIO and similar clients may benefit from adding an extra 
> > > context
> > > parameter to device_prep_slave_sg() callback.
> > > See https://lkml.org/lkml/2011/10/24/275 for more details.
> > >
> > > Adding the context parameter will allow to pass client/target specific
> > > information associated with an individual data transfer request.
> > >
> > > In the case of RapidIO support this additional information consists of 
> > > target
> > > destination ID and its buffer address (which is not mapped into the local 
> > > CPU
> > > memory space). Because a single RapidIO-capable DMA channel may queue data
> > > transfer requests to different target devices, the per-request 
> > > configuration
> > > is required.
> > >
> > > The proposed change eliminates need for new subsystem-specific API.
> > > Existing DMA_SLAVE clients will ignore the new parameter.
> > >
> > > This RFC only demonstrates the API change and does not include 
> > > corresponding
> > > changes to existing DMA_SLAVE clients. Complete set of patches will be 
> > > provided
> > > after (if) this API change is accepted.
> >
> > This looks good to me. But was thinking if we need to add this new
> > parameter for other slave calls (circular, interleaved, memcpy...)
> > 
> 
> I agree that cyclic and interleaved calls may benefit from adding that 
> parameter as well.
> Benefits to the cyclic call are straightforward - same as dma_slave.
> Adding a context parameter to the interleaved transfers may be more future 
> proofing option
> than an immediate need. Memcopy and other calls that deal with local memory 
> transfers
> probably should be left untouched.
> 
> What if we limit modifications to:
> 1) three calls (slave, cyclic and interleaved) OR
> 2) two (slave and cyclic) at this moment?
> 
> I am just more focused on dma_slave just because it fits well to provide RDMA
> over RapidIO fabric.
> 
> If everybody agrees, I can go ahead and make changes to all three at once.
For now we need at least slave and cyclic, so pls go ahead and make these 
changes.
For interleaved, we might need it sooner [1], but I would think it would
need few more changes to the API, so it can be rolled as part of those
changes.

-- 
~Vinod

[1]: https://lkml.org/lkml/2012/1/30/48

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback

2012-01-31 Thread Vinod Koul
On Wed, 2012-02-01 at 01:09 +0100, Guennadi Liakhovetski wrote:
> On Mon, 30 Jan 2012, Vinod Koul wrote:
> 
> > On Thu, 2012-01-26 at 16:22 -0500, Alexandre Bounine wrote:
> > > As we agreed during our discussion about adding DMA Engine support for 
> > > RapidIO
> > > subsystem, RapidIO and similar clients may benefit from adding an extra 
> > > context
> > > parameter to device_prep_slave_sg() callback.
> > > See https://lkml.org/lkml/2011/10/24/275 for more details.
> > > 
> > > Adding the context parameter will allow to pass client/target specific
> > > information associated with an individual data transfer request.
> > > 
> > > In the case of RapidIO support this additional information consists of 
> > > target
> > > destination ID and its buffer address (which is not mapped into the local 
> > > CPU
> > > memory space). Because a single RapidIO-capable DMA channel may queue data
> > > transfer requests to different target devices, the per-request 
> > > configuration
> > > is required.
> > > 
> > > The proposed change eliminates need for new subsystem-specific API.
> > > Existing DMA_SLAVE clients will ignore the new parameter.
> > > 
> > > This RFC only demonstrates the API change and does not include 
> > > corresponding
> > > changes to existing DMA_SLAVE clients. Complete set of patches will be 
> > > provided
> > > after (if) this API change is accepted.
> > This looks good to me. But was thinking if we need to add this new
> > parameter for other slave calls (circular, interleaved, memcpy...)
> 
> Yes, we (shdma.c) also need to pass additional slave configuration 
> information to the dmaengine driver and I also was thinking about 
> extending the existing API, but my ideas were going more in the direction 
> of adding a parameter to __dma_request_channel() along the lines of
So your question is more on the lines of channel mapping/allocation?
The approach here is to pass controller specific parameters which are
required to setup the respective transfer. Since this is dependent on
each transfer, this needs to be passed in respective prepare.

The two things are completely orthogonal and shouldn't be clubbed.
For your issue we need a separate debate on how to solve this... I am
open to ideas...


-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] dmaengine/dma_slave: add context parameter to prep_slave_sg callback

2012-02-01 Thread Vinod Koul
On Wed, 2012-02-01 at 12:58 +0100, Guennadi Liakhovetski wrote:
> > The two things are completely orthogonal and shouldn't be clubbed.
> > For your issue we need a separate debate on how to solve this... I am
> > open to ideas...
> 
> Well, I'm not sure whether they are necessarily always orthogonal, they 
> don't seem so in my case at least. We definitely can use our approach - 
> configure the channel during allocation. I _think_ we could also perform 
> the configuration on a per-transfer basis, during the prepare stage, as 
> this RFC is suggesting, but that definitely would require reworking the 
> driver somewhat and changing the concept. The current concept is a fixed 
> DMA channel allocation to slaves for as long as the slave is using DMA. 
> This is simpler, avoids some overhead during operation and fits well with 
> the dmaengine PRIVATE channel concept. So, given the choice, we would 
> prefer to perform the configuration during channel allocation.
> 
> Maybe there are cases, where the driver absolutely needs this additional 
> information during allocation, in which case my proposal would be the only 
> way to go for them.
what are you trying to address, sending controller specific information
at allocation or the channel allocation itself. I kind of sense both.
But apprach here is discussed is to pass paramters which are required
for each transfer, not static for a channel, hence the additional
controller specific parameter in respective prepare. 
> 
> I'll post an RFC soon - stay tuned:-) 
Patch is always the best idea :-)

-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

2013-07-15 Thread Vinod Koul
On Fri, May 31, 2013 at 05:09:51PM -0700, Dan Williams wrote:
> On Thu, May 30, 2013 at 10:47 AM, Vinod Koul  wrote:
> > On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
> >> Here is a set of small independent patches that clean up or fix minor 
> >> things
> >> across DMA slave drivers.
> > The series looks fine. I am going to wait a day more and apply, pls speak 
> > up if
> > you disagree and ack if you agree
> 
> Looks ok to me.  Reminds we can probably take this one step further
> and provide a generic implementation for the common case.  It's just a
> bit inconsistent though that some engines will poll the completion
> handler (try to advance the state of the last completed cookie)
> whereas others just assume things will be completed asynchronously.
agree with that. These are the inconsistencies some of which are based on
hardware and some are user iterpretations...

--
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

2013-07-15 Thread Vinod Koul
On Thu, May 30, 2013 at 09:32:19PM +0300, Andy Shevchenko wrote:
> >> Here is a set of small independent patches that clean up or fix minor 
> >> things
> >> across DMA slave drivers.

Applied thanks

--
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

2013-07-15 Thread Vinod Koul
On Mon, Jul 15, 2013 at 01:21:17PM +0300, Andy Shevchenko wrote:
> On Mon, 2013-07-15 at 15:07 +0530, Vinod Koul wrote: 
> > On Thu, May 30, 2013 at 09:32:19PM +0300, Andy Shevchenko wrote:
> > > >> Here is a set of small independent patches that clean up or fix minor 
> > > >> things
> > > >> across DMA slave drivers.
> > 
> > Applied thanks
> 
> Thank you. You were faster than me, I was just about to send rebased
> version.
:)

I suspected changes are trivial and it should apply or with slight modfications.
it did apply cleanly so no reason to delay applying on shiny new -rc1

For folks in this part of world, monday morning Linus drops the rc1 usually, so
get staright to it!

--
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v6 3/3] DMA: Freescale: update driver to support 8-channel DMA engine

2013-07-28 Thread Vinod Koul
On Fri, Jul 26, 2013 at 06:27:16PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> This patch adds support to 8-channel DMA engine, thus the driver works for 
> both
> the new 8-channel and the legacy 4-channel DMA engines.
> 
> Signed-off-by: Hongbo Zhang 
This looks fine. I need someone to ACK the DT bindings for the series to go in

~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v7 0/3] DMA: Freescale: Add support for 8-channel DMA engine

2013-07-29 Thread Vinod Koul
On Mon, Jul 29, 2013 at 06:49:01PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> Hi Vinod, Dan, Scott and Leo, please have a look at these V7 patches.
The dma relates changes look okay to me.

I need someone to review and ACK the DT bindings.

~Vinod
> 
> Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch set
> adds support this DMA engine.
> 
> V6->V7 changes:
> - only remove unnecessary "CHIP-dma" explanations in [1/3]
> 
> V5->V6 changes:
> - minor updates of descriptions in binding document and Kconfig
> - remove [4/4], that should be another patch in future
> 
> V4->V5 changes:
> - update description in the dt binding document, to make it more resonable
> - add new patch [4/4] to eliminate a compiling warning which already exists
>   for a long time
> 
> V3->V4 changes:
> - introduce new patch [1/3] to revise the legacy dma binding document
> - and then add new paragraph to describe new dt node binding in [2/3]
> - rebase to latest kernel v3.11-rc1
> 
> V2->V3 changes:
> - edit Documentation/devicetree/bindings/powerpc/fsl/dma.txt
> - edit text string in Kconfig and the driver files, using "elo series" to
>   mention all the current "elo*"
> 
> V1->V2 changes:
> - removed the codes handling the register dgsr1, since it isn't used currently
> - renamed the DMA DT compatible to "fsl,elo3-dma"
> - renamed the new dts files to "elo3-dma-.dtsi"
> 
> Hongbo Zhang (3):
>   DMA: Freescale: revise device tree binding document
>   DMA: Freescale: Add new 8-channel DMA engine device tree nodes
>   DMA: Freescale: update driver to support 8-channel DMA engine
> 
>  .../devicetree/bindings/powerpc/fsl/dma.txt|  114 
> +++-
>  arch/powerpc/boot/dts/fsl/b4si-post.dtsi   |4 +-
>  arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi  |   81 ++
>  arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi  |   81 ++
>  arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
>  drivers/dma/Kconfig|9 +-
>  drivers/dma/fsldma.c   |9 +-
>  drivers/dma/fsldma.h   |2 +-
>  8 files changed, 264 insertions(+), 40 deletions(-)
>  create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
>  create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
> 
> -- 
> 1.7.9.5
> 
> 
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v7 0/3] DMA: Freescale: Add support for 8-channel DMA engine

2013-08-20 Thread Vinod Koul
On Tue, Aug 20, 2013 at 04:33:46PM +0800, Hongbo Zhang wrote:
> On 07/29/2013 06:59 PM, Vinod Koul wrote:
> >On Mon, Jul 29, 2013 at 06:49:01PM +0800, hongbo.zh...@freescale.com wrote:
> >>From: Hongbo Zhang 
> >>
> >>Hi Vinod, Dan, Scott and Leo, please have a look at these V7 patches.
> >The dma relates changes look okay to me.
> >
> >I need someone to review and ACK the DT bindings.
> >
> >~Vinod
> Vinod,
> Are you using this tree?
> http://git.infradead.org/users/vkoul/slave-dma.git
Yes

> Did you merge these patches?
No

As I said I would like someone who know DT and dma binding to ack them. I see
devicetree ML has been cced, can Arnd or someone else review these...

~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH 23/51] DMA-API: dma: pl08x: add dma_set_mask_and_coherent() call

2013-09-23 Thread Vinod Koul
On Thu, Sep 19, 2013 at 10:48:01PM +0100, Russell King wrote:
> The DMA API requires drivers to call the appropriate dma_set_mask()
> functions before doing any DMA mapping.  Add this required call to
> the AMBA PL08x driver.
> 
> Signed-off-by: Russell King 
Acked-by: Vinod Koul 

~Vinod
> ---
>  drivers/dma/amba-pl08x.c |5 +
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> index fce46c5..e51a983 100644
> --- a/drivers/dma/amba-pl08x.c
> +++ b/drivers/dma/amba-pl08x.c
> @@ -2055,6 +2055,11 @@ static int pl08x_probe(struct amba_device *adev, const 
> struct amba_id *id)
>   if (ret)
>   return ret;
>  
> + /* Ensure that we can do DMA */
> + ret = dma_set_mask_and_coherent(&adev->dev, DMA_BIT_MASK(32));
> + if (ret)
> + goto out_no_pl08x;
> +
>   /* Create the driver state holder */
>   pl08x = kzalloc(sizeof(*pl08x), GFP_KERNEL);
>   if (!pl08x) {
> -- 
> 1.7.4.4
> 
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH 43/51] DMA-API: dma: edma.c: no need to explicitly initialize DMA masks

2013-09-23 Thread Vinod Koul
On Fri, Sep 20, 2013 at 12:15:39AM +0100, Russell King wrote:
> register_platform_device_full() can setup the DMA mask provided the
> appropriate member is set in struct platform_device_info.  So lets
> make that be the case.  This avoids a direct reference to the DMA
> masks by this driver.
> 
> Signed-off-by: Russell King 
Acked-by: Vinod Koul 

This also brings me question that should we force the driver to use the
dma_set_mask_and_coherent() API or they have below flexiblity too?

~Vinod

> ---
>  drivers/dma/edma.c |6 ++
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index ff50ff4..7f9fe30 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -702,11 +702,13 @@ static struct platform_device *pdev0, *pdev1;
>  static const struct platform_device_info edma_dev_info0 = {
>   .name = "edma-dma-engine",
>   .id = 0,
> + .dma_mask = DMA_BIT_MASK(32),
>  };
>  
>  static const struct platform_device_info edma_dev_info1 = {
>   .name = "edma-dma-engine",
>   .id = 1,
> + .dma_mask = DMA_BIT_MASK(32),
>  };


>  
>  static int edma_init(void)
> @@ -720,8 +722,6 @@ static int edma_init(void)
>   ret = PTR_ERR(pdev0);
>   goto out;
>   }
> - pdev0->dev.dma_mask = &pdev0->dev.coherent_dma_mask;
> - pdev0->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>   }
>  
>   if (EDMA_CTLRS == 2) {
> @@ -731,8 +731,6 @@ static int edma_init(void)
>   platform_device_unregister(pdev0);
>   ret = PTR_ERR(pdev1);
>   }
> - pdev1->dev.dma_mask = &pdev1->dev.coherent_dma_mask;
> - pdev1->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>   }
>  
>  out:
> -- 
> 1.7.4.4
> 
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCH 24/51] DMA-API: dma: pl330: add dma_set_mask_and_coherent() call

2013-09-23 Thread Vinod Koul
On Sat, Sep 21, 2013 at 09:00:00PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 20, 2013 at 07:26:27PM +0200, Heiko Stübner wrote:
> > Am Donnerstag, 19. September 2013, 23:49:01 schrieb Russell King:
> > > The DMA API requires drivers to call the appropriate dma_set_mask()
> > > functions before doing any DMA mapping.  Add this required call to
> > > the AMBA PL08x driver.
> > ^--- copy and paste error - should of course be PL330
> 
> Fixed, thanks.
with fixed changelog...

Acked-by: Vinod Koul 

~Vinod

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 0/9] DMA engine cookie handling cleanups

2012-03-07 Thread Vinod Koul
On Tue, 2012-03-06 at 22:33 +, Russell King - ARM Linux wrote:
> [v2 - more or less same description.  Including lakml in cc for the full
> set]
> 
> This patch series cleans up the handling of cookies in DMA engine drivers.
> This is done by providing a set of inline library functions for common
> tasks:
> 
> - moving the 'last completed cookie' into struct dma_chan - everyone
>   has this in their driver private channel data structure
> 
> - consolidate allocation of cookies to DMA descriptors
> 
> - common way to update 'last completed cookie' value
> 
> - standard way to implement tx_status callback and update the residue
> 
> - consolidate initialization of cookies
> 
> - update implementations differing from the majority of DMA engine drivers
>   to behave the same as the majority implementation in respect of cookies
> 
> What this means is that we get to the point where all DMA engine drivers
> will hand out cookie value '2' as the first, and incrementing cookie
> values up to INT_MAX, returning to cookie '1' as the next cookie.
> 
> Think of this patch series as round 1...  I am hoping over time that more
> code can be consolidated between the DMA engine drivers and end up with a
> consistent way to handle various common themes in DMA engine hardware
> (like physical channel<->peripheral request signal selection.)
Thanks Russell,

I have tested this on atom today, and as expected works flawlessly :)
After all acks, I can merge or these can go thru your tree with my Ack.
Either way is okay.

I applied the v2 on a branch and also rebased on top of slave-dma.next.
There were few conflicts in imx-dma.c. Sacha, Javier, pls see that merge
is right.

This branch (rmk_cookie_fixes2_rebased) is not yet pushed, as I am not
able to connect to infradead.org, should be done when server is back.



-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 0/9] DMA engine cookie handling cleanups

2012-03-12 Thread Vinod Koul
On Mon, 2012-03-12 at 16:11 +, Russell King - ARM Linux wrote:
> On Wed, Mar 07, 2012 at 07:24:26PM +0530, Vinod Koul wrote:
> > On Tue, 2012-03-06 at 22:33 +, Russell King - ARM Linux wrote:
> > > [v2 - more or less same description.  Including lakml in cc for the full
> > > set]
> > > 
> > > This patch series cleans up the handling of cookies in DMA engine drivers.
> > > This is done by providing a set of inline library functions for common
> > > tasks:
> > > 
> > > - moving the 'last completed cookie' into struct dma_chan - everyone
> > >   has this in their driver private channel data structure
> > > 
> > > - consolidate allocation of cookies to DMA descriptors
> > > 
> > > - common way to update 'last completed cookie' value
> > > 
> > > - standard way to implement tx_status callback and update the residue
> > > 
> > > - consolidate initialization of cookies
> > > 
> > > - update implementations differing from the majority of DMA engine drivers
> > >   to behave the same as the majority implementation in respect of cookies
> > > 
> > > What this means is that we get to the point where all DMA engine drivers
> > > will hand out cookie value '2' as the first, and incrementing cookie
> > > values up to INT_MAX, returning to cookie '1' as the next cookie.
> > > 
> > > Think of this patch series as round 1...  I am hoping over time that more
> > > code can be consolidated between the DMA engine drivers and end up with a
> > > consistent way to handle various common themes in DMA engine hardware
> > > (like physical channel<->peripheral request signal selection.)
> > Thanks Russell,
> > 
> > I have tested this on atom today, and as expected works flawlessly :)
> > After all acks, I can merge or these can go thru your tree with my Ack.
> > Either way is okay.
> > 
> > I applied the v2 on a branch and also rebased on top of slave-dma.next.
> > There were few conflicts in imx-dma.c. Sacha, Javier, pls see that merge
> > is right.
> > 
> > This branch (rmk_cookie_fixes2_rebased) is not yet pushed, as I am not
> > able to connect to infradead.org, should be done when server is back.
> 
> Are you going to pick up Shawn Guo's tested-by for these patches, or are
> we saying its too late for that now?
Sure, his and others we have received on this series.
Also, IIRC there was one fixup from Jassi on pl330?

I should be able to merge this tomorrow, if thats fine with you.



-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2 0/9] DMA engine cookie handling cleanups

2012-03-13 Thread Vinod Koul
On Mon, 2012-03-12 at 21:53 +0530, Vinod Koul wrote:
> > > I applied the v2 on a branch and also rebased on top of
> slave-dma.next.
> > > There were few conflicts in imx-dma.c. Sacha, Javier, pls see that
> merge
> > > is right.
> > > 
> > > This branch (rmk_cookie_fixes2_rebased) is not yet pushed, as I am
> not
> > > able to connect to infradead.org, should be done when server is
> back.
> > 
> > Are you going to pick up Shawn Guo's tested-by for these patches, or
> are
> > we saying its too late for that now?
> Sure, his and others we have received on this series.
> Also, IIRC there was one fixup from Jassi on pl330?
> 
> I should be able to merge this tomorrow, if thats fine with you. 
I have merged this to my next, but haven't pushed out yet. This is
available on branch next_cookie2_merge.
Also I got few (more than expected build failures) and a merge issue on
imx driver. Sascha, Russell can you see if fix on imx is right

Please see if the below patch is the right fix for build failures in
addition to one suggested by Jassi.

---x-----x------

>From 949ff5b8d46b5e3435d21b2651ce3a2599208d44 Mon Sep 17 00:00:00 2001
From: Vinod Koul 
Date: Tue, 13 Mar 2012 11:58:12 +0530
Subject: [PATCH] dmaengine: fix for cookie changes and merge

Fixed trivial issues in drivers:
drivers/dma/imx-sdma.c
drivers/dma/intel_mid_dma.c
drivers/dma/ioat/dma_v3.c
    drivers/dma/iop-adma.c
drivers/dma/sirf-dma.c
drivers/dma/timb_dma.c

Signed-off-by: Vinod Koul 
---
 drivers/dma/imx-sdma.c  |1 +
 drivers/dma/intel_mid_dma.c |1 +
 drivers/dma/ioat/dma_v3.c   |1 +
 drivers/dma/iop-adma.c  |1 +
 drivers/dma/sirf-dma.c  |2 ++
 drivers/dma/timb_dma.c  |6 +-
 6 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index ccfc7c4..81f9d57 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1127,6 +1127,7 @@ static void sdma_issue_pending(struct dma_chan *chan)
struct sdma_engine *sdma = sdmac->sdma;
 
if (sdmac->status == DMA_IN_PROGRESS)
+   sdma_enable_channel(sdma, sdmac->channel);
 }
 
 #define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V134
diff --git a/drivers/dma/intel_mid_dma.c b/drivers/dma/intel_mid_dma.c
index d599d96..2449812 100644
--- a/drivers/dma/intel_mid_dma.c
+++ b/drivers/dma/intel_mid_dma.c
@@ -477,6 +477,7 @@ static enum dma_status intel_mid_dma_tx_status(struct 
dma_chan *chan,
dma_cookie_t cookie,
struct dma_tx_state *txstate)
 {
+   struct intel_mid_dma_chan *midc = to_intel_mid_dma_chan(chan);
enum dma_status ret;
 
ret = dma_cookie_status(chan, cookie, txstate);
diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
index 145eda2..2c4476c 100644
--- a/drivers/dma/ioat/dma_v3.c
+++ b/drivers/dma/ioat/dma_v3.c
@@ -61,6 +61,7 @@
 #include 
 #include 
 #include 
+#include "../dmaengine.h"
 #include "registers.h"
 #include "hw.h"
 #include "dma.h"
diff --git a/drivers/dma/iop-adma.c b/drivers/dma/iop-adma.c
index 1f3a703..4499f88 100644
--- a/drivers/dma/iop-adma.c
+++ b/drivers/dma/iop-adma.c
@@ -894,6 +894,7 @@ static enum dma_status iop_adma_status(struct dma_chan 
*chan,
struct dma_tx_state *txstate)
 {
struct iop_adma_chan *iop_chan = to_iop_adma_chan(chan);
+   int ret;
 
ret = dma_cookie_status(chan, cookie, txstate);
if (ret == DMA_SUCCESS)
diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c
index a2cde85..45ba352 100644
--- a/drivers/dma/sirf-dma.c
+++ b/drivers/dma/sirf-dma.c
@@ -18,6 +18,8 @@
 #include 
 #include 
 
+#include "dmaengine.h"
+
 #define SIRFSOC_DMA_DESCRIPTORS 16
 #define SIRFSOC_DMA_CHANNELS16
 
diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c
index 7805996..d408c22 100644
--- a/drivers/dma/timb_dma.c
+++ b/drivers/dma/timb_dma.c
@@ -510,17 +510,13 @@ static void td_free_chan_resources(struct dma_chan *chan)
 static enum dma_status td_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
struct dma_tx_state *txstate)
 {
-   struct timb_dma_chan *td_chan =
-   container_of(chan, struct timb_dma_chan, chan);
enum dma_status ret;
 
dev_dbg(chan2dev(chan), "%s: Entry\n", __func__);
 
ret = dma_cookie_status(chan, cookie, txstate);
 
-   dev_dbg(chan2dev(chan),
-   "%s: exit, ret: %d, last_complete: %d, last_used: %d\n",
-   __func__, ret, last_complete, last_used);
+   dev_dbg(chan2dev(chan), "%s: e

Re: [PATCH v2 0/9] DMA engine cookie handling cleanups

2012-03-13 Thread Vinod Koul
On Tue, 2012-03-13 at 12:31 +, Russell King - ARM Linux wrote:
> On Tue, Mar 13, 2012 at 02:10:36PM +0530, Vinod Koul wrote:
> > Please see if the below patch is the right fix for build failures in
> > addition to one suggested by Jassi.
> 
> I'm not sure that Jassi's solution is correct - and I'm wondering whether
> any of the DMA engine drivers do the right thing when transfers are
> terminated.  Is it right for the DMA status function to return IN_PROGRESS
> for a previously submitted cookie which has been terminated?
> 
> I can see two answers to that, both equally valid:
> 
> 1. It allows you to find out exactly where the DMA engine got to before
>the transfer was terminated, and therefore recover from the termination
>if you wish to.
> 
> 2. Returning in-progress when a cookie will never be completed is
>misleading, and could be misinterpreted by users of the tx_status
>function, especially if they are waiting for a particular transaction
>to complete.
> 
> Maybe we need to introduce a DMA_TERMINATED status?
I would agree with you that DMA_TERMINATED seems to be correct option.
IN_PROGRESS would certainly confuse... 
I will drop Jassi's fix from this branch. Care to send the patch?
> 
> > ---x-x--
> > 
> > >From 949ff5b8d46b5e3435d21b2651ce3a2599208d44 Mon Sep 17 00:00:00 2001
> > From: Vinod Koul 
> > Date: Tue, 13 Mar 2012 11:58:12 +0530
> > Subject: [PATCH] dmaengine: fix for cookie changes and merge
> > 
> > Fixed trivial issues in drivers:
> > drivers/dma/imx-sdma.c
> > drivers/dma/intel_mid_dma.c
> > drivers/dma/ioat/dma_v3.c
> > drivers/dma/iop-adma.c
> > drivers/dma/sirf-dma.c
> > drivers/dma/timb_dma.c
> > 
> > Signed-off-by: Vinod Koul 
> > ---
> >  drivers/dma/imx-sdma.c  |1 +
> >  drivers/dma/intel_mid_dma.c |1 +
> >  drivers/dma/ioat/dma_v3.c   |1 +
> >  drivers/dma/iop-adma.c  |1 +
> >  drivers/dma/sirf-dma.c  |2 ++
> >  drivers/dma/timb_dma.c  |6 +-
> >  6 files changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index ccfc7c4..81f9d57 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -1127,6 +1127,7 @@ static void sdma_issue_pending(struct dma_chan *chan)
> > struct sdma_engine *sdma = sdmac->sdma;
> >  
> > if (sdmac->status == DMA_IN_PROGRESS)
> > +   sdma_enable_channel(sdma, sdmac->channel);
> >  }
> >  
> >  #define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V134
> 
> This looks like a merge conflict resolution.  I don't see this being
> caused by my patches as I haven't touched this function.
> 
> > diff --git a/drivers/dma/intel_mid_dma.c b/drivers/dma/intel_mid_dma.c
> > index d599d96..2449812 100644
> > --- a/drivers/dma/intel_mid_dma.c
> > +++ b/drivers/dma/intel_mid_dma.c
> > @@ -477,6 +477,7 @@ static enum dma_status intel_mid_dma_tx_status(struct 
> > dma_chan *chan,
> > dma_cookie_t cookie,
> > struct dma_tx_state *txstate)
> >  {
> > +   struct intel_mid_dma_chan *midc = to_intel_mid_dma_chan(chan);
> > enum dma_status ret;
> >  
> > ret = dma_cookie_status(chan, cookie, txstate);
> 
> Ditto (my patches don't introduce new this new midc, nor do they remove
> that line.)
> 
> > diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
> > index 145eda2..2c4476c 100644
> > --- a/drivers/dma/ioat/dma_v3.c
> > +++ b/drivers/dma/ioat/dma_v3.c
> > @@ -61,6 +61,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include "../dmaengine.h"
> >  #include "registers.h"
> >  #include "hw.h"
> >  #include "dma.h"
> > diff --git a/drivers/dma/iop-adma.c b/drivers/dma/iop-adma.c
> > index 1f3a703..4499f88 100644
> > --- a/drivers/dma/iop-adma.c
> > +++ b/drivers/dma/iop-adma.c
> > @@ -894,6 +894,7 @@ static enum dma_status iop_adma_status(struct dma_chan 
> > *chan,
> > struct dma_tx_state *txstate)
> >  {
> > struct iop_adma_chan *iop_chan = to_iop_adma_chan(chan);
> > +   int ret;
> 
> This was "enum dma_status ret;" before I accidentally removed it.  It
> probably should be again, rather than an int.
> 
> >  
> > ret = dma_cookie_status(chan, cooki

Re: [PATCH v2 0/9] DMA engine cookie handling cleanups

2012-03-19 Thread Vinod Koul
On Tue, 2012-03-13 at 20:08 +0530, Vinod Koul wrote:
> On Tue, 2012-03-13 at 12:31 +, Russell King - ARM Linux wrote:
> > On Tue, Mar 13, 2012 at 02:10:36PM +0530, Vinod Koul wrote:
> > > Please see if the below patch is the right fix for build failures in
> > > addition to one suggested by Jassi.
> > 
> > I'm not sure that Jassi's solution is correct - and I'm wondering whether
> > any of the DMA engine drivers do the right thing when transfers are
> > terminated.  Is it right for the DMA status function to return IN_PROGRESS
> > for a previously submitted cookie which has been terminated?
> > 
> > I can see two answers to that, both equally valid:
> > 
> > 1. It allows you to find out exactly where the DMA engine got to before
> >the transfer was terminated, and therefore recover from the termination
> >if you wish to.
> > 
> > 2. Returning in-progress when a cookie will never be completed is
> >misleading, and could be misinterpreted by users of the tx_status
> >function, especially if they are waiting for a particular transaction
> >to complete.
> > 
> > Maybe we need to introduce a DMA_TERMINATED status?
> I would agree with you that DMA_TERMINATED seems to be correct option.
> IN_PROGRESS would certainly confuse... 
> I will drop Jassi's fix from this branch. Care to send the patch? 

Even after adding such a state in dmaengine for DMA_TERMINATED, it
doesn't make much sense. In TERMINATE_ALL we do not update the
chan->complete value. So after client has terminated the channel it can
easily test to see if cookie is completed (before terminate will return
SUCCESS) or aborted (will return DMA_IN_PROGRESS)
So at present I am leaning on 1 :)

If I don't hear any opposition I will merge this (with Jassi's fix) and
then send pull to Linus

-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v5 1/2] dmaengine: Add context parameter to prep_dma_sg and prep_interleaved_dma

2012-03-26 Thread Vinod Koul
On Thu, 2012-03-22 at 19:52 +0530, Ravi Kumar V wrote:
> Add new context parameter to DMA SG and Interleaveid mode for passing
Typo  
> per transfer specific private data, using this it enables the
> dma devices which needs to pass the parameters which changes per
> each transfer
> 
> Signed-off-by: Ravi Kumar V 
> ---
>  drivers/dma/fsldma.c|2 +-
>  drivers/dma/sirf-dma.c  |2 +-
>  drivers/dma/ste_dma40.c |2 +-
>  drivers/misc/carma/carma-fpga-program.c |2 +-
>  drivers/misc/carma/carma-fpga.c |7 +++
>  include/linux/dmaengine.h   |   22 --
>  6 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index b98070c..f9f77db 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -645,7 +645,7 @@ fail:
>  static struct dma_async_tx_descriptor *fsl_dma_prep_sg(struct dma_chan 
> *dchan,
>   struct scatterlist *dst_sg, unsigned int dst_nents,
>   struct scatterlist *src_sg, unsigned int src_nents,
> - unsigned long flags)
> + unsigned long flags, void *context)
>  {
>   struct fsl_desc_sw *first = NULL, *prev = NULL, *new = NULL;
>   struct fsldma_chan *chan = to_fsl_chan(dchan);
> diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c
> index 2333810..ff4d344 100644
> --- a/drivers/dma/sirf-dma.c
> +++ b/drivers/dma/sirf-dma.c
> @@ -428,7 +428,7 @@ sirfsoc_dma_tx_status(struct dma_chan *chan, dma_cookie_t 
> cookie,
>  
>  static struct dma_async_tx_descriptor *sirfsoc_dma_prep_interleaved(
>   struct dma_chan *chan, struct dma_interleaved_template *xt,
> - unsigned long flags)
> + unsigned long flags, void *context)
>  {
>   struct sirfsoc_dma *sdma = dma_chan_to_sirfsoc_dma(chan);
>   struct sirfsoc_dma_chan *schan = dma_chan_to_sirfsoc_dma_chan(chan);
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index cc5ecbc..2f58ba9 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -2287,7 +2287,7 @@ static struct dma_async_tx_descriptor *
>  d40_prep_memcpy_sg(struct dma_chan *chan,
>  struct scatterlist *dst_sg, unsigned int dst_nents,
>  struct scatterlist *src_sg, unsigned int src_nents,
> -unsigned long dma_flags)
> +unsigned long dma_flags, void *context)
>  {
>   if (dst_nents != src_nents)
>   return NULL;
> diff --git a/drivers/misc/carma/carma-fpga-program.c 
> b/drivers/misc/carma/carma-fpga-program.c
> index a2d25e4..3739a12 100644
> --- a/drivers/misc/carma/carma-fpga-program.c
> +++ b/drivers/misc/carma/carma-fpga-program.c
> @@ -530,7 +530,7 @@ static noinline int fpga_program_dma(struct fpga_dev 
> *priv)
>   }
>  
>   /* setup and submit the DMA transaction */
> - tx = chan->device->device_prep_dma_sg(chan,
> + tx = chan->device->dmaengine_prep_dma_sg(chan,
> table.sgl, num_pages,
> vb->sglist, vb->sglen, 0);
No the idea is that clients will not know anything about additionlay
parameter hence avoiding abuse. You need to reread the patches sent by
Alexandre.

You need to 
1) add wrappers over interleaved api which dont expose this additional
parameter
2) move existing users to use these wrappers
3) add a new API which has your additional argument (not an opaque
object) and this calls .device_xx callback with additional arg.
4. Above can be under conditional of your specific subsystem where these
parameters are valid.

>   if (!tx) {
> diff --git a/drivers/misc/carma/carma-fpga.c b/drivers/misc/carma/carma-fpga.c
> index 14e974b2..be0baf6 100644
> --- a/drivers/misc/carma/carma-fpga.c
> +++ b/drivers/misc/carma/carma-fpga.c
> @@ -638,10 +638,9 @@ static int data_submit_dma(struct fpga_device *priv, 
> struct data_buf *buf)
>*/
>  
>   /* setup the scatterlist to scatterlist transfer */
> - tx = chan->device->device_prep_dma_sg(chan,
> -   dst_sg, dst_nents,
> -   src_sg, src_nents,
> -   0);
> + tx = dmaengine_prep_dma_sg(chan, dst_sg, dst_nents,
> +  src_sg, src_nents,
> +  0);
>   if (!tx) {
>   dev_err(priv->dev, "unable to prep scatterlist DMA\n");
>   return -ENOMEM;
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 679b349..68a57da 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -570,7 +570,7 @@ struct dma_device {
>   struct dma_chan *chan,
>   struct scatterlist *dst_sg, unsigned int dst_nents,
>   struct scatterlist *src_sg, unsigned int src_nents,
> - 

Re: [PATCH v5 1/2] dmaengine: Add context parameter to prep_dma_sg and prep_interleaved_dma

2012-03-26 Thread Vinod Koul
On Mon, 2012-03-26 at 12:25 +0530, Trilok Soni wrote:
> >
> > You need to
> > 1) add wrappers over interleaved api which dont expose this
> additional
> > parameter
> > 2) move existing users to use these wrappers
> 
> Above two steps would be exactly same as 
> https://lkml.org/lkml/2012/3/8/401 - [PATCH 1/2 V2]
> dmaengine/dma_slave: 
> introduce inline wrappers, right?
> 
> > 3) add a new API which has your additional argument (not an opaque
> > object) and this calls .device_xx callback with additional arg.
> > 4. Above can be under conditional of your specific subsystem where
> these
> > parameters are valid.
> 
> Now, this would be different from what Alexandre had submitted, since
> he 
> had added "void *context" parameter directly to existing callbacks 
> dma_slave_sg and and cyclic under struct dma_device.
In that case the wrappers existed, so he modified them and ensured all
clients use these and not the .device callbacks

So you need to create the new wrappers without any context parameter and
ensure all clients use these.
> 
> And you prefer that we add new callbacks under "struct dma_device"
> for 
> our specific requirement with new name and with that extra non-opaque 
> object. After that add wrappers for these two new callbacks say
> dmaengine_prep_dma_sg_ext and dmagengine_prep_interleaved_dma_ext
> (same
> name goes for callback).
> 
> 
-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH V2 1/2] rapidio: add DMA engine support for RIO data transfers

2012-03-27 Thread Vinod Koul
On Mon, 2012-03-26 at 16:45 -0400, Alexandre Bounine wrote:
> Adds DMA Engine framework support into RapidIO subsystem.
> Uses DMA Engine DMA_SLAVE interface to generate data transfers to/from
> remote RapidIO target devices.
> Introduces RapidIO-specific wrapper for prep_slave_sg() interface
> with an extra parameter to pass target specific information.
> Uses scatterlist to describe local data buffer. Address flat data buffer
> on a remote side.
> 
> Signed-off-by: Alexandre Bounine 
Acked by: Vinod Koul 
> ---
> 
> This patch is applicable to linux-next-20120326 and after.
> 
> v2: Uses updated DMA engine prep_slave_sg() interface.
> See https://lkml.org/lkml/2012/3/8/373 for more details.
> 
>  drivers/rapidio/Kconfig   |   14 
>  drivers/rapidio/rio.c |   81 
> +
>  include/linux/dmaengine.h |   12 +++
>  include/linux/rio.h   |   47 ++
>  include/linux/rio_drv.h   |9 +
>  5 files changed, 163 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/rapidio/Kconfig b/drivers/rapidio/Kconfig
> index bc87192..6194d35 100644
> --- a/drivers/rapidio/Kconfig
> +++ b/drivers/rapidio/Kconfig
> @@ -22,6 +22,20 @@ config RAPIDIO_ENABLE_RX_TX_PORTS
> ports for Input/Output direction to allow other traffic
> than Maintenance transfers.
>  
> +config RAPIDIO_DMA_ENGINE
> + bool "DMA Engine support for RapidIO"
> + depends on RAPIDIO
> + select DMADEVICES
> + select DMA_ENGINE
> + help
> +   Say Y here if you want to use DMA Engine frameork for RapidIO data
> +   transfers to/from target RIO devices. RapidIO uses NREAD and
> +   NWRITE (NWRITE_R, SWRITE) requests to transfer data between local
> +   memory and memory on remote target device. You need a DMA controller
> +   capable to perform data transfers to/from RapidIO.
> +
> +   If you are unsure about this, say Y here.
> +
>  config RAPIDIO_DEBUG
>   bool "RapidIO subsystem debug messages"
>   depends on RAPIDIO
> diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
> index 86c9a09..c40665a 100644
> --- a/drivers/rapidio/rio.c
> +++ b/drivers/rapidio/rio.c
> @@ -1121,6 +1121,87 @@ int rio_std_route_clr_table(struct rio_mport *mport, 
> u16 destid, u8 hopcount,
>   return 0;
>  }
>  
> +#ifdef CONFIG_RAPIDIO_DMA_ENGINE
> +
> +static bool rio_chan_filter(struct dma_chan *chan, void *arg)
> +{
> + struct rio_dev *rdev = arg;
> +
> + /* Check that DMA device belongs to the right MPORT */
> + return (rdev->net->hport ==
> + container_of(chan->device, struct rio_mport, dma));
> +}
> +
> +/**
> + * rio_request_dma - request RapidIO capable DMA channel that supports
> + *   specified target RapidIO device.
> + * @rdev: RIO device control structure
> + *
> + * Returns pointer to allocated DMA channel or NULL if failed.
> + */
> +struct dma_chan *rio_request_dma(struct rio_dev *rdev)
> +{
> + dma_cap_mask_t mask;
> + struct dma_chan *dchan;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_SLAVE, mask);
> + dchan = dma_request_channel(mask, rio_chan_filter, rdev);
> +
> + return dchan;
> +}
> +EXPORT_SYMBOL_GPL(rio_request_dma);
> +
> +/**
> + * rio_release_dma - release specified DMA channel
> + * @dchan: DMA channel to release
> + */
> +void rio_release_dma(struct dma_chan *dchan)
> +{
> + dma_release_channel(dchan);
> +}
> +EXPORT_SYMBOL_GPL(rio_release_dma);
> +
> +/**
> + * rio_dma_prep_slave_sg - RapidIO specific wrapper
> + *   for device_prep_slave_sg callback defined by DMAENGINE.
> + * @rdev: RIO device control structure
> + * @dchan: DMA channel to configure
> + * @data: RIO specific data descriptor
> + * @direction: DMA data transfer direction (TO or FROM the device)
> + * @flags: dmaengine defined flags
> + *
> + * Initializes RapidIO capable DMA channel for the specified data transfer.
> + * Uses DMA channel private extension to pass information related to remote
> + * target RIO device.
> + * Returns pointer to DMA transaction descriptor or NULL if failed.
> + */
> +struct dma_async_tx_descriptor *rio_dma_prep_slave_sg(struct rio_dev *rdev,
> + struct dma_chan *dchan, struct rio_dma_data *data,
> + enum dma_transfer_direction direction, unsigned long flags)
> +{
> + struct dma_async_tx_descriptor *txd = NULL;
> + struct rio_dma_ext rio_ext;
> +
> + if (dchan->device->device_prep_slave_sg == NULL) {
> + pr_err("%s: prep_rio_sg == NULL\n", __func__);
> + return NUL

Re: [PATCH 5/9] dmaengine: provide a common function for completing a dma descriptor

2012-04-23 Thread Vinod Koul
On Mon, 2012-04-23 at 10:50 +0100, Russell King - ARM Linux wrote:
> On Mon, Apr 23, 2012 at 06:40:06PM +0900, Boojin Kim wrote:
> > I met a problem on DMA cyclic mode (DMA_CYCLIC) for sound playback.
> > Kernel BUG occurs during DMA transfer with DMA cyclic mode.
> > This patch makes the cookies into zero. But, cookies should be kept
> > during cyclic mode because cyclic mode re-uses the cookies.
> 
> The protection is there to prevent cookies being accidentally re-used.
> If you're running a cyclic transfer, even then you shouldn't be completing
> the same cookie time and time again - I think Vinod also concurs with this.
Right :)
I recently committed patch for imx-dma which doesn't mark the cyclic
descriptor as complete. Descriptor represents a transaction and makes no
sense to complete t if the transaction is still continuing.
> 
> I think our preference is for cyclic transfers to entire remain uncompleted,
> or to get a new cookie each time they allegedly "complete".
No it is not complete. Cyclic never completes, it aborts when user
wants. The "notification" interrupt is for updating the
counters/notifying (timestamp/periods elapsed in sound), and shouldn't
be used for anything else

-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC v9 2/6] dma: mpc512x: add support for peripheral transfers

2014-03-19 Thread Vinod Koul
On Wed, Mar 19, 2014 at 05:26:47PM +0400, Alexander Popov wrote:
> Hello Andy
> 
> 2014-03-14 13:47 GMT+04:00 Andy Shevchenko 
> :
> > On Wed, 2014-03-12 at 15:47 +0400, Alexander Popov wrote:
> >> + case DMA_SLAVE_CONFIG:
> >> + /* Constraints:
> >> +  *  - only transfers between a peripheral device and
> >> +  * memory are supported;
> >> +  *  - minimal transfer chunk is 4 bytes and consequently
> >> +  * source and destination addresses must be 4-byte 
> >> aligned
> >> +  * and transfer size must be aligned on (4 * maxburst)
> >> +  * boundary;
> >> +  *  - during the transfer RAM address is being incremented by
> >> +  * the size of minimal transfer chunk;
> >> +  *  - peripheral port's address is constant during the 
> >> transfer.
> >> +  */
> >> +
> >> + cfg = (void *)arg;
> >> +
> >> + if (!is_slave_direction(cfg->direction))
> >> + return -EINVAL;
> >
> > As far as I understand the intention you have not to use direction field
> > in the dma_slave_config. It will be removed once.
> >
> >> +
> >> + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES &&
> >> + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES)
> >> + return -EINVAL;
> >> +
> >> + spin_lock_irqsave(&mchan->lock, flags);
> >> +
> >> + if (cfg->direction == DMA_DEV_TO_MEM) {
> >> + mchan->per_paddr = cfg->src_addr;
> >> + mchan->tcd_nunits = cfg->src_maxburst;
> >> + } else {
> >> + mchan->per_paddr = cfg->dst_addr;
> >> + mchan->tcd_nunits = cfg->dst_maxburst;
> >> + }
> >
> > Ditto.
> 
> Excuse me, I don't understand this point.
> I have to use cfg->direction because in case of DMA_DEV_TO_MEM
> I use cfg->SRC_addr and cfg->SRC_maxburst and in case of
> DMA_MEM_TO_DEV I use cfg->DST_addr and cfg->DST_maxburst.
> Is it correct?
You store the complete config for both source and destination.
Then based on the descriptor direction you can retrive the values from channel
context and program

This way you _dont_ need to fix the direction and can use it both ways!

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 6/7] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave

2014-03-26 Thread Vinod Koul
On Thu, 2014-01-16 at 13:47 +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> The usage of spin_lock_irqsave() is a stronger locking mechanism than is
> required throughout the driver. The minimum locking required should be used
> instead. Interrupts will be turned off and context will be saved, it is
> unnecessary to use irqsave.
> 
> This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All
> manipulation of protected fields is done using tasklet context or weaker, 
> which
> makes spin_lock_bh() the correct choice.
> 
> Signed-off-by: Hongbo Zhang 
> Signed-off-by: Qiang Liu 
> ---
>  drivers/dma/fsldma.c |   25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index bbace54..437794e 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -396,10 +396,9 @@ static dma_cookie_t fsl_dma_tx_submit(struct 
> dma_async_tx_descriptor *tx)
>   struct fsldma_chan *chan = to_fsl_chan(tx->chan);
>   struct fsl_desc_sw *desc = tx_to_fsl_desc(tx);
>   struct fsl_desc_sw *child;
> - unsigned long flags;
>   dma_cookie_t cookie = -EINVAL;
>  
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>  
>   /*
>* assign cookies to all of the software descriptors
> @@ -412,7 +411,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct 
> dma_async_tx_descriptor *tx)
>   /* put this transaction onto the tail of the pending queue */
>   append_ld_queue(chan, desc);
>  
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>  
>   return cookie;
>  }
> @@ -731,15 +730,14 @@ static void fsldma_free_desc_list_reverse(struct 
> fsldma_chan *chan,
>  static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
>  {
>   struct fsldma_chan *chan = to_fsl_chan(dchan);
> - unsigned long flags;
>  
>   chan_dbg(chan, "free all channel resources\n");
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>   fsldma_cleanup_descriptors(chan);
>   fsldma_free_desc_list(chan, &chan->ld_pending);
>   fsldma_free_desc_list(chan, &chan->ld_running);
>   fsldma_free_desc_list(chan, &chan->ld_completed);
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>  
>   dma_pool_destroy(chan->desc_pool);
>   chan->desc_pool = NULL;
> @@ -958,7 +956,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  {
>   struct dma_slave_config *config;
>   struct fsldma_chan *chan;
> - unsigned long flags;
>   int size;
>  
>   if (!dchan)
> @@ -968,7 +965,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  
>   switch (cmd) {
>   case DMA_TERMINATE_ALL:
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>  
>   /* Halt the DMA engine */
>   dma_halt(chan);
> @@ -979,7 +976,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>   fsldma_free_desc_list(chan, &chan->ld_completed);
>   chan->idle = true;
>  
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>   return 0;
>  
>   case DMA_SLAVE_CONFIG:
> @@ -1021,11 +1018,10 @@ static int fsl_dma_device_control(struct dma_chan 
> *dchan,
>  static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
>  {
>   struct fsldma_chan *chan = to_fsl_chan(dchan);
> - unsigned long flags;
>  
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>   fsl_chan_xfer_ld_queue(chan);
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>  }
>  
>  /**
> @@ -1124,11 +1120,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void 
> *data)
>  static void dma_do_tasklet(unsigned long data)
>  {
>   struct fsldma_chan *chan = (struct fsldma_chan *)data;
> - unsigned long flags;
>  
>   chan_dbg(chan, "tasklet entry\n");
>  
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
okay here is the problem :(

You moved to _bh variant. So if you grab the lock in rest of the code
and irq gets triggered then here we will be spinning to grab the 

Re: [PATCH 6/7] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave

2014-03-29 Thread Vinod Koul
On Fri, Mar 28, 2014 at 02:33:37PM +0800, Hongbo Zhang wrote:
> 
> On 03/26/2014 03:01 PM, Vinod Koul wrote:
> >On Thu, 2014-01-16 at 13:47 +0800, hongbo.zh...@freescale.com wrote:
> >>From: Hongbo Zhang 
> >>
> >>The usage of spin_lock_irqsave() is a stronger locking mechanism than is
> >>required throughout the driver. The minimum locking required should be used
> >>instead. Interrupts will be turned off and context will be saved, it is
> >>unnecessary to use irqsave.
> >>
> >>This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). 
> >>All
> >>manipulation of protected fields is done using tasklet context or weaker, 
> >>which
> >>makes spin_lock_bh() the correct choice.
> >>


> >>  /**
> >>@@ -1124,11 +1120,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void 
> >>*data)
> >>  static void dma_do_tasklet(unsigned long data)
> >>  {
> >>struct fsldma_chan *chan = (struct fsldma_chan *)data;
> >>-   unsigned long flags;
> >>chan_dbg(chan, "tasklet entry\n");
> >>-   spin_lock_irqsave(&chan->desc_lock, flags);
> >>+   spin_lock_bh(&chan->desc_lock);
> >okay here is the problem :(
> >
> >You moved to _bh variant. So if you grab the lock in rest of the code
> >and irq gets triggered then here we will be spinning to grab the lock.
> >So effectively you made right locking solution into deadlock situation!
> 
> If the rest code grabs lock by spin_lock_bh(), and if irq raised,
> the tasklet could not be executed because it has been disabled by
> the _bh variant function.
yes if you are accessing resources only in tasklet and rest of the code, then
_bh variant works well. The problem here is usage in irq handler

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 6/7] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave

2014-04-02 Thread Vinod Koul
On Mon, Mar 31, 2014 at 12:08:55PM +0800, Hongbo Zhang wrote:
> 
> On 03/29/2014 09:45 PM, Vinod Koul wrote:
> >On Fri, Mar 28, 2014 at 02:33:37PM +0800, Hongbo Zhang wrote:
> >>On 03/26/2014 03:01 PM, Vinod Koul wrote:
> >>>On Thu, 2014-01-16 at 13:47 +0800, hongbo.zh...@freescale.com wrote:
> >>>>From: Hongbo Zhang 
> >>>>
> >>>>The usage of spin_lock_irqsave() is a stronger locking mechanism than is
> >>>>required throughout the driver. The minimum locking required should be 
> >>>>used
> >>>>instead. Interrupts will be turned off and context will be saved, it is
> >>>>unnecessary to use irqsave.
> >>>>
> >>>>This patch changes all instances of spin_lock_irqsave() to 
> >>>>spin_lock_bh(). All
> >>>>manipulation of protected fields is done using tasklet context or weaker, 
> >>>>which
> >>>>makes spin_lock_bh() the correct choice.
> >>>>
> >
> >>>>  /**
> >>>>@@ -1124,11 +1120,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void 
> >>>>*data)
> >>>>  static void dma_do_tasklet(unsigned long data)
> >>>>  {
> >>>>  struct fsldma_chan *chan = (struct fsldma_chan *)data;
> >>>>- unsigned long flags;
> >>>>  chan_dbg(chan, "tasklet entry\n");
> >>>>- spin_lock_irqsave(&chan->desc_lock, flags);
> >>>>+ spin_lock_bh(&chan->desc_lock);
> >>>okay here is the problem :(
> >>>
> >>>You moved to _bh variant. So if you grab the lock in rest of the code
> >>>and irq gets triggered then here we will be spinning to grab the lock.
> >>>So effectively you made right locking solution into deadlock situation!
> >>If the rest code grabs lock by spin_lock_bh(), and if irq raised,
> >>the tasklet could not be executed because it has been disabled by
> >>the _bh variant function.
> >yes if you are accessing resources only in tasklet and rest of the code, then
> >_bh variant works well. The problem here is usage in irq handler
> >
> 
> The name dma_do_tasklet may mislead, it is tasklet handler, not irq
> handler, not a trigger to load tasklet.
> the irq handler is fsldma_chan_irq, and I don't use lock in it.
sorry my bad, i misread this as code in fsldma_chan_irq() insteadof
dma_do_tasklet(). In that case patch is doing the right thing.

-- 
~Vinod
> 
> If it is the problem, I would like to change dma_do_tasklet to
> dma_tasklet to eliminate misleading.
> 
> 
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 1/8] DMA: Freescale: remove the unnecessary FSL_DMA_LD_DEBUG

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:44PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> Some codes are calling chan_dbg with FSL_DMA_LD_DEBUG surrounded, it is really
> unnecessary to use such a macro because chan_dbg is a wrapper of dev_dbg, we 
> do
> have corresponding DEBUG macro to switch on/off dev_dbg, and most of the other
> codes are also calling chan_dbg directly without using FSL_DMA_LD_DEBUG.
>
Applied, thanks

-- 
~Vinod
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 3/8] DMA: Freescale: remove attribute DMA_INTERRUPT of dmaengine

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:46PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> Delete attribute DMA_INTERRUPT because fsldma doesn't support this function,
> exception will be thrown if talitos is used to offload xor at the same time.
> 
Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 4/8] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:47PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> There are several places where descriptors are freed using identical code.
> This patch puts this code into a function to reduce code duplication.
> 
Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 2/8] DMA: Freescale: unify register access methods

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:45PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> Methods of accessing DMA controller registers are inconsistent, some registers
> are accessed by DMA_IN/OUT directly, while others are accessed by functions
> get/set_* which are wrappers of DMA_IN/OUT, and even for the BCR register, it
> is read by get_bcr but written by DMA_OUT.
> This patch unifies the inconsistent methods, all registers are accessed by
> get/set_* now.
> 
Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 6/8] DMA: Freescale: change descriptor release process for supporting async_tx

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:49PM +0800, hongbo.zh...@freescale.com wrote:

This need review from Dan ...

-- 
~Vinod
> From: Hongbo Zhang 
> 
> Fix the potential risk when enable config NET_DMA and ASYNC_TX. Async_tx is
> lack of support in current release process of dma descriptor, all descriptors
> will be released whatever is acked or no-acked by async_tx, so there is a
> potential race condition when dma engine is uesd by others clients (e.g. when
> enable NET_DMA to offload TCP).
> 
> In our case, a race condition which is raised when use both of talitos and
> dmaengine to offload xor is because napi scheduler will sync all pending
> requests in dma channels, it affects the process of raid operations due to
> ack_tx is not checked in fsl dma. The no-acked descriptor is freed which is
> submitted just now, as a dependent tx, this freed descriptor trigger
> BUG_ON(async_tx_test_ack(depend_tx)) in async_tx_submit().
> 
> TASK = ee1a94a0[1390] 'md0_raid5' THREAD: ecf4 CPU: 0
> GPR00: 0001 ecf41ca0 ee44/921a94a0 003f 0001 c00593e4  
> 0001
> GPR08:  a7a7a7a7 0001 045/92002 42028042 100a38d4 ed576d98 
> 
> GPR16: ed5a11b0  2b162000 0200 046/92000 2d555000 ed3015e8 
> c15a7aa0
> GPR24:  c155fc40  ecb63220 ecf41d28 e47/92f640bb0 ef640c30 
> ecf41ca0
> NIP [c02b048c] async_tx_submit+0x6c/0x2b4
> LR [c02b068c] async_tx_submit+0x26c/0x2b4
> Call Trace:
> [ecf41ca0] [c02b068c] async_tx_submit+0x26c/0x2b448/92 (unreliable)
> [ecf41cd0] [c02b0a4c] async_memcpy+0x240/0x25c
> [ecf41d20] [c0421064] async_copy_data+0xa0/0x17c
> [ecf41d70] [c0421cf4] __raid_run_ops+0x874/0xe10
> [ecf41df0] [c0426ee4] handle_stripe+0x820/0x25e8
> [ecf41e90] [c0429080] raid5d+0x3d4/0x5b4
> [ecf41f40] [c04329b8] md_thread+0x138/0x16c
> [ecf41f90] [c008277c] kthread+0x8c/0x90
> [ecf41ff0] [c0011630] kernel_thread+0x4c/0x68
> 
> Another modification in this patch is the change of completed descriptors,
> there is a potential risk which caused by exception interrupt, all descriptors
> in ld_running list are seemed completed when an interrupt raised, it works 
> fine
> under normal condition, but if there is an exception occured, it cannot work 
> as
> our excepted. Hardware should not be depend on s/w list, the right way is to
> read current descriptor address register to find the last completed 
> descriptor.
> If an interrupt is raised by an error, all descriptors in ld_running should 
> not
> be seemed finished, or these unfinished descriptors in ld_running will be
> released wrongly.
> 
> A simple way to reproduce:
> Enable dmatest first, then insert some bad descriptors which can trigger
> Programming Error interrupts before the good descriptors. Last, the good
> descriptors will be freed before they are processsed because of the exception
> intrerrupt.
> 
> Note: the bad descriptors are only for simulating an exception interrupt.  
> This
> case can illustrate the potential risk in current fsl-dma very well.
> 
> Signed-off-by: Hongbo Zhang 
> Signed-off-by: Qiang Liu 
> Signed-off-by: Ira W. Snyder 
> ---
>  drivers/dma/fsldma.c |  197 
> --
>  drivers/dma/fsldma.h |   17 -
>  2 files changed, 159 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index e0fec68..374ca97 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -459,6 +459,88 @@ static struct fsl_desc_sw 
> *fsl_dma_alloc_descriptor(struct fsldma_chan *chan)
>  }
>  
>  /**
> + * fsldma_clean_completed_descriptor - free all descriptors which
> + * has been completed and acked
> + * @chan: Freescale DMA channel
> + *
> + * This function is used on all completed and acked descriptors.
> + * All descriptors should only be freed in this function.
> + */
> +static void fsldma_clean_completed_descriptor(struct fsldma_chan *chan)
> +{
> + struct fsl_desc_sw *desc, *_desc;
> +
> + /* Run the callback for each descriptor, in order */
> + list_for_each_entry_safe(desc, _desc, &chan->ld_completed, node)
> + if (async_tx_test_ack(&desc->async_tx))
> + fsl_dma_free_descriptor(chan, desc);
> +}
> +
> +/**
> + * fsldma_run_tx_complete_actions - cleanup a single link descriptor
> + * @chan: Freescale DMA channel
> + * @desc: descriptor to cleanup and free
> + * @cookie: Freescale DMA transaction identifier
> + *
> + * This function is used on a descriptor which has been executed by the DMA
> + * controller. It will run any callbacks, submit any dependencies.
> + */
> +static dma_cookie_t fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
> + struct fsl_desc_sw *desc, dma_cookie_t cookie)
> +{
> + struct dma_async_tx_descriptor *txd = &desc->async_tx;
> + dma_cookie_t ret = cookie;
> +
> + BUG_ON(txd->cookie < 0);
> +
> + if (txd->cookie > 0) {
> + ret = txd->cookie;
> +
> + /* Run the link descr

Re: [PATCH v4 7/8] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:50PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> The usage of spin_lock_irqsave() is a stronger locking mechanism than is
> required throughout the driver. The minimum locking required should be used
> instead. Interrupts will be turned off and context will be saved, it is
> unnecessary to use irqsave.
> 
> This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All
> manipulation of protected fields is done using tasklet context or weaker, 
> which
> makes spin_lock_bh() the correct choice.
>

This doesnt apply, perhpas due to depends on 6/8

-- 
~Vinod
 
> Signed-off-by: Hongbo Zhang 
> Signed-off-by: Qiang Liu 
> ---
>  drivers/dma/fsldma.c |   25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 374ca97..6e1c9b3 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -396,10 +396,9 @@ static dma_cookie_t fsl_dma_tx_submit(struct 
> dma_async_tx_descriptor *tx)
>   struct fsldma_chan *chan = to_fsl_chan(tx->chan);
>   struct fsl_desc_sw *desc = tx_to_fsl_desc(tx);
>   struct fsl_desc_sw *child;
> - unsigned long flags;
>   dma_cookie_t cookie = -EINVAL;
>  
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>  
>   /*
>* assign cookies to all of the software descriptors
> @@ -412,7 +411,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct 
> dma_async_tx_descriptor *tx)
>   /* put this transaction onto the tail of the pending queue */
>   append_ld_queue(chan, desc);
>  
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>  
>   return cookie;
>  }
> @@ -726,15 +725,14 @@ static void fsldma_free_desc_list_reverse(struct 
> fsldma_chan *chan,
>  static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
>  {
>   struct fsldma_chan *chan = to_fsl_chan(dchan);
> - unsigned long flags;
>  
>   chan_dbg(chan, "free all channel resources\n");
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>   fsldma_cleanup_descriptors(chan);
>   fsldma_free_desc_list(chan, &chan->ld_pending);
>   fsldma_free_desc_list(chan, &chan->ld_running);
>   fsldma_free_desc_list(chan, &chan->ld_completed);
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>  
>   dma_pool_destroy(chan->desc_pool);
>   chan->desc_pool = NULL;
> @@ -953,7 +951,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  {
>   struct dma_slave_config *config;
>   struct fsldma_chan *chan;
> - unsigned long flags;
>   int size;
>  
>   if (!dchan)
> @@ -963,7 +960,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  
>   switch (cmd) {
>   case DMA_TERMINATE_ALL:
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>  
>   /* Halt the DMA engine */
>   dma_halt(chan);
> @@ -974,7 +971,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>   fsldma_free_desc_list(chan, &chan->ld_completed);
>   chan->idle = true;
>  
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>   return 0;
>  
>   case DMA_SLAVE_CONFIG:
> @@ -1016,11 +1013,10 @@ static int fsl_dma_device_control(struct dma_chan 
> *dchan,
>  static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
>  {
>   struct fsldma_chan *chan = to_fsl_chan(dchan);
> - unsigned long flags;
>  
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>   fsl_chan_xfer_ld_queue(chan);
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>  }
>  
>  /**
> @@ -1119,11 +1115,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void 
> *data)
>  static void dma_do_tasklet(unsigned long data)
>  {
>   struct fsldma_chan *chan = (struct fsldma_chan *)data;
> - unsigned long flags;
>  
>   chan_dbg(chan, "tasklet entry\n");
>  
> - spin_lock_irqsave(&chan->desc_lock, flags);
> + spin_lock_bh(&chan->desc_lock);
>  
>   /* the hardware is now idle and ready for more */
>   chan->idle = true;
> @@ -1131,7 +1126,7 @@ static void dma_do_tasklet(unsigned long data)
>   /* Run all cleanup for descriptors which have been completed */
>   fsldma_cleanup_descriptors(chan);
>  
> - spin_unlock_irqrestore(&chan->desc_lock, flags);
> + spin_unlock_bh(&chan->desc_lock);
>  
>   chan_dbg(chan, "tasklet exit\n");
>  }
> -- 
> 1.7.9.5
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel

Re: [PATCH v4 5/8] DMA: Freescale: move functions to avoid forward declarations

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:48PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> These functions will be modified in the next patch in the series. By moving 
> the
> function in a patch separate from the changes, it will make review easier.
>
Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 8/8] DMA: Freescale: add suspend resume functions for DMA driver

2014-05-02 Thread Vinod Koul
On Fri, Apr 18, 2014 at 04:17:51PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> This patch adds suspend resume functions for Freescale DMA driver.
> .prepare callback is used to stop further descriptors from being added into 
> the
> pending queue, and also issue pending queues into execution if there is any.
> .suspend callback makes sure all the pending jobs are cleaned up and all the
> channels are idle, and save the mode registers.
> .resume callback re-initializes the channels by restore the mode registers.
> 

> +
> +static const struct dev_pm_ops fsldma_pm_ops = {
> + .prepare= fsldma_prepare,
> + .suspend= fsldma_suspend,
> + .resume = fsldma_resume,
> +};
I think this is not correct. We discussed this sometime back on list. The
DMAengine drivers should use late resume and early suspend to ensure they get
suspended after clients (who should use normal ones) and resume before them

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v12 3/7] dma: mpc512x: add support for peripheral transfers

2014-05-02 Thread Vinod Koul
On Wed, Apr 23, 2014 at 05:53:25PM +0400, Alexander Popov wrote:
> Introduce support for slave s/g transfer preparation and the associated
> device control callback in the MPC512x DMA controller driver, which adds
> support for data transfers between memory and peripheral I/O to the
> previously supported mem-to-mem transfers.
> 
> Signed-off-by: Alexander Popov 
> ---
>  drivers/dma/mpc512x_dma.c | 239 
> +-
>  1 file changed, 234 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 0b17f4d..ca2fe03 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -2,6 +2,7 @@
>   * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
>   * Copyright (C) Semihalf 2009
>   * Copyright (C) Ilya Yanok, Emcraft Systems 2010
> + * Copyright (C) Alexander Popov, Promcontroller 2014
>   *
>   * Written by Piotr Ziecik . Hardware description
>   * (defines, structures and comments) was taken from MPC5121 DMA driver
> @@ -29,8 +30,17 @@
>   */
>  
>  /*
> - * This is initial version of MPC5121 DMA driver. Only memory to memory
> - * transfers are supported (tested using dmatest module).
> + * MPC512x and MPC8308 DMA driver. It supports
> + * memory to memory data transfers (tested using dmatest module) and
> + * data transfers between memory and peripheral I/O memory
> + * by means of slave s/g with these limitations:
> + *  - chunked transfers (transfers with more than one part) are refused
> + * as long as proper support for scatter/gather is missing;
> + *  - transfers on MPC8308 always start from software as this SoC appears
> + * not to have external request lines for peripheral flow control;
> + *  - minimal memory <-> I/O memory transfer chunk is 4 bytes and 
> consequently
> + * source and destination addresses must be 4-byte aligned
> + * and transfer size must be aligned on (4 * maxburst) boundary;
>   */
>  
>  #include 
> @@ -189,6 +199,7 @@ struct mpc_dma_desc {
>   dma_addr_t  tcd_paddr;
>   int error;
>   struct list_headnode;
> + int will_access_peripheral;
>  };
>  
>  struct mpc_dma_chan {
> @@ -201,6 +212,12 @@ struct mpc_dma_chan {
>   struct mpc_dma_tcd  *tcd;
>   dma_addr_t  tcd_paddr;
>  
> + /* Settings for access to peripheral FIFO */
> + dma_addr_t  src_per_paddr;
> + u32 src_tcd_nunits;
> + dma_addr_t  dst_per_paddr;
> + u32 dst_tcd_nunits;
> +
>   /* Lock for this structure */
>   spinlock_t  lock;
>  };
> @@ -251,8 +268,23 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>   struct mpc_dma_desc *mdesc;
>   int cid = mchan->chan.chan_id;
>  
> - /* Move all queued descriptors to active list */
> - list_splice_tail_init(&mchan->queued, &mchan->active);
> + while (!list_empty(&mchan->queued)) {
> + mdesc = list_first_entry(&mchan->queued,
> + struct mpc_dma_desc, node);
> + /*
> +  * Grab either several mem-to-mem transfer descriptors
> +  * or one peripheral transfer descriptor,
> +  * don't mix mem-to-mem and peripheral transfer descriptors
> +  * within the same 'active' list.
> +  */
> + if (mdesc->will_access_peripheral) {
> + if (list_empty(&mchan->active))
> + list_move_tail(&mdesc->node, &mchan->active);
> + break;
> + } else {
> + list_move_tail(&mdesc->node, &mchan->active);
> + }
> + }
>  
>   /* Chain descriptors into one transaction */
>   list_for_each_entry(mdesc, &mchan->active, node) {
> @@ -278,7 +310,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>  
>   if (first != prev)
>   mdma->tcd[cid].e_sg = 1;
> - out_8(&mdma->regs->dmassrt, cid);
> +
> + if (mdma->is_mpc8308) {
> + /* MPC8308, no request lines, software initiated start */
> + out_8(&mdma->regs->dmassrt, cid);
> + } else if (first->will_access_peripheral) {
> + /* Peripherals involved, start by external request signal */
> + out_8(&mdma->regs->dmaserq, cid);
> + } else {
> + /* Memory to memory transfer, software initiated start */
> + out_8(&mdma->regs->dmassrt, cid);
> + }
>  }
>  
>  /* Handle interrupt on one half of DMA controller (32 channels) */
> @@ -596,6 +638,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t 
> dst, dma_addr_t src,
>   }
>  
>   mdesc->error = 0;
> + mdesc->will_access_peripheral = 0;
>   tcd = mdesc->tcd;
>  
>   /* Prepare Transf

Re: [PATCH RFC v12 5/7] dma: of: add common xlate function for matching by channel id

2014-05-02 Thread Vinod Koul
On Wed, Apr 23, 2014 at 05:53:27PM +0400, Alexander Popov wrote:
> This patch adds a new common OF dma xlate callback function which will match a
> channel by it's id. The binding expects one integer argument which it will 
> use to
> lookup the channel by the id.
> 
> Unlike of_dma_simple_xlate this function is able to handle a system with
> multiple DMA controllers. When registering the of dma provider with
> of_dma_controller_register a pointer to the dma_device struct which is
> associated with the dt node needs to passed as the data parameter.
> New function will use this pointer to match only channels which belong to the
> specified DMA controller.

This patch needs review by DT folks too

> 
> Signed-off-by: Alexander Popov 
> ---
>  drivers/dma/of-dma.c   | 35 +++
>  include/linux/of_dma.h |  4 
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> index e8fe9dc..d5fbeaa 100644
> --- a/drivers/dma/of-dma.c
> +++ b/drivers/dma/of-dma.c
> @@ -218,3 +218,38 @@ struct dma_chan *of_dma_simple_xlate(struct 
> of_phandle_args *dma_spec,
>   &dma_spec->args[0]);
>  }
>  EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
> +
> +/**
> + * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel 
> id
> + * @dma_spec:pointer to DMA specifier as found in the device tree
> + * @of_dma:  pointer to DMA controller data
> + *
> + * This function can be used as the of xlate callback for DMA driver which 
> wants
> + * to match the channel based on the channel id. When using this xlate 
> function
> + * the #dma-cells propety of the DMA controller dt node needs to be set to 1.
> + * The data parameter of of_dma_controller_register must be a pointer to the
> + * dma_device struct the function should match upon.
> + *
> + * Returns pointer to appropriate dma channel on success or NULL on error.
> + */
> +struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
> +  struct of_dma *ofdma)
> +{
> + struct dma_device *dev = ofdma->of_dma_data;
> + struct dma_chan *chan, *candidate = NULL;
> +
> + if (!dev || dma_spec->args_count != 1)
> + return NULL;
> +
> + list_for_each_entry(chan, &dev->channels, device_node)
> + if (chan->chan_id == dma_spec->args[0]) {
> + candidate = chan;
> + break;
> + }
> +
> + if (!candidate)
> + return NULL;
> +
> + return dma_get_slave_channel(candidate);
> +}
> +EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id);
> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> index ae36298..56bc026 100644
> --- a/include/linux/of_dma.h
> +++ b/include/linux/of_dma.h
> @@ -41,6 +41,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct 
> device_node *np,
>const char *name);
>  extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>   struct of_dma *ofdma);
> +extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args 
> *dma_spec,
> + struct of_dma *ofdma);
>  #else
>  static inline int of_dma_controller_register(struct device_node *np,
>   struct dma_chan *(*of_dma_xlate)
> @@ -66,6 +68,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct 
> of_phandle_args *dma_s
>   return NULL;
>  }
>  
> +#define of_dma_xlate_by_chan_id NULL
> +
>  #endif
>  
>  #endif /* __LINUX_OF_DMA_H */
> -- 
> 1.8.4.2
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v12 1/7] dma: mpc512x: reorder mpc8308 specific instructions

2014-05-02 Thread Vinod Koul
On Wed, Apr 23, 2014 at 05:53:23PM +0400, Alexander Popov wrote:
> Concentrate the specific code for MPC8308 in the 'if' branch
> and handle MPC512x in the 'else' branch.
> This modification only reorders instructions but doesn't change behaviour.
> 
> Signed-off-by: Alexander Popov 
> Acked-by: Anatolij Gustschin 

Applied, thanks

-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v12 2/7] dma: mpc512x: separate 'compatible' values for MPC512x and MPC8308

2014-05-02 Thread Vinod Koul
On Wed, Apr 23, 2014 at 05:53:24PM +0400, Alexander Popov wrote:
> MPC512x and MPC8308 have similar DMA controllers, but are independent SoCs.
> DMA controller driver should have separate 'compatible' values for these SoCs.
> 
> Signed-off-by: Alexander Popov 
Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v12 4/7] dma: mpc512x: fix freeing resources in mpc_dma_probe() and mpc_dma_remove()

2014-05-02 Thread Vinod Koul
On Wed, Apr 23, 2014 at 05:53:26PM +0400, Alexander Popov wrote:
> Fix mpc_dma_probe() error path and mpc_dma_remove(): manually free IRQs and
> dispose IRQ mappings before devm_* takes care of other resources.
> Moreover replace devm_request_irq() with request_irq() since there is no need
> to use it because the original code always frees IRQ manually with
> devm_free_irq(). Replace devm_free_irq() with free_irq() accordingly.
Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v12 3/7] dma: mpc512x: add support for peripheral transfers

2014-05-20 Thread Vinod Koul
On Thu, May 08, 2014 at 01:49:20PM +0400, Alexander Popov wrote:
> >> + case DMA_SLAVE_CONFIG:
> >> + /*
> >> +  * Constraints:
> >> +  *  - only transfers between a peripheral device and
> >> +  * memory are supported;
> >> +  *  - minimal transfer chunk is 4 bytes and consequently
> >> +  * source and destination addresses must be 4-byte 
> >> aligned
> >> +  * and transfer size must be aligned on (4 * maxburst)
> >> +  * boundary;
> >> +  *  - during the transfer RAM address is being incremented by
> >> +  * the size of minimal transfer chunk;
> >> +  *  - peripheral port's address is constant during the 
> >> transfer.
> >> +  */
> >> +
> >> + cfg = (void *)arg;
> >> +
> >> + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES ||
> >> + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES ||
> > and why this limtation, doesnt seem covered above?
> I created this limitation because FIFO registers of LPC and SDHC
> support _only_ 4-byte access.
> 
> I tried to cover this limitation in the statement "minimal transfer chunk
> is 4 bytes". Should I make it more explicit?
expose these as capablities and try to use these in your client driver. Already
we have audio drivers using those...

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 8/8] DMA: Freescale: add suspend resume functions for DMA driver

2014-05-20 Thread Vinod Koul
On Thu, May 08, 2014 at 05:52:37PM +0800, Hongbo Zhang wrote:
> 
> On 05/07/2014 04:31 PM, Shevchenko, Andriy wrote:
> >On Sun, 2014-05-04 at 18:22 +0800, Hongbo Zhang wrote:
> >>On 05/03/2014 12:46 AM, Vinod Koul wrote:
> >>>On Fri, Apr 18, 2014 at 04:17:51PM +0800, hongbo.zh...@freescale.com wrote:
> >>>>From: Hongbo Zhang 
> >>>>
> >>>>This patch adds suspend resume functions for Freescale DMA driver.
> >>>>.prepare callback is used to stop further descriptors from being added 
> >>>>into the
> >>>>pending queue, and also issue pending queues into execution if there is 
> >>>>any.
> >>>>.suspend callback makes sure all the pending jobs are cleaned up and all 
> >>>>the
> >>>>channels are idle, and save the mode registers.
> >>>>.resume callback re-initializes the channels by restore the mode 
> >>>>registers.
> >>>>
> >>>>+
> >>>>+static const struct dev_pm_ops fsldma_pm_ops = {
> >>>>+ .prepare= fsldma_prepare,
> >>>>+ .suspend= fsldma_suspend,
> >>>>+ .resume = fsldma_resume,
> >>>>+};
> >>>I think this is not correct. We discussed this sometime back on list. The
> >>>DMAengine drivers should use late resume and early suspend to ensure they 
> >>>get
> >>>suspended after clients (who should use normal ones) and resume before them
> >>>
> >>OK, will update it like this:
> >>use .suspend to take place of current .prepare
> >Could you remove this at all?
> >
> >Answering to your previous statements I could say that.
> >Device drivers (DMAc users) that don't implement .suspend callback are
> >on their own with troubles, you have not to care about them in the DMA
> >driver.
> 
> Thanks for pointing out this issue.
> Then how to handle the descriptors in the pending list if there is any?
> a. let them finished.
> but if the DMA user has already suspended prior DMA controller,
> it is meaningless somehow and may even ask for trouble.
> b. don't touch them.
> after resume these pending descriptors could be executed, it is
> also meaningless because the resumed DMA user may in different state
> from before being suspended.
> c. delete them.
> should we do this? is is a bit crude?
> d. return a non-success value
> then the whole suspend process is reversed, e.g. suspend fails.
> I've looked through some dma drivers, most of them is in case b.
Yes and that makese sense.

In calssic suspend case we maybe in middle so graceful behaviour would be to for
client to PAUSE or terminate and then suspend followed by DMA suspend.
You need to rely on client doing the right thing here

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v13 1/5] dmaengine: fix comment typo

2014-05-21 Thread Vinod Koul
On Thu, May 15, 2014 at 06:15:31PM +0400, Alexander Popov wrote:
> Fix comment typo.

Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v13 2/5] dma: mpc512x: add support for peripheral transfers

2014-05-21 Thread Vinod Koul
On Thu, May 15, 2014 at 06:15:32PM +0400, Alexander Popov wrote:
> Introduce support for slave s/g transfer preparation and the associated
> device control callback in the MPC512x DMA controller driver, which adds
> support for data transfers between memory and peripheral I/O to the
> previously supported mem-to-mem transfers.

Applied, after fixing sybstem name to "dmaengine"

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH RFC v13 0/5] MPC512x DMA slave s/g support, OF DMA lookup

2014-05-21 Thread Vinod Koul
On Thu, May 15, 2014 at 06:15:30PM +0400, Alexander Popov wrote:
> 2013/7/14 Gerhard Sittig :
> > this series
> > - introduces slave s/g support (that's support for DMA transfers which
> >involve peripherals in contrast to mem-to-mem transfers)
> > - adds device tree based lookup support for DMA channels
> > - combines floating patches and related feedback which already covered
> >several aspects of what the suggested LPB driver needs, to demonstrate
> >how integration might be done
> 
> ...
> 
> Changes in v12:
>  A new patch (part 2/7) is added to this series.
>  Part 6/7:
>  - change the description of 'compatible' property according part 2/7;
>  - improve the document according Gerhard's feedback;
> 
> Parts 1/7, 2/7 and 4/7 have been applied by Vinod Koul and
> are excluded from v13.
> 
> Changes in v13:
>  A new patch (part 1/5) is added to this series.
>  Part 2/5:
>  - fix style issue;
>  - improve comments;

You need to cc DT- list for 3 to 5 patches. Need ack before we can apply.
I suspect 4/5 should come first out of these 3.

Also in future pls use right subsystem name "dmaengine". Also why are these
tagged as RFC still?

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v5 0/3] DMA: Freescale: driver cleanups and enhancements

2014-07-14 Thread Vinod Koul
On Wed, May 21, 2014 at 04:03:00PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> Hi Dan,
> Please have a look at this 3/3 as Vinod mentioned.
> 
> Hi Vinod Koul,
> Please have a look at the v5 patch set.

I dont see any objection, so applying all 3 with subject changed to reflect
correct subsystem name

-- 
~Vinod

> 
> v4 -> v5 changes:
>  - since previous 5 of 8 patches have been merged by Vinod, this iteration oly
>inludes the last 3 patches of v4.
>  - patches order is changed for being reviewed and merged easier.
>  - remove the .prepare functions, and use the suspend_late and resume_early in
>the suspend-and-resume patch.
> 
> v3 -> v4 changes:
>  - Fixed a typo in [2/8] commit message.
>  - There was a potential double call of list_del() when apply [4/8] only,
>although this defect is removed again in later [6/8]. This version 
>eliminates this problem by updating [4/8] and [6/8] slightly.
>  - Updated [8/8] to use register access method introduced by [2/8]
> 
> v2 -> v3 change:
> Only add "chan->pm_state = RUNNING" for patch[8/8].
> 
> v1 -> v2 change:
> The only one change is introducing a new patch[1/7] to remove the unnecessary
> macro FSL_DMA_LD_DEBUG, thus the total patches number is 8 now (was 7)
> 
> v1 notes:
> Note that patch 2~6 had beed sent out for upstream before, but were together
> with other storage patches at that time, that was not easy for being reviewed
> and merged, so I send them separately this time.
> 
> Hongbo Zhang (3):
>   DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
>   DMA: Freescale: add suspend resume functions for DMA driver
>   DMA: Freescale: change descriptor release process for supporting
> async_tx
> 
>  drivers/dma/fsldma.c |  297 
> ++
>  drivers/dma/fsldma.h |   32 +-
>  2 files changed, 260 insertions(+), 69 deletions(-)
> 
> -- 
> 1.7.9.5
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 0/3] dmaengine: mpc512x: add device tree binding document and DMA channel lookup

2014-07-25 Thread Vinod Koul
On Wed, Jun 25, 2014 at 02:52:57PM +0400, Alexander Popov wrote:
> This patch series introduces a device tree binding document for
> the MPC512x DMA controller and adds device tree based DMA channel lookup
> for it.
> 
> This version contains the improved device tree binding document:
> #dma-cells is made a required property, as it must be according
> dma/dma.txt document.

Applied, thanks

-- 
~Vinod

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/6] dmaengine: remove FSLDMA_EXTERNAL_START

2014-10-11 Thread Vinod Koul
FSLDMA_EXTERNAL_START is one of the custom methods in device_control. Since
we are planning to deprecate device_control, we should move this to an API.

This serries adds the fsl_dma_external_start() API for users and also
converts the users.

I would like this to be merged thru dmanegine tree due to new dependency.

Vinod Koul (6):
  dmaengine: add dmaengine_prep_dma_sg() helper
  dmaengine: freescale: add and export fsl_dma_external_start()
  carma-fpga: use dmaengine_xxx() API
  carma-fpga: move to fsl_dma_external_start()
  dmaengine: freescale: remove FSLDMA_EXTERNAL_START control method
  dmaengine: remove FSLDMA_EXTERNAL_START

 drivers/dma/fsldma.c|   25 +++--
 drivers/misc/carma/carma-fpga-program.c |   12 ++--
 include/linux/dmaengine.h   |   13 ++---
 include/linux/fsldma.h  |   13 +
 4 files changed, 44 insertions(+), 19 deletions(-)
 create mode 100644 include/linux/fsldma.h

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/6] dmaengine: add dmaengine_prep_dma_sg() helper

2014-10-11 Thread Vinod Koul
This was only prep API which didnt have an helper

Signed-off-by: Vinod Koul 
---
 include/linux/dmaengine.h |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 3d291f5..ce8a08e 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -757,6 +757,16 @@ static inline struct dma_async_tx_descriptor 
*dmaengine_prep_interleaved_dma(
return chan->device->device_prep_interleaved_dma(chan, xt, flags);
 }
 
+static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_sg(
+   struct dma_chan *chan,
+   struct scatterlist *dst_sg, unsigned int dst_nents,
+   struct scatterlist *src_sg, unsigned int src_nents,
+   unsigned long flags)
+{
+   return chan->device->device_prep_dma_sg(chan, dst_sg, dst_nents,
+   src_sg, src_nents, flags);
+}
+
 static inline int dma_get_slave_caps(struct dma_chan *chan, struct 
dma_slave_caps *caps)
 {
if (!chan || !caps)
-- 
1.7.0.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/6] dmaengine: freescale: add and export fsl_dma_external_start()

2014-10-11 Thread Vinod Koul
The freescale driver uses custom device control FSLDMA_EXTERNAL_START to
put the controller in external start mode.
Since we are planning to deprecate the device control, move this to exported
API. Subsequent patches will remove the FSLDMA_EXTERNAL_START

Signed-off-by: Vinod Koul 
---
 drivers/dma/fsldma.c   |   16 +++-
 include/linux/fsldma.h |   13 +
 2 files changed, 28 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/fsldma.h

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index d5d6885..0cded86 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -36,7 +36,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include "dmaengine.h"
 #include "fsldma.h"
 
@@ -367,6 +367,20 @@ static void fsl_chan_toggle_ext_start(struct fsldma_chan 
*chan, int enable)
chan->feature &= ~FSL_DMA_CHAN_START_EXT;
 }
 
+int fsl_dma_external_start(struct dma_chan *dchan, int enable)
+{
+   struct fsldma_chan *chan;
+
+   if (!dchan)
+   return -EINVAL;
+
+   chan = to_fsl_chan(dchan);
+
+   fsl_chan_toggle_ext_start(chan, enable);
+   return 0;
+}
+EXPORT_SYMBOL_GPL(fsl_dma_external_start);
+
 static void append_ld_queue(struct fsldma_chan *chan, struct fsl_desc_sw *desc)
 {
struct fsl_desc_sw *tail = to_fsl_desc(chan->ld_pending.prev);
diff --git a/include/linux/fsldma.h b/include/linux/fsldma.h
new file mode 100644
index 000..b213c02
--- /dev/null
+++ b/include/linux/fsldma.h
@@ -0,0 +1,13 @@
+/*
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef FSL_DMA_H
+#define FSL_DMA_H
+/* fsl dma API for enxternal start */
+int fsl_dma_external_start(struct dma_chan *dchan, int enable);
+
+#endif
-- 
1.7.0.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/6] carma-fpga: use dmaengine_xxx() API

2014-10-11 Thread Vinod Koul
The drivers should use dmaengine_slave_config() and dmaengine_prep_dma_sg()
API instead of accessing the device_control which will be deprecated soon

Signed-off-by: Vinod Koul 
---
 drivers/misc/carma/carma-fpga-program.c |9 -
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/carma/carma-fpga-program.c 
b/drivers/misc/carma/carma-fpga-program.c
index 7be8983..fd0cb8b 100644
--- a/drivers/misc/carma/carma-fpga-program.c
+++ b/drivers/misc/carma/carma-fpga-program.c
@@ -518,8 +518,7 @@ static noinline int fpga_program_dma(struct fpga_dev *priv)
config.direction = DMA_MEM_TO_DEV;
config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
config.dst_maxburst = fpga_fifo_size(priv->regs) / 2 / 4;
-   ret = chan->device->device_control(chan, DMA_SLAVE_CONFIG,
-  (unsigned long)&config);
+   ret = dmaengine_slave_config(chan, &config);
if (ret) {
dev_err(priv->dev, "DMA slave configuration failed\n");
goto out_dma_unmap;
@@ -532,9 +531,9 @@ static noinline int fpga_program_dma(struct fpga_dev *priv)
}
 
/* setup and submit the DMA transaction */
-   tx = chan->device->device_prep_dma_sg(chan,
- table.sgl, num_pages,
- vb->sglist, vb->sglen, 0);
+
+   tx = dmaengine_prep_dma_sg(chan, table.sgl, num_pages,
+   vb->sglist, vb->sglen, 0);
if (!tx) {
dev_err(priv->dev, "Unable to prep DMA transaction\n");
ret = -ENOMEM;
-- 
1.7.0.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 5/6] dmaengine: freescale: remove FSLDMA_EXTERNAL_START control method

2014-10-11 Thread Vinod Koul
since users have been move to fsl_dma_external_start() API, so remove this
now

Signed-off-by: Vinod Koul 
---
 drivers/dma/fsldma.c |9 -
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 0cded86..994bcb2 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1012,15 +1012,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
chan->set_request_count(chan, size);
return 0;
 
-   case FSLDMA_EXTERNAL_START:
-
-   /* make sure the channel supports external start */
-   if (!chan->toggle_ext_start)
-   return -ENXIO;
-
-   chan->toggle_ext_start(chan, arg);
-   return 0;
-
default:
return -ENXIO;
}
-- 
1.7.0.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 4/6] carma-fpga: move to fsl_dma_external_start()

2014-10-11 Thread Vinod Koul
carma-fpga driver uses device control with custom FSLDMA_EXTERNAL_START
command. Since we wnat to deprecate the device control, move this driver to
use new fsl_dma_external_start() API

Signed-off-by: Vinod Koul 
---
 drivers/misc/carma/carma-fpga-program.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/carma/carma-fpga-program.c 
b/drivers/misc/carma/carma-fpga-program.c
index fd0cb8b..298f912 100644
--- a/drivers/misc/carma/carma-fpga-program.c
+++ b/drivers/misc/carma/carma-fpga-program.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -524,7 +525,7 @@ static noinline int fpga_program_dma(struct fpga_dev *priv)
goto out_dma_unmap;
}
 
-   ret = chan->device->device_control(chan, FSLDMA_EXTERNAL_START, 1);
+   ret = fsl_dma_external_start(chan, 1)
if (ret) {
dev_err(priv->dev, "DMA external control setup failed\n");
goto out_dma_unmap;
-- 
1.7.0.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 6/6] dmaengine: remove FSLDMA_EXTERNAL_START

2014-10-11 Thread Vinod Koul
as users have been converted, so no need of this custom method

Signed-off-by: Vinod Koul 
---
 include/linux/dmaengine.h |3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index ce8a08e..3254a03 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -199,15 +199,12 @@ enum dma_ctrl_flags {
  * configuration data in statically from the platform). An additional
  * argument of struct dma_slave_config must be passed in with this
  * command.
- * @FSLDMA_EXTERNAL_START: this command will put the Freescale DMA controller
- * into external start mode.
  */
 enum dma_ctrl_cmd {
DMA_TERMINATE_ALL,
DMA_PAUSE,
DMA_RESUME,
DMA_SLAVE_CONFIG,
-   FSLDMA_EXTERNAL_START,
 };
 
 /**
-- 
1.7.0.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.

2014-10-17 Thread Vinod Koul
On Fri, Oct 17, 2014 at 02:13:20AM +, Xuelin Shi wrote:
> Hi Dan & Vinod,
> 
> I have sent out the v4 of this patch and not received any further feedback 
> yet.
> 
> This patch looks ruled out from the patchwork. 
> https://patchwork.kernel.org/project/linux-dmaengine/list/?page=2
> 
> So do you know what happened to this patch?

First pls do not top post on mailing list

Yes I did clean patchworks this week for older patches, can you please
resubmit and we can review them

Thanks
-- 
~Vinod

> 
> Thanks,
> Xuelin Shi
> 
> 
> -Original Message-
> From: Shi Xuelin-B29237 
> Sent: 2014年4月15日 11:08
> To: 'Dan Williams'
> Cc: Koul, Vinod; andriy.shevche...@intel.com; dmaeng...@vger.kernel.org; 
> linuxppc-dev; Rai Harninder-B01044; Burmi Naveen-B16502
> Subject: RE: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.
> 
> Yes, "depend on !ASYNC_TX_CHANNEL_SWITCH" is better since fsldma selects this 
> condition.
> 
> Thanks,
> Xuelin Shi
> 
> -Original Message-
> From: Dan Williams [mailto:dan.j.willi...@intel.com]
> Sent: 2014年4月15日 8:30
> To: Shi Xuelin-B29237
> Cc: Koul, Vinod; andriy.shevche...@intel.com; dmaeng...@vger.kernel.org; 
> linuxppc-dev; Rai Harninder-B01044; Burmi Naveen-B16502
> Subject: Re: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.
> 
> On Sun, Apr 13, 2014 at 7:48 PM, Xuelin Shi  wrote:
> > Hi Dan,
> >
> > fsl dma device and fsl raid device are two differenct devices that 
> > both provide async_memcpy capability, so I use !FSL_DMA to disable the fsl 
> > dma device.
> >
> > That's to say, either select fsldma device, either fsl raid device.
> >
> 
> Right, but that's not what your proposed Kconfig dependency line does.
> 
> You want something like "depends on FSL_SOC && !(FSL_DMA || FSL_DMA=m)"
> 
> However, the more problematic option is ASYNC_TX_CHANNEL_SWITCH.  That option 
> is problematic for RAID, so I propose "depend on !ASYNC_TX_CHANNEL_SWITCH" 
> since that addresses both problems.
> 
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RESEND PATCH v4] dmaengine: Driver support for FSL RaidEngine device.

2014-12-05 Thread Vinod Koul
On Fri, Oct 17, 2014 at 03:28:20PM +0800, xuelin@freescale.com wrote:
> +/*
> + * drivers/dma/fsl_raid.c
> + *
> + * Freescale RAID Engine device driver
> + *
> + * Author:
> + *   Harninder Rai 
> + *   Naveen Burmi 
> + *
> + * Rewrite:
> + *   Xuelin Shi 
> + *
> + * Copyright (c) 2010-2014 Freescale Semiconductor, Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in the
> + *   documentation and/or other materials provided with the distribution.
> + * * Neither the name of Freescale Semiconductor nor the
> + *   names of its contributors may be used to endorse or promote products
> + *   derived from this software without specific prior written 
> permission.
hmmm, this doesnt sound right. BSD header in kernel code
I am not a lawyer but for kernel this doesn't sound right. Why cant this be
only GPL? Why does this deviate from norm?

> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR 
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
> AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
> THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * Theory of operation:
> + *
> + * General capabilities:
> + *   RAID Engine (RE) block is capable of offloading XOR, memcpy and P/Q
> + *   calculations required in RAID5 and RAID6 operations. RE driver
> + *   registers with Linux's ASYNC layer as dma driver. RE hardware
> + *   maintains strict ordering of the requests through chained
> + *   command queueing.
okay I see driver is use re_xxx which is a very common term imo. I think we
need to protect the symbols by adding fsl_re_ tag. otherwise it will
conflict if someone does generic raid engine and decides to name it re_xxx

> + *
> + * Data flow:
> + *   Software RAID layer of Linux (MD layer) maintains RAID partitions,
> + *   strips, stripes etc. It sends requests to the underlying AYSNC layer
> + *   which further passes it to RE driver. ASYNC layer decides which request
> + *   goes to which job ring of RE hardware. For every request processed by
> + *   RAID Engine, driver gets an interrupt unless coalescing is set. The
> + *   per job ring interrupt handler checks the status register for errors,
> + *   clears the interrupt and leave the post interrupt processing to the irq
> + *   thread.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "dmaengine.h"
> +#include "fsl_raid.h"
> +
> +#define MAX_XOR_SRCS 16
> +#define MAX_PQ_SRCS  16
> +#define MAX_INITIAL_DESCS256
> +#define MAX_DESCS_LIMIT  (4 * MAX_INITIAL_DESCS)
> +#define FRAME_FORMAT 0x1
> +#define MAX_DATA_LENGTH  (1024*1024)
these need to be namespaced

> +
> +static enum dma_status re_jr_tx_status(struct dma_chan *chan,
> + dma_cookie_t cookie, struct dma_tx_state *txstate)
> +{
> + enum dma_status ret;
> + struct re_jr *jr = container_of(chan, struct re_jr, chan);
> +
> + ret = dma_cookie_status(chan, cookie, txstate);
> +
> + if (ret != DMA_COMPLETE) {
> + re_jr_cleanup_descs(jr);
why do you do cleanup here?

> + ret = dma_cookie_status(chan, cookie, txstate);
and then call again
> + }

this is clearly not the expectation of tx_status callback.

  * device_tx_status
 - Should report the bytes left to go over on the given channel
 - Should only care about the transaction descriptor passed as
   argument, not the currently active one on a given channel
 - The tx_state argument might be NULL
 - Should use dma_set_residue to report it
 - In the case of a cyclic transfer, it should only take into
   account the current period.
   

Re: [PATCH 1/1] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory

2013-05-02 Thread Vinod Koul
On Wed, May 01, 2013 at 03:28:09PM +0400, Alexander Popov wrote:
> The initial version of this driver supports only memory to memory
> data transfers.
> 
> Data transfers between memory and i/o memory require more delicate TCD
> (Transfer Control Descriptor) configuration and DMA channel service requests
> via hardware.
> 
> dma_device.device_control callback function is needed to configure
> DMA channel to work with i/o memory.
> 
> Signed-off-by: Alexander Popov 
> ---
>  drivers/dma/mpc512x_dma.c | 230 
> ++
>  1 file changed, 192 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index 2d95673..8aedff1 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> @@ -2,6 +2,7 @@
>   * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
>   * Copyright (C) Semihalf 2009
>   * Copyright (C) Ilya Yanok, Emcraft Systems 2010
> + * Copyright (C) Alexander Popov, Promcontroller 2013
>   *
>   * Written by Piotr Ziecik . Hardware description
>   * (defines, structures and comments) was taken from MPC5121 DMA driver
> @@ -28,11 +29,6 @@
>   * file called COPYING.
>   */
>  
> -/*
> - * This is initial version of MPC5121 DMA driver. Only memory to memory
> - * transfers are supported (tested using dmatest module).
> - */
> -
>  #include 
>  #include 
>  #include 
> @@ -183,6 +179,8 @@ struct mpc_dma_desc {
>  
>  struct mpc_dma_chan {
>   struct dma_chan chan;
> + enum dma_transfer_direction dir;
> + enum dma_slave_buswidth slave_reg_width;
>   struct list_headfree;
>   struct list_headprepared;
>   struct list_headqueued;
> @@ -190,6 +188,7 @@ struct mpc_dma_chan {
>   struct list_headcompleted;
>   struct mpc_dma_tcd  *tcd;
>   dma_addr_t  tcd_paddr;
> + u32 tcd_nunits;
>  
>   /* Lock for this structure */
>   spinlock_t  lock;
> @@ -268,7 +267,11 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>  
>   if (first != prev)
>   mdma->tcd[cid].e_sg = 1;
> - out_8(&mdma->regs->dmassrt, cid);
> +
> + if (first->tcd->biter != 1) /* Request channel service by... */
> + out_8(&mdma->regs->dmaserq, cid); /* hardware */
> + else
> + out_8(&mdma->regs->dmassrt, cid); /* software */
>  }
>  
>  /* Handle interrupt on one half of DMA controller (32 channels) */
> @@ -567,7 +570,42 @@ mpc_dma_tx_status(struct dma_chan *chan, dma_cookie_t 
> cookie,
>   return ret;
>  }
>  
> -/* Prepare descriptor for memory to memory copy */
> +static int mpc_dma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> + unsigned long arg)
> +{
> + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
> + struct dma_slave_config *cfg = (void *)arg;
> + int ret = 0;
> +
> + if (!chan)
> + return -EINVAL;
> +
> + if (cmd == DMA_SLAVE_CONFIG && cfg) {
> + if (cfg->direction == DMA_DEV_TO_MEM) {
> + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_UNDEFINED)
> + mchan->slave_reg_width = cfg->src_addr_width;
> + else
> + return -EINVAL;
> + mchan->dir = DMA_DEV_TO_MEM;
> + mchan->tcd_nunits = cfg->src_maxburst;
you need to save the slave addr too.

> + } else if (cfg->direction == DMA_MEM_TO_DEV) {
> + if (cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_UNDEFINED)
> + mchan->slave_reg_width = cfg->dst_addr_width;
> + else
> + return -EINVAL;
> + mchan->dir = DMA_MEM_TO_DEV;
> + mchan->tcd_nunits = cfg->dst_maxburst;
> + } else {
> + mchan->dir = DMA_MEM_TO_MEM;
> + mchan->slave_reg_width = DMA_SLAVE_BUSWIDTH_UNDEFINED;
> + mchan->tcd_nunits = 0;
> + }
> + } else
> + return -ENOSYS;
ENXIO?

while at it, consider a different way:

if (cmd != DMA_SLAVE_CONFIG || !cfg)
return -ENXIO;

then you can shift the sholw code one indent left, makes it look a little
better.

> +
> + return ret;
> +}
> +
>  static struct dma_async_tx_descriptor *
>  mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
>   size_t len, unsigned long flags)
> @@ -577,6 +615,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t 
> dst, dma_addr_t src,
>   struct mpc_dma_desc *mdesc = NULL;
>   struct mpc_dma_tcd *tcd;
>   unsigned long iflags;
> + u32 iter = 0;
>  
>   /* Get free descriptor */
>   spin_lock_i

Re: [PATCH 00/12] dma: various minor clean ups for slave drivers

2013-05-30 Thread Vinod Koul
On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
> Here is a set of small independent patches that clean up or fix minor things
> across DMA slave drivers.
The series looks fine. I am going to wait a day more and apply, pls speak up if
you disagree and ack if you agree

--
~Vinod
> 
> Andy Shevchenko (12):
>   imx-sdma: remove useless variable
>   mxs-dma: remove useless variable
>   edma: no need to assign residue to 0 explicitly
>   ep93xx_dma: remove useless use of lock
>   fsldma: remove useless use of lock
>   mmp_pdma: remove useless use of lock
>   mpc512x_dma: remove useless use of lock
>   pch_dma: remove useless use of lock
>   tegra20-apb-dma: remove useless use of lock
>   ipu_idmac: re-use dma_cookie_status()
>   mmp_tdma: set cookies as well when asked for tx status
>   txx9dmac: return DMA_SUCCESS immediately from device_tx_status()
> 
>  drivers/dma/edma.c|  2 --
>  drivers/dma/ep93xx_dma.c  | 10 +-
>  drivers/dma/fsldma.c  | 10 +-
>  drivers/dma/imx-sdma.c|  9 +++--
>  drivers/dma/ipu/ipu_idmac.c   |  5 +
>  drivers/dma/mmp_pdma.c| 10 +-
>  drivers/dma/mmp_tdma.c|  3 ++-
>  drivers/dma/mpc512x_dma.c | 10 +-
>  drivers/dma/mxs-dma.c |  4 +---
>  drivers/dma/pch_dma.c |  9 +
>  drivers/dma/tegra20-apb-dma.c |  8 +++-
>  drivers/dma/txx9dmac.c| 13 ++---
>  12 files changed, 21 insertions(+), 72 deletions(-)
> 
> -- 
> 1.8.2.rc0.22.gb3600c3
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc: add explicit OF includes for ppc4xx

2013-11-11 Thread Vinod Koul
On Sun, Nov 10, 2013 at 11:35:43PM -0600, Rob Herring wrote:
> From: Rob Herring 
> 
> Commit b5b4bb3f6a11f9 (of: only include prom.h on sparc) removed implicit
> includes of of_*.h headers by powerpc's prom.h. Some PPC4xx components
> were missed in initial clean-up patch, so add the necessary includes
> to fix ppc4xx builds.
> 
> Signed-off-by: Rob Herring 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Tejun Heo 
> Cc: Matt Mackall 
> Cc: Herbert Xu 
> Cc: "David S. Miller" 
> Cc: Vinod Koul 
> Cc: Dan Williams 
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-cry...@vger.kernel.org
> ---
> I intend to send this patch to Linus with the rest of the DT clean-up
> series.
> 
> Rob
> 
>  arch/powerpc/sysdev/ppc4xx_ocm.c | 1 +
>  arch/powerpc/sysdev/xilinx_intc.c| 1 +
>  drivers/ata/sata_dwc_460ex.c | 2 ++
>  drivers/char/hw_random/ppc4xx-rng.c  | 1 +
>  drivers/crypto/amcc/crypto4xx_core.c | 3 +++
>  drivers/dma/ppc4xx/adma.c| 2 ++
For this:

Acked-by Vinod Koul 

--
~Vinod
>  6 files changed, 10 insertions(+)
> 
> diff --git a/arch/powerpc/sysdev/ppc4xx_ocm.c 
> b/arch/powerpc/sysdev/ppc4xx_ocm.c
> index 1b15f93..b7c4345 100644
> --- a/arch/powerpc/sysdev/ppc4xx_ocm.c
> +++ b/arch/powerpc/sysdev/ppc4xx_ocm.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/powerpc/sysdev/xilinx_intc.c 
> b/arch/powerpc/sysdev/xilinx_intc.c
> index f4fdc94..83f943a 100644
> --- a/arch/powerpc/sysdev/xilinx_intc.c
> +++ b/arch/powerpc/sysdev/xilinx_intc.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 2e39173..523524b 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -31,6 +31,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/char/hw_random/ppc4xx-rng.c 
> b/drivers/char/hw_random/ppc4xx-rng.c
> index 732c330..521f76b 100644
> --- a/drivers/char/hw_random/ppc4xx-rng.c
> +++ b/drivers/char/hw_random/ppc4xx-rng.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> diff --git a/drivers/crypto/amcc/crypto4xx_core.c 
> b/drivers/crypto/amcc/crypto4xx_core.c
> index f88e3d8..efaf630 100644
> --- a/drivers/crypto/amcc/crypto4xx_core.c
> +++ b/drivers/crypto/amcc/crypto4xx_core.c
> @@ -27,6 +27,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
> index 370ff82..e24b5ef 100644
> --- a/drivers/dma/ppc4xx/adma.c
> +++ b/drivers/dma/ppc4xx/adma.c
> @@ -42,6 +42,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> -- 
> 1.8.1.2
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

2013-11-11 Thread Vinod Koul
On Mon, Oct 28, 2013 at 05:58:42AM +, Xiubo Li-B47053 wrote:
> Hi Dan, Vinod,
> 
> 
> > > +static int fsl_sai_probe(struct platform_device *pdev) {
> > [...]
> > > +
> > > + sai->dma_params_rx.addr = res->start + SAI_RDR;
> > > + sai->dma_params_rx.maxburst = 6;
> > > + index = of_property_match_string(np, "dma-names", "rx");
> > > + ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> > > + &dma_args);
> > > + if (ret)
> > > + return ret;
> > > + sai->dma_params_rx.slave_id = dma_args.args[1];
> > > +
> > > + sai->dma_params_tx.addr = res->start + SAI_TDR;
> > > + sai->dma_params_tx.maxburst = 6;
> > > + index = of_property_match_string(np, "dma-names", "tx");
> > > + ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
> > > + &dma_args);
> > > + if (ret)
> > > + return ret;
> > > + sai->dma_params_tx.slave_id = dma_args.args[1];
> > 
> > The driver should not have to manually parse the dma devicetree
> > properties, this is something that should be handled by the dma engine
> > driver.
> > 
> 
> What do you think about the DMA slave_id ?
> I have been noticed by one colleague that this should be parsed here, which
> is from your opinions ?
Sure slave_id can be parsed here, but IMO it should be programmed via the
dma_slave_confog into the respective channel

--
~Vinod
> 
> 
> > > +
> > > + ret = snd_soc_register_component(&pdev->dev, &fsl_component,
> > > + &fsl_sai_dai, 1);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = fsl_pcm_dma_init(pdev);
> > > + if (ret)
> > > + goto out;
> 
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v11 0/3] DMA: Freescale: Add support for 8-channel DMA engine

2013-11-13 Thread Vinod Koul
On Thu, Sep 26, 2013 at 05:33:40PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> Hi DMA and DT maintainers, please have a look at these V11 patches.
> 
> Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch set
> adds support this DMA engine.
> 
Applied all

Thanks
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v7 0/4] Add dual-fifo mode support of i.MX ssi

2013-11-28 Thread Vinod Koul
On Wed, Nov 13, 2013 at 10:55:23PM +0800, Nicolin Chen wrote:
>  * ! This series of patches has a direct dependency between them. When
>  * ! applying them, we need to apply to one single branch. Otherwise,
>  * ! it would break currect branches.
Applied, thanks

--
~Vinod
> 
> Changelog
> v7:
>  * Appended missing Acked-by to all four patches.
>  * Sorry that I didn't add them at the first place.
> v6:
>  * PATCH-1: Use goto err_firmware instead of return directly.
>  *
>  * Nothing changes for the other three ack-ed patches
> v5:
>  * PATCH-3: Add period size constraint when using dual fifo mode
>  *
>  * Nothing changes for the other three patches
> v4:
>  * PATCH-3: Drop useless register configuration.
>  *
>  * Nothing changes for the other three patches
> v3:
>  * PATCH-1: Add comments to indicate the end of v1 and v2 array.
>  * PATCH-3: Use better way to keep watermark as even number.
>  *
>  * Nothing changes for PATCH-2 and PATCH-4
> v2:
>  * Instead of adding rogue scripts to current SDMA driver based on firmware
>  * V1, we define the new SDMA firmware as version 2 and bisect the PATCH-1
>  * to two patches: The first is to add version check code to the SDMA driver;
>  * And the second is to add SSI dual FIFO DMATYPE.
>  *
>  * Nothing changes for the last two patches.
> v1:
>  * SSI can reduce hardware overrun/underrun possibility when using dual
>  * fifo mode. To support this mode, we need to first update sdma sciprt
>  * list, and then enable dual fifo BIT in SSI driver, and last update DT
>  * bindings of i.MX series.
> 
> Nicolin Chen (4):
>   dma: imx-sdma: Add sdma firmware version 2 support
>   dma: imx-sdma: Add new dma type for ssi dual fifo script
>   ASoC: fsl_ssi: Add dual fifo mode support
>   ARM: dts: imx: use dual-fifo sdma script for ssi
> 
>  .../devicetree/bindings/dma/fsl-imx-sdma.txt   |  1 +
>  arch/arm/boot/dts/imx51.dtsi   |  4 ++--
>  arch/arm/boot/dts/imx53.dtsi   |  4 ++--
>  arch/arm/boot/dts/imx6qdl.dtsi | 12 +-
>  arch/arm/boot/dts/imx6sl.dtsi  | 12 +-
>  drivers/dma/imx-sdma.c | 19 ++-
>  include/linux/platform_data/dma-imx-sdma.h |  5 
>  include/linux/platform_data/dma-imx.h  |  1 +
>  sound/soc/fsl/fsl_ssi.c| 27 
> +-
>  9 files changed, 67 insertions(+), 18 deletions(-)
> 
> -- 
> 1.8.4
> 
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH RFC v6 4/5] dma: mpc512x: register for device tree channel lookup

2014-01-09 Thread Vinod Koul
On Wed, Jan 08, 2014 at 05:47:19PM +0100, Gerhard Sittig wrote:
> [ dropping devicetree from the Cc: list ]
> 
> [ what is the semantics of DMA_PRIVATE capability flag?
>   is documentation available beyond the initial commit message?
>   need individual channels be handled instead of controllers? ]

The DMA_PRIVATE means that your channels are not to be used for global memcpy,
as one can do in async cases (this is hwere DMAengine came into existence)

If the device has the capablity of doing genric memcpy then it should not set
this. For slave dma usage the dam channel can transfer data to a specfic
slave device(s), hence we should use this is geric fashion so setting
DMA_PRIVATE makes sense in those cases.

> 
> On Sat, Jan 04, 2014 at 00:54 +0400, Alexander Popov wrote:
> > 
> > Hello Gerhard.
> > Thanks for your review.
> > 
> > 2013/12/26 Gerhard Sittig :
> > > [ dropping devicetree, we're DMA specific here ]
> > >
> > > On Tue, Dec 24, 2013 at 16:06 +0400, Alexander Popov wrote:
> > >>
> > >> --- a/drivers/dma/mpc512x_dma.c
> > >> +++ b/drivers/dma/mpc512x_dma.c
> > >> [ ... ]
> > >> @@ -950,6 +951,7 @@ static int mpc_dma_probe(struct platform_device *op)
> > >>   INIT_LIST_HEAD(&dma->channels);
> > >>   dma_cap_set(DMA_MEMCPY, dma->cap_mask);
> > >>   dma_cap_set(DMA_SLAVE, dma->cap_mask);
> > >> + dma_cap_set(DMA_PRIVATE, dma->cap_mask);
> > >>
> > >>   for (i = 0; i < dma->chancnt; i++) {
> > >>   mchan = &mdma->channels[i];
> > >
> > > What are the implications of this?  Is a comment due?
> > 
> > I've involved DMA_PRIVATE flag because new of_dma_xlate_by_chan_id()
> > uses dma_get_slave_channel() instead of dma_request_channel()
> > (PATCH RFC v6 3/5). This flag is implicitly set in dma_request_channel(),
> > but is not set in dma_get_slave_channel().
Which makes me thing you are targetting slave usages. Do you intend to use for
mempcy too on all controllers you support. in that case you should set it
selectively.

> > There are only two places in the mainline kernel, where
> > dma_get_slave_channel() is used. I've picked up the idea
> > at one of these places. Please look at this patch:
> > http://www.spinics.net/lists/arm-kernel/msg268718.html
> 
> I agree that the change looks simple, and there is no doubt that
> other drivers apply the flag.  None of this I questioned.  Yet
> I'm afraid that the implications are rather huge.
> 
> Unless I miss something, I'd happily learn where I'm wrong.
> 
> > > I haven't found documentation about the DMA_PRIVATE flag, only
> > > saw commit 59b5ec21446b9 "dmaengine: introduce
> > > dma_request_channel and private channels".
> > 
> > Unfortunately I didn't find any description of DMA_PRIVATE flag too.
> > But the comment at the beginning of drivers/dma/dmaengine.c
> > may give a clue. Quotation:
> >   * subsystem can get access to a channel by calling dmaengine_get() 
> > followed
> >   * by dma_find_channel(), or if it has need for an exclusive channel
> > it can call
> >   * dma_request_channel().  Once a channel is allocated a reference is taken
> >   * against its corresponding driver to disable removal.
> > 
> > DMA_PRIVATE capability flag might indicate that the DMA controller
> > can provide exclusive channels to its clients. Please correct me if I'm 
> > wrong.
> > 
> > > Alex, unless I'm
> > > missing something this one-line change is quite a change in
> > > semantics, and has dramatic influence on the code's behaviour
> > > (ignores the DMA controller when looking for channels that can do
> > > mem-to-mem transfers)
> > 
> > Excuse me, Gerhard, I don't see what you mean.
> > Could you point to the corresponding code?
> 
> You did see `git show 59b5ec21446b9`, didn't you?  The commit
> message strongly suggests that DMA_PRIVATE applies to the whole
> DMA controller and excludes _all_ of its channels from the
> general purpose allocator which mem-to-mem transfers appear to be
> using.  It's not just a hint, but an active decision to reject
> requests.
> 
> Not only checking code references, but doing a text search,
> reveals one more comment on the DMA_PRIVATE flag in a crypto
> related document, which supports my interpretation:
> Documentation/crypto/async-tx-api.txt:203
> 
> 
> Can somebody ACK or NAK my interpretation?  Dan, you committed
> this change which introduced the DMA_PRIVATE logic.  What was the
> motivation for it, or the goal to achieve?  Do other platforms
> have several dedicated DMA controllers, some for peripherals and
> some for memory transfers?  Should the "private" flag apply to
> channels and not whole controllers?  Am I over-estimating the
> benefit or importance of DMA supported memory transfers?

The DMA_PRIVATE flag is more on how the channel is allocated and will it be used
by generic allocator or not. You cna still use mecpy ops for a controller with
DMA_PRIVATE flag if the controller supports.
> 
> 
> Still I see a difference in the lookup approaches:  Yours applies
> DMA_PRIVATE global

Re: [PATCH] DMA: Freescale: change BWC from 256 bytes to 1024 bytes

2014-01-20 Thread Vinod Koul
On Thu, Jan 16, 2014 at 02:10:53PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> Freescale DMA has a feature of BandWidth Control (ab. BWC), which is currently
> 256 bytes and should be changed to 1024 bytes for best DMA throughput.
> Changing BWC from 256 to 1024 will improve DMA performance much, in cases
> whatever one channel is running or multi channels are running simultanously,
> large or small buffers are copied.  And this change doesn't impact memory
> access performance remarkably, lmbench tests show that for some cases the
> memory performance are decreased very slightly, while the others are even
> better.
> Tested on T4240.

Applied, thanks

--
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 3/7] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication

2014-03-11 Thread Vinod Koul
On Thu, Jan 16, 2014 at 01:47:22PM +0800, hongbo.zh...@freescale.com wrote:
> From: Hongbo Zhang 
> 
> There are several places where descriptors are freed using identical code.
> This patch puts this code into a function to reduce code duplication.
> 
> Signed-off-by: Hongbo Zhang 
> Signed-off-by: Qiang Liu 
> ---
>  drivers/dma/fsldma.c |   38 --
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 95236e6..ad73538 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -418,6 +418,21 @@ static dma_cookie_t fsl_dma_tx_submit(struct 
> dma_async_tx_descriptor *tx)
>  }
>  
>  /**
> + * fsl_dma_free_descriptor - Free descriptor from channel's DMA pool.
> + * @chan : Freescale DMA channel
> + * @desc: descriptor to be freed
> + */
> +static void fsl_dma_free_descriptor(struct fsldma_chan *chan,
> + struct fsl_desc_sw *desc)
> +{
> + list_del(&desc->node);
> +#ifdef FSL_DMA_LD_DEBUG
> + chan_dbg(chan, "LD %p free\n", desc);
> +#endif
why not wrap the define stuff in the defination of chan_dbg rather than its
usage :(

-- 
~Vinod

> + dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
> +}
> +
> +/**
>   * fsl_dma_alloc_descriptor - Allocate descriptor from channel's DMA pool.
>   * @chan : Freescale DMA channel
>   *
> @@ -491,13 +506,8 @@ static void fsldma_free_desc_list(struct fsldma_chan 
> *chan,
>  {
>   struct fsl_desc_sw *desc, *_desc;
>  
> - list_for_each_entry_safe(desc, _desc, list, node) {
> - list_del(&desc->node);
> -#ifdef FSL_DMA_LD_DEBUG
> - chan_dbg(chan, "LD %p free\n", desc);
> -#endif
> - dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
> - }
> + list_for_each_entry_safe(desc, _desc, list, node)
> + fsl_dma_free_descriptor(chan, desc);
>  }
>  
>  static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
> @@ -505,13 +515,8 @@ static void fsldma_free_desc_list_reverse(struct 
> fsldma_chan *chan,
>  {
>   struct fsl_desc_sw *desc, *_desc;
>  
> - list_for_each_entry_safe_reverse(desc, _desc, list, node) {
> - list_del(&desc->node);
> -#ifdef FSL_DMA_LD_DEBUG
> - chan_dbg(chan, "LD %p free\n", desc);
> -#endif
> - dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
> - }
> + list_for_each_entry_safe_reverse(desc, _desc, list, node)
> + fsl_dma_free_descriptor(chan, desc);
>  }
>  
>  /**
> @@ -827,10 +832,7 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan 
> *chan,
>   dma_run_dependencies(txd);
>  
>   dma_descriptor_unmap(txd);
> -#ifdef FSL_DMA_LD_DEBUG
> - chan_dbg(chan, "LD %p free\n", desc);
> -#endif
> - dma_pool_free(chan->desc_pool, desc, txd->phys);
> + fsl_dma_free_descriptor(chan, desc);
>  }
>  
>  /**
> -- 
> 1.7.9.5
> 
> 
> 

-- 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v5] dmaengine: Driver support for FSL RaidEngine device.

2015-02-10 Thread Vinod Koul
On Mon, Dec 15, 2014 at 10:32:28AM +0800, xuelin@freescale.com wrote:

> +/* Copy descriptor from per chan software queue into hardware job ring */
> +static void fsl_re_issue_pending(struct dma_chan *chan)
> +{
> + struct fsl_re_chan *re_chan;
> + int avail;
> + struct fsl_re_desc *desc, *_desc;
> + unsigned long flags;
> +
> + re_chan = container_of(chan, struct fsl_re_chan, chan);
> +
> + spin_lock_irqsave(&re_chan->desc_lock, flags);
> + avail = FSL_RE_SLOT_AVAIL(
> + in_be32(&re_chan->jrregs->inbring_slot_avail));
okay this seems slots avail in hw
> +
> + list_for_each_entry_safe(desc, _desc, &re_chan->submit_q, node) {
> + if (!avail)
> + break;
and why break inside the loop. You shouldn't even enter this only to keep
breaking!


> +static void fsl_re_dequeue(unsigned long data)
> +{
> + struct fsl_re_chan *re_chan;
> + struct fsl_re_desc *desc, *_desc;
> + struct fsl_re_hw_desc *hwdesc;
> + unsigned long flags;
> + unsigned int count, oub_count;
> + int found;
> +
> + re_chan = dev_get_drvdata((struct device *)data);
> +
> + fsl_re_cleanup_descs(re_chan);
> +
> + spin_lock_irqsave(&re_chan->desc_lock, flags);
> + count = FSL_RE_SLOT_FULL(in_be32(&re_chan->jrregs->oubring_slot_full));
> + while (count--) {
> + found = 0;
> + hwdesc = &re_chan->oub_ring_virt_addr[re_chan->oub_count];
> + list_for_each_entry_safe(desc, _desc, &re_chan->active_q,
> +  node) {
> + /* compare the hw dma addr to find the completed */
> + if (desc->hwdesc.lbea32 == hwdesc->lbea32 &&
> + desc->hwdesc.addr_low == hwdesc->addr_low) {
> + found = 1;
> + break;
> + }
> + }
> +
> + BUG_ON(!found);
you are killing the machine here. I don't think it too severe and can't be
handled gracefully?

> +static struct fsl_re_desc *fsl_re_chan_alloc_desc(struct fsl_re_chan 
> *re_chan,
> +   unsigned long flags)
> +{
> + struct fsl_re_desc *desc = NULL;
> + void *cf;
> + dma_addr_t paddr;
> + unsigned long lock_flag;
> +
> + fsl_re_cleanup_descs(re_chan);
> +
> + spin_lock_irqsave(&re_chan->desc_lock, lock_flag);
> + if (!list_empty(&re_chan->free_q)) {
> + /* take one desc from free_q */
> + desc = list_first_entry(&re_chan->free_q,
> + struct fsl_re_desc, node);
> + list_del(&desc->node);
> +
> + desc->async_tx.flags = flags;
> + }
> + spin_unlock_irqrestore(&re_chan->desc_lock, lock_flag);
> +
> + if (!desc) {
> + desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> + cf = dma_pool_alloc(re_chan->re_dev->cf_desc_pool, GFP_NOWAIT,
> + &paddr);
> + if (!desc || !cf) {
> + kfree(desc);
and this is buggy, you can fail in desc while cf is okay..!

> + return NULL;
> + }
> +
> + desc = fsl_re_init_desc(re_chan, desc, cf, paddr);
> + desc->async_tx.flags = flags;
> +
> + spin_lock_irqsave(&re_chan->desc_lock, lock_flag);
> + re_chan->alloc_count++;
> + spin_unlock_irqrestore(&re_chan->desc_lock, lock_flag);
> + }
> +
> + return desc;
> +}
> +
> +static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
> + struct dma_chan *chan, dma_addr_t dest, dma_addr_t *src,
> + unsigned int src_cnt, const unsigned char *scf, size_t len,
> + unsigned long flags)
> +{
> + struct fsl_re_chan *re_chan;
> + struct fsl_re_desc *desc;
> + struct fsl_re_xor_cdb *xor;
> + struct fsl_re_cmpnd_frame *cf;
> + u32 cdb;
> + unsigned int i, j;
> + unsigned int save_src_cnt = src_cnt;
> + int cont_q = 0;
> +
> + if (len > FSL_RE_MAX_DATA_LEN) {
> + pr_err("Length greater than %d not supported\n",
> +FSL_RE_MAX_DATA_LEN);
printing length will be helpful

btw whats wrong with dev_err. You do realize that someone who seems this will
have no clue which device is reporting this

> +
> +static int fsl_re_alloc_chan_resources(struct dma_chan *chan)
> +{
> + struct fsl_re_chan *re_chan;
> + struct fsl_re_desc *desc;
> + void *cf;
> + dma_addr_t paddr;
> + int i;
> +
> + re_chan = container_of(chan, struct fsl_re_chan, chan);
> + for (i = 0; i < FSL_RE_MIN_DESCS; i++) {
> + desc = kzalloc(sizeof(*desc), GFP_KERNEL);
> + cf = dma_pool_alloc(re_chan->re_dev->cf_desc_pool, GFP_KERNEL,
> + &paddr);
> + if (!desc || !cf) {
> + kfree(desc);
> + break;
he

Re: [PATCH] dmaengine: fsldma: Mark expected switch fall-through

2019-08-12 Thread Vinod Koul
On 11-08-19, 19:22, Gustavo A. R. Silva wrote:
> Mark switch cases where we are expecting to fall through.
> 
> Fix the following warning (Building: powerpc-ppa8548_defconfig powerpc):
> 
> drivers/dma/fsldma.c: In function ‘fsl_dma_chan_probe’:
> drivers/dma/fsldma.c:1165:26: warning: this statement may fall through 
> [-Wimplicit-fallthrough=]
>chan->toggle_ext_pause = fsl_chan_toggle_ext_pause;
>~~~^~~
> drivers/dma/fsldma.c:1166:2: note: here
>   case FSL_DMA_IP_83XX:
>   ^~~~

Applied, thanks

-- 
~Vinod


Re: [PATCH 1/2] dmaengine: fsldma: Fix a resource leak in the remove function

2021-01-12 Thread Vinod Koul
On 12-12-20, 17:05, Christophe JAILLET wrote:
> A 'irq_dispose_mapping()' call is missing in the remove function.
> Add it.
> 
> This is needed to undo the 'irq_of_parse_and_map() call from the probe
> function and already part of the error handling path of the probe function.
> 
> It was added in the probe function only in commit d3f620b2c4fe ("fsldma:
> simplify IRQ probing and handling")

Applied both, thanks

-- 
~Vinod


Re: [PATCH 00/59] dma: Convert to platform remove callback returning void

2023-09-28 Thread Vinod Koul


On Tue, 19 Sep 2023 15:31:08 +0200, Uwe Kleine-König wrote:
> this series convert nearly all platform drivers below drivers/dma to use
> .remove_new(). The motivation is to get rid of an integer return code
> that is (mostly) ignored by the platform driver core and error prone on
> the driver side.
> 
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
> 
> [...]

Applied, thanks!

[01/59] dma: altera-msgdma: Convert to platform remove callback returning void
commit: 8876762f285fff08b1aefeba52c7157a2c9beec1
[02/59] dma: apple-admac: Convert to platform remove callback returning void
commit: e7d5aa30c8a19e6d9c4ec19d3b9e501df22e1d1a
[03/59] dma: at_hdmac: Convert to platform remove callback returning void
commit: ae3f38e495b42494414cbace3b3bd3cb6dc10506
[04/59] dma: at_xdmac: Convert to platform remove callback returning void
commit: b13af3c41bad5f4e45a73d6978f31ad48ffc2dee
[05/59] dma: bcm-sba-raid: Convert to platform remove callback returning void
commit: 017df796a3635edee5169a0700634da6dac92f6a
[06/59] dma: bcm2835-dma: Convert to platform remove callback returning void
commit: 8f63a2da288454e9228f6b438b86627f6ceae36b
[07/59] dma: bestcomm: bestcomm: Convert to platform remove callback returning 
void
commit: 7689bca111997e0d7a12cf6457fc26a52b8c800f
[08/59] dma: dma-axi-dmac: Convert to platform remove callback returning void
commit: b5f095a70117629c3abb49bcb07dfa954d275e99
[09/59] dma: dma-jz4780: Convert to platform remove callback returning void
commit: a8c85540bee12d492904a430c0c1105a4b7b101c
[10/59] dma: dw-axi-dmac: dw-axi-dmac-platform: Convert to platform remove 
callback returning void
commit: c689a2fd2a3f2a0a9e775c59c0f02ea64677c254
[11/59] dma: dw: platform: Convert to platform remove callback returning void
commit: 67572bfe2e35c232c3497b3fc8babbfe62600ce0
[12/59] dma: fsl-edma-main: Convert to platform remove callback returning void
commit: fa13c3ef3f45bca5a1474755dac57bfaf28ef61b
[13/59] dma: fsl-qdma: Convert to platform remove callback returning void
commit: fe3d44cdaea41173e50833440db84982e2a8d2a7
[14/59] dma: fsl_raid: Convert to platform remove callback returning void
commit: 37b24b50c5f8ad9037fbe81f1ee43f5e16fb5334
[15/59] dma: fsldma: Convert to platform remove callback returning void
commit: d69f80110da5d0e665d7f2872bf2185fe7f14409
[16/59] dma: idma64: Convert to platform remove callback returning void
commit: e8da277fbb8701308cfd2337afb13d34cc233349
[17/59] dma: img-mdc-dma: Convert to platform remove callback returning void
commit: 6e1b4a907e860071d957baee688a30a0bff102ef
[18/59] dma: imx-dma: Convert to platform remove callback returning void
commit: 14c49dd0c34e457d71494b04290c27d7a14584d7
[19/59] dma: imx-sdma: Convert to platform remove callback returning void
commit: 06e4f653fafdeec7b13958b771226efa1cdf76d2
[20/59] dma: k3dma: Convert to platform remove callback returning void
commit: 3faf902cb808163e9e65bf568c235a272215aed2
[21/59] dma: mcf-edma-main: Convert to platform remove callback returning void
commit: 48236cb8314238917788f73353290dd1afb9a7c6
[22/59] dma: mediatek: mtk-cqdma: Convert to platform remove callback returning 
void
commit: bdeb61f5180efc920cf55b966a2a30bc2336d6d4
[23/59] dma: mediatek: mtk-hsdma: Convert to platform remove callback returning 
void
commit: 97283173effa6e53ea698726ea5d07f7ca06e5cf
[24/59] dma: mediatek: mtk-uart-apdma: Convert to platform remove callback 
returning void
commit: 4db30945a001ac97b5044db2aa2990b8e7df9452
[25/59] dma: mmp_pdma: Convert to platform remove callback returning void
commit: c0f0d93fc1da36de564da9b3f0462b5bdc4b1948
[26/59] dma: mmp_tdma: Convert to platform remove callback returning void
commit: f543b251500a0de589bc4c97da45b88410bf7c56
[27/59] dma: moxart-dma: Convert to platform remove callback returning void
commit: 1a65831fa037457114ca75ea262d87fbde3158f0
[28/59] dma: mpc512x_dma: Convert to platform remove callback returning void
commit: 80d0159bbe80d8de6f64c0a8554b4066c66eb378
[29/59] dma: mv_xor_v2: Convert to platform remove callback returning void
commit: 733dbb8d62f33448c0d7470ba06ce39bdd790ddd
[30/59] dma: nbpfaxi: Convert to platform remove callback returning void
commit: 44d5338c4a5d7f3f1ef07d502b9db22a29a8756a
[31/59] dma: owl-dma: Convert to platform remove callback returning void
commit: 1260486a347567c33e0118626bb8778e342e6080
[32/59] dma: ppc4xx: adma: Convert to platform remove callback returning void
commit: 5f8f212fb416dc3600046955fb008732a512fa5c
[33/59] dma: pxa_dma: Convert to platform remove callback returning void
commit: 44ea88715d37dc31145b7712b5474808258bab5f
[34/59] dma: qcom: bam_dma: Convert to platform remove callback returning void
  

Re: [PATCH 00/59] dma: Convert to platform remove callback returning void

2023-09-28 Thread Vinod Koul
On 19-09-23, 15:31, Uwe Kleine-König wrote:
> Hello,
> 
> this series convert nearly all platform drivers below drivers/dma to use
> .remove_new(). The motivation is to get rid of an integer return code
> that is (mostly) ignored by the platform driver core and error prone on
> the driver side.

I have applied this, with change of subsystem to dmaengine: xxx
> 
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
> 
> There are 4 drivers I didn't convert in this series:
> 
>   drivers/dma/milbeaut-hdmac.c
>   drivers/dma/milbeaut-xdmac.c
>   drivers/dma/uniphier-mdmac.c
>   drivers/dma/uniphier-xdmac.c
> 
> These all might return early in .remove() if dmaengine_terminate_sync()
> fails. I only looked deeper into the first one, and this shows exactly
> the error that is easy to make with .remove() returning an int: When
> returning early from .remove(), some cleanup (here:
> dma_async_device_unregister()) is skipped. So the dma device stays
> known, but the device is still unregistered and the devm allocated stuff
> (here e.g. *mdev) is freed. So it can probably easily happen, that
> something tries to use the dma device and this will likely result in an
> oops.

We should convert these too, thanks for your work for the conversion

-- 
~Vinod


Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Vinod Koul
Hi Allen,

Subsytem is dmaengine, can you rename this to dmaengine: ...

On 27-03-24, 16:03, Allen Pais wrote:
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.

Thanks for conversion, am happy with BH alternative as it helps in
dmaengine where we need shortest possible time between tasklet and
interrupt handling to maximize dma performance

> 
> This patch converts drivers/dma/* from tasklet to BH workqueue.

> 
> Based on the work done by Tejun Heo 
> Branch: git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> 
> Signed-off-by: Allen Pais 
> ---
>  drivers/dma/altera-msgdma.c   | 15 
>  drivers/dma/apple-admac.c | 15 
>  drivers/dma/at_hdmac.c|  2 +-
>  drivers/dma/at_xdmac.c| 15 
>  drivers/dma/bcm2835-dma.c |  2 +-
>  drivers/dma/dma-axi-dmac.c|  2 +-
>  drivers/dma/dma-jz4780.c  |  2 +-
>  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c|  2 +-
>  drivers/dma/dw-edma/dw-edma-core.c|  2 +-
>  drivers/dma/dw/core.c | 13 +++
>  drivers/dma/dw/regs.h |  3 +-
>  drivers/dma/ep93xx_dma.c  | 15 
>  drivers/dma/fsl-edma-common.c |  2 +-
>  drivers/dma/fsl-qdma.c|  2 +-
>  drivers/dma/fsl_raid.c| 11 +++---
>  drivers/dma/fsl_raid.h|  2 +-
>  drivers/dma/fsldma.c  | 15 
>  drivers/dma/fsldma.h  |  3 +-
>  drivers/dma/hisi_dma.c|  2 +-
>  drivers/dma/hsu/hsu.c |  2 +-
>  drivers/dma/idma64.c  |  4 +--
>  drivers/dma/img-mdc-dma.c |  2 +-
>  drivers/dma/imx-dma.c | 27 +++---
>  drivers/dma/imx-sdma.c|  6 ++--
>  drivers/dma/ioat/dma.c| 17 -
>  drivers/dma/ioat/dma.h|  5 +--
>  drivers/dma/ioat/init.c   |  2 +-
>  drivers/dma/k3dma.c   | 19 +-
>  drivers/dma/mediatek/mtk-cqdma.c  | 35 ++-
>  drivers/dma/mediatek/mtk-hsdma.c  |  2 +-
>  drivers/dma/mediatek/mtk-uart-apdma.c |  4 +--
>  drivers/dma/mmp_pdma.c| 13 +++
>  drivers/dma/mmp_tdma.c| 11 +++---
>  drivers/dma/mpc512x_dma.c | 17 -
>  drivers/dma/mv_xor.c  | 13 +++
>  drivers/dma/mv_xor.h  |  5 +--
>  drivers/dma/mv_xor_v2.c   | 23 ++--
>  drivers/dma/mxs-dma.c | 13 +++
>  drivers/dma/nbpfaxi.c | 15 
>  drivers/dma/owl-dma.c |  2 +-
>  drivers/dma/pch_dma.c | 17 -
>  drivers/dma/pl330.c   | 31 
>  drivers/dma/plx_dma.c | 13 +++
>  drivers/dma/ppc4xx/adma.c | 17 -
>  drivers/dma/ppc4xx/adma.h |  5 +--
>  drivers/dma/pxa_dma.c |  2 +-
>  drivers/dma/qcom/bam_dma.c| 35 ++-
>  drivers/dma/qcom/gpi.c| 18 +-
>  drivers/dma/qcom/hidma.c  | 11 +++---
>  drivers/dma/qcom/hidma.h  |  5 +--
>  drivers/dma/qcom/hidma_ll.c   | 11 +++---
>  drivers/dma/qcom/qcom_adm.c   |  2 +-
>  drivers/dma/sa11x0-dma.c  | 27 +++---
>  drivers/dma/sf-pdma/sf-pdma.c | 23 ++--
>  drivers/dma/sf-pdma/sf-pdma.h |  5 +--
>  drivers/dma/sprd-dma.c|  2 +-
>  drivers/dma/st_fdma.c |  2 +-
>  drivers/dma/ste_dma40.c   | 17 -
>  drivers/dma/sun6i-dma.c   | 33 -
>  drivers/dma/tegra186-gpc-dma.c|  2 +-
>  drivers/dma/tegra20-apb-dma.c | 19 +-
>  drivers/dma/tegra210-adma.c   |  2 +-
>  drivers/dma/ti/edma.c |  2 +-
>  drivers/dma/ti/k3-udma.c  | 11 +++---
>  drivers/dma/ti/omap-dma.c |  2 +-
>  drivers/dma/timb_dma.c| 23 ++--
>  drivers/dma/txx9dmac.c| 29 +++
>  drivers/dma/txx9dmac.h  

Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Vinod Koul
On 28-03-24, 11:08, Arnd Bergmann wrote:
> On Thu, Mar 28, 2024, at 06:55, Vinod Koul wrote:
> > On 27-03-24, 16:03, Allen Pais wrote:
> >> The only generic interface to execute asynchronously in the BH context is
> >> tasklet; however, it's marked deprecated and has some design flaws. To
> >> replace tasklets, BH workqueue support was recently added. A BH workqueue
> >> behaves similarly to regular workqueues except that the queued work items
> >> are executed in the BH context.
> >
> > Thanks for conversion, am happy with BH alternative as it helps in
> > dmaengine where we need shortest possible time between tasklet and
> > interrupt handling to maximize dma performance
> 
> I still feel that we want something different for dmaengine,
> at least in the long run. As we have discussed in the past,
> the tasklet context in these drivers is what the callbacks
> from the dma client device is run in, and a lot of these probably
> want something other than tasklet context, e.g. just call
> complete() on a client-provided completion structure.
> 
> Instead of open-coding the use of the system_bh_wq in each
> dmaengine, how about we start with a custom WQ_BH
> specifically for the dmaengine subsystem and wrap them
> inside of another interface.
> 
> Since almost every driver associates the tasklet with the
> dma_chan, we could go one step further and add the
> work_queue structure directly into struct dma_chan,
> with the wrapper operating on the dma_chan rather than
> the work_queue.

I think that is very great idea. having this wrapped in dma_chan would
be very good way as well

Am not sure if Allen is up for it :-)

-- 
~Vinod


Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-29 Thread Vinod Koul
On 28-03-24, 13:01, Allen wrote:
> > >> > Since almost every driver associates the tasklet with the
> > >> > dma_chan, we could go one step further and add the
> > >> > work_queue structure directly into struct dma_chan,
> > >> > with the wrapper operating on the dma_chan rather than
> > >> > the work_queue.
> > >>
> > >> I think that is very great idea. having this wrapped in dma_chan would
> > >> be very good way as well
> > >>
> > >> Am not sure if Allen is up for it :-)
> > >
> > >  Thanks Arnd, I know we did speak about this at LPC. I did start
> > > working on using completion. I dropped it as I thought it would
> > > be easier to move to workqueues.
> >
> > It's definitely easier to do the workqueue conversion as a first
> > step, and I agree adding support for the completion right away is
> > probably too much. Moving the work_struct into the dma_chan
> > is probably not too hard though, if you leave your current
> > approach for the cases where the tasklet is part of the
> > dma_dev rather than the dma_chan.
> >
> 
>  Alright, I will work on moving work_struck into the dma_chan and
> leave the dma_dev as is (using bh workqueues) and post a RFC.
> Once reviewed, I could move to the next step.

That might be better from a performance pov but the current design is a
global tasklet and not a per chan one... We would need to carefully
review and test this for sure

-- 
~Vinod


Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-29 Thread Vinod Koul
On 28-03-24, 12:39, Allen wrote:

> > I think that is very great idea. having this wrapped in dma_chan would
> > be very good way as well
> >
> > Am not sure if Allen is up for it :-)
> 
>  Thanks Arnd, I know we did speak about this at LPC. I did start
> working on using completion. I dropped it as I thought it would
> be easier to move to workqueues.
> 
> Vinod, I would like to give this a shot and put out a RFC, I would
> really appreciate review and feedback.

Sounds like a good plan to me

-- 
~Vinod


Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-04-02 Thread Vinod Koul
On 02-04-24, 14:25, Linus Walleij wrote:
> Hi Allen,
> 
> thanks for your patch!
> 
> On Wed, Mar 27, 2024 at 5:03 PM Allen Pais  wrote:
> 
> > The only generic interface to execute asynchronously in the BH context is
> > tasklet; however, it's marked deprecated and has some design flaws. To
> > replace tasklets, BH workqueue support was recently added. A BH workqueue
> > behaves similarly to regular workqueues except that the queued work items
> > are executed in the BH context.
> >
> > This patch converts drivers/dma/* from tasklet to BH workqueue.
> >
> > Based on the work done by Tejun Heo 
> > Branch: git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> >
> > Signed-off-by: Allen Pais 
> (...)
> > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> (...)
> > if (d40c->pending_tx)
> > -   tasklet_schedule(&d40c->tasklet);
> > +   queue_work(system_bh_wq, &d40c->work);
> 
> Why is "my" driver not allowed to use system_bh_highpri_wq?
> 
> I can't see the reasoning between some drivers using system_bh_wq
> and others being highpri?
> 
> Given the DMA usecase I would expect them all to be high prio.

It didnt use tasklet_hi_schedule(), I guess Allen has done the
conversion of tasklet_schedule -> system_bh_wq and tasklet_hi_schedule
-> system_bh_highpri_wq

Anyway, we are going to use a dma queue so should be better performance

-- 
~Vinod


Re: [PATCH v2] dmaengine: Explicitly include correct DT includes

2023-08-01 Thread Vinod Koul


On Tue, 18 Jul 2023 08:31:35 -0600, Rob Herring wrote:
> The DT of_device.h and of_platform.h date back to the separate
> of_platform_bus_type before it as merged into the regular platform bus.
> As part of that merge prepping Arm DT support 13 years ago, they
> "temporarily" include each other. They also include platform_device.h
> and of.h. As a result, there's a pretty much random mix of those include
> files used throughout the tree. In order to detangle these headers and
> replace the implicit includes with struct declarations, users need to
> explicitly include the correct includes.
> 
> [...]

Applied, thanks!

[1/1] dmaengine: Explicitly include correct DT includes
  commit: 897500c7ea91702966adb9b412fa39400b4edee6

Best regards,
-- 
~Vinod




Re: [PATCH v9 00/10] phy: Add support for Lynx 10G SerDes

2023-01-18 Thread Vinod Koul
On 17-01-23, 11:46, Sean Anderson wrote:
> 
> I noticed that this series is marked "changes requested" on patchwork.
> However, I have received only automated feedback. I have done my best
> effort to address feedback I have received on prior revisions. I would
> appreciate getting another round of review before resending this series.

Looking at the series, looks like kernel-bot sent some warnings on the
series so I was expecting an updated series for review

-- 
~Vinod


Re: [PATCH v9 00/10] phy: Add support for Lynx 10G SerDes

2023-01-20 Thread Vinod Koul
On 19-01-23, 11:22, Sean Anderson wrote:
> On 1/18/23 11:54, Vinod Koul wrote:
> > On 17-01-23, 11:46, Sean Anderson wrote:
> >> 
> >> I noticed that this series is marked "changes requested" on patchwork.
> >> However, I have received only automated feedback. I have done my best
> >> effort to address feedback I have received on prior revisions. I would
> >> appreciate getting another round of review before resending this series.
> > 
> > Looking at the series, looks like kernel-bot sent some warnings on the
> > series so I was expecting an updated series for review
> > 
> 
> Generally, multiple reviewers will comment on a patch, even if another
> reviewer finds something which needs to be changed. This is a one-line
> fix, so I would appreciate getting more substantial feedback before
> respinning. Every time I send a new series I have to rebase and test on
> hardware. It's work that I would rather do when there is something to be
> gained.

I review to apply, if I can apply, I would typically skip this

-- 
~Vinod


Re: [PATCH v1 1/1] treewide: Align match_string() with sysfs_match_string()

2024-06-04 Thread Vinod Koul
On 02-06-24, 18:57, Andy Shevchenko wrote:
> Make two APIs look similar. Hence convert match_string() to be
> a 2-argument macro. In order to avoid unneeded churn, convert
> all users as well. There is no functional change intended.
> 

..

>  drivers/phy/mediatek/phy-mtk-tphy.c   |  8 ++---
>  drivers/phy/tegra/xusb.c  |  4 +--

Acked-by: Vinod Koul 

-- 
~Vinod


Re: [Patch v4 06/10] dmaengine: Add dma router for pl08x in LPC32XX SoC

2024-06-23 Thread Vinod Koul
Hi Piotr,

Any reason why dmaengine parts cant be sent separately, why are they
clubbed together, I dont see any obvious dependencies...

On 20-06-24, 19:56, Piotr Wojtaszczyk wrote:
> LPC32XX connects few of its peripherals to pl08x DMA thru a multiplexer,
> this driver allows to route a signal request line thru the multiplexer for
> given peripheral.

What is the difference b/w this and lpc18xx driver, why not reuse that
one?

> 
> Signed-off-by: Piotr Wojtaszczyk 
> ---
> Changes for v4:
> - This patch is new in v4
> 
>  MAINTAINERS  |   1 +
>  drivers/dma/Kconfig  |   9 ++
>  drivers/dma/Makefile |   1 +
>  drivers/dma/lpc32xx-dmamux.c | 195 +++
>  4 files changed, 206 insertions(+)
>  create mode 100644 drivers/dma/lpc32xx-dmamux.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fadf1baafd89..5ffe988ee282 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2403,6 +2403,7 @@ R:  Vladimir Zapolskiy 
>  L:   linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
>  S:   Maintained
>  F:   Documentation/devicetree/bindings/dma/nxp,lpc3220-dmamux.yaml
> +F:   drivers/dma/lpc32xx-dmamux.c
>  N:   lpc32xx
>  
>  ARM/Marvell Dove/MV78xx0/Orion SOC support
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 002a5ec80620..aeace3d7e066 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -378,6 +378,15 @@ config LPC18XX_DMAMUX
> Enable support for DMA on NXP LPC18xx/43xx platforms
> with PL080 and multiplexed DMA request lines.
>  
> +config LPC32XX_DMAMUX
> + bool "NXP LPC32xx DMA MUX for PL080"
> + depends on ARCH_LPC32XX || COMPILE_TEST
> + depends on OF && AMBA_PL08X
> + select MFD_SYSCON
> + help
> +   Support for PL080 multiplexed DMA request lines on
> +   LPC32XX platrofm.
> +
>  config LS2X_APB_DMA
>   tristate "Loongson LS2X APB DMA support"
>   depends on LOONGARCH || COMPILE_TEST
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 802ca916f05f..6f1350b62e7f 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_INTEL_IOATDMA) += ioat/
>  obj-y += idxd/
>  obj-$(CONFIG_K3_DMA) += k3dma.o
>  obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
> +obj-$(CONFIG_LPC32XX_DMAMUX) += lpc32xx-dmamux.o
>  obj-$(CONFIG_LS2X_APB_DMA) += ls2x-apb-dma.o
>  obj-$(CONFIG_MILBEAUT_HDMAC) += milbeaut-hdmac.o
>  obj-$(CONFIG_MILBEAUT_XDMAC) += milbeaut-xdmac.o
> diff --git a/drivers/dma/lpc32xx-dmamux.c b/drivers/dma/lpc32xx-dmamux.c
> new file mode 100644
> index ..4e6ce6026164
> --- /dev/null
> +++ b/drivers/dma/lpc32xx-dmamux.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright 2024 Timesys Corporation 
> +//
> +// Based on TI DMA Crossbar driver by:
> +//   Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
> +//   Author: Peter Ujfalusi 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define LPC32XX_SSP_CLK_CTRL 0x78
> +#define LPC32XX_I2S_CLK_CTRL 0x7c
> +
> +struct lpc32xx_dmamux {
> + int signal;
> + char *name_sel0;
> + char *name_sel1;
> + int muxval;
> + int muxreg;
> + int bit;
> + bool busy;
> +};
> +
> +/* From LPC32x0 User manual "3.2.1 DMA request signals" */
> +static struct lpc32xx_dmamux lpc32xx_muxes[] = {
> + {
> + .signal = 3,
> + .name_sel0 = "spi2-rx-tx",
> + .name_sel1 = "ssp1-rx",
> + .muxreg = LPC32XX_SSP_CLK_CTRL,
> + .bit = 5,
> + },
> + {
> + .signal = 10,
> + .name_sel0 = "uart7-rx",
> + .name_sel1 = "i2s1-dma1",
> + .muxreg = LPC32XX_I2S_CLK_CTRL,
> + .bit = 4,
> + },
> + {
> + .signal = 11,
> + .name_sel0 = "spi1-rx-tx",
> + .name_sel1 = "ssp1-tx",
> + .muxreg = LPC32XX_SSP_CLK_CTRL,
> + .bit = 4,
> + },
> + {
> + .signal = 14,
> + .name_sel0 = "none",
> + .name_sel1 = "ssp0-rx",
> + .muxreg = LPC32XX_SSP_CLK_CTRL,
> + .bit = 3,
> + },
> + {
> + .signal = 15,
> + .name_sel0 = "none",
> + .name_sel1 = "ssp0-tx",
> + .muxreg = LPC32XX_SSP_CLK_CTRL,
> + .bit = 2,
> + },
> +};
> +
> +struct lpc32xx_dmamux_data {
> + struct dma_router dmarouter;
> + struct regmap *reg;
> + spinlock_t lock; /* protects busy status flag */
> +};
> +
> +static void lpc32xx_dmamux_release(struct device *dev, void *route_data)
> +{
> + struct lpc32xx_dmamux_data *dmamux = dev_get_drvdata(dev);
> + struct lpc32xx_dmamux *mux = route_data;
> + unsigned long flags;
> +
> + dev_dbg(dev, "releasing dma request signal %d routed to %s\n",
> + mux->signal, mux

Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64

2019-02-03 Thread Vinod Koul
On 25-01-19, 05:54, Peng Ma wrote:
> Hi Vinod,
> 
> Sorry to replay late.
> 1:This patch has already send to the patchwork.
>   Please see the patch link: https://patchwork.kernel.org/patch/10741521/
> 2:I have already compile the fsl patches on arm and powerpc after patched 
> https://patchwork.kernel.org/patch/10741521/
>   The compile will successful, please let me know the reported regression 
> results, thanks very much.

And I thought there were further comments  on this patch. I have applied
this, please watch for failures reported if any..
-- 
~Vinod


Re: [PATCH] mm: Replace all open encodings for NUMA_NO_NODE

2018-11-24 Thread Vinod Koul
On 23-11-18, 15:24, Anshuman Khandual wrote:

> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -386,7 +386,8 @@ EXPORT_SYMBOL(dma_issue_pending_all);
>  static bool dma_chan_is_local(struct dma_chan *chan, int cpu)
>  {
>   int node = dev_to_node(chan->device->dev);
> - return node == -1 || cpumask_test_cpu(cpu, cpumask_of_node(node));
> + return node == NUMA_NO_NODE ||
> + cpumask_test_cpu(cpu, cpumask_of_node(node));
>  }

I do not see dev_to_node being updated first, that returns -1 so I would
prefer to check for -1 unless it return NUMA_NO_NODE

-- 
~Vinod


Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE

2018-11-27 Thread Vinod Koul
On 26-11-18, 17:56, Anshuman Khandual wrote:
> At present there are multiple places where invalid node number is encoded
> as -1. Even though implicitly understood it is always better to have macros
> in there. Replace these open encodings for an invalid node number with the
> global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
> 'invalid node' from various places redirecting them to a common definition.
> 

>  drivers/dma/dmaengine.c       |  4 +++-


Acked-by: Vinod Koul 

-- 
~Vinod


Re: [v11 1/7] dmaengine: fsldma: Replace DMA_IN/OUT by FSL_DMA_IN/OUT

2018-12-05 Thread Vinod Koul
On 30-10-18, 10:35, Peng Ma wrote:
> From: Wen He 
> 
> This patch implement a standard macro call functions is
> used to NXP dma drivers.

Applied all except DTS patches, thanks
-- 
~Vinod


Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64

2019-01-19 Thread Vinod Koul
On 24-12-18, 05:29, Peng Ma wrote:
> Hi Scott,
> 
> Oh, I did not see the in_XX64/out_XX64 supported only __powerpc64__ just now.
> Thanks for your reminder.

Can you send the formal patch for this...

FWIW, fsl patches were not merged last cycle because of reported
regression...

> 
> #ifdef __powerpc64__
> 
> #ifdef __BIG_ENDIAN__
> DEF_MMIO_OUT_D(out_be64, 64, std);
> DEF_MMIO_IN_D(in_be64, 64, ld);
> 
> /* There is no asm instructions for 64 bits reverse loads and stores */
> static inline u64 in_le64(const volatile u64 __iomem *addr)
> {
> return swab64(in_be64(addr));
> }
> 
> static inline void out_le64(volatile u64 __iomem *addr, u64 val)
> {
> out_be64(addr, swab64(val));
> }
> #else
> DEF_MMIO_OUT_D(out_le64, 64, std);
> DEF_MMIO_IN_D(in_le64, 64, ld);
> 
> /* There is no asm instructions for 64 bits reverse loads and stores */
> static inline u64 in_be64(const volatile u64 __iomem *addr)
> {
> return swab64(in_le64(addr));
> }
> 
> static inline void out_be64(volatile u64 __iomem *addr, u64 val)
> {
> out_le64(addr, swab64(val));
> }
> 
> #endif
> #endif /* __powerpc64__ */
> 
> Best Regards,
> Peng
> >-Original Message-
> >From: Scott Wood 
> >Sent: 2018年12月24日 12:46
> >To: Peng Ma ; Leo Li ; Zhang Wei
> >
> >Cc: linuxppc-dev@lists.ozlabs.org; dmaeng...@vger.kernel.org; Wen He
> >
> >Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
> >powerpc64
> >
> >On Mon, 2018-12-24 at 03:42 +, Peng Ma wrote:
> >> Hi Scott,
> >>
> >> You are right, we should support powerpc64, so could I changed it as
> >> fallows:
> >>
> >> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index
> >> 88db939..057babf 100644
> >> --- a/drivers/dma/fsldma.h
> >> +++ b/drivers/dma/fsldma.h
> >> @@ -202,35 +202,10 @@ struct fsldma_chan {
> >>  #define fsl_iowrite32(v, p)out_le32(p, v)
> >>  #define fsl_iowrite32be(v, p)  out_be32(p, v)
> >>
> >> -#ifndef __powerpc64__
> >> -static u64 fsl_ioread64(const u64 __iomem *addr) -{
> >> -   u32 fsl_addr = lower_32_bits(addr);
> >> -   u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
> >> -
> >> -   return fsl_addr_hi | in_le32((u32 *)fsl_addr);
> >> -}
> >> -
> >> -static void fsl_iowrite64(u64 val, u64 __iomem *addr) -{
> >> -   out_le32((u32 __iomem *)addr + 1, val >> 32);
> >> -   out_le32((u32 __iomem *)addr, (u32)val);
> >> -}
> >> -
> >> -static u64 fsl_ioread64be(const u64 __iomem *addr) -{
> >> -   u32 fsl_addr = lower_32_bits(addr);
> >> -   u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
> >> -
> >> -   return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
> >> -}
> >> -
> >> -static void fsl_iowrite64be(u64 val, u64 __iomem *addr) -{
> >> -   out_be32((u32 __iomem *)addr, val >> 32);
> >> -   out_be32((u32 __iomem *)addr + 1, (u32)val);
> >> -}
> >> -#endif
> >> +#define fsl_ioread64(p)in_le64(p)
> >> +#define fsl_ioread64be(p)  in_be64(p)
> >> +#define fsl_iowrite64(v, p)out_le64(p, v)
> >> +#define fsl_iowrite64be(v, p)  out_be64(p, v)
> >>  #endif
> >
> >Then you'll break 32-bit, assuming those fake-it-with-two-32-bit-accesses
> >were actually needed.
> >
> >-Scott
> >
> 

-- 
~Vinod


Re: [PATCH] dmaengine:fsldma: fix memory leak

2016-03-09 Thread Vinod Koul
On Tue, Mar 08, 2016 at 02:02:01PM +0800, xuelin@nxp.com wrote:
> From: Xuelin Shi 
> 
> adding unmap of sources and destinations while doing dequeue.

Applied, thanks

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 06/14] dmaengine: bcm2835: DT spelling s/interrupts-names/interrupt-names/

2016-04-25 Thread Vinod Koul
On Wed, Apr 20, 2016 at 05:32:11PM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven 
> ---
>  Documentation/devicetree/bindings/sound/davinci-mcbsp.txt | 2 +-

This change does not apply for me, can you please split it up and send the
sound ones thru sound tree

-- 
~Vinod

>  drivers/dma/bcm2835-dma.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt 
> b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
> index 55b53e1fd72c9d6e..e0b6165c9cfcec19 100644
> --- a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
> +++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
> @@ -43,7 +43,7 @@ mcbsp0: mcbsp@1d1 {
>   <0x0031 0x1000>;
>   reg-names = "mpu", "dat";
>   interrupts = <97 98>;
> - interrupts-names = "rx", "tx";
> + interrupt-names = "rx", "tx";
>   dmas = <&edma0 3 1
>   &edma0 2 1>;
>   dma-names = "tx", "rx";
> diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
> index 974015193b93cdb3..d724393e904e9a41 100644
> --- a/drivers/dma/bcm2835-dma.c
> +++ b/drivers/dma/bcm2835-dma.c
> @@ -974,7 +974,7 @@ static int bcm2835_dma_probe(struct platform_device *pdev)
>  
>   /* legacy device tree case handling */
>   dev_warn_once(&pdev->dev,
> -   "missing interrupts-names property in device tree 
> - legacy interpretation is used");
> +   "missing interrupt-names property in device tree 
> - legacy interpretation is used");
>   /*
>* in case of channel >= 11
>* use the 11th interrupt and that is shared
> -- 
> 1.9.1
> 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/5] Use dma_pool_zalloc

2016-05-02 Thread Vinod Koul
On Fri, Apr 29, 2016 at 10:09:07PM +0200, Julia Lawall wrote:
> Dma_pool_zalloc combines dma_pool_alloc and memset 0.  The semantic patch
> that makes this transformation is as follows: (http://coccinelle.lip6.fr/)

Applied all dmaengine patches

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 00/23] ata: sata_dwc_460ex: make it working again

2016-05-09 Thread Vinod Koul
On Mon, May 09, 2016 at 10:59:32PM +0300, Andy Shevchenko wrote:
> On Mon, May 9, 2016 at 10:05 PM, Tejun Heo  wrote:
> > On Mon, May 09, 2016 at 10:13:59AM +0100, Måns Rullgård wrote:
> >> Andy Shevchenko  writes:
> >>
> >> > On Mon, May 9, 2016 at 4:09 AM, Tejun Heo  wrote:
> >> >> On Sun, May 08, 2016 at 04:00:08PM -0400, Tejun Heo wrote:
> >> >>> Hello, Andy.
> >> >>>
> >> >>> On Wed, May 04, 2016 at 03:22:51PM +0300, Andy Shevchenko wrote:
> >> >>> > Tejun, since Vinod applied all necessary patches into his tree, the
> >> >>> > series now has just a dependency to whatever branch / tag he marks 
> >> >>> > for
> >> >>> > it.
> >> >>> > Do we have a chance to see the SATA series be applied in your tree?
> >> >>>
> >> >>> Applied 1-22 to libata/for-4.7.  There was a couple trivial conflicts
> >> >>> which I resolved while applying but it'd be great if you can check
> >> >>> whether everything looks okay.
> >> >>>
> >> >>>  
> >> >>> https://git.kernel.org/cgit/linux/kernel/git/tj/libata.git/log/?h=for-4.7
> >> >>
> >> >> Oops, build failure.  Reverted for now.
> >> >
> >> > I suppose you have to pull material from Vinod.
> >>
> >> The failure the build bot reported is due to the DMA patches not being
> >> in the tree.
> >
> > Which tree should I pull?
> 
> slave-dma [1], branch topic/dw. But I think Vinod can tell us which
> tag/branch will be immutable. Vinod?

Please use branch topic/dw. I will not rebase this before sending to Linus.

Also if you prefer a signed tag, then please use dmaengine-topic-dw-4.7-rc1

> 
> [1] http://git.infradead.org/users/vkoul/slave-dma.git

Thanks
-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 07/61] dma: simplify getting .drvdata

2018-04-21 Thread Vinod Koul
On Thu, Apr 19, 2018 at 04:05:37PM +0200, Wolfram Sang wrote:
> We should get drvdata from struct device directly. Going via
> platform_device is an unneeded step back and forth.

Do you mind splitting this per driver please, that makes it easy to manage
for me :)

-- 
~Vinod


Re: [PATCH 0/6] tree-wide: simplify getting .drvdata

2018-04-22 Thread Vinod Koul
On Sun, Apr 22, 2018 at 11:14:08AM +0200, Wolfram Sang wrote:
> I got tired of fixing this in Renesas drivers manually, so I took the big
> hammer. Remove this cumbersome code pattern which got copy-pasted too much
> already:
> 
> - struct platform_device *pdev = to_platform_device(dev);
> - struct ep93xx_keypad *keypad = platform_get_drvdata(pdev);
> + struct ep93xx_keypad *keypad = dev_get_drvdata(dev);
> 
> I send this out as one patch per directory per subsystem. I think they should
> be applied individually. If you prefer a broken out series per subsystem, I 
> can
> provide this as well. Just mail me.

Applied all, thanks

-- 
~Vinod


Re: [PATCH] drivers/dma: NO_IRQ removal from powerpc-only drivers

2016-09-14 Thread Vinod Koul
On Sat, Sep 10, 2016 at 07:56:04PM +1000, Michael Ellerman wrote:
> We'd like to eventually remove NO_IRQ on powerpc, so remove usages of it
> from powerpc-only drivers.

Applied after fixing subsystem name

-- 
~Vinod


Re: [PATCH] dma/fsldma : Unmap region obtained by of_iomap

2016-09-30 Thread Vinod Koul
On Wed, Sep 28, 2016 at 04:15:11PM +0530, Arvind Yadav wrote:
> Free memory mapping, if probe is not successful.

Please use proper subsystem tags for patches. Hint: use git log to find that
out

Applied after fixing the tag

-- 
~Vinod


Re: [PATCH 8/8] dmaengine: Remove site specific OOM error messages on kzalloc

2016-06-15 Thread Vinod Koul
On Wed, Jun 08, 2016 at 02:12:27PM +0200, Linus Walleij wrote:
> On Tue, Jun 7, 2016 at 7:38 PM, Peter Griffin  
> wrote:
> 
> > If kzalloc() fails it will issue it's own error message including
> > a dump_stack(). So remove the site specific error messages.
> >
> > Signed-off-by: Peter Griffin 
> 
> Acked-by: Linus Walleij 
> 
> A few subsystems may use a cleanup like this...
> I wonder how many unnecessary prints I've introduced
> myself :P

Yes that should be case with mine too :)

I think adding a coccinelle script and doing this treewise might be great

Thanks
-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 7/8] dmaengine: tegra20-apb-dma: Only calculate residue if txstate exists.

2016-06-21 Thread Vinod Koul
On Wed, Jun 08, 2016 at 09:51:57AM +0100, Jon Hunter wrote:
> Hi Peter,
> 
> On 07/06/16 18:38, Peter Griffin wrote:
> > There is no point calculating the residue if there is
> > no txstate to store the value.
> > 
> > Signed-off-by: Peter Griffin 
> > ---
> >  drivers/dma/tegra20-apb-dma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> > index 01e316f..7f4af8c 100644
> > --- a/drivers/dma/tegra20-apb-dma.c
> > +++ b/drivers/dma/tegra20-apb-dma.c
> > @@ -814,7 +814,7 @@ static enum dma_status tegra_dma_tx_status(struct 
> > dma_chan *dc,
> > unsigned int residual;
> >  
> > ret = dma_cookie_status(dc, cookie, txstate);
> > -   if (ret == DMA_COMPLETE)
> > +   if (ret == DMA_COMPLETE || !txstate)
> > return ret;
> 
> Thanks for reporting this. I agree that we should not do this, however, 
> looking at the code for Tegra, I am wondering if this could change the
> actual state that is returned. Looking at dma_cookie_status() it will
> call dma_async_is_complete() which will return either DMA_COMPLETE or
> DMA_IN_PROGRESS. It could be possible that the actual state for the
> DMA transfer in the tegra driver is DMA_ERROR, so I am wondering if we
> should do something like the following  ...

This one is stopping code execution when residue is not valid. Do notice
that it check for DMA_COMPLETE OR txstate. In other cases, wit will return
'that' state when txstate is NULL.

I am going to apply this.

> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 01e316f73559..45edab7418d0 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -822,13 +822,8 @@ static enum dma_status tegra_dma_tx_status(struct 
> dma_chan *dc,
> /* Check on wait_ack desc status */
> list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) {
> if (dma_desc->txd.cookie == cookie) {
> -   residual =  dma_desc->bytes_requested -
> -   (dma_desc->bytes_transferred %
> -   dma_desc->bytes_requested);
> -   dma_set_residue(txstate, residual);
> ret = dma_desc->dma_status;
> -   spin_unlock_irqrestore(&tdc->lock, flags);
> -   return ret;
> +   goto found;
> }
> }
>  
> @@ -836,17 +831,23 @@ static enum dma_status tegra_dma_tx_status(struct 
> dma_chan *dc,
> list_for_each_entry(sg_req, &tdc->pending_sg_req, node) {
> dma_desc = sg_req->dma_desc;
> if (dma_desc->txd.cookie == cookie) {
> -   residual =  dma_desc->bytes_requested -
> -   (dma_desc->bytes_transferred %
> -   dma_desc->bytes_requested);
> -   dma_set_residue(txstate, residual);
> ret = dma_desc->dma_status;
> -   spin_unlock_irqrestore(&tdc->lock, flags);
> -   return ret;
> +   goto found;
> }
> }
>  
> -   dev_dbg(tdc2dev(tdc), "cookie %d does not found\n", cookie);
> +   dev_warn(tdc2dev(tdc), "cookie %d not found\n", cookie);
> +   spin_unlock_irqrestore(&tdc->lock, flags);
> +   return ret;
> +
> +found:
> +   if (txstate) {
> +   residual = dma_desc->bytes_requested -
> +  (dma_desc->bytes_transferred %
> +   dma_desc->bytes_requested);
> +   dma_set_residue(txstate, residual);
> +   }
> +

I feel this optimizes stuff, which seems okay. Feel free to send as proper
patch.

> spin_unlock_irqrestore(&tdc->lock, flags);
> return ret;
>  }
> 
> Cheers
> Jon
> 
> -- 
> nvpublic

-- 
~Vinod
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

  1   2   >