DMA Engine development
 help / color / mirror / Atom feed
* [2/8,v4] Documentation: bindings: dma: Add binding for dma-channel-mask
From: Vinod Koul @ 2019-01-20 11:06 UTC (permalink / raw)
  To: John Stultz
  Cc: Manivannan Sadhasivam, lkml, Rob Herring, Mark Rutland,
	Tanglei Han, Zhuangluan Su, Ryan Grachek,
	open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On 17-01-19, 09:43, John Stultz wrote:
> On Thu, Jan 17, 2019 at 9:08 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Wed, Jan 16, 2019 at 09:10:23AM -0800, John Stultz wrote:
> > > Some dma channels can be reserved for secure mode or other
> > > hardware on the SoC, so provide a binding for a bitmask
> > > listing the available channels for the kernel to use.
> > >
> > > This follows the pre-existing bcm,dma-channel-mask binding.
> > >
> > > Cc: Vinod Koul <vkoul@kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Tanglei Han <hantanglei@huawei.com>
> > > Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> > > Cc: Ryan Grachek <ryan@edited.us>
> > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Cc: dmaengine@vger.kernel.org
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > ---
> > > v3: Renamed to hisi-dma-avail-chan
> > > v4: Reworked to generic dma-channel-mask
> > > ---
> > >  Documentation/devicetree/bindings/dma/dma.txt | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/dma/dma.txt b/Documentation/devicetree/bindings/dma/dma.txt
> > > index 6312fb0..eeb4e4d 100644
> > > --- a/Documentation/devicetree/bindings/dma/dma.txt
> > > +++ b/Documentation/devicetree/bindings/dma/dma.txt
> > > @@ -16,6 +16,9 @@ Optional properties:
> > >  - dma-channels:      Number of DMA channels supported by the controller.
> > >  - dma-requests:      Number of DMA request signals supported by the
> > >                       controller.
> > > +- dma-channel-mask:  Bitmask of available DMA channels in ascending order
> > > +                     that are not reserved by firmware and are available to
> > > +                     the kernel. i.e. first channel corresponds to LSB.
> >
> > A general assumption is, "dma-channel-mask" refers to the bit fields of
> > the channels which needs to be masked. But here, it refers to the channels
> > which are available. Doesn't it contradict?
> 
> Hrm. So while I can sort of understand the common usage of "mask" as
> to "hide", thus the desire to have a bitfield mean "the channels we
> hide" or "don't use", but in my experience bitmasking is more commonly
> used to keep only a portion of the the bits, so from that perspective
> its more intuitive that a mask be the channels we keep to use. So I'm
> not sure if your suggestion makes it more clear.
> 
> But I'm not very particular here, so I'll defer to others on this.

Given the context and documentation which explicitly says it is bitmask
of available channels, i think we are fine :)

^ permalink raw reply

* dmaengine: at_xdmac: Fix wrongfull report of a channel as in use
From: Vinod Koul @ 2019-01-20 11:04 UTC (permalink / raw)
  To: Codrin.Ciubotariu
  Cc: Ludovic.Desroches, dmaengine, linux-arm-kernel, linux-kernel

Hi Codrin,

On 17-01-19, 16:10, Codrin.Ciubotariu@microchip.com wrote:
> From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> 
> atchan->status is used for two things:
>  - pass channel interrupts status from interrupt handler to tasklet;
>  - channel information like whether it is cyclic or paused;
> 
> Since these operations have nothing in common, this patch adds a
> different struct member to keep the interrupts status.
> 
> Fixes a bug in which a channel is wrongfully reported as in use when
> trying to obtain a new descriptior for a previously used cyclic channel.

This looks reasonable but am unable to see how the bug is fixed. Perhaps
would be great to split the bug part (which can go to fixes) and remove
the reuse of variable as a subsequent patch..

> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
>  drivers/dma/at_xdmac.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 4e557684f792..fe69dccfa0c0 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -203,6 +203,7 @@ struct at_xdmac_chan {
>  	u32				save_cim;
>  	u32				save_cnda;
>  	u32				save_cndc;
> +	u32				irq_status;
>  	unsigned long			status;
>  	struct tasklet_struct		tasklet;
>  	struct dma_slave_config		sconfig;
> @@ -1580,8 +1581,8 @@ static void at_xdmac_tasklet(unsigned long data)
>  	struct at_xdmac_desc	*desc;
>  	u32			error_mask;
>  
> -	dev_dbg(chan2dev(&atchan->chan), "%s: status=0x%08lx\n",
> -		 __func__, atchan->status);
> +	dev_dbg(chan2dev(&atchan->chan), "%s: status=0x%08x\n",
> +		__func__, atchan->irq_status);
>  
>  	error_mask = AT_XDMAC_CIS_RBEIS
>  		     | AT_XDMAC_CIS_WBEIS
> @@ -1589,15 +1590,15 @@ static void at_xdmac_tasklet(unsigned long data)
>  
>  	if (at_xdmac_chan_is_cyclic(atchan)) {
>  		at_xdmac_handle_cyclic(atchan);
> -	} else if ((atchan->status & AT_XDMAC_CIS_LIS)
> -		   || (atchan->status & error_mask)) {
> +	} else if ((atchan->irq_status & AT_XDMAC_CIS_LIS)
> +		   || (atchan->irq_status & error_mask)) {
>  		struct dma_async_tx_descriptor  *txd;
>  
> -		if (atchan->status & AT_XDMAC_CIS_RBEIS)
> +		if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
>  			dev_err(chan2dev(&atchan->chan), "read bus error!!!");
> -		if (atchan->status & AT_XDMAC_CIS_WBEIS)
> +		if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
>  			dev_err(chan2dev(&atchan->chan), "write bus error!!!");
> -		if (atchan->status & AT_XDMAC_CIS_ROIS)
> +		if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
>  			dev_err(chan2dev(&atchan->chan), "request overflow error!!!");
>  
>  		spin_lock(&atchan->lock);
> @@ -1652,7 +1653,7 @@ static irqreturn_t at_xdmac_interrupt(int irq, void *dev_id)
>  			atchan = &atxdmac->chan[i];
>  			chan_imr = at_xdmac_chan_read(atchan, AT_XDMAC_CIM);
>  			chan_status = at_xdmac_chan_read(atchan, AT_XDMAC_CIS);
> -			atchan->status = chan_status & chan_imr;
> +			atchan->irq_status = chan_status & chan_imr;
>  			dev_vdbg(atxdmac->dma.dev,
>  				 "%s: chan%d: imr=0x%x, status=0x%x\n",
>  				 __func__, i, chan_imr, chan_status);
> @@ -1666,7 +1667,7 @@ static irqreturn_t at_xdmac_interrupt(int irq, void *dev_id)
>  				 at_xdmac_chan_read(atchan, AT_XDMAC_CDA),
>  				 at_xdmac_chan_read(atchan, AT_XDMAC_CUBC));
>  
> -			if (atchan->status & (AT_XDMAC_CIS_RBEIS | AT_XDMAC_CIS_WBEIS))
> +			if (atchan->irq_status & (AT_XDMAC_CIS_RBEIS | AT_XDMAC_CIS_WBEIS))
>  				at_xdmac_write(atxdmac, AT_XDMAC_GD, atchan->mask);
>  
>  			tasklet_schedule(&atchan->tasklet);
> -- 
> 2.17.1

^ permalink raw reply

* [v2] dmaengine: usb-dmac: Make DMAC system sleep callbacks explicit
From: Vinod Koul @ 2019-01-20 11:00 UTC (permalink / raw)
  To: Yoshihiro Shimoda; +Cc: dmaengine, linux-renesas-soc, stable, Phuong Nguyen

On 17-01-19, 17:44, Yoshihiro Shimoda wrote:
> From: Phuong Nguyen <phuong.nguyen.xw@renesas.com>
> 
> This commit fixes the issue that USB-DMAC hangs silently after system
> resumes on R-Car Gen3 hence renesas_usbhs will not work correctly
> when using USB-DMAC for bulk transfer e.g. ethernet or serial
> gadgets.
> 
> The issue can be reproduced by these steps:
>  1. modprobe g_serial
>  2. Suspend and resume system.
>  3. connect a usb cable to host side
>  4. Transfer data from Host to Target
>  5. cat /dev/ttyGS0 (Target side)
>  6. echo "test" > /dev/ttyACM0 (Host side)
> 
> The 'cat' will not result anything. However, system still can work
> normally.
> 
> Currently, USB-DMAC driver does not have system sleep callbacks hence
> this driver relies on the PM core to force runtime suspend/resume to
> suspend and reinitialize USB-DMAC during system resume. After
> the commit 17218e0092f8 ("PM / genpd: Stop/start devices without
> pm_runtime_force_suspend/resume()"), PM core will not force
> runtime suspend/resume anymore so this issue happens.
> 
> To solve this, make system suspend resume explicit by using
> pm_runtime_force_{suspend,resume}() as the system sleep callbacks.
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is used to make sure USB-DMAC
> suspended after and initialized before renesas_usbhs."

Applied, thanks

^ permalink raw reply

* dma: imx-sdma: pass ->dev to dma_alloc_coherent() API
From: Vinod Koul @ 2019-01-20 10:56 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: dmaengine@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
	dan.j.williams@intel.com, linux-kernel@vger.kernel.org, S.j. Wang,
	Andy Duan, Robin Gong

On 11-01-19, 14:29, Daniel Baluta wrote:
> From: Andy Duan <fugang.duan@nxp.com>
> 
> Pass ->dev to dma_alloc_coherent() API. We need this
> becase dma_alloc_coherent() makes use of dev parameter
  ^^^^^
Typo

> and receiving NULL will result in a crash.

Applied after fixing typo and subsystem name, thanks

^ permalink raw reply

* dmaengine: imx-dma: fix wrong callback invoke
From: Vinod Koul @ 2019-01-20 10:52 UTC (permalink / raw)
  To: Leonid Iziumtsev, Fabio Estevam
  Cc: dmaengine, Dan Williams, linux-kernel, m.grzeschik

On 15-01-19, 17:15, Leonid Iziumtsev wrote:
> Once the "ld_queue" list is not empty, next descriptor will migrate
> into "ld_active" list. The "desc" variable will be overwritten
> during that transition. And later the dmaengine_desc_get_callback_invoke()
> will use it as an argument. As result we invoke wrong callback.
> 
> That behaviour was in place since:
> commit fcaaba6c7136 ("dmaengine: imx-dma: fix callback path in tasklet").
> But after commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job")
> things got worse, since possible delay between tasklet_schedule()
> from DMA irq handler and actual tasklet function execution got bigger.
> And that gave more time for new DMA request to be submitted and
> to be put into "ld_queue" list.
> 
> It has been noticed that DMA issue is causing problems for "mxc-mmc"
> driver. While stressing the system with heavy network traffic and
> writing/reading to/from sd card simultaneously the timeout may happen:
> 
> 10013000.sdhci: mxcmci_watchdog: read time out (status = 0x30004900)
> 
> That often lead to file system corruption.

This looks reasonable to me and I think should go to stable as well.
Fabio can we get some testing done on this patch

> 
> Signed-off-by: Leonid Iziumtsev <leonid.iziumtsev@gmail.com>
> ---
>  drivers/dma/imx-dma.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
> index c2fff3f6c9ca..4a09af3cd546 100644
> --- a/drivers/dma/imx-dma.c
> +++ b/drivers/dma/imx-dma.c
> @@ -618,7 +618,7 @@ static void imxdma_tasklet(unsigned long data)
>  {
>  	struct imxdma_channel *imxdmac = (void *)data;
>  	struct imxdma_engine *imxdma = imxdmac->imxdma;
> -	struct imxdma_desc *desc;
> +	struct imxdma_desc *desc, *next_desc;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&imxdma->lock, flags);
> @@ -648,10 +648,10 @@ static void imxdma_tasklet(unsigned long data)
>  	list_move_tail(imxdmac->ld_active.next, &imxdmac->ld_free);
>  
>  	if (!list_empty(&imxdmac->ld_queue)) {
> -		desc = list_first_entry(&imxdmac->ld_queue, struct imxdma_desc,
> -					node);
> +		next_desc = list_first_entry(&imxdmac->ld_queue,
> +					     struct imxdma_desc, node);
>  		list_move_tail(imxdmac->ld_queue.next, &imxdmac->ld_active);
> -		if (imxdma_xfer_desc(desc) < 0)
> +		if (imxdma_xfer_desc(next_desc) < 0)
>  			dev_warn(imxdma->dev, "%s: channel: %d couldn't xfer desc\n",
>  				 __func__, imxdmac->channel);
>  	}
> -- 
> 2.11.0

^ permalink raw reply

* [v2,2/3] dma: imx-sdma: add clock ratio 1:1 check
From: Daniel Baluta @ 2019-01-20  9:58 UTC (permalink / raw)
  To: Angus Ainslie (Purism)
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, Linux Kernel Mailing List

On Sun, Jan 20, 2019 at 4:32 AM Angus Ainslie (Purism) <angus@akkea.ca> wrote:
>
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> based on NXP commit MLK-16841-1

Hi Angus,

Thanks for doing this!

I'm not sure specifying the MLK here helps. I think it would be better
to somehow add the original Signed-off-by and mention that the commit
was pulled from NXP linux-imx tree.
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
>  .../devicetree/bindings/dma/fsl-imx-sdma.txt  |  1 +
>  drivers/dma/imx-sdma.c                        | 20 +++++++++++++++----
>  2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> index 3c9a57a8443b..17544c1820b7 100644
> --- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> +++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> @@ -67,6 +67,7 @@ Optional properties:
>      reg is the GPR register offset.
>      shift is the bit position inside the GPR register.
>      val is the value of the bit (0 or 1).
> +- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
>
>  Examples:
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..65dada21d3c1 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
>         unsigned int                    irq;
>         dma_addr_t                      bd0_phys;
>         struct sdma_buffer_descriptor   *bd0;
> +       /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> +       bool                            clk_ratio;
>  };
>
>  static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
>                 dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
>         /* Set bits of CONFIG register with dynamic context switching */
> -       if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> -               writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> +       if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
> +               if (sdma->clk_ratio)
> +                       reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
> +               else
> +                       reg = SDMA_H_CONFIG_CSM;
> +
> +               writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> +       }
>
>         return ret;
>  }
> @@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
>         writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
>         /* Set bits of CONFIG register but with static context switching */
> -       /* FIXME: Check whether to set ACR bit depending on clock ratios */
> -       writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> +       if (sdma->clk_ratio)
> +               writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> +       else
> +               writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
>         writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device *pdev)
>         if (!sdma)
>                 return -ENOMEM;
>
> +       sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
> +
>         spin_lock_init(&sdma->channel_0_lock);
>
>         sdma->dev = &pdev->dev;
> --
> 2.17.1
>

^ permalink raw reply

* [v2,1/3] dma: imx-sdma: fix NULL pointer de-reference
From: Daniel Baluta @ 2019-01-20  9:54 UTC (permalink / raw)
  To: Angus Ainslie (Purism)
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, Linux Kernel Mailing List

On Sun, Jan 20, 2019 at 4:34 AM Angus Ainslie (Purism) <angus@akkea.ca> wrote:
>
> On the imx8mq I get NULL pointer de-deference errors if the device
> isn't passed in during allocation.
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>

Hi Angus,

I have already sent a fix for this:

https://patchwork.kernel.org/patch/10758203/

^ permalink raw reply

* [2/2] dmaengine: imx-dma: change return of 'imxdma_sg_next' to void
From: Vinod Koul @ 2019-01-20  6:36 UTC (permalink / raw)
  To: dmaengine; +Cc: festevam, Vinod Koul

The return value of function 'imxdma_sg_next' is not checked anywhere,
so make it void return type.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/dma/imx-dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
index aba1380c3321..5a923490dc8b 100644
--- a/drivers/dma/imx-dma.c
+++ b/drivers/dma/imx-dma.c
@@ -278,7 +278,7 @@ static int imxdma_hw_chain(struct imxdma_channel *imxdmac)
 /*
  * imxdma_sg_next - prepare next chunk for scatter-gather DMA emulation
  */
-static inline int imxdma_sg_next(struct imxdma_desc *d)
+static inline void imxdma_sg_next(struct imxdma_desc *d)
 {
 	struct imxdma_channel *imxdmac = to_imxdma_chan(d->desc.chan);
 	struct imxdma_engine *imxdma = imxdmac->imxdma;
@@ -303,8 +303,6 @@ static inline int imxdma_sg_next(struct imxdma_desc *d)
 		 imx_dmav1_readl(imxdma, DMA_DAR(imxdmac->channel)),
 		 imx_dmav1_readl(imxdma, DMA_SAR(imxdmac->channel)),
 		 imx_dmav1_readl(imxdma, DMA_CNTR(imxdmac->channel)));
-
-	return now;
 }
 
 static void imxdma_enable_hw(struct imxdma_desc *d)

^ permalink raw reply related

* [1/2] dmaengine: imx-dma: change variable 'now' type to size_t
From: Vinod Koul @ 2019-01-20  6:36 UTC (permalink / raw)
  To: dmaengine; +Cc: festevam, Vinod Koul

now is used to keep size and it is better to change the variable
type to size_t

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/dma/imx-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
index fa3ef43fe314..aba1380c3321 100644
--- a/drivers/dma/imx-dma.c
+++ b/drivers/dma/imx-dma.c
@@ -283,7 +283,7 @@ static inline int imxdma_sg_next(struct imxdma_desc *d)
 	struct imxdma_channel *imxdmac = to_imxdma_chan(d->desc.chan);
 	struct imxdma_engine *imxdma = imxdmac->imxdma;
 	struct scatterlist *sg = d->sg;
-	unsigned long now;
+	size_t now;
 
 	now = min_t(size_t, d->len, sg_dma_len(sg));
 	if (d->len != IMX_DMA_LENGTH_LOOP)

^ permalink raw reply related

* dma: ixm-dma: fix warning comparison of distinct pointer types
From: Vinod Koul @ 2019-01-20  5:46 UTC (permalink / raw)
  To: Anders Roxell; +Cc: Fabio Estevam, Dan Williams, dmaengine, linux-kernel

On 15-01-19, 15:14, Anders Roxell wrote:
> On Mon, 14 Jan 2019 at 22:24, Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Anders,
> >
> > On Thu, Jan 10, 2019 at 9:15 AM Anders Roxell <anders.roxell@linaro.org> wrote:
> > >
> > > The warning got introduced by commit 930507c18304 ("arm64: add basic
> > > Kconfig symbols for i.MX8"). Since it got enabled for arm64. The warning
> > > haven't been seen before since size_t was 'unsigned int' when built on
> > > arm32.
> > >
> > > ../drivers/dma/imx-dma.c: In function ‘imxdma_sg_next’:
> > > ../include/linux/kernel.h:846:29: warning: comparison of distinct pointer types lacks a cast
> > >    (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> > >                              ^~
> > > ../include/linux/kernel.h:860:4: note: in expansion of macro ‘__typecheck’
> > >    (__typecheck(x, y) && __no_side_effects(x, y))
> > >     ^~~~~~~~~~~
> > > ../include/linux/kernel.h:870:24: note: in expansion of macro ‘__safe_cmp’
> > >   __builtin_choose_expr(__safe_cmp(x, y), \
> > >                         ^~~~~~~~~~
> > > ../include/linux/kernel.h:879:19: note: in expansion of macro ‘__careful_cmp’
> > >  #define min(x, y) __careful_cmp(x, y, <)
> > >                    ^~~~~~~~~~~~~
> > > ../drivers/dma/imx-dma.c:288:8: note: in expansion of macro ‘min’
> > >   now = min(d->len, sg_dma_len(sg));
> > >         ^~~
> > >
> > > Rework so that we use min_t and pass in the size_t that returns the
> > > minimum of two values, using the specified type.
> > >
> > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> >
> > There is a typo in the Subject: s/ixm/imx/ and the prefix should be
> > dmaengine instead:
> >
> > dmaengine: imx-dma: fix warning comparison of distinct pointer types
> >
> > With that fixed:
> >
> > Reviewed-by: Fabio Estevam <festevam@gmail.com>
> 
> OK, thanks Fabio.
> 
> Dan, do you want me to resend the patch with an updated shortlog or will
> you do that when you apply the patch?

applied after fixing subsystem name and driver name

^ permalink raw reply

* [v9,1/2] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Vinod Koul @ 2019-01-20  5:27 UTC (permalink / raw)
  To: Long Cheng
  Cc: Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee, Sean Wang,
	Nicolas Boichat, Matthias Brugger, Dan Williams,
	Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
	linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
	srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu

On 10-01-19, 18:33, Long Cheng wrote:
> On Fri, 2019-01-04 at 22:49 +0530, Vinod Koul wrote:
> > On 02-01-19, 10:12, Long Cheng wrote:
> > > In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> > > If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> > > the performance, can enable the function.
> > 
> > Is the DMA controller UART specific, can it work with other controllers
> > as well, if so you should get rid of uart name in patch
> 
> I don't know that it can work or not on other controller. but it's for
> MediaTek SOC

What I meant was that if can work with other controllers (users) apart
from UART, how about say audio, spi etc!!

> 
> > > +#define MTK_UART_APDMA_CHANNELS		(CONFIG_SERIAL_8250_NR_UARTS * 2)
> > 
> > Why are the channels not coming from DT?
> > 
> 
> i will using dma-requests install of it.
> 
> > > +
> > > +#define VFF_EN_B		BIT(0)
> > > +#define VFF_STOP_B		BIT(0)
> > > +#define VFF_FLUSH_B		BIT(0)
> > > +#define VFF_4G_SUPPORT_B	BIT(0)
> > > +#define VFF_RX_INT_EN0_B	BIT(0)	/*rx valid size >=  vff thre*/
> > > +#define VFF_RX_INT_EN1_B	BIT(1)
> > > +#define VFF_TX_INT_EN_B		BIT(0)	/*tx left size >= vff thre*/
> > 
> > space around /* space */ also run checkpatch to check for style errors
> > 
> 
> ok.
> 
> > > +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> > > +{
> > > +	unsigned int len, send, left, wpt, d_wpt, tmp;
> > > +	int ret;
> > > +
> > > +	left = mtk_uart_apdma_read(c, VFF_LEFT_SIZE);
> > > +	if (!left) {
> > > +		mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > > +		return;
> > > +	}
> > > +
> > > +	/* Wait 1sec for flush,  can't sleep*/
> > > +	ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
> > > +			tmp != VFF_FLUSH_B, 0, 1000000);
> > > +	if (ret)
> > > +		dev_warn(c->vc.chan.device->dev, "tx: fail, debug=0x%x\n",
> > > +			mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> > > +
> > > +	send = min_t(unsigned int, left, c->desc->avail_len);
> > > +	wpt = mtk_uart_apdma_read(c, VFF_WPT);
> > > +	len = mtk_uart_apdma_read(c, VFF_LEN);
> > > +
> > > +	d_wpt = wpt + send;
> > > +	if ((d_wpt & VFF_RING_SIZE) >= len) {
> > > +		d_wpt = d_wpt - len;
> > > +		d_wpt = d_wpt ^ VFF_RING_WRAP;
> > > +	}
> > > +	mtk_uart_apdma_write(c, VFF_WPT, d_wpt);
> > > +
> > > +	c->desc->avail_len -= send;
> > > +
> > > +	mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> > > +	if (mtk_uart_apdma_read(c, VFF_FLUSH) == 0U)
> > > +		mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
> > > +}
> > > +
> > > +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> > > +{
> > > +	struct mtk_uart_apdma_desc *d = c->desc;
> > > +	unsigned int len, wg, rg, cnt;
> > > +
> > > +	if ((mtk_uart_apdma_read(c, VFF_VALID_SIZE) == 0U) ||
> > > +		!d || !vchan_next_desc(&c->vc))
> > > +		return;
> > > +
> > > +	len = mtk_uart_apdma_read(c, VFF_LEN);
> > > +	rg = mtk_uart_apdma_read(c, VFF_RPT);
> > > +	wg = mtk_uart_apdma_read(c, VFF_WPT);
> > > +	if ((rg ^ wg) & VFF_RING_WRAP)
> > > +		cnt = (wg & VFF_RING_SIZE) + len - (rg & VFF_RING_SIZE);
> > > +	else
> > > +		cnt = (wg & VFF_RING_SIZE) - (rg & VFF_RING_SIZE);
> > > +
> > > +	c->rx_status = cnt;
> > > +	mtk_uart_apdma_write(c, VFF_RPT, wg);
> > > +
> > > +	list_del(&d->vd.node);
> > > +	vchan_cookie_complete(&d->vd);
> > > +}
> > 
> > this looks odd, why do you have different rx and tx start routines?
> > 
> 
> Would you like explain it in more detail? thanks.
> In tx function, will wait the last data flush done. and the count the
> size that send.
> In Rx function, will count the size that receive.
> Any way, in rx / tx, need andle WPT or RPT.
> 
> > > +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> > > +{
> > > +	struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	u32 tmp;
> > > +	int ret;
> > > +
> > > +	pm_runtime_get_sync(mtkd->ddev.dev);
> > > +
> > > +	mtk_uart_apdma_write(c, VFF_ADDR, 0);
> > > +	mtk_uart_apdma_write(c, VFF_THRE, 0);
> > > +	mtk_uart_apdma_write(c, VFF_LEN, 0);
> > > +	mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);
> > > +
> > > +	ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp,
> > > +			tmp == 0, 10, 100);
> > > +	if (ret) {
> > > +		dev_err(chan->device->dev, "dma reset: fail, timeout\n");
> > > +		return ret;
> > > +	}
> > 
> > register read does reset?
> > 
> 
> 'mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);' is reset. resd just
> poll reset done.
> 
> > > +
> > > +	if (!c->requested) {
> > > +		c->requested = true;
> > > +		ret = request_irq(mtkd->dma_irq[chan->chan_id],
> > > +				  mtk_uart_apdma_irq_handler, IRQF_TRIGGER_NONE,
> > > +				  KBUILD_MODNAME, chan);
> > 
> > why is the irq not requested in driver probe?
> > 
> 
> I have explained in below,
> http://lists.infradead.org/pipermail/linux-mediatek/2018-December/016418.html
> 
> > > +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> > > +					 dma_cookie_t cookie,
> > > +					 struct dma_tx_state *txstate)
> > > +{
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	enum dma_status ret;
> > > +	unsigned long flags;
> > > +
> > > +	if (!txstate)
> > > +		return DMA_ERROR;
> > > +
> > > +	ret = dma_cookie_status(chan, cookie, txstate);
> > > +	spin_lock_irqsave(&c->vc.lock, flags);
> > > +	if (ret == DMA_IN_PROGRESS) {
> > > +		c->rx_status = mtk_uart_apdma_read(c, VFF_RPT) & VFF_RING_SIZE;
> > > +		dma_set_residue(txstate, c->rx_status);
> > > +	} else if (ret == DMA_COMPLETE && c->cfg.direction == DMA_DEV_TO_MEM) {
> > 
> > why set reside when it is complete? also reside can be null, that should
> > be checked as well
> > 
> In different status, set different reside.

Can you explain that..

> 
> > > +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> > > +	(struct dma_chan *chan, struct scatterlist *sgl,
> > > +	unsigned int sglen, enum dma_transfer_direction dir,
> > > +	unsigned long tx_flags, void *context)
> > > +{
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	struct mtk_uart_apdma_desc *d;
> > > +
> > > +	if ((dir != DMA_DEV_TO_MEM) &&
> > > +		(dir != DMA_MEM_TO_DEV)) {
> > > +		dev_err(chan->device->dev, "bad direction\n");
> > > +		return NULL;
> > > +	}
> > 
> > we have a macro for this
> 
> Thanks for your suggestion. i will modify it.
> > 
> > > +
> > > +	/* Now allocate and setup the descriptor */
> > > +	d = kzalloc(sizeof(*d), GFP_ATOMIC);
> > > +	if (!d)
> > > +		return NULL;
> > > +
> > > +	/* sglen is 1 */
> > 
> > ?
> > 
> > > +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> > > +				struct dma_slave_config *cfg)
> > > +{
> > > +	struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> > > +	struct mtk_uart_apdmadev *mtkd =
> > > +				to_mtk_uart_apdma_dev(c->vc.chan.device);
> > > +
> > > +	c->cfg = *cfg;
> > > +
> > > +	if (cfg->direction == DMA_DEV_TO_MEM) {
> > 
> > fg->direction is deprecated, in fact I have removed all users recently,
> > please do not use this
> 
> You will remove 'direction' in 'struct dma_slave_config'? if remove, how
> do confirm direction?

Yes please use the direction argument in prep_xx calls

^ permalink raw reply

* [-next] dmaengine: xilinx_dma: remove set but not used variable 'tail_segment'
From: Vinod Koul @ 2019-01-20  5:23 UTC (permalink / raw)
  To: YueHaibing
  Cc: Dan Williams, Michal Simek, Radhey Shyam Pandey, Andrea Merello,
	Appana Durga Kedareswara Rao, dmaengine, linux-arm-kernel,
	kernel-janitors

On 09-01-19, 12:10, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/dma/xilinx/xilinx_dma.c: In function 'xilinx_vdma_start_transfer':
> drivers/dma/xilinx/xilinx_dma.c:1104:33: warning:
>  variable 'tail_segment' set but not used [-Wunused-but-set-variable]
> 
> It not used since commit b8349172b400 ("dmaengine: xilinx_dma: Drop SG support
> for VDMA IP")

Applied, thanks

^ permalink raw reply

* dmaengine: axi-dmac: Use struct_size() in kzalloc()
From: Vinod Koul @ 2019-01-20  5:22 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Lars-Peter Clausen, Dan Williams, dmaengine, linux-kernel

On 09-01-19, 08:44, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding the
> size of a structure that has a zero-sized array at the end, along with memory
> for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     void *entry[];
> };
> 
> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.

Applied, thanks

^ permalink raw reply

* dmaengine: timb_dma: Use struct_size() in kzalloc()
From: Vinod Koul @ 2019-01-20  5:20 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: Dan Williams, dmaengine, linux-kernel

On 08-01-19, 09:58, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding the
> size of a structure that has a zero-sized array at the end, along with memory
> for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     void *entry[];
> };
> 
> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.

Applied, thanks

^ permalink raw reply

* dmaengine: tegra210-adma: Use struct_size() in devm_kzalloc()
From: Vinod Koul @ 2019-01-20  5:18 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Laxman Dewangan, Jon Hunter, Dan Williams, Thierry Reding,
	dmaengine, linux-tegra, linux-kernel

On 07-01-19, 11:06, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     void *entry[];
> };
> 
> instance = devm_kzalloc(dev, sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
> 
> instance = devm_kzalloc(dev, struct_size(instance, entry, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.

Applied, thanks

^ permalink raw reply

* [v2,1/2] dmaengine: qcom_hidma: initialize tx flags in hidma_prep_dma_*
From: Vinod Koul @ 2019-01-20  5:14 UTC (permalink / raw)
  To: Shunyong Yang
  Cc: okaya, andy.gross, david.brown, dan.j.williams, dmaengine,
	linux-kernel, Joey Zheng

On 07-01-19, 09:32, Shunyong Yang wrote:
> In async_tx_test_ack(), it uses flags in struct dma_async_tx_descriptor
> to check the ACK status. As hidma reuses the descriptor in a free list
> when hidma_prep_dma_*(memcpy/memset) is called, the flag will keep ACKed
> if the descriptor has been used before. This will cause a BUG_ON in
> async_tx_quiesce().
> 
>   kernel BUG at crypto/async_tx/async_tx.c:282!
>   Internal error: Oops - BUG: 0 1 SMP
>   ...
>   task: ffff8017dd3ec000 task.stack: ffff8017dd3e8000
>   PC is at async_tx_quiesce+0x54/0x78 [async_tx]
>   LR is at async_trigger_callback+0x98/0x110 [async_tx]
> 
> This patch initializes flags in dma_async_tx_descriptor by the flags
> passed from the caller when hidma_prep_dma_*(memcpy/memset) is called.

Applied both, thanks

^ permalink raw reply

* [v2,3/3] imx8mq.dtsi: add the sdma nodes
From: Angus Ainslie @ 2019-01-20  2:31 UTC (permalink / raw)
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, linux-kernel, Angus Ainslie (Purism)

Add the sdma nodes to the base devicetree for the imx8mq

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index c0402375e7c1..0b9a9b5ae7b7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -336,6 +336,19 @@
 				clocks = <&clk IMX8MQ_CLK_WDOG3_ROOT>;
 				status = "disabled";
 			};
+
+			sdma2: sdma@302c0000 {
+				compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+				reg = <0x302c0000 0x10000>;
+				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
+					<&clk IMX8MQ_CLK_SDMA2_ROOT>;
+				clock-names = "ipg", "ahb";
+				#dma-cells = <3>;
+				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+				fsl,ratio-1-1;
+				status = "disabled";
+			};
 		};
 
 		bus@30400000 { /* AIPS2 */
@@ -370,6 +383,8 @@
 				clocks = <&clk IMX8MQ_CLK_UART3_ROOT>,
 				         <&clk IMX8MQ_CLK_UART3_ROOT>;
 				clock-names = "ipg", "per";
+				dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
+				dma-names = "rx", "tx";
 				status = "disabled";
 			};
 
@@ -381,6 +396,8 @@
 				clocks = <&clk IMX8MQ_CLK_UART2_ROOT>,
 				         <&clk IMX8MQ_CLK_UART2_ROOT>;
 				clock-names = "ipg", "per";
+				dmas = <&sdma1 24 4 0>, <&sdma1 25 4 0>;
+				dma-names = "rx", "tx";
 				status = "disabled";
 			};
 
@@ -432,6 +449,8 @@
 				clocks = <&clk IMX8MQ_CLK_UART4_ROOT>,
 				         <&clk IMX8MQ_CLK_UART4_ROOT>;
 				clock-names = "ipg", "per";
+				dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
+				dma-names = "rx", "tx";
 				status = "disabled";
 			};
 
@@ -465,6 +484,18 @@
 				status = "disabled";
 			};
 
+			sdma1: sdma@30bd0000 {
+				compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+				reg = <0x30bd0000 0x10000>;
+				interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
+					<&clk IMX8MQ_CLK_SDMA1_ROOT>;
+				clock-names = "ipg", "ahb";
+				#dma-cells = <3>;
+				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+				status = "disabled";
+			};
+
 			fec1: ethernet@30be0000 {
 				compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
 				reg = <0x30be0000 0x10000>;

^ permalink raw reply related

* [v2,2/3] dma: imx-sdma: add clock ratio 1:1 check
From: Angus Ainslie @ 2019-01-20  2:31 UTC (permalink / raw)
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, linux-kernel, Angus Ainslie (Purism)

On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.

based on NXP commit MLK-16841-1

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 .../devicetree/bindings/dma/fsl-imx-sdma.txt  |  1 +
 drivers/dma/imx-sdma.c                        | 20 +++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 3c9a57a8443b..17544c1820b7 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -67,6 +67,7 @@ Optional properties:
     reg is the GPR register offset.
     shift is the bit position inside the GPR register.
     val is the value of the bit (0 or 1).
+- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
 
 Examples:
 
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..65dada21d3c1 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
 	unsigned int			irq;
 	dma_addr_t			bd0_phys;
 	struct sdma_buffer_descriptor	*bd0;
+	/* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+	bool				clk_ratio;
 };
 
 static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
 		dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
 
 	/* Set bits of CONFIG register with dynamic context switching */
-	if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
-		writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+	if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
+		if (sdma->clk_ratio)
+			reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
+		else
+			reg = SDMA_H_CONFIG_CSM;
+
+		writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+	}
 
 	return ret;
 }
@@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
 	writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
 
 	/* Set bits of CONFIG register but with static context switching */
-	/* FIXME: Check whether to set ACR bit depending on clock ratios */
-	writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+	if (sdma->clk_ratio)
+		writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+	else
+		writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
 
 	writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
 
@@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device *pdev)
 	if (!sdma)
 		return -ENOMEM;
 
+	sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
+
 	spin_lock_init(&sdma->channel_0_lock);
 
 	sdma->dev = &pdev->dev;

^ permalink raw reply related

* [v2,1/3] dma: imx-sdma: fix NULL pointer de-reference
From: Angus Ainslie @ 2019-01-20  2:31 UTC (permalink / raw)
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, linux-kernel, Angus Ainslie (Purism)

On the imx8mq I get NULL pointer de-deference errors if the device
isn't passed in during allocation.

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 drivers/dma/imx-sdma.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 86708fb9bda1..0b3a67ff8e82 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -677,7 +677,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 	int ret;
 	unsigned long flags;
 
-	buf_virt = dma_alloc_coherent(NULL, size, &buf_phys, GFP_KERNEL);
+	buf_virt = dma_alloc_coherent(sdma->dev, size, &buf_phys, GFP_KERNEL);
 	if (!buf_virt) {
 		return -ENOMEM;
 	}
@@ -696,7 +696,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 
 	spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
 
-	dma_free_coherent(NULL, size, buf_virt, buf_phys);
+	dma_free_coherent(sdma->dev, size, buf_virt, buf_phys);
 
 	return ret;
 }
@@ -1182,8 +1182,8 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
 {
 	int ret = -EBUSY;
 
-	sdma->bd0 = dma_alloc_coherent(NULL, PAGE_SIZE, &sdma->bd0_phys,
-				       GFP_NOWAIT);
+	sdma->bd0 = dma_alloc_coherent(sdma->dev, PAGE_SIZE, &sdma->bd0_phys,
+					GFP_NOWAIT);
 	if (!sdma->bd0) {
 		ret = -ENOMEM;
 		goto out;
@@ -1205,8 +1205,8 @@ 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_alloc_coherent(NULL, bd_size, &desc->bd_phys,
-				      GFP_NOWAIT);
+	desc->bd = dma_alloc_coherent(desc->sdmac->sdma->dev, bd_size,
+			&desc->bd_phys,	GFP_NOWAIT);
 	if (!desc->bd) {
 		ret = -ENOMEM;
 		goto out;
@@ -1219,7 +1219,8 @@ static void sdma_free_bd(struct sdma_desc *desc)
 {
 	u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
 
-	dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
+	dma_free_coherent(desc->sdmac->sdma->dev, bd_size, desc->bd,
+			desc->bd_phys);
 }
 
 static void sdma_desc_free(struct virt_dma_desc *vd)
@@ -1842,7 +1843,7 @@ static int sdma_init(struct sdma_engine *sdma)
 	/* Be sure SDMA has not started yet */
 	writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
 
-	sdma->channel_control = dma_alloc_coherent(NULL,
+	sdma->channel_control = dma_alloc_coherent(sdma->dev,
 			MAX_DMA_CHANNELS * sizeof (struct sdma_channel_control) +
 			sizeof(struct sdma_context_data),
 			&ccb_phys, GFP_KERNEL);

^ permalink raw reply related

* [3/3] imx8mq.dtsi: add the sdma nodes
From: Angus Ainslie @ 2019-01-20  1:58 UTC (permalink / raw)
  To: angus
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, linux-kernel

Add the sdma nodes to the base devicetree for the imx8mq

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index c0402375e7c1..0b9a9b5ae7b7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -336,6 +336,19 @@
 				clocks = <&clk IMX8MQ_CLK_WDOG3_ROOT>;
 				status = "disabled";
 			};
+
+			sdma2: sdma@302c0000 {
+				compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+				reg = <0x302c0000 0x10000>;
+				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
+					<&clk IMX8MQ_CLK_SDMA2_ROOT>;
+				clock-names = "ipg", "ahb";
+				#dma-cells = <3>;
+				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+				fsl,ratio-1-1;
+				status = "disabled";
+			};
 		};
 
 		bus@30400000 { /* AIPS2 */
@@ -370,6 +383,8 @@
 				clocks = <&clk IMX8MQ_CLK_UART3_ROOT>,
 				         <&clk IMX8MQ_CLK_UART3_ROOT>;
 				clock-names = "ipg", "per";
+				dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
+				dma-names = "rx", "tx";
 				status = "disabled";
 			};
 
@@ -381,6 +396,8 @@
 				clocks = <&clk IMX8MQ_CLK_UART2_ROOT>,
 				         <&clk IMX8MQ_CLK_UART2_ROOT>;
 				clock-names = "ipg", "per";
+				dmas = <&sdma1 24 4 0>, <&sdma1 25 4 0>;
+				dma-names = "rx", "tx";
 				status = "disabled";
 			};
 
@@ -432,6 +449,8 @@
 				clocks = <&clk IMX8MQ_CLK_UART4_ROOT>,
 				         <&clk IMX8MQ_CLK_UART4_ROOT>;
 				clock-names = "ipg", "per";
+				dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
+				dma-names = "rx", "tx";
 				status = "disabled";
 			};
 
@@ -465,6 +484,18 @@
 				status = "disabled";
 			};
 
+			sdma1: sdma@30bd0000 {
+				compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
+				reg = <0x30bd0000 0x10000>;
+				interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
+					<&clk IMX8MQ_CLK_SDMA1_ROOT>;
+				clock-names = "ipg", "ahb";
+				#dma-cells = <3>;
+				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
+				status = "disabled";
+			};
+
 			fec1: ethernet@30be0000 {
 				compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
 				reg = <0x30be0000 0x10000>;

^ permalink raw reply related

* [2/3] dma: imx-sdma: add clock ratio 1:1 check
From: Angus Ainslie @ 2019-01-20  1:58 UTC (permalink / raw)
  To: angus
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, linux-kernel

On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.

based on NXP commit MLK-16841-1

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 .../devicetree/bindings/dma/fsl-imx-sdma.txt  |  2 +-
 drivers/dma/imx-sdma.c                        | 20 +++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 3c9a57a8443b..b3fbac401bd0 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -67,7 +67,7 @@ Optional properties:
     reg is the GPR register offset.
     shift is the bit position inside the GPR register.
     val is the value of the bit (0 or 1).
-
+- fsl,ratio-1-1: AHB/SDMA core clock ration 1:1, 2:1 without this.
 Examples:
 
 sdma@83fb0000 {
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 064bc50d1e99..616ffd6b301e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
 	unsigned int			irq;
 	dma_addr_t			bd0_phys;
 	struct sdma_buffer_descriptor	*bd0;
+	/* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+	bool				clk_ratio;
 };
 
 static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
 		dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
 
 	/* Set bits of CONFIG register with dynamic context switching */
-	if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
-		writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+	if (readl(sdma->regs + SDMA_H_CONFIG) == 0) {
+		if (sdma->clk_ratio)
+			reg = SDMA_H_CONFIG_CSM | SDMA_H_CONFIG_ACR;
+		else
+			reg = SDMA_H_CONFIG_CSM;
+
+		writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+	}
 
 	return ret;
 }
@@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
 	writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
 
 	/* Set bits of CONFIG register but with static context switching */
-	/* FIXME: Check whether to set ACR bit depending on clock ratios */
-	writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+	if (sdma->clk_ratio)
+		writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+	else
+		writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
 
 	writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
 
@@ -1975,6 +1985,8 @@ static int sdma_probe(struct platform_device *pdev)
 	if (!sdma)
 		return -ENOMEM;
 
+	sdma->clk_ratio = of_property_read_bool(np, "fsl,ratio-1-1");
+
 	spin_lock_init(&sdma->channel_0_lock);
 
 	sdma->dev = &pdev->dev;

^ permalink raw reply related

* [1/3] dma: imx-sdma: fix NULL pointer de-reference
From: Angus Ainslie @ 2019-01-20  1:58 UTC (permalink / raw)
  To: angus
  Cc: Vinod Koul, dmaengine, NXP Linux Team, Pengutronix Kernel Team,
	linux-arm-kernel, linux-kernel

On the imx8mq I get NULL pointer de-deference errors if the device
isn't passed in during allocation.

Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 drivers/dma/imx-sdma.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 86708fb9bda1..064bc50d1e99 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -677,7 +677,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 	int ret;
 	unsigned long flags;
 
-	buf_virt = dma_alloc_coherent(NULL, size, &buf_phys, GFP_KERNEL);
+	buf_virt = dma_alloc_coherent(sdma->dev, size, &buf_phys, GFP_KERNEL);
 	if (!buf_virt) {
 		return -ENOMEM;
 	}
@@ -696,7 +696,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 
 	spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
 
-	dma_free_coherent(NULL, size, buf_virt, buf_phys);
+	dma_free_coherent(sdma->dev, size, buf_virt, buf_phys);
 
 	return ret;
 }
@@ -1182,8 +1182,8 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
 {
 	int ret = -EBUSY;
 
-	sdma->bd0 = dma_alloc_coherent(NULL, PAGE_SIZE, &sdma->bd0_phys,
-				       GFP_NOWAIT);
+	sdma->bd0 = dma_zalloc_coherent(sdma->dev, PAGE_SIZE, &sdma->bd0_phys,
+					GFP_NOWAIT);
 	if (!sdma->bd0) {
 		ret = -ENOMEM;
 		goto out;
@@ -1205,8 +1205,8 @@ 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_alloc_coherent(NULL, bd_size, &desc->bd_phys,
-				      GFP_NOWAIT);
+	desc->bd = dma_zalloc_coherent(desc->sdmac->sdma->dev, bd_size,
+			&desc->bd_phys,	GFP_NOWAIT);
 	if (!desc->bd) {
 		ret = -ENOMEM;
 		goto out;
@@ -1219,7 +1219,8 @@ static void sdma_free_bd(struct sdma_desc *desc)
 {
 	u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
 
-	dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
+	dma_free_coherent(desc->sdmac->sdma->dev, bd_size, desc->bd,
+			desc->bd_phys);
 }
 
 static void sdma_desc_free(struct virt_dma_desc *vd)
@@ -1842,7 +1843,7 @@ static int sdma_init(struct sdma_engine *sdma)
 	/* Be sure SDMA has not started yet */
 	writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
 
-	sdma->channel_control = dma_alloc_coherent(NULL,
+	sdma->channel_control = dma_alloc_coherent(sdma->dev,
 			MAX_DMA_CHANNELS * sizeof (struct sdma_channel_control) +
 			sizeof(struct sdma_context_data),
 			&ccb_phys, GFP_KERNEL);

^ permalink raw reply related

* [RFC,v3,1/7] dmaengine: Add Synopsys eDMA IP core driver
From: Andy Shevchenko @ 2019-01-19 16:21 UTC (permalink / raw)
  To: Gustavo Pimentel
  Cc: Jose Abreu, linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
	Vinod Koul, Dan Williams, Eugeniy Paltsev, Russell King,
	Niklas Cassel, Joao Pinto, Luis de Oliveira, Vitor Soares,
	Nelson Costa, Pedro Sousa

On Wed, Jan 16, 2019 at 11:53:00AM +0000, Gustavo Pimentel wrote:
> On 16/01/2019 10:21, Jose Abreu wrote:
> > On 1/11/2019 6:33 PM, Gustavo Pimentel wrote:

> >> +	/* Free irqs */
> > 
> > But devm will automatically free it, no ?
> 
> Yes, it shouldn't be there. I'll remove it.

It depends. If the driver is using tasklets it must free it explicitly like this.

P.S. devm_*_irq() is quite doubtful to have in the first place.

^ permalink raw reply

* [RFC,v3,5/7] dmaengine: Add Synopsys eDMA IP PCIe glue-logic
From: Andy Shevchenko @ 2019-01-19 15:45 UTC (permalink / raw)
  To: Gustavo Pimentel
  Cc: linux-pci@vger.kernel.org, dmaengine@vger.kernel.org, Vinod Koul,
	Dan Williams, Eugeniy Paltsev, Russell King, Niklas Cassel,
	Lorenzo Pieralisi, Joao Pinto, Jose Abreu, Luis Oliveira,
	Vitor Soares, Nelson Costa, Pedro Sousa

On Tue, Jan 15, 2019 at 12:48:34PM +0000, Gustavo Pimentel wrote:
> On 15/01/2019 05:43, Andy Shevchenko wrote:
> > On Mon, Jan 14, 2019 at 11:38:02AM +0000, Gustavo Pimentel wrote:
> >> On 11/01/2019 19:47, Andy Shevchenko wrote:
> >>> On Fri, Jan 11, 2019 at 07:33:41PM +0100, Gustavo Pimentel wrote:

> >>>> +static bool disable_msix;
> >>>> +module_param(disable_msix, bool, 0644);
> >>>> +MODULE_PARM_DESC(disable_msix, "Disable MSI-X interrupts");
> >>>
> >>> Why?!
> >>> We are no allow new module parameters without very strong arguments.
> >>
> >> Since this is a reference driver and might be used to test customized HW
> >> solutions, I added this parameter to allow the possibility to test the solution
> >> forcing the MSI feature binding. This is required specially if who will test
> >> this solution has a Root Complex with both features available (MSI and MSI-X),
> >> because the Kernel will give always preference to MSI-X binding (assuming that
> >> the EP has also both features available).
> > 
> > Yes, you may do it for testing purposes, but it doesn't fit the kernel standards.
> 
> Ok, but how should I proceed? May I leave it or substitute by another way to do
> it? If so, how?
> As I said, the intended is to be only used for this test case, on normal
> operation the parameter it should be always false.

Keep out-of-tree patch for your needs.

> >>>> +	if (!pdata) {
> >>>> +		dev_err(dev, "%s missing data structure\n", pci_name(pdev));
> >>>> +		return -EFAULT;
> >>>> +	}
> >>>
> >>> Useless check.
> >>
> >> Why? It's just a precaution, isn't it a good practice always to think of the
> >> worst case?
> > 
> > You just can put an implicit requirement of pdata rather than doing this
> 
> Ok, how can I do it? What I should add to the code to force that?

Not adding, removing. That's what I put before.

> 
> > useless check. I don't believe it would make sense to have NULL pdata for the
> > driver since it wouldn't be functional anyhow.
> 
> Yes, you're right without pdata the driver can't do anything.

^ permalink raw reply

* dmaengine: fsl-dpaa2-qdma: Add qDMA controller driver for Layerscape SoCs
From: Vinod Koul @ 2019-01-19 13:08 UTC (permalink / raw)
  To: Peng Ma; +Cc: dan.j.williams, leoyang.li, jerry.huang, dmaengine, linux-kernel

On 25-12-18, 11:04, Peng Ma wrote:
> Add NXP Data Path Acceleration Architecture 2 (dpaa2) queue direct
> memory(qDMA) controller driver support.
> This module can be found on NXP LS2 SoCs.
> 
> Signed-off-by: Peng Ma <peng.ma@nxp.com>
> ---
>  drivers/dma/Kconfig                         |    2 +
>  drivers/dma/Makefile                        |    1 +
>  drivers/dma/fsl-dpaa2-qdma/Kconfig          |    8 +
>  drivers/dma/fsl-dpaa2-qdma/Makefile         |    8 +
>  drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.c     |  781 +++++++++++++++++++++++++++
>  drivers/dma/fsl-dpaa2-qdma/dpaa2-qdma.h     |  152 ++++++
>  drivers/dma/fsl-dpaa2-qdma/dpdmai.c         |  483 +++++++++++++++++
>  drivers/dma/fsl-dpaa2-qdma/fsl_dpdmai.h     |  524 ++++++++++++++++++
>  drivers/dma/fsl-dpaa2-qdma/fsl_dpdmai_cmd.h |  197 +++++++
>  9 files changed, 2156 insertions(+), 0 deletions(-)

Sorry I cant review a patch that is this big. Please split and explain
why this is such a big driver...

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox