Re: [PATCH 1/3] dmaengine: add dma_get_slave_sg_limits()
On Thu, Jul 18, 2013 at 11:46:39AM -0500, Joel Fernandes wrote:
The API is optionally implemented by dmaengine drivers and when
unimplemented will return a NULL pointer. A client driver using
this API provides the required dma channel, address width, and
burst size of the transfer. dma_get_slave_sg_limits() returns an
SG limits structure with the maximum number and size of SG segments
that the given channel can handle.
Please look at what's already in struct device:
struct device {
...
struct device_dma_parameters *dma_parms;
...
};
This provides:
struct device_dma_parameters {
/*
* a low level driver may set these to teach IOMMU code about
* sg limitations.
*/
unsigned int max_segment_size;
unsigned long segment_boundary_mask;
};
Now, these are helpfully accessed via:
dma_get_max_seg_size(dev)
dma_set_max_seg_size(dev)
dma_get_seg_boundary(dev)
dma_set_seg_boundary(dev, mask)
Drivers already use these to work out how to construct the scatterlist
before passing it to the DMA API, which means that they should also be
used when creating a scatterlist for the DMA engine (think about it -
you have to use the DMA API to map the buffers for the DMA engine too.)
So, we already have two properties defined on a per-device basis: the
maximum size of a scatterlist segment, and the boundary over which any
segment must not cross.
The former ties up with your max_seg_len() property, though arguably it
may depend on the DMA engine access size. The problem with implementing
this new API though is that the subsystems (such as SCSI) which already
use dma_get_max_seg_size() will be at odds with what is possible via the
DMA engine.
I strongly suggest using the infrastructure at device level and not
implementing some private DMA engine API to convey this information.
As for the maximum number of scatterlist entries, really that's a bug in
the DMA engine implementations if they can't accept arbitary lengths.
I've created DMA engine drivers for implementations where you have to
program each segment individually, ones which can have the current and
next segments, as well as those which can walk a list. Provided you get
informed of a transfer being completed, there really is no reason for a
DMA engine driver to limit the number of scatterlist entries that it
will accept.
--
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831iu=/4140/ostg.clktrk
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
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. -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 2/5] spi: pl022: use generic DMA slave configuration if possible
On Thu, Feb 07, 2013 at 07:29:17PM +0100, Linus Walleij wrote: Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted. I think you're thinking of empty_zero_page which is used to provide the initial BSS pages for user apps. -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 2/5] spi: pl022: use generic DMA slave configuration if possible
On Thu, Feb 07, 2013 at 10:15:48PM +0100, Arnd Bergmann wrote: On Thursday 07 February 2013 21:19:04 Linus Walleij wrote: On Thu, Feb 7, 2013 at 8:42 PM, Arnd Bergmann a...@arndb.de wrote: On Thursday 07 February 2013, Linus Walleij wrote: Actually I once read about a feature where the kernel provides a static page full of zeroes or something like this, that would be ideal to use in cases like this, then all of this dummy page allocation and freeing can be deleted. You mean empty_zero_page? That only works if this page is read-only from the perspective of the DMA controller, but then it would be a good fit, yes. That's actually how it's used. SPI is symmetric, and in the DMA case we're not poking data into the buffers from the CPU so the controller need something - anything - to stream to the block. If we can use that page we'll even save a few remaps. I'm slightly worried about the caching effects though. The idea of the empty-zero page is that all user processes get it when they read a page before they write to it, so the data in it can essentially always be cache-hot. If we do DMA from that page to a device what would be the overhead of flushing the (clean) cache lines? If it's DMA _to_ a device, then we will only ever clean the lines prior to a transfer, never invalidate them. So that's not really a concern. (There better not be any dirty cache lines associated with the empty zero page either.) -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Mon, Feb 04, 2013 at 09:47:38PM +, 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.) -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
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? -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Tue, Feb 05, 2013 at 04:47:05PM +, 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.) -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Tue, Feb 05, 2013 at 04:30:45PM +0100, Linus Walleij wrote: On Mon, Feb 4, 2013 at 10:54 PM, Cyril Chemparathy cy...@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. -- Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Mon, Feb 04, 2013 at 05:41:53PM +0200, Felipe Balbi wrote: Hi, On Fri, Feb 01, 2013 at 09:30:03PM +, 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... -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
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... -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
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... -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Sat, Feb 02, 2013 at 10:18:51AM +, 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 +, 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 +, 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
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Fri, Feb 01, 2013 at 01:59:59PM -0500, Matt Porter wrote: On Fri, Feb 01, 2013 at 07:52:46PM +, 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. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
On Fri, Feb 01, 2013 at 10:41:08AM -0800, Tony Lindgren wrote: * Matt Porter mpor...@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. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
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. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
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. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
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. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_jan ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: remove check for bits_per_word on transfer
On Mon, Dec 17, 2012 at 11:21:57PM +0530, Laxman Dewangan wrote:
When spi client does the spi transfer and does not sets
the bits_per_word for each transfer then set it as default
of spi device in spi core before calling low level transfer.
Err, sorry? To me, the above doesn't make too much sense when considering
what's in the patch. Please can you improve the description? Thanks.
Removing the similar code from different low level drivers as
it its duplicate checks and it is no more require.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
This is the continuation of feedback got from Jonas on
change
spi: make sure all transfer has bits_per_word set
where I made the change in only tegra slink driver.
This patch remove the similar code form all drivers.
drivers/spi/spi-altera.c|2 +-
drivers/spi/spi-bfin-sport.c|3 +--
drivers/spi/spi-bfin5xx.c |3 +--
drivers/spi/spi-bitbang.c |6 +++---
drivers/spi/spi-clps711x.c |2 +-
drivers/spi/spi-coldfire-qspi.c |3 +--
drivers/spi/spi-ep93xx.c|2 +-
drivers/spi/spi-s3c64xx.c |2 +-
drivers/spi/spi-sirf.c |3 +--
drivers/spi/spi-tegra20-slink.c |9 +++--
drivers/spi/spi-txx9.c |6 ++
11 files changed, 16 insertions(+), 25 deletions(-)
diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index 5e7314a..a537f8d 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -134,7 +134,7 @@ static int altera_spi_txrx(struct spi_device *spi, struct
spi_transfer *t)
hw-tx = t-tx_buf;
hw-rx = t-rx_buf;
hw-count = 0;
- hw-bytes_per_word = (t-bits_per_word ? : spi-bits_per_word) / 8;
+ hw-bytes_per_word = t-bits_per_word / 8;
hw-len = t-len / hw-bytes_per_word;
if (hw-irq = 0) {
diff --git a/drivers/spi/spi-bfin-sport.c b/drivers/spi/spi-bfin-sport.c
index ac7ffca..39b0d17 100644
--- a/drivers/spi/spi-bfin-sport.c
+++ b/drivers/spi/spi-bfin-sport.c
@@ -416,8 +416,7 @@ bfin_sport_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
/* Bits per word setup */
- bits_per_word = transfer-bits_per_word ? :
- message-spi-bits_per_word ? : 8;
+ bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0)
drv_data-ops = bfin_sport_transfer_ops_u16;
else
diff --git a/drivers/spi/spi-bfin5xx.c b/drivers/spi/spi-bfin5xx.c
index 0429d83..7d7c991 100644
--- a/drivers/spi/spi-bfin5xx.c
+++ b/drivers/spi/spi-bfin5xx.c
@@ -642,8 +642,7 @@ static void bfin_spi_pump_transfers(unsigned long data)
drv_data-cs_change = transfer-cs_change;
/* Bits per word setup */
- bits_per_word = transfer-bits_per_word ? :
- message-spi-bits_per_word ? : 8;
+ bits_per_word = transfer-bits_per_word;
if (bits_per_word % 16 == 0) {
drv_data-n_bytes = bits_per_word/8;
drv_data-len = (transfer-len) 1;
diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c
index 8b3d8ef..61beaec 100644
--- a/drivers/spi/spi-bitbang.c
+++ b/drivers/spi/spi-bitbang.c
@@ -69,7 +69,7 @@ static unsigned bitbang_txrx_8(
unsignedns,
struct spi_transfer *t
) {
- unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+ unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u8*tx = t-tx_buf;
u8 *rx = t-rx_buf;
@@ -95,7 +95,7 @@ static unsigned bitbang_txrx_16(
unsignedns,
struct spi_transfer *t
) {
- unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+ unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u16 *tx = t-tx_buf;
u16 *rx = t-rx_buf;
@@ -121,7 +121,7 @@ static unsigned bitbang_txrx_32(
unsignedns,
struct spi_transfer *t
) {
- unsignedbits = t-bits_per_word ? : spi-bits_per_word;
+ unsignedbits = t-bits_per_word;
unsignedcount = t-len;
const u32 *tx = t-tx_buf;
u32 *rx = t-rx_buf;
diff --git a/drivers/spi/spi-clps711x.c b/drivers/spi/spi-clps711x.c
index 1366c46..a11cbf0 100644
--- a/drivers/spi/spi-clps711x.c
+++ b/drivers/spi/spi-clps711x.c
@@ -68,7 +68,7 @@ static int spi_clps711x_setup_xfer(struct spi_device *spi,
struct spi_transfer *xfer)
{
u32 speed = xfer-speed_hz ? : spi-max_speed_hz;
- u8 bpw = xfer-bits_per_word ? : spi-bits_per_word;
+ u8 bpw = xfer-bits_per_word;
struct spi_clps711x_data *hw = spi_master_get_devdata(spi-master);
if (bpw != 8) {
diff
Re: [PATCH 2/2] Revert spi/pl022: enable runtime PM
On Tue, Oct 23, 2012 at 12:10:19PM +0100, Mark Brown wrote: On Tue, Oct 23, 2012 at 12:31:06PM +0200, Ulf Hansson wrote: It seems like this patch did not get applied together with: [PATCH 1/2] Revert spi/pl022: fix spi-pl022 pm enable at probe The problem was likely my fault, since there were a lot of resends. Anyway, it should still be possible to apply. I *really* should not need to remind you to: - Not top post - Send patches in a form which can be applied if you want them applying How about actually applying the damned patch from the two-part patch set, rather than only the first, both of which has been around for almost a month now, and fixes what should never have gone into mainline in the first place? -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 2/2] Revert spi/pl022: enable runtime PM
On Tue, Oct 23, 2012 at 12:41:31PM +0100, Mark Brown wrote: On Tue, Oct 23, 2012 at 12:16:56PM +0100, Russell King - ARM Linux wrote: On Tue, Oct 23, 2012 at 12:10:19PM +0100, Mark Brown wrote: I *really* should not need to remind you to: - Not top post - Send patches in a form which can be applied if you want them applying How about actually applying the damned patch from the two-part patch set, rather than only the first, both of which has been around for almost a month now, and fixes what should never have gone into mainline in the first place? Well, the reason for the second point is that I don't have a copy of the patch any more, I need a copy of it before I can do anything. Half the reason these contentless top posts are annoying is that even if there's something to do it's not actionable. I have to say I had been under the impression that Linus' series that I applied the other day dealt with all the outstanding stuff here; the issues with this have been the awful changelogs and the overalapping sets of patches. Given that Linus is the one who introduced the fuckups in the first place, and afaics Linus' series has not been posted anywhere I can see, and I have been through the issues and worked with Ulf to get them fixed, I find that to be down right insulting by you. You received *BOTH* of Ulf's patches on the 5th October. Both needed to be applied *TOGETHER* to reduce the window where things got worse before they got better. But for some goddamned unknown reason you decided to only take the first but totally and utterly ignored the second. Well done, you've probably crapped this driver up again. Way to go Mark. Now, tell us _exactly_ what patches you've applied to this driver. Post them to this mailing list so we can see them, and see what state this driver is now in. IMHO, you're being a terrible maintainer here - and this is being shown by the need to get you to post all the patches you've currently queued up for this. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 2/2] Revert spi/pl022: enable runtime PM
On Tue, Oct 23, 2012 at 12:47:23PM +0100, Russell King - ARM Linux wrote: On Tue, Oct 23, 2012 at 12:41:31PM +0100, Mark Brown wrote: On Tue, Oct 23, 2012 at 12:16:56PM +0100, Russell King - ARM Linux wrote: On Tue, Oct 23, 2012 at 12:10:19PM +0100, Mark Brown wrote: I *really* should not need to remind you to: - Not top post - Send patches in a form which can be applied if you want them applying How about actually applying the damned patch from the two-part patch set, rather than only the first, both of which has been around for almost a month now, and fixes what should never have gone into mainline in the first place? Well, the reason for the second point is that I don't have a copy of the patch any more, I need a copy of it before I can do anything. Half the reason these contentless top posts are annoying is that even if there's something to do it's not actionable. I have to say I had been under the impression that Linus' series that I applied the other day dealt with all the outstanding stuff here; the issues with this have been the awful changelogs and the overalapping sets of patches. Given that Linus is the one who introduced the fuckups in the first place, and afaics Linus' series has not been posted anywhere I can see, and I have been through the issues and worked with Ulf to get them fixed, I find that to be down right insulting by you. You received *BOTH* of Ulf's patches on the 5th October. Both needed to be applied *TOGETHER* to reduce the window where things got worse before they got better. But for some goddamned unknown reason you decided to only take the first but totally and utterly ignored the second. Well done, you've probably crapped this driver up again. Way to go Mark. Now, tell us _exactly_ what patches you've applied to this driver. Post them to this mailing list so we can see them, and see what state this driver is now in. IMHO, you're being a terrible maintainer here - and this is being shown by the need to get you to post all the patches you've currently queued up for this. Oh, and if you're unwilling to do that, then I'm taking both of Ulf's patches, committing them to my fixes branch, and sending them off to Linus tonight - because they are _NEEDED_ for the -rc series to fix this driver. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 1/3] Revert spi/pl022: fix spi-pl022 pm enable at probe
On Sun, Sep 30, 2012 at 10:44:17AM +0200, Linus Walleij wrote: On Fri, Sep 28, 2012 at 1:21 PM, Ulf Hansson ulf.hans...@stericsson.com wrote: From: Ulf Hansson ulf.hans...@linaro.org This reverts commit 6887237cd7da904184dab2750504040c68f3a080. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Why? It was removed in this commit, is it wrong? Linus, please read the previous thread on pl022. The long and short of it is that your commit 2fb30d1147c599f5657e8c62c862f9a0f58d9d99 (spi/pl022: enable runtime PM) is totally broken, and Michel came along trying to fix the resulting breakage by partially removing your commit in 6887237cd7da904184dab2750504040c68f3a080 (spi/pl022: fix spi-pl022 pm enable at probe). The reason why your commit is wrong is: pm_runtime_get_noresume(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); ret = pcdrv-probe(pcdev, id); So, adding this in spi-pl022 probe: + pm_runtime_enable(dev); + pm_runtime_resume(dev); makes the sequence: pm_runtime_get_noresume(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); pm_runtime_enable(dev); pm_runtime_resume(dev); which is absolute rubbish. Your commit should never have gone into any tree. The real answer is to revert both commits to get the driver back to a sane state, before then progressing with the driver in a sane manner. This is what Ulf is doing. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://ad.doubleclick.net/clk;258768047;13503038;j? http://info.appdynamics.com/FreeJavaPerformanceDownload.html ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [RFC PATCH 01/13] ARM: davinci: move private EDMA API to arm/common
On Thu, Sep 20, 2012 at 10:43:34AM -0400, Matt Porter wrote: Move mach-davinci/dma.c to common/edma.c so it can be used by OMAP (specifically AM33xx atm) as well. This just moves the private EDMA API but does not support OMAP. Signed-off-by: Matt Porter mpor...@ti.com --- arch/arm/Kconfig |1 + arch/arm/common/Kconfig|3 + arch/arm/common/Makefile |1 + arch/arm/common/edma.c | 1588 arch/arm/include/asm/mach/edma.h | 267 + asm/mach should not be used as a dumping ground for platform header files. It is there to provide the interfaces between generic ARM architecture code and platform code. (At least four files that are there at the moment need to be moved out of there - patch series to follow...) -- Got visibility? Most devs has no idea what their production app looks like. Find out how fast your code is with AppDynamics Lite. http://ad.doubleclick.net/clk;262219671;13503038;y? http://info.appdynamics.com/FreeJavaPerformanceDownload.html ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [RFC PATCH 01/13] ARM: davinci: move private EDMA API to arm/common
On Fri, Sep 21, 2012 at 09:33:42AM +, Hebbar, Gururaja wrote: On Fri, Sep 21, 2012 at 14:59:23, Russell King - ARM Linux wrote: On Thu, Sep 20, 2012 at 10:43:34AM -0400, Matt Porter wrote: Move mach-davinci/dma.c to common/edma.c so it can be used by OMAP (specifically AM33xx atm) as well. This just moves the private EDMA API but does not support OMAP. Signed-off-by: Matt Porter mpor...@ti.com --- arch/arm/Kconfig |1 + arch/arm/common/Kconfig|3 + arch/arm/common/Makefile |1 + arch/arm/common/edma.c | 1588 arch/arm/include/asm/mach/edma.h | 267 + asm/mach should not be used as a dumping ground for platform header files. It is there to provide the interfaces between generic ARM architecture code and platform code. (At least four files that are there at the moment need to be moved out of there - patch series to follow...) Can this be moved to include/linux/platform_data/ ? Here's the pertinant question: is it platform data? Looking at the file, it appears to be internal data structures and register definitions for the driver itself. Therefore, it isn't platform data, and it shouldn't be living separately from the driver. If the driver itself only makes use of the data structures, the data structures should be defined either within the driver, or a header file co-located next to the driver itself. The same goes for register definitions too. The only structure that I can find which isn't internal to the driver is struct edma_soc_info, struct edma_rsv_info, and the enum dma_event_q. Those can go to include/linux/platform_data, but the rest should not. -- Got visibility? Most devs has no idea what their production app looks like. Find out how fast your code is with AppDynamics Lite. http://ad.doubleclick.net/clk;262219671;13503038;y? http://info.appdynamics.com/FreeJavaPerformanceDownload.html ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [RFC PATCH 08/13] mmc: omap_hsmmc: limit max_segs with the EDMA DMAC
On Fri, Sep 21, 2012 at 10:45:29PM +0530, S, Venkatraman wrote: On Thu, Sep 20, 2012 at 8:13 PM, Matt Porter mpor...@ti.com wrote: The EDMA DMAC has a hardware limitation that prevents supporting scatter gather lists with any number of segments. Since the EDMA DMA Engine driver sets the maximum segments to 16, we do the same. Note: this can be removed once the DMA Engine API supports an API to query the DMAC's segment limitations. I wouldn't want to bind the properties of EDMA to omap_hsmmc as this patch suggests. Why don't we have a max_segs property, which when explicitly specified in DT, will override the default ? Why not have a generic way that DMA engine can export these kinds of properties? -- Got visibility? Most devs has no idea what their production app looks like. Find out how fast your code is with AppDynamics Lite. http://ad.doubleclick.net/clk;262219671;13503038;y? http://info.appdynamics.com/FreeJavaPerformanceDownload.html ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [RFC PATCH 01/13] ARM: davinci: move private EDMA API to arm/common
On Fri, Sep 21, 2012 at 02:34:46PM -0400, Matt Porter wrote: On Fri, Sep 21, 2012 at 10:42:05AM +0100, Russell King wrote: Here's the pertinant question: is it platform data? Looking at the file, it appears to be internal data structures and register definitions for the driver itself. Therefore, it isn't platform data, and it shouldn't be living separately from the driver. If the driver itself only makes use of the data structures, the data structures should be defined either within the driver, or a header file co-located next to the driver itself. The same goes for register definitions too. The only structure that I can find which isn't internal to the driver is struct edma_soc_info, struct edma_rsv_info, and the enum dma_event_q. Those can go to include/linux/platform_data, but the rest should not. Ok, but is it ok to keep the actual private EDMA API portion in arch/arm/include/asm/mach/? It's not a problem to move the internal portions to a local include and that pdata to the appropriate place. Move the platform data parts to include/linux/platform_data. Move the driver specific parts to be either in the .c file for the driver, or in a .h file _along_ _side_ the .c file. Private data for drivers should be kept as close to the driver as possible, whether that be in the same .c file as the driver itself, or a header co-located with the .c file. -- Got visibility? Most devs has no idea what their production app looks like. Find out how fast your code is with AppDynamics Lite. http://ad.doubleclick.net/clk;262219671;13503038;y? http://info.appdynamics.com/FreeJavaPerformanceDownload.html ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 1/3] dmaengine: add TI EDMA DMA engine driver
On Thu, Aug 16, 2012 at 05:44:29PM -0400, Matt Porter wrote: Add a DMA engine driver for the TI EDMA controller. This driver is implemented as a wrapper around the existing DaVinci private DMA implementation. This approach allows for incremental conversion of each peripheral driver to the DMA engine API. The EDMA driver supports slave transfers but does not yet support cyclic transfers. Have you looked at the virt-dma support? That should allow you to avoid some common errors, like forgetting that stuff submitted but not issued should not be started even if the channel is currently running. -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] SPI: OMAP: fix over-eager devm_xxx() conversion (was: Re: [CFT 07/11] spi: omap2-mcspi: add DMA engine support)
Okay, I'm going to queue this up in my tree for -rc as no one seems to
be listening to any of the emails I've sent on Thursday.
On Thu, Jun 14, 2012 at 03:07:12PM +0100, Russell King - ARM Linux wrote:
From: Russell King rmk+ker...@arm.linux.org.uk
Subject: [PATCH] SPI: OMAP: fix over-eager devm_xxx() conversion
1a77b127ae (OMAP : SPI : use devm_* functions) converted the SPI
device controller state to use devm_kzalloc(). Unfortunately, this
is used against an unbound struct device, which results in the
following when the device is eventually bound to its driver:
[ cut here ]
WARNING: at /home/rmk/git/linux-rmk/drivers/base/dd.c:257
driver_probe_device+0x78/0x21c()
Modules linked in:
Backtrace:
[c0017d0c] (dump_backtrace+0x0/0x10c) from [c033e208]
(dump_stack+0x18/0x1c) r7: r6:c01ff28c r5:c040050c r4:0101
[c033e1f0] (dump_stack+0x0/0x1c) from [c00337ec]
(warn_slowpath_common+0x58/0x70)
[c0033794] (warn_slowpath_common+0x0/0x70) from [c0033828]
(warn_slowpath_null+0x24/0x2c)
[c0033804] (warn_slowpath_null+0x0/0x2c) from [c01ff28c]
(driver_probe_device+0x78/0x21c)
[c01ff214] (driver_probe_device+0x0/0x21c) from [c01ff49c]
(__driver_attach+0x6c/0x90)
[c01ff430] (__driver_attach+0x0/0x90) from [c01fda70]
(bus_for_each_dev+0x58/0x98)
[c01fda18] (bus_for_each_dev+0x0/0x98) from [c01ff0f4]
(driver_attach+0x20/0x28)
[c01ff0d4] (driver_attach+0x0/0x28) from [c01fe2f4]
(bus_add_driver+0xb4/0x230)
[c01fe240] (bus_add_driver+0x0/0x230) from [c01ffb24]
(driver_register+0xac/0x138)
[c01ffa78] (driver_register+0x0/0x138) from [c0215d4c]
(spi_register_driver+0x4c/0x60)
[c0215d00] (spi_register_driver+0x0/0x60) from [c045414c]
(ks8851_init+0x14/0x1c)
[c0454138] (ks8851_init+0x0/0x1c) from [c0008770]
(do_one_initcall+0x9c/0x164)
[c00086d4] (do_one_initcall+0x0/0x164) from [c0436410]
(kernel_init+0x128/0x210)
[c04362e8] (kernel_init+0x0/0x210) from [c0038754] (do_exit+0x0/0x72c)
---[ end trace 4dcda79f5e89dd84 ]---
ks8851 spi1.0: message enable is 0
ks8851 spi1.0: eth0: revision 0, MAC 08:00:28:01:4d:c6, IRQ 194, has EEPROM
Fix this by partially reverting the original commit.
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
---
drivers/spi/spi-omap2-mcspi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 9d3409a..6263b0f 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -829,7 +829,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
mcspi_dma = mcspi-dma_channels[spi-chip_select];
if (!cs) {
- cs = devm_kzalloc(spi-dev , sizeof *cs, GFP_KERNEL);
+ cs = kzalloc(sizeof *cs, GFP_KERNEL);
if (!cs)
return -ENOMEM;
cs-base = mcspi-base + spi-chip_select * 0x14;
@@ -869,6 +869,7 @@ static void omap2_mcspi_cleanup(struct spi_device *spi)
cs = spi-controller_state;
list_del(cs-node);
+ kfree(cs);
}
if (spi-chip_select spi-master-num_chipselect) {
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [CFT 07/11] spi: omap2-mcspi: add DMA engine support
On Thu, Jun 07, 2012 at 12:08:35PM +0100, Russell King wrote: Add DMA engine support to the OMAP SPI driver. This supplements the private DMA API implementation contained within this driver, and the driver can be independently switched at build time between using DMA engine and the private DMA API for the transmit and receive sides. Tested-by: Shubhrajyoti shubhrajy...@ti.com Acked-by: Grant Likely grant.lik...@secretlab.ca Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Right, now that we have working OMAP in mainline again... [ cut here ] WARNING: at lib/dma-debug.c:865 check_unmap+0x1a0/0x6f8() ks8851 spi1.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x9f5c3002] [size=592 bytes] Modules linked in: Backtrace: [c0017dd0] (dump_backtrace+0x0/0x10c) from [c0346870] (dump_stack+0x18/0x1c) r7:df79fd78 r6:c01a2200 r5:c04036bd r4:0361 [c0346858] (dump_stack+0x0/0x1c) from [c0033c48] (warn_slowpath_common+0x58/0x70) [c0033bf0] (warn_slowpath_common+0x0/0x70) from [c0033d04] (warn_slowpath_fmt+0x38/0x40) r8:df79fdf8 r7: r6:0250 r5: r4:9f5c3002 [c0033ccc] (warn_slowpath_fmt+0x0/0x40) from [c01a2200] (check_unmap+0x1a0/0x6f8) r3:c040ea22 r2:c0403a0f [c01a2060] (check_unmap+0x0/0x6f8) from [c01a2978] (debug_dma_unmap_page+0x74/0x80) [c01a2904] (debug_dma_unmap_page+0x0/0x80) from [c021bd64] (omap2_mcspi_txrx_dma+0x33c/0x54c) [c021ba28] (omap2_mcspi_txrx_dma+0x0/0x54c) from [c021c590] (omap2_mcspi_work+0x1b8/0x2b8) [c021c3d8] (omap2_mcspi_work+0x0/0x2b8) from [c021c974] (omap2_mcspi_transfer_one_message+0x2e4/0x310) [c021c690] (omap2_mcspi_transfer_one_message+0x0/0x310) from [c021a9f8] (spi_pump_messages+0x130/0x154) [c021a8c8] (spi_pump_messages+0x0/0x154) from [c00508dc] (kthread_worker_fn+0x108/0x188) r7:df6a3d94 r6:df79e000 r5:df6a3d90 r4:df6a3da4 [c00507d4] (kthread_worker_fn+0x0/0x188) from [c0050a60] (kthread+0x8c/0x98) [c00509d4] (kthread+0x0/0x98) from [c0039134] (do_exit+0x0/0x314) r7:0013 r6:c0039134 r5:c00509d4 r4:df443d78 ---[ end trace 1b75b31a2719ed1f ]--- So, trying to figure this out... the result is not nice. If the spi message has is_dma_mapped = false, then we potentially map the DMA buffers against mcspi-dev. This struct device is the same as the master-dev.parent. However, when we come to complete a transfer, we unmap them against the spi_device's struct device - in other words a different device. That's the reason for the warning. However, when using DMA engine, both of these struct device's are the wrong one to be using - the right one to use is the one assocated with the DMA engine. However, this presents a problem with transfers with is_dma_mapped = true. SPI device drivers appear to assume that the right struct device to use to map for DMA is master-dev.parent. That's fine if your SPI master device is the struct device performing the DMA, but with DMA engine involved, this is not true. Not sure at the moment what to do about that one. -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [CFT 07/11] spi: omap2-mcspi: add DMA engine support
On Thu, Jun 14, 2012 at 12:53:35PM +0100, Russell King - ARM Linux wrote: On Thu, Jun 07, 2012 at 12:08:35PM +0100, Russell King wrote: Add DMA engine support to the OMAP SPI driver. This supplements the private DMA API implementation contained within this driver, and the driver can be independently switched at build time between using DMA engine and the private DMA API for the transmit and receive sides. Tested-by: Shubhrajyoti shubhrajy...@ti.com Acked-by: Grant Likely grant.lik...@secretlab.ca Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk Right, now that we have working OMAP in mainline again... Another warning: [ cut here ] WARNING: at /home/rmk/git/linux-rmk/drivers/base/dd.c:257 driver_probe_device+0x78/0x21c() Modules linked in: Backtrace: [c0017d0c] (dump_backtrace+0x0/0x10c) from [c033e208] (dump_stack+0x18/0x1c) r7: r6:c01ff28c r5:c040050c r4:0101 [c033e1f0] (dump_stack+0x0/0x1c) from [c00337ec] (warn_slowpath_common+0x58/0x70) [c0033794] (warn_slowpath_common+0x0/0x70) from [c0033828] (warn_slowpath_null+0x24/0x2c) r8:c04aa4d8 r7:c04aa63c r6:de70ce00 r5:de70ce34 r4:de70ce00 [c0033804] (warn_slowpath_null+0x0/0x2c) from [c01ff28c] (driver_probe_device+0x78/0x21c) [c01ff214] (driver_probe_device+0x0/0x21c) from [c01ff49c] (__driver_attach+0x6c/0x90) r7:de443ec8 r6:c04aa63c r5:de70ce34 r4:de70ce00 [c01ff430] (__driver_attach+0x0/0x90) from [c01fda70] (bus_for_each_dev+0x58/0x98) r6:c04aa63c r5:c01ff430 r4: [c01fda18] (bus_for_each_dev+0x0/0x98) from [c01ff0f4] (driver_attach+0x20/0x28) r7:de6c9e80 r6:c04aa63c r5:c04aa63c r4:c0465b80 [c01ff0d4] (driver_attach+0x0/0x28) from [c01fe2f4] (bus_add_driver+0xb4/0x230) [c01fe240] (bus_add_driver+0x0/0x230) from [c01ffb24] (driver_register+0xac/0x138) [c01ffa78] (driver_register+0x0/0x138) from [c0215d4c] (spi_register_driver+0x4c/0x60) r8:005b r7:c045f848 r6:0006 r5:0018 r4:c0465b80 [c0215d00] (spi_register_driver+0x0/0x60) from [c045414c] (ks8851_init+0x14/0x1c) [c0454138] (ks8851_init+0x0/0x1c) from [c0008770] (do_one_initcall+0x9c/0x164) [c00086d4] (do_one_initcall+0x0/0x164) from [c0436410] (kernel_init+0x128/0x210) [c04362e8] (kernel_init+0x0/0x210) from [c0038754] (do_exit+0x0/0x72c) ---[ end trace 4dcda79f5e89dd84 ]--- ks8851 spi1.0: message enable is 0 ks8851 spi1.0: eth0: revision 0, MAC 08:00:28:01:4d:c6, IRQ 194, has EEPROM The relevant line: WARN_ON(!list_empty(dev-devres_head)); Which suggests that someone is using devres against the struct device for the KS8851 device before the driver is bound. That's a bug, plain and simple. I've not yet been able to track down what it is or where it's being done, but it is something that has been introduced during the last merge window. devm_* APIs should only be used by _drivers_ against the struct device that they are driving - because the lifetime of these things is bounded by the point at which the driver is bound to that struct device, to the point that it is unbound from that struct device (and at that point, all devm_* stuff against the struct device gets destroyed.) -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH] SPI: OMAP: fix over-eager devm_xxx() conversion (was: Re: [CFT 07/11] spi: omap2-mcspi: add DMA engine support)
From: Russell King rmk+ker...@arm.linux.org.uk
Subject: [PATCH] SPI: OMAP: fix over-eager devm_xxx() conversion
1a77b127ae (OMAP : SPI : use devm_* functions) converted the SPI
device controller state to use devm_kzalloc(). Unfortunately, this
is used against an unbound struct device, which results in the
following when the device is eventually bound to its driver:
[ cut here ]
WARNING: at /home/rmk/git/linux-rmk/drivers/base/dd.c:257
driver_probe_device+0x78/0x21c()
Modules linked in:
Backtrace:
[c0017d0c] (dump_backtrace+0x0/0x10c) from [c033e208]
(dump_stack+0x18/0x1c) r7: r6:c01ff28c r5:c040050c r4:0101
[c033e1f0] (dump_stack+0x0/0x1c) from [c00337ec]
(warn_slowpath_common+0x58/0x70)
[c0033794] (warn_slowpath_common+0x0/0x70) from [c0033828]
(warn_slowpath_null+0x24/0x2c)
[c0033804] (warn_slowpath_null+0x0/0x2c) from [c01ff28c]
(driver_probe_device+0x78/0x21c)
[c01ff214] (driver_probe_device+0x0/0x21c) from [c01ff49c]
(__driver_attach+0x6c/0x90)
[c01ff430] (__driver_attach+0x0/0x90) from [c01fda70]
(bus_for_each_dev+0x58/0x98)
[c01fda18] (bus_for_each_dev+0x0/0x98) from [c01ff0f4]
(driver_attach+0x20/0x28)
[c01ff0d4] (driver_attach+0x0/0x28) from [c01fe2f4]
(bus_add_driver+0xb4/0x230)
[c01fe240] (bus_add_driver+0x0/0x230) from [c01ffb24]
(driver_register+0xac/0x138)
[c01ffa78] (driver_register+0x0/0x138) from [c0215d4c]
(spi_register_driver+0x4c/0x60)
[c0215d00] (spi_register_driver+0x0/0x60) from [c045414c]
(ks8851_init+0x14/0x1c)
[c0454138] (ks8851_init+0x0/0x1c) from [c0008770]
(do_one_initcall+0x9c/0x164)
[c00086d4] (do_one_initcall+0x0/0x164) from [c0436410]
(kernel_init+0x128/0x210)
[c04362e8] (kernel_init+0x0/0x210) from [c0038754] (do_exit+0x0/0x72c)
---[ end trace 4dcda79f5e89dd84 ]---
ks8851 spi1.0: message enable is 0
ks8851 spi1.0: eth0: revision 0, MAC 08:00:28:01:4d:c6, IRQ 194, has EEPROM
Fix this by partially reverting the original commit.
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
---
drivers/spi/spi-omap2-mcspi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 9d3409a..6263b0f 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -829,7 +829,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
mcspi_dma = mcspi-dma_channels[spi-chip_select];
if (!cs) {
- cs = devm_kzalloc(spi-dev , sizeof *cs, GFP_KERNEL);
+ cs = kzalloc(sizeof *cs, GFP_KERNEL);
if (!cs)
return -ENOMEM;
cs-base = mcspi-base + spi-chip_select * 0x14;
@@ -869,6 +869,7 @@ static void omap2_mcspi_cleanup(struct spi_device *spi)
cs = spi-controller_state;
list_del(cs-node);
+ kfree(cs);
}
if (spi-chip_select spi-master-num_chipselect) {
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [CFT 07/11] spi: omap2-mcspi: add DMA engine support
On Thu, Jun 14, 2012 at 01:08:43PM +0100, Russell King - ARM Linux wrote:
On Thu, Jun 14, 2012 at 12:53:35PM +0100, Russell King - ARM Linux wrote:
On Thu, Jun 07, 2012 at 12:08:35PM +0100, Russell King wrote:
Add DMA engine support to the OMAP SPI driver. This supplements the
private DMA API implementation contained within this driver, and the
driver can be independently switched at build time between using DMA
engine and the private DMA API for the transmit and receive sides.
Tested-by: Shubhrajyoti shubhrajy...@ti.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
Right, now that we have working OMAP in mainline again...
Another warning:
[ cut here ]
WARNING: at /home/rmk/git/linux-rmk/drivers/base/dd.c:257
driver_probe_device+0x78/0x21c()
Modules linked in:
Backtrace:
[c0017d0c] (dump_backtrace+0x0/0x10c) from [c033e208]
(dump_stack+0x18/0x1c) r7: r6:c01ff28c r5:c040050c r4:0101
[c033e1f0] (dump_stack+0x0/0x1c) from [c00337ec]
(warn_slowpath_common+0x58/0x70)
[c0033794] (warn_slowpath_common+0x0/0x70) from [c0033828]
(warn_slowpath_null+0x24/0x2c)
r8:c04aa4d8 r7:c04aa63c r6:de70ce00 r5:de70ce34 r4:de70ce00
[c0033804] (warn_slowpath_null+0x0/0x2c) from [c01ff28c]
(driver_probe_device+0x78/0x21c)
[c01ff214] (driver_probe_device+0x0/0x21c) from [c01ff49c]
(__driver_attach+0x6c/0x90)
r7:de443ec8 r6:c04aa63c r5:de70ce34 r4:de70ce00
[c01ff430] (__driver_attach+0x0/0x90) from [c01fda70]
(bus_for_each_dev+0x58/0x98)
r6:c04aa63c r5:c01ff430 r4:
[c01fda18] (bus_for_each_dev+0x0/0x98) from [c01ff0f4]
(driver_attach+0x20/0x28)
r7:de6c9e80 r6:c04aa63c r5:c04aa63c r4:c0465b80
[c01ff0d4] (driver_attach+0x0/0x28) from [c01fe2f4]
(bus_add_driver+0xb4/0x230)
[c01fe240] (bus_add_driver+0x0/0x230) from [c01ffb24]
(driver_register+0xac/0x138)
[c01ffa78] (driver_register+0x0/0x138) from [c0215d4c]
(spi_register_driver+0x4c/0x60)
r8:005b r7:c045f848 r6:0006 r5:0018 r4:c0465b80
[c0215d00] (spi_register_driver+0x0/0x60) from [c045414c]
(ks8851_init+0x14/0x1c)
[c0454138] (ks8851_init+0x0/0x1c) from [c0008770]
(do_one_initcall+0x9c/0x164)
[c00086d4] (do_one_initcall+0x0/0x164) from [c0436410]
(kernel_init+0x128/0x210)
[c04362e8] (kernel_init+0x0/0x210) from [c0038754] (do_exit+0x0/0x72c)
---[ end trace 4dcda79f5e89dd84 ]---
ks8851 spi1.0: message enable is 0
ks8851 spi1.0: eth0: revision 0, MAC 08:00:28:01:4d:c6, IRQ 194, has EEPROM
The relevant line:
WARN_ON(!list_empty(dev-devres_head));
Which suggests that someone is using devres against the struct device for
the KS8851 device before the driver is bound. That's a bug, plain and
simple. I've not yet been able to track down what it is or where it's
being done, but it is something that has been introduced during the last
merge window.
devm_* APIs should only be used by _drivers_ against the struct device
that they are driving - because the lifetime of these things is bounded
by the point at which the driver is bound to that struct device, to the
point that it is unbound from that struct device (and at that point,
all devm_* stuff against the struct device gets destroyed.)
This commit introduced the bug:
commit 1a77b127ae147f5827043a9896d7f4cb248b402e
Author: Shubhrajyoti D shubhrajy...@ti.com
Date: Sat Mar 17 12:44:01 2012 +0530
OMAP : SPI : use devm_* functions
The various devm_* functions allocate memory that is released when a driver
detaches. This patch uses devm_request_and_ioremap
to request memory in probe function. Since the freeing is not
needed the calls are deleted from remove function.Also use
use devm_kzalloc for the cs memory allocation.
Signed-off-by: Shubhrajyoti D shubhrajy...@ti.com
and sure enough, reverting this makes the warning go away.
Specifically, it is this part which is the culpret:
diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 7745f91..cb2c0e3 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -789,7 +789,7 @@ static int omap2_mcspi_setup(struct spi_device *spi)
mcspi_dma = mcspi-dma_channels[spi-chip_select];
if (!cs) {
- cs = kzalloc(sizeof *cs, GFP_KERNEL);
+ cs = devm_kzalloc(spi-dev , sizeof *cs, GFP_KERNEL);
if (!cs)
return -ENOMEM;
cs-base = mcspi-base + spi-chip_select * 0x14;
@@ -831,7 +831,6 @@ static void omap2_mcspi_cleanup(struct spi_device *spi)
cs = spi-controller_state;
list_del(cs-node);
- kfree(spi-controller_state);
}
if (spi-chip_select spi-master-num_chipselect) {
because, at the time when omap2_mcspi_setup() is called, spi-dev is
not bound, and so is outside of the devres valid lifetime of that
struct device
Re: [PATCH/RFC v2] ARM: amba: Remove AMBA level regulator support
On Fri, Apr 06, 2012 at 03:49:45PM +0800, Shawn Guo wrote: I believe this patch fixes the amba probe failure that impacts any AMBA platform build with CONFIG_REGULATOR enabled. Could you consider to send the fix for -rc soon? No idea. I don't have Mark's patch in the patch system. The patch ARM: amba: adapt to regulator probe deferral change from me could be ignored now. But I do have yours in there. And what do I do with your other patch in there touching this same area as well? Please, sort it out between you and once the patch system is right, let me know. -- Better than sec? Nothing is better than sec when it comes to monitoring Big Data applications. Try Boundary one-second resolution app monitoring today. Free. http://p.sf.net/sfu/Boundary-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH/RFC v2] ARM: amba: Remove AMBA level regulator support
On Tue, Apr 10, 2012 at 09:35:43AM +0100, Mark Brown wrote: On Tue, Apr 10, 2012 at 09:31:30AM +0100, Russell King - ARM Linux wrote: On Fri, Apr 06, 2012 at 03:49:45PM +0800, Shawn Guo wrote: I believe this patch fixes the amba probe failure that impacts any AMBA platform build with CONFIG_REGULATOR enabled. Could you consider to send the fix for -rc soon? No idea. I don't have Mark's patch in the patch system. It's there as far as I can tell - it's 7366/1, which is visible in the web interface at: http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7366/1 Is there something else I need to do? Ok, I thought 7366/1 and 7367/1 were part of the same looking at just the index. In any case, what would really help is if people would discard patches they no longer want applying. -- Better than sec? Nothing is better than sec when it comes to monitoring Big Data applications. Try Boundary one-second resolution app monitoring today. Free. http://p.sf.net/sfu/Boundary-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH/RFC v2] ARM: amba: Remove AMBA level regulator support
On Mon, Apr 02, 2012 at 11:14:28AM +0100, Mark Brown wrote: On Mon, Apr 02, 2012 at 12:39:11AM +0200, Linus Walleij wrote: Mark: as Rabin says the v1 patch is probably fine, are you pushing this to ARM SoC or into Russell's patch tracker? I was waiting for some pronouncement from Russell - looking at the git logs it seems the patch tracker is the normal way to merge things. Well, this stuff was invented by Linus, so its Linus' decision about how to deal with the power control. -- This SF email is sponsosred by: Try Windows Azure free for 90 days Click Here http://p.sf.net/sfu/sfd2d-msazure ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[SPI] Fix section mismatch in spi-pl022.c
From: Russell King rmk+ker...@arm.linux.org.uk
WARNING: drivers/spi/built-in.o(.devinit.text+0xdb8): Section mismatch in
reference from the function pl022_probe() to the function
.init.text:pl022_dma_probe()
The function __devinit pl022_probe() references
a function __init pl022_dma_probe().
If pl022_dma_probe is only used by pl022_probe then
annotate pl022_dma_probe with a matching annotation.
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
---
drivers/spi/spi-pl022.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 2f9cb43..f37ad22 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -1083,7 +1083,7 @@ static int configure_dma(struct pl022 *pl022)
return -ENOMEM;
}
-static int __init pl022_dma_probe(struct pl022 *pl022)
+static int __devinit pl022_dma_probe(struct pl022 *pl022)
{
dma_cap_mask_t mask;
--
Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-dev2
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v2] spi: QUP based bus driver for Qualcomm MSM chipsets
Not specific to this patch, but I notice that you have in your headers: In-Reply-To: y References: y This is rather annoying, because it means that anyone else who has the same lines ends up having their messages attached to your messages as part of the same thread. I think you need to read the documentation for git-send-email again - and check that you're using the arguments concerning message ids and threading correctly. Thanks. -- Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] RFC: spi/sa1100: rewrite the SA1100 SPI driver
On Thu, Jan 19, 2012 at 06:49:25PM +0100, Linus Walleij wrote: On Wed, Jan 18, 2012 at 1:30 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Jan 18, 2012 at 01:13:29PM +0100, Linus Walleij wrote: We usually merge drivers for microwire, TI, Motorola ... etc into drivers/spi as well. It's a good enough fit, the differences are very small. This is how we configure mode from the PL022 driver platform data: How do you send audio data at 16-bit 2 channel 48kHz continuously with the SPI subsystem? Can't say because we haven't used it for audio transfers. However I am pretty convinced that it'd work because we're running a 20 Mbit data link on that SPI port. To get the high data rates we have an internal message queue in the driver that saturates the SPI port. Sometimes we even start to run parts of the driver in parallel on two cores: CPU0 is handling IRQs from the driver while CPU1 is preparing messages to/from the SPI subsystem. A SMP system, which SA1100 is not. With audio on the SA1100, we have to keep both DMA transfer buffer pointers filled to ensure that audio does not suffer. I don't see submitting multiple SPI transfers into the SPI subsystem would be able to do that without waiting for the previous transfer to stop completely before starting the next - and that implies the DMA transfer completes before starting to program up the next one. What I'm saying is that SPI will want DMA activity to complete _and_ end before it starts on the next transfer, which is going to create a break in the data output. Given that audio here just requires the SSP to be configured, and then everything else is just DMA, I don't see why we need to waste cycles fiddling with the SPI subsystem with its inherent batching of transfers. -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] RFC: spi/sa1100: rewrite the SA1100 SPI driver
On Wed, Jan 18, 2012 at 01:13:29PM +0100, Linus Walleij wrote: On Wed, Jan 18, 2012 at 12:13 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: Please don't delete the above definitions. OK don't want to break anything... The SSP interface is more than just SPI (it's microwire and TI format, and I have a working ASoC driver for the Assabet.) We usually merge drivers for microwire, TI, Motorola ... etc into drivers/spi as well. It's a good enough fit, the differences are very small. This is how we configure mode from the PL022 driver platform data: How do you send audio data at 16-bit 2 channel 48kHz continuously with the SPI subsystem? -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] RFC: spi/sa1100: rewrite the SA1100 SPI driver
On Wed, Jan 18, 2012 at 12:30:37PM +, Russell King - ARM Linux wrote: On Wed, Jan 18, 2012 at 01:13:29PM +0100, Linus Walleij wrote: On Wed, Jan 18, 2012 at 12:13 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: Please don't delete the above definitions. OK don't want to break anything... The SSP interface is more than just SPI (it's microwire and TI format, and I have a working ASoC driver for the Assabet.) We usually merge drivers for microwire, TI, Motorola ... etc into drivers/spi as well. It's a good enough fit, the differences are very small. This is how we configure mode from the PL022 driver platform data: How do you send audio data at 16-bit 2 channel 48kHz continuously with the SPI subsystem? Actually, feel free to move these definitions; my ASoC SSP driver already contains a cleaner copy of them. -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] RFC: spi/sa1100: rewrite the SA1100 SPI driver
On Wed, Jan 18, 2012 at 12:00:36AM +0100, Linus Walleij wrote: diff --git a/arch/arm/mach-sa1100/include/mach/SA-1100.h b/arch/arm/mach-sa1100/include/mach/SA-1100.h index bae8296..ed68746 100644 --- a/arch/arm/mach-sa1100/include/mach/SA-1100.h +++ b/arch/arm/mach-sa1100/include/mach/SA-1100.h @@ -727,86 +727,10 @@ #define MCCR1_F10MHz (MCCR1_CFS*1) /* Freq. (fmc) = ~ 10 MHz */ /* (9.585 MHz)*/ - -/* - * Synchronous Serial Port (SSP) control registers - * - * Registers - *Ser4SSCR0 Serial port 4 Synchronous Serial Port (SSP) Control - * Register 0 (read/write). - *Ser4SSCR1 Serial port 4 Synchronous Serial Port (SSP) Control - * Register 1 (read/write). - * [Bits SPO and SP are only implemented in versions 2.0 - * (rev. = 8) and higher of the StrongARM SA-1100.] - *Ser4SSDR Serial port 4 Synchronous Serial Port (SSP) Data - * Register (read/write). - *Ser4SSSR Serial port 4 Synchronous Serial Port (SSP) Status - * Register (read/write). - * - * Clocks - *fxtl, Txtl Frequency, period of the system crystal (3.6864 MHz - * or 3.5795 MHz). - *fss, Tss Frequency, period of the SSP communication. - */ - -#define Ser4SSCR0__REG(0x80070060) /* Ser. port 4 SSP Control Reg. 0 */ -#define Ser4SSCR1__REG(0x80070064) /* Ser. port 4 SSP Control Reg. 1 */ -#define Ser4SSDR __REG(0x8007006C) /* Ser. port 4 SSP Data Reg. */ -#define Ser4SSSR __REG(0x80070074) /* Ser. port 4 SSP Status Reg. */ - -#define SSCR0_DSSFld (4, 0) /* Data Size - 1 Select [3..15]*/ -#define SSCR0_DataSize(Size) /* Data Size Select [4..16] */ \ - (((Size) - 1) FShft (SSCR0_DSS)) -#define SSCR0_FRFFld (2, 4) /* FRame Format*/ -#define SSCR0_Motorola /* Motorola Serial Peripheral */ \ - /* Interface (SPI) format */ \ - (0 FShft (SSCR0_FRF)) -#define SSCR0_TI /* Texas Instruments Synchronous */ \ - /* Serial format */ \ - (1 FShft (SSCR0_FRF)) -#define SSCR0_National /* National Microwire format */ \ - (2 FShft (SSCR0_FRF)) -#define SSCR0_SSE0x0080 /* SSP Enable */ -#define SSCR0_SCRFld (8, 8) /* Serial Clock Rate divisor/2 - 1 */ - /* fss = fxtl/(2*(SCR + 1))*/ - /* Tss = 2*(SCR + 1)*Txtl */ -#define SSCR0_SerClkDiv(Div) /* Serial Clock Divisor [2..512] */ \ - (((Div) - 2)/2 FShft (SSCR0_SCR)) - /* fss = fxtl/(2*Floor (Div/2)) */ - /* Tss = 2*Floor (Div/2)*Txtl */ -#define SSCR0_CeilSerClkDiv(Div) /* Ceil. of SerClkDiv [2..512]*/ \ - (((Div) - 1)/2 FShft (SSCR0_SCR)) - /* fss = fxtl/(2*Ceil (Div/2))*/ - /* Tss = 2*Ceil (Div/2)*Txtl */ - -#define SSCR1_RIE0x0001 /* Receive FIFO 1/2-full or more */ - /* Interrupt Enable*/ -#define SSCR1_TIE0x0002 /* Transmit FIFO 1/2-full or less */ - /* Interrupt Enable*/ -#define SSCR1_LBM0x0004 /* Look-Back Mode */ -#define SSCR1_SPO0x0008 /* Sample clock (SCLK) POlarity*/ -#define SSCR1_SClkIactL (SSCR1_SPO*0) /* Sample Clock Inactive Low */ -#define SSCR1_SClkIactH (SSCR1_SPO*1) /* Sample Clock Inactive High */ -#define SSCR1_SP 0x0010 /* Sample clock (SCLK) Phase */ -#define SSCR1_SClk1P (SSCR1_SP*0)/* Sample Clock active 1 Period */ - /* after frame (SFRM, 1st edge) */ -#define SSCR1_SClk1_2P (SSCR1_SP*1)/* Sample Clock active 1/2 Period */ - /* after frame (SFRM, 1st edge) */ -#define SSCR1_ECS0x0020 /* External Clock Select */ -#define SSCR1_IntClk (SSCR1_ECS*0) /* Internal Clock */ -#define SSCR1_ExtClk (SSCR1_ECS*1) /* External Clock (GPIO [19]) */ - -#define SSDR_DATAFld (16, 0) /* receive/transmit DATA FIFOs */ - -#define SSSR_TNF 0x0002 /* Transmit FIFO Not Full (read) */ -#define SSSR_RNE 0x0004 /* Receive FIFO Not Empty (read) */ -#define SSSR_BSY 0x0008 /* SSP
Re: [GIT PULL] ARM: amba: Enable module alias autogeneration for AMBA drivers
On Mon, Dec 19, 2011 at 01:24:09PM +, Dave Martin wrote: Hi Russell, This one isn't urgent, but I'm not seeing the amba modalias patches anywhere yet. Did you have any outstanding concerns which need to be resolved? If you can suggest when/if these are likely to merge that would be great. Since Alessandro is now expecting to have to rebase on top of the amba additions anyway, we shouldn't need to worry about conflicting with his patches. Of course, if you've already merged these somewhere, then there's no problem. I have pulled it, I just haven't merged it in anywhere yet. -- Write once. Port to many. Get the SDK and tools to simplify cross-platform app development. Create new or port existing apps to sell to consumers worldwide. Explore the Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join http://p.sf.net/sfu/intel-appdev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/pl022: Enable clock in probe an use runtime_idle
On Thu, Nov 03, 2011 at 02:59:53PM +0100, Ulf Hansson wrote:
@@ -2342,11 +2350,19 @@ static int pl022_runtime_resume(struct device *dev)
return 0;
}
+
+static int pl022_runtime_idle(struct device *dev)
+{
+ pm_runtime_suspend(dev);
+ return 0;
+}
#endif
static const struct dev_pm_ops pl022_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(pl022_suspend, pl022_resume)
- SET_RUNTIME_PM_OPS(pl022_runtime_suspend, pl022_runtime_resume, NULL)
+ SET_RUNTIME_PM_OPS(pl022_runtime_suspend,
+ pl022_runtime_resume,
+ pl022_runtime_idle)
This is an unnecessary change.
The bus-level ops runtime PM ops call pm_generic_runtime_idle() when
its 'runtime_idle' operation is invoked. Let's look at the code
there:
int pm_generic_runtime_idle(struct device *dev)
{
const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
if (pm pm-runtime_idle) {
int ret = pm-runtime_idle(dev);
if (ret)
return ret;
}
pm_runtime_suspend(dev);
return 0;
}
If the driver has a NULL runtime idle, then generic code will call
pm_runtime_suspend() for the device. So, adding a runtime_idle callback
to a driver to explicitly call pm_runtime_suspend() is not required.
You are somewhat correct. But the patch is still needed as is!
No it is not required, by any means shape or form.
Reason is simply that after a probe, driver core is calling
pm_runtime_put_sync. This will not go through the
pm_generic_runtime_idle function, but directly to __pm_runtime_idle.
Let's look at the code:
static inline int pm_runtime_put_sync(struct device *dev)
{
return __pm_runtime_idle(dev, RPM_GET_PUT);
}
int __pm_runtime_idle(struct device *dev, int rpmflags)
{
...
spin_lock_irqsave(dev-power.lock, flags);
retval = rpm_idle(dev, rpmflags);
spin_unlock_irqrestore(dev-power.lock, flags);
...
}
static int rpm_idle(struct device *dev, int rpmflags)
{
int (*callback)(struct device *);
...
if (dev-pm_domain)
callback = dev-pm_domain-ops.runtime_idle;
else if (dev-type dev-type-pm)
callback = dev-type-pm-runtime_idle;
else if (dev-class dev-class-pm)
callback = dev-class-pm-runtime_idle;
else if (dev-bus dev-bus-pm)
callback = dev-bus-pm-runtime_idle;
else
callback = NULL;
if (callback)
__rpm_callback(callback, dev);
...
}
static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
__releases(dev-power.lock) __acquires(dev-power.lock)
{
...
retval = cb(dev);
...
}
Nothing in there calls down to the _driver_ level PM ops from the core
runtime PM code. What will happen is that this statement will assign
the callback pointer:
callback = dev-bus-pm-runtime_idle;
and dev-bus-pm will be amba_pm. Its runtime idle function will be
pm_generic_runtime_idle. As I quoted above:
int pm_generic_runtime_idle(struct device *dev)
{
const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
if (pm pm-runtime_idle) {
int ret = pm-runtime_idle(dev);
if (ret)
return ret;
}
pm_runtime_suspend(dev);
return 0;
}
This is the only way you get down to the driver-level pm-runtime_idle
callback.
Please describe what benefit having *THIS* pm-runtime_idle(dev) pointing
at your new function:
+static int pl022_runtime_idle(struct device *dev)
+{
+ pm_runtime_suspend(dev);
+ return 0;
+}
gains us over the case where pm-runtime_idle is NULL inside
pm_generic_runtime_idle().
--
RSA(R) Conference 2012
Save $700 by Nov 18
Register now
http://p.sf.net/sfu/rsa-sfdev2dev1
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver
On Thu, Nov 03, 2011 at 05:51:13PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: this is an other fix I do not want to fix 2 issue in one commit and I do care of mismatch erros I fix them all the time I send patch for some rm9200 few weeks ago It should not be a separate patch - it's all to do with fixing the original problem. -- RSA(R) Conference 2012 Save $700 by Nov 18 Register now http://p.sf.net/sfu/rsa-sfdev2dev1 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v2] atmel/spi: fix missing probe during the switch to module_platform_driver
On Thu, Nov 03, 2011 at 07:03:38PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: On 17:43 Thu 03 Nov , Russell King - ARM Linux wrote: On Thu, Nov 03, 2011 at 05:51:13PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: this is an other fix I do not want to fix 2 issue in one commit and I do care of mismatch erros I fix them all the time I send patch for some rm9200 few weeks ago It should not be a separate patch - it's all to do with fixing the original problem. yes when we was supposed to swtch to module_platform_driver we use the .probe which is suposed to be __devinit in this case but I was prefering to do it in 2 steps Then do the __devinit transformation _first_ so you don't introduce a new bug in the middle of your 'fix'. But I personally think you're just creating needless churn by splitting this one logical change. -- RSA(R) Conference 2012 Save $700 by Nov 18 Register now http://p.sf.net/sfu/rsa-sfdev2dev1 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 1/2 v4] atmel/spi: fix missing probe
On Fri, Nov 04, 2011 at 01:57:40AM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote: Commit 940ab889 drivercore: Add helper macro for platform_driver boilerplate converted this driver to use module_platform_driver, but due to the use of platform_driver_probe(), this resulted in the call to atmel_spi_probe being lost. Place the call to this function into the driver structure. Let me say again, this should not be the first patch in the series. If you're trying to fix a bug don't introduce a new bug by fixing another bug. If you did this with, say, the VM or FS subsystem, you'll have this patch reverted. Moreover, it's bad practice to do fixes in this way, especially when it's trivial to do it _right_ as I've already mentioned several times. I still maintain that it should be _one_ patch fixing these issues, not two. -- RSA(R) Conference 2012 Save $700 by Nov 18 Register now http://p.sf.net/sfu/rsa-sfdev2dev1 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/pl022: Enable clock in probe an use runtime_idle
On Fri, Oct 21, 2011 at 04:08:44PM +0200, Ulf Hansson wrote:
Since we are always runtime resumed when leaving probe
the clock must be enabled. To accomplish that we are able
to be runtime suspended after probe in the case when no
request is going to be recieved, a runtime_idle function
has been implemented.
Change-Id: I6cb86f2cad30ecaab16f512daf4674b039b18213
Signed-off-by: Ulf Hansson ulf.hans...@stericsson.com
Reviewed-on: http://gerrit.lud.stericsson.com/gerrit/34447
Sorry Ulf, this patch has issues.
drivers/spi/spi-pl022.c | 18 +-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index f103e47..ad48fba 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -2184,6 +2184,12 @@ pl022_probe(struct amba_device *adev, const struct
amba_id *id)
goto err_clk_prep;
}
+ status = clk_enable(pl022-clk);
+ if (status) {
+ dev_err(adev-dev, could not enable SSP/SPI bus clock\n);
+ goto err_no_clk_en;
+ }
+
/* Disable SSP */
writew((readw(SSP_CR1(pl022-virtbase)) (~SSP_CR1_MASK_SSE)),
SSP_CR1(pl022-virtbase));
Ok - this is a bug fix.
@@ -2237,6 +2243,8 @@ pl022_probe(struct amba_device *adev, const struct
amba_id *id)
free_irq(adev-irq[0], pl022);
err_no_irq:
+ clk_disable(pl022-clk);
+ err_no_clk_en:
clk_unprepare(pl022-clk);
err_clk_prep:
clk_put(pl022-clk);
As is this.
@@ -2342,11 +2350,19 @@ static int pl022_runtime_resume(struct device *dev)
return 0;
}
+
+static int pl022_runtime_idle(struct device *dev)
+{
+ pm_runtime_suspend(dev);
+ return 0;
+}
#endif
static const struct dev_pm_ops pl022_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(pl022_suspend, pl022_resume)
- SET_RUNTIME_PM_OPS(pl022_runtime_suspend, pl022_runtime_resume, NULL)
+ SET_RUNTIME_PM_OPS(pl022_runtime_suspend,
+pl022_runtime_resume,
+pl022_runtime_idle)
This is an unnecessary change.
The bus-level ops runtime PM ops call pm_generic_runtime_idle() when
its 'runtime_idle' operation is invoked. Let's look at the code
there:
int pm_generic_runtime_idle(struct device *dev)
{
const struct dev_pm_ops *pm = dev-driver ? dev-driver-pm : NULL;
if (pm pm-runtime_idle) {
int ret = pm-runtime_idle(dev);
if (ret)
return ret;
}
pm_runtime_suspend(dev);
return 0;
}
If the driver has a NULL runtime idle, then generic code will call
pm_runtime_suspend() for the device. So, adding a runtime_idle callback
to a driver to explicitly call pm_runtime_suspend() is not required.
--
RSA#174; Conference 2012
Save $700 by Nov 18
Register now#33;
http://p.sf.net/sfu/rsa-sfdev2dev1
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 2/3] spi/pl022: disable the PL022 block when unused
On Tue, Oct 18, 2011 at 02:43:27PM +0530, Viresh Kumar wrote: On 10/17/2011 6:42 PM, Linus WALLEIJ wrote: From: Virupax Sadashivpetimath virupax.sadashivpetim...@stericsson.com Make sure we clear the enable bit when the block is not used. This will save some energy in certain hardware versions. Signed-off-by: Virupax Sadashivpetimath virupax.sadashivpetim...@stericsson.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/spi/spi-pl022.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 91700bb..0a1d8ed 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -513,7 +513,12 @@ static void giveback(struct pl022 *pl022) msg-state = NULL; if (msg-complete) msg-complete(msg-context); - /* This message is completed, so let's turn off the clocks power */ + + /* disable the SPI/SSP operation */ + writew((readw(SSP_CR1(pl022-virtbase)) + (~SSP_CR1_MASK_SSE)), SSP_CR1(pl022-virtbase)); + + /* This message is completed, so let's turn off the clock! */ clk_disable(pl022-clk); amba_pclk_disable(pl022-adev); amba_vcore_disable(pl022-adev); Reviewed-by: Viresh Kumar viresh.ku...@st.com Note that I have some changes around here in my amba branch (which are dependent on other changes I have for the primecell runtime pm.) -- All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 3/3] spi/pl022: skip default configuration before suspending
On Tue, Oct 18, 2011 at 02:43:34PM +0530, Viresh Kumar wrote: On 10/17/2011 6:42 PM, Linus WALLEIJ wrote: From: Virupax Sadashivpetimath virupax.sadashivpetim...@stericsson.com The loading of the default configuration before suspending has been in the driver since its inception, but it is not really needed. Especially so since we take to all the trouble of enabling and disabling power and clock just to do this. Let's scrap this now. Signed-off-by: Virupax Sadashivpetimath virupax.sadashivpetim...@stericsson.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/spi/spi-pl022.c |5 - 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 0a1d8ed..29dc70f 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -2342,11 +2342,6 @@ static int pl022_suspend(struct amba_device *adev, pm_message_t state) return status; } - amba_vcore_enable(adev); - amba_pclk_enable(adev); - load_ssp_default_config(pl022); - amba_pclk_disable(adev); - amba_vcore_disable(adev); dev_dbg(adev-dev, suspended\n); return 0; } Reviewed-by: Viresh Kumar viresh.ku...@st.com This definitely conflicts with patches I have queued for this window. -- All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: Fix builderror in spi-pl022.c
On Tue, Sep 20, 2011 at 10:45:41PM +0200, Linus Walleij wrote: On Mon, Sep 5, 2011 at 2:45 PM, Linus Walleij linus.wall...@linaro.org wrote: On Sun, Aug 28, 2011 at 9:26 PM, Peter Huewe peterhu...@gmx.de wrote: This patch fixes a build error, introduced by commit (67fc8b9f, PM: add runtime PM support to core Primecell driver) which unfortunately was a little bit incomplete and did contain a typo (11 instead of 22). I'm not sure how this patch could have been tested back then, if it doesn't even compile ;) Grant can you please apply this patch? Right now linux-next is breaking because of this missing patch... Ping on this... ignore if it's been picked already. Isn't it already fixed? I've had this in my tree for quite some time (since 5th September) which should also be in linux-next. b21c57c ARM: 7079/1: spi: Fix builderror in spi-pl022.c 612f2eb PM: add runtime PM support to MMCI 67fc8b9 PM: add runtime PM support to core Primecell driver -- All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2dcopy1 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: Fix builderror in spi-pl022.c
On Sun, Aug 28, 2011 at 09:26:59PM +0200, Peter Huewe wrote: This patch fixes a build error, introduced by commit (67fc8b9f, PM: add runtime PM support to core Primecell driver) which unfortunately was a little bit incomplete and did contain a typo (11 instead of 22). I'm not sure how this patch could have been tested back then, if it doesn't even compile ;) Maybe it was tested on configs without CONFIG_SUSPEND enabled - which is highly likely since my ARM development boards don't support S2RAM. That's the problem with ifdef'ing code. It makes it impossible to ensure that it's properly tested. -- EMC VNX: the world's simplest storage, starting under $10K The only unified storage solution that offers unified management Up to 160% more powerful than alternatives and 25% more efficient. Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH V2 3/6] spi/spi-pl022: Don't allocate more sg than required.
On Wed, Aug 10, 2011 at 02:20:56PM +0530, Viresh Kumar wrote: In routine configure_dma(), if transfer-len = PAGE_SIZE, then pages is one more than required. While leads to one more sg getting allocated. This is wrong. Correct this to allocate correct number of sg. Signed-off-by: Viresh Kumar viresh.ku...@st.com Tested-by: Linus Walleij linus.wall...@linaro.org --- drivers/spi/spi-pl022.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 80116be..1c8b9ec 100644 --- a/drivers/spi/spi-pl022.c +++ b/drivers/spi/spi-pl022.c @@ -1016,7 +1016,8 @@ static int configure_dma(struct pl022 *pl022) dmaengine_slave_config(txchan, tx_conf); /* Create sglists for the transfers */ - pages = (pl022-cur_transfer-len PAGE_SHIFT) + 1; + pages = ((pl022-cur_transfer-len + (1 PAGE_SHIFT) - 1) + PAGE_SHIFT); It would be far better for this to be: pages = DIV_ROUND_UP(pl022-cur_transfer-len, PAGE_SIZE); The compiler will probably optimize it to the same code anyway. -- uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH V2 6/6] spi/spi-pl022: Request/free DMA channels as and when required.
On Wed, Aug 10, 2011 at 02:20:59PM +0530, Viresh Kumar wrote: Currently we request DMA channels at probe time and free them at remove. They are always occupied, irrespective of their usage. They must be allocated when they are required and must be freed after we are done with transfers. So that they can be used by other users. Which DMA engine driver requires this? Normally, when we have DMA engine drivers with multiple request signals, the slave peripheral side publishes several virtual channels which are claimed by the peripheral drivers. This (amongst other things) allows the peripheral drivers to hold claim to one of the virtual channels all the time that it's required. This actually results in better usage of the DMA controller, as the virtual channels can be assigned to physical channels dynamically according to what the physical channels are doing. Plus, actually, the idea that DMA channels should be a short-term thing is broken as soon as you start considering things like UARTs, where you need a channel tied up long-term for the receive side of things. -- uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH V2 6/6] spi/spi-pl022: Request/free DMA channels as and when required.
On Wed, Aug 10, 2011 at 03:31:42PM +0530, Koul, Vinod wrote: I would agree on both counts :) In some cases it does make sense to hold the channel for the lifetime like uart or where the channel has been tied to an interface by SoC designer. But in some cases like dw_dmac it seems you can assign channels dynamically to each usage, and runtime allocation ensures we have best utilization. If dw_dmac can assign channels dynamically at runtime to request signals, it is no different from pl08x, where we have essentially the same capability, and we do have the virtual channel support. The virtual channel support is far more flexible than picking a physical channel at allocation time, because it means you can reassign the virtual channel at any point when a transfer is not in progress. Plus it means that you don't have to keep doing the channel allocation, configuration and freeing on every transfer which would be hugely wasteful. Not to mention that it burdens peripheral drivers with unnecessary additional complexity - which means additional bugs. I would encourage all DMA engine drivers which have this capability to switch to a virtual channel setup to ensure maximum interoperability between different peripheral drivers. I'd also suggest that we probably want to make the virtual layer a library for DMA engine implementations to use. We really don't want every DMA engine implementation re-creating that support time and time again. I'll look into pulling the virtual channel stuff out of PL08x over the next month or so. -- uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH V2 6/6] spi/spi-pl022: Request/free DMA channels as and when required.
On Wed, Aug 10, 2011 at 03:39:28PM +0530, Jassi Brar wrote: On Wed, Aug 10, 2011 at 2:59 PM, viresh kumar viresh.ku...@st.com wrote: On 08/10/2011 02:30 PM, Russell King - ARM Linux wrote: They must be allocated when they are required and must be freed after we are done with transfers. So that they can be used by other users. Which DMA engine driver requires this? dw_dmac.c Normally, when we have DMA engine drivers with multiple request signals, the slave peripheral side publishes several virtual channels which are claimed by the peripheral drivers. This (amongst other things) allows the peripheral drivers to hold claim to one of the virtual channels all the time that it's required. If users of DMA expect DMA engine drivers to work this way, then we should have this mentioned clearly in DMA slave documentation. The requirement stems from the fact that most DMACs(esp third party) could be made to reroute req-signals by the platform, it has not much to do with the API. IMO, all dmac drivers should be implemented that way to be on the safer side. No it isn't. It's to do with how the physical channels are used. -- uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH V2 6/6] spi/spi-pl022: Request/free DMA channels as and when required.
On Wed, Aug 10, 2011 at 04:01:13PM +0530, Jassi Brar wrote: On Wed, Aug 10, 2011 at 3:31 PM, Koul, Vinod vinod.k...@intel.com wrote: On Wed, 2011-08-10 at 14:59 +0530, viresh kumar wrote: On 08/10/2011 02:30 PM, Russell King - ARM Linux wrote: They must be allocated when they are required and must be freed after we are done with transfers. So that they can be used by other users. Which DMA engine driver requires this? dw_dmac.c Normally, when we have DMA engine drivers with multiple request signals, the slave peripheral side publishes several virtual channels which are claimed by the peripheral drivers. This (amongst other things) allows the peripheral drivers to hold claim to one of the virtual channels all the time that it's required. If users of DMA expect DMA engine drivers to work this way, then we should have this mentioned clearly in DMA slave documentation. @Dan/Vinod: What do you say? I would agree on both counts :) In some cases it does make sense to hold the channel for the lifetime like uart or where the channel has been tied to an interface by SoC designer. But in some cases like dw_dmac it seems you can assign channels dynamically to each usage, and runtime allocation ensures we have best utilization. So i would argue that there is no one size fits all here, if you can manage channels dynamically and utilize more efficiently then go ahead, but if you cant (h/w and usage constraint) then you should not be forced to do so. The idea is to have channel allocation as purely a s/w thing - no actual commitment of h/w resources. So we can afford to have channel allocated for the whole lifetime of a client. Some dmac drivers are written 'improperly', keeping in mind the platforms that have fixed ReqSig-Peri map and no more clients than actual req-sigs are active simultaneously. But such dmac drivers will fail if a new platform decides to hijack req-signals. So, imho, it is absolutely a good thing for every dmac driver to be designed for re-routable ReqSig-Peri map... which would force their design to allocate virtual/software channels to clients without commit much(any?) h/w resources. We discussed channel allocation at Linaro. However, I am now _really_ disappointed. I discussed this with Linus on the bus back from Cambridge in the evening, and it appears that the story you gave me was inaccurate - Linus had not agreed to your proposal and saw more or less the same problems with it which I've been on at you about via your other email alias/lkml. So that's essentially invalidated everything we discussed there as part of my thinking was if Linus is happy with it, then I am now convinced that you'll say *anything* just to get your idea into the kernel. So, the stakes have now been raised further for you: what I want to see from you is a _working_ _implementation_ for those three platforms which I outlined. I don't want more discussion. I want patches from you. Nothing else. We'll then review the code changes themselves rather than a vague idea communicated by email/verbally. -- uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v3] pxa2xx_spi: fix memory corruption
On Fri, Jul 15, 2011 at 03:31:06PM -0600, Grant Likely wrote: ... plus care must be taken not to accidentally reload the line into cache after it has been pushed out for DMA, which is a risk on structures with embedded DMA buffers if other non-DMA elements end up in the same cache line. This is the situation I was wondering about. I don't think it matters one bit in this case - we're not caring about the actual data read/written in the DMA, we just need to do DMA to keep the controller happy. That's especially true as we don't set the address increment bit in the DMA command register, so we end up accessing the same RAM location time and time again for each DMA transfer. So please, merge the patch - it fixes a serious memory-scribbling bug. -- AppSumo Presents a FREE Video for the SourceForge Community by Eric Ries, the creator of the Lean Startup Methodology on Lean Startup Secrets Revealed. This video shows you how to validate your ideas, optimize your ideas and identify your business strategy. http://p.sf.net/sfu/appsumosfdev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v3] pxa2xx_spi: fix memory corruption
On Thu, Jul 14, 2011 at 08:53:31PM -0600, Grant Likely wrote: + u8 null_dma_buf_unaligned[16]; Don't dma buffers need to be cache-line aligned? How large is the actual transfer? Using the __aligned() or __cacheline_aligned attribute is the correct way to make sure you've got a data buffer that can be used for DMA mixed with other stuff. Then you don't need to fool around with PTR_ALIGN or anything. Err, did you not read the whole patch? + drv_data-null_dma_buf = + (u32 *)PTR_ALIGN(drv_data-null_dma_buf_unaligned, 8); -- AppSumo Presents a FREE Video for the SourceForge Community by Eric Ries, the creator of the Lean Startup Methodology on Lean Startup Secrets Revealed. This video shows you how to validate your ideas, optimize your ideas and identify your business strategy. http://p.sf.net/sfu/appsumosfdev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v3] pxa2xx_spi: fix memory corruption
On Fri, Jul 15, 2011 at 01:50:03PM -0600, Grant Likely wrote: On Fri, Jul 15, 2011 at 09:12:42AM +0100, Russell King - ARM Linux wrote: On Thu, Jul 14, 2011 at 08:53:31PM -0600, Grant Likely wrote: + u8 null_dma_buf_unaligned[16]; Don't dma buffers need to be cache-line aligned? How large is the actual transfer? Using the __aligned() or __cacheline_aligned attribute is the correct way to make sure you've got a data buffer that can be used for DMA mixed with other stuff. Then you don't need to fool around with PTR_ALIGN or anything. Err, did you not read the whole patch? + drv_data-null_dma_buf = + (u32 *)PTR_ALIGN(drv_data-null_dma_buf_unaligned, 8); I read a lot of patches yesterday. I may very well have missed something. I still don't see what you're referring to though. If the __aligned() was used inside the structure definition, then there would be no need to have both the null_dma_buf pointer and the null_dma_buf_unaligned buffer. It would just be a correctly aligned null_dma_buf. That depends on the alignment guarantees from kmalloc, which may not be 8 bytes - we have this: #if defined(CONFIG_AEABI) (__LINUX_ARM_ARCH__ = 5) #define ARCH_SLAB_MINALIGN 8 #endif so presumably on !AEABI or arches ARMv5, kmalloc _can_ return less than 8 byte alignments. Which makes using __aligned() in the definition useless. Plus, I was asking about whether it was valid to use the structure as allocated in DMA operations since it may very well end up in the same cache line as the allocated structure. Firstly, that could mean DMA and the cache referencing the same memory which could cause corruption, and secondly on ARM isn't it a problem to have DMA buffers in memory that is also cache mapped? For the second point, that depends on whether you're talking about the coherent stuff or the streaming stuff. The coherent DMA API has entirely different semantics to streaming DMA API. The coherent DMA API allows for simultaneous access to the buffer by both the DMA device and the host CPU. The streaming DMA API only allows exclusive access by either the DMA device or the host CPU. Therefore, with the streaming DMA API, the only thing that's required is to ensure that data is visible in some manner to the DMA device. If the DMA device can read from the CPU cache, then probably nothing's required. If not, then the data must be evicted from as many levels of cache that are necessary to make it visible. Conversely, for DMA writes, what matters is the visibility of the data to the host CPU. That approach does not work with the coherent DMA API. Take a network driver TX ring. Consider the effect of the following series of actions to see why it won't work: - host CPU reads a word from the DMA buffer. This brings in a whole cache line. - network device writes to the previous descriptor (which overlaps the just read cache line) to change its status - host CPU updates the next descriptor and writes the cache line back XXX overwriting the network device's write to the previous descriptor. So, coherent DMA is special because there's no exclusiveness there. -- AppSumo Presents a FREE Video for the SourceForge Community by Eric Ries, the creator of the Lean Startup Methodology on Lean Startup Secrets Revealed. This video shows you how to validate your ideas, optimize your ideas and identify your business strategy. http://p.sf.net/sfu/appsumosfdev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] pxa2xx_spi: fix memory corruption
On Sun, Jul 10, 2011 at 01:05:48AM +0200, Marek Vasut wrote: On Saturday, July 09, 2011 11:14:58 PM Vasily Khoruzhick wrote: pxa2xx_spi_probe allocates struct driver_data and null_dma_buf at same time via spi_alloc_master(), but then calculates null_dma_buf pointer incorrectly, and it causes memory corruption later if DMA usage is enabled. Signed-off-by: Vasily Khoruzhick anars...@gmail.com --- drivers/spi/pxa2xx_spi.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c index dc25bee..ef38fbf 100644 --- a/drivers/spi/pxa2xx_spi.c +++ b/drivers/spi/pxa2xx_spi.c @@ -1569,7 +1569,7 @@ static int __devinit pxa2xx_spi_probe(struct platform_device *pdev) master-transfer = transfer; drv_data-ssp_type = ssp-type; - drv_data-null_dma_buf = (u32 *)ALIGN((u32)(drv_data + + drv_data-null_dma_buf = (u32 *)ALIGN(((u32)drv_data + sizeof(struct driver_data)), 8); This thing looks a bit disturbing in itself. Like, where the heck is that thing pointing in the end ? Since some data are written to address in null_dma_buf ... isn't this just changing the corruption impact ? /* Allocate master with space for drv_data and null dma buffer */ master = spi_alloc_master(dev, sizeof(struct driver_data) + 16); So there's 16 bytes at the end of driver_data. However: (u32)(drv_data + sizeof(struct driver_data)) is pointer arithmetic. drv_data points at an object of sizeof(struct driver_data). Adding one to this increments the pointer by sizeof(struct driver_data) bytes. So the above expression increments the pointer by sizeof(struct driver_data)*sizeof(struct driver_data) bytes, which is obviously complete rubbish. ((u32)drv_data + sizeof(struct driver_data)) casts drv_data to a u32 first, then adds the sizeof(struct driver_data) which moves us into the 16 bytes allocated off the end of the struct. -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: reorganize drivers
On Mon, Jun 06, 2011 at 11:00:38AM +0200, Arnd Bergmann wrote: This leaves out the two most common buses, USB and PCI, mostly because the directories contain a lot of stuff that is not really bus code but actual drivers. It does include i2c and spi, which stick out by being a lot larger than most others. Opinions? Move or don't move? I don't see much point in adding an additional level of directories. All that it'll do is add to the shell filename completion workload and wear out our tab keys faster... We'll still have these directories with not a lot under them. -- Simplify data backup and recovery for your virtual environment with vRanger. Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Discover what all the cheering's about. Get your free trial download today. http://p.sf.net/sfu/quest-dev2dev2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: reorganize drivers
On Mon, Jun 06, 2011 at 08:39:04AM -0600, Grant Likely wrote: However, there are a number of bus types that have infrastructure, but no actual drivers associated with them, like mca, amba and clk. It might make sense to move those bus types into drivers/base alongside the platform_bus_type implementation. More or less agreed - I'm not sure we should think about clk just yet because we don't know what that'll end up looking like yet... -- Simplify data backup and recovery for your virtual environment with vRanger. Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Discover what all the cheering's about. Get your free trial download today. http://p.sf.net/sfu/quest-dev2dev2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH resend] spi/amba-pl022: work in polling or interrupt mode if pl022_dma_probe fails
On Mon, May 16, 2011 at 04:56:59PM +0530, viresh kumar wrote: On 05/16/2011 04:34 PM, Russell King - ARM Linux wrote: On Mon, May 16, 2011 at 09:40:10AM +0530, Viresh Kumar wrote: If pl022_dma_probe fails, we can try to transfer data in polling or interrupt mode. Also, set platform_info-enable_dma to 0, so that no other code tries to use dma. Signed-off-by: Viresh Kumar viresh.ku...@st.com Acked-by: Linus Walleij linus.wall...@linaro.org Shouldn't this go via the SPI people rather than my tree? This can, but i have seen amba drivers patches going through your drivers branch, so sent to tracker. Yes, because I authored quite a few primecell drivers, and I continue to maintain those drivers. The SPI driver is not one of those though. Should i get them through spi-devel?? Yes please. -- Achieve unprecedented app performance and reliability What every C/C++ and Fortran developer should know. Learn how Intel has extended the reach of its next-generation tools to help boost performance applications - inlcuding clusters. http://p.sf.net/sfu/intel-dev2devmay ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: tegra: don't treat NULL clk as an error
On Mon, Jan 10, 2011 at 01:58:12PM -0700, Grant Likely wrote: On Mon, Jan 10, 2011 at 4:05 AM, Jamie Iles ja...@jamieiles.com wrote: Some platforms have been known to return NULL from clk_get() if they support only a single struct clk. Whilst tegra doesn't do this, make the drivers consistent with others. Cc: Erik Gilling konk...@android.com Signed-off-by: Jamie Iles ja...@jamieiles.com Hi James, If NULL does get returned, say due to a future change to the clock code, then this change causes the driver to oops. I'm not going to apply this patch. Please apply it - the clock API just defines struct clk as a cookie where errors are IS_ERR/PTR_ERR. Other values must be considered by drivers as perfectly valid, including NULL. -- Gaining the trust of online customers is vital for the success of any company that requires sensitive data to be transmitted over the Web. Learn how to best implement a security strategy that keeps consumers' information secure and instills the confidence they need to proceed with transactions. http://p.sf.net/sfu/oracle-sfdevnl ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi: tegra: don't treat NULL clk as an error
On Mon, Jan 10, 2011 at 04:24:17PM -0700, Grant Likely wrote: I was actually looking at the implementation of clk_enable() which is called by the driver and doesn't do any NULL checking. But I suppose that it then becomes the clock-code's own fault if it returns NULL and then oopses on a NULL being passed to it. Okay, I'll apply it. Yes absolutely. The clk API must eat whatever cookies it produces, even if they contain dead flies rather than currants. ;) -- Gaining the trust of online customers is vital for the success of any company that requires sensitive data to be transmitted over the Web. Learn how to best implement a security strategy that keeps consumers' information secure and instills the confidence they need to proceed with transactions. http://p.sf.net/sfu/oracle-sfdevnl ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 1/9] spi/pxa2xx: don't use subys initcall for driver init
On Thu, Nov 25, 2010 at 06:14:49PM -0700, Grant Likely wrote: On Thu, Nov 25, 2010 at 4:54 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Nov 24, 2010 at 04:39:05PM +0100, Sebastian Andrzej Siewior wrote: Mark Brown wrote: On Wed, Nov 24, 2010 at 03:09:25PM +0100, Sebastian Andrzej Siewior wrote: I've been pointed out to this commit but I don't understand _why_. The part I don't get is so it can be used with cpufreq. Is it refered to a driver or the subsystem as it? We need the regulators for the CPU rails to start before the cpufreq driver starts so cpufreq can talk to them, and since the regulators may be SPI attached this means we also need the SPI controller to start before cpufreq. cpufreq starts at vanilla init time. After digging through the code I think I've found it. pxa_cpu_init() registers a cpufreq client. cpufreq calls init and pxa then regulator_get() to get the regulator and I guess this is the problem. So I would suggest to defer pxa_cpu_init() via late_initcall(). The other way around will force you to hack the init code for various drivers to make it work. Why should the PXA code change when you haven't explained _why_ you want to change the SPI driver to conform to your idea? The idea is to get away from trying to resolve probe order issue by messing with the initcall level in spi and i2c bus drivers. It's fragile, and it doesn't work with drivers as modules. Instead, we're investigating making the dependencies explicit in the board support code. However, hacking other initcalls to enable what I want to do on the spi bus drivers isn't a solution, it is just moving the problem. I know that's what _you're_ doing, but that doesn't seem to be what Sebastian is doing. My question was targeted towards at Sebastian. -- Increase Visibility of Your 3D Game App Earn a Chance To Win $500! Tap into the largest installed PC base get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [sodaville] [PATCH 1/9] spi/pxa2xx: don't use subys initcall for driver init
On Fri, Nov 26, 2010 at 11:50:28AM +0100, Sebastian Andrzej Siewior wrote: * Russell King - ARM Linux | 2010-11-25 23:54:15 [+]: Why should the PXA code change when you haven't explained _why_ you want to change the SPI driver to conform to your idea? The problem was, that the platform driver never got probed after I registered the PCI driver. For that reason I made the patch attached. It got lost while moving the tree forward and I did not notice it earlier. While at it, I changed the subsys_init to module_init because it looked wrong. At this time I was also thinking about using one module for the platform and PCI code but never got to it. This is one of the problems of the foo_driver_probe() idea - if the device is not present at the time the driver is registered, then the driver loses out completely. This would seem to be exagerated as you're creating this platform device from a PCI device - who's to say that someone won't unbind the PCI device and re-bind it later, causing the platform device to be deleted and re-created? As such, I think the patch you've shown in this email is more appropriate (getting rid of the platform_driver_probe()) than trying to sort out init-level dependencies. Really, the init-level dependencies in this case are not the problem - the use of platform_driver_probe() is. -- Increase Visibility of Your 3D Game App Earn a Chance To Win $500! Tap into the largest installed PC base get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 1/9] spi/pxa2xx: don't use subys initcall for driver init
On Wed, Nov 24, 2010 at 04:39:05PM +0100, Sebastian Andrzej Siewior wrote: Mark Brown wrote: On Wed, Nov 24, 2010 at 03:09:25PM +0100, Sebastian Andrzej Siewior wrote: I've been pointed out to this commit but I don't understand _why_. The part I don't get is so it can be used with cpufreq. Is it refered to a driver or the subsystem as it? We need the regulators for the CPU rails to start before the cpufreq driver starts so cpufreq can talk to them, and since the regulators may be SPI attached this means we also need the SPI controller to start before cpufreq. cpufreq starts at vanilla init time. After digging through the code I think I've found it. pxa_cpu_init() registers a cpufreq client. cpufreq calls init and pxa then regulator_get() to get the regulator and I guess this is the problem. So I would suggest to defer pxa_cpu_init() via late_initcall(). The other way around will force you to hack the init code for various drivers to make it work. Why should the PXA code change when you haven't explained _why_ you want to change the SPI driver to conform to your idea? -- Increase Visibility of Your 3D Game App Earn a Chance To Win $500! Tap into the largest installed PC base get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v5 01/12] misc: add driver for sequencer serial port
On Tue, Nov 16, 2010 at 01:35:40PM -0700, Grant Likely wrote: I'll let Russel make the decision here; but I must admit I'm puzzled. ^ Grr. I'm ambivalent over it, provided that the author realises that the __raw variants have no ordering guarantees other than their accesses being ordered only with respect to themselves and nothing else. If we do want to go down the route of creating some official native endian accessors, then I'm fine with that too. -- Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today http://p.sf.net/sfu/msIE9-sfdev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH v3 01/12] misc: add driver for sequencer serial port
On Fri, Oct 22, 2010 at 03:33:43PM -0400, Cyril Chemparathy wrote:
On 10/22/2010 08:48 AM, Arnd Bergmann wrote:
On Friday 22 October 2010 14:39:33 Cyril Chemparathy wrote:
+/* Register Access Helpers */
+static inline u32 ssp_read(struct ti_ssp *ssp, int reg)
+{
+return __raw_readl(ssp-regs + reg);
+}
+
+static inline void ssp_write(struct ti_ssp *ssp, int reg, u32 val)
+{
+__raw_writel(val, ssp-regs + reg);
+}
Why are the __raw functions used here?
These registers are to be accessed native endian at all times, and
therefore the le32 conversion done otherwise is inappropriate.
Won't that break on out-of-order CPUs that need the extra synchronization
done in readl/writel?
AFAICS, ioremap()ed space on ARMv6 should be strongly ordered.
No. ioremap'd space is device memory on ARMv6 and above, which means
if you care about the ordering of writes to device vs memory, you
need barriers.
Nevertheless, individual reads/writes to devices will be in program
order, but writes may be delayed.
--
Nokia and ATT present the 2010 Calling All Innovators-North America contest
Create new apps games for the Nokia N8 for consumers in U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store
http://p.sf.net/sfu/nokia-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 04/16] spi/imx: save the spi chip select in config struct, not the gpio to use
On Fri, Sep 17, 2010 at 01:11:14PM +0200, Lothar Waßmann wrote:
Hi,
diff --git a/drivers/spi/spi_imx.c b/drivers/spi/spi_imx.c
index 60a52d1..23db984 100644
--- a/drivers/spi/spi_imx.c
+++ b/drivers/spi/spi_imx.c
@@ -56,7 +56,7 @@ struct spi_imx_config {
unsigned int speed_hz;
unsigned int bpw;
unsigned int mode;
- int cs;
+ u8 cs;
^^
u8 cs in spi_imx_config.
};
enum spi_imx_devtype {
@@ -218,6 +218,7 @@ static int __maybe_unused spi_imx0_4_config(struct
spi_imx_data *spi_imx,
struct spi_imx_config *config)
{
unsigned int reg = MX31_CSPICTRL_ENABLE | MX31_CSPICTRL_MASTER;
+ int cs = spi_imx-chipselect[config-cs];
which is used as an index to this array (which presumably is an array of
ints.) Let's hope that it is 256 entries long (maybe it should be
checked before use?)
reg |= spi_imx_clkdiv_2(spi_imx-spi_clk, config-speed_hz)
MX31_CSPICTRL_DR_SHIFT;
@@ -230,9 +231,8 @@ static int __maybe_unused spi_imx0_4_config(struct
spi_imx_data *spi_imx,
reg |= MX31_CSPICTRL_POL;
if (config-mode SPI_CS_HIGH)
reg |= MX31_CSPICTRL_SSPOL;
- if (config-cs 0) {
- reg |= (config-cs + 32) MX31_CSPICTRL_CS_SHIFT;
- }
+ if (cs 0)
This can never be true, since an 'int' promoted from 'u8' will always
be positive!
This is the value of chipselect[config-cs] not of the u8 config-cs.
--
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH v6 2/2] ep93xx: SPI driver platform support code
On Tue, May 04, 2010 at 12:21:36PM -0500, H Hartley Sweeten wrote: That patch is already present in linux-next. If Russell think there might be an issue you could rebase this patch on the linux-next tree. If this patch depends on 5998/1, then I'll take it via the ep93xx branch (which has 5998/1 on.) It's only touching files in arch/arm so there shouldn't be any problem with that. -- ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH] ARM: Add spi controller driver support for NUC900
On Thu, Nov 19, 2009 at 04:40:50PM +0800, Wan ZongShun wrote: 2009/11/19 Andrew Morton a...@linux-foundation.org: I don't know, because I don't know what operation the hardware needs to stop it from generating interrupts. Perhaps that's clk_disable()? The interrupt will be not occur as long as I run clk_disable(). Once you've stopped the source of interrupts then the code should wait for the IRQ handler to complete if it's running on another CPU. Yes, free_irq() does that. So, regarding my system of single CPU, maybe I need put this 'clk_disable()' in the front of function of w90p910_spi_remove(). right? Depending on the hardware, that's not the right answer. If turning off the clock also causes register accesses to the device to abort, it is a very dangerous thing to do. It can also be dangerous if the clock is used to synchronise the interrupt output - it can lead to the output being permanently asserted if the clock is turned off with it asserted. Normally devices have an interrupt enable register. You should disable all interrupts from the device using this register after unregistering the driver from the (SPI) subsystem. -- Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH] Add spi resource define for w90p910 spi driver
On Wed, Jul 22, 2009 at 06:25:41PM +0800, Wan ZongShun wrote: This is a spi resource define patch for w90p910 spi driver. Ok. -- ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH] rename spi device in clock api for w90p910
On Wed, Jul 22, 2009 at 06:28:11PM +0800, Wan ZongShun wrote: When programming clock api, I named this spi device as w90p910-usi, now, I think named it as w90p910-spi better Ok. In terms of patch ordering, I think this patch should be applied before the patch adding the spi device. -- ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [RFC] generic iMX-spi driver
On Thu, Apr 30, 2009 at 11:44:43PM +0200, Guennadi Liakhovetski wrote: what's the state of this patch? Is a new revision planned soon to go in for 2.6.31? No idea, as far as I can see after my last comments, it's dead. -- Register Now Save for Velocity, the Web Performance Operations Conference from O'Reilly Media. Velocity features a full day of expert-led, hands-on workshops and two days of sessions from industry leaders in dedicated Performance Operations tracks. Use code vel09scf and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [RFC] generic iMX-spi driver
On Thu, Feb 26, 2009 at 11:38:44AM +0100, Wolfram Sang wrote:
On Fri, Feb 20, 2009 at 04:41:21PM +0100, Holger Schurig wrote:
+struct spi_core_version {
+ u32 rxdata;
+ u32 txdata;
+ u32 ctrl;
+ u32 int_en;
+ u32 status;
+ u32 test;
+ u32 period;
+ u32 dma;
+ u32 reset;
+
+ u32 int_te;
+ u32 int_th;
+ u32 int_ro;
+ u32 int_te_en;
+ u32 int_th_en;
+ u32 int_ro_en;
+ u32 xch;
+
+ u8 bitcount_shift;
+ u8 sspol_shift;
+ u8 ssctl_shift;
+ u8 pha_shift;
+ u8 pol_shift;
+ u8 datarate_shift;
+ u8 reset_val;
+
+ u32 max_divider;
+ u32 max_bitcount;
+ u32 default_ctrl;
+ u32 (*calc_datarate)(struct driver_data *drv_data, u32
speed_hz, u32 *val);
+};
A number of u8's in the middle of a structure where later
32 bit long entries are comming?
Move that to the end or change it to 32 (I yet have to verify if
the compiler creates stupid or good code when accessing u8 out of
a struct, but I assume that on ARM the code is more-or-less stupid).
Well, could be that I thought too much about size vs. performance issues
here. My idea was to put the variables needed in hot-paths to the front
of the struct.
That is a good reason for ordering structs like this. ARM does generate
good code for accessing bytes in structures - it does have a byte load
instruction.
What it does do though is want to naturally align structure members, so
it's a good idea to group u8's together up to the next members natural
boundary. So:
struct {
u32 hot_32_1;
u8 hot_8_1;
u8 hot_8_2;
u32 hot_32_2;
u32 cold_32_1;
u8 cold_8_1;
u8 cold_8_2;
u32 cold_32_2;
};
would be a silly layout - you'd end up adding an additional four bytes for
no gain. Instead:
struct {
u32 hot_32_1;
u8 hot_8_1;
u8 hot_8_2;
u8 cold_8_1;
u8 cold_8_2;
u32 hot_32_2;
u32 cold_32_1;
u32 cold_32_2;
};
would be optimal use of memory without breaking the proximty of the hot
members (since you've put the two cold bytes into the wastage bytes.)
--
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [spi-devel-general] [PATCH] jffs2 summary allocation
On Fri, Apr 04, 2008 at 12:48:12PM -0700, Andrew Morton wrote:
On Fri, 4 Apr 2008 10:23:55 + (GMT)
Michael Trimarchi [EMAIL PROTECTED] wrote:
Hi,
I apply this patch to fix this oops.
Unable to handle kernel NULL pointer dereference at virtual address
pgd = c0004000
[] *pgd=
stopped custom tracer.
Internal error: Oops: 817 [#1] PREEMPT
Modules linked in:
CPU: 0Not tainted (2.6.24-rc5-rt1 #37)
PC is at dma_cache_maint+0x40/0x80
LR is at atmel_spi_transfer+0x94/0x178
pc : [c002488c]lr : [c013eedc]psr: 2013
sp : c044db84 ip : c044db94 fp : c044db90
r10: r9 : r8 : c04e4c00
r7 : c03ee310 r6 : c044dcfc r5 : c109d3bc r4 : c044dcd8
r3 : r2 : 0001 r1 : c109d7dc r0 : c109d3bc
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 0005317f Table: 20588000 DAC: 0017
Process jffs2_gcd_mtd1 (pid: 313, stack limit = 0xc044c258)
Stack: (0xc044db84 to 0xc044e000)
...
Backtrace:
[c002484c] (dma_cache_maint+0x0/0x80) from [c013eedc]
(atmel_spi_transfer+0x94/0x178)
[c013ee48] (atmel_spi_transfer+0x0/0x178) from [c013e124]
(spi_sync+0x74/0x98)
[c013e0b0] (spi_sync+0x0/0x98) from [c013dcb0]
(dataflash_write+0x1b0/0x270)
r8:14bf r7:0420 r6:c0446000 r5:0420 r4:00a5f800
[c013db00] (dataflash_write+0x0/0x270) from [c013a00c]
(part_write+0xa8/0xb0)
[c0139f64] (part_write+0x0/0xb0) from [c00e0724]
(jffs2_flash_writev+0x278/0x434)
r6:c04d9000 r5:0420 r4:0420
[c00e04b0] (jffs2_flash_writev+0x4/0x434) from [c00e1f40]
(jffs2_sum_write_sumnode+0x334/0x420)
[c00e1c0c] (jffs2_sum_write_sumnode+0x0/0x420) from [c00d5ca0]
(jffs2_do_reserve_space+0x94/0x3c8)
[c00d5c0c] (jffs2_do_reserve_space+0x0/0x3c8) from [c00d6014]
(jffs2_reserve_space_gc+0x40/0x78)
[c00d5fd4] (jffs2_reserve_space_gc+0x0/0x78) from [c00da938]
(jffs2_garbage_collect_pristine+0x5c/0x3a8)
[c00da8dc] (jffs2_garbage_collect_pristine+0x0/0x3a8) from [c00dc32c]
(jffs2_garbage_collect_pass+0x590/0x714)
[c00dbd9c] (jffs2_garbage_collect_pass+0x0/0x714) from [c00dd730]
(jffs2_garbage_collect_thread+0x100/0x18c)
[c00dd630] (jffs2_garbage_collect_thread+0x0/0x18c) from [c0039818]
(do_exit+0x0/0x73c)
Code: 9a01 e15c0003 3a01 e3a03000 (e5833000)
--- a/fs/jffs2/summary.c~jffs2-summary-allocation
+++ a/fs/jffs2/summary.c
@@ -17,7 +17,6 @@
#include linux/pagemap.h
#include linux/crc32.h
#include linux/compiler.h
-#include linux/vmalloc.h
#include nodelist.h
#include debug.h
@@ -30,7 +29,7 @@ int jffs2_sum_init(struct jffs2_sb_info
return -ENOMEM;
}
- c-summary-sum_buf = vmalloc(c-sector_size);
+ c-summary-sum_buf = kmalloc(c-sector_size, GFP_KERNEL);
if (!c-summary-sum_buf) {
JFFS2_WARNING(Can't allocate buffer for writing out summary
information!\n);
@@ -49,7 +48,7 @@ void jffs2_sum_exit(struct jffs2_sb_info
jffs2_sum_disable_collecting(c-summary);
- vfree(c-summary-sum_buf);
+ kfree(c-summary-sum_buf);
c-summary-sum_buf = NULL;
kfree(c-summary);
_
All this does is switch sum_buf from vmalloced-memory over to
kmalloced-memory.
I'm assuming from the trace that the arm code tried to put that memory
under DMA (or at least, passed it into part of the DMA management code to
get the various caches sorted out) and that the arm DMA support code
doesn't like being given vmalloced memory.
So the question is: who is wrong here? Is jffs wrong to use vmalloced
memory in this application, or is arm wrong to not handle it?
If ARM is wrong, we're going to have to walk page tables and all that
mess so that we can perform L2 cache maintainence for DMA... which
becomes quite expensive and I believe starts to make DMA pointless on
ARM CPUs.
The DMA docs say this of dma_map_coherent():
Notes: Not all memory regions in a machine can be mapped by this
API. Further, regions that appear to be physically contiguous in
kernel virtual space may not be contiguous as physical memory. Since
this API does not provide any scatter/gather capability, it will fail
if the user tries to map a non-physically contiguous piece of memory.
For this reason, it is recommended that memory mapped by this API be
obtained only from sources which guarantee it to be physically contiguous
(like kmalloc).
vmalloc memory is not physically contiguous, so the attempt to create
the mapping fails.
-
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
