How about a gpio_get(device *, char *) function?

How about a gpio_get(device *, char *) function?

[Alex Courbot]

Hi,

Would anyone be opposed to having a gpio_get() function that works similarly 
to e.g. regulator_get() and clk_get()?

I can see some good reasons to have this:

- Less platform data to pass to drivers,
- Consistency between different subsystems. Regulator, clock, PWM, ... all use 
this scheme.
- The "device-specific indirection" could make some DT structures more 
reusable. Right now the only way to address a GPIO through the DT is via a 
phandle that includes the GPIO number - thus hard-coded.

The implementation would be rather simple, and the function would just return 
the right GPIO number (acquired through gpio_request).

Rationale for this: I would like to be able to share power sequences between 
devices, e.g. to completely extract the per-device resources from the 
sequence. Every power sequence step references either a regulator, PWM, or 
GPIO. For regulators and PWMs separation is easy because their subsystems 
provide regulator_get() and pwm_get() which allow the resource to be 
referenced by name in the sequence, and resolved to different instances 
depending on the device. GPIOs, on the other hand, can only be referenced by 
number - and that makes it necessary to duplicate the sequence's structure in 
memory for every device that may use it. It if was possible to reference GPIOs 
by names that resolve to different GPIO numbers according to the device, then 
the problem would be solved.

There are probably other use-cases that would benefit from this, if you know of 
one please feel free to share.

Thanks,
Alex.

Re: How about a gpio_get(device *, char *) function?

[Stephen Warren]

On 10/31/2012 03:04 AM, Alex Courbot wrote:
> Hi,
> 
> Would anyone be opposed to having a gpio_get() function that works similarly 
> to e.g. regulator_get() and clk_get()?

One major stumbling block is that with device tree, each individual
binding gets to decide on the specific naming of the propert{y,ies} that
define the GPIO(s) for the device, and so there's no way to provide a
generic implementation of that function.

Related, I've always wished that DT nodes looked like:

device {
    reg = <...>;
    compatible = <...>;
    resources {
        pwms = <...>;
        regulators = <...>;
        clocks = <...>;
        gpios = <...>;
        other-devices = <...>; /* for custom API dependencies */
    };
    config {
        /* device-specific properties */
    };
    child-busses {
        0 = { ... };
        1 = { ... };
    };
};

... specifically so that all resource allocation, and perhaps even child
bus enumeration, could be completely standardized in the DT/device core.
This could also feed into deferred probe, which could then be purely
implemented inside the DT/driver core. However, that'd require something
incompatible like "device tree 2.0"

Re: How about a gpio_get(device *, char *) function?

[Alex Courbot]

On Wednesday 31 October 2012 23:25:41 Stephen Warren wrote:
> On 10/31/2012 03:04 AM, Alex Courbot wrote:
> > Hi,
> > 
> > Would anyone be opposed to having a gpio_get() function that works
> > similarly to e.g. regulator_get() and clk_get()?
> 
> One major stumbling block is that with device tree, each individual
> binding gets to decide on the specific naming of the propert{y,ies} that
> define the GPIO(s) for the device, and so there's no way to provide a
> generic implementation of that function.

The idea is not to make every GPIOs declared so far accessible through this 
function - as you point out this would be tricky at best - but to also define 
how they should be properly declared (similarly to regulators and pals) for 
bindings that need to use get_gpio(). Existing bindings and drivers that can 
live without it should not be modified. And now that you mention it, the end of 
the GPIO declaration anarchy would be another point in favor of this feature.

Now I am aware that almost every subsystem comes with its own scheme for 
declaring resources in the DT and there will be an long fight to decide which 
one should apply here, but I'm willing to take that road.

> Related, I've always wished that DT nodes looked like:
> 
> device {
>     reg = <...>;
>     compatible = <...>;
>     resources {
>         pwms = <...>;
>         regulators = <...>;
>         clocks = <...>;
>         gpios = <...>;
>         other-devices = <...>; /* for custom API dependencies */
>     };
>     config {
>         /* device-specific properties */
>     };
>     child-busses {
>         0 = { ... };
>         1 = { ... };
>     };
> };
> 
> ... specifically so that all resource allocation, and perhaps even child
> bus enumeration, could be completely standardized in the DT/device core.
> This could also feed into deferred probe, which could then be purely
> implemented inside the DT/driver core. However, that'd require something
> incompatible like "device tree 2.0"

I think this would be awesome. This could even probably be implemented without 
breaking things, if everything takes place inside well-defined subnodes of the 
device.

Alex.

Re: How about a gpio_get(device *, char *) function?

[Linus Walleij]

On Wed, Oct 31, 2012 at 10:04 AM, Alex Courbot <acourbot@nvidia.com> wrote:

> Would anyone be opposed to having a gpio_get() function that works similarly
> to e.g. regulator_get() and clk_get()?

I understand the concept and why you want to do this.

However I think the global GPIO numberspace defeats the
purpose.

gpio_get() should get an abstract handle just like clk_get() or
regulator_get(), not a fixed numeral.

That is the only way to really transit away from the global GPIO
numberspace.

The proper refactoring I can see is to introduce an orthogonal
mechanism that will return something like a struct gpio_handle *
when you call gpio_get(), and then use a new set of accessor
functions to manipulate the GPIO, like gpio_handle_set()/ etc
for all known GPIO operations.

So defined in a new #ifdef CONFIG_GPIO_HANDLES or so
in a new <linux/gpio-handles.h> header and like the other
subsystems handling abstract resources just passing an
opaque pointer cookie around.

Then when migrating a driver to use this mechanism, only
include <linux/gpio-handles.h> and make that one driver only
use the abstract interface and get rid of the global numberspace.

Then we can have drivers that under a transition period will
support both the global numberspace and this new
handle-based access pattern.

Then migrate all clients over to <linux/gpio-handle.h> and
finally kill off the old global numberspace and only use
handles with that driver.

So a much more thorough refactoring is needed, just doing
a simple int lookup returning global GPIO numbers will likely
only make the current mess worse.

This more ambitious refactoring path is going to be a large
endavour and will likely go through a lot of reviews and
debate, but something like this is badly needed for the
GPIO subsystem.

Maybe Grant will have different opinions though so this is
not the final word on this issue...

Yours,
Linus Walleij

Re: How about a gpio_get(device *, char *) function?

[Alex Courbot]

Hi Linus, thanks for the reply!

On Monday 05 November 2012 02:04:33 Linus Walleij wrote:
> On Wed, Oct 31, 2012 at 10:04 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> > Would anyone be opposed to having a gpio_get() function that works
> > similarly to e.g. regulator_get() and clk_get()?
> 
> I understand the concept and why you want to do this.
> 
> However I think the global GPIO numberspace defeats the
> purpose.
> 
> gpio_get() should get an abstract handle just like clk_get() or
> regulator_get(), not a fixed numeral.
> 
> That is the only way to really transit away from the global GPIO
> numberspace.

Interesting. I see you already gave the whole thing a thought. What I don't 
understand however is what is so wrong with the current GPIO numberspace that 
you want to replace it? Whether we use simple integers or blind pointers, the 
adressable space will basically remain the same. GPIO numbers can actually be 
considered as handles, and actually I would not mind typedef'ing "int" to a 
GPIO handle type in order to add more opacity to the framework.

Also the current DT bindings will likely continue to require the legacy API 
anyway, so I am not sure we can make it go away.

My initial thought was to build something on top of the existing scheme to 
address my immediate needs - what you are talking about is much more scary. :) 
Could you elaborate on your motivations for such a radical direction? 

Thanks,
Alex.

Re: How about a gpio_get(device *, char *) function?

[Linus Walleij]

On Mon, Nov 5, 2012 at 8:31 AM, Alex Courbot <acourbot@nvidia.com> wrote:

> Interesting. I see you already gave the whole thing a thought. What I don't
> understand however is what is so wrong with the current GPIO numberspace that
> you want to replace it? Whether we use simple integers or blind pointers, the
> adressable space will basically remain the same. GPIO numbers can actually be
> considered as handles, and actually I would not mind typedef'ing "int" to a
> GPIO handle type in order to add more opacity to the framework.

So the basic problem is scalability and multiplatform support.

Currently we have a global "roof" on GPIO numbers, ARCH_NR_GPIOS,
which is helpfully set to 256 in include/asm-generic/gpio.h.

In the embedded case "ARCH" is a particular board for a particular
SoC so this number is actually roof:ed for some arbitrary combination
of SoC + electronics, at compile time, for that very machine.

So what happens when we try to achieve multiplatform support in the
ARM tree (same for others, I expect ACPI etc to run into the same
problem) is that this has to be set to some arbitrarily high value so
that all GPIOs will fit, and they are not sparse, so if you're just using
GPIOs 0 .. 15 and 199935..200000 you will need to reserve
200000 * sizeof(int) bytes.

So to avoid this, arch maintainers try with different clever
macros to pack the GPIO number assignments per board
downward to begin at 0 and keep the array at minimum size,
and do not dynamically grab GPIO numbers as they like.

Contrast this to IRQs which are in the CONFIG_SPARSE_IRQ
case stored in a radix tree and IRQs can be sparse like this
without wasting much memory. So IRQdomains can for example
just grab a number of free IRQ descriptors and the actual
numbers returned do not matter.

Using static number assignments also has the side effect
that developers will just int mygpio = 42; directly in the code
to cut corners and invites such laziness instead of allocating
and propagating resources. With abstract handles this does
not happen.

So moving forward we need a dynamic, radix tree of sparse GPIOs,
and we need to follow the design pattern of IRQ descriptors to
keep the kernel consistent, for example pinctrl does this with
pins - these are sparse and dynamically allocated. They do not
have integer numbers other than locally for a particular pin
controller.

> Also the current DT bindings will likely continue to require the legacy API
> anyway, so I am not sure we can make it go away.

We can make any DT business go away as long as all DTS
files are maintained in the kernel and especially if their
structure is not future-proof. The "DT contract" to make
bindings stable and everlasting is being broken every day
as things are right now.

Not that I know the DT stuff by
heart, but if the way we're doing GPIO in DT is requiring the
OS to use a certain type of static array implementation it must
be wrong since DT is supposed to be OS-neutral.

> My initial thought was to build something on top of the existing scheme to
> address my immediate needs - what you are talking about is much more scary. :)
> Could you elaborate on your motivations for such a radical direction?

See above.

I suspect Grant have similar thoughts, but let's see...

Yours,
Linus Walleij

Re: How about a gpio_get(device *, char *) function?

[Grant Likely]

On Mon, 5 Nov 2012 13:09:20 +0100, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Nov 5, 2012 at 8:31 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> 
> > Interesting. I see you already gave the whole thing a thought. What I don't
> > understand however is what is so wrong with the current GPIO numberspace that
> > you want to replace it? Whether we use simple integers or blind pointers, the
> > adressable space will basically remain the same. GPIO numbers can actually be
> > considered as handles, and actually I would not mind typedef'ing "int" to a
> > GPIO handle type in order to add more opacity to the framework.
> 
> So the basic problem is scalability and multiplatform support.
> 
> Currently we have a global "roof" on GPIO numbers, ARCH_NR_GPIOS,
> which is helpfully set to 256 in include/asm-generic/gpio.h.
> 
> In the embedded case "ARCH" is a particular board for a particular
> SoC so this number is actually roof:ed for some arbitrary combination
> of SoC + electronics, at compile time, for that very machine.
> 
> So what happens when we try to achieve multiplatform support in the
> ARM tree (same for others, I expect ACPI etc to run into the same
> problem) is that this has to be set to some arbitrarily high value so
> that all GPIOs will fit, and they are not sparse, so if you're just using
> GPIOs 0 .. 15 and 199935..200000 you will need to reserve
> 200000 * sizeof(int) bytes.
> 
> So to avoid this, arch maintainers try with different clever
> macros to pack the GPIO number assignments per board
> downward to begin at 0 and keep the array at minimum size,
> and do not dynamically grab GPIO numbers as they like.
> 
> Contrast this to IRQs which are in the CONFIG_SPARSE_IRQ
> case stored in a radix tree and IRQs can be sparse like this
> without wasting much memory. So IRQdomains can for example
> just grab a number of free IRQ descriptors and the actual
> numbers returned do not matter.
> 
> Using static number assignments also has the side effect
> that developers will just int mygpio = 42; directly in the code
> to cut corners and invites such laziness instead of allocating
> and propagating resources. With abstract handles this does
> not happen.
> 
> So moving forward we need a dynamic, radix tree of sparse GPIOs,
> and we need to follow the design pattern of IRQ descriptors to
> keep the kernel consistent, for example pinctrl does this with
> pins - these are sparse and dynamically allocated. They do not
> have integer numbers other than locally for a particular pin
> controller.
> 
> > Also the current DT bindings will likely continue to require the legacy API
> > anyway, so I am not sure we can make it go away.

It's not. Nothing in the binding is dictating using the current kernel
API. All that stuff can be refactored without changing the binding.

> We can make any DT business go away as long as all DTS
> files are maintained in the kernel and especially if their
> structure is not future-proof. The "DT contract" to make
> bindings stable and everlasting is being broken every day
> as things are right now.

Don't go down that path. Firmware bindings are hard and plenty of people
have experienced the pain of bindings breaking, but *planning* to break
bindings makes it orders of magnitude worse. At least try to preserve
compatibility.

> Not that I know the DT stuff by
> heart, but if the way we're doing GPIO in DT is requiring the
> OS to use a certain type of static array implementation it must
> be wrong since DT is supposed to be OS-neutral.

g.

Re: How about a gpio_get(device *, char *) function?

[Stephen Warren]

On 11/04/2012 11:04 AM, Linus Walleij wrote:
> On Wed, Oct 31, 2012 at 10:04 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> 
>> Would anyone be opposed to having a gpio_get() function that works similarly
>> to e.g. regulator_get() and clk_get()?
> 
> I understand the concept and why you want to do this.
> 
> However I think the global GPIO numberspace defeats the
> purpose.
> 
> gpio_get() should get an abstract handle just like clk_get() or
> regulator_get(), not a fixed numeral.

I don't really see why the return type of gpio_get() influences whether
it can be implemented or not.

If gpio_get() were implemented today, it could return an integer with
the same value as any other GPIO functions use already.

With board files, some "gpio map" table would simply contain the same
int GPIO ID value the table as is used anywhere else already. With DT,
the same xlate function would translate from DT GPIO-chip-relative
IDs/specifiers into the global number space in the same way that we do
today via other APIs.

If the GPIO subsystem were reworked as you propose, this API could be
reworked in exactly the same way, or if implemented after the rework, it
would return whatever handle type was in use at the time.

Re: How about a gpio_get(device *, char *) function?

[Alex Courbot]

On Tuesday 06 November 2012 01:35:11 Stephen Warren wrote:
> On 11/04/2012 11:04 AM, Linus Walleij wrote:
> > On Wed, Oct 31, 2012 at 10:04 AM, Alex Courbot <acourbot@nvidia.com> 
wrote:
> >> Would anyone be opposed to having a gpio_get() function that works
> >> similarly to e.g. regulator_get() and clk_get()?
> > 
> > I understand the concept and why you want to do this.
> > 
> > However I think the global GPIO numberspace defeats the
> > purpose.
> > 
> > gpio_get() should get an abstract handle just like clk_get() or
> > regulator_get(), not a fixed numeral.
> 
> I don't really see why the return type of gpio_get() influences whether
> it can be implemented or not.
> 
> If gpio_get() were implemented today, it could return an integer with
> the same value as any other GPIO functions use already.
> 
> With board files, some "gpio map" table would simply contain the same
> int GPIO ID value the table as is used anywhere else already. With DT,
> the same xlate function would translate from DT GPIO-chip-relative
> IDs/specifiers into the global number space in the same way that we do
> today via other APIs.
> 
> If the GPIO subsystem were reworked as you propose, this API could be
> reworked in exactly the same way, or if implemented after the rework, it
> would return whatever handle type was in use at the time.

How about, in a first time (and because I'd also like to get the power seqs 
moving on), a typedef from int to gpio_handle_t and a first implementation of 
the gpio_handle_*() API that would just call the existing integer-based API 
(apart from gpio_handle_get())? That way things will not break when we switch 
to a real handle.

Alex.

Re: How about a gpio_get(device *, char *) function?

[Linus Walleij]

On Tue, Nov 6, 2012 at 2:33 AM, Alex Courbot <acourbot@nvidia.com> wrote:

> How about, in a first time (and because I'd also like to get the power seqs
> moving on), a typedef from int to gpio_handle_t and a first implementation of
> the gpio_handle_*() API that would just call the existing integer-based API
> (apart from gpio_handle_get())? That way things will not break when we switch
> to a real handle.

I'm afraid of typedef:ing gpio_handle_t to int because it sort of
encourages non-handlers to be used mixed with the old integers.

I would prefer to create, e.g. in <linux/gpio/consumer.h>
something like:

struct gpio;

struct gpio *gpio_get(struct device *dev, const char *name);

int gpio_get_value(struct gpio *g);

Nothing more! I.e. struct gpio is an opaque cookie, nothing to be known
about it.

And then have the drivers using this *ONLY* include <linux/gpio/consumer.h>
and not <linux/gpio.h>. So drivers have no clue what struct gpio is and
will only operate on it using functions.

This follows the pattern set by Thomas Gleixner for e.g. IRQ descriptors,
and the style was also used in the redesign of the struct clk *.

Of course the cross-referencing implementation can in e.g.
drivers/gpio/gpiodev.c internally define that like this:

#include <linux/gpio.h>

/**
  * @gpio: pointer to global GPIO number
  */
struct gpio {
    int gpio;
};

struct gpio *gpio_get(struct device *dev, const char *name)
{
   /* Lookup in cross-ref table etc */
}

int gpioh_get_value(struct gpio *g)
{
   return gpio_get_value(g->gpio);
}

(...)

Then we can work from there. I think it adds the proper
opaqueness factor.

I don't really like the "gpioh_*" prefix instead of just gpio_* but
I guess there is not polymorphism to exploit as transition
path here :-P

Yours,
Linus Walleij

Re: How about a gpio_get(device *, char *) function?

[Alex Courbot]

On Thursday 08 November 2012 05:24:19 Linus Walleij wrote:
> I would prefer to create, e.g. in <linux/gpio/consumer.h>
> something like:
> 
> struct gpio;
> 
> struct gpio *gpio_get(struct device *dev, const char *name);
> 
> int gpio_get_value(struct gpio *g);
> 
> Nothing more! I.e. struct gpio is an opaque cookie, nothing to be known
> about it.
> 
> And then have the drivers using this *ONLY* include <linux/gpio/consumer.h>
> and not <linux/gpio.h>. So drivers have no clue what struct gpio is and
> will only operate on it using functions.
> 
> This follows the pattern set by Thomas Gleixner for e.g. IRQ descriptors,
> and the style was also used in the redesign of the struct clk *.

Also pretty similar to the regulator framework, which might itself have been 
inspired by IRQs and clocks.

> Of course the cross-referencing implementation can in e.g.
> drivers/gpio/gpiodev.c internally define that like this:
> 
> #include <linux/gpio.h>
> 
> /**
>   * @gpio: pointer to global GPIO number
>   */
> struct gpio {
>     int gpio;
> };
> 
> struct gpio *gpio_get(struct device *dev, const char *name)
> {
>    /* Lookup in cross-ref table etc */
> }
> 
> int gpioh_get_value(struct gpio *g)
> {
>    return gpio_get_value(g->gpio);
> }
> 
> (...)
> 
> Then we can work from there. I think it adds the proper
> opaqueness factor.

Looks nice, simple and to the point! Let's see if I can find the time to cook 
something based on this.

Thanks!
Alex.

Re: How about a gpio_get(device *, char *) function?

[Alex Courbot]

On Thursday 08 November 2012 05:24:19 Linus Walleij wrote:
> On Tue, Nov 6, 2012 at 2:33 AM, Alex Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > How about, in a first time (and because I'd also like to get the power
> > seqs
> > moving on), a typedef from int to gpio_handle_t and a first implementation
> > of the gpio_handle_*() API that would just call the existing
> > integer-based API (apart from gpio_handle_get())? That way things will
> > not break when we switch to a real handle.
> 
> I'm afraid of typedef:ing gpio_handle_t to int because it sort of
> encourages non-handlers to be used mixed with the old integers.
> 
> I would prefer to create, e.g. in <linux/gpio/consumer.h>
> something like:
> 
> struct gpio;
> 
> struct gpio *gpio_get(struct device *dev, const char *name);
> 
> int gpio_get_value(struct gpio *g);
> 
> Nothing more! I.e. struct gpio is an opaque cookie, nothing to be known
> about it.

However these is already a struct gpio declared in linux/gpio.h. Shall the 
opaque handler be renamed something like "struct gpioh", or is your idea to 
make both APIs mutually exclusive?

Alex.

Re: How about a gpio_get(device *, char *) function?

[Linus Walleij]

On Thu, Nov 8, 2012 at 7:23 AM, Alex Courbot <acourbot@nvidia.com> wrote:
> On Thursday 08 November 2012 05:24:19 Linus Walleij wrote:
>> On Tue, Nov 6, 2012 at 2:33 AM, Alex Courbot <acourbot@nvidia.com> wrote:
>> > How about, in a first time (and because I'd also like to get the power
>> > seqs
>> > moving on), a typedef from int to gpio_handle_t and a first implementation
>> > of the gpio_handle_*() API that would just call the existing
>> > integer-based API (apart from gpio_handle_get())? That way things will
>> > not break when we switch to a real handle.
>>
>> I'm afraid of typedef:ing gpio_handle_t to int because it sort of
>> encourages non-handlers to be used mixed with the old integers.
>>
>> I would prefer to create, e.g. in <linux/gpio/consumer.h>
>> something like:
>>
>> struct gpio;
>>
>> struct gpio *gpio_get(struct device *dev, const char *name);
>>
>> int gpio_get_value(struct gpio *g);
>>
>> Nothing more! I.e. struct gpio is an opaque cookie, nothing to be known
>> about it.
>
> However these is already a struct gpio declared in linux/gpio.h. Shall the
> opaque handler be renamed something like "struct gpioh", or is your idea to
> make both APIs mutually exclusive?

Basically I'd like the API's to be mutually execlusive.

But maybe there is a namespace clash anyway, since the
handler code will have to #linclude <linux/gpio.h>

This is one of the rare cases where I'd maybe like to
even #undef gpio in the core code just to be able to
mask that defintion of "gpio" with the "gpio" from the
new API.

I really like the fact that it will bite your hand if you try
to use both APIs at once, you could even introduce some

#define DO_NOT_INCLUDE_LINUX_GPIO_H
in <linux/gpio/consumer.h>

And
#define DO_NOT_INCLUDE_GPIO_CONSUMER_H
in <linux/gpio.h>

and then put things like:

#ifdef DO_NOT_INCLUDE_LINUX_GPIO_H
#error "Trying to use mutually exclusive interfaces"
#endif

into <linux/gpio.h>...

Yours,
Linus Walleij

Re: How about a gpio_get(device *, char *) function?

[Linus Walleij]

On Mon, Nov 5, 2012 at 6:35 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> [Me]
>> gpio_get() should get an abstract handle just like clk_get() or
>> regulator_get(), not a fixed numeral.
>
> I don't really see why the return type of gpio_get() influences whether
> it can be implemented or not.

It doesn't influence that, but I want to follow the opaqueness design
pattern from irq descriptors and struct clk.

> With board files, some "gpio map" table would simply contain the same
> int GPIO ID value the table as is used anywhere else already. With DT,
> the same xlate function would translate from DT GPIO-chip-relative
> IDs/specifiers into the global number space in the same way that we do
> today via other APIs.

Yes, this part I buy into, just want to see how we can move forward
from there. The coplete nightmare is to introduce something into DT
that nails down a global GPIO numberspace... but I think that is not
the case atleast.

> If the GPIO subsystem were reworked as you propose, this API could be
> reworked in exactly the same way, or if implemented after the rework, it
> would return whatever handle type was in use at the time.

Yes, I just think we should return an opaque struct from day 1, so
just a little, little bit more to shield us.

Yours,
Linus Walleij

Re: How about a gpio_get(device *, char *) function?

[Grant Likely]

On Wed, 7 Nov 2012 22:28:01 +0100, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Nov 5, 2012 at 6:35 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > [Me]
> >> gpio_get() should get an abstract handle just like clk_get() or
> >> regulator_get(), not a fixed numeral.
> >
> > I don't really see why the return type of gpio_get() influences whether
> > it can be implemented or not.
> 
> It doesn't influence that, but I want to follow the opaqueness design
> pattern from irq descriptors and struct clk.

Right. I like the pattern too. Unforutunately that means dealing with
somewhere on the order of 2500 callers of the old API. :-(

However, I don't think that the GPIO numberspace issue is completely
intertwined with opaqifying the gpio handles. The numberspace can be
fixed with the current API if someone creates a sparse gpio
registrations.

I don't have any problem with a gpio_get function, but I do agree that
making it return an opaque handle is how it should be written with a new
set of accessors. The handle should probably be simply the pointer to
the &gpio_desc[number] which is a private table in gpiolib.c. The
definition of it isn't available outside of gpiolib.c

In fact, the old functions should be redefined in terms of getting the
gpio_desc from the irq number and calling the new functions.

> 
> > With board files, some "gpio map" table would simply contain the same
> > int GPIO ID value the table as is used anywhere else already. With DT,
> > the same xlate function would translate from DT GPIO-chip-relative
> > IDs/specifiers into the global number space in the same way that we do
> > today via other APIs.
> 
> Yes, this part I buy into, just want to see how we can move forward
> from there. The coplete nightmare is to introduce something into DT
> that nails down a global GPIO numberspace... but I think that is not
> the case atleast.
> 
> > If the GPIO subsystem were reworked as you propose, this API could be
> > reworked in exactly the same way, or if implemented after the rework, it
> > would return whatever handle type was in use at the time.
> 
> Yes, I just think we should return an opaque struct from day 1, so
> just a little, little bit more to shield us.
> 
> Yours,
> Linus Walleij

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: How about a gpio_get(device *, char *) function?

[Alex Courbot]

On Monday 26 November 2012 19:14:31 Grant Likely wrote:
> I don't have any problem with a gpio_get function, but I do agree that
> making it return an opaque handle is how it should be written with a new
> set of accessors. The handle should probably be simply the pointer to
> the &gpio_desc[number] which is a private table in gpiolib.c. The
> definition of it isn't available outside of gpiolib.c

That looks like a reasonable approach, but this would make the new API 
available only to systems that use GPIOlib. Shouldn't we be concerned about 
making this available to all GPIO implementations? Or is GPIOlib so widely 
used that we don't care?

Right now I have a very simple wrapper (for testing purposes) around the 
current integer-base GPIO namespace that accepts tables mapping consumers to 
GPIO numbers, much like Thierry did for the PWM subsystem. Integrating it into 
GPIOlib does not seem to be much more difficult ; it would require some 
refactoring though as most of the code should be shared by the two APIs.

This also seems to be the right opportunity (although not directly related) to 
switch the gpio_desc table into something more flexible. Two approaches come to 
mind: either a linked-list of gpio_chips ordered by base GPIO, or a radix-
tree. The small number of gpio chips in a system seem to make the first 
approach reasonable enough - GPIO lookup time would become linear instead of 
constant, but it should not be noticeable from the consumer perspective.

Alex.

Re: How about a gpio_get(device *, char *) function?

[Grant Likely]

On Wed, 28 Nov 2012 12:38:38 +0900, Alex Courbot <acourbot@nvidia.com> wrote:
> On Monday 26 November 2012 19:14:31 Grant Likely wrote:
> > I don't have any problem with a gpio_get function, but I do agree that
> > making it return an opaque handle is how it should be written with a new
> > set of accessors. The handle should probably be simply the pointer to
> > the &gpio_desc[number] which is a private table in gpiolib.c. The
> > definition of it isn't available outside of gpiolib.c
> 
> That looks like a reasonable approach, but this would make the new API 
> available only to systems that use GPIOlib. Shouldn't we be concerned about 
> making this available to all GPIO implementations? Or is GPIOlib so widely 
> used that we don't care?

I'm tempted to say non-gpiolib is not supported. However, there isn't
anything that would prevent non-gpiolib users from implementing the api
themselves, but they'd need to provide their own handle..

g.

Re: How about a gpio_get(device *, char *) function?

[Linus Walleij]

On Thu, Nov 29, 2012 at 6:34 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed, 28 Nov 2012 12:38:38 +0900, Alex Courbot <acourbot@nvidia.com> wrote:
>> On Monday 26 November 2012 19:14:31 Grant Likely wrote:
>> > I don't have any problem with a gpio_get function, but I do agree that
>> > making it return an opaque handle is how it should be written with a new
>> > set of accessors. The handle should probably be simply the pointer to
>> > the &gpio_desc[number] which is a private table in gpiolib.c. The
>> > definition of it isn't available outside of gpiolib.c
>>
>> That looks like a reasonable approach, but this would make the new API
>> available only to systems that use GPIOlib. Shouldn't we be concerned about
>> making this available to all GPIO implementations? Or is GPIOlib so widely
>> used that we don't care?
>
> I'm tempted to say non-gpiolib is not supported. However, there isn't
> anything that would prevent non-gpiolib users from implementing the api
> themselves, but they'd need to provide their own handle..

I get the creeps when you say that ...

Now I think I have to put on my TODO to remove a few custom GPIO
implementations so it feels better. ;-)

Yours,
Linus Walleij

Re: How about a gpio_get(device *, char *) function?

[Grant Likely]

On Sat, 1 Dec 2012 19:41:39 +0100, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Nov 29, 2012 at 6:34 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On Wed, 28 Nov 2012 12:38:38 +0900, Alex Courbot <acourbot@nvidia.com> wrote:
> >> On Monday 26 November 2012 19:14:31 Grant Likely wrote:
> >> > I don't have any problem with a gpio_get function, but I do agree that
> >> > making it return an opaque handle is how it should be written with a new
> >> > set of accessors. The handle should probably be simply the pointer to
> >> > the &gpio_desc[number] which is a private table in gpiolib.c. The
> >> > definition of it isn't available outside of gpiolib.c
> >>
> >> That looks like a reasonable approach, but this would make the new API
> >> available only to systems that use GPIOlib. Shouldn't we be concerned about
> >> making this available to all GPIO implementations? Or is GPIOlib so widely
> >> used that we don't care?
> >
> > I'm tempted to say non-gpiolib is not supported. However, there isn't
> > anything that would prevent non-gpiolib users from implementing the api
> > themselves, but they'd need to provide their own handle..
> 
> I get the creeps when you say that ...

hahaha.  Well, what else do we do? By definiton the custom
implementations are custom. We've got no way to support them unless we
cast the gpio number to the gpio handle in that case. That would work
but it would be mighty ugly.

g.

Re: How about a gpio_get(device *, char *) function?

[Grant Likely]

On Wed, 31 Oct 2012 18:04:09 +0900, Alex Courbot <acourbot@nvidia.com> wrote:
> Hi,
> 
> Would anyone be opposed to having a gpio_get() function that works similarly 
> to e.g. regulator_get() and clk_get()?
> 
> I can see some good reasons to have this:
> 
> - Less platform data to pass to drivers,
> - Consistency between different subsystems. Regulator, clock, PWM, ... all use 
> this scheme.
> - The "device-specific indirection" could make some DT structures more 
> reusable. Right now the only way to address a GPIO through the DT is via a 
> phandle that includes the GPIO number - thus hard-coded.
> 
> The implementation would be rather simple, and the function would just return 
> the right GPIO number (acquired through gpio_request).

I've got no problem with it, but the devil is in the API details. Draft
something up (unless you already have and I just haven't seen it yet...
I'll get to it).  :-)

BTW, I would prefer a system that resolves the gpio at .probe() time
instead of at registration time. That makes deferred probing easier.

g.

> 
> Rationale for this: I would like to be able to share power sequences between 
> devices, e.g. to completely extract the per-device resources from the 
> sequence. Every power sequence step references either a regulator, PWM, or 
> GPIO. For regulators and PWMs separation is easy because their subsystems 
> provide regulator_get() and pwm_get() which allow the resource to be 
> referenced by name in the sequence, and resolved to different instances 
> depending on the device. GPIOs, on the other hand, can only be referenced by 
> number - and that makes it necessary to duplicate the sequence's structure in 
> memory for every device that may use it. It if was possible to reference GPIOs 
> by names that resolve to different GPIO numbers according to the device, then 
> the problem would be solved.
> 
> There are probably other use-cases that would benefit from this, if you know of 
> one please feel free to share.
> 
> Thanks,
> Alex.
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.