lkml.org 
[lkml]   [2012]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] dw_dmac: make driver endianness configurable
    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 <hein_tibosch@yahoo.es> 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 <hein_tibosch@yahoo.es>
    >
    > ---
    > 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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2012-08-28 10:21    [W:5.726 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site