* [PATCH 1/2] dw_dmac: make driver endianness configurable @ 2012-08-26 20:53 Hein Tibosch 2012-08-27 7:03 ` Hans-Christian Egtvedt 0 siblings, 1 reply; 11+ messages in thread From: Hein Tibosch @ 2012-08-26 20:53 UTC (permalink / raw) To: viresh kumar Cc: spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, egtvedt, Andrew Morton, Arnd Bergmann The dw_dmac was originally developed for avr32 to be used with the Synopsys DesignWare AHB DMA controller. After 2.6.38, device access was done with the little-endian readl/writel functions. This didn't work on the avr32 platform, because it needs native-endian (i.e. big-endian) accessors. This patch makes the endianness configurable using 'DW_DMAC_BE', which will default be true for AVR32 Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es> --- drivers/dma/Kconfig | 8 ++++++++ drivers/dma/dw_dmac_regs.h | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 0 deletions(-) diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index aadeb5b..3635daf 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -89,6 +89,14 @@ config DW_DMAC Support the Synopsys DesignWare AHB DMA controller. This can be integrated in chips such as the Atmel AT32ap7000. +config DW_DMAC_BE + bool "Synopsys DesignWare AHB DMA needs big endian access" + default y if AVR32 + depends on DW_DMAC + help + Say yes if access to the Synopsys DesignWare AHB DMA controller + should be big endian, such as for Atmel AT32ap7000 + config AT_HDMAC tristate "Atmel AHB DMA support" depends on ARCH_AT91 diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h index f298f69..cf048c3 100644 --- a/drivers/dma/dw_dmac_regs.h +++ b/drivers/dma/dw_dmac_regs.h @@ -175,11 +175,22 @@ __dwc_regs(struct dw_dma_chan *dwc) return dwc->ch_regs; } +#ifdef CONFIG_DW_DMAC_BE + +#define channel_readl(dwc, name) \ + ioread32be(&(__dwc_regs(dwc)->name)) +#define channel_writel(dwc, name, val) \ + iowrite32be((val), &(__dwc_regs(dwc)->name)) + +#else + #define channel_readl(dwc, name) \ readl(&(__dwc_regs(dwc)->name)) #define channel_writel(dwc, name, val) \ writel((val), &(__dwc_regs(dwc)->name)) +#endif + static inline struct dw_dma_chan *to_dw_dma_chan(struct dma_chan *chan) { return container_of(chan, struct dw_dma_chan, chan); @@ -201,11 +212,23 @@ static inline struct dw_dma_regs __iomem *__dw_regs(struct dw_dma *dw) return dw->regs; } +#ifdef CONFIG_DW_DMAC_BE + +#define dma_readl(dwc, name) \ + ioread32be(&(__dw_regs(dw)->name)) +#define dma_writel(dwc, name, val) \ + iowrite32be((val), &(__dw_regs(dw)->name)) + +#else + #define dma_readl(dw, name) \ readl(&(__dw_regs(dw)->name)) #define dma_writel(dw, name, val) \ writel((val), &(__dw_regs(dw)->name)) +#endif + #define channel_set_bit(dw, reg, mask) \ dma_writel(dw, reg, ((mask) << 8) | (mask)) #define channel_clear_bit(dw, reg, mask) \ -- 1.7.8.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-26 20:53 [PATCH 1/2] dw_dmac: make driver endianness configurable Hein Tibosch @ 2012-08-27 7:03 ` Hans-Christian Egtvedt 2012-08-27 8:47 ` Hein Tibosch 0 siblings, 1 reply; 11+ messages in thread From: Hans-Christian Egtvedt @ 2012-08-27 7:03 UTC (permalink / raw) To: Hein Tibosch Cc: viresh kumar, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann Around Mon 27 Aug 2012 04:53:02 +0800 or thereabout, Hein Tibosch wrote: > The dw_dmac was originally developed for avr32 to be used with the > Synopsys DesignWare AHB DMA controller. After 2.6.38, device access was done > with the little-endian readl/writel functions. This didn't work on the > avr32 platform, because it needs native-endian (i.e. big-endian) accessors. > This patch makes the endianness configurable using 'DW_DMAC_BE', > which will default be true for AVR32 I think the English in kconfig could use some brushing up. > Signed-off-by: Hein Tibosch <hein_tibosch@yahoo.es> > --- > drivers/dma/Kconfig | 8 ++++++++ > drivers/dma/dw_dmac_regs.h | 23 +++++++++++++++++++++++ > 2 files changed, 31 insertions(+), 0 deletions(-) > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > index aadeb5b..3635daf 100644 > --- a/drivers/dma/Kconfig > +++ b/drivers/dma/Kconfig > @@ -89,6 +89,14 @@ config DW_DMAC > Support the Synopsys DesignWare AHB DMA controller. This > can be integrated in chips such as the Atmel AT32ap7000. > > +config DW_DMAC_BE > This name isn't that long, so we could skip the abbreviation of big endian; DW_DMAC_BIG_ENDIAN_IO or something similar? > + bool "Synopsys DesignWare AHB DMA needs big endian access" > bool "Use big endian I/O register access" > + default y if AVR32 > + depends on DW_DMAC > + help > + Say yes if access to the Synopsys DesignWare AHB DMA controller > + should be big endian, such as for Atmel AT32ap7000 > + "Say yes here to use big endian I/O access when reading and writing to the DMA controller registers. This is needed on some platforms, like the Atmel AVR32 architecture. If unsure, use the default setting." <snipp patch diff> -- mvh Hans-Christian Egtvedt ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-27 7:03 ` Hans-Christian Egtvedt @ 2012-08-27 8:47 ` Hein Tibosch 2012-08-27 11:14 ` Hans-Christian Egtvedt 0 siblings, 1 reply; 11+ messages in thread From: Hein Tibosch @ 2012-08-27 8:47 UTC (permalink / raw) To: Hans-Christian Egtvedt Cc: viresh kumar, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann On 8/27/2012 3:03 PM, Hans-Christian Egtvedt wrote: > I think the English in kconfig could use some brushing up. > >> +config DW_DMAC_BE > > > This name isn't that long, so we could skip the abbreviation of big endian; > DW_DMAC_BIG_ENDIAN_IO or something similar? > >> + bool "Synopsys DesignWare AHB DMA needs big endian access" >> > bool "Use big endian I/O register access" Brushing up the config items: +config DW_DMAC_BIG_ENDIAN_IO + bool "Use big endian I/O register access" + default y if AVR32 + depends on DW_DMAC + help + Say yes here to use big endian I/O access when reading and writing + to the DMA controller registers. This is needed on some platforms, + like the Atmel AVR32 architecture. + + If unsure, use the default setting. And as I'd like to define the maximum memory transfer width in the same Kconfig: +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 Would this be better? Thanks, Hein ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-27 8:47 ` Hein Tibosch @ 2012-08-27 11:14 ` Hans-Christian Egtvedt 2012-08-27 14:58 ` Hein Tibosch 0 siblings, 1 reply; 11+ messages in thread From: Hans-Christian Egtvedt @ 2012-08-27 11:14 UTC (permalink / raw) To: Hein Tibosch Cc: viresh kumar, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann Around Mon 27 Aug 2012 16:47:40 +0800 or thereabout, Hein Tibosch wrote: > On 8/27/2012 3:03 PM, Hans-Christian Egtvedt wrote: >> I think the English in kconfig could use some brushing up. >> >>> +config DW_DMAC_BE >> >> >> This name isn't that long, so we could skip the abbreviation of big endian; >> DW_DMAC_BIG_ENDIAN_IO or something similar? >> >>> + bool "Synopsys DesignWare AHB DMA needs big endian access" >>> >> bool "Use big endian I/O register access" > > Brushing up the config items: > > +config DW_DMAC_BIG_ENDIAN_IO > + bool "Use big endian I/O register access" > + default y if AVR32 > + depends on DW_DMAC > + help > + Say yes here to use big endian I/O access when reading and writing > + to the DMA controller registers. This is needed on some platforms, > + like the Atmel AVR32 architecture. > + > + If unsure, use the default setting. This sounds good in my ears, but I don't speak English natively. > And as I'd like to define the maximum memory transfer width in the same > Kconfig: > > +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. -- mvh Hans-Christian Egtvedt ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-27 11:14 ` Hans-Christian Egtvedt @ 2012-08-27 14:58 ` Hein Tibosch 2012-08-28 3:23 ` Viresh Kumar 0 siblings, 1 reply; 11+ messages in thread From: Hein Tibosch @ 2012-08-27 14:58 UTC (permalink / raw) To: Hans-Christian Egtvedt Cc: viresh kumar, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann On 8/27/2012 7:14 PM, Hans-Christian Egtvedt wrote: > Around Mon 27 Aug 2012 16:47:40 +0800 or thereabout, Hein Tibosch wrote: >> On 8/27/2012 3:03 PM, Hans-Christian Egtvedt wrote: >>> Brushing up the config items: >>> >>> +config DW_DMAC_BIG_ENDIAN_IO >>> + bool "Use big endian I/O register access" >>> + default y if AVR32 >>> + depends on DW_DMAC >>> + help >>> + Say yes here to use big endian I/O access when reading and writing >>> + to the DMA controller registers. This is needed on some platforms, >>> + like the Atmel AVR32 architecture. >>> + >>> + If unsure, use the default setting. > This sounds good in my ears, but I don't speak English natively. I think you Norwegians are doing very well in English And btw, I'm not from .es but from .nl, which is very close to England >> And as I'd like to define the maximum memory transfer width in the same >> Kconfig: >> >> +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? Hein ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-27 14:58 ` Hein Tibosch @ 2012-08-28 3:23 ` Viresh Kumar 2012-08-28 6:55 ` Hein Tibosch 0 siblings, 1 reply; 11+ messages in thread From: Viresh Kumar @ 2012-08-28 3:23 UTC (permalink / raw) To: Hein Tibosch Cc: Hans-Christian Egtvedt, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann 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. viresh ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-28 3:23 ` Viresh Kumar @ 2012-08-28 6:55 ` Hein Tibosch 2012-08-28 7:05 ` Viresh Kumar 2012-08-28 7:39 ` Felipe Balbi 0 siblings, 2 replies; 11+ messages in thread From: Hein Tibosch @ 2012-08-28 6:55 UTC (permalink / raw) To: Viresh Kumar Cc: Hans-Christian Egtvedt, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann 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; }; /* bursts size */ -- 1.7.8.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-28 6:55 ` Hein Tibosch @ 2012-08-28 7:05 ` Viresh Kumar 2012-08-28 7:39 ` Felipe Balbi 1 sibling, 0 replies; 11+ messages in thread From: Viresh Kumar @ 2012-08-28 7:05 UTC (permalink / raw) To: Hein Tibosch Cc: Hans-Christian Egtvedt, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann On 28 August 2012 12:25, Hein Tibosch <hein_tibosch@yahoo.es> wrote: > 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 I will go for the earlier solution that you sent: with max-mem-width. 0 = 64 bit, so nothing to be changed for ARM as global structures would be getting initialized to zero 1=32 bit, pass this for AVR32. -- viresh ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-28 6:55 ` Hein Tibosch 2012-08-28 7:05 ` Viresh Kumar @ 2012-08-28 7:39 ` Felipe Balbi 2012-09-03 7:50 ` Andy Shevchenko 1 sibling, 1 reply; 11+ messages in thread From: Felipe Balbi @ 2012-08-28 7:39 UTC (permalink / raw) To: Hein Tibosch Cc: Viresh Kumar, Hans-Christian Egtvedt, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Andrew Morton, Arnd Bergmann [-- Attachment #1: Type: text/plain, Size: 5638 bytes --] 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 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-08-28 7:39 ` Felipe Balbi @ 2012-09-03 7:50 ` Andy Shevchenko 2012-09-03 14:16 ` Felipe Balbi 0 siblings, 1 reply; 11+ messages in thread From: Andy Shevchenko @ 2012-09-03 7:50 UTC (permalink / raw) To: balbi Cc: Hein Tibosch, Viresh Kumar, Hans-Christian Egtvedt, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Arnd Bergmann On Tue, Aug 28, 2012 at 10:39 AM, Felipe Balbi <balbi@ti.com> wrote: > 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. You right. I have a patch to support few options like that already. However, I can't share it yet by some legal reasons. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dw_dmac: make driver endianness configurable 2012-09-03 7:50 ` Andy Shevchenko @ 2012-09-03 14:16 ` Felipe Balbi 0 siblings, 0 replies; 11+ messages in thread From: Felipe Balbi @ 2012-09-03 14:16 UTC (permalink / raw) To: Andy Shevchenko Cc: balbi, Hein Tibosch, Viresh Kumar, Hans-Christian Egtvedt, spear-devel, Linux Kernel Mailing List, ludovic.desroches, Havard Skinnemoen, Nicolas Ferre, Arnd Bergmann [-- Attachment #1: Type: text/plain, Size: 626 bytes --] On Mon, Sep 03, 2012 at 10:50:44AM +0300, Andy Shevchenko wrote: > On Tue, Aug 28, 2012 at 10:39 AM, Felipe Balbi <balbi@ti.com> wrote: > > 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. > You right. I have a patch to support few options like that already. > However, I can't share > it yet by some legal reasons. fair enough, in that case it'd be best to hold this patch and provide something better later ;-) -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-09-03 14:20 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-26 20:53 [PATCH 1/2] dw_dmac: make driver endianness configurable Hein Tibosch 2012-08-27 7:03 ` Hans-Christian Egtvedt 2012-08-27 8:47 ` Hein Tibosch 2012-08-27 11:14 ` Hans-Christian Egtvedt 2012-08-27 14:58 ` Hein Tibosch 2012-08-28 3:23 ` Viresh Kumar 2012-08-28 6:55 ` Hein Tibosch 2012-08-28 7:05 ` Viresh Kumar 2012-08-28 7:39 ` Felipe Balbi 2012-09-03 7:50 ` Andy Shevchenko 2012-09-03 14:16 ` Felipe Balbi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox