diff mbox

[v13] ath10k: add LED and GPIO controlling support for various chipsets

Message ID 1523027875-5143-1-git-send-email-kvalo@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Kalle Valo April 6, 2018, 3:17 p.m. UTC
From: Sebastian Gottschall <s.gottschall@newmedia-net.de>

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.

Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
[kvalo: major reorg and cleanup]
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---

v13:

* only compile tested!

* fix all checkpatch warnings

* fix commit log

* sizeof(struct ath10k_gpiocontrol) -> sizeof(*gpio)

* unsigned -> unsigned int

* remove GPIOLIB code, that should be added in a separate patch

* rename gpio.c to leds.c

* add leds.h

* rename some functions:

  ath10k_attach_led() -> ath10k_leds_register()
  ath10k_unregister_led() -> ath10k_leds_unregister()
  ath10k_reset_led_pin() -> ath10k_leds_start()

* call ath10k_leds_unregister() before ath10k_thermal_unregister() to preserve ordering

* call ath10k_leds_start() only from ath10k_core_start() and not from mac.c

* rename struct ath10k_gpiocontrol as anonymous function under struct
  ath10k::leds, no need for memory allocation

* merge ath10k_add_led() to ath10k_attach_led(), which is it's only caller

* remove #if IS_ENABLED() checks from most of places, memory savings from those were not worth it

* Kconfig help text improvement and move it lower in the menu, also don't enable it by default

* switch to set_brightness_blocking() so that the callback can sleep,
  then no need to use ath10k_wmi_cmd_send_nowait() and can take mutex
  to access ar->state

* don't touch ath10k_wmi_pdev_get_temperature()

* as QCA6174/QCA9377 are not (yet) supported don't add the command to WMI-TLV interface

* remove debugfs interface, that should be added in another patch

* cleanup includes


 drivers/net/wireless/ath/ath10k/Kconfig   |  10 +++
 drivers/net/wireless/ath/ath10k/Makefile  |   1 +
 drivers/net/wireless/ath/ath10k/core.c    |  22 +++++++
 drivers/net/wireless/ath/ath10k/core.h    |   9 ++-
 drivers/net/wireless/ath/ath10k/hw.h      |   1 +
 drivers/net/wireless/ath/ath10k/leds.c    | 103 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/leds.h    |  45 +++++++++++++
 drivers/net/wireless/ath/ath10k/mac.c     |   1 +
 drivers/net/wireless/ath/ath10k/wmi-ops.h |  32 ++++++++++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +
 drivers/net/wireless/ath/ath10k/wmi.c     |  54 ++++++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.h     |  35 ++++++++++
 12 files changed, 314 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/wireless/ath/ath10k/leds.c
 create mode 100644 drivers/net/wireless/ath/ath10k/leds.h

Comments

Kalle Valo April 6, 2018, 3:22 p.m. UTC | #1
Kalle Valo <kvalo@codeaurora.org> writes:

> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
>
> 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.
>
> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>

From and Signed-off-by fields do not match. Sebastian, which domain
should it be?

> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
> [kvalo: major reorg and cleanup]
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> ---
>
> v13:
>
> * only compile tested!

So testing is very welcome.

> --- a/drivers/net/wireless/ath/ath10k/Kconfig
> +++ b/drivers/net/wireless/ath/ath10k/Kconfig
> @@ -52,6 +52,16 @@ config ATH10K_DEBUGFS
>  
>  	  If unsure, say Y to make it easier to debug problems.
>  
> +config ATH10K_LEDS
> +	bool "Atheros ath10k LED 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. If unsure, say N.
> +

I started to wonder is this Kconfig option even needed, feels pretty
pointless. Why not just make config ATH10K depend on LED stuff, just
like ath9k already does?

> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -25,6 +25,7 @@
>  #include <linux/pci.h>
>  #include <linux/uuid.h>
>  #include <linux/time.h>
> +#include <linux/leds.h>
>  
>  #include "htt.h"
>  #include "htc.h"
> @@ -883,7 +884,6 @@ struct ath10k {
>  	u32 low_5ghz_chan;
>  	u32 high_5ghz_chan;
>  	bool ani_enabled;
> -
>  	bool p2p;
>  
>  	struct {

Unrelated change, I'll fix that in v14.
Sebastian Gottschall April 6, 2018, 6:31 p.m. UTC | #2
Hello Kalle

you removed the call to leds_start from certain locations but you seem 
to have ignored the comment i wrote above the function call.
there is a reason why i reinitialize the gpio output state in these 
locations. the firmware for 9984 and 99xx resets the gpio registers
at certain points. this will lead to a non working led code. so you must 
set the gpio output to high again and this is the reason why the 
function is called "reset_led_pin" and not led_start
because it doesnt start the led in any way. it just resets the output state
there is only one work around you may do.
you set the gpio out register to high on every led trigger, but this is 
what i wanted to avoid to save cpu time since a wmi call is more 
expensive than just a register write.

so if you want to follow this up. remove ath10k_leds_start
and insert

ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0,
			       WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);

in ath10k_leds_set_brightness_blocking

but in any way the function name ath10k_leds_start is wrong. it doesnt 
describe the behaviour of the function


Am 06.04.2018 um 17:17 schrieb Kalle Valo:
> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
>
> 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.
>
> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
> [kvalo: major reorg and cleanup]
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> ---
>
> v13:
>
> * only compile tested!
>
> * fix all checkpatch warnings
>
> * fix commit log
>
> * sizeof(struct ath10k_gpiocontrol) -> sizeof(*gpio)
>
> * unsigned -> unsigned int
>
> * remove GPIOLIB code, that should be added in a separate patch
>
> * rename gpio.c to leds.c
>
> * add leds.h
>
> * rename some functions:
>
>    ath10k_attach_led() -> ath10k_leds_register()
>    ath10k_unregister_led() -> ath10k_leds_unregister()
>    ath10k_reset_led_pin() -> ath10k_leds_start()
>
> * call ath10k_leds_unregister() before ath10k_thermal_unregister() to preserve ordering
>
> * call ath10k_leds_start() only from ath10k_core_start() and not from mac.c
>
> * rename struct ath10k_gpiocontrol as anonymous function under struct
>    ath10k::leds, no need for memory allocation
>
> * merge ath10k_add_led() to ath10k_attach_led(), which is it's only caller
>
> * remove #if IS_ENABLED() checks from most of places, memory savings from those were not worth it
>
> * Kconfig help text improvement and move it lower in the menu, also don't enable it by default
>
> * switch to set_brightness_blocking() so that the callback can sleep,
>    then no need to use ath10k_wmi_cmd_send_nowait() and can take mutex
>    to access ar->state
>
> * don't touch ath10k_wmi_pdev_get_temperature()
>
> * as QCA6174/QCA9377 are not (yet) supported don't add the command to WMI-TLV interface
>
> * remove debugfs interface, that should be added in another patch
>
> * cleanup includes
>
>
>   drivers/net/wireless/ath/ath10k/Kconfig   |  10 +++
>   drivers/net/wireless/ath/ath10k/Makefile  |   1 +
>   drivers/net/wireless/ath/ath10k/core.c    |  22 +++++++
>   drivers/net/wireless/ath/ath10k/core.h    |   9 ++-
>   drivers/net/wireless/ath/ath10k/hw.h      |   1 +
>   drivers/net/wireless/ath/ath10k/leds.c    | 103 ++++++++++++++++++++++++++++++
>   drivers/net/wireless/ath/ath10k/leds.h    |  45 +++++++++++++
>   drivers/net/wireless/ath/ath10k/mac.c     |   1 +
>   drivers/net/wireless/ath/ath10k/wmi-ops.h |  32 ++++++++++
>   drivers/net/wireless/ath/ath10k/wmi-tlv.c |   2 +
>   drivers/net/wireless/ath/ath10k/wmi.c     |  54 ++++++++++++++++
>   drivers/net/wireless/ath/ath10k/wmi.h     |  35 ++++++++++
>   12 files changed, 314 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/net/wireless/ath/ath10k/leds.c
>   create mode 100644 drivers/net/wireless/ath/ath10k/leds.h
>
> diff --git a/drivers/net/wireless/ath/ath10k/Kconfig b/drivers/net/wireless/ath/ath10k/Kconfig
> index deb5ae21a559..c4108c1bbe92 100644
> --- a/drivers/net/wireless/ath/ath10k/Kconfig
> +++ b/drivers/net/wireless/ath/ath10k/Kconfig
> @@ -52,6 +52,16 @@ config ATH10K_DEBUGFS
>   
>   	  If unsure, say Y to make it easier to debug problems.
>   
> +config ATH10K_LEDS
> +	bool "Atheros ath10k LED 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. If unsure, say N.
> +
>   config ATH10K_SPECTRAL
>   	bool "Atheros ath10k spectral scan support"
>   	depends on ATH10K_DEBUGFS
> diff --git a/drivers/net/wireless/ath/ath10k/Makefile b/drivers/net/wireless/ath/ath10k/Makefile
> index 6739ac26fd29..16c822421c39 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) += leds.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 8a3020dbd4cf..be5958e98102 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -34,6 +34,7 @@
>   #include "testmode.h"
>   #include "wmi-ops.h"
>   #include "coredump.h"
> +#include "leds.h"
>   
>   unsigned int ath10k_debug_mask;
>   static unsigned int ath10k_cryptmode_param;
> @@ -66,6 +67,7 @@ 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,
>   		.patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
>   		.uart_pin = 7,
>   		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,
> @@ -95,6 +97,7 @@ 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,
>   		.patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
>   		.uart_pin = 7,
>   		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,
> @@ -124,6 +127,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
>   		.id = QCA9887_HW_1_0_VERSION,
>   		.dev_id = QCA9887_1_0_DEVICE_ID,
>   		.name = "qca9887 hw1.0",
> +		.led_pin = 1,
>   		.patch_load_addr = QCA9887_HW_1_0_PATCH_LOAD_ADDR,
>   		.uart_pin = 7,
>   		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,
> @@ -268,6 +272,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
>   		.id = QCA99X0_HW_2_0_DEV_VERSION,
>   		.dev_id = QCA99X0_2_0_DEVICE_ID,
>   		.name = "qca99x0 hw2.0",
> +		.led_pin = 17,
>   		.patch_load_addr = QCA99X0_HW_2_0_PATCH_LOAD_ADDR,
>   		.uart_pin = 7,
>   		.otp_exe_param = 0x00000700,
> @@ -302,6 +307,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
>   		.id = QCA9984_HW_1_0_DEV_VERSION,
>   		.dev_id = QCA9984_1_0_DEVICE_ID,
>   		.name = "qca9984/qca9994 hw1.0",
> +		.led_pin = 17,
>   		.patch_load_addr = QCA9984_HW_1_0_PATCH_LOAD_ADDR,
>   		.uart_pin = 7,
>   		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH,
> @@ -341,6 +347,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
>   		.id = QCA9888_HW_2_0_DEV_VERSION,
>   		.dev_id = QCA9888_2_0_DEVICE_ID,
>   		.name = "qca9888 hw2.0",
> +		.led_pin = 17,
>   		.patch_load_addr = QCA9888_HW_2_0_PATCH_LOAD_ADDR,
>   		.uart_pin = 7,
>   		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH,
> @@ -2377,6 +2384,10 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
>   	if (status)
>   		goto err_hif_stop;
>   
> +	status = ath10k_leds_start(ar);
> +	if (status)
> +		goto err_hif_stop;
> +
>   	return 0;
>   
>   err_hif_stop:
> @@ -2623,9 +2634,18 @@ static void ath10k_core_register_work(struct work_struct *work)
>   		goto err_spectral_destroy;
>   	}
>   
> +	status = ath10k_leds_register(ar);
> +	if (status) {
> +		ath10k_err(ar, "could not register leds: %d\n",
> +			   status);
> +		goto err_thermal_unregister;
> +	}
> +
>   	set_bit(ATH10K_FLAG_CORE_REGISTERED, &ar->dev_flags);
>   	return;
>   
> +err_thermal_unregister:
> +	ath10k_thermal_unregister(ar);
>   err_spectral_destroy:
>   	ath10k_spectral_destroy(ar);
>   err_debug_destroy:
> @@ -2659,6 +2679,8 @@ void ath10k_core_unregister(struct ath10k *ar)
>   	if (!test_bit(ATH10K_FLAG_CORE_REGISTERED, &ar->dev_flags))
>   		return;
>   
> +	ath10k_leds_unregister(ar);
> +
>   	ath10k_thermal_unregister(ar);
>   	/* Stop spectral before unregistering from mac80211 to remove the
>   	 * relayfs debugfs file cleanly. Otherwise the parent debugfs tree
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index c17d805d68cc..937344f57c07 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -25,6 +25,7 @@
>   #include <linux/pci.h>
>   #include <linux/uuid.h>
>   #include <linux/time.h>
> +#include <linux/leds.h>
>   
>   #include "htt.h"
>   #include "htc.h"
> @@ -883,7 +884,6 @@ struct ath10k {
>   	u32 low_5ghz_chan;
>   	u32 high_5ghz_chan;
>   	bool ani_enabled;
> -
>   	bool p2p;
>   
>   	struct {
> @@ -1074,6 +1074,13 @@ struct ath10k {
>   	} testmode;
>   
>   	struct {
> +		struct gpio_led wifi_led;
> +		struct led_classdev cdev;
> +		char label[48];
> +		u32 gpio_state_pin;
> +	} leds;
> +
> +	struct {
>   		/* protected by data_lock */
>   		u32 fw_crash_counter;
>   		u32 fw_warm_reset_counter;
> diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
> index 413b1b4321f7..ce8377c477a4 100644
> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -495,6 +495,7 @@ struct ath10k_hw_params {
>   	const char *name;
>   	u32 patch_load_addr;
>   	int uart_pin;
> +	int led_pin;
>   	u32 otp_exe_param;
>   
>   	/* Type of hw cycle counter wraparound logic, for more info
> diff --git a/drivers/net/wireless/ath/ath10k/leds.c b/drivers/net/wireless/ath/ath10k/leds.c
> new file mode 100644
> index 000000000000..8daa3d8fd3b9
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath10k/leds.c
> @@ -0,0 +1,103 @@
> +/*
> + * Copyright (c) 2005-2011 Atheros Communications Inc.
> + * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
> + * Copyright (c) 2018 Sebastian Gottschall <s.gottschall@dd-wrt.com>
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <linux/leds.h>
> +
> +#include "leds.h"
> +
> +#include "core.h"
> +#include "wmi.h"
> +#include "wmi-ops.h"
> +
> +static int ath10k_leds_set_brightness_blocking(struct led_classdev *led_cdev,
> +					       enum led_brightness brightness)
> +{
> +	struct ath10k *ar = container_of(led_cdev, struct ath10k,
> +					 leds.cdev);
> +	struct gpio_led *led = &ar->leds.wifi_led;
> +
> +	mutex_lock(&ar->conf_mutex);
> +
> +	if (ar->state != ATH10K_STATE_ON)
> +		goto out;
> +
> +	ar->leds.gpio_state_pin = (brightness != LED_OFF) ^ led->active_low;
> +	ath10k_wmi_gpio_output(ar, led->gpio, ar->leds.gpio_state_pin);
> +
> +out:
> +	mutex_unlock(&ar->conf_mutex);
> +
> +	return 0;
> +}
> +
> +int ath10k_leds_start(struct ath10k *ar)
> +{
> +	if (ar->hw_params.led_pin == 0)
> +		/* leds not supported */
> +		return 0;
> +
> +	/* under some circumstances, the gpio pin gets reconfigured
> +	 * to default state by the firmware, so we need to
> +	 * reconfigure it this behaviour has only ben seen on
> +	 * QCA9984 and QCA99XX devices so far
> +	 */
> +	ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0,
> +			       WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
> +	ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1);
> +
> +	return 0;
> +}
> +
> +int ath10k_leds_register(struct ath10k *ar)
> +{
> +	int ret;
> +
> +	if (ar->hw_params.led_pin == 0)
> +		/* leds not supported */
> +		return 0;
> +
> +	snprintf(ar->leds.label, sizeof(ar->leds.label), "ath10k-%s",
> +		 wiphy_name(ar->hw->wiphy));
> +	ar->leds.wifi_led.active_low = 1;
> +	ar->leds.wifi_led.gpio = ar->hw_params.led_pin;
> +	ar->leds.wifi_led.name = ar->leds.label;
> +	ar->leds.wifi_led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
> +
> +	ar->leds.cdev.name = ar->leds.label;
> +	ar->leds.cdev.brightness_set_blocking = ath10k_leds_set_brightness_blocking;
> +
> +	/* FIXME: this assignment doesn't make sense as it's NULL, remove it? */
> +	ar->leds.cdev.default_trigger = ar->leds.wifi_led.default_trigger;
> +
> +	ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &ar->leds.cdev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +void ath10k_leds_unregister(struct ath10k *ar)
> +{
> +	if (ar->hw_params.led_pin == 0)
> +		/* leds not supported */
> +		return;
> +
> +	led_classdev_unregister(&ar->leds.cdev);
> +}
> +
> diff --git a/drivers/net/wireless/ath/ath10k/leds.h b/drivers/net/wireless/ath/ath10k/leds.h
> new file mode 100644
> index 000000000000..8a302c5b9516
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath10k/leds.h
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +#ifndef _LEDS_H_
> +#define _LEDS_H_
> +
> +#include "core.h"
> +
> +#if IS_ENABLED(CONFIG_ATH10K_LEDS)
> +
> +void ath10k_leds_unregister(struct ath10k *ar);
> +int ath10k_leds_start(struct ath10k *ar);
> +int ath10k_leds_register(struct ath10k *ar);
> +
> +#else
> +
> +static inline void ath10k_leds_unregister(struct ath10k *ar)
> +{
> +}
> +
> +static inline int ath10k_leds_start(struct ath10k *ar)
> +{
> +	return 0;
> +}
> +
> +static inline int ath10k_leds_register(struct ath10k *ar)
> +{
> +	return 0;
> +}
> +
> +#endif
> +
> +#endif /* _LEDS_H_ */
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index bf05a3689558..4676c5bc9a4a 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -33,6 +33,7 @@
>   #include "wmi-tlv.h"
>   #include "wmi-ops.h"
>   #include "wow.h"
> +#include "leds.h"
>   
>   /*********/
>   /* Rates */
> diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
> index c35e45340b4f..f815092d6087 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
> @@ -203,7 +203,10 @@ struct wmi_ops {
>   	struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value);
>   	struct sk_buff *(*gen_pdev_get_tpc_table_cmdid)(struct ath10k *ar,
>   							u32 param);
> +	struct sk_buff *(*gen_gpio_config)(struct ath10k *ar, u32 gpio_num,
> +					   u32 input, u32 pull_type, u32 intr_mode);
>   
> +	struct sk_buff *(*gen_gpio_output)(struct ath10k *ar, u32 gpio_num, u32 set);
>   };
>   
>   int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id);
> @@ -981,6 +984,35 @@ ath10k_wmi_force_fw_hang(struct ath10k *ar,
>   	return ath10k_wmi_cmd_send(ar, skb, ar->wmi.cmd->force_fw_hang_cmdid);
>   }
>   
> +static inline int ath10k_wmi_gpio_config(struct ath10k *ar, u32 gpio_num,
> +					 u32 input, u32 pull_type, u32 intr_mode)
> +{
> +	struct sk_buff *skb;
> +
> +	if (!ar->wmi.ops->gen_gpio_config)
> +		return -EOPNOTSUPP;
> +
> +	skb = ar->wmi.ops->gen_gpio_config(ar, gpio_num, input, pull_type, intr_mode);
> +	if (IS_ERR(skb))
> +		return PTR_ERR(skb);
> +
> +	return ath10k_wmi_cmd_send(ar, skb, ar->wmi.cmd->gpio_config_cmdid);
> +}
> +
> +static inline int ath10k_wmi_gpio_output(struct ath10k *ar, u32 gpio_num, u32 set)
> +{
> +	struct sk_buff *skb;
> +
> +	if (!ar->wmi.ops->gen_gpio_config)
> +		return -EOPNOTSUPP;
> +
> +	skb = ar->wmi.ops->gen_gpio_output(ar, gpio_num, set);
> +	if (IS_ERR(skb))
> +		return PTR_ERR(skb);
> +
> +	return ath10k_wmi_cmd_send(ar, skb, ar->wmi.cmd->gpio_output_cmdid);
> +}
> +
>   static inline int
>   ath10k_wmi_dbglog_cfg(struct ath10k *ar, u64 module_enable, u32 log_level)
>   {
> diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> index 9d1b0a459069..96246120c2ef 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> @@ -3815,6 +3815,8 @@ static const struct wmi_ops wmi_tlv_ops = {
>   	.gen_echo = ath10k_wmi_tlv_op_gen_echo,
>   	.gen_vdev_spectral_conf = ath10k_wmi_tlv_op_gen_vdev_spectral_conf,
>   	.gen_vdev_spectral_enable = ath10k_wmi_tlv_op_gen_vdev_spectral_enable,
> +	/* .gen_gpio_config not implemented */
> +	/* .gen_gpio_output not implemented */
>   };
>   
>   static const struct wmi_peer_flags_map wmi_tlv_peer_flags_map = {
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index c5e1ca5945db..39f82d9e8cdf 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -6953,6 +6953,49 @@ ath10k_wmi_op_gen_peer_set_param(struct ath10k *ar, u32 vdev_id,
>   	return skb;
>   }
>   
> +static struct sk_buff *ath10k_wmi_op_gen_gpio_config(struct ath10k *ar,
> +						     u32 gpio_num, u32 input,
> +						     u32 pull_type, u32 intr_mode)
> +{
> +	struct wmi_gpio_config_cmd *cmd;
> +	struct sk_buff *skb;
> +
> +	skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
> +	if (!skb)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cmd = (struct wmi_gpio_config_cmd *)skb->data;
> +	cmd->pull_type = __cpu_to_le32(pull_type);
> +	cmd->gpio_num = __cpu_to_le32(gpio_num);
> +	cmd->input = __cpu_to_le32(input);
> +	cmd->intr_mode = __cpu_to_le32(intr_mode);
> +
> +	ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi gpio_config gpio_num 0x%08x input 0x%08x pull_type 0x%08x intr_mode 0x%08x\n",
> +		   gpio_num, input, pull_type, intr_mode);
> +
> +	return skb;
> +}
> +
> +static struct sk_buff *ath10k_wmi_op_gen_gpio_output(struct ath10k *ar,
> +						     u32 gpio_num, u32 set)
> +{
> +	struct wmi_gpio_output_cmd *cmd;
> +	struct sk_buff *skb;
> +
> +	skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
> +	if (!skb)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cmd = (struct wmi_gpio_output_cmd *)skb->data;
> +	cmd->gpio_num = __cpu_to_le32(gpio_num);
> +	cmd->set = __cpu_to_le32(set);
> +
> +	ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi gpio_output gpio_num 0x%08x set 0x%08x\n",
> +		   gpio_num, set);
> +
> +	return skb;
> +}
> +
>   static struct sk_buff *
>   ath10k_wmi_op_gen_set_psmode(struct ath10k *ar, u32 vdev_id,
>   			     enum wmi_sta_ps_mode psmode)
> @@ -8543,6 +8586,9 @@ static const struct wmi_ops wmi_ops = {
>   	.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
>   	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
>   	.gen_echo = ath10k_wmi_op_gen_echo,
> +	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
> +	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
> +
>   	/* .gen_bcn_tmpl not implemented */
>   	/* .gen_prb_tmpl not implemented */
>   	/* .gen_p2p_go_bcn_ie not implemented */
> @@ -8613,6 +8659,8 @@ static const struct wmi_ops wmi_10_1_ops = {
>   	.fw_stats_fill = ath10k_wmi_10x_op_fw_stats_fill,
>   	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
>   	.gen_echo = ath10k_wmi_op_gen_echo,
> +	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
> +	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
>   	/* .gen_bcn_tmpl not implemented */
>   	/* .gen_prb_tmpl not implemented */
>   	/* .gen_p2p_go_bcn_ie not implemented */
> @@ -8684,6 +8732,8 @@ static const struct wmi_ops wmi_10_2_ops = {
>   	.gen_delba_send = ath10k_wmi_op_gen_delba_send,
>   	.fw_stats_fill = ath10k_wmi_10x_op_fw_stats_fill,
>   	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
> +	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
> +	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
>   	/* .gen_pdev_enable_adaptive_cca not implemented */
>   };
>   
> @@ -8754,6 +8804,8 @@ static const struct wmi_ops wmi_10_2_4_ops = {
>   	.gen_pdev_enable_adaptive_cca =
>   		ath10k_wmi_op_gen_pdev_enable_adaptive_cca,
>   	.get_vdev_subtype = ath10k_wmi_10_2_4_op_get_vdev_subtype,
> +	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
> +	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
>   	/* .gen_bcn_tmpl not implemented */
>   	/* .gen_prb_tmpl not implemented */
>   	/* .gen_p2p_go_bcn_ie not implemented */
> @@ -8831,6 +8883,8 @@ static const struct wmi_ops wmi_10_4_ops = {
>   	.gen_pdev_bss_chan_info_req = ath10k_wmi_10_2_op_gen_pdev_bss_chan_info,
>   	.gen_echo = ath10k_wmi_op_gen_echo,
>   	.gen_pdev_get_tpc_config = ath10k_wmi_10_2_4_op_gen_pdev_get_tpc_config,
> +	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
> +	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
>   };
>   
>   int ath10k_wmi_attach(struct ath10k *ar)
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
> index 6fbc84c29521..a1662ed64af0 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> @@ -2922,6 +2922,41 @@ enum wmi_10_4_feature_mask {
>   
>   };
>   
> +/* WMI_GPIO_CONFIG_CMDID */
> +enum {
> +	WMI_GPIO_PULL_NONE,
> +	WMI_GPIO_PULL_UP,
> +	WMI_GPIO_PULL_DOWN,
> +};
> +
> +enum {
> +	WMI_GPIO_INTTYPE_DISABLE,
> +	WMI_GPIO_INTTYPE_RISING_EDGE,
> +	WMI_GPIO_INTTYPE_FALLING_EDGE,
> +	WMI_GPIO_INTTYPE_BOTH_EDGE,
> +	WMI_GPIO_INTTYPE_LEVEL_LOW,
> +	WMI_GPIO_INTTYPE_LEVEL_HIGH
> +};
> +
> +/* WMI_GPIO_CONFIG_CMDID */
> +struct wmi_gpio_config_cmd {
> +	__le32 gpio_num;             /* GPIO number to be setup */
> +	__le32 input;                /* 0 - Output/ 1 - Input */
> +	__le32 pull_type;            /* Pull type defined above */
> +	__le32 intr_mode;            /* Interrupt mode defined above (Input) */
> +} __packed;
> +
> +/* WMI_GPIO_OUTPUT_CMDID */
> +struct wmi_gpio_output_cmd {
> +	__le32 gpio_num;    /* GPIO number to be setup */
> +	__le32 set;         /* Set the GPIO pin*/
> +} __packed;
> +
> +/* WMI_GPIO_INPUT_EVENTID */
> +struct wmi_gpio_input_event {
> +	__le32 gpio_num;    /* GPIO number which changed state */
> +} __packed;
> +
>   struct wmi_ext_resource_config_10_4_cmd {
>   	/* contains enum wmi_host_platform_type */
>   	__le32 host_platform_config;
Stefan Lippers-Hollmann April 8, 2018, 8:21 a.m. UTC | #3
Hi

On 2018-04-06, Kalle Valo wrote:
> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
> 
> 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.
> 
> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
> [kvalo: major reorg and cleanup]
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Tested-by: Stefan Lippers-Hollmann <s.l-h@gmx.de>

[...]

I've been able to test v13 of this patch, including the changes regarding 
removing ath10k_leds_start() and moving the ath10k_wmi_gpio_config() call
into ath10k_leds_set_brightness_blocking() as raised by Sebastian 
Gottschall, on a ZyXEL NBG6817 (using two QCA9984 wlan cards and firmware 
10.4-3.5.3-00053) running OpenWrt/ master r6644-eda27d7557 (ipq8065, 
kernel 4.9.92 with backports wt-2017-11-01-0-gfe248fc2c180/ 
v4.14-rc2-1-31-g86cf0e5d).

/sys/class/leds/ath10k-phy0/ and /sys/class/leds/ath10k-phy1/ are
registered correctly and the white wlan LEDs connected to the ath10k 
GPIOs are now working as designed (controlled via phy0tpt/ phy1tpt).

Thanks a lot to everyone involved!

Regards
	Stefan Lippers-Hollmann
Kalle Valo April 9, 2018, 3:49 p.m. UTC | #4
Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:

> you removed the call to leds_start from certain locations but you seem
> to have ignored the comment i wrote above the function call.

IIRC I moved the comment to ath10k_leds_start().

> there is a reason why i reinitialize the gpio output state in these
> locations. the firmware for 9984 and 99xx resets the gpio registers
> at certain points. this will lead to a non working led code.

What are the certain points exactly? I tried to be careful that from
firmware's point of view the functionality is the same, even if I moved
the call to a different location. Did you test the patch? Is it broken
now?

> so you must set the gpio output to high again and this is the reason
> why the function is called "reset_led_pin" and not led_start because
> it doesnt start the led in any way.

The naming refers to phases of ath10k initialisation which are:

create() - destroy()
register() - unregister()
start() - stop()

So the naming doesn't mean that every ath10k_foo_start() has to start
something, it just describes the phase of driver initialisation it's
called in.

> it just resets the output state there is only one work around you may
> do. you set the gpio out register to high on every led trigger, but
> this is what i wanted to avoid to save cpu time since a wmi call is
> more expensive than just a register write.

Is this really so frequently called that we need to think about CPU
time? How often are we expecting the LED state to change?

But I'm not really following you, from firmware point of view the
functionality should be the same as with your patch. 

> so if you want to follow this up. remove ath10k_leds_start
> and insert
>
> ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0,
> 			       WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
>
> in ath10k_leds_set_brightness_blocking

Calling ath10k_wmi_gpio_config() every time sounds like quite odd to me
and your patch didn't do that either. Are you sure this is really
needed?
Sebastian Gottschall April 10, 2018, 10:33 a.m. UTC | #5
Am 09.04.2018 um 17:49 schrieb Kalle Valo:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>
>> you removed the call to leds_start from certain locations but you seem
>> to have ignored the comment i wrote above the function call.
> IIRC I moved the comment to ath10k_leds_start().
>
>> there is a reason why i reinitialize the gpio output state in these
>> locations. the firmware for 9984 and 99xx resets the gpio registers
>> at certain points. this will lead to a non working led code.
> What are the certain points exactly? I tried to be careful that from
> firmware's point of view the functionality is the same, even if I moved
> the call to a different location. Did you test the patch? Is it broken
> now?
i reviewed the firmware code as well, but havent found the reason. can 
be core/chipset specific and not firmware software related.
i just can say it doesnt happen on 988x, just newer chipsets are affected
> The naming refers to phases of ath10k initialisation which are:
>
> create() - destroy()
> register() - unregister()
> start() - stop()
>
> So the naming doesn't mean that every ath10k_foo_start() has to start
> something, it just describes the phase of driver initialisation it's
> called in.
yes, but its not a initialisation too. its just a gpio pin reset. the 
initialisation is
the trigger code itself from my point of view
> Is this really so frequently called that we need to think about CPU
> time? How often are we expecting the LED state to change?
>
> But I'm not really following you, from firmware point of view the
> functionality should be the same as with your patch.
a typical tpt trigger as used in openwrt for instance may trigger it 
several times per second.
i mean it might be really just a micro effect, but i just save cpu time 
whereever i can since i'm focused in
embedded development
>
>> so if you want to follow this up. remove ath10k_leds_start
>> and insert
>>
>> ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0,
>>                    WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
>>
>> in ath10k_leds_set_brightness_blocking
> Calling ath10k_wmi_gpio_config() every time sounds like quite odd to me
> and your patch didn't do that either. Are you sure this is really
> needed?
yes. i tested this. you must set the gpio to output before setting it to 
any value, in case the output state was resetted back to input or any 
other default value. (which it does on 9984 at certain events)
i tested this on a netgear r7800 which is ipq8064 based with 2 9984 
chipsets.
but i cannot reproduce this on mips routers with 988x chipsets.

but as you say. its odd to call it every time. so i just call it with 
the reset method when neccessary. in our case, when interface operation 
starts.
Sven Eckelmann Feb. 26, 2019, 9:16 a.m. UTC | #6
On Friday, 6 April 2018 17:17:55 CET Kalle Valo wrote:
> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
> 
> 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.
> 
> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
> [kvalo: major reorg and cleanup]
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>


This patch was imported to OpenWrt in commit 61d57a2f88b9 ("mac80211: ath10k 
add leds support") and broke the 11s support for IPQ4019 and QCA4019 (5GHz) 
firmware versions 10.4-3.5.3-00053, 10.4-3.5.3-00057, 10.4-3.6-00140:

    [  221.620803] ath10k_pci 0000:01:00.0: wmi command 36967 timeout, restarting hardware
    [  221.744056] ieee80211 phy0: Hardware restart was requested
    [  225.130829] ath10k_pci 0000:01:00.0: failed to receive control response completion, polling..
    [  226.170824] ath10k_pci 0000:01:00.0: Service connect timeout
    [  226.170871] ath10k_pci 0000:01:00.0: failed to connect htt (-110)
    [  226.252248] ath10k_pci 0000:01:00.0: Could not init core: -110

This was tested on an A62 with following wireless config:

    config wifi-device 'radio0'
            option type 'mac80211'
            option channel '36'
            option hwmode '11a'
            option path 'soc/40000000.pci/pci0000:00/0000:00:00.0/0000:01:00.0'
            option htmode 'VHT80'
            option disabled '0'
            option country US
    
    config wifi-device 'radio1'
            option type 'mac80211'
            option channel '11'
            option hwmode '11g'
            option path 'platform/soc/a000000.wifi'
            option htmode 'HT20'
            option disabled '0'
            option country US
    
    config wifi-device 'radio2'
            option type 'mac80211'
            option channel '149'
            option hwmode '11a'
            option path 'platform/soc/a800000.wifi'
            option htmode 'VHT80'
            option disabled '0'
            option country US
    
    config wifi-iface 'mesh0'
        option device 'radio0'
        option ifname 'mesh0'
        option network 'nwi_mesh0'
        option mode 'mesh'
        option mesh_id 'TestMesh'
        option mesh_fwding '1'
        option encryption 'none'
    
    config wifi-iface 'mesh1'
        option device 'radio1'
        option ifname 'mesh1'
        option network 'nwi_mesh1'
        option mode 'mesh'
        option mesh_id 'TestMesh'
        option encryption 'none'
    
    
    config wifi-iface 'mesh2'
        option device 'radio2'
        option ifname 'mesh2'
        option network 'nwi_mesh2'
        option mode 'mesh'
        option mesh_id 'TestMesh'
        option mesh_fwding '1'
        option encryption 'none

Kind regards,
	Sven
Sebastian Gottschall May 20, 2020, 7:39 a.m. UTC | #7
this code is not in use in its original form for ipq4019.
i have seen that his patch is also dropped from ath.git but is still in 
use by openwrt.
could somone clarify the state here and why it was dropped?
the original patch i wrote does exclude the soc chipsets, but the patch 
was later reorganized and some part have been rewritten
so i'm not sure if it covers the scenario mentioned here, which i did 
take care of

Sebastian

Am 26.02.2019 um 10:16 schrieb Sven Eckelmann:
> On Friday, 6 April 2018 17:17:55 CET Kalle Valo wrote:
>> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
>>
>> 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.
>>
>> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
>> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
>> [kvalo: major reorg and cleanup]
>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>
> This patch was imported to OpenWrt in commit 61d57a2f88b9 ("mac80211: ath10k
> add leds support") and broke the 11s support for IPQ4019 and QCA4019 (5GHz)
> firmware versions 10.4-3.5.3-00053, 10.4-3.5.3-00057, 10.4-3.6-00140:
>
>      [  221.620803] ath10k_pci 0000:01:00.0: wmi command 36967 timeout, restarting hardware
>      [  221.744056] ieee80211 phy0: Hardware restart was requested
>      [  225.130829] ath10k_pci 0000:01:00.0: failed to receive control response completion, polling..
>      [  226.170824] ath10k_pci 0000:01:00.0: Service connect timeout
>      [  226.170871] ath10k_pci 0000:01:00.0: failed to connect htt (-110)
>      [  226.252248] ath10k_pci 0000:01:00.0: Could not init core: -110
>
> This was tested on an A62 with following wireless config:
>
>      config wifi-device 'radio0'
>              option type 'mac80211'
>              option channel '36'
>              option hwmode '11a'
>              option path 'soc/40000000.pci/pci0000:00/0000:00:00.0/0000:01:00.0'
>              option htmode 'VHT80'
>              option disabled '0'
>              option country US
>      
>      config wifi-device 'radio1'
>              option type 'mac80211'
>              option channel '11'
>              option hwmode '11g'
>              option path 'platform/soc/a000000.wifi'
>              option htmode 'HT20'
>              option disabled '0'
>              option country US
>      
>      config wifi-device 'radio2'
>              option type 'mac80211'
>              option channel '149'
>              option hwmode '11a'
>              option path 'platform/soc/a800000.wifi'
>              option htmode 'VHT80'
>              option disabled '0'
>              option country US
>      
>      config wifi-iface 'mesh0'
>          option device 'radio0'
>          option ifname 'mesh0'
>          option network 'nwi_mesh0'
>          option mode 'mesh'
>          option mesh_id 'TestMesh'
>          option mesh_fwding '1'
>          option encryption 'none'
>      
>      config wifi-iface 'mesh1'
>          option device 'radio1'
>          option ifname 'mesh1'
>          option network 'nwi_mesh1'
>          option mode 'mesh'
>          option mesh_id 'TestMesh'
>          option encryption 'none'
>      
>      
>      config wifi-iface 'mesh2'
>          option device 'radio2'
>          option ifname 'mesh2'
>          option network 'nwi_mesh2'
>          option mode 'mesh'
>          option mesh_id 'TestMesh'
>          option mesh_fwding '1'
>          option encryption 'none
>
> Kind regards,
> 	Sven
Vincent Wiemann May 20, 2020, 10:40 a.m. UTC | #8
Hi Sebastian,

I don't know why it was dropped, but I can say that the LED control code was kind of
annoying me. Even when the LED was turned of, it "flickered" when it was set disabled.
Unfortunately I didn't have time to look into it, yet.

Best,

Vincent

On 20.05.20 09:39, Sebastian Gottschall wrote:
> this code is not in use in its original form for ipq4019.
> i have seen that his patch is also dropped from ath.git but is still in use by openwrt.
> could somone clarify the state here and why it was dropped?
> the original patch i wrote does exclude the soc chipsets, but the patch was later reorganized and some part have been rewritten
> so i'm not sure if it covers the scenario mentioned here, which i did take care of
> 
> Sebastian
> 
> Am 26.02.2019 um 10:16 schrieb Sven Eckelmann:
>> On Friday, 6 April 2018 17:17:55 CET Kalle Valo wrote:
>>> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
>>>
>>> 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.
>>>
>>> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
>>> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
>>> [kvalo: major reorg and cleanup]
>>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>>
>> This patch was imported to OpenWrt in commit 61d57a2f88b9 ("mac80211: ath10k
>> add leds support") and broke the 11s support for IPQ4019 and QCA4019 (5GHz)
>> firmware versions 10.4-3.5.3-00053, 10.4-3.5.3-00057, 10.4-3.6-00140:
>>
>>      [  221.620803] ath10k_pci 0000:01:00.0: wmi command 36967 timeout, restarting hardware
>>      [  221.744056] ieee80211 phy0: Hardware restart was requested
>>      [  225.130829] ath10k_pci 0000:01:00.0: failed to receive control response completion, polling..
>>      [  226.170824] ath10k_pci 0000:01:00.0: Service connect timeout
>>      [  226.170871] ath10k_pci 0000:01:00.0: failed to connect htt (-110)
>>      [  226.252248] ath10k_pci 0000:01:00.0: Could not init core: -110
>>
>> This was tested on an A62 with following wireless config:
>>
>>      config wifi-device 'radio0'
>>              option type 'mac80211'
>>              option channel '36'
>>              option hwmode '11a'
>>              option path 'soc/40000000.pci/pci0000:00/0000:00:00.0/0000:01:00.0'
>>              option htmode 'VHT80'
>>              option disabled '0'
>>              option country US
>>           config wifi-device 'radio1'
>>              option type 'mac80211'
>>              option channel '11'
>>              option hwmode '11g'
>>              option path 'platform/soc/a000000.wifi'
>>              option htmode 'HT20'
>>              option disabled '0'
>>              option country US
>>           config wifi-device 'radio2'
>>              option type 'mac80211'
>>              option channel '149'
>>              option hwmode '11a'
>>              option path 'platform/soc/a800000.wifi'
>>              option htmode 'VHT80'
>>              option disabled '0'
>>              option country US
>>           config wifi-iface 'mesh0'
>>          option device 'radio0'
>>          option ifname 'mesh0'
>>          option network 'nwi_mesh0'
>>          option mode 'mesh'
>>          option mesh_id 'TestMesh'
>>          option mesh_fwding '1'
>>          option encryption 'none'
>>           config wifi-iface 'mesh1'
>>          option device 'radio1'
>>          option ifname 'mesh1'
>>          option network 'nwi_mesh1'
>>          option mode 'mesh'
>>          option mesh_id 'TestMesh'
>>          option encryption 'none'
>>                config wifi-iface 'mesh2'
>>          option device 'radio2'
>>          option ifname 'mesh2'
>>          option network 'nwi_mesh2'
>>          option mode 'mesh'
>>          option mesh_id 'TestMesh'
>>          option mesh_fwding '1'
>>          option encryption 'none
>>
>> Kind regards,
>>     Sven
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
Sebastian Gottschall May 20, 2020, 1 p.m. UTC | #9
Am 20.05.2020 um 12:40 schrieb Vincent Wiemann:
> Hi Sebastian,
>
> I don't know why it was dropped, but I can say that the LED control code was kind of
> annoying me. Even when the LED was turned of, it "flickered" when it was set disabled.
> Unfortunately I didn't have time to look into it, yet.
the led code will just be used if you set a trigger. otherwise it doesnt 
touch the gpios.
the code itself was written to make use of the led's builtin to several 
routers. if you dont set a led trigger, nothing will happen

> Best,
>
> Vincent
>
> On 20.05.20 09:39, Sebastian Gottschall wrote:
>> this code is not in use in its original form for ipq4019.
>> i have seen that his patch is also dropped from ath.git but is still in use by openwrt.
>> could somone clarify the state here and why it was dropped?
>> the original patch i wrote does exclude the soc chipsets, but the patch was later reorganized and some part have been rewritten
>> so i'm not sure if it covers the scenario mentioned here, which i did take care of
>>
>> Sebastian
>>
>> Am 26.02.2019 um 10:16 schrieb Sven Eckelmann:
>>> On Friday, 6 April 2018 17:17:55 CET Kalle Valo wrote:
>>>> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
>>>>
>>>> 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.
>>>>
>>>> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
>>>> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
>>>> [kvalo: major reorg and cleanup]
>>>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>>> This patch was imported to OpenWrt in commit 61d57a2f88b9 ("mac80211: ath10k
>>> add leds support") and broke the 11s support for IPQ4019 and QCA4019 (5GHz)
>>> firmware versions 10.4-3.5.3-00053, 10.4-3.5.3-00057, 10.4-3.6-00140:
>>>
>>>       [  221.620803] ath10k_pci 0000:01:00.0: wmi command 36967 timeout, restarting hardware
>>>       [  221.744056] ieee80211 phy0: Hardware restart was requested
>>>       [  225.130829] ath10k_pci 0000:01:00.0: failed to receive control response completion, polling..
>>>       [  226.170824] ath10k_pci 0000:01:00.0: Service connect timeout
>>>       [  226.170871] ath10k_pci 0000:01:00.0: failed to connect htt (-110)
>>>       [  226.252248] ath10k_pci 0000:01:00.0: Could not init core: -110
>>>
>>> This was tested on an A62 with following wireless config:
>>>
>>>       config wifi-device 'radio0'
>>>               option type 'mac80211'
>>>               option channel '36'
>>>               option hwmode '11a'
>>>               option path 'soc/40000000.pci/pci0000:00/0000:00:00.0/0000:01:00.0'
>>>               option htmode 'VHT80'
>>>               option disabled '0'
>>>               option country US
>>>            config wifi-device 'radio1'
>>>               option type 'mac80211'
>>>               option channel '11'
>>>               option hwmode '11g'
>>>               option path 'platform/soc/a000000.wifi'
>>>               option htmode 'HT20'
>>>               option disabled '0'
>>>               option country US
>>>            config wifi-device 'radio2'
>>>               option type 'mac80211'
>>>               option channel '149'
>>>               option hwmode '11a'
>>>               option path 'platform/soc/a800000.wifi'
>>>               option htmode 'VHT80'
>>>               option disabled '0'
>>>               option country US
>>>            config wifi-iface 'mesh0'
>>>           option device 'radio0'
>>>           option ifname 'mesh0'
>>>           option network 'nwi_mesh0'
>>>           option mode 'mesh'
>>>           option mesh_id 'TestMesh'
>>>           option mesh_fwding '1'
>>>           option encryption 'none'
>>>            config wifi-iface 'mesh1'
>>>           option device 'radio1'
>>>           option ifname 'mesh1'
>>>           option network 'nwi_mesh1'
>>>           option mode 'mesh'
>>>           option mesh_id 'TestMesh'
>>>           option encryption 'none'
>>>                 config wifi-iface 'mesh2'
>>>           option device 'radio2'
>>>           option ifname 'mesh2'
>>>           option network 'nwi_mesh2'
>>>           option mode 'mesh'
>>>           option mesh_id 'TestMesh'
>>>           option mesh_fwding '1'
>>>           option encryption 'none
>>>
>>> Kind regards,
>>>      Sven
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel@lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>>
Vincent Wiemann May 20, 2020, 7:05 p.m. UTC | #10
Hi Sebastian,

On 20.05.20 15:00, Sebastian Gottschall wrote:
> 
> Am 20.05.2020 um 12:40 schrieb Vincent Wiemann:
>> Hi Sebastian,
>>
>> I don't know why it was dropped, but I can say that the LED control code was kind of
>> annoying me. Even when the LED was turned of, it "flickered" when it was set disabled.
>> Unfortunately I didn't have time to look into it, yet.

> the led code will just be used if you set a trigger. otherwise it doesnt touch the gpios.
> the code itself was written to make use of the led's builtin to several routers. if you dont set a led trigger, nothing will happen
> 

Thank you for your quick response... I'll try to reproduce the issue without your patch.
Maybe it's unrelated and a firmware-specific issue (official QCA9887).

One thing I've seen with your patch is that if I set the ath10k GPIO "steady on" it sometimes
(quite randomly) turns it off for a fraction of a second. It happens about 3 times a minute.
It's not a big deal. But maybe it's related to the flickering
I've observed and possibly also a firmware issue...

Best,

Vincent


>> Best,
>>
>> Vincent
>>
>> On 20.05.20 09:39, Sebastian Gottschall wrote:
>>> this code is not in use in its original form for ipq4019.
>>> i have seen that his patch is also dropped from ath.git but is still in use by openwrt.
>>> could somone clarify the state here and why it was dropped?
>>> the original patch i wrote does exclude the soc chipsets, but the patch was later reorganized and some part have been rewritten
>>> so i'm not sure if it covers the scenario mentioned here, which i did take care of
>>>
>>> Sebastian
>>>
>>> Am 26.02.2019 um 10:16 schrieb Sven Eckelmann:
>>>> On Friday, 6 April 2018 17:17:55 CET Kalle Valo wrote:
>>>>> From: Sebastian Gottschall <s.gottschall@newmedia-net.de>
>>>>>
>>>>> 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.
>>>>>
>>>>> Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
>>>>> Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
>>>>> [kvalo: major reorg and cleanup]
>>>>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>>>> This patch was imported to OpenWrt in commit 61d57a2f88b9 ("mac80211: ath10k
>>>> add leds support") and broke the 11s support for IPQ4019 and QCA4019 (5GHz)
>>>> firmware versions 10.4-3.5.3-00053, 10.4-3.5.3-00057, 10.4-3.6-00140:
>>>>
>>>>       [  221.620803] ath10k_pci 0000:01:00.0: wmi command 36967 timeout, restarting hardware
>>>>       [  221.744056] ieee80211 phy0: Hardware restart was requested
>>>>       [  225.130829] ath10k_pci 0000:01:00.0: failed to receive control response completion, polling..
>>>>       [  226.170824] ath10k_pci 0000:01:00.0: Service connect timeout
>>>>       [  226.170871] ath10k_pci 0000:01:00.0: failed to connect htt (-110)
>>>>       [  226.252248] ath10k_pci 0000:01:00.0: Could not init core: -110
>>>>
>>>> This was tested on an A62 with following wireless config:
>>>>
>>>>       config wifi-device 'radio0'
>>>>               option type 'mac80211'
>>>>               option channel '36'
>>>>               option hwmode '11a'
>>>>               option path 'soc/40000000.pci/pci0000:00/0000:00:00.0/0000:01:00.0'
>>>>               option htmode 'VHT80'
>>>>               option disabled '0'
>>>>               option country US
>>>>            config wifi-device 'radio1'
>>>>               option type 'mac80211'
>>>>               option channel '11'
>>>>               option hwmode '11g'
>>>>               option path 'platform/soc/a000000.wifi'
>>>>               option htmode 'HT20'
>>>>               option disabled '0'
>>>>               option country US
>>>>            config wifi-device 'radio2'
>>>>               option type 'mac80211'
>>>>               option channel '149'
>>>>               option hwmode '11a'
>>>>               option path 'platform/soc/a800000.wifi'
>>>>               option htmode 'VHT80'
>>>>               option disabled '0'
>>>>               option country US
>>>>            config wifi-iface 'mesh0'
>>>>           option device 'radio0'
>>>>           option ifname 'mesh0'
>>>>           option network 'nwi_mesh0'
>>>>           option mode 'mesh'
>>>>           option mesh_id 'TestMesh'
>>>>           option mesh_fwding '1'
>>>>           option encryption 'none'
>>>>            config wifi-iface 'mesh1'
>>>>           option device 'radio1'
>>>>           option ifname 'mesh1'
>>>>           option network 'nwi_mesh1'
>>>>           option mode 'mesh'
>>>>           option mesh_id 'TestMesh'
>>>>           option encryption 'none'
>>>>                 config wifi-iface 'mesh2'
>>>>           option device 'radio2'
>>>>           option ifname 'mesh2'
>>>>           option network 'nwi_mesh2'
>>>>           option mode 'mesh'
>>>>           option mesh_id 'TestMesh'
>>>>           option mesh_fwding '1'
>>>>           option encryption 'none
>>>>
>>>> Kind regards,
>>>>      Sven
>>> _______________________________________________
>>> openwrt-devel mailing list
>>> openwrt-devel@lists.openwrt.org
>>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>>>
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Kalle Valo May 22, 2020, 10:29 a.m. UTC | #11
(please don't top post)

Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:

> this code is not in use in its original form for ipq4019. i have seen
> that his patch is also dropped from ath.git but is still in use by
> openwrt. could somone clarify the state here and why it was dropped?

I dropped the patch because of the bug report from openwrt.
Sebastian Gottschall May 22, 2020, 2:26 p.m. UTC | #12
Am 22.05.2020 um 12:29 schrieb Kalle Valo:
> (please don't top post)
>
> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>
>> this code is not in use in its original form for ipq4019. i have seen
>> that his patch is also dropped from ath.git but is still in use by
>> openwrt. could somone clarify the state here and why it was dropped?
> I dropped the patch because of the bug report from openwrt.
can you show me the bug report? and openwrt is still using it as out of 
tree patch which is confusing here.
personally i'm using the patch on qca988x, qca9884 and also ipq40xx 
devices (40xx doesnt make any use of it of course)

any chipset which doesnt make use of the gpio_pin definition is unable 
to make use of it. if there is no trigger set, its also not used (hopefully)

but in any way i'm willig to fix any issue of there is a reproduceable 
issue.

Sebastian

>
Sven Eckelmann May 25, 2020, 9:22 a.m. UTC | #13
On Wednesday, 20 May 2020 09:39:45 CEST Sebastian Gottschall wrote:
[...]
> could somone clarify the state here and why it was dropped?
> the original patch i wrote does exclude the soc chipsets, but the patch 
> was later reorganized and some part have been rewritten
> so i'm not sure if it covers the scenario mentioned here, which i did 
[...]
> > This patch was imported to OpenWrt in commit 61d57a2f88b9 ("mac80211: ath10k
> > add leds support") and broke the 11s support for IPQ4019 and QCA4019 (5GHz)
> > firmware versions 10.4-3.5.3-00053, 10.4-3.5.3-00057, 10.4-3.6-00140:

Just noticed that there was a copy and paste error in my message. The 5GHz was 
an QCA9888 [1,2] and not an QCA4019. Otherwise the _pci error wouldn't have made 
any sense.

And I can only say at the moment (remember that this was observer 14 months 
ago), that it could be reproduced easily on IPQ40xx with an QCA9888 and the 
given config running OpenWrt reboot-9440-g0f89c17b57. The diffconfig (seed) of 
the installation was:

    CONFIG_TARGET_ipq40xx=y
    CONFIG_TARGET_ipq40xx_generic=y
    CONFIG_TARGET_ipq40xx_generic_DEVICE_openmesh_a62=y
    CONFIG_ATH10K_LEDS=y
    CONFIG_PACKAGE_ath10k-firmware-qca4019=y
    # CONFIG_PACKAGE_ath10k-firmware-qca4019-ct is not set
    CONFIG_PACKAGE_ath10k-firmware-qca9888=y
    # CONFIG_PACKAGE_ath10k-firmware-qca9888-ct is not set
    CONFIG_PACKAGE_kmod-ath10k=y
    # CONFIG_PACKAGE_kmod-ath10k-ct is not set
    # CONFIG_PACKAGE_kmod-hwmon-core is not set

And it still can with this OpenWrt version. But it doesn't seem to happen with 
the most recent OpenWrt reboot-13353-gb1604b744b. But there are nearly 4000 
commits inbetween. So no idea what changed (just a timing thing or an actual 
fix - no idea).

Btw. the wireless config was given in the original mail [2,3]

Kind regards,
	Sven

[1] https://openwrt.org/toh/hwdata/open-mesh/open-mesh_a62
[2] https://patchwork.kernel.org/patch/10723033/#22502191
[3] https://patchwork.kernel.org/patch/10327075/#22502179
Sebastian Gottschall May 25, 2020, 9:27 a.m. UTC | #14
Am 25.05.2020 um 11:22 schrieb Sven Eckelmann:
> On Wednesday, 20 May 2020 09:39:45 CEST Sebastian Gottschall wrote:
> [...]
>> could somone clarify the state here and why it was dropped?
>> the original patch i wrote does exclude the soc chipsets, but the patch
>> was later reorganized and some part have been rewritten
>> so i'm not sure if it covers the scenario mentioned here, which i did
> [...]
>>> This patch was imported to OpenWrt in commit 61d57a2f88b9 ("mac80211: ath10k
>>> add leds support") and broke the 11s support for IPQ4019 and QCA4019 (5GHz)
>>> firmware versions 10.4-3.5.3-00053, 10.4-3.5.3-00057, 10.4-3.6-00140:
> Just noticed that there was a copy and paste error in my message. The 5GHz was
> an QCA9888 [1,2] and not an QCA4019. Otherwise the _pci error wouldn't have made
> any sense.
>
> And I can only say at the moment (remember that this was observer 14 months
> ago), that it could be reproduced easily on IPQ40xx with an QCA9888 and the
> given config running OpenWrt reboot-9440-g0f89c17b57. The diffconfig (seed) of
> the installation was:
>
>      CONFIG_TARGET_ipq40xx=y
>      CONFIG_TARGET_ipq40xx_generic=y
>      CONFIG_TARGET_ipq40xx_generic_DEVICE_openmesh_a62=y
>      CONFIG_ATH10K_LEDS=y
>      CONFIG_PACKAGE_ath10k-firmware-qca4019=y
>      # CONFIG_PACKAGE_ath10k-firmware-qca4019-ct is not set
>      CONFIG_PACKAGE_ath10k-firmware-qca9888=y
>      # CONFIG_PACKAGE_ath10k-firmware-qca9888-ct is not set
>      CONFIG_PACKAGE_kmod-ath10k=y
>      # CONFIG_PACKAGE_kmod-ath10k-ct is not set
>      # CONFIG_PACKAGE_kmod-hwmon-core is not set
>
> And it still can with this OpenWrt version. But it doesn't seem to happen with
> the most recent OpenWrt reboot-13353-gb1604b744b. But there are nearly 4000
> commits inbetween. So no idea what changed (just a timing thing or an actual
> fix - no idea).
>
> Btw. the wireless config was given in the original mail [2,3]
>
> Kind regards,
> 	Sven
maybe openwrt installs a default trigger which doesnt make sense if 
nothing is connected to the cards gpio.
we can also modify the patch to exclude the 9888 from led support. you 
just need to remove the led_pin defintion from the hw definition.
this patch is mainly for wireless routers like the netgear r7800, r9000 
and some tplink archer models. it also works on mikrotik qca988x cards.
but even if the led_pin is set, it should not trigger any action until a 
led trigger is set with sysfs. such configurations should be 
architecture specific in any way

Sebastian
Sven Eckelmann May 25, 2020, 4:04 p.m. UTC | #15
On Monday, 25 May 2020 11:22:13 CEST Sven Eckelmann wrote:
[...]
> And it still can with this OpenWrt version. But it doesn't seem to happen with 
> the most recent OpenWrt reboot-13353-gb1604b744b. But there are nearly 4000 
> commits inbetween. So no idea what changed (just a timing thing or an actual 
> fix - no idea).

Seems like there is a fix which solves some lost interrupt problems for 
IPQ40xx. Without this change, I see the reported problem. And with the patch, 
it is gone. Or in commits:

* creates timeout problems: 46b949a067e5 ("ipq40xx: enlarge PCIe BAR size")
* works fine: 18e942b6c4e5 ("ipq40xx: fix pcie msi IRQ trigger level")

If you look in the git logs [1], you can see that the working commit is a 
child of the broken one. So at least from my point of view, my initial report 
is no blocker anymore for Sebastian's patch (or Kalle's version of it).

Btw. OpenWrt is automatically assigning a trigger (phy*tpt) for these LEDs.

Kind regards,
	Sven

[1] https://git.openwrt.org/?p=openwrt/openwrt.git;a=log;h=18e942b6c4e51a5a717a121697a63f3f98d93b19
Kalle Valo May 29, 2020, 3:34 p.m. UTC | #16
Sven Eckelmann <sven@narfation.org> writes:

> On Monday, 25 May 2020 11:22:13 CEST Sven Eckelmann wrote:
> [...]
>> And it still can with this OpenWrt version. But it doesn't seem to happen with 
>> the most recent OpenWrt reboot-13353-gb1604b744b. But there are nearly 4000 
>> commits inbetween. So no idea what changed (just a timing thing or an actual 
>> fix - no idea).
>
> Seems like there is a fix which solves some lost interrupt problems for 
> IPQ40xx. Without this change, I see the reported problem. And with the patch, 
> it is gone. Or in commits:
>
> * creates timeout problems: 46b949a067e5 ("ipq40xx: enlarge PCIe BAR size")
> * works fine: 18e942b6c4e5 ("ipq40xx: fix pcie msi IRQ trigger level")
>
> If you look in the git logs [1], you can see that the working commit is a 
> child of the broken one. So at least from my point of view, my initial report 
> is no blocker anymore for Sebastian's patch (or Kalle's version of it).

Great. If the patch is good to take can someone rebase the latest
version and resubmit, please?
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/Kconfig b/drivers/net/wireless/ath/ath10k/Kconfig
index deb5ae21a559..c4108c1bbe92 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -52,6 +52,16 @@  config ATH10K_DEBUGFS
 
 	  If unsure, say Y to make it easier to debug problems.
 
+config ATH10K_LEDS
+	bool "Atheros ath10k LED 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. If unsure, say N.
+
 config ATH10K_SPECTRAL
 	bool "Atheros ath10k spectral scan support"
 	depends on ATH10K_DEBUGFS
diff --git a/drivers/net/wireless/ath/ath10k/Makefile b/drivers/net/wireless/ath/ath10k/Makefile
index 6739ac26fd29..16c822421c39 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) += leds.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 8a3020dbd4cf..be5958e98102 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -34,6 +34,7 @@ 
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "coredump.h"
+#include "leds.h"
 
 unsigned int ath10k_debug_mask;
 static unsigned int ath10k_cryptmode_param;
@@ -66,6 +67,7 @@  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,
 		.patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
 		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,
@@ -95,6 +97,7 @@  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,
 		.patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
 		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,
@@ -124,6 +127,7 @@  static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.id = QCA9887_HW_1_0_VERSION,
 		.dev_id = QCA9887_1_0_DEVICE_ID,
 		.name = "qca9887 hw1.0",
+		.led_pin = 1,
 		.patch_load_addr = QCA9887_HW_1_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
 		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL,
@@ -268,6 +272,7 @@  static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.id = QCA99X0_HW_2_0_DEV_VERSION,
 		.dev_id = QCA99X0_2_0_DEVICE_ID,
 		.name = "qca99x0 hw2.0",
+		.led_pin = 17,
 		.patch_load_addr = QCA99X0_HW_2_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
 		.otp_exe_param = 0x00000700,
@@ -302,6 +307,7 @@  static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.id = QCA9984_HW_1_0_DEV_VERSION,
 		.dev_id = QCA9984_1_0_DEVICE_ID,
 		.name = "qca9984/qca9994 hw1.0",
+		.led_pin = 17,
 		.patch_load_addr = QCA9984_HW_1_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
 		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH,
@@ -341,6 +347,7 @@  static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.id = QCA9888_HW_2_0_DEV_VERSION,
 		.dev_id = QCA9888_2_0_DEVICE_ID,
 		.name = "qca9888 hw2.0",
+		.led_pin = 17,
 		.patch_load_addr = QCA9888_HW_2_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
 		.cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH,
@@ -2377,6 +2384,10 @@  int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
 	if (status)
 		goto err_hif_stop;
 
+	status = ath10k_leds_start(ar);
+	if (status)
+		goto err_hif_stop;
+
 	return 0;
 
 err_hif_stop:
@@ -2623,9 +2634,18 @@  static void ath10k_core_register_work(struct work_struct *work)
 		goto err_spectral_destroy;
 	}
 
+	status = ath10k_leds_register(ar);
+	if (status) {
+		ath10k_err(ar, "could not register leds: %d\n",
+			   status);
+		goto err_thermal_unregister;
+	}
+
 	set_bit(ATH10K_FLAG_CORE_REGISTERED, &ar->dev_flags);
 	return;
 
+err_thermal_unregister:
+	ath10k_thermal_unregister(ar);
 err_spectral_destroy:
 	ath10k_spectral_destroy(ar);
 err_debug_destroy:
@@ -2659,6 +2679,8 @@  void ath10k_core_unregister(struct ath10k *ar)
 	if (!test_bit(ATH10K_FLAG_CORE_REGISTERED, &ar->dev_flags))
 		return;
 
+	ath10k_leds_unregister(ar);
+
 	ath10k_thermal_unregister(ar);
 	/* Stop spectral before unregistering from mac80211 to remove the
 	 * relayfs debugfs file cleanly. Otherwise the parent debugfs tree
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c17d805d68cc..937344f57c07 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -25,6 +25,7 @@ 
 #include <linux/pci.h>
 #include <linux/uuid.h>
 #include <linux/time.h>
+#include <linux/leds.h>
 
 #include "htt.h"
 #include "htc.h"
@@ -883,7 +884,6 @@  struct ath10k {
 	u32 low_5ghz_chan;
 	u32 high_5ghz_chan;
 	bool ani_enabled;
-
 	bool p2p;
 
 	struct {
@@ -1074,6 +1074,13 @@  struct ath10k {
 	} testmode;
 
 	struct {
+		struct gpio_led wifi_led;
+		struct led_classdev cdev;
+		char label[48];
+		u32 gpio_state_pin;
+	} leds;
+
+	struct {
 		/* protected by data_lock */
 		u32 fw_crash_counter;
 		u32 fw_warm_reset_counter;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 413b1b4321f7..ce8377c477a4 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -495,6 +495,7 @@  struct ath10k_hw_params {
 	const char *name;
 	u32 patch_load_addr;
 	int uart_pin;
+	int led_pin;
 	u32 otp_exe_param;
 
 	/* Type of hw cycle counter wraparound logic, for more info
diff --git a/drivers/net/wireless/ath/ath10k/leds.c b/drivers/net/wireless/ath/ath10k/leds.c
new file mode 100644
index 000000000000..8daa3d8fd3b9
--- /dev/null
+++ b/drivers/net/wireless/ath/ath10k/leds.c
@@ -0,0 +1,103 @@ 
+/*
+ * Copyright (c) 2005-2011 Atheros Communications Inc.
+ * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018 Sebastian Gottschall <s.gottschall@dd-wrt.com>
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <linux/leds.h>
+
+#include "leds.h"
+
+#include "core.h"
+#include "wmi.h"
+#include "wmi-ops.h"
+
+static int ath10k_leds_set_brightness_blocking(struct led_classdev *led_cdev,
+					       enum led_brightness brightness)
+{
+	struct ath10k *ar = container_of(led_cdev, struct ath10k,
+					 leds.cdev);
+	struct gpio_led *led = &ar->leds.wifi_led;
+
+	mutex_lock(&ar->conf_mutex);
+
+	if (ar->state != ATH10K_STATE_ON)
+		goto out;
+
+	ar->leds.gpio_state_pin = (brightness != LED_OFF) ^ led->active_low;
+	ath10k_wmi_gpio_output(ar, led->gpio, ar->leds.gpio_state_pin);
+
+out:
+	mutex_unlock(&ar->conf_mutex);
+
+	return 0;
+}
+
+int ath10k_leds_start(struct ath10k *ar)
+{
+	if (ar->hw_params.led_pin == 0)
+		/* leds not supported */
+		return 0;
+
+	/* under some circumstances, the gpio pin gets reconfigured
+	 * to default state by the firmware, so we need to
+	 * reconfigure it this behaviour has only ben seen on
+	 * QCA9984 and QCA99XX devices so far
+	 */
+	ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0,
+			       WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE);
+	ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1);
+
+	return 0;
+}
+
+int ath10k_leds_register(struct ath10k *ar)
+{
+	int ret;
+
+	if (ar->hw_params.led_pin == 0)
+		/* leds not supported */
+		return 0;
+
+	snprintf(ar->leds.label, sizeof(ar->leds.label), "ath10k-%s",
+		 wiphy_name(ar->hw->wiphy));
+	ar->leds.wifi_led.active_low = 1;
+	ar->leds.wifi_led.gpio = ar->hw_params.led_pin;
+	ar->leds.wifi_led.name = ar->leds.label;
+	ar->leds.wifi_led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+
+	ar->leds.cdev.name = ar->leds.label;
+	ar->leds.cdev.brightness_set_blocking = ath10k_leds_set_brightness_blocking;
+
+	/* FIXME: this assignment doesn't make sense as it's NULL, remove it? */
+	ar->leds.cdev.default_trigger = ar->leds.wifi_led.default_trigger;
+
+	ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &ar->leds.cdev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+void ath10k_leds_unregister(struct ath10k *ar)
+{
+	if (ar->hw_params.led_pin == 0)
+		/* leds not supported */
+		return;
+
+	led_classdev_unregister(&ar->leds.cdev);
+}
+
diff --git a/drivers/net/wireless/ath/ath10k/leds.h b/drivers/net/wireless/ath/ath10k/leds.h
new file mode 100644
index 000000000000..8a302c5b9516
--- /dev/null
+++ b/drivers/net/wireless/ath/ath10k/leds.h
@@ -0,0 +1,45 @@ 
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#ifndef _LEDS_H_
+#define _LEDS_H_
+
+#include "core.h"
+
+#if IS_ENABLED(CONFIG_ATH10K_LEDS)
+
+void ath10k_leds_unregister(struct ath10k *ar);
+int ath10k_leds_start(struct ath10k *ar);
+int ath10k_leds_register(struct ath10k *ar);
+
+#else
+
+static inline void ath10k_leds_unregister(struct ath10k *ar)
+{
+}
+
+static inline int ath10k_leds_start(struct ath10k *ar)
+{
+	return 0;
+}
+
+static inline int ath10k_leds_register(struct ath10k *ar)
+{
+	return 0;
+}
+
+#endif
+
+#endif /* _LEDS_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bf05a3689558..4676c5bc9a4a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -33,6 +33,7 @@ 
 #include "wmi-tlv.h"
 #include "wmi-ops.h"
 #include "wow.h"
+#include "leds.h"
 
 /*********/
 /* Rates */
diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index c35e45340b4f..f815092d6087 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -203,7 +203,10 @@  struct wmi_ops {
 	struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value);
 	struct sk_buff *(*gen_pdev_get_tpc_table_cmdid)(struct ath10k *ar,
 							u32 param);
+	struct sk_buff *(*gen_gpio_config)(struct ath10k *ar, u32 gpio_num,
+					   u32 input, u32 pull_type, u32 intr_mode);
 
+	struct sk_buff *(*gen_gpio_output)(struct ath10k *ar, u32 gpio_num, u32 set);
 };
 
 int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id);
@@ -981,6 +984,35 @@  ath10k_wmi_force_fw_hang(struct ath10k *ar,
 	return ath10k_wmi_cmd_send(ar, skb, ar->wmi.cmd->force_fw_hang_cmdid);
 }
 
+static inline int ath10k_wmi_gpio_config(struct ath10k *ar, u32 gpio_num,
+					 u32 input, u32 pull_type, u32 intr_mode)
+{
+	struct sk_buff *skb;
+
+	if (!ar->wmi.ops->gen_gpio_config)
+		return -EOPNOTSUPP;
+
+	skb = ar->wmi.ops->gen_gpio_config(ar, gpio_num, input, pull_type, intr_mode);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	return ath10k_wmi_cmd_send(ar, skb, ar->wmi.cmd->gpio_config_cmdid);
+}
+
+static inline int ath10k_wmi_gpio_output(struct ath10k *ar, u32 gpio_num, u32 set)
+{
+	struct sk_buff *skb;
+
+	if (!ar->wmi.ops->gen_gpio_config)
+		return -EOPNOTSUPP;
+
+	skb = ar->wmi.ops->gen_gpio_output(ar, gpio_num, set);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	return ath10k_wmi_cmd_send(ar, skb, ar->wmi.cmd->gpio_output_cmdid);
+}
+
 static inline int
 ath10k_wmi_dbglog_cfg(struct ath10k *ar, u64 module_enable, u32 log_level)
 {
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 9d1b0a459069..96246120c2ef 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -3815,6 +3815,8 @@  static const struct wmi_ops wmi_tlv_ops = {
 	.gen_echo = ath10k_wmi_tlv_op_gen_echo,
 	.gen_vdev_spectral_conf = ath10k_wmi_tlv_op_gen_vdev_spectral_conf,
 	.gen_vdev_spectral_enable = ath10k_wmi_tlv_op_gen_vdev_spectral_enable,
+	/* .gen_gpio_config not implemented */
+	/* .gen_gpio_output not implemented */
 };
 
 static const struct wmi_peer_flags_map wmi_tlv_peer_flags_map = {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index c5e1ca5945db..39f82d9e8cdf 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -6953,6 +6953,49 @@  ath10k_wmi_op_gen_peer_set_param(struct ath10k *ar, u32 vdev_id,
 	return skb;
 }
 
+static struct sk_buff *ath10k_wmi_op_gen_gpio_config(struct ath10k *ar,
+						     u32 gpio_num, u32 input,
+						     u32 pull_type, u32 intr_mode)
+{
+	struct wmi_gpio_config_cmd *cmd;
+	struct sk_buff *skb;
+
+	skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
+	if (!skb)
+		return ERR_PTR(-ENOMEM);
+
+	cmd = (struct wmi_gpio_config_cmd *)skb->data;
+	cmd->pull_type = __cpu_to_le32(pull_type);
+	cmd->gpio_num = __cpu_to_le32(gpio_num);
+	cmd->input = __cpu_to_le32(input);
+	cmd->intr_mode = __cpu_to_le32(intr_mode);
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi gpio_config gpio_num 0x%08x input 0x%08x pull_type 0x%08x intr_mode 0x%08x\n",
+		   gpio_num, input, pull_type, intr_mode);
+
+	return skb;
+}
+
+static struct sk_buff *ath10k_wmi_op_gen_gpio_output(struct ath10k *ar,
+						     u32 gpio_num, u32 set)
+{
+	struct wmi_gpio_output_cmd *cmd;
+	struct sk_buff *skb;
+
+	skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
+	if (!skb)
+		return ERR_PTR(-ENOMEM);
+
+	cmd = (struct wmi_gpio_output_cmd *)skb->data;
+	cmd->gpio_num = __cpu_to_le32(gpio_num);
+	cmd->set = __cpu_to_le32(set);
+
+	ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi gpio_output gpio_num 0x%08x set 0x%08x\n",
+		   gpio_num, set);
+
+	return skb;
+}
+
 static struct sk_buff *
 ath10k_wmi_op_gen_set_psmode(struct ath10k *ar, u32 vdev_id,
 			     enum wmi_sta_ps_mode psmode)
@@ -8543,6 +8586,9 @@  static const struct wmi_ops wmi_ops = {
 	.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
 	.gen_echo = ath10k_wmi_op_gen_echo,
+	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
+	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
+
 	/* .gen_bcn_tmpl not implemented */
 	/* .gen_prb_tmpl not implemented */
 	/* .gen_p2p_go_bcn_ie not implemented */
@@ -8613,6 +8659,8 @@  static const struct wmi_ops wmi_10_1_ops = {
 	.fw_stats_fill = ath10k_wmi_10x_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
 	.gen_echo = ath10k_wmi_op_gen_echo,
+	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
+	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
 	/* .gen_bcn_tmpl not implemented */
 	/* .gen_prb_tmpl not implemented */
 	/* .gen_p2p_go_bcn_ie not implemented */
@@ -8684,6 +8732,8 @@  static const struct wmi_ops wmi_10_2_ops = {
 	.gen_delba_send = ath10k_wmi_op_gen_delba_send,
 	.fw_stats_fill = ath10k_wmi_10x_op_fw_stats_fill,
 	.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
+	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
 	/* .gen_pdev_enable_adaptive_cca not implemented */
 };
 
@@ -8754,6 +8804,8 @@  static const struct wmi_ops wmi_10_2_4_ops = {
 	.gen_pdev_enable_adaptive_cca =
 		ath10k_wmi_op_gen_pdev_enable_adaptive_cca,
 	.get_vdev_subtype = ath10k_wmi_10_2_4_op_get_vdev_subtype,
+	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
+	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
 	/* .gen_bcn_tmpl not implemented */
 	/* .gen_prb_tmpl not implemented */
 	/* .gen_p2p_go_bcn_ie not implemented */
@@ -8831,6 +8883,8 @@  static const struct wmi_ops wmi_10_4_ops = {
 	.gen_pdev_bss_chan_info_req = ath10k_wmi_10_2_op_gen_pdev_bss_chan_info,
 	.gen_echo = ath10k_wmi_op_gen_echo,
 	.gen_pdev_get_tpc_config = ath10k_wmi_10_2_4_op_gen_pdev_get_tpc_config,
+	.gen_gpio_config = ath10k_wmi_op_gen_gpio_config,
+	.gen_gpio_output = ath10k_wmi_op_gen_gpio_output,
 };
 
 int ath10k_wmi_attach(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 6fbc84c29521..a1662ed64af0 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -2922,6 +2922,41 @@  enum wmi_10_4_feature_mask {
 
 };
 
+/* WMI_GPIO_CONFIG_CMDID */
+enum {
+	WMI_GPIO_PULL_NONE,
+	WMI_GPIO_PULL_UP,
+	WMI_GPIO_PULL_DOWN,
+};
+
+enum {
+	WMI_GPIO_INTTYPE_DISABLE,
+	WMI_GPIO_INTTYPE_RISING_EDGE,
+	WMI_GPIO_INTTYPE_FALLING_EDGE,
+	WMI_GPIO_INTTYPE_BOTH_EDGE,
+	WMI_GPIO_INTTYPE_LEVEL_LOW,
+	WMI_GPIO_INTTYPE_LEVEL_HIGH
+};
+
+/* WMI_GPIO_CONFIG_CMDID */
+struct wmi_gpio_config_cmd {
+	__le32 gpio_num;             /* GPIO number to be setup */
+	__le32 input;                /* 0 - Output/ 1 - Input */
+	__le32 pull_type;            /* Pull type defined above */
+	__le32 intr_mode;            /* Interrupt mode defined above (Input) */
+} __packed;
+
+/* WMI_GPIO_OUTPUT_CMDID */
+struct wmi_gpio_output_cmd {
+	__le32 gpio_num;    /* GPIO number to be setup */
+	__le32 set;         /* Set the GPIO pin*/
+} __packed;
+
+/* WMI_GPIO_INPUT_EVENTID */
+struct wmi_gpio_input_event {
+	__le32 gpio_num;    /* GPIO number which changed state */
+} __packed;
+
 struct wmi_ext_resource_config_10_4_cmd {
 	/* contains enum wmi_host_platform_type */
 	__le32 host_platform_config;