* [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
@ 2015-02-11 16:28 Rojhalat Ibrahim
2015-02-26 9:54 ` Alexandre Courbot
2015-03-05 8:58 ` Linus Walleij
0 siblings, 2 replies; 9+ messages in thread
From: Rojhalat Ibrahim @ 2015-02-11 16:28 UTC (permalink / raw)
To: linux-gpio
Cc: Alexandre Courbot, Alexandre Courbot, Linus Walleij,
Mika Westerberg, Rafael J. Wysocki, David Miller, netdev
Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
disposing of GPIO descriptors.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
---
Change log:
v5: no change
v4: use shorter names for members of struct gpio_descs
v3: no change
v2: use the new interface
drivers/net/phy/mdio-mux-gpio.c | 60 ++++++++++++-----------------------------
1 file changed, 17 insertions(+), 43 deletions(-)
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 320eb15..c49ad09 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -12,33 +12,30 @@
#include <linux/module.h>
#include <linux/phy.h>
#include <linux/mdio-mux.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
#define DRV_VERSION "1.1"
#define DRV_DESCRIPTION "GPIO controlled MDIO bus multiplexer driver"
-#define MDIO_MUX_GPIO_MAX_BITS 8
-
struct mdio_mux_gpio_state {
- struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
- unsigned int num_gpios;
+ struct gpio_descs *gpios;
void *mux_handle;
};
static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
void *data)
{
- int values[MDIO_MUX_GPIO_MAX_BITS];
- unsigned int n;
struct mdio_mux_gpio_state *s = data;
+ int values[s->gpios->ndescs];
+ unsigned int n;
if (current_child == desired_child)
return 0;
- for (n = 0; n < s->num_gpios; n++) {
+ for (n = 0; n < s->gpios->ndescs; n++)
values[n] = (desired_child >> n) & 1;
- }
- gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
+
+ gpiod_set_array_cansleep(s->gpios->ndescs, s->gpios->desc, values);
return 0;
}
@@ -46,56 +43,33 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
static int mdio_mux_gpio_probe(struct platform_device *pdev)
{
struct mdio_mux_gpio_state *s;
- int num_gpios;
- unsigned int n;
int r;
- if (!pdev->dev.of_node)
- return -ENODEV;
-
- num_gpios = of_gpio_count(pdev->dev.of_node);
- if (num_gpios <= 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
- return -ENODEV;
-
s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
if (!s)
return -ENOMEM;
- s->num_gpios = num_gpios;
-
- for (n = 0; n < num_gpios; ) {
- struct gpio_desc *gpio = gpiod_get_index(&pdev->dev, NULL, n,
- GPIOD_OUT_LOW);
- if (IS_ERR(gpio)) {
- r = PTR_ERR(gpio);
- goto err;
- }
- s->gpio[n] = gpio;
- n++;
- }
+ s->gpios = gpiod_get_array(&pdev->dev, NULL, GPIOD_OUT_LOW);
+ if (IS_ERR(s->gpios))
+ return PTR_ERR(s->gpios);
r = mdio_mux_init(&pdev->dev,
mdio_mux_gpio_switch_fn, &s->mux_handle, s);
- if (r == 0) {
- pdev->dev.platform_data = s;
- return 0;
- }
-err:
- while (n) {
- n--;
- gpiod_put(s->gpio[n]);
+ if (r != 0) {
+ gpiod_put_array(s->gpios);
+ return r;
}
- return r;
+
+ pdev->dev.platform_data = s;
+ return 0;
}
static int mdio_mux_gpio_remove(struct platform_device *pdev)
{
- unsigned int n;
struct mdio_mux_gpio_state *s = dev_get_platdata(&pdev->dev);
mdio_mux_uninit(s->mux_handle);
- for (n = 0; n < s->num_gpios; n++)
- gpiod_put(s->gpio[n]);
+ gpiod_put_array(s->gpios);
return 0;
}
--
2.0.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-02-11 16:28 [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions Rojhalat Ibrahim
@ 2015-02-26 9:54 ` Alexandre Courbot
2015-03-03 9:45 ` Rojhalat Ibrahim
2015-03-05 8:58 ` Linus Walleij
1 sibling, 1 reply; 9+ messages in thread
From: Alexandre Courbot @ 2015-02-26 9:54 UTC (permalink / raw)
To: Rojhalat Ibrahim
Cc: linux-gpio, Alexandre Courbot, Linus Walleij, Mika Westerberg,
Rafael J. Wysocki, David Miller, netdev
On Thu, Feb 12, 2015 at 1:28 AM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
> disposing of GPIO descriptors.
>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> ---
> Change log:
> v5: no change
> v4: use shorter names for members of struct gpio_descs
> v3: no change
> v2: use the new interface
>
> drivers/net/phy/mdio-mux-gpio.c | 60 ++++++++++++-----------------------------
> 1 file changed, 17 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
> index 320eb15..c49ad09 100644
> --- a/drivers/net/phy/mdio-mux-gpio.c
> +++ b/drivers/net/phy/mdio-mux-gpio.c
> @@ -12,33 +12,30 @@
> #include <linux/module.h>
> #include <linux/phy.h>
> #include <linux/mdio-mux.h>
> -#include <linux/of_gpio.h>
> +#include <linux/gpio/consumer.h>
>
> #define DRV_VERSION "1.1"
> #define DRV_DESCRIPTION "GPIO controlled MDIO bus multiplexer driver"
>
> -#define MDIO_MUX_GPIO_MAX_BITS 8
> -
> struct mdio_mux_gpio_state {
> - struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
> - unsigned int num_gpios;
> + struct gpio_descs *gpios;
> void *mux_handle;
> };
>
> static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
> void *data)
> {
> - int values[MDIO_MUX_GPIO_MAX_BITS];
> - unsigned int n;
> struct mdio_mux_gpio_state *s = data;
> + int values[s->gpios->ndescs];
> + unsigned int n;
>
> if (current_child == desired_child)
> return 0;
>
> - for (n = 0; n < s->num_gpios; n++) {
> + for (n = 0; n < s->gpios->ndescs; n++)
> values[n] = (desired_child >> n) & 1;
> - }
> - gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
> +
> + gpiod_set_array_cansleep(s->gpios->ndescs, s->gpios->desc, values);
One suggestion for a possible further improvement: it would be great
if the gpiod_set/get_array() functions would work on a struct
gpio_descs so users don't have to pass both the number of GPIOs and
the array.
I don't know whether it would be desirable to keep alternative
functions that preserve the current form, for users who want to set
multiple GPIOs but cannot use gpiod_get_array(). struct gpiod_descs is
easy to build, so maybe we don't need them?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-02-26 9:54 ` Alexandre Courbot
@ 2015-03-03 9:45 ` Rojhalat Ibrahim
2015-03-04 13:22 ` Alexandre Courbot
0 siblings, 1 reply; 9+ messages in thread
From: Rojhalat Ibrahim @ 2015-03-03 9:45 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-gpio, Alexandre Courbot, Linus Walleij, Mika Westerberg,
Rafael J. Wysocki, David Miller, netdev
On Thursday 26 February 2015 18:54:53 Alexandre Courbot wrote:
> On Thu, Feb 12, 2015 at 1:28 AM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> > Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
> > disposing of GPIO descriptors.
> >
> > Cc: David Miller <davem@davemloft.net>
> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> > ---
> > Change log:
> > v5: no change
> > v4: use shorter names for members of struct gpio_descs
> > v3: no change
> > v2: use the new interface
> >
> > drivers/net/phy/mdio-mux-gpio.c | 60 ++++++++++++-----------------------------
> > 1 file changed, 17 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
> > index 320eb15..c49ad09 100644
> > --- a/drivers/net/phy/mdio-mux-gpio.c
> > +++ b/drivers/net/phy/mdio-mux-gpio.c
> > @@ -12,33 +12,30 @@
> > #include <linux/module.h>
> > #include <linux/phy.h>
> > #include <linux/mdio-mux.h>
> > -#include <linux/of_gpio.h>
> > +#include <linux/gpio/consumer.h>
> >
> > #define DRV_VERSION "1.1"
> > #define DRV_DESCRIPTION "GPIO controlled MDIO bus multiplexer driver"
> >
> > -#define MDIO_MUX_GPIO_MAX_BITS 8
> > -
> > struct mdio_mux_gpio_state {
> > - struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
> > - unsigned int num_gpios;
> > + struct gpio_descs *gpios;
> > void *mux_handle;
> > };
> >
> > static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
> > void *data)
> > {
> > - int values[MDIO_MUX_GPIO_MAX_BITS];
> > - unsigned int n;
> > struct mdio_mux_gpio_state *s = data;
> > + int values[s->gpios->ndescs];
> > + unsigned int n;
> >
> > if (current_child == desired_child)
> > return 0;
> >
> > - for (n = 0; n < s->num_gpios; n++) {
> > + for (n = 0; n < s->gpios->ndescs; n++)
> > values[n] = (desired_child >> n) & 1;
> > - }
> > - gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
> > +
> > + gpiod_set_array_cansleep(s->gpios->ndescs, s->gpios->desc, values);
>
> One suggestion for a possible further improvement: it would be great
> if the gpiod_set/get_array() functions would work on a struct
> gpio_descs so users don't have to pass both the number of GPIOs and
> the array.
>
> I don't know whether it would be desirable to keep alternative
> functions that preserve the current form, for users who want to set
> multiple GPIOs but cannot use gpiod_get_array(). struct gpiod_descs is
> easy to build, so maybe we don't need them?
>
I thought about that, but didn't want to change the interface in this
patch series.
Furthermore there is this use case (my use case):
I acquire a descriptor array for multiple data outputs and (among others) a
single descriptor for a clock output. Afterwards I want to set the data bits
and simultaneously clear the clock bit (using gpiod_set_array) before setting
only the clock output (using gpiod_set_value).
Therefore I need an array containing the data bits and the clock bit which
is easy to build.
I could also create a struct gpiod_descs but it would be more complicated
since I would have to allocate a new struct before populating it with the
descriptors and also free the allocated memory afterwards. It's not really
a big deal but more complicated than before.
But this might not be a very common use case.
If we can assume that for the common use case the group of descriptors that
can be acquired using gpiod_get_array() is the same group that should be
set using gpiod_set_array(), it might make sense to change the interface.
Rojhalat
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-03-03 9:45 ` Rojhalat Ibrahim
@ 2015-03-04 13:22 ` Alexandre Courbot
2015-03-04 14:33 ` Rojhalat Ibrahim
0 siblings, 1 reply; 9+ messages in thread
From: Alexandre Courbot @ 2015-03-04 13:22 UTC (permalink / raw)
To: Rojhalat Ibrahim
Cc: linux-gpio, Alexandre Courbot, Linus Walleij, Mika Westerberg,
Rafael J. Wysocki, David Miller, netdev
On Tue, Mar 3, 2015 at 6:45 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> On Thursday 26 February 2015 18:54:53 Alexandre Courbot wrote:
>> On Thu, Feb 12, 2015 at 1:28 AM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
>> > Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
>> > disposing of GPIO descriptors.
>> >
>> > Cc: David Miller <davem@davemloft.net>
>> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
>> > ---
>> > Change log:
>> > v5: no change
>> > v4: use shorter names for members of struct gpio_descs
>> > v3: no change
>> > v2: use the new interface
>> >
>> > drivers/net/phy/mdio-mux-gpio.c | 60 ++++++++++++-----------------------------
>> > 1 file changed, 17 insertions(+), 43 deletions(-)
>> >
>> > diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
>> > index 320eb15..c49ad09 100644
>> > --- a/drivers/net/phy/mdio-mux-gpio.c
>> > +++ b/drivers/net/phy/mdio-mux-gpio.c
>> > @@ -12,33 +12,30 @@
>> > #include <linux/module.h>
>> > #include <linux/phy.h>
>> > #include <linux/mdio-mux.h>
>> > -#include <linux/of_gpio.h>
>> > +#include <linux/gpio/consumer.h>
>> >
>> > #define DRV_VERSION "1.1"
>> > #define DRV_DESCRIPTION "GPIO controlled MDIO bus multiplexer driver"
>> >
>> > -#define MDIO_MUX_GPIO_MAX_BITS 8
>> > -
>> > struct mdio_mux_gpio_state {
>> > - struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
>> > - unsigned int num_gpios;
>> > + struct gpio_descs *gpios;
>> > void *mux_handle;
>> > };
>> >
>> > static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
>> > void *data)
>> > {
>> > - int values[MDIO_MUX_GPIO_MAX_BITS];
>> > - unsigned int n;
>> > struct mdio_mux_gpio_state *s = data;
>> > + int values[s->gpios->ndescs];
>> > + unsigned int n;
>> >
>> > if (current_child == desired_child)
>> > return 0;
>> >
>> > - for (n = 0; n < s->num_gpios; n++) {
>> > + for (n = 0; n < s->gpios->ndescs; n++)
>> > values[n] = (desired_child >> n) & 1;
>> > - }
>> > - gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
>> > +
>> > + gpiod_set_array_cansleep(s->gpios->ndescs, s->gpios->desc, values);
>>
>> One suggestion for a possible further improvement: it would be great
>> if the gpiod_set/get_array() functions would work on a struct
>> gpio_descs so users don't have to pass both the number of GPIOs and
>> the array.
>>
>> I don't know whether it would be desirable to keep alternative
>> functions that preserve the current form, for users who want to set
>> multiple GPIOs but cannot use gpiod_get_array(). struct gpiod_descs is
>> easy to build, so maybe we don't need them?
>>
>
> I thought about that, but didn't want to change the interface in this
> patch series.
>
> Furthermore there is this use case (my use case):
>
> I acquire a descriptor array for multiple data outputs and (among others) a
> single descriptor for a clock output. Afterwards I want to set the data bits
> and simultaneously clear the clock bit (using gpiod_set_array) before setting
> only the clock output (using gpiod_set_value).
>
> Therefore I need an array containing the data bits and the clock bit which
> is easy to build.
>
> I could also create a struct gpiod_descs but it would be more complicated
> since I would have to allocate a new struct before populating it with the
> descriptors and also free the allocated memory afterwards. It's not really
> a big deal but more complicated than before.
>
> But this might not be a very common use case.
>
> If we can assume that for the common use case the group of descriptors that
> can be acquired using gpiod_get_array() is the same group that should be
> set using gpiod_set_array(), it might make sense to change the interface.
Ah, thanks for sharing your use-case. I wish I had heard it earlier as
it seems we should make things more flexible than they currently are.
If I followed you correctly, you need to call gpiod_get_array() to
obtain the data lines, and gpiod_get() for the clock line. Then you
need to allocate a new array of gpio_desc * and copy all the
descriptors there before calling gpiod_set_array(). So simply put, the
struct gpio_descs you obtained is just useless to you.
It seems like we have been doing things wrong. Maybe gpiod_get_array()
should simply take a pointer to a gpio_desc * array that it would
fill, as you originally proposed?
So now, the question is: do we need struct gpiod_descs at all? It can
help reducing the number of arguments passed to functions, but also
makes the whole API more rigid. Use it with gpiod_get_array(), and you
end up with unneeded copies and memory allocations. Pass it to
gpiod_set_array() and you cannot do things like setting only part of
the GPIOs you requested.
Argh, and looking closer there is some possible confusion between
gpiod_set_array() and gpiod_get_array(). One might expect the latter
to return the *values* of the GPIOs, considering the name of the
former, while it actually is the array counterpart of gpiod_get(). To
be consistent with the single descriptor API, I suppose we should
rename gpiod_set_array() to gpiod_set_array_value(). But that's a
separate issue.
For now, since you are the main user of the array API, what is your
opinion about gpiod_descs? Do you think it is worth making the API
less flexible just to not have to carry an array lengh separately?
Should we just get rid of it?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-03-04 13:22 ` Alexandre Courbot
@ 2015-03-04 14:33 ` Rojhalat Ibrahim
2015-03-05 9:04 ` Linus Walleij
0 siblings, 1 reply; 9+ messages in thread
From: Rojhalat Ibrahim @ 2015-03-04 14:33 UTC (permalink / raw)
To: Alexandre Courbot
Cc: linux-gpio, Alexandre Courbot, Linus Walleij, Mika Westerberg,
Rafael J. Wysocki, David Miller, netdev
On Wednesday 04 March 2015 22:22:24 Alexandre Courbot wrote:
> On Tue, Mar 3, 2015 at 6:45 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> > On Thursday 26 February 2015 18:54:53 Alexandre Courbot wrote:
> >> One suggestion for a possible further improvement: it would be great
> >> if the gpiod_set/get_array() functions would work on a struct
> >> gpio_descs so users don't have to pass both the number of GPIOs and
> >> the array.
> >>
> >> I don't know whether it would be desirable to keep alternative
> >> functions that preserve the current form, for users who want to set
> >> multiple GPIOs but cannot use gpiod_get_array(). struct gpiod_descs is
> >> easy to build, so maybe we don't need them?
> >>
> >
> > I thought about that, but didn't want to change the interface in this
> > patch series.
> >
> > Furthermore there is this use case (my use case):
> >
> > I acquire a descriptor array for multiple data outputs and (among others) a
> > single descriptor for a clock output. Afterwards I want to set the data bits
> > and simultaneously clear the clock bit (using gpiod_set_array) before setting
> > only the clock output (using gpiod_set_value).
> >
> > Therefore I need an array containing the data bits and the clock bit which
> > is easy to build.
> >
> > I could also create a struct gpiod_descs but it would be more complicated
> > since I would have to allocate a new struct before populating it with the
> > descriptors and also free the allocated memory afterwards. It's not really
> > a big deal but more complicated than before.
> >
> > But this might not be a very common use case.
> >
> > If we can assume that for the common use case the group of descriptors that
> > can be acquired using gpiod_get_array() is the same group that should be
> > set using gpiod_set_array(), it might make sense to change the interface.
>
> Ah, thanks for sharing your use-case. I wish I had heard it earlier as
> it seems we should make things more flexible than they currently are.
>
> If I followed you correctly, you need to call gpiod_get_array() to
> obtain the data lines, and gpiod_get() for the clock line. Then you
> need to allocate a new array of gpio_desc * and copy all the
> descriptors there before calling gpiod_set_array(). So simply put, the
> struct gpio_descs you obtained is just useless to you.
>
> It seems like we have been doing things wrong. Maybe gpiod_get_array()
> should simply take a pointer to a gpio_desc * array that it would
> fill, as you originally proposed?
>
> So now, the question is: do we need struct gpiod_descs at all? It can
> help reducing the number of arguments passed to functions, but also
> makes the whole API more rigid. Use it with gpiod_get_array(), and you
> end up with unneeded copies and memory allocations. Pass it to
> gpiod_set_array() and you cannot do things like setting only part of
> the GPIOs you requested.
>
> Argh, and looking closer there is some possible confusion between
> gpiod_set_array() and gpiod_get_array(). One might expect the latter
> to return the *values* of the GPIOs, considering the name of the
> former, while it actually is the array counterpart of gpiod_get(). To
> be consistent with the single descriptor API, I suppose we should
> rename gpiod_set_array() to gpiod_set_array_value(). But that's a
> separate issue.
>
> For now, since you are the main user of the array API, what is your
> opinion about gpiod_descs? Do you think it is worth making the API
> less flexible just to not have to carry an array lengh separately?
> Should we just get rid of it?
I don't think it's that bad.
As I said before my use case might be very different from the common use case.
Furthermore the alternative API, that fills a pre-allocated array, wouldn't
make things easier. I would still have an array of data lines and a separate
descriptor for the clock. For setting them all together I would still have to
create an array containing the data and the clock descriptors. Creating this
array is not a big deal because it can be a static global array or an on stack
array in the context of a function. So no calls to kalloc / kfree are needed.
And the interface as proposed in this series is very convenient for obtaining
all the GPIOs belonging to a group with a single function call and without
having to know the number of GPIOs within the group beforehand.
So if we want to support different use cases, I think it's quite good as it is.
People who want to set a group of GPIOs as obtained by gpiod_get_array() can
do so with a single call to gpiod_set_array(), the only overhead being that
they have to specify the two elements of struct gpiod_descs explicitly.
Likewise people who want to set a group of GPIOs obtained with a combination
of calls to gpiod_get_array() and gpiod_get() can do so too. They just have
to create that group first.
On the other hand if gpiod_set_array() would require a struct gpiod_descs as
argument the creation of a group for the second use case would become more
complicated as you would have to allocate a struct instead of an array, etc.
So let's just keep it the way it is and get this series merged.
About the confusing function names: I would be happy to submit a patch
renaming gpiod_set_array() to gpiod_set_array_value(), once this has been
merged. I'm a little concerned about the length of some function names though.
Isn't gpiod_set_raw_array_value_cansleep() a bit long?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-02-11 16:28 [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions Rojhalat Ibrahim
2015-02-26 9:54 ` Alexandre Courbot
@ 2015-03-05 8:58 ` Linus Walleij
2015-03-19 9:04 ` Rojhalat Ibrahim
1 sibling, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2015-03-05 8:58 UTC (permalink / raw)
To: Rojhalat Ibrahim, David Miller
Cc: linux-gpio, Alexandre Courbot, Alexandre Courbot,
Mika Westerberg, Rafael J. Wysocki, netdev
On Wed, Feb 11, 2015 at 5:28 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
> disposing of GPIO descriptors.
>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
This looks good to me, but I need David's ACK before I can
merge it through the GPIO tree.
David: is this OK with you?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-03-04 14:33 ` Rojhalat Ibrahim
@ 2015-03-05 9:04 ` Linus Walleij
2015-03-05 11:57 ` Rojhalat Ibrahim
0 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2015-03-05 9:04 UTC (permalink / raw)
To: Rojhalat Ibrahim
Cc: Alexandre Courbot, linux-gpio, Alexandre Courbot,
Mika Westerberg, Rafael J. Wysocki, David Miller, netdev
On Wed, Mar 4, 2015 at 3:33 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> And the interface as proposed in this series is very convenient for obtaining
> all the GPIOs belonging to a group with a single function call and without
> having to know the number of GPIOs within the group beforehand.
>
> So if we want to support different use cases, I think it's quite good as it is.
> People who want to set a group of GPIOs as obtained by gpiod_get_array() can
> do so with a single call to gpiod_set_array(), the only overhead being that
> they have to specify the two elements of struct gpiod_descs explicitly.
> Likewise people who want to set a group of GPIOs obtained with a combination
> of calls to gpiod_get_array() and gpiod_get() can do so too. They just have
> to create that group first.
>
> On the other hand if gpiod_set_array() would require a struct gpiod_descs as
> argument the creation of a group for the second use case would become more
> complicated as you would have to allocate a struct instead of an array, etc.
>
> So let's just keep it the way it is and get this series merged.
I've merged it.
But can you make a separate patch to Documentation/gpio/consumer.txt
describing the array usecase(s) a bit in detail so people realize when it's
good to use these functions?
> About the confusing function names: I would be happy to submit a patch
> renaming gpiod_set_array() to gpiod_set_array_value(), once this has been
> merged. I'm a little concerned about the length of some function names though.
> Isn't gpiod_set_raw_array_value_cansleep() a bit long?
Just patch it and we'll discuss it... :)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-03-05 9:04 ` Linus Walleij
@ 2015-03-05 11:57 ` Rojhalat Ibrahim
0 siblings, 0 replies; 9+ messages in thread
From: Rojhalat Ibrahim @ 2015-03-05 11:57 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Courbot, linux-gpio, Alexandre Courbot,
Mika Westerberg, Rafael J. Wysocki, David Miller, netdev
On Thursday 05 March 2015 10:04:20 Linus Walleij wrote:
> On Wed, Mar 4, 2015 at 3:33 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
>
> > And the interface as proposed in this series is very convenient for obtaining
> > all the GPIOs belonging to a group with a single function call and without
> > having to know the number of GPIOs within the group beforehand.
> >
> > So if we want to support different use cases, I think it's quite good as it is.
> > People who want to set a group of GPIOs as obtained by gpiod_get_array() can
> > do so with a single call to gpiod_set_array(), the only overhead being that
> > they have to specify the two elements of struct gpiod_descs explicitly.
> > Likewise people who want to set a group of GPIOs obtained with a combination
> > of calls to gpiod_get_array() and gpiod_get() can do so too. They just have
> > to create that group first.
> >
> > On the other hand if gpiod_set_array() would require a struct gpiod_descs as
> > argument the creation of a group for the second use case would become more
> > complicated as you would have to allocate a struct instead of an array, etc.
> >
> > So let's just keep it the way it is and get this series merged.
>
> I've merged it.
>
Thanks.
> But can you make a separate patch to Documentation/gpio/consumer.txt
> describing the array usecase(s) a bit in detail so people realize when it's
> good to use these functions?
>
Sure.
> > About the confusing function names: I would be happy to submit a patch
> > renaming gpiod_set_array() to gpiod_set_array_value(), once this has been
> > merged. I'm a little concerned about the length of some function names though.
> > Isn't gpiod_set_raw_array_value_cansleep() a bit long?
>
> Just patch it and we'll discuss it... :)
>
Ok. I'll submit a patch after you've merged patch 4/4 in this series in order
to avoid conflicts.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
2015-03-05 8:58 ` Linus Walleij
@ 2015-03-19 9:04 ` Rojhalat Ibrahim
0 siblings, 0 replies; 9+ messages in thread
From: Rojhalat Ibrahim @ 2015-03-19 9:04 UTC (permalink / raw)
To: David Miller
Cc: Linus Walleij, linux-gpio, Alexandre Courbot, Alexandre Courbot,
Mika Westerberg, Rafael J. Wysocki, netdev
Ping?
On Thursday 05 March 2015 09:58:00 Linus Walleij wrote:
> On Wed, Feb 11, 2015 at 5:28 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
>
> > Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
> > disposing of GPIO descriptors.
> >
> > Cc: David Miller <davem@davemloft.net>
> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
>
> This looks good to me, but I need David's ACK before I can
> merge it through the GPIO tree.
>
> David: is this OK with you?
>
> Yours,
> Linus Walleij
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-03-19 9:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-11 16:28 [PATCH v5 4/4] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions Rojhalat Ibrahim
2015-02-26 9:54 ` Alexandre Courbot
2015-03-03 9:45 ` Rojhalat Ibrahim
2015-03-04 13:22 ` Alexandre Courbot
2015-03-04 14:33 ` Rojhalat Ibrahim
2015-03-05 9:04 ` Linus Walleij
2015-03-05 11:57 ` Rojhalat Ibrahim
2015-03-05 8:58 ` Linus Walleij
2015-03-19 9:04 ` Rojhalat Ibrahim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.