| Message ID | 1514961731-1916-2-git-send-email-appanad@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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@vger.kernel.org More majordomo info at >http://vger.kernel.org/majordomo-info.html
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
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!
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 =
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(+)