Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets
On 2 March 2018 at 10:22, Sebastian Gottschall wrote: >>> leds-gpio is crap and limited. you can just register one platform data at >>> kernel runtime since its identified by its object name "led-gpio" but the >>> kernel forbidds to register 2 platform datas with the same name >>> consider the ar71xx devices with qca988x wifi chipsets. they all have >>> already a led platform data registered >>> at boottime. a second can't be registered anymore so gpio_led is useless >>> at >>> all for most developers on such platforms. its mainly used for early >>> kernel >>> platform data initialisation for system leds. >> >> If leds-gpio has limitations, please fix those, rather then >> introducing duplicated code. > > there is no duplicated code introduced and there is no solution for it. > consider that all wifi drivers with softled support > are going that way with registering a own led driver. see ath9k for > instance. gpio-led cannot be used for it and there is no way to > support multiple platform datas with the same name. its a kernel limitation I just reviewed some mips arch patch adding support for more LEDs for selected devices: [PATCH] MIPS: BCM47XX: Add Luxul XAP1500/XWR1750 WiFi LEDs https://patchwork.linux-mips.org/patch/18681/ It seems to be simply adding another call to the gpio_led_register_device(). It seems to me you can call that function multiple times and register multiple structs with LEDs. Isn't all you need something like this? static const struct gpio_led ath10k_leds[] = { { .name = "ath10k:color:function", .active_low = 1, .default_state = LEDS_GPIO_DEFSTATE_KEEP, } }; static struct gpio_led_platform_data bcm47xx_leds_pdata_extra = { leds = ath10k_leds; num_leds = ARRAY_SIZE(ath10k_leds); }; ath10k_leds.gpio = ar->hw_params.led_pin; gpio_led_register_device(0, &ath10k_leds); -- Rafał ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets
On 26 February 2018 at 09:44, wrote: > From: Sebastian Gottschall > > Adds LED and GPIO Control support for 988x, 9887, 9888, 99x0, 9984 based > chipsets with on chipset connected led's > using WMI Firmware API. > The LED device will get available named as "ath10k-phyX" at sysfs and can be > controlled with various triggers. > adds also debugfs interface for gpio control. Hi Sebastian, I just noticed this patch and have one question. It seems you register GPIO chip and that WiFi LED is controller by a GPIO. Shouldn't you use leds-gpio driver and just register platform device with gpio_led_platform_data? That way you could avoid some code duplication I think? It seems to be the purpose of leds-gpio driver. > Signed-off-by: Sebastian Gottschall > > v2 add correct gpio count per chipset and remove IPQ4019 support since this > chipset does not make use of specific gpios) > v5 fix compiling without LED_CLASS and GPIOLIB support, fix also error by > kbuild test robot which does not occur in standard builds. curious > v6 correct return values and fix comment style > v7 fix ath10k_unregister_led for compiling without LED_CLASS > v8 fix various code design issues reported by reviewers > v9 move led and led code to separate sourcefile (gpio.c) > v10 compile fix if gpiolib isnt included > v11 make register_gpio_chip static. advise by krobot > v12 fix warning > --- > drivers/net/wireless/ath/ath10k/Kconfig | 10 ++ > drivers/net/wireless/ath/ath10k/Makefile | 1 + > drivers/net/wireless/ath/ath10k/core.c| 28 - > drivers/net/wireless/ath/ath10k/core.h| 62 +- > drivers/net/wireless/ath/ath10k/debug.c | 146 ++ > drivers/net/wireless/ath/ath10k/gpio.c| 196 > ++ > drivers/net/wireless/ath/ath10k/hw.h | 2 + > drivers/net/wireless/ath/ath10k/mac.c | 5 + > drivers/net/wireless/ath/ath10k/wmi-ops.h | 36 +- > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 65 ++ > drivers/net/wireless/ath/ath10k/wmi.c | 46 +++ > drivers/net/wireless/ath/ath10k/wmi.h | 36 ++ > 12 files changed, 630 insertions(+), 3 deletions(-) > create mode 100644 drivers/net/wireless/ath/ath10k/gpio.c > > diff --git a/drivers/net/wireless/ath/ath10k/Kconfig > b/drivers/net/wireless/ath/ath10k/Kconfig > index deb5ae21a559..5d61d499dca4 100644 > --- a/drivers/net/wireless/ath/ath10k/Kconfig > +++ b/drivers/net/wireless/ath/ath10k/Kconfig > @@ -10,6 +10,16 @@ config ATH10K > >If you choose to build a module, it'll be called ath10k. > > +config ATH10K_LEDS > + bool "SoftLED Support" > + depends on ATH10K > + select MAC80211_LEDS > + select LEDS_CLASS > + select NEW_LEDS > + default y > + help > + This option is necessary, if you want LED support for chipset > connected led pins > + > config ATH10K_PCI > tristate "Atheros ath10k PCI support" > depends on ATH10K && PCI > diff --git a/drivers/net/wireless/ath/ath10k/Makefile > b/drivers/net/wireless/ath/ath10k/Makefile > index 6739ac26fd29..eccc9806fa43 100644 > --- a/drivers/net/wireless/ath/ath10k/Makefile > +++ b/drivers/net/wireless/ath/ath10k/Makefile > @@ -20,6 +20,7 @@ ath10k_core-$(CONFIG_NL80211_TESTMODE) += testmode.o > ath10k_core-$(CONFIG_ATH10K_TRACING) += trace.o > ath10k_core-$(CONFIG_THERMAL) += thermal.o > ath10k_core-$(CONFIG_MAC80211_DEBUGFS) += debugfs_sta.o > +ath10k_core-$(CONFIG_ATH10K_LEDS) += gpio.o > ath10k_core-$(CONFIG_PM) += wow.o > ath10k_core-$(CONFIG_DEV_COREDUMP) += coredump.o > > diff --git a/drivers/net/wireless/ath/ath10k/core.c > b/drivers/net/wireless/ath/ath10k/core.c > index f3ec13b80b20..d7f89ca98c2d 100644 > --- a/drivers/net/wireless/ath/ath10k/core.c > +++ b/drivers/net/wireless/ath/ath10k/core.c > @@ -21,6 +21,10 @@ > #include > #include > #include > +#include > +#include > +#include > + > > #include "core.h" > #include "mac.h" > @@ -65,6 +69,8 @@ static const struct ath10k_hw_params > ath10k_hw_params_list[] = { > .id = QCA988X_HW_2_0_VERSION, > .dev_id = QCA988X_2_0_DEVICE_ID, > .name = "qca988x hw2.0", > + .led_pin = 1, > + .gpio_count = 24, > .patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, > @@ -94,6 +100,8 @@ static const struct ath10k_hw_params > ath10k_hw_params_list[] = { > .id = QCA988X_HW_2_0_VERSION, > .dev_id = QCA988X_2_0_DEVICE_ID_UBNT, > .name = "qca988x hw2.0 ubiquiti", > + .led_pin = 1, > + .gpio_count = 24, > .patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, > @@ -123,6 +131,8 @@ static const struct ath10k_hw_params
Re: [PATCH] ath10k: Update available channel list for 5G radio
On 02/20/2017 06:08 AM, c_tr...@qti.qualcomm.com wrote: diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 3029f25..0840efb 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -7623,6 +7623,38 @@ static void ath10k_mac_op_sta_pre_rcu_remove(struct ieee80211_hw *hw, CHAN2G(14, 2484, 0), }; +static const struct ieee80211_channel ath10k_low_5ghz_channels[] = { + CHAN5G(36, 5180, 0), + CHAN5G(40, 5200, 0), + CHAN5G(44, 5220, 0), + CHAN5G(48, 5240, 0), + CHAN5G(52, 5260, 0), + CHAN5G(56, 5280, 0), + CHAN5G(60, 5300, 0), + CHAN5G(64, 5320, 0), +}; + +static const struct ieee80211_channel ath10k_high_5ghz_channels[] = { + CHAN5G(100, 5500, 0), + CHAN5G(104, 5520, 0), + CHAN5G(108, 5540, 0), + CHAN5G(112, 5560, 0), + CHAN5G(116, 5580, 0), + CHAN5G(120, 5600, 0), + CHAN5G(124, 5620, 0), + CHAN5G(128, 5640, 0), + CHAN5G(132, 5660, 0), + CHAN5G(136, 5680, 0), + CHAN5G(140, 5700, 0), + CHAN5G(144, 5720, 0), + CHAN5G(149, 5745, 0), + CHAN5G(153, 5765, 0), + CHAN5G(157, 5785, 0), + CHAN5G(161, 5805, 0), + CHAN5G(165, 5825, 0), + CHAN5G(169, 5845, 0), +}; This isn't too flexible. What if one day you'll see hardware that supports channels 100-144 only? It should be much better idea to just disable unavailable channels, see what we did in wiphy_freq_limits_apply (net-next). ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [ath6kl:pending 14/24] drivers/mtd/devices/bcm47xxsflash.c:299:2: error: implicit declaration of function 'ioremap_cache'
Resending reply with proper To/Cc. On 07/19/2016 03:14 AM, kbuild test robot wrote: tree: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending head: 7a1b79bd39dda1f12b9f6708583250f40354c981 commit: 57d8f7dd2132df3ac21044e93a8ecdc9744b4459 [14/24] bcma: allow enabling serial flash support on non-MIPS SoCs config: cris-allmodconfig (attached as .config) compiler: cris-linux-gcc (GCC) 4.6.3 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 57d8f7dd2132df3ac21044e93a8ecdc9744b4459 # save the attached .config to linux build tree make.cross ARCH=cris All error/warnings (new ones prefixed by >>): drivers/mtd/devices/bcm47xxsflash.c: In function 'bcm47xxsflash_bcma_probe': drivers/mtd/devices/bcm47xxsflash.c:299:2: error: implicit declaration of function 'ioremap_cache' [-Werror=implicit-function-declaration] drivers/mtd/devices/bcm47xxsflash.c:299:15: warning: assignment makes pointer from integer without a cast [enabled by default] cc1: some warnings being treated as errors Oh, great :( With commit: 5651d6aaf489 ("mtd: bcm47xxsflash: use ioremap_cache() instead of KSEG0ADDR()") we believed to make this driver compilable on all architectures. It seems that ioremap_cache is available only on arm, arm64, ia64, mips, sh, x86 and xtensa. I noticed that kernel/memremap.c implements ioremap_cache on its own for archs that don't provide it. There is a comment in this file: /* temporary while we convert existing ioremap_cache users to memremap */ Should we use memremap? I don't understand this function well, it seems I could call it with with MEMREMAP_WB flag (and trigger ioremap_cache call), but is it a correct thing to do? Or should we simply make some symbol (CONFIG_MTD_BCM47XXSFLASH?) depend on ARM || MIPS? vim +/ioremap_cache +299 drivers/mtd/devices/bcm47xxsflash.c 5fe42d5bf Rafał Miłecki 2012-09-17 283 5651d6aaf Brian Norris 2016-02-26 284 b47s = devm_kzalloc(dev, sizeof(*b47s), GFP_KERNEL); d2b1bd142 Libo Chen 2013-05-30 285 if (!b47s) d2b1bd142 Libo Chen 2013-05-30 286 return -ENOMEM; a2f74a7da Rafał Miłecki 2013-01-06 287 sflash->priv = b47s; a2f74a7da Rafał Miłecki 2013-01-06 288 5651d6aaf Brian Norris 2016-02-26 289 res = platform_get_resource(pdev, IORESOURCE_MEM, 0); 5651d6aaf Brian Norris 2016-02-26 290 if (!res) { 5651d6aaf Brian Norris 2016-02-26 291 dev_err(dev, "invalid resource\n"); 5651d6aaf Brian Norris 2016-02-26 292 return -EINVAL; 5651d6aaf Brian Norris 2016-02-26 293 } 5651d6aaf Brian Norris 2016-02-26 294 if (!devm_request_mem_region(dev, res->start, resource_size(res), 5651d6aaf Brian Norris 2016-02-26 295 res->name)) { 5651d6aaf Brian Norris 2016-02-26 296 dev_err(dev, "can't request region for resource %pR\n", res); 5651d6aaf Brian Norris 2016-02-26 297 return -EBUSY; 5651d6aaf Brian Norris 2016-02-26 298 } 5651d6aaf Brian Norris 2016-02-26 @299 b47s->window = ioremap_cache(res->start, resource_size(res)); 5651d6aaf Brian Norris 2016-02-26 300 if (!b47s->window) { 5651d6aaf Brian Norris 2016-02-26 301 dev_err(dev, "ioremap failed for resource %pR\n", res); 5651d6aaf Brian Norris 2016-02-26 302 return -ENOMEM; 5651d6aaf Brian Norris 2016-02-26 303 } 5651d6aaf Brian Norris 2016-02-26 304 41c81536e Rafał Miłecki 2013-03-06 305 b47s->bcma_cc = container_of(sflash, struct bcma_drv_cc, sflash); 265dfbd9a Rafał Miłecki 2013-03-24 306 b47s->cc_read = bcm47xxsflash_bcma_cc_read; 265dfbd9a Rafał Miłecki 2013-03-24 307 b47s->cc_write = bcm47xxsflash_bcma_cc_write; :: The code at line 299 was first introduced by commit :: 5651d6aaf489d1db48c253cf884b40214e91c2c5 mtd: bcm47xxsflash: use ioremap_cache() instead of KSEG0ADDR() :: TO: Brian Norris :: CC: Brian Norris --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [ath6kl:pending 14/24] drivers/mtd/devices/bcm47xxsflash.c:299:2: error: implicit declaration of function 'ioremap_cache'
On 07/19/2016 03:14 AM, kbuild test robot wrote: tree: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git pending head: 7a1b79bd39dda1f12b9f6708583250f40354c981 commit: 57d8f7dd2132df3ac21044e93a8ecdc9744b4459 [14/24] bcma: allow enabling serial flash support on non-MIPS SoCs config: cris-allmodconfig (attached as .config) compiler: cris-linux-gcc (GCC) 4.6.3 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 57d8f7dd2132df3ac21044e93a8ecdc9744b4459 # save the attached .config to linux build tree make.cross ARCH=cris All error/warnings (new ones prefixed by >>): drivers/mtd/devices/bcm47xxsflash.c: In function 'bcm47xxsflash_bcma_probe': drivers/mtd/devices/bcm47xxsflash.c:299:2: error: implicit declaration of function 'ioremap_cache' [-Werror=implicit-function-declaration] drivers/mtd/devices/bcm47xxsflash.c:299:15: warning: assignment makes pointer from integer without a cast [enabled by default] cc1: some warnings being treated as errors Oh, great :( With commit: 5651d6aaf489 ("mtd: bcm47xxsflash: use ioremap_cache() instead of KSEG0ADDR()") we believed to make this driver compilable on all architectures. It seems that ioremap_cache is available only on arm, arm64, ia64, mips, sh, x86 and xtensa. I noticed that kernel/memremap.c implements ioremap_cache on its own for archs that don't provide it. There is a comment in this file: /* temporary while we convert existing ioremap_cache users to memremap */ Should we use memremap? I don't understand this function well, it seems I could call it with with MEMREMAP_WB flag (and trigger ioremap_cache call), but is it a correct thing to do? Or should we simply make some symbol (CONFIG_MTD_BCM47XXSFLASH?) depend on ARM || MIPS? vim +/ioremap_cache +299 drivers/mtd/devices/bcm47xxsflash.c 5fe42d5bf Rafał Miłecki 2012-09-17 283 5651d6aaf Brian Norris 2016-02-26 284 b47s = devm_kzalloc(dev, sizeof(*b47s), GFP_KERNEL); d2b1bd142 Libo Chen 2013-05-30 285 if (!b47s) d2b1bd142 Libo Chen 2013-05-30 286 return -ENOMEM; a2f74a7da Rafał Miłecki 2013-01-06 287 sflash->priv = b47s; a2f74a7da Rafał Miłecki 2013-01-06 288 5651d6aaf Brian Norris 2016-02-26 289 res = platform_get_resource(pdev, IORESOURCE_MEM, 0); 5651d6aaf Brian Norris 2016-02-26 290 if (!res) { 5651d6aaf Brian Norris 2016-02-26 291 dev_err(dev, "invalid resource\n"); 5651d6aaf Brian Norris 2016-02-26 292 return -EINVAL; 5651d6aaf Brian Norris 2016-02-26 293 } 5651d6aaf Brian Norris 2016-02-26 294 if (!devm_request_mem_region(dev, res->start, resource_size(res), 5651d6aaf Brian Norris 2016-02-26 295 res->name)) { 5651d6aaf Brian Norris 2016-02-26 296 dev_err(dev, "can't request region for resource %pR\n", res); 5651d6aaf Brian Norris 2016-02-26 297 return -EBUSY; 5651d6aaf Brian Norris 2016-02-26 298 } 5651d6aaf Brian Norris 2016-02-26 @299 b47s->window = ioremap_cache(res->start, resource_size(res)); 5651d6aaf Brian Norris 2016-02-26 300 if (!b47s->window) { 5651d6aaf Brian Norris 2016-02-26 301 dev_err(dev, "ioremap failed for resource %pR\n", res); 5651d6aaf Brian Norris 2016-02-26 302 return -ENOMEM; 5651d6aaf Brian Norris 2016-02-26 303 } 5651d6aaf Brian Norris 2016-02-26 304 41c81536e Rafał Miłecki 2013-03-06 305 b47s->bcma_cc = container_of(sflash, struct bcma_drv_cc, sflash); 265dfbd9a Rafał Miłecki 2013-03-24 306 b47s->cc_read = bcm47xxsflash_bcma_cc_read; 265dfbd9a Rafał Miłecki 2013-03-24 307 b47s->cc_write = bcm47xxsflash_bcma_cc_write; :: The code at line 299 was first introduced by commit :: 5651d6aaf489d1db48c253cf884b40214e91c2c5 mtd: bcm47xxsflash: use ioremap_cache() instead of KSEG0ADDR() :: TO: Brian Norris :: CC: Brian Norris --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k