On Tue, Aug 28, 2012 at 02:55:35PM +0800, Hein Tibosch wrote: > On 8/28/2012 11:23 AM, Viresh Kumar wrote: > > On 27 August 2012 20:28, Hein Tibosch <[email protected]> wrote: > >>>> +config DW_DMAC_MEM_64_BIT > >>>> + bool "Allow 64-bit memory transfers" > >>>> + default y if !AVR32 > >>>> + depends on DW_DMAC > >>>> + help > >>>> + Say yes if the DMA controller may do 64-bit memory transfers > >>>> + For AVR32, say no because only up to 32-bit transfers are > >>>> + defined > >>> Is this sane to add? Could some non-AVR32 platforms use 64-bit and 32-bit > >>> depending on runtime configuration? E.g. if you build a kernel with > >>> support > >>> for multiple boards/processors, and there is a mix of 32-bit and 64-bit > >>> wide > >>> DMA support. > >>> > >>> I think it is better to select 32/64-bit at runtime. > >> I did that in the first patch, adding a new property to the dw_dma_slave > >> structure. It had the small disadvantage that some arch code had to be > >> adapted (at32ap700x.c). > >> > >> Viresh, what do you think? Add a property called "mem_64_bit_access" or so? > >> > >> Or should it be passed as a member of 'dw_dma_platform_data', because it > >> is a property of the (entire) DMA controller? > > I think second option is better. But can there be some supportive scenarios > > of > > first option? > > > > We have a system, with two different memory controllers, one supporting 32 > > bit > > and other 64 bit? > > > > Or what we can do now is: go with option 2, i.e. update dw_dma_platform_data > > and if some platform like what i mentioned above comes, then we can move it > > to slave data. > What about this: > > In case of AVR32, the arch code will indicate: > > static struct dw_dma_platform_data dw_dmac0_data = { > .nr_channels = 3, > /* DMAC supports up to 32-bit memory access */ > .mem_access_32_bit_only = true, > }; > > ARM users won't have to change anything because mem_access_32_bit_only > will be false and therefor 'dw->mem_64_bit' will become true > > Signed-off-by: Hein Tibosch <[email protected]> > > --- > drivers/dma/dw_dmac.c | 11 ++++++++--- > drivers/dma/dw_dmac_regs.h | 2 ++ > include/linux/dw_dmac.h | 2 ++ > 3 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > index 7212961..a37053c 100644 > --- a/drivers/dma/dw_dmac.c > +++ b/drivers/dma/dw_dmac.c > @@ -618,6 +618,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t > dest, dma_addr_t src, > size_t len, unsigned long flags) > { > struct dw_dma_chan *dwc = to_dw_dma_chan(chan); > + struct dw_dma *dw = to_dw_dma(dwc->chan.device); > struct dw_desc *desc; > struct dw_desc *first; > struct dw_desc *prev; > @@ -639,7 +640,7 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t > dest, dma_addr_t src, > * We can be a lot more clever here, but this should take care > * of the most common optimization. > */ > - if (!((src | dest | len) & 7)) > + if (dw->mem_64_bit && !((src | dest | len) & 7)) > src_width = dst_width = 3; > else if (!((src | dest | len) & 3)) > src_width = dst_width = 2; > @@ -710,6 +711,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct > scatterlist *sgl, > struct dw_dma_chan *dwc = to_dw_dma_chan(chan); > struct dw_dma_slave *dws = chan->private; > struct dma_slave_config *sconfig = &dwc->dma_sconfig; > + struct dw_dma *dw = to_dw_dma(dwc->chan.device); > struct dw_desc *prev; > struct dw_desc *first; > u32 ctllo; > @@ -746,7 +748,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct > scatterlist *sgl, > mem = sg_dma_address(sg); > len = sg_dma_len(sg); > > - if (!((mem | len) & 7)) > + if (dw->mem_64_bit && !((mem | len) & 7)) > mem_width = 3; > else if (!((mem | len) & 3)) > mem_width = 2; > @@ -813,7 +815,7 @@ slave_sg_todev_fill_desc: > mem = sg_dma_address(sg); > len = sg_dma_len(sg); > > - if (!((mem | len) & 7)) > + if (dw->mem_64_bit && !((mem | len) & 7)) > mem_width = 3; > else if (!((mem | len) & 3)) > mem_width = 2; > @@ -1419,6 +1421,9 @@ static int __init dw_probe(struct platform_device *pdev) > goto err_kfree; > } > > + /* Remember if 64-bit access to memory is allowed */ > + dw->mem_64_bit = !pdata->mem_access_32_bit_only; > + > dw->regs = ioremap(io->start, DW_REGLEN); > if (!dw->regs) { > err = -ENOMEM; > diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h > index 9758651..e24562e 100644 > --- a/drivers/dma/dw_dmac_regs.h > +++ b/drivers/dma/dw_dmac_regs.h > @@ -199,6 +199,8 @@ struct dw_dma { > struct clk *clk; > > u8 all_chan_mask; > + /* 64-bit access to memory is allowed */ > + bool mem_64_bit; > > struct dw_dma_chan chan[0]; > }; > diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h > index 2412e02..d01d63f 100644 > --- a/include/linux/dw_dmac.h > +++ b/include/linux/dw_dmac.h > @@ -29,6 +29,8 @@ struct dw_dma_platform_data { > #define CHAN_PRIORITY_ASCENDING 0 /* chan0 highest */ > #define CHAN_PRIORITY_DESCENDING 1 /* chan7 highest */ > unsigned char chan_priority; > + /* Make true if 64-bit access to memory is not implemented */ > + bool mem_access_32_bit_only; > };
Can you not read this from any internal register ? Synopsys generally adds a set of read only registers which we can use to guess which features where enabled in the IP when configuring it with their IP configuration tool. -- balbi
signature.asc
Description: Digital signature
