Message-ID: <45a44e480901092337vbd08832l79436786f848e102@mail.gmail.com>
Date:	Sat, 10 Jan 2009 15:37:19 +0800
From:	Jaya Kumar <jayakumar.lkml@...il.com>
To:	Robin Getz <rgetz@...ckfin.uclinux.org>
Cc:	David Brownell <david-b@...bell.net>,
	Eric Miao <eric.y.miao@...il.com>,
	Sam Ravnborg <sam@...nborg.org>,
	Eric Miao <eric.miao@...vell.com>,
	Haavard Skinnemoen <hskinnemoen@...el.com>,
	Philipp Zabel <philipp.zabel@...il.com>,
	Russell King <rmk@....linux.org.uk>,
	Ben Gardner <bgardner@...tec.com>, Greg KH <greg@...ah.com>,
	linux-arm-kernel@...ts.arm.linux.org.uk,
	linux-fbdev-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, linux-embedded@...r.kernel.org
Subject: Re: [RFC 2.6.27 1/1] gpiolib: add support for batch set of pins

On Wed, Jan 7, 2009 at 6:41 AM, Robin Getz <rgetz@...ckfin.uclinux.org> wrote:
> On Wed 31 Dec 2008 13:05, Jaya Kumar pondered:
>> On Thu, Jan 1, 2009 at 1:38 AM, Robin Getz <rgetz@...ckfin.uclinux.org> wrote:
>> > On Tue 30 Dec 2008 23:58, Jaya Kumar pondered:
>> >> On Tue, Dec 30, 2008 at 11:55 PM, Robin Getz <rgetz@...ckfin.uclinux.org> wrote:
>> >> > Yeah, I hadn't thought about spanning more than one gpio_chip. That's a good
>> >> > point.
>> >>
>> >> The currently posted code already supports spanning more than one gpio_chip.
>> >>
>> >
>> > But doesn't do all the other things that David suggested/requested.
>>
>> Hi Robin,
>>
>> Yes, you are right. My implementation does not support a driver that
>> needs to set/get more than 32-bits of gpio in a single call. I'm okay
>> with that restriction as I don't see a concrete use case for that.
>
> It's not the more than 32-bits that I'm concerned about - it is spanning
> more than one register. (if all the GPIOs that are left on the board are
> 2, 64, and 128, where 2, and 64 are part of the SOC's GPIO, and 128 is on
> a GPIO expander - which is a common use case - is this handled?)
>

Hi Robin,

You are correct in that the 2, 64, 128 case would not be accelarated
by my current implementation. The reason is that those 3 numbers can't
fit into a 32 bit bitmask (the mask is for consecutive bits). The user
would need to use the pre-existing single bit API for that scenario,
ie: gpio_set_value(2, val); (64, val), (128, val);

In my current patch, I'm trying to address the performance issue seen
by am300epd.c because of the use of gpio as a 16-bit data bus to
transfer framebuffer data using the current single bit gpio API. I
want to make my implementation more generic than just that which is
why I've added the bitmask and support up to 32-bits. I want to expand
on that to the extent that that can be done without diluting the
performance and without losing the simplicity of the current gpiolib
API.

After reading your points, I think I understand that you would prefer
a higher level API. I am thinking along the lines of:
array_of_gpio_numbers[] = [ 2 , 64, 128 ];
cookie = gpio_register_bundle(array_of_gpio_numbers, sizeof(array));
error = gpio_set_bundle(cookie, values[]);
error = gpio_get_bundle(cookie, values[]);
gpio_unregister_bundle(cookie);

I think that's elegant and I agree that it is feasible but it is a
level higher than what I'm trying to achieve. Would you agree with me
that the above API could be implemented on top of the lower level API
that I have proposed? To be specific, I imagine something along the
lines of:

int gpio_set_bundle(int cookie, u32 *values)
{
  offsets = retrieve_offsets_from_cookie(cookie);
  err = generate_chips_from_offsets(offsets, &chips);
  err = merge_consecutive_chips(chips, &merged_chips)
  foreach chip (merged_chips) {
   values = generate_values(chip, offset);
   mask = generate_mask(chip, offset);
   masklength = calculate_length(mask);
   gpio_set_batch(chip, offset, values, mask, masklength);
 }
}

forgive the naming, I'm just hurrying to illustrate what I mean.

So my goal is to get the lower level batch API working and solid. It
would solve the am300epd.c problem and put into place a framework for
others[1]. Once we've gotten to that level of support, I believe we
could start implementing the higher level API that you've described on
top of that. Does this sound like a reasonable plan that would address
your concerns?

Thanks,
jaya

[1]  The same underlying display controller that I seek to support,
broadsheet, will be used across more than just am300epd.c's pxa255. I
know with certainty that it has been used on iMX31L. Several other
SoCs including the S3C6410 are likely. So if we've got this lower
level batch API done, I hope to see it commonly implemented across
those other SoCs too in order to ensure all of those use cases are
accelerated. I am happy to work on that, especially if I can get
hardware. :-)

ps: I noticed you said the above 2, 64, 128 is a common use case. I
don't dispute that, but is that a case where acceleration is
essential? It would also help if I could look through an example of
this use case in the tree or elsewhere so that I can improve my
understanding.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists