[PATCH v3 0/2] DW DMAC: update device tree

[PATCH v3 0/2] DW DMAC: update device tree

[Eugeniy Paltsev]

It wasn't possible to enable some features like
memory-to-memory transfers or multi block transfers via DT.
It is fixed by these patches.

Changes for v3:
 * Update existing platform data.
   We don't need to update existing DTS because default logic 
   wasn't change: we don't set "is_nollp" if we read 
   configuration from DT before. And we don't set it now if
   "multi-block" property doesn't exist in DTS.

Changes for v2:
 * I thought about is_memcpy DT property: all known devices, which 
   use DT for configuration, support memory-to-memory transfers. 
   So we don't need to read it from DT. So enable it by default, 
   if we read configuration from DT.

 * Use "multi-block" instead of "hw-llp" name to be more clear.

 * Move adding DT property and adding documentation for this
   property to one patch.

Eugeniy Paltsev (2):
  DW DMAC: enable memory-to-memory transfers support
  DW DMAC: add multi-block property to device tree

 Documentation/devicetree/bindings/dma/snps-dma.txt |  2 ++
 drivers/dma/dw/core.c                              |  2 +-
 drivers/dma/dw/platform.c                          | 11 +++++++++++
 drivers/tty/serial/8250/8250_lpss.c                |  2 +-
 include/linux/platform_data/dma-dw.h               |  4 ++--
 5 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.5.5

[PATCH v3 1/2] DW DMAC: enable memory-to-memory transfers support

[Eugeniy Paltsev]

All known devices, which use DT for configuration, support
memory-to-memory transfers. So enable it by default, if we read
configuration from DT.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 drivers/dma/dw/platform.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index 5bda0eb..aa7a5c1 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
 	if (of_property_read_bool(np, "is_private"))
 		pdata->is_private = true;
 
+	/*
+	 * All known devices, which use DT for configuration, support
+	 * memory-to-memory transfers. So enable it by default.
+	 */
+	pdata->is_memcpy = true;
+
 	if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
 		pdata->chan_allocation_order = (unsigned char)tmp;
 
-- 
2.5.5

Re: [PATCH v3 1/2] DW DMAC: enable memory-to-memory transfers support

[Andy Shevchenko]

On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> All known devices, which use DT for configuration, support
> memory-to-memory transfers. So enable it by default, if we read
> configuration from DT.
> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>

You missed the given tag(s).

> ---
>  drivers/dma/dw/platform.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> index 5bda0eb..aa7a5c1 100644
> --- a/drivers/dma/dw/platform.c
> +++ b/drivers/dma/dw/platform.c
> @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
>  	if (of_property_read_bool(np, "is_private"))
>  		pdata->is_private = true;
>  
> +	/*
> +	 * All known devices, which use DT for configuration, support
> +	 * memory-to-memory transfers. So enable it by default.
> +	 */
> +	pdata->is_memcpy = true;
> +
>  	if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
>  		pdata->chan_allocation_order = (unsigned char)tmp;
>  

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

[PATCH v3 2/2] DW DMAC: add multi-block property to device tree

[Eugeniy Paltsev]

Several versions of DW DMAC have multi block transfers hardware
support. Hardware support of multi block transfers is disabled
by default if we use DT to configure DMAC and software emulation
of multi block transfers used instead.
Add multi-block property, so it is possible to enable hardware
multi block transfers (if present) via DT.

Switch from per device is_nollp variable to multi_block array
to be able enable/disable multi block transfers separately per
channel.

Update DT documentation.

Update existing platform data.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 Documentation/devicetree/bindings/dma/snps-dma.txt | 2 ++
 drivers/dma/dw/core.c                              | 2 +-
 drivers/dma/dw/platform.c                          | 5 +++++
 drivers/tty/serial/8250/8250_lpss.c                | 2 +-
 include/linux/platform_data/dma-dw.h               | 4 ++--
 5 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
index 0f55832..03d6d6d 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -27,6 +27,8 @@ Optional properties:
   that services interrupts for this device
 - is_private: The device channels should be marked as private and not for by the
   general purpose DMA channel allocator. False if not passed.
+- multi-block: Multi block transfers supported by hardware per AHB master.
+  0 (default): not supported, 1: supported.
 
 Example:
 
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index c2c0a61..f2a3d06 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1569,7 +1569,7 @@ int dw_dma_probe(struct dw_dma_chip *chip)
 				(dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0;
 		} else {
 			dwc->block_size = pdata->block_size;
-			dwc->nollp = pdata->is_nollp;
+			dwc->nollp = pdata->multi_block[i];
 		}
 	}
 
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index aa7a5c1..b262fd3 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -152,6 +152,11 @@ dw_dma_parse_dt(struct platform_device *pdev)
 			pdata->data_width[tmp] = BIT(arr[tmp] & 0x07);
 	}
 
+	if (!of_property_read_u32_array(np, "multi-block", arr, nr_masters)) {
+		for (tmp = 0; tmp < nr_masters; tmp++)
+			pdata->multi_block[tmp] = arr[tmp];
+	}
+
 	return pdata;
 }
 #else
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index f607946..58cbb30 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -157,12 +157,12 @@ static int byt_serial_setup(struct lpss8250 *lpss, struct uart_port *port)
 static const struct dw_dma_platform_data qrk_serial_dma_pdata = {
 	.nr_channels = 2,
 	.is_private = true,
-	.is_nollp = true,
 	.chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
 	.chan_priority = CHAN_PRIORITY_ASCENDING,
 	.block_size = 4095,
 	.nr_masters = 1,
 	.data_width = {4},
+	.multi_block = {0},
 };
 
 static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index 5f0e11e..0773bb4 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -40,19 +40,18 @@ struct dw_dma_slave {
  * @is_private: The device channels should be marked as private and not for
  *	by the general purpose DMA channel allocator.
  * @is_memcpy: The device channels do support memory-to-memory transfers.
- * @is_nollp: The device channels does not support multi block transfers.
  * @chan_allocation_order: Allocate channels starting from 0 or 7
  * @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
  *		(in bytes, power of 2)
+ * @multi_block: Multi block transfers supported by hardware per AHB master.
  */
 struct dw_dma_platform_data {
 	unsigned int	nr_channels;
 	bool		is_private;
 	bool		is_memcpy;
-	bool		is_nollp;
 #define CHAN_ALLOCATION_ASCENDING	0	/* zero to seven */
 #define CHAN_ALLOCATION_DESCENDING	1	/* seven to zero */
 	unsigned char	chan_allocation_order;
@@ -62,6 +61,7 @@ struct dw_dma_platform_data {
 	unsigned int	block_size;
 	unsigned char	nr_masters;
 	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
+	unsigned char	multi_block[DW_DMA_MAX_NR_MASTERS];
 };
 
 #endif /* _PLATFORM_DATA_DMA_DW_H */
-- 
2.5.5

Re: [PATCH v3 2/2] DW DMAC: add multi-block property to device tree

[Andy Shevchenko]

On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> Several versions of DW DMAC have multi block transfers hardware
> support. Hardware support of multi block transfers is disabled
> by default if we use DT to configure DMAC and software emulation
> of multi block transfers used instead.
> Add multi-block property, so it is possible to enable hardware
> multi block transfers (if present) via DT.
> 
> Switch from per device is_nollp variable to multi_block array
> to be able enable/disable multi block transfers separately per
> channel.
> 

> Update DT documentation.
> 
> Update existing platform data.

Kinda useless for commit message, but might go after --- delimiter.

> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  Documentation/devicetree/bindings/dma/snps-dma.txt | 2 ++
>  drivers/dma/dw/core.c                              | 2 +-
>  drivers/dma/dw/platform.c                          | 5 +++++
>  drivers/tty/serial/8250/8250_lpss.c                | 2 +-
>  include/linux/platform_data/dma-dw.h               | 4 ++--
>  5 files changed, 11 insertions(+), 4 deletions(-)

> --- a/Documentation/devicetree/bindings/dma/snps-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
> @@ -27,6 +27,8 @@ Optional properties:
>    that services interrupts for this device
>  - is_private: The device channels should be marked as private and not
> for by the
>    general purpose DMA channel allocator. False if not passed.
> +- multi-block: Multi block transfers supported by hardware per AHB
> master.
> +  0 (default): not supported, 1: supported.
>  
>  Example:
>  
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index c2c0a61..f2a3d06 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -1569,7 +1569,7 @@ int dw_dma_probe(struct dw_dma_chip *chip)
>  				(dwc_params >> DWC_PARAMS_MBLK_EN &
> 0x1) == 0;
>  		} else {
>  			dwc->block_size = pdata->block_size;
> -			dwc->nollp = pdata->is_nollp;
> +			dwc->nollp = pdata->multi_block[i];

You missed the point. You assign positive value to negative variable.
It's a bug. Have you tested this? How?

In case of positive property you have to update DTS. By the way, I'm
pretty sure that spare13xx boards has auto configuration enabled, though
it has to be checked with vendor (I assume you may have fast response
from them).

>  		}
>  	}
>  

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

Re: [PATCH v3 2/2] DW DMAC: add multi-block property to device tree

[Vinod Koul]

On Fri, Nov 18, 2016 at 09:33:13PM +0200, Andy Shevchenko wrote:
> > @@ -1569,7 +1569,7 @@ int dw_dma_probe(struct dw_dma_chip *chip)
> >  				(dwc_params >> DWC_PARAMS_MBLK_EN &
> > 0x1) == 0;
> >  		} else {
> >  			dwc->block_size = pdata->block_size;
> > -			dwc->nollp = pdata->is_nollp;
> > +			dwc->nollp = pdata->multi_block[i];
> 
> You missed the point. You assign positive value to negative variable.
> It's a bug. Have you tested this? How?
> 
> In case of positive property you have to update DTS. By the way, I'm
> pretty sure that spare13xx boards has auto configuration enabled, though
> it has to be checked with vendor (I assume you may have fast response
> from them).

Yeah why are we not using auto configuration here would be the first
question..

-- 
~Vinod

Re: [PATCH v3 2/2] DW DMAC: add multi-block property to device tree

[Arnd Bergmann]

On Friday, November 18, 2016 10:12:36 PM CET Eugeniy Paltsev wrote:
> +- multi-block: Multi block transfers supported by hardware per AHB master.
> +  0 (default): not supported, 1: supported.

Please clarify that this is an array with one cell per master.
My first thought was "why is this not a boolean property", and
I'm sure others might misread it the same way.

	Arnd

Re: [PATCH v3 0/2] DW DMAC: update device tree

[Andy Shevchenko]

On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> It wasn't possible to enable some features like
> memory-to-memory transfers or multi block transfers via DT.
> It is fixed by these patches.

First of all, please, give time to reviewers to comment the patches.
Usually it should be at least 24h (for the series that has been sent
first time 1 week approximately).

> 
> Changes for v3:
>  * Update existing platform data.
>    We don't need to update existing DTS because default logic 
>    wasn't change: we don't set "is_nollp" if we read 
>    configuration from DT before. And we don't set it now if
>    "multi-block" property doesn't exist in DTS.

See my comments in the patches.
And do not send the updated version earlier than Monday, please.

> 
> Changes for v2:
>  * I thought about is_memcpy DT property: all known devices, which 
>    use DT for configuration, support memory-to-memory transfers. 
>    So we don't need to read it from DT. So enable it by default, 
>    if we read configuration from DT.
> 
>  * Use "multi-block" instead of "hw-llp" name to be more clear.
> 
>  * Move adding DT property and adding documentation for this
>    property to one patch.
> 
> Eugeniy Paltsev (2):
>   DW DMAC: enable memory-to-memory transfers support
>   DW DMAC: add multi-block property to device tree
> 
>  Documentation/devicetree/bindings/dma/snps-dma.txt |  2 ++
>  drivers/dma/dw/core.c                              |  2 +-
>  drivers/dma/dw/platform.c                          | 11 +++++++++++
>  drivers/tty/serial/8250/8250_lpss.c                |  2 +-
>  include/linux/platform_data/dma-dw.h               |  4 ++--
>  5 files changed, 17 insertions(+), 4 deletions(-)
> 

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

Re: [PATCH v3 0/2] DW DMAC: update device tree

[Alexey Brodkin]

Hi Andy,

On Fri, 2016-11-18 at 21:26 +0200, Andy Shevchenko wrote:
> On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> > 
> > It wasn't possible to enable some features like
> > memory-to-memory transfers or multi block transfers via DT.
> > It is fixed by these patches.
> 
> First of all, please, give time to reviewers to comment the patches.
> Usually it should be at least 24h (for the series that has been sent
> first time 1 week approximately).

I'm not really sure a lot of people get disturbed by this series
and given this all has been discussed for months now I'd really like
to see changes required for our HW to work to land in upstream ASAP.

Too bad we're late for 4.9 (which is supposed to be the next LTS) but
we need to make sure this series hits 4.10 for sure.

Hope this race doesn't affect you that much.

-Alexey

Re: [PATCH v3 0/2] DW DMAC: update device tree

[Andy Shevchenko]

On Mon, 2016-11-21 at 10:02 +0000, Alexey Brodkin wrote:
> Hi Andy,
> 
> On Fri, 2016-11-18 at 21:26 +0200, Andy Shevchenko wrote:
> > On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> > > 
> > > It wasn't possible to enable some features like
> > > memory-to-memory transfers or multi block transfers via DT.
> > > It is fixed by these patches.
> > 
> > First of all, please, give time to reviewers to comment the patches.
> > Usually it should be at least 24h (for the series that has been sent
> > first time 1 week approximately).
> 
> I'm not really sure a lot of people get disturbed by this series
> and given this all has been discussed for months now I'd really like
> to see changes required for our HW to work to land in upstream ASAP.

I understand your concern, I'm often in the same position in many areas,
including this driver (I'm not a maintainer of slave DMA subsystem).

Though let's face the issues we have with the series:
- stuff regarding to style and alike (would be fixed in a day)
- DTS naming and conventions, this is apparently a big area, where I
might share opinion, but can't decide for
- last word by the subsystem maintainer

> Too bad we're late for 4.9 (which is supposed to be the next LTS) but
> > we need to make sure this series hits 4.10 for sure.

Vinod, is it possible to get in for this series (if we get Ack from DT
people)?

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

Re: [PATCH v3 0/2] DW DMAC: update device tree

[Vinod Koul]

On Mon, Nov 21, 2016 at 12:37:06PM +0200, Andy Shevchenko wrote:
> On Mon, 2016-11-21 at 10:02 +0000, Alexey Brodkin wrote:
> > Hi Andy,
> > 
> > On Fri, 2016-11-18 at 21:26 +0200, Andy Shevchenko wrote:
> > > On Fri, 2016-11-18 at 22:12 +0300, Eugeniy Paltsev wrote:
> > > > 
> > > > It wasn't possible to enable some features like
> > > > memory-to-memory transfers or multi block transfers via DT.
> > > > It is fixed by these patches.
> > > 
> > > First of all, please, give time to reviewers to comment the patches.
> > > Usually it should be at least 24h (for the series that has been sent
> > > first time 1 week approximately).
> > 
> > I'm not really sure a lot of people get disturbed by this series
> > and given this all has been discussed for months now I'd really like
> > to see changes required for our HW to work to land in upstream ASAP.
> 
> I understand your concern, I'm often in the same position in many areas,
> including this driver (I'm not a maintainer of slave DMA subsystem).
> 
> Though let's face the issues we have with the series:
> - stuff regarding to style and alike (would be fixed in a day)
> - DTS naming and conventions, this is apparently a big area, where I
> might share opinion, but can't decide for
> - last word by the subsystem maintainer
> 
> > Too bad we're late for 4.9 (which is supposed to be the next LTS) but
> > > we need to make sure this series hits 4.10 for sure.
> 
> Vinod, is it possible to get in for this series (if we get Ack from DT
> people)?

We still have a week or so... But holding race agaisnt upstream is a bad
idea... Doesnt work that way.

-- 
~Vinod