diff mbox series

[v2] brcm80211: brcmsmac: Move LEDs to GPIO descriptors

Message ID 20200711210150.4943-1-linus.walleij@linaro.org (mailing list archive)
State Accepted
Commit b424808115cb0141da851082d84ede20caa2215c
Delegated to: Kalle Valo
Headers show
Series [v2] brcm80211: brcmsmac: Move LEDs to GPIO descriptors | expand

Commit Message

Linus Walleij July 11, 2020, 9:01 p.m. UTC
The code in the BRCM80211 BRCMSMAC driver is using the legacy
GPIO API to to a complex check of the validity of the base of
the GPIO chip and whether it is present at all and then adding
an offset to the base of the chip.

Use the existing function to obtain a GPIO line internally
from a GPIO chip so we can use the offset directly and
modernize the code to use GPIO descriptors instead of integers
from the global GPIO numberspace.

Cc: Wright Feng <wright.feng@cypress.com>
Cc: Frank Kao <frank.kao@cypress.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Use the PTR_ERR() error code in an error message found
  uninitialized by the kernel test robot.
---
 .../broadcom/brcm80211/brcmsmac/led.c         | 62 ++++++++-----------
 .../broadcom/brcm80211/brcmsmac/led.h         |  6 +-
 2 files changed, 29 insertions(+), 39 deletions(-)

Comments

Kalle Valo July 14, 2020, 9:51 a.m. UTC | #1
Linus Walleij <linus.walleij@linaro.org> wrote:

> The code in the BRCM80211 BRCMSMAC driver is using the legacy
> GPIO API to to a complex check of the validity of the base of
> the GPIO chip and whether it is present at all and then adding
> an offset to the base of the chip.
> 
> Use the existing function to obtain a GPIO line internally
> from a GPIO chip so we can use the offset directly and
> modernize the code to use GPIO descriptors instead of integers
> from the global GPIO numberspace.
> 
> Cc: Wright Feng <wright.feng@cypress.com>
> Cc: Frank Kao <frank.kao@cypress.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Patch applied to wireless-drivers-next.git, thanks.

b424808115cb brcm80211: brcmsmac: Move LEDs to GPIO descriptors
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c
index c0a5449ed72c..c1b9ac692d26 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c
@@ -1,7 +1,9 @@ 
 // SPDX-License-Identifier: GPL-2.0
 #include <net/mac80211.h>
 #include <linux/bcma/bcma_driver_chipcommon.h>
-#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/machine.h>
+#include <linux/gpio/consumer.h>
 
 #include "mac80211_if.h"
 #include "pub.h"
@@ -19,16 +21,13 @@ 
 
 static void brcms_radio_led_ctrl(struct brcms_info *wl, bool state)
 {
-	if (wl->radio_led.gpio == -1)
+	if (!wl->radio_led.gpiod)
 		return;
 
-	if (wl->radio_led.active_low)
-		state = !state;
-
 	if (state)
-		gpio_set_value(wl->radio_led.gpio, 1);
+		gpiod_set_value(wl->radio_led.gpiod, 1);
 	else
-		gpio_set_value(wl->radio_led.gpio, 0);
+		gpiod_set_value(wl->radio_led.gpiod, 0);
 }
 
 
@@ -45,8 +44,8 @@  void brcms_led_unregister(struct brcms_info *wl)
 {
 	if (wl->led_dev.dev)
 		led_classdev_unregister(&wl->led_dev);
-	if (wl->radio_led.gpio != -1)
-		gpio_free(wl->radio_led.gpio);
+	if (wl->radio_led.gpiod)
+		gpiochip_free_own_desc(wl->radio_led.gpiod);
 }
 
 int brcms_led_register(struct brcms_info *wl)
@@ -61,12 +60,8 @@  int brcms_led_register(struct brcms_info *wl)
 		&sprom->gpio1,
 		&sprom->gpio2,
 		&sprom->gpio3 };
-	unsigned gpio = -1;
-	bool active_low = false;
-
-	/* none by default */
-	radio_led->gpio = -1;
-	radio_led->active_low = false;
+	int hwnum = -1;
+	enum gpio_lookup_flags lflags = GPIO_ACTIVE_HIGH;
 
 	if (!bcma_gpio || !gpio_is_valid(bcma_gpio->base))
 		return -ENODEV;
@@ -75,30 +70,26 @@  int brcms_led_register(struct brcms_info *wl)
 	for (i = 0; i < BRCMS_LED_NO; i++) {
 		u8 led = *leds[i];
 		if ((led & BRCMS_LED_BEH_MASK) == BRCMS_LED_RADIO) {
-			gpio = bcma_gpio->base + i;
+			hwnum = i;
 			if (led & BRCMS_LED_AL_MASK)
-				active_low = true;
+				lflags = GPIO_ACTIVE_LOW;
 			break;
 		}
 	}
 
-	if (gpio == -1 || !gpio_is_valid(gpio))
+	/* No LED, bail out */
+	if (hwnum == -1)
 		return -ENODEV;
 
-	/* request and configure LED gpio */
-	err = gpio_request_one(gpio,
-				active_low ? GPIOF_OUT_INIT_HIGH
-					: GPIOF_OUT_INIT_LOW,
-				"radio on");
-	if (err) {
-		wiphy_err(wl->wiphy, "requesting led gpio %d failed (err: %d)\n",
-			  gpio, err);
-		return err;
-	}
-	err = gpio_direction_output(gpio, 1);
-	if (err) {
-		wiphy_err(wl->wiphy, "cannot set led gpio %d to output (err: %d)\n",
-			  gpio, err);
+	/* Try to obtain this LED GPIO line */
+	radio_led->gpiod = gpiochip_request_own_desc(bcma_gpio, hwnum,
+						     "radio on", lflags,
+						     GPIOD_OUT_LOW);
+
+	if (IS_ERR(radio_led->gpiod)) {
+		err = PTR_ERR(radio_led->gpiod);
+		wiphy_err(wl->wiphy, "requesting led GPIO failed (err: %d)\n",
+			  err);
 		return err;
 	}
 
@@ -117,11 +108,8 @@  int brcms_led_register(struct brcms_info *wl)
 		return err;
 	}
 
-	wiphy_info(wl->wiphy, "registered radio enabled led device: %s gpio: %d\n",
-		   wl->radio_led.name,
-		   gpio);
-	radio_led->gpio = gpio;
-	radio_led->active_low = active_low;
+	wiphy_info(wl->wiphy, "registered radio enabled led device: %s\n",
+		   wl->radio_led.name);
 
 	return 0;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h
index 17a0b1f5dbcf..d65f5c268fd7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.h
@@ -16,10 +16,12 @@ 
 
 #ifndef _BRCM_LED_H_
 #define _BRCM_LED_H_
+
+struct gpio_desc;
+
 struct brcms_led {
 	char name[32];
-	unsigned gpio;
-	bool active_low;
+	struct gpio_desc *gpiod;
 };
 
 #ifdef CONFIG_BCMA_DRIVER_GPIO