| Message ID | CY1PR02MB1692ED5378E474DB42A9CD7ADC100@CY1PR02MB1692.namprd02.prod.outlook.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
--- 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) {