| Message ID | 20180220073259.17254-1-s.gottschall@dd-wrt.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Tue, Feb 20, 2018 at 08:32:59AM +0100, s.gottschall@dd-wrt.com 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> > >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 >Wq >--- > drivers/net/wireless/ath/ath10k/core.c | 183 +++++++++++++++++++++++++++++- > drivers/net/wireless/ath/ath10k/core.h | 20 +++- > drivers/net/wireless/ath/ath10k/debug.c | 146 ++++++++++++++++++++++++ > drivers/net/wireless/ath/ath10k/hw.h | 2 + > drivers/net/wireless/ath/ath10k/mac.c | 6 + > 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 ++++++ > 9 files changed, 536 insertions(+), 4 deletions(-) > >diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c >index f3ec13b80b20..e50977f97b18 100644 >--- a/drivers/net/wireless/ath/ath10k/core.c >+++ b/drivers/net/wireless/ath/ath10k/core.c >@@ -21,6 +21,10 @@ > #include <linux/dmi.h> > #include <linux/ctype.h> > #include <asm/byteorder.h> >+#include <linux/leds.h> >+#include <linux/platform_device.h> >+#include <linux/version.h> >+ > > #include "core.h" > #include "mac.h" >@@ -65,6 +69,8 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { > .id = QCA988X_HW_2_0_VERSION, > .dev_id = QCA988X_2_0_DEVICE_ID, > .name = "qca988x hw2.0", >+ .led_pin = 1, >+ .gpio_count = 24, > .patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, >@@ -94,6 +100,8 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { > .id = QCA988X_HW_2_0_VERSION, > .dev_id = QCA988X_2_0_DEVICE_ID_UBNT, > .name = "qca988x hw2.0 ubiquiti", >+ .led_pin = 1, >+ .gpio_count = 24, > .patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, >@@ -123,6 +131,8 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { > .id = QCA9887_HW_1_0_VERSION, > .dev_id = QCA9887_1_0_DEVICE_ID, > .name = "qca9887 hw1.0", >+ .led_pin = 1, >+ .gpio_count = 24, > .patch_load_addr = QCA9887_HW_1_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, >@@ -267,6 +277,8 @@ 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, >+ .gpio_count = 35, > .patch_load_addr = QCA99X0_HW_2_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .otp_exe_param = 0x00000700, >@@ -301,6 +313,8 @@ 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, >+ .gpio_count = 35, > .patch_load_addr = QCA9984_HW_1_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH, >@@ -340,6 +354,8 @@ 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, >+ .gpio_count = 35, > .patch_load_addr = QCA9888_HW_2_0_PATCH_LOAD_ADDR, > .uart_pin = 7, > .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH, >@@ -2132,12 +2148,148 @@ static int ath10k_core_reset_rx_filter(struct ath10k *ar) > return 0; > } > >+#ifdef CONFIG_GPIOLIB >+ >+static int ath10k_gpio_pin_cfg_input(struct gpio_chip *chip, unsigned offset) >+{ >+ struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); >+ ath10k_wmi_gpio_config(gpio->ar, offset, 1, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to input */ >+ gpio->gpio_state_dir = 1; >+ return 0; >+} >+ >+/* gpio_chip handler : set GPIO to output */ >+static int ath10k_gpio_pin_cfg_output(struct gpio_chip *chip, unsigned offset, >+ int value) >+{ >+ struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); >+ >+ ath10k_wmi_gpio_config(gpio->ar, offset, 0, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to output */ >+ ath10k_wmi_gpio_output(gpio->ar, offset, value); >+ gpio->gpio_state_dir = 0; >+ gpio->gpio_state_pin = value; >+ return 0; >+} >+ >+/* gpio_chip handler : query GPIO direction (0=out, 1=in) */ >+static int ath10k_gpio_pin_get_dir(struct gpio_chip *chip, unsigned offset) >+{ >+ struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); >+ >+ return gpio->gpio_state_dir; >+} >+ >+/* gpio_chip handler : get GPIO pin value */ >+static int ath10k_gpio_pin_get(struct gpio_chip *chip, unsigned offset) >+{ >+ struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); >+ >+ return gpio->gpio_state_pin; >+} >+ >+/* gpio_chip handler : set GPIO pin to value */ >+static void ath10k_gpio_pin_set(struct gpio_chip *chip, unsigned offset, >+ int value) >+{ >+ struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); >+ >+ ath10k_wmi_gpio_output(gpio->ar, offset, value); >+ gpio->gpio_state_pin = value; >+} >+ >+/* register GPIO chip */ >+static int ath10k_register_gpio_chip(struct ath10k *ar) >+{ >+ struct ath10k_gpiocontrol *gpio; >+ gpio = kzalloc(sizeof(struct ath10k_gpiocontrol), GFP_KERNEL); >+ if (!gpio) { >+ return -ENOMEM; >+ } >+ >+ snprintf(gpio->label, sizeof(gpio->label), "ath10k-%s", >+ wiphy_name(ar->hw->wiphy)); >+#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0) >+ gpio->gchip.parent = ar->dev; >+#else >+ gpio->gchip.dev = ar->dev; >+#endif >+ gpio->gchip.base = -1; /* determine base automatically */ >+ gpio->gchip.ngpio = ar->hw_params.gpio_count; >+ gpio->gchip.label = gpio->label; >+ gpio->gchip.direction_input = ath10k_gpio_pin_cfg_input; >+ gpio->gchip.direction_output = ath10k_gpio_pin_cfg_output; >+ gpio->gchip.get_direction = ath10k_gpio_pin_get_dir; >+ gpio->gchip.get = ath10k_gpio_pin_get; >+ gpio->gchip.set = ath10k_gpio_pin_set; >+ >+ if (gpiochip_add(&gpio->gchip)) { >+ printk(KERN_ERR "Error while registering gpio chip\n"); >+ return -1; >+ } >+ gpio->gchip.owner = NULL; >+ ar->gpio = gpio; >+ gpio->ar = ar; >+ return 0; >+} >+ >+#ifdef CONFIG_LEDS_CLASS >+static void ath10k_led_brightness(struct led_classdev *led_cdev, >+ enum led_brightness brightness) >+{ >+ struct ath10k_gpiocontrol *gpio = container_of(led_cdev, struct ath10k_gpiocontrol, cdev); >+ struct gpio_led *led = &gpio->wifi_led; >+ if (gpio->ar->state == ATH10K_STATE_ON) { >+ gpio->gpio_state_pin = (brightness != LED_OFF) ^ led->active_low; >+ ath10k_wmi_gpio_output(gpio->ar, led->gpio, gpio->gpio_state_pin); >+ } >+} >+ >+static int ath10k_add_led(struct ath10k *ar, struct gpio_led *gpioled) >+{ >+ int ret; >+ struct ath10k_gpiocontrol *gpio = ar->gpio; >+ if (!gpio) >+ return -1; >+ gpio->cdev.name = gpioled->name; >+ gpio->cdev.default_trigger = gpioled->default_trigger; >+ gpio->cdev.brightness_set = ath10k_led_brightness; >+ >+ ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &gpio->cdev); >+ if (ret < 0) >+ return ret; >+ >+ return 0; >+} >+#endif >+ >+#endif >+ >+/* remove GPIO chip */ >+static void ath10k_unregister_gpio_chip(struct ath10k *ar) >+{ >+#ifdef CONFIG_GPIOLIB >+ struct ath10k_gpiocontrol *gpio = ar->gpio; >+ if (gpio) { >+ gpiochip_remove(&gpio->gchip); >+ kfree(gpio); >+ } >+#endif >+} >+ >+static void ath10k_unregister_led(struct ath10k *ar) >+{ >+#if defined (CONFIG_GPIOLIB) && defined(CONFIG_LEDS_CLASS) >+ if (ar->gpio) >+ led_classdev_unregister(&ar->gpio->cdev); >+#endif >+} >+ >+ > int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, > const struct ath10k_fw_components *fw) > { > int status; > u32 val; >- > lockdep_assert_held(&ar->conf_mutex); > > clear_bit(ATH10K_FLAG_CRASH_FLUSH, &ar->dev_flags); >@@ -2372,8 +2524,32 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, > if (status) > goto err_hif_stop; > >- return 0; > >+#ifdef CONFIG_GPIOLIB >+ /* LED Code */ >+ if (ar->hw_params.led_pin) { /* only attach if non zero since some chipsets are unsupported yet */ >+ if (!ar->gpio_attached) { >+ status = ath10k_register_gpio_chip(ar); >+ if (status) { >+ goto err_no_led; >+ } >+#ifdef CONFIG_LEDS_CLASS >+ ar->gpio_attached = 1; >+ ar->gpio->wifi_led.active_low = 1; >+ ar->gpio->wifi_led.gpio = ar->hw_params.led_pin; >+ ar->gpio->wifi_led.name = ar->gpio->label; >+ ar->gpio->wifi_led.default_state = LEDS_GPIO_DEFSTATE_KEEP; >+ >+ ath10k_add_led(ar, &ar->gpio->wifi_led); >+#endif >+ } >+ ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to output */ >+ ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1); >+ } >+err_no_led:; >+#endif >+ >+ return 0; > err_hif_stop: > ath10k_hif_stop(ar); > err_htt_rx_detach: >@@ -2673,6 +2849,9 @@ void ath10k_core_unregister(struct ath10k *ar) > ath10k_core_free_board_files(ar); > > ath10k_debug_unregister(ar); >+ >+ ath10k_unregister_led(ar); >+ ath10k_unregister_gpio_chip(ar); > } > EXPORT_SYMBOL(ath10k_core_unregister); > >diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h >index c624b96f8b84..1340904582b7 100644 >--- a/drivers/net/wireless/ath/ath10k/core.h >+++ b/drivers/net/wireless/ath/ath10k/core.h >@@ -25,6 +25,8 @@ > #include <linux/pci.h> > #include <linux/uuid.h> > #include <linux/time.h> >+#include <linux/gpio.h> >+#include <linux/leds.h> > > #include "htt.h" > #include "htc.h" >@@ -812,6 +814,21 @@ struct ath10k_per_peer_tx_stats { > u32 reserved2; > }; > >+struct ath10k_gpiocontrol { >+ struct ath10k *ar; >+ u32 gpio_set_num, gpio_num, gpio_input, gpio_pull_type, gpio_intr_mode, gpio_set; /* since we have no gpio read method, these are the state variables for debugfs. */ I think the checkpatch.pl will complain above defintely. Best regards, >+#ifdef CONFIG_GPIOLIB >+ struct gpio_chip gchip; >+#endif >+#ifdef CONFIG_LEDS_CLASS >+ struct gpio_led wifi_led; >+ struct led_classdev cdev; >+#endif >+ char label[48]; >+ u32 gpio_state_dir; /* same as for debugfs, but for gpiochip implementation */ >+ u32 gpio_state_pin; >+}; >+ > struct ath10k { > struct ath_common ath_common; > struct ieee80211_hw *hw; >@@ -840,7 +857,8 @@ struct ath10k { > u32 low_5ghz_chan; > u32 high_5ghz_chan; > bool ani_enabled; >- >+ struct ath10k_gpiocontrol *gpio; >+ int gpio_attached; > bool p2p; > > struct { >diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c >index 1b9c092d210f..4ed3540669ec 100644 >--- a/drivers/net/wireless/ath/ath10k/debug.c >+++ b/drivers/net/wireless/ath/ath10k/debug.c >@@ -1084,6 +1084,128 @@ static ssize_t ath10k_write_fw_dbglog(struct file *file, > return ret; > } > >+ >+static ssize_t ath10k_read_gpio_config(struct file *file, char __user *user_buf, >+ size_t count, loff_t *ppos) >+{ >+ struct ath10k *ar = file->private_data; >+ struct ath10k_gpiocontrol *gpio = ar->gpio; >+ size_t len; >+ char buf[96]; >+ if (!gpio) >+ return 0; >+ >+ len = scnprintf(buf, sizeof(buf), "%u %u %u %u\n", gpio->gpio_num, gpio->gpio_input, gpio->gpio_pull_type, gpio->gpio_intr_mode); >+ >+ return simple_read_from_buffer(user_buf, count, ppos, buf, len); >+} >+ >+ >+static ssize_t ath10k_write_gpio_config(struct file *file, >+ const char __user *user_buf, >+ size_t count, loff_t *ppos) >+{ >+ struct ath10k *ar = file->private_data; >+ struct ath10k_gpiocontrol *gpio = ar->gpio; >+ int ret; >+ char buf[96]; >+ u32 gpio_num, input, pull_type, intr_mode; >+ if (!gpio) >+ return -EINVAL; >+ >+ simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count); >+ >+ /* make sure that buf is null terminated */ >+ buf[sizeof(buf) - 1] = 0; >+ >+ gpio->gpio_num = gpio_num; >+ gpio->gpio_input = input; >+ gpio->gpio_pull_type = pull_type; >+ gpio->gpio_intr_mode = intr_mode; >+ ret = sscanf(buf, "%u %u %u %u", &gpio_num, &input, &pull_type, &intr_mode); >+ >+ if (!ret) >+ return -EINVAL; >+ >+ >+ mutex_lock(&ar->conf_mutex); >+ >+ >+ ret = ath10k_wmi_gpio_config(ar, gpio_num, input, pull_type, intr_mode); >+ >+ if (ret) { >+ ath10k_warn(ar, "gpio_config cfg failed from debugfs: %d\n", ret); >+ goto exit; >+ } >+ ret = count; >+ >+exit: >+ mutex_unlock(&ar->conf_mutex); >+ >+ return ret; >+} >+ >+static ssize_t ath10k_read_gpio_output(struct file *file, char __user *user_buf, >+ size_t count, loff_t *ppos) >+{ >+ struct ath10k *ar = file->private_data; >+ struct ath10k_gpiocontrol *gpio = ar->gpio; >+ size_t len; >+ char buf[96]; >+ if (!gpio) >+ return 0; >+ >+ len = scnprintf(buf, sizeof(buf), "%u %u\n", gpio->gpio_num, gpio->gpio_set); >+ >+ return simple_read_from_buffer(user_buf, count, ppos, buf, len); >+} >+ >+ >+static ssize_t ath10k_write_gpio_output(struct file *file, >+ const char __user *user_buf, >+ size_t count, loff_t *ppos) >+{ >+ struct ath10k *ar = file->private_data; >+ struct ath10k_gpiocontrol *gpio = ar->gpio; >+ int ret; >+ char buf[96]; >+ u32 gpio_num, set; >+ if (!gpio) >+ return -EINVAL; >+ >+ simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count); >+ >+ /* make sure that buf is null terminated */ >+ buf[sizeof(buf) - 1] = 0; >+ >+ gpio->gpio_set_num = gpio_num; >+ gpio->gpio_set = set; >+ ret = sscanf(buf, "%u %u", &gpio_num, &set); >+ >+ if (!ret) >+ return -EINVAL; >+ >+ >+ mutex_lock(&ar->conf_mutex); >+ >+ >+ ret = ath10k_wmi_gpio_output(ar, gpio_num, set); >+ >+ if (ret) { >+ ath10k_warn(ar, "gpio_output cfg failed from debugfs: %d\n", ret); >+ goto exit; >+ } >+ ret = count; >+ >+exit: >+ mutex_unlock(&ar->conf_mutex); >+ >+ return ret; >+} >+ >+ >+ >+ > /* TODO: Would be nice to always support ethtool stats, would need to > * move the stats storage out of ath10k_debug, or always have ath10k_debug > * struct available.. >@@ -1252,6 +1374,24 @@ static const struct file_operations fops_fw_dbglog = { > .llseek = default_llseek, > }; > >+ >+ >+static const struct file_operations fops_gpio_output = { >+ .read = ath10k_read_gpio_output, >+ .write = ath10k_write_gpio_output, >+ .open = simple_open, >+ .owner = THIS_MODULE, >+ .llseek = default_llseek, >+}; >+ >+static const struct file_operations fops_gpio_config = { >+ .read = ath10k_read_gpio_config, >+ .write = ath10k_write_gpio_config, >+ .open = simple_open, >+ .owner = THIS_MODULE, >+ .llseek = default_llseek, >+}; >+ > static int ath10k_debug_cal_data_fetch(struct ath10k *ar) > { > u32 hi_addr; >@@ -2259,6 +2399,12 @@ int ath10k_debug_register(struct ath10k *ar) > debugfs_create_file("fw_dbglog", 0600, ar->debug.debugfs_phy, ar, > &fops_fw_dbglog); > >+ debugfs_create_file("gpio_output", 0600, ar->debug.debugfs_phy, ar, >+ &fops_gpio_output); >+ >+ debugfs_create_file("gpio_config", 0600, ar->debug.debugfs_phy, ar, >+ &fops_gpio_config); >+ > debugfs_create_file("cal_data", 0400, ar->debug.debugfs_phy, ar, > &fops_cal_data); > >diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h >index 413b1b4321f7..af91d0202e45 100644 >--- a/drivers/net/wireless/ath/ath10k/hw.h >+++ b/drivers/net/wireless/ath/ath10k/hw.h >@@ -495,6 +495,8 @@ struct ath10k_hw_params { > const char *name; > u32 patch_load_addr; > int uart_pin; >+ int led_pin; /* 1 for peregrine, 17 for beeliner */ >+ int gpio_count; /* 24 for peregrine, 35 for beeliner */ > u32 otp_exe_param; > > /* Type of hw cycle counter wraparound logic, for more info >diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c >index ebb3f1b046f3..f1932cc9f153 100644 >--- a/drivers/net/wireless/ath/ath10k/mac.c >+++ b/drivers/net/wireless/ath/ath10k/mac.c >@@ -4601,6 +4601,12 @@ static int ath10k_start(struct ieee80211_hw *hw) > switch (ar->state) { > case ATH10K_STATE_OFF: > ar->state = ATH10K_STATE_ON; >+ >+ /* need to reset gpio state */ >+ if (ar->hw_params.led_pin) { >+ 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); >+ } > break; > case ATH10K_STATE_RESTARTING: > ar->state = ATH10K_STATE_RESTARTED; >diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h >index 14093cfdc505..a38d6a8f7ec5 100644 >--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h >+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h >@@ -197,8 +197,13 @@ struct wmi_ops { > (struct ath10k *ar, > enum wmi_bss_survey_req_type type); > struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value); >+ >+ 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); > > static inline int >@@ -957,6 +962,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_nowait(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_nowait(ar, skb, ar->wmi.cmd->gpio_output_cmdid); >+} > > static inline int > ath10k_wmi_dbglog_cfg(struct ath10k *ar, u64 module_enable, u32 log_level) >@@ -1034,7 +1068,7 @@ ath10k_wmi_pdev_get_temperature(struct ath10k *ar) > if (IS_ERR(skb)) > return PTR_ERR(skb); > >- return ath10k_wmi_cmd_send(ar, skb, >+ return ath10k_wmi_cmd_send_nowait(ar, skb, > ar->wmi.cmd->pdev_get_temperature_cmdid); > } > >diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c >index ae77a007ae07..2bfba63f1dcf 100644 >--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c >+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c >@@ -3250,6 +3250,69 @@ ath10k_wmi_tlv_op_gen_echo(struct ath10k *ar, u32 value) > return skb; > } > >+static struct sk_buff * >+ath10k_wmi_tlv_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 wmi_tlv *tlv; >+ struct sk_buff *skb; >+ void *ptr; >+ size_t len; >+ >+ len = sizeof(*tlv) + sizeof(*cmd); >+ skb = ath10k_wmi_alloc_skb(ar, len); >+ if (!skb) >+ return ERR_PTR(-ENOMEM); >+ >+ ptr = (void *)skb->data; >+ tlv = ptr; >+ tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_GPIO_CONFIG_CMD); >+ tlv->len = __cpu_to_le16(sizeof(*cmd)); >+ >+ 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); >+ >+ ptr += sizeof(*tlv); >+ ptr += sizeof(*cmd); >+ >+ ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv 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_tlv_op_gen_gpio_output(struct ath10k *ar, u32 gpio_num, u32 set) >+{ >+ struct wmi_gpio_output_cmd *cmd; >+ struct wmi_tlv *tlv; >+ struct sk_buff *skb; >+ void *ptr; >+ size_t len; >+ >+ len = sizeof(*tlv) + sizeof(*cmd); >+ skb = ath10k_wmi_alloc_skb(ar, len); >+ if (!skb) >+ return ERR_PTR(-ENOMEM); >+ >+ ptr = (void *)skb->data; >+ tlv = ptr; >+ tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_GPIO_OUTPUT_CMD); >+ tlv->len = __cpu_to_le16(sizeof(*cmd)); >+ >+ cmd = (struct wmi_gpio_output_cmd *)skb->data; >+ cmd->gpio_num = __cpu_to_le32(gpio_num); >+ cmd->set = __cpu_to_le32(set); >+ >+ ptr += sizeof(*tlv); >+ ptr += sizeof(*cmd); >+ >+ ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv gpio_output gpio_num 0x%08x set 0x%08x\n", gpio_num, set); >+ return skb; >+} >+ >+ > static struct sk_buff * > ath10k_wmi_tlv_op_gen_vdev_spectral_conf(struct ath10k *ar, > const struct wmi_vdev_spectral_conf_arg *arg) >@@ -3727,6 +3790,8 @@ static const struct wmi_ops wmi_tlv_ops = { > .fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill, > .get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype, > .gen_echo = ath10k_wmi_tlv_op_gen_echo, >+ .gen_gpio_config = ath10k_wmi_tlv_op_gen_gpio_config, >+ .gen_gpio_output = ath10k_wmi_tlv_op_gen_gpio_output, > .gen_vdev_spectral_conf = ath10k_wmi_tlv_op_gen_vdev_spectral_conf, > .gen_vdev_spectral_enable = ath10k_wmi_tlv_op_gen_vdev_spectral_enable, > }; >diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c >index 58dc2189ba49..b56e5a673a8c 100644 >--- a/drivers/net/wireless/ath/ath10k/wmi.c >+++ b/drivers/net/wireless/ath/ath10k/wmi.c >@@ -6646,6 +6646,41 @@ 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) >@@ -8153,6 +8188,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 */ >@@ -8223,6 +8261,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 */ >@@ -8294,6 +8334,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 */ > }; > >@@ -8364,6 +8406,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 */ >@@ -8439,6 +8483,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 c7b30ed9015d..dc180a86dc3b 100644 >--- a/drivers/net/wireless/ath/ath10k/wmi.h >+++ b/drivers/net/wireless/ath/ath10k/wmi.h >@@ -2906,6 +2906,42 @@ 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; >-- >2.14.1 >
>> >> +struct ath10k_gpiocontrol { >> + struct ath10k *ar; >> + u32 gpio_set_num, gpio_num, gpio_input, gpio_pull_type, >> gpio_intr_mode, gpio_set; /* since we have no gpio read method, these >> are the state variables for debugfs. */ > > I think the checkpatch.pl will complain above defintely. > Best regards, because of the one liner? Sebastian
Hi Sebastian, On Mon, Feb 19, 2018 at 11:32 PM, <s.gottschall@dd-wrt.com> 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> > > 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 > Wq > --- > drivers/net/wireless/ath/ath10k/core.c | 183 +++++++++++++++++++++++++++++- > drivers/net/wireless/ath/ath10k/core.h | 20 +++- > drivers/net/wireless/ath/ath10k/debug.c | 146 ++++++++++++++++++++++++ > drivers/net/wireless/ath/ath10k/hw.h | 2 + > drivers/net/wireless/ath/ath10k/mac.c | 6 + > 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 ++++++ > 9 files changed, 536 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c > index f3ec13b80b20..e50977f97b18 100644 > --- a/drivers/net/wireless/ath/ath10k/core.c > +++ b/drivers/net/wireless/ath/ath10k/core.c ... > +static int ath10k_register_gpio_chip(struct ath10k *ar) > +{ > + struct ath10k_gpiocontrol *gpio; > + gpio = kzalloc(sizeof(struct ath10k_gpiocontrol), GFP_KERNEL); > + if (!gpio) { > + return -ENOMEM; > + } > + > + snprintf(gpio->label, sizeof(gpio->label), "ath10k-%s", > + wiphy_name(ar->hw->wiphy)); > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0) > + gpio->gchip.parent = ar->dev; > +#else > + gpio->gchip.dev = ar->dev; > +#endif > + gpio->gchip.base = -1; /* determine base automatically */ I may be wrong, but I think that version guards are unwelcome here. The patch is supposed to go and apply specifically to the upstream version, which is clearly >= KERNEL_VERSION(4,5,0). I think you can drop this, only use the 'gpio->gchip.parent = ar->dev;' and call it good. This fixup should be picked up by Backports. I understand that you might need to keep it for your internal tree, but I don't think the maintainers want to see this. > +/* remove GPIO chip */ > +static void ath10k_unregister_gpio_chip(struct ath10k *ar) > +{ > +#ifdef CONFIG_GPIOLIB > + struct ath10k_gpiocontrol *gpio = ar->gpio; > + if (gpio) { > + gpiochip_remove(&gpio->gchip); > + kfree(gpio); > + } > +#endif > +} Instead of entering/exiting the guards inside every function, I think the usually accepted way to handle this is to place a block in the that looks basically like: #ifdef CONFIG_GPIOLIB static void ath10k_unregister_gpio_chip(struct ath10k *ar) { struct ath10k_gpiocontrol *gpio = ar->gpio; if (gpio) { gpiochip_remove(&gpio->gchip); kfree(gpio); } } next function.... next function... #else static void ath10k_unregister_gpio_chip(struct ath10k *ar) { } next function.... next function... #endif If they weren't static, you'd do it in the .h file, with the declaration of the full function there and the definition in the .c. I may be nit-picking and your style is perfectly acceptable to the other maintainers. > + > +static void ath10k_unregister_led(struct ath10k *ar) > +{ > +#if defined (CONFIG_GPIOLIB) && defined(CONFIG_LEDS_CLASS) > + if (ar->gpio) > + led_classdev_unregister(&ar->gpio->cdev); > +#endif > +} > + > + > int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, > const struct ath10k_fw_components *fw) > { > int status; > u32 val; > - > lockdep_assert_held(&ar->conf_mutex); > Whitespace change for no apparent reason. > @@ -2372,8 +2524,32 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, > if (status) > goto err_hif_stop; > > - return 0; > > +#ifdef CONFIG_GPIOLIB > + /* LED Code */ > + if (ar->hw_params.led_pin) { /* only attach if non zero since some chipsets are unsupported yet */ > + if (!ar->gpio_attached) { > + status = ath10k_register_gpio_chip(ar); > + if (status) { > + goto err_no_led; > + } > +#ifdef CONFIG_LEDS_CLASS > + ar->gpio_attached = 1; > + ar->gpio->wifi_led.active_low = 1; > + ar->gpio->wifi_led.gpio = ar->hw_params.led_pin; > + ar->gpio->wifi_led.name = ar->gpio->label; > + ar->gpio->wifi_led.default_state = LEDS_GPIO_DEFSTATE_KEEP; > + > + ath10k_add_led(ar, &ar->gpio->wifi_led); > +#endif > + } > + ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to output */ > + ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1); > + } > +err_no_led:; > +#endif > + > + return 0; > err_hif_stop: > ath10k_hif_stop(ar); > err_htt_rx_detach: The guards make it harder to read. In this chunk, every component should be defined acceptably even if the CONFIG flags aren't defined because they're all under your control. Make sure they are, and then you can drop the guards here. The only trick is to be sure that any accessed struct members that are protected by guards are abstracted and only accessed inside your guarded functions. Alternately, you could encapsulate it into a function ath10k_core_gpios_start() and just have the single call there. The additional benefit being that you don't need the err_no_led: jump. > diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h > index c624b96f8b84..1340904582b7 100644 > --- a/drivers/net/wireless/ath/ath10k/core.h > +++ b/drivers/net/wireless/ath/ath10k/core.h > @@ -25,6 +25,8 @@ > #include <linux/pci.h> > #include <linux/uuid.h> > #include <linux/time.h> > +#include <linux/gpio.h> > +#include <linux/leds.h> > > #include "htt.h" > #include "htc.h" > @@ -812,6 +814,21 @@ struct ath10k_per_peer_tx_stats { > u32 reserved2; > }; > > +struct ath10k_gpiocontrol { > + struct ath10k *ar; > + u32 gpio_set_num, gpio_num, gpio_input, gpio_pull_type, gpio_intr_mode, gpio_set; /* since we have no gpio read method, these are the state variables for debugfs. */ > +#ifdef CONFIG_GPIOLIB > + struct gpio_chip gchip; > +#endif > +#ifdef CONFIG_LEDS_CLASS > + struct gpio_led wifi_led; > + struct led_classdev cdev; > +#endif > + char label[48]; > + u32 gpio_state_dir; /* same as for debugfs, but for gpiochip implementation */ > + u32 gpio_state_pin; > +}; > + > struct ath10k { > struct ath_common ath_common; > struct ieee80211_hw *hw; > @@ -840,7 +857,8 @@ struct ath10k { > u32 low_5ghz_chan; > u32 high_5ghz_chan; > bool ani_enabled; > - > + struct ath10k_gpiocontrol *gpio; > + int gpio_attached; > bool p2p; > > struct { > diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c > index 1b9c092d210f..4ed3540669ec 100644 > --- a/drivers/net/wireless/ath/ath10k/debug.c > +++ b/drivers/net/wireless/ath/ath10k/debug.c > @@ -1084,6 +1084,128 @@ static ssize_t ath10k_write_fw_dbglog(struct file *file, > return ret; > } > > + > +static ssize_t ath10k_read_gpio_config(struct file *file, char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct ath10k *ar = file->private_data; > + struct ath10k_gpiocontrol *gpio = ar->gpio; > + size_t len; > + char buf[96]; > + if (!gpio) > + return 0; > + > + len = scnprintf(buf, sizeof(buf), "%u %u %u %u\n", gpio->gpio_num, gpio->gpio_input, gpio->gpio_pull_type, gpio->gpio_intr_mode); > + > + return simple_read_from_buffer(user_buf, count, ppos, buf, len); > +} > + > + > +static ssize_t ath10k_write_gpio_config(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct ath10k *ar = file->private_data; > + struct ath10k_gpiocontrol *gpio = ar->gpio; > + int ret; > + char buf[96]; > + u32 gpio_num, input, pull_type, intr_mode; > + if (!gpio) > + return -EINVAL; > + > + simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count); > + > + /* make sure that buf is null terminated */ > + buf[sizeof(buf) - 1] = 0; > + > + gpio->gpio_num = gpio_num; > + gpio->gpio_input = input; > + gpio->gpio_pull_type = pull_type; > + gpio->gpio_intr_mode = intr_mode; > + ret = sscanf(buf, "%u %u %u %u", &gpio_num, &input, &pull_type, &intr_mode); > + > + if (!ret) > + return -EINVAL; > + > + > + mutex_lock(&ar->conf_mutex); > + > + > + ret = ath10k_wmi_gpio_config(ar, gpio_num, input, pull_type, intr_mode); > + > + if (ret) { > + ath10k_warn(ar, "gpio_config cfg failed from debugfs: %d\n", ret); > + goto exit; > + } > + ret = count; > + > +exit: > + mutex_unlock(&ar->conf_mutex); > + > + return ret; > +} > + > +static ssize_t ath10k_read_gpio_output(struct file *file, char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct ath10k *ar = file->private_data; > + struct ath10k_gpiocontrol *gpio = ar->gpio; > + size_t len; > + char buf[96]; > + if (!gpio) > + return 0; > + > + len = scnprintf(buf, sizeof(buf), "%u %u\n", gpio->gpio_num, gpio->gpio_set); > + > + return simple_read_from_buffer(user_buf, count, ppos, buf, len); > +} > + > + > +static ssize_t ath10k_write_gpio_output(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct ath10k *ar = file->private_data; > + struct ath10k_gpiocontrol *gpio = ar->gpio; > + int ret; > + char buf[96]; > + u32 gpio_num, set; > + if (!gpio) > + return -EINVAL; > + > + simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count); > + > + /* make sure that buf is null terminated */ > + buf[sizeof(buf) - 1] = 0; > + > + gpio->gpio_set_num = gpio_num; > + gpio->gpio_set = set; > + ret = sscanf(buf, "%u %u", &gpio_num, &set); > + > + if (!ret) > + return -EINVAL; > + > + > + mutex_lock(&ar->conf_mutex); > + > + > + ret = ath10k_wmi_gpio_output(ar, gpio_num, set); > + > + if (ret) { > + ath10k_warn(ar, "gpio_output cfg failed from debugfs: %d\n", ret); > + goto exit; > + } > + ret = count; > + > +exit: > + mutex_unlock(&ar->conf_mutex); > + > + return ret; > +} > + > + > + > + > /* TODO: Would be nice to always support ethtool stats, would need to > * move the stats storage out of ath10k_debug, or always have ath10k_debug > * struct available.. > @@ -1252,6 +1374,24 @@ static const struct file_operations fops_fw_dbglog = { > .llseek = default_llseek, > }; > > + > + > +static const struct file_operations fops_gpio_output = { > + .read = ath10k_read_gpio_output, > + .write = ath10k_write_gpio_output, > + .open = simple_open, > + .owner = THIS_MODULE, > + .llseek = default_llseek, > +}; > + > +static const struct file_operations fops_gpio_config = { > + .read = ath10k_read_gpio_config, > + .write = ath10k_write_gpio_config, > + .open = simple_open, > + .owner = THIS_MODULE, > + .llseek = default_llseek, > +}; > + > static int ath10k_debug_cal_data_fetch(struct ath10k *ar) > { > u32 hi_addr; > @@ -2259,6 +2399,12 @@ int ath10k_debug_register(struct ath10k *ar) > debugfs_create_file("fw_dbglog", 0600, ar->debug.debugfs_phy, ar, > &fops_fw_dbglog); > > + debugfs_create_file("gpio_output", 0600, ar->debug.debugfs_phy, ar, > + &fops_gpio_output); > + > + debugfs_create_file("gpio_config", 0600, ar->debug.debugfs_phy, ar, > + &fops_gpio_config); > + > debugfs_create_file("cal_data", 0400, ar->debug.debugfs_phy, ar, > &fops_cal_data); > > diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h > index 413b1b4321f7..af91d0202e45 100644 > --- a/drivers/net/wireless/ath/ath10k/hw.h > +++ b/drivers/net/wireless/ath/ath10k/hw.h > @@ -495,6 +495,8 @@ struct ath10k_hw_params { > const char *name; > u32 patch_load_addr; > int uart_pin; > + int led_pin; /* 1 for peregrine, 17 for beeliner */ > + int gpio_count; /* 24 for peregrine, 35 for beeliner */ > u32 otp_exe_param; > > /* Type of hw cycle counter wraparound logic, for more info > diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c > index ebb3f1b046f3..f1932cc9f153 100644 > --- a/drivers/net/wireless/ath/ath10k/mac.c > +++ b/drivers/net/wireless/ath/ath10k/mac.c > @@ -4601,6 +4601,12 @@ static int ath10k_start(struct ieee80211_hw *hw) > switch (ar->state) { > case ATH10K_STATE_OFF: > ar->state = ATH10K_STATE_ON; > + > + /* need to reset gpio state */ > + if (ar->hw_params.led_pin) { > + 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); > + } > break; > case ATH10K_STATE_RESTARTING: > ar->state = ATH10K_STATE_RESTARTED; > diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h > index 14093cfdc505..a38d6a8f7ec5 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi-ops.h > +++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h > @@ -197,8 +197,13 @@ struct wmi_ops { > (struct ath10k *ar, > enum wmi_bss_survey_req_type type); > struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value); > + > + 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); > > static inline int > @@ -957,6 +962,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_nowait(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_nowait(ar, skb, ar->wmi.cmd->gpio_output_cmdid); > +} > > static inline int > ath10k_wmi_dbglog_cfg(struct ath10k *ar, u64 module_enable, u32 log_level) > @@ -1034,7 +1068,7 @@ ath10k_wmi_pdev_get_temperature(struct ath10k *ar) > if (IS_ERR(skb)) > return PTR_ERR(skb); > > - return ath10k_wmi_cmd_send(ar, skb, > + return ath10k_wmi_cmd_send_nowait(ar, skb, > ar->wmi.cmd->pdev_get_temperature_cmdid); > } > > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c > index ae77a007ae07..2bfba63f1dcf 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c > @@ -3250,6 +3250,69 @@ ath10k_wmi_tlv_op_gen_echo(struct ath10k *ar, u32 value) > return skb; > } > > +static struct sk_buff * > +ath10k_wmi_tlv_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 wmi_tlv *tlv; > + struct sk_buff *skb; > + void *ptr; > + size_t len; > + > + len = sizeof(*tlv) + sizeof(*cmd); > + skb = ath10k_wmi_alloc_skb(ar, len); > + if (!skb) > + return ERR_PTR(-ENOMEM); > + > + ptr = (void *)skb->data; > + tlv = ptr; > + tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_GPIO_CONFIG_CMD); > + tlv->len = __cpu_to_le16(sizeof(*cmd)); > + > + 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); > + > + ptr += sizeof(*tlv); > + ptr += sizeof(*cmd); > + > + ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv 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_tlv_op_gen_gpio_output(struct ath10k *ar, u32 gpio_num, u32 set) > +{ > + struct wmi_gpio_output_cmd *cmd; > + struct wmi_tlv *tlv; > + struct sk_buff *skb; > + void *ptr; > + size_t len; > + > + len = sizeof(*tlv) + sizeof(*cmd); > + skb = ath10k_wmi_alloc_skb(ar, len); > + if (!skb) > + return ERR_PTR(-ENOMEM); > + > + ptr = (void *)skb->data; > + tlv = ptr; > + tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_GPIO_OUTPUT_CMD); > + tlv->len = __cpu_to_le16(sizeof(*cmd)); > + > + cmd = (struct wmi_gpio_output_cmd *)skb->data; > + cmd->gpio_num = __cpu_to_le32(gpio_num); > + cmd->set = __cpu_to_le32(set); > + > + ptr += sizeof(*tlv); > + ptr += sizeof(*cmd); > + > + ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv gpio_output gpio_num 0x%08x set 0x%08x\n", gpio_num, set); > + return skb; > +} > + > + > static struct sk_buff * > ath10k_wmi_tlv_op_gen_vdev_spectral_conf(struct ath10k *ar, > const struct wmi_vdev_spectral_conf_arg *arg) > @@ -3727,6 +3790,8 @@ static const struct wmi_ops wmi_tlv_ops = { > .fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill, > .get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype, > .gen_echo = ath10k_wmi_tlv_op_gen_echo, > + .gen_gpio_config = ath10k_wmi_tlv_op_gen_gpio_config, > + .gen_gpio_output = ath10k_wmi_tlv_op_gen_gpio_output, > .gen_vdev_spectral_conf = ath10k_wmi_tlv_op_gen_vdev_spectral_conf, > .gen_vdev_spectral_enable = ath10k_wmi_tlv_op_gen_vdev_spectral_enable, > }; > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c > index 58dc2189ba49..b56e5a673a8c 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi.c > +++ b/drivers/net/wireless/ath/ath10k/wmi.c > @@ -6646,6 +6646,41 @@ 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) > @@ -8153,6 +8188,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 */ > @@ -8223,6 +8261,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 */ > @@ -8294,6 +8334,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 */ > }; > > @@ -8364,6 +8406,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 */ > @@ -8439,6 +8483,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 c7b30ed9015d..dc180a86dc3b 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi.h > +++ b/drivers/net/wireless/ath/ath10k/wmi.h > @@ -2906,6 +2906,42 @@ 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; > -- > 2.14.1 > The rest of this looks OK to me. - Steve -- Steve deRosier Cal-Sierra Consulting LLC https://www.cal-sierra.com/
Am 20.02.2018 um 17:52 schrieb Steve deRosier: > >> +static int ath10k_register_gpio_chip(struct ath10k *ar) >> +{ >> + struct ath10k_gpiocontrol *gpio; >> + gpio = kzalloc(sizeof(struct ath10k_gpiocontrol), GFP_KERNEL); >> + if (!gpio) { >> + return -ENOMEM; >> + } >> + >> + snprintf(gpio->label, sizeof(gpio->label), "ath10k-%s", >> + wiphy_name(ar->hw->wiphy)); >> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0) >> + gpio->gchip.parent = ar->dev; >> +#else >> + gpio->gchip.dev = ar->dev; >> +#endif >> + gpio->gchip.base = -1; /* determine base automatically */ > I may be wrong, but I think that version guards are unwelcome here. > The patch is supposed to go and apply specifically to the upstream > version, which is clearly >= KERNEL_VERSION(4,5,0). I think you can > drop this, only use the 'gpio->gchip.parent = ar->dev;' and call it > good. This fixup should be picked up by Backports. I understand that > you might need to keep it for your internal tree, but I don't think > the maintainers want to see this. the original patch comes from my sources which are supposed to work with compat-wireless für multiple kernels. but for sure i can change that :-) > > >> +/* remove GPIO chip */ >> +static void ath10k_unregister_gpio_chip(struct ath10k *ar) >> +{ >> +#ifdef CONFIG_GPIOLIB >> + struct ath10k_gpiocontrol *gpio = ar->gpio; >> + if (gpio) { >> + gpiochip_remove(&gpio->gchip); >> + kfree(gpio); >> + } >> +#endif >> +} > Instead of entering/exiting the guards inside every function, I think > the usually accepted way to handle this is to place a block in the > that looks basically like: > > #ifdef CONFIG_GPIOLIB > static void ath10k_unregister_gpio_chip(struct ath10k *ar) { > struct ath10k_gpiocontrol *gpio = ar->gpio; > if (gpio) { > gpiochip_remove(&gpio->gchip); > kfree(gpio); > } > } > > next function.... > next function... > > #else > static void ath10k_unregister_gpio_chip(struct ath10k *ar) { > } > next function.... > next function... > #endif > > If they weren't static, you'd do it in the .h file, with the > declaration of the full function there and the definition in the .c. > > I may be nit-picking and your style is perfectly acceptable to the > other maintainers. i'm not a fan of bloat code. and making funktions static which are just called one will allow the compiler to optimize the code better than making it non static. that makes no sense here for sure. its just a question how you handle it. > > >> + >> +static void ath10k_unregister_led(struct ath10k *ar) >> +{ >> +#if defined (CONFIG_GPIOLIB) && defined(CONFIG_LEDS_CLASS) >> + if (ar->gpio) >> + led_classdev_unregister(&ar->gpio->cdev); >> +#endif >> +} >> + >> + >> int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, >> const struct ath10k_fw_components *fw) >> { >> int status; >> u32 val; >> - >> lockdep_assert_held(&ar->conf_mutex); >> > Whitespace change for no apparent reason. count it as typo > >> @@ -2372,8 +2524,32 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, >> if (status) >> goto err_hif_stop; >> >> - return 0; >> >> +#ifdef CONFIG_GPIOLIB >> + /* LED Code */ >> + if (ar->hw_params.led_pin) { /* only attach if non zero since some chipsets are unsupported yet */ >> + if (!ar->gpio_attached) { >> + status = ath10k_register_gpio_chip(ar); >> + if (status) { >> + goto err_no_led; >> + } >> +#ifdef CONFIG_LEDS_CLASS >> + ar->gpio_attached = 1; >> + ar->gpio->wifi_led.active_low = 1; >> + ar->gpio->wifi_led.gpio = ar->hw_params.led_pin; >> + ar->gpio->wifi_led.name = ar->gpio->label; >> + ar->gpio->wifi_led.default_state = LEDS_GPIO_DEFSTATE_KEEP; >> + >> + ath10k_add_led(ar, &ar->gpio->wifi_led); >> +#endif >> + } >> + ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to output */ >> + ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1); >> + } >> +err_no_led:; >> +#endif >> + >> + return 0; >> err_hif_stop: >> ath10k_hif_stop(ar); >> err_htt_rx_detach: > The guards make it harder to read. In this chunk, every component > should be defined acceptably even if the CONFIG flags aren't defined > because they're all under your control. Make sure they are, and then > you can drop the guards here. The only trick is to be sure that any > accessed struct members that are protected by guards are abstracted > and only accessed inside your guarded functions. Alternately, you > could encapsulate it into a function ath10k_core_gpios_start() and > just have the single call there. The additional benefit being that you > don't need the err_no_led: jump. true, but consider that required structures like gpio_chip (ar->gpio) and led_classdev arent defined in the kernel if these CONFIG options arent configured. so i may move it into a separate function ,but the guards will remain. thats not my fault. thats a fault of the kernel api. > The rest of this looks OK to me. okay. i already changed some code, based on your requests but will wait for reply of you and for other comments until i send out a new version > > - Steve > > -- > Steve deRosier > Cal-Sierra Consulting LLC > https://www.cal-sierra.com/ >
On Tue, Feb 20, 2018 at 10:06 AM, Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote: > Am 20.02.2018 um 17:52 schrieb Steve deRosier: >> >> >>> +static int ath10k_register_gpio_chip(struct ath10k *ar) >>> +{ >>> + struct ath10k_gpiocontrol *gpio; >>> + gpio = kzalloc(sizeof(struct ath10k_gpiocontrol), GFP_KERNEL); >>> + if (!gpio) { >>> + return -ENOMEM; >>> + } >>> + >>> + snprintf(gpio->label, sizeof(gpio->label), "ath10k-%s", >>> + wiphy_name(ar->hw->wiphy)); >>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0) >>> + gpio->gchip.parent = ar->dev; >>> +#else >>> + gpio->gchip.dev = ar->dev; >>> +#endif >>> + gpio->gchip.base = -1; /* determine base automatically */ >> >> I may be wrong, but I think that version guards are unwelcome here. >> The patch is supposed to go and apply specifically to the upstream >> version, which is clearly >= KERNEL_VERSION(4,5,0). I think you can >> drop this, only use the 'gpio->gchip.parent = ar->dev;' and call it >> good. This fixup should be picked up by Backports. I understand that >> you might need to keep it for your internal tree, but I don't think >> the maintainers want to see this. > > the original patch comes from my sources which are supposed to work with > compat-wireless für multiple kernels. but for sure i can change that :-) > I understand. I've got some code that looks similar. However, I don't think the practice is to allow it upstream, the practice now is to rely on Backports to know about these tranformations. Someone here will correct me if I'm wrong. Just prune it for going upstream. :-) >> >> >>> +/* remove GPIO chip */ >>> +static void ath10k_unregister_gpio_chip(struct ath10k *ar) >>> +{ >>> +#ifdef CONFIG_GPIOLIB >>> + struct ath10k_gpiocontrol *gpio = ar->gpio; >>> + if (gpio) { >>> + gpiochip_remove(&gpio->gchip); >>> + kfree(gpio); >>> + } >>> +#endif >>> +} >> >> Instead of entering/exiting the guards inside every function, I think >> the usually accepted way to handle this is to place a block in the >> that looks basically like: >> >> #ifdef CONFIG_GPIOLIB >> static void ath10k_unregister_gpio_chip(struct ath10k *ar) { >> struct ath10k_gpiocontrol *gpio = ar->gpio; >> if (gpio) { >> gpiochip_remove(&gpio->gchip); >> kfree(gpio); >> } >> } >> >> next function.... >> next function... >> >> #else >> static void ath10k_unregister_gpio_chip(struct ath10k *ar) { >> } >> next function.... >> next function... >> #endif >> >> If they weren't static, you'd do it in the .h file, with the >> declaration of the full function there and the definition in the .c. >> >> I may be nit-picking and your style is perfectly acceptable to the >> other maintainers. > > i'm not a fan of bloat code. and making funktions static which are just > called one will allow the compiler to optimize the code better > than making it non static. that makes no sense here for sure. its just a > question how you handle it. Agreed, I am also not a fan of bloat code. I wasn't suggesting you move it to the .h, just saying that if it were not static, that's how I'd suggest doing it. In this case, keep it static, keep it in the .c. And use the blocking I suggested. You will get the optimizer friendly result you're looking for, but make it more readable and more-inline with the dominant style for these things. >> >> >> >>> + >>> +static void ath10k_unregister_led(struct ath10k *ar) >>> +{ >>> +#if defined (CONFIG_GPIOLIB) && defined(CONFIG_LEDS_CLASS) >>> + if (ar->gpio) >>> + led_classdev_unregister(&ar->gpio->cdev); >>> +#endif >>> +} >>> + >>> + >>> int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode >>> mode, >>> const struct ath10k_fw_components *fw) >>> { >>> int status; >>> u32 val; >>> - >>> lockdep_assert_held(&ar->conf_mutex); >>> >> Whitespace change for no apparent reason. > > count it as typo > >> >>> @@ -2372,8 +2524,32 @@ int ath10k_core_start(struct ath10k *ar, enum >>> ath10k_firmware_mode mode, >>> if (status) >>> goto err_hif_stop; >>> >>> - return 0; >>> >>> +#ifdef CONFIG_GPIOLIB >>> + /* LED Code */ >>> + if (ar->hw_params.led_pin) { /* only attach if non zero since >>> some chipsets are unsupported yet */ >>> + if (!ar->gpio_attached) { >>> + status = ath10k_register_gpio_chip(ar); >>> + if (status) { >>> + goto err_no_led; >>> + } >>> +#ifdef CONFIG_LEDS_CLASS >>> + ar->gpio_attached = 1; >>> + ar->gpio->wifi_led.active_low = 1; >>> + ar->gpio->wifi_led.gpio = ar->hw_params.led_pin; >>> + ar->gpio->wifi_led.name = ar->gpio->label; >>> + ar->gpio->wifi_led.default_state = >>> LEDS_GPIO_DEFSTATE_KEEP; >>> + >>> + ath10k_add_led(ar, &ar->gpio->wifi_led); >>> +#endif >>> + } >>> + ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0, >>> WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to output */ >>> + ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1); >>> + } >>> +err_no_led:; >>> +#endif >>> + >>> + return 0; >>> err_hif_stop: >>> ath10k_hif_stop(ar); >>> err_htt_rx_detach: >> >> The guards make it harder to read. In this chunk, every component >> should be defined acceptably even if the CONFIG flags aren't defined >> because they're all under your control. Make sure they are, and then >> you can drop the guards here. The only trick is to be sure that any >> accessed struct members that are protected by guards are abstracted >> and only accessed inside your guarded functions. Alternately, you >> could encapsulate it into a function ath10k_core_gpios_start() and >> just have the single call there. The additional benefit being that you >> don't need the err_no_led: jump. > > true, but consider that required structures like gpio_chip (ar->gpio) and > led_classdev arent defined in the kernel if these CONFIG options arent > configured. so i may move it into a separate function ,but the guards will > remain. > thats not my fault. thats a fault of the kernel api. > Hm. I assumed the names were still valid, even if the functionality that uses it wasn't. Understood. In that case, I suggest moving the code to a separate static function that can be guarded. It keeps the mess out of the main startup function, makes it easier to read as it collapses to a single self-documented function name, and gets rid of the label and jump. >> The rest of this looks OK to me. > > okay. i already changed some code, based on your requests but will wait for > reply of you and for other comments until i send out a new version I appreciate the extra work you've put in. I hadn't expected my comments would've generated so much extra work; sorry about that. It's looking good. - Steve -- Steve deRosier Cal-Sierra Consulting LLC https://www.cal-sierra.com/
> Agreed, I am also not a fan of bloat code. I wasn't suggesting you > move it to the .h, just saying that if it were not static, that's how > I'd suggest doing it. > > In this case, keep it static, keep it in the .c. And use the blocking > I suggested. You will get the optimizer friendly result you're looking > for, but make it more readable and more-inline with the dominant style > for these things. thats what i did already in my sourcetree > >> true, but consider that required structures like gpio_chip (ar->gpio) and >> led_classdev arent defined in the kernel if these CONFIG options arent >> configured. so i may move it into a separate function ,but the guards will >> remain. >> thats not my fault. thats a fault of the kernel api. >> > Hm. I assumed the names were still valid, even if the functionality > that uses it wasn't. Understood. > > In that case, I suggest moving the code to a separate static function > that can be guarded. It keeps the mess out of the main startup > function, makes it easier to read as it collapses to a single > self-documented function name, and gets rid of the label and jump. yes but we have 2 guards. GPIOLIB and LED_CLASS so the mess remaines in the same way with no change. the same unreadable code just moved (personally i think its not that unreadable since its small) > >>> The rest of this looks OK to me. >> okay. i already changed some code, based on your requests but will wait for >> reply of you and for other comments until i send out a new version > I appreciate the extra work you've put in. I hadn't expected my > comments would've generated so much extra work; sorry about that. It's > looking good. i just want it upstream. i have alot of usefull patches in my backhand and its now getting hard to maintain the code with new ath10k patches. and at the end its not that much work as it seems. just a question of some seconds todo that changes. i can live with it. Sebastian
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index f3ec13b80b20..e50977f97b18 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -21,6 +21,10 @@ #include <linux/dmi.h> #include <linux/ctype.h> #include <asm/byteorder.h> +#include <linux/leds.h> +#include <linux/platform_device.h> +#include <linux/version.h> + #include "core.h" #include "mac.h" @@ -65,6 +69,8 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .id = QCA988X_HW_2_0_VERSION, .dev_id = QCA988X_2_0_DEVICE_ID, .name = "qca988x hw2.0", + .led_pin = 1, + .gpio_count = 24, .patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR, .uart_pin = 7, .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, @@ -94,6 +100,8 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .id = QCA988X_HW_2_0_VERSION, .dev_id = QCA988X_2_0_DEVICE_ID_UBNT, .name = "qca988x hw2.0 ubiquiti", + .led_pin = 1, + .gpio_count = 24, .patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR, .uart_pin = 7, .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, @@ -123,6 +131,8 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = { .id = QCA9887_HW_1_0_VERSION, .dev_id = QCA9887_1_0_DEVICE_ID, .name = "qca9887 hw1.0", + .led_pin = 1, + .gpio_count = 24, .patch_load_addr = QCA9887_HW_1_0_PATCH_LOAD_ADDR, .uart_pin = 7, .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_ALL, @@ -267,6 +277,8 @@ 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, + .gpio_count = 35, .patch_load_addr = QCA99X0_HW_2_0_PATCH_LOAD_ADDR, .uart_pin = 7, .otp_exe_param = 0x00000700, @@ -301,6 +313,8 @@ 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, + .gpio_count = 35, .patch_load_addr = QCA9984_HW_1_0_PATCH_LOAD_ADDR, .uart_pin = 7, .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH, @@ -340,6 +354,8 @@ 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, + .gpio_count = 35, .patch_load_addr = QCA9888_HW_2_0_PATCH_LOAD_ADDR, .uart_pin = 7, .cc_wraparound_type = ATH10K_HW_CC_WRAP_SHIFTED_EACH, @@ -2132,12 +2148,148 @@ static int ath10k_core_reset_rx_filter(struct ath10k *ar) return 0; } +#ifdef CONFIG_GPIOLIB + +static int ath10k_gpio_pin_cfg_input(struct gpio_chip *chip, unsigned offset) +{ + struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); + ath10k_wmi_gpio_config(gpio->ar, offset, 1, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to input */ + gpio->gpio_state_dir = 1; + return 0; +} + +/* gpio_chip handler : set GPIO to output */ +static int ath10k_gpio_pin_cfg_output(struct gpio_chip *chip, unsigned offset, + int value) +{ + struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); + + ath10k_wmi_gpio_config(gpio->ar, offset, 0, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to output */ + ath10k_wmi_gpio_output(gpio->ar, offset, value); + gpio->gpio_state_dir = 0; + gpio->gpio_state_pin = value; + return 0; +} + +/* gpio_chip handler : query GPIO direction (0=out, 1=in) */ +static int ath10k_gpio_pin_get_dir(struct gpio_chip *chip, unsigned offset) +{ + struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); + + return gpio->gpio_state_dir; +} + +/* gpio_chip handler : get GPIO pin value */ +static int ath10k_gpio_pin_get(struct gpio_chip *chip, unsigned offset) +{ + struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); + + return gpio->gpio_state_pin; +} + +/* gpio_chip handler : set GPIO pin to value */ +static void ath10k_gpio_pin_set(struct gpio_chip *chip, unsigned offset, + int value) +{ + struct ath10k_gpiocontrol *gpio = container_of(chip, struct ath10k_gpiocontrol, gchip); + + ath10k_wmi_gpio_output(gpio->ar, offset, value); + gpio->gpio_state_pin = value; +} + +/* register GPIO chip */ +static int ath10k_register_gpio_chip(struct ath10k *ar) +{ + struct ath10k_gpiocontrol *gpio; + gpio = kzalloc(sizeof(struct ath10k_gpiocontrol), GFP_KERNEL); + if (!gpio) { + return -ENOMEM; + } + + snprintf(gpio->label, sizeof(gpio->label), "ath10k-%s", + wiphy_name(ar->hw->wiphy)); +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0) + gpio->gchip.parent = ar->dev; +#else + gpio->gchip.dev = ar->dev; +#endif + gpio->gchip.base = -1; /* determine base automatically */ + gpio->gchip.ngpio = ar->hw_params.gpio_count; + gpio->gchip.label = gpio->label; + gpio->gchip.direction_input = ath10k_gpio_pin_cfg_input; + gpio->gchip.direction_output = ath10k_gpio_pin_cfg_output; + gpio->gchip.get_direction = ath10k_gpio_pin_get_dir; + gpio->gchip.get = ath10k_gpio_pin_get; + gpio->gchip.set = ath10k_gpio_pin_set; + + if (gpiochip_add(&gpio->gchip)) { + printk(KERN_ERR "Error while registering gpio chip\n"); + return -1; + } + gpio->gchip.owner = NULL; + ar->gpio = gpio; + gpio->ar = ar; + return 0; +} + +#ifdef CONFIG_LEDS_CLASS +static void ath10k_led_brightness(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + struct ath10k_gpiocontrol *gpio = container_of(led_cdev, struct ath10k_gpiocontrol, cdev); + struct gpio_led *led = &gpio->wifi_led; + if (gpio->ar->state == ATH10K_STATE_ON) { + gpio->gpio_state_pin = (brightness != LED_OFF) ^ led->active_low; + ath10k_wmi_gpio_output(gpio->ar, led->gpio, gpio->gpio_state_pin); + } +} + +static int ath10k_add_led(struct ath10k *ar, struct gpio_led *gpioled) +{ + int ret; + struct ath10k_gpiocontrol *gpio = ar->gpio; + if (!gpio) + return -1; + gpio->cdev.name = gpioled->name; + gpio->cdev.default_trigger = gpioled->default_trigger; + gpio->cdev.brightness_set = ath10k_led_brightness; + + ret = led_classdev_register(wiphy_dev(ar->hw->wiphy), &gpio->cdev); + if (ret < 0) + return ret; + + return 0; +} +#endif + +#endif + +/* remove GPIO chip */ +static void ath10k_unregister_gpio_chip(struct ath10k *ar) +{ +#ifdef CONFIG_GPIOLIB + struct ath10k_gpiocontrol *gpio = ar->gpio; + if (gpio) { + gpiochip_remove(&gpio->gchip); + kfree(gpio); + } +#endif +} + +static void ath10k_unregister_led(struct ath10k *ar) +{ +#if defined (CONFIG_GPIOLIB) && defined(CONFIG_LEDS_CLASS) + if (ar->gpio) + led_classdev_unregister(&ar->gpio->cdev); +#endif +} + + int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, const struct ath10k_fw_components *fw) { int status; u32 val; - lockdep_assert_held(&ar->conf_mutex); clear_bit(ATH10K_FLAG_CRASH_FLUSH, &ar->dev_flags); @@ -2372,8 +2524,32 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode, if (status) goto err_hif_stop; - return 0; +#ifdef CONFIG_GPIOLIB + /* LED Code */ + if (ar->hw_params.led_pin) { /* only attach if non zero since some chipsets are unsupported yet */ + if (!ar->gpio_attached) { + status = ath10k_register_gpio_chip(ar); + if (status) { + goto err_no_led; + } +#ifdef CONFIG_LEDS_CLASS + ar->gpio_attached = 1; + ar->gpio->wifi_led.active_low = 1; + ar->gpio->wifi_led.gpio = ar->hw_params.led_pin; + ar->gpio->wifi_led.name = ar->gpio->label; + ar->gpio->wifi_led.default_state = LEDS_GPIO_DEFSTATE_KEEP; + + ath10k_add_led(ar, &ar->gpio->wifi_led); +#endif + } + ath10k_wmi_gpio_config(ar, ar->hw_params.led_pin, 0, WMI_GPIO_PULL_NONE, WMI_GPIO_INTTYPE_DISABLE); /* configure to output */ + ath10k_wmi_gpio_output(ar, ar->hw_params.led_pin, 1); + } +err_no_led:; +#endif + + return 0; err_hif_stop: ath10k_hif_stop(ar); err_htt_rx_detach: @@ -2673,6 +2849,9 @@ void ath10k_core_unregister(struct ath10k *ar) ath10k_core_free_board_files(ar); ath10k_debug_unregister(ar); + + ath10k_unregister_led(ar); + ath10k_unregister_gpio_chip(ar); } EXPORT_SYMBOL(ath10k_core_unregister); diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index c624b96f8b84..1340904582b7 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -25,6 +25,8 @@ #include <linux/pci.h> #include <linux/uuid.h> #include <linux/time.h> +#include <linux/gpio.h> +#include <linux/leds.h> #include "htt.h" #include "htc.h" @@ -812,6 +814,21 @@ struct ath10k_per_peer_tx_stats { u32 reserved2; }; +struct ath10k_gpiocontrol { + struct ath10k *ar; + u32 gpio_set_num, gpio_num, gpio_input, gpio_pull_type, gpio_intr_mode, gpio_set; /* since we have no gpio read method, these are the state variables for debugfs. */ +#ifdef CONFIG_GPIOLIB + struct gpio_chip gchip; +#endif +#ifdef CONFIG_LEDS_CLASS + struct gpio_led wifi_led; + struct led_classdev cdev; +#endif + char label[48]; + u32 gpio_state_dir; /* same as for debugfs, but for gpiochip implementation */ + u32 gpio_state_pin; +}; + struct ath10k { struct ath_common ath_common; struct ieee80211_hw *hw; @@ -840,7 +857,8 @@ struct ath10k { u32 low_5ghz_chan; u32 high_5ghz_chan; bool ani_enabled; - + struct ath10k_gpiocontrol *gpio; + int gpio_attached; bool p2p; struct { diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index 1b9c092d210f..4ed3540669ec 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -1084,6 +1084,128 @@ static ssize_t ath10k_write_fw_dbglog(struct file *file, return ret; } + +static ssize_t ath10k_read_gpio_config(struct file *file, char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + struct ath10k_gpiocontrol *gpio = ar->gpio; + size_t len; + char buf[96]; + if (!gpio) + return 0; + + len = scnprintf(buf, sizeof(buf), "%u %u %u %u\n", gpio->gpio_num, gpio->gpio_input, gpio->gpio_pull_type, gpio->gpio_intr_mode); + + return simple_read_from_buffer(user_buf, count, ppos, buf, len); +} + + +static ssize_t ath10k_write_gpio_config(struct file *file, + const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + struct ath10k_gpiocontrol *gpio = ar->gpio; + int ret; + char buf[96]; + u32 gpio_num, input, pull_type, intr_mode; + if (!gpio) + return -EINVAL; + + simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count); + + /* make sure that buf is null terminated */ + buf[sizeof(buf) - 1] = 0; + + gpio->gpio_num = gpio_num; + gpio->gpio_input = input; + gpio->gpio_pull_type = pull_type; + gpio->gpio_intr_mode = intr_mode; + ret = sscanf(buf, "%u %u %u %u", &gpio_num, &input, &pull_type, &intr_mode); + + if (!ret) + return -EINVAL; + + + mutex_lock(&ar->conf_mutex); + + + ret = ath10k_wmi_gpio_config(ar, gpio_num, input, pull_type, intr_mode); + + if (ret) { + ath10k_warn(ar, "gpio_config cfg failed from debugfs: %d\n", ret); + goto exit; + } + ret = count; + +exit: + mutex_unlock(&ar->conf_mutex); + + return ret; +} + +static ssize_t ath10k_read_gpio_output(struct file *file, char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + struct ath10k_gpiocontrol *gpio = ar->gpio; + size_t len; + char buf[96]; + if (!gpio) + return 0; + + len = scnprintf(buf, sizeof(buf), "%u %u\n", gpio->gpio_num, gpio->gpio_set); + + return simple_read_from_buffer(user_buf, count, ppos, buf, len); +} + + +static ssize_t ath10k_write_gpio_output(struct file *file, + const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + struct ath10k_gpiocontrol *gpio = ar->gpio; + int ret; + char buf[96]; + u32 gpio_num, set; + if (!gpio) + return -EINVAL; + + simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf, count); + + /* make sure that buf is null terminated */ + buf[sizeof(buf) - 1] = 0; + + gpio->gpio_set_num = gpio_num; + gpio->gpio_set = set; + ret = sscanf(buf, "%u %u", &gpio_num, &set); + + if (!ret) + return -EINVAL; + + + mutex_lock(&ar->conf_mutex); + + + ret = ath10k_wmi_gpio_output(ar, gpio_num, set); + + if (ret) { + ath10k_warn(ar, "gpio_output cfg failed from debugfs: %d\n", ret); + goto exit; + } + ret = count; + +exit: + mutex_unlock(&ar->conf_mutex); + + return ret; +} + + + + /* TODO: Would be nice to always support ethtool stats, would need to * move the stats storage out of ath10k_debug, or always have ath10k_debug * struct available.. @@ -1252,6 +1374,24 @@ static const struct file_operations fops_fw_dbglog = { .llseek = default_llseek, }; + + +static const struct file_operations fops_gpio_output = { + .read = ath10k_read_gpio_output, + .write = ath10k_write_gpio_output, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + +static const struct file_operations fops_gpio_config = { + .read = ath10k_read_gpio_config, + .write = ath10k_write_gpio_config, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + static int ath10k_debug_cal_data_fetch(struct ath10k *ar) { u32 hi_addr; @@ -2259,6 +2399,12 @@ int ath10k_debug_register(struct ath10k *ar) debugfs_create_file("fw_dbglog", 0600, ar->debug.debugfs_phy, ar, &fops_fw_dbglog); + debugfs_create_file("gpio_output", 0600, ar->debug.debugfs_phy, ar, + &fops_gpio_output); + + debugfs_create_file("gpio_config", 0600, ar->debug.debugfs_phy, ar, + &fops_gpio_config); + debugfs_create_file("cal_data", 0400, ar->debug.debugfs_phy, ar, &fops_cal_data); diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h index 413b1b4321f7..af91d0202e45 100644 --- a/drivers/net/wireless/ath/ath10k/hw.h +++ b/drivers/net/wireless/ath/ath10k/hw.h @@ -495,6 +495,8 @@ struct ath10k_hw_params { const char *name; u32 patch_load_addr; int uart_pin; + int led_pin; /* 1 for peregrine, 17 for beeliner */ + int gpio_count; /* 24 for peregrine, 35 for beeliner */ u32 otp_exe_param; /* Type of hw cycle counter wraparound logic, for more info diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index ebb3f1b046f3..f1932cc9f153 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -4601,6 +4601,12 @@ static int ath10k_start(struct ieee80211_hw *hw) switch (ar->state) { case ATH10K_STATE_OFF: ar->state = ATH10K_STATE_ON; + + /* need to reset gpio state */ + if (ar->hw_params.led_pin) { + 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); + } break; case ATH10K_STATE_RESTARTING: ar->state = ATH10K_STATE_RESTARTED; diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h index 14093cfdc505..a38d6a8f7ec5 100644 --- a/drivers/net/wireless/ath/ath10k/wmi-ops.h +++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h @@ -197,8 +197,13 @@ struct wmi_ops { (struct ath10k *ar, enum wmi_bss_survey_req_type type); struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value); + + 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); static inline int @@ -957,6 +962,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_nowait(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_nowait(ar, skb, ar->wmi.cmd->gpio_output_cmdid); +} static inline int ath10k_wmi_dbglog_cfg(struct ath10k *ar, u64 module_enable, u32 log_level) @@ -1034,7 +1068,7 @@ ath10k_wmi_pdev_get_temperature(struct ath10k *ar) if (IS_ERR(skb)) return PTR_ERR(skb); - return ath10k_wmi_cmd_send(ar, skb, + return ath10k_wmi_cmd_send_nowait(ar, skb, ar->wmi.cmd->pdev_get_temperature_cmdid); } diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c index ae77a007ae07..2bfba63f1dcf 100644 --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c @@ -3250,6 +3250,69 @@ ath10k_wmi_tlv_op_gen_echo(struct ath10k *ar, u32 value) return skb; } +static struct sk_buff * +ath10k_wmi_tlv_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 wmi_tlv *tlv; + struct sk_buff *skb; + void *ptr; + size_t len; + + len = sizeof(*tlv) + sizeof(*cmd); + skb = ath10k_wmi_alloc_skb(ar, len); + if (!skb) + return ERR_PTR(-ENOMEM); + + ptr = (void *)skb->data; + tlv = ptr; + tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_GPIO_CONFIG_CMD); + tlv->len = __cpu_to_le16(sizeof(*cmd)); + + 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); + + ptr += sizeof(*tlv); + ptr += sizeof(*cmd); + + ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv 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_tlv_op_gen_gpio_output(struct ath10k *ar, u32 gpio_num, u32 set) +{ + struct wmi_gpio_output_cmd *cmd; + struct wmi_tlv *tlv; + struct sk_buff *skb; + void *ptr; + size_t len; + + len = sizeof(*tlv) + sizeof(*cmd); + skb = ath10k_wmi_alloc_skb(ar, len); + if (!skb) + return ERR_PTR(-ENOMEM); + + ptr = (void *)skb->data; + tlv = ptr; + tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_GPIO_OUTPUT_CMD); + tlv->len = __cpu_to_le16(sizeof(*cmd)); + + cmd = (struct wmi_gpio_output_cmd *)skb->data; + cmd->gpio_num = __cpu_to_le32(gpio_num); + cmd->set = __cpu_to_le32(set); + + ptr += sizeof(*tlv); + ptr += sizeof(*cmd); + + ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv gpio_output gpio_num 0x%08x set 0x%08x\n", gpio_num, set); + return skb; +} + + static struct sk_buff * ath10k_wmi_tlv_op_gen_vdev_spectral_conf(struct ath10k *ar, const struct wmi_vdev_spectral_conf_arg *arg) @@ -3727,6 +3790,8 @@ static const struct wmi_ops wmi_tlv_ops = { .fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill, .get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype, .gen_echo = ath10k_wmi_tlv_op_gen_echo, + .gen_gpio_config = ath10k_wmi_tlv_op_gen_gpio_config, + .gen_gpio_output = ath10k_wmi_tlv_op_gen_gpio_output, .gen_vdev_spectral_conf = ath10k_wmi_tlv_op_gen_vdev_spectral_conf, .gen_vdev_spectral_enable = ath10k_wmi_tlv_op_gen_vdev_spectral_enable, }; diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c index 58dc2189ba49..b56e5a673a8c 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.c +++ b/drivers/net/wireless/ath/ath10k/wmi.c @@ -6646,6 +6646,41 @@ 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) @@ -8153,6 +8188,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 */ @@ -8223,6 +8261,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 */ @@ -8294,6 +8334,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 */ }; @@ -8364,6 +8406,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 */ @@ -8439,6 +8483,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 c7b30ed9015d..dc180a86dc3b 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.h +++ b/drivers/net/wireless/ath/ath10k/wmi.h @@ -2906,6 +2906,42 @@ 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;