Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
On Thu, Nov 25, 2010 at 8:05 AM, Kamoolkar, Mugdha mug...@ti.com wrote:
Consider a software module on top of the hwspinlock, which provides a
generic way for multi-core critical section, say GateMP. This module enables
users to create critical section objects by name. Any other client can open
the critical section by name and get a handle to it. I would expect this
module to make a call to request a resource when creating the GateMP object.
Suppose that the object is actually created by the remote core, and the call
comes to Linux on the host processor to allocate the system resource (as the
owner of the system resources). It will call hwspinlock_request, get a
handle, get the ID from it, and return the ID to the remote processor. There
is no point in the remote processor holding the handle that's not valid in
its virtual space. The ID, in this case, is the single portable value that
every processor understands in a different way. When this object were being
deleted, the ID would be passed to Linux, and a corresponding Linux entity
would then have to get the handle from the ID and call _free.
How is it different from any other hardware resource that the host
will allocate on behalf of the slaves ? Do we have such _open API for,
e.g., dmtimer ?
It looks like this is a broader problem that needs to be solved by the
kernel module that will manage the resources of the remote processors.
Similarly, suppose the creation is happening from user-space, the user-space
object should store the ID in the user object, and get the handle from the
ID when performing any actions on the lock from kernel-side.
The user space interface is not yet defined, but -
One option is to expose a char device for every hwspinlock device,
that can only be opened by a single user (which, after successfully
opening it, will become its owner). Once that user will close the
device (either consciously or by crashing), the kernel will free the
resource. It doesn't look like an _open API is needed here.
For example:
struct hwspinlock *hwspinlock_get_handle(unsigned int id);
I'm afraid such an API will be easily abused, e.g., drivers that will
try to use a predefined hwspinlock id without taking care of reserving
it early enough will probably use it to overcome an oops this
hwspinlock has already been assigned to someone else.
Really? Why would they intentionally destabilize the system like this???
These things just happen.. even by mistake.
But I rather focus on the why yes rather than on the why not.
Let's define the problem exactly, and then see how to solve it
correctly. Btw it might still end up being an _open API if we find it
as the best solution (but let's first see that we understand the
problem first).
No real issues with this, just seems less intuitive. I just expected all the
module APIs to follow same convention module_API to make them more
intuitive.
Then I'll change it, it does might look better.
+int __hwspin_lock_timeout(struct hwspinlock *hwlock, unsigned long to,
+ int mode, unsigned long *flags)
+{
+ int ret;
+
+ for (;;) {
+ /* Try to take the hwspinlock */
+ ret = __hwspin_trylock(hwlock, mode, flags);
+ if (ret != -EBUSY)
+ break;
+
+ /*
+ * The lock is already taken, let's check if the user
wants
+ * us to try again
+ */
+ if (to time_is_before_eq_jiffies(to))
+ return -ETIMEDOUT;
+
+ /*
+ * Allow platform-specific relax handlers to prevent
+ * hogging the interconnect (no sleeping, though)
+ */
+ if (hwlock-ops-relax)
+ hwlock-ops-relax(hwlock);
There should be a way to have an escape mechanism for the case where a
deadlock
has occurred due to remote side taking the spinlock and crashing.
This is exactly what the timeout parameter is for..
I was looking at the situation where someone does not use the lock with
timeout.
In that case, do we say that there's no way out?
If one decides to use the simple _lock version, then yes (just like
with spin_lock()).
But if the user cares about an optional crash of the remote task, then
definitely the _timeout version should be used - that's why we
provided it.
We can remove the _lock version altogether, but I don't think it's a
good thing. A user can still use the _timeout version with infinite
timeout, which would yield the same result. The _lock version is
provided for simple cases, where a timeout isn't required.
Thanks,
Ohad.
And the system
will hang and need a re-boot? Or do we still want to enable some way to
forcibly break the wait after it has happened for an unreasonably long time?
Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
On Thu, Nov 25, 2010 at 10:22 PM, David Brownell davi...@pacbell.net wrote: So there's no strong reason to think this is actually ggeneric. Function names no longer specify OMAP, but without other hardware under the interface, calling it generic reflects more optimism than reality. (That was the implication of my observations...) Well, it's not omap-specific anymore. Drivers can stay platform-agnostic, and other vendors can plug in anything they need in order to use the same drivers (including software implementations as mentioned below). As I mentioned, the other two options are going back to an omap specific driver (which will omapify drivers who use it), or putting the driver in the omap arch folders (which some people doesn't seem to like, e.g. http://www.spinics.net/lists/linux-arm-msm/msg00324.html). Which one of those would you prefer to see ? I guess that by now we have all three implementations so it's pretty easy to switch :) To find other hardware spinlocks, you might be able to look at fault tolerant multiprocessors. Ages ago I worked with one of those, where any spinlock failures integrated with CPU/OS fault detection; HW cwould yank (checkpointed) CPU boards off the bus so they could be recovered (some might continue later from checkpoints, etc.)... Is that HW supported by Linux today ? Any chance you can share a link or any other info about it ? This way platforms [2] can easily plug into the framework anything they need to achieve multi-core synchronization. E.g., even in case a platform doesn't have dedicated silicon, but still need this functionality, it can still plug in an implementation which is based on Peterson's shared memory mutual exclusion algorithm And maybe also standard Linux spinlocks? Umm, possibly. Though I admit it sounds awkward a bit. It feels like platforms, on which a mere spinlock is enough to achieve multi core synchronization, will not really need any of the IPC drivers that are going to use this hwspinlock framework. But I guess we need to see a use case first. I seem to recall some iterations of the real-time patches doing a lot of work to generalize spinlocks, since they needed multiple variants. It might be worth following in those footsteps. (Though I'm not sure they were thinking much about hardware support . Any chance you can point me at a specific discussion or patchset that you feel may be relevant ? Thanks! Ohad. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
explicit comment saying this shouldn't happen). Thanks! Ohad. + + module_put(hwlock-owner); + +out: + spin_unlock(hwspinlock_tree_lock); + return ret; +} +EXPORT_SYMBOL_GPL(hwspinlock_free); + +MODULE_LICENSE(GPL v2); +MODULE_DESCRIPTION(Generic hardware spinlock interface); +MODULE_AUTHOR(Ohad Ben-Cohen o...@wizery.com); -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
On Fri, Nov 26, 2010 at 11:18 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Fri, Nov 26, 2010 at 10:53:10AM +0200, Ohad Ben-Cohen wrote:
+int __hwspin_trylock(struct hwspinlock *hwlock, int mode, unsigned long
*flags)
+{
+ int ret;
+
+ if (unlikely(!hwlock)) {
+ pr_err(invalid hwlock\n);
These kind of errors can get very spammy for buggy drivers.
Yeah, but that's the purpose - I want to catch such egregious drivers
who try to crash the kernel.
That can be better - because you get a backtrace, and it causes people
to report the problem rather than just ignore it. It may also prevent
the driver author releasing his code (as it won't work on their
initial testing.)
...
If it's extremely buggy behaviour then the drivers deserve to crash.
Such stuff should cause them not to get out the door. A simple printk
with an error return can just be ignored.
I like this approach too, but recently we had a few privilege
escalation exploits which involved NULL dereference kernel bugs
(process context mapped address 0 despite a positive mmap_min_addr).
Since we can't rely on the oops to always happen, I decided not to
omit the NULL checks.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
On Fri, Nov 26, 2010 at 12:45 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Fri, Nov 26, 2010 at 12:16:39PM +0200, Ohad Ben-Cohen wrote:
On Fri, Nov 26, 2010 at 11:18 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Fri, Nov 26, 2010 at 10:53:10AM +0200, Ohad Ben-Cohen wrote:
+int __hwspin_trylock(struct hwspinlock *hwlock, int mode, unsigned
long *flags)
+{
+ int ret;
+
+ if (unlikely(!hwlock)) {
+ pr_err(invalid hwlock\n);
These kind of errors can get very spammy for buggy drivers.
Yeah, but that's the purpose - I want to catch such egregious drivers
who try to crash the kernel.
That can be better - because you get a backtrace, and it causes people
to report the problem rather than just ignore it. It may also prevent
the driver author releasing his code (as it won't work on their
initial testing.)
...
If it's extremely buggy behaviour then the drivers deserve to crash.
Such stuff should cause them not to get out the door. A simple printk
with an error return can just be ignored.
I like this approach too, but recently we had a few privilege
escalation exploits which involved NULL dereference kernel bugs
(process context mapped address 0 despite a positive mmap_min_addr).
That's not a concern on ARM.
Good point, thanks. The problem is that we can't rely on that in a
generic interface.
I remember a recent discussion where you suggested to have a
conditional check that will be compiled out on architectures where we
can rely on NULL dereference to produce an oops; something like that
can be handy here.
But then there's the other (quite reasonable) claim that says we
shouldn't crash the machine because of a non fatal bug: if a crappy
driver messes up, the user (not the developer) will most probably
prefer the machine to keep running with degraded functionality rather
than boot.
... which gets us back to pr_err.
at virtual address 0 does not rely on mmap_min_addr - in fact, we can't
use this as it's tuneable to enforce this requirement.
It's highly illegal on ARM - as ARM CPUs without vector remap place the
hardware vectors at virtual address 0, and as such allowing the user to
map a page there will take the system down. So we have this code in the
mmap path:
#define arch_mmap_check(addr, len, flags) \
(((flags) MAP_FIXED (addr) FIRST_USER_ADDRESS) ? -EINVAL : 0)
which prevents any attempt what so ever, irrespective of the mmap_min_addr
setting, to create a userspace induced mapping at address 0.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] omap: zoom: wl1271 slot is MMC_CAP_POWER_OFF_CARD
This patch complements ed919b0 mmc: sdio: fix runtime PM anomalies by
introducing MMC_CAP_POWER_OFF_CARD by declaring MMC_CAP_POWER_OFF_CARD
on the ZOOM's wl1271 mmc slot.
This is required in order not to break runtime PM support for the wl1271
sdio driver.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
Quick summary:
After adding SDIO runtime PM support, we realized there are some
board/host/card setups that are incapable of powering off the card
after boot. For details, see:
http://thread.gmane.org/gmane.linux.kernel.mmc/4342/focus=4579
As a result, we added MMC_CAP_POWER_OFF_CARD which should be explicitly set
by setups that _do_ support powering off the card.
This ensures we don't break existing functionality: SDIO core will enable
runtime PM for cards only if that cap is set.
As a result, the card will be powered down after boot, and will only
be powered up again when a driver is loaded (and then it's up to the
driver whether power will be kept or not).
To complement that fix (which was just merged upstream), we need this
patch too in 2.6.37, otherwise wl1271_sdio will break.
Pandora/Beagle wl12xx users: you need a similar patch as well, as this one
only takes care of ZOOM (I don't have those other setups and preferred not to
send a patch without testing).
arch/arm/mach-omap2/board-zoom-peripherals.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c
b/arch/arm/mach-omap2/board-zoom-peripherals.c
index 86c9b21..9db9203 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -216,7 +216,7 @@ static struct omap2_hsmmc_info mmc[] __initdata = {
{
.name = wl1271,
.mmc= 3,
- .caps = MMC_CAP_4_BIT_DATA,
+ .caps = MMC_CAP_4_BIT_DATA | MMC_CAP_POWER_OFF_CARD,
.gpio_wp= -EINVAL,
.gpio_cd= -EINVAL,
.nonremovable = true,
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
On Sat, Nov 27, 2010 at 12:53 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Sat, Nov 27, 2010 at 12:18:55AM +0200, Ohad Ben-Cohen wrote:
But then there's the other (quite reasonable) claim that says we
shouldn't crash the machine because of a non fatal bug: if a crappy
driver messes up, the user (not the developer) will most probably
prefer the machine to keep running with degraded functionality rather
than boot.
There's also the quite reasonable expectation that we shouldn't corrupt
user data. With locking interfaces, if someone abuses them and they
fail to work, then the risk is data corruption due to races. The safe
thing in that case is to panic - terminate that thread before it does
anything unsafe, thereby preventing data corruption.
Makes sense.
I considered hwspinlock as a peripheral which doesn't qualify to take
down the system on failure, but in general I agree that there indeed
might be critical user data involved. Especially if this is a
framework and not just another driver.
But as a framework that can be used on non ARM architectures as well,
I do prefer to check for NULL pointers and not rely on the Oops.
If we had a macro that would be compiled out on architectures that
reliably produces an Oops on NULL dereference, but otherwise, would
BUG_ON on them, that should satisfy everyone.
The BUG_ON_MAPPABLE_NULL() macro below should achieve exactly that,
please tell me what you think, thanks!
arch/arm/include/asm/mman.h |1 +
include/asm-generic/bug.h | 40
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/mman.h b/arch/arm/include/asm/mman.h
index 41f99c5..d4ee003 100644
--- a/arch/arm/include/asm/mman.h
+++ b/arch/arm/include/asm/mman.h
@@ -1,4 +1,5 @@
#include asm-generic/mman.h
+#include asm/pgtable.h
#define arch_mmap_check(addr, len, flags) \
(((flags) MAP_FIXED (addr) FIRST_USER_ADDRESS) ? -EINVAL : 0)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index c2c9ba0..d211d9c 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -2,6 +2,11 @@
#define _ASM_GENERIC_BUG_H
#include linux/compiler.h
+#include asm/mman.h
+
+#ifndef arch_mmap_check
+#define arch_mmap_check(addr, len, flags) (0)
+#endif
#ifdef CONFIG_BUG
@@ -53,6 +58,41 @@ struct bug_entry {
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
#endif
+/**
+ * BUG_ON_MAPPABLE_NULL() - panic on NULL only if address 0 is mappable
+ * @addr: address to check
+ *
+ * In general, NULL dereference Oopses are not desirable, since they take down
+ * the system with them and make the user extremely unhappy. So as a general
+ * rule kernel code should avoid dereferencing NULL pointers by doing a
+ * simple check (when appropriate), and if needed, continue operating
+ * with reduced functionality rather than crash.
+ *
+ * _Critical_ kernel code, OTOH, that should not (/cannot) keep running when
+ * given an unexpected NULL pointer, should just crash. On some architectures,
+ * a NULL dereference will always reliably produce an Oops. On others, where
+ * the zero address can be mmapped, an Oops is not guaranteed. Relying on
+ * NULL dereference Oopses to happen on these architectures might lead to
+ * data corruptions (system will keep running despite a critical bug and
+ * the results will be horribly undefined). In addition, these situations
+ * can also have security implications - we have seen several privilege
+ * escalation exploits with which an attacker gained full control over the
+ * system due to NULL dereference bugs.
+ *
+ * This macro will BUG_ON if @addr is NULL on architectures where the zero
+ * addresses can be mapped. On other architectures, where the zero address
+ * can never be mapped, this macro is compiled out, so the system will just
+ * Oops when the @addr is dereferenced.
+ *
+ * As with BUG_ON, use this macro only if a NULL @addr cannot be tolerated.
+ * If proceeding with degraded functionality is an option, it's much
+ * better to just simply check for the NULL and returning instead of crashing
+ * the system.
+ */
+#define BUG_ON_MAPPABLE_NULL(addr) do { \
+ if (arch_mmap_check(0, 1, MAP_FIXED) == 0) BUG_ON(!addr); \
+} while(0)
+
/*
* WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report
* significant issues that need prompt attention if they should ever
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
On Sat, Nov 27, 2010 at 3:24 AM, David Brownell davi...@pacbell.net wrote: Your intent generic is fine, but you've not achieved it and thus I think you shouldn't imply that you have. Dropping the word generic should suffice; it _is_ a framework, and maybe the next person working with hardware spinlocks can finish generalizing (and add use cases). Sure, I can drop the word generic. Haven't looked at RT in a long time. Just look at the current RT patchset to see if it still has several spinlock variants. ISTR the raw spinlock stuff came from there not long ago. Much compile-time magic was involved in switching between variants. I'll take a look there and see if I catch anything relevant. Thanks, Ohad. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
Hi Olof,
On Sat, Nov 27, 2010 at 12:51 AM, Olof Johansson o...@lixom.net wrote:
+int __hwspin_trylock(struct hwspinlock *hwlock, int mode, unsigned long
*flags)
+{
+ int ret;
+
+ if (unlikely(!hwlock)) {
+ pr_err(invalid hwlock\n);
These kind of errors can get very spammy for buggy drivers.
Yeah, but that's the purpose - I want to catch such egregious drivers
who try to crash the kernel.
...
Anyway, above plus a __WARN() would be OK with me.
Np, but let's wait a bit to see if something like the
BUG_ON_MAPPABLE_NULL macro materializes or not. That should satisfy
everyone in a generic way.
+ /*
+ * We can be sure the other core's memory operations
+ * are observable to us only _after_ we successfully take
+ * the hwspinlock, so we must make sure that subsequent memory
+ * operations will not be reordered before we actually took the
+ * hwspinlock.
+ *
+ * Note: the implicit memory barrier of the spinlock above is too
+ * early, so we need this additional explicit memory barrier.
+ */
+ mb();
rmb() should be sufficient here.
It's not.
We need to make sure that our writes, too, will not be reordered
before that barrier. Otherwise, we might end up changing protected
shared memory during the critical section of the remote processor
(before we acquire the lock we must not read from, or write to, the
protected shared memory).
I guess I need to add a comment about this.
How is the shared memory mapped? It should be mapped such that speculative
writes shouldn't be happening, and presumably such that hardware-triggered
prefetces won't happen either.
That's up to the users of hwspinlock to take care of, we shouldn't
worry about it here.
For those cases a DMB should be sufficient
on ARM, a DSB shouldn't be needed?
Maybe it would make more sense to move the barriers down into the
implementation-specific layer, where the appropriate low-level barries
can be used instead of the generic rmb()/wmb()/mb() ones that tend to
be heavy-handed where implementations don't match generic requirements.
...
Still, could be useful to push down to hardware-specific layers and use
just the specific barrier needed.
But that's what mb/wmb/rmb are for - we use mb because we care of both
read and write reordering - and then it's up to platform-specific code
to implement it correctly. We shouldn't worry about the
platform-specific implementation of mb/wmb/rmb in drivers.
+ /* this implies an unrecoverable bug. at least rant */
+ WARN_ON(tmp != hwlock);
I don't see how this could ever happen?
It can't. Unless there's a bug somewhere.
If it's an unrecoverable bug, then you should panic(). Or BUG_ON(),
which can also be compiled out.
Sure.
Anyway, I'm not going to argue more over this one (and the others like
it). It just seemed redundant and other subsystems tend to rely on
low-level services working correctly. :)
It's not entirely about checking other subsystems: in a way, one can
also look at it as sanity checks to the hwspinlock framework itself.
For example, when we fetch an hwlock object, and then make sure the id
of the hwlock really match the id we looked for, we get a sanity-level
confidence that our data isn't messed up, and that the radix tree was
pretty much used correctly so far.
If, for some reason, that check would fail, that doesn't necessarily
mean the radix code is faulty: it just means something went really
bad, and for some reason the incorrect object is stored with the id we
looked for. It can also be hwspinlock's fault.
It's important to detect such an anomaly as early as possible,
otherwise the wrong lock is going to be used, user data might be
corrupted, and in general, it's going to be vry hard to debug
this.
So I consider this a low-cost check with very high value, and if it
would be triggered even once in the lifetime of hwspinlock, it's well
worth it.
+/**
+ * hwspinlock_unregister() - unregister an hw spinlock
+ * @id: index of the specific hwspinlock to unregister
+ *
+ * This function should be called from the underlying platform-specific
+ * implementation, to unregister an existing (and unused) hwspinlock.
+ *
+ * Can be called from an atomic context (will not sleep) but not from
+ * within interrupt context.
+ *
+ * Returns the address of hwspinlock @id on success, or NULL on failure
Why not just return int for success / fail and have the driver keep track
of the lock pointers too?
Because it's elegant. This way drivers don't need to keep track of the
pointers.
It can be changed, with an extra cost of code (duplicated for each
implementation) and memory, but why would we want to do that ?
Functions should be short and sweet, and do just one thing
Do you consider radix_tree_delete() inappropriate then ?
It is now a hwspinlock_unregister_and_return(). Making a function to
lookup
Re: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
On Tue, Nov 30, 2010 at 11:00 AM, Tony Lindgren t...@atomide.com wrote: Do we even need the hwspin_lock variants, I personally don't have any specific use case in mind. It's just a simple wrapper over the _timeout variants, provided for API completeness, but - why can't we always use the hwspin_lock_timeout variants? We can. I can just remove the _lock variants. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] Introduce hardware spinlock framework
OMAP4 introduces a Hardware Spinlock device, which provides hardware assistance for synchronization and mutual exclusion between heterogeneous processors and those not operating under a single, shared operating system (e.g. OMAP4 has dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP). The intention of this hardware device is to allow remote processors, that have no alternative mechanism to accomplish synchronization and mutual exclusion operations, to share resources (such as memory and/or any other hardware resource). This patchset adds hwspinlock framework that makes it possible for drivers to use those hwspinlock devices and stay platform-independent. Currently there are two users for this hwspinlock interface: 1. Inter-processor communications: on OMAP4, cpu-intensive multimedia tasks are offloaded by the host to the remote M3 and/or C64x+ slave processors. To achieve fast message-based communications, a minimal kernel support is needed to deliver messages arriving from a remote processor to the appropriate user process. This communication is based on a simple data structure that is shared between the remote processors, and access to it is synchronized using the hwspinlock module (remote processor directly places new messages in this shared data structure). 2. On some OMAP4 boards, the I2C bus is shared between the A9 and the M3, and the hwspinlock is used to synchronize access to it. While (2) can get away with an omap-specific hwspinlock implementation, (1) is by no means omap-specific, and a common hwspinlock interface is needed to keep it generic, in hopes that it will be useful for other platforms as well. Changes v2-v3: - Remove the timeout-less _lock API variant (Tony) - s/state-io_base/io_base/ (Ionut) - Remove the generic wording (David) - s/hwspinlock_/hwspin_lock_/ (Mugdha) - Use MAX_SCHEDULE_TIMEOUT to indicate no timeout (Mugdha) - Be more verbose on egregious API misuse (Olof) - locking API misuse is BUG_ON material (Russell) Note: Russell also suggested compiling out NULL checks on ARM. I've posted an initial proposal for this (see https://lkml.org/lkml/2010/11/29/96), which I'm going to resubmit separately. If accepted, I'll adopt hwspinlocks to use it. Patches are tested against linux-omap-2.6 master, which is 2.6.37-rc4 plus omap material. All, but the third (the hwmod omap patch), apply on top of mainline 2.6.37-rc4 as well Changes v1-v2: - Convert to a generic interface (Tony) - API should silently succeed if framework isn't built (Greg) - Don't use ERR_PTR pattern (Grant) - Use tristate, fix and extend commentary (Kevin) - Provide API flexibility regarding irq handling (Arnd, Grant) Note: after reviewing OMAP's L4 access times, and comparing them with external memory latencies, I can say that there is no notable difference. Because of that, we can safely treat the hwspinlock like we do with regular spinlocks: preemption should be disabled, but whether to disable interrupts or not is up to the caller. So despite the TRM's recommendation to always disable local interrupts when taking an OMAP Hardware Spinlock, I have decided to allow callers not to do that (by providing the full extent of hwspin_lock(), hwspin_lock_irq() and hwspin_lock_irqsave() API). Just like regular spinlocks, it's up to the callers to decide whether interrupts should be disabled or not. Sleeping, btw, is still prohibited of course. Contributions: Previous versions of an omap-specific hwspinlock driver circulated in linux-omap several times, and received substantial attention and contribution from many developers (see [1][2][3][4][5][6]): Simon Que did the initial implementation and pushed several iterations Benoit Cousson provided extensive review, help, improvements and hwmod support Hari Kanigeri helped out when Simon was away Sanjeev Premi, Santosh Shilimkar and Nishanth Menon did lots of review I'd like to thank Benoit Cousson, Steve Krueger, Hari Kanigeri, Nourredine Hamoudi and Richard Woodruff for useful discussions about the OMAP Spinlock requirements and use-cases. Relevant linux-omap threads: [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/38755 [2] http://thread.gmane.org/gmane.linux.ports.arm.omap/38917 [3] http://thread.gmane.org/gmane.linux.ports.arm.omap/39187 [4] http://thread.gmane.org/gmane.linux.ports.arm.omap/39365 [5] http://thread.gmane.org/gmane.linux.ports.arm.omap/39815 [6] http://thread.gmane.org/gmane.linux.ports.arm.omap/40901 Benoit Cousson (1): OMAP4: hwmod data: Add hwspinlock Ohad Ben-Cohen (1): drivers: hwspinlock: add framework Simon Que (2): drivers: hwspinlock: add OMAP implementation omap: add hwspinlock device Documentation/hwspinlock.txt | 299 +++ arch/arm/mach-omap2/Makefile |1 + arch/arm/mach-omap2/hwspinlock.c | 63 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 64 drivers/Kconfig|2 + drivers
[PATCH v3 1/4] drivers: hwspinlock: add framework
Add a platform-independent hwspinlock framework. Hardware spinlock devices are needed, e.g., in order to access data that is shared between remote processors, that otherwise have no alternative mechanism to accomplish synchronization and mutual exclusion operations. Signed-off-by: Ohad Ben-Cohen o...@wizery.com Cc: Hari Kanigeri h-kanige...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Kevin Hilman khil...@deeprootsystems.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Tony Lindgren t...@atomide.com --- Documentation/hwspinlock.txt | 299 ++ drivers/Kconfig |2 + drivers/Makefile |1 + drivers/hwspinlock/Kconfig | 13 + drivers/hwspinlock/Makefile |5 + drivers/hwspinlock/hwspinlock.h | 61 drivers/hwspinlock/hwspinlock_core.c | 557 ++ include/linux/hwspinlock.h | 298 ++ 8 files changed, 1236 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwspinlock.txt create mode 100644 drivers/hwspinlock/Kconfig create mode 100644 drivers/hwspinlock/Makefile create mode 100644 drivers/hwspinlock/hwspinlock.h create mode 100644 drivers/hwspinlock/hwspinlock_core.c create mode 100644 include/linux/hwspinlock.h diff --git a/Documentation/hwspinlock.txt b/Documentation/hwspinlock.txt new file mode 100644 index 000..65324ce --- /dev/null +++ b/Documentation/hwspinlock.txt @@ -0,0 +1,299 @@ +Hardware Spinlock Framework + +1. Introduction + +Hardware spinlock modules provide hardware assistance for synchronization +and mutual exclusion between heterogeneous processors and those not operating +under a single, shared operating system. + +For example, OMAP4 has dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP, +each of which is running a different Operating System (the master, A9, +is usually running Linux and the slave processors, the M3 and the DSP, +are running some flavor of RTOS). + +A generic hwspinlock framework allows platform-independent drivers to use +the hwspinlock device in order to access data structures that are shared +between remote processors, that otherwise have no alternative mechanism +to accomplish synchronization and mutual exclusion operations. + +This is necessary, for example, for Inter-processor communications: +on OMAP4, cpu-intensive multimedia tasks are offloaded by the host to the +remote M3 and/or C64x+ slave processors (by an IPC subsystem called Syslink). + +To achieve fast message-based communications, a minimal kernel support +is needed to deliver messages arriving from a remote processor to the +appropriate user process. + +This communication is based on simple data structures that is shared between +the remote processors, and access to it is synchronized using the hwspinlock +module (remote processor directly places new messages in this shared data +structure). + +A common hwspinlock interface makes it possible to have generic, platform- +independent, drivers. + +2. User API + + struct hwspinlock *hwspin_lock_request(void); + - dynamically assign an hwspinlock and return its address, or NULL + in case an unused hwspinlock isn't available. Users of this + API will usually want to communicate the lock's id to the remote core + before it can be used to achieve synchronization. + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + struct hwspinlock *hwspin_lock_request_specific(unsigned int id); + - assign a specific hwspinlock id and return its address, or NULL + if that hwspinlock is already in use. Usually board code will + be calling this function in order to reserve specific hwspinlock + ids for predefined purposes. + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + int hwspin_lock_free(struct hwspinlock *hwlock); + - free a previously-assigned hwspinlock; returns 0 on success, or an + appropriate error code on failure (e.g. -EINVAL if the hwspinlock + is already free). + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + int hwspin_lock_timeout(struct hwspinlock *hwlock, unsigned long timeout); + - lock a previously-assigned hwspinlock with a timeout limit (specified in + jiffies). If the hwspinlock is already taken, the function will busy loop + waiting for it to be released, but give up when the timeout meets jiffies. + If timeout is 0, the function will never give up (therefore if a faulty + remote core never releases the hwspinlock, it will deadlock). + Upon a successful return from this function, preemption is disabled so + the caller must not sleep, and is advised to release the hwspinlock as + soon as possible, in order to minimize remote cores polling on the + hardware interconnect. + Returns 0
[PATCH v3 3/4] OMAP4: hwmod data: Add hwspinlock
From: Benoit Cousson b-cous...@ti.com
Add hwspinlock hwmod data for OMAP4 chip
Signed-off-by: Cousson, Benoit b-cous...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Paul Walmsley p...@pwsan.com
---
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 64
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index d258936..e577d54 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -1376,6 +1376,67 @@ static struct omap_hwmod omap44xx_gpio6_hwmod = {
.slaves_cnt = ARRAY_SIZE(omap44xx_gpio6_slaves),
.omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
};
+
+/*
+ * 'spinlock' class
+ * spinlock provides hardware assistance for synchronizing the processes
+ * running on multiple processors
+ */
+
+static struct omap_hwmod_class_sysconfig omap44xx_spinlock_sysc = {
+ .rev_offs = 0x,
+ .sysc_offs = 0x0010,
+ .syss_offs = 0x0014,
+ .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_CLOCKACTIVITY |
+ SYSC_HAS_ENAWAKEUP | SYSC_HAS_SIDLEMODE |
+ SYSC_HAS_SOFTRESET | SYSS_HAS_RESET_STATUS),
+ .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+ .sysc_fields= omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap44xx_spinlock_hwmod_class = {
+ .name = spinlock,
+ .sysc = omap44xx_spinlock_sysc,
+};
+
+/* spinlock */
+static struct omap_hwmod omap44xx_spinlock_hwmod;
+static struct omap_hwmod_addr_space omap44xx_spinlock_addrs[] = {
+ {
+ .pa_start = 0x4a0f6000,
+ .pa_end = 0x4a0f6fff,
+ .flags = ADDR_TYPE_RT
+ },
+};
+
+/* l4_cfg - spinlock */
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__spinlock = {
+ .master = omap44xx_l4_cfg_hwmod,
+ .slave = omap44xx_spinlock_hwmod,
+ .clk= l4_div_ck,
+ .addr = omap44xx_spinlock_addrs,
+ .addr_cnt = ARRAY_SIZE(omap44xx_spinlock_addrs),
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* spinlock slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_spinlock_slaves[] = {
+ omap44xx_l4_cfg__spinlock,
+};
+
+static struct omap_hwmod omap44xx_spinlock_hwmod = {
+ .name = spinlock,
+ .class = omap44xx_spinlock_hwmod_class,
+ .prcm = {
+ .omap4 = {
+ .clkctrl_reg = OMAP4430_CM_L4CFG_HW_SEM_CLKCTRL,
+ },
+ },
+ .slaves = omap44xx_spinlock_slaves,
+ .slaves_cnt = ARRAY_SIZE(omap44xx_spinlock_slaves),
+ .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
/* dmm class */
omap44xx_dmm_hwmod,
@@ -1418,6 +1479,9 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
omap44xx_uart2_hwmod,
omap44xx_uart3_hwmod,
omap44xx_uart4_hwmod,
+
+ /* spinlock class */
+ omap44xx_spinlock_hwmod,
NULL,
};
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] drivers: hwspinlock: add OMAP implementation
From: Simon Que s...@ti.com
Add hwspinlock support for the OMAP4 Hardware Spinlock device.
The Hardware Spinlock device on OMAP4 provides hardware assistance
for synchronization between the multiple processors in the system
(dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP).
[o...@wizery.com: adapt to hwspinlock framework, tidy up]
Signed-off-by: Simon Que s...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Krishnamoorthy, Balaji T balaj...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Tony Lindgren t...@atomide.com
---
drivers/hwspinlock/Kconfig |9 ++
drivers/hwspinlock/Makefile |1 +
drivers/hwspinlock/omap_hwspinlock.c | 231 ++
3 files changed, 241 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwspinlock/omap_hwspinlock.c
diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index 9dd8db4..eb4af28 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -11,3 +11,12 @@ config HWSPINLOCK
coprocessors).
If unsure, say N.
+
+config HWSPINLOCK_OMAP
+ tristate OMAP Hardware Spinlock device
+ depends on HWSPINLOCK ARCH_OMAP4
+ help
+ Say y here to support the OMAP Hardware Spinlock device (firstly
+ introduced in OMAP4).
+
+ If unsure, say N.
diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile
index b9d2b9f..5729a3f 100644
--- a/drivers/hwspinlock/Makefile
+++ b/drivers/hwspinlock/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_HWSPINLOCK) += hwspinlock_core.o
+obj-$(CONFIG_HWSPINLOCK_OMAP) += omap_hwspinlock.o
diff --git a/drivers/hwspinlock/omap_hwspinlock.c
b/drivers/hwspinlock/omap_hwspinlock.c
new file mode 100644
index 000..b5867e1
--- /dev/null
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -0,0 +1,231 @@
+/*
+ * OMAP hardware spinlock driver
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Contact: Simon Que s...@ti.com
+ * Hari Kanigeri h-kanige...@ti.com
+ * Ohad Ben-Cohen o...@wizery.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/device.h
+#include linux/delay.h
+#include linux/io.h
+#include linux/log2.h
+#include linux/pm_runtime.h
+#include linux/slab.h
+#include linux/spinlock.h
+#include linux/hwspinlock.h
+#include linux/platform_device.h
+
+#include hwspinlock.h
+
+/* Spinlock register offsets */
+#define SYSSTATUS_OFFSET 0x0014
+#define LOCK_BASE_OFFSET 0x0800
+
+#define SPINLOCK_NUMLOCKS_BIT_OFFSET (24)
+
+/* Possible values of SPINLOCK_LOCK_REG */
+#define SPINLOCK_NOTTAKEN (0) /* free */
+#define SPINLOCK_TAKEN (1) /* locked */
+
+#define to_omap_hwspinlock(lock) \
+ container_of(lock, struct omap_hwspinlock, lock)
+
+struct omap_hwspinlock {
+ struct hwspinlock lock;
+ void __iomem *addr;
+};
+
+struct omap_hwspinlock_state {
+ int num_locks; /* Total number of locks in system */
+ void __iomem *io_base; /* Mapped base address */
+};
+
+static int omap_hwspinlock_trylock(struct hwspinlock *lock)
+{
+ struct omap_hwspinlock *omap_lock = to_omap_hwspinlock(lock);
+
+ /* attempt to acquire the lock by reading its value */
+ return (SPINLOCK_NOTTAKEN == readl(omap_lock-addr));
+}
+
+static void omap_hwspinlock_unlock(struct hwspinlock *lock)
+{
+ struct omap_hwspinlock *omap_lock = to_omap_hwspinlock(lock);
+
+ /* release the lock by writing 0 to it */
+ writel(SPINLOCK_NOTTAKEN, omap_lock-addr);
+}
+
+/*
+ * relax the OMAP interconnect while spinning on it.
+ *
+ * The specs recommended that the retry delay time will be
+ * just over half of the time that a requester would be
+ * expected to hold the lock.
+ *
+ * The number below is taken from an hardware specs example,
+ * obviously it is somewhat arbitrary.
+ */
+static void omap_hwspinlock_relax(struct hwspinlock *lock)
+{
+ ndelay(50);
+}
+
+static const struct hwspinlock_ops omap_hwspinlock_ops = {
+ .trylock = omap_hwspinlock_trylock,
+ .unlock = omap_hwspinlock_unlock,
+ .relax = omap_hwspinlock_relax,
+};
+
+static int __devinit omap_hwspinlock_probe(struct platform_device *pdev)
+{
+ struct omap_hwspinlock *omap_lock;
+ struct
[PATCH v3 4/4] omap: add hwspinlock device
From: Simon Que s...@ti.com
Build and register an hwspinlock platform device.
Although only OMAP4 supports the hardware spinlock module (for now),
it is still safe to run this initcall on all omaps, because hwmod lookup
will simply fail on hwspinlock-less platforms.
Signed-off-by: Simon Que s...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Paul Walmsley p...@pwsan.com
---
arch/arm/mach-omap2/Makefile |1 +
arch/arm/mach-omap2/hwspinlock.c | 63 ++
2 files changed, 64 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/hwspinlock.c
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index fbc8739..0ea2df3 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -197,3 +197,4 @@ obj-y += $(smc91x-m)
$(smc91x-y)
smsc911x-$(CONFIG_SMSC911X):= gpmc-smsc911x.o
obj-y += $(smsc911x-m) $(smsc911x-y)
+obj-$(CONFIG_ARCH_OMAP4) += hwspinlock.o
diff --git a/arch/arm/mach-omap2/hwspinlock.c b/arch/arm/mach-omap2/hwspinlock.c
new file mode 100644
index 000..06d4a80
--- /dev/null
+++ b/arch/arm/mach-omap2/hwspinlock.c
@@ -0,0 +1,63 @@
+/*
+ * OMAP hardware spinlock device initialization
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Contact: Simon Que s...@ti.com
+ * Hari Kanigeri h-kanige...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/init.h
+#include linux/err.h
+
+#include plat/omap_hwmod.h
+#include plat/omap_device.h
+
+struct omap_device_pm_latency omap_spinlock_latency[] = {
+ {
+ .deactivate_func = omap_device_idle_hwmods,
+ .activate_func = omap_device_enable_hwmods,
+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+ }
+};
+
+int __init hwspinlocks_init(void)
+{
+ int retval = 0;
+ struct omap_hwmod *oh;
+ struct omap_device *od;
+ const char *oh_name = spinlock;
+ const char *dev_name = omap_hwspinlock;
+
+ /*
+* Hwmod lookup will fail in case our platform doesn't support the
+* hardware spinlock module, so it is safe to run this initcall
+* on all omaps
+*/
+ oh = omap_hwmod_lookup(oh_name);
+ if (oh == NULL)
+ return -EINVAL;
+
+ od = omap_device_build(dev_name, 0, oh, NULL, 0,
+ omap_spinlock_latency,
+ ARRAY_SIZE(omap_spinlock_latency), false);
+ if (IS_ERR(od)) {
+ pr_err(Can't build omap_device for %s:%s\n, dev_name,
+ oh_name);
+ retval = PTR_ERR(od);
+ }
+
+ return retval;
+}
+/* early board code might need to reserve specific hwspinlock instances */
+postcore_initcall(hwspinlocks_init);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
On Sat, Dec 4, 2010 at 2:29 AM, David Daney dda...@caviumnetworks.com wrote: Does anything other than OMAP4 have one of these things? I'm not aware of any, but David Brownell had some ideas about it in our previous v2 discussion (see http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39413.html). Btw, you might want to read throughout that entire discussion with David, since it was touching the same questions you raise here. And are there any devices that are commonly encountered across more than one platform that might have these hardware spinlocks, thus making it advantageous to have a common framework? Such devices are just multiple processors that have no alternative mechanism to accomplish synchronization. OMAP4 has a few non-coherent heterogeneous processors that do not support fancy synchronization primitives, so it needs this hwspinlock peripheral. Otherwise, I don't know how common that is (/might become), and I'd hate speculating, but I suspect that OMAP is not going to be the only platform with multiple coprocessors, that have no other means of achieving synchronization, and with which inter-processor communications is desired. If not why a general purpose framework for a very chip specific feature? We started out with an omap-specific driver (see first iteration at http://lwn.net/Articles/410375/), but were asked to make it a platform-agnostic framework, in order to keep the IPC drivers that will use it generic (and assuming that more users will show up for such framework). To me it sounds reasonable, but again, I prefer not to speculate how many users will show up for this, if any. Both ways (framework / omap-specific driver) will work for us just fine. There are chips with hardware atomic memory, but the only time it makes sense to use it is in conjunction with specialize on-chip devices. So the implementation details are kept in the drivers rather than creating a general purpose hwatomic (with its hwatomic_add() et al.) type. This case is a bit different IMHO: some of the potential users of hwspinlocks are just generic IPC drivers that are by no means platform-specific. It's not a special on-chip device: it's just one processor, trying to achieve mutual exclusion with another processor, before manipulating some shared data structure. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] add BUG_ON_MAPPABLE_NULL macro
Introduce BUG_ON_MAPPABLE_NULL in order to eliminate redundant BUG_ON
code, checking for NULL addresses, on architectures where the zero
address can never be mapped.
Originally proposed by Russell King li...@arm.linux.org.uk
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
Compile tested on ARM and x86-64.
Relevant threads:
1. Original proposal by Russell - http://lkml.org/lkml/2010/1/21/238
2. Recent discussion - https://lkml.org/lkml/2010/11/26/78
Notes:
* Implementation still feels hacky, especially since we don't care about the
len param of arch_mmap_check.
* Just like BUG_ON, this new macro is compiled out on !CONFIG_BUG. We might
want to add a CONFIG_BUG commentary, so users will at least be aware of the
security implications of compiling this out.
* To get an (extremely!) rough upper bound of the profit of this macro, I did:
1. find . -name '*.[ch]' | xargs sed -i 's/BUG_ON(!/BUG_ON_MAPPABLE_NULL(!/'
2. removed some obviously bogus sed hits
With omap2plus_defconfig, uImage shrank by ~4Kb (obviously this doesn't include
the potential gain in modules)
arch/arm/include/asm/mman.h |2 +
include/asm-generic/bug.h | 46 +++
2 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/mman.h b/arch/arm/include/asm/mman.h
index 41f99c5..f0c5d58 100644
--- a/arch/arm/include/asm/mman.h
+++ b/arch/arm/include/asm/mman.h
@@ -1,4 +1,6 @@
#include asm-generic/mman.h
+#include asm/pgtable.h
+#include asm/errno.h
#define arch_mmap_check(addr, len, flags) \
(((flags) MAP_FIXED (addr) FIRST_USER_ADDRESS) ? -EINVAL : 0)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index c2c9ba0..0171a30 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -2,6 +2,11 @@
#define _ASM_GENERIC_BUG_H
#include linux/compiler.h
+#include asm/mman.h
+
+#ifndef arch_mmap_check
+#define arch_mmap_check(addr, len, flags) (0)
+#endif
#ifdef CONFIG_BUG
@@ -53,6 +58,47 @@ struct bug_entry {
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
#endif
+/**
+ * BUG_ON_MAPPABLE_NULL() - BUG_ON(condition) only if address 0 is mappable
+ * @condition: condition to check, should contain a NULL check
+ *
+ * In general, NULL dereference Oopses are not desirable, since they take down
+ * the system with them and make the user extremely unhappy. So as a general
+ * rule drivers should avoid dereferencing NULL pointers by doing a simple
+ * check (when appropriate), and just return an error rather than crash.
+ * This way the system, despite having reduced functionality, will just keep
+ * running rather than immediately reboot.
+ *
+ * _Critical_ kernel code, OTOH, that should not (/cannot) keep running when
+ * given an unexpected NULL pointer, should just crash. On some architectures,
+ * a NULL dereference will always reliably produce an Oops. On others, where
+ * the zero address can be mmapped, an Oops is not guaranteed. Relying on
+ * NULL dereference Oopses to happen on these architectures might lead to
+ * data corruptions (system will keep running despite a critical bug and
+ * the results will be horribly undefined). In addition, these situations
+ * can also have security implications - we have seen several privilege
+ * escalation exploits with which an attacker gained full control over the
+ * system due to NULL dereference bugs.
+ *
+ * This macro will BUG_ON if @condition is true on architectures where the zero
+ * address can be mapped. On other architectures, where the zero address
+ * can never be mapped, this macro is compiled out. It only makes sense to
+ * use this macro if @condition contains a NULL check, in order to optimize
that
+ * check out on architectures where the zero address can never be mapped.
+ * On such architectures, those checks are not necessary, since the code
+ * itself will reliably reproduce an Oops as soon as the NULL address will
+ * be dereferenced.
+ *
+ * As with BUG_ON, use this macro only if @condition cannot be tolerated.
+ * If proceeding with degraded functionality is an option, it's much
+ * better to just simply check for @condition and return some error code rather
+ * than crash the system.
+ */
+#define BUG_ON_MAPPABLE_NULL(cond) do { \
+ if (arch_mmap_check(0, 1, MAP_FIXED) == 0) \
+ BUG_ON(cond); \
+} while (0)
+
/*
* WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report
* significant issues that need prompt attention if they should ever
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] add BUG_ON_MAPPABLE_NULL macro
On Wed, Dec 8, 2010 at 2:10 AM, Andrew Morton a...@linux-foundation.org wrote:
Every time someone sends me a patch with text after the ---, I decide
it was good changelog material and I promote it to above the ---.
How's about you save us the effort :)
sure :)
+/**
+ * BUG_ON_MAPPABLE_NULL() - BUG_ON(condition) only if address 0 is mappable
+ * @condition: condition to check, should contain a NULL check
+ *
+ * In general, NULL dereference Oopses are not desirable, since they take
down
+ * the system with them and make the user extremely unhappy. So as a general
+ * rule drivers should avoid dereferencing NULL pointers by doing a simple
s/drivers/code/
definitely.
+ * check (when appropriate), and just return an error rather than crash.
+ * This way the system, despite having reduced functionality, will just keep
+ * running rather than immediately reboot.
+ *
+ * _Critical_ kernel code, OTOH, that should not (/cannot) keep running when
+ * given an unexpected NULL pointer, should just crash. On some
architectures,
+ * a NULL dereference will always reliably produce an Oops. On others, where
+ * the zero address can be mmapped, an Oops is not guaranteed. Relying on
+ * NULL dereference Oopses to happen on these architectures might lead to
+ * data corruptions (system will keep running despite a critical bug and
+ * the results will be horribly undefined). In addition, these situations
+ * can also have security implications - we have seen several privilege
+ * escalation exploits with which an attacker gained full control over the
+ * system due to NULL dereference bugs.
yup.
+ * This macro will BUG_ON if @condition is true on architectures where the
zero
+ * address can be mapped. On other architectures, where the zero address
+ * can never be mapped, this macro is compiled out. It only makes sense to
+ * use this macro if @condition contains a NULL check, in order to optimize
that
+ * check out on architectures where the zero address can never be mapped.
+ * On such architectures, those checks are not necessary, since the code
+ * itself will reliably reproduce an Oops as soon as the NULL address will
+ * be dereferenced.
+ *
+ * As with BUG_ON, use this macro only if @condition cannot be tolerated.
+ * If proceeding with degraded functionality is an option, it's much
+ * better to just simply check for @condition and return some error code
rather
+ * than crash the system.
+ */
+#define BUG_ON_MAPPABLE_NULL(cond) do { \
+ if (arch_mmap_check(0, 1, MAP_FIXED) == 0) \
+ BUG_ON(cond); \
+} while (0)
- arch_mmap_check() didn't have any documentation. Please fix?
Sure.
- arch_mmap_check() is a pretty poor identifier (identifiers which
include the word check are usually poor ones). Maybe
arch_address_accessible()?
Maybe arch_address_mappable() might be more accurate (an address might
be accessible but not mappable due to some other arch-specific usage)
?
But, do you think it makes sense to rename arch_mmap_check() without
adding any functional benefit to it ?
arch_mmap_check is already defined today for ARM, IA64, MN10300, S390
and SPARC, and used by get_unmapped_area() [mmap.c], and this patch
adds a second usage of it.
To rename it, the patch will have to touch all of those architectures,
and usually, patches that carry stylistic changes in areas which they
don't add any functional benefit to, are considered pretty annoying...
- I worry about arch_mmap_check(). Is it expected that it will
perform a runtime check, like probe_kernel_address()? Spell out the
expectations, please.
Yes, arch_mmap_check does perform a runtime check: get_unmapped_area()
calls it with its addr, len and flags parameters.
But, since BUG_ON_MAPPABLE_NULL() calls it with constant values, I
expect arch_mmap_check() to be compiled out (tested on ARM and
X86_64).
- BUG_ON_MAPPABLE_NULL() will evaluate arch_mmap_check() even if
CONFIG_BUG=n. Seems bad, depending on what those unspelled-out
expectations are!
Definitely. I'll add:
#define BUG_ON_MAPPABLE_NULL(condition) do { if (condition) ; } while(0)
in case CONFIG_BUG=n.
- BUG_ON_MAPPABLE_NULL() is a mouthful. I can't immedaitely think of
anythinjg nicer :(
Heartily agree...
- Is BUG_ON_MAPPABLE_NULL() really the interface you want? Or do we
just want an interface which checks a pointer for nearly-nullness?
I guess we can start with BUG_ON_MAPPABLE_NULL(), since this is what
people commonly check today, and we will now be able to optimize those
checks out when they are unnecessary.
Thanks a lot,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mmc1: error -110 whilst initialising SDIO card
On Sat, Dec 11, 2010 at 9:59 PM, Elvis Dowson elvis.dow...@mac.com wrote: mmc1: card claims to support voltages below the defined range. These will be ignored. This is expected - not an error. When I build the TI WiLink drivers from the ti rowboat hardware/ti/wlan project The linux-omap community can help you with mainline drivers, but no so much with proprietary ones. Why don't you use the upstream wl12xx driver ? [ 250.135864] mmc1: error -110 whilst initialising SDIO card How can I troubleshoot this issue? Try to find what triggers this error. Start with mmc_attach_sdio() - it should be around there. If you ask me, I'd start with making sure that all the jumpers on your Murata module are set correctly. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Inconsistent lock state caused by omap_mbox_msg_send() called from tidspbridge
Hi Laurent,
On Sun, Dec 12, 2010 at 2:44 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com [ 191.085479] {IN-SOFTIRQ-W}
state was registered at:
[ 191.090576] [c008e914] __lock_acquire+0x5f0/0x17c4
[ 191.095947] [c008fbc0] lock_acquire+0xd8/0xfc
[ 191.100860] [c0373d3c] _raw_spin_lock+0x30/0x40
[ 191.105957] [bf000744] omap_mbox_msg_send+0x18/0xa4 [mailbox]
[ 191.112335] [bf014a08] sm_interrupt_dsp+0xe0/0x114 [bridgedriver]
[ 191.119201] [bf0122d4] io_dpc+0x364/0x6c4 [bridgedriver]
[ 191.125152] [c0067294] tasklet_action+0x70/0x100
[ 191.130371] [c0067974] __do_softirq+0xc4/0x1b4
[ 191.135375] [c0067b44] do_softirq+0x44/0x68
[ 191.140136] [c0067bcc] run_ksoftirqd+0x64/0x10c
[ 191.145233] [c007c3a0] kthread+0x84/0x8c
[ 191.149688] [c003aad0] kernel_thread_exit+0x0/0x8
[ 191.154968] irq event stamp: 3602
[ 191.158447] hardirqs last enabled at (3602): [c0067dd0]
local_bh_enable_ip+0xe0/0xf4
[ 191.166809] hardirqs last disabled at (3600): [c0067d50]
local_bh_enable_ip+0x60/0xf4
[ 191.175170] softirqs last enabled at (3601): [bf00f848]
bridge_chnl_add_io_req+0x2f4/0x348 [bridgedriver]
[ 191.185485] softirqs last disabled at (3599): [c0373f24]
_raw_spin_lock_bh+0x14/0x4c
[ 191.193756]
[ 191.193756] other info that might help us debug this:
[ 191.200592] 1 lock held by dummy/1544:
[ 191.204498] #0: (node_mgr_obj-node_mgr_lock){+.+.+.}, at: [bf025da0]
node_create+0x88/0x28c [bridgedriver]
[ 191.215240]
[ 191.215240] stack backtrace:
[ 191.219818] [c003eca4] (unwind_backtrace+0x0/0xec) from [c008caf4]
(print_usage_bug+0x170/0x1b4)
[ 191.229370] [c008caf4] (print_usage_bug+0x170/0x1b4) from [c008ce90]
(mark_lock+0x358/0x628)
[ 191.238555] [c008ce90] (mark_lock+0x358/0x628) from [c008e9a0]
(__lock_acquire+0x67c/0x17c4)
[ 191.247741] [c008e9a0] (__lock_acquire+0x67c/0x17c4) from [c008fbc0]
(lock_acquire+0xd8/0xfc)
[ 191.257019] [c008fbc0] (lock_acquire+0xd8/0xfc) from [c0373d3c]
(_raw_spin_lock+0x30/0x40)
[ 191.266021] [c0373d3c] (_raw_spin_lock+0x30/0x40) from [bf000744]
(omap_mbox_msg_send+0x18/0xa4 [mailbox])
[ 191.276519] [bf000744] (omap_mbox_msg_send+0x18/0xa4 [mailbox]) from
[bf014a08] (sm_interrupt_dsp+0xe0/0x114 [bridgedriver])
[ 191.288696] [bf014a08] (sm_interrupt_dsp+0xe0/0x114 [bridgedriver]) from
[bf00f85c] (bridge_chnl_add_io_req+0x308/0x348 [bridgedriver])
[ 191.301910] [bf00f85c] (bridge_chnl_add_io_req+0x308/0x348
[bridgedriver]) from [bf021694] (send_message+0x14c/0x164 [bridgedriver])
[ 191.314880] [bf021694] (send_message+0x14c/0x164 [bridgedriver]) from
[bf021f18] (disp_node_create+0x4c8/0x508 [bridgedriver])
[ 191.327301] [bf021f18] (disp_node_create+0x4c8/0x508 [bridgedriver]) from
[bf025ea8] (node_create+0x190/0x28c [bridgedriver])
[ 191.339630] [bf025ea8] (node_create+0x190/0x28c [bridgedriver]) from
[bf019018] (api_call_dev_ioctl+0xf0/0x118 [bridgedriver])
[ 191.352081] [bf019018] (api_call_dev_ioctl+0xf0/0x118 [bridgedriver])
from [bf02e0dc] (bridge_ioctl+0x148/0x17c [bridgedriver])
[ 191.364532] [bf02e0dc] (bridge_ioctl+0x148/0x17c [bridgedriver]) from
[c00f5870] (vfs_ioctl+0x20/0x3c)
[ 191.374633] [c00f5870] (vfs_ioctl+0x20/0x3c) from [c00f5f5c]
(do_vfs_ioctl+0x4fc/0x544)
[ 191.383361] [c00f5f5c] (do_vfs_ioctl+0x4fc/0x544) from [c00f5ff0]
(sys_ioctl+0x4c/0x6c)
[ 191.392089] [c00f5ff0] (sys_ioctl+0x4c/0x6c) from [c003a0c0]
(ret_fast_syscall+0x0/0x3c)
Googling for a solution returned a pretty long mail thread (http://www.mail-
archive.com/linux-omap@vger.kernel.org/msg30492.html)
Yeah, the problem was (is??) that dspbridge uses mailbox from both
process context and softirq context (its tasklet).
and a dspbridge patch
(https://patchwork.kernel.org/patch/107522/) part of a bigger patch set.
That code doesn't seem to have hit mainline. Do you have a status update on
this ?
Big parts of it are being pursued by Hari nowadays, but anyway they
are not relevant to the lockdep problem, so don't wait for it...
There are two possible solutions to the lockdep problem:
1. Have dspbridge defer messages to a workqueue instead of a tasklet
or
2. Add sufficient locking into mailbox that would allow softirq usage too
I recommended to follow (1), but that needs verification that
performance isn't hurt (it shouldn't since that context is anyway
meant to defer messages in case the bridge is still waiting for
previous ones to be processed by the dsp).
But otherwise (2) will work too (see proposal by Deepak in the
original bug report).
Looping in Omar, Fernando, Felipe, Deepak, Hari,...
Thanks,
Ohad.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Inconsistent lock state caused by omap_mbox_msg_send() called from tidspbridge
On Sun, Dec 12, 2010 at 4:15 PM, Ionut Nicu ionut.n...@mindbit.ro wrote: I noticed this too, but this patch should fix it: https://patchwork.kernel.org/patch/365292/ True. And in this patch the move to spin_lock_bh() is justifiable, too, since it adds a sending path which is parallel to the mailbox tasklet. Ionut. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Inconsistent lock state caused by omap_mbox_msg_send() called from tidspbridge
On Mon, Dec 13, 2010 at 6:45 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com On Sun, Dec 12, 2010 at 4:15 PM, Ionut Nicu ionut.n...@mindbit.ro wrote: I noticed this too, but this patch should fix it: https://patchwork.kernel.org/patch/365292/ True. And in this patch the move to spin_lock_bh() is justifiable, too, since it adds a sending path which is parallel to the mailbox tasklet. Is this patch set ready for inclusion in the mainline kernel, or does it need to be reworked ? Better let Hari answer this one. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
Hi Greg, Tony, On Sat, Dec 4, 2010 at 1:50 AM, Ohad Ben-Cohen o...@wizery.com wrote: OMAP4 introduces a Hardware Spinlock device, which provides hardware assistance for synchronization and mutual exclusion between heterogeneous processors and those not operating under a single, shared operating system (e.g. OMAP4 has dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP). The intention of this hardware device is to allow remote processors, that have no alternative mechanism to accomplish synchronization and mutual exclusion operations, to share resources (such as memory and/or any other hardware resource). This patchset adds hwspinlock framework that makes it possible for drivers to use those hwspinlock devices and stay platform-independent. ... Documentation/hwspinlock.txt | 299 +++ arch/arm/mach-omap2/Makefile | 1 + arch/arm/mach-omap2/hwspinlock.c | 63 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 64 drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/hwspinlock/Kconfig | 22 ++ drivers/hwspinlock/Makefile | 6 + drivers/hwspinlock/hwspinlock.h | 61 +++ drivers/hwspinlock/hwspinlock_core.c | 557 drivers/hwspinlock/omap_hwspinlock.c | 231 include/linux/hwspinlock.h | 298 +++ 12 files changed, 1605 insertions(+), 0 deletions(-) Can you please have a look and say if this looks OK ? If it does, where would you prefer this to go through ? Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
On Tue, Dec 14, 2010 at 7:06 PM, Greg KH g...@kroah.com wrote: Can you please have a look and say if this looks OK ? Look at what, I don't see a patch here. Here's the complete patchset: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39833.html If you prefer, I can resubmit. I've seen a lot of discussion about this, are all of the review comments now addressed? Yes, all comments were addressed in this v3 iteration, and this thread has been idle for about 10 days. Like the most important one, why is this generic code if it's only for one specific platform? We started out with an omap-specific driver, but Tony preferred that we make this a platform-agnostic framework, in order to keep the IPC drivers that will use it generic, and assuming that more users will show up for such framework. To me it sounds reasonable, but both ways (framework / omap-specific driver) will work for us just fine, and I can switch back to a misc driver if this is preferred. The complete discussion of v1 is at: http://comments.gmane.org/gmane.linux.kernel/1049802 We also discussed this at v2 of the patches with David, see the complete discussion at: http://comments.gmane.org/gmane.linux.kernel/1067016 Thanks, Ohad. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: boards w/ wl12xx should select REGULATOR_FIXED_VOLTAGE
Hi Tony, On Wed, Nov 24, 2010 at 12:04 PM, Ohad Ben-Cohen o...@wizery.com wrote: Power to the wl12xx wlan device is controlled by a fixed regulator. Boards that have the wl12xx should select REGULATOR_FIXED_VOLTAGE so users will not be baffled. Not sure if you picked this up ? We need it because otherwise new wl12xx users can spend many hours until they figure out what's the problem.. Please tell me if there's any problem with this one. Thanks, Ohad. Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- arch/arm/mach-omap2/Kconfig | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index fc3a181..cc386da 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -190,6 +190,7 @@ config MACH_OMAP3_PANDORA depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB + select REGULATOR_FIXED_VOLTAGE config MACH_OMAP3_TOUCHBOOK bool OMAP3 Touch Book @@ -235,6 +236,7 @@ config MACH_OMAP_ZOOM2 select SERIAL_8250 select SERIAL_CORE_CONSOLE select SERIAL_8250_CONSOLE + select REGULATOR_FIXED_VOLTAGE config MACH_OMAP_ZOOM3 bool OMAP3630 Zoom3 board @@ -244,6 +246,7 @@ config MACH_OMAP_ZOOM3 select SERIAL_8250 select SERIAL_CORE_CONSOLE select SERIAL_8250_CONSOLE + select REGULATOR_FIXED_VOLTAGE config MACH_CM_T35 bool CompuLab CM-T35 module -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
On Tue, Dec 14, 2010 at 8:40 PM, Ohad Ben-Cohen o...@wizery.com wrote: On Tue, Dec 14, 2010 at 7:06 PM, Greg KH g...@kroah.com wrote: Like the most important one, why is this generic code if it's only for one specific platform? We started out with an omap-specific driver, but Tony preferred that we make this a platform-agnostic framework, in order to keep the IPC drivers that will use it generic, and assuming that more users will show up for such framework. I was just told about additional users for this (thanks Mugdha): 1) There are several platforms (such as omap3530 and omapl1xx) that have no such hardware support, but would still need to use the same IPC drivers (to communicate with their DSP). The only way to achieve mutual exclusion on those platforms is by using a shared-memory synchronization algorithm called Peterson's Algorithm [1]. We would still need the same hwspinlock framework for that - the busy looping, the timeout, the various locking schemes, the resource allocation - are all still valid. The only difference is the actual lock implementation. 2) The C6474, which is a multi-core DSP device [2], have Linux running on one of its cores, and would be using the same IPC drivers, too. C6474 has hardware support for synchronization, which would also benefit from such hwspinlock module (btw the C6474 has a richer hardware module that would need more than the hwspinlock framework offer today - it also supports queuing, owner semantics and interrupt notification to let a processor know when it acquires a lock, so it wouldn't have to spin..) Thanks, Ohad. [1] http://en.wikipedia.org/wiki/Peterson's_algorithm [2] http://focus.ti.com/docs/prod/folders/print/tms320c6474.html To me it sounds reasonable, but both ways (framework / omap-specific driver) will work for us just fine, and I can switch back to a misc driver if this is preferred. The complete discussion of v1 is at: http://comments.gmane.org/gmane.linux.kernel/1049802 We also discussed this at v2 of the patches with David, see the complete discussion at: http://comments.gmane.org/gmane.linux.kernel/1067016 Thanks, Ohad. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
On Thu, Dec 16, 2010 at 11:08 PM, Greg KH g...@kroah.com wrote: I've seen a lot of discussion about this, are all of the review comments now addressed? Yes, all comments were addressed in this v3 iteration, and this thread has been idle for about 10 days. That's because we are all busy with other things :( Oh sure, my intention was only positive (to demonstrate that there are no outstanding comments), sorry if it sounded differently than intended. Sorry, I really don't have the time at the moment to review this code, nor does it seem to affect areas that I maintain, so I don't understand what you are looking for from me. Sorry for the noise - I contacted you because you maintain the driver core, in the hope you could suggest where this can go through. Tony, Andrew, can you please have a look ? Any comment or suggestion is appreciated. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
Hi Felipe,
On Mon, Dec 20, 2010 at 8:43 PM, Felipe Contreras
felipe.contre...@nokia.com wrote:
We need to protect not only the dmm_map list, but the individual
map_obj's, otherwise, we might be building the scatter-gather list with
garbage.
Thanks for reporting this !
So, use the existing proc_lock for that.
I don't like this.
You are taking this lock over the entire proc_begin_dma() and
proc_end_dma() functions, and by that artificially imposing sequential
behavior of their execution (system-wide).
I strongly prefer protecting the dmm map object itself, and not the
code that uses it, in order to avoid redundant bottlenecks.
Something like the patch below should fix the panics without globally
locking the dma functions (UNTESTED! I still want to have a gst-dsp
setup like yours so I can reproduce the issues you see. If needed,
I'll get to it).
Please note: the code below assumes that applications calls
proc_end_dma() for every proc_begin_dma(). This is anyway mandatory,
because otherwise we are risking data corruptions, but we know older
applications (e.g. probably TI samples) don't do that (since the
dspbridge's DMA API is relatively new). If we want/need to support
those older applications with this patch, it's a 2-liner change.
But there is another issue. I'm not quite sure how gst-dsp is using
the bridge DMA API, but those panics may suggest that there are
additional races here: if the application unmaps a memory region,
before the DMA operation completes (i.e. proc_end_dma() completed
execution), the application will face:
1. errors: when proc_end_dma() does get invoked, the map_obj will
already be destroyed, and find_containing_mapping() will simply fail
2. data corruptions: as a result of (1), dma_unmap_sg() will not be
called, and that may result in data corruptions and generally is very
bad
We can treat this as an application error, but we don't have to, and
it will actually be very clean if we won't.
Given the below patch, it should be quite easy to solve those
additional races too, e.g., by adding a boolean 'dying' flag to each
map_obj, which would (on un_map) mark an object as not available for a
new DMA operation (proc_start_dma), but it'd still wait until all of
its referenced are removed (proc_end_dma), and only then it will be
finally removed from the list.
Such a change will only work with applications that calls
proc_end_dma() for each proc_start_dma(). How is gst-dsp in that
respect ? do we want to keep supporting older applications which don't
do that ? if we want, we can still support both old and new API with
some ioctl games (introduce new ioctl commands which will work the new
way, and keep the old ones around, maybe only mark them as
deprecated).
Thanks,
Ohad.
---
.../staging/tidspbridge/include/dspbridge/drv.h|2 +
drivers/staging/tidspbridge/rmgr/proc.c| 39 ++--
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/tidspbridge/include/dspbridge/drv.h
b/drivers/staging/tidspbridge/include/dspbridge/drv.h
index c1f363e..cad0626 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/drv.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/drv.h
@@ -25,6 +25,7 @@
#include dspbridge/drvdefs.h
#include linux/idr.h
+#include linux/atomic.h
#define DRV_ASSIGN 1
#define DRV_RELEASE0
@@ -106,6 +107,7 @@ struct dmm_map_object {
u32 num_usr_pgs;
struct page **pages;
struct bridge_dma_map_info dma_info;
+ atomic_t refcnt;
};
/* Used for DMM reserved memory accounting */
diff --git a/drivers/staging/tidspbridge/rmgr/proc.c
b/drivers/staging/tidspbridge/rmgr/proc.c
index b47d7aa..4aabfcc 100644
--- a/drivers/staging/tidspbridge/rmgr/proc.c
+++ b/drivers/staging/tidspbridge/rmgr/proc.c
@@ -112,6 +112,26 @@ static s32 get_envp_count(char **envp);
static char **prepend_envp(char **new_envp, char **envp, s32 envp_elems,
s32 cnew_envp, char *sz_var);
+/*
+ * Grab the dmm_map_object's reference count. This operation is valid only
+ * when map_obj is ALREADY grabbed, e.g., it is still in the dmm_map list
+ * and list's lock is taken
+ */
+static inline void map_obj_get(struct dmm_map_object *map_obj)
+{
+ atomic_inc(map_obj-refcnt);
+}
+
+/* Ungrab map_obj and destroy it, if it was the last reference */
+static inline void map_obj_put(struct dmm_map_object *map_obj)
+{
+ if (atomic_dec_and_test(map_obj-refcnt)) {
+ kfree(map_obj-dma_info.sg);
+ kfree(map_obj-pages);
+ kfree(map_obj);
+ }
+}
+
/* remember mapping information */
static struct dmm_map_object *add_mapping_info(struct process_context *pr_ctxt,
u32 mpu_addr, u32 dsp_addr, u32 size)
@@ -143,6 +163,8 @@ static struct dmm_map_object
*add_mapping_info(struct process_context *pr_ctxt,
map_obj-dsp_addr = dsp_addr;
map_obj-size = size;
map_obj-num_usr_pgs =
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
Hi Felipe,
On Tue, Dec 21, 2010 at 6:44 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
Wouldn't you want the proc_*_dma() operations to finish before
unmaping the pages?
Yes, but that's not what your patch is doing exactly: it serializes
the execution of proc_begin_dma(), proc_end_dma(), proc_map() and
proc_un_map() using a single global mutex and by that prevents their
concurrent execution (system-wide).
I understand your intention: you don't want proc_un_map() to kick in
in the middle of a proc_*_dma() operation. That's fine, but the patch
has a side effect, which serializes the DMA operations themselves, and
prevents their concurrent execution (even if they are for separate
memory addresses, or invoked by different processes, etc...).
Looking ahead, this DMM code is going to be extracted into an
independent module, where it will be shared between dspbridge and
syslink (the IPC framework for OMAP4 and forth): both projects use
this DMM concept, and it won't make sense for syslink to duplicate the
code. So even if today's dspbridge use cases doesn't have a lot of
concurrency, and performance is satisfying even in light of your
patch, we still want the code to be ready for more demanding use cases
(several remote processors, several multimedia processes running on
the host, several concurrent multimedia streams, etc...). If we use
coarse-grained locking now, it will just make it harder to remove it
later when scalability issues will show up (see the BKL removal
efforts...).
So I prefer we don't add any locking to the proc_*_dma() API at all.
Instead, let's just take a reference count every time a map_obj is
found (and therefore is about to be used), and release it after it is
used. And now, inside proc_un_map(), if we notice that our map_obj is
still being used, it means the application called us prematurely and
we can't proceed. Something like (revised the patch from my previous
email):
diff --git a/drivers/staging/tidspbridge/include/dspbridge/drv.h
b/drivers/staging/tidspbridge/include/dspbridge/drv.h
index c1f363e..cad0626 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/drv.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/drv.h
@@ -25,6 +25,7 @@
#include dspbridge/drvdefs.h
#include linux/idr.h
+#include linux/atomic.h
#define DRV_ASSIGN 1
#define DRV_RELEASE0
@@ -106,6 +107,7 @@ struct dmm_map_object {
u32 num_usr_pgs;
struct page **pages;
struct bridge_dma_map_info dma_info;
+ atomic_t refcnt;
};
/* Used for DMM reserved memory accounting */
diff --git a/drivers/staging/tidspbridge/rmgr/proc.c
b/drivers/staging/tidspbridge/rmgr/proc.c
index b47d7aa..d060692 100644
--- a/drivers/staging/tidspbridge/rmgr/proc.c
+++ b/drivers/staging/tidspbridge/rmgr/proc.c
@@ -112,9 +112,37 @@ static s32 get_envp_count(char **envp);
static char **prepend_envp(char **new_envp, char **envp, s32 envp_elems,
s32 cnew_envp, char *sz_var);
+/* Increase map_obj's reference count */
+static inline void map_obj_get(struct dmm_map_object *map_obj)
+{
+ atomic_inc(map_obj-refcnt);
+}
+
+/* Decrease map_obj's reference count */
+static inline void map_obj_put(struct dmm_map_object *map_obj)
+{
+ atomic_dec(map_obj-refcnt);
+}
+
+/**
+ * is_map_obj_used() - check out whether a given map_obj is still being used
+ * @map_obj: a dmm map object
+ *
+ * Returns 'true' if a given map_obj is being used, or 'false' otherwise.
+ *
+ * This function must be used while the dmm map list spinlock is taken,
+ * otherwise it is not safe to use its answer (if the list is not locked,
+ * someone can find and start using a map_obj immediately after this functions
+ * replys 'false'.
+ */
+static inline bool is_map_obj_used(struct dmm_map_object *map_obj)
+{
+ return atomic_read(map_obj-refcnt) ? true : false;
+}
+
/* remember mapping information */
-static struct dmm_map_object *add_mapping_info(struct process_context *pr_ctxt,
- u32 mpu_addr, u32 dsp_addr, u32 size)
+static struct dmm_map_object *create_mapping_info(u32 mpu_addr, u32 dsp_addr,
+ u32 size)
{
struct dmm_map_object *map_obj;
@@ -144,11 +172,15 @@ static struct dmm_map_object
*add_mapping_info(struct process_context *pr_ctxt,
map_obj-size = size;
map_obj-num_usr_pgs = num_usr_pgs;
+ return map_obj;
+}
+
+static void add_mapping_info(struct process_context *pr_ctxt,
+ struct dmm_map_object *map_obj)
+{
spin_lock(pr_ctxt-dmm_map_lock);
list_add(map_obj-link, pr_ctxt-dmm_map_list);
spin_unlock(pr_ctxt-dmm_map_lock);
-
- return map_obj;
}
static int match_exact_map_obj(struct dmm_map_object *map_obj,
@@ -162,10 +194,18 @@ static int match_exact_map_obj(struct
dmm_map_object *map_obj,
map_obj-size == size;
}
-static void
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
On Mon, Dec 27, 2010 at 3:55 PM, Felipe Contreras felipe.contre...@nokia.com wrote: So, effectively, serializing the proc_begin_dma() and proc_end_dma() would not affect anyone negatively for the time being. You can never really tell who is using the kernel (or will be using this kernel version), how and under which workload. For the long-term goal I agree with that approach, however, first, I think my patch should be applied, since it's fixing a problem using an already existing and actively excersised mechanism. In fact, I think this should be pushed to 2.6.37 as: 1) prevents a kernel panic 2) the issue is reproducible and clearly identified 3) the patch is small and obvious Both patches are (IMHO). But frankly I don't mind your patch will be applied now as long as it doesn't stay. I can rebase my patch against it after it is applied, and send separately. This approach looks cleaner, however, we need a flag in remove_mapping_information() in order to force the removal, otherwise there will be memory leaks. Imagine a process crashes, and remove_mapping_information() returns -EBUSY. Can't happen; both proc_*_dma() operations decrease the reference count before exiting, so it's not up to the application. Sure, but I see this as a broader effort to have finer locking, part of this should be to remove the already existing proc_lock. Having bad locking is not an excuse for adding more. Anyway, as I said, I don't really mind your patch will be applied as long as it is a temporary workaround. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
On Tue, Dec 28, 2010 at 12:36 PM, Felipe Contreras felipe.contre...@gmail.com wrote: You can never really tell who is using the kernel (or will be using this kernel version), how and under which workload. No, but it's better to address real issues rather than hypothetical. Right. and both patches do that. one adds locks, and one doesn't. However, as I sad, everybody is using proc_map() and proc_un_map() which take a lock, and there are no complaints. I didn't complain about that; I didn't like you adding locks to the DMA API. Ok, can I get your Ack? Frankly, I don't like the locks you are adding. But as I said, I wouldn't resist them as long as it's temporary. Then why did you add that check for is_map_obj_used(), and then return -EBUSY? If that can happen, then it can happen when the application is crashing; user-space crashes while kernel-space is in the middle of a proc_*_dma() operation. I still don't know how exactly you triggered the bug: is gst-dsp multithreaded ? and one of its threads invoked proc_un_map() while another thread called proc_begin_dma() ? Anyhow, a thread that is calling proc_*_dma() will both increase the reference count and decrease it back before going back to user space. Otherwise your patch would be problematic as well - who will unlock the mutex you take in proc_*_dma() ? Sure, but I see this as a broader effort to have finer locking, part of this should be to remove the already existing proc_lock. Having bad locking is not an excuse for adding more. No, but not being a permanent solution is not an excuse for not fixing a kernel panic right away. Right. But we have a fix that doesn't add any additional locking... I don't see why it can't be taken now, but as I said, I wouldn't resist staging it for the next cycle. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
On Tue, Dec 28, 2010 at 2:12 PM, Felipe Contreras felipe.contre...@gmail.com wrote: I haven't investigated why that happens, but kernel-space should not panic regardless of what user-space does. Agree of course. But that's not what we were discussing... Anyhow, a thread that is calling proc_*_dma() will both increase the reference count and decrease it back before going back to user space. Otherwise your patch would be problematic as well - who will unlock the mutex you take in proc_*_dma() ? I'm saying that user-space might crash *before* proc_*_dma() finishes, before the reference count has been decreased. In my patch there would be no issue because proc_un_map() would wait until proc_*_dma() has released the lock. But what will happen if, as you say, user-space would crash before proc_*_dma() has released the lock ? how could proc_un_map() run ? Because: 1) Your patch changes 114 lines, mine 18 2) It hasn't been reviewed, nor tested by other people 3) At least I see a potential issue 4) 2.6.37 is imminent IMO one patch has chances going into 2.6.37, the other one doesn't. I don't see the problem of pushing my patch to 2.6.37, and once your patch has been properly reviewed, and tested, put it for the 2.6.38 merge window. Anyway, it's looking more and more that this patch would not be ack'ed in time, so I guess we would have to wait to see what other people (Omar?) say, which would probably be 2.6.38 timeline. This is all good, and I have no problem with it. As I said, I don't resist your patch as a temporary fix. But it doesn't mean I like it... -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
On Tue, Dec 28, 2010 at 2:24 PM, Felipe Contreras felipe.contre...@gmail.com wrote: user-space crashed, not kernel-space; the code would continue to run and eventually release the lock. So you'll have to be more specific about the scenario you are describing. If there's a user thread that is still running the proc_*_dma() function, and we agree that this thread keeps running until completion and then returns to user space, what's the problem ? If that user thread will crash, drv_remove_all_resources() will clean up all map_obj's. Yeah, so the chances of getting this fixed on 2.6.37 are dimmed. I wouldn't worry about that.. In the worst case, Cc: sta...@kernel.org will push the fix into 2.6.37.x.. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
On Tue, Dec 28, 2010 at 2:12 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, Dec 28, 2010 at 12:56 PM, Ohad Ben-Cohen o...@wizery.com wrote: I still don't know how exactly you triggered the bug: is gst-dsp multithreaded ? and one of its threads invoked proc_un_map() while another thread called proc_begin_dma() ? I haven't investigated why that happens Btw, I still think you should look into this. The kernel panic will be solved, but you may still have a race there that can lead to data corruption: if proc_un_map will be fast enough, it will acquire the proc_lock mutex before proc_begin_dma(), and then you will miss a cache operation. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] staging: tidspbridge: protect dmm_map properly
On Wed, Dec 29, 2010 at 11:39 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, Dec 28, 2010 at 2:37 PM, Ohad Ben-Cohen o...@wizery.com wrote: On Tue, Dec 28, 2010 at 2:24 PM, Felipe Contreras felipe.contre...@gmail.com wrote: user-space crashed, not kernel-space; the code would continue to run and eventually release the lock. So you'll have to be more specific about the scenario you are describing. If there's a user thread that is still running the proc_*_dma() function, and we agree that this thread keeps running until completion and then returns to user space, what's the problem ? The problem is if the user-space process crashes exactly in the middle of it, *before* completing. With locks there's no problem, as proc_un_map() would wait for the lock in my patch. In your patch it would not wait, just return -EBUSY. We have two threads. One called proc_un_map(), and one called proc_begin_dma(). The first crashed, but the second didn't. it still holds the bridge device open. When it will exit, and release the device, then drv_remove_all_resources() will be called, and all the map_obj's will be cleaned. If that user thread will crash, drv_remove_all_resources() will clean up all map_obj's. Not if a proc_*_dma() is still running. It will be called after it will return, and its thread will exit (or crash). -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
Hi Andrew, On Sat, Dec 18, 2010 at 2:53 AM, Tony Lindgren t...@atomide.com wrote: * Ohad Ben-Cohen o...@wizery.com [101216 13:34]: Tony, Andrew, can you please have a look ? Any comment or suggestion is appreciated. Looks sane to me from omap point of view and it seems the locks are now all timeout based: Acked-by: Tony Lindgren t...@atomide.com Can you please have a look at this patch set (see link no. [1] below) ? Short summary: This hwspinlock framework adds support for hardware-based locking devices, needed to accomplish synchronization and mutual exclusion operations between remote processors that have no alternative mechanism to do so. This patch set adds a framework and an OMAP implementation. It had circulated several times in the relevant mailing lists, and all comments were addressed. The third version of this patch set [1] was submitted about a month ago and so far there is no outstanding comment. Users for this framework that we are aware of: 1. OMAP4 and Davinci Netra (DM8168): both of these SoC have the same hwspinlock module and will share the same host implementation. 2. Other platforms (such as omap3530 and omapl1xx) that have no such hardware support, but would still need to achieve synchronization (to communicate with their DSP). The only way to achieve mutual exclusion on those platforms is by using a shared-memory synchronization algorithm called Peterson's Algorithm [2]. We would still need the same hwspinlock framework for that - the busy looping, the timeout, the various locking schemes, the lock resource allocation - are all still valid. The only difference would be the actual lock implementation, therefore we will add another host implementation for these platforms. 3. The C6474, a multi-core DSP device [3], have Linux running on one of its cores, and hardware support for synchronization (btw the C6474 has a richer hardware module that would need more than the hwspinlock framework offer today - it also supports queuing, owner semantics and interrupt notification to let a processor know when it acquires a lock, so it wouldn't have to spin..). Disclaimer: it will probably take some time until c6x support is merged upstream, but this is something that is being actively worked on [4]. Any comment or suggestion is appreciated. Thanks, Ohad. [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg39833.html [2] http://en.wikipedia.org/wiki/Peterson's_algorithm [3] http://focus.ti.com/docs/prod/folders/print/tms320c6474.html [4] http://www.linux-c6x.org/wiki/index.php/Main_Page -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: boards w/ wl12xx should select REGULATOR_FIXED_VOLTAGE
On Wed, Nov 24, 2010 at 12:04 PM, Ohad Ben-Cohen o...@wizery.com wrote: Power to the wl12xx wlan device is controlled by a fixed regulator. Boards that have the wl12xx should select REGULATOR_FIXED_VOLTAGE so users will not be baffled. Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- Hi Tony, Gentle reminder, Thanks, Ohad. arch/arm/mach-omap2/Kconfig | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index fc3a181..cc386da 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -190,6 +190,7 @@ config MACH_OMAP3_PANDORA depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB + select REGULATOR_FIXED_VOLTAGE config MACH_OMAP3_TOUCHBOOK bool OMAP3 Touch Book @@ -235,6 +236,7 @@ config MACH_OMAP_ZOOM2 select SERIAL_8250 select SERIAL_CORE_CONSOLE select SERIAL_8250_CONSOLE + select REGULATOR_FIXED_VOLTAGE config MACH_OMAP_ZOOM3 bool OMAP3630 Zoom3 board @@ -244,6 +246,7 @@ config MACH_OMAP_ZOOM3 select SERIAL_8250 select SERIAL_CORE_CONSOLE select SERIAL_8250_CONSOLE + select REGULATOR_FIXED_VOLTAGE config MACH_CM_T35 bool CompuLab CM-T35 module -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: boards w/ wl12xx should select REGULATOR_FIXED_VOLTAGE
On Tue, Jan 4, 2011 at 9:17 PM, Tony Lindgren t...@atomide.com wrote: * Ohad Ben-Cohen o...@wizery.com [110104 10:04]: On Wed, Nov 24, 2010 at 12:04 PM, Ohad Ben-Cohen o...@wizery.com wrote: Power to the wl12xx wlan device is controlled by a fixed regulator. Boards that have the wl12xx should select REGULATOR_FIXED_VOLTAGE so users will not be baffled. Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- Hi Tony, Gentle reminder, This is queued for 2.6.38 in omap-for-linus as commit 7c50152f0851e44ef7491546a29fddbbea47735b? Thanks! Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
On Tue, Jan 4, 2011 at 10:19 PM, Andrew Morton a...@linux-foundation.org wrote: Acked-by: Tony Lindgren t...@atomide.com Can you please have a look at this patch set (see link no. [1] below) ? I looked - it looks reasonable. This is exactly the wrong time to be looking at large new patchsets - please refresh, retest and resend after -rc1 is released? Sure, thanks ! -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Setting MMC_CAP_POWER_OFF_CARD on mmc2 leads to filesystem problems on mmc1
Hi Koen,
On Fri, Jan 21, 2011 at 4:49 PM, Luciano Coelho coe...@ti.com wrote:
My patch basically does:
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -270,7 +270,7 @@ static struct omap2_hsmmc_info mmc[] = {
{
.name = wl1271,
.mmc = 2,
- .caps = MMC_CAP_4_BIT_DATA,
+ .caps = MMC_CAP_4_BIT_DATA |
MMC_CAP_POWER_OFF_CARD,
And does NOT touch mmc1. But after adding MMC_CAP_POWER_OFF_CARD I get tons
of:
Hmm. The snippet above looks different in your patch.
It seems that you're adding a new mmcbbt array, along with the
existing mmc one, but still using unchanged board-omap3beagle code,
and I suspect you have some unhealthy mmc/mmcbbt references.
Particularly, look at this:
@@ -384,7 +445,14 @@ static int beagle_twl_gpio_setup(struct device *dev,
}
/* gpio + 0 is mmc0_cd (input/IRQ) */
mmc[0].gpio_cd = gpio + 0;
+#if defined(CONFIG_WL1271) || defined(CONFIG_WL1271_MODULE)
+ if(!strcmp(expansionboard_name, fixme-beagletoy))
+ omap2_hsmmc_init(mmcbbt);
+ else
+ omap2_hsmmc_init(mmc);
+#else
omap2_hsmmc_init(mmc);
+#endif
/* link regulators to MMC adapters */
beagle_vmmc1_supply.dev = mmc[0].dev;
When WL1271 is configured, and you have your fixme-beagletoy
expansionboard around, you're only initializing mmcbbt, but still
using mmc for regulators references.
Care to check if fixing that makes your issues go away ?
Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Setting MMC_CAP_POWER_OFF_CARD on mmc2 leads to filesystem problems on mmc1
On Sat, Jan 22, 2011 at 5:30 PM, Koen Kooi k...@dominion.thruhere.net wrote: That was indeed the problem, not I get: [ 35.417053] wl1271: firmware booted (Rev 6.1.0.0.343) and root@beagleboard-xm-next:~# ifconfig wlan0 hw eth 00:11:22:33:44:55 root@beagleboard-xm-next:~# iwlist wlan0 scanning | grep ESSID | wc -l 13 That get's me a lot further! Cool ! -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/4] Introduce hardware spinlock framework
and contribution from many developers (see [1][2][3][4][5][6]): Simon Que did the initial implementation and pushed several iterations Benoit Cousson provided extensive review, help, improvements and hwmod support Hari Kanigeri helped out when Simon was away Sanjeev Premi, Santosh Shilimkar and Nishanth Menon did lots of review I'd like to thank Benoit Cousson, Steve Krueger, Hari Kanigeri, Nourredine Hamoudi and Richard Woodruff for useful discussions about the OMAP Spinlock requirements and use-cases. Relevant linux-omap threads: [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/38755 [2] http://thread.gmane.org/gmane.linux.ports.arm.omap/38917 [3] http://thread.gmane.org/gmane.linux.ports.arm.omap/39187 [4] http://thread.gmane.org/gmane.linux.ports.arm.omap/39365 [5] http://thread.gmane.org/gmane.linux.ports.arm.omap/39815 [6] http://thread.gmane.org/gmane.linux.ports.arm.omap/40901 Benoit Cousson (1): OMAP4: hwmod data: Add hwspinlock Ohad Ben-Cohen (1): drivers: hwspinlock: add framework Simon Que (2): drivers: hwspinlock: add OMAP implementation omap: add hwspinlock device Documentation/hwspinlock.txt | 299 +++ arch/arm/mach-omap2/Makefile |1 + arch/arm/mach-omap2/hwspinlock.c | 63 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 63 drivers/Kconfig|2 + drivers/Makefile |2 + drivers/hwspinlock/Kconfig | 22 ++ drivers/hwspinlock/Makefile|6 + drivers/hwspinlock/hwspinlock.h| 61 +++ drivers/hwspinlock/hwspinlock_core.c | 557 drivers/hwspinlock/omap_hwspinlock.c | 231 include/linux/hwspinlock.h | 298 +++ 12 files changed, 1605 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwspinlock.txt create mode 100644 arch/arm/mach-omap2/hwspinlock.c create mode 100644 drivers/hwspinlock/Kconfig create mode 100644 drivers/hwspinlock/Makefile create mode 100644 drivers/hwspinlock/hwspinlock.h create mode 100644 drivers/hwspinlock/hwspinlock_core.c create mode 100644 drivers/hwspinlock/omap_hwspinlock.c create mode 100644 include/linux/hwspinlock.h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] drivers: hwspinlock: add framework
Add a platform-independent hwspinlock framework. Hardware spinlock devices are needed, e.g., in order to access data that is shared between remote processors, that otherwise have no alternative mechanism to accomplish synchronization and mutual exclusion operations. Signed-off-by: Ohad Ben-Cohen o...@wizery.com Cc: Hari Kanigeri h-kanige...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Kevin Hilman khil...@ti.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Arnd Bergmann a...@arndb.de Cc: Paul Walmsley p...@pwsan.com Acked-by: Tony Lindgren t...@atomide.com --- Documentation/hwspinlock.txt | 299 ++ drivers/Kconfig |2 + drivers/Makefile |2 + drivers/hwspinlock/Kconfig | 13 + drivers/hwspinlock/Makefile |5 + drivers/hwspinlock/hwspinlock.h | 61 drivers/hwspinlock/hwspinlock_core.c | 557 ++ include/linux/hwspinlock.h | 298 ++ 8 files changed, 1237 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwspinlock.txt create mode 100644 drivers/hwspinlock/Kconfig create mode 100644 drivers/hwspinlock/Makefile create mode 100644 drivers/hwspinlock/hwspinlock.h create mode 100644 drivers/hwspinlock/hwspinlock_core.c create mode 100644 include/linux/hwspinlock.h diff --git a/Documentation/hwspinlock.txt b/Documentation/hwspinlock.txt new file mode 100644 index 000..65324ce --- /dev/null +++ b/Documentation/hwspinlock.txt @@ -0,0 +1,299 @@ +Hardware Spinlock Framework + +1. Introduction + +Hardware spinlock modules provide hardware assistance for synchronization +and mutual exclusion between heterogeneous processors and those not operating +under a single, shared operating system. + +For example, OMAP4 has dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP, +each of which is running a different Operating System (the master, A9, +is usually running Linux and the slave processors, the M3 and the DSP, +are running some flavor of RTOS). + +A generic hwspinlock framework allows platform-independent drivers to use +the hwspinlock device in order to access data structures that are shared +between remote processors, that otherwise have no alternative mechanism +to accomplish synchronization and mutual exclusion operations. + +This is necessary, for example, for Inter-processor communications: +on OMAP4, cpu-intensive multimedia tasks are offloaded by the host to the +remote M3 and/or C64x+ slave processors (by an IPC subsystem called Syslink). + +To achieve fast message-based communications, a minimal kernel support +is needed to deliver messages arriving from a remote processor to the +appropriate user process. + +This communication is based on simple data structures that is shared between +the remote processors, and access to it is synchronized using the hwspinlock +module (remote processor directly places new messages in this shared data +structure). + +A common hwspinlock interface makes it possible to have generic, platform- +independent, drivers. + +2. User API + + struct hwspinlock *hwspin_lock_request(void); + - dynamically assign an hwspinlock and return its address, or NULL + in case an unused hwspinlock isn't available. Users of this + API will usually want to communicate the lock's id to the remote core + before it can be used to achieve synchronization. + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + struct hwspinlock *hwspin_lock_request_specific(unsigned int id); + - assign a specific hwspinlock id and return its address, or NULL + if that hwspinlock is already in use. Usually board code will + be calling this function in order to reserve specific hwspinlock + ids for predefined purposes. + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + int hwspin_lock_free(struct hwspinlock *hwlock); + - free a previously-assigned hwspinlock; returns 0 on success, or an + appropriate error code on failure (e.g. -EINVAL if the hwspinlock + is already free). + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + int hwspin_lock_timeout(struct hwspinlock *hwlock, unsigned long timeout); + - lock a previously-assigned hwspinlock with a timeout limit (specified in + jiffies). If the hwspinlock is already taken, the function will busy loop + waiting for it to be released, but give up when the timeout meets jiffies. + If timeout is 0, the function will never give up (therefore if a faulty + remote core never releases the hwspinlock, it will deadlock). + Upon a successful return from this function, preemption is disabled so + the caller must not sleep, and is advised to release the hwspinlock as + soon as possible, in order to minimize remote cores
[PATCH v4 2/4] drivers: hwspinlock: add OMAP implementation
From: Simon Que s...@ti.com
Add hwspinlock support for the OMAP4 Hardware Spinlock device.
The Hardware Spinlock device on OMAP4 provides hardware assistance
for synchronization between the multiple processors in the system
(dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP).
[o...@wizery.com: adapt to hwspinlock framework, tidy up]
Signed-off-by: Simon Que s...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Krishnamoorthy, Balaji T balaj...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Arnd Bergmann a...@arndb.de
Cc: Paul Walmsley p...@pwsan.com
Acked-by: Tony Lindgren t...@atomide.com
---
drivers/hwspinlock/Kconfig |9 ++
drivers/hwspinlock/Makefile |1 +
drivers/hwspinlock/omap_hwspinlock.c | 231 ++
3 files changed, 241 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwspinlock/omap_hwspinlock.c
diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index 9dd8db4..eb4af28 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -11,3 +11,12 @@ config HWSPINLOCK
coprocessors).
If unsure, say N.
+
+config HWSPINLOCK_OMAP
+ tristate OMAP Hardware Spinlock device
+ depends on HWSPINLOCK ARCH_OMAP4
+ help
+ Say y here to support the OMAP Hardware Spinlock device (firstly
+ introduced in OMAP4).
+
+ If unsure, say N.
diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile
index b9d2b9f..5729a3f 100644
--- a/drivers/hwspinlock/Makefile
+++ b/drivers/hwspinlock/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_HWSPINLOCK) += hwspinlock_core.o
+obj-$(CONFIG_HWSPINLOCK_OMAP) += omap_hwspinlock.o
diff --git a/drivers/hwspinlock/omap_hwspinlock.c
b/drivers/hwspinlock/omap_hwspinlock.c
new file mode 100644
index 000..03e1d60
--- /dev/null
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -0,0 +1,231 @@
+/*
+ * OMAP hardware spinlock driver
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Contact: Simon Que s...@ti.com
+ * Hari Kanigeri h-kanige...@ti.com
+ * Ohad Ben-Cohen o...@wizery.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/device.h
+#include linux/delay.h
+#include linux/io.h
+#include linux/bitops.h
+#include linux/pm_runtime.h
+#include linux/slab.h
+#include linux/spinlock.h
+#include linux/hwspinlock.h
+#include linux/platform_device.h
+
+#include hwspinlock.h
+
+/* Spinlock register offsets */
+#define SYSSTATUS_OFFSET 0x0014
+#define LOCK_BASE_OFFSET 0x0800
+
+#define SPINLOCK_NUMLOCKS_BIT_OFFSET (24)
+
+/* Possible values of SPINLOCK_LOCK_REG */
+#define SPINLOCK_NOTTAKEN (0) /* free */
+#define SPINLOCK_TAKEN (1) /* locked */
+
+#define to_omap_hwspinlock(lock) \
+ container_of(lock, struct omap_hwspinlock, lock)
+
+struct omap_hwspinlock {
+ struct hwspinlock lock;
+ void __iomem *addr;
+};
+
+struct omap_hwspinlock_state {
+ int num_locks; /* Total number of locks in system */
+ void __iomem *io_base; /* Mapped base address */
+};
+
+static int omap_hwspinlock_trylock(struct hwspinlock *lock)
+{
+ struct omap_hwspinlock *omap_lock = to_omap_hwspinlock(lock);
+
+ /* attempt to acquire the lock by reading its value */
+ return (SPINLOCK_NOTTAKEN == readl(omap_lock-addr));
+}
+
+static void omap_hwspinlock_unlock(struct hwspinlock *lock)
+{
+ struct omap_hwspinlock *omap_lock = to_omap_hwspinlock(lock);
+
+ /* release the lock by writing 0 to it */
+ writel(SPINLOCK_NOTTAKEN, omap_lock-addr);
+}
+
+/*
+ * relax the OMAP interconnect while spinning on it.
+ *
+ * The specs recommended that the retry delay time will be
+ * just over half of the time that a requester would be
+ * expected to hold the lock.
+ *
+ * The number below is taken from an hardware specs example,
+ * obviously it is somewhat arbitrary.
+ */
+static void omap_hwspinlock_relax(struct hwspinlock *lock)
+{
+ ndelay(50);
+}
+
+static const struct hwspinlock_ops omap_hwspinlock_ops = {
+ .trylock = omap_hwspinlock_trylock,
+ .unlock = omap_hwspinlock_unlock,
+ .relax = omap_hwspinlock_relax,
+};
+
+static int __devinit omap_hwspinlock_probe(struct platform_device *pdev
[PATCH v4 3/4] OMAP4: hwmod data: Add hwspinlock
From: Benoit Cousson b-cous...@ti.com
Add hwspinlock hwmod data for OMAP4 chip
Signed-off-by: Cousson, Benoit b-cous...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Paul Walmsley p...@pwsan.com
Acked-by: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 63
1 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index c2806bd..e1c8e6d 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2001,6 +2001,66 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = {
.omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
};
+/*
+ * 'spinlock' class
+ * spinlock provides hardware assistance for synchronizing the processes
+ * running on multiple processors
+ */
+
+static struct omap_hwmod_class_sysconfig omap44xx_spinlock_sysc = {
+ .rev_offs = 0x,
+ .sysc_offs = 0x0010,
+ .syss_offs = 0x0014,
+ .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_CLOCKACTIVITY |
+ SYSC_HAS_ENAWAKEUP | SYSC_HAS_SIDLEMODE |
+ SYSC_HAS_SOFTRESET | SYSS_HAS_RESET_STATUS),
+ .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+ .sysc_fields= omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap44xx_spinlock_hwmod_class = {
+ .name = spinlock,
+ .sysc = omap44xx_spinlock_sysc,
+};
+
+/* spinlock */
+static struct omap_hwmod omap44xx_spinlock_hwmod;
+static struct omap_hwmod_addr_space omap44xx_spinlock_addrs[] = {
+ {
+ .pa_start = 0x4a0f6000,
+ .pa_end = 0x4a0f6fff,
+ .flags = ADDR_TYPE_RT
+ },
+};
+
+/* l4_cfg - spinlock */
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__spinlock = {
+ .master = omap44xx_l4_cfg_hwmod,
+ .slave = omap44xx_spinlock_hwmod,
+ .clk= l4_div_ck,
+ .addr = omap44xx_spinlock_addrs,
+ .addr_cnt = ARRAY_SIZE(omap44xx_spinlock_addrs),
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* spinlock slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_spinlock_slaves[] = {
+ omap44xx_l4_cfg__spinlock,
+};
+
+static struct omap_hwmod omap44xx_spinlock_hwmod = {
+ .name = spinlock,
+ .class = omap44xx_spinlock_hwmod_class,
+ .prcm = {
+ .omap4 = {
+ .clkctrl_reg = OMAP4430_CM_L4CFG_HW_SEM_CLKCTRL,
+ },
+ },
+ .slaves = omap44xx_spinlock_slaves,
+ .slaves_cnt = ARRAY_SIZE(omap44xx_spinlock_slaves),
+ .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
/* dmm class */
@@ -2068,6 +2128,9 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
omap44xx_wd_timer2_hwmod,
omap44xx_wd_timer3_hwmod,
+
+ /* spinlock class */
+ omap44xx_spinlock_hwmod,
NULL,
};
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] omap: add hwspinlock device
From: Simon Que s...@ti.com
Build and register an hwspinlock platform device.
Although only OMAP4 supports the hardware spinlock module (for now),
it is still safe to run this initcall on all omaps, because hwmod lookup
will simply fail on hwspinlock-less platforms.
Signed-off-by: Simon Que s...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Acked-by: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/Makefile |1 +
arch/arm/mach-omap2/hwspinlock.c | 63 ++
2 files changed, 64 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/hwspinlock.c
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 1c0c2b0..7b9abe1 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -242,3 +242,4 @@ obj-y += $(smc91x-m)
$(smc91x-y)
smsc911x-$(CONFIG_SMSC911X):= gpmc-smsc911x.o
obj-y += $(smsc911x-m) $(smsc911x-y)
+obj-$(CONFIG_ARCH_OMAP4) += hwspinlock.o
diff --git a/arch/arm/mach-omap2/hwspinlock.c b/arch/arm/mach-omap2/hwspinlock.c
new file mode 100644
index 000..06d4a80
--- /dev/null
+++ b/arch/arm/mach-omap2/hwspinlock.c
@@ -0,0 +1,63 @@
+/*
+ * OMAP hardware spinlock device initialization
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Contact: Simon Que s...@ti.com
+ * Hari Kanigeri h-kanige...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/init.h
+#include linux/err.h
+
+#include plat/omap_hwmod.h
+#include plat/omap_device.h
+
+struct omap_device_pm_latency omap_spinlock_latency[] = {
+ {
+ .deactivate_func = omap_device_idle_hwmods,
+ .activate_func = omap_device_enable_hwmods,
+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+ }
+};
+
+int __init hwspinlocks_init(void)
+{
+ int retval = 0;
+ struct omap_hwmod *oh;
+ struct omap_device *od;
+ const char *oh_name = spinlock;
+ const char *dev_name = omap_hwspinlock;
+
+ /*
+* Hwmod lookup will fail in case our platform doesn't support the
+* hardware spinlock module, so it is safe to run this initcall
+* on all omaps
+*/
+ oh = omap_hwmod_lookup(oh_name);
+ if (oh == NULL)
+ return -EINVAL;
+
+ od = omap_device_build(dev_name, 0, oh, NULL, 0,
+ omap_spinlock_latency,
+ ARRAY_SIZE(omap_spinlock_latency), false);
+ if (IS_ERR(od)) {
+ pr_err(Can't build omap_device for %s:%s\n, dev_name,
+ oh_name);
+ retval = PTR_ERR(od);
+ }
+
+ return retval;
+}
+/* early board code might need to reserve specific hwspinlock instances */
+postcore_initcall(hwspinlocks_init);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] drivers: hwspinlock: add framework
On Tue, Feb 1, 2011 at 1:38 AM, Andrew Morton a...@linux-foundation.org wrote: It's a little irritating having two hwspinlock.h's. hwspinlock_internal.h wold be a conventional approach. But it's not a big deal. ... +/** + * __hwspin_lock_timeout() - lock an hwspinlock with timeout limit + * @hwlock: the hwspinlock to be locked + * @timeout: timeout value in jiffies hm, why in jiffies? The problem here is that lazy programmers will use hwspin_lock_timeout(lock, 10, ...) and their code will work happily with HZ=100 but will explode with HZ=1000. IOW, this interface *requires* that all callers perform a seconds-to-jiffies conversion before calling hwspin_lock_timeout(). So why not reduce their effort and their ability to make mistakes by defining the API to take seconds? I considered that, but then decided to use jiffies in order to be consistent with wait_event_timeout/schedule_timeout (although I don't return the remaining jiffies in case the lock is taken before the timeout elapses), and also to allow user-selected granularity. But I do kind of like the idea of not using jiffies. We can probably even move to msecs, since anyway this is an error condition, and people who needs a quick check should just use the trylock() version. I'll do a quick respin of the patches with that and the hwspinlock_internal.h comment above. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] drivers: hwspinlock: add framework
On Tue, Feb 1, 2011 at 8:38 AM, Andrew Morton a...@linux-foundation.org wrote: I'll do a quick respin of the patches with that and the hwspinlock_internal.h comment above. OK.. The patch series looks OK to me. Can I add your Acked-by on the non-omap parts when I respin the series ? Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] drivers: hwspinlock: add framework
On Tue, Feb 1, 2011 at 9:40 AM, Andrew Morton a...@linux-foundation.org wrote: On Tue, 1 Feb 2011 09:36:22 +0200 Ohad Ben-Cohen o...@wizery.com wrote: On Tue, Feb 1, 2011 at 8:38 AM, Andrew Morton a...@linux-foundation.org wrote: I'll do a quick respin of the patches with that and the hwspinlock_internal.h comment above. OK.. The patch series looks OK to me. Can I add your Acked-by on the non-omap parts when I respin the series ? spose so. I don't normally do acked-by's. I think it's my way of avoiding getting blamed when it all blows up. I don't want to be breaking old habits then :) Maybe Tony or Russell or Greg can grab them if they like the look of it all? Let's just wait a bit for Tony's or Russell's or Greg's answer. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Setting MMC_CAP_POWER_OFF_CARD on mmc2 leads to filesystem problems on mmc1
On Wed, Feb 2, 2011 at 2:11 AM, Tony Lindgren t...@atomide.com wrote: * Ohad Ben-Cohen o...@wizery.com [110122 07:47]: On Sat, Jan 22, 2011 at 5:30 PM, Koen Kooi k...@dominion.thruhere.net wrote: That was indeed the problem, not I get: [ 35.417053] wl1271: firmware booted (Rev 6.1.0.0.343) and root@beagleboard-xm-next:~# ifconfig wlan0 hw eth 00:11:22:33:44:55 root@beagleboard-xm-next:~# iwlist wlan0 scanning | grep ESSID | wc -l 13 That get's me a lot further! Cool ! Hmm, do we need to still patch something for the -rc cycle for this? I don't think so; the problem wasn't in the existing mainline code, but in the new functionality developed (adding support for the beagletoy expansion board), so this is not -rc material. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/4] Introduce hardware spinlock framework
interrupts should be disabled or not. Sleeping, btw, is still prohibited of course. Contributions: Previous versions of an omap-specific hwspinlock driver circulated in linux-omap several times, and received substantial attention and contribution from many developers (see [1][2][3][4][5][6]): Simon Que did the initial implementation and pushed several iterations Benoit Cousson provided extensive review, help, improvements and hwmod support Hari Kanigeri helped out when Simon was away Sanjeev Premi, Santosh Shilimkar and Nishanth Menon did lots of review I'd like to thank Benoit Cousson, Steve Krueger, Hari Kanigeri, Nourredine Hamoudi and Richard Woodruff for useful discussions about the OMAP Spinlock requirements and use-cases. Relevant linux-omap threads: [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/38755 [2] http://thread.gmane.org/gmane.linux.ports.arm.omap/38917 [3] http://thread.gmane.org/gmane.linux.ports.arm.omap/39187 [4] http://thread.gmane.org/gmane.linux.ports.arm.omap/39365 [5] http://thread.gmane.org/gmane.linux.ports.arm.omap/39815 [6] http://thread.gmane.org/gmane.linux.ports.arm.omap/40901 Benoit Cousson (1): OMAP4: hwmod data: Add hwspinlock Ohad Ben-Cohen (1): drivers: hwspinlock: add framework Simon Que (2): drivers: hwspinlock: add OMAP implementation omap: add hwspinlock device Documentation/hwspinlock.txt | 293 +++ arch/arm/mach-omap2/Makefile |1 + arch/arm/mach-omap2/hwspinlock.c | 63 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 63 drivers/Kconfig|2 + drivers/Makefile |2 + drivers/hwspinlock/Kconfig | 22 ++ drivers/hwspinlock/Makefile|6 + drivers/hwspinlock/hwspinlock_core.c | 548 drivers/hwspinlock/hwspinlock_internal.h | 61 +++ drivers/hwspinlock/omap_hwspinlock.c | 231 include/linux/hwspinlock.h | 292 +++ 12 files changed, 1584 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwspinlock.txt create mode 100644 arch/arm/mach-omap2/hwspinlock.c create mode 100644 drivers/hwspinlock/Kconfig create mode 100644 drivers/hwspinlock/Makefile create mode 100644 drivers/hwspinlock/hwspinlock_core.c create mode 100644 drivers/hwspinlock/hwspinlock_internal.h create mode 100644 drivers/hwspinlock/omap_hwspinlock.c create mode 100644 include/linux/hwspinlock.h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/4] drivers: hwspinlock: add OMAP implementation
From: Simon Que s...@ti.com
Add hwspinlock support for the OMAP4 Hardware Spinlock device.
The Hardware Spinlock device on OMAP4 provides hardware assistance
for synchronization between the multiple processors in the system
(dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP).
[o...@wizery.com: adapt to hwspinlock framework, tidy up]
Signed-off-by: Simon Que s...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Krishnamoorthy, Balaji T balaj...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Arnd Bergmann a...@arndb.de
Cc: Paul Walmsley p...@pwsan.com
Cc: Russell King li...@arm.linux.org.uk
Acked-by: Tony Lindgren t...@atomide.com
---
drivers/hwspinlock/Kconfig |9 ++
drivers/hwspinlock/Makefile |1 +
drivers/hwspinlock/omap_hwspinlock.c | 231 ++
3 files changed, 241 insertions(+), 0 deletions(-)
create mode 100644 drivers/hwspinlock/omap_hwspinlock.c
diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index 9dd8db4..eb4af28 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -11,3 +11,12 @@ config HWSPINLOCK
coprocessors).
If unsure, say N.
+
+config HWSPINLOCK_OMAP
+ tristate OMAP Hardware Spinlock device
+ depends on HWSPINLOCK ARCH_OMAP4
+ help
+ Say y here to support the OMAP Hardware Spinlock device (firstly
+ introduced in OMAP4).
+
+ If unsure, say N.
diff --git a/drivers/hwspinlock/Makefile b/drivers/hwspinlock/Makefile
index b9d2b9f..5729a3f 100644
--- a/drivers/hwspinlock/Makefile
+++ b/drivers/hwspinlock/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_HWSPINLOCK) += hwspinlock_core.o
+obj-$(CONFIG_HWSPINLOCK_OMAP) += omap_hwspinlock.o
diff --git a/drivers/hwspinlock/omap_hwspinlock.c
b/drivers/hwspinlock/omap_hwspinlock.c
new file mode 100644
index 000..a8f0273
--- /dev/null
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -0,0 +1,231 @@
+/*
+ * OMAP hardware spinlock driver
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Contact: Simon Que s...@ti.com
+ * Hari Kanigeri h-kanige...@ti.com
+ * Ohad Ben-Cohen o...@wizery.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/device.h
+#include linux/delay.h
+#include linux/io.h
+#include linux/bitops.h
+#include linux/pm_runtime.h
+#include linux/slab.h
+#include linux/spinlock.h
+#include linux/hwspinlock.h
+#include linux/platform_device.h
+
+#include hwspinlock_internal.h
+
+/* Spinlock register offsets */
+#define SYSSTATUS_OFFSET 0x0014
+#define LOCK_BASE_OFFSET 0x0800
+
+#define SPINLOCK_NUMLOCKS_BIT_OFFSET (24)
+
+/* Possible values of SPINLOCK_LOCK_REG */
+#define SPINLOCK_NOTTAKEN (0) /* free */
+#define SPINLOCK_TAKEN (1) /* locked */
+
+#define to_omap_hwspinlock(lock) \
+ container_of(lock, struct omap_hwspinlock, lock)
+
+struct omap_hwspinlock {
+ struct hwspinlock lock;
+ void __iomem *addr;
+};
+
+struct omap_hwspinlock_state {
+ int num_locks; /* Total number of locks in system */
+ void __iomem *io_base; /* Mapped base address */
+};
+
+static int omap_hwspinlock_trylock(struct hwspinlock *lock)
+{
+ struct omap_hwspinlock *omap_lock = to_omap_hwspinlock(lock);
+
+ /* attempt to acquire the lock by reading its value */
+ return (SPINLOCK_NOTTAKEN == readl(omap_lock-addr));
+}
+
+static void omap_hwspinlock_unlock(struct hwspinlock *lock)
+{
+ struct omap_hwspinlock *omap_lock = to_omap_hwspinlock(lock);
+
+ /* release the lock by writing 0 to it */
+ writel(SPINLOCK_NOTTAKEN, omap_lock-addr);
+}
+
+/*
+ * relax the OMAP interconnect while spinning on it.
+ *
+ * The specs recommended that the retry delay time will be
+ * just over half of the time that a requester would be
+ * expected to hold the lock.
+ *
+ * The number below is taken from an hardware specs example,
+ * obviously it is somewhat arbitrary.
+ */
+static void omap_hwspinlock_relax(struct hwspinlock *lock)
+{
+ ndelay(50);
+}
+
+static const struct hwspinlock_ops omap_hwspinlock_ops = {
+ .trylock = omap_hwspinlock_trylock,
+ .unlock = omap_hwspinlock_unlock,
+ .relax = omap_hwspinlock_relax,
+};
+
+static int __devinit
[PATCH v5 1/4] drivers: hwspinlock: add framework
Add a platform-independent hwspinlock framework. Hardware spinlock devices are needed, e.g., in order to access data that is shared between remote processors, that otherwise have no alternative mechanism to accomplish synchronization and mutual exclusion operations. Signed-off-by: Ohad Ben-Cohen o...@wizery.com Cc: Hari Kanigeri h-kanige...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Kevin Hilman khil...@ti.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Paul Walmsley p...@pwsan.com Cc: Russell King li...@arm.linux.org.uk Acked-by: Tony Lindgren t...@atomide.com Acked-by: Arnd Bergmann a...@arndb.de --- Documentation/hwspinlock.txt | 293 drivers/Kconfig |2 + drivers/Makefile |2 + drivers/hwspinlock/Kconfig | 13 + drivers/hwspinlock/Makefile |5 + drivers/hwspinlock/hwspinlock_core.c | 548 ++ drivers/hwspinlock/hwspinlock_internal.h | 61 include/linux/hwspinlock.h | 292 8 files changed, 1216 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwspinlock.txt create mode 100644 drivers/hwspinlock/Kconfig create mode 100644 drivers/hwspinlock/Makefile create mode 100644 drivers/hwspinlock/hwspinlock_core.c create mode 100644 drivers/hwspinlock/hwspinlock_internal.h create mode 100644 include/linux/hwspinlock.h diff --git a/Documentation/hwspinlock.txt b/Documentation/hwspinlock.txt new file mode 100644 index 000..7dcd1a4 --- /dev/null +++ b/Documentation/hwspinlock.txt @@ -0,0 +1,293 @@ +Hardware Spinlock Framework + +1. Introduction + +Hardware spinlock modules provide hardware assistance for synchronization +and mutual exclusion between heterogeneous processors and those not operating +under a single, shared operating system. + +For example, OMAP4 has dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP, +each of which is running a different Operating System (the master, A9, +is usually running Linux and the slave processors, the M3 and the DSP, +are running some flavor of RTOS). + +A generic hwspinlock framework allows platform-independent drivers to use +the hwspinlock device in order to access data structures that are shared +between remote processors, that otherwise have no alternative mechanism +to accomplish synchronization and mutual exclusion operations. + +This is necessary, for example, for Inter-processor communications: +on OMAP4, cpu-intensive multimedia tasks are offloaded by the host to the +remote M3 and/or C64x+ slave processors (by an IPC subsystem called Syslink). + +To achieve fast message-based communications, a minimal kernel support +is needed to deliver messages arriving from a remote processor to the +appropriate user process. + +This communication is based on simple data structures that is shared between +the remote processors, and access to it is synchronized using the hwspinlock +module (remote processor directly places new messages in this shared data +structure). + +A common hwspinlock interface makes it possible to have generic, platform- +independent, drivers. + +2. User API + + struct hwspinlock *hwspin_lock_request(void); + - dynamically assign an hwspinlock and return its address, or NULL + in case an unused hwspinlock isn't available. Users of this + API will usually want to communicate the lock's id to the remote core + before it can be used to achieve synchronization. + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + struct hwspinlock *hwspin_lock_request_specific(unsigned int id); + - assign a specific hwspinlock id and return its address, or NULL + if that hwspinlock is already in use. Usually board code will + be calling this function in order to reserve specific hwspinlock + ids for predefined purposes. + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + int hwspin_lock_free(struct hwspinlock *hwlock); + - free a previously-assigned hwspinlock; returns 0 on success, or an + appropriate error code on failure (e.g. -EINVAL if the hwspinlock + is already free). + Can be called from an atomic context (this function will not sleep) but + not from within interrupt context. + + int hwspin_lock_timeout(struct hwspinlock *hwlock, unsigned int timeout); + - lock a previously-assigned hwspinlock with a timeout limit (specified in + msecs). If the hwspinlock is already taken, the function will busy loop + waiting for it to be released, but give up when the timeout elapses. + Upon a successful return from this function, preemption is disabled so + the caller must not sleep, and is advised to release the hwspinlock as + soon as possible, in order to minimize remote cores polling on the + hardware interconnect. + Returns 0 when successful
[PATCH v5 3/4] OMAP4: hwmod data: Add hwspinlock
From: Benoit Cousson b-cous...@ti.com
Add hwspinlock hwmod data for OMAP4 chip
Signed-off-by: Cousson, Benoit b-cous...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Paul Walmsley p...@pwsan.com
Acked-by: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 63
1 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index c2806bd..e1c8e6d 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2001,6 +2001,66 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = {
.omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
};
+/*
+ * 'spinlock' class
+ * spinlock provides hardware assistance for synchronizing the processes
+ * running on multiple processors
+ */
+
+static struct omap_hwmod_class_sysconfig omap44xx_spinlock_sysc = {
+ .rev_offs = 0x,
+ .sysc_offs = 0x0010,
+ .syss_offs = 0x0014,
+ .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_CLOCKACTIVITY |
+ SYSC_HAS_ENAWAKEUP | SYSC_HAS_SIDLEMODE |
+ SYSC_HAS_SOFTRESET | SYSS_HAS_RESET_STATUS),
+ .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+ .sysc_fields= omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap44xx_spinlock_hwmod_class = {
+ .name = spinlock,
+ .sysc = omap44xx_spinlock_sysc,
+};
+
+/* spinlock */
+static struct omap_hwmod omap44xx_spinlock_hwmod;
+static struct omap_hwmod_addr_space omap44xx_spinlock_addrs[] = {
+ {
+ .pa_start = 0x4a0f6000,
+ .pa_end = 0x4a0f6fff,
+ .flags = ADDR_TYPE_RT
+ },
+};
+
+/* l4_cfg - spinlock */
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__spinlock = {
+ .master = omap44xx_l4_cfg_hwmod,
+ .slave = omap44xx_spinlock_hwmod,
+ .clk= l4_div_ck,
+ .addr = omap44xx_spinlock_addrs,
+ .addr_cnt = ARRAY_SIZE(omap44xx_spinlock_addrs),
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* spinlock slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_spinlock_slaves[] = {
+ omap44xx_l4_cfg__spinlock,
+};
+
+static struct omap_hwmod omap44xx_spinlock_hwmod = {
+ .name = spinlock,
+ .class = omap44xx_spinlock_hwmod_class,
+ .prcm = {
+ .omap4 = {
+ .clkctrl_reg = OMAP4430_CM_L4CFG_HW_SEM_CLKCTRL,
+ },
+ },
+ .slaves = omap44xx_spinlock_slaves,
+ .slaves_cnt = ARRAY_SIZE(omap44xx_spinlock_slaves),
+ .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
/* dmm class */
@@ -2068,6 +2128,9 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
omap44xx_wd_timer2_hwmod,
omap44xx_wd_timer3_hwmod,
+
+ /* spinlock class */
+ omap44xx_spinlock_hwmod,
NULL,
};
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/4] omap: add hwspinlock device
From: Simon Que s...@ti.com
Build and register an hwspinlock platform device.
Although only OMAP4 supports the hardware spinlock module (for now),
it is still safe to run this initcall on all omaps, because hwmod lookup
will simply fail on hwspinlock-less platforms.
Signed-off-by: Simon Que s...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Acked-by: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/Makefile |1 +
arch/arm/mach-omap2/hwspinlock.c | 63 ++
2 files changed, 64 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/hwspinlock.c
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 1c0c2b0..7b9abe1 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -242,3 +242,4 @@ obj-y += $(smc91x-m)
$(smc91x-y)
smsc911x-$(CONFIG_SMSC911X):= gpmc-smsc911x.o
obj-y += $(smsc911x-m) $(smsc911x-y)
+obj-$(CONFIG_ARCH_OMAP4) += hwspinlock.o
diff --git a/arch/arm/mach-omap2/hwspinlock.c b/arch/arm/mach-omap2/hwspinlock.c
new file mode 100644
index 000..06d4a80
--- /dev/null
+++ b/arch/arm/mach-omap2/hwspinlock.c
@@ -0,0 +1,63 @@
+/*
+ * OMAP hardware spinlock device initialization
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Contact: Simon Que s...@ti.com
+ * Hari Kanigeri h-kanige...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/init.h
+#include linux/err.h
+
+#include plat/omap_hwmod.h
+#include plat/omap_device.h
+
+struct omap_device_pm_latency omap_spinlock_latency[] = {
+ {
+ .deactivate_func = omap_device_idle_hwmods,
+ .activate_func = omap_device_enable_hwmods,
+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+ }
+};
+
+int __init hwspinlocks_init(void)
+{
+ int retval = 0;
+ struct omap_hwmod *oh;
+ struct omap_device *od;
+ const char *oh_name = spinlock;
+ const char *dev_name = omap_hwspinlock;
+
+ /*
+* Hwmod lookup will fail in case our platform doesn't support the
+* hardware spinlock module, so it is safe to run this initcall
+* on all omaps
+*/
+ oh = omap_hwmod_lookup(oh_name);
+ if (oh == NULL)
+ return -EINVAL;
+
+ od = omap_device_build(dev_name, 0, oh, NULL, 0,
+ omap_spinlock_latency,
+ ARRAY_SIZE(omap_spinlock_latency), false);
+ if (IS_ERR(od)) {
+ pr_err(Can't build omap_device for %s:%s\n, dev_name,
+ oh_name);
+ retval = PTR_ERR(od);
+ }
+
+ return retval;
+}
+/* early board code might need to reserve specific hwspinlock instances */
+postcore_initcall(hwspinlocks_init);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] omap: mailbox: cleanup simplify
A few simple patches that cleanup and simplifies mailbox. The last patch is the most interesting - It converts mailbox to use kfifo as the underlying queueing implementation instead of using the block API. There're also additional performance patches on the way, we are internally testing them now. Please review and let me know your comments. Thanks, Ohad Ben-Cohen (4): omap: mailbox cleanup: convert rwlocks to spinlock omap: mailbox cleanup: split MODULE_AUTHOR line omap: mailbox: fix reverse likeliness omap: mailbox: convert block api to kfifo arch/arm/mach-omap2/mailbox.c |3 +- arch/arm/plat-omap/include/plat/mailbox.h |5 +- arch/arm/plat-omap/mailbox.c | 135 + 3 files changed, 68 insertions(+), 75 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
rwlocks are slower and have potential starvation issues so spinlocks are
generally preferred
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/mailbox.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 08a2df7..d73d51a 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@
static struct workqueue_struct *mboxd;
static struct omap_mbox *mboxes;
-static DEFINE_RWLOCK(mboxes_lock);
+static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
@@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const char *name)
struct omap_mbox *mbox;
int ret;
- read_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
mbox = *(find_mboxes(name));
if (mbox == NULL) {
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ERR_PTR(-ENOENT);
}
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
ret = omap_mbox_startup(mbox);
if (ret)
@@ -363,15 +363,15 @@ int omap_mbox_register(struct device *parent, struct
omap_mbox *mbox)
if (mbox-next)
return -EBUSY;
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
tmp = find_mboxes(mbox-name);
if (*tmp) {
ret = -EBUSY;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
goto err_find;
}
*tmp = mbox;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return 0;
@@ -384,18 +384,18 @@ int omap_mbox_unregister(struct omap_mbox *mbox)
{
struct omap_mbox **tmp;
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
tmp = mboxes;
while (*tmp) {
if (mbox == *tmp) {
*tmp = mbox-next;
mbox-next = NULL;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return 0;
}
tmp = (*tmp)-next;
}
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return -EINVAL;
}
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line
use multiple MODULE_AUTHOR lines for multiple authors Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- arch/arm/mach-omap2/mailbox.c |3 ++- arch/arm/plat-omap/mailbox.c |3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 318f363..763272c 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -486,5 +486,6 @@ module_exit(omap2_mbox_exit); MODULE_LICENSE(GPL v2); MODULE_DESCRIPTION(omap mailbox: omap2/3/4 architecture specific functions); -MODULE_AUTHOR(Hiroshi DOYU hiroshi.d...@nokia.com, Paul Mundt); +MODULE_AUTHOR(Hiroshi DOYU hiroshi.d...@nokia.com); +MODULE_AUTHOR(Paul Mundt); MODULE_ALIAS(platform:DRV_NAME); diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index d73d51a..8abf0e8 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -419,4 +419,5 @@ module_exit(omap_mbox_exit); MODULE_LICENSE(GPL v2); MODULE_DESCRIPTION(omap mailbox: interrupt driven messaging); -MODULE_AUTHOR(Toshihiro Kobayashi and Hiroshi DOYU); +MODULE_AUTHOR(Toshihiro Kobayashi); +MODULE_AUTHOR(Hiroshi DOYU); -- 1.6.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] omap: mailbox: fix reverse likeliness
Fix reverse likeliness
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/mailbox.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 8abf0e8..72b17ad 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
fail_alloc_txq:
free_irq(mbox-irq, mbox);
fail_request_irq:
- if (unlikely(mbox-ops-shutdown))
+ if (likely(mbox-ops-shutdown))
mbox-ops-shutdown(mbox);
return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
- if (unlikely(mbox-ops-shutdown)) {
+ if (likely(mbox-ops-shutdown)) {
write_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] omap: mailbox: convert block api to kfifo
The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
arch/arm/plat-omap/include/plat/mailbox.h |5 +-
arch/arm/plat-omap/mailbox.c | 108 +
2 files changed, 52 insertions(+), 61 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h
b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..014cc58 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -7,6 +7,7 @@
#include linux/workqueue.h
#include linux/blkdev.h
#include linux/interrupt.h
+#include linux/kfifo.h
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -19,6 +20,8 @@ typedef int __bitwise omap_mbox_type_t;
#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
+#define MBOX_KFIFO_SIZE(256)
+
struct omap_mbox_ops {
omap_mbox_type_ttype;
int (*startup)(struct omap_mbox *mbox);
@@ -42,7 +45,7 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
- struct request_queue*queue;
+ struct kfifofifo;
struct work_struct work;
struct tasklet_struct tasklet;
int (*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 72b17ad..b1324f3 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -26,6 +26,7 @@
#include linux/device.h
#include linux/delay.h
#include linux/slab.h
+#include linux/kfifo.h
#include plat/mailbox.h
@@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox,
omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox,
mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox-txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox-txq-queue;
+ spin_lock(mq-lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(mq-fifo) sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(mq-fifo, (unsigned char *)msg, sizeof(msg));
+ if (unlikely(len != sizeof(msg))) {
+ pr_err(%s: kfifo_in anomaly\n, __func__);
+ ret = -ENOMEM;
+ }
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(mbox-txq-tasklet);
- return 0;
+out:
+ spin_unlock(mq-lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox-txq-queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox-txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq-special);
- if (ret) {
+ while (kfifo_len(mq-fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq);
- return;
+ break;
}
- blk_end_request_all(rq, 0);
+
+ ret = kfifo_out(mq-fifo, (unsigned char *)msg,
+ sizeof(msg));
+ if (unlikely(ret != sizeof(msg)))
+ pr_err(%s: kfifo_out anomaly\n, __func__);
+
+ mbox_fifo_write(mbox, msg);
}
}
@@ -131,36 +137,22 @@ static void mbox_rx_work(struct work_struct *work)
{
struct omap_mbox_queue *mq =
container_of(work, struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq-queue-queuedata;
- struct request_queue *q = mbox-rxq-queue;
- struct request *rq;
mbox_msg_t msg;
- unsigned long flags;
+ int len;
- while (1) {
- spin_lock_irqsave(q-queue_lock, flags);
- rq = blk_fetch_request(q);
- spin_unlock_irqrestore(q-queue_lock, flags);
- if (!rq)
- break;
+ while (kfifo_len(mq-fifo
Re: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
Hi Hiroshi, On Wed, Apr 28, 2010 at 10:50 AM, Hiroshi DOYU hiroshi.d...@nokia.com wrote: Hi Ohad, From: ext Ohad Ben-Cohen o...@wizery.com Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock Date: Tue, 27 Apr 2010 19:56:19 +0200 rwlocks are slower and have potential starvation issues so spinlocks are generally preferred Would it be possible to explain the above a bit more? Sure, sorry for the laconic description. Jonathan Corbet wrote a nice summary about this: http://lwn.net/Articles/364583/ We could switch to rcu, but it's really an overkill because we don't really have a high bandwidth of readers (omap_mbox_get is not being called so much). The only disadvantage of a plain spinlock is that readers now will have to wait in the line, but since omap_mbox_get isn't called so frequently, I guess that by moving to spinlocks the average performance will actually increase (since spinlocks are faster and most likely there will not be multiple concurrent calls to omap_mbox_get). Anyway I only consider this as a cleanup and not really a performance issue, as mboxes_lock is not really on a hot path. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Hi Hiroshi,
On Wed, Apr 28, 2010 at 8:52 AM, Hiroshi DOYU hiroshi.d...@nokia.com wrote:
Hi Ohad,
From: ext Ohad Ben-Cohen o...@wizery.com
Subject: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Tue, 27 Apr 2010 19:56:22 +0200
The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
arch/arm/plat-omap/include/plat/mailbox.h | 5 +-
arch/arm/plat-omap/mailbox.c | 108
+
2 files changed, 52 insertions(+), 61 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h
b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..014cc58 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -7,6 +7,7 @@
#include linux/workqueue.h
#include linux/blkdev.h
#include linux/interrupt.h
+#include linux/kfifo.h
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -19,6 +20,8 @@ typedef int __bitwise omap_mbox_type_t;
#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
+#define MBOX_KFIFO_SIZE (256)
Can this be a module parameter? Then, OEM could set their optimized value.
Sure.
+
struct omap_mbox_ops {
omap_mbox_type_t type;
int (*startup)(struct omap_mbox *mbox);
@@ -42,7 +45,7 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
- struct request_queue *queue;
+ struct kfifo fifo;
struct work_struct work;
struct tasklet_struct tasklet;
int (*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 72b17ad..b1324f3 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -26,6 +26,7 @@
#include linux/device.h
#include linux/delay.h
#include linux/slab.h
+#include linux/kfifo.h
#include plat/mailbox.h
@@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox,
omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox,
mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox-txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox-txq-queue;
+ spin_lock(mq-lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(mq-fifo) sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(mq-fifo, (unsigned char *)msg, sizeof(msg));
+ if (unlikely(len != sizeof(msg))) {
+ pr_err(%s: kfifo_in anomaly\n, __func__);
+ ret = -ENOMEM;
+ }
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(mbox-txq-tasklet);
- return 0;
+out:
+ spin_unlock(mq-lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox-txq-queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox-txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq-special);
- if (ret) {
+ while (kfifo_len(mq-fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq);
- return;
+ break;
}
- blk_end_request_all(rq, 0);
+
+ ret = kfifo_out(mq-fifo, (unsigned char *)msg,
+ sizeof(msg));
+ if (unlikely(ret != sizeof(msg)))
+ pr_err(%s: kfifo_out anomaly\n, __func__);
No error recovery? same for other anomalies.
I thought about it too, but eventually I think it doesn't really make
a lot of sense:
The only reason that kfifo_out/kfifo_in can fail here is if there's
not enough data/space (respectively).
Since we are checking for the availability of data/space before calling
kfifo_out/kfifo_in, it cannot really fail.
If it does fail, that's a critical bug
Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
On Wed, Apr 28, 2010 at 8:56 AM, Hiroshi DOYU hiroshi.d...@nokia.com wrote: From: ext Ohad Ben-Cohen o...@wizery.com Subject: [PATCH 4/4] omap: mailbox: convert block api to kfifo Date: Tue, 27 Apr 2010 19:56:22 +0200 The underlying buffering implementation of mailbox is converted from block API to kfifo due to the simplicity and speed of kfifo. Signed-off-by: Ohad Ben-Cohen o...@wizery.com Signed-off-by: Hari Kanigeri h-kanige...@ti.com --- arch/arm/plat-omap/include/plat/mailbox.h | 5 +- arch/arm/plat-omap/mailbox.c | 108 + 2 files changed, 52 insertions(+), 61 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h index 729166b..014cc58 100644 --- a/arch/arm/plat-omap/include/plat/mailbox.h +++ b/arch/arm/plat-omap/include/plat/mailbox.h @@ -7,6 +7,7 @@ #include linux/workqueue.h #include linux/blkdev.h Also the above is not necessary? Good point. It will be removed. Thanks, Ohad. #include linux/interrupt.h +#include linux/kfifo.h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
On Wed, Apr 28, 2010 at 2:16 PM, Hiroshi DOYU hiroshi.d...@nokia.com wrote:
From: ext Ohad Ben-Cohen o...@wizery.com
Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Wed, 28 Apr 2010 13:02:06 +0200
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 72b17ad..b1324f3 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -26,6 +26,7 @@
#include linux/device.h
#include linux/delay.h
#include linux/slab.h
+#include linux/kfifo.h
#include plat/mailbox.h
@@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox,
omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox,
mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox-txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox-txq-queue;
+ spin_lock(mq-lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(mq-fifo) sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(mq-fifo, (unsigned char *)msg, sizeof(msg));
+ if (unlikely(len != sizeof(msg))) {
+ pr_err(%s: kfifo_in anomaly\n, __func__);
+ ret = -ENOMEM;
+ }
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(mbox-txq-tasklet);
- return 0;
+out:
+ spin_unlock(mq-lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox-txq-queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox-txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq-special);
- if (ret) {
+ while (kfifo_len(mq-fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq);
- return;
+ break;
}
- blk_end_request_all(rq, 0);
+
+ ret = kfifo_out(mq-fifo, (unsigned char *)msg,
+ sizeof(msg));
+ if (unlikely(ret != sizeof(msg)))
+ pr_err(%s: kfifo_out anomaly\n, __func__);
No error recovery? same for other anomalies.
I thought about it too, but eventually I think it doesn't really make
a lot of sense:
The only reason that kfifo_out/kfifo_in can fail here is if there's
not enough data/space (respectively).
Since we are checking for the availability of data/space before calling
kfifo_out/kfifo_in, it cannot really fail.
If it does fail, that's a critical bug that we cannot really recover from.
Only reasonable explanation can be a bug in the code (either ours or
kfifo's),
and with such a bug it really feels futile to put some recovery.
So maybe we should really make this a WARN_ON.
What do you think ?
Makes sense. What about BUG_ON if it shouldn't happen theoretically?
I'm not sure this bug is critical enough to panic and enforce the user
to reboot immediately - he can probably still do a clean shutdown here.
Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
On Wed, Apr 28, 2010 at 2:52 PM, Hiroshi DOYU hiroshi.d...@nokia.com wrote:
From: ext Ohad Ben-Cohen o...@wizery.com
Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Wed, 28 Apr 2010 13:25:41 +0200
On Wed, Apr 28, 2010 at 2:16 PM, Hiroshi DOYU hiroshi.d...@nokia.com wrote:
From: ext Ohad Ben-Cohen o...@wizery.com
Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Wed, 28 Apr 2010 13:02:06 +0200
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 72b17ad..b1324f3 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -26,6 +26,7 @@
#include linux/device.h
#include linux/delay.h
#include linux/slab.h
+#include linux/kfifo.h
#include plat/mailbox.h
@@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox,
omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox,
mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox-txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox-txq-queue;
+ spin_lock(mq-lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(mq-fifo) sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(mq-fifo, (unsigned char *)msg, sizeof(msg));
+ if (unlikely(len != sizeof(msg))) {
+ pr_err(%s: kfifo_in anomaly\n, __func__);
+ ret = -ENOMEM;
+ }
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(mbox-txq-tasklet);
- return 0;
+out:
+ spin_unlock(mq-lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox-txq-queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox-txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq-special);
- if (ret) {
+ while (kfifo_len(mq-fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq);
- return;
+ break;
}
- blk_end_request_all(rq, 0);
+
+ ret = kfifo_out(mq-fifo, (unsigned char *)msg,
+
sizeof(msg));
+ if (unlikely(ret != sizeof(msg)))
+ pr_err(%s: kfifo_out anomaly\n, __func__);
No error recovery? same for other anomalies.
I thought about it too, but eventually I think it doesn't really make
a lot of sense:
The only reason that kfifo_out/kfifo_in can fail here is if there's
not enough data/space (respectively).
Since we are checking for the availability of data/space before calling
kfifo_out/kfifo_in, it cannot really fail.
If it does fail, that's a critical bug that we cannot really recover from.
Only reasonable explanation can be a bug in the code (either ours or
kfifo's),
and with such a bug it really feels futile to put some recovery.
So maybe we should really make this a WARN_ON.
What do you think ?
Makes sense. What about BUG_ON if it shouldn't happen theoretically?
I'm not sure this bug is critical enough to panic and enforce the user
to reboot immediately - he can probably still do a clean shutdown here.
I agree that WARN_ON would be enough for this case. I just thought of
the rareness of this triggering.
Yeah, I was thinking exactly the same, and wanted to put BUG_ON too initially,
but the combination of its calling panic and its header comment
convinced me otherwise:
/*
* Don't use BUG() or BUG_ON() unless there's really no way out; one
* example might be detecting data structure corruption in the middle
* of an operation that can't be backed out of. If the (sub)system
* can somehow continue operating, perhaps with reduced functionality,
* it's probably not BUG-worthy.
*
* If you're tempted to BUG(), think again: is completely giving up
* really the *only* solution? There are usually better options, where
* users don't need to reboot ASAP and can mostly shut down cleanly.
*/
--
To unsubscribe from this list: send the line
Re: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
Hi Hari,
On Thu, Apr 29, 2010 at 3:44 PM, Kanigeri, Hari h-kanige...@ti.com wrote:
Ohad,
Looks like the conversion was missed in few places resulting in compile
warnings.
Thanks for pointing that out - that's really strange, it somehow
slipped in the rebase.
The interesting part is that because you compile for OMAP4 (SMP),
you got to see the warnings, while I didn't because I compiled for OMAP3
(in the UP case there's no real locking and both types of locks are
actually the same).
Please see the below fix. Let me know if you agree with the change.
Sure. I'll add that missing part back together with the review comments,
and submit a v2 patchset.
Thanks again,
Ohad.
#
[PATCH] omap: mailbox: rwlocks to spinlock: compilation fix
fixed the missed rwlocks in few places resultion in following
compiler warning.
arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_startup':
arch/arm/plat-omap/mailbox.c:246: warning: passing argument 1 of
'_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:251: warning: passing argument 1 of
'_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:255: warning: passing argument 1 of
'_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_fini':
arch/arm/plat-omap/mailbox.c:301: warning: passing argument 1 of
'_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:306: warning: passing argument 1 of
'_raw_write_unlock' from incompatible pointer type
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
arch/arm/plat-omap/mailbox.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 27a8d98..d6a700d 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -243,16 +243,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
struct omap_mbox_queue *mq;
if (likely(mbox-ops-startup)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (!mbox_configured)
ret = mbox-ops-startup(mbox);
if (unlikely(ret)) {
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ret;
}
mbox_configured++;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
ret = request_irq(mbox-irq, mbox_interrupt, IRQF_SHARED,
@@ -298,12 +298,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
if (likely(mbox-ops-shutdown)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
if (!mbox_configured)
mbox-ops-shutdown(mbox);
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
}
--
-Original Message-
From: Ohad Ben-Cohen [mailto:o...@wizery.com]
Sent: Tuesday, April 27, 2010 12:56 PM
To: linux-omap@vger.kernel.org
Cc: Kanigeri, Hari; Hiroshi Doyu; Ohad Ben-Cohen
Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks
to spinlock
rwlocks are slower and have potential starvation issues so
spinlocks are generally preferred
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/mailbox.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c
b/arch/arm/plat-omap/mailbox.c index 08a2df7..d73d51a 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@
static struct workqueue_struct *mboxd;
static struct omap_mbox *mboxes;
-static DEFINE_RWLOCK(mboxes_lock);
+static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
@@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const
char *name)
struct omap_mbox *mbox;
int ret;
- read_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
mbox = *(find_mboxes(name));
if (mbox == NULL) {
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ERR_PTR(-ENOENT);
}
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
ret = omap_mbox_startup(mbox);
if (ret)
@@ -363,15 +363,15 @@ int omap_mbox_register(struct device
*parent, struct omap_mbox *mbox)
if (mbox-next)
return -EBUSY;
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
tmp = find_mboxes(mbox-name);
if (*tmp) {
ret = -EBUSY;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
goto err_find;
}
*tmp = mbox
Re: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
On Thu, Apr 29, 2010 at 4:14 PM, Ohad Ben-Cohen o...@wizery.com wrote:
Hi Hari,
On Thu, Apr 29, 2010 at 3:44 PM, Kanigeri, Hari h-kanige...@ti.com wrote:
Ohad,
Looks like the conversion was missed in few places resulting in compile
warnings.
Thanks for pointing that out - that's really strange, it somehow
slipped in the rebase.
The interesting part is that because you compile for OMAP4 (SMP),
you got to see the warnings, while I didn't because I compiled for OMAP3
(in the UP case there's no real locking and both types of locks are
actually the same).
Please see the below fix. Let me know if you agree with the change.
Sure. I'll add that missing part back together with the review comments,
and submit a v2 patchset.
If anyone needs the complete patch now, the original can be found here:
http://dev.omapzoom.org/?p=tisyslink/kernel-syslink.git;a=commitdiff;h=8da0385a57cc470ee0a013b164fd3d2438455e79
Thanks,
Ohad.
Thanks again,
Ohad.
#
[PATCH] omap: mailbox: rwlocks to spinlock: compilation fix
fixed the missed rwlocks in few places resultion in following
compiler warning.
arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_startup':
arch/arm/plat-omap/mailbox.c:246: warning: passing argument 1 of
'_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:251: warning: passing argument 1 of
'_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:255: warning: passing argument 1 of
'_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_fini':
arch/arm/plat-omap/mailbox.c:301: warning: passing argument 1 of
'_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:306: warning: passing argument 1 of
'_raw_write_unlock' from incompatible pointer type
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
arch/arm/plat-omap/mailbox.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 27a8d98..d6a700d 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -243,16 +243,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
struct omap_mbox_queue *mq;
if (likely(mbox-ops-startup)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (!mbox_configured)
ret = mbox-ops-startup(mbox);
if (unlikely(ret)) {
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ret;
}
mbox_configured++;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
ret = request_irq(mbox-irq, mbox_interrupt, IRQF_SHARED,
@@ -298,12 +298,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
if (likely(mbox-ops-shutdown)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
if (!mbox_configured)
mbox-ops-shutdown(mbox);
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
}
--
-Original Message-
From: Ohad Ben-Cohen [mailto:o...@wizery.com]
Sent: Tuesday, April 27, 2010 12:56 PM
To: linux-omap@vger.kernel.org
Cc: Kanigeri, Hari; Hiroshi Doyu; Ohad Ben-Cohen
Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks
to spinlock
rwlocks are slower and have potential starvation issues so
spinlocks are generally preferred
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/mailbox.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c
b/arch/arm/plat-omap/mailbox.c index 08a2df7..d73d51a 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@
static struct workqueue_struct *mboxd;
static struct omap_mbox *mboxes;
-static DEFINE_RWLOCK(mboxes_lock);
+static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
@@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const
char *name)
struct omap_mbox *mbox;
int ret;
- read_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
mbox = *(find_mboxes(name));
if (mbox == NULL) {
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ERR_PTR(-ENOENT);
}
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
ret = omap_mbox_startup(mbox);
if (ret)
@@ -363,15 +363,15 @@ int omap_mbox_register(struct device
*parent, struct omap_mbox *mbox)
if (mbox-next)
return -EBUSY
[RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
This patchset introduces an approach to eliminate the direct calls to follow_page and to the low level cache APIs. The patchset works by caching the page information while memory is mapped, and then using that information later when needed instead of calling follow_page. The low level cache API is then replaced by the standard DMA API. A few key points in the current approach that I'd be happy to hear your feedback about: 1. The new mapping + page information is currently cached in the proc object, but it might fit better inside dmm map objects (by enhancing the DMM layer to support the required data caching, storing and retrieving). 2. The information is stored in a linked list. That's pretty fine as long as the number of memory mappings per application is not extremely high. If that assumption is wrong, a different underlying data structure might be better (hash table, priority tree, etc..). 3. Moving to standard DMA API completely changes the user's point of view; users should no longer think in terms of which cache manipulation is required, but instead, they should just tell dspbridge before a DMA transfer begins, and after it ends. Between the begin and end calls, the buffer belongs to the DSP and should not be accessed by the user. The patchset renames the flush ioctl to begin_dma_to_dsp and the invalidate ioctl to begin_dma_from_dsp. Both functions eventually call dma_map_sg, with the former requesting a DMA_BIDIRECTIONAL direction, and the latter requesting a DMA_FROM_DEVICE direction. In addition, the patchset adds two new APIs which calls dma_unmap_sg: end_dma_to_dsp and end_dma_from_dsp. Ideally, there would be only a single begin_dma command and a single end_dma one, which would accept an additional parameter that will determine the direction of the transfer. Such an approach would be more versatile and cleaner, but it would also break all user space apps that use dspbridge today. Notes: 1. During my tests, a few testsuite scenarios failed due to the fact that the test cases called flush/invalidate on a memory buffer it didn't map beforehand. I consider that as a misuse of the API, and thus a testsuite error. 2. The global bridge device struct is used by adding an 'extern' to proc. This issue should be handled in a different patch series (the struct should not be global. instead, it should be accessible to the dspbridge code via one of the context objects. This way we will also be able to transform pr_* prints to dev_* prints). 3. The patchset is not yet rebased on the latest dspbridge commits. It's currently based on 13e2573f2162b76d45313e790fc67a0d7672930b. 4. The patchset was tested with the bridge testsuite and the dmm sample application running on ZOOM3 / 2.6.33. I'd like to thank Hari and Fernando for initial review of this patchset. Please review and let me know your comments. Thanks, Ohad. --- If you want, you can also reach me at ohadb at ti dot com . Ohad Ben-Cohen (6): DSPBRIDGE: add memory_map_info to PROC DSPBRIDGE: remember mapping and page info in proc_map DSPBRIDGE: remove mapping information in proc_unmap DSPBRIDGE: do not call follow_page DSPBRIDGE: do not use low level cache manipulation API DSPBRIDGE: add dspbridge API to mark end of DMA arch/arm/plat-omap/include/dspbridge/_dcd.h |6 +- arch/arm/plat-omap/include/dspbridge/proc.h |8 +- arch/arm/plat-omap/include/dspbridge/wcdioctl.h |6 +- arch/arm/plat-omap/include/dspbridge/wmd.h |3 +- drivers/dsp/bridge/pmgr/wcd.c | 39 ++- drivers/dsp/bridge/rmgr/proc.c | 440 --- drivers/dsp/bridge/wmd/io_sm.c | 10 +- drivers/dsp/bridge/wmd/tiomap3430.c | 11 +- 8 files changed, 448 insertions(+), 75 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/6] DSPBRIDGE: add memory_map_info to PROC
Add the memory_map_info structure which will be used to maintain
mapping and page information. Every memory mapping requested by the
MM application will be remembered and kept in a linked list
inside the proc object.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
drivers/dsp/bridge/rmgr/proc.c | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index 1f7dd09..95a194a 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -17,6 +17,8 @@
*/
/* Host OS */
+#include linux/list.h
+#include linux/spinlock.h
#include dspbridge/host_os.h
/* --- DSP/BIOS Bridge */
@@ -102,6 +104,20 @@ struct proc_object {
struct bridge_drv_interface *intf_fxns; /* Function interface to WMD */
char *psz_last_coff;
struct list_head proc_list;
+
+ /* memory mapping information */
+ struct list_head maps;
+ spinlock_t maps_lock;
+};
+
+/* used to cache memory mapping information */
+struct memory_map_info {
+ struct list_head node;
+ struct page **pages;
+ u32 mpu_addr;
+ u32 dsp_addr;
+ u32 size;
+ u32 num_usr_pgs;
};
static u32 refs;
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/6] DSPBRIDGE: remember mapping and page info in proc_map
Every time the MM application calls proc_map to map
a memory area, remember the details of that mapping,
together with the related page structures.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/include/dspbridge/wmd.h |3 +-
drivers/dsp/bridge/rmgr/proc.c | 55 ++--
drivers/dsp/bridge/wmd/io_sm.c | 10 +++--
drivers/dsp/bridge/wmd/tiomap3430.c| 11 +-
4 files changed, 68 insertions(+), 11 deletions(-)
diff --git a/arch/arm/plat-omap/include/dspbridge/wmd.h
b/arch/arm/plat-omap/include/dspbridge/wmd.h
index f9883db..37ee0f3 100644
--- a/arch/arm/plat-omap/include/dspbridge/wmd.h
+++ b/arch/arm/plat-omap/include/dspbridge/wmd.h
@@ -182,7 +182,8 @@ typedef dsp_status(*fxn_brd_memwrite) (struct
wmd_dev_context
typedef dsp_status(*fxn_brd_memmap) (struct wmd_dev_context
* hDevContext, u32 ul_mpu_addr,
u32 ulVirtAddr, u32 ul_num_bytes,
-u32 ulMapAttrs);
+u32 ulMapAttrs,
+struct page **mapped_pages);
/*
* bridge_brd_mem_un_map
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index 95a194a..b03232f 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -130,6 +130,45 @@ static s32 get_envp_count(char **envp);
static char **prepend_envp(char **new_envp, char **envp, s32 envp_elems,
s32 cnew_envp, char *szVar);
+/* remember mapping information */
+static struct memory_map_info *add_mapping_info(struct proc_object *pr_obj,
+ u32 mpu_addr, u32 dsp_addr, u32 size)
+{
+ struct memory_map_info *map_info;
+
+ u32 num_usr_pgs = size / PG_SIZE4K;
+
+ pr_debug(%s: adding map info: mpu_addr 0x%x virt 0x%x size 0x%x\n,
+ __func__, mpu_addr,
+ dsp_addr, size);
+
+ map_info = kzalloc(sizeof(struct memory_map_info), GFP_KERNEL);
+ if (!map_info) {
+ pr_err(%s: kzalloc failed\n, __func__);
+ return NULL;
+ }
+ INIT_LIST_HEAD(map_info-node);
+
+ map_info-pages = kcalloc(num_usr_pgs, sizeof(struct page *),
+ GFP_KERNEL);
+ if (!map_info-pages) {
+ pr_err(%s: kzalloc failed\n, __func__);
+ kfree(map_info);
+ return NULL;
+ }
+
+ map_info-mpu_addr = mpu_addr;
+ map_info-dsp_addr = dsp_addr;
+ map_info-size = size;
+ map_info-num_usr_pgs = num_usr_pgs;
+
+ spin_lock(pr_obj-maps_lock);
+ list_add(map_info-node, pr_obj-maps);
+ spin_unlock(pr_obj-maps_lock);
+
+ return map_info;
+}
+
/*
* proc_attach
* Purpose:
@@ -185,6 +224,8 @@ proc_attach(u32 processor_id,
p_proc_object-process = current-tgid;
INIT_LIST_HEAD(p_proc_object-proc_list);
+ INIT_LIST_HEAD(p_proc_object-maps);
+ spin_lock_init(p_proc_object-maps_lock);
if (attr_in)
p_proc_object-utimeout = attr_in-utimeout;
@@ -1091,6 +1132,7 @@ dsp_status proc_map(void *hprocessor, void *pmpu_addr,
u32 ul_size,
dsp_status status = DSP_SOK;
struct proc_object *p_proc_object = (struct proc_object *)hprocessor;
struct dmm_map_object *map_obj;
+ struct memory_map_info *map_info;
#ifdef CONFIG_BRIDGE_CACHE_LINE_CHECK
if ((ul_map_attr BUFMODE_MASK) != RBUF) {
@@ -1121,10 +1163,15 @@ dsp_status proc_map(void *hprocessor, void *pmpu_addr,
u32 ul_size,
/* Add mapping to the page tables. */
if (DSP_SUCCEEDED(status)) {
-
- status = (*p_proc_object-intf_fxns-pfn_brd_mem_map)
- (p_proc_object-hwmd_context, pa_align, va_align,
-size_align, ul_map_attr);
+ /* cache mapping information */
+ map_info = add_mapping_info(p_proc_object, pa_align, va_align,
+ size_align);
+ if (!map_info)
+ status = DSP_EMEMORY;
+ else
+ status = (*p_proc_object-intf_fxns-pfn_brd_mem_map)
+ (p_proc_object-hwmd_context, pa_align, va_align,
+size_align, ul_map_attr, map_info-pages);
}
if (DSP_SUCCEEDED(status)) {
/* Mapped address = MSB of VA | LSB of PA */
diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
index 1b5d977..d8cd9e3 100644
--- a/drivers/dsp/bridge/wmd/io_sm.c
+++ b/drivers/dsp/bridge/wmd/io_sm.c
@@ -505,7 +505,8 @@ dsp_status bridge_io_on_loaded(struct io_mgr *hio_mgr
[RFC/PATCH 3/6] DSPBRIDGE: remove mapping information in proc_unmap
Clean up all mapping information resources whenever
a buffer is unmapped.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
drivers/dsp/bridge/rmgr/proc.c | 43
1 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index b03232f..ebb11b1 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -169,6 +169,46 @@ static struct memory_map_info *add_mapping_info(struct
proc_object *pr_obj,
return map_info;
}
+static int match_exact_map_info(struct memory_map_info *map_info,
+ u32 dsp_addr, u32 size)
+{
+ return map_info-dsp_addr == dsp_addr
+ map_info-size == size;
+}
+
+static void remove_mapping_information(struct proc_object *pr_obj,
+ u32 dsp_addr, u32 size)
+{
+ struct memory_map_info *map_info;
+ struct list_head *iter;
+
+ pr_debug(%s: looking for virt 0x%x size 0x%x\n, __func__,
+ dsp_addr, size);
+
+ spin_lock(pr_obj-maps_lock);
+ list_for_each(iter, pr_obj-maps) {
+ map_info = list_entry(iter, struct memory_map_info, node);
+ pr_debug(%s: candidate: mpu_addr 0x%x virt 0x%x size 0x%x\n,
+ __func__,
+ map_info-mpu_addr,
+ map_info-dsp_addr,
+ map_info-size);
+
+ if (match_exact_map_info(map_info, dsp_addr, size)) {
+ pr_debug(%s: match, deleting map info\n, __func__);
+ list_del(map_info-node);
+ kfree(map_info-pages);
+ kfree(map_info);
+ goto out;
+ }
+ pr_debug(%s: candidate didn't match\n, __func__);
+ }
+
+ pr_err(%s: failed to find given map info\n, __func__);
+out:
+ spin_unlock(pr_obj-maps_lock);
+}
+
/*
* proc_attach
* Purpose:
@@ -1508,6 +1548,9 @@ dsp_status proc_un_map(void *hprocessor, void *map_addr,
status = (*p_proc_object-intf_fxns-pfn_brd_mem_un_map)
(p_proc_object-hwmd_context, va_align, size_align);
}
+
+ remove_mapping_information(p_proc_object, va_align, size_align);
+
mutex_unlock(proc_lock);
if (DSP_FAILED(status))
goto func_end;
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 4/6] DSPBRIDGE: do not call follow_page
Eliminate the call to follow_page. Instead, use the page
information that was kept during the proc_map call.
This also has the advantage that users can now only
specify memory areas that were previously mapped.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
drivers/dsp/bridge/rmgr/proc.c | 143 ++--
1 files changed, 93 insertions(+), 50 deletions(-)
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index ebb11b1..bbc7e0f 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -209,6 +209,71 @@ out:
spin_unlock(pr_obj-maps_lock);
}
+static int match_containing_map_info(struct memory_map_info *map_info,
+ u32 mpu_addr, u32 size)
+{
+ u32 map_info_end = map_info-mpu_addr + map_info-size;
+
+ return mpu_addr = map_info-mpu_addr
+ mpu_addr + size = map_info_end;
+}
+
+static struct memory_map_info *find_containing_mapping(
+ struct proc_object *pr_obj,
+ u32 mpu_addr, u32 size)
+{
+ struct memory_map_info *map_info;
+ struct list_head *iter;
+ pr_debug(%s: looking for mpu_addr 0x%x size 0x%x\n, __func__,
+ mpu_addr, size);
+
+ spin_lock(pr_obj-maps_lock);
+ list_for_each(iter, pr_obj-maps) {
+ map_info = list_entry(iter, struct memory_map_info, node);
+ pr_debug(%s: candidate: mpu_addr 0x%x virt 0x%x size 0x%x\n,
+ __func__,
+ map_info-mpu_addr,
+ map_info-dsp_addr,
+ map_info-size);
+ if (match_containing_map_info(map_info, mpu_addr, size)) {
+ pr_debug(%s: match!\n, __func__);
+ goto out;
+ }
+
+ pr_debug(%s: no match!\n, __func__);
+ }
+
+ map_info = NULL;
+out:
+ spin_unlock(pr_obj-maps_lock);
+ return map_info;
+}
+
+static int find_first_page_in_cache(struct memory_map_info *map_info,
+ unsigned long mpu_addr)
+{
+ u32 mapped_base_page = map_info-mpu_addr PAGE_SHIFT;
+ u32 requested_base_page = mpu_addr PAGE_SHIFT;
+ int pg_index = requested_base_page - mapped_base_page;
+
+ if (pg_index 0 || pg_index = map_info-num_usr_pgs) {
+ pr_err(%s: failed (got %d)\n, __func__, pg_index);
+ return -1;
+ }
+
+ pr_debug(%s: first page is %d\n, __func__, pg_index);
+ return pg_index;
+}
+
+static inline struct page *get_mapping_page(struct memory_map_info *map_info,
+ int pg_i)
+{
+ if (pg_i 0 || pg_i = map_info-num_usr_pgs)
+ return NULL;
+
+ return map_info-pages[pg_i];
+}
+
/*
* proc_attach
* Purpose:
@@ -561,23 +626,30 @@ dsp_status proc_enum_nodes(void *hprocessor, void
**node_tab,
}
/* Cache operation against kernel address instead of users */
-static int memory_sync_page(struct vm_area_struct *vma, unsigned long start,
- ssize_t len, enum dsp_flushtype ftype)
+static int memory_sync_page(struct memory_map_info *map_info,
+ unsigned long start, ssize_t len, enum dsp_flushtype ftype)
{
struct page *page;
void *kaddr;
unsigned long offset;
ssize_t rest;
+ int pg_i;
+
+ pg_i = find_first_page_in_cache(map_info, start);
+ if (pg_i 0) {
+ pr_err(%s: failed to find first page in cache\n, __func__);
+ return -EINVAL;
+ }
while (len) {
- page = follow_page(vma, start, FOLL_GET);
+ page = get_mapping_page(map_info, pg_i);
if (!page) {
pr_err(%s: no page for %08lx\n, __func__, start);
return -EINVAL;
} else if (IS_ERR(page)) {
pr_err(%s: err page for %08lx(%lu)\n, __func__, start,
- IS_ERR(page));
- return IS_ERR(page);
+ PTR_ERR(page));
+ return PTR_ERR(page);
}
offset = start ~PAGE_MASK;
@@ -586,59 +658,22 @@ static int memory_sync_page(struct vm_area_struct *vma,
unsigned long start,
mem_flush_cache(kaddr, rest, ftype);
kunmap(page);
- put_page(page);
len -= rest;
start += rest;
+ pg_i++;
}
return 0;
}
-/* Check if the given area blongs to process virtul memory address space */
-static int
[RFC/PATCH 5/6] DSPBRIDGE: do not use low level cache manipulation API
Instead of using low level cache manipulation API,
use the standard DMA API. This changes the concept
of the dspbridge cache API a little, hence the naming changes:
* Flush marks the beginning of a DMA transfer from the MPU
to the DSP.
* Invalidate marks the beginning of a DMA transfer from the DSP
to the MPU.
Both of these actions eventually build a scatter gatter list
using the page information that was kept during proc_map,
and feed it to the standard dma_map_sg API.
Note that now users cannot manipulate the cache state of any random
address; if the buffer is not part of a previous memory mapping of that
application, the request is denied.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/include/dspbridge/_dcd.h |4 +-
arch/arm/plat-omap/include/dspbridge/proc.h |4 +-
arch/arm/plat-omap/include/dspbridge/wcdioctl.h |4 +-
drivers/dsp/bridge/pmgr/wcd.c | 12 +-
drivers/dsp/bridge/rmgr/proc.c | 152 ++-
5 files changed, 134 insertions(+), 42 deletions(-)
diff --git a/arch/arm/plat-omap/include/dspbridge/_dcd.h
b/arch/arm/plat-omap/include/dspbridge/_dcd.h
index 1350feb..0af4a31 100644
--- a/arch/arm/plat-omap/include/dspbridge/_dcd.h
+++ b/arch/arm/plat-omap/include/dspbridge/_dcd.h
@@ -110,9 +110,9 @@ extern u32 procwrap_reserve_memory(union Trapped_Args
*args, void *pr_ctxt);
extern u32 procwrap_un_reserve_memory(union Trapped_Args *args, void *pr_ctxt);
extern u32 procwrap_map(union Trapped_Args *args, void *pr_ctxt);
extern u32 procwrap_un_map(union Trapped_Args *args, void *pr_ctxt);
-extern u32 procwrap_flush_memory(union Trapped_Args *args, void *pr_ctxt);
+extern u32 procwrap_begin_dma_to_dsp(union Trapped_Args *args, void *pr_ctxt);
extern u32 procwrap_stop(union Trapped_Args *args, void *pr_ctxt);
-extern u32 procwrap_invalidate_memory(union Trapped_Args *args, void *pr_ctxt);
+extern u32 procwrap_begin_dma_from_dsp(union Trapped_Args *args, void
*pr_ctxt);
/* NODE wrapper functions */
extern u32 nodewrap_allocate(union Trapped_Args *args, void *pr_ctxt);
diff --git a/arch/arm/plat-omap/include/dspbridge/proc.h
b/arch/arm/plat-omap/include/dspbridge/proc.h
index 0707739..f8450a6 100644
--- a/arch/arm/plat-omap/include/dspbridge/proc.h
+++ b/arch/arm/plat-omap/include/dspbridge/proc.h
@@ -472,7 +472,7 @@ extern dsp_status proc_stop(void *hprocessor);
* Details:
* All the arguments are currently ignored.
*/
-extern dsp_status proc_flush_memory(void *hprocessor,
+extern dsp_status proc_begin_dma_to_dsp(void *hprocessor,
void *pmpu_addr, u32 ul_size, u32 ul_flags);
/*
@@ -493,7 +493,7 @@ extern dsp_status proc_flush_memory(void *hprocessor,
* Details:
* All the arguments are currently ignored.
*/
-extern dsp_status proc_invalidate_memory(void *hprocessor,
+extern dsp_status proc_begin_dma_from_dsp(void *hprocessor,
void *pmpu_addr, u32 ul_size);
/*
diff --git a/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
b/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
index b6a4dda..aba2078 100644
--- a/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
+++ b/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
@@ -452,9 +452,9 @@ union Trapped_Args {
#define PROC_UNRSVMEM _IOW(DB, DB_IOC(DB_PROC, 11), unsigned long)
#define PROC_MAPMEM_IOWR(DB, DB_IOC(DB_PROC, 12), unsigned long)
#define PROC_UNMAPMEM _IOR(DB, DB_IOC(DB_PROC, 13), unsigned long)
-#define PROC_FLUSHMEMORY _IOW(DB, DB_IOC(DB_PROC, 14), unsigned long)
+#define PROC_BEGINDMATODSP _IOW(DB, DB_IOC(DB_PROC, 14), unsigned long)
#define PROC_STOP _IOWR(DB, DB_IOC(DB_PROC, 15), unsigned long)
-#define PROC_INVALIDATEMEMORY _IOW(DB, DB_IOC(DB_PROC, 16), unsigned long)
+#define PROC_BEGINDMAFROMDSP _IOW(DB, DB_IOC(DB_PROC, 16), unsigned long)
/* NODE Module */
#define NODE_ALLOCATE _IOWR(DB, DB_IOC(DB_NODE, 0), unsigned long)
diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index 15a05a6..89243f1 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -111,9 +111,9 @@ static struct wcd_cmd proc_cmd[] = {
{procwrap_un_reserve_memory}, /* PROC_UNRSVMEM */
{procwrap_map}, /* PROC_MAPMEM */
{procwrap_un_map}, /* PROC_UNMAPMEM */
- {procwrap_flush_memory},/* PROC_FLUSHMEMORY */
+ {procwrap_begin_dma_to_dsp},/* PROC_BEGINDMATODSP */
{procwrap_stop},/* PROC_STOP */
- {procwrap_invalidate_memory}, /* PROC_INVALIDATEMEMORY */
+ {procwrap_begin_dma_from_dsp}, /* PROC_BEGINDMAFROMDSP */
};
/* NODE wrapper functions */
@@ -680,7 +680,7 @@ u32 procwrap_enum_node_info(union Trapped_Args *args, void
*pr_ctxt)
/*
* procwrap_flush_memory
*/
-u32
[RFC/PATCH 6/6] DSPBRIDGE: add dspbridge API to mark end of DMA
Standard DMA API is built upon the notion of buffer ownership.
The buffer is either exclusively owned by the MPU (and therefore
may be accessed by it) or exclusively owned by the DMA device
(in our case, the dsp remote processor).
This patch adds the missing dspbridge API with which the MM
application can mark the end of a DMA transfer (and thus regain
ownership of the buffer).
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/include/dspbridge/_dcd.h |2 +
arch/arm/plat-omap/include/dspbridge/proc.h |4 +
arch/arm/plat-omap/include/dspbridge/wcdioctl.h |2 +
drivers/dsp/bridge/pmgr/wcd.c | 27 +++
drivers/dsp/bridge/rmgr/proc.c | 87 +++
5 files changed, 122 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/include/dspbridge/_dcd.h
b/arch/arm/plat-omap/include/dspbridge/_dcd.h
index 0af4a31..adfcf67 100644
--- a/arch/arm/plat-omap/include/dspbridge/_dcd.h
+++ b/arch/arm/plat-omap/include/dspbridge/_dcd.h
@@ -113,6 +113,8 @@ extern u32 procwrap_un_map(union Trapped_Args *args, void
*pr_ctxt);
extern u32 procwrap_begin_dma_to_dsp(union Trapped_Args *args, void *pr_ctxt);
extern u32 procwrap_stop(union Trapped_Args *args, void *pr_ctxt);
extern u32 procwrap_begin_dma_from_dsp(union Trapped_Args *args, void
*pr_ctxt);
+extern u32 procwrap_end_dma_to_dsp(union Trapped_Args *args, void *pr_ctxt);
+extern u32 procwrap_end_dma_from_dsp(union Trapped_Args *args, void *pr_ctxt);
/* NODE wrapper functions */
extern u32 nodewrap_allocate(union Trapped_Args *args, void *pr_ctxt);
diff --git a/arch/arm/plat-omap/include/dspbridge/proc.h
b/arch/arm/plat-omap/include/dspbridge/proc.h
index f8450a6..7a7b8e8 100644
--- a/arch/arm/plat-omap/include/dspbridge/proc.h
+++ b/arch/arm/plat-omap/include/dspbridge/proc.h
@@ -474,6 +474,8 @@ extern dsp_status proc_stop(void *hprocessor);
*/
extern dsp_status proc_begin_dma_to_dsp(void *hprocessor,
void *pmpu_addr, u32 ul_size, u32 ul_flags);
+extern dsp_status proc_end_dma_to_dsp(void *hprocessor,
+ void *pmpu_addr, u32 ul_size, u32 ul_flags);
/*
* proc_invalidate_memory
@@ -495,6 +497,8 @@ extern dsp_status proc_begin_dma_to_dsp(void *hprocessor,
*/
extern dsp_status proc_begin_dma_from_dsp(void *hprocessor,
void *pmpu_addr, u32 ul_size);
+extern dsp_status proc_end_dma_from_dsp(void *hprocessor,
+void *pmpu_addr, u32 ul_size);
/*
* proc_map
diff --git a/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
b/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
index aba2078..a6debf2 100644
--- a/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
+++ b/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
@@ -455,6 +455,8 @@ union Trapped_Args {
#define PROC_BEGINDMATODSP _IOW(DB, DB_IOC(DB_PROC, 14), unsigned long)
#define PROC_STOP _IOWR(DB, DB_IOC(DB_PROC, 15), unsigned long)
#define PROC_BEGINDMAFROMDSP _IOW(DB, DB_IOC(DB_PROC, 16), unsigned long)
+#define PROC_ENDDMATODSP _IOW(DB, DB_IOC(DB_PROC, 17), unsigned long)
+#define PROC_ENDDMAFROMDSP _IOW(DB, DB_IOC(DB_PROC, 18), unsigned long)
/* NODE Module */
#define NODE_ALLOCATE _IOWR(DB, DB_IOC(DB_NODE, 0), unsigned long)
diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index 89243f1..ae6e8ab 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -114,6 +114,8 @@ static struct wcd_cmd proc_cmd[] = {
{procwrap_begin_dma_to_dsp},/* PROC_BEGINDMATODSP */
{procwrap_stop},/* PROC_STOP */
{procwrap_begin_dma_from_dsp}, /* PROC_BEGINDMAFROMDSP */
+ {procwrap_end_dma_to_dsp}, /* PROC_ENDDMATODSP */
+ {procwrap_end_dma_from_dsp},/* PROC_ENDDMAFROMDSP */
};
/* NODE wrapper functions */
@@ -709,6 +711,31 @@ u32 procwrap_begin_dma_from_dsp(union Trapped_Args *args,
void *pr_ctxt)
return status;
}
+u32 procwrap_end_dma_to_dsp(union Trapped_Args *args, void *pr_ctxt)
+{
+ dsp_status status;
+
+ if (args-args_proc_flushmemory.ul_flags
+ PROC_WRITEBACK_INVALIDATE_MEM)
+ return DSP_EINVALIDARG;
+
+ status = proc_end_dma_to_dsp(args-args_proc_flushmemory.hprocessor,
+ args-args_proc_flushmemory.pmpu_addr,
+ args-args_proc_flushmemory.ul_size,
+ args-args_proc_flushmemory.ul_flags);
+ return status;
+}
+
+u32 procwrap_end_dma_from_dsp(union Trapped_Args *args, void *pr_ctxt)
+{
+ dsp_status status;
+
+ status =
+ proc_end_dma_from_dsp(args-args_proc_invalidatememory.hprocessor,
+ args
Re: [RFC/PATCH 5/6] DSPBRIDGE: do not use low level cache manipulation API
On Sat, May 1, 2010 at 11:44 PM, Ohad Ben-Cohen o...@wizery.com wrote:
Instead of using low level cache manipulation API,
use the standard DMA API. This changes the concept
of the dspbridge cache API a little, hence the naming changes:
* Flush marks the beginning of a DMA transfer from the MPU
to the DSP.
* Invalidate marks the beginning of a DMA transfer from the DSP
to the MPU.
Both of these actions eventually build a scatter gatter list
using the page information that was kept during proc_map,
and feed it to the standard dma_map_sg API.
Note that now users cannot manipulate the cache state of any random
address; if the buffer is not part of a previous memory mapping of that
application, the request is denied.
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/include/dspbridge/_dcd.h | 4 +-
arch/arm/plat-omap/include/dspbridge/proc.h | 4 +-
arch/arm/plat-omap/include/dspbridge/wcdioctl.h | 4 +-
drivers/dsp/bridge/pmgr/wcd.c | 12 +-
drivers/dsp/bridge/rmgr/proc.c | 152
++-
5 files changed, 134 insertions(+), 42 deletions(-)
diff --git a/arch/arm/plat-omap/include/dspbridge/_dcd.h
b/arch/arm/plat-omap/include/dspbridge/_dcd.h
index 1350feb..0af4a31 100644
--- a/arch/arm/plat-omap/include/dspbridge/_dcd.h
+++ b/arch/arm/plat-omap/include/dspbridge/_dcd.h
@@ -110,9 +110,9 @@ extern u32 procwrap_reserve_memory(union Trapped_Args
*args, void *pr_ctxt);
extern u32 procwrap_un_reserve_memory(union Trapped_Args *args, void
*pr_ctxt);
extern u32 procwrap_map(union Trapped_Args *args, void *pr_ctxt);
extern u32 procwrap_un_map(union Trapped_Args *args, void *pr_ctxt);
-extern u32 procwrap_flush_memory(union Trapped_Args *args, void *pr_ctxt);
+extern u32 procwrap_begin_dma_to_dsp(union Trapped_Args *args, void
*pr_ctxt);
extern u32 procwrap_stop(union Trapped_Args *args, void *pr_ctxt);
-extern u32 procwrap_invalidate_memory(union Trapped_Args *args, void
*pr_ctxt);
+extern u32 procwrap_begin_dma_from_dsp(union Trapped_Args *args, void
*pr_ctxt);
/* NODE wrapper functions */
extern u32 nodewrap_allocate(union Trapped_Args *args, void *pr_ctxt);
diff --git a/arch/arm/plat-omap/include/dspbridge/proc.h
b/arch/arm/plat-omap/include/dspbridge/proc.h
index 0707739..f8450a6 100644
--- a/arch/arm/plat-omap/include/dspbridge/proc.h
+++ b/arch/arm/plat-omap/include/dspbridge/proc.h
@@ -472,7 +472,7 @@ extern dsp_status proc_stop(void *hprocessor);
* Details:
* All the arguments are currently ignored.
*/
-extern dsp_status proc_flush_memory(void *hprocessor,
+extern dsp_status proc_begin_dma_to_dsp(void *hprocessor,
void *pmpu_addr, u32 ul_size, u32
ul_flags);
/*
@@ -493,7 +493,7 @@ extern dsp_status proc_flush_memory(void *hprocessor,
* Details:
* All the arguments are currently ignored.
*/
-extern dsp_status proc_invalidate_memory(void *hprocessor,
+extern dsp_status proc_begin_dma_from_dsp(void *hprocessor,
void *pmpu_addr, u32 ul_size);
/*
diff --git a/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
b/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
index b6a4dda..aba2078 100644
--- a/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
+++ b/arch/arm/plat-omap/include/dspbridge/wcdioctl.h
@@ -452,9 +452,9 @@ union Trapped_Args {
#define PROC_UNRSVMEM _IOW(DB, DB_IOC(DB_PROC, 11), unsigned long)
#define PROC_MAPMEM _IOWR(DB, DB_IOC(DB_PROC, 12), unsigned long)
#define PROC_UNMAPMEM _IOR(DB, DB_IOC(DB_PROC, 13), unsigned long)
-#define PROC_FLUSHMEMORY _IOW(DB, DB_IOC(DB_PROC, 14), unsigned long)
+#define PROC_BEGINDMATODSP _IOW(DB, DB_IOC(DB_PROC, 14), unsigned long)
#define PROC_STOP _IOWR(DB, DB_IOC(DB_PROC, 15), unsigned long)
-#define PROC_INVALIDATEMEMORY _IOW(DB, DB_IOC(DB_PROC, 16), unsigned long)
+#define PROC_BEGINDMAFROMDSP _IOW(DB, DB_IOC(DB_PROC, 16), unsigned long)
/* NODE Module */
#define NODE_ALLOCATE _IOWR(DB, DB_IOC(DB_NODE, 0), unsigned long)
diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index 15a05a6..89243f1 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -111,9 +111,9 @@ static struct wcd_cmd proc_cmd[] = {
{procwrap_un_reserve_memory}, /* PROC_UNRSVMEM */
{procwrap_map}, /* PROC_MAPMEM */
{procwrap_un_map}, /* PROC_UNMAPMEM */
- {procwrap_flush_memory}, /* PROC_FLUSHMEMORY */
+ {procwrap_begin_dma_to_dsp}, /* PROC_BEGINDMATODSP */
{procwrap_stop}, /* PROC_STOP */
- {procwrap_invalidate_memory}, /* PROC_INVALIDATEMEMORY */
+ {procwrap_begin_dma_from_dsp}, /* PROC_BEGINDMAFROMDSP */
};
/* NODE wrapper functions
[PATCH v2 0/4] omap: mailbox: cleanup simplify
This series include comments from Hiroshi and Hari (thanks!). Changes since v1: - add mbox_kfifo_size module parameter - WARN_ON kfifo anomalies - remove redundant blkdev.h file - fix rwlocks-spinlock conversion Thanks, Ohad. --- If you want, you can also reach me at ohadb at ti dot com . Ohad Ben-Cohen (4): omap: mailbox: convert rwlocks to spinlock omap: mailbox cleanup: split MODULE_AUTHOR line omap: mailbox: fix reverse likeliness omap: mailbox: convert block api to kfifo arch/arm/mach-omap2/mailbox.c |3 +- arch/arm/plat-omap/Kconfig|9 ++ arch/arm/plat-omap/include/plat/mailbox.h |4 +- arch/arm/plat-omap/mailbox.c | 144 + 4 files changed, 79 insertions(+), 81 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] omap: mailbox: convert rwlocks to spinlock
rwlocks are slower and have potential starvation issues
therefore spinlocks are generally preferred.
see also: http://lwn.net/Articles/364583/
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Signed-off-by: Kanigeri Hari h-kanige...@ti.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/mailbox.c | 30 +++---
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 08a2df7..af3a6ac 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@
static struct workqueue_struct *mboxd;
static struct omap_mbox *mboxes;
-static DEFINE_RWLOCK(mboxes_lock);
+static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
@@ -249,16 +249,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
struct omap_mbox_queue *mq;
if (likely(mbox-ops-startup)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (!mbox_configured)
ret = mbox-ops-startup(mbox);
if (unlikely(ret)) {
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ret;
}
mbox_configured++;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
ret = request_irq(mbox-irq, mbox_interrupt, IRQF_SHARED,
@@ -304,12 +304,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
if (unlikely(mbox-ops-shutdown)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
if (!mbox_configured)
mbox-ops-shutdown(mbox);
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
}
@@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const char *name)
struct omap_mbox *mbox;
int ret;
- read_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
mbox = *(find_mboxes(name));
if (mbox == NULL) {
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ERR_PTR(-ENOENT);
}
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
ret = omap_mbox_startup(mbox);
if (ret)
@@ -363,15 +363,15 @@ int omap_mbox_register(struct device *parent, struct
omap_mbox *mbox)
if (mbox-next)
return -EBUSY;
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
tmp = find_mboxes(mbox-name);
if (*tmp) {
ret = -EBUSY;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
goto err_find;
}
*tmp = mbox;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return 0;
@@ -384,18 +384,18 @@ int omap_mbox_unregister(struct omap_mbox *mbox)
{
struct omap_mbox **tmp;
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
tmp = mboxes;
while (*tmp) {
if (mbox == *tmp) {
*tmp = mbox-next;
mbox-next = NULL;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return 0;
}
tmp = (*tmp)-next;
}
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return -EINVAL;
}
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line
use multiple MODULE_AUTHOR lines for multiple authors Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- If you want, you can also reach me at ohadb at ti dot com . arch/arm/mach-omap2/mailbox.c |3 ++- arch/arm/plat-omap/mailbox.c |3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 318f363..763272c 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -486,5 +486,6 @@ module_exit(omap2_mbox_exit); MODULE_LICENSE(GPL v2); MODULE_DESCRIPTION(omap mailbox: omap2/3/4 architecture specific functions); -MODULE_AUTHOR(Hiroshi DOYU hiroshi.d...@nokia.com, Paul Mundt); +MODULE_AUTHOR(Hiroshi DOYU hiroshi.d...@nokia.com); +MODULE_AUTHOR(Paul Mundt); MODULE_ALIAS(platform:DRV_NAME); diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index af3a6ac..5140efc 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -419,4 +419,5 @@ module_exit(omap_mbox_exit); MODULE_LICENSE(GPL v2); MODULE_DESCRIPTION(omap mailbox: interrupt driven messaging); -MODULE_AUTHOR(Toshihiro Kobayashi and Hiroshi DOYU); +MODULE_AUTHOR(Toshihiro Kobayashi); +MODULE_AUTHOR(Hiroshi DOYU); -- 1.6.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
On Sun, May 2, 2010 at 4:17 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, May 1, 2010 at 11:44 PM, Ohad Ben-Cohen o...@wizery.com wrote: This patchset introduces an approach to eliminate the direct calls to follow_page and to the low level cache APIs. The patchset works by caching the page information while memory is mapped, and then using that information later when needed instead of calling follow_page. The low level cache API is then replaced by the standard DMA API. Finally! Very interesting patch indeed. Thanks! A few key points in the current approach that I'd be happy to hear your feedback about: 1. The new mapping + page information is currently cached in the proc object, but it might fit better inside dmm map objects (by enhancing the DMM layer to support the required data caching, storing and retrieving). Sounds like a good idea. Great, I'll look into this. 2. The information is stored in a linked list. That's pretty fine as long as the number of memory mappings per application is not extremely high. If that assumption is wrong, a different underlying data structure might be better (hash table, priority tree, etc..). I think a linked list is fine for now. AFAIK only a limited number of mmaps happen at the same time, usually 4. Thanks for confirming. 3. Moving to standard DMA API completely changes the user's point of view; users should no longer think in terms of which cache manipulation is required, but instead, they should just tell dspbridge before a DMA transfer begins, and after it ends. Between the begin and end calls, the buffer belongs to the DSP and should not be accessed by the user. This is really nice. That API should have been that way since the beginning. The patchset renames the flush ioctl to begin_dma_to_dsp and the invalidate ioctl to begin_dma_from_dsp. Both functions eventually call dma_map_sg, with the former requesting a DMA_BIDIRECTIONAL direction, and the latter requesting a DMA_FROM_DEVICE direction. In addition, the patchset adds two new APIs which calls dma_unmap_sg: end_dma_to_dsp and end_dma_from_dsp. Ideally, there would be only a single begin_dma command and a single end_dma one, which would accept an additional parameter that will determine the direction of the transfer. Such an approach would be more versatile and cleaner, but it would also break all user space apps that use dspbridge today. If I understand correctly all user-space apps would be broken anyway because they are not issuing the end_dma calls. At the very least they need to be updated to use them. Basically you're right, but the patches currently silently allow today's user space to somehow work (because if you don't use dma bounce buffers and you feel lucky about speculative prefetching then you might get away with not calling dma unmap). I did that on purpose, to ease testing migration, but didn't explicitely said it because frankly it's just wrong. Also, in Nokia we patched the bridgedriver to be able to have the 3 operations available from user-space (clean, inv, and flush), so we would be very interested in having the direction of the transfer available. Thanks, that's important to know. What do you say about the following plan then: - I'll add a single pair of begin_dma and end_dma commands that will take the additional direction parameter as described above. I'll then covert the existing flush invalidate commands to use this begin_dma command supplying it the appropriate direction argument. - In a separate patch, I'll deprecate flush invalidate entirely (usage of this deprecated API will fail, resulting in a guiding error message). We get a sane and versatile (and platform-independent) implementation that always work, but breaks user space. If anyone would need to work with current user space, the deprecating patch can always be reverted locally to get back a flush invalidate implementations that (somehow) work. Anyway, I'm sure that breaking user space can be somewhat traumatic so let's make sure we get an across-the-board support for this. Thanks a lot for the comments! Ohad. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
On Mon, May 3, 2010 at 8:30 AM, Hiroshi DOYU hiroshi.d...@nokia.com wrote: Hi Ohad, From: ext Ohad Ben-Cohen o...@wizery.com Subject: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo Date: Sun, 2 May 2010 17:44:31 +0200 The underlying buffering implementation of mailbox is converted from block API to kfifo due to the simplicity and speed of kfifo. The default size of the kfifo buffer is set to 256 bytes. This value is configurable at compile time (via CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at runtime (via the omap_kfifo_size module parameter). mbox_kfifo_size? sure. thanks for noticing. Signed-off-by: Ohad Ben-Cohen o...@wizery.com Signed-off-by: Hari Kanigeri h-kanige...@ti.com --- If you want, you can also reach me at ohadb at ti dot com . arch/arm/plat-omap/Kconfig | 9 +++ arch/arm/plat-omap/include/plat/mailbox.h | 4 +- arch/arm/plat-omap/mailbox.c | 107 + 3 files changed, 58 insertions(+), 62 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
On Mon, May 3, 2010 at 9:07 AM, Hiroshi DOYU hiroshi.d...@nokia.com wrote: From: Hiroshi DOYU hiroshi.d...@nokia.com Subject: Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo Date: Mon, 03 May 2010 08:30:36 +0300 (EEST) Hi Ohad, From: ext Ohad Ben-Cohen o...@wizery.com Subject: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo Date: Sun, 2 May 2010 17:44:31 +0200 The underlying buffering implementation of mailbox is converted from block API to kfifo due to the simplicity and speed of kfifo. The default size of the kfifo buffer is set to 256 bytes. This value is configurable at compile time (via CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at runtime (via the omap_kfifo_size module parameter). mbox_kfifo_size? It may be also nice if there's some checking of 'mbox_kfifo_size % 4 == 0' along with 'CONFIG_OMAP_MBOX_KFIFO_SIZE % 4 == 0'? Good point. I'll send a new patch together with this: diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index a500ac4..fb3d452 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -21,6 +21,7 @@ * */ +#include linux/kernel.h #include linux/module.h #include linux/interrupt.h #include linux/device.h @@ -394,6 +395,10 @@ static int __init omap_mbox_init(void) if (!mboxd) return -ENOMEM; + /* kfifo size sanity check: alignment and minimal size */ + mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t)); + mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, sizeof(mbox_msg_t)); + return 0; } module_init(omap_mbox_init); Signed-off-by: Ohad Ben-Cohen o...@wizery.com Signed-off-by: Hari Kanigeri h-kanige...@ti.com --- If you want, you can also reach me at ohadb at ti dot com . arch/arm/plat-omap/Kconfig | 9 +++ arch/arm/plat-omap/include/plat/mailbox.h | 4 +- arch/arm/plat-omap/mailbox.c | 107 + 3 files changed, 58 insertions(+), 62 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] omap: mailbox: convert block api to kfifo
The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.
The default size of the kfifo buffer is set to 256 bytes.
This value is configurable at compile time (via
CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
runtime (via the mbox_kfifo_size module parameter).
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
if you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/Kconfig|9 +++
arch/arm/plat-omap/include/plat/mailbox.h |4 +-
arch/arm/plat-omap/mailbox.c | 112 +---
3 files changed, 63 insertions(+), 62 deletions(-)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 6da796e..f4b0029 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -106,6 +106,15 @@ config OMAP_MBOX_FWK
Say Y here if you want to use OMAP Mailbox framework support for
DSP, IVA1.0 and IVA2 in OMAP1/2/3.
+config OMAP_MBOX_KFIFO_SIZE
+ int Mailbox kfifo default buffer size (bytes)
+ depends on OMAP_MBOX_FWK
+ default 256
+ help
+ Specify the default size of mailbox's kfifo buffers (bytes).
+ This can also be changed at runtime (via the mbox_kfifo_size
+ module parameter).
+
config OMAP_IOMMU
tristate
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h
b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..0c3c4a5 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -5,8 +5,8 @@
#include linux/wait.h
#include linux/workqueue.h
-#include linux/blkdev.h
#include linux/interrupt.h
+#include linux/kfifo.h
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -42,7 +42,7 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
- struct request_queue*queue;
+ struct kfifofifo;
struct work_struct work;
struct tasklet_struct tasklet;
int (*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5309213..8d1581f 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -21,11 +21,14 @@
*
*/
+#include linux/kernel.h
#include linux/module.h
#include linux/interrupt.h
#include linux/device.h
#include linux/delay.h
#include linux/slab.h
+#include linux/kfifo.h
+#include linux/err.h
#include plat/mailbox.h
@@ -35,6 +38,10 @@ static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
+static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
+module_param(mbox_kfifo_size, uint, S_IRUGO);
+MODULE_PARM_DESC(mbox_kfifo_size, Size of omap's mailbox kfifo (bytes));
+
/* Mailbox FIFO handle functions */
static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
{
@@ -67,7 +74,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox,
omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +85,50 @@ static int __mbox_msg_send(struct omap_mbox *mbox,
mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox-txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox-txq-queue;
+ spin_lock(mq-lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(mq-fifo) sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(mq-fifo, (unsigned char *)msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(mbox-txq-tasklet);
- return 0;
+out:
+ spin_unlock(mq-lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox-txq-queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox-txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq-special);
- if (ret) {
+ while (kfifo_len(mq-fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq
Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness
On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren t...@atomide.com wrote:
* Ohad Ben-Cohen o...@wizery.com [100502 08:40]:
Fix reverse likeliness
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/mailbox.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..5309213 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
fail_alloc_txq:
free_irq(mbox-irq, mbox);
fail_request_irq:
- if (unlikely(mbox-ops-shutdown))
+ if (likely(mbox-ops-shutdown))
mbox-ops-shutdown(mbox);
return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
- if (unlikely(mbox-ops-shutdown)) {
+ if (likely(mbox-ops-shutdown)) {
spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
Does this code path need to be optimized? :)
How about just get rid of the (un)likely here?
I like this :)
If we're at it, there are additional cold-path (un)likely macros I
want to target:
From a921f13dadc02106a2cabb15e3813411d3fcb3a8 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen o...@wizery.com
Date: Sat, 17 Apr 2010 01:57:43 +0300
Subject: [PATCH 3/4] omap: mailbox: remove (un)likely macros from cold paths
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/mailbox.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..7c60550 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -248,12 +248,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
int ret = 0;
struct omap_mbox_queue *mq;
- if (likely(mbox-ops-startup)) {
+ if (mbox-ops-startup) {
spin_lock(mboxes_lock);
if (!mbox_configured)
ret = mbox-ops-startup(mbox);
- if (unlikely(ret)) {
+ if (ret) {
spin_unlock(mboxes_lock);
return ret;
}
@@ -263,7 +263,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
ret = request_irq(mbox-irq, mbox_interrupt, IRQF_SHARED,
mbox-name, mbox);
- if (unlikely(ret)) {
+ if (ret) {
printk(KERN_ERR
failed to register mailbox interrupt:%d\n, ret);
goto fail_request_irq;
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
fail_alloc_txq:
free_irq(mbox-irq, mbox);
fail_request_irq:
- if (unlikely(mbox-ops-shutdown))
+ if (mbox-ops-shutdown)
mbox-ops-shutdown(mbox);
return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
- if (unlikely(mbox-ops-shutdown)) {
+ if (mbox-ops-shutdown) {
spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
--
1.6.3.3
I'll wait a day or two for more comments, and send a v3 series.
Thanks,
Ohad.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness
On Wed, May 5, 2010 at 6:21 PM, Tony Lindgren t...@atomide.com wrote:
* Ohad Ben-Cohen o...@wizery.com [100504 04:42]:
On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren t...@atomide.com wrote:
* Ohad Ben-Cohen o...@wizery.com [100502 08:40]:
Fix reverse likeliness
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/mailbox.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..5309213 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
fail_alloc_txq:
free_irq(mbox-irq, mbox);
fail_request_irq:
- if (unlikely(mbox-ops-shutdown))
+ if (likely(mbox-ops-shutdown))
mbox-ops-shutdown(mbox);
return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
- if (unlikely(mbox-ops-shutdown)) {
+ if (likely(mbox-ops-shutdown)) {
spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
Does this code path need to be optimized? :)
How about just get rid of the (un)likely here?
I like this :)
If we're at it, there are additional cold-path (un)likely macros I
want to target:
Looks good to me.
Hiroshi, care to ack/nak all the mailbox and iommu patches you want
me to merge? Or if you have them in some git branch against mainline
-rc6 that would be cool too.
I'll post a v3 with all recent updated in a few minutes. I hope it helps.
Thanks,
Ohad.
We need to get the ones we want to merge into linux-arm-kernel for
review soon, I don't think they been posted there yet.
Regards,
Tony
From a921f13dadc02106a2cabb15e3813411d3fcb3a8 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen o...@wizery.com
Date: Sat, 17 Apr 2010 01:57:43 +0300
Subject: [PATCH 3/4] omap: mailbox: remove (un)likely macros from cold paths
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
arch/arm/plat-omap/mailbox.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..7c60550 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -248,12 +248,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
int ret = 0;
struct omap_mbox_queue *mq;
- if (likely(mbox-ops-startup)) {
+ if (mbox-ops-startup) {
spin_lock(mboxes_lock);
if (!mbox_configured)
ret = mbox-ops-startup(mbox);
- if (unlikely(ret)) {
+ if (ret) {
spin_unlock(mboxes_lock);
return ret;
}
@@ -263,7 +263,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
ret = request_irq(mbox-irq, mbox_interrupt, IRQF_SHARED,
mbox-name, mbox);
- if (unlikely(ret)) {
+ if (ret) {
printk(KERN_ERR
failed to register mailbox interrupt:%d\n, ret);
goto fail_request_irq;
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
fail_alloc_txq:
free_irq(mbox-irq, mbox);
fail_request_irq:
- if (unlikely(mbox-ops-shutdown))
+ if (mbox-ops-shutdown)
mbox-ops-shutdown(mbox);
return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
- if (unlikely(mbox-ops-shutdown)) {
+ if (mbox-ops-shutdown) {
spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
--
1.6.3.3
I'll wait a day or two for more comments, and send a v3 series.
Thanks,
Ohad.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] omap: mailbox: cleanup simplify
This series includes comments from Hiroshi and Tony (thanks!). Changes since v2: - add mbox_kfifo_size module parameter sanity checks - remove (un)likely macros from cold mailbox paths Thanks, Ohad. --- If you want, you can also reach me at ohadb at ti dot com . Ohad Ben-Cohen (4): omap: mailbox: convert rwlocks to spinlock omap: mailbox cleanup: split MODULE_AUTHOR line omap: mailbox: remove (un)likely macros from cold paths omap: mailbox: convert block api to kfifo arch/arm/mach-omap2/mailbox.c |3 +- arch/arm/plat-omap/Kconfig|9 ++ arch/arm/plat-omap/include/plat/mailbox.h |4 +- arch/arm/plat-omap/mailbox.c | 155 ++--- 4 files changed, 87 insertions(+), 84 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] omap: mailbox: convert rwlocks to spinlock
rwlocks are slower and have potential starvation issues
therefore spinlocks are generally preferred.
see also: http://lwn.net/Articles/364583/
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Signed-off-by: Kanigeri Hari h-kanige...@ti.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/mailbox.c | 30 +++---
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 08a2df7..af3a6ac 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@
static struct workqueue_struct *mboxd;
static struct omap_mbox *mboxes;
-static DEFINE_RWLOCK(mboxes_lock);
+static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
@@ -249,16 +249,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
struct omap_mbox_queue *mq;
if (likely(mbox-ops-startup)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (!mbox_configured)
ret = mbox-ops-startup(mbox);
if (unlikely(ret)) {
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ret;
}
mbox_configured++;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
ret = request_irq(mbox-irq, mbox_interrupt, IRQF_SHARED,
@@ -304,12 +304,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
if (unlikely(mbox-ops-shutdown)) {
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
if (!mbox_configured)
mbox-ops-shutdown(mbox);
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
}
}
@@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const char *name)
struct omap_mbox *mbox;
int ret;
- read_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
mbox = *(find_mboxes(name));
if (mbox == NULL) {
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return ERR_PTR(-ENOENT);
}
- read_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
ret = omap_mbox_startup(mbox);
if (ret)
@@ -363,15 +363,15 @@ int omap_mbox_register(struct device *parent, struct
omap_mbox *mbox)
if (mbox-next)
return -EBUSY;
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
tmp = find_mboxes(mbox-name);
if (*tmp) {
ret = -EBUSY;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
goto err_find;
}
*tmp = mbox;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return 0;
@@ -384,18 +384,18 @@ int omap_mbox_unregister(struct omap_mbox *mbox)
{
struct omap_mbox **tmp;
- write_lock(mboxes_lock);
+ spin_lock(mboxes_lock);
tmp = mboxes;
while (*tmp) {
if (mbox == *tmp) {
*tmp = mbox-next;
mbox-next = NULL;
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return 0;
}
tmp = (*tmp)-next;
}
- write_unlock(mboxes_lock);
+ spin_unlock(mboxes_lock);
return -EINVAL;
}
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line
use multiple MODULE_AUTHOR lines for multiple authors Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- If you want, you can also reach me at ohadb at ti dot com . arch/arm/mach-omap2/mailbox.c |3 ++- arch/arm/plat-omap/mailbox.c |3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 318f363..763272c 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -486,5 +486,6 @@ module_exit(omap2_mbox_exit); MODULE_LICENSE(GPL v2); MODULE_DESCRIPTION(omap mailbox: omap2/3/4 architecture specific functions); -MODULE_AUTHOR(Hiroshi DOYU hiroshi.d...@nokia.com, Paul Mundt); +MODULE_AUTHOR(Hiroshi DOYU hiroshi.d...@nokia.com); +MODULE_AUTHOR(Paul Mundt); MODULE_ALIAS(platform:DRV_NAME); diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index af3a6ac..5140efc 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -419,4 +419,5 @@ module_exit(omap_mbox_exit); MODULE_LICENSE(GPL v2); MODULE_DESCRIPTION(omap mailbox: interrupt driven messaging); -MODULE_AUTHOR(Toshihiro Kobayashi and Hiroshi DOYU); +MODULE_AUTHOR(Toshihiro Kobayashi); +MODULE_AUTHOR(Hiroshi DOYU); -- 1.6.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/4] omap: mailbox: remove (un)likely macros from cold paths
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/mailbox.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..7c60550 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -248,12 +248,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
int ret = 0;
struct omap_mbox_queue *mq;
- if (likely(mbox-ops-startup)) {
+ if (mbox-ops-startup) {
spin_lock(mboxes_lock);
if (!mbox_configured)
ret = mbox-ops-startup(mbox);
- if (unlikely(ret)) {
+ if (ret) {
spin_unlock(mboxes_lock);
return ret;
}
@@ -263,7 +263,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
ret = request_irq(mbox-irq, mbox_interrupt, IRQF_SHARED,
mbox-name, mbox);
- if (unlikely(ret)) {
+ if (ret) {
printk(KERN_ERR
failed to register mailbox interrupt:%d\n, ret);
goto fail_request_irq;
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
fail_alloc_txq:
free_irq(mbox-irq, mbox);
fail_request_irq:
- if (unlikely(mbox-ops-shutdown))
+ if (mbox-ops-shutdown)
mbox-ops-shutdown(mbox);
return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox-irq, mbox);
- if (unlikely(mbox-ops-shutdown)) {
+ if (mbox-ops-shutdown) {
spin_lock(mboxes_lock);
if (mbox_configured 0)
mbox_configured--;
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] omap: mailbox: convert block api to kfifo
The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.
The default size of the kfifo buffer is set to 256 bytes.
This value is configurable at compile time (via
CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
runtime (via the mbox_kfifo_size module parameter).
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
If you want, you can also reach me at ohadb at ti dot com .
arch/arm/plat-omap/Kconfig|9 +++
arch/arm/plat-omap/include/plat/mailbox.h |4 +-
arch/arm/plat-omap/mailbox.c | 112 +---
3 files changed, 63 insertions(+), 62 deletions(-)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 6da796e..f4b0029 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -106,6 +106,15 @@ config OMAP_MBOX_FWK
Say Y here if you want to use OMAP Mailbox framework support for
DSP, IVA1.0 and IVA2 in OMAP1/2/3.
+config OMAP_MBOX_KFIFO_SIZE
+ int Mailbox kfifo default buffer size (bytes)
+ depends on OMAP_MBOX_FWK
+ default 256
+ help
+ Specify the default size of mailbox's kfifo buffers (bytes).
+ This can also be changed at runtime (via the mbox_kfifo_size
+ module parameter).
+
config OMAP_IOMMU
tristate
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h
b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..0c3c4a5 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -5,8 +5,8 @@
#include linux/wait.h
#include linux/workqueue.h
-#include linux/blkdev.h
#include linux/interrupt.h
+#include linux/kfifo.h
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -42,7 +42,7 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
- struct request_queue*queue;
+ struct kfifofifo;
struct work_struct work;
struct tasklet_struct tasklet;
int (*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 7c60550..908d9d2 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -21,11 +21,14 @@
*
*/
+#include linux/kernel.h
#include linux/module.h
#include linux/interrupt.h
#include linux/device.h
#include linux/delay.h
#include linux/slab.h
+#include linux/kfifo.h
+#include linux/err.h
#include plat/mailbox.h
@@ -35,6 +38,10 @@ static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
+static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
+module_param(mbox_kfifo_size, uint, S_IRUGO);
+MODULE_PARM_DESC(mbox_kfifo_size, Size of omap's mailbox kfifo (bytes));
+
/* Mailbox FIFO handle functions */
static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
{
@@ -67,7 +74,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox,
omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +85,50 @@ static int __mbox_msg_send(struct omap_mbox *mbox,
mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox-txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox-txq-queue;
+ spin_lock(mq-lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(mq-fifo) sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(mq-fifo, (unsigned char *)msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(mbox-txq-tasklet);
- return 0;
+out:
+ spin_unlock(mq-lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox-txq-queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox-txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq-special);
- if (ret) {
+ while (kfifo_len(mq-fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
Hi Felipe, On Fri, May 14, 2010 at 10:27 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Hi, I spent some time looking deeper into this patch series, and I have some doubts. On Sun, May 2, 2010 at 8:47 PM, Ohad Ben-Cohen o...@wizery.com wrote: On Sun, May 2, 2010 at 4:17 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, May 1, 2010 at 11:44 PM, Ohad Ben-Cohen o...@wizery.com wrote: The patchset renames the flush ioctl to begin_dma_to_dsp and the invalidate ioctl to begin_dma_from_dsp. Both functions eventually call dma_map_sg, with the former requesting a DMA_BIDIRECTIONAL direction, and the latter requesting a DMA_FROM_DEVICE direction. In addition, the patchset adds two new APIs which calls dma_unmap_sg: end_dma_to_dsp and end_dma_from_dsp. Ideally, there would be only a single begin_dma command and a single end_dma one, which would accept an additional parameter that will determine the direction of the transfer. Such an approach would be more versatile and cleaner, but it would also break all user space apps that use dspbridge today. If I understand correctly all user-space apps would be broken anyway because they are not issuing the end_dma calls. At the very least they need to be updated to use them. Basically you're right, but the patches currently silently allow today's user space to somehow work (because if you don't use dma bounce buffers and you feel lucky about speculative prefetching then you might get away with not calling dma unmap). I did that on purpose, to ease testing migration, but didn't explicitely said it because frankly it's just wrong. I looked into the dma code (I guess it's in arch/arm/mm/dma-mapping.c) and I don't understand what dma_unmap_sg is supposed to do. Portable code must use the dma_unmap_* APIs. As you mentioned, one of the things it's crucial for (but not used in our case) is bounce buffers: e.g. when doing a DMA from a device using a buffer that is out of the platform's DMA reach, the DMA API uses bounce buffers: data DMA'ed to that bounce buffer, and then the dma unmap copies it back to the original buffer). Another thing unmap is taking care of is speculative prefetch: modern ARM processors can access the buffer during DMA in order to speculatively prefetch data into the cache. Naturally this can seriously break a DMA from a device, so unmap is invalidating the caches to prevent this. The current dspbridge cache API is mostly relying on the application to do the right thing: application programmer should decide when to clean, flush or invalidate the caches in order to have a successful DMA operation. This is prone to mistakes, it's not portable, and of course not upstreamable. Using the standard DMA API we take the freedom from the application programmer - we just ask him/her to tell us before a DMA operation begins, and after it ends. The DMA API is then responsible to do the right thing. first some safe buffer is searched, and if there isn't any... then it depends on the direction whether something is actually done or not. I guess it depends whether our arch has dmabounce or not, which I have no idea, but if we do, wouldn't skiping dma_unmap calls leak some massive amount of safe buffers? Also, in Nokia we patched the bridgedriver to be able to have the 3 operations available from user-space (clean, inv, and flush), so we would be very interested in having the direction of the transfer available. Thanks, that's important to know. It's not that critical, but I guess it can't hurt to do the right thing. What do you say about the following plan then: - I'll add a single pair of begin_dma and end_dma commands that will take the additional direction parameter as described above. I'll then covert the existing flush invalidate commands to use this begin_dma command supplying it the appropriate direction argument. Sounds perfect, but I wonder if the deprecated flush invalidate would really work. See previous comments. - In a separate patch, I'll deprecate flush invalidate entirely (usage of this deprecated API will fail, resulting in a guiding error message). We get a sane and versatile (and platform-independent) implementation that always work, but breaks user space. If anyone would need to work with current user space, the deprecating patch can always be reverted locally to get back a flush invalidate implementations that (somehow) work. User-space API is being broken all the time in dspbridge. The difference is that this one might require nontrivial changes. But it must be done. So, I tried your patches, and a simple test app worked fine without modification, but a real video decoding hanged the device completely... some spinlock was stuck. I don't know if it's because of your patches, or because of the state of the bridge at that point. I'll try first to rebase to the latest to have a better idea of what's happening. I
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
Hi Felipe,
On Sun, May 16, 2010 at 8:35 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Fri, May 14, 2010 at 10:27 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
I spent some time looking deeper into this patch series, and I have some
doubts.
On Sun, May 2, 2010 at 8:47 PM, Ohad Ben-Cohen o...@wizery.com wrote:
Basically you're right, but the patches currently silently allow
today's user space
to somehow work (because if you don't use dma bounce buffers and you feel
lucky
about speculative prefetching then you might get away with not calling
dma unmap).
I did that on purpose, to ease testing migration, but didn't
explicitely said it because
frankly it's just wrong.
I looked into the dma code (I guess it's in arch/arm/mm/dma-mapping.c)
and I don't understand what dma_unmap_sg is supposed to do. I see that
first some safe buffer is searched, and if there isn't any... then
it depends on the direction whether something is actually done or not.
I guess it depends whether our arch has dmabounce or not, which I have
no idea, but if we do, wouldn't skiping dma_unmap calls leak some
massive amount of safe buffers?
Now I understand better; arch/arm/mm/dma-mapping.c will not be used
unless CONFIG_DMABOUNCE=y, which is not the case for OMAP3.
So, as you can see in arch/arm/include/asm/dma-mapping.h, dma_unmap_sg
is a noop.
This has changed since 2.6.34, when support was added
to ARM speculative prefetching. dma_unmap_* API is now
responsible to invalidate caches when data was moved in from the device
(regardless of CONFIG_DMABOUNCE).
Dspbridge really must support this, and applications should start
using it.
Whether to deprecate the old API ? Eventually I think we should,
but probably not anytime soon. Let's take the kernel approach of minimizing
user space pain: keep the old API around, mark it as candidate for
deprecation, and rethink in the future.
Thanks,
Ohad.
static inline void dma_unmap_single(struct device *dev, dma_addr_t handle,
size_t size, enum dma_data_direction dir)
{
/* nothing to do */
}
So, in the end, PROC_BEGINDMATODSP and PROC_BEGINDMAFROMDSP would do
exactly the same as PROC_FLUSHMEMORY and PROC_INVALIDATEMEMORY
(dmac_op_range/outer_io_range). And
PROC_ENDDMATODSP/PROC_ENDDMAFROMDSP don't do anything. Therefore even
if user-space updates to the new API, there's no change.
I don't think it makes sense to push for this new API since there will
be absolutely no gain.
What do you say about the following plan then:
- I'll add a single pair of begin_dma and end_dma commands that will
take the additional
direction parameter as described above. I'll then covert the existing
flush invalidate commands to use this begin_dma command supplying it
the appropriate direction argument.
Sounds perfect, but I wonder if the deprecated flush invalidate
would really work. See previous comments.
Actually it would work. I like this approach because it doesn't break
ABI, and doesn't change the semantics unless the new ioctls are used.
- In a separate patch, I'll deprecate flush invalidate entirely
(usage of this deprecated
API will fail, resulting in a guiding error message).
I don't think there's any need for deprecation.
We get a sane and versatile (and platform-independent) implementation
that always work,
but breaks user space. If anyone would need to work with current user space,
the deprecating patch can always be reverted locally to get back a
flush invalidate
implementations that (somehow) work.
I still would like the new API for the reason I mentioned before: so
that user-space can clean/invalidate/flush.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/6] DSPBRIDGE: remember mapping and page info in proc_map
Hi Felipe, On Sat, May 15, 2010 at 11:34 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, May 1, 2010 at 11:44 PM, Ohad Ben-Cohen o...@wizery.com wrote: Every time the MM application calls proc_map to map a memory area, remember the details of that mapping, together with the related page structures. Signed-off-by: Ohad Ben-Cohen o...@wizery.com --- If you want, you can also reach me at ohadb at ti dot com . @@ -948,6 +949,7 @@ static dsp_status bridge_dev_create(OUT struct wmd_dev_context **ppDevContext, status = DSP_EMEMORY; goto func_end; } + status = cfg_get_host_resources((struct cfg_devnode *) drv_get_first_dev_extension(), resources); Completely unrelated change. Moreover, this creates conflicts on future commits since that code is gone. Which change are you referring to ? This is merely a newline :) Or are you referring to something else ? Thanks, Ohad. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/6] DSPBRIDGE: remove mapping information in proc_unmap
Hi Felipe, On Sat, May 15, 2010 at 11:38 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, May 1, 2010 at 11:44 PM, Ohad Ben-Cohen o...@wizery.com wrote: Clean up all mapping information resources whenever a buffer is unmapped. If I understand correctly the previous patch doesn't make sense on it's own because it will leak memory. Therefore it should be squashed with this one. Otherwise a git bisect might land on a very bad place. It's good for reviewing though. Squashed it will be. Thanks for your review, Ohad. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
On Mon, May 17, 2010 at 2:15 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Hi Ohad, On Mon, May 17, 2010 at 1:35 AM, Ohad Ben-Cohen o...@wizery.com wrote: On Fri, May 14, 2010 at 10:27 PM, Felipe Contreras felipe.contre...@gmail.com wrote: I looked into the dma code (I guess it's in arch/arm/mm/dma-mapping.c) and I don't understand what dma_unmap_sg is supposed to do. Portable code must use the dma_unmap_* APIs. As you mentioned, one of the things it's crucial for (but not used in our case) is bounce buffers: e.g. when doing a DMA from a device using a buffer that is out of the platform's DMA reach, the DMA API uses bounce buffers: data DMA'ed to that bounce buffer, and then the dma unmap copies it back to the original buffer). Yes, but in dspbridge the most important use-case is video encoding/decoding, so we definitely don't want bouncing buffers. Besides, from what I can see very few platforms need them. Sure. I just mentioned that as one reason why portable code that does DMA must use the unmap API. Another thing unmap is taking care of is speculative prefetch: modern ARM processors can access the buffer during DMA in order to speculatively prefetch data into the cache. Naturally this can seriously break a DMA from a device, so unmap is invalidating the caches to prevent this. The current dspbridge cache API is mostly relying on the application to do the right thing: application programmer should decide when to clean, flush or invalidate the caches in order to have a successful DMA operation. This is prone to mistakes, it's not portable, and of course not upstreamable. Using the standard DMA API we take the freedom from the application programmer - we just ask him/her to tell us before a DMA operation begins, and after it ends. The DMA API is then responsible to do the right thing. It is a higher level of abstraction, but the application still needs to do the right thing, and errors can still happen. Anyway, I see now the changes 2.6.34. can I read at your latest posts that after rebasing to newest dspbridge code the issue doesn't reproduce anymore ? Please tell me if it's back. Yes... I haven't tried in the old commit you started from + the patch Omar suggested, but I guess there's no need for that any more. I can play with this code now :) -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
On Mon, May 17, 2010 at 2:15 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Hi Ohad, On Mon, May 17, 2010 at 1:35 AM, Ohad Ben-Cohen o...@wizery.com wrote: On Fri, May 14, 2010 at 10:27 PM, Felipe Contreras felipe.contre...@gmail.com wrote: I looked into the dma code (I guess it's in arch/arm/mm/dma-mapping.c) and I don't understand what dma_unmap_sg is supposed to do. Portable code must use the dma_unmap_* APIs. As you mentioned, one of the things it's crucial for (but not used in our case) is bounce buffers: e.g. when doing a DMA from a device using a buffer that is out of the platform's DMA reach, the DMA API uses bounce buffers: data DMA'ed to that bounce buffer, and then the dma unmap copies it back to the original buffer). Yes, but in dspbridge the most important use-case is video encoding/decoding, so we definitely don't want bouncing buffers. Besides, from what I can see very few platforms need them. Another thing unmap is taking care of is speculative prefetch: modern ARM processors can access the buffer during DMA in order to speculatively prefetch data into the cache. Naturally this can seriously break a DMA from a device, so unmap is invalidating the caches to prevent this. The current dspbridge cache API is mostly relying on the application to do the right thing: application programmer should decide when to clean, flush or invalidate the caches in order to have a successful DMA operation. This is prone to mistakes, it's not portable, and of course not upstreamable. Using the standard DMA API we take the freedom from the application programmer - we just ask him/her to tell us before a DMA operation begins, and after it ends. The DMA API is then responsible to do the right thing. It is a higher level of abstraction, but the application still needs to do the right thing, and errors can still happen. Anyway, I see now the changes 2.6.34. can I read at your latest posts that after rebasing to newest dspbridge code the issue doesn't reproduce anymore ? Please tell me if it's back. Yes... I haven't tried in the old commit you started from + the patch Omar suggested, but I guess there's no need for that any more. I can play with this code now :) Out of curiosity, what board/environment do you use to play with the code ? I'd like to run the same use cases you do, so I can reproduce any issue you may bump into. Thanks, Ohad. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
Hi Felipe, On Mon, May 17, 2010 at 3:05 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, May 17, 2010 at 2:25 AM, Ohad Ben-Cohen o...@wizery.com wrote: Out of curiosity, what board/environment do you use to play with the code ? I'd like to run the same use cases you do, so I can reproduce any issue you may bump into. I use a beagleboard, use the DSP firmware in L23.i3.3[1], do simple tests with dsp-test[2], and real tests with gst-dsp[3]. I just pushed my latest changes of dsp-tools, which include an app called dsp-test that sends buffers back and forth (like TI's dmmcopy.out), but has options to trigger MMU faults, change buffer sizes, and number of iterations, along with a trivial DSP socket-node. The whole thing (app + socket-node) is 15K and you don't even need dynreg.out to load. Also, the code is really really simple. There are no dependencies, so you can just type 'make'. I'm not sure if you are familiar with GStreamer, but that's pretty much all you need to compile gst-dsp. If you have problems we have a relatively active mailing list. Thanks for the detailed answer! I'll surely look into this. Ohad. Cheers. [1] http://www.omapedia.org/wiki/L23.i3.3_Release_Notes [2] http://github.com/felipec/dsp-tools [3] http://github.com/felipec/gst-dsp -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
On Mon, May 17, 2010 at 2:51 AM, Felipe Contreras felipe.contre...@gmail.com wrote: So currently (v2.6.33), before sending a read-olny buffer (TO_DEVICE), we do dmac_flush_range (should be clean, but whatever) and before sending a write-only (FROM_DEVICE), we do dmac_inv_range. On v2.6.33 the same could be achieved with only dma_map_* functions, but on v2.6.34 that's not the case. Sorry, I didn't completely follow you here, let's take a look at the mapping code: ENTRY(v7_dma_map_area) add r1, r1, r0 teq r2, #DMA_FROM_DEVICE beq v7_dma_inv_range b v7_dma_clean_range ENDPROC(v7_dma_map_area) DMA_FROM_DEVICE will get the cache invalidated and then cleaned, and DMA_BIDIRECTIONAL/DMA_TO_DEVICE will get the cache only cleaned. Unfortunately I don't have a setup right now to test this, but the code seems to be ok for our needs, don't you think ? Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
On Tue, May 18, 2010 at 2:02 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, May 18, 2010 at 11:05 AM, Ohad Ben-Cohen o...@wizery.com wrote: On Mon, May 17, 2010 at 2:51 AM, Felipe Contreras felipe.contre...@gmail.com wrote: So currently (v2.6.33), before sending a read-olny buffer (TO_DEVICE), we do dmac_flush_range (should be clean, but whatever) and before sending a write-only (FROM_DEVICE), we do dmac_inv_range. On v2.6.33 the same could be achieved with only dma_map_* functions, but on v2.6.34 that's not the case. Sorry, I didn't completely follow you here, let's take a look at the mapping code: ENTRY(v7_dma_map_area) add r1, r1, r0 teq r2, #DMA_FROM_DEVICE beq v7_dma_inv_range b v7_dma_clean_range ENDPROC(v7_dma_map_area) DMA_FROM_DEVICE will get the cache invalidated and then cleaned, and DMA_BIDIRECTIONAL/DMA_TO_DEVICE will get the cache only cleaned. No, when DMA_FROM_DEVICE, only v7_dma_inv_range is executed (see the mov pc, lr at the end). Oh, sure. thanks for pointing that out. Unfortunately I don't have a setup right now to test this, but the code seems to be ok for our needs, don't you think ? But yeah, actually that fits our needs; calling the dma_map only, while still wrong, will give us the same behavior we have right now (v2.6.33). Great, so are you ok with this patchset proposal ? I'll just add support for the VM_IO path you mentioned. Thanks, Ohad. So my table was wrong, it's actually: in: begin BIDI: dma_clean_range out: begin FROM_DEV: dma_inv_range out: end FROM_DEV: dma_inv_range in: end BIDI: dma_inv_range Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
On Tue, May 18, 2010 at 2:43 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, May 18, 2010 at 2:14 PM, Ohad Ben-Cohen o...@wizery.com wrote: Unfortunately I don't have a setup right now to test this, but the code seems to be ok for our needs, don't you think ? But yeah, actually that fits our needs; calling the dma_map only, while still wrong, will give us the same behavior we have right now (v2.6.33). Great, so are you ok with this patchset proposal ? I thought you were going to add separate ioctls, one for dma_map, and another for dma_unmap that receive direction as an argument. Then, map the current PROC_FLUSH/INVALIDATE to those without changing their semantics, but marking them as deprecated. Yes, I am (this and a few other small stuff we mentioned in the threads) - I was just asking generally about the move to dma_(un)map_* API. Anyway, I'll prepare a v2 and resubmit. I'll just add support for the VM_IO path you mentioned. Cool. I actually tried your patches to render to the framebuffer, and everything seemed to work fine. I didn't check for error codes or anything, so I'm not sure what's going on. How is the framebuffer mmap'ed ? Can you please tell me more about this scenario ? (applications + drivers involved). How do I test this scenario ? using the tools you sent me ? Do I have to have a beagle board or will ZOOM do ? Thanks, Ohad. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/6] DSPBRIDGE: fix mem+cache API issues
On Tue, May 18, 2010 at 3:24 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Cool. I actually tried your patches to render to the framebuffer, and everything seemed to work fine. I didn't check for error codes or anything, so I'm not sure what's going on. How is the framebuffer mmap'ed ? mmap(NULL, self-mem_info.size, PROT_WRITE, MAP_SHARED, self-overlay_fd, 0); Can you please tell me more about this scenario ? (applications + drivers involved). I use gst-omapfb[1], and then it's very easy with a gst-launch command Ok, thanks. I think I know why it worked for you - The framebuffer is mmap'ed as uncacheable on ARM (check out fb_pgprotect in fbmem.c), which makes the whole dspbridge cache manipulations on it redundant. In our case the cache operations silently failed, but it didn't matter. We can probably just return whenever a dspbridge DMA operation will be requested on VM_IO buffers (unless there are other VM_IO dspbrdige use cases which are different). -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
