Serious error in the code
Quoted 4 lines · On Sat, Mar 2, 2013 at 7:59 AM, <[email protected]> wrote:
Hello,
During porting we found a serious error:
Function
static inline int__atomic_add_unless(atomic_t *v, int a, int u)is
implemented wrong.
Quoted 1 line
Hexagon implementation returns 0 or 1, while it _must_ return old value.
linux/atomic.h defines it as:
Quoted 5 lines
static inline int
atomic_add_unless(atomic_t *v, int a, int u){ return __atomic_add_unless(v, a, u) != u; }So it's really supposed for
__atomic_add_unlessreturn old value, not
0/1
Quoted 2 lines
flag.
And finallyatomic_add_unlessreturns 0/1 flag by comparing values. I
think developer was confused by atomic_add_unless() description which is
Quoted 2 lines
different from
__atomic_add_unless().
This was a very awful error. . .dunno how it worked before.
You are correct, this was a bug. We have this fixed locally, but I have not had a chance to upstream the fix. I'm actually in the process of trying to clean up all our local changes for upstreaming.
If you find other stuff or have questions, feel free to post about it. It sounds like you've made pretty good progress.
Quoted 2 lines
However this is not the first error in the code - for example
hexagon_dma_init()is never called outside.
We actually weren't using DMA in the example "platform". We've been
calling it at board level init, which I think I've seen other architectures do as well.
But now that you mention it, perhaps a call to it should be placed somewhere in the initcalls.
Thanks,
Richard Kuo
--
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
Quoted 5 lines · On 5 March 2013 13:19, <[email protected]> wrote:
Quoted 1 line · On Sat, Mar 2, 2013 at 7:59 AM, <[email protected]> wrote:
Hello,
If you find other stuff or have questions, feel free to post about it. It sounds like you've made pretty good progress.
Well, the meta-issue is that he has code that runs on bare-metal hexagon, without the hypervisor, and the question becomes "should this code be accepted and merged?" (I think the answer is yes) and "how?" (I'm thinking this would be with ifdef's and CONFIG_HVM) and finally, "when?" (he claims not to be ready yet, with cleanup required. but I'm very much in the "push early and push often" camp).
-- Linas