[PATCH v2 0/4] dmaengine: xilinx_dma: Bug fixes

[PATCH v2 0/4] dmaengine: xilinx_dma: Bug fixes

[Kedareswara rao Appana]

This patch series does the below
--> Fixes sparse warnings in the driver.
--> properly configures the SG mode bit in the driver for cdma.
--> populates dma caps properly.

This patch series got created on top of linux tag 4.15-rc4
i.e slave-dma.git next branch

Kedareswara rao Appana (4):
  dmaengine: xilinx_dma: populate dma caps properly
  dmaengine: xilinx_dma: properly configure the SG mode bit in the
    driver for cdma
  dmaengine: xilinx_dma: Fix warning variable prev set but not used
  dmaengine: xilinx_dma: Free BD consistent memory

 drivers/dma/xilinx/xilinx_dma.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Kedareswara rao Appana]

When client driver uses dma_get_slave_caps() api,
it checks for certain fields of dma_device struct
currently driver is not settings the directions and addr_widths
fields resulting dma_get_slave_caps() returning failure.

This patch fixes this issue by populating proper values
to the struct dma_device directions and addr_widths fields.

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v2:
--> Improved commit message title and description 
as suggested by Vinod.

 drivers/dma/xilinx/xilinx_dma.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 88d317d..21ac954 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -2398,6 +2398,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
 		chan->direction = DMA_MEM_TO_DEV;
 		chan->id = chan_id;
 		chan->tdest = chan_id;
+		xdev->common.directions = BIT(DMA_MEM_TO_DEV);
 
 		chan->ctrl_offset = XILINX_DMA_MM2S_CTRL_OFFSET;
 		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
@@ -2415,6 +2416,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
 		chan->direction = DMA_DEV_TO_MEM;
 		chan->id = chan_id;
 		chan->tdest = chan_id - xdev->nr_channels;
+		xdev->common.directions |= BIT(DMA_DEV_TO_MEM);
 
 		chan->ctrl_offset = XILINX_DMA_S2MM_CTRL_OFFSET;
 		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
@@ -2629,6 +2631,8 @@ static int xilinx_dma_probe(struct platform_device *pdev)
 		dma_cap_set(DMA_PRIVATE, xdev->common.cap_mask);
 	}
 
+	xdev->common.dst_addr_widths = BIT(addr_width / 8);
+	xdev->common.src_addr_widths = BIT(addr_width / 8);
 	xdev->common.device_alloc_chan_resources =
 				xilinx_dma_alloc_chan_resources;
 	xdev->common.device_free_chan_resources =
-- 
2.7.4

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Vinod Koul]

On Wed, Jan 03, 2018 at 12:12:08PM +0530, Kedareswara rao Appana wrote:
> When client driver uses dma_get_slave_caps() api,
> it checks for certain fields of dma_device struct
> currently driver is not settings the directions and addr_widths
> fields resulting dma_get_slave_caps() returning failure.
> 
> This patch fixes this issue by populating proper values
> to the struct dma_device directions and addr_widths fields.
> 
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> Changes for v2:
> --> Improved commit message title and description 
> as suggested by Vinod.
> 
>  drivers/dma/xilinx/xilinx_dma.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 88d317d..21ac954 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -2398,6 +2398,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>  		chan->direction = DMA_MEM_TO_DEV;
>  		chan->id = chan_id;
>  		chan->tdest = chan_id;
> +		xdev->common.directions = BIT(DMA_MEM_TO_DEV);
>  
>  		chan->ctrl_offset = XILINX_DMA_MM2S_CTRL_OFFSET;
>  		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
> @@ -2415,6 +2416,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>  		chan->direction = DMA_DEV_TO_MEM;
>  		chan->id = chan_id;
>  		chan->tdest = chan_id - xdev->nr_channels;
> +		xdev->common.directions |= BIT(DMA_DEV_TO_MEM);
>  
>  		chan->ctrl_offset = XILINX_DMA_S2MM_CTRL_OFFSET;
>  		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
> @@ -2629,6 +2631,8 @@ static int xilinx_dma_probe(struct platform_device *pdev)
>  		dma_cap_set(DMA_PRIVATE, xdev->common.cap_mask);
>  	}
>  
> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
> +	xdev->common.src_addr_widths = BIT(addr_width / 8);

Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers? What is
value of addr_width here typically? Usually controllers can support
different widths and this is a surprise that you support only one value

-- 
~Vinod

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Appana Durga Kedareswara Rao]

Hi Vinod,

	Thanks for the review.... 
<Snip>
>> @@ -2398,6 +2398,7 @@ static int xilinx_dma_chan_probe(struct
>xilinx_dma_device *xdev,
>>  		chan->direction = DMA_MEM_TO_DEV;
>>  		chan->id = chan_id;
>>  		chan->tdest = chan_id;
>> +		xdev->common.directions = BIT(DMA_MEM_TO_DEV);
>>
>>  		chan->ctrl_offset = XILINX_DMA_MM2S_CTRL_OFFSET;
>>  		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) { @@ -
>2415,6
>> +2416,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>>  		chan->direction = DMA_DEV_TO_MEM;
>>  		chan->id = chan_id;
>>  		chan->tdest = chan_id - xdev->nr_channels;
>> +		xdev->common.directions |= BIT(DMA_DEV_TO_MEM);
>>
>>  		chan->ctrl_offset = XILINX_DMA_S2MM_CTRL_OFFSET;
>>  		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) { @@ -
>2629,6
>> +2631,8 @@ static int xilinx_dma_probe(struct platform_device *pdev)
>>  		dma_cap_set(DMA_PRIVATE, xdev->common.cap_mask);
>>  	}
>>
>> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
>> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
>
>Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers? What is value
>of addr_width here typically? Usually controllers can support different widths and
>this is a surprise that you support only one value

Controller supports address width of 32 and 64.
addr_width typical values are 32-bit or 64-bit .
Here addr_width is device-tree parameter...
my understanding of src_addr_widths/dst_addr_widths is, it is a bit mask of the 
address with in bytes that DMA supports, please correct if my understanding is wrong.

Regards,
Kedar.

>
>--
>~Vinod

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Vinod Koul]

On Mon, Jan 08, 2018 at 10:52:01AM +0000, Appana Durga Kedareswara Rao wrote:
> Hi Vinod,
> 
> 	Thanks for the review.... 
> <Snip>
> >> @@ -2398,6 +2398,7 @@ static int xilinx_dma_chan_probe(struct
> >xilinx_dma_device *xdev,
> >>  		chan->direction = DMA_MEM_TO_DEV;
> >>  		chan->id = chan_id;
> >>  		chan->tdest = chan_id;
> >> +		xdev->common.directions = BIT(DMA_MEM_TO_DEV);
> >>
> >>  		chan->ctrl_offset = XILINX_DMA_MM2S_CTRL_OFFSET;
> >>  		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) { @@ -
> >2415,6
> >> +2416,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
> >>  		chan->direction = DMA_DEV_TO_MEM;
> >>  		chan->id = chan_id;
> >>  		chan->tdest = chan_id - xdev->nr_channels;
> >> +		xdev->common.directions |= BIT(DMA_DEV_TO_MEM);
> >>
> >>  		chan->ctrl_offset = XILINX_DMA_S2MM_CTRL_OFFSET;
> >>  		if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) { @@ -
> >2629,6
> >> +2631,8 @@ static int xilinx_dma_probe(struct platform_device *pdev)
> >>  		dma_cap_set(DMA_PRIVATE, xdev->common.cap_mask);
> >>  	}
> >>
> >> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
> >> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
> >
> >Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers? What is value
> >of addr_width here typically? Usually controllers can support different widths and
> >this is a surprise that you support only one value
> 
> Controller supports address width of 32 and 64.

Then this should have both 32 and 64 values here

> addr_width typical values are 32-bit or 64-bit .
> Here addr_width is device-tree parameter...
> my understanding of src_addr_widths/dst_addr_widths is, it is a bit mask of the 
> address with in bytes that DMA supports, please correct if my understanding is wrong.
> 
> Regards,
> Kedar.
> 
> >
> >--
> >~Vinod

-- 
~Vinod

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Appana Durga Kedareswara Rao]

Hi,

<Snip>
>> >> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
>> >> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
>> >
>> >Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers?
>> >What is value of addr_width here typically? Usually controllers can
>> >support different widths and this is a surprise that you support only
>> >one value
>>
>> Controller supports address width of 32 and 64.
>
>Then this should have both 32 and 64 values here

Address width is configurable parameter at the h/w level.
Since this IP is a soft IP user can create a design with either 
32-bit or 64-bit address configuration. 
Currently we are reading this configuration through device-tree (xlnx, addr-width property) 
https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git/tree/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt#n19
Based on the h/w configuration setting the dst_addr_widths/src_addr_widths variables in this patch.
Please let me know if you are still not clear with my explanation will explain in detail... 

Regards,
Kedar.

>
>> addr_width typical values are 32-bit or 64-bit .
>> Here addr_width is device-tree parameter...
>> my understanding of src_addr_widths/dst_addr_widths is, it is a bit
>> mask of the address with in bytes that DMA supports, please correct if my
>understanding is wrong.
>>
>> Regards,
>> Kedar.
>>
>> >
>> >--
>> >~Vinod
>
>--
>~Vinod

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Vinod Koul]

On Mon, Jan 08, 2018 at 05:25:01PM +0000, Appana Durga Kedareswara Rao wrote:
> Hi,
> 
> <Snip>
> >> >> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
> >> >> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
> >> >
> >> >Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers?
> >> >What is value of addr_width here typically? Usually controllers can
> >> >support different widths and this is a surprise that you support only
> >> >one value
> >>
> >> Controller supports address width of 32 and 64.
> >
> >Then this should have both 32 and 64 values here
> 
> Address width is configurable parameter at the h/w level.
> Since this IP is a soft IP user can create a design with either 
> 32-bit or 64-bit address configuration. 

and not both right?

> Currently we are reading this configuration through device-tree (xlnx, addr-width property) 
> https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git/tree/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt#n19
> Based on the h/w configuration setting the dst_addr_widths/src_addr_widths variables in this patch.
> Please let me know if you are still not clear with my explanation will explain in detail... 
> 
> Regards,
> Kedar.
> 
> >
> >> addr_width typical values are 32-bit or 64-bit .
> >> Here addr_width is device-tree parameter...
> >> my understanding of src_addr_widths/dst_addr_widths is, it is a bit
> >> mask of the address with in bytes that DMA supports, please correct if my
> >understanding is wrong.
> >>
> >> Regards,
> >> Kedar.
> >>
> >> >
> >> >--
> >> >~Vinod
> >
> >--
> >~Vinod

-- 
~Vinod

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Appana Durga Kedareswara Rao]

Hi,

>On Mon, Jan 08, 2018 at 05:25:01PM +0000, Appana Durga Kedareswara Rao
>wrote:
>> Hi,
>>
>> <Snip>
>> >> >> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
>> >> >> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
>> >> >
>> >> >Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers?
>> >> >What is value of addr_width here typically? Usually controllers
>> >> >can support different widths and this is a surprise that you
>> >> >support only one value
>> >>
>> >> Controller supports address width of 32 and 64.
>> >
>> >Then this should have both 32 and 64 values here
>>
>> Address width is configurable parameter at the h/w level.
>> Since this IP is a soft IP user can create a design with either 32-bit
>> or 64-bit address configuration.
>
>and not both right?

Yes not both at the same time... 
Axi dma controller can be configured for either 32-bit or 64-bit address...

Regards,
Kedar.

>
>> Currently we are reading this configuration through device-tree (xlnx,
>> addr-width property)
>> https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git/tr
>> ee/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt#n19
>> Based on the h/w configuration setting the dst_addr_widths/src_addr_widths
>variables in this patch.
>> Please let me know if you are still not clear with my explanation will explain in
>detail...
>>
>> Regards,
>> Kedar.
>>
>> >
>> >> addr_width typical values are 32-bit or 64-bit .
>> >> Here addr_width is device-tree parameter...
>> >> my understanding of src_addr_widths/dst_addr_widths is, it is a bit
>> >> mask of the address with in bytes that DMA supports, please correct
>> >> if my
>> >understanding is wrong.
>> >>
>> >> Regards,
>> >> Kedar.
>> >>
>> >> >
>> >> >--
>> >> >~Vinod
>> >
>> >--
>> >~Vinod
>
>--
>~Vinod
>--
>To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body
>of a message to majordomo at vger.kernel.org More majordomo info at
>http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Vinod Koul]

On Tue, Jan 09, 2018 at 04:48:10AM +0000, Appana Durga Kedareswara Rao wrote:
> Hi,
> 
> >On Mon, Jan 08, 2018 at 05:25:01PM +0000, Appana Durga Kedareswara Rao
> >wrote:
> >> Hi,
> >>
> >> <Snip>
> >> >> >> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
> >> >> >> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
> >> >> >
> >> >> >Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers?
> >> >> >What is value of addr_width here typically? Usually controllers
> >> >> >can support different widths and this is a surprise that you
> >> >> >support only one value
> >> >>
> >> >> Controller supports address width of 32 and 64.
> >> >
> >> >Then this should have both 32 and 64 values here
> >>
> >> Address width is configurable parameter at the h/w level.
> >> Since this IP is a soft IP user can create a design with either 32-bit
> >> or 64-bit address configuration.
> >
> >and not both right?
> 
> Yes not both at the same time... 
> Axi dma controller can be configured for either 32-bit or 64-bit address...

So my suspicion was correct.  I would suggest you to read up on the
documentation again. The src/dst_addr_widths has _nothing_ to do with 32/64
bit addresses used.

It is the capability of the dma controller to do transfers with data width as
8bits, 16 bits, so on. iKey is "data width" and not address type.
This typically translates to DMA FIFO configuration of the controller!

-- 
~Vinod

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Appana Durga Kedareswara Rao]

Hi,

	Thanks for the review... 

>On Tue, Jan 09, 2018 at 04:48:10AM +0000, Appana Durga Kedareswara Rao
>wrote:
>> Hi,
>>
>> >On Mon, Jan 08, 2018 at 05:25:01PM +0000, Appana Durga Kedareswara
>> >Rao
>> >wrote:
>> >> Hi,
>> >>
>> >> <Snip>
>> >> >> >> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
>> >> >> >> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
>> >> >> >
>> >> >> >Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers?
>> >> >> >What is value of addr_width here typically? Usually controllers
>> >> >> >can support different widths and this is a surprise that you
>> >> >> >support only one value
>> >> >>
>> >> >> Controller supports address width of 32 and 64.
>> >> >
>> >> >Then this should have both 32 and 64 values here
>> >>
>> >> Address width is configurable parameter at the h/w level.
>> >> Since this IP is a soft IP user can create a design with either
>> >> 32-bit or 64-bit address configuration.
>> >
>> >and not both right?
>>
>> Yes not both at the same time...
>> Axi dma controller can be configured for either 32-bit or 64-bit address...
>
>So my suspicion was correct.  I would suggest you to read up on the
>documentation again. The src/dst_addr_widths has _nothing_ to do with 32/64
>bit addresses used.
>
>It is the capability of the dma controller to do transfers with data width as 8bits,
>16 bits, so on. iKey is "data width" and not address type.
>This typically translates to DMA FIFO configuration of the controller!

Thanks for the detailed explanation... 
I have gone through the spec again controller does supports 1 byte, 2 byte, 4 byte up to 128 byte transfers.
In order to do variable length transfers user needs to drive a valid value to the tkeep strobe signal at the h/w level.
And user needs to configure the below parameters c_m_axis_mm2s_tdata_width or c_m_axis_s2mm_tdata_width
With desired configuration at the h/w level.
Controller supports data width of 8, 16, 32, 64, 128, 256, 512 and 1,024 bits 
(i.e. c_m_axis_mm2s_tdata_width/ c_m_axis_s2mm_tdata_width parameters range)

At the s/w level currently we are getting c_m_axis_mm2s_tdata_width/ c_m_axis_s2mm_tdata_width
Configuration as xlnx,datawidth property in the device-tree.

So proper values for the src/dst_addr width fields should be, datawidth property in bytes.
Please correct me if I am wrong... 

Changes looks like below...
Here width is in bytes based on the h/w configuration... 

--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -2411,6 +2411,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
                chan->direction = DMA_MEM_TO_DEV;
                chan->id = chan_id;
                chan->tdest = chan_id;
+               xdev->common.directions = BIT(DMA_MEM_TO_DEV);
+               xdev->common.src_addr_widths = BIT(width);
 
                chan->ctrl_offset = XILINX_DMA_MM2S_CTRL_OFFSET;
                if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
@@ -2428,6 +2430,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
                chan->direction = DMA_DEV_TO_MEM;
                chan->id = chan_id;
                chan->tdest = chan_id - xdev->nr_channels;
+               xdev->common.directions |= BIT(DMA_DEV_TO_MEM);
+               xdev->common.dst_addr_widths = BIT(width);
 
                chan->ctrl_offset = XILINX_DMA_S2MM_CTRL_OFFSET;
                if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {


Regards,
Kedar.

>
>--
>~Vinod

[PATCH v2 1/4] dmaengine: xilinx_dma: populate dma caps properly

[Vinod Koul]

On Tue, Jan 09, 2018 at 07:36:11AM +0000, Appana Durga Kedareswara Rao wrote:
> Hi,
> 
> 	Thanks for the review... 
> 
> >On Tue, Jan 09, 2018 at 04:48:10AM +0000, Appana Durga Kedareswara Rao
> >wrote:
> >> Hi,
> >>
> >> >On Mon, Jan 08, 2018 at 05:25:01PM +0000, Appana Durga Kedareswara
> >> >Rao
> >> >wrote:
> >> >> Hi,
> >> >>
> >> >> <Snip>
> >> >> >> >> +	xdev->common.dst_addr_widths = BIT(addr_width / 8);
> >> >> >> >> +	xdev->common.src_addr_widths = BIT(addr_width / 8);
> >> >> >> >
> >> >> >> >Do you not support trf of 1byte, 2 bytes, or 4 bytes wide transfers?
> >> >> >> >What is value of addr_width here typically? Usually controllers
> >> >> >> >can support different widths and this is a surprise that you
> >> >> >> >support only one value
> >> >> >>
> >> >> >> Controller supports address width of 32 and 64.
> >> >> >
> >> >> >Then this should have both 32 and 64 values here
> >> >>
> >> >> Address width is configurable parameter at the h/w level.
> >> >> Since this IP is a soft IP user can create a design with either
> >> >> 32-bit or 64-bit address configuration.
> >> >
> >> >and not both right?
> >>
> >> Yes not both at the same time...
> >> Axi dma controller can be configured for either 32-bit or 64-bit address...
> >
> >So my suspicion was correct.  I would suggest you to read up on the
> >documentation again. The src/dst_addr_widths has _nothing_ to do with 32/64
> >bit addresses used.
> >
> >It is the capability of the dma controller to do transfers with data width as 8bits,
> >16 bits, so on. iKey is "data width" and not address type.
> >This typically translates to DMA FIFO configuration of the controller!
> 
> Thanks for the detailed explanation... 

Welcome but I don't understand why you xilinx folks cant wrap your replies,
it is *very* hard to read on screens with 80chars

> I have gone through the spec again controller does supports 1 byte, 2
> byte, 4 byte up to 128 byte transfers.

I think you are talking about length and NOT width

> In order to do variable length transfers user needs to drive a valid value
> to the tkeep strobe signal at the h/w level.

bingo

> And user needs to configure the below parameters c_m_axis_mm2s_tdata_width
> or c_m_axis_s2mm_tdata_width With desired configuration at the h/w level.
> Controller supports data width of 8, 16, 32, 64, 128, 256, 512 and 1,024
> bits (i.e. c_m_axis_mm2s_tdata_width/ c_m_axis_s2mm_tdata_width parameters
> range)

holy cow, did you read that right. and 1024 bit width, how wide is your FIFO

I think you are off the rails here, please get your length and width
concepts right, they are NOT same and refer to different things and cannot be
used interchangably

> 
> At the s/w level currently we are getting c_m_axis_mm2s_tdata_width/ c_m_axis_s2mm_tdata_width
> Configuration as xlnx,datawidth property in the device-tree.
> 
> So proper values for the src/dst_addr width fields should be, datawidth property in bytes.
> Please correct me if I am wrong... 
> 
> Changes looks like below...
> Here width is in bytes based on the h/w configuration... 
> 
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -2411,6 +2411,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>                 chan->direction = DMA_MEM_TO_DEV;
>                 chan->id = chan_id;
>                 chan->tdest = chan_id;
> +               xdev->common.directions = BIT(DMA_MEM_TO_DEV);
> +               xdev->common.src_addr_widths = BIT(width);
>  
>                 chan->ctrl_offset = XILINX_DMA_MM2S_CTRL_OFFSET;
>                 if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
> @@ -2428,6 +2430,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
>                 chan->direction = DMA_DEV_TO_MEM;
>                 chan->id = chan_id;
>                 chan->tdest = chan_id - xdev->nr_channels;
> +               xdev->common.directions |= BIT(DMA_DEV_TO_MEM);
> +               xdev->common.dst_addr_widths = BIT(width);
>  
>                 chan->ctrl_offset = XILINX_DMA_S2MM_CTRL_OFFSET;
>                 if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
> 
> 
> Regards,
> Kedar.
> 
> >
> >--
> >~Vinod
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
~Vinod

[PATCH v2 2/4] dmaengine: xilinx_dma: properly configure the SG mode bit in the driver for cdma

[Kedareswara rao Appana]

If the hardware is configured for Scatter Gather(SG) mode,
and hardware is idle, in the control register SG mode bit
must be set to a 0 then back to 1 by the software, to force
the CDMA SG engine to use a new value written to the CURDESC_PNTR
register, failure to do so could result errors from the dmaengine.

This patch updates the same.

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v2:
--> Improved commit message title and description 
as suggested by Vinod.

 drivers/dma/xilinx/xilinx_dma.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 21ac954..8467671 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1204,6 +1204,12 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
 	}
 
 	if (chan->has_sg) {
+		dma_ctrl_clr(chan, XILINX_DMA_REG_DMACR,
+			     XILINX_CDMA_CR_SGMODE);
+
+		dma_ctrl_set(chan, XILINX_DMA_REG_DMACR,
+			     XILINX_CDMA_CR_SGMODE);
+
 		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
 			     head_desc->async_tx.phys);
 
@@ -2052,6 +2058,10 @@ static int xilinx_dma_terminate_all(struct dma_chan *dchan)
 		chan->cyclic = false;
 	}
 
+	if ((chan->xdev->dma_config->dmatype == XDMA_TYPE_CDMA) && chan->has_sg)
+		dma_ctrl_clr(chan, XILINX_DMA_REG_DMACR,
+			     XILINX_CDMA_CR_SGMODE);
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v2 2/4] dmaengine: xilinx_dma: properly configure the SG mode bit in the driver for cdma

[Vinod Koul]

On Wed, Jan 03, 2018 at 12:12:09PM +0530, Kedareswara rao Appana wrote:
> If the hardware is configured for Scatter Gather(SG) mode,
> and hardware is idle, in the control register SG mode bit
> must be set to a 0 then back to 1 by the software, to force
> the CDMA SG engine to use a new value written to the CURDESC_PNTR
> register, failure to do so could result errors from the dmaengine.

Applied 2-4, thanks

-- 
~Vinod

[PATCH v2 3/4] dmaengine: xilinx_dma: Fix warning variable prev set but not used

[Kedareswara rao Appana]

This patch fixes the below sparse warning in the driver
drivers/dma/xilinx/xilinx_dma.c: In function ?xilinx_vdma_dma_prep_interleaved?:
drivers/dma/xilinx/xilinx_dma.c:1614:43: warning: variable ?prev? set but not used [-Wunused-but-set-variable]
  struct xilinx_vdma_tx_segment *segment, *prev = NULL;

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v2:
--> Improved commit message title and description 
as suggested by Vinod.

 drivers/dma/xilinx/xilinx_dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8467671..845e638 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1611,7 +1611,7 @@ xilinx_vdma_dma_prep_interleaved(struct dma_chan *dchan,
 {
 	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
 	struct xilinx_dma_tx_descriptor *desc;
-	struct xilinx_vdma_tx_segment *segment, *prev = NULL;
+	struct xilinx_vdma_tx_segment *segment;
 	struct xilinx_vdma_desc_hw *hw;
 
 	if (!is_slave_direction(xt->dir))
@@ -1665,8 +1665,6 @@ xilinx_vdma_dma_prep_interleaved(struct dma_chan *dchan,
 	/* Insert the segment into the descriptor segments list. */
 	list_add_tail(&segment->node, &desc->segments);
 
-	prev = segment;
-
 	/* Link the last hardware descriptor with the first. */
 	segment = list_first_entry(&desc->segments,
 				   struct xilinx_vdma_tx_segment, node);
-- 
2.7.4

[PATCH v2 4/4] dmaengine: xilinx_dma: Free BD consistent memory

[Kedareswara rao Appana]

Free BD consistent memory while freeing the channel
i.e in free_chan_resources.

Signed-off-by: Radhey Shyam Pandey <radheys@xilinx.com>
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v2:
--> None.

 drivers/dma/xilinx/xilinx_dma.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 845e638..a9edbd8 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -764,6 +764,11 @@ static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
 		INIT_LIST_HEAD(&chan->free_seg_list);
 		spin_unlock_irqrestore(&chan->lock, flags);
 
+		/* Free memory that is allocated for BD */
+		dma_free_coherent(chan->dev, sizeof(*chan->seg_v) *
+				  XILINX_DMA_NUM_DESCS, chan->seg_v,
+				  chan->seg_p);
+
 		/* Free Memory that is allocated for cyclic DMA Mode */
 		dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v),
 				  chan->cyclic_seg_v, chan->cyclic_seg_p);
-- 
2.7.4