| Message ID | 20170917142510.3111-1-stefan.bruens@rwth-aachen.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sun, Sep 17, 2017 at 04:25:10PM +0200, Stefan Brüns wrote: > The driver would happily accept buswidth of 16/32/64 bytes and program > garbage to its registers. Peter? > > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> > --- > drivers/dma/edma.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > index 3879f80a4815..c944ea339425 100644 > --- a/drivers/dma/edma.c > +++ b/drivers/dma/edma.c > @@ -887,8 +887,8 @@ static int edma_slave_config(struct dma_chan *chan, > { > struct edma_chan *echan = to_edma_chan(chan); > > - if (cfg->src_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES || > - cfg->dst_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES) > + if (cfg->src_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES || > + cfg->dst_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES) > return -EINVAL; > > memcpy(&echan->cfg, cfg, sizeof(echan->cfg)); > -- > 2.14.1 >
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 2017-09-21 20:17, Vinod Koul wrote: > On Sun, Sep 17, 2017 at 04:25:10PM +0200, Stefan Brüns wrote: >> The driver would happily accept buswidth of 16/32/64 bytes and program >> garbage to its registers. > > Peter? Well, eDMA as such has no limitation on the buswidth, the addr_width become ACNT, counter for the inner loop - which we use to represent the word length. In theory we could support 32K-1 long word with eDMA, which we actually do in case of memcpy. But the driver does state that it supports bus width of 1, 2, 3 and 4 bytes. These are form the past and I never felt the need to check them. I think it is fine to reject salve config when asking for longer dev width. I might revisit later if the need comes.. Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > >> >> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> >> --- >> drivers/dma/edma.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c >> index 3879f80a4815..c944ea339425 100644 >> --- a/drivers/dma/edma.c >> +++ b/drivers/dma/edma.c >> @@ -887,8 +887,8 @@ static int edma_slave_config(struct dma_chan *chan, >> { >> struct edma_chan *echan = to_edma_chan(chan); >> >> - if (cfg->src_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES || >> - cfg->dst_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES) >> + if (cfg->src_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES || >> + cfg->dst_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES) >> return -EINVAL; >> >> memcpy(&echan->cfg, cfg, sizeof(echan->cfg)); >> -- >> 2.14.1 >> > - Péter -- 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
Stefan, Vinod, Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 2017-09-21 21:29, Peter Ujfalusi wrote: > > > On 2017-09-21 20:17, Vinod Koul wrote: >> On Sun, Sep 17, 2017 at 04:25:10PM +0200, Stefan Brüns wrote: >>> The driver would happily accept buswidth of 16/32/64 bytes and program >>> garbage to its registers. >> >> Peter? > > Well, eDMA as such has no limitation on the buswidth, the addr_width > become ACNT, counter for the inner loop - which we use to represent the > word length. In theory we could support 32K-1 long word with eDMA, which > we actually do in case of memcpy. > > But the driver does state that it supports bus width of 1, 2, 3 and 4 bytes. > > These are form the past and I never felt the need to check them. > > I think it is fine to reject salve config when asking for longer dev > width. I might revisit later if the need comes.. > > Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com> If it is not too late, I have second thoughts about this.. It might be better to remove the src_addr_width/dst_addr_width check from edma_slave_config() and add all DMA_SLAVE_BUSWIDTH_* to EDMA_DMA_BUSWIDTHS. > >> >>> >>> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> >>> --- >>> drivers/dma/edma.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c >>> index 3879f80a4815..c944ea339425 100644 >>> --- a/drivers/dma/edma.c >>> +++ b/drivers/dma/edma.c >>> @@ -887,8 +887,8 @@ static int edma_slave_config(struct dma_chan *chan, >>> { >>> struct edma_chan *echan = to_edma_chan(chan); >>> >>> - if (cfg->src_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES || >>> - cfg->dst_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES) >>> + if (cfg->src_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES || >>> + cfg->dst_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES) >>> return -EINVAL; >>> >>> memcpy(&echan->cfg, cfg, sizeof(echan->cfg)); >>> -- >>> 2.14.1 >>> >> > > - Péter > - Péter -- 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 Fri, Sep 22, 2017 at 09:16:13AM +0300, Peter Ujfalusi wrote: > Stefan, Vinod, > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 2017-09-21 21:29, Peter Ujfalusi wrote: > > > > > > On 2017-09-21 20:17, Vinod Koul wrote: > >> On Sun, Sep 17, 2017 at 04:25:10PM +0200, Stefan Brüns wrote: > >>> The driver would happily accept buswidth of 16/32/64 bytes and program > >>> garbage to its registers. > >> > >> Peter? > > > > Well, eDMA as such has no limitation on the buswidth, the addr_width > > become ACNT, counter for the inner loop - which we use to represent the > > word length. In theory we could support 32K-1 long word with eDMA, which > > we actually do in case of memcpy. > > > > But the driver does state that it supports bus width of 1, 2, 3 and 4 bytes. > > > > These are form the past and I never felt the need to check them. > > > > I think it is fine to reject salve config when asking for longer dev > > width. I might revisit later if the need comes.. > > > > Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > If it is not too late, I have second thoughts about this.. > It might be better to remove the src_addr_width/dst_addr_width check > from edma_slave_config() and add all DMA_SLAVE_BUSWIDTH_* to > EDMA_DMA_BUSWIDTHS. fine by me, pls send updated..
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 3879f80a4815..c944ea339425 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -887,8 +887,8 @@ static int edma_slave_config(struct dma_chan *chan, { struct edma_chan *echan = to_edma_chan(chan); - if (cfg->src_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES || - cfg->dst_addr_width == DMA_SLAVE_BUSWIDTH_8_BYTES) + if (cfg->src_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES || + cfg->dst_addr_width >= DMA_SLAVE_BUSWIDTH_8_BYTES) return -EINVAL; memcpy(&echan->cfg, cfg, sizeof(echan->cfg));
The driver would happily accept buswidth of 16/32/64 bytes and program garbage to its registers. Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> --- drivers/dma/edma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)