netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function
@ 2014-11-20 12:24 Rojhalat Ibrahim
  2014-11-21 20:13 ` David Miller
  2014-11-25  3:39 ` Alexandre Courbot
  0 siblings, 2 replies; 6+ messages in thread
From: Rojhalat Ibrahim @ 2014-11-20 12:24 UTC (permalink / raw)
  To: Florian Fainelli, netdev, David Daney
  Cc: linux-gpio, Linus Walleij, Alexandre Courbot

Convert mdio-mux-gpio to the GPIO descriptor interface and use the new
gpiod_set_array function to set all output signals simultaneously.

Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
--
This patch depends on the gpiod_set_array function, which is available in
the linux-gpio devel tree.

 drivers/net/phy/mdio-mux-gpio.c |   35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 0966951..3fdea96 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -14,13 +14,13 @@
 #include <linux/mdio-mux.h>
 #include <linux/of_gpio.h>
 
-#define DRV_VERSION "1.0"
+#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 {
-	int gpio[MDIO_MUX_GPIO_MAX_BITS];
+	struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
 	unsigned int num_gpios;
 	void *mux_handle;
 };
@@ -28,29 +28,23 @@ struct mdio_mux_gpio_state {
 static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
 				   void *data)
 {
-	int change;
+	int values[MDIO_MUX_GPIO_MAX_BITS];
 	unsigned int n;
 	struct mdio_mux_gpio_state *s = data;
 
 	if (current_child == desired_child)
 		return 0;
 
-	change = current_child == -1 ? -1 : current_child ^ desired_child;
-
 	for (n = 0; n < s->num_gpios; n++) {
-		if (change & 1)
-			gpio_set_value_cansleep(s->gpio[n],
-						(desired_child & 1) != 0);
-		change >>= 1;
-		desired_child >>= 1;
+		values[n] = (desired_child >> n) & 1;
 	}
+	gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
 
 	return 0;
 }
 
 static int mdio_mux_gpio_probe(struct platform_device *pdev)
 {
-	enum of_gpio_flags f;
 	struct mdio_mux_gpio_state *s;
 	int num_gpios;
 	unsigned int n;
@@ -70,20 +64,17 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
 	s->num_gpios = num_gpios;
 
 	for (n = 0; n < num_gpios; ) {
-		int gpio = of_get_gpio_flags(pdev->dev.of_node, n, &f);
-		if (gpio < 0) {
-			r = (gpio == -ENODEV) ? -EPROBE_DEFER : gpio;
+		struct gpio_desc *gpio = gpiod_get_index(&pdev->dev,
+							 "mdio-mux-gpio", n);
+		if (IS_ERR(gpio)) {
+			r = PTR_ERR(gpio);
 			goto err;
 		}
 		s->gpio[n] = gpio;
 
 		n++;
 
-		r = gpio_request(gpio, "mdio_mux_gpio");
-		if (r)
-			goto err;
-
-		r = gpio_direction_output(gpio, 0);
+		r = gpiod_direction_output(gpio, 0);
 		if (r)
 			goto err;
 	}
@@ -98,15 +89,19 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
 err:
 	while (n) {
 		n--;
-		gpio_free(s->gpio[n]);
+		gpiod_put(s->gpio[n]);
 	}
 	return r;
 }
 
 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]);
+	}
 	return 0;
 }
 
--
2.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function
  2014-11-20 12:24 [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function Rojhalat Ibrahim
@ 2014-11-21 20:13 ` David Miller
  2014-11-24  8:35   ` Rojhalat Ibrahim
  2014-11-25  3:39 ` Alexandre Courbot
  1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2014-11-21 20:13 UTC (permalink / raw)
  To: imr; +Cc: f.fainelli, netdev, david.daney, linux-gpio, linus.walleij, acourbot

From: Rojhalat Ibrahim <imr@rtschenk.de>
Date: Thu, 20 Nov 2014 13:24:44 +0100

> Convert mdio-mux-gpio to the GPIO descriptor interface and use the new
> gpiod_set_array function to set all output signals simultaneously.
> 
> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> --
> This patch depends on the gpiod_set_array function, which is available in
> the linux-gpio devel tree.

Then I really can't apply it to the networking GIT tree.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function
  2014-11-21 20:13 ` David Miller
@ 2014-11-24  8:35   ` Rojhalat Ibrahim
  2014-11-24 18:06     ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Rojhalat Ibrahim @ 2014-11-24  8:35 UTC (permalink / raw)
  To: David Miller
  Cc: f.fainelli, netdev, david.daney, linux-gpio, linus.walleij, acourbot

On Friday 21 November 2014 15:13:01 David Miller wrote:
> From: Rojhalat Ibrahim <imr@rtschenk.de>
> Date: Thu, 20 Nov 2014 13:24:44 +0100
> 
> > Convert mdio-mux-gpio to the GPIO descriptor interface and use the new
> > gpiod_set_array function to set all output signals simultaneously.
> > 
> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> > --
> > This patch depends on the gpiod_set_array function, which is available in
> > the linux-gpio devel tree.
> 
> Then I really can't apply it to the networking GIT tree.
> 

Then maybe you could ack the patch and then Linus Walleij could apply it to
the GPIO tree, right?

   Rojhalat



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function
  2014-11-24  8:35   ` Rojhalat Ibrahim
@ 2014-11-24 18:06     ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2014-11-24 18:06 UTC (permalink / raw)
  To: imr; +Cc: f.fainelli, netdev, david.daney, linux-gpio, linus.walleij, acourbot

From: Rojhalat Ibrahim <imr@rtschenk.de>
Date: Mon, 24 Nov 2014 09:35:02 +0100

> On Friday 21 November 2014 15:13:01 David Miller wrote:
>> From: Rojhalat Ibrahim <imr@rtschenk.de>
>> Date: Thu, 20 Nov 2014 13:24:44 +0100
>> 
>> > Convert mdio-mux-gpio to the GPIO descriptor interface and use the new
>> > gpiod_set_array function to set all output signals simultaneously.
>> > 
>> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
>> > --
>> > This patch depends on the gpiod_set_array function, which is available in
>> > the linux-gpio devel tree.
>> 
>> Then I really can't apply it to the networking GIT tree.
>> 
> 
> Then maybe you could ack the patch and then Linus Walleij could apply it to
> the GPIO tree, right?

Yep, I can do that:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function
  2014-11-20 12:24 [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function Rojhalat Ibrahim
  2014-11-21 20:13 ` David Miller
@ 2014-11-25  3:39 ` Alexandre Courbot
  2014-11-25  8:13   ` Rojhalat Ibrahim
  1 sibling, 1 reply; 6+ messages in thread
From: Alexandre Courbot @ 2014-11-25  3:39 UTC (permalink / raw)
  To: Rojhalat Ibrahim
  Cc: Florian Fainelli, netdev, David Daney, linux-gpio, Linus Walleij,
	Alexandre Courbot

On Thu, Nov 20, 2014 at 9:24 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> Convert mdio-mux-gpio to the GPIO descriptor interface and use the new
> gpiod_set_array function to set all output signals simultaneously.
>
> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> --
> This patch depends on the gpiod_set_array function, which is available in
> the linux-gpio devel tree.
>
>  drivers/net/phy/mdio-mux-gpio.c |   35 +++++++++++++++--------------------
>  1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
> index 0966951..3fdea96 100644
> --- a/drivers/net/phy/mdio-mux-gpio.c
> +++ b/drivers/net/phy/mdio-mux-gpio.c
> @@ -14,13 +14,13 @@
>  #include <linux/mdio-mux.h>
>  #include <linux/of_gpio.h>
>
> -#define DRV_VERSION "1.0"
> +#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 {
> -       int gpio[MDIO_MUX_GPIO_MAX_BITS];
> +       struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
>         unsigned int num_gpios;
>         void *mux_handle;
>  };
> @@ -28,29 +28,23 @@ struct mdio_mux_gpio_state {
>  static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
>                                    void *data)
>  {
> -       int change;
> +       int values[MDIO_MUX_GPIO_MAX_BITS];
>         unsigned int n;
>         struct mdio_mux_gpio_state *s = data;
>
>         if (current_child == desired_child)
>                 return 0;
>
> -       change = current_child == -1 ? -1 : current_child ^ desired_child;
> -
>         for (n = 0; n < s->num_gpios; n++) {
> -               if (change & 1)
> -                       gpio_set_value_cansleep(s->gpio[n],
> -                                               (desired_child & 1) != 0);
> -               change >>= 1;
> -               desired_child >>= 1;
> +               values[n] = (desired_child >> n) & 1;
>         }
> +       gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
>
>         return 0;
>  }
>
>  static int mdio_mux_gpio_probe(struct platform_device *pdev)
>  {
> -       enum of_gpio_flags f;
>         struct mdio_mux_gpio_state *s;
>         int num_gpios;
>         unsigned int n;
> @@ -70,20 +64,17 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
>         s->num_gpios = num_gpios;
>
>         for (n = 0; n < num_gpios; ) {
> -               int gpio = of_get_gpio_flags(pdev->dev.of_node, n, &f);
> -               if (gpio < 0) {
> -                       r = (gpio == -ENODEV) ? -EPROBE_DEFER : gpio;
> +               struct gpio_desc *gpio = gpiod_get_index(&pdev->dev,
> +                                                        "mdio-mux-gpio", n);

Doesn't this change introduce some incompatibility against the current
DT bindings? of_get_gpio_flags() looks for a "gpios" property, while
your call to gpiod_get_index() will look for "mdio-mux-gpio-gpios". It
should probably be changed to gpiod_get_index(&pdev->dev, NULL, n).

... or even be changed to gpiod_get_index(&pdev->dev, NULL, n,
GPIOD_OUT_LOW) as the calls to gpiod_get() functions are being updated
to take an initial state (both variants are currently usable but the
former one will disappear in the future). This will also allow you to
get rid of your call to gpiod_direction_output().

Side-note: it would be nice to have a gpiod_get_array() call that does
exactly what this loop does, and returns an array of gpios directly
usable by gpiod_set_array*(). And a matching gpiod_put_array() of
course.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function
  2014-11-25  3:39 ` Alexandre Courbot
@ 2014-11-25  8:13   ` Rojhalat Ibrahim
  0 siblings, 0 replies; 6+ messages in thread
From: Rojhalat Ibrahim @ 2014-11-25  8:13 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Florian Fainelli, netdev, David Daney, linux-gpio, Linus Walleij,
	Alexandre Courbot

On Tuesday 25 November 2014 12:39:13 Alexandre Courbot wrote:
> On Thu, Nov 20, 2014 at 9:24 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> > @@ -70,20 +64,17 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
> >         s->num_gpios = num_gpios;
> >
> >         for (n = 0; n < num_gpios; ) {
> > -               int gpio = of_get_gpio_flags(pdev->dev.of_node, n, &f);
> > -               if (gpio < 0) {
> > -                       r = (gpio == -ENODEV) ? -EPROBE_DEFER : gpio;
> > +               struct gpio_desc *gpio = gpiod_get_index(&pdev->dev,
> > +                                                        "mdio-mux-gpio", n);
> 
> Doesn't this change introduce some incompatibility against the current
> DT bindings? of_get_gpio_flags() looks for a "gpios" property, while
> your call to gpiod_get_index() will look for "mdio-mux-gpio-gpios". It
> should probably be changed to gpiod_get_index(&pdev->dev, NULL, n).
> 

You are right, of course. I'll fix it and post a new patch version.

> ... or even be changed to gpiod_get_index(&pdev->dev, NULL, n,
> GPIOD_OUT_LOW) as the calls to gpiod_get() functions are being updated
> to take an initial state (both variants are currently usable but the
> former one will disappear in the future). This will also allow you to
> get rid of your call to gpiod_direction_output().
> 

Will do that.

> Side-note: it would be nice to have a gpiod_get_array() call that does
> exactly what this loop does, and returns an array of gpios directly
> usable by gpiod_set_array*(). And a matching gpiod_put_array() of
> course.
> 

I'll add that to my to-do-list.


Thanks
   Rojhalat


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-11-25  8:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-20 12:24 [PATCH] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function Rojhalat Ibrahim
2014-11-21 20:13 ` David Miller
2014-11-24  8:35   ` Rojhalat Ibrahim
2014-11-24 18:06     ` David Miller
2014-11-25  3:39 ` Alexandre Courbot
2014-11-25  8:13   ` Rojhalat Ibrahim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).