* [PATCH v3 2/5] dt-bindings: dmaengine: xilinx_dma: add optional xlnx,sg-length-width property
2018-06-25 9:27 [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Andrea Merello
@ 2018-06-25 9:27 ` Andrea Merello
2018-06-25 17:29 ` Rob Herring
2018-06-25 9:27 ` [PATCH v3 3/5] dmaengine: xilinx_dma: program hardware supported buffer length Andrea Merello
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Andrea Merello @ 2018-06-25 9:27 UTC (permalink / raw)
To: vkoul, dan.j.williams, michal.simek, appana.durga.rao, dmaengine
Cc: linux-arm-kernel, linux-kernel, Andrea Merello, Rob Herring,
Mark Rutland, devicetree, Radhey Shyam Pandey
The width of the "length register" cannot be autodetected, and it is now
specified with a DT property. Add DOC for it.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
Changes in v2:
- change property name
- property is now optional
- cc DT maintainer
Changes in v3:
- reword
- cc DT maintainerS and ML
---
Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index a2b8bfaec43c..c894abe28baa 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -41,6 +41,7 @@ Optional properties:
- xlnx,include-sg: Tells configured for Scatter-mode in
the hardware.
Optional properties for AXI DMA:
+- xlnx,sg-length-width: Should be the width of the length register as configured in h/w.
- xlnx,mcdma: Tells whether configured for multi-channel mode in the hardware.
Optional properties for VDMA:
- xlnx,flush-fsync: Tells which channel to Flush on Frame sync.
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 2/5] dt-bindings: dmaengine: xilinx_dma: add optional xlnx,sg-length-width property
2018-06-25 9:27 ` [PATCH v3 2/5] dt-bindings: dmaengine: xilinx_dma: add optional xlnx,sg-length-width property Andrea Merello
@ 2018-06-25 17:29 ` Rob Herring
0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2018-06-25 17:29 UTC (permalink / raw)
To: Andrea Merello
Cc: vkoul, dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-arm-kernel, linux-kernel, Mark Rutland, devicetree,
Radhey Shyam Pandey
On Mon, Jun 25, 2018 at 11:27:21AM +0200, Andrea Merello wrote:
> The width of the "length register" cannot be autodetected, and it is now
> specified with a DT property. Add DOC for it.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> ---
> Changes in v2:
> - change property name
> - property is now optional
> - cc DT maintainer
> Changes in v3:
> - reword
> - cc DT maintainerS and ML
> ---
> Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> index a2b8bfaec43c..c894abe28baa 100644
> --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> @@ -41,6 +41,7 @@ Optional properties:
> - xlnx,include-sg: Tells configured for Scatter-mode in
> the hardware.
> Optional properties for AXI DMA:
> +- xlnx,sg-length-width: Should be the width of the length register as configured in h/w.
What are the units? What are valid values? What is the default if the
property is not present?
> - xlnx,mcdma: Tells whether configured for multi-channel mode in the hardware.
> Optional properties for VDMA:
> - xlnx,flush-fsync: Tells which channel to Flush on Frame sync.
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/5] dmaengine: xilinx_dma: program hardware supported buffer length
2018-06-25 9:27 [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Andrea Merello
2018-06-25 9:27 ` [PATCH v3 2/5] dt-bindings: dmaengine: xilinx_dma: add optional xlnx,sg-length-width property Andrea Merello
@ 2018-06-25 9:27 ` Andrea Merello
2018-06-25 9:27 ` [PATCH v3 4/5] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather Andrea Merello
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Andrea Merello @ 2018-06-25 9:27 UTC (permalink / raw)
To: vkoul, dan.j.williams, michal.simek, appana.durga.rao, dmaengine
Cc: linux-arm-kernel, linux-kernel, Radhey Shyam Pandey, Rob Herring,
Mark Rutland, devicetree, Andrea Merello
From: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
AXI-DMA IP supports configurable (c_sg_length_width) buffer length
register width, hence read buffer length (xlnx,sg-length-width) DT
property and ensure that driver doesn't program buffer length
exceeding the supported limit. For VDMA and CDMA there is no change.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Andrea Merello <andrea.merello@gmail.com> [rebase, reword]
---
Changes in v2:
- drop original patch and replace with the one in Xilinx tree
Changes in v3:
- cc DT maintainers/ML
---
drivers/dma/xilinx/xilinx_dma.c | 35 ++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 113d9bf1b6a1..7f0ab904b749 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -158,7 +158,8 @@
#define XILINX_DMA_REG_BTT 0x28
/* AXI DMA Specific Masks/Bit fields */
-#define XILINX_DMA_MAX_TRANS_LEN GENMASK(22, 0)
+#define XILINX_DMA_MAX_TRANS_LEN_MIN 8
+#define XILINX_DMA_MAX_TRANS_LEN_MAX 23
#define XILINX_DMA_CR_COALESCE_MAX GENMASK(23, 16)
#define XILINX_DMA_CR_CYCLIC_BD_EN_MASK BIT(4)
#define XILINX_DMA_CR_COALESCE_SHIFT 16
@@ -418,6 +419,7 @@ struct xilinx_dma_config {
* @rxs_clk: DMA s2mm stream clock
* @nr_channels: Number of channels DMA device supports
* @chan_id: DMA channel identifier
+ * @max_buffer_len: Max buffer length
*/
struct xilinx_dma_device {
void __iomem *regs;
@@ -437,6 +439,7 @@ struct xilinx_dma_device {
struct clk *rxs_clk;
u32 nr_channels;
u32 chan_id;
+ u32 max_buffer_len;
};
/* Macros */
@@ -985,7 +988,7 @@ static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
list_for_each_entry(segment, &desc->segments, node) {
hw = &segment->hw;
residue += (hw->control - hw->status) &
- XILINX_DMA_MAX_TRANS_LEN;
+ chan->xdev->max_buffer_len;
}
}
spin_unlock_irqrestore(&chan->lock, flags);
@@ -1237,7 +1240,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
/* Start the transfer */
dma_ctrl_write(chan, XILINX_DMA_REG_BTT,
- hw->control & XILINX_DMA_MAX_TRANS_LEN);
+ hw->control & chan->xdev->max_buffer_len);
}
list_splice_tail_init(&chan->pending_list, &chan->active_list);
@@ -1340,7 +1343,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
/* Start the transfer */
dma_ctrl_write(chan, XILINX_DMA_REG_BTT,
- hw->control & XILINX_DMA_MAX_TRANS_LEN);
+ hw->control & chan->xdev->max_buffer_len);
}
list_splice_tail_init(&chan->pending_list, &chan->active_list);
@@ -1701,7 +1704,7 @@ xilinx_cdma_prep_memcpy(struct dma_chan *dchan, dma_addr_t dma_dst,
struct xilinx_cdma_tx_segment *segment;
struct xilinx_cdma_desc_hw *hw;
- if (!len || len > XILINX_DMA_MAX_TRANS_LEN)
+ if (!len || len > chan->xdev->max_buffer_len)
return NULL;
desc = xilinx_dma_alloc_tx_descriptor(chan);
@@ -1792,7 +1795,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
* making sure it is less than the hw limit
*/
copy = min_t(size_t, sg_dma_len(sg) - sg_used,
- XILINX_DMA_MAX_TRANS_LEN);
+ chan->xdev->max_buffer_len);
if ((copy + sg_used < sg_dma_len(sg)) &&
chan->xdev->common.copy_align) {
@@ -1907,7 +1910,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
* making sure it is less than the hw limit
*/
copy = min_t(size_t, period_len - sg_used,
- XILINX_DMA_MAX_TRANS_LEN);
+ chan->xdev->max_buffer_len);
if ((copy + sg_used < period_len) &&
chan->xdev->common.copy_align) {
@@ -2590,7 +2593,7 @@ static int xilinx_dma_probe(struct platform_device *pdev)
struct xilinx_dma_device *xdev;
struct device_node *child, *np = pdev->dev.of_node;
struct resource *io;
- u32 num_frames, addr_width;
+ u32 num_frames, addr_width, len_width;
int i, err;
/* Allocate and initialize the DMA engine structure */
@@ -2622,8 +2625,22 @@ static int xilinx_dma_probe(struct platform_device *pdev)
/* Retrieve the DMA engine properties from the device tree */
xdev->has_sg = of_property_read_bool(node, "xlnx,include-sg");
- if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA)
+ xdev->max_buffer_len = GENMASK(XILINX_DMA_MAX_TRANS_LEN_MAX - 1, 0);
+
+ if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
xdev->mcdma = of_property_read_bool(node, "xlnx,mcdma");
+ if (!of_property_read_u32(node, "xlnx,sg-length-width",
+ &len_width)) {
+ if (len_width < XILINX_DMA_MAX_TRANS_LEN_MIN ||
+ len_width > XILINX_DMA_MAX_TRANS_LEN_MAX) {
+ dev_warn(xdev->dev,
+ "invalid xlnx,sg-length-width property value using default width\n");
+ } else {
+ xdev->max_buffer_len = GENMASK(len_width - 1,
+ 0);
+ }
+ }
+ }
if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
err = of_property_read_u32(node, "xlnx,num-fstores",
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 4/5] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather
2018-06-25 9:27 [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Andrea Merello
2018-06-25 9:27 ` [PATCH v3 2/5] dt-bindings: dmaengine: xilinx_dma: add optional xlnx,sg-length-width property Andrea Merello
2018-06-25 9:27 ` [PATCH v3 3/5] dmaengine: xilinx_dma: program hardware supported buffer length Andrea Merello
@ 2018-06-25 9:27 ` Andrea Merello
2018-06-29 7:37 ` Vinod
2018-06-25 9:27 ` [PATCH v3 5/5] dt-bindings: dmaengine: xilinx_dma: drop has-sg property Andrea Merello
2018-06-29 7:25 ` [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Vinod
4 siblings, 1 reply; 12+ messages in thread
From: Andrea Merello @ 2018-06-25 9:27 UTC (permalink / raw)
To: vkoul, dan.j.williams, michal.simek, appana.durga.rao, dmaengine
Cc: linux-arm-kernel, linux-kernel, Andrea Merello, Rob Herring,
Mark Rutland, devicetree, Radhey Shyam Pandey
The AXIDMA and CDMA HW can be either direct-access or scatter-gather
version. These are SW incompatible.
The driver can handle both version: a DT property was used to
tell the driver whether to assume the HW is is scatter-gather mode.
This patch makes the driver to autodetect this information. The DT
property is not required anymore.
No changes for VDMA.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
Changes in v2:
- autodetect only in !VDMA case
Changes in v3:
- cc DT maintainers/ML
---
drivers/dma/xilinx/xilinx_dma.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 7f0ab904b749..43fcc71ff287 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -86,6 +86,7 @@
#define XILINX_DMA_DMASR_DMA_DEC_ERR BIT(6)
#define XILINX_DMA_DMASR_DMA_SLAVE_ERR BIT(5)
#define XILINX_DMA_DMASR_DMA_INT_ERR BIT(4)
+#define XILINX_DMA_DMASR_SG_MASK BIT(3)
#define XILINX_DMA_DMASR_IDLE BIT(1)
#define XILINX_DMA_DMASR_HALTED BIT(0)
#define XILINX_DMA_DMASR_DELAY_MASK GENMASK(31, 24)
@@ -406,7 +407,6 @@ struct xilinx_dma_config {
* @dev: Device Structure
* @common: DMA device structure
* @chan: Driver specific DMA channel
- * @has_sg: Specifies whether Scatter-Gather is present or not
* @mcdma: Specifies whether Multi-Channel is present or not
* @flush_on_fsync: Flush on frame sync
* @ext_addr: Indicates 64 bit addressing is supported by dma device
@@ -426,7 +426,6 @@ struct xilinx_dma_device {
struct device *dev;
struct dma_device common;
struct xilinx_dma_chan *chan[XILINX_DMA_MAX_CHANS_PER_DEVICE];
- bool has_sg;
bool mcdma;
u32 flush_on_fsync;
bool ext_addr;
@@ -2393,7 +2392,6 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
chan->dev = xdev->dev;
chan->xdev = xdev;
- chan->has_sg = xdev->has_sg;
chan->desc_pendingcount = 0x0;
chan->ext_addr = xdev->ext_addr;
/* This variable ensures that descriptors are not
@@ -2486,6 +2484,15 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
chan->stop_transfer = xilinx_dma_stop_transfer;
}
+ /* check if SG is enabled (only for AXIDMA and CDMA) */
+ if (xdev->dma_config->dmatype != XDMA_TYPE_VDMA) {
+ if (dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
+ XILINX_DMA_DMASR_SG_MASK)
+ chan->has_sg = true;
+ dev_dbg(chan->dev, "ch %d: SG %s\n", chan->id,
+ chan->has_sg ? "enabled" : "disabled");
+ }
+
/* Initialize the tasklet */
tasklet_init(&chan->tasklet, xilinx_dma_do_tasklet,
(unsigned long)chan);
@@ -2624,7 +2631,6 @@ static int xilinx_dma_probe(struct platform_device *pdev)
return PTR_ERR(xdev->regs);
/* Retrieve the DMA engine properties from the device tree */
- xdev->has_sg = of_property_read_bool(node, "xlnx,include-sg");
xdev->max_buffer_len = GENMASK(XILINX_DMA_MAX_TRANS_LEN_MAX - 1, 0);
if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 4/5] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather
2018-06-25 9:27 ` [PATCH v3 4/5] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather Andrea Merello
@ 2018-06-29 7:37 ` Vinod
2018-06-29 7:53 ` Andrea Merello
0 siblings, 1 reply; 12+ messages in thread
From: Vinod @ 2018-06-29 7:37 UTC (permalink / raw)
To: Andrea Merello
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-arm-kernel, linux-kernel, Rob Herring, Mark Rutland,
devicetree, Radhey Shyam Pandey
On 25-06-18, 11:27, Andrea Merello wrote:
> The AXIDMA and CDMA HW can be either direct-access or scatter-gather
> version. These are SW incompatible.
>
> The driver can handle both version: a DT property was used to
^^^^
versions
> tell the driver whether to assume the HW is is scatter-gather mode.
^^^^^
is in?
>
> This patch makes the driver to autodetect this information. The DT
> property is not required anymore.
>
> No changes for VDMA.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> ---
> Changes in v2:
> - autodetect only in !VDMA case
> Changes in v3:
> - cc DT maintainers/ML
> ---
> drivers/dma/xilinx/xilinx_dma.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 7f0ab904b749..43fcc71ff287 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -86,6 +86,7 @@
> #define XILINX_DMA_DMASR_DMA_DEC_ERR BIT(6)
> #define XILINX_DMA_DMASR_DMA_SLAVE_ERR BIT(5)
> #define XILINX_DMA_DMASR_DMA_INT_ERR BIT(4)
> +#define XILINX_DMA_DMASR_SG_MASK BIT(3)
> #define XILINX_DMA_DMASR_IDLE BIT(1)
> #define XILINX_DMA_DMASR_HALTED BIT(0)
> #define XILINX_DMA_DMASR_DELAY_MASK GENMASK(31, 24)
> @@ -406,7 +407,6 @@ struct xilinx_dma_config {
> * @dev: Device Structure
> * @common: DMA device structure
> * @chan: Driver specific DMA channel
> - * @has_sg: Specifies whether Scatter-Gather is present or not
> * @mcdma: Specifies whether Multi-Channel is present or not
> * @flush_on_fsync: Flush on frame sync
> * @ext_addr: Indicates 64 bit addressing is supported by dma device
> @@ -426,7 +426,6 @@ struct xilinx_dma_device {
> struct device *dev;
> struct dma_device common;
> struct xilinx_dma_chan *chan[XILINX_DMA_MAX_CHANS_PER_DEVICE];
> - bool has_sg;
> bool mcdma;
> u32 flush_on_fsync;
> bool ext_addr;
> @@ -2393,7 +2392,6 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>
> chan->dev = xdev->dev;
> chan->xdev = xdev;
> - chan->has_sg = xdev->has_sg;
> chan->desc_pendingcount = 0x0;
> chan->ext_addr = xdev->ext_addr;
> /* This variable ensures that descriptors are not
> @@ -2486,6 +2484,15 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
> chan->stop_transfer = xilinx_dma_stop_transfer;
> }
>
> + /* check if SG is enabled (only for AXIDMA and CDMA) */
> + if (xdev->dma_config->dmatype != XDMA_TYPE_VDMA) {
> + if (dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
> + XILINX_DMA_DMASR_SG_MASK)
why not read this for VDMA too, will it return false?
> + chan->has_sg = true;
> + dev_dbg(chan->dev, "ch %d: SG %s\n", chan->id,
> + chan->has_sg ? "enabled" : "disabled");
this debug print can be removed
--
~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 4/5] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather
2018-06-29 7:37 ` Vinod
@ 2018-06-29 7:53 ` Andrea Merello
0 siblings, 0 replies; 12+ messages in thread
From: Andrea Merello @ 2018-06-29 7:53 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-arm-kernel, linux-kernel, Rob Herring, Mark Rutland,
devicetree, Radhey Shyam Pandey
On Fri, Jun 29, 2018 at 9:37 AM, Vinod <vkoul@kernel.org> wrote:
> On 25-06-18, 11:27, Andrea Merello wrote:
>> The AXIDMA and CDMA HW can be either direct-access or scatter-gather
>> version. These are SW incompatible.
>>
>> The driver can handle both version: a DT property was used to
> ^^^^
> versions
>
OK
>> tell the driver whether to assume the HW is is scatter-gather mode.
> ^^^^^
> is in?
Yes, it is. Thanks
>>
>> This patch makes the driver to autodetect this information. The DT
>> property is not required anymore.
>>
>> No changes for VDMA.
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
>> ---
>> Changes in v2:
>> - autodetect only in !VDMA case
>> Changes in v3:
>> - cc DT maintainers/ML
>> ---
>> drivers/dma/xilinx/xilinx_dma.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index 7f0ab904b749..43fcc71ff287 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -86,6 +86,7 @@
>> #define XILINX_DMA_DMASR_DMA_DEC_ERR BIT(6)
>> #define XILINX_DMA_DMASR_DMA_SLAVE_ERR BIT(5)
>> #define XILINX_DMA_DMASR_DMA_INT_ERR BIT(4)
>> +#define XILINX_DMA_DMASR_SG_MASK BIT(3)
>> #define XILINX_DMA_DMASR_IDLE BIT(1)
>> #define XILINX_DMA_DMASR_HALTED BIT(0)
>> #define XILINX_DMA_DMASR_DELAY_MASK GENMASK(31, 24)
>> @@ -406,7 +407,6 @@ struct xilinx_dma_config {
>> * @dev: Device Structure
>> * @common: DMA device structure
>> * @chan: Driver specific DMA channel
>> - * @has_sg: Specifies whether Scatter-Gather is present or not
>> * @mcdma: Specifies whether Multi-Channel is present or not
>> * @flush_on_fsync: Flush on frame sync
>> * @ext_addr: Indicates 64 bit addressing is supported by dma device
>> @@ -426,7 +426,6 @@ struct xilinx_dma_device {
>> struct device *dev;
>> struct dma_device common;
>> struct xilinx_dma_chan *chan[XILINX_DMA_MAX_CHANS_PER_DEVICE];
>> - bool has_sg;
>> bool mcdma;
>> u32 flush_on_fsync;
>> bool ext_addr;
>> @@ -2393,7 +2392,6 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>>
>> chan->dev = xdev->dev;
>> chan->xdev = xdev;
>> - chan->has_sg = xdev->has_sg;
>> chan->desc_pendingcount = 0x0;
>> chan->ext_addr = xdev->ext_addr;
>> /* This variable ensures that descriptors are not
>> @@ -2486,6 +2484,15 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>> chan->stop_transfer = xilinx_dma_stop_transfer;
>> }
>>
>> + /* check if SG is enabled (only for AXIDMA and CDMA) */
>> + if (xdev->dma_config->dmatype != XDMA_TYPE_VDMA) {
>> + if (dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
>> + XILINX_DMA_DMASR_SG_MASK)
>
> why not read this for VDMA too, will it return false?
AFAIK this bit is reserved in VDMA, so reading it can lead to any
random thing.. I would say that potentially it could even be used for
other purposes in future IP releases..
>> + chan->has_sg = true;
>> + dev_dbg(chan->dev, "ch %d: SG %s\n", chan->id,
>> + chan->has_sg ? "enabled" : "disabled");
>
> this debug print can be removed
IMHO if someone will ever enable debug prints for debugging something
and then he/she reports us a log, then it would be useful to see in
the log if the IP was SG or not..
> --
> ~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 5/5] dt-bindings: dmaengine: xilinx_dma: drop has-sg property
2018-06-25 9:27 [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Andrea Merello
` (2 preceding siblings ...)
2018-06-25 9:27 ` [PATCH v3 4/5] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather Andrea Merello
@ 2018-06-25 9:27 ` Andrea Merello
2018-06-25 17:30 ` Rob Herring
2018-06-29 7:25 ` [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Vinod
4 siblings, 1 reply; 12+ messages in thread
From: Andrea Merello @ 2018-06-25 9:27 UTC (permalink / raw)
To: vkoul, dan.j.williams, michal.simek, appana.durga.rao, dmaengine
Cc: linux-arm-kernel, linux-kernel, Andrea Merello, Rob Herring,
Mark Rutland, devicetree, Radhey Shyam Pandey
This property is not needed anymore, because the driver now autodetects it.
Delete references in documentation.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
Changes in v2:
- cc DT maintainer
Changes in v3:
- cc DT maintainerS/ML
---
Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 3 ---
1 file changed, 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index c894abe28baa..09a41843cbc0 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -37,9 +37,6 @@ Required properties:
Required properties for VDMA:
- xlnx,num-fstores: Should be the number of framebuffers as configured in h/w.
-Optional properties:
-- xlnx,include-sg: Tells configured for Scatter-mode in
- the hardware.
Optional properties for AXI DMA:
- xlnx,sg-length-width: Should be the width of the length register as configured in h/w.
- xlnx,mcdma: Tells whether configured for multi-channel mode in the hardware.
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 5/5] dt-bindings: dmaengine: xilinx_dma: drop has-sg property
2018-06-25 9:27 ` [PATCH v3 5/5] dt-bindings: dmaengine: xilinx_dma: drop has-sg property Andrea Merello
@ 2018-06-25 17:30 ` Rob Herring
0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2018-06-25 17:30 UTC (permalink / raw)
To: Andrea Merello
Cc: vkoul, dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-arm-kernel, linux-kernel, Mark Rutland, devicetree,
Radhey Shyam Pandey
On Mon, Jun 25, 2018 at 11:27:24AM +0200, Andrea Merello wrote:
> This property is not needed anymore, because the driver now autodetects it.
> Delete references in documentation.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> ---
> Changes in v2:
> - cc DT maintainer
> Changes in v3:
> - cc DT maintainerS/ML
> ---
> Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 3 ---
> 1 file changed, 3 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
2018-06-25 9:27 [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Andrea Merello
` (3 preceding siblings ...)
2018-06-25 9:27 ` [PATCH v3 5/5] dt-bindings: dmaengine: xilinx_dma: drop has-sg property Andrea Merello
@ 2018-06-29 7:25 ` Vinod
2018-06-29 7:46 ` Andrea Merello
4 siblings, 1 reply; 12+ messages in thread
From: Vinod @ 2018-06-29 7:25 UTC (permalink / raw)
To: Andrea Merello
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-arm-kernel, linux-kernel, Radhey Shyam Pandey
On 25-06-18, 11:27, Andrea Merello wrote:
> Whenever a single or cyclic transaction is prepared, the driver
> could eventually split it over several SG descriptors in order
> to deal with the HW maximum transfer length.
>
> This could end up in DMA operations starting from a misaligned
> address. This seems fatal for the HW if DRE is not enabled.
>
> This patch eventually adjusts the transfer size in order to make sure
> all operations start from an aligned address.
>
> Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> ---
> Changes in v2:
> - don't introduce copy_mask field, rather rely on already-esistent
> copy_align field. Suggested by Radhey Shyam Pandey
> - reword title
> Changes in v3:
> - fix bug introduced in v2: wrong copy size when DRE is enabled
> use implementation suggested by Radhey Shyam Pandey
> ---
> drivers/dma/xilinx/xilinx_dma.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 27b523530c4a..113d9bf1b6a1 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1793,6 +1793,16 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
> */
> copy = min_t(size_t, sg_dma_len(sg) - sg_used,
> XILINX_DMA_MAX_TRANS_LEN);
> +
> + if ((copy + sg_used < sg_dma_len(sg)) &&
> + chan->xdev->common.copy_align) {
> + /*
> + * If this is not the last descriptor, make sure
> + * the next one will be properly aligned
> + */
> + copy = rounddown(copy,
> + (1 << chan->xdev->common.copy_align));
> + }
> hw = &segment->hw;
>
> /* Fill in the descriptor */
> @@ -1898,6 +1908,16 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
> */
> copy = min_t(size_t, period_len - sg_used,
> XILINX_DMA_MAX_TRANS_LEN);
> +
> + if ((copy + sg_used < period_len) &&
> + chan->xdev->common.copy_align) {
> + /*
> + * If this is not the last descriptor, make sure
> + * the next one will be properly aligned
> + */
> + copy = rounddown(copy,
> + (1 << chan->xdev->common.copy_align));
> + }
same code pasted twice, can we have a routine for this... perhaps more
code can be made common too
--
~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
2018-06-29 7:25 ` [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors Vinod
@ 2018-06-29 7:46 ` Andrea Merello
2018-06-29 8:19 ` Vinod
0 siblings, 1 reply; 12+ messages in thread
From: Andrea Merello @ 2018-06-29 7:46 UTC (permalink / raw)
To: Vinod
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-arm-kernel, linux-kernel, Radhey Shyam Pandey
On Fri, Jun 29, 2018 at 9:25 AM, Vinod <vkoul@kernel.org> wrote:
> On 25-06-18, 11:27, Andrea Merello wrote:
>> Whenever a single or cyclic transaction is prepared, the driver
>> could eventually split it over several SG descriptors in order
>> to deal with the HW maximum transfer length.
>>
>> This could end up in DMA operations starting from a misaligned
>> address. This seems fatal for the HW if DRE is not enabled.
>>
>> This patch eventually adjusts the transfer size in order to make sure
>> all operations start from an aligned address.
>>
>> Cc: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
>> ---
>> Changes in v2:
>> - don't introduce copy_mask field, rather rely on already-esistent
>> copy_align field. Suggested by Radhey Shyam Pandey
>> - reword title
>> Changes in v3:
>> - fix bug introduced in v2: wrong copy size when DRE is enabled
>> use implementation suggested by Radhey Shyam Pandey
>> ---
>> drivers/dma/xilinx/xilinx_dma.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index 27b523530c4a..113d9bf1b6a1 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -1793,6 +1793,16 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
>> */
>> copy = min_t(size_t, sg_dma_len(sg) - sg_used,
>> XILINX_DMA_MAX_TRANS_LEN);
>> +
>> + if ((copy + sg_used < sg_dma_len(sg)) &&
>> + chan->xdev->common.copy_align) {
>> + /*
>> + * If this is not the last descriptor, make sure
>> + * the next one will be properly aligned
>> + */
>> + copy = rounddown(copy,
>> + (1 << chan->xdev->common.copy_align));
>> + }
>> hw = &segment->hw;
>>
>> /* Fill in the descriptor */
>> @@ -1898,6 +1908,16 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
>> */
>> copy = min_t(size_t, period_len - sg_used,
>> XILINX_DMA_MAX_TRANS_LEN);
>> +
>> + if ((copy + sg_used < period_len) &&
>> + chan->xdev->common.copy_align) {
>> + /*
>> + * If this is not the last descriptor, make sure
>> + * the next one will be properly aligned
>> + */
>> + copy = rounddown(copy,
>> + (1 << chan->xdev->common.copy_align));
>> + }
>
> same code pasted twice, can we have a routine for this... perhaps more
> code can be made common too
Yes, I see.. Indeed there was duplicated code before this series and
it is still there after it.
I can see if we can have a routine as you suggested at least for the
code portions touched by this patch. Do you eventually want this extra
change to be done in the same patch 1/5 or do you want a separate
patch i.e. 2/6 or 6/6 ?
> --
> ~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/5] dmaengine: xilinx_dma: in axidma slave_sg and dma_cylic mode align split descriptors
2018-06-29 7:46 ` Andrea Merello
@ 2018-06-29 8:19 ` Vinod
0 siblings, 0 replies; 12+ messages in thread
From: Vinod @ 2018-06-29 8:19 UTC (permalink / raw)
To: Andrea Merello
Cc: dan.j.williams, michal.simek, appana.durga.rao, dmaengine,
linux-arm-kernel, linux-kernel, Radhey Shyam Pandey
On 29-06-18, 09:46, Andrea Merello wrote:
> On Fri, Jun 29, 2018 at 9:25 AM, Vinod <vkoul@kernel.org> wrote:
> >> +
> >> + if ((copy + sg_used < period_len) &&
> >> + chan->xdev->common.copy_align) {
> >> + /*
> >> + * If this is not the last descriptor, make sure
> >> + * the next one will be properly aligned
> >> + */
> >> + copy = rounddown(copy,
> >> + (1 << chan->xdev->common.copy_align));
> >> + }
> >
> > same code pasted twice, can we have a routine for this... perhaps more
> > code can be made common too
>
> Yes, I see.. Indeed there was duplicated code before this series and
> it is still there after it.
>
> I can see if we can have a routine as you suggested at least for the
> code portions touched by this patch. Do you eventually want this extra
> change to be done in the same patch 1/5 or do you want a separate
> patch i.e. 2/6 or 6/6 ?
Each patch should do one thing, so would make sense to move first and
then add you on top of that. 1/6 commonize and 2/6 add this bit.
--
~Vinod
^ permalink raw reply [flat|nested] 12+ messages in thread