| Message ID | 1437447666-7012-1-git-send-email-jun.nie@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
2015-07-21 11:01 GMT+08:00 Jun Nie <jun.nie@linaro.org>: > Align src and dst width to fix data alignment issue. Hardware > burst length limitation can be addressed well too. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/dma/zx296702_dma.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c > index ec470bc..4757f74 100644 > --- a/drivers/dma/zx296702_dma.c > +++ b/drivers/dma/zx296702_dma.c > @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) > > val = readl_relaxed(phy->base + REG_ZX_CTRL); > val &= ~ZX_CH_ENABLE; > + val |= ZX_FORCE_CLOSE; > writel_relaxed(val, phy->base + REG_ZX_CTRL); > > val = 0x1 << phy->idx; > @@ -475,13 +476,12 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) > * We need make sure dst len not exceed MAX LEN. > */ > dst_width = zx_dma_burst_width(cfg->dst_addr_width); > - maxburst = cfg->dst_maxburst * cfg->dst_addr_width > - / DMA_SLAVE_BUSWIDTH_8_BYTES; > + maxburst = cfg->dst_maxburst; > maxburst = maxburst < ZX_MAX_BURST_LEN ? > maxburst : ZX_MAX_BURST_LEN; > c->ccfg = ZX_DST_FIFO_MODE | ZX_CH_ENABLE > | ZX_SRC_BURST_LEN(maxburst - 1) > - | ZX_SRC_BURST_WIDTH(ZX_DMA_WIDTH_64BIT) > + | ZX_SRC_BURST_WIDTH(dst_width) > | ZX_DST_BURST_WIDTH(dst_width); > break; > case DMA_DEV_TO_MEM: > @@ -493,7 +493,7 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) > c->ccfg = ZX_SRC_FIFO_MODE | ZX_CH_ENABLE > | ZX_SRC_BURST_LEN(maxburst - 1) > | ZX_SRC_BURST_WIDTH(src_width) > - | ZX_DST_BURST_WIDTH(ZX_DMA_WIDTH_64BIT); > + | ZX_DST_BURST_WIDTH(src_width); > break; > default: > return -EINVAL; > -- > 1.9.1 > Hi Vinod, The two patches are based on your topic branch topic/zxdma. Please help review and apply to that branch. Thank you! Jun -- 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
2015-07-21 11:02 GMT+08:00 Jun Nie <jun.nie@linaro.org>: > 2015-07-21 11:01 GMT+08:00 Jun Nie <jun.nie@linaro.org>: >> Align src and dst width to fix data alignment issue. Hardware >> burst length limitation can be addressed well too. >> >> Signed-off-by: Jun Nie <jun.nie@linaro.org> >> --- >> drivers/dma/zx296702_dma.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c >> index ec470bc..4757f74 100644 >> --- a/drivers/dma/zx296702_dma.c >> +++ b/drivers/dma/zx296702_dma.c >> @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) >> >> val = readl_relaxed(phy->base + REG_ZX_CTRL); >> val &= ~ZX_CH_ENABLE; >> + val |= ZX_FORCE_CLOSE; >> writel_relaxed(val, phy->base + REG_ZX_CTRL); >> >> val = 0x1 << phy->idx; >> @@ -475,13 +476,12 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) >> * We need make sure dst len not exceed MAX LEN. >> */ >> dst_width = zx_dma_burst_width(cfg->dst_addr_width); >> - maxburst = cfg->dst_maxburst * cfg->dst_addr_width >> - / DMA_SLAVE_BUSWIDTH_8_BYTES; >> + maxburst = cfg->dst_maxburst; >> maxburst = maxburst < ZX_MAX_BURST_LEN ? >> maxburst : ZX_MAX_BURST_LEN; >> c->ccfg = ZX_DST_FIFO_MODE | ZX_CH_ENABLE >> | ZX_SRC_BURST_LEN(maxburst - 1) >> - | ZX_SRC_BURST_WIDTH(ZX_DMA_WIDTH_64BIT) >> + | ZX_SRC_BURST_WIDTH(dst_width) >> | ZX_DST_BURST_WIDTH(dst_width); >> break; >> case DMA_DEV_TO_MEM: >> @@ -493,7 +493,7 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) >> c->ccfg = ZX_SRC_FIFO_MODE | ZX_CH_ENABLE >> | ZX_SRC_BURST_LEN(maxburst - 1) >> | ZX_SRC_BURST_WIDTH(src_width) >> - | ZX_DST_BURST_WIDTH(ZX_DMA_WIDTH_64BIT); >> + | ZX_DST_BURST_WIDTH(src_width); >> break; >> default: >> return -EINVAL; >> -- >> 1.9.1 >> > Hi Vinod, > > The two patches are based on your topic branch topic/zxdma. Please > help review and apply to that branch. Thank you! > > Jun Vinod, Could you help check these patches? Thank you! Jun -- 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
2015-07-29 14:10 GMT+08:00 Jun Nie <jun.nie@linaro.org>: > 2015-07-21 11:02 GMT+08:00 Jun Nie <jun.nie@linaro.org>: >> 2015-07-21 11:01 GMT+08:00 Jun Nie <jun.nie@linaro.org>: >>> Align src and dst width to fix data alignment issue. Hardware >>> burst length limitation can be addressed well too. >>> >>> Signed-off-by: Jun Nie <jun.nie@linaro.org> >>> --- >>> drivers/dma/zx296702_dma.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c >>> index ec470bc..4757f74 100644 >>> --- a/drivers/dma/zx296702_dma.c >>> +++ b/drivers/dma/zx296702_dma.c >>> @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) >>> >>> val = readl_relaxed(phy->base + REG_ZX_CTRL); >>> val &= ~ZX_CH_ENABLE; >>> + val |= ZX_FORCE_CLOSE; >>> writel_relaxed(val, phy->base + REG_ZX_CTRL); >>> >>> val = 0x1 << phy->idx; >>> @@ -475,13 +476,12 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) >>> * We need make sure dst len not exceed MAX LEN. >>> */ >>> dst_width = zx_dma_burst_width(cfg->dst_addr_width); >>> - maxburst = cfg->dst_maxburst * cfg->dst_addr_width >>> - / DMA_SLAVE_BUSWIDTH_8_BYTES; >>> + maxburst = cfg->dst_maxburst; >>> maxburst = maxburst < ZX_MAX_BURST_LEN ? >>> maxburst : ZX_MAX_BURST_LEN; >>> c->ccfg = ZX_DST_FIFO_MODE | ZX_CH_ENABLE >>> | ZX_SRC_BURST_LEN(maxburst - 1) >>> - | ZX_SRC_BURST_WIDTH(ZX_DMA_WIDTH_64BIT) >>> + | ZX_SRC_BURST_WIDTH(dst_width) >>> | ZX_DST_BURST_WIDTH(dst_width); >>> break; >>> case DMA_DEV_TO_MEM: >>> @@ -493,7 +493,7 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) >>> c->ccfg = ZX_SRC_FIFO_MODE | ZX_CH_ENABLE >>> | ZX_SRC_BURST_LEN(maxburst - 1) >>> | ZX_SRC_BURST_WIDTH(src_width) >>> - | ZX_DST_BURST_WIDTH(ZX_DMA_WIDTH_64BIT); >>> + | ZX_DST_BURST_WIDTH(src_width); >>> break; >>> default: >>> return -EINVAL; >>> -- >>> 1.9.1 >>> >> Hi Vinod, >> >> The two patches are based on your topic branch topic/zxdma. Please >> help review and apply to that branch. Thank you! >> >> Jun > > Vinod, > > Could you help check these patches? Thank you! > > Jun Hi Vinod, Any comments on the 2 patches here? Jun -- 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 Tue, Jul 21, 2015 at 11:01:05AM +0800, Jun Nie wrote: > Align src and dst width to fix data alignment issue. Hardware > burst length limitation can be addressed well too. What is the data alignment issue you have? Why not document that in code and changelog? > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/dma/zx296702_dma.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c > index ec470bc..4757f74 100644 > --- a/drivers/dma/zx296702_dma.c > +++ b/drivers/dma/zx296702_dma.c > @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) > > val = readl_relaxed(phy->base + REG_ZX_CTRL); > val &= ~ZX_CH_ENABLE; > + val |= ZX_FORCE_CLOSE; how is this related to data width?
2015-08-05 11:24 GMT+08:00 Vinod Koul <vinod.koul@intel.com>: > On Tue, Jul 21, 2015 at 11:01:05AM +0800, Jun Nie wrote: >> Align src and dst width to fix data alignment issue. Hardware >> burst length limitation can be addressed well too. > What is the data alignment issue you have? Why not document that in code and > changelog? > > >> >> Signed-off-by: Jun Nie <jun.nie@linaro.org> >> --- >> drivers/dma/zx296702_dma.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c >> index ec470bc..4757f74 100644 >> --- a/drivers/dma/zx296702_dma.c >> +++ b/drivers/dma/zx296702_dma.c >> @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) >> >> val = readl_relaxed(phy->base + REG_ZX_CTRL); >> val &= ~ZX_CH_ENABLE; >> + val |= ZX_FORCE_CLOSE; > how is this related to data width? Burst length has limitation and destination burst length is calculated from other src/dst parameters. So it is safe to align src/dst data width to avoid to run into limitation. Jun > > -- > ~Vinod >> writel_relaxed(val, phy->base + REG_ZX_CTRL); >> >> val = 0x1 << phy->idx; >> @@ -475,13 +476,12 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) >> * We need make sure dst len not exceed MAX LEN. >> */ >> dst_width = zx_dma_burst_width(cfg->dst_addr_width); >> - maxburst = cfg->dst_maxburst * cfg->dst_addr_width >> - / DMA_SLAVE_BUSWIDTH_8_BYTES; >> + maxburst = cfg->dst_maxburst; >> maxburst = maxburst < ZX_MAX_BURST_LEN ? >> maxburst : ZX_MAX_BURST_LEN; >> c->ccfg = ZX_DST_FIFO_MODE | ZX_CH_ENABLE >> | ZX_SRC_BURST_LEN(maxburst - 1) >> - | ZX_SRC_BURST_WIDTH(ZX_DMA_WIDTH_64BIT) >> + | ZX_SRC_BURST_WIDTH(dst_width) >> | ZX_DST_BURST_WIDTH(dst_width); >> break; >> case DMA_DEV_TO_MEM: >> @@ -493,7 +493,7 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) >> c->ccfg = ZX_SRC_FIFO_MODE | ZX_CH_ENABLE >> | ZX_SRC_BURST_LEN(maxburst - 1) >> | ZX_SRC_BURST_WIDTH(src_width) >> - | ZX_DST_BURST_WIDTH(ZX_DMA_WIDTH_64BIT); >> + | ZX_DST_BURST_WIDTH(src_width); >> break; >> default: >> return -EINVAL; >> -- >> 1.9.1 -- 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 Wed, Aug 05, 2015 at 11:32:00AM +0800, Jun Nie wrote: > 2015-08-05 11:24 GMT+08:00 Vinod Koul <vinod.koul@intel.com>: > > On Tue, Jul 21, 2015 at 11:01:05AM +0800, Jun Nie wrote: > >> Align src and dst width to fix data alignment issue. Hardware > >> burst length limitation can be addressed well too. > > What is the data alignment issue you have? Why not document that in code and > > changelog? ?? > >> > >> Signed-off-by: Jun Nie <jun.nie@linaro.org> > >> --- > >> drivers/dma/zx296702_dma.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c > >> index ec470bc..4757f74 100644 > >> --- a/drivers/dma/zx296702_dma.c > >> +++ b/drivers/dma/zx296702_dma.c > >> @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) > >> > >> val = readl_relaxed(phy->base + REG_ZX_CTRL); > >> val &= ~ZX_CH_ENABLE; > >> + val |= ZX_FORCE_CLOSE; > > how is this related to data width? > > Burst length has limitation and destination burst length is calculated > from other src/dst parameters. So it is safe to align src/dst data > width to avoid to run into limitation. thats fine but what does it have to do with terminate and ZX_FORCE_CLOSE?
2015-08-05 11:38 GMT+08:00 Vinod Koul <vinod.koul@intel.com>: > On Wed, Aug 05, 2015 at 11:32:00AM +0800, Jun Nie wrote: >> 2015-08-05 11:24 GMT+08:00 Vinod Koul <vinod.koul@intel.com>: >> > On Tue, Jul 21, 2015 at 11:01:05AM +0800, Jun Nie wrote: >> >> Align src and dst width to fix data alignment issue. Hardware >> >> burst length limitation can be addressed well too. >> > What is the data alignment issue you have? Why not document that in code and >> > changelog? > > ?? > When buffer size is not multiple of burst length * data width, src/dst data width should be identical for these residue data. Will doc it in commit log and code in next version patch. > >> >> >> >> Signed-off-by: Jun Nie <jun.nie@linaro.org> >> >> --- >> >> drivers/dma/zx296702_dma.c | 8 ++++---- >> >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> >> >> diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c >> >> index ec470bc..4757f74 100644 >> >> --- a/drivers/dma/zx296702_dma.c >> >> +++ b/drivers/dma/zx296702_dma.c >> >> @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) >> >> >> >> val = readl_relaxed(phy->base + REG_ZX_CTRL); >> >> val &= ~ZX_CH_ENABLE; >> >> + val |= ZX_FORCE_CLOSE; >> > how is this related to data width? >> >> Burst length has limitation and destination burst length is calculated >> from other src/dst parameters. So it is safe to align src/dst data >> width to avoid to run into limitation. > > thats fine but what does it have to do with terminate and ZX_FORCE_CLOSE? It fix other bug that's really do not relate to data width. Hardware will not stop till all transmit ion is done when enable bit is cleared. Cyclic DMA does not stop without the change as a result of ring DMA chain. Seems it should be changed in cyclic dma patch. A new patch should be prepare now, right? Jun > > -- > ~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 Wed, Aug 05, 2015 at 12:15:05PM +0800, Jun Nie wrote: > 2015-08-05 11:38 GMT+08:00 Vinod Koul <vinod.koul@intel.com>: > > On Wed, Aug 05, 2015 at 11:32:00AM +0800, Jun Nie wrote: > >> 2015-08-05 11:24 GMT+08:00 Vinod Koul <vinod.koul@intel.com>: > >> > On Tue, Jul 21, 2015 at 11:01:05AM +0800, Jun Nie wrote: > >> >> Align src and dst width to fix data alignment issue. Hardware > >> >> burst length limitation can be addressed well too. > >> > What is the data alignment issue you have? Why not document that in code and > >> > changelog? > > > > ?? > > > When buffer size is not multiple of burst length * data width, src/dst > data width should be identical for these residue data. Will doc it in > commit log and code in next version patch. > > > >> >> > >> >> Signed-off-by: Jun Nie <jun.nie@linaro.org> > >> >> --- > >> >> drivers/dma/zx296702_dma.c | 8 ++++---- > >> >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> >> > >> >> diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c > >> >> index ec470bc..4757f74 100644 > >> >> --- a/drivers/dma/zx296702_dma.c > >> >> +++ b/drivers/dma/zx296702_dma.c > >> >> @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) > >> >> > >> >> val = readl_relaxed(phy->base + REG_ZX_CTRL); > >> >> val &= ~ZX_CH_ENABLE; > >> >> + val |= ZX_FORCE_CLOSE; > >> > how is this related to data width? > >> > >> Burst length has limitation and destination burst length is calculated > >> from other src/dst parameters. So it is safe to align src/dst data > >> width to avoid to run into limitation. > > > > thats fine but what does it have to do with terminate and ZX_FORCE_CLOSE? > > It fix other bug that's really do not relate to data width. Hardware > will not stop till all transmit ion is done when enable bit is > cleared. Cyclic DMA does not stop without the change as a result of > ring DMA chain. Seems it should be changed in cyclic dma patch. A new > patch should be prepare now, right? Yes please.. I have applied cyclic patch so would prefer updates...
diff --git a/drivers/dma/zx296702_dma.c b/drivers/dma/zx296702_dma.c index ec470bc..4757f74 100644 --- a/drivers/dma/zx296702_dma.c +++ b/drivers/dma/zx296702_dma.c @@ -143,6 +143,7 @@ static void zx_dma_terminate_chan(struct zx_dma_phy *phy, struct zx_dma_dev *d) val = readl_relaxed(phy->base + REG_ZX_CTRL); val &= ~ZX_CH_ENABLE; + val |= ZX_FORCE_CLOSE; writel_relaxed(val, phy->base + REG_ZX_CTRL); val = 0x1 << phy->idx; @@ -475,13 +476,12 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) * We need make sure dst len not exceed MAX LEN. */ dst_width = zx_dma_burst_width(cfg->dst_addr_width); - maxburst = cfg->dst_maxburst * cfg->dst_addr_width - / DMA_SLAVE_BUSWIDTH_8_BYTES; + maxburst = cfg->dst_maxburst; maxburst = maxburst < ZX_MAX_BURST_LEN ? maxburst : ZX_MAX_BURST_LEN; c->ccfg = ZX_DST_FIFO_MODE | ZX_CH_ENABLE | ZX_SRC_BURST_LEN(maxburst - 1) - | ZX_SRC_BURST_WIDTH(ZX_DMA_WIDTH_64BIT) + | ZX_SRC_BURST_WIDTH(dst_width) | ZX_DST_BURST_WIDTH(dst_width); break; case DMA_DEV_TO_MEM: @@ -493,7 +493,7 @@ static int zx_pre_config(struct zx_dma_chan *c, enum dma_transfer_direction dir) c->ccfg = ZX_SRC_FIFO_MODE | ZX_CH_ENABLE | ZX_SRC_BURST_LEN(maxburst - 1) | ZX_SRC_BURST_WIDTH(src_width) - | ZX_DST_BURST_WIDTH(ZX_DMA_WIDTH_64BIT); + | ZX_DST_BURST_WIDTH(src_width); break; default: return -EINVAL;
Align src and dst width to fix data alignment issue. Hardware burst length limitation can be addressed well too. Signed-off-by: Jun Nie <jun.nie@linaro.org> --- drivers/dma/zx296702_dma.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)