Hello, I wrote:
Signed-off-by: Steve Chen <[email protected]> Signed-off-by: Mark Greer <[email protected]> Signed-off-by: Sergei Shtylyov <[email protected]>
Mostly, it looks pretty good. Some comments below...
Index: linux-2.6/arch/arm/common/cp_intc.c =================================================================== --- /dev/null +++ linux-2.6/arch/arm/common/cp_intc.c @@ -0,0 +1,159 @@
[...]
+static int cp_intc_set_irq_type(unsigned int irq, unsigned int flow_type) +{ + unsigned reg = irq >> 5;
minor nitpick: for a little more clarity, you can use BIT_WORD() here.
OK.
+ unsigned mask = irq & 0x1f;
maybe a #define here instead of hard-coded constant.
What kind of #define you'd want? This is symmetric to the above, i.e. it gets a bit number within the 32-bit word. I'm not seeing anything giving that in <linux/bitops.h>... though well, I can come up with something.
When writing this, I started to suspect that this looks fishy, and it does indeed. This code is simply wrong, it should be 1 << (irq & 0x1f), i.e. BIT_MASK(irq). Sigh, Steve, this one I've missed... :-/
WBR, Sergei _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
