All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems)
@ 2016-03-18 14:24 Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 01/15] dmaengine: dw: fix master selection Andy Shevchenko
                   ` (15 more replies)
  0 siblings, 16 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

This patch series (v2: http://www.spinics.net/lists/dmaengine/msg08274.html)
contains a number of mostly minor fixes and cleanups for the DW DMA driver. A
couple of them affect the DT binding so these may need to be updated to
maintain compatibility. The rest should be relatively straight-forward.

This version has been tested on the following bare metal platforms:
- ATNGW100 (avr32 based platform) with MMC
- Sam460ex (powerpc 44x based platform) with SATA
- Intel Braswell with UART
- Intel Galileo (Intel Quark based platform) with UART

(SATA driver and Intel Galileo UART support are based on this series and not
 available in upstream yet)

Vinod, there are few patch sets developed on top of this one, so, the idea is
to keep this in an immuutable branch / tag.

Changes since v2:
- add patch 1 to fix master selection which was broken for long time
- remove "use field-by-field initialization" patch since like Mans metioned in
  has mostly no value and even might increase error prone
- rebase on top of recent linux-next
- wide testing on several platforms

Changes since v1:
- zeroing struct dw_dma_slave before use
- fall back to old data_width property if data-width is not found
- append tags for few patches
- correct title of cover letter
- rebase on top of recent linux-next

Andy Shevchenko (11):
  dmaengine: dw: fix master selection
  dmaengine: dw: rename masters to reflect actual topology
  dmaengine: dw: substitute dma_read_byaddr by dma_readl_native
  dmaengine: dw: revisit data_width property
  dmaengine: dw: define counter variables as unsigned int
  dmaengine: dw: keep entire platform data in struct dw_dma
  dmaengine: dw: pass platform data via struct dw_dma_chip
  dmaengine: dw: move dwc->paused to dwc->flags
  dmaengine: dw: move dwc->initialized to dwc->flags
  dmaengine: dw: move residue to a descriptor
  dmaengine: dw: set cdesc to NULL when free cyclic transfers

Mans Rullgard (4):
  dmaengine: dw: set src and dst master select according to xfer
    direction
  dmaengine: dw: fix byte order of hw descriptor fields
  dmaengine: dw: set LMS field in descriptors
  dmaengine: dw: clear LLP_[SD]_EN bits in last descriptor of a chain

 Documentation/devicetree/bindings/dma/snps-dma.txt |   9 +-
 arch/arc/boot/dts/abilis_tb10x.dtsi                |   2 +-
 arch/arm/boot/dts/spear13xx.dtsi                   |   4 +-
 arch/avr32/mach-at32ap/at32ap700x.c                |  16 +-
 drivers/ata/sata_dwc_460ex.c                       |   6 +-
 drivers/dma/dw/core.c                              | 355 ++++++++++-----------
 drivers/dma/dw/pci.c                               |   5 +-
 drivers/dma/dw/platform.c                          |  27 +-
 drivers/dma/dw/regs.h                              |  55 ++--
 drivers/spi/spi-pxa2xx-pci.c                       |   8 +-
 drivers/tty/serial/8250/8250_pci.c                 |   8 +-
 include/linux/dma/dw.h                             |   5 +-
 include/linux/platform_data/dma-dw.h               |  15 +-
 sound/soc/intel/common/sst-firmware.c              |   2 +-
 14 files changed, 268 insertions(+), 249 deletions(-)

-- 
2.7.0

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-04-04 17:03   ` Vinod Koul
  2016-03-18 14:24 ` [PATCH v3 02/15] dmaengine: dw: rename masters to reflect actual topology Andy Shevchenko
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta
  Cc: stable

The commit 895005202987 ("dmaengine: dw: apply both HS interfaces and remove
slave_id usage") cleaned up the code to avoid usage of depricated slave_id
member of generic slave configuration.

Meanwhile it broke the master selection by removing important call to
dwc_set_masters() in ->device_alloc_chan_resources() which copied masters from
custom slave configuration to the internal channel structure.

Everything works until now since there is no customized connection of
DesignWare DMA IP to the bus, i.e. one bus and one or more masters are in use.
The configurations where 2 masters are connected to the different masters are
not working anymore. We are expecting one user of such configuration and need
to select masters properly. Besides that it is obviously a performance
regression since only one master is in use in multi-master configuration.

Select masters in accordance to what user asked for. Keep this patch in a form
more suitable for back porting.

We are safe to take necessary data in ->device_alloc_chan_resources() because
we don't support generic slave configuration embedded into custom one, and thus
the only way to provide it is to use parameter to a filter function which is
called exactly before channel resource allocation.

Fixes: 895005202987 ("dmaengine: dw: apply both HS interfaces and remove slave_id usage")
Cc: stable@vger.kernel.org
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 5ad0ec1..ba46628 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -130,26 +130,14 @@ static void dwc_desc_put(struct dw_dma_chan *dwc, struct dw_desc *desc)
 static void dwc_initialize(struct dw_dma_chan *dwc)
 {
 	struct dw_dma *dw = to_dw_dma(dwc->chan.device);
-	struct dw_dma_slave *dws = dwc->chan.private;
 	u32 cfghi = DWC_CFGH_FIFO_MODE;
 	u32 cfglo = DWC_CFGL_CH_PRIOR(dwc->priority);
 
 	if (dwc->initialized == true)
 		return;
 
-	if (dws) {
-		/*
-		 * We need controller-specific data to set up slave
-		 * transfers.
-		 */
-		BUG_ON(!dws->dma_dev || dws->dma_dev != dw->dma.dev);
-
-		cfghi |= DWC_CFGH_DST_PER(dws->dst_id);
-		cfghi |= DWC_CFGH_SRC_PER(dws->src_id);
-	} else {
-		cfghi |= DWC_CFGH_DST_PER(dwc->dst_id);
-		cfghi |= DWC_CFGH_SRC_PER(dwc->src_id);
-	}
+	cfghi |= DWC_CFGH_DST_PER(dwc->dst_id);
+	cfghi |= DWC_CFGH_SRC_PER(dwc->src_id);
 
 	channel_writel(dwc, CFG_LO, cfglo);
 	channel_writel(dwc, CFG_HI, cfghi);
@@ -941,7 +929,7 @@ bool dw_dma_filter(struct dma_chan *chan, void *param)
 	struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
 	struct dw_dma_slave *dws = param;
 
-	if (!dws || dws->dma_dev != chan->device->dev)
+	if (dws->dma_dev != chan->device->dev)
 		return false;
 
 	/* We have to copy data since dws can be temporary storage */
@@ -1165,6 +1153,11 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
 	 * doesn't mean what you think it means), and status writeback.
 	 */
 
+	/*
+	 * We need controller-specific data to set up slave transfers.
+	 */
+	BUG_ON(chan->private && !dw_dma_filter(chan, chan->private));
+
 	/* Enable controller here if needed */
 	if (!dw->in_use)
 		dw_dma_on(dw);
@@ -1226,6 +1219,14 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	spin_lock_irqsave(&dwc->lock, flags);
 	list_splice_init(&dwc->free_list, &list);
 	dwc->descs_allocated = 0;
+
+	/* Clear custom channel configuration */
+	dwc->src_id = 0;
+	dwc->dst_id = 0;
+
+	dwc->src_master = 0;
+	dwc->dst_master = 0;
+
 	dwc->initialized = false;
 
 	/* Disable interrupts */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 02/15] dmaengine: dw: rename masters to reflect actual topology
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 01/15] dmaengine: dw: fix master selection Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 03/15] dmaengine: dw: set src and dst master select according to xfer direction Andy Shevchenko
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

The source and destination masters are reflecting buses or their layers to
where the different devices can be connected. The patch changes the master
names to reflect which one is related to which independently on the transfer
direction.

The outcome of the change is that the memory data width is now always limited
by a data width of the master which is dedicated to communicate to memory.

The patch will not break anything since all current users have the same data
width for all masters. Though it would be nice to revisit avr32 platforms to
check what is the actual hardware topology in use there. It seems that it has
one bus and two masters on it as stated by Table 8-2, that's why everything
works independently on the master in use. The purpose of the sequential patch
is to fix the driver for configuration of more than one bus.

The change is done in the assumption that src_master and dst_master are
reflecting a connection to the memory and peripheral correspondently on avr32
and otherwise on the rest.

Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>
Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/devicetree/bindings/dma/snps-dma.txt |  4 ++--
 arch/avr32/mach-at32ap/at32ap700x.c                | 16 ++++++++--------
 drivers/ata/sata_dwc_460ex.c                       |  4 ++--
 drivers/dma/dw/core.c                              | 19 +++++++++----------
 drivers/dma/dw/platform.c                          | 12 ++++++------
 drivers/dma/dw/regs.h                              |  4 ++--
 drivers/spi/spi-pxa2xx-pci.c                       |  8 ++++----
 drivers/tty/serial/8250/8250_pci.c                 |  8 ++++----
 include/linux/platform_data/dma-dw.h               |  8 ++++----
 9 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
index c261598..c99c1ff 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -47,8 +47,8 @@ The four cells in order are:
 
 1. A phandle pointing to the DMA controller
 2. The DMA request line number
-3. Source master for transfers on allocated channel
-4. Destination master for transfers on allocated channel
+3. Memory master for transfers on allocated channel
+4. Peripheral master for transfers on allocated channel
 
 Example:
 	
diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
index bf445aa..00d6dcc 100644
--- a/arch/avr32/mach-at32ap/at32ap700x.c
+++ b/arch/avr32/mach-at32ap/at32ap700x.c
@@ -1365,8 +1365,8 @@ at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
 	slave->dma_dev = &dw_dmac0_device.dev;
 	slave->src_id = 0;
 	slave->dst_id = 1;
-	slave->src_master = 1;
-	slave->dst_master = 0;
+	slave->m_master = 1;
+	slave->p_master = 0;
 
 	data->dma_slave = slave;
 	data->dma_filter = at32_mci_dma_filter;
@@ -2061,16 +2061,16 @@ at32_add_device_ac97c(unsigned int id, struct ac97c_platform_data *data,
 	if (flags & AC97C_CAPTURE) {
 		rx_dws->dma_dev = &dw_dmac0_device.dev;
 		rx_dws->src_id = 3;
-		rx_dws->src_master = 0;
-		rx_dws->dst_master = 1;
+		rx_dws->m_master = 0;
+		rx_dws->p_master = 1;
 	}
 
 	/* Check if DMA slave interface for playback should be configured. */
 	if (flags & AC97C_PLAYBACK) {
 		tx_dws->dma_dev = &dw_dmac0_device.dev;
 		tx_dws->dst_id = 4;
-		tx_dws->src_master = 0;
-		tx_dws->dst_master = 1;
+		tx_dws->m_master = 0;
+		tx_dws->p_master = 1;
 	}
 
 	if (platform_device_add_data(pdev, data,
@@ -2141,8 +2141,8 @@ at32_add_device_abdac(unsigned int id, struct atmel_abdac_pdata *data)
 
 	dws->dma_dev = &dw_dmac0_device.dev;
 	dws->dst_id = 2;
-	dws->src_master = 0;
-	dws->dst_master = 1;
+	dws->m_master = 0;
+	dws->p_master = 1;
 
 	if (platform_device_add_data(pdev, data,
 				sizeof(struct atmel_abdac_pdata)))
diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 9020349..80bdcab 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -201,8 +201,8 @@ static struct sata_dwc_host_priv host_pvt;
 static struct dw_dma_slave sata_dwc_dma_dws = {
 	.src_id = 0,
 	.dst_id = 0,
-	.src_master = 0,
-	.dst_master = 1,
+	.m_master = 1,
+	.p_master = 0,
 };
 
 /*
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index ba46628..9112891 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -50,8 +50,8 @@
 		 | DWC_CTLL_SRC_MSIZE(_smsize)			\
 		 | DWC_CTLL_LLP_D_EN				\
 		 | DWC_CTLL_LLP_S_EN				\
-		 | DWC_CTLL_DMS(_dwc->dst_master)		\
-		 | DWC_CTLL_SMS(_dwc->src_master));		\
+		 | DWC_CTLL_DMS(_dwc->p_master)			\
+		 | DWC_CTLL_SMS(_dwc->m_master));		\
 	})
 
 /*
@@ -709,8 +709,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 
 	dwc->direction = DMA_MEM_TO_MEM;
 
-	data_width = min_t(unsigned int, dw->data_width[dwc->src_master],
-			   dw->data_width[dwc->dst_master]);
+	data_width = dw->data_width[dwc->m_master];
 
 	src_width = dst_width = min_t(unsigned int, data_width,
 				      dwc_fast_ffs(src | dest | len));
@@ -802,7 +801,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
 			DWC_CTLL_FC(DW_DMA_FC_D_M2P);
 
-		data_width = dw->data_width[dwc->src_master];
+		data_width = dw->data_width[dwc->m_master];
 
 		for_each_sg(sgl, sg, sg_len, i) {
 			struct dw_desc	*desc;
@@ -859,7 +858,7 @@ slave_sg_todev_fill_desc:
 		ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
 			DWC_CTLL_FC(DW_DMA_FC_D_P2M);
 
-		data_width = dw->data_width[dwc->dst_master];
+		data_width = dw->data_width[dwc->m_master];
 
 		for_each_sg(sgl, sg, sg_len, i) {
 			struct dw_desc	*desc;
@@ -937,8 +936,8 @@ bool dw_dma_filter(struct dma_chan *chan, void *param)
 	dwc->src_id = dws->src_id;
 	dwc->dst_id = dws->dst_id;
 
-	dwc->src_master = dws->src_master;
-	dwc->dst_master = dws->dst_master;
+	dwc->m_master = dws->m_master;
+	dwc->p_master = dws->p_master;
 
 	return true;
 }
@@ -1224,8 +1223,8 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	dwc->src_id = 0;
 	dwc->dst_id = 0;
 
-	dwc->src_master = 0;
-	dwc->dst_master = 0;
+	dwc->m_master = 0;
+	dwc->p_master = 0;
 
 	dwc->initialized = false;
 
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index 26edbe3..23616c5 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -42,13 +42,13 @@ static struct dma_chan *dw_dma_of_xlate(struct of_phandle_args *dma_spec,
 
 	slave.src_id = dma_spec->args[0];
 	slave.dst_id = dma_spec->args[0];
-	slave.src_master = dma_spec->args[1];
-	slave.dst_master = dma_spec->args[2];
+	slave.m_master = dma_spec->args[1];
+	slave.p_master = dma_spec->args[2];
 
 	if (WARN_ON(slave.src_id >= DW_DMA_MAX_NR_REQUESTS ||
 		    slave.dst_id >= DW_DMA_MAX_NR_REQUESTS ||
-		    slave.src_master >= dw->nr_masters ||
-		    slave.dst_master >= dw->nr_masters))
+		    slave.m_master >= dw->nr_masters ||
+		    slave.p_master >= dw->nr_masters))
 		return NULL;
 
 	dma_cap_zero(cap);
@@ -66,8 +66,8 @@ static bool dw_dma_acpi_filter(struct dma_chan *chan, void *param)
 		.dma_dev = dma_spec->dev,
 		.src_id = dma_spec->slave_id,
 		.dst_id = dma_spec->slave_id,
-		.src_master = 1,
-		.dst_master = 0,
+		.m_master = 0,
+		.p_master = 1,
 	};
 
 	return dw_dma_filter(chan, &slave);
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 0a50c18..a63d62b 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -249,8 +249,8 @@ struct dw_dma_chan {
 	/* custom slave configuration */
 	u8			src_id;
 	u8			dst_id;
-	u8			src_master;
-	u8			dst_master;
+	u8			m_master;
+	u8			p_master;
 
 	/* configuration passed via .device_config */
 	struct dma_slave_config dma_sconfig;
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index 520ed1d..4fd7f98 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -144,16 +144,16 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 		struct dw_dma_slave *slave = c->tx_param;
 
 		slave->dma_dev = &dma_dev->dev;
-		slave->src_master = 1;
-		slave->dst_master = 0;
+		slave->m_master = 0;
+		slave->p_master = 1;
 	}
 
 	if (c->rx_param) {
 		struct dw_dma_slave *slave = c->rx_param;
 
 		slave->dma_dev = &dma_dev->dev;
-		slave->src_master = 1;
-		slave->dst_master = 0;
+		slave->m_master = 0;
+		slave->p_master = 1;
 	}
 
 	spi_pdata.dma_filter = lpss_dma_filter;
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 98862aa..5eea74d 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1454,13 +1454,13 @@ byt_serial_setup(struct serial_private *priv,
 		return -EINVAL;
 	}
 
-	rx_param->src_master = 1;
-	rx_param->dst_master = 0;
+	rx_param->m_master = 0;
+	rx_param->p_master = 1;
 
 	dma->rxconf.src_maxburst = 16;
 
-	tx_param->src_master = 1;
-	tx_param->dst_master = 0;
+	tx_param->m_master = 0;
+	tx_param->p_master = 1;
 
 	dma->txconf.dst_maxburst = 16;
 
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index 03b6095..b881b97 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -21,15 +21,15 @@
  * @dma_dev:	required DMA master device
  * @src_id:	src request line
  * @dst_id:	dst request line
- * @src_master: src master for transfers on allocated channel.
- * @dst_master: dest master for transfers on allocated channel.
+ * @m_master:	memory master for transfers on allocated channel
+ * @p_master:	peripheral master for transfers on allocated channel
  */
 struct dw_dma_slave {
 	struct device		*dma_dev;
 	u8			src_id;
 	u8			dst_id;
-	u8			src_master;
-	u8			dst_master;
+	u8			m_master;
+	u8			p_master;
 };
 
 /**
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 03/15] dmaengine: dw: set src and dst master select according to xfer direction
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 01/15] dmaengine: dw: fix master selection Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 02/15] dmaengine: dw: rename masters to reflect actual topology Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 04/15] dmaengine: dw: fix byte order of hw descriptor fields Andy Shevchenko
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta
  Cc: Mans Rullgard

From: Mans Rullgard <mans@mansr.com>

On some architectures the DMA controller can have two masters connected to
different buses and thus access to memory is possible only through one and
to peripheral through the other.

This patch changes the src and dst master setting to match the direction
of the transfer.

Signed-off-by: Mans Rullgard <mans@mansr.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 9112891..66d8276 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -45,13 +45,17 @@
 			DW_DMA_MSIZE_16;			\
 		u8 _dmsize = _is_slave ? _sconfig->dst_maxburst :	\
 			DW_DMA_MSIZE_16;			\
+		u8 _dms = (_dwc->direction == DMA_MEM_TO_DEV) ?		\
+			_dwc->p_master : _dwc->m_master;		\
+		u8 _sms = (_dwc->direction == DMA_DEV_TO_MEM) ?		\
+			_dwc->p_master : _dwc->m_master;		\
 								\
 		(DWC_CTLL_DST_MSIZE(_dmsize)			\
 		 | DWC_CTLL_SRC_MSIZE(_smsize)			\
 		 | DWC_CTLL_LLP_D_EN				\
 		 | DWC_CTLL_LLP_S_EN				\
-		 | DWC_CTLL_DMS(_dwc->p_master)			\
-		 | DWC_CTLL_SMS(_dwc->m_master));		\
+		 | DWC_CTLL_DMS(_dms)				\
+		 | DWC_CTLL_SMS(_sms));				\
 	})
 
 /*
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 04/15] dmaengine: dw: fix byte order of hw descriptor fields
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (2 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 03/15] dmaengine: dw: set src and dst master select according to xfer direction Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 05/15] dmaengine: dw: set LMS field in descriptors Andy Shevchenko
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta
  Cc: Mans Rullgard

From: Mans Rullgard <mans@mansr.com>

If the DMA controller uses a different byte order than the host CPU,
the hardware linked list descriptor fields need to be byte-swapped.

This patch makes the driver write these fields using the same byte
order it uses for mmio accesses to the DMA engine. I do not know
if this is guaranteed to always be correct.

Signed-off-by: Mans Rullgard <mans@mansr.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 123 +++++++++++++++++++++++++-------------------------
 drivers/dma/dw/regs.h |  32 ++++++++++---
 2 files changed, 87 insertions(+), 68 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 66d8276..279abff 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -201,12 +201,12 @@ static inline void dwc_do_single_block(struct dw_dma_chan *dwc,
 	 * Software emulation of LLP mode relies on interrupts to continue
 	 * multi block transfer.
 	 */
-	ctllo = desc->lli.ctllo | DWC_CTLL_INT_EN;
+	ctllo = lli_read(desc, ctllo) | DWC_CTLL_INT_EN;
 
-	channel_writel(dwc, SAR, desc->lli.sar);
-	channel_writel(dwc, DAR, desc->lli.dar);
+	channel_writel(dwc, SAR, lli_read(desc, sar));
+	channel_writel(dwc, DAR, lli_read(desc, dar));
 	channel_writel(dwc, CTL_LO, ctllo);
-	channel_writel(dwc, CTL_HI, desc->lli.ctlhi);
+	channel_writel(dwc, CTL_HI, lli_read(desc, ctlhi));
 	channel_set_bit(dw, CH_EN, dwc->mask);
 
 	/* Move pointer to next descriptor */
@@ -424,7 +424,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 		}
 
 		/* Check first descriptors llp */
-		if (desc->lli.llp == llp) {
+		if (lli_read(desc, llp) == llp) {
 			/* This one is currently in progress */
 			dwc->residue -= dwc_get_sent(dwc);
 			spin_unlock_irqrestore(&dwc->lock, flags);
@@ -433,7 +433,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 		dwc->residue -= desc->len;
 		list_for_each_entry(child, &desc->tx_list, desc_node) {
-			if (child->lli.llp == llp) {
+			if (lli_read(child, llp) == llp) {
 				/* Currently in progress */
 				dwc->residue -= dwc_get_sent(dwc);
 				spin_unlock_irqrestore(&dwc->lock, flags);
@@ -461,10 +461,14 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	spin_unlock_irqrestore(&dwc->lock, flags);
 }
 
-static inline void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_lli *lli)
+static inline void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_desc *desc)
 {
 	dev_crit(chan2dev(&dwc->chan), "  desc: s0x%x d0x%x l0x%x c0x%x:%x\n",
-		 lli->sar, lli->dar, lli->llp, lli->ctlhi, lli->ctllo);
+		 lli_read(desc, sar),
+		 lli_read(desc, dar),
+		 lli_read(desc, llp),
+		 lli_read(desc, ctlhi),
+		 lli_read(desc, ctllo));
 }
 
 static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
@@ -500,9 +504,9 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	 */
 	dev_WARN(chan2dev(&dwc->chan), "Bad descriptor submitted for DMA!\n"
 				       "  cookie: %d\n", bad_desc->txd.cookie);
-	dwc_dump_lli(dwc, &bad_desc->lli);
+	dwc_dump_lli(dwc, bad_desc);
 	list_for_each_entry(child, &bad_desc->tx_list, desc_node)
-		dwc_dump_lli(dwc, &child->lli);
+		dwc_dump_lli(dwc, child);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
@@ -575,7 +579,7 @@ static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
 		dma_writel(dw, CLEAR.XFER, dwc->mask);
 
 		for (i = 0; i < dwc->cdesc->periods; i++)
-			dwc_dump_lli(dwc, &dwc->cdesc->desc[i]->lli);
+			dwc_dump_lli(dwc, dwc->cdesc->desc[i]);
 
 		spin_unlock_irqrestore(&dwc->lock, flags);
 	}
@@ -734,25 +738,24 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 		if (!desc)
 			goto err_desc_get;
 
-		desc->lli.sar = src + offset;
-		desc->lli.dar = dest + offset;
-		desc->lli.ctllo = ctllo;
-		desc->lli.ctlhi = xfer_count;
+		lli_write(desc, sar, src + offset);
+		lli_write(desc, dar, dest + offset);
+		lli_write(desc, ctllo, ctllo);
+		lli_write(desc, ctlhi, xfer_count);
 		desc->len = xfer_count << src_width;
 
 		if (!first) {
 			first = desc;
 		} else {
-			prev->lli.llp = desc->txd.phys;
-			list_add_tail(&desc->desc_node,
-					&first->tx_list);
+			lli_write(prev, llp, desc->txd.phys);
+			list_add_tail(&desc->desc_node, &first->tx_list);
 		}
 		prev = desc;
 	}
 
 	if (flags & DMA_PREP_INTERRUPT)
 		/* Trigger interrupt after last block */
-		prev->lli.ctllo |= DWC_CTLL_INT_EN;
+		lli_set(prev, ctllo, DWC_CTLL_INT_EN);
 
 	prev->lli.llp = 0;
 	first->txd.flags = flags;
@@ -822,9 +825,9 @@ slave_sg_todev_fill_desc:
 			if (!desc)
 				goto err_desc_get;
 
-			desc->lli.sar = mem;
-			desc->lli.dar = reg;
-			desc->lli.ctllo = ctllo | DWC_CTLL_SRC_WIDTH(mem_width);
+			lli_write(desc, sar, mem);
+			lli_write(desc, dar, reg);
+			lli_write(desc, ctllo, ctllo | DWC_CTLL_SRC_WIDTH(mem_width));
 			if ((len >> mem_width) > dwc->block_size) {
 				dlen = dwc->block_size << mem_width;
 				mem += dlen;
@@ -834,15 +837,14 @@ slave_sg_todev_fill_desc:
 				len = 0;
 			}
 
-			desc->lli.ctlhi = dlen >> mem_width;
+			lli_write(desc, ctlhi, dlen >> mem_width);
 			desc->len = dlen;
 
 			if (!first) {
 				first = desc;
 			} else {
-				prev->lli.llp = desc->txd.phys;
-				list_add_tail(&desc->desc_node,
-						&first->tx_list);
+				lli_write(prev, llp, desc->txd.phys);
+				list_add_tail(&desc->desc_node, &first->tx_list);
 			}
 			prev = desc;
 			total_len += dlen;
@@ -879,9 +881,9 @@ slave_sg_fromdev_fill_desc:
 			if (!desc)
 				goto err_desc_get;
 
-			desc->lli.sar = reg;
-			desc->lli.dar = mem;
-			desc->lli.ctllo = ctllo | DWC_CTLL_DST_WIDTH(mem_width);
+			lli_write(desc, sar, reg);
+			lli_write(desc, dar, mem);
+			lli_write(desc, ctllo, ctllo | DWC_CTLL_DST_WIDTH(mem_width));
 			if ((len >> reg_width) > dwc->block_size) {
 				dlen = dwc->block_size << reg_width;
 				mem += dlen;
@@ -890,15 +892,14 @@ slave_sg_fromdev_fill_desc:
 				dlen = len;
 				len = 0;
 			}
-			desc->lli.ctlhi = dlen >> reg_width;
+			lli_write(desc, ctlhi, dlen >> reg_width);
 			desc->len = dlen;
 
 			if (!first) {
 				first = desc;
 			} else {
-				prev->lli.llp = desc->txd.phys;
-				list_add_tail(&desc->desc_node,
-						&first->tx_list);
+				lli_write(prev, llp, desc->txd.phys);
+				list_add_tail(&desc->desc_node, &first->tx_list);
 			}
 			prev = desc;
 			total_len += dlen;
@@ -913,7 +914,7 @@ slave_sg_fromdev_fill_desc:
 
 	if (flags & DMA_PREP_INTERRUPT)
 		/* Trigger interrupt after last block */
-		prev->lli.ctllo |= DWC_CTLL_INT_EN;
+		lli_set(prev, ctllo, DWC_CTLL_INT_EN);
 
 	prev->lli.llp = 0;
 	first->total_len = total_len;
@@ -1397,50 +1398,50 @@ struct dw_cyclic_desc *dw_dma_cyclic_prep(struct dma_chan *chan,
 
 		switch (direction) {
 		case DMA_MEM_TO_DEV:
-			desc->lli.dar = sconfig->dst_addr;
-			desc->lli.sar = buf_addr + (period_len * i);
-			desc->lli.ctllo = (DWC_DEFAULT_CTLLO(chan)
-					| DWC_CTLL_DST_WIDTH(reg_width)
-					| DWC_CTLL_SRC_WIDTH(reg_width)
-					| DWC_CTLL_DST_FIX
-					| DWC_CTLL_SRC_INC
-					| DWC_CTLL_INT_EN);
-
-			desc->lli.ctllo |= sconfig->device_fc ?
-				DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
-				DWC_CTLL_FC(DW_DMA_FC_D_M2P);
+			lli_write(desc, dar, sconfig->dst_addr);
+			lli_write(desc, sar, buf_addr + period_len * i);
+			lli_write(desc, ctllo, (DWC_DEFAULT_CTLLO(chan)
+				| DWC_CTLL_DST_WIDTH(reg_width)
+				| DWC_CTLL_SRC_WIDTH(reg_width)
+				| DWC_CTLL_DST_FIX
+				| DWC_CTLL_SRC_INC
+				| DWC_CTLL_INT_EN));
+
+			lli_set(desc, ctllo, sconfig->device_fc ?
+					DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
+					DWC_CTLL_FC(DW_DMA_FC_D_M2P));
 
 			break;
 		case DMA_DEV_TO_MEM:
-			desc->lli.dar = buf_addr + (period_len * i);
-			desc->lli.sar = sconfig->src_addr;
-			desc->lli.ctllo = (DWC_DEFAULT_CTLLO(chan)
-					| DWC_CTLL_SRC_WIDTH(reg_width)
-					| DWC_CTLL_DST_WIDTH(reg_width)
-					| DWC_CTLL_DST_INC
-					| DWC_CTLL_SRC_FIX
-					| DWC_CTLL_INT_EN);
-
-			desc->lli.ctllo |= sconfig->device_fc ?
-				DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
-				DWC_CTLL_FC(DW_DMA_FC_D_P2M);
+			lli_write(desc, dar, buf_addr + period_len * i);
+			lli_write(desc, sar, sconfig->src_addr);
+			lli_write(desc, ctllo, (DWC_DEFAULT_CTLLO(chan)
+				| DWC_CTLL_SRC_WIDTH(reg_width)
+				| DWC_CTLL_DST_WIDTH(reg_width)
+				| DWC_CTLL_DST_INC
+				| DWC_CTLL_SRC_FIX
+				| DWC_CTLL_INT_EN));
+
+			lli_set(desc, ctllo, sconfig->device_fc ?
+					DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
+					DWC_CTLL_FC(DW_DMA_FC_D_P2M));
 
 			break;
 		default:
 			break;
 		}
 
-		desc->lli.ctlhi = (period_len >> reg_width);
+		lli_write(desc, ctlhi, period_len >> reg_width);
 		cdesc->desc[i] = desc;
 
 		if (last)
-			last->lli.llp = desc->txd.phys;
+			lli_write(last, llp, desc->txd.phys);
 
 		last = desc;
 	}
 
 	/* Let's make a cyclic list */
-	last->lli.llp = cdesc->desc[0]->txd.phys;
+	lli_write(last, llp, cdesc->desc[0]->txd.phys);
 
 	dev_dbg(chan2dev(&dwc->chan),
 			"cyclic prepared buf %pad len %zu period %zu periods %d\n",
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index a63d62b..6571100 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -308,26 +308,44 @@ static inline struct dw_dma *to_dw_dma(struct dma_device *ddev)
 	return container_of(ddev, struct dw_dma, dma);
 }
 
+#ifdef CONFIG_DW_DMAC_BIG_ENDIAN_IO
+typedef __be32 __dw32;
+#else
+typedef __le32 __dw32;
+#endif
+
 /* LLI == Linked List Item; a.k.a. DMA block descriptor */
 struct dw_lli {
 	/* values that are not changed by hardware */
-	u32		sar;
-	u32		dar;
-	u32		llp;		/* chain to next lli */
-	u32		ctllo;
+	__dw32		sar;
+	__dw32		dar;
+	__dw32		llp;		/* chain to next lli */
+	__dw32		ctllo;
 	/* values that may get written back: */
-	u32		ctlhi;
+	__dw32		ctlhi;
 	/* sstat and dstat can snapshot peripheral register state.
 	 * silicon config may discard either or both...
 	 */
-	u32		sstat;
-	u32		dstat;
+	__dw32		sstat;
+	__dw32		dstat;
 };
 
 struct dw_desc {
 	/* FIRST values the hardware uses */
 	struct dw_lli			lli;
 
+#ifdef CONFIG_DW_DMAC_BIG_ENDIAN_IO
+#define lli_set(d, reg, v)		((d)->lli.reg |= cpu_to_be32(v))
+#define lli_clear(d, reg, v)		((d)->lli.reg &= ~cpu_to_be32(v))
+#define lli_read(d, reg)		be32_to_cpu((d)->lli.reg)
+#define lli_write(d, reg, v)		((d)->lli.reg = cpu_to_be32(v))
+#else
+#define lli_set(d, reg, v)		((d)->lli.reg |= cpu_to_le32(v))
+#define lli_clear(d, reg, v)		((d)->lli.reg &= ~cpu_to_le32(v))
+#define lli_read(d, reg)		le32_to_cpu((d)->lli.reg)
+#define lli_write(d, reg, v)		((d)->lli.reg = cpu_to_le32(v))
+#endif
+
 	/* THEN values for driver housekeeping */
 	struct list_head		desc_node;
 	struct list_head		tx_list;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 05/15] dmaengine: dw: set LMS field in descriptors
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (3 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 04/15] dmaengine: dw: fix byte order of hw descriptor fields Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 06/15] dmaengine: dw: clear LLP_[SD]_EN bits in last descriptor of a chain Andy Shevchenko
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta
  Cc: Mans Rullgard

From: Mans Rullgard <mans@mansr.com>

The LMS field indicates from which master the descriptor is to be
read.  This patch assumes this is always the same as the memory
side in a peripheral transfer which is true for all known systems.

Signed-off-by: Mans Rullgard <mans@mansr.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 26 ++++++++++++++------------
 drivers/dma/dw/regs.h |  4 ++++
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 279abff..7cbc2a9 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -217,6 +217,7 @@ static inline void dwc_do_single_block(struct dw_dma_chan *dwc,
 static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first)
 {
 	struct dw_dma	*dw = to_dw_dma(dwc->chan.device);
+	u8		lms = DWC_LLP_LMS(dwc->m_master);
 	unsigned long	was_soft_llp;
 
 	/* ASSERT:  channel is idle */
@@ -252,9 +253,8 @@ static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first)
 
 	dwc_initialize(dwc);
 
-	channel_writel(dwc, LLP, first->txd.phys);
-	channel_writel(dwc, CTL_LO,
-			DWC_CTLL_LLP_D_EN | DWC_CTLL_LLP_S_EN);
+	channel_writel(dwc, LLP, first->txd.phys | lms);
+	channel_writel(dwc, CTL_LO, DWC_CTLL_LLP_D_EN | DWC_CTLL_LLP_S_EN);
 	channel_writel(dwc, CTL_HI, 0);
 	channel_set_bit(dw, CH_EN, dwc->mask);
 }
@@ -418,7 +418,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 		dwc->residue = desc->total_len;
 
 		/* Check first descriptors addr */
-		if (desc->txd.phys == llp) {
+		if (desc->txd.phys == DWC_LLP_LOC(llp)) {
 			spin_unlock_irqrestore(&dwc->lock, flags);
 			return;
 		}
@@ -705,6 +705,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 	unsigned int		dst_width;
 	unsigned int		data_width;
 	u32			ctllo;
+	u8			lms = DWC_LLP_LMS(dwc->m_master);
 
 	dev_vdbg(chan2dev(chan),
 			"%s: d%pad s%pad l0x%zx f0x%lx\n", __func__,
@@ -747,7 +748,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 		if (!first) {
 			first = desc;
 		} else {
-			lli_write(prev, llp, desc->txd.phys);
+			lli_write(prev, llp, desc->txd.phys | lms);
 			list_add_tail(&desc->desc_node, &first->tx_list);
 		}
 		prev = desc;
@@ -779,6 +780,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	struct dw_desc		*prev;
 	struct dw_desc		*first;
 	u32			ctllo;
+	u8			lms = DWC_LLP_LMS(dwc->m_master);
 	dma_addr_t		reg;
 	unsigned int		reg_width;
 	unsigned int		mem_width;
@@ -843,7 +845,7 @@ slave_sg_todev_fill_desc:
 			if (!first) {
 				first = desc;
 			} else {
-				lli_write(prev, llp, desc->txd.phys);
+				lli_write(prev, llp, desc->txd.phys | lms);
 				list_add_tail(&desc->desc_node, &first->tx_list);
 			}
 			prev = desc;
@@ -898,7 +900,7 @@ slave_sg_fromdev_fill_desc:
 			if (!first) {
 				first = desc;
 			} else {
-				lli_write(prev, llp, desc->txd.phys);
+				lli_write(prev, llp, desc->txd.phys | lms);
 				list_add_tail(&desc->desc_node, &first->tx_list);
 			}
 			prev = desc;
@@ -1327,6 +1329,7 @@ struct dw_cyclic_desc *dw_dma_cyclic_prep(struct dma_chan *chan,
 	struct dw_cyclic_desc		*retval = NULL;
 	struct dw_desc			*desc;
 	struct dw_desc			*last = NULL;
+	u8				lms = DWC_LLP_LMS(dwc->m_master);
 	unsigned long			was_cyclic;
 	unsigned int			reg_width;
 	unsigned int			periods;
@@ -1435,13 +1438,13 @@ struct dw_cyclic_desc *dw_dma_cyclic_prep(struct dma_chan *chan,
 		cdesc->desc[i] = desc;
 
 		if (last)
-			lli_write(last, llp, desc->txd.phys);
+			lli_write(last, llp, desc->txd.phys | lms);
 
 		last = desc;
 	}
 
 	/* Let's make a cyclic list */
-	lli_write(last, llp, cdesc->desc[0]->txd.phys);
+	lli_write(last, llp, cdesc->desc[0]->txd.phys | lms);
 
 	dev_dbg(chan2dev(&dwc->chan),
 			"cyclic prepared buf %pad len %zu period %zu periods %d\n",
@@ -1643,9 +1646,8 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 			dwc->block_size = pdata->block_size;
 
 			/* Check if channel supports multi block transfer */
-			channel_writel(dwc, LLP, 0xfffffffc);
-			dwc->nollp =
-				(channel_readl(dwc, LLP) & 0xfffffffc) == 0;
+			channel_writel(dwc, LLP, DWC_LLP_LOC(0xffffffff));
+			dwc->nollp = DWC_LLP_LOC(channel_readl(dwc, LLP)) == 0;
 			channel_writel(dwc, LLP, 0);
 		}
 	}
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 6571100..59d6cec 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -143,6 +143,10 @@ enum dw_dma_msize {
 	DW_DMA_MSIZE_256,
 };
 
+/* Bitfields in LLP */
+#define DWC_LLP_LMS(x)		((x) & 3)	/* list master select */
+#define DWC_LLP_LOC(x)		((x) & ~3)	/* next lli */
+
 /* Bitfields in CTL_LO */
 #define DWC_CTLL_INT_EN		(1 << 0)	/* irqs enabled? */
 #define DWC_CTLL_DST_WIDTH(n)	((n)<<1)	/* bytes per element */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 06/15] dmaengine: dw: clear LLP_[SD]_EN bits in last descriptor of a chain
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (4 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 05/15] dmaengine: dw: set LMS field in descriptors Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 07/15] dmaengine: dw: substitute dma_read_byaddr by dma_readl_native Andy Shevchenko
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta
  Cc: Mans Rullgard

From: Mans Rullgard <mans@mansr.com>

The datasheet requires that the LLP_[SD]_EN bits be cleared whenever
LLP.LOC is zero, i.e. in the last descriptor of a multi-block chain.
Make the driver do this.

Signed-off-by: Mans Rullgard <mans@mansr.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 7cbc2a9..4ac12ee 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -759,6 +759,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 		lli_set(prev, ctllo, DWC_CTLL_INT_EN);
 
 	prev->lli.llp = 0;
+	lli_clear(prev, ctllo, DWC_CTLL_LLP_D_EN | DWC_CTLL_LLP_S_EN);
 	first->txd.flags = flags;
 	first->total_len = len;
 
@@ -919,6 +920,7 @@ slave_sg_fromdev_fill_desc:
 		lli_set(prev, ctllo, DWC_CTLL_INT_EN);
 
 	prev->lli.llp = 0;
+	lli_clear(prev, ctllo, DWC_CTLL_LLP_D_EN | DWC_CTLL_LLP_S_EN);
 	first->total_len = total_len;
 
 	return &first->txd;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 07/15] dmaengine: dw: substitute dma_read_byaddr by dma_readl_native
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (5 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 06/15] dmaengine: dw: clear LLP_[SD]_EN bits in last descriptor of a chain Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 08/15] dmaengine: dw: revisit data_width property Andy Shevchenko
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

Since struct dw_dma is allocated and regs member is assigned properly we can
use standard IO accessors to the DMA registers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 8 +++-----
 drivers/dma/dw/regs.h | 4 ----
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 4ac12ee..4748df9 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1526,7 +1526,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 	pm_runtime_get_sync(chip->dev);
 
 	if (!pdata) {
-		dw_params = dma_read_byaddr(chip->regs, DW_PARAMS);
+		dw_params = dma_readl(dw, DW_PARAMS);
 		dev_dbg(chip->dev, "DW_PARAMS: 0x%08x\n", dw_params);
 
 		autocfg = dw_params >> DW_PARAMS_EN & 1;
@@ -1626,11 +1626,9 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 
 		/* Hardware configuration */
 		if (autocfg) {
-			unsigned int dwc_params;
 			unsigned int r = DW_DMA_MAX_NR_CHANNELS - i - 1;
-			void __iomem *addr = chip->regs + r * sizeof(u32);
-
-			dwc_params = dma_read_byaddr(addr, DWC_PARAMS);
+			void __iomem *addr = &__dw_regs(dw)->DWC_PARAMS[r];
+			unsigned int dwc_params = dma_readl_native(addr);
 
 			dev_dbg(chip->dev, "DWC_PARAMS[%d]: 0x%08x\n", i,
 					   dwc_params);
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 59d6cec..feb3a4a 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -114,10 +114,6 @@ struct dw_dma_regs {
 #define dma_writel_native writel
 #endif
 
-/* To access the registers in early stage of probe */
-#define dma_read_byaddr(addr, name) \
-	dma_readl_native((addr) + offsetof(struct dw_dma_regs, name))
-
 /* Bitfields in DW_PARAMS */
 #define DW_PARAMS_NR_CHAN	8		/* number of channels */
 #define DW_PARAMS_NR_MASTER	11		/* number of AHB masters */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (6 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 07/15] dmaengine: dw: substitute dma_read_byaddr by dma_readl_native Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-04-13 15:57   ` Vinod Koul
  2016-03-18 14:24 ` [PATCH v3 09/15] dmaengine: dw: define counter variables as unsigned int Andy Shevchenko
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

There are several changes are done here:

 - Convert the property to be in bytes

   Much more convenient than keeping encoded value.

 - Use one value for all AHB masters for now

   It seems in practice we have no controllers where masters have different
   data bus width, we still might return to distinct values when there is a use
   case.

 - Rename data_width to data-width in the device tree bindings.

 - While here, replace dwc_fast_ffs() by __ffs().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 Documentation/devicetree/bindings/dma/snps-dma.txt |  5 ++-
 arch/arc/boot/dts/abilis_tb10x.dtsi                |  2 +-
 arch/arm/boot/dts/spear13xx.dtsi                   |  4 +--
 drivers/dma/dw/core.c                              | 40 +++-------------------
 drivers/dma/dw/platform.c                          | 10 +++---
 drivers/dma/dw/regs.h                              |  2 +-
 include/linux/platform_data/dma-dw.h               |  5 ++-
 7 files changed, 18 insertions(+), 50 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
index c99c1ff..eb48229 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -13,8 +13,7 @@ Required properties:
 - chan_priority: priority of channels. 0 (default): increase from chan 0->n, 1:
   increase from chan n->0
 - block_size: Maximum block size supported by the controller
-- data_width: Maximum data width supported by hardware per AHB master
-  (0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
+- data-width: Maximum data width supported by hardware (in bytes, power of 2)
 
 
 Optional properties:
@@ -38,7 +37,7 @@ Example:
 		chan_allocation_order = <1>;
 		chan_priority = <1>;
 		block_size = <0xfff>;
-		data_width = <3 3>;
+		data-width = <8>;
 	};
 
 DMA clients connected to the Designware DMA controller must use the format
diff --git a/arch/arc/boot/dts/abilis_tb10x.dtsi b/arch/arc/boot/dts/abilis_tb10x.dtsi
index cfb5052..2f53bed 100644
--- a/arch/arc/boot/dts/abilis_tb10x.dtsi
+++ b/arch/arc/boot/dts/abilis_tb10x.dtsi
@@ -112,7 +112,7 @@
 			chan_allocation_order = <0>;
 			chan_priority = <1>;
 			block_size = <0x7ff>;
-			data_width = <2>;
+			data-width = <4>;
 			clocks = <&ahb_clk>;
 			clock-names = "hclk";
 		};
diff --git a/arch/arm/boot/dts/spear13xx.dtsi b/arch/arm/boot/dts/spear13xx.dtsi
index 14594ce..474b66f 100644
--- a/arch/arm/boot/dts/spear13xx.dtsi
+++ b/arch/arm/boot/dts/spear13xx.dtsi
@@ -117,7 +117,7 @@
 			chan_priority = <1>;
 			block_size = <0xfff>;
 			dma-masters = <2>;
-			data_width = <3 3>;
+			data-width = <8>;
 		};
 
 		dma@eb000000 {
@@ -133,7 +133,7 @@
 			chan_allocation_order = <1>;
 			chan_priority = <1>;
 			block_size = <0xfff>;
-			data_width = <3 3>;
+			data-width = <8>;
 		};
 
 		fsmc: flash@b0000000 {
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 4748df9..79dbfc3 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -155,21 +155,6 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
 
 /*----------------------------------------------------------------------*/
 
-static inline unsigned int dwc_fast_ffs(unsigned long long v)
-{
-	/*
-	 * We can be a lot more clever here, but this should take care
-	 * of the most common optimization.
-	 */
-	if (!(v & 7))
-		return 3;
-	else if (!(v & 3))
-		return 2;
-	else if (!(v & 1))
-		return 1;
-	return 0;
-}
-
 static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc)
 {
 	dev_err(chan2dev(&dwc->chan),
@@ -703,7 +688,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 	size_t			offset;
 	unsigned int		src_width;
 	unsigned int		dst_width;
-	unsigned int		data_width;
 	u32			ctllo;
 	u8			lms = DWC_LLP_LMS(dwc->m_master);
 
@@ -718,10 +702,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 
 	dwc->direction = DMA_MEM_TO_MEM;
 
-	data_width = dw->data_width[dwc->m_master];
-
-	src_width = dst_width = min_t(unsigned int, data_width,
-				      dwc_fast_ffs(src | dest | len));
+	src_width = dst_width = __ffs(dw->data_width | src | dest | len);
 
 	ctllo = DWC_DEFAULT_CTLLO(chan)
 			| DWC_CTLL_DST_WIDTH(dst_width)
@@ -785,7 +766,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	dma_addr_t		reg;
 	unsigned int		reg_width;
 	unsigned int		mem_width;
-	unsigned int		data_width;
 	unsigned int		i;
 	struct scatterlist	*sg;
 	size_t			total_len = 0;
@@ -811,8 +791,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
 			DWC_CTLL_FC(DW_DMA_FC_D_M2P);
 
-		data_width = dw->data_width[dwc->m_master];
-
 		for_each_sg(sgl, sg, sg_len, i) {
 			struct dw_desc	*desc;
 			u32		len, dlen, mem;
@@ -820,8 +798,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 			mem = sg_dma_address(sg);
 			len = sg_dma_len(sg);
 
-			mem_width = min_t(unsigned int,
-					  data_width, dwc_fast_ffs(mem | len));
+			mem_width = __ffs(dw->data_width | mem | len);
 
 slave_sg_todev_fill_desc:
 			desc = dwc_desc_get(dwc);
@@ -867,8 +844,6 @@ slave_sg_todev_fill_desc:
 		ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
 			DWC_CTLL_FC(DW_DMA_FC_D_P2M);
 
-		data_width = dw->data_width[dwc->m_master];
-
 		for_each_sg(sgl, sg, sg_len, i) {
 			struct dw_desc	*desc;
 			u32		len, dlen, mem;
@@ -876,8 +851,7 @@ slave_sg_todev_fill_desc:
 			mem = sg_dma_address(sg);
 			len = sg_dma_len(sg);
 
-			mem_width = min_t(unsigned int,
-					  data_width, dwc_fast_ffs(mem | len));
+			mem_width = __ffs(dw->data_width | mem | len);
 
 slave_sg_fromdev_fill_desc:
 			desc = dwc_desc_get(dwc);
@@ -1544,10 +1518,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 		/* Get hardware configuration parameters */
 		pdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN & 7) + 1;
 		pdata->nr_masters = (dw_params >> DW_PARAMS_NR_MASTER & 3) + 1;
-		for (i = 0; i < pdata->nr_masters; i++) {
-			pdata->data_width[i] =
-				(dw_params >> DW_PARAMS_DATA_WIDTH(i) & 3) + 2;
-		}
+		pdata->data_width = 4 << (dw_params >> DW_PARAMS_DATA_WIDTH(0) & 3);
 		max_blk_size = dma_readl(dw, MAX_BLK_SIZE);
 
 		/* Fill platform data with the default values */
@@ -1569,8 +1540,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 
 	/* Get hardware configuration parameters */
 	dw->nr_masters = pdata->nr_masters;
-	for (i = 0; i < dw->nr_masters; i++)
-		dw->data_width[i] = pdata->data_width[i];
+	dw->data_width = pdata->data_width;
 
 	/* Calculate all channel mask before DMA setup */
 	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index 23616c5..4cf399d 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -102,8 +102,8 @@ dw_dma_parse_dt(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct dw_dma_platform_data *pdata;
-	u32 tmp, arr[DW_DMA_MAX_NR_MASTERS];
 	u32 nr_channels;
+	u32 tmp;
 
 	if (!np) {
 		dev_err(&pdev->dev, "Missing DT data\n");
@@ -138,10 +138,10 @@ dw_dma_parse_dt(struct platform_device *pdev)
 		pdata->nr_masters = tmp;
 	}
 
-	if (!of_property_read_u32_array(np, "data_width", arr,
-				pdata->nr_masters))
-		for (tmp = 0; tmp < pdata->nr_masters; tmp++)
-			pdata->data_width[tmp] = arr[tmp];
+	if (!of_property_read_u32(np, "data-width", &tmp))
+		pdata->data_width = tmp;
+	else if (!of_property_read_u32(np, "data_width", &tmp))
+		pdata->data_width = BIT(tmp & 0x07);
 
 	return pdata;
 }
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index feb3a4a..d0f9173 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -285,7 +285,7 @@ struct dw_dma {
 
 	/* hardware configuration */
 	unsigned char		nr_masters;
-	unsigned char		data_width[DW_DMA_MAX_NR_MASTERS];
+	unsigned char		data_width;
 };
 
 static inline struct dw_dma_regs __iomem *__dw_regs(struct dw_dma *dw)
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index b881b97..09cf5c1 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -42,8 +42,7 @@ struct dw_dma_slave {
  * @chan_priority: Set channel priority increasing from 0 to 7 or 7 to 0.
  * @block_size: Maximum block size supported by the controller
  * @nr_masters: Number of AHB masters supported by the controller
- * @data_width: Maximum data width supported by hardware per AHB master
- *		(0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
+ * @data_width: Maximum data width supported by hardware (in bytes, power of 2)
  */
 struct dw_dma_platform_data {
 	unsigned int	nr_channels;
@@ -57,7 +56,7 @@ struct dw_dma_platform_data {
 	unsigned char	chan_priority;
 	unsigned short	block_size;
 	unsigned char	nr_masters;
-	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
+	unsigned char	data_width;
 };
 
 #endif /* _PLATFORM_DATA_DMA_DW_H */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 09/15] dmaengine: dw: define counter variables as unsigned int
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (7 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 08/15] dmaengine: dw: revisit data_width property Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 10/15] dmaengine: dw: keep entire platform data in struct dw_dma Andy Shevchenko
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

The code is fixed to satisfy a compiler otherwise we have

drivers/dma/dw/core.c: In function ‘dwc_handle_cyclic’:
drivers/dma/dw/core.c:568: warning: comparison between signed and unsigned
drivers/dma/dw/core.c: In function ‘dw_dma_tasklet’:
drivers/dma/dw/core.c:590: warning: comparison between signed and unsigned
drivers/dma/dw/core.c: In function ‘dw_dma_off’:
drivers/dma/dw/core.c:1103: warning: comparison between signed and unsigned
drivers/dma/dw/core.c: In function ‘dw_dma_cyclic_free’:
drivers/dma/dw/core.c:1469: warning: comparison between signed and unsigned
drivers/dma/dw/core.c: In function ‘dw_dma_probe’:
drivers/dma/dw/core.c:1574: warning: comparison between signed and unsigned

There is no functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 79dbfc3..5ce698f 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -542,7 +542,7 @@ static void dwc_handle_cyclic(struct dw_dma *dw, struct dw_dma_chan *dwc,
 	 */
 	if (unlikely(status_err & dwc->mask) ||
 			unlikely(status_xfer & dwc->mask)) {
-		int i;
+		unsigned int i;
 
 		dev_err(chan2dev(&dwc->chan),
 			"cyclic DMA unexpected %s interrupt, stopping DMA transfer\n",
@@ -582,7 +582,7 @@ static void dw_dma_tasklet(unsigned long data)
 	u32 status_block;
 	u32 status_xfer;
 	u32 status_err;
-	int i;
+	unsigned int i;
 
 	status_block = dma_readl(dw, RAW.BLOCK);
 	status_xfer = dma_readl(dw, RAW.XFER);
@@ -1089,7 +1089,7 @@ static void dwc_issue_pending(struct dma_chan *chan)
 
 static void dw_dma_off(struct dw_dma *dw)
 {
-	int i;
+	unsigned int i;
 
 	dma_writel(dw, CFG, 0);
 
@@ -1451,7 +1451,7 @@ void dw_dma_cyclic_free(struct dma_chan *chan)
 	struct dw_dma_chan	*dwc = to_dw_dma_chan(chan);
 	struct dw_dma		*dw = to_dw_dma(dwc->chan.device);
 	struct dw_cyclic_desc	*cdesc = dwc->cdesc;
-	int			i;
+	unsigned int		i;
 	unsigned long		flags;
 
 	dev_dbg(chan2dev(&dwc->chan), "%s\n", __func__);
@@ -1487,8 +1487,8 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 	bool			autocfg = false;
 	unsigned int		dw_params;
 	unsigned int		max_blk_size = 0;
+	unsigned int		i;
 	int			err;
-	int			i;
 
 	dw = devm_kzalloc(chip->dev, sizeof(*dw), GFP_KERNEL);
 	if (!dw)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 10/15] dmaengine: dw: keep entire platform data in struct dw_dma
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (8 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 09/15] dmaengine: dw: define counter variables as unsigned int Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 11/15] dmaengine: dw: pass platform data via struct dw_dma_chip Andy Shevchenko
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

Keep the entire platform data in the struct dw_dma.
It makes the driver a bit cleaner.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c                | 31 ++++++++++++++++---------------
 drivers/dma/dw/platform.c            |  4 ++--
 drivers/dma/dw/regs.h                |  5 ++---
 include/linux/platform_data/dma-dw.h |  2 +-
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 5ce698f..c001e56 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -702,7 +702,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 
 	dwc->direction = DMA_MEM_TO_MEM;
 
-	src_width = dst_width = __ffs(dw->data_width | src | dest | len);
+	src_width = dst_width = __ffs(dw->pdata->data_width | src | dest | len);
 
 	ctllo = DWC_DEFAULT_CTLLO(chan)
 			| DWC_CTLL_DST_WIDTH(dst_width)
@@ -798,7 +798,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 			mem = sg_dma_address(sg);
 			len = sg_dma_len(sg);
 
-			mem_width = __ffs(dw->data_width | mem | len);
+			mem_width = __ffs(dw->pdata->data_width | mem | len);
 
 slave_sg_todev_fill_desc:
 			desc = dwc_desc_get(dwc);
@@ -851,7 +851,7 @@ slave_sg_todev_fill_desc:
 			mem = sg_dma_address(sg);
 			len = sg_dma_len(sg);
 
-			mem_width = __ffs(dw->data_width | mem | len);
+			mem_width = __ffs(dw->pdata->data_width | mem | len);
 
 slave_sg_fromdev_fill_desc:
 			desc = dwc_desc_get(dwc);
@@ -1486,7 +1486,6 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 	struct dw_dma		*dw;
 	bool			autocfg = false;
 	unsigned int		dw_params;
-	unsigned int		max_blk_size = 0;
 	unsigned int		i;
 	int			err;
 
@@ -1494,6 +1493,10 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 	if (!dw)
 		return -ENOMEM;
 
+	dw->pdata = devm_kzalloc(chip->dev, sizeof(*dw->pdata), GFP_KERNEL);
+	if (!dw->pdata)
+		return -ENOMEM;
+
 	dw->regs = chip->regs;
 	chip->dw = dw;
 
@@ -1509,17 +1512,14 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 			goto err_pdata;
 		}
 
-		pdata = devm_kzalloc(chip->dev, sizeof(*pdata), GFP_KERNEL);
-		if (!pdata) {
-			err = -ENOMEM;
-			goto err_pdata;
-		}
+		/* Reassign the platform data pointer */
+		pdata = dw->pdata;
 
 		/* Get hardware configuration parameters */
 		pdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN & 7) + 1;
 		pdata->nr_masters = (dw_params >> DW_PARAMS_NR_MASTER & 3) + 1;
 		pdata->data_width = 4 << (dw_params >> DW_PARAMS_DATA_WIDTH(0) & 3);
-		max_blk_size = dma_readl(dw, MAX_BLK_SIZE);
+		pdata->block_size = dma_readl(dw, MAX_BLK_SIZE);
 
 		/* Fill platform data with the default values */
 		pdata->is_private = true;
@@ -1529,6 +1529,11 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 	} else if (pdata->nr_channels > DW_DMA_MAX_NR_CHANNELS) {
 		err = -EINVAL;
 		goto err_pdata;
+	} else {
+		memcpy(dw->pdata, pdata, sizeof(*dw->pdata));
+
+		/* Reassign the platform data pointer */
+		pdata = dw->pdata;
 	}
 
 	dw->chan = devm_kcalloc(chip->dev, pdata->nr_channels, sizeof(*dw->chan),
@@ -1538,10 +1543,6 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 		goto err_pdata;
 	}
 
-	/* Get hardware configuration parameters */
-	dw->nr_masters = pdata->nr_masters;
-	dw->data_width = pdata->data_width;
-
 	/* Calculate all channel mask before DMA setup */
 	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
 
@@ -1609,7 +1610,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 			 * up to 0x0a for 4095.
 			 */
 			dwc->block_size =
-				(4 << ((max_blk_size >> 4 * i) & 0xf)) - 1;
+				(4 << ((pdata->block_size >> 4 * i) & 0xf)) - 1;
 			dwc->nollp =
 				(dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0;
 		} else {
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index 4cf399d..a7d6420 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -47,8 +47,8 @@ static struct dma_chan *dw_dma_of_xlate(struct of_phandle_args *dma_spec,
 
 	if (WARN_ON(slave.src_id >= DW_DMA_MAX_NR_REQUESTS ||
 		    slave.dst_id >= DW_DMA_MAX_NR_REQUESTS ||
-		    slave.m_master >= dw->nr_masters ||
-		    slave.p_master >= dw->nr_masters))
+		    slave.m_master >= dw->pdata->nr_masters ||
+		    slave.p_master >= dw->pdata->nr_masters))
 		return NULL;
 
 	dma_cap_zero(cap);
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index d0f9173..4424940 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -283,9 +283,8 @@ struct dw_dma {
 	u8			all_chan_mask;
 	u8			in_use;
 
-	/* hardware configuration */
-	unsigned char		nr_masters;
-	unsigned char		data_width;
+	/* platform data */
+	struct dw_dma_platform_data	*pdata;
 };
 
 static inline struct dw_dma_regs __iomem *__dw_regs(struct dw_dma *dw)
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index 09cf5c1..06227d0 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -54,7 +54,7 @@ struct dw_dma_platform_data {
 #define CHAN_PRIORITY_ASCENDING		0	/* chan0 highest */
 #define CHAN_PRIORITY_DESCENDING	1	/* chan7 highest */
 	unsigned char	chan_priority;
-	unsigned short	block_size;
+	unsigned int	block_size;
 	unsigned char	nr_masters;
 	unsigned char	data_width;
 };
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 11/15] dmaengine: dw: pass platform data via struct dw_dma_chip
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (9 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 10/15] dmaengine: dw: keep entire platform data in struct dw_dma Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 12/15] dmaengine: dw: move dwc->paused to dwc->flags Andy Shevchenko
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

We pass struct dw_dma_chip to the dw_dma_probe() anyway, thus we may use it to
pass platform data as well.

While here, constify the source of platform data.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ata/sata_dwc_460ex.c          | 2 +-
 drivers/dma/dw/core.c                 | 9 +++++----
 drivers/dma/dw/pci.c                  | 5 +++--
 drivers/dma/dw/platform.c             | 5 +++--
 include/linux/dma/dw.h                | 5 ++++-
 sound/soc/intel/common/sst-firmware.c | 2 +-
 6 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 80bdcab..2cb6f7e 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -1248,7 +1248,7 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 	hsdev->dma->dev = &ofdev->dev;
 
 	/* Initialize AHB DMAC */
-	err = dw_dma_probe(hsdev->dma, NULL);
+	err = dw_dma_probe(hsdev->dma);
 	if (err)
 		goto error_dma_iomap;
 
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index c001e56..77df7ec 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1481,8 +1481,9 @@ EXPORT_SYMBOL(dw_dma_cyclic_free);
 
 /*----------------------------------------------------------------------*/
 
-int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
+int dw_dma_probe(struct dw_dma_chip *chip)
 {
+	struct dw_dma_platform_data *pdata;
 	struct dw_dma		*dw;
 	bool			autocfg = false;
 	unsigned int		dw_params;
@@ -1502,7 +1503,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 
 	pm_runtime_get_sync(chip->dev);
 
-	if (!pdata) {
+	if (!chip->pdata) {
 		dw_params = dma_readl(dw, DW_PARAMS);
 		dev_dbg(chip->dev, "DW_PARAMS: 0x%08x\n", dw_params);
 
@@ -1526,11 +1527,11 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
 		pdata->is_memcpy = true;
 		pdata->chan_allocation_order = CHAN_ALLOCATION_ASCENDING;
 		pdata->chan_priority = CHAN_PRIORITY_ASCENDING;
-	} else if (pdata->nr_channels > DW_DMA_MAX_NR_CHANNELS) {
+	} else if (chip->pdata->nr_channels > DW_DMA_MAX_NR_CHANNELS) {
 		err = -EINVAL;
 		goto err_pdata;
 	} else {
-		memcpy(dw->pdata, pdata, sizeof(*dw->pdata));
+		memcpy(dw->pdata, chip->pdata, sizeof(*dw->pdata));
 
 		/* Reassign the platform data pointer */
 		pdata = dw->pdata;
diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
index 358f968..0ae6c3b 100644
--- a/drivers/dma/dw/pci.c
+++ b/drivers/dma/dw/pci.c
@@ -17,8 +17,8 @@
 
 static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid)
 {
+	const struct dw_dma_platform_data *pdata = (void *)pid->driver_data;
 	struct dw_dma_chip *chip;
-	struct dw_dma_platform_data *pdata = (void *)pid->driver_data;
 	int ret;
 
 	ret = pcim_enable_device(pdev);
@@ -49,8 +49,9 @@ static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid)
 	chip->dev = &pdev->dev;
 	chip->regs = pcim_iomap_table(pdev)[0];
 	chip->irq = pdev->irq;
+	chip->pdata = pdata;
 
-	ret = dw_dma_probe(chip, pdata);
+	ret = dw_dma_probe(chip);
 	if (ret)
 		return ret;
 
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index a7d6420..7dca5e3 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -158,7 +158,7 @@ static int dw_probe(struct platform_device *pdev)
 	struct dw_dma_chip *chip;
 	struct device *dev = &pdev->dev;
 	struct resource *mem;
-	struct dw_dma_platform_data *pdata;
+	const struct dw_dma_platform_data *pdata;
 	int err;
 
 	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
@@ -183,6 +183,7 @@ static int dw_probe(struct platform_device *pdev)
 		pdata = dw_dma_parse_dt(pdev);
 
 	chip->dev = dev;
+	chip->pdata = pdata;
 
 	chip->clk = devm_clk_get(chip->dev, "hclk");
 	if (IS_ERR(chip->clk))
@@ -193,7 +194,7 @@ static int dw_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 
-	err = dw_dma_probe(chip, pdata);
+	err = dw_dma_probe(chip);
 	if (err)
 		goto err_dw_dma_probe;
 
diff --git a/include/linux/dma/dw.h b/include/linux/dma/dw.h
index 7145644..f2e538a 100644
--- a/include/linux/dma/dw.h
+++ b/include/linux/dma/dw.h
@@ -27,6 +27,7 @@ struct dw_dma;
  * @regs:		memory mapped I/O space
  * @clk:		hclk clock
  * @dw:			struct dw_dma that is filed by dw_dma_probe()
+ * @pdata:		pointer to platform data
  */
 struct dw_dma_chip {
 	struct device	*dev;
@@ -34,10 +35,12 @@ struct dw_dma_chip {
 	void __iomem	*regs;
 	struct clk	*clk;
 	struct dw_dma	*dw;
+
+	const struct dw_dma_platform_data	*pdata;
 };
 
 /* Export to the platform drivers */
-int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata);
+int dw_dma_probe(struct dw_dma_chip *chip);
 int dw_dma_remove(struct dw_dma_chip *chip);
 
 /* DMA API extensions */
diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c
index ef4881e..2599352 100644
--- a/sound/soc/intel/common/sst-firmware.c
+++ b/sound/soc/intel/common/sst-firmware.c
@@ -203,7 +203,7 @@ static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem,
 
 	chip->dev = dev;
 
-	err = dw_dma_probe(chip, NULL);
+	err = dw_dma_probe(chip);
 	if (err)
 		return ERR_PTR(err);
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 12/15] dmaengine: dw: move dwc->paused to dwc->flags
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (10 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 11/15] dmaengine: dw: pass platform data via struct dw_dma_chip Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 13/15] dmaengine: dw: move dwc->initialized " Andy Shevchenko
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

We have already dedicated variable for flags, therefore no need to create an
additional storage for that. Convert dwc->paused to use dwc->flags.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 12 +++++-------
 drivers/dma/dw/regs.h |  2 +-
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 77df7ec..b481296 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -973,7 +973,7 @@ static int dwc_pause(struct dma_chan *chan)
 	while (!(channel_readl(dwc, CFG_LO) & DWC_CFGL_FIFO_EMPTY) && count--)
 		udelay(2);
 
-	dwc->paused = true;
+	set_bit(DW_DMA_IS_PAUSED, &dwc->flags);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
@@ -986,7 +986,7 @@ static inline void dwc_chan_resume(struct dw_dma_chan *dwc)
 
 	channel_writel(dwc, CFG_LO, cfglo & ~DWC_CFGL_CH_SUSP);
 
-	dwc->paused = false;
+	clear_bit(DW_DMA_IS_PAUSED, &dwc->flags);
 }
 
 static int dwc_resume(struct dma_chan *chan)
@@ -994,12 +994,10 @@ static int dwc_resume(struct dma_chan *chan)
 	struct dw_dma_chan	*dwc = to_dw_dma_chan(chan);
 	unsigned long		flags;
 
-	if (!dwc->paused)
-		return 0;
-
 	spin_lock_irqsave(&dwc->lock, flags);
 
-	dwc_chan_resume(dwc);
+	if (test_bit(DW_DMA_IS_PAUSED, &dwc->flags))
+		dwc_chan_resume(dwc);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
@@ -1068,7 +1066,7 @@ dwc_tx_status(struct dma_chan *chan,
 	if (ret != DMA_COMPLETE)
 		dma_set_residue(txstate, dwc_get_residue(dwc));
 
-	if (dwc->paused && ret == DMA_IN_PROGRESS)
+	if (test_bit(DW_DMA_IS_PAUSED, &dwc->flags) && ret == DMA_IN_PROGRESS)
 		return DMA_PAUSED;
 
 	return ret;
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 4424940..6355f6c 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -216,6 +216,7 @@ enum dw_dma_msize {
 enum dw_dmac_flags {
 	DW_DMA_IS_CYCLIC = 0,
 	DW_DMA_IS_SOFT_LLP = 1,
+	DW_DMA_IS_PAUSED = 2,
 };
 
 struct dw_dma_chan {
@@ -224,7 +225,6 @@ struct dw_dma_chan {
 	u8				mask;
 	u8				priority;
 	enum dma_transfer_direction	direction;
-	bool				paused;
 	bool				initialized;
 
 	/* software emulation of the LLP transfers */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 13/15] dmaengine: dw: move dwc->initialized to dwc->flags
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (11 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 12/15] dmaengine: dw: move dwc->paused to dwc->flags Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 14/15] dmaengine: dw: move residue to a descriptor Andy Shevchenko
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

We have already dedicated variable for flags, therefore no need to create an
additional storage for that. Covert dwc->initialized to use dwc->flags.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 8 ++++----
 drivers/dma/dw/regs.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index b481296..1e8425b 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -137,7 +137,7 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
 	u32 cfghi = DWC_CFGH_FIFO_MODE;
 	u32 cfglo = DWC_CFGL_CH_PRIOR(dwc->priority);
 
-	if (dwc->initialized == true)
+	if (test_bit(DW_DMA_IS_INITIALIZED, &dwc->flags))
 		return;
 
 	cfghi |= DWC_CFGH_DST_PER(dwc->dst_id);
@@ -150,7 +150,7 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
 	channel_set_bit(dw, MASK.XFER, dwc->mask);
 	channel_set_bit(dw, MASK.ERROR, dwc->mask);
 
-	dwc->initialized = true;
+	set_bit(DW_DMA_IS_INITIALIZED, &dwc->flags);
 }
 
 /*----------------------------------------------------------------------*/
@@ -1101,7 +1101,7 @@ static void dw_dma_off(struct dw_dma *dw)
 		cpu_relax();
 
 	for (i = 0; i < dw->dma.chancnt; i++)
-		dw->chan[i].initialized = false;
+		clear_bit(DW_DMA_IS_INITIALIZED, &dw->chan[i].flags);
 }
 
 static void dw_dma_on(struct dw_dma *dw)
@@ -1207,7 +1207,7 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	dwc->m_master = 0;
 	dwc->p_master = 0;
 
-	dwc->initialized = false;
+	clear_bit(DW_DMA_IS_INITIALIZED, &dwc->flags);
 
 	/* Disable interrupts */
 	channel_clear_bit(dw, MASK.XFER, dwc->mask);
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 6355f6c..2c0561d 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -217,6 +217,7 @@ enum dw_dmac_flags {
 	DW_DMA_IS_CYCLIC = 0,
 	DW_DMA_IS_SOFT_LLP = 1,
 	DW_DMA_IS_PAUSED = 2,
+	DW_DMA_IS_INITIALIZED = 3,
 };
 
 struct dw_dma_chan {
@@ -225,7 +226,6 @@ struct dw_dma_chan {
 	u8				mask;
 	u8				priority;
 	enum dma_transfer_direction	direction;
-	bool				initialized;
 
 	/* software emulation of the LLP transfers */
 	struct list_head	*tx_node_active;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 14/15] dmaengine: dw: move residue to a descriptor
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (12 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 13/15] dmaengine: dw: move dwc->initialized " Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-03-18 14:24 ` [PATCH v3 15/15] dmaengine: dw: set cdesc to NULL when free cyclic transfers Andy Shevchenko
  2016-04-13 16:08 ` [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Vinod Koul
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

Residue is a property of any active descriptor. So, any descriptor may be in
different state but residue is a feature of active descriptor. Check if the
asked descriptor is active and return proper residue value for it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 60 ++++++++++++++++++++++++++++++++++-----------------
 drivers/dma/dw/regs.h |  2 +-
 2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 1e8425b..78be785 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -227,7 +227,7 @@ static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first)
 
 		dwc_initialize(dwc);
 
-		dwc->residue = first->total_len;
+		first->residue = first->total_len;
 		dwc->tx_node_active = &first->tx_list;
 
 		/* Submit first block */
@@ -357,11 +357,11 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 			head = &desc->tx_list;
 			if (active != head) {
-				/* Update desc to reflect last sent one */
-				if (active != head->next)
-					desc = to_dw_desc(active->prev);
-
-				dwc->residue -= desc->len;
+				/* Update residue to reflect last sent descriptor */
+				if (active == head->next)
+					desc->residue -= desc->len;
+				else
+					desc->residue -= to_dw_desc(active->prev)->len;
 
 				child = to_dw_desc(active);
 
@@ -376,8 +376,6 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 			clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
 		}
 
-		dwc->residue = 0;
-
 		spin_unlock_irqrestore(&dwc->lock, flags);
 
 		dwc_complete_all(dw, dwc);
@@ -385,7 +383,6 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	}
 
 	if (list_empty(&dwc->active_list)) {
-		dwc->residue = 0;
 		spin_unlock_irqrestore(&dwc->lock, flags);
 		return;
 	}
@@ -400,7 +397,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 	list_for_each_entry_safe(desc, _desc, &dwc->active_list, desc_node) {
 		/* Initial residue value */
-		dwc->residue = desc->total_len;
+		desc->residue = desc->total_len;
 
 		/* Check first descriptors addr */
 		if (desc->txd.phys == DWC_LLP_LOC(llp)) {
@@ -411,20 +408,20 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 		/* Check first descriptors llp */
 		if (lli_read(desc, llp) == llp) {
 			/* This one is currently in progress */
-			dwc->residue -= dwc_get_sent(dwc);
+			desc->residue -= dwc_get_sent(dwc);
 			spin_unlock_irqrestore(&dwc->lock, flags);
 			return;
 		}
 
-		dwc->residue -= desc->len;
+		desc->residue -= desc->len;
 		list_for_each_entry(child, &desc->tx_list, desc_node) {
 			if (lli_read(child, llp) == llp) {
 				/* Currently in progress */
-				dwc->residue -= dwc_get_sent(dwc);
+				desc->residue -= dwc_get_sent(dwc);
 				spin_unlock_irqrestore(&dwc->lock, flags);
 				return;
 			}
-			dwc->residue -= child->len;
+			desc->residue -= child->len;
 		}
 
 		/*
@@ -1033,16 +1030,37 @@ static int dwc_terminate_all(struct dma_chan *chan)
 	return 0;
 }
 
-static inline u32 dwc_get_residue(struct dw_dma_chan *dwc)
+static struct dw_desc *dwc_find_desc(struct dw_dma_chan *dwc, dma_cookie_t c)
+{
+	struct dw_desc *desc;
+
+	list_for_each_entry(desc, &dwc->active_list, desc_node)
+		if (desc->txd.cookie == c)
+			return desc;
+
+	return NULL;
+}
+
+static u32 dwc_get_residue(struct dw_dma_chan *dwc, dma_cookie_t cookie)
 {
+	struct dw_desc *desc;
 	unsigned long flags;
 	u32 residue;
 
 	spin_lock_irqsave(&dwc->lock, flags);
 
-	residue = dwc->residue;
-	if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags) && residue)
-		residue -= dwc_get_sent(dwc);
+	desc = dwc_find_desc(dwc, cookie);
+	if (desc) {
+		if (desc == dwc_first_active(dwc)) {
+			residue = desc->residue;
+			if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags) && residue)
+				residue -= dwc_get_sent(dwc);
+		} else {
+			residue = desc->total_len;
+		}
+	} else {
+		residue = 0;
+	}
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 	return residue;
@@ -1063,8 +1081,10 @@ dwc_tx_status(struct dma_chan *chan,
 	dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
 
 	ret = dma_cookie_status(chan, cookie, txstate);
-	if (ret != DMA_COMPLETE)
-		dma_set_residue(txstate, dwc_get_residue(dwc));
+	if (ret == DMA_COMPLETE)
+		return ret;
+
+	dma_set_residue(txstate, dwc_get_residue(dwc, cookie));
 
 	if (test_bit(DW_DMA_IS_PAUSED, &dwc->flags) && ret == DMA_IN_PROGRESS)
 		return DMA_PAUSED;
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 2c0561d..5b3ea2d 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -237,7 +237,6 @@ struct dw_dma_chan {
 	struct list_head	active_list;
 	struct list_head	queue;
 	struct list_head	free_list;
-	u32			residue;
 	struct dw_cyclic_desc	*cdesc;
 
 	unsigned int		descs_allocated;
@@ -351,6 +350,7 @@ struct dw_desc {
 	struct dma_async_tx_descriptor	txd;
 	size_t				len;
 	size_t				total_len;
+	u32				residue;
 };
 
 #define to_dw_desc(h)	list_entry(h, struct dw_desc, desc_node)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 15/15] dmaengine: dw: set cdesc to NULL when free cyclic transfers
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (13 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 14/15] dmaengine: dw: move residue to a descriptor Andy Shevchenko
@ 2016-03-18 14:24 ` Andy Shevchenko
  2016-04-13 16:08 ` [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Vinod Koul
  15 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-03-18 14:24 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

To be sure we have the cyclic transfers already gone we set cdesc to NULL. It
will prevent the double free.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 78be785..86e55ab 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1493,6 +1493,8 @@ void dw_dma_cyclic_free(struct dma_chan *chan)
 	kfree(cdesc->desc);
 	kfree(cdesc);
 
+	dwc->cdesc = NULL;
+
 	clear_bit(DW_DMA_IS_CYCLIC, &dwc->flags);
 }
 EXPORT_SYMBOL(dw_dma_cyclic_free);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-03-18 14:24 ` [PATCH v3 01/15] dmaengine: dw: fix master selection Andy Shevchenko
@ 2016-04-04 17:03   ` Vinod Koul
  2016-04-04 17:10     ` Andy Shevchenko
  0 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2016-04-04 17:03 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta, stable

On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:
> +	/*
> +	 * We need controller-specific data to set up slave transfers.
> +	 */
> +	BUG_ON(chan->private && !dw_dma_filter(chan, chan->private));

I don't think BUG_ON is apt here, gracefully failing and printing that can
be better..

-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-04-04 17:03   ` Vinod Koul
@ 2016-04-04 17:10     ` Andy Shevchenko
  2016-04-06 18:56         ` Vinod Koul
  0 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-04 17:10 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta, stable

On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
> On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:
> > 
> > +	/*
> > +	 * We need controller-specific data to set up slave
> > transfers.
> > +	 */
> > +	BUG_ON(chan->private && !dw_dma_filter(chan, chan-
> > >private));
> I don't think BUG_ON is apt here, gracefully failing and printing
> that can
> be better..

Hm, this is coming from the existing code.

I would prefer to keep this one as is since it's targeted to stable@,
and I may issue sequential patch to replace BUG_ON. Sounds okay?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-04-04 17:10     ` Andy Shevchenko
@ 2016-04-06 18:56         ` Vinod Koul
  0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2016-04-06 18:56 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta, stable

On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
> On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
> > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:
> > > 
> > > +	/*
> > > +	 * We need controller-specific data to set up slave
> > > transfers.
> > > +	 */
> > > +	BUG_ON(chan->private && !dw_dma_filter(chan, chan-
> > > >private));
> > I don't think BUG_ON is apt here, gracefully failing and printing
> > that can
> > be better..
> 
> Hm, this is coming from the existing code.
> 
> I would prefer to keep this one as is since it's targeted to stable@,
> and I may issue sequential patch to replace BUG_ON. Sounds okay?

But since this is going stable, how about we remove here in the patch and
benefit stable tree as well?

Thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
@ 2016-04-06 18:56         ` Vinod Koul
  0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2016-04-06 18:56 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta, stable

On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
> On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
> > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:
> > > 
> > > +	/*
> > > +	�* We need controller-specific data to set up slave
> > > transfers.
> > > +	�*/
> > > +	BUG_ON(chan->private && !dw_dma_filter(chan, chan-
> > > >private));
> > I don't think BUG_ON is apt here, gracefully failing and printing
> > that can
> > be better..
> 
> Hm, this is coming from the existing code.
> 
> I would prefer to keep this one as is since it's targeted to stable@,
> and I may issue sequential patch to replace BUG_ON. Sounds okay?

But since this is going stable, how about we remove here in the patch and
benefit stable tree as well?

Thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-04-06 18:56         ` Vinod Koul
  (?)
@ 2016-04-06 19:56         ` Andy Shevchenko
  2016-04-06 21:09             ` Koul, Vinod
  -1 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-06 19:56 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Andy Shevchenko, Viresh Kumar, linux-kernel, dmaengine,
	Rob Herring, Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta, stable

On Wed, Apr 6, 2016 at 9:56 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
>> On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
>> > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:
>> > >
>> > > + /*
>> > > +  * We need controller-specific data to set up slave
>> > > transfers.
>> > > +  */
>> > > + BUG_ON(chan->private && !dw_dma_filter(chan, chan-
>> > > >private));
>> > I don't think BUG_ON is apt here, gracefully failing and printing
>> > that can
>> > be better..
>>
>> Hm, this is coming from the existing code.
>>
>> I would prefer to keep this one as is since it's targeted to stable@,
>> and I may issue sequential patch to replace BUG_ON. Sounds okay?
>
> But since this is going stable, how about we remove here in the patch and
> benefit stable tree as well?

I'm fine with that. I will send you patch tomorrow.Other than that do
we have any issues with the rest?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-04-06 19:56         ` Andy Shevchenko
@ 2016-04-06 21:09             ` Koul, Vinod
  0 siblings, 0 replies; 35+ messages in thread
From: Koul, Vinod @ 2016-04-06 21:09 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: linux-kernel, robh+dt, vireshk, stable, broonie, mark.rutland,
	dmaengine, gregkh, vgupta, tj, andriy.shevchenko, egtvedt

On Wed, 2016-04-06 at 22:56 +0300, Andy Shevchenko wrote:
> On Wed, Apr 6, 2016 at 9:56 PM, Vinod Koul <vinod.koul@intel.com>
> wrote:
> > On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
> > > On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
> > > > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:
> > > > > 
> > > > > + /*
> > > > > +  * We need controller-specific data to set up slave
> > > > > transfers.
> > > > > +  */
> > > > > + BUG_ON(chan->private && !dw_dma_filter(chan, chan-
> > > > > > private));
> > > > I don't think BUG_ON is apt here, gracefully failing and
> > > > printing
> > > > that can
> > > > be better..
> > > 
> > > Hm, this is coming from the existing code.
> > > 
> > > I would prefer to keep this one as is since it's targeted to
> > > stable@,
> > > and I may issue sequential patch to replace BUG_ON. Sounds okay?
> > 
> > But since this is going stable, how about we remove here in the
> > patch and
> > benefit stable tree as well?
> 
> I'm fine with that. I will send you patch tomorrow.Other than that do
> we have any issues with the rest?

I did look at few more they looked fine, but didn't look at entire
series


-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
@ 2016-04-06 21:09             ` Koul, Vinod
  0 siblings, 0 replies; 35+ messages in thread
From: Koul, Vinod @ 2016-04-06 21:09 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: linux-kernel, robh+dt, vireshk, stable, broonie, mark.rutland,
	dmaengine, gregkh, vgupta, tj, andriy.shevchenko, egtvedt

On Wed, 2016-04-06 at 22:56 +0300, Andy Shevchenko wrote:
> On Wed, Apr 6, 2016 at 9:56 PM, Vinod Koul <vinod.koul@intel.com>
> wrote:
> > On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
> > > On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
> > > > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:
> > > > > 
> > > > > + /*
> > > > > +  * We need controller-specific data to set up slave
> > > > > transfers.
> > > > > +  */
> > > > > + BUG_ON(chan->private && !dw_dma_filter(chan, chan-
> > > > > > private));
> > > > I don't think BUG_ON is apt here, gracefully failing and
> > > > printing
> > > > that can
> > > > be better..
> > > 
> > > Hm, this is coming from the existing code.
> > > 
> > > I would prefer to keep this one as is since it's targeted to
> > > stable@,
> > > and I may issue sequential patch to replace BUG_ON. Sounds okay?
> > 
> > But since this is going stable, how about we remove here in the
> > patch and
> > benefit stable tree as well?
> 
> I'm fine with that. I will send you patch tomorrow.Other than that do
> we have any issues with the rest?

I did look at few more they looked fine, but didn't look at entire
series


-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-04-06 21:09             ` Koul, Vinod
@ 2016-04-07 13:03               ` Andy Shevchenko
  -1 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-07 13:03 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: linux-kernel, robh+dt, vireshk, stable, broonie, mark.rutland,
	dmaengine, gregkh, vgupta, tj, andriy.shevchenko, egtvedt

On Thu, Apr 7, 2016 at 12:09 AM, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Wed, 2016-04-06 at 22:56 +0300, Andy Shevchenko wrote:
>> On Wed, Apr 6, 2016 at 9:56 PM, Vinod Koul <vinod.koul@intel.com>
>> wrote:
>> > On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
>> > > On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
>> > > > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:

>> > > > > + /*
>> > > > > +  * We need controller-specific data to set up slave
>> > > > > transfers.
>> > > > > +  */
>> > > > > + BUG_ON(chan->private && !dw_dma_filter(chan, chan-
>> > > > > > private));

>> > > > I don't think BUG_ON is apt here, gracefully failing and
>> > > > printing
>> > > > that can
>> > > > be better..

By the way, what level do you want to have warning on?
I consider now between dev_warn() and dev_WARN()...

>> do
>> we have any issues with the rest?
>
> I did look at few more they looked fine, but didn't look at entire
> series

Thanks, got it!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
@ 2016-04-07 13:03               ` Andy Shevchenko
  0 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-07 13:03 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: linux-kernel, robh+dt, vireshk, stable, broonie, mark.rutland,
	dmaengine, gregkh, vgupta, tj, andriy.shevchenko, egtvedt

On Thu, Apr 7, 2016 at 12:09 AM, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Wed, 2016-04-06 at 22:56 +0300, Andy Shevchenko wrote:
>> On Wed, Apr 6, 2016 at 9:56 PM, Vinod Koul <vinod.koul@intel.com>
>> wrote:
>> > On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
>> > > On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
>> > > > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko wrote:

>> > > > > + /*
>> > > > > +  * We need controller-specific data to set up slave
>> > > > > transfers.
>> > > > > +  */
>> > > > > + BUG_ON(chan->private && !dw_dma_filter(chan, chan-
>> > > > > > private));

>> > > > I don't think BUG_ON is apt here, gracefully failing and
>> > > > printing
>> > > > that can
>> > > > be better..

By the way, what level do you want to have warning on?
I consider now between dev_warn() and dev_WARN()...

>> do
>> we have any issues with the rest?
>
> I did look at few more they looked fine, but didn't look at entire
> series

Thanks, got it!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v3.5 1/1] dmaengine: dw: fix master selection
  2016-04-06 18:56         ` Vinod Koul
  (?)
  (?)
@ 2016-04-08 13:22         ` Andy Shevchenko
  -1 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-08 13:22 UTC (permalink / raw)
  To: Viresh Kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	dmaengine, Rob Herring, Hans-Christian Egtvedt, Tejun Heo,
	Mark Brown, Greg Kroah-Hartman, Mark Rutland, Vineet Gupta
  Cc: stable

The commit 895005202987 ("dmaengine: dw: apply both HS interfaces and remove
slave_id usage") cleaned up the code to avoid usage of depricated slave_id
member of generic slave configuration.

Meanwhile it broke the master selection by removing important call to
dwc_set_masters() in ->device_alloc_chan_resources() which copied masters from
custom slave configuration to the internal channel structure.

Everything works until now since there is no customized connection of
DesignWare DMA IP to the bus, i.e. one bus and one or more masters are in use.
The configurations where 2 masters are connected to the different masters are
not working anymore. We are expecting one user of such configuration and need
to select masters properly. Besides that it is obviously a performance
regression since only one master is in use in multi-master configuration.

Select masters in accordance with what user asked for. Keep this patch in a form
more suitable for back porting.

We are safe to take necessary data in ->device_alloc_chan_resources() because
we don't support generic slave configuration embedded into custom one, and thus
the only way to provide such is to use the parameter to a filter function which
is called exactly before channel resource allocation.

While here, replase BUG_ON to less noisy dev_warn() and prevent channel
allocation in case of error.

Fixes: 895005202987 ("dmaengine: dw: apply both HS interfaces and remove slave_id usage")
Cc: stable@vger.kernel.org
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw/core.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 5ad0ec1..97199b3 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -130,26 +130,14 @@ static void dwc_desc_put(struct dw_dma_chan *dwc, struct dw_desc *desc)
 static void dwc_initialize(struct dw_dma_chan *dwc)
 {
 	struct dw_dma *dw = to_dw_dma(dwc->chan.device);
-	struct dw_dma_slave *dws = dwc->chan.private;
 	u32 cfghi = DWC_CFGH_FIFO_MODE;
 	u32 cfglo = DWC_CFGL_CH_PRIOR(dwc->priority);
 
 	if (dwc->initialized == true)
 		return;
 
-	if (dws) {
-		/*
-		 * We need controller-specific data to set up slave
-		 * transfers.
-		 */
-		BUG_ON(!dws->dma_dev || dws->dma_dev != dw->dma.dev);
-
-		cfghi |= DWC_CFGH_DST_PER(dws->dst_id);
-		cfghi |= DWC_CFGH_SRC_PER(dws->src_id);
-	} else {
-		cfghi |= DWC_CFGH_DST_PER(dwc->dst_id);
-		cfghi |= DWC_CFGH_SRC_PER(dwc->src_id);
-	}
+	cfghi |= DWC_CFGH_DST_PER(dwc->dst_id);
+	cfghi |= DWC_CFGH_SRC_PER(dwc->src_id);
 
 	channel_writel(dwc, CFG_LO, cfglo);
 	channel_writel(dwc, CFG_HI, cfghi);
@@ -941,7 +929,7 @@ bool dw_dma_filter(struct dma_chan *chan, void *param)
 	struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
 	struct dw_dma_slave *dws = param;
 
-	if (!dws || dws->dma_dev != chan->device->dev)
+	if (dws->dma_dev != chan->device->dev)
 		return false;
 
 	/* We have to copy data since dws can be temporary storage */
@@ -1165,6 +1153,14 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
 	 * doesn't mean what you think it means), and status writeback.
 	 */
 
+	/*
+	 * We need controller-specific data to set up slave transfers.
+	 */
+	if (chan->private && !dw_dma_filter(chan, chan->private)) {
+		dev_warn(chan2dev(chan), "Wrong controller-specific data\n");
+		return -EINVAL;
+	}
+
 	/* Enable controller here if needed */
 	if (!dw->in_use)
 		dw_dma_on(dw);
@@ -1226,6 +1222,14 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	spin_lock_irqsave(&dwc->lock, flags);
 	list_splice_init(&dwc->free_list, &list);
 	dwc->descs_allocated = 0;
+
+	/* Clear custom channel configuration */
+	dwc->src_id = 0;
+	dwc->dst_id = 0;
+
+	dwc->src_master = 0;
+	dwc->dst_master = 0;
+
 	dwc->initialized = false;
 
 	/* Disable interrupts */
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 01/15] dmaengine: dw: fix master selection
  2016-04-06 21:09             ` Koul, Vinod
  (?)
  (?)
@ 2016-04-13 13:36             ` Andy Shevchenko
  -1 siblings, 0 replies; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-13 13:36 UTC (permalink / raw)
  To: Koul, Vinod, andy.shevchenko
  Cc: linux-kernel, robh+dt, vireshk, stable, broonie, mark.rutland,
	dmaengine, gregkh, vgupta, tj, egtvedt

On Wed, 2016-04-06 at 21:09 +0000, Koul, Vinod wrote:
> On Wed, 2016-04-06 at 22:56 +0300, Andy Shevchenko wrote:
> > 
> > On Wed, Apr 6, 2016 at 9:56 PM, Vinod Koul <vinod.koul@intel.com>
> > wrote:
> > > 
> > > On Mon, Apr 04, 2016 at 08:10:54PM +0300, Andy Shevchenko wrote:
> > > > 
> > > > On Mon, 2016-04-04 at 10:03 -0700, Vinod Koul wrote:
> > > > > 
> > > > > On Fri, Mar 18, 2016 at 04:24:40PM +0200, Andy Shevchenko
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > + /*
> > > > > > +  * We need controller-specific data to set up slave
> > > > > > transfers.
> > > > > > +  */
> > > > > > + BUG_ON(chan->private && !dw_dma_filter(chan, chan-
> > > > > > > 
> > > > > > > private));
> > > > > I don't think BUG_ON is apt here, gracefully failing and
> > > > > printing
> > > > > that can
> > > > > be better..
> > > > Hm, this is coming from the existing code.
> > > > 
> > > > I would prefer to keep this one as is since it's targeted to
> > > > stable@,
> > > > and I may issue sequential patch to replace BUG_ON. Sounds okay?
> > > But since this is going stable, how about we remove here in the
> > > patch and
> > > benefit stable tree as well?
> > I'm fine with that. I will send you patch tomorrow.Other than that
> > do
> > we have any issues with the rest?
> I did look at few more they looked fine, but didn't look at entire
> series

I've sent an update to patch 1 with "PATCH v3.5" in the subject.

Anything else should I do?

At least Intel Quark UART series (published) and SATA DWC 460ex (not
published in mailing list yet) are relying on this. It would be nice to
have immutable branch as soon as everything okay with the series.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-03-18 14:24 ` [PATCH v3 08/15] dmaengine: dw: revisit data_width property Andy Shevchenko
@ 2016-04-13 15:57   ` Vinod Koul
  2016-04-13 16:05     ` Andy Shevchenko
  0 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2016-04-13 15:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

On Fri, Mar 18, 2016 at 04:24:47PM +0200, Andy Shevchenko wrote:
> There are several changes are done here:
> 
>  - Convert the property to be in bytes
> 
>    Much more convenient than keeping encoded value.
> 
>  - Use one value for all AHB masters for now
> 
>    It seems in practice we have no controllers where masters have different
>    data bus width, we still might return to distinct values when there is a use
>    case.
> 
>  - Rename data_width to data-width in the device tree bindings.
> 
>  - While here, replace dwc_fast_ffs() by __ffs().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  Documentation/devicetree/bindings/dma/snps-dma.txt |  5 ++-
>  arch/arc/boot/dts/abilis_tb10x.dtsi                |  2 +-
>  arch/arm/boot/dts/spear13xx.dtsi                   |  4 +--
>  drivers/dma/dw/core.c                              | 40 +++-------------------
>  drivers/dma/dw/platform.c                          | 10 +++---
>  drivers/dma/dw/regs.h                              |  2 +-
>  include/linux/platform_data/dma-dw.h               |  5 ++-
>  7 files changed, 18 insertions(+), 50 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
> index c99c1ff..eb48229 100644
> --- a/Documentation/devicetree/bindings/dma/snps-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
> @@ -13,8 +13,7 @@ Required properties:
>  - chan_priority: priority of channels. 0 (default): increase from chan 0->n, 1:
>    increase from chan n->0
>  - block_size: Maximum block size supported by the controller
> -- data_width: Maximum data width supported by hardware per AHB master
> -  (0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
> +- data-width: Maximum data width supported by hardware (in bytes, power of 2)

You missed that binding are treated as ABI, see
Documentation/devicetree/bindings/ABI.txt

The driver should still work with older DT implementation, so you need to
keep support for that in driver and hence I don't see any benfit we would
get from doing both in driver!

>  
>  
>  Optional properties:
> @@ -38,7 +37,7 @@ Example:
>  		chan_allocation_order = <1>;
>  		chan_priority = <1>;
>  		block_size = <0xfff>;
> -		data_width = <3 3>;
> +		data-width = <8>;
>  	};
>  
>  DMA clients connected to the Designware DMA controller must use the format
> diff --git a/arch/arc/boot/dts/abilis_tb10x.dtsi b/arch/arc/boot/dts/abilis_tb10x.dtsi
> index cfb5052..2f53bed 100644
> --- a/arch/arc/boot/dts/abilis_tb10x.dtsi
> +++ b/arch/arc/boot/dts/abilis_tb10x.dtsi
> @@ -112,7 +112,7 @@
>  			chan_allocation_order = <0>;
>  			chan_priority = <1>;
>  			block_size = <0x7ff>;
> -			data_width = <2>;
> +			data-width = <4>;
>  			clocks = <&ahb_clk>;
>  			clock-names = "hclk";
>  		};
> diff --git a/arch/arm/boot/dts/spear13xx.dtsi b/arch/arm/boot/dts/spear13xx.dtsi
> index 14594ce..474b66f 100644
> --- a/arch/arm/boot/dts/spear13xx.dtsi
> +++ b/arch/arm/boot/dts/spear13xx.dtsi
> @@ -117,7 +117,7 @@
>  			chan_priority = <1>;
>  			block_size = <0xfff>;
>  			dma-masters = <2>;
> -			data_width = <3 3>;
> +			data-width = <8>;
>  		};
>  
>  		dma@eb000000 {
> @@ -133,7 +133,7 @@
>  			chan_allocation_order = <1>;
>  			chan_priority = <1>;
>  			block_size = <0xfff>;
> -			data_width = <3 3>;
> +			data-width = <8>;
>  		};
>  
>  		fsmc: flash@b0000000 {
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index 4748df9..79dbfc3 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -155,21 +155,6 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
>  
>  /*----------------------------------------------------------------------*/
>  
> -static inline unsigned int dwc_fast_ffs(unsigned long long v)
> -{
> -	/*
> -	 * We can be a lot more clever here, but this should take care
> -	 * of the most common optimization.
> -	 */
> -	if (!(v & 7))
> -		return 3;
> -	else if (!(v & 3))
> -		return 2;
> -	else if (!(v & 1))
> -		return 1;
> -	return 0;
> -}
> -
>  static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc)
>  {
>  	dev_err(chan2dev(&dwc->chan),
> @@ -703,7 +688,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>  	size_t			offset;
>  	unsigned int		src_width;
>  	unsigned int		dst_width;
> -	unsigned int		data_width;
>  	u32			ctllo;
>  	u8			lms = DWC_LLP_LMS(dwc->m_master);
>  
> @@ -718,10 +702,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>  
>  	dwc->direction = DMA_MEM_TO_MEM;
>  
> -	data_width = dw->data_width[dwc->m_master];
> -
> -	src_width = dst_width = min_t(unsigned int, data_width,
> -				      dwc_fast_ffs(src | dest | len));
> +	src_width = dst_width = __ffs(dw->data_width | src | dest | len);
>  
>  	ctllo = DWC_DEFAULT_CTLLO(chan)
>  			| DWC_CTLL_DST_WIDTH(dst_width)
> @@ -785,7 +766,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  	dma_addr_t		reg;
>  	unsigned int		reg_width;
>  	unsigned int		mem_width;
> -	unsigned int		data_width;
>  	unsigned int		i;
>  	struct scatterlist	*sg;
>  	size_t			total_len = 0;
> @@ -811,8 +791,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  		ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
>  			DWC_CTLL_FC(DW_DMA_FC_D_M2P);
>  
> -		data_width = dw->data_width[dwc->m_master];
> -
>  		for_each_sg(sgl, sg, sg_len, i) {
>  			struct dw_desc	*desc;
>  			u32		len, dlen, mem;
> @@ -820,8 +798,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  			mem = sg_dma_address(sg);
>  			len = sg_dma_len(sg);
>  
> -			mem_width = min_t(unsigned int,
> -					  data_width, dwc_fast_ffs(mem | len));
> +			mem_width = __ffs(dw->data_width | mem | len);
>  
>  slave_sg_todev_fill_desc:
>  			desc = dwc_desc_get(dwc);
> @@ -867,8 +844,6 @@ slave_sg_todev_fill_desc:
>  		ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
>  			DWC_CTLL_FC(DW_DMA_FC_D_P2M);
>  
> -		data_width = dw->data_width[dwc->m_master];
> -
>  		for_each_sg(sgl, sg, sg_len, i) {
>  			struct dw_desc	*desc;
>  			u32		len, dlen, mem;
> @@ -876,8 +851,7 @@ slave_sg_todev_fill_desc:
>  			mem = sg_dma_address(sg);
>  			len = sg_dma_len(sg);
>  
> -			mem_width = min_t(unsigned int,
> -					  data_width, dwc_fast_ffs(mem | len));
> +			mem_width = __ffs(dw->data_width | mem | len);
>  
>  slave_sg_fromdev_fill_desc:
>  			desc = dwc_desc_get(dwc);
> @@ -1544,10 +1518,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
>  		/* Get hardware configuration parameters */
>  		pdata->nr_channels = (dw_params >> DW_PARAMS_NR_CHAN & 7) + 1;
>  		pdata->nr_masters = (dw_params >> DW_PARAMS_NR_MASTER & 3) + 1;
> -		for (i = 0; i < pdata->nr_masters; i++) {
> -			pdata->data_width[i] =
> -				(dw_params >> DW_PARAMS_DATA_WIDTH(i) & 3) + 2;
> -		}
> +		pdata->data_width = 4 << (dw_params >> DW_PARAMS_DATA_WIDTH(0) & 3);
>  		max_blk_size = dma_readl(dw, MAX_BLK_SIZE);
>  
>  		/* Fill platform data with the default values */
> @@ -1569,8 +1540,7 @@ int dw_dma_probe(struct dw_dma_chip *chip, struct dw_dma_platform_data *pdata)
>  
>  	/* Get hardware configuration parameters */
>  	dw->nr_masters = pdata->nr_masters;
> -	for (i = 0; i < dw->nr_masters; i++)
> -		dw->data_width[i] = pdata->data_width[i];
> +	dw->data_width = pdata->data_width;
>  
>  	/* Calculate all channel mask before DMA setup */
>  	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
> diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> index 23616c5..4cf399d 100644
> --- a/drivers/dma/dw/platform.c
> +++ b/drivers/dma/dw/platform.c
> @@ -102,8 +102,8 @@ dw_dma_parse_dt(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
>  	struct dw_dma_platform_data *pdata;
> -	u32 tmp, arr[DW_DMA_MAX_NR_MASTERS];
>  	u32 nr_channels;
> +	u32 tmp;
>  
>  	if (!np) {
>  		dev_err(&pdev->dev, "Missing DT data\n");
> @@ -138,10 +138,10 @@ dw_dma_parse_dt(struct platform_device *pdev)
>  		pdata->nr_masters = tmp;
>  	}
>  
> -	if (!of_property_read_u32_array(np, "data_width", arr,
> -				pdata->nr_masters))
> -		for (tmp = 0; tmp < pdata->nr_masters; tmp++)
> -			pdata->data_width[tmp] = arr[tmp];
> +	if (!of_property_read_u32(np, "data-width", &tmp))
> +		pdata->data_width = tmp;
> +	else if (!of_property_read_u32(np, "data_width", &tmp))
> +		pdata->data_width = BIT(tmp & 0x07);
>  
>  	return pdata;
>  }
> diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
> index feb3a4a..d0f9173 100644
> --- a/drivers/dma/dw/regs.h
> +++ b/drivers/dma/dw/regs.h
> @@ -285,7 +285,7 @@ struct dw_dma {
>  
>  	/* hardware configuration */
>  	unsigned char		nr_masters;
> -	unsigned char		data_width[DW_DMA_MAX_NR_MASTERS];
> +	unsigned char		data_width;
>  };
>  
>  static inline struct dw_dma_regs __iomem *__dw_regs(struct dw_dma *dw)
> diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
> index b881b97..09cf5c1 100644
> --- a/include/linux/platform_data/dma-dw.h
> +++ b/include/linux/platform_data/dma-dw.h
> @@ -42,8 +42,7 @@ struct dw_dma_slave {
>   * @chan_priority: Set channel priority increasing from 0 to 7 or 7 to 0.
>   * @block_size: Maximum block size supported by the controller
>   * @nr_masters: Number of AHB masters supported by the controller
> - * @data_width: Maximum data width supported by hardware per AHB master
> - *		(0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
> + * @data_width: Maximum data width supported by hardware (in bytes, power of 2)
>   */
>  struct dw_dma_platform_data {
>  	unsigned int	nr_channels;
> @@ -57,7 +56,7 @@ struct dw_dma_platform_data {
>  	unsigned char	chan_priority;
>  	unsigned short	block_size;
>  	unsigned char	nr_masters;
> -	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
> +	unsigned char	data_width;
>  };
>  
>  #endif /* _PLATFORM_DATA_DMA_DW_H */
> -- 
> 2.7.0
> 

-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-04-13 15:57   ` Vinod Koul
@ 2016-04-13 16:05     ` Andy Shevchenko
  2016-04-13 16:17       ` Vinod Koul
  0 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-13 16:05 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

On Wed, 2016-04-13 at 21:27 +0530, Vinod Koul wrote:
> On Fri, Mar 18, 2016 at 04:24:47PM +0200, Andy Shevchenko wrote:
> > 
> > There are several changes are done here:
> > 
> >  - Convert the property to be in bytes
> > 
> >    Much more convenient than keeping encoded value.
> > 
> >  - Use one value for all AHB masters for now
> > 
> >    It seems in practice we have no controllers where masters have
> > different
> >    data bus width, we still might return to distinct values when
> > there is a use
> >    case.
> > 
> >  - Rename data_width to data-width in the device tree bindings.
> > 
> >  - While here, replace dwc_fast_ffs() by __ffs().
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  Documentation/devicetree/bindings/dma/snps-dma.txt |  5 ++-
> >  arch/arc/boot/dts/abilis_tb10x.dtsi                |  2 +-
> >  arch/arm/boot/dts/spear13xx.dtsi                   |  4 +--
> >  drivers/dma/dw/core.c                              | 40 +++------
> > -------------
> >  drivers/dma/dw/platform.c                          | 10 +++---
> >  drivers/dma/dw/regs.h                              |  2 +-
> >  include/linux/platform_data/dma-dw.h               |  5 ++-
> >  7 files changed, 18 insertions(+), 50 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt
> > b/Documentation/devicetree/bindings/dma/snps-dma.txt
> > index c99c1ff..eb48229 100644
> > --- a/Documentation/devicetree/bindings/dma/snps-dma.txt
> > +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
> > @@ -13,8 +13,7 @@ Required properties:
> >  - chan_priority: priority of channels. 0 (default): increase from
> > chan 0->n, 1:
> >    increase from chan n->0
> >  - block_size: Maximum block size supported by the controller
> > -- data_width: Maximum data width supported by hardware per AHB
> > master
> > -  (0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
> > +- data-width: Maximum data width supported by hardware (in bytes,
> > power of 2)
> You missed that binding are treated as ABI, see
> Documentation/devicetree/bindings/ABI.txt
> 
> The driver should still work with older DT implementation, so you need
> to
> keep support for that in driver and hence I don't see any benfit we
> would
> get from doing both in driver!

The device tree is screwed by a process that allows to do almost
whatever you want. Here I'm trying to rectify the usage of the field.

The old is still supported and benefit is apparently in unifying
standard properties across the drivers.

> 
> > 
> >  
> >  
> >  Optional properties:
> > @@ -38,7 +37,7 @@ Example:
> >  		chan_allocation_order = <1>;
> >  		chan_priority = <1>;
> >  		block_size = <0xfff>;
> > -		data_width = <3 3>;
> > +		data-width = <8>;
> >  	};
> >  
> >  DMA clients connected to the Designware DMA controller must use the
> > format
> > diff --git a/arch/arc/boot/dts/abilis_tb10x.dtsi
> > b/arch/arc/boot/dts/abilis_tb10x.dtsi
> > index cfb5052..2f53bed 100644
> > --- a/arch/arc/boot/dts/abilis_tb10x.dtsi
> > +++ b/arch/arc/boot/dts/abilis_tb10x.dtsi
> > @@ -112,7 +112,7 @@
> >  			chan_allocation_order = <0>;
> >  			chan_priority = <1>;
> >  			block_size = <0x7ff>;
> > -			data_width = <2>;
> > +			data-width = <4>;
> >  			clocks = <&ahb_clk>;
> >  			clock-names = "hclk";
> >  		};
> > diff --git a/arch/arm/boot/dts/spear13xx.dtsi
> > b/arch/arm/boot/dts/spear13xx.dtsi
> > index 14594ce..474b66f 100644
> > --- a/arch/arm/boot/dts/spear13xx.dtsi
> > +++ b/arch/arm/boot/dts/spear13xx.dtsi
> > @@ -117,7 +117,7 @@
> >  			chan_priority = <1>;
> >  			block_size = <0xfff>;
> >  			dma-masters = <2>;
> > -			data_width = <3 3>;
> > +			data-width = <8>;
> >  		};
> >  
> >  		dma@eb000000 {
> > @@ -133,7 +133,7 @@
> >  			chan_allocation_order = <1>;
> >  			chan_priority = <1>;
> >  			block_size = <0xfff>;
> > -			data_width = <3 3>;
> > +			data-width = <8>;
> >  		};
> >  
> >  		fsmc: flash@b0000000 {
> > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> > index 4748df9..79dbfc3 100644
> > --- a/drivers/dma/dw/core.c
> > +++ b/drivers/dma/dw/core.c
> > @@ -155,21 +155,6 @@ static void dwc_initialize(struct dw_dma_chan
> > *dwc)
> >  
> >  /*-----------------------------------------------------------------
> > -----*/
> >  
> > -static inline unsigned int dwc_fast_ffs(unsigned long long v)
> > -{
> > -	/*
> > -	 * We can be a lot more clever here, but this should take
> > care
> > -	 * of the most common optimization.
> > -	 */
> > -	if (!(v & 7))
> > -		return 3;
> > -	else if (!(v & 3))
> > -		return 2;
> > -	else if (!(v & 1))
> > -		return 1;
> > -	return 0;
> > -}
> > -
> >  static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc)
> >  {
> >  	dev_err(chan2dev(&dwc->chan),
> > @@ -703,7 +688,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan,
> > dma_addr_t dest, dma_addr_t src,
> >  	size_t			offset;
> >  	unsigned int		src_width;
> >  	unsigned int		dst_width;
> > -	unsigned int		data_width;
> >  	u32			ctllo;
> >  	u8			lms = DWC_LLP_LMS(dwc->m_master);
> >  
> > @@ -718,10 +702,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan,
> > dma_addr_t dest, dma_addr_t src,
> >  
> >  	dwc->direction = DMA_MEM_TO_MEM;
> >  
> > -	data_width = dw->data_width[dwc->m_master];
> > -
> > -	src_width = dst_width = min_t(unsigned int, data_width,
> > -				      dwc_fast_ffs(src | dest |
> > len));
> > +	src_width = dst_width = __ffs(dw->data_width | src | dest |
> > len);
> >  
> >  	ctllo = DWC_DEFAULT_CTLLO(chan)
> >  			| DWC_CTLL_DST_WIDTH(dst_width)
> > @@ -785,7 +766,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct
> > scatterlist *sgl,
> >  	dma_addr_t		reg;
> >  	unsigned int		reg_width;
> >  	unsigned int		mem_width;
> > -	unsigned int		data_width;
> >  	unsigned int		i;
> >  	struct scatterlist	*sg;
> >  	size_t			total_len = 0;
> > @@ -811,8 +791,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct
> > scatterlist *sgl,
> >  		ctllo |= sconfig->device_fc ?
> > DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
> >  			DWC_CTLL_FC(DW_DMA_FC_D_M2P);
> >  
> > -		data_width = dw->data_width[dwc->m_master];
> > -
> >  		for_each_sg(sgl, sg, sg_len, i) {
> >  			struct dw_desc	*desc;
> >  			u32		len, dlen, mem;
> > @@ -820,8 +798,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct
> > scatterlist *sgl,
> >  			mem = sg_dma_address(sg);
> >  			len = sg_dma_len(sg);
> >  
> > -			mem_width = min_t(unsigned int,
> > -					  data_width,
> > dwc_fast_ffs(mem | len));
> > +			mem_width = __ffs(dw->data_width | mem |
> > len);
> >  
> >  slave_sg_todev_fill_desc:
> >  			desc = dwc_desc_get(dwc);
> > @@ -867,8 +844,6 @@ slave_sg_todev_fill_desc:
> >  		ctllo |= sconfig->device_fc ?
> > DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
> >  			DWC_CTLL_FC(DW_DMA_FC_D_P2M);
> >  
> > -		data_width = dw->data_width[dwc->m_master];
> > -
> >  		for_each_sg(sgl, sg, sg_len, i) {
> >  			struct dw_desc	*desc;
> >  			u32		len, dlen, mem;
> > @@ -876,8 +851,7 @@ slave_sg_todev_fill_desc:
> >  			mem = sg_dma_address(sg);
> >  			len = sg_dma_len(sg);
> >  
> > -			mem_width = min_t(unsigned int,
> > -					  data_width,
> > dwc_fast_ffs(mem | len));
> > +			mem_width = __ffs(dw->data_width | mem |
> > len);
> >  
> >  slave_sg_fromdev_fill_desc:
> >  			desc = dwc_desc_get(dwc);
> > @@ -1544,10 +1518,7 @@ int dw_dma_probe(struct dw_dma_chip *chip,
> > struct dw_dma_platform_data *pdata)
> >  		/* Get hardware configuration parameters */
> >  		pdata->nr_channels = (dw_params >>
> > DW_PARAMS_NR_CHAN & 7) + 1;
> >  		pdata->nr_masters = (dw_params >>
> > DW_PARAMS_NR_MASTER & 3) + 1;
> > -		for (i = 0; i < pdata->nr_masters; i++) {
> > -			pdata->data_width[i] =
> > -				(dw_params >>
> > DW_PARAMS_DATA_WIDTH(i) & 3) + 2;
> > -		}
> > +		pdata->data_width = 4 << (dw_params >>
> > DW_PARAMS_DATA_WIDTH(0) & 3);
> >  		max_blk_size = dma_readl(dw, MAX_BLK_SIZE);
> >  
> >  		/* Fill platform data with the default values */
> > @@ -1569,8 +1540,7 @@ int dw_dma_probe(struct dw_dma_chip *chip,
> > struct dw_dma_platform_data *pdata)
> >  
> >  	/* Get hardware configuration parameters */
> >  	dw->nr_masters = pdata->nr_masters;
> > -	for (i = 0; i < dw->nr_masters; i++)
> > -		dw->data_width[i] = pdata->data_width[i];
> > +	dw->data_width = pdata->data_width;
> >  
> >  	/* Calculate all channel mask before DMA setup */
> >  	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
> > diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> > index 23616c5..4cf399d 100644
> > --- a/drivers/dma/dw/platform.c
> > +++ b/drivers/dma/dw/platform.c
> > @@ -102,8 +102,8 @@ dw_dma_parse_dt(struct platform_device *pdev)
> >  {
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct dw_dma_platform_data *pdata;
> > -	u32 tmp, arr[DW_DMA_MAX_NR_MASTERS];
> >  	u32 nr_channels;
> > +	u32 tmp;
> >  
> >  	if (!np) {
> >  		dev_err(&pdev->dev, "Missing DT data\n");
> > @@ -138,10 +138,10 @@ dw_dma_parse_dt(struct platform_device *pdev)
> >  		pdata->nr_masters = tmp;
> >  	}
> >  
> > -	if (!of_property_read_u32_array(np, "data_width", arr,
> > -				pdata->nr_masters))
> > -		for (tmp = 0; tmp < pdata->nr_masters; tmp++)
> > -			pdata->data_width[tmp] = arr[tmp];
> > +	if (!of_property_read_u32(np, "data-width", &tmp))
> > +		pdata->data_width = tmp;
> > +	else if (!of_property_read_u32(np, "data_width", &tmp))
> > +		pdata->data_width = BIT(tmp & 0x07);
> >  
> >  	return pdata;
> >  }
> > diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
> > index feb3a4a..d0f9173 100644
> > --- a/drivers/dma/dw/regs.h
> > +++ b/drivers/dma/dw/regs.h
> > @@ -285,7 +285,7 @@ struct dw_dma {
> >  
> >  	/* hardware configuration */
> >  	unsigned char		nr_masters;
> > -	unsigned char		data_width[DW_DMA_MAX_NR_MASTE
> > RS];
> > +	unsigned char		data_width;
> >  };
> >  
> >  static inline struct dw_dma_regs __iomem *__dw_regs(struct dw_dma
> > *dw)
> > diff --git a/include/linux/platform_data/dma-dw.h
> > b/include/linux/platform_data/dma-dw.h
> > index b881b97..09cf5c1 100644
> > --- a/include/linux/platform_data/dma-dw.h
> > +++ b/include/linux/platform_data/dma-dw.h
> > @@ -42,8 +42,7 @@ struct dw_dma_slave {
> >   * @chan_priority: Set channel priority increasing from 0 to 7 or 7
> > to 0.
> >   * @block_size: Maximum block size supported by the controller
> >   * @nr_masters: Number of AHB masters supported by the controller
> > - * @data_width: Maximum data width supported by hardware per AHB
> > master
> > - *		(0 - 8bits, 1 - 16bits, ..., 5 - 256bits)
> > + * @data_width: Maximum data width supported by hardware (in bytes,
> > power of 2)
> >   */
> >  struct dw_dma_platform_data {
> >  	unsigned int	nr_channels;
> > @@ -57,7 +56,7 @@ struct dw_dma_platform_data {
> >  	unsigned char	chan_priority;
> >  	unsigned short	block_size;
> >  	unsigned char	nr_masters;
> > -	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
> > +	unsigned char	data_width;
> >  };
> >  
> >  #endif /* _PLATFORM_DATA_DMA_DW_H */
> > -- 
> > 2.7.0
> > 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems)
  2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
                   ` (14 preceding siblings ...)
  2016-03-18 14:24 ` [PATCH v3 15/15] dmaengine: dw: set cdesc to NULL when free cyclic transfers Andy Shevchenko
@ 2016-04-13 16:08 ` Vinod Koul
  15 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2016-04-13 16:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

On Fri, Mar 18, 2016 at 04:24:39PM +0200, Andy Shevchenko wrote:
> This patch series (v2: http://www.spinics.net/lists/dmaengine/msg08274.html)
> contains a number of mostly minor fixes and cleanups for the DW DMA driver. A
> couple of them affect the DT binding so these may need to be updated to
> maintain compatibility. The rest should be relatively straight-forward.

Applied 1 thru 6, 8, 9 didnt, and then all after 10th or so.

> Vinod, there are few patch sets developed on top of this one, so, the idea is
> to keep this in an immuutable branch / tag.

Sure, or we can push the other dw patches here too and Greg uses that for
serial ones..

Thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-04-13 16:05     ` Andy Shevchenko
@ 2016-04-13 16:17       ` Vinod Koul
  2016-04-13 16:21         ` Andy Shevchenko
  0 siblings, 1 reply; 35+ messages in thread
From: Vinod Koul @ 2016-04-13 16:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

On Wed, Apr 13, 2016 at 07:05:48PM +0300, Andy Shevchenko wrote:
> On Wed, 2016-04-13 at 21:27 +0530, Vinod Koul wrote:
> > On Fri, Mar 18, 2016 at 04:24:47PM +0200, Andy Shevchenko wrote:
> > The driver should still work with older DT implementation, so you need
> > to
> > keep support for that in driver and hence I don't see any benfit we
> > would
> > get from doing both in driver!
> 
> The device tree is screwed by a process that allows to do almost
> whatever you want. Here I'm trying to rectify the usage of the field.

Well at least we should try to do the right thing which means pushing back
on ABI breakage..

> The old is still supported and benefit is apparently in unifying
> standard properties across the drivers.

Hrmmm how is that?

> > > @@ -102,8 +102,8 @@ dw_dma_parse_dt(struct platform_device *pdev)
> > >  {
> > >  	struct device_node *np = pdev->dev.of_node;
> > >  	struct dw_dma_platform_data *pdata;
> > > -	u32 tmp, arr[DW_DMA_MAX_NR_MASTERS];
> > >  	u32 nr_channels;
> > > +	u32 tmp;
> > >  
> > >  	if (!np) {
> > >  		dev_err(&pdev->dev, "Missing DT data\n");
> > > @@ -138,10 +138,10 @@ dw_dma_parse_dt(struct platform_device *pdev)
> > >  		pdata->nr_masters = tmp;
> > >  	}
> > >  
> > > -	if (!of_property_read_u32_array(np, "data_width", arr,
> > > -				pdata->nr_masters))
> > > -		for (tmp = 0; tmp < pdata->nr_masters; tmp++)
> > > -			pdata->data_width[tmp] = arr[tmp];

You stop reading the array

> > > +	if (!of_property_read_u32(np, "data-width", &tmp))
> > > +		pdata->data_width = tmp;
> > > +	else if (!of_property_read_u32(np, "data_width", &tmp))
> > > +		pdata->data_width = BIT(tmp & 0x07);

And read a value? how will this work with older array?

-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-04-13 16:17       ` Vinod Koul
@ 2016-04-13 16:21         ` Andy Shevchenko
  2016-04-13 16:40           ` Mark Brown
  0 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-13 16:21 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Mark Brown,
	Greg Kroah-Hartman, Mark Rutland, Vineet Gupta

On Wed, 2016-04-13 at 21:47 +0530, Vinod Koul wrote:
> On Wed, Apr 13, 2016 at 07:05:48PM +0300, Andy Shevchenko wrote:
> > 
> > On Wed, 2016-04-13 at 21:27 +0530, Vinod Koul wrote:
> > > 
> > > On Fri, Mar 18, 2016 at 04:24:47PM +0200, Andy Shevchenko wrote:
> > > The driver should still work with older DT implementation, so you
> > > need
> > > to
> > > keep support for that in driver and hence I don't see any benfit
> > > we
> > > would
> > > get from doing both in driver!
> > The device tree is screwed by a process that allows to do almost
> > whatever you want. Here I'm trying to rectify the usage of the
> > field.
> Well at least we should try to do the right thing which means pushing
> back
> on ABI breakage..

There is no such.

> 
> > 
> > The old is still supported and benefit is apparently in unifying
> > standard properties across the drivers.
> Hrmmm how is that?

The common usage for data-width property is "in bytes". And I like the
idea. I don't know why at all I chose to keep encoded value there in the
first place and no one commented at that time. I suppose because of
screwed device tree process. I think now it's better to follow some
standard / registered properties in new drivers.

> 
> > 
> > > 
> > > > 
> > > > @@ -102,8 +102,8 @@ dw_dma_parse_dt(struct platform_device
> > > > *pdev)
> > > >  {
> > > >  	struct device_node *np = pdev->dev.of_node;
> > > >  	struct dw_dma_platform_data *pdata;
> > > > -	u32 tmp, arr[DW_DMA_MAX_NR_MASTERS];
> > > >  	u32 nr_channels;
> > > > +	u32 tmp;
> > > >  
> > > >  	if (!np) {
> > > >  		dev_err(&pdev->dev, "Missing DT data\n");
> > > > @@ -138,10 +138,10 @@ dw_dma_parse_dt(struct platform_device
> > > > *pdev)
> > > >  		pdata->nr_masters = tmp;
> > > >  	}
> > > >  
> > > > -	if (!of_property_read_u32_array(np, "data_width", arr,
> > > > -				pdata->nr_masters))
> > > > -		for (tmp = 0; tmp < pdata->nr_masters; tmp++)
> > > > -			pdata->data_width[tmp] = arr[tmp];
> You stop reading the array

Yes, due to "- Use one value for all AHB masters for now". No reason to
read all of them.

> > > > +	if (!of_property_read_u32(np, "data-width", &tmp))
> > > > +		pdata->data_width = tmp;
> > > > +	else if (!of_property_read_u32(np, "data_width", &tmp))
> > > > +		pdata->data_width = BIT(tmp & 0x07);
> And read a value? how will this work with older array?

It will read first element.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-04-13 16:21         ` Andy Shevchenko
@ 2016-04-13 16:40           ` Mark Brown
  2016-04-15 12:45             ` Andy Shevchenko
  0 siblings, 1 reply; 35+ messages in thread
From: Mark Brown @ 2016-04-13 16:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Vinod Koul, Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Greg Kroah-Hartman,
	Mark Rutland, Vineet Gupta

[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]

On Wed, Apr 13, 2016 at 07:21:53PM +0300, Andy Shevchenko wrote:
> On Wed, 2016-04-13 at 21:47 +0530, Vinod Koul wrote:
> > On Wed, Apr 13, 2016 at 07:05:48PM +0300, Andy Shevchenko wrote:

> > > The old is still supported and benefit is apparently in unifying
> > > standard properties across the drivers.

> > Hrmmm how is that?

> The common usage for data-width property is "in bytes". And I like the
> idea. I don't know why at all I chose to keep encoded value there in the
> first place and no one commented at that time. I suppose because of
> screwed device tree process. I think now it's better to follow some
> standard / registered properties in new drivers.

You're unfortunately still breaking compatibility with existing DTs
using this property.  Now, it does appear that there is very little use
of this DMA controller on DT systems and judging by the somewhat odd
compatible string and in tree DTs most of those are legacy so perhaps
this isn't the end of the world but this isn't something that should be
dismissed as a simple cleanup.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-04-13 16:40           ` Mark Brown
@ 2016-04-15 12:45             ` Andy Shevchenko
  2016-04-16  5:45               ` Vinod Koul
  0 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2016-04-15 12:45 UTC (permalink / raw)
  To: Mark Brown
  Cc: Vinod Koul, Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Greg Kroah-Hartman,
	Mark Rutland, Vineet Gupta

On Wed, 2016-04-13 at 17:40 +0100, Mark Brown wrote:
> On Wed, Apr 13, 2016 at 07:21:53PM +0300, Andy Shevchenko wrote:
> > 
> > On Wed, 2016-04-13 at 21:47 +0530, Vinod Koul wrote:
> > > 
> > > On Wed, Apr 13, 2016 at 07:05:48PM +0300, Andy Shevchenko wrote:
> > 
> > > 
> > > > 
> > > > The old is still supported and benefit is apparently in unifying
> > > > standard properties across the drivers.
> > 
> > > 
> > > Hrmmm how is that?
> > 
> > The common usage for data-width property is "in bytes". And I like
> > the
> > idea. I don't know why at all I chose to keep encoded value there in
> > the
> > first place and no one commented at that time. I suppose because of
> > screwed device tree process. I think now it's better to follow some
> > standard / registered properties in new drivers.
> You're unfortunately still breaking compatibility with existing DTs
> using this property.  Now, it does appear that there is very little
> use
> of this DMA controller on DT systems and judging by the somewhat odd
> compatible string and in tree DTs most of those are legacy so perhaps
> this isn't the end of the world but this isn't something that should
> be
> dismissed as a simple cleanup.

Well, does everyone agree that keeping data-width a) with dash in the
name and b) in bytes is good approach?

I will keep an array and support for old encoded property though.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 08/15] dmaengine: dw: revisit data_width property
  2016-04-15 12:45             ` Andy Shevchenko
@ 2016-04-16  5:45               ` Vinod Koul
  0 siblings, 0 replies; 35+ messages in thread
From: Vinod Koul @ 2016-04-16  5:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mark Brown, Viresh Kumar, linux-kernel, dmaengine, Rob Herring,
	Hans-Christian Egtvedt, Tejun Heo, Greg Kroah-Hartman,
	Mark Rutland, Vineet Gupta

On Fri, Apr 15, 2016 at 03:45:34PM +0300, Andy Shevchenko wrote:
> On Wed, 2016-04-13 at 17:40 +0100, Mark Brown wrote:
> > On Wed, Apr 13, 2016 at 07:21:53PM +0300, Andy Shevchenko wrote:
> > > 
> > > On Wed, 2016-04-13 at 21:47 +0530, Vinod Koul wrote:
> > > > 
> > > > On Wed, Apr 13, 2016 at 07:05:48PM +0300, Andy Shevchenko wrote:
> > > 
> > > > 
> > > > > 
> > > > > The old is still supported and benefit is apparently in unifying
> > > > > standard properties across the drivers.
> > > 
> > > > 
> > > > Hrmmm how is that?
> > > 
> > > The common usage for data-width property is "in bytes". And I like
> > > the
> > > idea. I don't know why at all I chose to keep encoded value there in
> > > the
> > > first place and no one commented at that time. I suppose because of
> > > screwed device tree process. I think now it's better to follow some
> > > standard / registered properties in new drivers.
> > You're unfortunately still breaking compatibility with existing DTs
> > using this property.  Now, it does appear that there is very little
> > use
> > of this DMA controller on DT systems and judging by the somewhat odd
> > compatible string and in tree DTs most of those are legacy so perhaps
> > this isn't the end of the world but this isn't something that should
> > be
> > dismissed as a simple cleanup.
> 
> Well, does everyone agree that keeping data-width a) with dash in the
> name and b) in bytes is good approach?
> 
> I will keep an array and support for old encoded property though.

That would be preferred

Thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2016-04-16  5:40 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-18 14:24 [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 01/15] dmaengine: dw: fix master selection Andy Shevchenko
2016-04-04 17:03   ` Vinod Koul
2016-04-04 17:10     ` Andy Shevchenko
2016-04-06 18:56       ` Vinod Koul
2016-04-06 18:56         ` Vinod Koul
2016-04-06 19:56         ` Andy Shevchenko
2016-04-06 21:09           ` Koul, Vinod
2016-04-06 21:09             ` Koul, Vinod
2016-04-07 13:03             ` Andy Shevchenko
2016-04-07 13:03               ` Andy Shevchenko
2016-04-13 13:36             ` Andy Shevchenko
2016-04-08 13:22         ` [PATCH v3.5 1/1] " Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 02/15] dmaengine: dw: rename masters to reflect actual topology Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 03/15] dmaengine: dw: set src and dst master select according to xfer direction Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 04/15] dmaengine: dw: fix byte order of hw descriptor fields Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 05/15] dmaengine: dw: set LMS field in descriptors Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 06/15] dmaengine: dw: clear LLP_[SD]_EN bits in last descriptor of a chain Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 07/15] dmaengine: dw: substitute dma_read_byaddr by dma_readl_native Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 08/15] dmaengine: dw: revisit data_width property Andy Shevchenko
2016-04-13 15:57   ` Vinod Koul
2016-04-13 16:05     ` Andy Shevchenko
2016-04-13 16:17       ` Vinod Koul
2016-04-13 16:21         ` Andy Shevchenko
2016-04-13 16:40           ` Mark Brown
2016-04-15 12:45             ` Andy Shevchenko
2016-04-16  5:45               ` Vinod Koul
2016-03-18 14:24 ` [PATCH v3 09/15] dmaengine: dw: define counter variables as unsigned int Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 10/15] dmaengine: dw: keep entire platform data in struct dw_dma Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 11/15] dmaengine: dw: pass platform data via struct dw_dma_chip Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 12/15] dmaengine: dw: move dwc->paused to dwc->flags Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 13/15] dmaengine: dw: move dwc->initialized " Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 14/15] dmaengine: dw: move residue to a descriptor Andy Shevchenko
2016-03-18 14:24 ` [PATCH v3 15/15] dmaengine: dw: set cdesc to NULL when free cyclic transfers Andy Shevchenko
2016-04-13 16:08 ` [PATCH v3 00/15] Fixes / cleanups in dw_dmac (affects on few subsystems) Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.