* [v2,3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Lucas Stach @ 2018-09-14 17:06 UTC (permalink / raw)
To: Vinod Koul
Cc: dmaengine, linux-kernel, Robin Gong, linux-imx, kernel,
patchwork-lst
The dmaengine documentation states that device_terminate_all may be
asynchronous and need not wait for the active transfers have stopped.
This allows us to move most of the functionality currently implemented
in the sdma channel termination function to run in a worker, outside
of any atomic context. Moving this out of atomic context has two
benefits: we can now sleep while waiting for the channel to terminate,
instead of busy waiting and the freeing of the dma descriptors happens
with IRQs enabled, getting rid of a warning in the dma mapping code.
As the termination is now asnc, we need to implement the
device_synchronize dma engine function, which simply waits for the
worker to finish its execution.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
v2: Keep vchan_get_all_descriptors in the terminate_all call, so the
worker doesn't corrupt the next transfer if that's already in
the setup process.
---
drivers/dma/imx-sdma.c | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8d2fec8b16cc..da41e8fbf151 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -32,6 +32,7 @@
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/of_dma.h>
+#include <linux/workqueue.h>
#include <asm/irq.h>
#include <linux/platform_data/dma-imx-sdma.h>
@@ -375,6 +376,8 @@ struct sdma_channel {
u32 shp_addr, per_addr;
enum dma_status status;
struct imx_dma_data data;
+ struct work_struct terminate_worker;
+ struct list_head deferred_desc_list;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -1025,31 +1028,47 @@ static int sdma_disable_channel(struct dma_chan *chan)
return 0;
}
+static void sdma_channel_terminate(struct work_struct *work)
+{
+ struct sdma_channel *sdmac = container_of(work, struct sdma_channel,
+ terminate_worker);
+
+ /*
+ * According to NXP R&D team a delay of one BD SDMA cost time
+ * (maximum is 1ms) should be added after disable of the channel
+ * bit, to ensure SDMA core has really been stopped after SDMA
+ * clients call .device_terminate_all.
+ */
+ usleep_range(1000, 2000);
+
+ vchan_dma_desc_free_list(&sdmac->vc, &sdmac->deferred_desc_list);
+ INIT_LIST_HEAD(&sdmac->deferred_desc_list);
+}
static int sdma_disable_channel_with_delay(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
unsigned long flags;
- LIST_HEAD(head);
sdma_disable_channel(chan);
+
spin_lock_irqsave(&sdmac->vc.lock, flags);
- vchan_get_all_descriptors(&sdmac->vc, &head);
+ vchan_get_all_descriptors(&sdmac->vc, &sdmac->deferred_desc_list);
sdmac->desc = NULL;
spin_unlock_irqrestore(&sdmac->vc.lock, flags);
- vchan_dma_desc_free_list(&sdmac->vc, &head);
- /*
- * According to NXP R&D team a delay of one BD SDMA cost time
- * (maximum is 1ms) should be added after disable of the channel
- * bit, to ensure SDMA core has really been stopped after SDMA
- * clients call .device_terminate_all.
- */
- mdelay(1);
+ schedule_work(&sdmac->terminate_worker);
return 0;
}
+static void sdma_channel_synchronize(struct dma_chan *chan)
+{
+ struct sdma_channel *sdmac = to_sdma_chan(chan);
+
+ flush_work(&sdmac->terminate_worker);
+}
+
static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
{
struct sdma_engine *sdma = sdmac->sdma;
@@ -1993,6 +2012,8 @@ static int sdma_probe(struct platform_device *pdev)
sdmac->channel = i;
sdmac->vc.desc_free = sdma_desc_free;
+ INIT_WORK(&sdmac->terminate_worker, sdma_channel_terminate);
+ INIT_LIST_HEAD(&sdmac->deferred_desc_list);
/*
* Add the channel to the DMAC list. Do not add channel 0 though
* because we need it internally in the SDMA driver. This also means
@@ -2045,6 +2066,7 @@ static int sdma_probe(struct platform_device *pdev)
sdma->dma_device.device_prep_dma_cyclic = sdma_prep_dma_cyclic;
sdma->dma_device.device_config = sdma_config;
sdma->dma_device.device_terminate_all = sdma_disable_channel_with_delay;
+ sdma->dma_device.device_synchronize = sdma_channel_synchronize;
sdma->dma_device.src_addr_widths = SDMA_DMA_BUSWIDTHS;
sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS;
sdma->dma_device.directions = SDMA_DMA_DIRECTIONS;
^ permalink raw reply related
* [v2,2/4] Revert "dmaengine: imx-sdma: alloclate bd memory from dma pool"
From: Lucas Stach @ 2018-09-14 17:06 UTC (permalink / raw)
To: Vinod Koul
Cc: dmaengine, linux-kernel, Robin Gong, linux-imx, kernel,
patchwork-lst
This reverts commit fe5b85c656bc. The SDMA engine needs the descriptors to
be contiguous in memory. As the dma pool API is only able to provide a
single descriptor per alloc invocation there is no guarantee that multiple
descriptors satisfy this requirement. Also the code in question is broken
as it only allocates memory for a single descriptor, without looking at the
number of descriptors required for the transfer, leading to out-of-bounds
accesses when the descriptors are written.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 3bca5e0c715f..8d2fec8b16cc 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -24,7 +24,6 @@
#include <linux/spinlock.h>
#include <linux/device.h>
#include <linux/dma-mapping.h>
-#include <linux/dmapool.h>
#include <linux/firmware.h>
#include <linux/slab.h>
#include <linux/platform_device.h>
@@ -376,7 +375,6 @@ struct sdma_channel {
u32 shp_addr, per_addr;
enum dma_status status;
struct imx_dma_data data;
- struct dma_pool *bd_pool;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -1192,10 +1190,11 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
static int sdma_alloc_bd(struct sdma_desc *desc)
{
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
int ret = 0;
- desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC,
- &desc->bd_phys);
+ desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
+ GFP_ATOMIC);
if (!desc->bd) {
ret = -ENOMEM;
goto out;
@@ -1206,7 +1205,9 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
static void sdma_free_bd(struct sdma_desc *desc)
{
- dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys);
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
+
+ dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
}
static void sdma_desc_free(struct virt_dma_desc *vd)
@@ -1272,10 +1273,6 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
if (ret)
goto disable_clk_ahb;
- sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev,
- sizeof(struct sdma_buffer_descriptor),
- 32, 0);
-
return 0;
disable_clk_ahb:
@@ -1304,9 +1301,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
clk_disable(sdma->clk_ipg);
clk_disable(sdma->clk_ahb);
-
- dma_pool_destroy(sdmac->bd_pool);
- sdmac->bd_pool = NULL;
}
static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
^ permalink raw reply related
* [v2,1/4] Revert "dmaengine: imx-sdma: Use GFP_NOWAIT for dma allocations"
From: Lucas Stach @ 2018-09-14 17:06 UTC (permalink / raw)
To: Vinod Koul
Cc: dmaengine, linux-kernel, Robin Gong, linux-imx, kernel,
patchwork-lst
This reverts commit c1199875d327, as this depends on another commit
that is going to be reverted.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index b4ec2d20e661..3bca5e0c715f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1194,8 +1194,8 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
{
int ret = 0;
- desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_NOWAIT,
- &desc->bd_phys);
+ desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC,
+ &desc->bd_phys);
if (!desc->bd) {
ret = -ENOMEM;
goto out;
^ permalink raw reply related
* dmaengine: rcar-dmac: set scatter/gather max segment size
From: Wolfram Sang @ 2018-09-14 15:43 UTC (permalink / raw)
To: dmaengine; +Cc: linux-renesas-soc, Geert Uytterhoeven, Wolfram Sang
Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a
device_dma_parameters structure and filling in the max segment size.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Change since RFC: use better value, confirmed with Geert (thanks!)
drivers/dma/sh/rcar-dmac.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 48ee35e2bce6..74fa2b1a6a86 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -198,6 +198,7 @@ struct rcar_dmac {
struct dma_device engine;
struct device *dev;
void __iomem *iomem;
+ struct device_dma_parameters parms;
unsigned int n_channels;
struct rcar_dmac_chan *channels;
@@ -1792,6 +1793,8 @@ static int rcar_dmac_probe(struct platform_device *pdev)
dmac->dev = &pdev->dev;
platform_set_drvdata(pdev, dmac);
+ dmac->dev->dma_parms = &dmac->parms;
+ dma_set_max_seg_size(dmac->dev, RCAR_DMATCR_MASK);
dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40));
ret = rcar_dmac_parse_of(&pdev->dev, dmac);
^ permalink raw reply related
* [RFC] dmaengine: rcar-dmac: set scatter/gather max segment size
From: Wolfram Sang @ 2018-09-14 13:35 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, dmaengine, Linux-Renesas, Yoshihiro Shimoda
Hi Geert,
> > + dmac->dev->dma_parms = &dmac->parms;
> > + dma_set_max_seg_size(dmac->dev, 0x01000000);
>
> That is one too much, cfr.
>
> drivers/dma/sh/rcar-dmac.c:#define RCAR_DMATCR_MASK 0x00ffffff
I see. Will fix and send an updated patch. Nice to see that I was not
totally wrong but just off-by-one.
> And yes, this depends on the data length, which is not known at probe time.
> Of course, dma_set_max_seg_size(dmac->dev, RCAR_DMATCR_MASK)
> should be safe, albeit (slightly) suboptimal.
True. But still *way* better than the current default of 64K.
Thanks for the heads up!
Regards,
Wolfram
^ permalink raw reply
* [-next,v2] dmaengine: mcf-edma: fix x86_64 allmodconfig compilation warning
From: Angelo Dureghello @ 2018-09-14 12:13 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Vinod Koul, dmaengine, Stephen Rothwell
Hi Geert,
On Fri, Sep 14, 2018 at 01:35:22PM +0200, Geert Uytterhoeven wrote:
> Hi Angelo,
>
> Thanks for your patch!
>
> On Fri, Sep 14, 2018 at 9:12 AM Angelo Dureghello <angelo@sysam.it> wrote:
> > This patch fixes the compilation warning reported
> > during x86_64 allmodconfig build.
>
> Please quote the warning, so people know what it is about:
>
> drivers/dma/mcf-edma.c: In function 'mcf_edma_filter_fn':
> drivers/dma/mcf-edma.c:296:33: warning: cast from pointer to
> integer of different size [-Wpointer-to-int-cast]
> return (mcf_chan->slave_id == (u32)param);
>
Ok done.
> > Reported-By: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> > ---
> > Changes for v2:
> > - added Reported-By
> > ---
> > drivers/dma/mcf-edma.c | 3 ++-
> > include/linux/platform_data/dma-mcf-edma.h | 2 +-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c
> > index 4d30d5302649..e08e2409a102 100644
> > --- a/drivers/dma/mcf-edma.c
> > +++ b/drivers/dma/mcf-edma.c
> > @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param)
> > {
> > if (chan->device->dev->driver == &mcf_edma_driver.driver) {
> > struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan);
> > + unsigned int req = *(unsigned int *)param;
>
> This looks a bit hackish to me.
> >
> > - return (mcf_chan->slave_id == (u32)param);
>
> The recommended way to cast from pointers to integers is to
> use uintptr_t, which always has the same size as a pointer, i.e.:
>
slave_id is acually an u32. So, if i understsand, when compiled
on x86_64 using uintptr_t, the cast should trigger the same issue.
I preferred, as i.e. ti/omap dma driver is doing, to simply
use a pointer to an int (int[]) as passed type in param,
so i can then indirectly access the integer. I tested
proper functionality in ColdFire and that there are no warnings
anymore in both ColdFire and x86_64 allmodconfig so the patch,
hopefully, should be legal.
> return (mcf_chan->slave_id == (uintptr_t)param);
>
> > + return (mcf_chan->slave_id == req);
> > }
> >
> > return false;
> > diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
> > index d718ccfa3421..97cb79bda646 100644
> > --- a/include/linux/platform_data/dma-mcf-edma.h
> > +++ b/include/linux/platform_data/dma-mcf-edma.h
> > @@ -21,7 +21,7 @@ struct dma_slave_map;
> >
> > bool mcf_edma_filter_fn(struct dma_chan *chan, void *param);
> >
> > -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch)
> > +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) })
> >
> > /**
> > * struct mcf_edma_platform_data - platform specific data for eDMA engine
>
> Gr{oetje,eeting}s,
>
> Geert
>
Regards,
Angelo
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* [RFC] dmaengine: rcar-dmac: set scatter/gather max segment size
From: Geert Uytterhoeven @ 2018-09-14 12:13 UTC (permalink / raw)
To: Wolfram Sang; +Cc: dmaengine, Linux-Renesas, Yoshihiro Shimoda
Hi Wolfram,
On Thu, Sep 13, 2018 at 4:53 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a
> device_dma_parameters structure and filling in the max segment size.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> According to this discussion [1], this is the intended way of setting
> dma_parms. I am not 100% sure about the value, though. I took the maximum value
> of the DMA Transfer Count Registers which is 16M. I'd think the real maximum
> needs to be multiplied with the data length which is varying, though? Geert,
> what do you think?
>
> [1] https://www.spinics.net/lists/iommu/msg29861.html
>
> drivers/dma/sh/rcar-dmac.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 48ee35e2bce6..bb3631f6c41c 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -198,6 +198,7 @@ struct rcar_dmac {
> struct dma_device engine;
> struct device *dev;
> void __iomem *iomem;
> + struct device_dma_parameters parms;
>
> unsigned int n_channels;
> struct rcar_dmac_chan *channels;
> @@ -1792,6 +1793,8 @@ static int rcar_dmac_probe(struct platform_device *pdev)
>
> dmac->dev = &pdev->dev;
> platform_set_drvdata(pdev, dmac);
> + dmac->dev->dma_parms = &dmac->parms;
> + dma_set_max_seg_size(dmac->dev, 0x01000000);
That is one too much, cfr.
drivers/dma/sh/rcar-dmac.c:#define RCAR_DMATCR_MASK 0x00ffffff
And commit d716d9b702bb759d ("dmaengine: rcar-dmac: fix max_chunk_size for
R-Car Gen3"), which did:
- max_chunk_size = (RCAR_DMATCR_MASK + 1) << desc->xfer_shift;
+ max_chunk_size = RCAR_DMATCR_MASK << desc->xfer_shift;
And yes, this depends on the data length, which is not known at probe time.
Of course, dma_set_max_seg_size(dmac->dev, RCAR_DMATCR_MASK)
should be safe, albeit (slightly) suboptimal.
But I doubt anyone is really doing such large transfers (up to
(16 Mi - 1) * 64 bytes!).
> dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40));
>
> ret = rcar_dmac_parse_of(&pdev->dev, dmac);
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* [-next,v2] dmaengine: mcf-edma: fix x86_64 allmodconfig compilation warning
From: Vinod Koul @ 2018-09-14 11:48 UTC (permalink / raw)
To: Angelo Dureghello; +Cc: dmaengine, sfr
On 14-09-18, 09:11, Angelo Dureghello wrote:
> This patch fixes the compilation warning reported
> during x86_64 allmodconfig build.
How does it do so, I have no clue what to expect. Please describe the
change done here in the log...
Patch title also doesn't tell me anything about the fix.
>
> Reported-By: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
> Changes for v2:
> - added Reported-By
> ---
> drivers/dma/mcf-edma.c | 3 ++-
> include/linux/platform_data/dma-mcf-edma.h | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c
> index 4d30d5302649..e08e2409a102 100644
> --- a/drivers/dma/mcf-edma.c
> +++ b/drivers/dma/mcf-edma.c
> @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param)
> {
> if (chan->device->dev->driver == &mcf_edma_driver.driver) {
> struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan);
> + unsigned int req = *(unsigned int *)param;
>
> - return (mcf_chan->slave_id == (u32)param);
> + return (mcf_chan->slave_id == req);
> }
>
> return false;
> diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
> index d718ccfa3421..97cb79bda646 100644
> --- a/include/linux/platform_data/dma-mcf-edma.h
> +++ b/include/linux/platform_data/dma-mcf-edma.h
> @@ -21,7 +21,7 @@ struct dma_slave_map;
>
> bool mcf_edma_filter_fn(struct dma_chan *chan, void *param);
>
> -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch)
> +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) })
I dont think you answered me about this, why is this change required and
in the context of current patch, what does it fix?
^ permalink raw reply
* [-next,v2] dmaengine: mcf-edma: fix x86_64 allmodconfig compilation warning
From: Geert Uytterhoeven @ 2018-09-14 11:35 UTC (permalink / raw)
To: Angelo Dureghello; +Cc: Vinod Koul, dmaengine, Stephen Rothwell
Hi Angelo,
Thanks for your patch!
On Fri, Sep 14, 2018 at 9:12 AM Angelo Dureghello <angelo@sysam.it> wrote:
> This patch fixes the compilation warning reported
> during x86_64 allmodconfig build.
Please quote the warning, so people know what it is about:
drivers/dma/mcf-edma.c: In function 'mcf_edma_filter_fn':
drivers/dma/mcf-edma.c:296:33: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
return (mcf_chan->slave_id == (u32)param);
> Reported-By: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
> Changes for v2:
> - added Reported-By
> ---
> drivers/dma/mcf-edma.c | 3 ++-
> include/linux/platform_data/dma-mcf-edma.h | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c
> index 4d30d5302649..e08e2409a102 100644
> --- a/drivers/dma/mcf-edma.c
> +++ b/drivers/dma/mcf-edma.c
> @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param)
> {
> if (chan->device->dev->driver == &mcf_edma_driver.driver) {
> struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan);
> + unsigned int req = *(unsigned int *)param;
This looks a bit hackish to me.
>
> - return (mcf_chan->slave_id == (u32)param);
The recommended way to cast from pointers to integers is to
use uintptr_t, which always has the same size as a pointer, i.e.:
return (mcf_chan->slave_id == (uintptr_t)param);
> + return (mcf_chan->slave_id == req);
> }
>
> return false;
> diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
> index d718ccfa3421..97cb79bda646 100644
> --- a/include/linux/platform_data/dma-mcf-edma.h
> +++ b/include/linux/platform_data/dma-mcf-edma.h
> @@ -21,7 +21,7 @@ struct dma_slave_map;
>
> bool mcf_edma_filter_fn(struct dma_chan *chan, void *param);
>
> -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch)
> +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) })
>
> /**
> * struct mcf_edma_platform_data - platform specific data for eDMA engine
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Lucas Stach @ 2018-09-14 9:42 UTC (permalink / raw)
To: Robin Gong, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
Hi Robin,
Am Mittwoch, den 12.09.2018, 09:47 +0000 schrieb Robin Gong:
> My testcase is simple loopback test as attached. Are you sure your serial port running in DMA mode?
> By default, DMA not enabled on i.mx7d, you can enable it in uart device node of dts as below:
> + dmas = <&sdma 32 4 0>, <&sdma 33 4 0>;
> + dma-names = "rx", "tx"
> Please flow below command(ttymxc5 on i.mx7d-sdb board) to run loopback test:
> ./mxc_uart_stress_test.out /dev/ttymxc5 2000000 D L 100 100 L
Thanks. With this testcase I was able to reproduce the issue on i.MX6.
The v2 patchset I just sent out fixed this, so should hopefully be good
enough to go in now.
Regards,
Lucas
> > -----Original Message-----
> > > > From: Lucas Stach <l.stach@pengutronix.de>
> > Sent: 2018年9月12日 17:22
> > > > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> > > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > termination via worker
> >
> > Hi Robin,
> >
> > Am Dienstag, den 11.09.2018, 08:18 +0000 schrieb Robin Gong:
> > > Hi Lucas,
> > > I have quick test for UART, seems this patch set broke uart function
> > > on i.mx7d, I believe it should be same on other i.mx6 family. Could you double
> >
> > check it?
> >
> > Can you describe your testcase and the issue you've seen in more detail? I'm
> > running this patch series on a system that does a lot of communication with an
> > UART attached Co-processor and I'm not seeing any issues.
> >
> > Regards,
> > Lucas
> >
> > > > -----Original Message-----
> > > > > > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > Sent: 2018年9月10日 17:59
> > > > > > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> > > > > > > > > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > > kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > > > termination via worker
> > > >
> > > > Am Dienstag, den 04.09.2018, 02:36 +0000 schrieb Robin Gong:
> > > > > > -----Original Message-----
> > > > > > > > > > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > > > Sent: 2018年9月3日 21:12
> > > > > > > > > > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.o
> > > > > > rg>
> > > > > > > > > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > > > > > > > > > > > ; kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > > > > > termination via worker
> > > > > >
> > > > > > Am Montag, den 03.09.2018, 08:59 +0000 schrieb Robin Gong:
> > > > > > > > -----Original Message-----
> > > > > > > > > > > > > > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > > > > > Sent: 2018年9月3日 16:41
> > > > > > > > > > > > > > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kern
> > > > > > > > el.o
> > > > > > > > rg>
> > > > > > > > > > > > > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.
> > > > > > > > com>
> > > > > > > > > > > > > > > > ; kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > > > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement
> > > > > > > > channel termination via worker
> > > > > > > >
> > > > > > > > Hi Robin,
> > > > > > > >
> > > > > > > > Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong:
> > > > > > > > > Hi Lucas,
> > > > > > > > > Seems I miss your previous mail. Thanks for your patch,
> > > > > > > > > but if move most jobs of sdma_disable_channel_with_delay()
> > > > > > > > > into worker, that will bring another race condition that
> > > > > > > > > upper driver such as Audio terminate channel and free
> > > > > > > > > resource of dma channel without really channel stop, if
> > > > > > > > > dma transfer done interrupt come after that, oops or
> > > > > > > > > kernel cash may be caught.
> > > > > > > > > Leave 'sdmac->desc = NULL' in the
> > > > > > > >
> > > > > > > > sdma_disable_channel_with_delay() may fix such potential
> > > > > > > > issue.
> > > > > > > >
> > > > > > > > No, there is no such issue. The audio channel terminate will
> > > > > > > > call dmaengine_terminate_sync(), which internally calls
> > > > > > > > dmaengine_terminate_async() and then does a
> > > > > > > > dmaengine_synchronize().
> > > > > > > > As this patchset implements the device_synchronize function
> > > > > > > > in the sdma driver, this will wait for the worker to finish
> > > > > > > > its execution, so there is no race condition to worry about
> > > > > > > > here.
> > > > > > > >
> > > > > > > > Regards,
> > > > > > > > Lucas
> > > > > > >
> > > > > > > Yes, but how about other drivers which not call
> > > > > > > dmaengine_terminate_sync()?
> > > > > >
> > > > > > Please read the dmaengine documentation. device_terminate_all
> > > > > > has no requirement that the transfer is actually canceled when
> > > > > > the call returns. If the caller needs a guarantee that the
> > > > > > channel is stopped it _must_ call device_synchronize.
> > > > >
> > > > > I know that, but the fact is some driver still use
> > > > > dmaengine_terminate_all() such as
> > > > > Spi/uart driver. My concern is how to avoid to break their
> > > > > function.
> > > >
> > > > They should simply be fixed to not use a deprecated function. Both
> > > > of those are only using device_terminate_all in error or shutdown
> > > > paths, so the risk of races is pretty minimal even with the current
> > > > code. And I think the SPI driver is trivial to fix, as we can just
> > > > use the terminate_sync variant there. The UART driver is a bit more
> > > > tricky.
> > > >
> > > > Regards,
> > > > Lucas
^ permalink raw reply
* [1/2] ACPI / LPSS: Add alternative ACPI HIDs for Cherry Trail DMA controllers
From: Rafael J. Wysocki @ 2018-09-14 8:40 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede
Cc: Len Brown, Viresh Kumar, Vinod Koul, linux-acpi, Dan Williams,
dmaengine
On Wednesday, August 29, 2018 6:50:53 PM CEST Andy Shevchenko wrote:
> On Mon, Aug 27, 2018 at 09:45:44AM +0200, Hans de Goede wrote:
> > Bay and Cherry Trail DSTDs represent a different set of devices depending
> > on which OS the device think it is booting. One set of decices for Windows
> > and another set of devices for Android which targets the Android-x86 Linux
> > kernel fork (which e.g. used to have its own display driver instead of
> > using the i915 driver).
> >
> > Which set of devices we are actually going to get is out of our control,
> > this is controlled by the ACPI OSID variable, which gets either set through
> > an EFI setup option, or sometimes is autodetected. So we need to support
> > both.
> >
> > This commit adds support for the 80862286 and 808622C0 ACPI HIDs which we
> > get for the first resp. second DMA controller on Cherry Trail devices when
> > OSID is set to Android.
> >
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> > drivers/acpi/acpi_lpss.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> > index 0569972b02e1..95d9c45e13cc 100644
> > --- a/drivers/acpi/acpi_lpss.c
> > +++ b/drivers/acpi/acpi_lpss.c
> > @@ -327,9 +327,11 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = {
> > { "INT33FC", },
> >
> > /* Braswell LPSS devices */
> > + { "80862286", LPSS_ADDR(lpss_dma_desc) },
> > { "80862288", LPSS_ADDR(bsw_pwm_dev_desc) },
> > { "8086228A", LPSS_ADDR(bsw_uart_dev_desc) },
> > { "8086228E", LPSS_ADDR(bsw_spi_dev_desc) },
> > + { "808622C0", LPSS_ADDR(lpss_dma_desc) },
> > { "808622C1", LPSS_ADDR(bsw_i2c_dev_desc) },
> >
> > /* Broadwell LPSS devices */
>
>
Patch applied, thanks!
^ permalink raw reply
* [-next,v2] dmaengine: mcf-edma: fix x86_64 allmodconfig compilation warning
From: Angelo Dureghello @ 2018-09-14 7:11 UTC (permalink / raw)
To: vinod.koul; +Cc: dmaengine, sfr, Angelo Dureghello
This patch fixes the compilation warning reported
during x86_64 allmodconfig build.
Reported-By: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Angelo Dureghello <angelo@sysam.it>
---
Changes for v2:
- added Reported-By
---
drivers/dma/mcf-edma.c | 3 ++-
include/linux/platform_data/dma-mcf-edma.h | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c
index 4d30d5302649..e08e2409a102 100644
--- a/drivers/dma/mcf-edma.c
+++ b/drivers/dma/mcf-edma.c
@@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param)
{
if (chan->device->dev->driver == &mcf_edma_driver.driver) {
struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan);
+ unsigned int req = *(unsigned int *)param;
- return (mcf_chan->slave_id == (u32)param);
+ return (mcf_chan->slave_id == req);
}
return false;
diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
index d718ccfa3421..97cb79bda646 100644
--- a/include/linux/platform_data/dma-mcf-edma.h
+++ b/include/linux/platform_data/dma-mcf-edma.h
@@ -21,7 +21,7 @@ struct dma_slave_map;
bool mcf_edma_filter_fn(struct dma_chan *chan, void *param);
-#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch)
+#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) })
/**
* struct mcf_edma_platform_data - platform specific data for eDMA engine
^ permalink raw reply related
* [-next] dmaengine: mcf-edma: fix x86_64 allmodconfig compilation warning
From: Vinod Koul @ 2018-09-14 6:29 UTC (permalink / raw)
To: Angelo Dureghello; +Cc: dmaengine, sfr
On 13-09-18, 20:47, Angelo Dureghello wrote:
> This patch fixes the compilation warning reported
> during x86_64 allmodconfig build.
>
Please do add credit to Stephan for reporting by using Reported-By tag
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
> drivers/dma/mcf-edma.c | 3 ++-
> include/linux/platform_data/dma-mcf-edma.h | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c
> index 4d30d5302649..e08e2409a102 100644
> --- a/drivers/dma/mcf-edma.c
> +++ b/drivers/dma/mcf-edma.c
> @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param)
> {
> if (chan->device->dev->driver == &mcf_edma_driver.driver) {
> struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan);
> + unsigned int req = *(unsigned int *)param;
>
> - return (mcf_chan->slave_id == (u32)param);
> + return (mcf_chan->slave_id == req);
> }
>
> return false;
> diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
> index d718ccfa3421..97cb79bda646 100644
> --- a/include/linux/platform_data/dma-mcf-edma.h
> +++ b/include/linux/platform_data/dma-mcf-edma.h
> @@ -21,7 +21,7 @@ struct dma_slave_map;
>
> bool mcf_edma_filter_fn(struct dma_chan *chan, void *param);
>
> -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch)
> +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) })
Why this change
^ permalink raw reply
* [-next] dmaengine: mcf-edma: fix x86_64 allmodconfig compilation warning
From: Angelo Dureghello @ 2018-09-13 18:47 UTC (permalink / raw)
To: vinod.koul; +Cc: dmaengine, sfr, Angelo Dureghello
This patch fixes the compilation warning reported
during x86_64 allmodconfig build.
Signed-off-by: Angelo Dureghello <angelo@sysam.it>
---
drivers/dma/mcf-edma.c | 3 ++-
include/linux/platform_data/dma-mcf-edma.h | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c
index 4d30d5302649..e08e2409a102 100644
--- a/drivers/dma/mcf-edma.c
+++ b/drivers/dma/mcf-edma.c
@@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param)
{
if (chan->device->dev->driver == &mcf_edma_driver.driver) {
struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan);
+ unsigned int req = *(unsigned int *)param;
- return (mcf_chan->slave_id == (u32)param);
+ return (mcf_chan->slave_id == req);
}
return false;
diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h
index d718ccfa3421..97cb79bda646 100644
--- a/include/linux/platform_data/dma-mcf-edma.h
+++ b/include/linux/platform_data/dma-mcf-edma.h
@@ -21,7 +21,7 @@ struct dma_slave_map;
bool mcf_edma_filter_fn(struct dma_chan *chan, void *param);
-#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch)
+#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) })
/**
* struct mcf_edma_platform_data - platform specific data for eDMA engine
^ permalink raw reply related
* [RFC] dmaengine: rcar-dmac: set scatter/gather max segment size
From: Wolfram Sang @ 2018-09-13 14:52 UTC (permalink / raw)
To: dmaengine; +Cc: linux-renesas-soc, Geert Uytterhoeven, Wolfram Sang
Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a
device_dma_parameters structure and filling in the max segment size.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
According to this discussion [1], this is the intended way of setting
dma_parms. I am not 100% sure about the value, though. I took the maximum value
of the DMA Transfer Count Registers which is 16M. I'd think the real maximum
needs to be multiplied with the data length which is varying, though? Geert,
what do you think?
[1] https://www.spinics.net/lists/iommu/msg29861.html
drivers/dma/sh/rcar-dmac.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 48ee35e2bce6..bb3631f6c41c 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -198,6 +198,7 @@ struct rcar_dmac {
struct dma_device engine;
struct device *dev;
void __iomem *iomem;
+ struct device_dma_parameters parms;
unsigned int n_channels;
struct rcar_dmac_chan *channels;
@@ -1792,6 +1793,8 @@ static int rcar_dmac_probe(struct platform_device *pdev)
dmac->dev = &pdev->dev;
platform_set_drvdata(pdev, dmac);
+ dmac->dev->dma_parms = &dmac->parms;
+ dma_set_max_seg_size(dmac->dev, 0x01000000);
dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40));
ret = rcar_dmac_parse_of(&pdev->dev, dmac);
^ permalink raw reply related
* [01/12] dmaengine: at_hdmac: remove dma_slave_config direction usage
From: Ludovic Desroches @ 2018-09-13 8:23 UTC (permalink / raw)
To: Vinod Koul; +Cc: dmaengine
On Tue, Sep 11, 2018 at 02:05:25PM +0530, Vinod Koul wrote:
> dma_slave_config direction was marked as deprecated quite some
> time back, remove the usage from this driver so that the field
> can be removed
>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
> CC: Ludovic Desroches <ludovic.desroches@microchip.com>
>
> drivers/dma/at_hdmac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 75f38d19fcbe..7cbac6e8c113 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -1320,7 +1320,7 @@ atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> if (unlikely(!is_slave_direction(direction)))
> goto err_out;
>
> - if (sconfig->direction == DMA_MEM_TO_DEV)
> + if (direction == DMA_MEM_TO_DEV)
> reg_width = convert_buswidth(sconfig->dst_addr_width);
> else
> reg_width = convert_buswidth(sconfig->src_addr_width);
> --
> 2.14.4
>
^ permalink raw reply
* [v3,2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver
From: Masahiro Yamada @ 2018-09-13 0:51 UTC (permalink / raw)
To: Vinod Koul, dmaengine
Cc: Masami Hiramatsu, Jassi Brar, Masahiro Yamada, linux-kernel,
Dan Williams, linux-arm-kernel
The MIO DMAC (Media IO DMA Controller) is used in UniPhier LD4,
Pro4, and sLD8 SoCs.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Changes in v3:
- Get residue from on-flight chunk
- Use GFP_NOWAIT instead of GFP_KERNEL in prep_slave_sg
- Use device_config hook
- Add NULL pointer check for txstate
Changes in v2:
- Use platform_irq_count() to get the number of channels
MAINTAINERS | 1 +
drivers/dma/Kconfig | 11 +
drivers/dma/Makefile | 1 +
drivers/dma/uniphier-mdmac.c | 495 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 508 insertions(+)
create mode 100644 drivers/dma/uniphier-mdmac.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 82ea427..5c0794a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2193,6 +2193,7 @@ F: arch/arm/mm/cache-uniphier.c
F: arch/arm64/boot/dts/socionext/uniphier*
F: drivers/bus/uniphier-system-bus.c
F: drivers/clk/uniphier/
+F: drivers/dmaengine/uniphier-mdmac.c
F: drivers/gpio/gpio-uniphier.c
F: drivers/i2c/busses/i2c-uniphier*
F: drivers/irqchip/irq-uniphier-aidet.c
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index dacf3f4..8b8c7f0 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -576,6 +576,17 @@ config TIMB_DMA
help
Enable support for the Timberdale FPGA DMA engine.
+config UNIPHIER_MDMAC
+ tristate "UniPhier MIO DMAC"
+ depends on ARCH_UNIPHIER || COMPILE_TEST
+ depends on OF
+ select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
+ help
+ Enable support for the MIO DMAC (Media I/O DMA controller) on the
+ UniPhier platform. This DMA controller is used as the external
+ DMA engine of the SD/eMMC controllers of the LD4, Pro4, sLD8 SoCs.
+
config XGENE_DMA
tristate "APM X-Gene DMA support"
depends on ARCH_XGENE || COMPILE_TEST
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index c91702d..973a170 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o
obj-$(CONFIG_TEGRA20_APB_DMA) += tegra20-apb-dma.o
obj-$(CONFIG_TEGRA210_ADMA) += tegra210-adma.o
obj-$(CONFIG_TIMB_DMA) += timb_dma.o
+obj-$(CONFIG_UNIPHIER_MDMAC) += uniphier-mdmac.o
obj-$(CONFIG_XGENE_DMA) += xgene-dma.o
obj-$(CONFIG_ZX_DMA) += zx_dma.o
obj-$(CONFIG_ST_FDMA) += st_fdma.o
diff --git a/drivers/dma/uniphier-mdmac.c b/drivers/dma/uniphier-mdmac.c
new file mode 100644
index 0000000..e620dbf
--- /dev/null
+++ b/drivers/dma/uniphier-mdmac.c
@@ -0,0 +1,495 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2018 Socionext Inc.
+// Author: Masahiro Yamada <yamada.masahiro@socionext.com>
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "virt-dma.h"
+
+/* registers common for all channels */
+#define UNIPHIER_MDMAC_CMD 0x000 // issue DMA start/abort
+#define UNIPHIER_MDMAC_CMD_ABORT BIT(31) // 1: abort, 0: start
+
+/* per-channel registers */
+#define UNIPHIER_MDMAC_CH_OFFSET 0x100
+#define UNIPHIER_MDMAC_CH_STRIDE 0x040
+
+#define UNIPHIER_MDMAC_CH_IRQ_STAT 0x010 // current hw status (RO)
+#define UNIPHIER_MDMAC_CH_IRQ_REQ 0x014 // latched STAT (WOC)
+#define UNIPHIER_MDMAC_CH_IRQ_EN 0x018 // IRQ enable mask
+#define UNIPHIER_MDMAC_CH_IRQ_DET 0x01c // REQ & EN (RO)
+#define UNIPHIER_MDMAC_CH_IRQ__ABORT BIT(13)
+#define UNIPHIER_MDMAC_CH_IRQ__DONE BIT(1)
+#define UNIPHIER_MDMAC_CH_SRC_MODE 0x020 // mode of source
+#define UNIPHIER_MDMAC_CH_DEST_MODE 0x024 // mode of destination
+#define UNIPHIER_MDMAC_CH_MODE__ADDR_INC (0 << 4)
+#define UNIPHIER_MDMAC_CH_MODE__ADDR_DEC (1 << 4)
+#define UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED (2 << 4)
+#define UNIPHIER_MDMAC_CH_SRC_ADDR 0x028 // source address
+#define UNIPHIER_MDMAC_CH_DEST_ADDR 0x02c // destination address
+#define UNIPHIER_MDMAC_CH_SIZE 0x030 // transfer bytes
+
+struct uniphier_mdmac_desc {
+ struct virt_dma_desc vd;
+ struct scatterlist *sgl;
+ unsigned int sg_len;
+ unsigned int sg_cur;
+ enum dma_transfer_direction dir;
+};
+
+struct uniphier_mdmac_chan {
+ struct virt_dma_chan vc;
+ struct uniphier_mdmac_device *mdev;
+ struct uniphier_mdmac_desc *md;
+ void __iomem *reg_ch_base;
+ unsigned int chan_id;
+};
+
+struct uniphier_mdmac_device {
+ struct dma_device ddev;
+ struct clk *clk;
+ void __iomem *reg_base;
+ struct uniphier_mdmac_chan channels[0];
+};
+
+static struct uniphier_mdmac_chan *to_uniphier_mdmac_chan(
+ struct virt_dma_chan *vc)
+{
+ return container_of(vc, struct uniphier_mdmac_chan, vc);
+}
+
+static struct uniphier_mdmac_desc *to_uniphier_mdmac_desc(
+ struct virt_dma_desc *vd)
+{
+ return container_of(vd, struct uniphier_mdmac_desc, vd);
+}
+
+/* mc->vc.lock must be held by caller */
+static struct uniphier_mdmac_desc *__uniphier_mdmac_next_desc(
+ struct uniphier_mdmac_chan *mc)
+{
+ struct virt_dma_desc *vd;
+
+ vd = vchan_next_desc(&mc->vc);
+ if (!vd) {
+ mc->md = NULL;
+ return NULL;
+ }
+
+ list_del(&vd->node);
+
+ mc->md = to_uniphier_mdmac_desc(vd);
+
+ return mc->md;
+}
+
+/* mc->vc.lock must be held by caller */
+static void __uniphier_mdmac_handle(struct uniphier_mdmac_chan *mc,
+ struct uniphier_mdmac_desc *md)
+{
+ struct uniphier_mdmac_device *mdev = mc->mdev;
+ struct scatterlist *sg;
+ u32 irq_flag = UNIPHIER_MDMAC_CH_IRQ__DONE;
+ u32 src_mode, src_addr, dest_mode, dest_addr, chunk_size;
+
+ sg = &md->sgl[md->sg_cur];
+
+ if (md->dir == DMA_MEM_TO_DEV) {
+ src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
+ src_addr = sg_dma_address(sg);
+ dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
+ dest_addr = 0;
+ } else {
+ src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
+ src_addr = 0;
+ dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
+ dest_addr = sg_dma_address(sg);
+ }
+
+ chunk_size = sg_dma_len(sg);
+
+ writel(src_mode, mc->reg_ch_base + UNIPHIER_MDMAC_CH_SRC_MODE);
+ writel(dest_mode, mc->reg_ch_base + UNIPHIER_MDMAC_CH_DEST_MODE);
+ writel(src_addr, mc->reg_ch_base + UNIPHIER_MDMAC_CH_SRC_ADDR);
+ writel(dest_addr, mc->reg_ch_base + UNIPHIER_MDMAC_CH_DEST_ADDR);
+ writel(chunk_size, mc->reg_ch_base + UNIPHIER_MDMAC_CH_SIZE);
+
+ /* write 1 to clear */
+ writel(irq_flag, mc->reg_ch_base + UNIPHIER_MDMAC_CH_IRQ_REQ);
+
+ writel(irq_flag, mc->reg_ch_base + UNIPHIER_MDMAC_CH_IRQ_EN);
+
+ writel(BIT(mc->chan_id), mdev->reg_base + UNIPHIER_MDMAC_CMD);
+}
+
+/* mc->vc.lock must be held by caller */
+static void __uniphier_mdmac_start(struct uniphier_mdmac_chan *mc)
+{
+ struct uniphier_mdmac_desc *md;
+
+ md = __uniphier_mdmac_next_desc(mc);
+ if (md)
+ __uniphier_mdmac_handle(mc, md);
+}
+
+/* mc->vc.lock must be held by caller */
+static int __uniphier_mdmac_abort(struct uniphier_mdmac_chan *mc)
+{
+ struct uniphier_mdmac_device *mdev = mc->mdev;
+ u32 irq_flag = UNIPHIER_MDMAC_CH_IRQ__ABORT;
+ u32 val;
+
+ /* write 1 to clear */
+ writel(irq_flag, mc->reg_ch_base + UNIPHIER_MDMAC_CH_IRQ_REQ);
+
+ writel(UNIPHIER_MDMAC_CMD_ABORT | BIT(mc->chan_id),
+ mdev->reg_base + UNIPHIER_MDMAC_CMD);
+
+ /*
+ * Abort should be accepted soon. We poll the bit here instead of
+ * waiting for the interrupt.
+ */
+ return readl_poll_timeout(mc->reg_ch_base + UNIPHIER_MDMAC_CH_IRQ_REQ,
+ val, val & irq_flag, 0, 20);
+}
+
+/* mc->vc.lock must be held by caller */
+static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_chan *mc,
+ struct uniphier_mdmac_desc *md)
+{
+ u32 residue;
+ int i;
+
+ /* residue from the on-flight chunk */
+ residue = readl(mc->reg_ch_base + UNIPHIER_MDMAC_CH_SIZE);
+
+ /* residue from the queued chunks */
+ for (i = md->sg_cur; i < md->sg_len; i++)
+ residue += sg_dma_len(&md->sgl[i]);
+
+ return residue;
+}
+
+static irqreturn_t uniphier_mdmac_interrupt(int irq, void *dev_id)
+{
+ struct uniphier_mdmac_chan *mc = dev_id;
+ struct uniphier_mdmac_desc *md;
+ irqreturn_t ret = IRQ_HANDLED;
+ u32 irq_stat;
+
+ spin_lock(&mc->vc.lock);
+
+ irq_stat = readl(mc->reg_ch_base + UNIPHIER_MDMAC_CH_IRQ_DET);
+
+ /*
+ * Some channels share a single interrupt line. If the IRQ status is 0,
+ * this is probably triggered by a different channel.
+ */
+ if (!irq_stat) {
+ ret = IRQ_NONE;
+ goto out;
+ }
+
+ /* write 1 to clear */
+ writel(irq_stat, mc->reg_ch_base + UNIPHIER_MDMAC_CH_IRQ_REQ);
+
+ /*
+ * UNIPHIER_MDMAC_CH_IRQ__DONE interrupt is asserted even when the DMA
+ * is aborted. To distinguish the normal completion and the abort,
+ * check mc->md. If it is NULL, we are aborting.
+ */
+ md = mc->md;
+ if (!md)
+ goto out;
+
+ md->sg_cur++;
+
+ if (md->sg_cur >= md->sg_len) {
+ vchan_cookie_complete(&md->vd);
+ md = __uniphier_mdmac_next_desc(mc);
+ if (!md)
+ goto out;
+ }
+
+ __uniphier_mdmac_handle(mc, md);
+
+out:
+ spin_unlock(&mc->vc.lock);
+
+ return ret;
+}
+
+static struct dma_async_tx_descriptor *uniphier_mdmac_prep_slave_sg(
+ struct dma_chan *chan,
+ struct scatterlist *sgl,
+ unsigned int sg_len,
+ enum dma_transfer_direction direction,
+ unsigned long flags, void *context)
+{
+ struct virt_dma_chan *vc = to_virt_chan(chan);
+ struct uniphier_mdmac_desc *md;
+
+ if (!is_slave_direction(direction))
+ return NULL;
+
+ md = kzalloc(sizeof(*md), GFP_NOWAIT);
+ if (!md)
+ return NULL;
+
+ md->sgl = sgl;
+ md->sg_len = sg_len;
+ md->dir = direction;
+
+ return vchan_tx_prep(vc, &md->vd, flags);
+}
+
+static int uniphier_mdmac_config(struct dma_chan *chan,
+ struct dma_slave_config *config)
+{
+ /* Nothing in struct dma_slave_config is configurable. */
+ return 0;
+}
+
+static int uniphier_mdmac_terminate_all(struct dma_chan *chan)
+{
+ struct virt_dma_chan *vc = to_virt_chan(chan);
+ struct uniphier_mdmac_chan *mc = to_uniphier_mdmac_chan(vc);
+ unsigned long flags;
+ int ret = 0;
+ LIST_HEAD(head);
+
+ spin_lock_irqsave(&vc->lock, flags);
+
+ if (mc->md) {
+ vchan_terminate_vdesc(&mc->md->vd);
+ mc->md = NULL;
+ ret = __uniphier_mdmac_abort(mc);
+ }
+ vchan_get_all_descriptors(vc, &head);
+
+ spin_unlock_irqrestore(&vc->lock, flags);
+
+ vchan_dma_desc_free_list(vc, &head);
+
+ return ret;
+}
+
+static void uniphier_mdmac_synchronize(struct dma_chan *chan)
+{
+ vchan_synchronize(to_virt_chan(chan));
+}
+
+static enum dma_status uniphier_mdmac_tx_status(struct dma_chan *chan,
+ dma_cookie_t cookie,
+ struct dma_tx_state *txstate)
+{
+ struct virt_dma_chan *vc;
+ struct virt_dma_desc *vd;
+ struct uniphier_mdmac_chan *mc;
+ struct uniphier_mdmac_desc *md = NULL;
+ enum dma_status stat;
+ unsigned long flags;
+
+ stat = dma_cookie_status(chan, cookie, txstate);
+ /* Return immediately if we do not need to compute the residue. */
+ if (stat == DMA_COMPLETE || !txstate)
+ return stat;
+
+ vc = to_virt_chan(chan);
+
+ spin_lock_irqsave(&vc->lock, flags);
+
+ mc = to_uniphier_mdmac_chan(vc);
+
+ if (mc->md && mc->md->vd.tx.cookie == cookie)
+ md = mc->md;
+
+ if (!md) {
+ vd = vchan_find_desc(vc, cookie);
+ if (vd)
+ md = to_uniphier_mdmac_desc(vd);
+ }
+
+ if (md)
+ txstate->residue = __uniphier_mdmac_get_residue(mc, md);
+
+ spin_unlock_irqrestore(&vc->lock, flags);
+
+ return stat;
+}
+
+static void uniphier_mdmac_issue_pending(struct dma_chan *chan)
+{
+ struct virt_dma_chan *vc = to_virt_chan(chan);
+ struct uniphier_mdmac_chan *mc = to_uniphier_mdmac_chan(vc);
+ unsigned long flags;
+
+ spin_lock_irqsave(&vc->lock, flags);
+
+ if (vchan_issue_pending(vc) && !mc->md)
+ __uniphier_mdmac_start(mc);
+
+ spin_unlock_irqrestore(&vc->lock, flags);
+}
+
+static void uniphier_mdmac_desc_free(struct virt_dma_desc *vd)
+{
+ kfree(to_uniphier_mdmac_desc(vd));
+}
+
+static int uniphier_mdmac_chan_init(struct platform_device *pdev,
+ struct uniphier_mdmac_device *mdev,
+ int chan_id)
+{
+ struct device *dev = &pdev->dev;
+ struct uniphier_mdmac_chan *mc = &mdev->channels[chan_id];
+ char *irq_name;
+ int irq, ret;
+
+ irq = platform_get_irq(pdev, chan_id);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "failed to get IRQ number for ch%d\n",
+ chan_id);
+ return irq;
+ }
+
+ irq_name = devm_kasprintf(dev, GFP_KERNEL, "uniphier-mio-dmac-ch%d",
+ chan_id);
+ if (!irq_name)
+ return -ENOMEM;
+
+ ret = devm_request_irq(dev, irq, uniphier_mdmac_interrupt,
+ IRQF_SHARED, irq_name, mc);
+ if (ret)
+ return ret;
+
+ mc->mdev = mdev;
+ mc->reg_ch_base = mdev->reg_base + UNIPHIER_MDMAC_CH_OFFSET +
+ UNIPHIER_MDMAC_CH_STRIDE * chan_id;
+ mc->chan_id = chan_id;
+ mc->vc.desc_free = uniphier_mdmac_desc_free;
+ vchan_init(&mc->vc, &mdev->ddev);
+
+ return 0;
+}
+
+static int uniphier_mdmac_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct uniphier_mdmac_device *mdev;
+ struct dma_device *ddev;
+ struct resource *res;
+ int nr_chans, ret, i;
+
+ nr_chans = platform_irq_count(pdev);
+ if (nr_chans < 0)
+ return nr_chans;
+
+ ret = dma_set_mask(dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
+
+ mdev = devm_kzalloc(dev, struct_size(mdev, channels, nr_chans),
+ GFP_KERNEL);
+ if (!mdev)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mdev->reg_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mdev->reg_base))
+ return PTR_ERR(mdev->reg_base);
+
+ mdev->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(mdev->clk)) {
+ dev_err(dev, "failed to get clock\n");
+ return PTR_ERR(mdev->clk);
+ }
+
+ ret = clk_prepare_enable(mdev->clk);
+ if (ret)
+ return ret;
+
+ ddev = &mdev->ddev;
+ ddev->dev = dev;
+ dma_cap_set(DMA_PRIVATE, ddev->cap_mask);
+ ddev->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_UNDEFINED);
+ ddev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_UNDEFINED);
+ ddev->directions = BIT(DMA_MEM_TO_DEV) | BIT(DMA_DEV_TO_MEM);
+ ddev->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
+ ddev->device_prep_slave_sg = uniphier_mdmac_prep_slave_sg;
+ ddev->device_config = uniphier_mdmac_config;
+ ddev->device_terminate_all = uniphier_mdmac_terminate_all;
+ ddev->device_synchronize = uniphier_mdmac_synchronize;
+ ddev->device_tx_status = uniphier_mdmac_tx_status;
+ ddev->device_issue_pending = uniphier_mdmac_issue_pending;
+ INIT_LIST_HEAD(&ddev->channels);
+
+ for (i = 0; i < nr_chans; i++) {
+ ret = uniphier_mdmac_chan_init(pdev, mdev, i);
+ if (ret)
+ goto disable_clk;
+ }
+
+ ret = dma_async_device_register(ddev);
+ if (ret)
+ goto disable_clk;
+
+ ret = of_dma_controller_register(dev->of_node, of_dma_xlate_by_chan_id,
+ ddev);
+ if (ret)
+ goto unregister_dmac;
+
+ platform_set_drvdata(pdev, mdev);
+
+ return 0;
+
+unregister_dmac:
+ dma_async_device_unregister(ddev);
+disable_clk:
+ clk_disable_unprepare(mdev->clk);
+
+ return ret;
+}
+
+static int uniphier_mdmac_remove(struct platform_device *pdev)
+{
+ struct uniphier_mdmac_device *mdev = platform_get_drvdata(pdev);
+
+ of_dma_controller_free(pdev->dev.of_node);
+ dma_async_device_unregister(&mdev->ddev);
+ clk_disable_unprepare(mdev->clk);
+
+ return 0;
+}
+
+static const struct of_device_id uniphier_mdmac_match[] = {
+ { .compatible = "socionext,uniphier-mio-dmac" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, uniphier_mdmac_match);
+
+static struct platform_driver uniphier_mdmac_driver = {
+ .probe = uniphier_mdmac_probe,
+ .remove = uniphier_mdmac_remove,
+ .driver = {
+ .name = "uniphier-mio-dmac",
+ .of_match_table = uniphier_mdmac_match,
+ },
+};
+module_platform_driver(uniphier_mdmac_driver);
+
+MODULE_AUTHOR("Masahiro Yamada <yamada.masahiro@socionext.com>");
+MODULE_DESCRIPTION("UniPhier MIO DMAC driver");
+MODULE_LICENSE("GPL v2");
^ permalink raw reply related
* [v3,1/2] dt-bindings: dmaengine: add DT binding for UniPhier MIO DMAC
From: Masahiro Yamada @ 2018-09-13 0:51 UTC (permalink / raw)
To: Vinod Koul, dmaengine
Cc: Masami Hiramatsu, Jassi Brar, Masahiro Yamada, devicetree,
linux-kernel, Rob Herring, Mark Rutland, linux-arm-kernel
The MIO DMAC (Media IO DMA Controller) is used in UniPhier LD4,
Pro4, and sLD8 SoCs.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes in v3:
- Add Rob's Reviewed-by
Changes in v2:
- Rename the node "dmac" to "dma-controller"
- Remove dma-channels property
.../devicetree/bindings/dma/uniphier-mio-dmac.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/uniphier-mio-dmac.txt
diff --git a/Documentation/devicetree/bindings/dma/uniphier-mio-dmac.txt b/Documentation/devicetree/bindings/dma/uniphier-mio-dmac.txt
new file mode 100644
index 0000000..b12388d
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/uniphier-mio-dmac.txt
@@ -0,0 +1,25 @@
+UniPhier Media IO DMA controller
+
+This works as an external DMA engine for SD/eMMC controllers etc.
+found in UniPhier LD4, Pro4, sLD8 SoCs.
+
+Required properties:
+- compatible: should be "socionext,uniphier-mio-dmac".
+- reg: offset and length of the register set for the device.
+- interrupts: a list of interrupt specifiers associated with the DMA channels.
+- clocks: a single clock specifier.
+- #dma-cells: should be <1>. The single cell represents the channel index.
+
+Example:
+ dmac: dma-controller@5a000000 {
+ compatible = "socionext,uniphier-mio-dmac";
+ reg = <0x5a000000 0x1000>;
+ interrupts = <0 68 4>, <0 68 4>, <0 69 4>, <0 70 4>,
+ <0 71 4>, <0 72 4>, <0 73 4>, <0 74 4>;
+ clocks = <&mio_clk 7>;
+ #dma-cells = <1>;
+ };
+
+Note:
+In the example above, "interrupts = <0 68 4>, <0 68 4>, ..." is not a typo.
+The first two channels share a single interrupt line.
^ permalink raw reply related
* dmaengine: ep93xx: Return proper enum in ep93xx_dma_chan_direction
From: Nathan Chancellor @ 2018-09-12 18:36 UTC (permalink / raw)
To: Nick Desaulniers; +Cc: dan.j.williams, vkoul, dmaengine, LKML
On Wed, Sep 12, 2018 at 10:00:57AM -0700, Nick Desaulniers wrote:
> On Tue, Sep 11, 2018 at 4:40 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Clang warns when implicitly converting from one enumerated type to
> > another. Avoid this by using the equivalent value from the expected
> > type.
> >
> > In file included from drivers/dma/ep93xx_dma.c:30:
> > ./include/linux/platform_data/dma-ep93xx.h:88:10: warning: implicit
> > conversion from enumeration type 'enum dma_data_direction' to different
> > enumeration type 'enum dma_transfer_direction' [-Wenum-conversion]
> > return DMA_NONE;
> > ~~~~~~ ^~~~~~~~
> > 1 warning generated.
> >
> > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > include/linux/platform_data/dma-ep93xx.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/platform_data/dma-ep93xx.h b/include/linux/platform_data/dma-ep93xx.h
> > index f8f1f6b952a6..eb9805bb3fe8 100644
> > --- a/include/linux/platform_data/dma-ep93xx.h
> > +++ b/include/linux/platform_data/dma-ep93xx.h
> > @@ -85,7 +85,7 @@ static inline enum dma_transfer_direction
> > ep93xx_dma_chan_direction(struct dma_chan *chan)
> > {
> > if (!ep93xx_dma_chan_is_m2p(chan))
> > - return DMA_NONE;
> > + return DMA_TRANS_NONE;
>
> Looks like this changes the value of the returned enum from:
> DMA_NONE: 3
> DMA_TRANS_NONE: 4
>
> Hopefully the DMA maintainer can review if this was ever correct?
> Looks like the callers of ep93xx_dma_chan_direction were comparing
> this return type, so those comparisons would likely always be failing
> if `!ep93xx_dma_chan_is_m2p(chan)`.
>
> Normally this warning (-Wenum-conversion) is kind of noisey, but it's
> concerning when the enumerated values evaluate to different integers.
> I think it's found an actual bug here.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
I should have highlighted this in the actual commit message but I
checked all of the callsites of ep93xx_dma_chan_direction and one
directly refers to an enum value, DMA_MEM_TO_DEV; every other one
just checks if the value is or isn't equal to another value of this
enum type (DMA_NONE isn't referenced anywhere else in this driver).
I don't believe this results in a funcitonal change.
Thanks for the review!
Nathan
> >
> > /* even channels are for TX, odd for RX */
> > return (chan->chan_id % 2 == 0) ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
> > --
> > 2.18.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply
* dmaengine: ep93xx: Return proper enum in ep93xx_dma_chan_direction
From: Nick Desaulniers @ 2018-09-12 17:00 UTC (permalink / raw)
To: Nathan Chancellor; +Cc: dan.j.williams, vkoul, dmaengine, LKML
On Tue, Sep 11, 2018 at 4:40 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns when implicitly converting from one enumerated type to
> another. Avoid this by using the equivalent value from the expected
> type.
>
> In file included from drivers/dma/ep93xx_dma.c:30:
> ./include/linux/platform_data/dma-ep93xx.h:88:10: warning: implicit
> conversion from enumeration type 'enum dma_data_direction' to different
> enumeration type 'enum dma_transfer_direction' [-Wenum-conversion]
> return DMA_NONE;
> ~~~~~~ ^~~~~~~~
> 1 warning generated.
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> include/linux/platform_data/dma-ep93xx.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/platform_data/dma-ep93xx.h b/include/linux/platform_data/dma-ep93xx.h
> index f8f1f6b952a6..eb9805bb3fe8 100644
> --- a/include/linux/platform_data/dma-ep93xx.h
> +++ b/include/linux/platform_data/dma-ep93xx.h
> @@ -85,7 +85,7 @@ static inline enum dma_transfer_direction
> ep93xx_dma_chan_direction(struct dma_chan *chan)
> {
> if (!ep93xx_dma_chan_is_m2p(chan))
> - return DMA_NONE;
> + return DMA_TRANS_NONE;
Looks like this changes the value of the returned enum from:
DMA_NONE: 3
DMA_TRANS_NONE: 4
Hopefully the DMA maintainer can review if this was ever correct?
Looks like the callers of ep93xx_dma_chan_direction were comparing
this return type, so those comparisons would likely always be failing
if `!ep93xx_dma_chan_is_m2p(chan)`.
Normally this warning (-Wenum-conversion) is kind of noisey, but it's
concerning when the enumerated values evaluate to different integers.
I think it's found an actual bug here.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> /* even channels are for TX, odd for RX */
> return (chan->chan_id % 2 == 0) ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM;
> --
> 2.18.0
>
^ permalink raw reply
* dma: sh: convert to SPDX identifiers
From: Vinod Koul @ 2018-09-12 12:09 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Kuninori Morimoto, Linux-Renesas, Dan Williams,
Geert Uytterhoeven, Simon Horman, dmaengine
On 12-09-18, 14:05, Geert Uytterhoeven wrote:
> Hi Vinod,
>
> On Wed, Sep 12, 2018 at 11:43 AM Vinod <vkoul@kernel.org> wrote:
> > Btw I have sent the series for dma_slave_config direction removal, sh/
> > drivers turned to be bit more, can you guys take care of it please?
>
> I'm sorry, I can't seem to find that.
I though I had Cced renesas folks including you, but looks like I missed
you, apologies
> Can you please provide a link to the patch series?
https://www.spinics.net/lists/dmaengine/msg16551.html
Thanks
^ permalink raw reply
* dma: sh: convert to SPDX identifiers
From: Geert Uytterhoeven @ 2018-09-12 12:05 UTC (permalink / raw)
To: vkoul
Cc: Kuninori Morimoto, Linux-Renesas, Dan Williams,
Geert Uytterhoeven, Simon Horman, dmaengine
Hi Vinod,
On Wed, Sep 12, 2018 at 11:43 AM Vinod <vkoul@kernel.org> wrote:
> Btw I have sent the series for dma_slave_config direction removal, sh/
> drivers turned to be bit more, can you guys take care of it please?
I'm sorry, I can't seem to find that.
Can you please provide a link to the patch series?
Thanks!
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Robin Gong @ 2018-09-12 9:47 UTC (permalink / raw)
To: Lucas Stach, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
My testcase is simple loopback test as attached. Are you sure your serial port running in DMA mode?
By default, DMA not enabled on i.mx7d, you can enable it in uart device node of dts as below:
+ dmas = <&sdma 32 4 0>, <&sdma 33 4 0>;
+ dma-names = "rx", "tx"
Please flow below command(ttymxc5 on i.mx7d-sdb board) to run loopback test:
./mxc_uart_stress_test.out /dev/ttymxc5 2000000 D L 100 100 L
> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2018年9月12日 17:22
> To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> kernel@pengutronix.de; patchwork-lst@pengutronix.de
> Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> termination via worker
>
> Hi Robin,
>
> Am Dienstag, den 11.09.2018, 08:18 +0000 schrieb Robin Gong:
> > Hi Lucas,
> > I have quick test for UART, seems this patch set broke uart function
> > on i.mx7d, I believe it should be same on other i.mx6 family. Could you double
> check it?
>
> Can you describe your testcase and the issue you've seen in more detail? I'm
> running this patch series on a system that does a lot of communication with an
> UART attached Co-processor and I'm not seeing any issues.
>
> Regards,
> Lucas
>
> > > -----Original Message-----
> > > From: Lucas Stach <l.stach@pengutronix.de>
> > > Sent: 2018年9月10日 17:59
> > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > > termination via worker
> > >
> > > Am Dienstag, den 04.09.2018, 02:36 +0000 schrieb Robin Gong:
> > > > > -----Original Message-----
> > > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > > Sent: 2018年9月3日 21:12
> > > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.o
> > > > > rg>
> > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > > > > ; kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > > > > termination via worker
> > > > >
> > > > > Am Montag, den 03.09.2018, 08:59 +0000 schrieb Robin Gong:
> > > > > > > -----Original Message-----
> > > > > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > > > > Sent: 2018年9月3日 16:41
> > > > > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kern
> > > > > > > el.o
> > > > > > > rg>
> > > > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.
> > > > > > > com>
> > > > > > > ; kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement
> > > > > > > channel termination via worker
> > > > > > >
> > > > > > > Hi Robin,
> > > > > > >
> > > > > > > Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong:
> > > > > > > > Hi Lucas,
> > > > > > > > Seems I miss your previous mail. Thanks for your patch,
> > > > > > > > but if move most jobs of sdma_disable_channel_with_delay()
> > > > > > > > into worker, that will bring another race condition that
> > > > > > > > upper driver such as Audio terminate channel and free
> > > > > > > > resource of dma channel without really channel stop, if
> > > > > > > > dma transfer done interrupt come after that, oops or
> > > > > > > > kernel cash may be caught.
> > > > > > > > Leave 'sdmac->desc = NULL' in the
> > > > > > >
> > > > > > > sdma_disable_channel_with_delay() may fix such potential
> > > > > > > issue.
> > > > > > >
> > > > > > > No, there is no such issue. The audio channel terminate will
> > > > > > > call dmaengine_terminate_sync(), which internally calls
> > > > > > > dmaengine_terminate_async() and then does a
> > > > > > > dmaengine_synchronize().
> > > > > > > As this patchset implements the device_synchronize function
> > > > > > > in the sdma driver, this will wait for the worker to finish
> > > > > > > its execution, so there is no race condition to worry about
> > > > > > > here.
> > > > > > >
> > > > > > > Regards,
> > > > > > > Lucas
> > > > > >
> > > > > > Yes, but how about other drivers which not call
> > > > > > dmaengine_terminate_sync()?
> > > > >
> > > > > Please read the dmaengine documentation. device_terminate_all
> > > > > has no requirement that the transfer is actually canceled when
> > > > > the call returns. If the caller needs a guarantee that the
> > > > > channel is stopped it _must_ call device_synchronize.
> > > >
> > > > I know that, but the fact is some driver still use
> > > > dmaengine_terminate_all() such as
> > > > Spi/uart driver. My concern is how to avoid to break their
> > > > function.
> > >
> > > They should simply be fixed to not use a deprecated function. Both
> > > of those are only using device_terminate_all in error or shutdown
> > > paths, so the risk of races is pretty minimal even with the current
> > > code. And I think the SPI driver is trivial to fix, as we can just
> > > use the terminate_sync variant there. The UART driver is a bit more
> > > tricky.
> > >
> > > Regards,
> > > Lucas
^ permalink raw reply
* dma: sh: convert to SPDX identifiers
From: Vinod Koul @ 2018-09-12 9:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Kuninori Morimoto, Linux-Renesas, Dan Williams,
Geert Uytterhoeven, Simon Horman, dmaengine
On 11-09-18, 10:17, Geert Uytterhoeven wrote:
> Hi Vinod,
>
> On Tue, Sep 11, 2018 at 9:48 AM Vinod <vkoul@kernel.org> wrote:
> > On 07-09-18, 01:58, Kuninori Morimoto wrote:
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > >
> > > This patch updates license to use SPDX-License-Identifier
> > > instead of verbose license text.
> >
> > Thanks but the style is not consistent in files :(
> >
> > Can we use one only?
>
> Please read
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst#n69
>
> 2. Style:
>
> The SPDX license identifier is added in form of a comment. The comment
> style depends on the file type::
>
> C source: // SPDX-License-Identifier: <SPDX License Expression>
> C header: /* SPDX-License-Identifier: <SPDX License Expression> */
Right and I missed patch is doing for both c & h files, my bad
Applying this now.
Btw I have sent the series for dma_slave_config direction removal, sh/
drivers turned to be bit more, can you guys take care of it please?
^ permalink raw reply
* [3/4] dmaengine: imx-sdma: implement channel termination via worker
From: Lucas Stach @ 2018-09-12 9:22 UTC (permalink / raw)
To: Robin Gong, Vinod Koul
Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
patchwork-lst@pengutronix.de
Hi Robin,
Am Dienstag, den 11.09.2018, 08:18 +0000 schrieb Robin Gong:
> Hi Lucas,
> I have quick test for UART, seems this patch set broke uart function on i.mx7d,
> I believe it should be same on other i.mx6 family. Could you double check it?
Can you describe your testcase and the issue you've seen in more
detail? I'm running this patch series on a system that does a lot of
communication with an UART attached Co-processor and I'm not seeing any
issues.
Regards,
Lucas
> > -----Original Message-----
> > From: Lucas Stach <l.stach@pengutronix.de>
> > Sent: 2018年9月10日 17:59
> > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.org>
> > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > termination via worker
> >
> > Am Dienstag, den 04.09.2018, 02:36 +0000 schrieb Robin Gong:
> > > > -----Original Message-----
> > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > Sent: 2018年9月3日 21:12
> > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kernel.o
> > > > rg>
> > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > > > ;
> > > > kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement channel
> > > > termination via worker
> > > >
> > > > Am Montag, den 03.09.2018, 08:59 +0000 schrieb Robin Gong:
> > > > > > -----Original Message-----
> > > > > > From: Lucas Stach <l.stach@pengutronix.de>
> > > > > > Sent: 2018年9月3日 16:41
> > > > > > To: Robin Gong <yibin.gong@nxp.com>; Vinod Koul <vkoul@kern
> > > > > > el.o
> > > > > > rg>
> > > > > > Cc: dmaengine@vger.kernel.org; dl-linux-imx <linux-imx@nxp.
> > > > > > com>
> > > > > > ; kernel@pengutronix.de; patchwork-lst@pengutronix.de
> > > > > > Subject: Re: [PATCH 3/4] dmaengine: imx-sdma: implement
> > > > > > channel
> > > > > > termination via worker
> > > > > >
> > > > > > Hi Robin,
> > > > > >
> > > > > > Am Freitag, den 31.08.2018, 09:49 +0000 schrieb Robin Gong:
> > > > > > > Hi Lucas,
> > > > > > > Seems I miss your previous mail. Thanks for your patch,
> > > > > > > but
> > > > > > > if move most jobs of sdma_disable_channel_with_delay()
> > > > > > > into
> > > > > > > worker, that will bring another race condition that upper
> > > > > > > driver such as Audio terminate channel and free resource
> > > > > > > of
> > > > > > > dma channel without really channel stop, if dma transfer
> > > > > > > done
> > > > > > > interrupt come after that, oops or kernel cash may be
> > > > > > > caught.
> > > > > > > Leave 'sdmac->desc = NULL' in the
> > > > > >
> > > > > > sdma_disable_channel_with_delay() may fix such potential
> > > > > > issue.
> > > > > >
> > > > > > No, there is no such issue. The audio channel terminate
> > > > > > will
> > > > > > call dmaengine_terminate_sync(), which internally calls
> > > > > > dmaengine_terminate_async() and then does a
> > > > > > dmaengine_synchronize().
> > > > > > As this patchset implements the device_synchronize function
> > > > > > in
> > > > > > the sdma driver, this will wait for the worker to finish
> > > > > > its
> > > > > > execution, so there is no race condition to worry about
> > > > > > here.
> > > > > >
> > > > > > Regards,
> > > > > > Lucas
> > > > >
> > > > > Yes, but how about other drivers which not call
> > > > > dmaengine_terminate_sync()?
> > > >
> > > > Please read the dmaengine documentation. device_terminate_all
> > > > has no
> > > > requirement that the transfer is actually canceled when the
> > > > call
> > > > returns. If the caller needs a guarantee that the channel is
> > > > stopped
> > > > it _must_ call device_synchronize.
> > >
> > > I know that, but the fact is some driver still use
> > > dmaengine_terminate_all() such as
> > > Spi/uart driver. My concern is how to avoid to break their
> > > function.
> >
> > They should simply be fixed to not use a deprecated function. Both
> > of those are
> > only using device_terminate_all in error or shutdown paths, so the
> > risk of races
> > is pretty minimal even with the current code. And I think the SPI
> > driver is trivial
> > to fix, as we can just use the terminate_sync variant there. The
> > UART driver
> > is a bit more tricky.
> >
> > Regards,
> > Lucas
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox