Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-04 Thread Roland Stigge
Hi,

On 04/10/12 01:07, Linus Walleij wrote:
>> What do others think? JC? Linus? I'm considering this (unsigned int
>> data) a valid option.
> 
> I think we mostly use an unsigned long for such stuff as IRQ flags
> and ioctl() parameters in the kernel.
> 
> In this case it has the upside that it will be 32bit on 32bit systems
> and 64bit on 64bit systems if I'm not mistaken.

Fine. Will try to prepare a patch tomorrow, including fallback to single
GPIO handling (if the driver doesn't implement block operations) and
omitting the sysfs interface in the first patch.

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-04 Thread Roland Stigge
Hi,

On 04/10/12 01:07, Linus Walleij wrote:
 What do others think? JC? Linus? I'm considering this (unsigned int
 data) a valid option.
 
 I think we mostly use an unsigned long for such stuff as IRQ flags
 and ioctl() parameters in the kernel.
 
 In this case it has the upside that it will be 32bit on 32bit systems
 and 64bit on 64bit systems if I'm not mistaken.

Fine. Will try to prepare a patch tomorrow, including fallback to single
GPIO handling (if the driver doesn't implement block operations) and
omitting the sysfs interface in the first patch.

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-03 Thread Linus Walleij
On Sun, Sep 30, 2012 at 5:46 PM, Roland Stigge  wrote:
> On 30/09/12 17:19, Stijn Devriendt wrote:
>>
>> Rules are rules, but why make the interface overly complex when
>> the multi-value file is saner, cleaner and simpler?
>
> Simply because they won't (and probably shouldn't) accept it mainline.

We, including you and Stijn *are* the mainline ... ;-)

The only reason I really dislike it is that the GPIO sysfs interface is
scary as it is, so I don't want to add to it if we can instead push to
reform it into something more sane.

Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-03 Thread Linus Walleij
On Sun, Sep 30, 2012 at 12:50 PM, Roland Stigge  wrote:

> Besides what I discussed with JC and Linus, I find the unsigned int
> (i.e. u32 or u64, depending on the arch) quite appealing. It is a nice
> compromise between my general bit mapped data model (variable size *u8
> array) and the bool *values list. Even maps easily onto a single sysfs
> entry for values, by abstracting a gpio list to an actual data word.
>
> What do others think? JC? Linus? I'm considering this (unsigned int
> data) a valid option.

I think we mostly use an unsigned long for such stuff as IRQ flags
and ioctl() parameters in the kernel.

In this case it has the upside that it will be 32bit on 32bit systems
and 64bit on 64bit systems if I'm not mistaken.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-03 Thread Mark Brown
On Thu, Sep 27, 2012 at 11:22:02PM +0200, Roland Stigge wrote:

> The recurring task of providing simultaneous access to GPIO lines (especially
> for bit banging protocols) needs an appropriate API.

> This patch adds a kernel internal "Block GPIO" API that enables simultaneous
> access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
> sysfs interface (/sys/class/gpio/gpiochipXX/block).

It would be very useful if this had the option of falling back to
addressing the GPIOs one by one.  There's some usages that aren't
performance dependent and it'd make drivers able to run even if they're
suboptimal.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-03 Thread Mark Brown
On Thu, Sep 27, 2012 at 11:22:02PM +0200, Roland Stigge wrote:

 The recurring task of providing simultaneous access to GPIO lines (especially
 for bit banging protocols) needs an appropriate API.

 This patch adds a kernel internal Block GPIO API that enables simultaneous
 access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
 sysfs interface (/sys/class/gpio/gpiochipXX/block).

It would be very useful if this had the option of falling back to
addressing the GPIOs one by one.  There's some usages that aren't
performance dependent and it'd make drivers able to run even if they're
suboptimal.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-03 Thread Linus Walleij
On Sun, Sep 30, 2012 at 12:50 PM, Roland Stigge sti...@antcom.de wrote:

 Besides what I discussed with JC and Linus, I find the unsigned int
 (i.e. u32 or u64, depending on the arch) quite appealing. It is a nice
 compromise between my general bit mapped data model (variable size *u8
 array) and the bool *values list. Even maps easily onto a single sysfs
 entry for values, by abstracting a gpio list to an actual data word.

 What do others think? JC? Linus? I'm considering this (unsigned int
 data) a valid option.

I think we mostly use an unsigned long for such stuff as IRQ flags
and ioctl() parameters in the kernel.

In this case it has the upside that it will be 32bit on 32bit systems
and 64bit on 64bit systems if I'm not mistaken.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-10-03 Thread Linus Walleij
On Sun, Sep 30, 2012 at 5:46 PM, Roland Stigge sti...@antcom.de wrote:
 On 30/09/12 17:19, Stijn Devriendt wrote:

 Rules are rules, but why make the interface overly complex when
 the multi-value file is saner, cleaner and simpler?

 Simply because they won't (and probably shouldn't) accept it mainline.

We, including you and Stijn *are* the mainline ... ;-)

The only reason I really dislike it is that the GPIO sysfs interface is
scary as it is, so I don't want to add to it if we can instead push to
reform it into something more sane.

Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
On 30/09/12 17:19, Stijn Devriendt wrote:
>> If I understand correctly, it's a violation (single-value should hold
>> for read and write).
>>
>> To solve it, I have the following in mind: /sys/.../gpiogroupXXX/
>> contains files "bit0" ... "bit31" which contain a gpio number each,
>> empty if "unconnected".
> 
> Unfortunately that means you can't atomically create a group.

I don't see a big advantage of having atomic create/request. Most
important is set/get, isn't it? I assume the following usage pattern:

* Create(request) - non atomic (maybe atomic but why not add GPIOs later?)
* Set - atomic
* Get - atomic
* ...

> It also creates a mess to keep ordering intact and to either
> keep the current pin state or override it at allocation-time.

Ordering should stay intact, and later add/delete operations could be
possible. I meant bit0 ... bit31 in the gpio block as such:

bit0  - "80"
bit1  - ""  (i.e. unconnected)
bit2  - "85"
bit3  - "2"
...
bit31 - ""

This scheme can support multiple gpio_chips, as discussed with Linus and
JC, which of course can't always guarantee real simultaneous I/O but
provide virtual I/O word access (32bit/64bit).

> Rules are rules, but why make the interface overly complex when
> the multi-value file is saner, cleaner and simpler?

Simply because they won't (and probably shouldn't) accept it mainline.

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Sun, Sep 30, 2012 at 5:09 PM, Roland Stigge  wrote:
> Hi Stijn,
>
> On 30/09/12 16:52, Stijn Devriendt wrote:
>>> One question: How did you solve the one-value-per-file in the sysfs
>>> interface?
>>>
>> By exporting the group as a whole:
>> /sys/.../gpiogroup248/value
>> where value contains a decimal representing the group value.
>> Again, this respects the ordering of the pins:
>>
>> Actual pins: 0x2D (b 0010 1101)
>> Selected pins: 6 3 0 1
>> Readout: 6 (b 0 1 1 0)
>>
>> The export sysfs file does, however, accept multiple gpio IDs for groups.
>> Not sure if this is a 'violation' per se...
>
> If I understand correctly, it's a violation (single-value should hold
> for read and write).
>
> To solve it, I have the following in mind: /sys/.../gpiogroupXXX/
> contains files "bit0" ... "bit31" which contain a gpio number each,
> empty if "unconnected".

Unfortunately that means you can't atomically create a group.
It also creates a mess to keep ordering intact and to either
keep the current pin state or override it at allocation-time.

Rules are rules, but why make the interface overly complex when
the multi-value file is saner, cleaner and simpler?
I know I'm on the happy/corporate side of things and I can violate
whatever rule I can if it gets our product shipping, but I'd still
propose to have the multi-value approach, even in mainline.

Regards,
Stijn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Sun, Sep 30, 2012 at 12:34 PM, Roland Stigge  wrote:
> On 29/09/12 21:57, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> Problem here is that it's only an intermediate format since hardware
>>> often needs special preparation of the data.
>>>
>>> But will evaluate what makes most sense.
>> the key point here is to avoid to manipualte data each time we call
>> gpio_block_set
>>
>> hardware specific will have to be handle at driver level
>
> Understand, thanks! I'm just trying to prevent overly complex API because:
>
> * In our discussed scheme, the driver still needs to convert the data bits
The u32 mask scheme fits simple-gpio drivers (1 register for input/output,
1 for direction, e.g. mpc8xxx driver) pretty well. Consider that to get
true synchronous output behavior of multiple pins, this is probably the only
option anyway.
For the cavium octeon driver (which doesn't seem to be upstream yet; sports
a single register for readout, per-pin write register) you can only emulate
group behavior by looping over the mask, without synchronous behavior.

> * In practice, the block gpio API is especially useful for use on single
> gpio_chips (only there, a real simultaneous i/o is possible anyway)
> * Wouldn't introduce this kind of optimization in lack of measurable
> improvement
> * The actual i/o data bits still need handling, generating a bit CPU
> load anyway.

I believe it's not worth it to try and save some tens of CPU cycles, considering
that GPIO pins might be on some slower bus anyway. Even then, simple-gpio
drivers have 0 added overhead, because they can use the mask
straight away. That means all processing is done in gpiolib's code and
is limited
to a loop of 32 iterations at most...

For drivers where performance really is critical, you could probably precompute
the needed values:
sclmask = gpio_group_precompute(MY_I2C_SCL)
sdamask = gpio_group_precompute(MY_I2C_SDA)
gpio_group_set_direct(sclmask | sdamask)

Regards,
Stijn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
Hi Stijn,

On 30/09/12 16:52, Stijn Devriendt wrote:
>> One question: How did you solve the one-value-per-file in the sysfs
>> interface?
>>
> By exporting the group as a whole:
> /sys/.../gpiogroup248/value
> where value contains a decimal representing the group value.
> Again, this respects the ordering of the pins:
> 
> Actual pins: 0x2D (b 0010 1101)
> Selected pins: 6 3 0 1
> Readout: 6 (b 0 1 1 0)
> 
> The export sysfs file does, however, accept multiple gpio IDs for groups.
> Not sure if this is a 'violation' per se...

If I understand correctly, it's a violation (single-value should hold
for read and write).

To solve it, I have the following in mind: /sys/.../gpiogroupXXX/
contains files "bit0" ... "bit31" which contain a gpio number each,
empty if "unconnected".

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Sun, Sep 30, 2012 at 12:50 PM, Roland Stigge  wrote:
> On 30/09/12 11:35, Stijn Devriendt wrote:
>> In our kernel tree we have similar code. If you like I can request
>> permission
>> to share. I can, however, already give you an update on the basic
>> structure, perhaps
>> it's useful now.
>>
>> For the first part, the drivers need to implement a the gpio interface
>> for groups.
>> gpio_set_multi, gpio_get_multi, gpio_direction_input_multi,
>> gpio_direction_output_multi. Each of them gets a 'u32 mask'.
>>
>> Secondly, gpiolib gets some new code to handle groups:
>> groups are requested via a list of gpio ids. Mind that order is respected:
>> request( [1, 5, 2, 4] ) followed by a set(0x5) will translate to
>> gpio_set_multi( 0x18 ). An opaque gpio_group struct is used to keep track.
>> This means the gpiolib interface also has a u32 mask, but translation is
>> done for the gpio-drivers.
>>
>> There is some code to request groups via device-tree (again respecting
>> order)
>> and there are also platform driver structures.
>>
>> gpiolib was also extended to export groups into sysfs, respecting policy
>> (input, output, user-selectable) and to make softlinks to groups in other
>> driver's subdir. (One driver we use this in is a power-sequencer with 2
>> gpios selecting a margining profile, this driver then has the gpio_group
>> exported in it's sysfs dir as .../profile, allowing H/W engineers to select
>> the profile without voltage glitches)
>>
>> There's also a separate driver, that does nothing more than exporting
>> both individual pins and groups to userspace based on platform description
>> or devicetree. This is probably less interesting for mainline, since we're
>> abusing device-tree to do away with some init script that can do the same.
>>
>> The rationale behind a 32bit mask is that typical processors can at most
>> set one processor-word worth of GPIOs at once and there are probably
>> few chips with over 32GPIOs on a single gpio_chip anyway.
>> Nevertheless, in the era of 64bit, it's definitely possible to go for
>> u64 instead.
>
> Hi Stijn,
>
> thank you for your notes!
>
> Besides what I discussed with JC and Linus, I find the unsigned int
> (i.e. u32 or u64, depending on the arch) quite appealing. It is a nice
> compromise between my general bit mapped data model (variable size *u8
> array) and the bool *values list. Even maps easily onto a single sysfs
> entry for values, by abstracting a gpio list to an actual data word.
>
> What do others think? JC? Linus? I'm considering this (unsigned int
> data) a valid option.
>
> One question: How did you solve the one-value-per-file in the sysfs
> interface?
>
By exporting the group as a whole:
/sys/.../gpiogroup248/value
where value contains a decimal representing the group value.
Again, this respects the ordering of the pins:

Actual pins: 0x2D (b 0010 1101)
Selected pins: 6 3 0 1
Readout: 6 (b 0 1 1 0)

The export sysfs file does, however, accept multiple gpio IDs for groups.
Not sure if this is a 'violation' per se... If the user stores a single value
he gets a single pin, multiple (space-separated) values give him a group.

Regards,
Stijn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
On 30/09/12 11:35, Stijn Devriendt wrote:
> In our kernel tree we have similar code. If you like I can request
> permission
> to share. I can, however, already give you an update on the basic
> structure, perhaps
> it's useful now.
> 
> For the first part, the drivers need to implement a the gpio interface
> for groups.
> gpio_set_multi, gpio_get_multi, gpio_direction_input_multi,
> gpio_direction_output_multi. Each of them gets a 'u32 mask'.
> 
> Secondly, gpiolib gets some new code to handle groups:
> groups are requested via a list of gpio ids. Mind that order is respected:
> request( [1, 5, 2, 4] ) followed by a set(0x5) will translate to
> gpio_set_multi( 0x18 ). An opaque gpio_group struct is used to keep track.
> This means the gpiolib interface also has a u32 mask, but translation is
> done for the gpio-drivers.
> 
> There is some code to request groups via device-tree (again respecting
> order)
> and there are also platform driver structures.
> 
> gpiolib was also extended to export groups into sysfs, respecting policy
> (input, output, user-selectable) and to make softlinks to groups in other
> driver's subdir. (One driver we use this in is a power-sequencer with 2
> gpios selecting a margining profile, this driver then has the gpio_group
> exported in it's sysfs dir as .../profile, allowing H/W engineers to select
> the profile without voltage glitches)
> 
> There's also a separate driver, that does nothing more than exporting
> both individual pins and groups to userspace based on platform description
> or devicetree. This is probably less interesting for mainline, since we're
> abusing device-tree to do away with some init script that can do the same.
> 
> The rationale behind a 32bit mask is that typical processors can at most
> set one processor-word worth of GPIOs at once and there are probably
> few chips with over 32GPIOs on a single gpio_chip anyway.
> Nevertheless, in the era of 64bit, it's definitely possible to go for
> u64 instead.

Hi Stijn,

thank you for your notes!

Besides what I discussed with JC and Linus, I find the unsigned int
(i.e. u32 or u64, depending on the arch) quite appealing. It is a nice
compromise between my general bit mapped data model (variable size *u8
array) and the bool *values list. Even maps easily onto a single sysfs
entry for values, by abstracting a gpio list to an actual data word.

What do others think? JC? Linus? I'm considering this (unsigned int
data) a valid option.

One question: How did you solve the one-value-per-file in the sysfs
interface?

Thanks in advance!

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
On 29/09/12 21:57, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> Problem here is that it's only an intermediate format since hardware
>> often needs special preparation of the data.
>>
>> But will evaluate what makes most sense.
> the key point here is to avoid to manipualte data each time we call
> gpio_block_set
> 
> hardware specific will have to be handle at driver level

Understand, thanks! I'm just trying to prevent overly complex API because:

* In our discussed scheme, the driver still needs to convert the data bits
* In practice, the block gpio API is especially useful for use on single
gpio_chips (only there, a real simultaneous i/o is possible anyway)
* Wouldn't introduce this kind of optimization in lack of measurable
improvement
* The actual i/o data bits still need handling, generating a bit CPU
load anyway.

Trying to provide as simple API as possible.

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Thu, Sep 27, 2012 at 11:22 PM, Roland Stigge  wrote:
>
> The recurring task of providing simultaneous access to GPIO lines
> (especially
> for bit banging protocols) needs an appropriate API.
>
> This patch adds a kernel internal "Block GPIO" API that enables
> simultaneous
> access to several GPIOs in the same gpio_chip (bit mapped). Further, it
> adds a
> sysfs interface (/sys/class/gpio/gpiochipXX/block).
>
> Signed-off-by: Roland Stigge 
>
> ---
> NOTE: This is only useful if individual drivers implement the .get_block()
> and
> .set_block() functions. I'm providing an example implementation for
> max730x
> (see next patch), and can provide further driver patches after API review.
>
> Thanks in advance!
>

Hi Roland,

In our kernel tree we have similar code. If you like I can request
permission
to share. I can, however, already give you an update on the basic structure,
perhaps
it's useful now.

For the first part, the drivers need to implement a the gpio interface for
groups.
gpio_set_multi, gpio_get_multi, gpio_direction_input_multi,
gpio_direction_output_multi. Each of them gets a 'u32 mask'.

Secondly, gpiolib gets some new code to handle groups:
groups are requested via a list of gpio ids. Mind that order is respected:
request( [1, 5, 2, 4] ) followed by a set(0x5) will translate to
gpio_set_multi( 0x18 ). An opaque gpio_group struct is used to keep track.
This means the gpiolib interface also has a u32 mask, but translation is
done for the gpio-drivers.

There is some code to request groups via device-tree (again respecting
order)
and there are also platform driver structures.

gpiolib was also extended to export groups into sysfs, respecting policy
(input, output, user-selectable) and to make softlinks to groups in other
driver's subdir. (One driver we use this in is a power-sequencer with 2
gpios selecting a margining profile, this driver then has the gpio_group
exported in it's sysfs dir as .../profile, allowing H/W engineers to select
the profile without voltage glitches)

There's also a separate driver, that does nothing more than exporting
both individual pins and groups to userspace based on platform description
or devicetree. This is probably less interesting for mainline, since we're
abusing device-tree to do away with some init script that can do the same.

The rationale behind a 32bit mask is that typical processors can at most
set one processor-word worth of GPIOs at once and there are probably
few chips with over 32GPIOs on a single gpio_chip anyway.
Nevertheless, in the era of 64bit, it's definitely possible to go for u64
instead.

Let me know if this looks like something usable to you.

Regards,
Stijn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Thu, Sep 27, 2012 at 11:22 PM, Roland Stigge sti...@antcom.de wrote:

 The recurring task of providing simultaneous access to GPIO lines
 (especially
 for bit banging protocols) needs an appropriate API.

 This patch adds a kernel internal Block GPIO API that enables
 simultaneous
 access to several GPIOs in the same gpio_chip (bit mapped). Further, it
 adds a
 sysfs interface (/sys/class/gpio/gpiochipXX/block).

 Signed-off-by: Roland Stigge sti...@antcom.de

 ---
 NOTE: This is only useful if individual drivers implement the .get_block()
 and
 .set_block() functions. I'm providing an example implementation for
 max730x
 (see next patch), and can provide further driver patches after API review.

 Thanks in advance!


Hi Roland,

In our kernel tree we have similar code. If you like I can request
permission
to share. I can, however, already give you an update on the basic structure,
perhaps
it's useful now.

For the first part, the drivers need to implement a the gpio interface for
groups.
gpio_set_multi, gpio_get_multi, gpio_direction_input_multi,
gpio_direction_output_multi. Each of them gets a 'u32 mask'.

Secondly, gpiolib gets some new code to handle groups:
groups are requested via a list of gpio ids. Mind that order is respected:
request( [1, 5, 2, 4] ) followed by a set(0x5) will translate to
gpio_set_multi( 0x18 ). An opaque gpio_group struct is used to keep track.
This means the gpiolib interface also has a u32 mask, but translation is
done for the gpio-drivers.

There is some code to request groups via device-tree (again respecting
order)
and there are also platform driver structures.

gpiolib was also extended to export groups into sysfs, respecting policy
(input, output, user-selectable) and to make softlinks to groups in other
driver's subdir. (One driver we use this in is a power-sequencer with 2
gpios selecting a margining profile, this driver then has the gpio_group
exported in it's sysfs dir as .../profile, allowing H/W engineers to select
the profile without voltage glitches)

There's also a separate driver, that does nothing more than exporting
both individual pins and groups to userspace based on platform description
or devicetree. This is probably less interesting for mainline, since we're
abusing device-tree to do away with some init script that can do the same.

The rationale behind a 32bit mask is that typical processors can at most
set one processor-word worth of GPIOs at once and there are probably
few chips with over 32GPIOs on a single gpio_chip anyway.
Nevertheless, in the era of 64bit, it's definitely possible to go for u64
instead.

Let me know if this looks like something usable to you.

Regards,
Stijn
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
On 29/09/12 21:57, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Problem here is that it's only an intermediate format since hardware
 often needs special preparation of the data.

 But will evaluate what makes most sense.
 the key point here is to avoid to manipualte data each time we call
 gpio_block_set
 
 hardware specific will have to be handle at driver level

Understand, thanks! I'm just trying to prevent overly complex API because:

* In our discussed scheme, the driver still needs to convert the data bits
* In practice, the block gpio API is especially useful for use on single
gpio_chips (only there, a real simultaneous i/o is possible anyway)
* Wouldn't introduce this kind of optimization in lack of measurable
improvement
* The actual i/o data bits still need handling, generating a bit CPU
load anyway.

Trying to provide as simple API as possible.

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
On 30/09/12 11:35, Stijn Devriendt wrote:
 In our kernel tree we have similar code. If you like I can request
 permission
 to share. I can, however, already give you an update on the basic
 structure, perhaps
 it's useful now.
 
 For the first part, the drivers need to implement a the gpio interface
 for groups.
 gpio_set_multi, gpio_get_multi, gpio_direction_input_multi,
 gpio_direction_output_multi. Each of them gets a 'u32 mask'.
 
 Secondly, gpiolib gets some new code to handle groups:
 groups are requested via a list of gpio ids. Mind that order is respected:
 request( [1, 5, 2, 4] ) followed by a set(0x5) will translate to
 gpio_set_multi( 0x18 ). An opaque gpio_group struct is used to keep track.
 This means the gpiolib interface also has a u32 mask, but translation is
 done for the gpio-drivers.
 
 There is some code to request groups via device-tree (again respecting
 order)
 and there are also platform driver structures.
 
 gpiolib was also extended to export groups into sysfs, respecting policy
 (input, output, user-selectable) and to make softlinks to groups in other
 driver's subdir. (One driver we use this in is a power-sequencer with 2
 gpios selecting a margining profile, this driver then has the gpio_group
 exported in it's sysfs dir as .../profile, allowing H/W engineers to select
 the profile without voltage glitches)
 
 There's also a separate driver, that does nothing more than exporting
 both individual pins and groups to userspace based on platform description
 or devicetree. This is probably less interesting for mainline, since we're
 abusing device-tree to do away with some init script that can do the same.
 
 The rationale behind a 32bit mask is that typical processors can at most
 set one processor-word worth of GPIOs at once and there are probably
 few chips with over 32GPIOs on a single gpio_chip anyway.
 Nevertheless, in the era of 64bit, it's definitely possible to go for
 u64 instead.

Hi Stijn,

thank you for your notes!

Besides what I discussed with JC and Linus, I find the unsigned int
(i.e. u32 or u64, depending on the arch) quite appealing. It is a nice
compromise between my general bit mapped data model (variable size *u8
array) and the bool *values list. Even maps easily onto a single sysfs
entry for values, by abstracting a gpio list to an actual data word.

What do others think? JC? Linus? I'm considering this (unsigned int
data) a valid option.

One question: How did you solve the one-value-per-file in the sysfs
interface?

Thanks in advance!

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Sun, Sep 30, 2012 at 12:50 PM, Roland Stigge sti...@antcom.de wrote:
 On 30/09/12 11:35, Stijn Devriendt wrote:
 In our kernel tree we have similar code. If you like I can request
 permission
 to share. I can, however, already give you an update on the basic
 structure, perhaps
 it's useful now.

 For the first part, the drivers need to implement a the gpio interface
 for groups.
 gpio_set_multi, gpio_get_multi, gpio_direction_input_multi,
 gpio_direction_output_multi. Each of them gets a 'u32 mask'.

 Secondly, gpiolib gets some new code to handle groups:
 groups are requested via a list of gpio ids. Mind that order is respected:
 request( [1, 5, 2, 4] ) followed by a set(0x5) will translate to
 gpio_set_multi( 0x18 ). An opaque gpio_group struct is used to keep track.
 This means the gpiolib interface also has a u32 mask, but translation is
 done for the gpio-drivers.

 There is some code to request groups via device-tree (again respecting
 order)
 and there are also platform driver structures.

 gpiolib was also extended to export groups into sysfs, respecting policy
 (input, output, user-selectable) and to make softlinks to groups in other
 driver's subdir. (One driver we use this in is a power-sequencer with 2
 gpios selecting a margining profile, this driver then has the gpio_group
 exported in it's sysfs dir as .../profile, allowing H/W engineers to select
 the profile without voltage glitches)

 There's also a separate driver, that does nothing more than exporting
 both individual pins and groups to userspace based on platform description
 or devicetree. This is probably less interesting for mainline, since we're
 abusing device-tree to do away with some init script that can do the same.

 The rationale behind a 32bit mask is that typical processors can at most
 set one processor-word worth of GPIOs at once and there are probably
 few chips with over 32GPIOs on a single gpio_chip anyway.
 Nevertheless, in the era of 64bit, it's definitely possible to go for
 u64 instead.

 Hi Stijn,

 thank you for your notes!

 Besides what I discussed with JC and Linus, I find the unsigned int
 (i.e. u32 or u64, depending on the arch) quite appealing. It is a nice
 compromise between my general bit mapped data model (variable size *u8
 array) and the bool *values list. Even maps easily onto a single sysfs
 entry for values, by abstracting a gpio list to an actual data word.

 What do others think? JC? Linus? I'm considering this (unsigned int
 data) a valid option.

 One question: How did you solve the one-value-per-file in the sysfs
 interface?

By exporting the group as a whole:
/sys/.../gpiogroup248/value
where value contains a decimal representing the group value.
Again, this respects the ordering of the pins:

Actual pins: 0x2D (b 0010 1101)
Selected pins: 6 3 0 1
Readout: 6 (b 0 1 1 0)

The export sysfs file does, however, accept multiple gpio IDs for groups.
Not sure if this is a 'violation' per se... If the user stores a single value
he gets a single pin, multiple (space-separated) values give him a group.

Regards,
Stijn
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
Hi Stijn,

On 30/09/12 16:52, Stijn Devriendt wrote:
 One question: How did you solve the one-value-per-file in the sysfs
 interface?

 By exporting the group as a whole:
 /sys/.../gpiogroup248/value
 where value contains a decimal representing the group value.
 Again, this respects the ordering of the pins:
 
 Actual pins: 0x2D (b 0010 1101)
 Selected pins: 6 3 0 1
 Readout: 6 (b 0 1 1 0)
 
 The export sysfs file does, however, accept multiple gpio IDs for groups.
 Not sure if this is a 'violation' per se...

If I understand correctly, it's a violation (single-value should hold
for read and write).

To solve it, I have the following in mind: /sys/.../gpiogroupXXX/
contains files bit0 ... bit31 which contain a gpio number each,
empty if unconnected.

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Sun, Sep 30, 2012 at 12:34 PM, Roland Stigge sti...@antcom.de wrote:
 On 29/09/12 21:57, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Problem here is that it's only an intermediate format since hardware
 often needs special preparation of the data.

 But will evaluate what makes most sense.
 the key point here is to avoid to manipualte data each time we call
 gpio_block_set

 hardware specific will have to be handle at driver level

 Understand, thanks! I'm just trying to prevent overly complex API because:

 * In our discussed scheme, the driver still needs to convert the data bits
The u32 mask scheme fits simple-gpio drivers (1 register for input/output,
1 for direction, e.g. mpc8xxx driver) pretty well. Consider that to get
true synchronous output behavior of multiple pins, this is probably the only
option anyway.
For the cavium octeon driver (which doesn't seem to be upstream yet; sports
a single register for readout, per-pin write register) you can only emulate
group behavior by looping over the mask, without synchronous behavior.

 * In practice, the block gpio API is especially useful for use on single
 gpio_chips (only there, a real simultaneous i/o is possible anyway)
 * Wouldn't introduce this kind of optimization in lack of measurable
 improvement
 * The actual i/o data bits still need handling, generating a bit CPU
 load anyway.

I believe it's not worth it to try and save some tens of CPU cycles, considering
that GPIO pins might be on some slower bus anyway. Even then, simple-gpio
drivers have 0 added overhead, because they can use the mask
straight away. That means all processing is done in gpiolib's code and
is limited
to a loop of 32 iterations at most...

For drivers where performance really is critical, you could probably precompute
the needed values:
sclmask = gpio_group_precompute(MY_I2C_SCL)
sdamask = gpio_group_precompute(MY_I2C_SDA)
gpio_group_set_direct(sclmask | sdamask)

Regards,
Stijn
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Stijn Devriendt
On Sun, Sep 30, 2012 at 5:09 PM, Roland Stigge sti...@antcom.de wrote:
 Hi Stijn,

 On 30/09/12 16:52, Stijn Devriendt wrote:
 One question: How did you solve the one-value-per-file in the sysfs
 interface?

 By exporting the group as a whole:
 /sys/.../gpiogroup248/value
 where value contains a decimal representing the group value.
 Again, this respects the ordering of the pins:

 Actual pins: 0x2D (b 0010 1101)
 Selected pins: 6 3 0 1
 Readout: 6 (b 0 1 1 0)

 The export sysfs file does, however, accept multiple gpio IDs for groups.
 Not sure if this is a 'violation' per se...

 If I understand correctly, it's a violation (single-value should hold
 for read and write).

 To solve it, I have the following in mind: /sys/.../gpiogroupXXX/
 contains files bit0 ... bit31 which contain a gpio number each,
 empty if unconnected.

Unfortunately that means you can't atomically create a group.
It also creates a mess to keep ordering intact and to either
keep the current pin state or override it at allocation-time.

Rules are rules, but why make the interface overly complex when
the multi-value file is saner, cleaner and simpler?
I know I'm on the happy/corporate side of things and I can violate
whatever rule I can if it gets our product shipping, but I'd still
propose to have the multi-value approach, even in mainline.

Regards,
Stijn
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-30 Thread Roland Stigge
On 30/09/12 17:19, Stijn Devriendt wrote:
 If I understand correctly, it's a violation (single-value should hold
 for read and write).

 To solve it, I have the following in mind: /sys/.../gpiogroupXXX/
 contains files bit0 ... bit31 which contain a gpio number each,
 empty if unconnected.
 
 Unfortunately that means you can't atomically create a group.

I don't see a big advantage of having atomic create/request. Most
important is set/get, isn't it? I assume the following usage pattern:

* Create(request) - non atomic (maybe atomic but why not add GPIOs later?)
* Set - atomic
* Get - atomic
* ...

 It also creates a mess to keep ordering intact and to either
 keep the current pin state or override it at allocation-time.

Ordering should stay intact, and later add/delete operations could be
possible. I meant bit0 ... bit31 in the gpio block as such:

bit0  - 80
bit1  -   (i.e. unconnected)
bit2  - 85
bit3  - 2
...
bit31 - 

This scheme can support multiple gpio_chips, as discussed with Linus and
JC, which of course can't always guarantee real simultaneous I/O but
provide virtual I/O word access (32bit/64bit).

 Rules are rules, but why make the interface overly complex when
 the multi-value file is saner, cleaner and simpler?

Simply because they won't (and probably shouldn't) accept it mainline.

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-29 Thread Jean-Christophe PLAGNIOL-VILLARD
On 20:32 Fri 28 Sep , Roland Stigge wrote:
> Hi,
> 
> On 28/09/12 18:01, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>Maybe like this, for some struct block *?
> >>
> >>block = set_block_prepare(gc, pins, values, size);
> >>if (block) {
> >>set_block(gc, block);
> >>...
> >>set_block_unprepare(gc, block);
> >>}
> >>
> >>Would mean that all supported drivers would need to implement those 3
> >>new functions... Need to be careful about not introducing bloat...
> >the prepare is gpiolib specific, it will be a helper to conver a gpio list to
> >a gpio block list
> >
> >I was thinking more
> >
> >block = gpio_block_prepare(pins, size);
> >
> >gpio_block_set_value(pin0, val);
> >gpio_block_set_value(pin1, val);
> >gpio_block_set_value(pin2, val);
> >gpio_block_set(block);
> >
> >andfor get
> >
> >gpio_block_get(block)
> >val = gpio_block_get_value(block, pin0);
> >val = gpio_block_get_value(block, pin1);
> >
> >for the gpio driver ti's transparent
> 
> Problem here is that it's only an intermediate format since hardware
> often needs special preparation of the data.
> 
> But will evaluate what makes most sense.
the key point here is to avoid to manipualte data each time we call
gpio_block_set

hardware specific will have to be handle at driver level

Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-29 Thread Jean-Christophe PLAGNIOL-VILLARD
On 20:32 Fri 28 Sep , Roland Stigge wrote:
 Hi,
 
 On 28/09/12 18:01, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Maybe like this, for some struct block *?
 
 block = set_block_prepare(gc, pins, values, size);
 if (block) {
 set_block(gc, block);
 ...
 set_block_unprepare(gc, block);
 }
 
 Would mean that all supported drivers would need to implement those 3
 new functions... Need to be careful about not introducing bloat...
 the prepare is gpiolib specific, it will be a helper to conver a gpio list to
 a gpio block list
 
 I was thinking more
 
 block = gpio_block_prepare(pins, size);
 
 gpio_block_set_value(pin0, val);
 gpio_block_set_value(pin1, val);
 gpio_block_set_value(pin2, val);
 gpio_block_set(block);
 
 andfor get
 
 gpio_block_get(block)
 val = gpio_block_get_value(block, pin0);
 val = gpio_block_get_value(block, pin1);
 
 for the gpio driver ti's transparent
 
 Problem here is that it's only an intermediate format since hardware
 often needs special preparation of the data.
 
 But will evaluate what makes most sense.
the key point here is to avoid to manipualte data each time we call
gpio_block_set

hardware specific will have to be handle at driver level

Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge

Hi,

On 28/09/12 18:01, Jean-Christophe PLAGNIOL-VILLARD wrote:

Maybe like this, for some struct block *?

block = set_block_prepare(gc, pins, values, size);
if (block) {
set_block(gc, block);
...
set_block_unprepare(gc, block);
}

Would mean that all supported drivers would need to implement those 3
new functions... Need to be careful about not introducing bloat...

the prepare is gpiolib specific, it will be a helper to conver a gpio list to
a gpio block list

I was thinking more

block = gpio_block_prepare(pins, size);

gpio_block_set_value(pin0, val);
gpio_block_set_value(pin1, val);
gpio_block_set_value(pin2, val);
gpio_block_set(block);

andfor get

gpio_block_get(block)
val = gpio_block_get_value(block, pin0);
val = gpio_block_get_value(block, pin1);

for the gpio driver ti's transparent


Problem here is that it's only an intermediate format since hardware 
often needs special preparation of the data.


But will evaluate what makes most sense.

Thanks for your notes!

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 13:32 Fri 28 Sep , Roland Stigge wrote:
> On 09/28/2012 12:28 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> gpio0 = {1};
> >> set0 = {1};
> >>
> >> gpio1 = {33, 34};
> > here should be
> > 
> > pin0 = {1};
> > set0 = {1};
> > 
> > pin1 = {1, 2, 23};
> > set1 = {0, 0, 1};
> > 
> > set_blocks(gpio_chip0, pin0, set0, 1);
> > set_blocks(gpio_chip1, pin1, set1, 3);
> > 
> > You may need to add a prepare to do not do the conversion between array and
> > gpio_chip array as I guess you will work on gpio block with multiple time
> > acces
> 
> Maybe like this, for some struct block *?
> 
> block = set_block_prepare(gc, pins, values, size);
> if (block) {
>   set_block(gc, block);
>   ...
>   set_block_unprepare(gc, block);
> }
> 
> Would mean that all supported drivers would need to implement those 3
> new functions... Need to be careful about not introducing bloat...
the prepare is gpiolib specific, it will be a helper to conver a gpio list to
a gpio block list

I was thinking more

block = gpio_block_prepare(pins, size);

gpio_block_set_value(pin0, val);
gpio_block_set_value(pin1, val);
gpio_block_set_value(pin2, val);
gpio_block_set(block);

andfor get

gpio_block_get(block)
val = gpio_block_get_value(block, pin0);
val = gpio_block_get_value(block, pin1);

for the gpio driver ti's transparent

Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 11:08 Fri 28 Sep , Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 10:51 Fri 28 Sep , Roland Stigge wrote:
> > Hi!
> > 
> > On 09/28/2012 09:51 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > >> Right - will add checking for the request state of the respective GPIOs.
> > >>
> > >> The list of GPIOs to handle is defined by the offset (specified GPIO)
> > >> and bitmapped list.
> > >>
> > >> If it looks more natural, I can change this to a list of ints specifying
> > >> GPIOs directly.
> > > you pass the correctly information to the gpiolib
> > > 
> > > as if you do not request the gpio the gpiolib will auto request the gpios
> > > so you api will be 
> > > int gpio_get_block(unsigned int *gpios, u8* values, size_t size);
> > > 
> > > return an error
> > > 
> > > int gpio_set_block(unsigned int *gpios, u8* set, size_t size);
> > > 
> > > so you do not care about the banks you work on the gpiolib framework will 
> > > call
> > > each gpio_chip seperatly. If the set_block get_block is not availlable the
> > > gpiolib could fallback to get/set
> > > 
> > > inside of the gpiolib that you call each bank with a bitmapped list is 
> > > correct
> > > but not in the public gpiolib API
> > 
> > Good idea! Talking about the public API (your above gpio_set_block()):
> > *gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
> > list specified in *gpios)? To prevent confusion about what the size
> > argument means (number of gpios in *gpios _or_ number of bytes in the
> > bitmap *set) - wouldn't it be clearer to have a "bool *set" and "bool
> > *values" list?
> public API list of gpio as example
> 
> gpios = {1, 33, 34, 55};
> set = {1, 0, 0 ,1};
> 
> gpio_set_blocks(gpios, set, 4);
> 
> private you do just provide the array related to the gpio_chip
> lets assume 4 bank with 32 gpio each
> 
> gpio0 = {1};
> set0 = {1};
> 
> gpio1 = {33, 34};
here should be

pin0 = {1};
set0 = {1};

pin1 = {1, 2, 23};
set1 = {0, 0, 1};

set_blocks(gpio_chip0, pin0, set0, 1);
set_blocks(gpio_chip1, pin1, set1, 3);

You may need to add a prepare to do not do the conversion between array and
gpio_chip array as I guess you will work on gpio block with multiple time
acces

Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
Hi,

On 09/28/2012 01:34 PM, Linus Walleij wrote:
>> Or some kind of new character device node?
> 
> Yes, I don't know if we should create /dev/gpioN or /dev/pinctrlN for
> this, and add GPIO functions to pinctrl (while wrapping it in the
> gpiolib to aid migration) the latter would encourage users of the
> new ABI to switch to writing pinctrl drivers.

Will have a look at this.

>> Otherwise, I need to think about leaving out the sysfs for this purpose.
> 
> Do you really need it?

In more that half of the cases, I needed userspace access. So I'm quite
interested in it.

Thanks for your notes.

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Linus Walleij
On Fri, Sep 28, 2012 at 11:52 AM, Roland Stigge  wrote:

> It's hard to do the one-value-per-file right for a several-gpios-at-once
> goal. :-)

That's true. This is one of the reasons I think GPIO chips should
be /dev/gpioN device nodes and this business be handled by
ioctl():s instead, it is harder from an ABI point of view but can
do several things at once.

> I originally had a one-value solution: A bit map, continuously
> hex coded, like in the original kernel API idea (e.g. 0x000F0A0010).
> Wasn't sure because it encodes GPIO numbers in a weird way.

Yeah, well that is not so nice either.

> Strictly formally: Isn't a comma-separated list of a GPIO block (e.g.
> "80,81,85") a singe value in a sense? :-)

I think it's called an array ...

> Or other possibilities? Maybe
> some node in /proc?

Heaven's no.

> Or some kind of new character device node?

Yes, I don't know if we should create /dev/gpioN or /dev/pinctrlN for
this, and add GPIO functions to pinctrl (while wrapping it in the
gpiolib to aid migration) the latter would encourage users of the
new ABI to switch to writing pinctrl drivers.

> Otherwise, I need to think about leaving out the sysfs for this purpose.

Do you really need it?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
On 09/28/2012 12:28 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> gpio0 = {1};
>> set0 = {1};
>>
>> gpio1 = {33, 34};
> here should be
> 
> pin0 = {1};
> set0 = {1};
> 
> pin1 = {1, 2, 23};
> set1 = {0, 0, 1};
> 
> set_blocks(gpio_chip0, pin0, set0, 1);
> set_blocks(gpio_chip1, pin1, set1, 3);
> 
> You may need to add a prepare to do not do the conversion between array and
> gpio_chip array as I guess you will work on gpio block with multiple time
> acces

Maybe like this, for some struct block *?

block = set_block_prepare(gc, pins, values, size);
if (block) {
set_block(gc, block);
...
set_block_unprepare(gc, block);
}

Would mean that all supported drivers would need to implement those 3
new functions... Need to be careful about not introducing bloat...

Thanks,

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
On 09/28/2012 11:14 AM, Linus Walleij wrote:
>> @@ -686,6 +731,13 @@ read-only attributes:
>>
>> "ngpio" ... how many GPIOs this manges (N to N + ngpio - 1)
>>
>> +   "block" ... get/set Block GPIO:
>> +   * reads: space separated list of GPIO inputs of this 
>> chip that
>> + are set to 1, e.g. "83 85 87 99"
>> +   * write: space separated list of GPIO outputs of this 
>> chip
>> + that are to be set or cleared, e.g. "80 -83 -85" 
>> (prefix
>> + "-" clears)
> 
> This sort of breaks the sysfs convention of one value per file,
> does it not?
> 
> It's not like I have some better idea, just we need to think about
> other possible solutions.
> 
> The GPIO sysfs interface is not universally liked. What are the
> typical applications you have for this? Industrial control by
> bit-banging userspace processes?

Yes, I had several projects in the past with the need of setting groups
of GPIOs at once (typically, 8 bit busses via GPIO lines), so needed to
provide some hacks. Don't want to do this over and over again. :-)

Bit-banging in kernel and userspace.

It's hard to do the one-value-per-file right for a several-gpios-at-once
goal. :-) I originally had a one-value solution: A bit map, continuously
hex coded, like in the original kernel API idea (e.g. 0x000F0A0010).
Wasn't sure because it encodes GPIO numbers in a weird way.

Strictly formally: Isn't a comma-separated list of a GPIO block (e.g.
"80,81,85") a singe value in a sense? :-) Or other possibilities? Maybe
some node in /proc? Or some kind of new character device node?

Otherwise, I need to think about leaving out the sysfs for this purpose.

Thanks in advance,

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
On 09/28/2012 11:08 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> Good idea! Talking about the public API (your above gpio_set_block()):
>> *gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
>> list specified in *gpios)? To prevent confusion about what the size
>> argument means (number of gpios in *gpios _or_ number of bytes in the
>> bitmap *set) - wouldn't it be clearer to have a "bool *set" and "bool
>> *values" list?
> public API list of gpio as example
> 
> gpios = {1, 33, 34, 55};
> set = {1, 0, 0 ,1};
> 
> gpio_set_blocks(gpios, set, 4);
> 
> private you do just provide the array related to the gpio_chip
> lets assume 4 bank with 32 gpio each
> 
> gpio0 = {1};
> set0 = {1};
> 
> gpio1 = {33, 34};
> set1 = {0, 0};
> 
> gpio2 = {55};
> set2 = {1};
> 
> set_blocks(gpio_chip0, gpio0, set0, 1);
> set_blocks(gpio_chip1, gpio1, set1, 2);
> set_blocks(gpio_chip2, gpio2, set2, 1);

Good.

For the internal driver API (gpio_chip), we even don't really need the
first argument (gpio_chip) since we can infer it from the gpios.

Will provide an update.

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Linus Walleij
On Thu, Sep 27, 2012 at 11:22 PM, Roland Stigge  wrote:

Grant really need to look at this patch too...

> The recurring task of providing simultaneous access to GPIO lines (especially
> for bit banging protocols) needs an appropriate API.
>
> This patch adds a kernel internal "Block GPIO" API that enables simultaneous
> access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
> sysfs interface (/sys/class/gpio/gpiochipXX/block).

I've had others talk about the need for this in the past, I think it may have
been Bill Gatliff who brought it up.

I'm pretty sure it's a need for the industry so we really need something
like this, thanks for working on it Roland.

>  Documentation/gpio.txt |   52 +++
>  drivers/gpio/gpiolib.c |  121 
> +
>  include/asm-generic/gpio.h |7 ++
>  include/linux/gpio.h   |   24 
>  4 files changed, 204 insertions(+)

You probably want to patch Documentation/ABI/testing/sysfs-gpio as well.

> @@ -686,6 +731,13 @@ read-only attributes:
>
> "ngpio" ... how many GPIOs this manges (N to N + ngpio - 1)
>
> +   "block" ... get/set Block GPIO:
> +   * reads: space separated list of GPIO inputs of this chip 
> that
> + are set to 1, e.g. "83 85 87 99"
> +   * write: space separated list of GPIO outputs of this chip
> + that are to be set or cleared, e.g. "80 -83 -85" (prefix
> + "-" clears)

This sort of breaks the sysfs convention of one value per file,
does it not?

It's not like I have some better idea, just we need to think about
other possible solutions.

The GPIO sysfs interface is not universally liked. What are the
typical applications you have for this? Industrial control by
bit-banging userspace processes?

I've heard about people doing that kind of things and running
these processes as real-time scheduled and then running e.g.
factory lines and other scary stuff through the GPIO sysfs ...

J-C:s comments are valid as well, will not repeat them.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 10:51 Fri 28 Sep , Roland Stigge wrote:
> Hi!
> 
> On 09/28/2012 09:51 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> Right - will add checking for the request state of the respective GPIOs.
> >>
> >> The list of GPIOs to handle is defined by the offset (specified GPIO)
> >> and bitmapped list.
> >>
> >> If it looks more natural, I can change this to a list of ints specifying
> >> GPIOs directly.
> > you pass the correctly information to the gpiolib
> > 
> > as if you do not request the gpio the gpiolib will auto request the gpios
> > so you api will be 
> > int gpio_get_block(unsigned int *gpios, u8* values, size_t size);
> > 
> > return an error
> > 
> > int gpio_set_block(unsigned int *gpios, u8* set, size_t size);
> > 
> > so you do not care about the banks you work on the gpiolib framework will 
> > call
> > each gpio_chip seperatly. If the set_block get_block is not availlable the
> > gpiolib could fallback to get/set
> > 
> > inside of the gpiolib that you call each bank with a bitmapped list is 
> > correct
> > but not in the public gpiolib API
> 
> Good idea! Talking about the public API (your above gpio_set_block()):
> *gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
> list specified in *gpios)? To prevent confusion about what the size
> argument means (number of gpios in *gpios _or_ number of bytes in the
> bitmap *set) - wouldn't it be clearer to have a "bool *set" and "bool
> *values" list?
public API list of gpio as example

gpios = {1, 33, 34, 55};
set = {1, 0, 0 ,1};

gpio_set_blocks(gpios, set, 4);

private you do just provide the array related to the gpio_chip
lets assume 4 bank with 32 gpio each

gpio0 = {1};
set0 = {1};

gpio1 = {33, 34};
set1 = {0, 0};

gpio2 = {55};
set2 = {1};

set_blocks(gpio_chip0, gpio0, set0, 1);
set_blocks(gpio_chip1, gpio1, set1, 2);
set_blocks(gpio_chip2, gpio2, set2, 1);

Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
Hi!

On 09/28/2012 09:51 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> Right - will add checking for the request state of the respective GPIOs.
>>
>> The list of GPIOs to handle is defined by the offset (specified GPIO)
>> and bitmapped list.
>>
>> If it looks more natural, I can change this to a list of ints specifying
>> GPIOs directly.
> you pass the correctly information to the gpiolib
> 
> as if you do not request the gpio the gpiolib will auto request the gpios
> so you api will be 
> int gpio_get_block(unsigned int *gpios, u8* values, size_t size);
> 
> return an error
> 
> int gpio_set_block(unsigned int *gpios, u8* set, size_t size);
> 
> so you do not care about the banks you work on the gpiolib framework will call
> each gpio_chip seperatly. If the set_block get_block is not availlable the
> gpiolib could fallback to get/set
> 
> inside of the gpiolib that you call each bank with a bitmapped list is correct
> but not in the public gpiolib API

Good idea! Talking about the public API (your above gpio_set_block()):
*gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
list specified in *gpios)? To prevent confusion about what the size
argument means (number of gpios in *gpios _or_ number of bytes in the
bitmap *set) - wouldn't it be clearer to have a "bool *set" and "bool
*values" list?

>>> And how you can hope to describe this via DT
>>
>> Haven't had planned that yet. Finally, this interface should just be
>> another view on the GPIOs already requested / assigned. Or which
>> additional info do you mean?
> how do you plan to give the gpio base vai DT to the driver as via DT we just
> pass the list of GPIO to work on

Right. If I understand correctly, with the above discussed changes, this
"GPIO base" issue is gone...

Thanks for your feedback!

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 09:14 Fri 28 Sep , Roland Stigge wrote:
> 
> Hi!
> 
> On 09/28/2012 04:47 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> +Block GPIO (optional)
> >> +-
> >> +
> >> +The above described interface concentrates on handling single GPIOs.  
> >> However,
> >> +in applications where it is critical to set several GPIOs at once, this
> >> +interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
> >> +Consider a GPIO controller that is connected via a slow I2C line. When
> >> +switching two or more GPIOs one after another, there can be considerable 
> >> time
> >> +between those events. This is solved by an interface called Block GPIO:
> >> +
> >> +void gpio_get_block(unsigned int gpio, u8* values, size_t size);
> >> +void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
> >> +
> >> +The function gpio_get_block() detects the current state of several GPIOs 
> >> at
> >> +once, practically by doing only one query at the hardware level (e.g. 
> >> memory
> >> +mapped or via bus transfers like I2C). There are some limits to this 
> >> interface:
> >> +A certain gpio_chip (see below) must be specified via the gpio parameter 
> >> as the
> >> +first GPIO in the gpio_chip group. The Block GPIO interface only supports
> >> +simultaneous handling of GPIOs in the same gpio_chip group since different
> >> +gpio_chips typically map to different GPIO hardware blocks.
> > so basicaly you use a gpio numberthat you do not request, that is maybe
> > requested. This is broken if you want to get or set block you need to pass 
> > the
> > list of GPIO you want to control not some fancy magic
> 
> Right - will add checking for the request state of the respective GPIOs.
> 
> The list of GPIOs to handle is defined by the offset (specified GPIO)
> and bitmapped list.
> 
> If it looks more natural, I can change this to a list of ints specifying
> GPIOs directly.
you pass the correctly information to the gpiolib

as if you do not request the gpio the gpiolib will auto request the gpios
so you api will be 
int gpio_get_block(unsigned int *gpios, u8* values, size_t size);

return an error

int gpio_set_block(unsigned int *gpios, u8* set, size_t size);

so you do not care about the banks you work on the gpiolib framework will call
each gpio_chip seperatly. If the set_block get_block is not availlable the
gpiolib could fallback to get/set

inside of the gpiolib that you call each bank with a bitmapped list is correct
but not in the public gpiolib API

> 
> > And how you can hope to describe this via DT
> 
> Haven't had planned that yet. Finally, this interface should just be
> another view on the GPIOs already requested / assigned. Or which
> additional info do you mean?
how do you plan to give the gpio base vai DT to the driver as via DT we just
pass the list of GPIO to work on
> 
> Thanks for your notes!
> 
> Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
Hi!

On 09/28/2012 04:47 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> +Block GPIO (optional)
>> +-
>> +
>> +The above described interface concentrates on handling single GPIOs.  
>> However,
>> +in applications where it is critical to set several GPIOs at once, this
>> +interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
>> +Consider a GPIO controller that is connected via a slow I2C line. When
>> +switching two or more GPIOs one after another, there can be considerable 
>> time
>> +between those events. This is solved by an interface called Block GPIO:
>> +
>> +void gpio_get_block(unsigned int gpio, u8* values, size_t size);
>> +void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
>> +
>> +The function gpio_get_block() detects the current state of several GPIOs at
>> +once, practically by doing only one query at the hardware level (e.g. memory
>> +mapped or via bus transfers like I2C). There are some limits to this 
>> interface:
>> +A certain gpio_chip (see below) must be specified via the gpio parameter as 
>> the
>> +first GPIO in the gpio_chip group. The Block GPIO interface only supports
>> +simultaneous handling of GPIOs in the same gpio_chip group since different
>> +gpio_chips typically map to different GPIO hardware blocks.
> so basicaly you use a gpio numberthat you do not request, that is maybe
> requested. This is broken if you want to get or set block you need to pass the
> list of GPIO you want to control not some fancy magic

Right - will add checking for the request state of the respective GPIOs.

The list of GPIOs to handle is defined by the offset (specified GPIO)
and bitmapped list.

If it looks more natural, I can change this to a list of ints specifying
GPIOs directly.

> And how you can hope to describe this via DT

Haven't had planned that yet. Finally, this interface should just be
another view on the GPIOs already requested / assigned. Or which
additional info do you mean?

Thanks for your notes!

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
Hi!

On 09/28/2012 04:47 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 +Block GPIO (optional)
 +-
 +
 +The above described interface concentrates on handling single GPIOs.  
 However,
 +in applications where it is critical to set several GPIOs at once, this
 +interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
 +Consider a GPIO controller that is connected via a slow I2C line. When
 +switching two or more GPIOs one after another, there can be considerable 
 time
 +between those events. This is solved by an interface called Block GPIO:
 +
 +void gpio_get_block(unsigned int gpio, u8* values, size_t size);
 +void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
 +
 +The function gpio_get_block() detects the current state of several GPIOs at
 +once, practically by doing only one query at the hardware level (e.g. memory
 +mapped or via bus transfers like I2C). There are some limits to this 
 interface:
 +A certain gpio_chip (see below) must be specified via the gpio parameter as 
 the
 +first GPIO in the gpio_chip group. The Block GPIO interface only supports
 +simultaneous handling of GPIOs in the same gpio_chip group since different
 +gpio_chips typically map to different GPIO hardware blocks.
 so basicaly you use a gpio numberthat you do not request, that is maybe
 requested. This is broken if you want to get or set block you need to pass the
 list of GPIO you want to control not some fancy magic

Right - will add checking for the request state of the respective GPIOs.

The list of GPIOs to handle is defined by the offset (specified GPIO)
and bitmapped list.

If it looks more natural, I can change this to a list of ints specifying
GPIOs directly.

 And how you can hope to describe this via DT

Haven't had planned that yet. Finally, this interface should just be
another view on the GPIOs already requested / assigned. Or which
additional info do you mean?

Thanks for your notes!

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 09:14 Fri 28 Sep , Roland Stigge wrote:
 
 Hi!
 
 On 09/28/2012 04:47 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
  +Block GPIO (optional)
  +-
  +
  +The above described interface concentrates on handling single GPIOs.  
  However,
  +in applications where it is critical to set several GPIOs at once, this
  +interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
  +Consider a GPIO controller that is connected via a slow I2C line. When
  +switching two or more GPIOs one after another, there can be considerable 
  time
  +between those events. This is solved by an interface called Block GPIO:
  +
  +void gpio_get_block(unsigned int gpio, u8* values, size_t size);
  +void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
  +
  +The function gpio_get_block() detects the current state of several GPIOs 
  at
  +once, practically by doing only one query at the hardware level (e.g. 
  memory
  +mapped or via bus transfers like I2C). There are some limits to this 
  interface:
  +A certain gpio_chip (see below) must be specified via the gpio parameter 
  as the
  +first GPIO in the gpio_chip group. The Block GPIO interface only supports
  +simultaneous handling of GPIOs in the same gpio_chip group since different
  +gpio_chips typically map to different GPIO hardware blocks.
  so basicaly you use a gpio numberthat you do not request, that is maybe
  requested. This is broken if you want to get or set block you need to pass 
  the
  list of GPIO you want to control not some fancy magic
 
 Right - will add checking for the request state of the respective GPIOs.
 
 The list of GPIOs to handle is defined by the offset (specified GPIO)
 and bitmapped list.
 
 If it looks more natural, I can change this to a list of ints specifying
 GPIOs directly.
you pass the correctly information to the gpiolib

as if you do not request the gpio the gpiolib will auto request the gpios
so you api will be 
int gpio_get_block(unsigned int *gpios, u8* values, size_t size);

return an error

int gpio_set_block(unsigned int *gpios, u8* set, size_t size);

so you do not care about the banks you work on the gpiolib framework will call
each gpio_chip seperatly. If the set_block get_block is not availlable the
gpiolib could fallback to get/set

inside of the gpiolib that you call each bank with a bitmapped list is correct
but not in the public gpiolib API

 
  And how you can hope to describe this via DT
 
 Haven't had planned that yet. Finally, this interface should just be
 another view on the GPIOs already requested / assigned. Or which
 additional info do you mean?
how do you plan to give the gpio base vai DT to the driver as via DT we just
pass the list of GPIO to work on
 
 Thanks for your notes!
 
 Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
Hi!

On 09/28/2012 09:51 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Right - will add checking for the request state of the respective GPIOs.

 The list of GPIOs to handle is defined by the offset (specified GPIO)
 and bitmapped list.

 If it looks more natural, I can change this to a list of ints specifying
 GPIOs directly.
 you pass the correctly information to the gpiolib
 
 as if you do not request the gpio the gpiolib will auto request the gpios
 so you api will be 
 int gpio_get_block(unsigned int *gpios, u8* values, size_t size);
 
 return an error
 
 int gpio_set_block(unsigned int *gpios, u8* set, size_t size);
 
 so you do not care about the banks you work on the gpiolib framework will call
 each gpio_chip seperatly. If the set_block get_block is not availlable the
 gpiolib could fallback to get/set
 
 inside of the gpiolib that you call each bank with a bitmapped list is correct
 but not in the public gpiolib API

Good idea! Talking about the public API (your above gpio_set_block()):
*gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
list specified in *gpios)? To prevent confusion about what the size
argument means (number of gpios in *gpios _or_ number of bytes in the
bitmap *set) - wouldn't it be clearer to have a bool *set and bool
*values list?

 And how you can hope to describe this via DT

 Haven't had planned that yet. Finally, this interface should just be
 another view on the GPIOs already requested / assigned. Or which
 additional info do you mean?
 how do you plan to give the gpio base vai DT to the driver as via DT we just
 pass the list of GPIO to work on

Right. If I understand correctly, with the above discussed changes, this
GPIO base issue is gone...

Thanks for your feedback!

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 10:51 Fri 28 Sep , Roland Stigge wrote:
 Hi!
 
 On 09/28/2012 09:51 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
  Right - will add checking for the request state of the respective GPIOs.
 
  The list of GPIOs to handle is defined by the offset (specified GPIO)
  and bitmapped list.
 
  If it looks more natural, I can change this to a list of ints specifying
  GPIOs directly.
  you pass the correctly information to the gpiolib
  
  as if you do not request the gpio the gpiolib will auto request the gpios
  so you api will be 
  int gpio_get_block(unsigned int *gpios, u8* values, size_t size);
  
  return an error
  
  int gpio_set_block(unsigned int *gpios, u8* set, size_t size);
  
  so you do not care about the banks you work on the gpiolib framework will 
  call
  each gpio_chip seperatly. If the set_block get_block is not availlable the
  gpiolib could fallback to get/set
  
  inside of the gpiolib that you call each bank with a bitmapped list is 
  correct
  but not in the public gpiolib API
 
 Good idea! Talking about the public API (your above gpio_set_block()):
 *gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
 list specified in *gpios)? To prevent confusion about what the size
 argument means (number of gpios in *gpios _or_ number of bytes in the
 bitmap *set) - wouldn't it be clearer to have a bool *set and bool
 *values list?
public API list of gpio as example

gpios = {1, 33, 34, 55};
set = {1, 0, 0 ,1};

gpio_set_blocks(gpios, set, 4);

private you do just provide the array related to the gpio_chip
lets assume 4 bank with 32 gpio each

gpio0 = {1};
set0 = {1};

gpio1 = {33, 34};
set1 = {0, 0};

gpio2 = {55};
set2 = {1};

set_blocks(gpio_chip0, gpio0, set0, 1);
set_blocks(gpio_chip1, gpio1, set1, 2);
set_blocks(gpio_chip2, gpio2, set2, 1);

Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Linus Walleij
On Thu, Sep 27, 2012 at 11:22 PM, Roland Stigge sti...@antcom.de wrote:

Grant really need to look at this patch too...

 The recurring task of providing simultaneous access to GPIO lines (especially
 for bit banging protocols) needs an appropriate API.

 This patch adds a kernel internal Block GPIO API that enables simultaneous
 access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
 sysfs interface (/sys/class/gpio/gpiochipXX/block).

I've had others talk about the need for this in the past, I think it may have
been Bill Gatliff who brought it up.

I'm pretty sure it's a need for the industry so we really need something
like this, thanks for working on it Roland.

  Documentation/gpio.txt |   52 +++
  drivers/gpio/gpiolib.c |  121 
 +
  include/asm-generic/gpio.h |7 ++
  include/linux/gpio.h   |   24 
  4 files changed, 204 insertions(+)

You probably want to patch Documentation/ABI/testing/sysfs-gpio as well.

 @@ -686,6 +731,13 @@ read-only attributes:

 ngpio ... how many GPIOs this manges (N to N + ngpio - 1)

 +   block ... get/set Block GPIO:
 +   * reads: space separated list of GPIO inputs of this chip 
 that
 + are set to 1, e.g. 83 85 87 99
 +   * write: space separated list of GPIO outputs of this chip
 + that are to be set or cleared, e.g. 80 -83 -85 (prefix
 + - clears)

This sort of breaks the sysfs convention of one value per file,
does it not?

It's not like I have some better idea, just we need to think about
other possible solutions.

The GPIO sysfs interface is not universally liked. What are the
typical applications you have for this? Industrial control by
bit-banging userspace processes?

I've heard about people doing that kind of things and running
these processes as real-time scheduled and then running e.g.
factory lines and other scary stuff through the GPIO sysfs ...

J-C:s comments are valid as well, will not repeat them.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
On 09/28/2012 11:08 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 Good idea! Talking about the public API (your above gpio_set_block()):
 *gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
 list specified in *gpios)? To prevent confusion about what the size
 argument means (number of gpios in *gpios _or_ number of bytes in the
 bitmap *set) - wouldn't it be clearer to have a bool *set and bool
 *values list?
 public API list of gpio as example
 
 gpios = {1, 33, 34, 55};
 set = {1, 0, 0 ,1};
 
 gpio_set_blocks(gpios, set, 4);
 
 private you do just provide the array related to the gpio_chip
 lets assume 4 bank with 32 gpio each
 
 gpio0 = {1};
 set0 = {1};
 
 gpio1 = {33, 34};
 set1 = {0, 0};
 
 gpio2 = {55};
 set2 = {1};
 
 set_blocks(gpio_chip0, gpio0, set0, 1);
 set_blocks(gpio_chip1, gpio1, set1, 2);
 set_blocks(gpio_chip2, gpio2, set2, 1);

Good.

For the internal driver API (gpio_chip), we even don't really need the
first argument (gpio_chip) since we can infer it from the gpios.

Will provide an update.

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
On 09/28/2012 11:14 AM, Linus Walleij wrote:
 @@ -686,6 +731,13 @@ read-only attributes:

 ngpio ... how many GPIOs this manges (N to N + ngpio - 1)

 +   block ... get/set Block GPIO:
 +   * reads: space separated list of GPIO inputs of this 
 chip that
 + are set to 1, e.g. 83 85 87 99
 +   * write: space separated list of GPIO outputs of this 
 chip
 + that are to be set or cleared, e.g. 80 -83 -85 
 (prefix
 + - clears)
 
 This sort of breaks the sysfs convention of one value per file,
 does it not?
 
 It's not like I have some better idea, just we need to think about
 other possible solutions.
 
 The GPIO sysfs interface is not universally liked. What are the
 typical applications you have for this? Industrial control by
 bit-banging userspace processes?

Yes, I had several projects in the past with the need of setting groups
of GPIOs at once (typically, 8 bit busses via GPIO lines), so needed to
provide some hacks. Don't want to do this over and over again. :-)

Bit-banging in kernel and userspace.

It's hard to do the one-value-per-file right for a several-gpios-at-once
goal. :-) I originally had a one-value solution: A bit map, continuously
hex coded, like in the original kernel API idea (e.g. 0x000F0A0010).
Wasn't sure because it encodes GPIO numbers in a weird way.

Strictly formally: Isn't a comma-separated list of a GPIO block (e.g.
80,81,85) a singe value in a sense? :-) Or other possibilities? Maybe
some node in /proc? Or some kind of new character device node?

Otherwise, I need to think about leaving out the sysfs for this purpose.

Thanks in advance,

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
On 09/28/2012 12:28 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
 gpio0 = {1};
 set0 = {1};

 gpio1 = {33, 34};
 here should be
 
 pin0 = {1};
 set0 = {1};
 
 pin1 = {1, 2, 23};
 set1 = {0, 0, 1};
 
 set_blocks(gpio_chip0, pin0, set0, 1);
 set_blocks(gpio_chip1, pin1, set1, 3);
 
 You may need to add a prepare to do not do the conversion between array and
 gpio_chip array as I guess you will work on gpio block with multiple time
 acces

Maybe like this, for some struct block *?

block = set_block_prepare(gc, pins, values, size);
if (block) {
set_block(gc, block);
...
set_block_unprepare(gc, block);
}

Would mean that all supported drivers would need to implement those 3
new functions... Need to be careful about not introducing bloat...

Thanks,

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Linus Walleij
On Fri, Sep 28, 2012 at 11:52 AM, Roland Stigge sti...@antcom.de wrote:

 It's hard to do the one-value-per-file right for a several-gpios-at-once
 goal. :-)

That's true. This is one of the reasons I think GPIO chips should
be /dev/gpioN device nodes and this business be handled by
ioctl():s instead, it is harder from an ABI point of view but can
do several things at once.

 I originally had a one-value solution: A bit map, continuously
 hex coded, like in the original kernel API idea (e.g. 0x000F0A0010).
 Wasn't sure because it encodes GPIO numbers in a weird way.

Yeah, well that is not so nice either.

 Strictly formally: Isn't a comma-separated list of a GPIO block (e.g.
 80,81,85) a singe value in a sense? :-)

I think it's called an array ...

 Or other possibilities? Maybe
 some node in /proc?

Heaven's no.

 Or some kind of new character device node?

Yes, I don't know if we should create /dev/gpioN or /dev/pinctrlN for
this, and add GPIO functions to pinctrl (while wrapping it in the
gpiolib to aid migration) the latter would encourage users of the
new ABI to switch to writing pinctrl drivers.

 Otherwise, I need to think about leaving out the sysfs for this purpose.

Do you really need it?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge
Hi,

On 09/28/2012 01:34 PM, Linus Walleij wrote:
 Or some kind of new character device node?
 
 Yes, I don't know if we should create /dev/gpioN or /dev/pinctrlN for
 this, and add GPIO functions to pinctrl (while wrapping it in the
 gpiolib to aid migration) the latter would encourage users of the
 new ABI to switch to writing pinctrl drivers.

Will have a look at this.

 Otherwise, I need to think about leaving out the sysfs for this purpose.
 
 Do you really need it?

In more that half of the cases, I needed userspace access. So I'm quite
interested in it.

Thanks for your notes.

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 11:08 Fri 28 Sep , Jean-Christophe PLAGNIOL-VILLARD wrote:
 On 10:51 Fri 28 Sep , Roland Stigge wrote:
  Hi!
  
  On 09/28/2012 09:51 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
   Right - will add checking for the request state of the respective GPIOs.
  
   The list of GPIOs to handle is defined by the offset (specified GPIO)
   and bitmapped list.
  
   If it looks more natural, I can change this to a list of ints specifying
   GPIOs directly.
   you pass the correctly information to the gpiolib
   
   as if you do not request the gpio the gpiolib will auto request the gpios
   so you api will be 
   int gpio_get_block(unsigned int *gpios, u8* values, size_t size);
   
   return an error
   
   int gpio_set_block(unsigned int *gpios, u8* set, size_t size);
   
   so you do not care about the banks you work on the gpiolib framework will 
   call
   each gpio_chip seperatly. If the set_block get_block is not availlable the
   gpiolib could fallback to get/set
   
   inside of the gpiolib that you call each bank with a bitmapped list is 
   correct
   but not in the public gpiolib API
  
  Good idea! Talking about the public API (your above gpio_set_block()):
  *gpios is a list of GPIOs, but set is still bitmapped (mapped onto the
  list specified in *gpios)? To prevent confusion about what the size
  argument means (number of gpios in *gpios _or_ number of bytes in the
  bitmap *set) - wouldn't it be clearer to have a bool *set and bool
  *values list?
 public API list of gpio as example
 
 gpios = {1, 33, 34, 55};
 set = {1, 0, 0 ,1};
 
 gpio_set_blocks(gpios, set, 4);
 
 private you do just provide the array related to the gpio_chip
 lets assume 4 bank with 32 gpio each
 
 gpio0 = {1};
 set0 = {1};
 
 gpio1 = {33, 34};
here should be

pin0 = {1};
set0 = {1};

pin1 = {1, 2, 23};
set1 = {0, 0, 1};

set_blocks(gpio_chip0, pin0, set0, 1);
set_blocks(gpio_chip1, pin1, set1, 3);

You may need to add a prepare to do not do the conversion between array and
gpio_chip array as I guess you will work on gpio block with multiple time
acces

Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Jean-Christophe PLAGNIOL-VILLARD
On 13:32 Fri 28 Sep , Roland Stigge wrote:
 On 09/28/2012 12:28 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
  gpio0 = {1};
  set0 = {1};
 
  gpio1 = {33, 34};
  here should be
  
  pin0 = {1};
  set0 = {1};
  
  pin1 = {1, 2, 23};
  set1 = {0, 0, 1};
  
  set_blocks(gpio_chip0, pin0, set0, 1);
  set_blocks(gpio_chip1, pin1, set1, 3);
  
  You may need to add a prepare to do not do the conversion between array and
  gpio_chip array as I guess you will work on gpio block with multiple time
  acces
 
 Maybe like this, for some struct block *?
 
 block = set_block_prepare(gc, pins, values, size);
 if (block) {
   set_block(gc, block);
   ...
   set_block_unprepare(gc, block);
 }
 
 Would mean that all supported drivers would need to implement those 3
 new functions... Need to be careful about not introducing bloat...
the prepare is gpiolib specific, it will be a helper to conver a gpio list to
a gpio block list

I was thinking more

block = gpio_block_prepare(pins, size);

gpio_block_set_value(pin0, val);
gpio_block_set_value(pin1, val);
gpio_block_set_value(pin2, val);
gpio_block_set(block);

andfor get

gpio_block_get(block)
val = gpio_block_get_value(block, pin0);
val = gpio_block_get_value(block, pin1);

for the gpio driver ti's transparent

Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-28 Thread Roland Stigge

Hi,

On 28/09/12 18:01, Jean-Christophe PLAGNIOL-VILLARD wrote:

Maybe like this, for some struct block *?

block = set_block_prepare(gc, pins, values, size);
if (block) {
set_block(gc, block);
...
set_block_unprepare(gc, block);
}

Would mean that all supported drivers would need to implement those 3
new functions... Need to be careful about not introducing bloat...

the prepare is gpiolib specific, it will be a helper to conver a gpio list to
a gpio block list

I was thinking more

block = gpio_block_prepare(pins, size);

gpio_block_set_value(pin0, val);
gpio_block_set_value(pin1, val);
gpio_block_set_value(pin2, val);
gpio_block_set(block);

andfor get

gpio_block_get(block)
val = gpio_block_get_value(block, pin0);
val = gpio_block_get_value(block, pin1);

for the gpio driver ti's transparent


Problem here is that it's only an intermediate format since hardware 
often needs special preparation of the data.


But will evaluate what makes most sense.

Thanks for your notes!

Roland
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-27 Thread Jean-Christophe PLAGNIOL-VILLARD
On 23:22 Thu 27 Sep , Roland Stigge wrote:
> The recurring task of providing simultaneous access to GPIO lines (especially
> for bit banging protocols) needs an appropriate API.
> 
> This patch adds a kernel internal "Block GPIO" API that enables simultaneous
> access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
> sysfs interface (/sys/class/gpio/gpiochipXX/block).
> 
> Signed-off-by: Roland Stigge 
> 
> ---
> NOTE: This is only useful if individual drivers implement the .get_block() and
> .set_block() functions. I'm providing an example implementation for max730x
> (see next patch), and can provide further driver patches after API review.
> 
> Thanks in advance!
> 
>  Documentation/gpio.txt |   52 +++
>  drivers/gpio/gpiolib.c |  121 
> +
>  include/asm-generic/gpio.h |7 ++
>  include/linux/gpio.h   |   24 
>  4 files changed, 204 insertions(+)
> 
> --- linux-2.6.orig/Documentation/gpio.txt
> +++ linux-2.6/Documentation/gpio.txt
> @@ -439,6 +439,51 @@ slower clock delays the rising edge of S
>  signaling rate accordingly.
>  
>  
> +Block GPIO (optional)
> +-
> +
> +The above described interface concentrates on handling single GPIOs.  
> However,
> +in applications where it is critical to set several GPIOs at once, this
> +interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
> +Consider a GPIO controller that is connected via a slow I2C line. When
> +switching two or more GPIOs one after another, there can be considerable time
> +between those events. This is solved by an interface called Block GPIO:
> +
> +void gpio_get_block(unsigned int gpio, u8* values, size_t size);
> +void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
> +
> +The function gpio_get_block() detects the current state of several GPIOs at
> +once, practically by doing only one query at the hardware level (e.g. memory
> +mapped or via bus transfers like I2C). There are some limits to this 
> interface:
> +A certain gpio_chip (see below) must be specified via the gpio parameter as 
> the
> +first GPIO in the gpio_chip group. The Block GPIO interface only supports
> +simultaneous handling of GPIOs in the same gpio_chip group since different
> +gpio_chips typically map to different GPIO hardware blocks.
so basicaly you use a gpio numberthat you do not request, that is maybe
requested. This is broken if you want to get or set block you need to pass the
list of GPIO you want to control not some fancy magic

Otherwise this will end be broken code.

And how you can hope to describe this via DT

Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-27 Thread Roland Stigge
The recurring task of providing simultaneous access to GPIO lines (especially
for bit banging protocols) needs an appropriate API.

This patch adds a kernel internal "Block GPIO" API that enables simultaneous
access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
sysfs interface (/sys/class/gpio/gpiochipXX/block).

Signed-off-by: Roland Stigge 

---
NOTE: This is only useful if individual drivers implement the .get_block() and
.set_block() functions. I'm providing an example implementation for max730x
(see next patch), and can provide further driver patches after API review.

Thanks in advance!

 Documentation/gpio.txt |   52 +++
 drivers/gpio/gpiolib.c |  121 +
 include/asm-generic/gpio.h |7 ++
 include/linux/gpio.h   |   24 
 4 files changed, 204 insertions(+)

--- linux-2.6.orig/Documentation/gpio.txt
+++ linux-2.6/Documentation/gpio.txt
@@ -439,6 +439,51 @@ slower clock delays the rising edge of S
 signaling rate accordingly.
 
 
+Block GPIO (optional)
+-
+
+The above described interface concentrates on handling single GPIOs.  However,
+in applications where it is critical to set several GPIOs at once, this
+interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
+Consider a GPIO controller that is connected via a slow I2C line. When
+switching two or more GPIOs one after another, there can be considerable time
+between those events. This is solved by an interface called Block GPIO:
+
+void gpio_get_block(unsigned int gpio, u8* values, size_t size);
+void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
+
+The function gpio_get_block() detects the current state of several GPIOs at
+once, practically by doing only one query at the hardware level (e.g. memory
+mapped or via bus transfers like I2C). There are some limits to this interface:
+A certain gpio_chip (see below) must be specified via the gpio parameter as the
+first GPIO in the gpio_chip group. The Block GPIO interface only supports
+simultaneous handling of GPIOs in the same gpio_chip group since different
+gpio_chips typically map to different GPIO hardware blocks.
+
+The values and size (in bytes) arguments specify a bit field of consecutive
+values for the GPIOs in this gpio_chip group, relative to the specified GPIO.
+E.g., when the gpio_chip group contains 16 GPIOs (80-95), size is 2 and values
+points to an array of two bytes, the first of which contains the input values
+of GPIOs 80-87 and the second one the values of GPIOs 88-95.
+
+Setting and clearing can be done via gpio_set_block(). Similar to the values
+argument of gpio_get_block(), the arrays pointed to by set and clr contain bit
+mapped lists of GPIOs to set and clear. This way, it is possible to
+simultaneously set e.g. GPIOs 10 and 12 and clear GPIO 3, leaving the others in
+the current state. The size argument refers to both set and clr which must be
+sized equally.
+
+Another limit of this interface is that although gpio_get_block() and
+gpio_set_block() are valid for all gpio_chips, they only work as expected where
+the actual hardware really supports setting and clearing simultaneously. Some
+GPIO hardware can only set simultaneously or clear simultaneously, but not set
+and clear simultaneously.  Further, the respective GPIO driver must implement
+the .get_block() and .set_block() functions in their struct gpio_chip
+efficiently. If they default to NULL, gpiolib uses .get() and .set() functions
+as backup, which effectively leads to non-simultaneous GPIO handling. Please
+check the actual GPIO driver you are using.
+
+
 What do these conventions omit?
 ===
 One of the biggest things these conventions omit is pin multiplexing, since
@@ -686,6 +731,13 @@ read-only attributes:
 
"ngpio" ... how many GPIOs this manges (N to N + ngpio - 1)
 
+   "block" ... get/set Block GPIO:
+   * reads: space separated list of GPIO inputs of this chip 
that
+ are set to 1, e.g. "83 85 87 99"
+   * write: space separated list of GPIO outputs of this chip
+ that are to be set or cleared, e.g. "80 -83 -85" (prefix
+ "-" clears)
+
 Board documentation should in most cases cover what GPIOs are used for
 what purposes.  However, those numbers are not always stable; GPIOs on
 a daughtercard might be different depending on the base board being used,
--- linux-2.6.orig/drivers/gpio/gpiolib.c
+++ linux-2.6/drivers/gpio/gpiolib.c
@@ -589,10 +589,114 @@ static ssize_t chip_ngpio_show(struct de
 }
 static DEVICE_ATTR(ngpio, 0444, chip_ngpio_show, NULL);
 
+
+static ssize_t chip_block_show(struct device *dev,
+  struct device_attribute *attr, char *buf)
+{
+   struct gpio_chip *chip = dev_get_drvdata(dev);
+   size_t size = (chip->ngpio + 7) / 8;
+   u8 *bits;
+   int ret 

[PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-27 Thread Roland Stigge
The recurring task of providing simultaneous access to GPIO lines (especially
for bit banging protocols) needs an appropriate API.

This patch adds a kernel internal Block GPIO API that enables simultaneous
access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
sysfs interface (/sys/class/gpio/gpiochipXX/block).

Signed-off-by: Roland Stigge sti...@antcom.de

---
NOTE: This is only useful if individual drivers implement the .get_block() and
.set_block() functions. I'm providing an example implementation for max730x
(see next patch), and can provide further driver patches after API review.

Thanks in advance!

 Documentation/gpio.txt |   52 +++
 drivers/gpio/gpiolib.c |  121 +
 include/asm-generic/gpio.h |7 ++
 include/linux/gpio.h   |   24 
 4 files changed, 204 insertions(+)

--- linux-2.6.orig/Documentation/gpio.txt
+++ linux-2.6/Documentation/gpio.txt
@@ -439,6 +439,51 @@ slower clock delays the rising edge of S
 signaling rate accordingly.
 
 
+Block GPIO (optional)
+-
+
+The above described interface concentrates on handling single GPIOs.  However,
+in applications where it is critical to set several GPIOs at once, this
+interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
+Consider a GPIO controller that is connected via a slow I2C line. When
+switching two or more GPIOs one after another, there can be considerable time
+between those events. This is solved by an interface called Block GPIO:
+
+void gpio_get_block(unsigned int gpio, u8* values, size_t size);
+void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
+
+The function gpio_get_block() detects the current state of several GPIOs at
+once, practically by doing only one query at the hardware level (e.g. memory
+mapped or via bus transfers like I2C). There are some limits to this interface:
+A certain gpio_chip (see below) must be specified via the gpio parameter as the
+first GPIO in the gpio_chip group. The Block GPIO interface only supports
+simultaneous handling of GPIOs in the same gpio_chip group since different
+gpio_chips typically map to different GPIO hardware blocks.
+
+The values and size (in bytes) arguments specify a bit field of consecutive
+values for the GPIOs in this gpio_chip group, relative to the specified GPIO.
+E.g., when the gpio_chip group contains 16 GPIOs (80-95), size is 2 and values
+points to an array of two bytes, the first of which contains the input values
+of GPIOs 80-87 and the second one the values of GPIOs 88-95.
+
+Setting and clearing can be done via gpio_set_block(). Similar to the values
+argument of gpio_get_block(), the arrays pointed to by set and clr contain bit
+mapped lists of GPIOs to set and clear. This way, it is possible to
+simultaneously set e.g. GPIOs 10 and 12 and clear GPIO 3, leaving the others in
+the current state. The size argument refers to both set and clr which must be
+sized equally.
+
+Another limit of this interface is that although gpio_get_block() and
+gpio_set_block() are valid for all gpio_chips, they only work as expected where
+the actual hardware really supports setting and clearing simultaneously. Some
+GPIO hardware can only set simultaneously or clear simultaneously, but not set
+and clear simultaneously.  Further, the respective GPIO driver must implement
+the .get_block() and .set_block() functions in their struct gpio_chip
+efficiently. If they default to NULL, gpiolib uses .get() and .set() functions
+as backup, which effectively leads to non-simultaneous GPIO handling. Please
+check the actual GPIO driver you are using.
+
+
 What do these conventions omit?
 ===
 One of the biggest things these conventions omit is pin multiplexing, since
@@ -686,6 +731,13 @@ read-only attributes:
 
ngpio ... how many GPIOs this manges (N to N + ngpio - 1)
 
+   block ... get/set Block GPIO:
+   * reads: space separated list of GPIO inputs of this chip 
that
+ are set to 1, e.g. 83 85 87 99
+   * write: space separated list of GPIO outputs of this chip
+ that are to be set or cleared, e.g. 80 -83 -85 (prefix
+ - clears)
+
 Board documentation should in most cases cover what GPIOs are used for
 what purposes.  However, those numbers are not always stable; GPIOs on
 a daughtercard might be different depending on the base board being used,
--- linux-2.6.orig/drivers/gpio/gpiolib.c
+++ linux-2.6/drivers/gpio/gpiolib.c
@@ -589,10 +589,114 @@ static ssize_t chip_ngpio_show(struct de
 }
 static DEVICE_ATTR(ngpio, 0444, chip_ngpio_show, NULL);
 
+
+static ssize_t chip_block_show(struct device *dev,
+  struct device_attribute *attr, char *buf)
+{
+   struct gpio_chip *chip = dev_get_drvdata(dev);
+   size_t size = (chip-ngpio + 7) / 8;
+   u8 *bits;
+   int 

Re: [PATCH RFC 1/2] gpio: Add a block GPIO API to gpiolib

2012-09-27 Thread Jean-Christophe PLAGNIOL-VILLARD
On 23:22 Thu 27 Sep , Roland Stigge wrote:
 The recurring task of providing simultaneous access to GPIO lines (especially
 for bit banging protocols) needs an appropriate API.
 
 This patch adds a kernel internal Block GPIO API that enables simultaneous
 access to several GPIOs in the same gpio_chip (bit mapped). Further, it adds a
 sysfs interface (/sys/class/gpio/gpiochipXX/block).
 
 Signed-off-by: Roland Stigge sti...@antcom.de
 
 ---
 NOTE: This is only useful if individual drivers implement the .get_block() and
 .set_block() functions. I'm providing an example implementation for max730x
 (see next patch), and can provide further driver patches after API review.
 
 Thanks in advance!
 
  Documentation/gpio.txt |   52 +++
  drivers/gpio/gpiolib.c |  121 
 +
  include/asm-generic/gpio.h |7 ++
  include/linux/gpio.h   |   24 
  4 files changed, 204 insertions(+)
 
 --- linux-2.6.orig/Documentation/gpio.txt
 +++ linux-2.6/Documentation/gpio.txt
 @@ -439,6 +439,51 @@ slower clock delays the rising edge of S
  signaling rate accordingly.
  
  
 +Block GPIO (optional)
 +-
 +
 +The above described interface concentrates on handling single GPIOs.  
 However,
 +in applications where it is critical to set several GPIOs at once, this
 +interface doesn't work well, e.g. bit-banging protocols via GPIO lines.
 +Consider a GPIO controller that is connected via a slow I2C line. When
 +switching two or more GPIOs one after another, there can be considerable time
 +between those events. This is solved by an interface called Block GPIO:
 +
 +void gpio_get_block(unsigned int gpio, u8* values, size_t size);
 +void gpio_set_block(unsigned int gpio, u8* set, u8* clr, size_t size);
 +
 +The function gpio_get_block() detects the current state of several GPIOs at
 +once, practically by doing only one query at the hardware level (e.g. memory
 +mapped or via bus transfers like I2C). There are some limits to this 
 interface:
 +A certain gpio_chip (see below) must be specified via the gpio parameter as 
 the
 +first GPIO in the gpio_chip group. The Block GPIO interface only supports
 +simultaneous handling of GPIOs in the same gpio_chip group since different
 +gpio_chips typically map to different GPIO hardware blocks.
so basicaly you use a gpio numberthat you do not request, that is maybe
requested. This is broken if you want to get or set block you need to pass the
list of GPIO you want to control not some fancy magic

Otherwise this will end be broken code.

And how you can hope to describe this via DT

Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/