On Thursday 12 October 2006 4:58 pm, Kevin Hilman wrote: > > 2) In arch/arm/mach-davinci/gpio.c, the function gpio_set_direction() is > > declared as __init. This is quite restrictive, because there are a lot of > > situations where you may need to change a the direction of a GPIO > > dynamically (e.g. bidirectional data lines). > > Removal of __init is OK, but your patch also renames the version in > gpio.c (to something that is never called) and moves the function to > gpio.h. What's the reason for that?
In fact ... why didn't you just remove the "__init" declarations? The point of the inlined functions is to support serious bitbanging, by letting the operation map to a pair of machine instructions (load mask, write to constant address) when the GPIO number is a constant. Efficient in terms of both code space and cycle count. And then when the GPIO number is variable, at least there's only one copy of the more expensive code. But what you did with that function is _always_ space-inefficient. Every call site would have its own copy of the expensive logic, instead of having only a single out-of-line copy; bad for icache. > > 3) In include/asm-arm/arch-davinci/gpio.h, there is an error in the > > gpio_clear() function (the bit is always set). > > I don't see how this patch fixes that problem. The problem description is wrong. It only sets it for non-constant GPIO numbers ... for constants, it's correct (writing the "clear these bits" register). Clearly, the out-of-line code was cut'n'pasted later, and the initial testing covered the fast inlined code. :) - Dave _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
