On Saturday 27 December 2008, Jaya Kumar wrote:
> Oh, gosh darn it, how time has flown. My email above was to make sure
> I have understood the feedback. I assume I should just get started on
> implementing. Just to double check, the plan is:
> - add bitmask support.
> - add get_batch support
> - improve naming. I think gpio_get_batch/gpio_set_batch sounds good.
I was expecting to get some agreement on interfaces first.
> I plan to have something like:
>
> void gpio_set_batch(unsigned gpio, u32 values, u32 bitmask, int bitwidth);
> u32 gpio_get_batch(unsigned gpio, u32 bitmask, int bitwidth);
I still don't like the word "batch" here. Did you look
at the <linux/bitmask.h> interfaces as I suggested? They
would suggest something rather different:
- passing bitmasks as {unsigned long *bits, int count} tuples
- separate calls to:
* zero a set of bits (loop: gpio_set_value to 0)
* fill a set of bits (loop: gpio_set_value to 1)
* copy a set of bits (loop: gpio_set_value to src[i])
* read a set of bits (loop: dst[i] = gpio_get_value)
* ... maybe more
Any restriction to 32 bit value seems shortsighted. It would
make sense to wrap the GPIO bitmask descriptions in a struct,
letting drivers work with smaller sets -- probably most would
fit into a single "unsigned long".
> I think I should explain the bitmask and bitwidth choice. The intended
> use case is:
> for (i=0; i < 800*600; i++) {
> gpio_set_batch(...)
> }
>
> bitwidth (needed to iterate and map to chip ngpios) could be
> calculated from bitmask, but that involves iteratively counting bits
> from the mask, so we would have to do 800*600 bit counts. Unless, we
> do ugly things like cache the previous bitwidth/mask and compare
> against the current caller arguments. Given that the bitwidth would
> typically be a fixed value, I believe it is more intuitive for the
> caller to provide it, eg:
>
> gpio_set_batch(DB0, value, 0xFFFF, 16)
>
> which has the nice performance benefit of skipping all the bit
> counting in the most common use case scenario.
That doesn't explain the bit mask and bitwidth parameters at all.
> While we are here, I was thinking about it, and its better if I give
> gpio_request/free/direction_batch a miss for now.
Let's not and say we didn't. Providing incomplete interfaces
isn't really much help.
> Nothing prevents
> those features being added at a later point. That way I can focus on
> getting gpio_set/get_batch implemented correctly and merged as the
> first step.
First step needs to be defining the interface extensions needed.
- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
