DMA Engine development
 help / color / mirror / Atom feed
* [v2,1/1] dmaengine: imx-sdma: revert: add clock ratio 1:1 check
From: Fugang Duan @ 2019-03-22  3:36 UTC (permalink / raw)
  To: festevam@gmail.com, vinod.koul@intel.com, angus@akkea.ca
  Cc: dmaengine@vger.kernel.org, Andy Duan

This reverts commit 25aaa75df1e6 (dmaengine: imx-sdma: add clock ratio
1:1 check) due to break SDMA function on i.MX6SX platform.

Log:
imx-sdma 20ec000.sdma: Timeout waiting for CH0 ready

Fixes: 25aaa75df1e6 (dmaengine: imx-sdma: add clock ratio 1:1 check)
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 drivers/dma/imx-sdma.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 5f3c137..7fae4bf 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -441,8 +441,6 @@ 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,
@@ -665,11 +663,8 @@ 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 */
-	reg = readl(sdma->regs + SDMA_H_CONFIG);
-	if ((reg & SDMA_H_CONFIG_CSM) == 0) {
-		reg |= SDMA_H_CONFIG_CSM;
-		writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
-	}
+	if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
+		writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
 
 	return ret;
 }
@@ -1852,9 +1847,6 @@ static int sdma_init(struct sdma_engine *sdma)
 	if (ret)
 		goto disable_clk_ipg;
 
-	if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
-		sdma->clk_ratio = 1;
-
 	/* Be sure SDMA has not started yet */
 	writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
 
@@ -1895,10 +1887,8 @@ 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 */
-	if (sdma->clk_ratio)
-		writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
-	else
-		writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+	/* FIXME: Check whether to set ACR bit depending on clock ratios */
+	writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
 
 	writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
 

^ permalink raw reply related

* [1/1] dmaengine: imx-sdma: revert: add clock ratio 1:1 check
From: Fugang Duan @ 2019-03-22  3:31 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: vinod.koul@intel.com, angus@akkea.ca, dmaengine@vger.kernel.org

From: Fabio Estevam <festevam@gmail.com> Sent: Friday, March 22, 2019 11:11 AM
> On Fri, Mar 22, 2019 at 12:05 AM Andy Duan <fugang.duan@nxp.com> wrote:
> >
> > This reverts commit 25aaa75df1e6 (dmaengine: imx-sdma: add clock ratio
> > 1:1 check) due to break SDMA function on i.MX6SX platform.
> >
> > Log:
> > imx-sdma 20ec000.sdma: Timeout waiting for CH0 ready
> >
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> 
> Please add a Fixes tag, so that it can go to 5.1-rc as a fix.

Okay,  I will send the v2 to add the fixes tag.

^ permalink raw reply

* [1/1] dmaengine: imx-sdma: revert: add clock ratio 1:1 check
From: Fabio Estevam @ 2019-03-22  3:10 UTC (permalink / raw)
  To: Andy Duan; +Cc: vinod.koul@intel.com, angus@akkea.ca, dmaengine@vger.kernel.org

On Fri, Mar 22, 2019 at 12:05 AM Andy Duan <fugang.duan@nxp.com> wrote:
>
> This reverts commit 25aaa75df1e6 (dmaengine: imx-sdma: add clock ratio
> 1:1 check) due to break SDMA function on i.MX6SX platform.
>
> Log:
> imx-sdma 20ec000.sdma: Timeout waiting for CH0 ready
>
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>

Please add a Fixes tag, so that it can go to 5.1-rc as a fix.

^ permalink raw reply

* [1/1] dmaengine: imx-sdma: revert: add clock ratio 1:1 check
From: Fugang Duan @ 2019-03-22  3:05 UTC (permalink / raw)
  To: vinod.koul@intel.com, angus@akkea.ca; +Cc: dmaengine@vger.kernel.org, Andy Duan

This reverts commit 25aaa75df1e6 (dmaengine: imx-sdma: add clock ratio
1:1 check) due to break SDMA function on i.MX6SX platform.

Log:
imx-sdma 20ec000.sdma: Timeout waiting for CH0 ready

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 drivers/dma/imx-sdma.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 5f3c137..7fae4bf 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -441,8 +441,6 @@ 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,
@@ -665,11 +663,8 @@ 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 */
-	reg = readl(sdma->regs + SDMA_H_CONFIG);
-	if ((reg & SDMA_H_CONFIG_CSM) == 0) {
-		reg |= SDMA_H_CONFIG_CSM;
-		writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
-	}
+	if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
+		writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
 
 	return ret;
 }
@@ -1852,9 +1847,6 @@ static int sdma_init(struct sdma_engine *sdma)
 	if (ret)
 		goto disable_clk_ipg;
 
-	if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
-		sdma->clk_ratio = 1;
-
 	/* Be sure SDMA has not started yet */
 	writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
 
@@ -1895,10 +1887,8 @@ 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 */
-	if (sdma->clk_ratio)
-		writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
-	else
-		writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+	/* FIXME: Check whether to set ACR bit depending on clock ratios */
+	writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
 
 	writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
 

^ permalink raw reply related

* [v1] dmaengine: idma64: Use actual device for DMA transfers
From: Vinod Koul @ 2019-03-21 14:18 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: dmaengine, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
	linux-arm-kernel, Mark Brown, linux-spi, Greg Kroah-Hartman,
	linux-serial, linux-kernel

On 18-03-19, 18:39, Andy Shevchenko wrote:
> Intel IOMMU, when enabled, tries to find the domain of the device,
> assuming it's a PCI one, during DMA operations, such as mapping or
> unmapping. Since we are splitting the actual PCI device to couple of
> children via MFD framework (see drivers/mfd/intel-lpss.c for details),
> the DMA device appears to be a platform one, and thus not an actual one
> that performs DMA. In a such situation IOMMU can't find or allocate
> a proper domain for its operations. As a result, all DMA operations are
> failed.
> 
> In order to fix this, supply parent of the platform device
> to the DMA engine framework and fix filter functions accordingly.
> 
> We may rely on the fact that parent is a real PCI device, because no
> other configuration is present in the wild.

Applied, thanks

^ permalink raw reply

* dma:xgene-dma:move spin_lock_bh to spin_lock in tasklet
From: Jeff Xie @ 2019-03-19 16:45 UTC (permalink / raw)
  To: dan.j.williams, vkoul; +Cc: dmaengine, chongguiguzi, linux-kernel

It is unnecessary to call spin_lock_bh in a tasklet.

Signed-off-by: Jeff Xie <chongguiguzi@gmail.com>
---
 drivers/dma/xgene-dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c
index eafd6c4..0023236 100644
--- a/drivers/dma/xgene-dma.c
+++ b/drivers/dma/xgene-dma.c
@@ -703,7 +703,7 @@ static void xgene_dma_cleanup_descriptors(struct xgene_dma_chan *chan)
 
 	INIT_LIST_HEAD(&ld_completed);
 
-	spin_lock_bh(&chan->lock);
+	spin_lock(&chan->lock);
 
 	/* Clean already completed and acked descriptors */
 	xgene_dma_clean_completed_descriptor(chan);
@@ -772,7 +772,7 @@ static void xgene_dma_cleanup_descriptors(struct xgene_dma_chan *chan)
 	 */
 	xgene_chan_xfer_ld_pending(chan);
 
-	spin_unlock_bh(&chan->lock);
+	spin_unlock(&chan->lock);
 
 	/* Run the callback for each descriptor, in order */
 	list_for_each_entry_safe(desc_sw, _desc_sw, &ld_completed, node) {

^ permalink raw reply related

* [v1] dmaengine: idma64: Use actual device for DMA transfers
From: Greg Kroah-Hartman @ 2019-03-19 14:25 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Vinod Koul, dmaengine, Daniel Mack, Haojian Zhuang,
	Robert Jarzmik, linux-arm-kernel, Mark Brown, linux-spi,
	linux-serial, linux-kernel

On Mon, Mar 18, 2019 at 06:39:30PM +0300, Andy Shevchenko wrote:
> Intel IOMMU, when enabled, tries to find the domain of the device,
> assuming it's a PCI one, during DMA operations, such as mapping or
> unmapping. Since we are splitting the actual PCI device to couple of
> children via MFD framework (see drivers/mfd/intel-lpss.c for details),
> the DMA device appears to be a platform one, and thus not an actual one
> that performs DMA. In a such situation IOMMU can't find or allocate
> a proper domain for its operations. As a result, all DMA operations are
> failed.
> 
> In order to fix this, supply parent of the platform device
> to the DMA engine framework and fix filter functions accordingly.
> 
> We may rely on the fact that parent is a real PCI device, because no
> other configuration is present in the wild.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

For the tty/serial/ portion:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply

* [v1] dmaengine: idma64: Use actual device for DMA transfers
From: Mark Brown @ 2019-03-19 12:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Vinod Koul, dmaengine, Daniel Mack, Haojian Zhuang,
	Robert Jarzmik, linux-arm-kernel, linux-spi, Greg Kroah-Hartman,
	linux-serial, linux-kernel

On Mon, Mar 18, 2019 at 06:39:30PM +0300, Andy Shevchenko wrote:
> Intel IOMMU, when enabled, tries to find the domain of the device,
> assuming it's a PCI one, during DMA operations, such as mapping or
> unmapping. Since we are splitting the actual PCI device to couple of

Acked-by: Mark Brown <broonie@kernel.org>

^ permalink raw reply

* [v1] dmaengine: idma64: Use actual device for DMA transfers
From: Andy Shevchenko @ 2019-03-18 15:39 UTC (permalink / raw)
  To: Vinod Koul, dmaengine, Daniel Mack, Haojian Zhuang,
	Robert Jarzmik, linux-arm-kernel, Mark Brown, linux-spi,
	Greg Kroah-Hartman, linux-serial, linux-kernel
  Cc: Andy Shevchenko

Intel IOMMU, when enabled, tries to find the domain of the device,
assuming it's a PCI one, during DMA operations, such as mapping or
unmapping. Since we are splitting the actual PCI device to couple of
children via MFD framework (see drivers/mfd/intel-lpss.c for details),
the DMA device appears to be a platform one, and thus not an actual one
that performs DMA. In a such situation IOMMU can't find or allocate
a proper domain for its operations. As a result, all DMA operations are
failed.

In order to fix this, supply parent of the platform device
to the DMA engine framework and fix filter functions accordingly.

We may rely on the fact that parent is a real PCI device, because no
other configuration is present in the wild.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/idma64.c              | 6 ++++--
 drivers/dma/idma64.h              | 2 ++
 drivers/spi/spi-pxa2xx.c          | 7 +------
 drivers/tty/serial/8250/8250_dw.c | 4 ++--
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/idma64.c b/drivers/dma/idma64.c
index 0baf9797cc09..83796a33dc16 100644
--- a/drivers/dma/idma64.c
+++ b/drivers/dma/idma64.c
@@ -592,7 +592,7 @@ static int idma64_probe(struct idma64_chip *chip)
 	idma64->dma.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
 	idma64->dma.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
 
-	idma64->dma.dev = chip->dev;
+	idma64->dma.dev = chip->sysdev;
 
 	dma_set_max_seg_size(idma64->dma.dev, IDMA64C_CTLH_BLOCK_TS_MASK);
 
@@ -632,6 +632,7 @@ static int idma64_platform_probe(struct platform_device *pdev)
 {
 	struct idma64_chip *chip;
 	struct device *dev = &pdev->dev;
+	struct device *sysdev = dev->parent;
 	struct resource *mem;
 	int ret;
 
@@ -648,11 +649,12 @@ static int idma64_platform_probe(struct platform_device *pdev)
 	if (IS_ERR(chip->regs))
 		return PTR_ERR(chip->regs);
 
-	ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	ret = dma_coerce_mask_and_coherent(sysdev, DMA_BIT_MASK(64));
 	if (ret)
 		return ret;
 
 	chip->dev = dev;
+	chip->sysdev = sysdev;
 
 	ret = idma64_probe(chip);
 	if (ret)
diff --git a/drivers/dma/idma64.h b/drivers/dma/idma64.h
index 6b816878e5e7..baa32e1425de 100644
--- a/drivers/dma/idma64.h
+++ b/drivers/dma/idma64.h
@@ -216,12 +216,14 @@ static inline void idma64_writel(struct idma64 *idma64, int offset, u32 value)
 /**
  * struct idma64_chip - representation of iDMA 64-bit controller hardware
  * @dev:		struct device of the DMA controller
+ * @sysdev:		struct device of the physical device that does DMA
  * @irq:		irq line
  * @regs:		memory mapped I/O space
  * @idma64:		struct idma64 that is filed by idma64_probe()
  */
 struct idma64_chip {
 	struct device	*dev;
+	struct device	*sysdev;
 	int		irq;
 	void __iomem	*regs;
 	struct idma64	*idma64;
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index b6ddba833d02..5ea70f7d12e7 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1487,12 +1487,7 @@ static int pxa2xx_spi_get_port_id(struct acpi_device *adev)
 
 static bool pxa2xx_spi_idma_filter(struct dma_chan *chan, void *param)
 {
-	struct device *dev = param;
-
-	if (dev != chan->device->dev->parent)
-		return false;
-
-	return true;
+	return param == chan->device->dev;
 }
 
 #endif /* CONFIG_PCI */
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d31b975dd3fd..284e8d052fc3 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -365,7 +365,7 @@ static bool dw8250_fallback_dma_filter(struct dma_chan *chan, void *param)
 
 static bool dw8250_idma_filter(struct dma_chan *chan, void *param)
 {
-	return param == chan->device->dev->parent;
+	return param == chan->device->dev;
 }
 
 /*
@@ -434,7 +434,7 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 		data->uart_16550_compatible = true;
 	}
 
-	/* Platforms with iDMA */
+	/* Platforms with iDMA 64-bit */
 	if (platform_get_resource_byname(to_platform_device(p->dev),
 					 IORESOURCE_MEM, "lpss_priv")) {
 		data->dma.rx_param = p->dev->parent;

^ permalink raw reply related

* [v2] dmaengine: pl330: introduce debugfs interface
From: Katsuhiro Suzuki @ 2019-03-17 10:03 UTC (permalink / raw)
  To: Vinod Koul, dmaengine; +Cc: linux-kernel, Katsuhiro Suzuki

This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if a user specify DMA channel ID in devicetree. This interface will
be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f0000.dmac
PL330 physical channels:
THREAD:         CHANNEL:
--------        -----
0               8
1               9
2               11
3               12
4               14
5               15
6               10
7               --

Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
---
 drivers/dma/pl330.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..c72f6fd79c43 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/debugfs.h>
 #include <linux/kernel.h>
 #include <linux/io.h>
 #include <linux/init.h>
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
 	BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
 	BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
 
+#ifdef CONFIG_DEBUG_FS
+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+	struct pl330_dmac *pl330 = s->private;
+	int chans, pchs, ch, pr;
+
+	chans = pl330->pcfg.num_chan;
+	pchs = pl330->num_peripherals;
+
+	seq_puts(s, "PL330 physical channels:\n");
+	seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+	seq_puts(s, "--------\t-----\n");
+	for (ch = 0; ch < chans; ch++) {
+		struct pl330_thread *thrd = &pl330->channels[ch];
+		int found = -1;
+
+		for (pr = 0; pr < pchs; pr++) {
+			struct dma_pl330_chan *pch = &pl330->peripherals[pr];
+
+			if (!pch->thread || thrd->id != pch->thread->id)
+				continue;
+
+			found = pr;
+		}
+
+		seq_printf(s, "%d\t\t", thrd->id);
+		if (found == -1)
+			seq_puts(s, "--\n");
+		else
+			seq_printf(s, "%d\n", found);
+	}
+
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+	debugfs_create_file(dev_name(pl330->ddma.dev),
+			    S_IFREG | 0444, NULL, pl330,
+			    &pl330_debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
 /*
  * Runtime PM callbacks are provided by amba/bus.c driver.
  *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		dev_err(&adev->dev, "unable to set the seg size\n");
 
 
+	init_pl330_debugfs(pl330);
 	dev_info(&adev->dev,
 		"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
 	dev_info(&adev->dev,

^ permalink raw reply related

* dmaengine: pl330: introduce debugfs interface
From: Katsuhiro Suzuki @ 2019-03-17 10:02 UTC (permalink / raw)
  To: Vinod Koul, dmaengine; +Cc: linux-kernel

Hello,

Sorry, I got mistake in title of this patch...
Please ignore this patch.

Best Regards,
Katsuhiro Suzuki

On 2019/03/17 19:00, Katsuhiro Suzuki wrote:
> This patch adds debugfs interface to show the relationship between
> DMA threads (hardware resource for transferring data) and DMA
> channel ID of DMA slave.
> 
> Typically, PL330 has many slaves than number of DMA threads.
> So sometimes PL330 cannot allocate DMA threads for all slaves even
> if a user specify DMA channel ID in devicetree. This interface will
> be useful for checking that DMA threads are allocated or not.
> 
> Below is an output sample:
> 
> $ sudo cat /sys/kernel/debug/ff1f0000.dmac
> PL330 physical channels:
> THREAD:         CHANNEL:
> --------        -----
> 0               8
> 1               9
> 2               11
> 3               12
> 4               14
> 5               15
> 6               10
> 7               --
> 
> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> ---
>   drivers/dma/pl330.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index eec79fdf27a5..c72f6fd79c43 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -11,6 +11,7 @@
>    * (at your option) any later version.
>    */
>   
> +#include <linux/debugfs.h>
>   #include <linux/kernel.h>
>   #include <linux/io.h>
>   #include <linux/init.h>
> @@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
>   	BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
>   	BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
>   
> +#ifdef CONFIG_DEBUG_FS
> +static int pl330_debugfs_show(struct seq_file *s, void *data)
> +{
> +	struct pl330_dmac *pl330 = s->private;
> +	int chans, pchs, ch, pr;
> +
> +	chans = pl330->pcfg.num_chan;
> +	pchs = pl330->num_peripherals;
> +
> +	seq_puts(s, "PL330 physical channels:\n");
> +	seq_puts(s, "THREAD:\t\tCHANNEL:\n");
> +	seq_puts(s, "--------\t-----\n");
> +	for (ch = 0; ch < chans; ch++) {
> +		struct pl330_thread *thrd = &pl330->channels[ch];
> +		int found = -1;
> +
> +		for (pr = 0; pr < pchs; pr++) {
> +			struct dma_pl330_chan *pch = &pl330->peripherals[pr];
> +
> +			if (!pch->thread || thrd->id != pch->thread->id)
> +				continue;
> +
> +			found = pr;
> +		}
> +
> +		seq_printf(s, "%d\t\t", thrd->id);
> +		if (found == -1)
> +			seq_puts(s, "--\n");
> +		else
> +			seq_printf(s, "%d\n", found);
> +	}
> +
> +	return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
> +
> +static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
> +{
> +	debugfs_create_file(dev_name(pl330->ddma.dev),
> +			    S_IFREG | 0444, NULL, pl330,
> +			    &pl330_debugfs_fops);
> +}
> +#else
> +static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
> +{
> +}
> +#endif
> +
>   /*
>    * Runtime PM callbacks are provided by amba/bus.c driver.
>    *
> @@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>   		dev_err(&adev->dev, "unable to set the seg size\n");
>   
>   
> +	init_pl330_debugfs(pl330);
>   	dev_info(&adev->dev,
>   		"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
>   	dev_info(&adev->dev,
>

^ permalink raw reply

* dmaengine: pl330: introduce debugfs interface
From: Katsuhiro Suzuki @ 2019-03-17 10:00 UTC (permalink / raw)
  To: Vinod Koul, dmaengine; +Cc: linux-kernel, Katsuhiro Suzuki

This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if a user specify DMA channel ID in devicetree. This interface will
be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f0000.dmac
PL330 physical channels:
THREAD:         CHANNEL:
--------        -----
0               8
1               9
2               11
3               12
4               14
5               15
6               10
7               --

Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
---
 drivers/dma/pl330.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..c72f6fd79c43 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/debugfs.h>
 #include <linux/kernel.h>
 #include <linux/io.h>
 #include <linux/init.h>
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
 	BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
 	BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
 
+#ifdef CONFIG_DEBUG_FS
+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+	struct pl330_dmac *pl330 = s->private;
+	int chans, pchs, ch, pr;
+
+	chans = pl330->pcfg.num_chan;
+	pchs = pl330->num_peripherals;
+
+	seq_puts(s, "PL330 physical channels:\n");
+	seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+	seq_puts(s, "--------\t-----\n");
+	for (ch = 0; ch < chans; ch++) {
+		struct pl330_thread *thrd = &pl330->channels[ch];
+		int found = -1;
+
+		for (pr = 0; pr < pchs; pr++) {
+			struct dma_pl330_chan *pch = &pl330->peripherals[pr];
+
+			if (!pch->thread || thrd->id != pch->thread->id)
+				continue;
+
+			found = pr;
+		}
+
+		seq_printf(s, "%d\t\t", thrd->id);
+		if (found == -1)
+			seq_puts(s, "--\n");
+		else
+			seq_printf(s, "%d\n", found);
+	}
+
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+	debugfs_create_file(dev_name(pl330->ddma.dev),
+			    S_IFREG | 0444, NULL, pl330,
+			    &pl330_debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
 /*
  * Runtime PM callbacks are provided by amba/bus.c driver.
  *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		dev_err(&adev->dev, "unable to set the seg size\n");
 
 
+	init_pl330_debugfs(pl330);
 	dev_info(&adev->dev,
 		"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
 	dev_info(&adev->dev,

^ permalink raw reply related

* dmaengine: pl330: introduce debugfs interface
From: Katsuhiro Suzuki @ 2019-03-17  9:58 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dmaengine, linux-kernel

Hello Vinod,

Thank you for your comment.
I fix it all and re-post v2 patch.


On 2019/03/16 2:00, Vinod Koul wrote:
> On 15-03-19, 03:49, Katsuhiro Suzuki wrote:
>> This patch adds debugfs interface to show the relationship between
>> DMA threads (hardware resource for transferring data) and DMA
>> channel ID of DMA slave.
>>
>> Typically, PL330 has many slaves than number of DMA threads.
>> So sometimes PL330 cannot allocate DMA threads for all slaves even
>> if an user specify DMA channel ID in devicetree. This interface will
> 
> a user :)
> 
>> be useful for checking that DMA threads are allocated or not.
>>
>> Below is an output sample:
>>
>> $ sudo cat /sys/kernel/debug/ff1f0000.dmac
>> PL330 physical channels:
>> THREAD:         CHANNEL:
>> --------        -----
>> 0               8
>> 1               9
>> 2               11
>> 3               12
>> 4               14
>> 5               15
>> 6               10
>> 7               --
>>
>> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
>> ---
>>   drivers/dma/pl330.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index eec79fdf27a5..aab71863757c 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -11,6 +11,7 @@
>>    * (at your option) any later version.
>>    */
>>   
>> +#include <linux/debugfs.h>
>>   #include <linux/kernel.h>
>>   #include <linux/io.h>
>>   #include <linux/init.h>
>> @@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
>>   	BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
>>   	BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
>>   
>> +#ifdef CONFIG_DEBUG_FS
>> +static int pl330_debugfs_show(struct seq_file *s, void *data)
>> +{
>> +	struct pl330_dmac *pl330 = s->private;
>> +	int chans, pchs, ch, pr, found;
>> +
>> +	chans = pl330->pcfg.num_chan;
>> +	pchs = pl330->num_peripherals;
>> +
>> +	seq_puts(s, "PL330 physical channels:\n");
>> +	seq_puts(s, "THREAD:\t\tCHANNEL:\n");
>> +	seq_puts(s, "--------\t-----\n");
>> +	for (ch = 0; ch < chans; ch++) {
>> +		struct pl330_thread *thrd = &pl330->channels[ch];
>> +
>> +		found = -1;
> 
> found can be uninitialized if we skip this for, which should not be an
> issue as chans> 0!
> 

Exactly. Thanks, 'found' should be changed to for-loop local variable.


>> +		for (pr = 0; pr < pchs; pr++) {
>> +			struct dma_pl330_chan *pch = &pl330->peripherals[pr];
>> +
>> +			if (!pch->thread || thrd->id != pch->thread->id)
>> +				continue;
>> +
>> +			found = pr;
>> +		}
>> +
>> +		seq_printf(s, "%d\t\t", thrd->id);
>> +		if (found == -1)
>> +			seq_puts(s, "--\n");
>> +		else
>> +			seq_printf(s, "%d\n", found);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
>> +
>> +static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
>> +{
>> +	debugfs_create_file(dev_name(pl330->ddma.dev),
>> +			    S_IFREG | S_IRUGO, NULL, pl330,
> 
> are we not supposed to use octal permissions?
> 
>> +			    &pl330_debugfs_fops);
>> +}
>> +#else
>> +static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
>> +{
>> +}
>> +#endif
>> +
>>   /*
>>    * Runtime PM callbacks are provided by amba/bus.c driver.
>>    *
>> @@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>>   		dev_err(&adev->dev, "unable to set the seg size\n");
>>   
>>   
>> +	init_pl330_debugfs(pl330);
>>   	dev_info(&adev->dev,
>>   		"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
>>   	dev_info(&adev->dev,
>> -- 
>> 2.20.1
> 

Best Regards,
Katsuhiro Suzuki

^ permalink raw reply

* [5/6] dmaengine: sun6i: Add support for H6 DMA
From: Jernej Škrabec @ 2019-03-16 11:37 UTC (permalink / raw)
  To: Vinod Koul
  Cc: maxime.ripard, wens, robh+dt, mark.rutland, dan.j.williams,
	dmaengine, devicetree, linux-arm-kernel, linux-kernel,
	linux-sunxi

Dne sobota, 16. marec 2019 ob 12:13:24 CET je Vinod Koul napisal(a):
> On 07-03-19, 17:58, Jernej Skrabec wrote:
> > H6 DMA has more than 32 supported DRQs, which means that configuration
> > register is slightly rearranged. It also needs additional clock to be
> > enabled.
> 
> Okay how many register are rearraged in the new IP block. If there are
> large changes, consider using regmap_fields to abstract the register and
> bit differences..

Only one, config register. Regmap unfortunately is not an option here, because 
how DMA peripheral works. Register values are actually stored in linked list 
somewhere in RAM and when current DMA request is finished, DMA peripheral auto 
loads next set of registers from memory.

> 
> > Add support for it.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 44 +++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 42 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 6a37f8bb39b1..eceedd139651 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -69,14 +69,19 @@
> > 
> >  #define DMA_CHAN_CUR_CFG	0x0c
> >  #define DMA_CHAN_MAX_DRQ_A31		0x1f
> > 
> > +#define DMA_CHAN_MAX_DRQ_H6		0x3f
> > 
> >  #define DMA_CHAN_CFG_SRC_DRQ_A31(x)	((x) & DMA_CHAN_MAX_DRQ_A31)
> > 
> > +#define DMA_CHAN_CFG_SRC_DRQ_H6(x)	((x) & DMA_CHAN_MAX_DRQ_H6)
> > 
> >  #define DMA_CHAN_CFG_SRC_MODE_A31(x)	(((x) & 0x1) << 5)
> > 
> > +#define DMA_CHAN_CFG_SRC_MODE_H6(x)	(((x) & 0x1) << 8)
> > 
> >  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
> >  #define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
> >  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
> >  
> >  #define DMA_CHAN_CFG_DST_DRQ_A31(x)	(DMA_CHAN_CFG_SRC_DRQ_A31(x) << 
16)
> > 
> > +#define DMA_CHAN_CFG_DST_DRQ_H6(x)	(DMA_CHAN_CFG_SRC_DRQ_H6(x) << 16)
> > 
> >  #define DMA_CHAN_CFG_DST_MODE_A31(x)	(DMA_CHAN_CFG_SRC_MODE_A31(x) << 
16)
> > 
> > +#define DMA_CHAN_CFG_DST_MODE_H6(x)	(DMA_CHAN_CFG_SRC_MODE_H6(x) << 16)
> > 
> >  #define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) <<
> >  16)
> >  #define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 
16)
> >  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
> > 
> > @@ -319,12 +324,24 @@ static void sun6i_set_drq_a31(u32 *p_cfg, s8
> > src_drq, s8 dst_drq)> 
> >  		  DMA_CHAN_CFG_DST_DRQ_A31(dst_drq);
> >  
> >  }
> > 
> > +static void sun6i_set_drq_h6(u32 *p_cfg, s8 src_drq, s8 dst_drq)
> > +{
> > +	*p_cfg |= DMA_CHAN_CFG_SRC_DRQ_H6(src_drq) |
> > +		  DMA_CHAN_CFG_DST_DRQ_H6(dst_drq);
> > +}
> > +
> > 
> >  static void sun6i_set_mode_a31(u32 *p_cfg, s8 src_mode, s8 dst_mode)
> >  {
> >  
> >  	*p_cfg |= DMA_CHAN_CFG_SRC_MODE_A31(src_mode) |
> >  	
> >  		  DMA_CHAN_CFG_DST_MODE_A31(dst_mode);
> >  
> >  }
> > 
> > +static void sun6i_set_mode_h6(u32 *p_cfg, s8 src_mode, s8 dst_mode)
> > +{
> > +	*p_cfg |= DMA_CHAN_CFG_SRC_MODE_H6(src_mode) |
> > +		  DMA_CHAN_CFG_DST_MODE_H6(dst_mode);
> > +}
> > +
> > 
> >  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
> >  {
> >  
> >  	struct sun6i_desc *txd = pchan->desc;
> > 
> > @@ -1160,6 +1177,28 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg =
> > {> 
> >  			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
> >  
> >  };
> > 
> > +/*
> > + * The H6 binding uses the number of dma channels from the
> > + * device tree node.
> > + */
> > +static struct sun6i_dma_config sun50i_h6_dma_cfg = {
> > +	.clock_autogate_enable = sun6i_enable_clock_autogate_h3,
> > +	.set_burst_length = sun6i_set_burst_length_h3,
> > +	.set_drq          = sun6i_set_drq_h6,
> > +	.set_mode         = sun6i_set_mode_h6,
> > +	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> > +	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> > +	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > +			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> > +			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
> > +			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
> > +	.dst_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > +			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> > +			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
> > +			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
> > +	.mbus_clk = true,
> > +};
> > +
> > 
> >  /*
> >  
> >   * The V3s have only 8 physical channels, a maximum DRQ port id of 23,
> >   * and a total of 24 usable source and destination endpoints.
> > 
> > @@ -1190,6 +1229,7 @@ static const struct of_device_id sun6i_dma_match[] =
> > {> 
> >  	{ .compatible = "allwinner,sun8i-h3-dma", .data = 
&sun8i_h3_dma_cfg },
> >  	{ .compatible = "allwinner,sun8i-v3s-dma", .data = 
&sun8i_v3s_dma_cfg },
> >  	{ .compatible = "allwinner,sun50i-a64-dma", .data = 
&sun50i_a64_dma_cfg
> >  	},
> > 
> > +	{ .compatible = "allwinner,sun50i-h6-dma", .data = 
&sun50i_h6_dma_cfg },
> > 
> >  	{ /* sentinel */ }
> >  
> >  };
> >  MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> > 
> > @@ -1288,8 +1328,8 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)> 
> >  	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
> >  	if (ret && !sdc->max_request) {
> >  	
> >  		dev_info(&pdev->dev, "Missing dma-requests, using %u.
\n",
> > 
> > -			 DMA_CHAN_MAX_DRQ_A31);
> > -		sdc->max_request = DMA_CHAN_MAX_DRQ_A31;
> > +			 DMA_CHAN_MAX_DRQ_H6);
> > +		sdc->max_request = DMA_CHAN_MAX_DRQ_H6;
> > 
> >  	}
> >  	
> >  	/*

^ permalink raw reply

* [1/6] dt-bindings: arm64: allwinner: h6: Add binding for DMA controller
From: Jernej Škrabec @ 2019-03-16 11:27 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Rob Herring, maxime.ripard, wens, mark.rutland, dan.j.williams,
	dmaengine, devicetree, linux-arm-kernel, linux-kernel,
	linux-sunxi

Dne sobota, 16. marec 2019 ob 11:57:11 CET je Vinod Koul napisal(a):
> On 07-03-19, 17:58, Jernej Skrabec wrote:
> > DMA in H6 is similar to other DMA controller, except it is first which
> > supports more than 32 request sources and has 16 channels. It also needs
> > additional clock to be enabled.
> 
> Please see DT folks on this patch, when in doubt, use get_maintainers.pl
> Need an ack on this patch before applying

They are already in CC.

> 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  Documentation/devicetree/bindings/dma/sun6i-dma.txt | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> > b/Documentation/devicetree/bindings/dma/sun6i-dma.txt index
> > 7fccc20d8331..cae31f4e77ba 100644
> > --- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> > +++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> > 
> > @@ -28,12 +28,17 @@ Example:
> >  	};
> >  
> >  -------------------------------------------------------------------------
> >  -----> 
> > -For A64 DMA controller:
> > 
> > +For A64 and H6 DMA controller:
> >  Required properties:
> > -- compatible:	"allwinner,sun50i-a64-dma"
> > +- compatible:	Must be one of
> > +		  "allwinner,sun50i-a64-dma"
> > +		  "allwinner,sun50i-h6-dma"
> > 
> >  - dma-channels: Number of DMA channels supported by the controller.
> >  
> >  		Refer to Documentation/devicetree/bindings/dma/dma.txt
> > 
> > +- clocks:	In addition to parent AHB clock, it should also contain mbus
> > +		clock (H6 only)
> > +- clock-names:	Should contain "bus" and "mbus" (H6 only)
> > 
> >  - all properties above, i.e. reg, interrupts, clocks, resets and
> >  #dma-cells
> >  
> >  Optional properties:

^ permalink raw reply

* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
From: Jernej Škrabec @ 2019-03-16 11:23 UTC (permalink / raw)
  To: Vinod Koul
  Cc: maxime.ripard, wens, robh+dt, mark.rutland, dan.j.williams,
	dmaengine, devicetree, linux-arm-kernel, linux-kernel,
	linux-sunxi

Hi!

Dne sobota, 16. marec 2019 ob 12:07:53 CET je Vinod Koul napisal(a):
> On 07-03-19, 17:58, Jernej Skrabec wrote:
> > H6 DMA controller needs additional mbus clock to be enabled.
> > 
> > Add a quirk for it and handle it accordingly.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 0cd13f17fc11..761555080325 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -129,6 +129,7 @@ struct sun6i_dma_config {
> > 
> >  	u32 dst_burst_lengths;
> >  	u32 src_addr_widths;
> >  	u32 dst_addr_widths;
> > 
> > +	bool mbus_clk;
> > 
> >  };
> >  
> >  /*
> > 
> > @@ -182,6 +183,7 @@ struct sun6i_dma_dev {
> > 
> >  	struct dma_device	slave;
> >  	void __iomem		*base;
> >  	struct clk		*clk;
> > 
> > +	struct clk		*clk_mbus;
> 
> So rather than have mbus_clk and then a ptr, why not use the ptr and use
> NULL value to check for..?
> 

I'm not sure what you mean here. clk_mbus will hold a reference to a clock 
retrieved by devm_clk_get() so it has to be "struct clk *".

What I'm missing here?

> >  	int			irq;
> >  	spinlock_t		lock;
> >  	struct reset_control	*rstc;
> > 
> > @@ -1208,6 +1210,14 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)> 
> >  		return PTR_ERR(sdc->clk);
> >  	
> >  	}
> > 
> > +	if (sdc->cfg->mbus_clk) {
> 
> where is the populated? I was expecting this to be set based on DT!

Of course it is based on DT. Check patch 5, where quirks structure attached to 
H6 DMA compatible contains ".mbus_clk = true,". "sdc->cfg" points to this 
quirk structure.

> 
> > +		sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus");
> > +		if (IS_ERR(sdc->clk_mbus)) {
> > +			dev_err(&pdev->dev, "No mbus clock 
specified\n");
> > +			return PTR_ERR(sdc->clk_mbus);
> > +		}
> > +	}
> > +
> > 
> >  	sdc->rstc = devm_reset_control_get(&pdev->dev, NULL);
> >  	if (IS_ERR(sdc->rstc)) {
> >  	
> >  		dev_err(&pdev->dev, "No reset controller specified\n");
> > 
> > @@ -1312,11 +1322,19 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)> 
> >  		goto err_reset_assert;
> >  	
> >  	}
> > 
> > +	if (sdc->cfg->mbus_clk) {
> > +		ret = clk_prepare_enable(sdc->clk_mbus);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "Couldn't enable mbus 
clock\n");
> > +			goto err_clk_disable;
> > +		}
> > +	}
> > +
> > 
> >  	ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt, 
0,
> >  	
> >  			       dev_name(&pdev->dev), sdc);
> >  	
> >  	if (ret) {
> >  	
> >  		dev_err(&pdev->dev, "Cannot request IRQ\n");
> > 
> > -		goto err_clk_disable;
> > +		goto err_mbus_clk_disable;
> > 
> >  	}
> >  	
> >  	ret = dma_async_device_register(&sdc->slave);
> > 
> > @@ -1341,6 +1359,8 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)> 
> >  	dma_async_device_unregister(&sdc->slave);
> >  
> >  err_irq_disable:
> >  	sun6i_kill_tasklet(sdc);
> > 
> > +err_mbus_clk_disable:
> > +	clk_disable_unprepare(sdc->clk_mbus);
> > 
> >  err_clk_disable:
> >  	clk_disable_unprepare(sdc->clk);
> >  
> >  err_reset_assert:
> > @@ -1359,6 +1379,7 @@ static int sun6i_dma_remove(struct platform_device
> > *pdev)> 
> >  	sun6i_kill_tasklet(sdc);
> > 
> > +	clk_disable_unprepare(sdc->clk_mbus);
> > 
> >  	clk_disable_unprepare(sdc->clk);
> >  	reset_control_assert(sdc->rstc);

^ permalink raw reply

* [5/6] dmaengine: sun6i: Add support for H6 DMA
From: Vinod Koul @ 2019-03-16 11:13 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: maxime.ripard, wens, robh+dt, mark.rutland, dan.j.williams,
	dmaengine, devicetree, linux-arm-kernel, linux-kernel,
	linux-sunxi

On 07-03-19, 17:58, Jernej Skrabec wrote:
> H6 DMA has more than 32 supported DRQs, which means that configuration
> register is slightly rearranged. It also needs additional clock to be
> enabled.

Okay how many register are rearraged in the new IP block. If there are
large changes, consider using regmap_fields to abstract the register and
bit differences..

> 
> Add support for it.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/dma/sun6i-dma.c | 44 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 6a37f8bb39b1..eceedd139651 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -69,14 +69,19 @@
>  
>  #define DMA_CHAN_CUR_CFG	0x0c
>  #define DMA_CHAN_MAX_DRQ_A31		0x1f
> +#define DMA_CHAN_MAX_DRQ_H6		0x3f
>  #define DMA_CHAN_CFG_SRC_DRQ_A31(x)	((x) & DMA_CHAN_MAX_DRQ_A31)
> +#define DMA_CHAN_CFG_SRC_DRQ_H6(x)	((x) & DMA_CHAN_MAX_DRQ_H6)
>  #define DMA_CHAN_CFG_SRC_MODE_A31(x)	(((x) & 0x1) << 5)
> +#define DMA_CHAN_CFG_SRC_MODE_H6(x)	(((x) & 0x1) << 8)
>  #define DMA_CHAN_CFG_SRC_BURST_A31(x)	(((x) & 0x3) << 7)
>  #define DMA_CHAN_CFG_SRC_BURST_H3(x)	(((x) & 0x3) << 6)
>  #define DMA_CHAN_CFG_SRC_WIDTH(x)	(((x) & 0x3) << 9)
>  
>  #define DMA_CHAN_CFG_DST_DRQ_A31(x)	(DMA_CHAN_CFG_SRC_DRQ_A31(x) << 16)
> +#define DMA_CHAN_CFG_DST_DRQ_H6(x)	(DMA_CHAN_CFG_SRC_DRQ_H6(x) << 16)
>  #define DMA_CHAN_CFG_DST_MODE_A31(x)	(DMA_CHAN_CFG_SRC_MODE_A31(x) << 16)
> +#define DMA_CHAN_CFG_DST_MODE_H6(x)	(DMA_CHAN_CFG_SRC_MODE_H6(x) << 16)
>  #define DMA_CHAN_CFG_DST_BURST_A31(x)	(DMA_CHAN_CFG_SRC_BURST_A31(x) << 16)
>  #define DMA_CHAN_CFG_DST_BURST_H3(x)	(DMA_CHAN_CFG_SRC_BURST_H3(x) << 16)
>  #define DMA_CHAN_CFG_DST_WIDTH(x)	(DMA_CHAN_CFG_SRC_WIDTH(x) << 16)
> @@ -319,12 +324,24 @@ static void sun6i_set_drq_a31(u32 *p_cfg, s8 src_drq, s8 dst_drq)
>  		  DMA_CHAN_CFG_DST_DRQ_A31(dst_drq);
>  }
>  
> +static void sun6i_set_drq_h6(u32 *p_cfg, s8 src_drq, s8 dst_drq)
> +{
> +	*p_cfg |= DMA_CHAN_CFG_SRC_DRQ_H6(src_drq) |
> +		  DMA_CHAN_CFG_DST_DRQ_H6(dst_drq);
> +}
> +
>  static void sun6i_set_mode_a31(u32 *p_cfg, s8 src_mode, s8 dst_mode)
>  {
>  	*p_cfg |= DMA_CHAN_CFG_SRC_MODE_A31(src_mode) |
>  		  DMA_CHAN_CFG_DST_MODE_A31(dst_mode);
>  }
>  
> +static void sun6i_set_mode_h6(u32 *p_cfg, s8 src_mode, s8 dst_mode)
> +{
> +	*p_cfg |= DMA_CHAN_CFG_SRC_MODE_H6(src_mode) |
> +		  DMA_CHAN_CFG_DST_MODE_H6(dst_mode);
> +}
> +
>  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
>  {
>  	struct sun6i_desc *txd = pchan->desc;
> @@ -1160,6 +1177,28 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
>  			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
>  };
>  
> +/*
> + * The H6 binding uses the number of dma channels from the
> + * device tree node.
> + */
> +static struct sun6i_dma_config sun50i_h6_dma_cfg = {
> +	.clock_autogate_enable = sun6i_enable_clock_autogate_h3,
> +	.set_burst_length = sun6i_set_burst_length_h3,
> +	.set_drq          = sun6i_set_drq_h6,
> +	.set_mode         = sun6i_set_mode_h6,
> +	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> +	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> +	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
> +	.dst_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
> +			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
> +	.mbus_clk = true,
> +};
> +
>  /*
>   * The V3s have only 8 physical channels, a maximum DRQ port id of 23,
>   * and a total of 24 usable source and destination endpoints.
> @@ -1190,6 +1229,7 @@ static const struct of_device_id sun6i_dma_match[] = {
>  	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
>  	{ .compatible = "allwinner,sun8i-v3s-dma", .data = &sun8i_v3s_dma_cfg },
>  	{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
> +	{ .compatible = "allwinner,sun50i-h6-dma", .data = &sun50i_h6_dma_cfg },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> @@ -1288,8 +1328,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	ret = of_property_read_u32(np, "dma-requests", &sdc->max_request);
>  	if (ret && !sdc->max_request) {
>  		dev_info(&pdev->dev, "Missing dma-requests, using %u.\n",
> -			 DMA_CHAN_MAX_DRQ_A31);
> -		sdc->max_request = DMA_CHAN_MAX_DRQ_A31;
> +			 DMA_CHAN_MAX_DRQ_H6);
> +		sdc->max_request = DMA_CHAN_MAX_DRQ_H6;
>  	}
>  
>  	/*
> -- 
> 2.21.0

^ permalink raw reply

* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
From: Vinod Koul @ 2019-03-16 11:07 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: maxime.ripard, wens, robh+dt, mark.rutland, dan.j.williams,
	dmaengine, devicetree, linux-arm-kernel, linux-kernel,
	linux-sunxi

On 07-03-19, 17:58, Jernej Skrabec wrote:
> H6 DMA controller needs additional mbus clock to be enabled.
> 
> Add a quirk for it and handle it accordingly.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 0cd13f17fc11..761555080325 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -129,6 +129,7 @@ struct sun6i_dma_config {
>  	u32 dst_burst_lengths;
>  	u32 src_addr_widths;
>  	u32 dst_addr_widths;
> +	bool mbus_clk;
>  };
>  
>  /*
> @@ -182,6 +183,7 @@ struct sun6i_dma_dev {
>  	struct dma_device	slave;
>  	void __iomem		*base;
>  	struct clk		*clk;
> +	struct clk		*clk_mbus;

So rather than have mbus_clk and then a ptr, why not use the ptr and use
NULL value to check for..?

>  	int			irq;
>  	spinlock_t		lock;
>  	struct reset_control	*rstc;
> @@ -1208,6 +1210,14 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  		return PTR_ERR(sdc->clk);
>  	}
>  
> +	if (sdc->cfg->mbus_clk) {

where is the populated? I was expecting this to be set based on DT!

> +		sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus");
> +		if (IS_ERR(sdc->clk_mbus)) {
> +			dev_err(&pdev->dev, "No mbus clock specified\n");
> +			return PTR_ERR(sdc->clk_mbus);
> +		}
> +	}
> +
>  	sdc->rstc = devm_reset_control_get(&pdev->dev, NULL);
>  	if (IS_ERR(sdc->rstc)) {
>  		dev_err(&pdev->dev, "No reset controller specified\n");
> @@ -1312,11 +1322,19 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  		goto err_reset_assert;
>  	}
>  
> +	if (sdc->cfg->mbus_clk) {
> +		ret = clk_prepare_enable(sdc->clk_mbus);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Couldn't enable mbus clock\n");
> +			goto err_clk_disable;
> +		}
> +	}
> +
>  	ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt, 0,
>  			       dev_name(&pdev->dev), sdc);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Cannot request IRQ\n");
> -		goto err_clk_disable;
> +		goto err_mbus_clk_disable;
>  	}
>  
>  	ret = dma_async_device_register(&sdc->slave);
> @@ -1341,6 +1359,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	dma_async_device_unregister(&sdc->slave);
>  err_irq_disable:
>  	sun6i_kill_tasklet(sdc);
> +err_mbus_clk_disable:
> +	clk_disable_unprepare(sdc->clk_mbus);
>  err_clk_disable:
>  	clk_disable_unprepare(sdc->clk);
>  err_reset_assert:
> @@ -1359,6 +1379,7 @@ static int sun6i_dma_remove(struct platform_device *pdev)
>  
>  	sun6i_kill_tasklet(sdc);
>  
> +	clk_disable_unprepare(sdc->clk_mbus);
>  	clk_disable_unprepare(sdc->clk);
>  	reset_control_assert(sdc->rstc);
>  
> -- 
> 2.21.0

^ permalink raw reply

* [1/6] dt-bindings: arm64: allwinner: h6: Add binding for DMA controller
From: Vinod Koul @ 2019-03-16 10:57 UTC (permalink / raw)
  To: Jernej Skrabec, Rob Herring
  Cc: maxime.ripard, wens, mark.rutland, dan.j.williams, dmaengine,
	devicetree, linux-arm-kernel, linux-kernel, linux-sunxi

On 07-03-19, 17:58, Jernej Skrabec wrote:
> DMA in H6 is similar to other DMA controller, except it is first which
> supports more than 32 request sources and has 16 channels. It also needs
> additional clock to be enabled.

Please see DT folks on this patch, when in doubt, use get_maintainers.pl
Need an ack on this patch before applying

> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  Documentation/devicetree/bindings/dma/sun6i-dma.txt | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> index 7fccc20d8331..cae31f4e77ba 100644
> --- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
> @@ -28,12 +28,17 @@ Example:
>  	};
>  
>  ------------------------------------------------------------------------------
> -For A64 DMA controller:
> +For A64 and H6 DMA controller:
>  
>  Required properties:
> -- compatible:	"allwinner,sun50i-a64-dma"
> +- compatible:	Must be one of
> +		  "allwinner,sun50i-a64-dma"
> +		  "allwinner,sun50i-h6-dma"
>  - dma-channels: Number of DMA channels supported by the controller.
>  		Refer to Documentation/devicetree/bindings/dma/dma.txt
> +- clocks:	In addition to parent AHB clock, it should also contain mbus
> +		clock (H6 only)
> +- clock-names:	Should contain "bus" and "mbus" (H6 only)
>  - all properties above, i.e. reg, interrupts, clocks, resets and #dma-cells
>  
>  Optional properties:
> -- 
> 2.21.0

^ permalink raw reply

* dmaengine: pl330: introduce debugfs interface
From: Vinod Koul @ 2019-03-15 17:00 UTC (permalink / raw)
  To: Katsuhiro Suzuki; +Cc: dmaengine, linux-kernel

On 15-03-19, 03:49, Katsuhiro Suzuki wrote:
> This patch adds debugfs interface to show the relationship between
> DMA threads (hardware resource for transferring data) and DMA
> channel ID of DMA slave.
> 
> Typically, PL330 has many slaves than number of DMA threads.
> So sometimes PL330 cannot allocate DMA threads for all slaves even
> if an user specify DMA channel ID in devicetree. This interface will

a user :)

> be useful for checking that DMA threads are allocated or not.
> 
> Below is an output sample:
> 
> $ sudo cat /sys/kernel/debug/ff1f0000.dmac
> PL330 physical channels:
> THREAD:         CHANNEL:
> --------        -----
> 0               8
> 1               9
> 2               11
> 3               12
> 4               14
> 5               15
> 6               10
> 7               --
> 
> Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> ---
>  drivers/dma/pl330.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index eec79fdf27a5..aab71863757c 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -11,6 +11,7 @@
>   * (at your option) any later version.
>   */
>  
> +#include <linux/debugfs.h>
>  #include <linux/kernel.h>
>  #include <linux/io.h>
>  #include <linux/init.h>
> @@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
>  	BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
>  	BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
>  
> +#ifdef CONFIG_DEBUG_FS
> +static int pl330_debugfs_show(struct seq_file *s, void *data)
> +{
> +	struct pl330_dmac *pl330 = s->private;
> +	int chans, pchs, ch, pr, found;
> +
> +	chans = pl330->pcfg.num_chan;
> +	pchs = pl330->num_peripherals;
> +
> +	seq_puts(s, "PL330 physical channels:\n");
> +	seq_puts(s, "THREAD:\t\tCHANNEL:\n");
> +	seq_puts(s, "--------\t-----\n");
> +	for (ch = 0; ch < chans; ch++) {
> +		struct pl330_thread *thrd = &pl330->channels[ch];
> +
> +		found = -1;

found can be uninitialized if we skip this for, which should not be an
issue as chans> 0!

> +		for (pr = 0; pr < pchs; pr++) {
> +			struct dma_pl330_chan *pch = &pl330->peripherals[pr];
> +
> +			if (!pch->thread || thrd->id != pch->thread->id)
> +				continue;
> +
> +			found = pr;
> +		}
> +
> +		seq_printf(s, "%d\t\t", thrd->id);
> +		if (found == -1)
> +			seq_puts(s, "--\n");
> +		else
> +			seq_printf(s, "%d\n", found);
> +	}
> +
> +	return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
> +
> +static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
> +{
> +	debugfs_create_file(dev_name(pl330->ddma.dev),
> +			    S_IFREG | S_IRUGO, NULL, pl330,

are we not supposed to use octal permissions?

> +			    &pl330_debugfs_fops);
> +}
> +#else
> +static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
> +{
> +}
> +#endif
> +
>  /*
>   * Runtime PM callbacks are provided by amba/bus.c driver.
>   *
> @@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>  		dev_err(&adev->dev, "unable to set the seg size\n");
>  
>  
> +	init_pl330_debugfs(pl330);
>  	dev_info(&adev->dev,
>  		"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
>  	dev_info(&adev->dev,
> -- 
> 2.20.1

^ permalink raw reply

* dmaengine: pl330: introduce debugfs interface
From: Katsuhiro Suzuki @ 2019-03-14 18:49 UTC (permalink / raw)
  To: Vinod Koul, dmaengine; +Cc: linux-kernel, Katsuhiro Suzuki

This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if an user specify DMA channel ID in devicetree. This interface will
be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f0000.dmac
PL330 physical channels:
THREAD:         CHANNEL:
--------        -----
0               8
1               9
2               11
3               12
4               14
5               15
6               10
7               --

Signed-off-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
---
 drivers/dma/pl330.c | 51 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..aab71863757c 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/debugfs.h>
 #include <linux/kernel.h>
 #include <linux/io.h>
 #include <linux/init.h>
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
 	BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
 	BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
 
+#ifdef CONFIG_DEBUG_FS
+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+	struct pl330_dmac *pl330 = s->private;
+	int chans, pchs, ch, pr, found;
+
+	chans = pl330->pcfg.num_chan;
+	pchs = pl330->num_peripherals;
+
+	seq_puts(s, "PL330 physical channels:\n");
+	seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+	seq_puts(s, "--------\t-----\n");
+	for (ch = 0; ch < chans; ch++) {
+		struct pl330_thread *thrd = &pl330->channels[ch];
+
+		found = -1;
+		for (pr = 0; pr < pchs; pr++) {
+			struct dma_pl330_chan *pch = &pl330->peripherals[pr];
+
+			if (!pch->thread || thrd->id != pch->thread->id)
+				continue;
+
+			found = pr;
+		}
+
+		seq_printf(s, "%d\t\t", thrd->id);
+		if (found == -1)
+			seq_puts(s, "--\n");
+		else
+			seq_printf(s, "%d\n", found);
+	}
+
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+	debugfs_create_file(dev_name(pl330->ddma.dev),
+			    S_IFREG | S_IRUGO, NULL, pl330,
+			    &pl330_debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
 /*
  * Runtime PM callbacks are provided by amba/bus.c driver.
  *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		dev_err(&adev->dev, "unable to set the seg size\n");
 
 
+	init_pl330_debugfs(pl330);
 	dev_info(&adev->dev,
 		"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
 	dev_info(&adev->dev,

^ permalink raw reply related

* [v3,2/2] dmaengine: tegra210-adma: update system sleep callbacks
From: Jon Hunter @ 2019-03-13 12:11 UTC (permalink / raw)
  To: Sameer Pujar, dan.j.williams, vkoul
  Cc: treding, dmaengine, linux-tegra, linux-kernel

On 13/03/2019 11:32, Sameer Pujar wrote:
> If the driver is active till late suspend, where runtime PM cannot run,
> force suspend is essential in such case to put the device in low power
> state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
> used as system sleep callbacks during system wide PM transitions.
> Late system sleep callbacks are used to ensure, for instance, that the
> sound core has suspended any on-going activity, including stopping the
> ADMA if active, before we attempt to suspend the ADMA.
> 
> Suggested-by: Jonathan Hunter <jonathanh@nvidia.com>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  drivers/dma/tegra210-adma.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 650cd9c..253d312 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -796,17 +796,11 @@ static int tegra_adma_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int tegra_adma_pm_suspend(struct device *dev)
> -{
> -	return pm_runtime_suspended(dev) == false;
> -}
> -#endif
> -
>  static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
>  	SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
>  			   tegra_adma_runtime_resume, NULL)
> -	SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				     pm_runtime_force_resume)
>  };
>  
>  static struct platform_driver tegra_admac_driver = {

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

^ permalink raw reply

* [v3,2/2] dmaengine: tegra210-adma: update system sleep callbacks
From: Sameer Pujar @ 2019-03-13 11:32 UTC (permalink / raw)
  To: dan.j.williams, vkoul
  Cc: treding, jonathanh, dmaengine, linux-tegra, linux-kernel,
	Sameer Pujar

If the driver is active till late suspend, where runtime PM cannot run,
force suspend is essential in such case to put the device in low power
state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
used as system sleep callbacks during system wide PM transitions.
Late system sleep callbacks are used to ensure, for instance, that the
sound core has suspended any on-going activity, including stopping the
ADMA if active, before we attempt to suspend the ADMA.

Suggested-by: Jonathan Hunter <jonathanh@nvidia.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 drivers/dma/tegra210-adma.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 650cd9c..253d312 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -796,17 +796,11 @@ static int tegra_adma_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int tegra_adma_pm_suspend(struct device *dev)
-{
-	return pm_runtime_suspended(dev) == false;
-}
-#endif
-
 static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
 	SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
 			   tegra_adma_runtime_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				     pm_runtime_force_resume)
 };
 
 static struct platform_driver tegra_admac_driver = {

^ permalink raw reply related

* [v3,1/2] dmaengine: tegra210-adma: use devm_clk_*() helpers
From: Sameer Pujar @ 2019-03-13 11:32 UTC (permalink / raw)
  To: dan.j.williams, vkoul
  Cc: treding, jonathanh, dmaengine, linux-tegra, linux-kernel,
	Sameer Pujar

adma driver is using pm_clk_*() interface for managing clock resources.
With this it is observed that clocks remain ON always. This happens on
Tegra devices which use BPMP co-processor to manage clock resources,
where clocks are enabled during prepare phase. This is necessary because
clocks to BPMP are always blocking. When pm_clk_*() interface is used on
such Tegra devices, clock prepare count is not balanced till remove call
happens for the driver and hence clocks are seen ON always. Thus this
patch replaces pm_clk_*() with devm_clk_*() framework.

Suggested-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 drivers/dma/tegra210-adma.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 5ec0dd9..650cd9c 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -22,7 +22,6 @@
 #include <linux/of_device.h>
 #include <linux/of_dma.h>
 #include <linux/of_irq.h>
-#include <linux/pm_clock.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
@@ -141,6 +140,7 @@ struct tegra_adma {
 	struct dma_device		dma_dev;
 	struct device			*dev;
 	void __iomem			*base_addr;
+	struct clk			*ahub_clk;
 	unsigned int			nr_channels;
 	unsigned long			rx_requests_reserved;
 	unsigned long			tx_requests_reserved;
@@ -637,8 +637,9 @@ static int tegra_adma_runtime_suspend(struct device *dev)
 	struct tegra_adma *tdma = dev_get_drvdata(dev);
 
 	tdma->global_cmd = tdma_read(tdma, ADMA_GLOBAL_CMD);
+	clk_disable_unprepare(tdma->ahub_clk);
 
-	return pm_clk_suspend(dev);
+	return 0;
 }
 
 static int tegra_adma_runtime_resume(struct device *dev)
@@ -646,10 +647,11 @@ static int tegra_adma_runtime_resume(struct device *dev)
 	struct tegra_adma *tdma = dev_get_drvdata(dev);
 	int ret;
 
-	ret = pm_clk_resume(dev);
-	if (ret)
+	ret = clk_prepare_enable(tdma->ahub_clk);
+	if (ret) {
+		dev_err(dev, "ahub clk_enable failed: %d\n", ret);
 		return ret;
-
+	}
 	tdma_write(tdma, ADMA_GLOBAL_CMD, tdma->global_cmd);
 
 	return 0;
@@ -693,13 +695,11 @@ static int tegra_adma_probe(struct platform_device *pdev)
 	if (IS_ERR(tdma->base_addr))
 		return PTR_ERR(tdma->base_addr);
 
-	ret = pm_clk_create(&pdev->dev);
-	if (ret)
-		return ret;
-
-	ret = of_pm_clk_add_clk(&pdev->dev, "d_audio");
-	if (ret)
-		goto clk_destroy;
+	tdma->ahub_clk = devm_clk_get(&pdev->dev, "d_audio");
+	if (IS_ERR(tdma->ahub_clk)) {
+		dev_err(&pdev->dev, "Error: Missing ahub controller clock\n");
+		return PTR_ERR(tdma->ahub_clk);
+	}
 
 	pm_runtime_enable(&pdev->dev);
 
@@ -776,8 +776,6 @@ static int tegra_adma_probe(struct platform_device *pdev)
 	pm_runtime_put_sync(&pdev->dev);
 rpm_disable:
 	pm_runtime_disable(&pdev->dev);
-clk_destroy:
-	pm_clk_destroy(&pdev->dev);
 
 	return ret;
 }
@@ -794,7 +792,6 @@ static int tegra_adma_remove(struct platform_device *pdev)
 
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	pm_clk_destroy(&pdev->dev);
 
 	return 0;
 }

^ permalink raw reply related

* [v2,2/2] dmaengine: tegra210-adma: update system sleep callbacks
From: Sameer Pujar @ 2019-03-13 10:56 UTC (permalink / raw)
  To: Jon Hunter, dan.j.williams, vkoul
  Cc: treding, dmaengine, linux-tegra, linux-kernel

On 3/13/2019 4:19 PM, Jon Hunter wrote:
> On 13/03/2019 10:40, Sameer Pujar wrote:
>> On 3/13/2019 3:58 PM, Jon Hunter wrote:
>>> On 13/03/2019 05:43, Sameer Pujar wrote:
>>>> If the driver is active till late suspend, where runtime PM cannot run,
>>>> force suspend is essential in such case to put the device in low power
>>>> state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
>>>> used as system sleep callbacks during system wide PM transitions.
>>>>
>>>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>>>> ---
>>>>    drivers/dma/tegra210-adma.c | 10 ++--------
>>>>    1 file changed, 2 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>>>> index 650cd9c..be29171 100644
>>>> --- a/drivers/dma/tegra210-adma.c
>>>> +++ b/drivers/dma/tegra210-adma.c
>>>> @@ -796,17 +796,11 @@ static int tegra_adma_remove(struct
>>>> platform_device *pdev)
>>>>        return 0;
>>>>    }
>>>>    -#ifdef CONFIG_PM_SLEEP
>>>> -static int tegra_adma_pm_suspend(struct device *dev)
>>>> -{
>>>> -    return pm_runtime_suspended(dev) == false;
>>>> -}
>>>> -#endif
>>>> -
>>>>    static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
>>>>        SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
>>>>                   tegra_adma_runtime_resume, NULL)
>>>> -    SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
>>>> +    SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>>>> +                pm_runtime_force_resume)
>>>>    };
>>> Looking at our downstream kernel we use LATE_SYSTEM_SLEEP for these. Any
>>> reason why you changed this?
>> I think, I just wanted to replace function calls for system sleep here
>> and probably did
>> not see exactly what we have in downstream kernel at that point. Looking
>> at the commit
>> log in downstream, it might qualify for separate patch.
>> Let me know if you think, its better to add here.
> I see no reason to change this from what we have been using and testing
> downstream. I don't think that this warrants yet another patch for this.
> Furthermore, the changelog references 'late' so it does not seem to
> align with the change itself.
Agree, will update. Thanks.
>
> Cheers
> Jon
>

^ 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