| Message ID | 1446640074-29585-1-git-send-email-jesse.sung@canonical.com |
|---|---|
| State | New |
| Headers | show |
+ Ricardo 2015-11-04 20:27 GMT+08:00 Wen-chien Jesse Sung <jesse.sung@canonical.com>: > BugLink: https://launchpad.net/bugs/1512997 > > For Edge Gateway 5000/5100 only. > > Add code for controlling WiFi LED via firmware, and turns the LED on > and off when the interface is up and down accordingly. > > Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> > Tested-by: Gavin Lin <gavin.lin@canonical.com> > Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> > --- > drivers/net/wireless/mwifiex/fw.h | 18 ++++++++++++++++++ > drivers/net/wireless/mwifiex/main.c | 4 ++++ > drivers/net/wireless/mwifiex/main.h | 6 ++++++ > drivers/net/wireless/mwifiex/pcie.c | 6 ++++++ > drivers/net/wireless/mwifiex/sta_cmd.c | 29 +++++++++++++++++++++++++++++ > drivers/net/wireless/mwifiex/sta_cmdresp.c | 2 ++ > drivers/net/wireless/mwifiex/sta_ioctl.c | 20 ++++++++++++++++++++ > 7 files changed, 85 insertions(+) > > diff --git a/drivers/net/wireless/mwifiex/fw.h b/drivers/net/wireless/mwifiex/fw.h > index fb5936e..2bc6b93 100644 > --- a/drivers/net/wireless/mwifiex/fw.h > +++ b/drivers/net/wireless/mwifiex/fw.h > @@ -173,6 +173,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER { > #define TLV_TYPE_SCAN_CHANNEL_GAP (PROPRIETARY_TLV_BASE_ID + 197) > #define TLV_TYPE_API_REV (PROPRIETARY_TLV_BASE_ID + 199) > #define TLV_TYPE_CHANNEL_STATS (PROPRIETARY_TLV_BASE_ID + 198) > +#define TLV_TYPE_LED_CONTROL (PROPRIETARY_TLV_BASE_ID + 205) > > #define MWIFIEX_TX_DATA_BUF_SIZE_2K 2048 > > @@ -313,6 +314,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER { > #define HostCmd_CMD_802_11_AD_HOC_JOIN 0x002c > #define HostCmd_CMD_802_11_AD_HOC_STOP 0x0040 > #define HostCmd_CMD_802_11_MAC_ADDRESS 0x004D > +#define HostCmd_CMD_802_11_LED_CONTROL 0X004E > #define HostCmd_CMD_802_11D_DOMAIN_INFO 0x005b > #define HostCmd_CMD_802_11_KEY_MATERIAL 0x005e > #define HostCmd_CMD_802_11_BG_SCAN_QUERY 0x006c > @@ -1018,6 +1020,16 @@ struct ieee_types_oper_mode_ntf { > u8 oper_mode; > } __packed; > > +struct mwifiex_led_param { > + __le16 mode; > + __le16 on; > +} __packed; > + > +struct mwifiex_ie_types_led_param { > + struct mwifiex_ie_types_header header; > + struct mwifiex_led_param led_cfg; > +} __packed; > + > struct host_cmd_ds_802_11_ad_hoc_start { > u8 ssid[IEEE80211_MAX_SSID_LEN]; > u8 bss_mode; > @@ -1126,6 +1138,11 @@ struct host_cmd_ds_802_11_hs_cfg_enh { > } params; > } __packed; > > +struct host_cmd_ds_802_11_led_control { > + __le16 action; > + __le16 num_led; > +} __packed; > + > enum SNMP_MIB_INDEX { > OP_RATE_SET_I = 1, > DTIM_PERIOD_I = 3, > @@ -1901,6 +1918,7 @@ struct host_cmd_ds_command { > struct host_cmd_11ac_vht_cfg vht_cfg; > struct host_cmd_ds_coalesce_cfg coalesce_cfg; > struct host_cmd_ds_tdls_oper tdls_oper; > + struct host_cmd_ds_802_11_led_control led_cfg; > } params; > } __packed; > > diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c > index d4d2223..ea379f7 100644 > --- a/drivers/net/wireless/mwifiex/main.c > +++ b/drivers/net/wireless/mwifiex/main.c > @@ -562,7 +562,10 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter) > static int > mwifiex_open(struct net_device *dev) > { > + struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev); > + > netif_tx_start_all_queues(dev); > + mwifiex_set_led(priv->adapter, MWIFIEX_LED_ON); > return 0; > } > > @@ -581,6 +584,7 @@ mwifiex_close(struct net_device *dev) > priv->scan_aborting = true; > } > > + mwifiex_set_led(priv->adapter, MWIFIEX_LED_OFF); > return 0; > } > > diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h > index e66993c..fc3a128 100644 > --- a/drivers/net/wireless/mwifiex/main.h > +++ b/drivers/net/wireless/mwifiex/main.h > @@ -106,6 +106,10 @@ enum { > > #define PKT_TYPE_MGMT 0xE5 > > +#define MWIFIEX_LED_ON 1 > +#define MWIFIEX_LED_OFF 0 > +#define MWIFIEX_LED_MAX 3 > + > /* > * Do not check for data_received for USB, as data_received > * is handled in mwifiex_usb_recv for USB > @@ -582,6 +586,7 @@ struct mwifiex_private { > struct idr ack_status_frames; > /* spin lock for ack status */ > spinlock_t ack_status_lock; > + bool is_edge_gateway; > }; > > enum mwifiex_ba_status { > @@ -1208,6 +1213,7 @@ int mwifiex_wait_queue_complete(struct mwifiex_adapter *adapter, > struct cmd_ctrl_node *cmd_queued); > int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss, > struct cfg80211_ssid *req_ssid); > +int mwifiex_set_led(struct mwifiex_adapter *adapter, int on); > int mwifiex_cancel_hs(struct mwifiex_private *priv, int cmd_type); > int mwifiex_enable_hs(struct mwifiex_adapter *adapter); > int mwifiex_disable_auto_ds(struct mwifiex_private *priv); > diff --git a/drivers/net/wireless/mwifiex/pcie.c b/drivers/net/wireless/mwifiex/pcie.c > index c3a20f9..50c503f 100644 > --- a/drivers/net/wireless/mwifiex/pcie.c > +++ b/drivers/net/wireless/mwifiex/pcie.c > @@ -17,6 +17,7 @@ > * this warranty disclaimer. > */ > > +#include <linux/dmi.h> > #include <linux/firmware.h> > > #include "decl.h" > @@ -187,6 +188,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev, > const struct pci_device_id *ent) > { > struct pcie_service_card *card; > + struct mwifiex_private *priv; > > pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n", > pdev->vendor, pdev->device, pdev->revision); > @@ -213,6 +215,10 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev, > return -1; > } > > + priv = mwifiex_get_priv(card->adapter, MWIFIEX_BSS_ROLE_STA); > + if (dmi_match(DMI_PRODUCT_NAME, "Edge Gateway 5000") || > + dmi_match(DMI_PRODUCT_NAME, "Edge Gateway 5100")) > + priv->is_edge_gateway = true; > return 0; > } > > diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/sta_cmd.c > index 1c2ca29..dc1aed6 100644 > --- a/drivers/net/wireless/mwifiex/sta_cmd.c > +++ b/drivers/net/wireless/mwifiex/sta_cmd.c > @@ -384,6 +384,31 @@ mwifiex_cmd_802_11_hs_cfg(struct mwifiex_private *priv, > return 0; > } > > +static int mwifiex_cmd_802_11_led_cfg(struct mwifiex_private *priv, > + struct host_cmd_ds_command *cmd, > + u16 cmd_action, > + struct mwifiex_led_param *ledcfg_param) > +{ > + struct host_cmd_ds_802_11_led_control *led_cfg = &cmd->params.led_cfg; > + struct mwifiex_ie_types_led_param *led_tlv; > + u8 *pos; > + > + cmd->command = cpu_to_le16(HostCmd_CMD_802_11_LED_CONTROL); > + cmd->size = cpu_to_le16(S_DS_GEN); > + le16_add_cpu(&cmd->size, sizeof(struct host_cmd_ds_802_11_led_control)); > + > + led_cfg->action = cpu_to_le16(cmd_action); > + led_cfg->num_led = cpu_to_le16(MWIFIEX_LED_MAX); > + > + pos = (u8 *)led_cfg + sizeof(struct host_cmd_ds_802_11_led_control); > + led_tlv = (void *)pos; > + led_tlv->header.type = cpu_to_le16(TLV_TYPE_LED_CONTROL); > + led_tlv->header.len = cpu_to_le16(sizeof(struct mwifiex_led_param)); > + memcpy(&led_tlv->led_cfg, ledcfg_param, sizeof(struct mwifiex_led_param)); > + le16_add_cpu(&cmd->size, sizeof(struct mwifiex_ie_types_led_param)); > + return 0; > +} > + > /* > * This function prepares command to set/get MAC address. > * > @@ -1717,6 +1742,10 @@ int mwifiex_sta_prepare_cmd(struct mwifiex_private *priv, uint16_t cmd_no, > ret = mwifiex_cmd_802_11_hs_cfg(priv, cmd_ptr, cmd_action, > (struct mwifiex_hs_config_param *) data_buf); > break; > + case HostCmd_CMD_802_11_LED_CONTROL: > + ret = mwifiex_cmd_802_11_led_cfg(priv, cmd_ptr, cmd_action, > + data_buf); > + break; > case HostCmd_CMD_802_11_SCAN: > ret = mwifiex_cmd_802_11_scan(cmd_ptr, data_buf); > break; > diff --git a/drivers/net/wireless/mwifiex/sta_cmdresp.c b/drivers/net/wireless/mwifiex/sta_cmdresp.c > index b65e101..06583cf 100644 > --- a/drivers/net/wireless/mwifiex/sta_cmdresp.c > +++ b/drivers/net/wireless/mwifiex/sta_cmdresp.c > @@ -1117,6 +1117,8 @@ int mwifiex_process_sta_cmdresp(struct mwifiex_private *priv, u16 cmdresp_no, > case HostCmd_CMD_TDLS_OPER: > ret = mwifiex_ret_tdls_oper(priv, resp); > break; > + case HostCmd_CMD_802_11_LED_CONTROL: > + break; > default: > dev_err(adapter->dev, "CMD_RESP: unknown cmd response %#x\n", > resp->command); > diff --git a/drivers/net/wireless/mwifiex/sta_ioctl.c b/drivers/net/wireless/mwifiex/sta_ioctl.c > index 1626868..01041af 100644 > --- a/drivers/net/wireless/mwifiex/sta_ioctl.c > +++ b/drivers/net/wireless/mwifiex/sta_ioctl.c > @@ -17,6 +17,8 @@ > * this warranty disclaimer. > */ > > +#include <linux/dmi.h> > + > #include "decl.h" > #include "ioctl.h" > #include "util.h" > @@ -525,6 +527,24 @@ int mwifiex_enable_hs(struct mwifiex_adapter *adapter) > } > EXPORT_SYMBOL_GPL(mwifiex_enable_hs); > > +int mwifiex_set_led(struct mwifiex_adapter *adapter, int on) > +{ > + struct mwifiex_private *priv; > + struct mwifiex_led_param ledcfg; > + > + priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_STA); > + if (!priv->is_edge_gateway) > + return -ENODEV; > + > + memset(&ledcfg, 0, sizeof(struct mwifiex_led_param)); > + ledcfg.on = cpu_to_le16(on); > + > + return mwifiex_send_cmd(priv, > + HostCmd_CMD_802_11_LED_CONTROL, > + HostCmd_ACT_GEN_SET, 0, > + &ledcfg, true); > +} > + > /* > * IOCTL request handler to get BSS information. > * > -- > 2.5.0 >
The related bug report is lacking any reference on proper testing done. Neither with the HW in question nor with similar HW which might see regressions. Also is there any effort being done to push this upstream? Would this not be needed as well in Wily and onwards? -Stefan
On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: > BugLink: https://launchpad.net/bugs/1512997 > > For Edge Gateway 5000/5100 only. > > Add code for controlling WiFi LED via firmware, and turns the LED on > and off when the interface is up and down accordingly. > > Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> > Tested-by: Gavin Lin <gavin.lin@canonical.com> > Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> mac80211 already has led-trigger support for this sort of thing. Is there some reason that can't be utilized instead of hacking up the driver like this?
2015-11-05 0:21 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: > On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: >> BugLink: https://launchpad.net/bugs/1512997 >> >> For Edge Gateway 5000/5100 only. >> >> Add code for controlling WiFi LED via firmware, and turns the LED on >> and off when the interface is up and down accordingly. >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> >> Tested-by: Gavin Lin <gavin.lin@canonical.com> >> Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> > > mac80211 already has led-trigger support for this sort of thing. Is > there some reason that can't be utilized instead of hacking up the > driver like this? Humm.. This patch is based on a solution provided by Marvell. If led_trigger is preferred, I'll work on it. Thanks, Jesse
On Thu, Nov 05, 2015 at 05:45:07PM +0800, Jesse Sung wrote: > 2015-11-05 0:21 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: > > On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: > >> BugLink: https://launchpad.net/bugs/1512997 > >> > >> For Edge Gateway 5000/5100 only. > >> > >> Add code for controlling WiFi LED via firmware, and turns the LED on > >> and off when the interface is up and down accordingly. > >> > >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> > >> Tested-by: Gavin Lin <gavin.lin@canonical.com> > >> Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> > > > > mac80211 already has led-trigger support for this sort of thing. Is > > there some reason that can't be utilized instead of hacking up the > > driver like this? > > Humm.. This patch is based on a solution provided by Marvell. > If led_trigger is preferred, I'll work on it. I'm not sure whether or not the LED integration in mac80211 can provide what you want or not, I just wanted to know if you had considered it. If it doesn't then I don't have any big objections to the mwifiex changes, though I'm not crazy about the DMI stuff. The btusb changes are more troublesome though because that's a very generic driver, not hardware-specific like mwifiex. Seth
2015-11-05 22:45 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: > On Thu, Nov 05, 2015 at 05:45:07PM +0800, Jesse Sung wrote: >> 2015-11-05 0:21 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: >> > On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: >> >> BugLink: https://launchpad.net/bugs/1512997 >> >> >> >> For Edge Gateway 5000/5100 only. >> >> >> >> Add code for controlling WiFi LED via firmware, and turns the LED on >> >> and off when the interface is up and down accordingly. >> >> >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> >> >> Tested-by: Gavin Lin <gavin.lin@canonical.com> >> >> Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> >> > >> > mac80211 already has led-trigger support for this sort of thing. Is >> > there some reason that can't be utilized instead of hacking up the >> > driver like this? >> >> Humm.. This patch is based on a solution provided by Marvell. >> If led_trigger is preferred, I'll work on it. > > I'm not sure whether or not the LED integration in mac80211 can provide > what you want or not, I just wanted to know if you had considered it. > If it doesn't then I don't have any big objections to the mwifiex > changes, though I'm not crazy about the DMI stuff. I'll look into it and see it can be done with led_trigger. > The btusb changes are more troublesome though because that's a very > generic driver, not hardware-specific like mwifiex. All of the changes in btusb would not have effect if it's not run on Edge Gateway. is_edge_gateway in btusb_data would only be set when the dmi_match() returns true, and new code executes only when is_edge_gateway is true. > > Seth Thanks, Jesse
On Fri, Nov 06, 2015 at 12:47:54AM +0800, Jesse Sung wrote: > 2015-11-05 22:45 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: > > On Thu, Nov 05, 2015 at 05:45:07PM +0800, Jesse Sung wrote: > >> 2015-11-05 0:21 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: > >> > On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: > >> >> BugLink: https://launchpad.net/bugs/1512997 > >> >> > >> >> For Edge Gateway 5000/5100 only. > >> >> > >> >> Add code for controlling WiFi LED via firmware, and turns the LED on > >> >> and off when the interface is up and down accordingly. > >> >> > >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> > >> >> Tested-by: Gavin Lin <gavin.lin@canonical.com> > >> >> Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> > >> > > >> > mac80211 already has led-trigger support for this sort of thing. Is > >> > there some reason that can't be utilized instead of hacking up the > >> > driver like this? > >> > >> Humm.. This patch is based on a solution provided by Marvell. > >> If led_trigger is preferred, I'll work on it. > > > > I'm not sure whether or not the LED integration in mac80211 can provide > > what you want or not, I just wanted to know if you had considered it. > > If it doesn't then I don't have any big objections to the mwifiex > > changes, though I'm not crazy about the DMI stuff. > > I'll look into it and see it can be done with led_trigger. > > > The btusb changes are more troublesome though because that's a very > > generic driver, not hardware-specific like mwifiex. > > All of the changes in btusb would not have effect if it's not run on > Edge Gateway. > is_edge_gateway in btusb_data would only be set when the dmi_match() returns > true, and new code executes only when is_edge_gateway is true. Fine, but if we were to start doing this for multiple hardware problems then it quickly gets very messy, and it also creates more conflicts for backports. If the LED framework turns out to be appropriate for your needs then it could alleviate both of these problems. Seth
Hi Seth, For btusb, what if we move it to ubuntu/ and modify it only to match this specific VID/PID? If you think this will be good, just let us know. Jesse will work on a V2 patch in this way. For the wifi led, the patch is from the Marvell BSP directly. IMHO we better leave it as what it is. There may be chances that we may have to merge/cherry-pick some patches if Marvell releases a new version of BSP. Some background: the two patches are not for the coming SRU. They are for the re-spin of the last SRU. On Fri, Nov 6, 2015 at 1:13 AM, Seth Forshee <seth.forshee@canonical.com> wrote: > On Fri, Nov 06, 2015 at 12:47:54AM +0800, Jesse Sung wrote: >> 2015-11-05 22:45 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: >> > On Thu, Nov 05, 2015 at 05:45:07PM +0800, Jesse Sung wrote: >> >> 2015-11-05 0:21 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: >> >> > On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: >> >> >> BugLink: https://launchpad.net/bugs/1512997 >> >> >> >> >> >> For Edge Gateway 5000/5100 only. >> >> >> >> >> >> Add code for controlling WiFi LED via firmware, and turns the LED on >> >> >> and off when the interface is up and down accordingly. >> >> >> >> >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> >> >> >> Tested-by: Gavin Lin <gavin.lin@canonical.com> >> >> >> Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> >> >> > >> >> > mac80211 already has led-trigger support for this sort of thing. Is >> >> > there some reason that can't be utilized instead of hacking up the >> >> > driver like this? >> >> >> >> Humm.. This patch is based on a solution provided by Marvell. >> >> If led_trigger is preferred, I'll work on it. >> > >> > I'm not sure whether or not the LED integration in mac80211 can provide >> > what you want or not, I just wanted to know if you had considered it. >> > If it doesn't then I don't have any big objections to the mwifiex >> > changes, though I'm not crazy about the DMI stuff. >> >> I'll look into it and see it can be done with led_trigger. >> >> > The btusb changes are more troublesome though because that's a very >> > generic driver, not hardware-specific like mwifiex. >> >> All of the changes in btusb would not have effect if it's not run on >> Edge Gateway. >> is_edge_gateway in btusb_data would only be set when the dmi_match() returns >> true, and new code executes only when is_edge_gateway is true. > > Fine, but if we were to start doing this for multiple hardware problems > then it quickly gets very messy, and it also creates more conflicts for > backports. If the LED framework turns out to be appropriate for your > needs then it could alleviate both of these problems. > > Seth
2015-11-06 13:37 GMT+08:00 Keng-Yu Lin <kengyu@canonical.com>: > Hi Seth, > For btusb, what if we move it to ubuntu/ and modify it only to match > this specific VID/PID? If you think this will be good, just let us > know. Jesse will work on a V2 patch in this way. > > For the wifi led, the patch is from the Marvell BSP directly. IMHO > we better leave it as what it is. There may be chances that we may > have to merge/cherry-pick some patches if Marvell releases a new > version of BSP. > > Some background: the two patches are not for the coming SRU. They > are for the re-spin of the last SRU. > > > On Fri, Nov 6, 2015 at 1:13 AM, Seth Forshee <seth.forshee@canonical.com> wrote: >> On Fri, Nov 06, 2015 at 12:47:54AM +0800, Jesse Sung wrote: >>> 2015-11-05 22:45 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: >>> > On Thu, Nov 05, 2015 at 05:45:07PM +0800, Jesse Sung wrote: >>> >> 2015-11-05 0:21 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: >>> >> > On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: >>> >> >> BugLink: https://launchpad.net/bugs/1512997 >>> >> >> >>> >> >> For Edge Gateway 5000/5100 only. >>> >> >> >>> >> >> Add code for controlling WiFi LED via firmware, and turns the LED on >>> >> >> and off when the interface is up and down accordingly. >>> >> >> >>> >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> >>> >> >> Tested-by: Gavin Lin <gavin.lin@canonical.com> >>> >> >> Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> >>> >> > >>> >> > mac80211 already has led-trigger support for this sort of thing. Is >>> >> > there some reason that can't be utilized instead of hacking up the >>> >> > driver like this? >>> >> >>> >> Humm.. This patch is based on a solution provided by Marvell. >>> >> If led_trigger is preferred, I'll work on it. >>> > >>> > I'm not sure whether or not the LED integration in mac80211 can provide >>> > what you want or not, I just wanted to know if you had considered it. >>> > If it doesn't then I don't have any big objections to the mwifiex >>> > changes, though I'm not crazy about the DMI stuff. Since mwifiex is using FullMAC instead of mac80211, we can't use the led-trigger support in mac80211 in this case. >>> >>> I'll look into it and see it can be done with led_trigger. >>> >>> > The btusb changes are more troublesome though because that's a very >>> > generic driver, not hardware-specific like mwifiex. >>> >>> All of the changes in btusb would not have effect if it's not run on >>> Edge Gateway. >>> is_edge_gateway in btusb_data would only be set when the dmi_match() returns >>> true, and new code executes only when is_edge_gateway is true. >> >> Fine, but if we were to start doing this for multiple hardware problems >> then it quickly gets very messy, and it also creates more conflicts for >> backports. If the LED framework turns out to be appropriate for your >> needs then it could alleviate both of these problems. Since bluetooth stack doesn't do any led-trigger calls, we need to call functions to toggle LED in btusb anyway, no matter it is done directly or via LED framworks. To prevent conflicts for later backports, we may have a duplicate btusb in ubuntu/ matching VID/PID only for this module, just like what Kengyu suggested. But this also means that if there's any fix for btusb, it may need to be applied to both files, which introduces a different problem of maintenance. >> >> Seth Thanks, Jesse
On Wed, Nov 25, 2015 at 06:54:06PM +0800, Jesse Sung wrote: > 2015-11-06 13:37 GMT+08:00 Keng-Yu Lin <kengyu@canonical.com>: > > Hi Seth, > > For btusb, what if we move it to ubuntu/ and modify it only to match > > this specific VID/PID? If you think this will be good, just let us > > know. Jesse will work on a V2 patch in this way. > > > > For the wifi led, the patch is from the Marvell BSP directly. IMHO > > we better leave it as what it is. There may be chances that we may > > have to merge/cherry-pick some patches if Marvell releases a new > > version of BSP. > > > > Some background: the two patches are not for the coming SRU. They > > are for the re-spin of the last SRU. > > > > > > On Fri, Nov 6, 2015 at 1:13 AM, Seth Forshee <seth.forshee@canonical.com> wrote: > >> On Fri, Nov 06, 2015 at 12:47:54AM +0800, Jesse Sung wrote: > >>> 2015-11-05 22:45 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: > >>> > On Thu, Nov 05, 2015 at 05:45:07PM +0800, Jesse Sung wrote: > >>> >> 2015-11-05 0:21 GMT+08:00 Seth Forshee <seth.forshee@canonical.com>: > >>> >> > On Wed, Nov 04, 2015 at 08:27:53PM +0800, Wen-chien Jesse Sung wrote: > >>> >> >> BugLink: https://launchpad.net/bugs/1512997 > >>> >> >> > >>> >> >> For Edge Gateway 5000/5100 only. > >>> >> >> > >>> >> >> Add code for controlling WiFi LED via firmware, and turns the LED on > >>> >> >> and off when the interface is up and down accordingly. > >>> >> >> > >>> >> >> Signed-off-by: Wen-chien Jesse Sung <jesse.sung@canonical.com> > >>> >> >> Tested-by: Gavin Lin <gavin.lin@canonical.com> > >>> >> >> Reviewed-by: Keng-Yu Lin <kengyu@canonical.com> > >>> >> > > >>> >> > mac80211 already has led-trigger support for this sort of thing. Is > >>> >> > there some reason that can't be utilized instead of hacking up the > >>> >> > driver like this? > >>> >> > >>> >> Humm.. This patch is based on a solution provided by Marvell. > >>> >> If led_trigger is preferred, I'll work on it. > >>> > > >>> > I'm not sure whether or not the LED integration in mac80211 can provide > >>> > what you want or not, I just wanted to know if you had considered it. > >>> > If it doesn't then I don't have any big objections to the mwifiex > >>> > changes, though I'm not crazy about the DMI stuff. > > Since mwifiex is using FullMAC instead of mac80211, we can't use the led-trigger > support in mac80211 in this case. So it is, my mistake. > >>> > >>> I'll look into it and see it can be done with led_trigger. > >>> > >>> > The btusb changes are more troublesome though because that's a very > >>> > generic driver, not hardware-specific like mwifiex. > >>> > >>> All of the changes in btusb would not have effect if it's not run on > >>> Edge Gateway. > >>> is_edge_gateway in btusb_data would only be set when the dmi_match() returns > >>> true, and new code executes only when is_edge_gateway is true. > >> > >> Fine, but if we were to start doing this for multiple hardware problems > >> then it quickly gets very messy, and it also creates more conflicts for > >> backports. If the LED framework turns out to be appropriate for your > >> needs then it could alleviate both of these problems. > > Since bluetooth stack doesn't do any led-trigger calls, we need to call > functions to toggle LED in btusb anyway, no matter it is done directly or > via LED framworks. > > To prevent conflicts for later backports, we may have a duplicate btusb > in ubuntu/ matching VID/PID only for this module, just like what Kengyu > suggested. But this also means that if there's any fix for btusb, it may need > to be applied to both files, which introduces a different problem of > maintenance. To me that sounds like more maintenance, since you still have to resolve the conflicts but also need to apply the backports to the standard btusb driver. However it does make it it so that any mistakes in resolving those conflicts won't cause regressions for other hardware. The stable kernel team will end up bearing the brunt of the maintenance burden though, so their opinions on this should count more than mine. Seth
diff --git a/drivers/net/wireless/mwifiex/fw.h b/drivers/net/wireless/mwifiex/fw.h index fb5936e..2bc6b93 100644 --- a/drivers/net/wireless/mwifiex/fw.h +++ b/drivers/net/wireless/mwifiex/fw.h @@ -173,6 +173,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER { #define TLV_TYPE_SCAN_CHANNEL_GAP (PROPRIETARY_TLV_BASE_ID + 197) #define TLV_TYPE_API_REV (PROPRIETARY_TLV_BASE_ID + 199) #define TLV_TYPE_CHANNEL_STATS (PROPRIETARY_TLV_BASE_ID + 198) +#define TLV_TYPE_LED_CONTROL (PROPRIETARY_TLV_BASE_ID + 205) #define MWIFIEX_TX_DATA_BUF_SIZE_2K 2048 @@ -313,6 +314,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER { #define HostCmd_CMD_802_11_AD_HOC_JOIN 0x002c #define HostCmd_CMD_802_11_AD_HOC_STOP 0x0040 #define HostCmd_CMD_802_11_MAC_ADDRESS 0x004D +#define HostCmd_CMD_802_11_LED_CONTROL 0X004E #define HostCmd_CMD_802_11D_DOMAIN_INFO 0x005b #define HostCmd_CMD_802_11_KEY_MATERIAL 0x005e #define HostCmd_CMD_802_11_BG_SCAN_QUERY 0x006c @@ -1018,6 +1020,16 @@ struct ieee_types_oper_mode_ntf { u8 oper_mode; } __packed; +struct mwifiex_led_param { + __le16 mode; + __le16 on; +} __packed; + +struct mwifiex_ie_types_led_param { + struct mwifiex_ie_types_header header; + struct mwifiex_led_param led_cfg; +} __packed; + struct host_cmd_ds_802_11_ad_hoc_start { u8 ssid[IEEE80211_MAX_SSID_LEN]; u8 bss_mode; @@ -1126,6 +1138,11 @@ struct host_cmd_ds_802_11_hs_cfg_enh { } params; } __packed; +struct host_cmd_ds_802_11_led_control { + __le16 action; + __le16 num_led; +} __packed; + enum SNMP_MIB_INDEX { OP_RATE_SET_I = 1, DTIM_PERIOD_I = 3, @@ -1901,6 +1918,7 @@ struct host_cmd_ds_command { struct host_cmd_11ac_vht_cfg vht_cfg; struct host_cmd_ds_coalesce_cfg coalesce_cfg; struct host_cmd_ds_tdls_oper tdls_oper; + struct host_cmd_ds_802_11_led_control led_cfg; } params; } __packed; diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c index d4d2223..ea379f7 100644 --- a/drivers/net/wireless/mwifiex/main.c +++ b/drivers/net/wireless/mwifiex/main.c @@ -562,7 +562,10 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter) static int mwifiex_open(struct net_device *dev) { + struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev); + netif_tx_start_all_queues(dev); + mwifiex_set_led(priv->adapter, MWIFIEX_LED_ON); return 0; } @@ -581,6 +584,7 @@ mwifiex_close(struct net_device *dev) priv->scan_aborting = true; } + mwifiex_set_led(priv->adapter, MWIFIEX_LED_OFF); return 0; } diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h index e66993c..fc3a128 100644 --- a/drivers/net/wireless/mwifiex/main.h +++ b/drivers/net/wireless/mwifiex/main.h @@ -106,6 +106,10 @@ enum { #define PKT_TYPE_MGMT 0xE5 +#define MWIFIEX_LED_ON 1 +#define MWIFIEX_LED_OFF 0 +#define MWIFIEX_LED_MAX 3 + /* * Do not check for data_received for USB, as data_received * is handled in mwifiex_usb_recv for USB @@ -582,6 +586,7 @@ struct mwifiex_private { struct idr ack_status_frames; /* spin lock for ack status */ spinlock_t ack_status_lock; + bool is_edge_gateway; }; enum mwifiex_ba_status { @@ -1208,6 +1213,7 @@ int mwifiex_wait_queue_complete(struct mwifiex_adapter *adapter, struct cmd_ctrl_node *cmd_queued); int mwifiex_bss_start(struct mwifiex_private *priv, struct cfg80211_bss *bss, struct cfg80211_ssid *req_ssid); +int mwifiex_set_led(struct mwifiex_adapter *adapter, int on); int mwifiex_cancel_hs(struct mwifiex_private *priv, int cmd_type); int mwifiex_enable_hs(struct mwifiex_adapter *adapter); int mwifiex_disable_auto_ds(struct mwifiex_private *priv); diff --git a/drivers/net/wireless/mwifiex/pcie.c b/drivers/net/wireless/mwifiex/pcie.c index c3a20f9..50c503f 100644 --- a/drivers/net/wireless/mwifiex/pcie.c +++ b/drivers/net/wireless/mwifiex/pcie.c @@ -17,6 +17,7 @@ * this warranty disclaimer. */ +#include <linux/dmi.h> #include <linux/firmware.h> #include "decl.h" @@ -187,6 +188,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *ent) { struct pcie_service_card *card; + struct mwifiex_private *priv; pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n", pdev->vendor, pdev->device, pdev->revision); @@ -213,6 +215,10 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev, return -1; } + priv = mwifiex_get_priv(card->adapter, MWIFIEX_BSS_ROLE_STA); + if (dmi_match(DMI_PRODUCT_NAME, "Edge Gateway 5000") || + dmi_match(DMI_PRODUCT_NAME, "Edge Gateway 5100")) + priv->is_edge_gateway = true; return 0; } diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/sta_cmd.c index 1c2ca29..dc1aed6 100644 --- a/drivers/net/wireless/mwifiex/sta_cmd.c +++ b/drivers/net/wireless/mwifiex/sta_cmd.c @@ -384,6 +384,31 @@ mwifiex_cmd_802_11_hs_cfg(struct mwifiex_private *priv, return 0; } +static int mwifiex_cmd_802_11_led_cfg(struct mwifiex_private *priv, + struct host_cmd_ds_command *cmd, + u16 cmd_action, + struct mwifiex_led_param *ledcfg_param) +{ + struct host_cmd_ds_802_11_led_control *led_cfg = &cmd->params.led_cfg; + struct mwifiex_ie_types_led_param *led_tlv; + u8 *pos; + + cmd->command = cpu_to_le16(HostCmd_CMD_802_11_LED_CONTROL); + cmd->size = cpu_to_le16(S_DS_GEN); + le16_add_cpu(&cmd->size, sizeof(struct host_cmd_ds_802_11_led_control)); + + led_cfg->action = cpu_to_le16(cmd_action); + led_cfg->num_led = cpu_to_le16(MWIFIEX_LED_MAX); + + pos = (u8 *)led_cfg + sizeof(struct host_cmd_ds_802_11_led_control); + led_tlv = (void *)pos; + led_tlv->header.type = cpu_to_le16(TLV_TYPE_LED_CONTROL); + led_tlv->header.len = cpu_to_le16(sizeof(struct mwifiex_led_param)); + memcpy(&led_tlv->led_cfg, ledcfg_param, sizeof(struct mwifiex_led_param)); + le16_add_cpu(&cmd->size, sizeof(struct mwifiex_ie_types_led_param)); + return 0; +} + /* * This function prepares command to set/get MAC address. * @@ -1717,6 +1742,10 @@ int mwifiex_sta_prepare_cmd(struct mwifiex_private *priv, uint16_t cmd_no, ret = mwifiex_cmd_802_11_hs_cfg(priv, cmd_ptr, cmd_action, (struct mwifiex_hs_config_param *) data_buf); break; + case HostCmd_CMD_802_11_LED_CONTROL: + ret = mwifiex_cmd_802_11_led_cfg(priv, cmd_ptr, cmd_action, + data_buf); + break; case HostCmd_CMD_802_11_SCAN: ret = mwifiex_cmd_802_11_scan(cmd_ptr, data_buf); break; diff --git a/drivers/net/wireless/mwifiex/sta_cmdresp.c b/drivers/net/wireless/mwifiex/sta_cmdresp.c index b65e101..06583cf 100644 --- a/drivers/net/wireless/mwifiex/sta_cmdresp.c +++ b/drivers/net/wireless/mwifiex/sta_cmdresp.c @@ -1117,6 +1117,8 @@ int mwifiex_process_sta_cmdresp(struct mwifiex_private *priv, u16 cmdresp_no, case HostCmd_CMD_TDLS_OPER: ret = mwifiex_ret_tdls_oper(priv, resp); break; + case HostCmd_CMD_802_11_LED_CONTROL: + break; default: dev_err(adapter->dev, "CMD_RESP: unknown cmd response %#x\n", resp->command); diff --git a/drivers/net/wireless/mwifiex/sta_ioctl.c b/drivers/net/wireless/mwifiex/sta_ioctl.c index 1626868..01041af 100644 --- a/drivers/net/wireless/mwifiex/sta_ioctl.c +++ b/drivers/net/wireless/mwifiex/sta_ioctl.c @@ -17,6 +17,8 @@ * this warranty disclaimer. */ +#include <linux/dmi.h> + #include "decl.h" #include "ioctl.h" #include "util.h" @@ -525,6 +527,24 @@ int mwifiex_enable_hs(struct mwifiex_adapter *adapter) } EXPORT_SYMBOL_GPL(mwifiex_enable_hs); +int mwifiex_set_led(struct mwifiex_adapter *adapter, int on) +{ + struct mwifiex_private *priv; + struct mwifiex_led_param ledcfg; + + priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_STA); + if (!priv->is_edge_gateway) + return -ENODEV; + + memset(&ledcfg, 0, sizeof(struct mwifiex_led_param)); + ledcfg.on = cpu_to_le16(on); + + return mwifiex_send_cmd(priv, + HostCmd_CMD_802_11_LED_CONTROL, + HostCmd_ACT_GEN_SET, 0, + &ledcfg, true); +} + /* * IOCTL request handler to get BSS information. *