| Message ID | 1359742975-10421-2-git-send-email-mporter@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Matt Porter <mporter@ti.com> [130201 10:25]: > Move mach-davinci/dma.c to common/edma.c so it can be used > by OMAP (specifically AM33xx) as well. I think this should rather go to drivers/dma/? Tony
On Fri, Feb 01, 2013 at 06:41:08PM +0000, Tony Lindgren wrote: > * Matt Porter <mporter@ti.com> [130201 10:25]: > > Move mach-davinci/dma.c to common/edma.c so it can be used > > by OMAP (specifically AM33xx) as well. > > I think this should rather go to drivers/dma/? No, this is the private EDMA API. It's the analogous thing to the private OMAP dma API that is in plat-omap/dma.c. The actual dmaengine driver is in drivers/dma/edma.c as a wrapper around this...same way OMAP DMA engine conversion is being done. -Matt
Hi, On Fri, Feb 01, 2013 at 10:52:46PM +0300, Sergei Shtylyov wrote: > Hello. > > On 02/01/2013 09:49 PM, Matt Porter wrote: > > >>> Move mach-davinci/dma.c to common/edma.c so it can be used > >>> by OMAP (specifically AM33xx) as well. > > >> I think this should rather go to drivers/dma/? > > > No, this is the private EDMA API. It's the analogous thing to > > the private OMAP dma API that is in plat-omap/dma.c. The actual > > dmaengine driver is in drivers/dma/edma.c as a wrapper around > > this...same way OMAP DMA engine conversion is being done. > > Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed > that, instead of waiting indefinitely for TI to convert it to drivers/dma/ > directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. good point, do you wanna send some patches ? I guess to make the MUSB side simpler we would need musb-dma-engine glue to map dmaengine to the private MUSB API. Then we would have some starting point to also move inventra (and anybody else) to dmaengine API. Once that's done, we drop MUSB's private API.
On Fri, Feb 01, 2013 at 07:52:46PM +0000, Sergei Shtylyov wrote: > Hello. > > On 02/01/2013 09:49 PM, Matt Porter wrote: > > >>> Move mach-davinci/dma.c to common/edma.c so it can be used > >>> by OMAP (specifically AM33xx) as well. > > >> I think this should rather go to drivers/dma/? > > > No, this is the private EDMA API. It's the analogous thing to > > the private OMAP dma API that is in plat-omap/dma.c. The actual > > dmaengine driver is in drivers/dma/edma.c as a wrapper around > > this...same way OMAP DMA engine conversion is being done. > > Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed > that, instead of waiting indefinitely for TI to convert it to drivers/dma/ > directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. That is a shame. Yeah, I've pointed out that I was doing this exactly the same way as was acceptable for the OMAP DMA conversion since it was in RFC. The reasons are sound since in both cases, we have many drivers to convert that need to continue using the private DMA APIs. -Matt >
Hello. On 02/01/2013 09:49 PM, Matt Porter wrote: >>> Move mach-davinci/dma.c to common/edma.c so it can be used >>> by OMAP (specifically AM33xx) as well. >> I think this should rather go to drivers/dma/? > No, this is the private EDMA API. It's the analogous thing to > the private OMAP dma API that is in plat-omap/dma.c. The actual > dmaengine driver is in drivers/dma/edma.c as a wrapper around > this...same way OMAP DMA engine conversion is being done. Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed that, instead of waiting indefinitely for TI to convert it to drivers/dma/ directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. > -Matt WBR, Sergei
Hello. On 02/01/2013 09:58 PM, Felipe Balbi wrote: >>>>> Move mach-davinci/dma.c to common/edma.c so it can be used >>>>> by OMAP (specifically AM33xx) as well. >>>> I think this should rather go to drivers/dma/? >>> No, this is the private EDMA API. It's the analogous thing to >>> the private OMAP dma API that is in plat-omap/dma.c. The actual >>> dmaengine driver is in drivers/dma/edma.c as a wrapper around >>> this...same way OMAP DMA engine conversion is being done. >> Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed >> that, instead of waiting indefinitely for TI to convert it to drivers/dma/ >> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. > good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( > I guess to make the MUSB side simpler we would need musb-dma-engine glue > to map dmaengine to the private MUSB API. Then we would have some > starting point to also move inventra (and anybody else) to dmaengine > API. Why? Inventra is a dedicated device's private DMA controller, why make universal DMA driver for it? > Once that's done, we drop MUSB's private API. Don't think it's a good idea. WBR, Sergei
hi, On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: > > good point, do you wanna send some patches ? > > I have already sent them countless times and even stuck CPPI 4.1 support (in > arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the > patch. :-( sticking into arch/arm/common/ wasn't a nice move. But then again, so wasn't asking for the patch to be removed :-s > > I guess to make the MUSB side simpler we would need musb-dma-engine glue > > to map dmaengine to the private MUSB API. Then we would have some > > starting point to also move inventra (and anybody else) to dmaengine > > API. > > Why? Inventra is a dedicated device's private DMA controller, why make > universal DMA driver for it? because it doesn't make sense to support multiple DMA APIs. We can check from MUSB's registers if it was configured with Inventra DMA support and based on that we can register MUSB's own DMA Engine to dmaengine API. > > Once that's done, we drop MUSB's private API. > > Don't think it's a good idea. see above.
On Fri, Feb 01, 2013 at 10:56:00PM +0200, Felipe Balbi wrote: > hi, > > On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: > > > good point, do you wanna send some patches ? > > > > I have already sent them countless times and even stuck CPPI 4.1 support (in > > arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the > > patch. :-( > > sticking into arch/arm/common/ wasn't a nice move. But then again, so > wasn't asking for the patch to be removed :-s Err, patches don't get removed, they get moved to 'discarded'. > > > I guess to make the MUSB side simpler we would need musb-dma-engine glue > > > to map dmaengine to the private MUSB API. Then we would have some > > > starting point to also move inventra (and anybody else) to dmaengine > > > API. > > > > Why? Inventra is a dedicated device's private DMA controller, why make > > universal DMA driver for it? > > because it doesn't make sense to support multiple DMA APIs. We can check > from MUSB's registers if it was configured with Inventra DMA support and > based on that we can register MUSB's own DMA Engine to dmaengine API. Hang on. This is one of the DMA implementations which is closely coupled with the USB and only the USB? If it is... I thought this had been discussed _extensively_ before. I thought the resolution on it was: 1. It would not use the DMA engine API. 2. It would not live in arch/arm. 3. It would be placed nearby the USB driver it's associated with. (1) because we don't use APIs just for the hell of it - think. Do we use the DMA engine API for PCI bus mastering ethernet controllers? No. Do we use it for PCI bus mastering SCSI controllers? No. Because the DMA is integral to the rest of the device. The DMA engine API only makes sense if the DMA engine is a shared system resource.
Hello. On 02-02-2013 0:56, Felipe Balbi wrote: >>> good point, do you wanna send some patches ? >> I have already sent them countless times and even stuck CPPI 4.1 support (in >> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >> patch. :-( > sticking into arch/arm/common/ wasn't a nice move. Like with EDMA we have nothing else to do with CPPI 4.1 being shared by DaVinci-like and OMAP-like SOCs. Thank TI for creatring this mess. And actually even that is not a good place since I think I know of a MIPS SoC having CPPI 4.1 as well, just out of tree. > But then again, so > wasn't asking for the patch to be removed :-s Unfortunately, Russell has done it -- all that was discuseed without me in the loop even. :-/ >>> I guess to make the MUSB side simpler we would need musb-dma-engine glue >>> to map dmaengine to the private MUSB API. Then we would have some >>> starting point to also move inventra (and anybody else) to dmaengine >>> API. >> Why? Inventra is a dedicated device's private DMA controller, why make >> universal DMA driver for it? > because it doesn't make sense to support multiple DMA APIs. We can check > from MUSB's registers if it was configured with Inventra DMA support and > based on that we can register MUSB's own DMA Engine to dmaengine API. I still disagree. IMO drivers/dma/ is for standalone DMA engines. Else we could stick every bus mastering device's DMA engines there. CPPI 4.1 is in design standlone DMA engine, despite all in-tree implementations having it as subblock of MUSB and serving only MUSB. WBR, Sergei
Hello. On 01-02-2013 22:59, Matt Porter wrote: >>>>> Move mach-davinci/dma.c to common/edma.c so it can be used >>>>> by OMAP (specifically AM33xx) as well. >>>> I think this should rather go to drivers/dma/? >>> No, this is the private EDMA API. It's the analogous thing to >>> the private OMAP dma API that is in plat-omap/dma.c. The actual >>> dmaengine driver is in drivers/dma/edma.c as a wrapper around >>> this...same way OMAP DMA engine conversion is being done. >> Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed >> that, instead of waiting indefinitely for TI to convert it to drivers/dma/ >> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. > That is a shame. Yeah, I've pointed out that I was doing this exactly > the same way as was acceptable for the OMAP DMA conversion since it was > in RFC. The reasons are sound since in both cases, we have many drivers > to convert that need to continue using the private DMA APIs. In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't know them well). > -Matt WBR, Sergei
Hello. On 02-02-2013 1:30, Russell King - ARM Linux wrote: >> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: >>>> good point, do you wanna send some patches ? >>> I have already sent them countless times and even stuck CPPI 4.1 support (in >>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >>> patch. :-( >> sticking into arch/arm/common/ wasn't a nice move. But then again, so >> wasn't asking for the patch to be removed :-s > Err, patches don't get removed, they get moved to 'discarded'. Any chance to bring it back to life? :-) Although... drivers/usb/musb/cppi41.c would need to be somewhat reworked for at least AM35x and I don't have time. But that may change, of course. >>>> I guess to make the MUSB side simpler we would need musb-dma-engine glue >>>> to map dmaengine to the private MUSB API. Then we would have some >>>> starting point to also move inventra (and anybody else) to dmaengine >>>> API. >>> Why? Inventra is a dedicated device's private DMA controller, why make >>> universal DMA driver for it? >> because it doesn't make sense to support multiple DMA APIs. We can check >> from MUSB's registers if it was configured with Inventra DMA support and >> based on that we can register MUSB's own DMA Engine to dmaengine API. > Hang on. This is one of the DMA implementations which is closely > coupled with the USB and only the USB? If it is... > I thought this had been discussed _extensively_ before. I thought the > resolution on it was: > 1. It would not use the DMA engine API. > 2. It would not live in arch/arm. > 3. It would be placed nearby the USB driver it's associated with. > (1) because we don't use APIs just for the hell of it - think. Do we > use the DMA engine API for PCI bus mastering ethernet controllers? No. > Do we use it for PCI bus mastering SCSI controllers? No. Because the > DMA is integral to the rest of the device. > The DMA engine API only makes sense if the DMA engine is a shared > system resource. Thanks for such extensive wording in the support of my point. :-) WBR, Sergei
Hello. On 02-02-2013 1:30, Russell King - ARM Linux wrote: >> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: >>>> good point, do you wanna send some patches ? >>> I have already sent them countless times and even stuck CPPI 4.1 support (in >>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >>> patch. :-( >> sticking into arch/arm/common/ wasn't a nice move. But then again, so >> wasn't asking for the patch to be removed :-s > Err, patches don't get removed, they get moved to 'discarded'. >>>> I guess to make the MUSB side simpler we would need musb-dma-engine glue >>>> to map dmaengine to the private MUSB API. Then we would have some >>>> starting point to also move inventra (and anybody else) to dmaengine >>>> API. >>> Why? Inventra is a dedicated device's private DMA controller, why make >>> universal DMA driver for it? >> because it doesn't make sense to support multiple DMA APIs. We can check >> from MUSB's registers if it was configured with Inventra DMA support and >> based on that we can register MUSB's own DMA Engine to dmaengine API. > Hang on. This is one of the DMA implementations which is closely > coupled with the USB and only the USB? If it is... > I thought this had been discussed _extensively_ before. I thought the > resolution on it was: > 1. It would not use the DMA engine API. > 2. It would not live in arch/arm. > 3. It would be placed nearby the USB driver it's associated with. Note that all this doesn't apply to CPPI 4.1 controller (as contrasted to CPPI 3.0 support in MUSB aand EMAC drivers) -- it's shared by design. Just the implementations that are in tree have it as MUSB's sub-block, serving only MUSB. WBR, Sergei
On Sat, Feb 02, 2013 at 04:07:59AM +0400, Sergei Shtylyov wrote: > Hello. > > On 02-02-2013 1:30, Russell King - ARM Linux wrote: > >>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: >>>>> good point, do you wanna send some patches ? > >>>> I have already sent them countless times and even stuck CPPI 4.1 support (in >>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >>>> patch. :-( > >>> sticking into arch/arm/common/ wasn't a nice move. But then again, so >>> wasn't asking for the patch to be removed :-s > >> Err, patches don't get removed, they get moved to 'discarded'. > > Any chance to bring it back to life? :-) > Although... drivers/usb/musb/cppi41.c would need to be somewhat > reworked for at least AM35x and I don't have time. But that may change, > of course. Right, I've just looked back at the various meeting minutes from December 2010 when the CPPI stuff was discussed. Yes, I archive these things and all email discussions for referencing in cases like this. Unfortunately, they do not contain any useful information other than the topic having been brought up. At that point, the CPPI stuff was in mach-davinci, and I had suggested moving it into drivers/dma. The result of that was to say that it doesn't fit the DMA engine APIs. So someone came up with the idea of putting it in arch/arm/common - which I frankly ignored by email (how long have we been saying "no drivers in arch/arm" ?) Now, it would've been discussed in that meeting, but unfortunately no record exists of that. What does follow that meeting is a discussion trail. From what I can see there, but it looks to me like the decision was taken to move it to the DMA engine API, and work on sorting out MUSB was going to commence. The last email in that says "I'll get to that soon"... and that is also the final email I have on this topic. I guess if nothing has happened... Shrug, that's someone elses problem. Anyway, the answer for putting it in arch/arm/common hasn't changed, and really, where we are now, post Linus having a moan about the size of arch/arm, that answer is even more concrete in the negative. It's 54K of code which should not be under arch/arm at all. Anyway, if you need to look at the patch, it's 6305/1. Typing into the summary search box 'cppi' found it in one go.
Hello. On 02-02-2013 4:44, Russell King - ARM Linux wrote: >>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: >>>>>> good point, do you wanna send some patches ? >>>>> I have already sent them countless times and even stuck CPPI 4.1 support (in >>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >>>>> patch. :-( >>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so >>>> wasn't asking for the patch to be removed :-s >>> Err, patches don't get removed, they get moved to 'discarded'. >> Any chance to bring it back to life? :-) >> Although... drivers/usb/musb/cppi41.c would need to be somewhat >> reworked for at least AM35x and I don't have time. But that may change, >> of course. > Right, I've just looked back at the various meeting minutes from December > 2010 when the CPPI stuff was discussed. Yes, I archive these things and > all email discussions for referencing in cases like this. Thanks. > Unfortunately, they do not contain any useful information other than the > topic having been brought up. At that point, the CPPI stuff was in > mach-davinci, and I had suggested moving it into drivers/dma. I don't remember that, probably was out of the loop again. > The result of that was to say that it doesn't fit the DMA engine APIs. I remember this as a discussion happening post me sending the patch to the patch system and it being discarded... > So someone came up with the idea of putting it in arch/arm/common - which Probably was me. There was also idea of putting it into drivers/usb/musb/ -- which TI indeed followed in its Arago prject. I firmly denied that suggestion. > I frankly ignored by email (how long have we been saying "no drivers in > arch/arm" ?) But there *are* drivers there! And look at edma.c which is about to be moved there... Anyway, I haven't seen such warnings, probably was too late in the game. > Now, it would've been discussed in that meeting, but unfortunately no > record exists of that. What does follow that meeting is a discussion > trail. From what I can see there, but it looks to me like the decision > was taken to move it to the DMA engine API, and work on sorting out MUSB > was going to commence. > The last email in that says "I'll get to that soon"... and that is also > the final email I have on this topic. I guess if nothing has happened... > Shrug, that's someone elses problem. Well, as usual... :-( > Anyway, the answer for putting it in arch/arm/common hasn't changed, > and really, where we are now, post Linus having a moan about the size > of arch/arm, that answer is even more concrete in the negative. It's > 54K of code which should not be under arch/arm at all. > Anyway, if you need to look at the patch, it's 6305/1. Typing into the > summary search box 'cppi' found it in one go. Thanks, I remember this variant was under arch/arm/common/. Now however, I see what happened to that variant in somewhat different light. Looks like it was entirely your decision to discard the patch, without TI's request... WBR, Sergei
On Sat, Feb 02, 2013 at 06:09:24AM +0400, Sergei Shtylyov wrote: > Hello. > > On 02-02-2013 4:44, Russell King - ARM Linux wrote: > >>>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: >>>>>>> good point, do you wanna send some patches ? > >>>>>> I have already sent them countless times and even stuck CPPI 4.1 support (in >>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >>>>>> patch. :-( > >>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so >>>>> wasn't asking for the patch to be removed :-s > >>>> Err, patches don't get removed, they get moved to 'discarded'. > >>> Any chance to bring it back to life? :-) >>> Although... drivers/usb/musb/cppi41.c would need to be somewhat >>> reworked for at least AM35x and I don't have time. But that may change, >>> of course. > >> Right, I've just looked back at the various meeting minutes from December >> 2010 when the CPPI stuff was discussed. Yes, I archive these things and >> all email discussions for referencing in cases like this. > > Thanks. > >> Unfortunately, they do not contain any useful information other than the >> topic having been brought up. At that point, the CPPI stuff was in >> mach-davinci, and I had suggested moving it into drivers/dma. > > I don't remember that, probably was out of the loop again. > >> The result of that was to say that it doesn't fit the DMA engine APIs. > > I remember this as a discussion happening post me sending the patch to > the patch system and it being discarded... > >> So someone came up with the idea of putting it in arch/arm/common - which > > Probably was me. There was also idea of putting it into > drivers/usb/musb/ -- which TI indeed followed in its Arago prject. I > firmly denied that suggestion. > >> I frankly ignored by email (how long have we been saying "no drivers in >> arch/arm" ?) > > But there *are* drivers there! And look at edma.c which is about to be > moved there... Anyway, I haven't seen such warnings, probably was too > late in the game. I've already objected about the header moving to some random place in arch/arm/include. Really, edma.c needs to find another home too - but there's a difference here. edma.c is already present under arch/arm. CPPI is _not_. CPPI is new code appearing under arch/arm (you can see that for yourself by looking at the diffstat of 6305/1... it doesn't move files, it adds new code.) >> Now, it would've been discussed in that meeting, but unfortunately no >> record exists of that. What does follow that meeting is a discussion >> trail. From what I can see there, but it looks to me like the decision >> was taken to move it to the DMA engine API, and work on sorting out MUSB >> was going to commence. > >> The last email in that says "I'll get to that soon"... and that is also >> the final email I have on this topic. I guess if nothing has happened... >> Shrug, that's someone elses problem. > > Well, as usual... :-( > >> Anyway, the answer for putting it in arch/arm/common hasn't changed, >> and really, where we are now, post Linus having a moan about the size >> of arch/arm, that answer is even more concrete in the negative. It's >> 54K of code which should not be under arch/arm at all. > >> Anyway, if you need to look at the patch, it's 6305/1. Typing into the >> summary search box 'cppi' found it in one go. > > Thanks, I remember this variant was under arch/arm/common/. > Now however, I see what happened to that variant in somewhat different > light. Looks like it was entirely your decision to discard the patch, > without TI's request... Firstly, it is *my* perogative to say no to anything in arch/arm, and I really don't have to give reasons for it if I choose to. Secondly, it *was* discussed with TI, and the following thread of discussion (threaded to the minutes email) shows that *something* was going to happen _as a result of that meeting_ to address the problem of it being under arch/arm. And *therefore* it was discarded from the patch system - because there was expectation that it was going to get fixed. For christ sake, someone even agreed to do it. Even a target was mentioned, of 2.6.39. That was mentioned on 7th December 2010. And 6305/1 was discarded on 8th December 2010. Cause and effect. And yes, *you* were not part of that discussion. You work for Montavista which contracts with TI to provide this support. It is up to TI to pass stuff like this on to their contractors. There are two people on this thread CC list who were also involved or CC'd on the mails from the thread in 2010... Tony and Felipe. Unfortunately, the person who agreed to do the work is no longer in the land of the living. Yes I know it's inconvenient for people to die when they've still got lots of important work to do but that's what can happen...
On Sat, Feb 02, 2013 at 10:18:51AM +0000, Russell King - ARM Linux wrote: > On Sat, Feb 02, 2013 at 06:09:24AM +0400, Sergei Shtylyov wrote: > > Hello. > > > > On 02-02-2013 4:44, Russell King - ARM Linux wrote: > > > >>>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: > >>>>>>> good point, do you wanna send some patches ? > > > >>>>>> I have already sent them countless times and even stuck CPPI 4.1 support (in > >>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the > >>>>>> patch. :-( > > > >>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so > >>>>> wasn't asking for the patch to be removed :-s > > > >>>> Err, patches don't get removed, they get moved to 'discarded'. > > > >>> Any chance to bring it back to life? :-) > >>> Although... drivers/usb/musb/cppi41.c would need to be somewhat > >>> reworked for at least AM35x and I don't have time. But that may change, > >>> of course. > > > >> Right, I've just looked back at the various meeting minutes from December > >> 2010 when the CPPI stuff was discussed. Yes, I archive these things and > >> all email discussions for referencing in cases like this. > > > > Thanks. > > > >> Unfortunately, they do not contain any useful information other than the > >> topic having been brought up. At that point, the CPPI stuff was in > >> mach-davinci, and I had suggested moving it into drivers/dma. > > > > I don't remember that, probably was out of the loop again. Here you go - this goes back even _further_ - November 2009 - on the mailing list. The entire thread: http://lists.arm.linux.org.uk/lurker/thread/20091102.105759.a54cf3f5.en.html And selected emails from it: http://lists.arm.linux.org.uk/lurker/message/20091102.103706.38c029b5.en.html On Mon, Nov 02, 2009 at 10:37:06AM +0000, I wrote: | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote: | > Another option is to create arch/arm/ti-common to place all TI platform's | > common software, such as CPPI4.1 used both in DA8xx and AM3517. | | No thanks. I really don't see why we should allow TI to have yet more | directories scattered throughout the tree that are out of place with | existing conventions. | | And what is this CPPI thing anyway? | | http://acronyms.thefreedictionary.com/CPPI | | "Communications Port Programming Interface" seems to be about the best | applicable one from that list! | | If it's a USB DMA device (from the patches I can find, that seems to be | the case) then why can't it live in drivers/usb or drivers/dma ? And again: http://lists.arm.linux.org.uk/lurker/message/20091102.115458.61cde450.en.html On Mon, Nov 02, 2009 at 11:54:58AM +0000, I wrote: | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote: | > CPPI4.1 DMA engine can be used either by USB or by Ethernet interface though | > currently only USB is using it but in future even Ethernet devices may use it. | | drivers/dma does seem to be the right place for this. http://lists.arm.linux.org.uk/lurker/message/20091102.110217.adec3ca7.en.html Even Felipe Balbi said so: | you might want to provide support for it via drivers/dma and for the | musb stuff, you just add the wrappers musb uses. See how tusb6010_omap.c | uses OMAP's system dma which is also used by any other driver which | requests a dma channel. And it seems that _YOU_ did get the message - see your quoted text in: http://lists.arm.linux.org.uk/lurker/message/20091230.132240.ecd56b3d.en.html > We're currently having it there but the matter is it should be shred > between different platforms, so arch/arm/common/ seems like the right > place (which Russell didn't like, suggesting ill suited for that > drivers/dma/ instead). See - you acknowledge here that I don't like it. So you _KNOW_ my views on it in December 2009, contary to what you're saying in this thread. Yet, you persisted with putting it in arch/arm/common: http://lists.arm.linux.org.uk/lurker/message/20100515.181453.472c7c10.en.html | Changes since the previous version: | - moved everything from arch/arm/mach-davinci/ to arch/arm/common/; | - s/CONFIG_CPPI41/CONFIG_TI_CPPI41/, made that option invisible; | - added #include <linux/slab.h> for kzalloc(); | - switched alloc_queue() and cppi41_queue_free() to using bit operations; | - replaced 'static' linking_ram[] by local variable in cppi41_queue_mgr_init(); | - fixed pr_debug() in cppi41_dma_ctrlr_init() to print the real queue manager #. So, see, I had already objected to it being in arch/arm well before you stuck your patch into the patch system. And somehow you think that ignoring my previous comments and doing it anyway will result in progress? So, let's recap. The timeline behind this is: + 2 Nov 2009: Question asked about putting it in arch/arm/ti-common + I responded saying a clear no to that, suggesting other locations all of which were outside arch/arm. + I responded again saying an hour or two later saying the same thing. + Felipe Balbi agreed with drivers/dma. + 15 May 2010: v5 posted with it in arch/arm/common + 06 Aug 2010: put into patch system sa 6305/1 + 06 Dec 2010: TI meeting. + Pre-meeting notes show that my views on this are known: + I'll quote this from it: "Russell suggested to move the driver at drivers/dma/". + Raises concern that DMA engine API may not fit. + I respond to that concern as work has been done on the DMA engine API to improve the slave mode support recently as a result of Linus Walleij's work on AMBA PL08x DMA support. (I would _not_ have done so if I had changed my view about it being under drivers/dma/). + 07 Dec 2010: emails talking about moving MUSB over to DMA engine API so that MUSB should not care about its DMA backend (that being CPPI or some other one.) + Email with "Let's see if I can get it all done by 2.6.39." + 08 Dec 2010: patch 6305/1 discarded from the patch system as there now seems to be concensus on the issue. + 03 Jan 2011: you ask me why it was discarded http://lists.arm.linux.org.uk/lurker/thread/20110103.160610.8cbe8e7d.en.html + I respond "It may have been that it's inventing its own API rather than using something like the DMA engine API." + Ajay said: "This issue was discussed recently at TI and proposal was to place it to drivers/dma folder. Moreover, even Felipe also seems to move other musb DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma." Oh, and then we come to this interesting email from Felipe to you, in response to your reluctance to put it in drivers/dma. | Do I really have to spell it out ? Really ? | | You don't need to physically move the part of the code to drivers/dma, | but it has to use the API. The mentor DMA is internal to MUSB. | tusb6010_omap.c isn't. | | Where it makes sense to move the code under drivers/dma, it will be | done, where it doesn't, it won't be done, but it will use the same API. | That's all. | | The end goal is just to drop all these ad-hoc "APIs" for accessing DMA | on musb code. See the common theme here? I don't like it under arch/arm. I've been pretty _consistent_ in suggesting drivers/dma/ all through that... Others have said it. People even acknowledge that's what I've been saying, people who were not in the original discussion. What I think is this: it is _YOU_ who don't want to hear that message, so _YOU_ are intentionally ignoring it, and _YOU_ are looking for someone to blame for it. You've decided I'm to blame because _YOU_ aren't listening. The reason there hasn't been any progress on this is _NOT_ down to me. I've provided my feedback, and promptly been ignored. Others have told you the same thing, and promptly been ignored. Sorry, this is not my problem. This is entirely YOUR problem, one of your own making. But whatever. We are NOT going to put CPPI under arch/arm. Not now that during the last merge window Linus complained to Arnd about the amount of code coming through for arch/arm _AGAIN_. Not after last time Linus complained about TI OMAP which prompted the creation of arm-soc. AND THAT IS *FINAL*. CPPI DMA SUPPORT IS *NOT* GOING UNDER ARCH/ARM. PERIOD. NOT GOING TO HAPPEN. Is this clear enough yet, or how many more years of emails do you need yet more emails to get this message through?
On Fri, Feb 01, 2013 at 01:59:59PM -0500, Matt Porter wrote: > On Fri, Feb 01, 2013 at 07:52:46PM +0000, Sergei Shtylyov wrote: > > Hello. > > > > On 02/01/2013 09:49 PM, Matt Porter wrote: > > > > >>> Move mach-davinci/dma.c to common/edma.c so it can be used > > >>> by OMAP (specifically AM33xx) as well. > > > > >> I think this should rather go to drivers/dma/? > > > > > No, this is the private EDMA API. It's the analogous thing to > > > the private OMAP dma API that is in plat-omap/dma.c. The actual > > > dmaengine driver is in drivers/dma/edma.c as a wrapper around > > > this...same way OMAP DMA engine conversion is being done. > > > > Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed > > that, instead of waiting indefinitely for TI to convert it to drivers/dma/ > > directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. > > That is a shame. Yeah, I've pointed out that I was doing this exactly > the same way as was acceptable for the OMAP DMA conversion since it was > in RFC. The reasons are sound since in both cases, we have many drivers > to convert that need to continue using the private DMA APIs. It's very simple. The OMAP DMA stuff in plat-omap/dma.c has existed for a long time, well before things like the DMA engine API came into being. Same with a lot of the DMA code in arch/arm. It is therefore in the privileged position of having "grandfather" rights when it comes to existing in mainline. However, today what we're finding is that these private per-platform APIs are sub-optimal; they prevent the peripheral drivers in the drivers/ subtree from being re-used on other SoCs. So, even through they pre-exist the DMA engine support, they're being gradually converted to the API. Moreover, we have _far_ too much code in arch/arm. It's something that has been attracted complaints from Linus. It's something that's got us close to having updates to arch/arm refused during merge windows. It's got us close to having parts of arch/arm deleted from the mainline kernel. The long term plan is _still_ to remove arch/arm/*omap*/dma.c and move the whole thing to drivers/dma. That can only happen when everything is converted (or the drivers which aren't converted have been retired and deleted) - and provided that TI decide to continue funding that work. That's still uncertain since the whole TI OMAP thing imploded two months ago. Now, CPPI is brand new code to arch/arm - always has been. It post-dates the DMA engine API. And it's been said many times about moving it to drivers/dma. The problem is Sergei doesn't want to do it - he's anti the DMA engine API for whatever reasons he can dream up. He professes no knowledge of my dislike for having it in arch/arm, yet there's emails from years back showing that he knew about it. TI knows about it; Ajay about it. Yet... well... history speaks volumes about this. Now, there may be a new problem with CPPI. That being we're now requiring DT support. _Had_ this been done prior to the push for DT, then it would not have been a concern, because then the problem becomes one for DT. But now that OMAP is being converted to DT, and DT is The Way To Go now, that's an additional problem that needs to be grappled with - or it may make things easier. However, as I've said now many times: CPPI is _not_ going in arch/arm. Period. That makes it not my problem. Period. I really have nothing further to say on CPPI; everything that can be said has already been said. If there's still pressure to have it in arch/arm, I will _NOT_ respond to it, because there's nothing to be said about it which hasn't been said before. The only reason it's still being pressed for is a total reluctance to do anything about it being in arch/arm.
On Fri, Feb 01, 2013 at 10:41:08AM -0800, Tony Lindgren wrote: > * Matt Porter <mporter@ti.com> [130201 10:25]: > > Move mach-davinci/dma.c to common/edma.c so it can be used > > by OMAP (specifically AM33xx) as well. > > I think this should rather go to drivers/dma/? Yes, it should, but just like OMAP, there's a conversion effort that needs to be gone through. It has one point - and only one point - which allows its continued existence under arch/arm, and that is it already exists there. If it was new code, the answer would be a definite NACK, but it isn't. It's pre-existing code which is already in mainline. It's merely being moved. Another plus point for it is that there does seem to be a DMA engine driver for it, so hopefully we'll see it killed off in arch/arm soon.
On Sat, Feb 02, 2013 at 12:49:06PM +0000, Russell King wrote: > On Fri, Feb 01, 2013 at 10:41:08AM -0800, Tony Lindgren wrote: > > * Matt Porter <mporter@ti.com> [130201 10:25]: > > > Move mach-davinci/dma.c to common/edma.c so it can be used > > > by OMAP (specifically AM33xx) as well. > > > > I think this should rather go to drivers/dma/? > > Yes, it should, but just like OMAP, there's a conversion effort that needs > to be gone through. It has one point - and only one point - which allows > its continued existence under arch/arm, and that is it already exists > there. > > If it was new code, the answer would be a definite NACK, but it isn't. > It's pre-existing code which is already in mainline. It's merely being > moved. > > Another plus point for it is that there does seem to be a DMA engine > driver for it, so hopefully we'll see it killed off in arch/arm soon. That's definitely the plan. I was able to start this effort independently by converting the Davinci mmc and spi drivers to dmaengine before I took this step. I've got the next micro-step of addressing omap_hsmmc in process (pending on agreement on a dmaengine api change with Vinod), cleaning up the mcasp driver is also pending this series so that it can also be converted to dmaengine. Once mcasp (or "davinci audio") is converted, we're rid of all the in-kernel users of the private API and can get rid of this...which also is helping clean up mach-davinci, of course. If I can get your ack on this patch that should move things along to these next steps. Thanks, Matt
Hello. On 02-02-2013 14:18, Russell King - ARM Linux wrote: >>>>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: >>>>>>>> good point, do you wanna send some patches ? >>>>>>> I have already sent them countless times and even stuck CPPI 4.1 support (in >>>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >>>>>>> patch. :-( >>>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so >>>>>> wasn't asking for the patch to be removed :-s >>>>> Err, patches don't get removed, they get moved to 'discarded'. >>>> Any chance to bring it back to life? :-) >>>> Although... drivers/usb/musb/cppi41.c would need to be somewhat >>>> reworked for at least AM35x and I don't have time. But that may change, >>>> of course. >>> Right, I've just looked back at the various meeting minutes from December >>> 2010 when the CPPI stuff was discussed. Yes, I archive these things and >>> all email discussions for referencing in cases like this. >> Thanks. >>> Unfortunately, they do not contain any useful information other than the >>> topic having been brought up. At that point, the CPPI stuff was in >>> mach-davinci, and I had suggested moving it into drivers/dma. >> I don't remember that, probably was out of the loop again. I looked back at the history of CPPI 4.1 driver related threads, and found that Kevin Hilman gas suggested it too while the driver was in mach-davinci/ still... >>> The result of that was to say that it doesn't fit the DMA engine APIs. Right, I tried to fit it (in my thought only though) in and it didn't work out. >> I remember this as a discussion happening post me sending the patch to >> the patch system and it being discarded... Well, actually before doing this too... >>> So someone came up with the idea of putting it in arch/arm/common - which >> Probably was me. No, it was someone from TI. >> There was also idea of putting it into >> drivers/usb/musb/ -- which TI indeed followed in its Arago prject. I >> firmly denied that suggestion. Moving it to drivers/usb/ is probably the reason TI has been quite content with the situation -- their clients kept receiving MUSB DMA support on both OMAP-L1x and then Sitara, so all looked well for them. >>> I frankly ignored by email (how long have we been saying "no drivers in >>> arch/arm" ?) Well, maybe you should have said it one more time for those who were late in the game like me. >> But there *are* drivers there! And look at edma.c which is about to be >> moved there... Anyway, I haven't seen such warnings, probably was too >> late in the game. > I've already objected about the header moving to some random place in > arch/arm/include. Really, edma.c needs to find another home too - but > there's a difference here. edma.c is already present under arch/arm. > CPPI is _not_. CPPI is new code appearing under arch/arm (you can see > that for yourself by looking at the diffstat of 6305/1... it doesn't > move files, it adds new code.) Yes, of course, that's clear. >>> Now, it would've been discussed in that meeting, but unfortunately no >>> record exists of that. What does follow that meeting is a discussion >>> trail. From what I can see there, but it looks to me like the decision >>> was taken to move it to the DMA engine API, and work on sorting out MUSB >>> was going to commence. >>> The last email in that says "I'll get to that soon"... and that is also >>> the final email I have on this topic. I guess if nothing has happened... >>> Shrug, that's someone elses problem. >> Well, as usual... :-( >>> Anyway, the answer for putting it in arch/arm/common hasn't changed, >>> and really, where we are now, post Linus having a moan about the size >>> of arch/arm, that answer is even more concrete in the negative. It's >>> 54K of code which should not be under arch/arm at all. >>> Anyway, if you need to look at the patch, it's 6305/1. Typing into the >>> summary search box 'cppi' found it in one go. >> Thanks, I remember this variant was under arch/arm/common/. >> Now however, I see what happened to that variant in somewhat different >> light. Looks like it was entirely your decision to discard the patch, >> without TI's request... > Firstly, it is *my* perogative to say no to anything in arch/arm, and I > really don't have to give reasons for it if I choose to. That's clear. You're the ARM King. :-) > Secondly, it *was* discussed with TI, and the following thread of > discussion (threaded to the minutes email) shows that *something* was > going to happen _as a result of that meeting_ to address the problem of > it being under arch/arm. And *therefore* it was discarded from the patch > system - because there was expectation that it was going to get fixed. > For christ sake, someone even agreed to do it. Even a target was mentioned, > of 2.6.39. That was mentioned on 7th December 2010. And 6305/1 was > discarded on 8th December 2010. Cause and effect. > And yes, *you* were not part of that discussion. You work for Montavista > which contracts with TI to provide this support. Here you're not quite correct. TI did not prolongate contgract with MV after our releasing the support for OMAP-L137, which is early 2009, AFAIR. > It is up to TI to pass > stuff like this on to their contractors. As you can see, TI didn't feel obliged to do so already. > There are two people on this thread CC list who were also involved or > CC'd on the mails from the thread in 2010... Tony and Felipe. > Unfortunately, the person who agreed to do the work is no longer in the > land of the living. Yes I know it's inconvenient for people to die > when they've still got lots of important work to do but that's what can > happen... Hm... wasn't it David Brownell? He's the only person who I know has died recently who has dealt with DaVinci, MUSB and the releated stuff. WBR, Sergei
On Sat, Feb 02, 2013 at 08:27:42PM +0400, Sergei Shtylyov wrote: >> There are two people on this thread CC list who were also involved or >> CC'd on the mails from the thread in 2010... Tony and Felipe. >> Unfortunately, the person who agreed to do the work is no longer in the >> land of the living. Yes I know it's inconvenient for people to die >> when they've still got lots of important work to do but that's what can >> happen... > > Hm... wasn't it David Brownell? He's the only person who I know has > died recently who has dealt with DaVinci, MUSB and the releated stuff. Actually, it wasn't David who was going to do it - that's where the email thread gets messy because the mailer David was using makes no distinction in text format between what bits of text make up the original email being replied to, and which bits of text are written by David. It might have been Felipe; there appears to be an email from Felipe saying that as the immediate parent to David's email. But that's not really the point here. The point is that _someone_ agreed to put the work in, and _that_ agreement is what caused the patch to be discarded. And, as I've already explained, you brought up the subject of it being discarded shortly after, and it got discussed there _again_, and the same things were said _again_ by at least two people about it being in drivers/dma. But anyway, that's all past history. What was said back then about it being elsewhere in the tree is just as relevant today as it was back then. The only difference is that because that message wasn't received, it's now two years later with no progress on that. And that's got *nothing* what so ever to do with me. I know people like to blame me just because I'm apparantly the focus of the blame culture, but really this is getting beyond a joke. So, I want an apology from you for your insistance that I'm to blame for this. Moreover, _I_ want to know what is going to happen in the future with this so that I don't end up being blamed anymore for the lack of progress on this issue.
Hello. On 02-02-2013 16:17, Russell King - ARM Linux wrote: >>>>>>>>> good point, do you wanna send some patches ? >>>>>>>> I have already sent them countless times and even stuck CPPI 4.1 support (in >>>>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the >>>>>>>> patch. :-( >>>>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so >>>>>>> wasn't asking for the patch to be removed :-s >>>>>> Err, patches don't get removed, they get moved to 'discarded'. >>>>> Any chance to bring it back to life? :-) >>>>> Although... drivers/usb/musb/cppi41.c would need to be somewhat >>>>> reworked for at least AM35x and I don't have time. But that may change, >>>>> of course. >>>> Right, I've just looked back at the various meeting minutes from December >>>> 2010 when the CPPI stuff was discussed. Yes, I archive these things and >>>> all email discussions for referencing in cases like this. >>> Thanks. Thanks again for taking your time to rummage thru the mail archives. I also did it, however not thru all threads (it turned out that the placement of CPPI 4.1 code was discussed also in the MUSB DMA driver related threads). Anyway, I was not a position to do extensive searching as it was already dead of the night in Moscow. >>>> Unfortunately, they do not contain any useful information other than the >>>> topic having been brought up. At that point, the CPPI stuff was in >>>> mach-davinci, and I had suggested moving it into drivers/dma. >>> I don't remember that, probably was out of the loop again. > Here you go - this goes back even _further_ - November 2009 - on the > mailing list. The entire thread: > http://lists.arm.linux.org.uk/lurker/thread/20091102.105759.a54cf3f5.en.html > And selected emails from it: > http://lists.arm.linux.org.uk/lurker/message/20091102.103706.38c029b5.en.html > On Mon, Nov 02, 2009 at 10:37:06AM +0000, I wrote: > | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote: > | > Another option is to create arch/arm/ti-common to place all TI platform's > | > common software, such as CPPI4.1 used both in DA8xx and AM3517. > | > | No thanks. I really don't see why we should allow TI to have yet more > | directories scattered throughout the tree that are out of place with > | existing conventions. > | > | And what is this CPPI thing anyway? > | > | http://acronyms.thefreedictionary.com/CPPI > | > | "Communications Port Programming Interface" seems to be about the best > | applicable one from that list! > | > | If it's a USB DMA device (from the patches I can find, that seems to be > | the case) then why can't it live in drivers/usb or drivers/dma ? > And again: > http://lists.arm.linux.org.uk/lurker/message/20091102.115458.61cde450.en.html > On Mon, Nov 02, 2009 at 11:54:58AM +0000, I wrote: > | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote: > | > CPPI4.1 DMA engine can be used either by USB or by Ethernet interface though > | > currently only USB is using it but in future even Ethernet devices may use it. > | > | drivers/dma does seem to be the right place for this. > http://lists.arm.linux.org.uk/lurker/message/20091102.110217.adec3ca7.en.html > Even Felipe Balbi said so: > | you might want to provide support for it via drivers/dma and for the > | musb stuff, you just add the wrappers musb uses. See how tusb6010_omap.c > | uses OMAP's system dma which is also used by any other driver which > | requests a dma channel. > And it seems that _YOU_ did get the message - see your quoted text in: > http://lists.arm.linux.org.uk/lurker/message/20091230.132240.ecd56b3d.en.html >> We're currently having it there but the matter is it should be shred >> between different platforms, so arch/arm/common/ seems like the right >> place (which Russell didn't like, suggesting ill suited for that >> drivers/dma/ instead). > See - you acknowledge here that I don't like it. So you _KNOW_ my views > on it in December 2009, contary to what you're saying in this thread. OK, now it seems I misremembered. > Yet, you persisted with putting it in arch/arm/common: Being unable to fit it into drivers/dma/, and loating to place it into drivers/usb/, I had no other option. :-) > http://lists.arm.linux.org.uk/lurker/message/20100515.181453.472c7c10.en.html > | Changes since the previous version: > | - moved everything from arch/arm/mach-davinci/ to arch/arm/common/; > | - s/CONFIG_CPPI41/CONFIG_TI_CPPI41/, made that option invisible; > | - added #include <linux/slab.h> for kzalloc(); > | - switched alloc_queue() and cppi41_queue_free() to using bit operations; > | - replaced 'static' linking_ram[] by local variable in cppi41_queue_mgr_init(); > | - fixed pr_debug() in cppi41_dma_ctrlr_init() to print the real queue manager #. > So, see, I had already objected to it being in arch/arm well before > you stuck your patch into the patch system. And somehow you think > that ignoring my previous comments and doing it anyway will result in > progress? I probably did that out of hopelessness partly. > So, let's recap. The timeline behind this is: > + 2 Nov 2009: Question asked about putting it in arch/arm/ti-common > + I responded saying a clear no to that, suggesting other locations > all of which were outside arch/arm. > + I responded again saying an hour or two later saying the same thing. > + Felipe Balbi agreed with drivers/dma. > + 15 May 2010: v5 posted with it in arch/arm/common > + 06 Aug 2010: put into patch system sa 6305/1 > + 06 Dec 2010: TI meeting. > + Pre-meeting notes show that my views on this are known: > + I'll quote this from it: "Russell suggested to move the driver at > drivers/dma/". > + Raises concern that DMA engine API may not fit. > + I respond to that concern as work has been done on the DMA engine API > to improve the slave mode support recently as a result of Linus Walleij's > work on AMBA PL08x DMA support. > (I would _not_ have done so if I had changed my view about it being > under drivers/dma/). Unfortunately, as I see it, that work is far from enough for CPPI 4.1. I maybe don't know drivers/dma/ slave mode well, but adding CPPI 4.1 support would have required a complete redesign of all related interfaces. > + 07 Dec 2010: emails talking about moving MUSB over to DMA engine API > so that MUSB should not care about its DMA backend (that being CPPI > or some other one.) As we know, it was a bad idea generally. > + Email with "Let's see if I can get it all done by 2.6.39." Didn't ever see that one. > + 08 Dec 2010: patch 6305/1 discarded from the patch system as there now > seems to be concensus on the issue. > + 03 Jan 2011: you ask me why it was discarded > http://lists.arm.linux.org.uk/lurker/thread/20110103.160610.8cbe8e7d.en.html > + I respond "It may have been that it's inventing its own API rather > than using something like the DMA engine API." > + Ajay said: "This issue was discussed recently at TI and proposal was > to place it to drivers/dma folder. Moreover, even Felipe also seems > to move other musb DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma." > Oh, and then we come to this interesting email from Felipe to you, in > response to your reluctance to put it in drivers/dma. It was in reposnse to Ajay's email about Felipe's assumed intent to switch MUSB to drivers/dma/ API insted of its custom DMA API, AFAIR. To which I, of course, started to object. It wasn't about CPPI 4.1 driver at all. > | Do I really have to spell it out ? Really ? > | > | You don't need to physically move the part of the code to drivers/dma, > | but it has to use the API. The mentor DMA is internal to MUSB. > | tusb6010_omap.c isn't. > | > | Where it makes sense to move the code under drivers/dma, it will be > | done, where it doesn't, it won't be done, but it will use the same API. > | That's all. > | > | The end goal is just to drop all these ad-hoc "APIs" for accessing DMA > | on musb code. Now I don't even quite understand what (multiple?) ad-hoc APIs Felipe had in mind. There\ is one and only one DMA API in MUSB. Perhaps he meant the tusb6010_omap.c driver which is using OMAP DMA API? > See the common theme here? I don't like it under arch/arm. I've been > pretty _consistent_ in suggesting drivers/dma/ all through that... > Others have said it. People even acknowledge that's what I've been > saying, people who were not in the original discussion. > What I think is this: it is _YOU_ who don't want to hear that message, > so _YOU_ are intentionally ignoring it, and _YOU_ are looking for > someone to blame for it. You've decided I'm to blame because _YOU_ > aren't listening. My intent wasn't to lay blame on anybody, just to restore the sequence of events which passed completely around me. > The reason there hasn't been any progress on this is _NOT_ down to me. > I've provided my feedback, and promptly been ignored. Others have told > you the same thing, and promptly been ignored. Sorry, this is not my > problem. This is entirely YOUR problem, one of your own making. I didn't really ignore requests to move to drivers/dma/, I just honestly didn't see how it can be done, and frankly didnt have much time to massively rewrite my stuff written back in 2008 based on earlier TI work. Then my project to push OMAP-L137 support simply ended (I can't restore the date now, perhaps this was somewhere in 2010), so I simply couldn't continue to invest big efforts in this work. > But whatever. We are NOT going to put CPPI under arch/arm. Not now that > during the last merge window Linus complained to Arnd about the amount of > code coming through for arch/arm _AGAIN_. Not after last time Linus > complained about TI OMAP which prompted the creation of arm-soc. AND THAT > IS *FINAL*. CPPI DMA SUPPORT IS *NOT* GOING UNDER ARCH/ARM. PERIOD. NOT > GOING TO HAPPEN. > Is this clear enough yet, or how many more years of emails do you need > yet more emails to get this message through? It's crystal clear, thank you. I just thought something might have changed seeing how EDMA support moves to arch/arm/common/. Now I see it's not. WBR, Sergei
Hello. On 02-02-2013 20:45, Russell King - ARM Linux wrote: >>> There are two people on this thread CC list who were also involved or >>> CC'd on the mails from the thread in 2010... Tony and Felipe. >>> Unfortunately, the person who agreed to do the work is no longer in the >>> land of the living. Yes I know it's inconvenient for people to die >>> when they've still got lots of important work to do but that's what can >>> happen... >> Hm... wasn't it David Brownell? He's the only person who I know has >> died recently who has dealt with DaVinci, MUSB and the releated stuff. > Actually, it wasn't David who was going to do it - that's where the email > thread gets messy because the mailer David was using makes no distinction > in text format between what bits of text make up the original email being > replied to, and which bits of text are written by David. Hm, strange... > It might have been Felipe; there appears to be an email from Felipe saying > that as the immediate parent to David's email. But that's not really the > point here. The point is that _someone_ agreed to put the work in, and > _that_ agreement is what caused the patch to be discarded. > And, as I've already explained, you brought up the subject of it being > discarded shortly after, and it got discussed there _again_, and the > same things were said _again_ by at least two people about it being in > drivers/dma. It wasn't said that somebody concrete was going to work on it. I had to explcitly write an email laying all further responsibility on CPPI 4.1 support on TI back then. > But anyway, that's all past history. What was said back then about it > being elsewhere in the tree is just as relevant today as it was back > then. The only difference is that because that message wasn't received, > it's now two years later with no progress on that. And that's got > *nothing* what so ever to do with me. Yes, of course. In my original mail that started the discussion I said that we have to wait indefinitely for TI to write the new DMA driver. I just wondered wouldn't it be better to use the same approach as for EDMA with transitioning to drivers/dma/ step by step. > I know people like to blame me just because I'm apparantly the focus of > the blame culture, but really this is getting beyond a joke. > So, I want an apology from you for your insistance that I'm to blame > for this. OK, I apologise if you consider yourself the target of my blame. My aim was rather to establish the truth about that decision taken back in Dec 2010 -- which we seem to have achieved. > Moreover, _I_ want to know what is going to happen in the > future with this so that I don't end up being blamed anymore for the > lack of progress on this issue. Nothing. My blame for the lack of progress has long been laid on TI, after I explictly passed the responsibility for the driver to them. My intent with the mail that started the discussion was to probe whether we still have another opportunity of having MUSB DMA support for OMAP-L1x and Sitara. I just thought that you might have changed your mind somehow on the matter. WBR, Sergei
Hello. On 02-02-2013 16:45, Russell King - ARM Linux wrote: > Now, CPPI is brand new code to arch/arm - always has been. It post-dates > the DMA engine API. And it's been said many times about moving it to > drivers/dma. The problem is Sergei doesn't want to do it - he's anti the I *can't* do it, and I have denied all further responibility for it. > DMA engine API for whatever reasons he can dream up. He professes no I'm not dreaming anything up. Please understand that CPPI 4.1 is not just a normal DMA controller -- it's a complex of several devices with DMA controller being only one of them, and one that can't work autonomously but only thru the proxy of the queue manager. That queue manager stuff I found hard to fit into drivers/dma/ infrastructure. Myabe it was a honest mistake, of course. > knowledge of my dislike for having it in arch/arm, yet there's emails > from years back showing that he knew about it. TI knows about it; Ajay > about it. Yet... well... history speaks volumes about this. Some details have slipped rom my memory after that many years. Im sorry for that. > Now, there may be a new problem with CPPI. That being we're now requiring > DT support. _Had_ this been done prior to the push for DT, then it would > not have been a concern, because then the problem becomes one for DT. > But now that OMAP is being converted to DT, and DT is The Way To Go now, > that's an additional problem that needs to be grappled with - or it may > make things easier. DaVinci is also being converted to device tree. However, that support remains optional for now. Not sure it will make things easier, as we still have two distinct devices to declare in the device tree: DMA controller and queue manager, and we'll have to describe the interconnect between them too (as a props of DMA controller I guess)... WBR, Sergei
On Sat, Feb 02, 2013 at 12:01:37AM +0000, Sergei Shtylyov wrote: > Hello. > > On 01-02-2013 22:59, Matt Porter wrote: > > >>>>> Move mach-davinci/dma.c to common/edma.c so it can be used > >>>>> by OMAP (specifically AM33xx) as well. > > >>>> I think this should rather go to drivers/dma/? > > >>> No, this is the private EDMA API. It's the analogous thing to > >>> the private OMAP dma API that is in plat-omap/dma.c. The actual > >>> dmaengine driver is in drivers/dma/edma.c as a wrapper around > >>> this...same way OMAP DMA engine conversion is being done. > > >> Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed > >> that, instead of waiting indefinitely for TI to convert it to drivers/dma/ > >> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. > > > That is a shame. Yeah, I've pointed out that I was doing this exactly > > the same way as was acceptable for the OMAP DMA conversion since it was > > in RFC. The reasons are sound since in both cases, we have many drivers > > to convert that need to continue using the private DMA APIs. > > In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other > in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is > sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't > know them well). Well, it's pretty clear to me now that there's good reason for it not landing in arch/arm/ so the obvious path is to do the dmaengine conversion and put it in drivers/dma/ if it's really a generic dma engine. I'm not sure why you express concern over the dma engine api not fitting with CPPI 4.1? If it doesn't work, work with Vinod to fix the api. It's expected, I'm working on dmaengine API changes right now to deal with a limitation of EDMA that needs to be abstracted. As pointed out, edma.c is already here in arch/arm/, so moving it doesn't add something new. It does let us regression test all platforms that use it (both Davinci and AM33xx) as I go through the conversion process. -Matt
* Matt Porter <mporter@ti.com> [130202 10:10]: > If it doesn't work, work with Vinod to fix the api. It's expected, > I'm working on dmaengine API changes right now to deal with a > limitation of EDMA that needs to be abstracted. Regarding the DMA API limitations, I'm only aware of lack of capability to configure autoincrement at the device end. And that keeps us from converting all GPMC related devices using omap SDMA to use the DMA API. Are there other limitations currently known with the DMA API? Regards, Tony
Hello. On 02-02-2013 22:07, Matt Porter wrote: >>>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used >>>>>>> by OMAP (specifically AM33xx) as well. >>>>>> I think this should rather go to drivers/dma/? >>>>> No, this is the private EDMA API. It's the analogous thing to >>>>> the private OMAP dma API that is in plat-omap/dma.c. The actual >>>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around >>>>> this...same way OMAP DMA engine conversion is being done. >>>> Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed >>>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/ >>>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. >>> That is a shame. Yeah, I've pointed out that I was doing this exactly >>> the same way as was acceptable for the OMAP DMA conversion since it was >>> in RFC. The reasons are sound since in both cases, we have many drivers >>> to convert that need to continue using the private DMA APIs. >> In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other >> in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is >> sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't >> know them well). > Well, it's pretty clear to me now that there's good reason for it not > landing in arch/arm/ so the obvious path is to do the dmaengine > conversion and put it in drivers/dma/ if it's really a generic dma engine. > I'm not sure why you express concern over the dma engine api not fitting > with CPPI 4.1? It's not a DMA controller only, it's 3 distinct devices, with the DMA controller being one among them and using another one, the queue manager, as some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's the buffer manager. > If it doesn't work, work with Vinod to fix the api. It's > expected, I'm working on dmaengine API changes right now to deal with a > limitation of EDMA that needs to be abstracted. Sorry, now it's TI's task. I no longer have time to work on this (my internal project to push OMAP-L1x support upstream has expired at Sep 2010) and my future in MV is very uncertain at this moment. Most probably I'll leave it (or be forced to leave). > As pointed out, edma.c is already here in arch/arm/, so moving it doesn't > add something new. It does let us regression test all platforms that use it > (both Davinci and AM33xx) as I go through the conversion process. Could have been the same with CPPI 4.1 in theory if it was added to mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is much older of course, so it's probably more justified. > -Matt WBR, Sergei
On Sat, Feb 02, 2013 at 10:16:43AM -0800, Tony Lindgren wrote: > * Matt Porter <mporter@ti.com> [130202 10:10]: > > If it doesn't work, work with Vinod to fix the api. It's expected, > > I'm working on dmaengine API changes right now to deal with a > > limitation of EDMA that needs to be abstracted. > > Regarding the DMA API limitations, I'm only aware of lack of capability > to configure autoincrement at the device end. And that keeps us from > converting all GPMC related devices using omap SDMA to use the DMA API. > > Are there other limitations currently known with the DMA API? Yes, I called one out when this was first posted as an RFC and was working it in parallel with this support. This one blocks AM33XX support in mmc and is the reason I split all of the mmc support out of the base edma dmaengine for am33xx series. Independent of the blockage, whatever we finally settle on to address this API need will also cleanup the use of magic values in the davinci mmc driver (that's part of the second thread below). RFC posting: https://patchwork.kernel.org/patch/1583531/ Posting with initial attempt at a caps api: http://www.spinics.net/lists/linux-mmc/msg18351.html Also, I haven't fully vetted the situation with cyclic transfers and EDMA, however, I'm pretty sure that we'll need some API changes there. The reason is that some Davinci platforms have no FIFO on their McASP implementation (that was a new feature added on DA8xx and also AM33xx). As such they have audio support implemented using ping-pong buffering via an SRAM buffer. There's been a number of patches ahead of all this that myself and others have worked upstream to get the SRAM stuff to be Davinci-independent (genalloc). But, at the end of all of this, there's no notion in a cyclic transfer of doing synchronized ping-pong buffering using two chain channels. This is how it is implemented (and documented in EDMA docs going back to the DSPs this IP first showed up on) using the private API. I'll be looking at this soon, but, I'm more interested in 1) getting the base support in 2) then the mmc stuff blocking DT populated platforms using omap_hsmmc (split out and posted) 3) v3 of the caps api w/ vinod's concerns address (working it not) Normally, I'm not going to bring up the cyclic transfer issue until I have some code to show or reference for discussion...but it's worth being aware. But, in any case, I'm confident that will gate having the mcasp driver that am33xx also uses converted to dmaengine. Except for the gpmc limitation you menationed, that's the last in kernel user of the privat edma api needed to be converted such that we can kill off arch/arm/common/edma.c once it's taken in. -Matt
On Sat, Feb 02, 2013 at 07:06:06PM +0000, Sergei Shtylyov wrote: > Hello. > > On 02-02-2013 22:07, Matt Porter wrote: > > >>>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used > >>>>>>> by OMAP (specifically AM33xx) as well. > > >>>>>> I think this should rather go to drivers/dma/? > > >>>>> No, this is the private EDMA API. It's the analogous thing to > >>>>> the private OMAP dma API that is in plat-omap/dma.c. The actual > >>>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around > >>>>> this...same way OMAP DMA engine conversion is being done. > > >>>> Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed > >>>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/ > >>>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. > > >>> That is a shame. Yeah, I've pointed out that I was doing this exactly > >>> the same way as was acceptable for the OMAP DMA conversion since it was > >>> in RFC. The reasons are sound since in both cases, we have many drivers > >>> to convert that need to continue using the private DMA APIs. > > >> In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other > >> in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is > >> sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't > >> know them well). > > > Well, it's pretty clear to me now that there's good reason for it not > > landing in arch/arm/ so the obvious path is to do the dmaengine > > conversion and put it in drivers/dma/ if it's really a generic dma engine. > > I'm not sure why you express concern over the dma engine api not fitting > > with CPPI 4.1? > > It's not a DMA controller only, it's 3 distinct devices, with the DMA > controller being one among them and using another one, the queue manager, as > some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's > the buffer manager. Interesting, and you don't see a way to have this split between dmaengine and the musb driver? This all assumes there's even a match as RMK did raise concerns elsewhere in this thread. > > If it doesn't work, work with Vinod to fix the api. It's > > expected, I'm working on dmaengine API changes right now to deal with a > > limitation of EDMA that needs to be abstracted. > > Sorry, now it's TI's task. I no longer have time to work on this (my > internal project to push OMAP-L1x support upstream has expired at Sep 2010) > and my future in MV is very uncertain at this moment. Most probably I'll leave > it (or be forced to leave). Too bad, it's certainly "somebody's task". The people employed by TI have names too. ;) I suspect it falls to Felipe or Kishon these days, but I try to avoid USB as it's generally a source of pain. > > As pointed out, edma.c is already here in arch/arm/, so moving it doesn't > > add something new. It does let us regression test all platforms that use it > > (both Davinci and AM33xx) as I go through the conversion process. > > Could have been the same with CPPI 4.1 in theory if it was added to > mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is > much older of course, so it's probably more justified. Absolutely, timing is everything. I can assure you I've flamed enough people "internally" about leaving this EDMA dmaengine conversion for so long. As you might have guessed, AM33xx is a bit of a driver for it, but all of this would have been quite a bit simpler had somebody taken a little effort and moved edma to dmaengine years ago when slave dma support was added to dmaengine. ;) -Matt
Hello. On 02-02-2013 23:55, Matt Porter wrote: >>>>>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used >>>>>>>>> by OMAP (specifically AM33xx) as well. >>>>>>>> I think this should rather go to drivers/dma/? >>>>>>> No, this is the private EDMA API. It's the analogous thing to >>>>>>> the private OMAP dma API that is in plat-omap/dma.c. The actual >>>>>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around >>>>>>> this...same way OMAP DMA engine conversion is being done. >>>>>> Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed >>>>>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/ >>>>>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. >>>>> That is a shame. Yeah, I've pointed out that I was doing this exactly >>>>> the same way as was acceptable for the OMAP DMA conversion since it was >>>>> in RFC. The reasons are sound since in both cases, we have many drivers >>>>> to convert that need to continue using the private DMA APIs. >>>> In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other >>>> in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is >>>> sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't >>>> know them well). >>> Well, it's pretty clear to me now that there's good reason for it not >>> landing in arch/arm/ so the obvious path is to do the dmaengine >>> conversion and put it in drivers/dma/ if it's really a generic dma engine. >>> I'm not sure why you express concern over the dma engine api not fitting >>> with CPPI 4.1? >> It's not a DMA controller only, it's 3 distinct devices, with the DMA >> controller being one among them and using another one, the queue manager, as >> some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's >> the buffer manager. > Interesting, and you don't see a way to have this split between > dmaengine and the musb driver? This can't be split into the MUSB DMA driver, as the neither of CPPI 4.1 devices are really MUSB specific. The support should be generic. > This all assumes there's even a match as > RMK did raise concerns elsewhere in this thread. Didn't quite get this sentence. >>> If it doesn't work, work with Vinod to fix the api. It's >>> expected, I'm working on dmaengine API changes right now to deal with a >>> limitation of EDMA that needs to be abstracted. >> Sorry, now it's TI's task. I no longer have time to work on this (my >> internal project to push OMAP-L1x support upstream has expired at Sep 2010) If not somewhat earlier... anyway, I wasn't able to spent much time on this project in 2010. >> and my future in MV is very uncertain at this moment. Most probably I'll leave >> it (or be forced to leave). > Too bad, it's certainly "somebody's task". The people employed by TI > have names too. ;) I suspect it falls to Felipe or Kishon these days, > but I try to avoid USB as it's generally a source of pain. I'm probably masochistic then since I'm still sending MUSB patches, eben though I wasn't working on it at MV until I switched to Kontron boards 2 weeks ago. Now my task is fixing USB bugs on real Sitara with dual MUSB. :-) >>> As pointed out, edma.c is already here in arch/arm/, so moving it doesn't >>> add something new. It does let us regression test all platforms that use it >>> (both Davinci and AM33xx) as I go through the conversion process. >> Could have been the same with CPPI 4.1 in theory if it was added to >> mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is I don't know why Kevin didn't merge it then. I remembere he didn't like that it was not a proper platform driver and was tied with the platform code thru some variables, and I refused to change that... >> much older of course, so it's probably more justified. > Absolutely, timing is everything. I can assure you I've flamed enough > people "internally" about leaving this EDMA dmaengine conversion for so > long. As you might have guessed, AM33xx is a bit of a driver for it, but > all of this would have been quite a bit simpler had somebody taken a > little effort and moved edma to dmaengine years ago when slave dma > support was added to dmaengine. ;) Hm, it seems to have happened back in 2008 when I was working on CPPI 4.1 code. Too bad I only knew that drivers/dma/ are for accelerating RAIDs back then (if actually noot later than that). TI seems to put more efforts into its Arago project than in supoporting the cutting edge (or not) CPUs in mainline -- at lest things seem go there first. Many Arago patches never reached mainline (not that they can be applied without cleanup though). > -Matt WBR, Sergei
* Matt Porter <mporter@ti.com> [130202 11:51]: > On Sat, Feb 02, 2013 at 10:16:43AM -0800, Tony Lindgren wrote: > > * Matt Porter <mporter@ti.com> [130202 10:10]: > > > If it doesn't work, work with Vinod to fix the api. It's expected, > > > I'm working on dmaengine API changes right now to deal with a > > > limitation of EDMA that needs to be abstracted. > > > > Regarding the DMA API limitations, I'm only aware of lack of capability > > to configure autoincrement at the device end. And that keeps us from > > converting all GPMC related devices using omap SDMA to use the DMA API. > > > > Are there other limitations currently known with the DMA API? > > Yes, I called one out when this was first posted as an RFC and was > working it in parallel with this support. This one blocks AM33XX support > in mmc and is the reason I split all of the mmc support out of the base > edma dmaengine for am33xx series. Independent of the blockage, whatever > we finally settle on to address this API need will also cleanup the use > of magic values in the davinci mmc driver (that's part of the second > thread below). > > RFC posting: > https://patchwork.kernel.org/patch/1583531/ > Posting with initial attempt at a caps api: > http://www.spinics.net/lists/linux-mmc/msg18351.html OK thanks for the links, good to hear at least some work is happening on it. > Also, I haven't fully vetted the situation with cyclic transfers and > EDMA, however, I'm pretty sure that we'll need some API changes there. > The reason is that some Davinci platforms have no FIFO on their McASP > implementation (that was a new feature added on DA8xx and also AM33xx). > As such they have audio support implemented using ping-pong buffering > via an SRAM buffer. There's been a number of patches ahead of all this > that myself and others have worked upstream to get the SRAM stuff to be > Davinci-independent (genalloc). But, at the end of all of this, there's > no notion in a cyclic transfer of doing synchronized ping-pong buffering > using two chain channels. This is how it is implemented (and documented > in EDMA docs going back to the DSPs this IP first showed up on) using > the private API. I'll be looking at this soon, but, I'm more interested > in 1) getting the base support in 2) then the mmc stuff blocking DT > populated platforms using omap_hsmmc (split out and posted) 3) v3 of the > caps api w/ vinod's concerns address (working it not) > > Normally, I'm not going to bring up the cyclic transfer issue until I > have some code to show or reference for discussion...but it's worth > being aware. But, in any case, I'm confident that will gate having the > mcasp driver that am33xx also uses converted to dmaengine. Except for > the gpmc limitation you menationed, that's the last in kernel user of > the privat edma api needed to be converted such that we can kill off > arch/arm/common/edma.c once it's taken in. And then there's the case of device-to-device DMA that we're not currently doing luckily. And I don't think I've even seen that being used so we probably don't need to worry about that one right now. Regards, Tony
On Saturday 02 February 2013 04:07:59 Sergei Shtylyov wrote: > On 02-02-2013 1:30, Russell King - ARM Linux wrote: > >> because it doesn't make sense to support multiple DMA APIs. We can check > >> from MUSB's registers if it was configured with Inventra DMA support and > >> based on that we can register MUSB's own DMA Engine to dmaengine API. > > > Hang on. This is one of the DMA implementations which is closely > > coupled with the USB and only the USB? If it is... > > > I thought this had been discussed _extensively_ before. I thought the > > resolution on it was: > > 1. It would not use the DMA engine API. > > 2. It would not live in arch/arm. > > 3. It would be placed nearby the USB driver it's associated with. > > > (1) because we don't use APIs just for the hell of it - think. Do we > > use the DMA engine API for PCI bus mastering ethernet controllers? No. > > Do we use it for PCI bus mastering SCSI controllers? No. Because the > > DMA is integral to the rest of the device. > > > The DMA engine API only makes sense if the DMA engine is a shared > > system resource. > > Thanks for such extensive wording in the support of my point. Actually there is another problem with the musb driver, which is that you can only have one DMA controller configured at build time, and that currently prevents us from building a kernel that supports both the Inventra and TUSB variants together. With multiplatform coming up for both ux500 and omap2, we will also have to make a decision about ux500 vs omap being supported in DMA mode. I can see two ways out of this: either we extend the musb driver to have run-time registration of the DMA controller, which duplicates infrastructure that already exists in the dmaengine API, or we make the dmaengine back-end for musb the default and require all platforms to use that if they want to coexist with other platforms and also use DMA support in musb. Note that the ux500_dma code in musb already uses dmaengine. It could probably be extended to support any platform that has a dmaengine driver for the DMA controller used in MUSB. Arnd
Hi, On Fri, Feb 01, 2013 at 09:30:03PM +0000, Russell King - ARM Linux wrote: > > > > I guess to make the MUSB side simpler we would need musb-dma-engine glue > > > > to map dmaengine to the private MUSB API. Then we would have some > > > > starting point to also move inventra (and anybody else) to dmaengine > > > > API. > > > > > > Why? Inventra is a dedicated device's private DMA controller, why make > > > universal DMA driver for it? > > > > because it doesn't make sense to support multiple DMA APIs. We can check > > from MUSB's registers if it was configured with Inventra DMA support and > > based on that we can register MUSB's own DMA Engine to dmaengine API. > > Hang on. This is one of the DMA implementations which is closely > coupled with the USB and only the USB? If it is... > > I thought this had been discussed _extensively_ before. I thought the > resolution on it was: > 1. It would not use the DMA engine API. > 2. It would not live in arch/arm. > 3. It would be placed nearby the USB driver it's associated with. > > (1) because we don't use APIs just for the hell of it - think. Do we > use the DMA engine API for PCI bus mastering ethernet controllers? No. > Do we use it for PCI bus mastering SCSI controllers? No. Because the > DMA is integral to the rest of the device. that's not really a fair comparison, however. MUSB is used with several DMA engines. The only DMA engine which is really coupled with MUSB is the Inventra DMA engine which comes as an optional feature to the IP. Many users have opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. Granted, CPPI 4.1 makes some assumptions about the fact that it's handling USB tranfers, but nevertheless, the IP can be, and in fact is, used with many different DMA engines and driver needs to cope with it. Current DMA abstraction is quite poor, for example there's no way to compile support for multiple DMA engines. Code also makes certain, IMO unnecessary, assumptions about the underlying DMA engine (abstraction is poor, as said above but it we could follow MUSB's programming guide when it comes to programming DMA transfers). Considering all of the above, it's far better to use DMA engine and get rid of all the mess.
On Mon, Feb 04, 2013 at 05:41:53PM +0200, Felipe Balbi wrote: > Hi, > > On Fri, Feb 01, 2013 at 09:30:03PM +0000, Russell King - ARM Linux wrote: > > > > > I guess to make the MUSB side simpler we would need musb-dma-engine glue > > > > > to map dmaengine to the private MUSB API. Then we would have some > > > > > starting point to also move inventra (and anybody else) to dmaengine > > > > > API. > > > > > > > > Why? Inventra is a dedicated device's private DMA controller, why make > > > > universal DMA driver for it? > > > > > > because it doesn't make sense to support multiple DMA APIs. We can check > > > from MUSB's registers if it was configured with Inventra DMA support and > > > based on that we can register MUSB's own DMA Engine to dmaengine API. > > > > Hang on. This is one of the DMA implementations which is closely > > coupled with the USB and only the USB? If it is... > > > > I thought this had been discussed _extensively_ before. I thought the > > resolution on it was: > > 1. It would not use the DMA engine API. > > 2. It would not live in arch/arm. > > 3. It would be placed nearby the USB driver it's associated with. > > > > (1) because we don't use APIs just for the hell of it - think. Do we > > use the DMA engine API for PCI bus mastering ethernet controllers? No. > > Do we use it for PCI bus mastering SCSI controllers? No. Because the > > DMA is integral to the rest of the device. > > that's not really a fair comparison, however. MUSB is used with several > DMA engines. I only mentioned it because it _was_ brought up as an argument against using the DMA engine API in the previous discussions. I'm just reminding people what was discussed. > Considering all of the above, it's far better to use DMA engine and get > rid of all the mess. Which is what both you and I have been saying for the last 3 or so years on this subject...
Hi, On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: > > opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, > > Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. > > > Granted, CPPI 4.1 makes some assumptions about the fact that it's > > handling USB tranfers, > > What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just HW makes the asumptions > > but nevertheless, the IP can be, and in fact is, > > used with many different DMA engines and driver needs to cope with it. > > What IP, CPPI 4.1 or MUSB? MUSB > > Current DMA abstraction is quite poor, for example there's no way to > > compile support for multiple DMA engines. Code also makes certain, IMO > > unnecessary, assumptions about the underlying DMA engine (abstraction is > > poor, as said above but it we could follow MUSB's programming guide when > > it comes to programming DMA transfers). > > Don't know, I was quite content with the abstraction when writing CPPI 4.1 > driver for MUSB... look closer. The whole: if (is_cppi()) foo(); else if (is_inventra()) bar(); else if (is_omap_sdma()) baz(); is bogus. > > Considering all of the above, it's far better to use DMA engine and get > > rid of all the mess. > > In my eyes, getting rid of the mess doesn't justify breaking the rules that > Russell formulated above. MUSB is no PCI, there is no single, standard interface to the DMA engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA engine), every DMA engine comes with its own set of registers, its own programming model and so forth.
Hi, On Mon, Feb 04, 2013 at 08:54:17PM +0300, Sergei Shtylyov wrote: > > On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: > >>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, > >>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. > > >>> Granted, CPPI 4.1 makes some assumptions about the fact that it's > >>> handling USB tranfers, > > >> What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just > > > HW makes the asumptions > > Not true at all. There is a separate set of registers (at offset 0) which > copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything. CPPI 4.1 was made for USB transfers. > It's just the way the driver was written that it used both sets of registers but > this needs to be changed into more abstacted accesses to the USB-specific part, > to cope with it being different on different platfroms, like AM35x. The driver > as it was last posted, just needs rework now. are you saying you will do the work ? > >> Don't know, I was quite content with the abstraction when writing CPPI 4.1 > >> driver for MUSB... > > > look closer. The whole: > > > if (is_cppi()) > > foo(); > > else if (is_inventra()) > > bar(); > > else if (is_omap_sdma()) > > baz(); > > > is bogus. > > That part -- yes. There were attempt to get rid of this, but without changing > the DMA API. It was left halfway done after my only critical comment, IIRC. Will > we ever see the continuation of this effort? patches are welcome
On Mon, Feb 04, 2013 at 06:47:12PM +0200, Felipe Balbi wrote: > Hi, > > On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: > > In my eyes, getting rid of the mess doesn't justify breaking the rules that > > Russell formulated above. > > MUSB is no PCI, there is no single, standard interface to the DMA > engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA > engine), every DMA engine comes with its own set of registers, its own > programming model and so forth. Err, before you get too wrapped up in that argument... if you think there's anything like a common hardware interface for DMA on PCI, you'd be wrong. There isn't. What there is is a bus protocol for it. How the device decides to perform DMA is up to the device; there's no standard register definitions across all devices. Yes, some classes have a standard register set (like USB, and ATA) but others are totally device specific (eg, network chips). However, on PCI, the DMA support is always integrated with the rest of the device itself. That is not the case here. So, bringing PCI up into this discussion is totally irrelevant. As I've already explained, this was brought up in the _previous_ discussion as a reason why _not_ to use the DMA engine API for this. So, can we please leave PCI totally out of this? It didn't belong here 2-3 years ago, and it sure enough doesn't belong here now. It's nothing but pure distraction. And the more this goes on, the more I can see, yet again, the CPPI 4.1 going nowhere at all. As I can see it, there's nothing more to talk about. The issue has been extensively talked to death. What it needs now is not _more_ talk, it needs someone to actually go and _do_ something useful with it. I realise that may be difficult given the mess that TI must still be in internally since the upheaval of OMAP, but it sounds like there's a different group needing this stuff to work...
Hello. On 02/04/2013 06:41 PM, Felipe Balbi wrote: >>>>> I guess to make the MUSB side simpler we would need musb-dma-engine glue >>>>> to map dmaengine to the private MUSB API. Then we would have some >>>>> starting point to also move inventra (and anybody else) to dmaengine >>>>> API. >>>> Why? Inventra is a dedicated device's private DMA controller, why make >>>> universal DMA driver for it? >>> because it doesn't make sense to support multiple DMA APIs. We can check >>> from MUSB's registers if it was configured with Inventra DMA support and >>> based on that we can register MUSB's own DMA Engine to dmaengine API. >> Hang on. This is one of the DMA implementations which is closely >> coupled with the USB and only the USB? If it is... >> I thought this had been discussed _extensively_ before. I thought the >> resolution on it was: >> 1. It would not use the DMA engine API. >> 2. It would not live in arch/arm. >> 3. It would be placed nearby the USB driver it's associated with. >> (1) because we don't use APIs just for the hell of it - think. Do we >> use the DMA engine API for PCI bus mastering ethernet controllers? No. >> Do we use it for PCI bus mastering SCSI controllers? No. Because the >> DMA is integral to the rest of the device. > that's not really a fair comparison, however. MUSB is used with several > DMA engines. > The only DMA engine which is really coupled with MUSB is the Inventra > DMA engine which comes as an optional feature to the IP. Many users have That's not quite true. At least CPPI 3.0 is coupled with MUSB as well. The programming interface is MUSB specific, and differs from that of the EMAC driver which also uses CPPI 3.0 (however, the DMA descriptor format is the same between both, IIRC). > opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, > Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. > Granted, CPPI 4.1 makes some assumptions about the fact that it's > handling USB tranfers, What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just natural. Generic CPPI 4.1 support code (as was posted for both mach-dacinci/ or common/ placement) made no such assumptions. > but nevertheless, the IP can be, and in fact is, > used with many different DMA engines and driver needs to cope with it. What IP, CPPI 4.1 or MUSB? > Current DMA abstraction is quite poor, for example there's no way to > compile support for multiple DMA engines. Code also makes certain, IMO > unnecessary, assumptions about the underlying DMA engine (abstraction is > poor, as said above but it we could follow MUSB's programming guide when > it comes to programming DMA transfers). Don't know, I was quite content with the abstraction when writing CPPI 4.1 driver for MUSB... > Considering all of the above, it's far better to use DMA engine and get > rid of all the mess. In my eyes, getting rid of the mess doesn't justify breaking the rules that Russell formulated above. WBR, Sergei
Hello. On 02/04/2013 07:47 PM, Felipe Balbi wrote: > On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: >>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, >>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. >>> Granted, CPPI 4.1 makes some assumptions about the fact that it's >>> handling USB tranfers, >> What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just > HW makes the asumptions Not true at all. There is a separate set of registers (at offset 0) which copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything. It's just the way the driver was written that it used both sets of registers but this needs to be changed into more abstacted accesses to the USB-specific part, to cope with it being different on different platfroms, like AM35x. The driver as it was last posted, just needs rework now. >>> but nevertheless, the IP can be, and in fact is, >>> used with many different DMA engines and driver needs to cope with it. >> What IP, CPPI 4.1 or MUSB? > MUSB >>> Current DMA abstraction is quite poor, for example there's no way to >>> compile support for multiple DMA engines. Code also makes certain, IMO >>> unnecessary, assumptions about the underlying DMA engine (abstraction is >>> poor, as said above but it we could follow MUSB's programming guide when >>> it comes to programming DMA transfers). >> Don't know, I was quite content with the abstraction when writing CPPI 4.1 >> driver for MUSB... > look closer. The whole: > if (is_cppi()) > foo(); > else if (is_inventra()) > bar(); > else if (is_omap_sdma()) > baz(); > is bogus. That part -- yes. There were attempt to get rid of this, but without changing the DMA API. It was left halfway done after my only critical comment, IIRC. Will we ever see the continuation of this effort? >>> Considering all of the above, it's far better to use DMA engine and get >>> rid of all the mess. >> In my eyes, getting rid of the mess doesn't justify breaking the rules that >> Russell formulated above. > MUSB is no PCI, there is no single, standard interface to the DMA > engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA > engine), every DMA engine comes with its own set of registers, its own > programming model and so forth. Same can be said about PCI where each bus master has its own programming i/f -- so I didn't really dig this example. WBR, Sergei
Hello. On 02/04/2013 08:02 PM, Felipe Balbi wrote: >>> On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: >>>>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, >>>>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. >>>>> Granted, CPPI 4.1 makes some assumptions about the fact that it's >>>>> handling USB tranfers, >>>> What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just >>> HW makes the asumptions >> Not true at all. There is a separate set of registers (at offset 0) which >> copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything. > CPPI 4.1 was made for USB transfers. Utter nonsense, CPPI 4.1 is hardware abstract DMA engine. It's used for Ethernet transfers on out-of-tree platforms like mach-avalanche/. >> It's just the way the driver was written that it used both sets of registers but >> this needs to be changed into more abstacted accesses to the USB-specific part, >> to cope with it being different on different platfroms, like AM35x. The driver >> as it was last posted, just needs rework now. > are you saying you will do the work ? My efforts stopped to be funded by MV back in 2010. Hell, I'm not even working in MV as I used to, hanging on the verge of my current contract being terminated. >>>> Don't know, I was quite content with the abstraction when writing CPPI 4.1 >>>> driver for MUSB... >>> look closer. The whole: >>> if (is_cppi()) >>> foo(); >>> else if (is_inventra()) >>> bar(); >>> else if (is_omap_sdma()) >>> baz(); >>> is bogus. >> That part -- yes. There were attempt to get rid of this, but without changing >> the DMA API. It was left halfway done after my only critical comment, IIRC. Will >> we ever see the continuation of this effort? > patches are welcome There was a patch, it only needed comments addressed. I think the author was Heikki Krogerus. As for me, my time is very limited, so be thankful even for those scarce patches I'm sending out now -- I'm doing them in my copious free time. WBR, Sergei
On 02/04/2013 12:02 PM, Felipe Balbi wrote: > Hi, > > On Mon, Feb 04, 2013 at 08:54:17PM +0300, Sergei Shtylyov wrote: >>> On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: >>>>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, >>>>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. >> >>>>> Granted, CPPI 4.1 makes some assumptions about the fact that it's >>>>> handling USB tranfers, >> >>>> What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just >> >>> HW makes the asumptions >> >> Not true at all. There is a separate set of registers (at offset 0) which >> copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything. > > CPPI 4.1 was made for USB transfers. > I have been dealing with CPPI hardware on KeyStone platforms (CPPI 4.2). Our experiences with this DMA hardware may help with CPPI 4.1 on earlier generations. CPPI 4.2 serves as a truly common interface to a number of hardware blocks on KeyStone SoCs - including Ethernet, RapidIO, Crypto accelerators, and a bunch of other accelerator thingies. Given the commonality across subsystems, we've built a shared CPPI 4.2 DMA-Engine implementation. You can take a sneak peek at this implementation at [1]. Based on our experience with fitting multiple subsystems on top of this DMA-Engine driver, I must say that the DMA-Engine interface has proven to be a less than ideal fit for the network driver use case. The first problem is that the DMA-Engine interface expects to "push" completed traffic up into the upper layer as a part of its callback. This doesn't fit cleanly with NAPI, which expects to "pull" completed traffic from below in the NAPI poll. We've somehow kludged together a solution around this, but it isn't very elegant. The second problem is one of binding fixed DMA resources to fixed users. AFAICT, the stock DMA-Engine mechanism works best when one DMA resource is as good as any other. To get over this problem, we've added support for named channels, and drivers specifically request for a DMA resource by name. Again, this is less than ideal. We found that virtio devices offer a more elegant solution to this problem. First, the virtqueue interface is a much better fit into NAPI (callback --> napi schedule, napi poll --> get_buf), and this eliminates the need for aforementioned kludges in the code. Second, the virtio device infrastructure nicely uses the device model to solve the problem of binding DMA users to specific DMA resources. These patches haven't (yet) been posted on the MLs, but you can peek at [2]. While we are on the topic, I'd certainly appreciate feedback on the concept of using virtqueues as an interface to peripheral independent packet oriented DMA hardware. :-) Cheers -- Cyril [1] - http://arago-project.org/git/projects/?p=linux-keystone.git;a=shortlog;h=refs/heads/rebuild/23-drivers-dmaengine [2] - http://arago-project.org/git/projects/?p=linux-keystone.git;a=shortlog;h=refs/heads/rebuild/21-drivers-virtio
On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote: > Based on our experience with fitting multiple subsystems on top of this > DMA-Engine driver, I must say that the DMA-Engine interface has proven > to be a less than ideal fit for the network driver use case. > > The first problem is that the DMA-Engine interface expects to "push" > completed traffic up into the upper layer as a part of its callback. > This doesn't fit cleanly with NAPI, which expects to "pull" completed > traffic from below in the NAPI poll. We've somehow kludged together a > solution around this, but it isn't very elegant. I cannot understand the actual technical problem from the above paragraphs though. dmaengine doesn't have a concept of pushing nor polling, it basically copies streams of words from A to B, where A/B can be a device or a buffer, nothing else. The thing you're looking for sounds more like an adapter on top of dmaengine, which can surely be constructed, some drivers/dma/dmaengine-napi.c or whatever. > The second problem is one of binding fixed DMA resources to fixed users. > AFAICT, the stock DMA-Engine mechanism works best when one DMA > resource is as good as any other. The filter function picks a channel for whatever reason. That reason can be, well whatever. Some engines have a clever mechanism to select resources on the other end. Then for tying devices to channels we have the dmaengine DT branch: http://git.infradead.org/users/vkoul/slave-dma.git/shortlog/refs/heads/topic/dmaengine_dt This stuff didn't go into v3.8 but you can *sure* expect it to be in v3.9. Or are you referring to a multi-engine scenario? Say if there is engine A and B and depending on circumstances A or B may be preferred in some order (and permutations of this problem). That is currently identified as a shortcoming that we need help to address. > To get over this problem, we've added > support for named channels, and drivers specifically request for a DMA > resource by name. Again, this is less than ideal. Jon Hunter has been working on a mechanism to look up DMA channels from struct device *, dev_name() or a device tree node for example. Just like we do with clocks or regulators. Look at this patch from the dmaengine_dt branch: http://git.infradead.org/users/vkoul/slave-dma.git/commitdiff/528499a7037ebec0636d928f88cd783c618df3c5 Looks up an optionally named channel for a certain device. It currently only supports device tree, but you are free to patch in whatever mechanism you need there. Static tables in platform data works too. Just nobody did it. So go ahead and hack on dma_request_slave_channel(). (I would just branch of the DT branch.) > We found that virtio devices offer a more elegant solution to this > problem. First, the virtqueue interface is a much better fit into NAPI > (callback --> napi schedule, napi poll --> get_buf), and this eliminates > the need for aforementioned kludges in the code. Second, the virtio > device infrastructure nicely uses the device model to solve the problem > of binding DMA users to specific DMA resources. Not that I understand the polling issue, but it sounds to me like what Jon is doing is similar. Surely the way to look up resources cannot be paramount in this discussion, I think the real problem must be your specific networking usecase, so we need to drill into that. Yours, Linus Walleij
On Mon, Feb 04, 2013 at 09:29:46PM +0100, Linus Walleij wrote: > On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote: > > Based on our experience with fitting multiple subsystems on top of this > > DMA-Engine driver, I must say that the DMA-Engine interface has proven > > to be a less than ideal fit for the network driver use case. > > The first problem is that the DMA-Engine interface expects to "push" > > completed traffic up into the upper layer as a part of its callback. > > This doesn't fit cleanly with NAPI, which expects to "pull" completed > > traffic from below in the NAPI poll. We've somehow kludged together a > > solution around this, but it isn't very elegant. > I cannot understand the actual technical problem from the above > paragraphs though. dmaengine doesn't have a concept of pushing > nor polling, it basically copies streams of words from A to B, where > A/B can be a device or a buffer, nothing else. > The thing you're looking for sounds more like an adapter on top > of dmaengine, which can surely be constructed, some > drivers/dma/dmaengine-napi.c or whatever. Broadly speaking what NAPI wants is to never get any callbacks from the hardware (or DMAs). It wants to wake up periodically, take a look at what packets have been read by the hardware and process them. The goal is to have the DMAs sitting and running without disturbing the processor at all after the first packet has been handled.
On Mon, Feb 4, 2013 at 9:33 PM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Mon, Feb 04, 2013 at 09:29:46PM +0100, Linus Walleij wrote: >> On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote: > >> > Based on our experience with fitting multiple subsystems on top of this >> > DMA-Engine driver, I must say that the DMA-Engine interface has proven >> > to be a less than ideal fit for the network driver use case. > >> > The first problem is that the DMA-Engine interface expects to "push" >> > completed traffic up into the upper layer as a part of its callback. >> > This doesn't fit cleanly with NAPI, which expects to "pull" completed >> > traffic from below in the NAPI poll. We've somehow kludged together a >> > solution around this, but it isn't very elegant. > >> I cannot understand the actual technical problem from the above >> paragraphs though. dmaengine doesn't have a concept of pushing >> nor polling, it basically copies streams of words from A to B, where >> A/B can be a device or a buffer, nothing else. > >> The thing you're looking for sounds more like an adapter on top >> of dmaengine, which can surely be constructed, some >> drivers/dma/dmaengine-napi.c or whatever. > > Broadly speaking what NAPI wants is to never get any callbacks from the > hardware (or DMAs). It wants to wake up periodically, take a look at > what packets have been read by the hardware and process them. The goal > is to have the DMAs sitting and running without disturbing the processor > at all after the first packet has been handled. OK we should definately be able to encompass that in dmaengine quite easily. So I think the above concerns are moot. The callback we can set on cookies is entirely optional, and it's even implemented by each DMA engine, and some may not even support it but *require* polling, and then it won't even be implemented by the driver. Which probably stems from the original design of the dmaengine API, which was for TCP networking acceleration, mainly. Cyril, just stack up the cookies and take a sweep over them to see which ones are baked when the NAPI poll comes in -> problem solved. Yours, Linus Walleij
On Monday 04 February 2013, Linus Walleij wrote: > So I think the above concerns are moot. The callback we can > set on cookies is entirely optional, and it's even implemented by > each DMA engine, and some may not even support it but require > polling, and then it won't even be implemented by the driver. Just to ensure that everybody is talking about the same thing here: Is it just the callback that is optional, or also the interrupt coming from the hardware? With NAPI, you want to avoid both, since getting an interrupt for every packet adds noticeable overhead, but you still want to be able to tell the hardware that you are fed up with polling and would like to get an interrupt again when the next data arrives (ideally, after either a little time has passed after the next packet, or a specific number of packets has arrived). Arnd
On 02/04/2013 04:11 PM, Linus Walleij wrote: > On Mon, Feb 4, 2013 at 9:33 PM, Mark Brown > <broonie@opensource.wolfsonmicro.com> wrote: >> On Mon, Feb 04, 2013 at 09:29:46PM +0100, Linus Walleij wrote: >>> On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote: >> >>>> Based on our experience with fitting multiple subsystems on top of this >>>> DMA-Engine driver, I must say that the DMA-Engine interface has proven >>>> to be a less than ideal fit for the network driver use case. >> >>>> The first problem is that the DMA-Engine interface expects to "push" >>>> completed traffic up into the upper layer as a part of its callback. >>>> This doesn't fit cleanly with NAPI, which expects to "pull" completed >>>> traffic from below in the NAPI poll. We've somehow kludged together a >>>> solution around this, but it isn't very elegant. >> >>> I cannot understand the actual technical problem from the above >>> paragraphs though. dmaengine doesn't have a concept of pushing >>> nor polling, it basically copies streams of words from A to B, where >>> A/B can be a device or a buffer, nothing else. >> >>> The thing you're looking for sounds more like an adapter on top >>> of dmaengine, which can surely be constructed, some >>> drivers/dma/dmaengine-napi.c or whatever. >> >> Broadly speaking what NAPI wants is to never get any callbacks from the >> hardware (or DMAs). It wants to wake up periodically, take a look at >> what packets have been read by the hardware and process them. The goal >> is to have the DMAs sitting and running without disturbing the processor >> at all after the first packet has been handled. > > OK we should definately be able to encompass that in dmaengine > quite easily. > > So I think the above concerns are moot. The callback we can > set on cookies is entirely optional, and it's even implemented by > each DMA engine, and some may not even support it but *require* > polling, and then it won't even be implemented by the driver. > > Which probably stems from the original design of the dmaengine > API, which was for TCP networking acceleration, mainly. > > Cyril, just stack up the cookies and take a sweep over them to see > which ones are baked when the NAPI poll comes in -> problem > solved. > You're assuming that cookies complete in order. That is not necessarily true. Thanks -- Cyril.
On 02/04/2013 03:29 PM, Linus Walleij wrote: > On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote: > >> Based on our experience with fitting multiple subsystems on top of this >> DMA-Engine driver, I must say that the DMA-Engine interface has proven >> to be a less than ideal fit for the network driver use case. >> >> The first problem is that the DMA-Engine interface expects to "push" >> completed traffic up into the upper layer as a part of its callback. >> This doesn't fit cleanly with NAPI, which expects to "pull" completed >> traffic from below in the NAPI poll. We've somehow kludged together a >> solution around this, but it isn't very elegant. > > I cannot understand the actual technical problem from the above > paragraphs though. dmaengine doesn't have a concept of pushing > nor polling, it basically copies streams of words from A to B, where > A/B can be a device or a buffer, nothing else. > NAPI needs to switch between polled and interrupt driven modes of operation. Further, in a given poll, it needs to be able to limit the amount of traffic processed to a specified budget. > The thing you're looking for sounds more like an adapter on top > of dmaengine, which can surely be constructed, some > drivers/dma/dmaengine-napi.c or whatever. > I'm not debating the possibility of duct-taping a network driver on top of the dma-engine interface. That is very much doable, and we have done this already. Starting with a stock dma-engine driver, our first approach was to use dmaengine_pause() and dmaengine_resume() in the network driver to throttle completion callbacks per NAPI's needs. This worked, but it was ugly because the completion callback was now being used in a multi-purpose fashion - (a) as an interrupt notifier [to do a napi_schedule()], and (b) as a hand over mechanism for completed packets [within a napi_poll()]. The network driver needed to maintain a nasty little state machine for this, and this ended up being quite non-trivial after we'd fixed up most of the issues. Having learned our lessons from the first attempt, the second step was to add a separate notification callback from the dma-engine layer - a notification independent of any particular descriptor. With this callback in place, we got rid of some of the state machine crap in the network driver. The third step was to add a dmaengine_poll() call instead of constantly bouncing a channel between paused and running states. This further cleaned up some of the network driver code, but now the dma-engine driver looks like crap if it needs to support both the traditional and new (i.e. notify + poll) modes. This is where we are at today. Even with the addition of these extensions, the interaction between the network driver and the dma-engine driver is clumsy and involves multiple back and forth calls per packet. This is not elegant, and certainly not efficient. In comparison, the virtqueue interface is a natural fit with the network driver, and is free of the aforementioned problems. [...] > Surely the way to look up resources cannot be paramount in this > discussion, I think the real problem must be your specific networking > usecase, so we need to drill into that. > Agreed. The dma resource to driver binding is certainly a lesser problem than the first. There are a variety of schemes that we could cook up here (filter functions, named lookups, etc.). However, I think that these schemes offer advantages when the binding between the dma resource and the dma user is somewhat dynamic. On the other hand, when the binding between a dma resource and user is fixed by hardware and/or configuration, the driver model approach works better, IMHO. Thanks -- Cyril.
On Mon, Feb 04, 2013 at 09:47:38PM +0000, Arnd Bergmann wrote: > On Monday 04 February 2013, Linus Walleij wrote: > > So I think the above concerns are moot. The callback we can > > set on cookies is entirely optional, and it's even implemented by > > each DMA engine, and some may not even support it but require > > polling, and then it won't even be implemented by the driver. > > Just to ensure that everybody is talking about the same thing here: > Is it just the callback that is optional, or also the interrupt > coming from the hardware? If everyone implements stuff correctly, both. The callback most certainly is optional as things stand. The interrupt - that depends on the DMA engine. Some DMA engines you can't avoid it because you need to reprogram the hardware with the next+1 transfer upon completion of an existing transfer. Others may allow you to chain transfers in hardware. That's all up to how the DMA engine driver is implemented and how the hardware behaves. Now, there's another problem here: that is, people abuse the API. People don't pass DMA_CTRL_ACK | DMA_PREP_INTERRUPT into their operations by default. People like typing '0'. The intention of the "DMA_PREP_INTERRUPT" is significant here: it means "ask the hardware to send an interrupt upon completion of this transfer". Because soo many people like to type '0' instead in their DMA engine clients, it means that this flag is utterly useless today - you have to ignore it. So there's _no_ way for client drivers to actually tell the a DMA engine driver which _doesn't_ need to signal interrupts at the end of every transfer not to do so. So yes, the DMA engine API supports it. Whether the _implementations_ themselves do is very much hit and miss (and in reality is much more miss than hit.)
On Mon, Feb 04, 2013 at 04:54:45PM -0500, Cyril Chemparathy wrote: > You're assuming that cookies complete in order. That is not necessarily > true. Under what circumstances is that not true?
On Mon, Feb 4, 2013 at 10:54 PM, Cyril Chemparathy <cyril@ti.com> wrote: > On 02/04/2013 04:11 PM, Linus Walleij wrote: >> Cyril, just stack up the cookies and take a sweep over them to see >> which ones are baked when the NAPI poll comes in -> problem >> solved. > > You're assuming that cookies complete in order. That is not necessarily > true. So put them on a wait list? Surely you will have a list of pending cookies and pick from the front of the queue if there isn't a hole on queue position 0. Yours, Linus Walleij
On 02/05/2013 07:38 AM, Russell King - ARM Linux wrote: > On Mon, Feb 04, 2013 at 09:47:38PM +0000, Arnd Bergmann wrote: >> On Monday 04 February 2013, Linus Walleij wrote: >>> So I think the above concerns are moot. The callback we can >>> set on cookies is entirely optional, and it's even implemented by >>> each DMA engine, and some may not even support it but require >>> polling, and then it won't even be implemented by the driver. >> >> Just to ensure that everybody is talking about the same thing here: >> Is it just the callback that is optional, or also the interrupt >> coming from the hardware? > > If everyone implements stuff correctly, both. The callback most certainly > is optional as things stand. The interrupt - that depends on the DMA > engine. > > Some DMA engines you can't avoid it because you need to reprogram the > hardware with the next+1 transfer upon completion of an existing transfer. > Others may allow you to chain transfers in hardware. That's all up to > how the DMA engine driver is implemented and how the hardware behaves. > > Now, there's another problem here: that is, people abuse the API. People > don't pass DMA_CTRL_ACK | DMA_PREP_INTERRUPT into their operations by > default. People like typing '0'. > > The intention of the "DMA_PREP_INTERRUPT" is significant here: it means > "ask the hardware to send an interrupt upon completion of this transfer". > > Because soo many people like to type '0' instead in their DMA engine > clients, it means that this flag is utterly useless today - you have to > ignore it. So there's _no_ way for client drivers to actually tell the > a DMA engine driver which _doesn't_ need to signal interrupts at the end > of every transfer not to do so. > > So yes, the DMA engine API supports it. Whether the _implementations_ > themselves do is very much hit and miss (and in reality is much more > miss than hit.) > Don't these assume that the driver can determine the need for an interrupt upfront at prep/submit time? AFAICT, this assumption doesn't hold true with NAPI. Thanks -- Cyril.
On 02/05/2013 07:41 AM, Russell King - ARM Linux wrote: > On Mon, Feb 04, 2013 at 04:54:45PM -0500, Cyril Chemparathy wrote: >> You're assuming that cookies complete in order. That is not necessarily >> true. > > Under what circumstances is that not true? > Notably when hardware can prioritize certain classes of traffic over others, for instance with data center bridging mechanisms.
On Mon, Feb 4, 2013 at 11:30 PM, Cyril Chemparathy <cyril@ti.com> wrote: > NAPI needs to switch between polled and interrupt driven modes of operation. > Further, in a given poll, it needs to be able to limit the amount of traffic > processed to a specified budget. I don't think any of this is a problem. Polling, just scan cookies. Disable or ignore the callbacks. For IRQ mode, use the completion callback to push each cookie to NAPI, and thus let the IRQ drive the traffic. >> The thing you're looking for sounds more like an adapter on top >> of dmaengine, which can surely be constructed, some >> drivers/dma/dmaengine-napi.c or whatever. > > I'm not debating the possibility of duct-taping a network driver on top of > the dma-engine interface. That is very much doable, and we have done this > already. So it seems I have a different opinion on elegance. I think it can be a nice little adapter, and you're talking about duct-tape, like it's something ugly and crude. So let's get to the bottom of that. > Starting with a stock dma-engine driver, our first approach was to use > dmaengine_pause() and dmaengine_resume() in the network driver to throttle > completion callbacks per NAPI's needs. Why? Do you really need to limit this in the middle of transfers? I'm half-guessing that one transfer is typically something like one packet. Pausing and starting would be something you would use in case you had a circular buffer with an eternal ongoing transfer, and you wanted to slow that down. > Having learned our lessons from the first attempt, the second step was to > add a separate notification callback from the dma-engine layer - a > notification independent of any particular descriptor. With this callback > in place, we got rid of some of the state machine crap in the network > driver. > > The third step was to add a dmaengine_poll() call instead of constantly > bouncing a channel between paused and running states. This further cleaned > up some of the network driver code, but now the dma-engine driver looks like > crap if it needs to support both the traditional and new (i.e. notify + > poll) modes. This is where we are at today. I don't see why you have to modify dmaengine to do poll. It should be trivial as discussed to just keep track of the cookies, scan over them and poll out the completed work. Then to mitigate the load I guess you just do not issue too many dma transfers? Can you not regulate the workload by adjusting the number of transfer cookies you issue in parallel, and if they're not issued in parallel but adjacent, can you not just issue them so often? Or are you polling out half transfers or something, so that the granularity of one transfer/cookie would be enough? Maybe I'm not getting the picture here? Can you describe how the network stream is chopped into transfers in this hardware, I think that could ease our understanding? Especially I guess we need to know if the hardware is providing useful chunks like one packet or if it's just one eternal stream of bits that you then have to demux and put into skbufs or something. > Even with the addition of these extensions, the interaction between the > network driver and the dma-engine driver is clumsy and involves multiple > back and forth calls per packet. This is not elegant, and certainly not > efficient. In comparison, the virtqueue interface is a natural fit with the > network driver, and is free of the aforementioned problems. Yes the described approach and hacking around in dmaengine to do the polling seems ugly. But just a queue of cookies does not seem so ugly, rather the opposite. > [Russell writes] >> So yes, the DMA engine API supports it. Whether the _implementations_ >> themselves do is very much hit and miss (and in reality is much more >> miss than hit.) > > Don't these assume that the driver can determine the need for an interrupt > upfront at prep/submit time? AFAICT, this assumption doesn't hold true > with NAPI. Yes it does. You can however stop an ongoing transfer (by referring to the cookie), pick out the bytes transferred so far and trigger a new transfer using/not using an IRQ if you want. This is an abstracted way of doing the same brutal buffer slaying that I hear NAPI is doing to some network drivers. For example see the RX path of this driver: drivers/tty/serial/amba-pl011.c There is DMA for it, but we may stop the DMA transfer on an IRQ, take the partial buffer out and feed it to the TTY. Could just as well be a network packet. Sometimes it is, if there is a modem on the other end. RX DMA is triggered in pl011_dma_rx_trigger_dma(), then either pl011_dma_rx_callback() gets called if the DMA transfer completes, or we get an IRQ (like a timeout) and endup in pl011_dma_rx_irq(), where the transfer is stopped, buffer emtied and then we can decide what to do next. This could just as well have been some API calling in and saying "give me your buffer NOW". I think we need to look at an NAPI driver that does this trick so we understand what you want from the API. Yours, Linus Walleij
On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote: > For IRQ mode, use the completion callback to push each cookie > to NAPI, and thus let the IRQ drive the traffic. The whole purpose of NAPI is to avoid taking interrupts for completion of transfers. Anything that generates interrupts when NAPI is in polling mode is defeating the point.
On Tue, Feb 05, 2013 at 04:47:05PM +0000, Mark Brown wrote: > On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote: > > > For IRQ mode, use the completion callback to push each cookie > > to NAPI, and thus let the IRQ drive the traffic. > > The whole purpose of NAPI is to avoid taking interrupts for completion > of transfers. Anything that generates interrupts when NAPI is in > polling mode is defeating the point. Yes, but you're missing one very important point. If your DMA engine is decoupled from the network device, and the DMA engine relies upon interrupts to queue further transfers to move data into RAM, then you have two options: 1. Receive these interrupts so you can update the DMA engine for further data transfer. 2. Don't receive these interrupts, and cause the network device to error out with a FIFO overrun because its DMA requests have not been serviced. (which may raise an interrupt.)
On Tue, Feb 05, 2013 at 04:30:45PM +0100, Linus Walleij wrote: > On Mon, Feb 4, 2013 at 10:54 PM, Cyril Chemparathy <cyril@ti.com> wrote: > > On 02/04/2013 04:11 PM, Linus Walleij wrote: > > >> Cyril, just stack up the cookies and take a sweep over them to see > >> which ones are baked when the NAPI poll comes in -> problem > >> solved. > > > > You're assuming that cookies complete in order. That is not necessarily > > true. > > So put them on a wait list? Surely you will have a list of pending > cookies and pick from the front of the queue if there isn't a hole on > queue position 0. Not quite. The key is the cookie system DMA engine employs to indicate when a cookie is complete. Cookies between the "issued sequence" and "completed sequence" are defined to be in progress, everything else is defined to be completed. This means that if "completed sequence" is 1, and "issued sequence" is 5, then cookies with values 2, 3, 4, 5 are in progress. You can't mark sequence 4 as being complete until 2 and 3 have completed. If we need out-of-order completion, then that's a problem for the DMA engine API, because you'd need to radically change the way "completion" is marked.
On Tue, Feb 05, 2013 at 05:06:28PM +0000, Russell King - ARM Linux wrote: > On Tue, Feb 05, 2013 at 04:47:05PM +0000, Mark Brown wrote: > > On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote: > > > For IRQ mode, use the completion callback to push each cookie > > > to NAPI, and thus let the IRQ drive the traffic. > > The whole purpose of NAPI is to avoid taking interrupts for completion > > of transfers. Anything that generates interrupts when NAPI is in > > polling mode is defeating the point. > Yes, but you're missing one very important point. If your DMA engine > is decoupled from the network device, and the DMA engine relies upon > interrupts to queue further transfers to move data into RAM, then you > have two options: > 1. Receive these interrupts so you can update the DMA engine for > further data transfer. > 2. Don't receive these interrupts, and cause the network device to > error out with a FIFO overrun because its DMA requests have not > been serviced. (which may raise an interrupt.) There's still the third option of just implementing a non-NAPI driver, though I guess we've now built so much infrastructure on top of NAPI that it's still useful to pretend we can disable interrupts.
* Felipe Balbi <balbi@ti.com> [130204 07:46]: > > Current DMA abstraction is quite poor, for example there's no way to > compile support for multiple DMA engines. Code also makes certain, IMO > unnecessary, assumptions about the underlying DMA engine (abstraction is > poor, as said above but it we could follow MUSB's programming guide when > it comes to programming DMA transfers). > > Considering all of the above, it's far better to use DMA engine and get > rid of all the mess. How about just disable MUSB DMA support if ARCH_MULTIPLATFORM for now? That way MUSB can be fixed up first to support ARCH_MULTIPLATFORM using PIO while sorting out the DMA related issues. Regards, Tony
On Tue, Feb 5, 2013 at 5:47 PM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote: > >> For IRQ mode, use the completion callback to push each cookie >> to NAPI, and thus let the IRQ drive the traffic. > > The whole purpose of NAPI is to avoid taking interrupts for completion > of transfers. Anything that generates interrupts when NAPI is in > polling mode is defeating the point. So what I was trying to get across is that when you're in polling mode you do not set DMA_PREP_INTERRUPT on your transfers, just throw the obtained struct dma_async_tx_descriptor on some list and then when polling use dma_async_is_tx_complete() on the channel and the cookie inside the descriptor. I was trying to describe that you can move from IRQ mode to polling mode and back again by selectively choosing to set/not set the DMA_PREP_INTERRUPT flag. If polling is all you want you never set it. Then there is the fact that the transfer needs to have been flagged complete and it is indeed something that needs to be set in some bytes somewhere. By something. But it doesn't have to be an interrupt from the DMA controller. In such cases we use dma_async_is_tx_complete() with channel and cookies as parameter. This will call down into the driver chan->device->device_tx_status() and there we can actually poll the hardware to see if the transfer happens to be complete, and if it is flag it complete. Which is likely what we want. No interrupts, only function calls as far as I can see. (I bet Russell will poke a hole in my reasoning, but it's worth a try.) Yours, Linus Walleij
On Tue, Feb 5, 2013 at 6:14 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Feb 05, 2013 at 04:30:45PM +0100, Linus Walleij wrote: >> So put them on a wait list? Surely you will have a list of pending >> cookies and pick from the front of the queue if there isn't a hole on >> queue position 0. > > Not quite. The key is the cookie system DMA engine employs to indicate > when a cookie is complete. > > Cookies between the "issued sequence" and "completed sequence" are defined > to be in progress, everything else is defined to be completed. > > This means that if "completed sequence" is 1, and "issued sequence" is 5, > then cookies with values 2, 3, 4, 5 are in progress. You can't mark > sequence 4 as being complete until 2 and 3 have completed. Yes that is true. DMA transfers on a certain channel are defined as progressing linearly per-cookie. I wonder if that is a problem in this case though (actually it seems the reverse, this helps in Cyril's case.) > If we need out-of-order completion, then that's a problem for the DMA > engine API, because you'd need to radically change the way "completion" > is marked. True. I wonder if this usecase is ever going to be applicable however. It could maybe be useful in some instances of memcpy() I could dream up, whereas for device transfers it seems unlikely to me. Yours, Linus Walleij
On 02/05/2013 01:29 PM, Linus Walleij wrote: > On Tue, Feb 5, 2013 at 5:47 PM, Mark Brown > <broonie@opensource.wolfsonmicro.com> wrote: >> On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote: >> >>> For IRQ mode, use the completion callback to push each cookie >>> to NAPI, and thus let the IRQ drive the traffic. >> >> The whole purpose of NAPI is to avoid taking interrupts for completion >> of transfers. Anything that generates interrupts when NAPI is in >> polling mode is defeating the point. > > So what I was trying to get across is that when you're in polling > mode you do not set DMA_PREP_INTERRUPT on your transfers, > just throw the obtained struct dma_async_tx_descriptor on some > list and then when polling use dma_async_is_tx_complete() > on the channel and the cookie inside the descriptor. > > I was trying to describe that you can move from > IRQ mode to polling mode and back again by selectively > choosing to set/not set the DMA_PREP_INTERRUPT flag. > This does not work. At prep/submit time, the network driver does not know if a particular packet buffer needs an interrupt or not. It queues up a whole bunch of receive buffers upfront. These buffers simply sit on the hardware queue/ring until the NIC receives traffic. The driver throttles the receive processing rate by dynamically switching between interrupt and poll behaviors on the completion side, not on the submission side. > If polling is all you want you never set it. > Another point here is that it is not simply a polling vs. interrupt problem. The system needs to dynamically switch between the two behaviors depending on offered load conditions. This dynamic switching is key to balancing latency (under low load) and throughput (at high rates). It cannot be one or the other, it must be both. Once again, I'm fairly sure that suitably reworking or extending the dma-engine interfaces will allow network DMAs to fit in nicely. However, I'd also appreciate inputs on the alternative approach of using virtio devices as an interface to packet oriented DMA hardware. From my perspective this offers the following advantages, some of which I've already mentioned in earlier postings: 1. The virtqueue interface is nice and clean, it fits very well with networking concepts such as NAPI. In comparison, the dma-engine API will need extensions and/or rework to fit network DMAs. 2. Advantages from leveraging the virtio infrastructure. For example, if a DMA pipe to a remote processor is exposed as a virtio device, something like rpmsg could very naturally fit on top of this without any other added glue. 3. Advantages from leveraging the driver model for binding dma clients to dma hardware, instead of resorting to name lookups and such. Thanks -- Cyril.
On Tuesday 05 February 2013, Tony Lindgren wrote: > * Felipe Balbi <balbi@ti.com> [130204 07:46]: > > > > Current DMA abstraction is quite poor, for example there's no way to > > compile support for multiple DMA engines. Code also makes certain, IMO > > unnecessary, assumptions about the underlying DMA engine (abstraction is > > poor, as said above but it we could follow MUSB's programming guide when > > it comes to programming DMA transfers). > > > > Considering all of the above, it's far better to use DMA engine and get > > rid of all the mess. > > How about just disable MUSB DMA support if ARCH_MULTIPLATFORM for now? > That way MUSB can be fixed up first to support ARCH_MULTIPLATFORM > using PIO while sorting out the DMA related issues. Sounds ok to me. Someone still needs to do the work to make the non-DMA variants of MUSB coexist in one kernel, but as we discussed erlier, that should be much simpler. Arnd
On Tue, Feb 05, 2013 at 10:26:30PM +0000, Arnd Bergmann wrote: > On Tuesday 05 February 2013, Tony Lindgren wrote: > > * Felipe Balbi <balbi@ti.com> [130204 07:46]: > > > > > > Current DMA abstraction is quite poor, for example there's no way to > > > compile support for multiple DMA engines. Code also makes certain, IMO > > > unnecessary, assumptions about the underlying DMA engine (abstraction is > > > poor, as said above but it we could follow MUSB's programming guide when > > > it comes to programming DMA transfers). > > > > > > Considering all of the above, it's far better to use DMA engine and get > > > rid of all the mess. > > > > How about just disable MUSB DMA support if ARCH_MULTIPLATFORM for now? > > That way MUSB can be fixed up first to support ARCH_MULTIPLATFORM > > using PIO while sorting out the DMA related issues. > > Sounds ok to me. Someone still needs to do the work to make the non-DMA > variants of MUSB coexist in one kernel, but as we discussed erlier, that > should be much simpler. Yeah, I'm cooking some patches to at least make it buildable. Dropping unnecessary dependencies and marking da8xx and davinci as depending on BROKEN seems to be the easiest way; those two glues hasn't seen a real patch since 2010 or so.
Hi Matt, This version introduces build/bisect issues. On 2/1/2013 11:52 PM, Matt Porter wrote: > Move mach-davinci/dma.c to common/edma.c so it can be used > by OMAP (specifically AM33xx) as well. > > Signed-off-by: Matt Porter <mporter@ti.com> > Acked-by: Sekhar Nori <nsekhar@ti.com> > diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/common/edma.c > @@ -25,7 +25,7 @@ > #include <linux/io.h> > #include <linux/slab.h> > > -#include <mach/edma.h> > +#include <linux/platform_data/edma.h> > /*-----------------------------------------------------------------------*/ > +static int edma_of_read_u32_to_s8_array(const struct device_node *np, > + const char *propname, s8 *out_values, > + size_t sz) > +{ > + int ret; > + > + ret = of_property_read_u8_array(np, propname, out_values, sz); This needs <linux/of.h> to be included in this file. > +static int edma_xbar_event_map(struct device *dev, > + struct device_node *node, > + struct edma_soc_info *pdata, int len) > +{ > + int ret = 0; > + int i; > + struct resource res; > + void *xbar; > + const s16 (*xbar_chans)[2]; > + u32 shift, offset, mux; > + > + xbar_chans = devm_kzalloc(dev, > + len/sizeof(s16) + 2*sizeof(s16), > + GFP_KERNEL); > + if (!xbar_chans) > + return -ENOMEM; > + > + ret = of_address_to_resource(node, 1, &res); of_address_to_resource() needs <linux/of_address.h> > + if (IS_ERR_VALUE(ret)) This needs <linux/err.h> > + return -EIO; > + > + xbar = devm_ioremap(dev, res.start, resource_size(&res)); > + if (!xbar) > + return -ENOMEM; > + > + ret = edma_of_read_u32_to_s16_array(node, > + "ti,edma-xbar-event-map", > + (s16 *)xbar_chans, > + len/sizeof(u32)); > + if (IS_ERR_VALUE(ret)) > + return -EIO; > + > + for (i = 0; xbar_chans[i][0] != -1; i++) { > + shift = (xbar_chans[i][1] % 4) * 8; > + offset = xbar_chans[i][1] >> 2; > + offset <<= 2; > + mux = readl((void *)((u32)xbar + offset)); > + mux &= ~(0xff << shift); > + mux |= xbar_chans[i][0] << shift; > + writel(mux, (void *)((u32)xbar + offset)); > + } > + > + pdata->xbar_chans = xbar_chans; There is no member xbar_chans ATM. It seems to be introduced in 03/10. > + > +static struct of_dma_filter_info edma_filter_info = { of_dma_filter_info needs <linux/of_dma.h>. > + .filter_fn = edma_filter_fn, edma_filter_fn needs <linux/edma.h> BTW, I am not sure if you really intended to introduce EDMA DT support in this patch. I thought 1/10 was supposed to just move EDMA private API to a common place. If you really want to introduce DT support as well, that should be noted in the description. I think it is better done in a later patch. > diff --git a/sound/soc/davinci/davinci-sffsdr.c b/sound/soc/davinci/davinci-sffsdr.c > @@ -17,6 +17,7 @@ > #include <linux/timer.h> > #include <linux/interrupt.h> > #include <linux/platform_device.h> > +#include <linux/platform_data/edma.h> > #include <linux/gpio.h> > #include <sound/core.h> > #include <sound/pcm.h> > @@ -28,12 +29,14 @@ > #include <asm/plat-sffsdr/sffsdr-fpga.h> > #endif > > -#include <mach/edma.h> > > #include "../codecs/pcm3008.h" > #include "davinci-pcm.h" > #include "davinci-i2s.h" > > +#define DAVINCI_DMA_MCBSP_TX 2 > +#define DAVINCI_DMA_MCBSP_RX 3 > + > /* > * CLKX and CLKR are the inputs for the Sample Rate Generator. > * FSX and FSR are outputs, driven by the sample Rate Generator. > @@ -124,7 +127,7 @@ static struct resource sffsdr_snd_resources[] = { > > static struct evm_snd_platform_data sffsdr_snd_data = { > .tx_dma_ch = DAVINCI_DMA_MCBSP_TX, > - .rx_dma_ch = DAVINCI_DMA_MCBSP_RX, > + .rx_dma_ch = DAVINCI_DMA_MCBAP_RX, Typo here. Should be DAVINCI_DMA_MCBSP_RX. Unfortunately davinci-sffsdr.c seems to be broken already. Thanks, Sekhar
On Sat, Feb 09, 2013 at 09:35:53PM +0530, Sekhar Nori wrote: > On 2/1/2013 11:52 PM, Matt Porter wrote: > > + ret = of_address_to_resource(node, 1, &res); > > of_address_to_resource() needs <linux/of_address.h> > > > + if (IS_ERR_VALUE(ret)) > > This needs <linux/err.h> More importantly, is this the correct way to check for errors from of_address_to_resource() ? Grepping the source shows that either less than 0 or non-zero are the commonly used conditions here.
On Sat, Feb 09, 2013 at 09:35:53PM +0530, Sekhar Nori wrote: > Hi Matt, > > This version introduces build/bisect issues. Ok, see later comment which addresses this... > On 2/1/2013 11:52 PM, Matt Porter wrote: > > Move mach-davinci/dma.c to common/edma.c so it can be used > > by OMAP (specifically AM33xx) as well. > > > > Signed-off-by: Matt Porter <mporter@ti.com> > > Acked-by: Sekhar Nori <nsekhar@ti.com> > > > diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/common/edma.c > > > @@ -25,7 +25,7 @@ > > #include <linux/io.h> > > #include <linux/slab.h> > > > > -#include <mach/edma.h> > > +#include <linux/platform_data/edma.h> > > > /*-----------------------------------------------------------------------*/ > > +static int edma_of_read_u32_to_s8_array(const struct device_node *np, > > + const char *propname, s8 *out_values, > > + size_t sz) > > +{ > > + int ret; > > + > > + ret = of_property_read_u8_array(np, propname, out_values, sz); > > This needs <linux/of.h> to be included in this file. > > > +static int edma_xbar_event_map(struct device *dev, > > + struct device_node *node, > > + struct edma_soc_info *pdata, int len) > > +{ > > + int ret = 0; > > + int i; > > + struct resource res; > > + void *xbar; > > + const s16 (*xbar_chans)[2]; > > + u32 shift, offset, mux; > > + > > + xbar_chans = devm_kzalloc(dev, > > + len/sizeof(s16) + 2*sizeof(s16), > > + GFP_KERNEL); > > + if (!xbar_chans) > > + return -ENOMEM; > > + > > + ret = of_address_to_resource(node, 1, &res); > > of_address_to_resource() needs <linux/of_address.h> > > > + if (IS_ERR_VALUE(ret)) > > This needs <linux/err.h> > > > + return -EIO; > > + > > + xbar = devm_ioremap(dev, res.start, resource_size(&res)); > > + if (!xbar) > > + return -ENOMEM; > > + > > + ret = edma_of_read_u32_to_s16_array(node, > > + "ti,edma-xbar-event-map", > > + (s16 *)xbar_chans, > > + len/sizeof(u32)); > > + if (IS_ERR_VALUE(ret)) > > + return -EIO; > > + > > + for (i = 0; xbar_chans[i][0] != -1; i++) { > > + shift = (xbar_chans[i][1] % 4) * 8; > > + offset = xbar_chans[i][1] >> 2; > > + offset <<= 2; > > + mux = readl((void *)((u32)xbar + offset)); > > + mux &= ~(0xff << shift); > > + mux |= xbar_chans[i][0] << shift; > > + writel(mux, (void *)((u32)xbar + offset)); > > + } > > + > > + pdata->xbar_chans = xbar_chans; > > There is no member xbar_chans ATM. It seems to be introduced in 03/10. > > > + > > +static struct of_dma_filter_info edma_filter_info = { > > of_dma_filter_info needs <linux/of_dma.h>. > > > + .filter_fn = edma_filter_fn, > > edma_filter_fn needs <linux/edma.h> > > BTW, I am not sure if you really intended to introduce EDMA DT support > in this patch. I thought 1/10 was supposed to just move EDMA private API > to a common place. If you really want to introduce DT support as well, > that should be noted in the description. I think it is better done in a > later patch. This was a complete fubared rebase for this patch. As you noticed, I got hunks from 3/10 in there and the bisect test didn't run so it was missed. I've got this fixed up for v8 which address all these comments that stem from that issue. > > diff --git a/sound/soc/davinci/davinci-sffsdr.c b/sound/soc/davinci/davinci-sffsdr.c > > > @@ -17,6 +17,7 @@ > > #include <linux/timer.h> > > #include <linux/interrupt.h> > > #include <linux/platform_device.h> > > +#include <linux/platform_data/edma.h> > > #include <linux/gpio.h> > > #include <sound/core.h> > > #include <sound/pcm.h> > > @@ -28,12 +29,14 @@ > > #include <asm/plat-sffsdr/sffsdr-fpga.h> > > #endif > > > > -#include <mach/edma.h> > > > > #include "../codecs/pcm3008.h" > > #include "davinci-pcm.h" > > #include "davinci-i2s.h" > > > > +#define DAVINCI_DMA_MCBSP_TX 2 > > +#define DAVINCI_DMA_MCBSP_RX 3 > > + > > /* > > * CLKX and CLKR are the inputs for the Sample Rate Generator. > > * FSX and FSR are outputs, driven by the sample Rate Generator. > > @@ -124,7 +127,7 @@ static struct resource sffsdr_snd_resources[] = { > > > > static struct evm_snd_platform_data sffsdr_snd_data = { > > .tx_dma_ch = DAVINCI_DMA_MCBSP_TX, > > - .rx_dma_ch = DAVINCI_DMA_MCBSP_RX, > > > + .rx_dma_ch = DAVINCI_DMA_MCBAP_RX, > > Typo here. Should be DAVINCI_DMA_MCBSP_RX. Unfortunately > davinci-sffsdr.c seems to be broken already. Ok, fixed..definitely shouldn't break it more. -Matt
On Sat, Feb 09, 2013 at 08:08:50PM +0000, Russell King wrote: > On Sat, Feb 09, 2013 at 09:35:53PM +0530, Sekhar Nori wrote: > > On 2/1/2013 11:52 PM, Matt Porter wrote: > > > + ret = of_address_to_resource(node, 1, &res); > > > > of_address_to_resource() needs <linux/of_address.h> > > > > > + if (IS_ERR_VALUE(ret)) > > > > This needs <linux/err.h> > > More importantly, is this the correct way to check for errors from > of_address_to_resource() ? Grepping the source shows that either less > than 0 or non-zero are the commonly used conditions here. Yes, it's not correct. Fixed to check for non-zero/zero in the two cases where of_address_to_resource() is used. -Matt
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 67874b8..7637d31 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -932,6 +932,7 @@ config ARCH_DAVINCI select GENERIC_IRQ_CHIP select HAVE_IDE select NEED_MACH_GPIO_H + select TI_PRIV_EDMA select USE_OF select ZONE_DMA help diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig index 45ceeb0..9e32d0d 100644 --- a/arch/arm/common/Kconfig +++ b/arch/arm/common/Kconfig @@ -40,3 +40,6 @@ config SHARP_PARAM config SHARP_SCOOP bool + +config TI_PRIV_EDMA + bool diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile index e8a4e58..d09a39b 100644 --- a/arch/arm/common/Makefile +++ b/arch/arm/common/Makefile @@ -13,3 +13,4 @@ obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o obj-$(CONFIG_SHARP_SCOOP) += scoop.o obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o obj-$(CONFIG_ARM_TIMER_SP804) += timer-sp.o +obj-$(CONFIG_TI_PRIV_EDMA) += edma.o diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/common/edma.c similarity index 90% rename from arch/arm/mach-davinci/dma.c rename to arch/arm/common/edma.c index a685e97..aa4a49a 100644 --- a/arch/arm/mach-davinci/dma.c +++ b/arch/arm/common/edma.c @@ -25,7 +25,7 @@ #include <linux/io.h> #include <linux/slab.h> -#include <mach/edma.h> +#include <linux/platform_data/edma.h> /* Offsets matching "struct edmacc_param" */ #define PARM_OPT 0x00 @@ -1386,8 +1386,213 @@ void edma_clear_event(unsigned channel) EXPORT_SYMBOL(edma_clear_event); /*-----------------------------------------------------------------------*/ +static int edma_of_read_u32_to_s8_array(const struct device_node *np, + const char *propname, s8 *out_values, + size_t sz) +{ + int ret; + + ret = of_property_read_u8_array(np, propname, out_values, sz); + if (ret) + return ret; + + /* Terminate it */ + *out_values++ = -1; + *out_values++ = -1; + + return 0; +} + +static int edma_of_read_u32_to_s16_array(const struct device_node *np, + const char *propname, s16 *out_values, + size_t sz) +{ + int ret; + + ret = of_property_read_u16_array(np, propname, out_values, sz); + if (ret) + return ret; + + /* Terminate it */ + *out_values++ = -1; + *out_values++ = -1; + + return 0; +} + +static int edma_xbar_event_map(struct device *dev, + struct device_node *node, + struct edma_soc_info *pdata, int len) +{ + int ret = 0; + int i; + struct resource res; + void *xbar; + const s16 (*xbar_chans)[2]; + u32 shift, offset, mux; + + xbar_chans = devm_kzalloc(dev, + len/sizeof(s16) + 2*sizeof(s16), + GFP_KERNEL); + if (!xbar_chans) + return -ENOMEM; + + ret = of_address_to_resource(node, 1, &res); + if (IS_ERR_VALUE(ret)) + return -EIO; + + xbar = devm_ioremap(dev, res.start, resource_size(&res)); + if (!xbar) + return -ENOMEM; + + ret = edma_of_read_u32_to_s16_array(node, + "ti,edma-xbar-event-map", + (s16 *)xbar_chans, + len/sizeof(u32)); + if (IS_ERR_VALUE(ret)) + return -EIO; + + for (i = 0; xbar_chans[i][0] != -1; i++) { + shift = (xbar_chans[i][1] % 4) * 8; + offset = xbar_chans[i][1] >> 2; + offset <<= 2; + mux = readl((void *)((u32)xbar + offset)); + mux &= ~(0xff << shift); + mux |= xbar_chans[i][0] << shift; + writel(mux, (void *)((u32)xbar + offset)); + } + + pdata->xbar_chans = xbar_chans; + + return 0; +} + +static int edma_of_parse_dt(struct device *dev, + struct device_node *node, + struct edma_soc_info *pdata) +{ + int ret = 0; + u32 value; + struct property *prop; + size_t sz; + struct edma_rsv_info *rsv_info; + const s16 (*rsv_chans)[2], (*rsv_slots)[2]; + const s8 (*queue_tc_map)[2], (*queue_priority_map)[2]; + + memset(pdata, 0, sizeof(struct edma_soc_info)); + + ret = of_property_read_u32(node, "dma-channels", &value); + if (ret < 0) + return ret; + pdata->n_channel = value; + + ret = of_property_read_u32(node, "ti,edma-regions", &value); + if (ret < 0) + return ret; + pdata->n_region = value; + + ret = of_property_read_u32(node, "ti,edma-slots", &value); + if (ret < 0) + return ret; + pdata->n_slot = value; + + pdata->n_cc = 1; + pdata->n_tc = 3; + + rsv_info = + devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL); + if (!rsv_info) + return -ENOMEM; + pdata->rsv = rsv_info; + + /* Build the reserved channel/slots arrays */ + prop = of_find_property(node, "ti,edma-reserved-channels", &sz); + if (prop) { + rsv_chans = devm_kzalloc(dev, + sz/sizeof(s16) + 2*sizeof(s16), + GFP_KERNEL); + if (!rsv_chans) + return -ENOMEM; + pdata->rsv->rsv_chans = rsv_chans; + + ret = edma_of_read_u32_to_s16_array(node, + "ti,edma-reserved-channels", + (s16 *)rsv_chans, + sz/sizeof(u32)); + if (ret < 0) + return ret; + } + + prop = of_find_property(node, "ti,edma-reserved-slots", &sz); + if (prop) { + rsv_slots = devm_kzalloc(dev, + sz/sizeof(s16) + 2*sizeof(s16), + GFP_KERNEL); + if (!rsv_slots) + return -ENOMEM; + pdata->rsv->rsv_slots = rsv_slots; + + ret = edma_of_read_u32_to_s16_array(node, + "ti,edma-reserved-slots", + (s16 *)rsv_slots, + sz/sizeof(u32)); + if (ret < 0) + return ret; + } + + prop = of_find_property(node, "ti,edma-queue-tc-map", &sz); + if (!prop) + return -EINVAL; + + queue_tc_map = devm_kzalloc(dev, + sz/sizeof(s8) + 2*sizeof(s8), + GFP_KERNEL); + if (!queue_tc_map) + return -ENOMEM; + pdata->queue_tc_mapping = queue_tc_map; + + ret = edma_of_read_u32_to_s8_array(node, + "ti,edma-queue-tc-map", + (s8 *)queue_tc_map, + sz/sizeof(u32)); + if (ret < 0) + return ret; + + prop = of_find_property(node, "ti,edma-queue-priority-map", &sz); + if (!prop) + return -EINVAL; + + queue_priority_map = devm_kzalloc(dev, + sz/sizeof(s8) + 2*sizeof(s8), + GFP_KERNEL); + if (!queue_priority_map) + return -ENOMEM; + pdata->queue_priority_mapping = queue_priority_map; + + ret = edma_of_read_u32_to_s8_array(node, + "ti,edma-queue-tc-map", + (s8 *)queue_priority_map, + sz/sizeof(u32)); + if (ret < 0) + return ret; + + ret = of_property_read_u32(node, "ti,edma-default-queue", &value); + if (ret < 0) + return ret; + pdata->default_queue = value; + + prop = of_find_property(node, "ti,edma-xbar-event-map", &sz); + if (prop) + ret = edma_xbar_event_map(dev, node, pdata, sz); + + return ret; +} + +static struct of_dma_filter_info edma_filter_info = { + .filter_fn = edma_filter_fn, +}; -static int __init edma_probe(struct platform_device *pdev) +static int edma_probe(struct platform_device *pdev) { struct edma_soc_info **info = pdev->dev.platform_data; const s8 (*queue_priority_mapping)[2]; diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile index fb5c1aa..493a36b 100644 --- a/arch/arm/mach-davinci/Makefile +++ b/arch/arm/mach-davinci/Makefile @@ -5,7 +5,7 @@ # Common objects obj-y := time.o clock.o serial.o psc.o \ - dma.o usb.o common.o sram.o aemif.o + usb.o common.o sram.o aemif.o obj-$(CONFIG_DAVINCI_MUX) += mux.o diff --git a/arch/arm/mach-davinci/board-tnetv107x-evm.c b/arch/arm/mach-davinci/board-tnetv107x-evm.c index be30997..86f55ba 100644 --- a/arch/arm/mach-davinci/board-tnetv107x-evm.c +++ b/arch/arm/mach-davinci/board-tnetv107x-evm.c @@ -26,12 +26,12 @@ #include <linux/input.h> #include <linux/input/matrix_keypad.h> #include <linux/spi/spi.h> +#include <linux/platform_data/edma.h> #include <asm/mach/arch.h> #include <asm/mach-types.h> #include <mach/irqs.h> -#include <mach/edma.h> #include <mach/mux.h> #include <mach/cp_intc.h> #include <mach/tnetv107x.h> diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h index 12d544b..d26a6bc 100644 --- a/arch/arm/mach-davinci/davinci.h +++ b/arch/arm/mach-davinci/davinci.h @@ -23,9 +23,9 @@ #include <linux/platform_device.h> #include <linux/spi/spi.h> #include <linux/platform_data/davinci_asp.h> +#include <linux/platform_data/edma.h> #include <linux/platform_data/keyscan-davinci.h> #include <mach/hardware.h> -#include <mach/edma.h> #include <media/davinci/vpfe_capture.h> #include <media/davinci/vpif_types.h> diff --git a/arch/arm/mach-davinci/devices-tnetv107x.c b/arch/arm/mach-davinci/devices-tnetv107x.c index 773ab07..ba37760 100644 --- a/arch/arm/mach-davinci/devices-tnetv107x.c +++ b/arch/arm/mach-davinci/devices-tnetv107x.c @@ -18,10 +18,10 @@ #include <linux/dma-mapping.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/platform_data/edma.h> #include <mach/common.h> #include <mach/irqs.h> -#include <mach/edma.h> #include <mach/tnetv107x.h> #include "clock.h" diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c index 4c48a36..ca0c7b3 100644 --- a/arch/arm/mach-davinci/devices.c +++ b/arch/arm/mach-davinci/devices.c @@ -19,9 +19,10 @@ #include <mach/irqs.h> #include <mach/cputype.h> #include <mach/mux.h> -#include <mach/edma.h> #include <linux/platform_data/mmc-davinci.h> #include <mach/time.h> +#include <linux/platform_data/edma.h> + #include "davinci.h" #include "clock.h" @@ -34,6 +35,9 @@ #define DM365_MMCSD0_BASE 0x01D11000 #define DM365_MMCSD1_BASE 0x01D00000 +#define DAVINCI_DMA_MMCRXEVT 26 +#define DAVINCI_DMA_MMCTXEVT 27 + void __iomem *davinci_sysmod_base; void davinci_map_sysmod(void) diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c index b49c3b7..53998d8 100644 --- a/arch/arm/mach-davinci/dm355.c +++ b/arch/arm/mach-davinci/dm355.c @@ -19,7 +19,6 @@ #include <asm/mach/map.h> #include <mach/cputype.h> -#include <mach/edma.h> #include <mach/psc.h> #include <mach/mux.h> #include <mach/irqs.h> @@ -28,6 +27,7 @@ #include <mach/common.h> #include <linux/platform_data/spi-davinci.h> #include <mach/gpio-davinci.h> +#include <linux/platform_data/edma.h> #include "davinci.h" #include "clock.h" diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c index 6c39805..9b41d33 100644 --- a/arch/arm/mach-davinci/dm365.c +++ b/arch/arm/mach-davinci/dm365.c @@ -18,11 +18,11 @@ #include <linux/platform_device.h> #include <linux/dma-mapping.h> #include <linux/spi/spi.h> +#include <linux/platform_data/edma.h> #include <asm/mach/map.h> #include <mach/cputype.h> -#include <mach/edma.h> #include <mach/psc.h> #include <mach/mux.h> #include <mach/irqs.h> diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c index 11c79a3..a08910e 100644 --- a/arch/arm/mach-davinci/dm644x.c +++ b/arch/arm/mach-davinci/dm644x.c @@ -12,11 +12,11 @@ #include <linux/clk.h> #include <linux/serial_8250.h> #include <linux/platform_device.h> +#include <linux/platform_data/edma.h> #include <asm/mach/map.h> #include <mach/cputype.h> -#include <mach/edma.h> #include <mach/irqs.h> #include <mach/psc.h> #include <mach/mux.h> diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c index ac7b431..6d52a32 100644 --- a/arch/arm/mach-davinci/dm646x.c +++ b/arch/arm/mach-davinci/dm646x.c @@ -13,11 +13,11 @@ #include <linux/clk.h> #include <linux/serial_8250.h> #include <linux/platform_device.h> +#include <linux/platform_data/edma.h> #include <asm/mach/map.h> #include <mach/cputype.h> -#include <mach/edma.h> #include <mach/irqs.h> #include <mach/psc.h> #include <mach/mux.h> diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h index 700d311..9d77f9b 100644 --- a/arch/arm/mach-davinci/include/mach/da8xx.h +++ b/arch/arm/mach-davinci/include/mach/da8xx.h @@ -20,8 +20,8 @@ #include <linux/videodev2.h> #include <mach/serial.h> -#include <mach/edma.h> #include <mach/pm.h> +#include <linux/platform_data/edma.h> #include <linux/platform_data/i2c-davinci.h> #include <linux/platform_data/mmc-davinci.h> #include <linux/platform_data/usb-davinci.h> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 06ea4b8..9c7d16b 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -24,7 +24,7 @@ #include <linux/slab.h> #include <linux/spinlock.h> -#include <mach/edma.h> +#include <linux/platform_data/edma.h> #include "dmaengine.h" #include "virt-dma.h" diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c index 2063677..f5d46ea 100644 --- a/drivers/mmc/host/davinci_mmc.c +++ b/drivers/mmc/host/davinci_mmc.c @@ -35,6 +35,7 @@ #include <linux/edma.h> #include <linux/mmc/mmc.h> +#include <linux/platform_data/edma.h> #include <linux/platform_data/mmc-davinci.h> /* diff --git a/include/linux/mfd/davinci_voicecodec.h b/include/linux/mfd/davinci_voicecodec.h index 0ab6132..7dd6524 100644 --- a/include/linux/mfd/davinci_voicecodec.h +++ b/include/linux/mfd/davinci_voicecodec.h @@ -26,8 +26,7 @@ #include <linux/kernel.h> #include <linux/platform_device.h> #include <linux/mfd/core.h> - -#include <mach/edma.h> +#include <linux/platform_data/edma.h> /* * Register values. diff --git a/arch/arm/mach-davinci/include/mach/edma.h b/include/linux/platform_data/edma.h similarity index 59% rename from arch/arm/mach-davinci/include/mach/edma.h rename to include/linux/platform_data/edma.h index 7e84c90..2344ea2 100644 --- a/arch/arm/mach-davinci/include/mach/edma.h +++ b/include/linux/platform_data/edma.h @@ -1,28 +1,12 @@ /* - * TI DAVINCI dma definitions + * TI EDMA definitions * - * Copyright (C) 2006-2009 Texas Instruments. + * Copyright (C) 2006-2013 Texas Instruments. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the * Free Software Foundation; either version 2 of the License, or (at your * option) any later version. - * - * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED - * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN - * NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT - * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF - * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF - * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 675 Mass Ave, Cambridge, MA 02139, USA. - * */ /* @@ -69,11 +53,6 @@ struct edmacc_param { unsigned int ccnt; }; -#define CCINT0_INTERRUPT 16 -#define CCERRINT_INTERRUPT 17 -#define TCERRINT0_INTERRUPT 18 -#define TCERRINT1_INTERRUPT 19 - /* fields in edmacc_param.opt */ #define SAM BIT(0) #define DAM BIT(1) @@ -87,70 +66,6 @@ struct edmacc_param { #define TCCHEN BIT(22) #define ITCCHEN BIT(23) -#define TRWORD (0x7<<2) -#define PAENTRY (0x1ff<<5) - -/* Drivers should avoid using these symbolic names for dm644x - * channels, and use platform_device IORESOURCE_DMA resources - * instead. (Other DaVinci chips have different peripherals - * and thus have different DMA channel mappings.) - */ -#define DAVINCI_DMA_MCBSP_TX 2 -#define DAVINCI_DMA_MCBSP_RX 3 -#define DAVINCI_DMA_VPSS_HIST 4 -#define DAVINCI_DMA_VPSS_H3A 5 -#define DAVINCI_DMA_VPSS_PRVU 6 -#define DAVINCI_DMA_VPSS_RSZ 7 -#define DAVINCI_DMA_IMCOP_IMXINT 8 -#define DAVINCI_DMA_IMCOP_VLCDINT 9 -#define DAVINCI_DMA_IMCO_PASQINT 10 -#define DAVINCI_DMA_IMCOP_DSQINT 11 -#define DAVINCI_DMA_SPI_SPIX 16 -#define DAVINCI_DMA_SPI_SPIR 17 -#define DAVINCI_DMA_UART0_URXEVT0 18 -#define DAVINCI_DMA_UART0_UTXEVT0 19 -#define DAVINCI_DMA_UART1_URXEVT1 20 -#define DAVINCI_DMA_UART1_UTXEVT1 21 -#define DAVINCI_DMA_UART2_URXEVT2 22 -#define DAVINCI_DMA_UART2_UTXEVT2 23 -#define DAVINCI_DMA_MEMSTK_MSEVT 24 -#define DAVINCI_DMA_MMCRXEVT 26 -#define DAVINCI_DMA_MMCTXEVT 27 -#define DAVINCI_DMA_I2C_ICREVT 28 -#define DAVINCI_DMA_I2C_ICXEVT 29 -#define DAVINCI_DMA_GPIO_GPINT0 32 -#define DAVINCI_DMA_GPIO_GPINT1 33 -#define DAVINCI_DMA_GPIO_GPINT2 34 -#define DAVINCI_DMA_GPIO_GPINT3 35 -#define DAVINCI_DMA_GPIO_GPINT4 36 -#define DAVINCI_DMA_GPIO_GPINT5 37 -#define DAVINCI_DMA_GPIO_GPINT6 38 -#define DAVINCI_DMA_GPIO_GPINT7 39 -#define DAVINCI_DMA_GPIO_GPBNKINT0 40 -#define DAVINCI_DMA_GPIO_GPBNKINT1 41 -#define DAVINCI_DMA_GPIO_GPBNKINT2 42 -#define DAVINCI_DMA_GPIO_GPBNKINT3 43 -#define DAVINCI_DMA_GPIO_GPBNKINT4 44 -#define DAVINCI_DMA_TIMER0_TINT0 48 -#define DAVINCI_DMA_TIMER1_TINT1 49 -#define DAVINCI_DMA_TIMER2_TINT2 50 -#define DAVINCI_DMA_TIMER3_TINT3 51 -#define DAVINCI_DMA_PWM0 52 -#define DAVINCI_DMA_PWM1 53 -#define DAVINCI_DMA_PWM2 54 - -/* DA830 specific EDMA3 information */ -#define EDMA_DA830_NUM_DMACH 32 -#define EDMA_DA830_NUM_TCC 32 -#define EDMA_DA830_NUM_PARAMENTRY 128 -#define EDMA_DA830_NUM_EVQUE 2 -#define EDMA_DA830_NUM_TC 2 -#define EDMA_DA830_CHMAP_EXIST 0 -#define EDMA_DA830_NUM_REGIONS 4 -#define DA830_DMACH2EVENT_MAP0 0x000FC03Fu -#define DA830_DMACH2EVENT_MAP1 0x00000000u -#define DA830_EDMA_ARM_OWN 0x30FFCCFFu - /*ch_status paramater of callback function possible values*/ #define DMA_COMPLETE 1 #define DMA_CC_ERROR 2 diff --git a/include/linux/platform_data/spi-davinci.h b/include/linux/platform_data/spi-davinci.h index 7af305b..8dc2fa47 100644 --- a/include/linux/platform_data/spi-davinci.h +++ b/include/linux/platform_data/spi-davinci.h @@ -19,7 +19,7 @@ #ifndef __ARCH_ARM_DAVINCI_SPI_H #define __ARCH_ARM_DAVINCI_SPI_H -#include <mach/edma.h> +#include <linux/platform_data/edma.h> #define SPI_INTERN_CS 0xFF diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index d55e647..591f547 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -14,6 +14,7 @@ #include <linux/timer.h> #include <linux/interrupt.h> #include <linux/platform_device.h> +#include <linux/platform_data/edma.h> #include <linux/i2c.h> #include <sound/core.h> #include <sound/pcm.h> diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c index afab81f..9bdd71b 100644 --- a/sound/soc/davinci/davinci-pcm.c +++ b/sound/soc/davinci/davinci-pcm.c @@ -17,6 +17,7 @@ #include <linux/dma-mapping.h> #include <linux/kernel.h> #include <linux/genalloc.h> +#include <linux/platform_data/edma.h> #include <sound/core.h> #include <sound/pcm.h> diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h index b6ef703..fbb710c 100644 --- a/sound/soc/davinci/davinci-pcm.h +++ b/sound/soc/davinci/davinci-pcm.h @@ -14,7 +14,7 @@ #include <linux/genalloc.h> #include <linux/platform_data/davinci_asp.h> -#include <mach/edma.h> +#include <linux/platform_data/edma.h> struct davinci_pcm_dma_params { int channel; /* sync dma channel ID */ diff --git a/sound/soc/davinci/davinci-sffsdr.c b/sound/soc/davinci/davinci-sffsdr.c index 5be65aa..a45af64 100644 --- a/sound/soc/davinci/davinci-sffsdr.c +++ b/sound/soc/davinci/davinci-sffsdr.c @@ -17,6 +17,7 @@ #include <linux/timer.h> #include <linux/interrupt.h> #include <linux/platform_device.h> +#include <linux/platform_data/edma.h> #include <linux/gpio.h> #include <sound/core.h> #include <sound/pcm.h> @@ -28,12 +29,14 @@ #include <asm/plat-sffsdr/sffsdr-fpga.h> #endif -#include <mach/edma.h> #include "../codecs/pcm3008.h" #include "davinci-pcm.h" #include "davinci-i2s.h" +#define DAVINCI_DMA_MCBSP_TX 2 +#define DAVINCI_DMA_MCBSP_RX 3 + /* * CLKX and CLKR are the inputs for the Sample Rate Generator. * FSX and FSR are outputs, driven by the sample Rate Generator. @@ -124,7 +127,7 @@ static struct resource sffsdr_snd_resources[] = { static struct evm_snd_platform_data sffsdr_snd_data = { .tx_dma_ch = DAVINCI_DMA_MCBSP_TX, - .rx_dma_ch = DAVINCI_DMA_MCBSP_RX, + .rx_dma_ch = DAVINCI_DMA_MCBAP_RX, }; static struct platform_device *sffsdr_snd_device;