All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] brcm80211: brcmsmac: Move LEDs to GPIO descriptors
@ 2020-07-06  8:47 Linus Walleij
  2020-07-06 16:09   ` kernel test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Linus Walleij @ 2020-07-06  8:47 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng
  Cc: linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
	Linus Walleij, Frank Kao, Kalle Valo

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>
---
 .../broadcom/brcm80211/brcmsmac/led.c         | 63 ++++++++-----------
 .../broadcom/brcm80211/brcmsmac/led.h         |  6 +-
 2 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c
index c0a5449ed72c..099e8585f0d4 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,31 +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);
-		return 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)) {
+		wiphy_err(wl->wiphy, "requesting led GPIO failed (err: %d)\n",
+			  err);
+		return PTR_ERR(radio_led->gpiod);
 	}
 
 	snprintf(wl->radio_led.name, sizeof(wl->radio_led.name),
@@ -117,11 +107,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
-- 
2.25.4


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

* Re: [PATCH] brcm80211: brcmsmac: Move LEDs to GPIO descriptors
  2020-07-06  8:47 [PATCH] brcm80211: brcmsmac: Move LEDs to GPIO descriptors Linus Walleij
@ 2020-07-06 16:09   ` kernel test robot
  0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2020-07-06 16:09 UTC (permalink / raw)
  To: Linus Walleij, Arend van Spriel, Franky Lin, Hante Meuleman,
	Chi-Hsien Lin, Wright Feng
  Cc: kbuild-all, clang-built-linux, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, Linus Walleij,
	Frank Kao

[-- Attachment #1: Type: text/plain, Size: 3690 bytes --]

Hi Linus,

I love your patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on wireless-drivers/master v5.8-rc4 next-20200706]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Linus-Walleij/brcm80211-brcmsmac-Move-LEDs-to-GPIO-descriptors/20200706-165145
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project a378c0449507e00e96534ff9ce9034e185425182)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c:91:6: warning: variable 'err' is uninitialized when used here [-Wuninitialized]
                             err);
                             ^~~
   include/net/cfg80211.h:7837:35: note: expanded from macro 'wiphy_err'
           dev_err(&(wiphy)->dev, format, ##args)
                                            ^~~~
   include/linux/dev_printk.h:104:32: note: expanded from macro 'dev_err'
           _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
                                         ^~~~~~~~~~~
   drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c:53:12: note: initialize the variable 'err' to silence this warning
           int i, err;
                     ^
                      = 0
   1 warning generated.

vim +/err +91 drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c

    50	
    51	int brcms_led_register(struct brcms_info *wl)
    52	{
    53		int i, err;
    54		struct brcms_led *radio_led = &wl->radio_led;
    55		/* get CC core */
    56		struct bcma_drv_cc *cc_drv  = &wl->wlc->hw->d11core->bus->drv_cc;
    57		struct gpio_chip *bcma_gpio = &cc_drv->gpio;
    58		struct ssb_sprom *sprom = &wl->wlc->hw->d11core->bus->sprom;
    59		u8 *leds[] = { &sprom->gpio0,
    60			&sprom->gpio1,
    61			&sprom->gpio2,
    62			&sprom->gpio3 };
    63		int hwnum = -1;
    64		enum gpio_lookup_flags lflags = GPIO_ACTIVE_HIGH;
    65	
    66		if (!bcma_gpio || !gpio_is_valid(bcma_gpio->base))
    67			return -ENODEV;
    68	
    69		/* find radio enabled LED */
    70		for (i = 0; i < BRCMS_LED_NO; i++) {
    71			u8 led = *leds[i];
    72			if ((led & BRCMS_LED_BEH_MASK) == BRCMS_LED_RADIO) {
    73				hwnum = i;
    74				if (led & BRCMS_LED_AL_MASK)
    75					lflags = GPIO_ACTIVE_LOW;
    76				break;
    77			}
    78		}
    79	
    80		/* No LED, bail out */
    81		if (hwnum == -1)
    82			return -ENODEV;
    83	
    84		/* Try to obtain this LED GPIO line */
    85		radio_led->gpiod = gpiochip_request_own_desc(bcma_gpio, hwnum,
    86							     "radio on", lflags,
    87							     GPIOD_OUT_LOW);
    88	
    89		if (IS_ERR(radio_led->gpiod)) {
    90			wiphy_err(wl->wiphy, "requesting led GPIO failed (err: %d)\n",
  > 91				  err);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 75293 bytes --]

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

* Re: [PATCH] brcm80211: brcmsmac: Move LEDs to GPIO descriptors
@ 2020-07-06 16:09   ` kernel test robot
  0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2020-07-06 16:09 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3782 bytes --]

Hi Linus,

I love your patch! Perhaps something to improve:

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on wireless-drivers/master v5.8-rc4 next-20200706]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Linus-Walleij/brcm80211-brcmsmac-Move-LEDs-to-GPIO-descriptors/20200706-165145
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project a378c0449507e00e96534ff9ce9034e185425182)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c:91:6: warning: variable 'err' is uninitialized when used here [-Wuninitialized]
                             err);
                             ^~~
   include/net/cfg80211.h:7837:35: note: expanded from macro 'wiphy_err'
           dev_err(&(wiphy)->dev, format, ##args)
                                            ^~~~
   include/linux/dev_printk.h:104:32: note: expanded from macro 'dev_err'
           _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
                                         ^~~~~~~~~~~
   drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c:53:12: note: initialize the variable 'err' to silence this warning
           int i, err;
                     ^
                      = 0
   1 warning generated.

vim +/err +91 drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c

    50	
    51	int brcms_led_register(struct brcms_info *wl)
    52	{
    53		int i, err;
    54		struct brcms_led *radio_led = &wl->radio_led;
    55		/* get CC core */
    56		struct bcma_drv_cc *cc_drv  = &wl->wlc->hw->d11core->bus->drv_cc;
    57		struct gpio_chip *bcma_gpio = &cc_drv->gpio;
    58		struct ssb_sprom *sprom = &wl->wlc->hw->d11core->bus->sprom;
    59		u8 *leds[] = { &sprom->gpio0,
    60			&sprom->gpio1,
    61			&sprom->gpio2,
    62			&sprom->gpio3 };
    63		int hwnum = -1;
    64		enum gpio_lookup_flags lflags = GPIO_ACTIVE_HIGH;
    65	
    66		if (!bcma_gpio || !gpio_is_valid(bcma_gpio->base))
    67			return -ENODEV;
    68	
    69		/* find radio enabled LED */
    70		for (i = 0; i < BRCMS_LED_NO; i++) {
    71			u8 led = *leds[i];
    72			if ((led & BRCMS_LED_BEH_MASK) == BRCMS_LED_RADIO) {
    73				hwnum = i;
    74				if (led & BRCMS_LED_AL_MASK)
    75					lflags = GPIO_ACTIVE_LOW;
    76				break;
    77			}
    78		}
    79	
    80		/* No LED, bail out */
    81		if (hwnum == -1)
    82			return -ENODEV;
    83	
    84		/* Try to obtain this LED GPIO line */
    85		radio_led->gpiod = gpiochip_request_own_desc(bcma_gpio, hwnum,
    86							     "radio on", lflags,
    87							     GPIOD_OUT_LOW);
    88	
    89		if (IS_ERR(radio_led->gpiod)) {
    90			wiphy_err(wl->wiphy, "requesting led GPIO failed (err: %d)\n",
  > 91				  err);

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 75293 bytes --]

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

end of thread, other threads:[~2020-07-06 17:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06  8:47 [PATCH] brcm80211: brcmsmac: Move LEDs to GPIO descriptors Linus Walleij
2020-07-06 16:09 ` kernel test robot
2020-07-06 16:09   ` kernel test robot

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.