Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets

2018-03-07 Thread Rafał Miłecki 
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, _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

2018-02-27 Thread Rafał Miłecki 
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 =

Re: [PATCH] ath10k: Update available channel list for 5G radio

2017-02-19 Thread Rafał Miłecki 

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'

2016-07-18 Thread Rafał Miłecki 

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 <computersforpe...@gmail.com>
:: CC: Brian Norris <computersforpe...@gmail.com>

---
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