* [PATCH v3 09/11] mwifiex: sdio: don't check for NULL sdio_func
From: Amitkumar Karwar @ 2016-11-11 13:10 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris, Amitkumar Karwar
In-Reply-To: <1478869818-4340-1-git-send-email-akarwar@marvell.com>
From: Brian Norris <briannorris@chromium.org>
sdio_func is retrieved via container_of() and should never be NULL.
Checking for NULL just makes the logic more confusing than necessary.
Stop doing that.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v3: Same as v1 and v2
---
drivers/net/wireless/marvell/mwifiex/sdio.c | 40 +++++++++++------------------
1 file changed, 15 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 4c4b012..bee7326 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -190,15 +190,10 @@ static int mwifiex_sdio_resume(struct device *dev)
struct mwifiex_adapter *adapter;
mmc_pm_flag_t pm_flag = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- card = sdio_get_drvdata(func);
- if (!card || !card->adapter) {
- pr_err("resume: invalid card or adapter\n");
- return 0;
- }
- } else {
- pr_err("resume: sdio_func is not specified\n");
+ pm_flag = sdio_get_host_pm_caps(func);
+ card = sdio_get_drvdata(func);
+ if (!card || !card->adapter) {
+ dev_err(dev, "resume: invalid card or adapter\n");
return 0;
}
@@ -274,23 +269,18 @@ static int mwifiex_sdio_suspend(struct device *dev)
mmc_pm_flag_t pm_flag = 0;
int ret = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
- sdio_func_id(func), pm_flag);
- if (!(pm_flag & MMC_PM_KEEP_POWER)) {
- pr_err("%s: cannot remain alive while host is"
- " suspended\n", sdio_func_id(func));
- return -ENOSYS;
- }
+ pm_flag = sdio_get_host_pm_caps(func);
+ pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
+ sdio_func_id(func), pm_flag);
+ if (!(pm_flag & MMC_PM_KEEP_POWER)) {
+ dev_err(dev, "%s: cannot remain alive while host is"
+ " suspended\n", sdio_func_id(func));
+ return -ENOSYS;
+ }
- card = sdio_get_drvdata(func);
- if (!card) {
- dev_err(dev, "suspend: invalid card\n");
- return 0;
- }
- } else {
- pr_err("suspend: sdio_func is not specified\n");
+ card = sdio_get_drvdata(func);
+ if (!card) {
+ dev_err(dev, "suspend: invalid card\n");
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v3 11/11] mwifiex: pcie: stop checking for NULL adapter->card
From: Amitkumar Karwar @ 2016-11-11 13:10 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris
In-Reply-To: <1478869818-4340-1-git-send-email-akarwar@marvell.com>
From: Brian Norris <briannorris@chromium.org>
It should never be NULL here, and to think otherwise makes things
confusing.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
v3: Below checkpatch warnings are resolved
WARNING: please, no spaces at the start of a line
#50: FILE: drivers/net/wireless/marvell/mwifiex/pcie.c:3037:
+ } else {$
WARNING: please, no spaces at the start of a line
#62: FILE: drivers/net/wireless/marvell/mwifiex/pcie.c:3044:
+ }$
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 53 +++++++++++++----------------
1 file changed, 23 insertions(+), 30 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 3b6d908..fb2b22d 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -3021,31 +3021,28 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
{
struct pcie_service_card *card = adapter->card;
- struct pci_dev *pdev;
+ struct pci_dev *pdev = card->dev;
int i;
- if (card) {
- pdev = card->dev;
- if (card->msix_enable) {
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- synchronize_irq(card->msix_entries[i].vector);
+ if (card->msix_enable) {
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ synchronize_irq(card->msix_entries[i].vector);
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- free_irq(card->msix_entries[i].vector,
- &card->msix_ctx[i]);
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ free_irq(card->msix_entries[i].vector,
+ &card->msix_ctx[i]);
- card->msix_enable = 0;
- pci_disable_msix(pdev);
- } else {
- mwifiex_dbg(adapter, INFO,
- "%s(): calling free_irq()\n", __func__);
- free_irq(card->dev->irq, &card->share_irq_ctx);
+ card->msix_enable = 0;
+ pci_disable_msix(pdev);
+ } else {
+ mwifiex_dbg(adapter, INFO,
+ "%s(): calling free_irq()\n", __func__);
+ free_irq(card->dev->irq, &card->share_irq_ctx);
- if (card->msi_enable)
- pci_disable_msi(pdev);
- }
- card->adapter = NULL;
+ if (card->msi_enable)
+ pci_disable_msi(pdev);
}
+ card->adapter = NULL;
}
/* This function initializes the PCI-E host memory space, WCB rings, etc.
@@ -3128,18 +3125,14 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
adapter->seq_num = 0;
adapter->tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_4K;
- if (card) {
- if (reg->sleep_cookie)
- mwifiex_pcie_delete_sleep_cookie_buf(adapter);
-
- mwifiex_pcie_delete_cmdrsp_buf(adapter);
- mwifiex_pcie_delete_evtbd_ring(adapter);
- mwifiex_pcie_delete_rxbd_ring(adapter);
- mwifiex_pcie_delete_txbd_ring(adapter);
- card->cmdrsp_buf = NULL;
- }
+ if (reg->sleep_cookie)
+ mwifiex_pcie_delete_sleep_cookie_buf(adapter);
- return;
+ mwifiex_pcie_delete_cmdrsp_buf(adapter);
+ mwifiex_pcie_delete_evtbd_ring(adapter);
+ mwifiex_pcie_delete_rxbd_ring(adapter);
+ mwifiex_pcie_delete_txbd_ring(adapter);
+ card->cmdrsp_buf = NULL;
}
static struct mwifiex_if_ops pcie_ops = {
--
1.9.1
^ permalink raw reply related
* Re: [linuxwifi] [PATCH 1/2] iwlwifi: fix MODULE_FIRMWARE for 6030
From: Luca Coelho @ 2016-11-11 13:56 UTC (permalink / raw)
To: Jürg Billeter, Emmanuel Grumbach, Sara Sharon
Cc: linuxwifi, Kalle Valo, linux-kernel, linux-wireless
In-Reply-To: <20161010163001.22865-1-j@bitron.ch>
Hi Jürg,
On Mon, 2016-10-10 at 18:30 +0200, Jürg Billeter wrote:
> IWL6000G2B_UCODE_API_MAX is not defined. ucode_api_max of
> IWL_DEVICE_6030 uses IWL6000G2_UCODE_API_MAX. Use this also for
> MODULE_FIRMWARE.
>
> Fixes: 9d9b21d1b616 ("iwlwifi: remove IWL_*_UCODE_API_OK")
> Signed-off-by: Jürg Billeter <j@bitron.ch>
> ---
> drivers/net/wireless/intel/iwlwifi/iwl-6000.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> index 0b9f6a7..39335b7 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> @@ -371,4 +371,4 @@ const struct iwl_cfg iwl6000_3agn_cfg = {
> MODULE_FIRMWARE(IWL6000_MODULE_FIRMWARE(IWL6000_UCODE_API_MAX));
> MODULE_FIRMWARE(IWL6050_MODULE_FIRMWARE(IWL6050_UCODE_API_MAX));
> MODULE_FIRMWARE(IWL6005_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
> -MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2B_UCODE_API_MAX));
> +MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
Sorry I had missed these patches because linux-wireless@vger was not
CCed, so it didn't get to patchwork. I have applied both in our
internal tree and they'll reach the mainline in my next pull-request.
Thanks!
--
Cheers,
Luca.
^ permalink raw reply
* [PATCH] fixed hwsim beacon delta calculation
From: Benjamin Beichler @ 2016-11-11 16:37 UTC (permalink / raw)
To: linux-wireless, johannes; +Cc: Benjamin Beichler
Due to the cast from uint32_t to int64_t, a wrong next beacon timing is
calculated and effectively the beacon timer stops to work. This is
especially bad for 802.11s mesh networks, because discovery breaks
without beacons.
Signed-off-by: Benjamin Beichler <benjamin.beichler@uni-rostock.de>
---
drivers/net/wireless/mac80211_hwsim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 8f366cc..8d7b0c6 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -819,7 +819,7 @@ static void mac80211_hwsim_set_tsf(struct ieee80211_hw *hw,
data->bcn_delta = do_div(delta, bcn_int);
} else {
data->tsf_offset -= delta;
- data->bcn_delta = -do_div(delta, bcn_int);
+ data->bcn_delta = -(s64)(do_div(delta, bcn_int));
}
}
--
1.9.1
^ permalink raw reply related
* wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-11-11 17:20 UTC (permalink / raw)
To: Kalle Valo, Pavel Machek, Ivaylo Dimitrov, Sebastian Reichel,
Aaro Koskinen, Tony Lindgren
Cc: linux-wireless, netdev, linux-kernel
[-- Attachment #1: Type: Text/Plain, Size: 1626 bytes --]
Hi! I will open discussion about mac address and calibration data for
wl1251 wireless chip again...
Problem: Mac address & calibration data for wl1251 chip on Nokia N900
are stored on second nand partition (mtd1) in special proprietary format
which is used only for Nokia N900 (probably on N8x0 and N9 too).
Wireless driver wl1251.ko cannot work without mac address and
calibration data.
Absence of mac address cause that driver generates random mac address at
every kernel boot which has couple of problems (unstable identifier of
wireless device due to udev permanent storage rules; unpredictable
behaviour for dhcp mac address assignment, mac address filtering, ...).
Currently there is no way to set (permanent) mac address for network
interface from userspace. And it does not make sense to implement in
linux kernel large parser for proprietary format of second nand
partition where is mac address stored only for one device -- Nokia N900.
Driver wl1251.ko loads calibration data via request_firmware() for file
wl1251-nvs.bin. There are some "example" calibration file in linux-
firmware repository, but it is not suitable for normal usage as real
calibration data are per-device specific.
So questions are:
1) How to set mac address from userspace for that wl1251 interface? In
userspace I can write parser for that proprietary format of nand
partition and extract mac address from it
2) How to send calibration data to wl1251 driver? Those are again stored
in proprietary format and I can write userspace parser for it.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [RFC] change mac80211_hwsim tx_rates to ieee80211_tx_rate
From: Benjamin Beichler @ 2016-11-11 17:22 UTC (permalink / raw)
To: linux-wireless
Hi,
we are working on a sophisticated Wifi simulation framework based on
mac80211_hwsim. This includes the simulation of frame transmissions with
realistic channel access and channel conditions. Therefore we need
several information (e.g. long/short gi, usage of RTS/CTS, ...), which
are available on a per frame/rate basis from struct ieee80211_tx_rate.
But this information is thrown away by the conversation to struct
hwsim_tx_rate (eg. in mac80211_hwsim_tx_frame_nl)
Firstly I think this information is valuable and I don't believe, the 4
Byte per frame greatly influences speed. Moreover the explicit double
copy (firstly from the info->status.rates to tx_attempts, secondly by
nla_put), takes longer than simply put the whole info->status.rates into
the netlink message.
So I would propose to put the whole struct into the netlink messages,
but I think that will break up the communication to e.g. bob copelands
wmediumd and similar simulations. I would like to have our
Implementation working with mainline kernels and therefore ask how we
could achieve this easily.
Obviously, we could define another field in the hwsim messages, but as
bob copeland already stated, significantly more information within the
netlink messages could intensify the timing overhead of hwsim.
Any suggestions?
^ permalink raw reply
* Re: [PATCH] iwlwifi: fix undefined 6000G2B firmware MAX API version
From: Tj @ 2016-11-11 17:47 UTC (permalink / raw)
To: Luca Coelho, open list:INTEL WIRELESS WIFI LINK (iwlwifi)
Cc: Intel Linux Wireless (supporter:INTEL WIRELESS WIFI LINK (iwlwifi))
In-Reply-To: <1478853340.15030.21.camel@coelho.fi>
On 11/11/16 08:35, Luca Coelho wrote:
> Hi,
> On Thu, 2016-11-10 at 22:04 +0000, Tj wrote:
>> $ modinfo -F firmware iwlwifi | grep API
>> iwlwifi-6000g2b-IWL6000G2B_UCODE_API_MAX.ucode
>> $ modinfo -F vermagic iwlwifi
>> 4.9.0-040900rc4-lowlatency SMP preempt mod_unload modversions
>>
>> Change-Id: Ie21a4be0b12b520844c1da4a8bef9e8a0097d919
>> Signed-off-by: TJ <linux@iam.tj>
>> ---
>> drivers/net/wireless/intel/iwlwifi/iwl-6000.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
>> b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
>> index 0b9f6a7..19b85e8 100644
>> --- a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
>> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
>> @@ -34,6 +34,7 @@
>> #define IWL6000_UCODE_API_MAX 6
>> #define IWL6050_UCODE_API_MAX 5
>> #define IWL6000G2_UCODE_API_MAX 6
>> +#define IWL6000G2B_UCODE_API_MAX 6
>> #define IWL6035_UCODE_API_MAX 6
>> /* Lowest firmware API version supported */
> Thanks for your patch! But the correct thing to do would be to change
> the MODULE_FIRMWARE line instead of adding this define here. The 6030
> NIC uses IWL6000G2_UCODE_API_MAX in the config structure, so the
> MODULE_FIRMWARE macro should use that too.
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> index 0b9f6a7..39335b7 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> @@ -371,4 +371,4 @@ const struct iwl_cfg iwl6000_3agn_cfg = {
> MODULE_FIRMWARE(IWL6000_MODULE_FIRMWARE(IWL6000_UCODE_API_MAX));
> MODULE_FIRMWARE(IWL6050_MODULE_FIRMWARE(IWL6050_UCODE_API_MAX));
> MODULE_FIRMWARE(IWL6005_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
> -MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2B_UCODE_API_MAX));
> +MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
I don't think this is correct. There are different firmware files for
the 'g2a' (6005) and 'g2b' (6030) and as such the revisions for each
could be different, so setting the 6030 to use the same API_MAX as the
6005 looks wrong to me.
$ ls -1 /lib/firmware/iwlwifi-*6000g*
/lib/firmware/iwlwifi-6000g2a-5.ucode
/lib/firmware/iwlwifi-6000g2a-6.ucode
/lib/firmware/iwlwifi-6000g2b-6.ucode
^ permalink raw reply
* Re: [PATCH] iwlwifi: fix undefined 6000G2B firmware MAX API version
From: Luca Coelho @ 2016-11-11 19:01 UTC (permalink / raw)
To: Tj, open list:INTEL WIRELESS WIFI LINK (iwlwifi)
Cc: Intel Linux Wireless (supporter:INTEL WIRELESS WIFI LINK (iwlwifi))
In-Reply-To: <87adaa7e-3e64-51eb-f250-bf4f1c025b66@iam.tj>
On Fri, 2016-11-11 at 17:47 +0000, Tj wrote:
> On 11/11/16 08:35, Luca Coelho wrote:
> > Hi,
> > On Thu, 2016-11-10 at 22:04 +0000, Tj wrote:
> > > $ modinfo -F firmware iwlwifi | grep API
> > > iwlwifi-6000g2b-IWL6000G2B_UCODE_API_MAX.ucode
> > > $ modinfo -F vermagic iwlwifi
> > > 4.9.0-040900rc4-lowlatency SMP preempt mod_unload modversions
> > >
> > > Change-Id: Ie21a4be0b12b520844c1da4a8bef9e8a0097d919
> > > Signed-off-by: TJ <linux@iam.tj>
> > > ---
> > > drivers/net/wireless/intel/iwlwifi/iwl-6000.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > > b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > > index 0b9f6a7..19b85e8 100644
> > > --- a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > > @@ -34,6 +34,7 @@
> > > #define IWL6000_UCODE_API_MAX 6
> > > #define IWL6050_UCODE_API_MAX 5
> > > #define IWL6000G2_UCODE_API_MAX 6
> > > +#define IWL6000G2B_UCODE_API_MAX 6
> > > #define IWL6035_UCODE_API_MAX 6
> > > /* Lowest firmware API version supported */
> >
> > Thanks for your patch! But the correct thing to do would be to change
> > the MODULE_FIRMWARE line instead of adding this define here. The 6030
> > NIC uses IWL6000G2_UCODE_API_MAX in the config structure, so the
> > MODULE_FIRMWARE macro should use that too.
> >
> > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > index 0b9f6a7..39335b7 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-6000.c
> > @@ -371,4 +371,4 @@ const struct iwl_cfg iwl6000_3agn_cfg = {
> > MODULE_FIRMWARE(IWL6000_MODULE_FIRMWARE(IWL6000_UCODE_API_MAX));
> > MODULE_FIRMWARE(IWL6050_MODULE_FIRMWARE(IWL6050_UCODE_API_MAX));
> > MODULE_FIRMWARE(IWL6005_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
> > -MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2B_UCODE_API_MAX));
> > +MODULE_FIRMWARE(IWL6030_MODULE_FIRMWARE(IWL6000G2_UCODE_API_MAX));
>
> I don't think this is correct. There are different firmware files for
> the 'g2a' (6005) and 'g2b' (6030) and as such the revisions for each
> could be different, so setting the 6030 to use the same API_MAX as the
> 6005 looks wrong to me.
>
> $ ls -1 /lib/firmware/iwlwifi-*6000g*
> /lib/firmware/iwlwifi-6000g2a-5.ucode
> /lib/firmware/iwlwifi-6000g2a-6.ucode
> /lib/firmware/iwlwifi-6000g2b-6.ucode
Maybe you're right, but still, just changing the firmware name is not
enough. If that is really supposed to be the case (and TBH I really
don't know these old NICs), then the config structs should be changed
as well.
In any case, I've applied (internally, for now) the patches that Jürg
sent a while ago. One of them does pretty much the same thing I
suggested her.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH v2 3/3] mwifiex: Enable WoWLAN for both sdio and pcie
From: Brian Norris @ 2016-11-11 20:42 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
dmitry.torokhov
In-Reply-To: <1478862911-15498-3-git-send-email-akarwar@marvell.com>
On Fri, Nov 11, 2016 at 04:45:11PM +0530, Amitkumar Karwar wrote:
> From: Rajat Jain <rajatja@google.com>
>
> Commit ce4f6f0c353b ("mwifiex: add platform specific wakeup interrupt
> support") added WoWLAN feature only for sdio. This patch moves that
> code to the common module so that all the interface drivers can use
> it for free. It enables pcie and sdio for its use currently.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> v2: v1 doesn't apply smoothly on latest code due to recently merged
> patch "mwifiex: report error to PCIe for suspend failure". Minor
> conflict is resolved in v2
> ---
In case you haven't noticed my comments elsewhere, I'll repeat them
here: you're copy-and-pasting buggy code. That can be OK I suppose, but
please fix these issues, possibly in a follow-up patch.
> drivers/net/wireless/marvell/mwifiex/main.c | 41 ++++++++++++++++
> drivers/net/wireless/marvell/mwifiex/main.h | 25 ++++++++++
> drivers/net/wireless/marvell/mwifiex/pcie.c | 2 +
> drivers/net/wireless/marvell/mwifiex/sdio.c | 72 ++---------------------------
> drivers/net/wireless/marvell/mwifiex/sdio.h | 8 ----
> 5 files changed, 73 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index b2f3d96..20c9b77 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -1552,14 +1552,55 @@ void mwifiex_do_flr(struct mwifiex_adapter *adapter, bool prepare)
> }
> EXPORT_SYMBOL_GPL(mwifiex_do_flr);
>
> +static irqreturn_t mwifiex_irq_wakeup_handler(int irq, void *priv)
> +{
> + struct mwifiex_adapter *adapter = priv;
> +
> + if (adapter->irq_wakeup >= 0) {
> + dev_dbg(adapter->dev, "%s: wake by wifi", __func__);
> + adapter->wake_by_wifi = true;
This flag is unecessary and buggy. IIUC, you're trying to avoid calling
disable_irq() in resume(), if you've called it here?
> + disable_irq_nosync(irq);
...but this is unnecessary, I think, unless you're trying to make up for
buggy wakeup interrupts that keep firing? See my suggestion below.
> + }
> +
> + /* Notify PM core we are wakeup source */
> + pm_wakeup_event(adapter->dev, 0);
> +
> + return IRQ_HANDLED;
> +}
> +
> static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
> {
> + int ret;
> struct device *dev = adapter->dev;
>
> if (!dev->of_node)
> return;
>
> adapter->dt_node = dev->of_node;
> + adapter->irq_wakeup = irq_of_parse_and_map(adapter->dt_node, 0);
> + if (!adapter->irq_wakeup) {
> + dev_info(dev, "fail to parse irq_wakeup from device tree\n");
> + return;
> + }
> +
> + ret = devm_request_irq(dev, adapter->irq_wakeup,
> + mwifiex_irq_wakeup_handler, IRQF_TRIGGER_LOW,
> + "wifi_wake", adapter);
> + if (ret) {
> + dev_err(dev, "Failed to request irq_wakeup %d (%d)\n",
> + adapter->irq_wakeup, ret);
> + goto err_exit;
> + }
> +
> + disable_irq(adapter->irq_wakeup);
> + if (device_init_wakeup(dev, true)) {
> + dev_err(dev, "fail to init wakeup for mwifiex\n");
> + goto err_exit;
> + }
> + return;
> +
> +err_exit:
> + adapter->irq_wakeup = 0;
> }
>
> /*
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> index 0c07434..11abc49 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -1011,6 +1011,10 @@ struct mwifiex_adapter {
> bool usb_mc_setup;
> struct cfg80211_wowlan_nd_info *nd_info;
> struct ieee80211_regdomain *regd;
> +
> + /* Wake-on-WLAN (WoWLAN) */
> + int irq_wakeup;
> + bool wake_by_wifi;
> };
>
> void mwifiex_process_tx_queue(struct mwifiex_adapter *adapter);
> @@ -1410,6 +1414,27 @@ static inline u8 mwifiex_is_tdls_link_setup(u8 status)
> return false;
> }
>
> +/* Disable platform specific wakeup interrupt */
> +static inline void mwifiex_disable_wake(struct mwifiex_adapter *adapter)
> +{
> + if (adapter->irq_wakeup >= 0) {
> + disable_irq_wake(adapter->irq_wakeup);
> + if (!adapter->wake_by_wifi)
You're depending on the wake IRQ handler to set this flag, so you don't
disable the IRQ twice (the calls nest). But what if the IRQ is being
serviced concurrently with this? Then you'll double-disable the IRQ.
I believe you can just remove the disable_irq_nosync() from the handler,
kill the above flag, and just unconditionally disable the IRQ.
> + disable_irq(adapter->irq_wakeup);
> + }
> +}
> +
> +/* Enable platform specific wakeup interrupt */
> +static inline void mwifiex_enable_wake(struct mwifiex_adapter *adapter)
> +{
> + /* Enable platform specific wakeup interrupt */
> + if (adapter->irq_wakeup >= 0) {
> + adapter->wake_by_wifi = false;
> + enable_irq(adapter->irq_wakeup);
> + enable_irq_wake(adapter->irq_wakeup);
> + }
> +}
> +
> int mwifiex_init_shutdown_fw(struct mwifiex_private *priv,
> u32 func_init_shutdown);
> int mwifiex_add_card(void *, struct semaphore *, struct mwifiex_if_ops *, u8,
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index fe7f3b0..a38994e 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -131,6 +131,7 @@ static int mwifiex_pcie_suspend(struct device *dev)
> }
>
> adapter = card->adapter;
> + mwifiex_enable_wake(adapter);
>
> /* Enable the Host Sleep */
> if (!mwifiex_enable_hs(adapter)) {
^^^ What if this fails? Then you'll leave the wake IRQ enabled.
(You've propagated this error from the SDIO driver.)
> @@ -186,6 +187,7 @@ static int mwifiex_pcie_resume(struct device *dev)
>
> mwifiex_cancel_hs(mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_STA),
> MWIFIEX_ASYNC_CMD);
> + mwifiex_disable_wake(adapter);
>
> return 0;
> }
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 558743a..ff5bb45 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -79,67 +79,18 @@
> { }
> };
>
> -static irqreturn_t mwifiex_wake_irq_wifi(int irq, void *priv)
> -{
> - struct mwifiex_plt_wake_cfg *cfg = priv;
> -
> - if (cfg->irq_wifi >= 0) {
> - pr_info("%s: wake by wifi", __func__);
> - cfg->wake_by_wifi = true;
> - disable_irq_nosync(irq);
> - }
> -
> - /* Notify PM core we are wakeup source */
> - pm_wakeup_event(cfg->dev, 0);
> -
> - return IRQ_HANDLED;
> -}
> -
> /* This function parse device tree node using mmc subnode devicetree API.
> * The device node is saved in card->plt_of_node.
> * if the device tree node exist and include interrupts attributes, this
> * function will also request platform specific wakeup interrupt.
> */
> -static int mwifiex_sdio_probe_of(struct device *dev, struct sdio_mmc_card *card)
> +static int mwifiex_sdio_probe_of(struct device *dev)
> {
> - struct mwifiex_plt_wake_cfg *cfg;
> - int ret;
> -
> if (!of_match_node(mwifiex_sdio_of_match_table, dev->of_node)) {
> dev_err(dev, "required compatible string missing\n");
> return -EINVAL;
> }
>
> - card->plt_of_node = dev->of_node;
> - card->plt_wake_cfg = devm_kzalloc(dev, sizeof(*card->plt_wake_cfg),
> - GFP_KERNEL);
> - cfg = card->plt_wake_cfg;
> - if (cfg && card->plt_of_node) {
> - cfg->dev = dev;
> - cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
> - if (!cfg->irq_wifi) {
> - dev_dbg(dev,
> - "fail to parse irq_wifi from device tree\n");
> - } else {
> - ret = devm_request_irq(dev, cfg->irq_wifi,
> - mwifiex_wake_irq_wifi,
> - IRQF_TRIGGER_LOW,
> - "wifi_wake", cfg);
> - if (ret) {
> - dev_dbg(dev,
> - "Failed to request irq_wifi %d (%d)\n",
> - cfg->irq_wifi, ret);
> - card->plt_wake_cfg = NULL;
> - return 0;
> - }
> - disable_irq(cfg->irq_wifi);
> - }
> - }
> -
> - ret = device_init_wakeup(dev, true);
> - if (ret)
> - dev_err(dev, "fail to init wakeup for mwifiex");
> -
> return 0;
> }
>
> @@ -199,11 +150,9 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct sdio_mmc_card *card)
>
> /* device tree node parsing and platform specific configuration*/
> if (func->dev.of_node) {
> - ret = mwifiex_sdio_probe_of(&func->dev, card);
> - if (ret) {
> - dev_err(&func->dev, "SDIO dt node parse failed\n");
> + ret = mwifiex_sdio_probe_of(&func->dev);
> + if (ret)
> goto err_disable;
> - }
> valid_of_node = true;
> }
>
> @@ -269,12 +218,7 @@ static int mwifiex_sdio_resume(struct device *dev)
> mwifiex_cancel_hs(mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_STA),
> MWIFIEX_SYNC_CMD);
>
> - /* Disable platform specific wakeup interrupt */
> - if (card->plt_wake_cfg && card->plt_wake_cfg->irq_wifi >= 0) {
> - disable_irq_wake(card->plt_wake_cfg->irq_wifi);
> - if (!card->plt_wake_cfg->wake_by_wifi)
> - disable_irq(card->plt_wake_cfg->irq_wifi);
> - }
> + mwifiex_disable_wake(adapter);
>
> return 0;
> }
> @@ -354,13 +298,7 @@ static int mwifiex_sdio_suspend(struct device *dev)
> }
>
> adapter = card->adapter;
> -
> - /* Enable platform specific wakeup interrupt */
> - if (card->plt_wake_cfg && card->plt_wake_cfg->irq_wifi >= 0) {
> - card->plt_wake_cfg->wake_by_wifi = false;
> - enable_irq(card->plt_wake_cfg->irq_wifi);
> - enable_irq_wake(card->plt_wake_cfg->irq_wifi);
> - }
> + mwifiex_enable_wake(adapter);
>
> /* Enable the Host Sleep */
> if (!mwifiex_enable_hs(adapter)) {
Same problem here. Existing issue.
Brian
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h b/drivers/net/wireless/marvell/mwifiex/sdio.h
> index 07cdd23..b9fbc5c 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.h
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
> @@ -154,12 +154,6 @@
> a->mpa_rx.start_port = 0; \
> } while (0)
>
> -struct mwifiex_plt_wake_cfg {
> - struct device *dev;
> - int irq_wifi;
> - bool wake_by_wifi;
> -};
> -
> /* data structure for SDIO MPA TX */
> struct mwifiex_sdio_mpa_tx {
> /* multiport tx aggregation buffer pointer */
> @@ -243,8 +237,6 @@ struct mwifiex_sdio_card_reg {
> struct sdio_mmc_card {
> struct sdio_func *func;
> struct mwifiex_adapter *adapter;
> - struct device_node *plt_of_node;
> - struct mwifiex_plt_wake_cfg *plt_wake_cfg;
>
> const char *firmware;
> const struct mwifiex_sdio_card_reg *reg;
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v2 2/3] mwifiex: Introduce mwifiex_probe_of() to parse common properties
From: Brian Norris @ 2016-11-11 20:49 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
dmitry.torokhov
In-Reply-To: <1478862911-15498-2-git-send-email-akarwar@marvell.com>
On Fri, Nov 11, 2016 at 04:45:10PM +0530, Amitkumar Karwar wrote:
> From: Rajat Jain <rajatja@google.com>
>
> Introduce function mwifiex_probe_of() to parse common properties.
> Since the interface drivers get to decide whether or not the device
> tree node was a valid one (depending on the compatible property),
> let the interface drivers pass a flag to indicate whether the device
> tree node was a valid one.
Wait, what? I don't understand why this is needed. The current of_node
user (SDIO) always checks dev->of_node (if !NULL), and if it's not
matching, it rejects the device and doesn't even try to register the
card at all. That's a common pattern for DT-based drivers, and I don't
see why we need to do differently for any other driver (e.g., PCIe).
So...isn't 'dev->of_node != NULL' an equivalent test to 'of_node_valid'?
Or put another way, mwifiex_add_card() should never see a 'struct
device' whose of_node is not compatible. Do you agree?
> The function mwifiex_probe_of()nodetself is currently only a place
> holder with the next patch adding content to it.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> v2: Same as v1
> ---
> drivers/net/wireless/marvell/mwifiex/main.c | 15 ++++++++++++++-
> drivers/net/wireless/marvell/mwifiex/main.h | 2 +-
> drivers/net/wireless/marvell/mwifiex/pcie.c | 4 +++-
> drivers/net/wireless/marvell/mwifiex/sdio.c | 4 +++-
> drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 5 +----
> drivers/net/wireless/marvell/mwifiex/usb.c | 2 +-
> 6 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index dcceab2..b2f3d96 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -1552,6 +1552,16 @@ void mwifiex_do_flr(struct mwifiex_adapter *adapter, bool prepare)
> }
> EXPORT_SYMBOL_GPL(mwifiex_do_flr);
>
> +static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
> +{
> + struct device *dev = adapter->dev;
> +
> + if (!dev->of_node)
> + return;
> +
> + adapter->dt_node = dev->of_node;
> +}
> +
> /*
> * This function adds the card.
> *
> @@ -1568,7 +1578,7 @@ void mwifiex_do_flr(struct mwifiex_adapter *adapter, bool prepare)
> int
> mwifiex_add_card(void *card, struct semaphore *sem,
> struct mwifiex_if_ops *if_ops, u8 iface_type,
> - struct device *dev)
> + struct device *dev, bool of_node_valid)
> {
> struct mwifiex_adapter *adapter;
>
> @@ -1581,6 +1591,9 @@ void mwifiex_do_flr(struct mwifiex_adapter *adapter, bool prepare)
> }
>
> adapter->dev = dev;
> + if (of_node_valid)
> + mwifiex_probe_of(adapter);
> +
> adapter->iface_type = iface_type;
> adapter->card_sem = sem;
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> index 2664513..0c07434 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -1413,7 +1413,7 @@ static inline u8 mwifiex_is_tdls_link_setup(u8 status)
> int mwifiex_init_shutdown_fw(struct mwifiex_private *priv,
> u32 func_init_shutdown);
> int mwifiex_add_card(void *, struct semaphore *, struct mwifiex_if_ops *, u8,
> - struct device *);
> + struct device *, bool);
IOW, I think this extra bool flag is a bad idea.
Brian
> int mwifiex_remove_card(struct mwifiex_adapter *, struct semaphore *);
>
> void mwifiex_get_version(struct mwifiex_adapter *adapter, char *version,
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index de6939c..fe7f3b0 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -201,6 +201,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
> {
> struct pcie_service_card *card;
> + bool valid_of_node = false;
> int ret;
>
> pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n",
> @@ -228,10 +229,11 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
> ret = mwifiex_pcie_probe_of(&pdev->dev);
> if (ret)
> goto err_free;
> + valid_of_node = true;
> }
>
> ret = mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
> - MWIFIEX_PCIE, &pdev->dev);
> + MWIFIEX_PCIE, &pdev->dev, valid_of_node);
> if (ret) {
> pr_err("%s failed\n", __func__);
> goto err_free;
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index c95f41f..558743a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -156,6 +156,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct sdio_mmc_card *card)
> {
> int ret;
> struct sdio_mmc_card *card = NULL;
> + bool valid_of_node = false;
>
> pr_debug("info: vendor=0x%4.04X device=0x%4.04X class=%d function=%d\n",
> func->vendor, func->device, func->class, func->num);
> @@ -203,10 +204,11 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct sdio_mmc_card *card)
> dev_err(&func->dev, "SDIO dt node parse failed\n");
> goto err_disable;
> }
> + valid_of_node = true;
> }
>
> ret = mwifiex_add_card(card, &add_remove_card_sem, &sdio_ops,
> - MWIFIEX_SDIO, &func->dev);
> + MWIFIEX_SDIO, &func->dev, valid_of_node);
> if (ret) {
> dev_err(&func->dev, "add card failed\n");
> goto err_disable;
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index b697b61..bcd6408 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -2235,10 +2235,7 @@ int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init)
> * The cal-data can be read from device tree and/or
> * a configuration file and downloaded to firmware.
> */
> - if ((priv->adapter->iface_type == MWIFIEX_SDIO ||
> - priv->adapter->iface_type == MWIFIEX_PCIE) &&
> - adapter->dev->of_node) {
> - adapter->dt_node = adapter->dev->of_node;
> + if (adapter->dt_node) {
> if (of_property_read_u32(adapter->dt_node,
> "marvell,wakeup-pin",
> &data) == 0) {
> diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
> index f847fff..11c9629 100644
> --- a/drivers/net/wireless/marvell/mwifiex/usb.c
> +++ b/drivers/net/wireless/marvell/mwifiex/usb.c
> @@ -476,7 +476,7 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
> usb_set_intfdata(intf, card);
>
> ret = mwifiex_add_card(card, &add_remove_card_sem, &usb_ops,
> - MWIFIEX_USB, &card->udev->dev);
> + MWIFIEX_USB, &card->udev->dev, false);
> if (ret) {
> pr_err("%s: mwifiex_add_card failed: %d\n", __func__, ret);
> usb_reset_device(udev);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v2 3/3] mwifiex: Enable WoWLAN for both sdio and pcie
From: Brian Norris @ 2016-11-11 22:05 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
dmitry.torokhov
In-Reply-To: <20161111204235.GB111624@google.com>
Hi,
One correction to my review:
On Fri, Nov 11, 2016 at 12:42:36PM -0800, Brian Norris wrote:
> On Fri, Nov 11, 2016 at 04:45:11PM +0530, Amitkumar Karwar wrote:
> > --- a/drivers/net/wireless/marvell/mwifiex/main.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> > @@ -1552,14 +1552,55 @@ void mwifiex_do_flr(struct mwifiex_adapter *adapter, bool prepare)
> > }
> > EXPORT_SYMBOL_GPL(mwifiex_do_flr);
> >
> > +static irqreturn_t mwifiex_irq_wakeup_handler(int irq, void *priv)
> > +{
> > + struct mwifiex_adapter *adapter = priv;
> > +
> > + if (adapter->irq_wakeup >= 0) {
> > + dev_dbg(adapter->dev, "%s: wake by wifi", __func__);
> > + adapter->wake_by_wifi = true;
>
> This flag is unecessary and buggy. IIUC, you're trying to avoid calling
> disable_irq() in resume(), if you've called it here?
>
> > + disable_irq_nosync(irq);
>
> ...but this is unnecessary, I think, unless you're trying to make up for
> buggy wakeup interrupts that keep firing? See my suggestion below.
I think I figured out some of the logic here, and my suggestion was
somewhat wrong. This 'wake_by_wifi' flag seems to be used here to assist
with the fact that we're requesting a level-triggered interrupt, but
the card doesn't deassert the interrupt until much later -- when we
finish the wakeup handshake.
So I guess it is necessary (or at least, expedient) to disable the
interrupt here.
> > + }
> > +
> > + /* Notify PM core we are wakeup source */
> > + pm_wakeup_event(adapter->dev, 0);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
> > {
> > + int ret;
> > struct device *dev = adapter->dev;
> >
> > if (!dev->of_node)
> > return;
> >
> > adapter->dt_node = dev->of_node;
> > + adapter->irq_wakeup = irq_of_parse_and_map(adapter->dt_node, 0);
> > + if (!adapter->irq_wakeup) {
> > + dev_info(dev, "fail to parse irq_wakeup from device tree\n");
> > + return;
> > + }
> > +
> > + ret = devm_request_irq(dev, adapter->irq_wakeup,
> > + mwifiex_irq_wakeup_handler, IRQF_TRIGGER_LOW,
> > + "wifi_wake", adapter);
> > + if (ret) {
> > + dev_err(dev, "Failed to request irq_wakeup %d (%d)\n",
> > + adapter->irq_wakeup, ret);
> > + goto err_exit;
> > + }
> > +
> > + disable_irq(adapter->irq_wakeup);
> > + if (device_init_wakeup(dev, true)) {
> > + dev_err(dev, "fail to init wakeup for mwifiex\n");
> > + goto err_exit;
> > + }
> > + return;
> > +
> > +err_exit:
> > + adapter->irq_wakeup = 0;
> > }
> >
> > /*
> > diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> > index 0c07434..11abc49 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/main.h
> > +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> > @@ -1011,6 +1011,10 @@ struct mwifiex_adapter {
> > bool usb_mc_setup;
> > struct cfg80211_wowlan_nd_info *nd_info;
> > struct ieee80211_regdomain *regd;
> > +
> > + /* Wake-on-WLAN (WoWLAN) */
> > + int irq_wakeup;
> > + bool wake_by_wifi;
> > };
> >
> > void mwifiex_process_tx_queue(struct mwifiex_adapter *adapter);
> > @@ -1410,6 +1414,27 @@ static inline u8 mwifiex_is_tdls_link_setup(u8 status)
> > return false;
> > }
> >
> > +/* Disable platform specific wakeup interrupt */
> > +static inline void mwifiex_disable_wake(struct mwifiex_adapter *adapter)
> > +{
> > + if (adapter->irq_wakeup >= 0) {
> > + disable_irq_wake(adapter->irq_wakeup);
> > + if (!adapter->wake_by_wifi)
>
> You're depending on the wake IRQ handler to set this flag, so you don't
> disable the IRQ twice (the calls nest). But what if the IRQ is being
> serviced concurrently with this? Then you'll double-disable the IRQ.
>
> I believe you can just remove the disable_irq_nosync() from the handler,
> kill the above flag, and just unconditionally disable the IRQ.
So my suggestion here was wrong; we shouldn't completely kill the check
for ->wake_by_wifi I don't think, but we *should* wait for the interrupt
handler to complete before checking the flag. i.e., synchronize_irq():
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 4063086ab5b8..e9446eeafb9d 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -841,6 +841,11 @@ void mwifiex_disable_wake(struct mwifiex_plt_wake_cfg *wake_cfg)
{
if (wake_cfg && wake_cfg->irq_wifi >= 0) {
disable_irq_wake(wake_cfg->irq_wifi);
+ /*
+ * Disable the wake IRQ only if it didn't already fire (and
+ * disable itself).
+ */
+ synchronize_irq(wake_cfg->irq_wifi);
if (!wake_cfg->wake_by_wifi)
disable_irq(wake_cfg->irq_wifi);
}
I'd appreciate if you bugfix this, either before or after this patch.
Brian
> > + disable_irq(adapter->irq_wakeup);
> > + }
> > +}
> > +
> > +/* Enable platform specific wakeup interrupt */
> > +static inline void mwifiex_enable_wake(struct mwifiex_adapter *adapter)
> > +{
> > + /* Enable platform specific wakeup interrupt */
> > + if (adapter->irq_wakeup >= 0) {
> > + adapter->wake_by_wifi = false;
> > + enable_irq(adapter->irq_wakeup);
> > + enable_irq_wake(adapter->irq_wakeup);
> > + }
> > +}
> > +
> > int mwifiex_init_shutdown_fw(struct mwifiex_private *priv,
> > u32 func_init_shutdown);
> > int mwifiex_add_card(void *, struct semaphore *, struct mwifiex_if_ops *, u8,
^ permalink raw reply related
* Re: [RFC] change mac80211_hwsim tx_rates to ieee80211_tx_rate
From: Johannes Berg @ 2016-11-12 21:08 UTC (permalink / raw)
To: Benjamin Beichler, linux-wireless
In-Reply-To: <dcc2ec52-abe0-3182-e7f5-d1df40fd492b@uni-rostock.de>
> So I would propose to put the whole struct into the netlink messages,
This is a terrible idea, since internal changes to this struct would
break the userland API/ABI. hwsim seems perhaps less important than
most APIs, but there is wmediumd etc. already.
> but I think that will break up the communication to e.g. bob
> copelands
> wmediumd and similar simulations. I would like to have our
> Implementation working with mainline kernels and therefore ask how we
> could achieve this easily.
>
> Obviously, we could define another field in the hwsim messages, but
> as bob copeland already stated, significantly more information within
> the netlink messages could intensify the timing overhead of hwsim.
I don't think we have any other choice but add the relevant fields as
proper attributes.
johannes
^ permalink raw reply
* [PATCH] rtl8xxxu: Fix failure to reconnect to AP
From: Barry Day @ 2016-11-13 11:17 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Kalle Valo, linux-wireless
The rtl8192e and rtl8723 fail to reconnect to an AP after being
disconnected. Ths patch fixes that without affecting the rtl8192cu.
I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
After going through the orginal realtek code for the rtl8723, I am
confident the patch is applicable to both.
Signed-off-by: Barry Day <briselec@gmail.com>
---
rtl8xxxu_core.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
index 04141e5..6ac10d2 100644
--- a/rtl8xxxu_core.c
+++ b/rtl8xxxu_core.c
@@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
u8 macid, bool connect)
{
+ u8 val8;
struct h2c_cmd h2c;
memset(&h2c, 0, sizeof(struct h2c_cmd));
h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT;
- if (connect)
+ if (connect) {
h2c.media_status_rpt.parm |= BIT(0);
- else
- h2c.media_status_rpt.parm &= ~BIT(0);
+ rtl8xxxu_gen2_h2c_cmd(priv, &h2c,
+ sizeof(h2c.media_status_rpt));
+ } else {
+ val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
+ val8 &= ~BEACON_FUNCTION_ENABLE;
+
+ rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
+ rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
+ rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
+ }
- rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
}
void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
@@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
sgi = 1;
rcu_read_unlock();
+ rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
+
priv->fops->update_rate_mask(priv, ramask, sgi);
rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
--
2.9.2
^ permalink raw reply related
* Re: Problems getting mwifiex with sd8887 to work
From: Wolfram Sang @ 2016-11-13 14:13 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless@vger.kernel.org, Nishant Sarmukadam, Kalle Valo
In-Reply-To: <507fff29bd2048c2b2295beb451943b5@SC-EXCH04.marvell.com>
[-- Attachment #1: Type: text/plain, Size: 237 bytes --]
> 0x242 command failure is expected for sd8887, as It's not supported.
Is the same true for 0x10f?
mwifiex_sdio mmc2:0001:1: CMD_RESP: cmd 0x10f error, result=0x2
Otherwise I can connect to the network and run performance tests now.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 3.2 017/152] x86/quirks: Add early quirk to reset Apple AirPort card
From: Ben Hutchings @ 2016-11-14 0:14 UTC (permalink / raw)
To: linux-kernel, stable
Cc: akpm, Denys Vlasenko, linux-wireless, H. Peter Anvin,
Andy Lutomirski, Thomas Gleixner, Matt Fleming, Peter Zijlstra,
Konstantin Simanov, Borislav Petkov, linux-pci, Brian Gerst,
Michael Buesch, Andrew Worsley, Lukas Wunner,
Rafał Miłecki, Chris Bainbridge, b43-dev, Bryan Paradis,
Linus Torvalds, Josh Poimboeuf, Chris Milsted, Matthew Garrett,
Bjorn Helgaas, Yinghai Lu, Ingo Molnar
In-Reply-To: <lsq.1479082446.271293126@decadent.org.uk>
3.2.84-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Lukas Wunner <lukas@wunner.de>
commit abb2bafd295fe962bbadc329dbfb2146457283ac upstream.
The EFI firmware on Macs contains a full-fledged network stack for
downloading OS X images from osrecovery.apple.com. Unfortunately
on Macs introduced 2011 and 2012, EFI brings up the Broadcom 4331
wireless card on every boot and leaves it enabled even after
ExitBootServices has been called. The card continues to assert its IRQ
line, causing spurious interrupts if the IRQ is shared. It also corrupts
memory by DMAing received packets, allowing for remote code execution
over the air. This only stops when a driver is loaded for the wireless
card, which may be never if the driver is not installed or blacklisted.
The issue seems to be constrained to the Broadcom 4331. Chris Milsted
has verified that the newer Broadcom 4360 built into the MacBookPro11,3
(2013/2014) does not exhibit this behaviour. The chances that Apple will
ever supply a firmware fix for the older machines appear to be zero.
The solution is to reset the card on boot by writing to a reset bit in
its mmio space. This must be done as an early quirk and not as a plain
vanilla PCI quirk to successfully combat memory corruption by DMAed
packets: Matthew Garrett found out in 2012 that the packets are written
to EfiBootServicesData memory (http://mjg59.dreamwidth.org/11235.html).
This type of memory is made available to the page allocator by
efi_free_boot_services(). Plain vanilla PCI quirks run much later, in
subsys initcall level. In-between a time window would be open for memory
corruption. Random crashes occurring in this time window and attributed
to DMAed packets have indeed been observed in the wild by Chris
Bainbridge.
When Matthew Garrett analyzed the memory corruption issue in 2012, he
sought to fix it with a grub quirk which transitions the card to D3hot:
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=9d34bb85da56
This approach does not help users with other bootloaders and while it
may prevent DMAed packets, it does not cure the spurious interrupts
emanating from the card. Unfortunately the card's mmio space is
inaccessible in D3hot, so to reset it, we have to undo the effect of
Matthew's grub patch and transition the card back to D0.
Note that the quirk takes a few shortcuts to reduce the amount of code:
The size of BAR 0 and the location of the PM capability is identical
on all affected machines and therefore hardcoded. Only the address of
BAR 0 differs between models. Also, it is assumed that the BCMA core
currently mapped is the 802.11 core. The EFI driver seems to always take
care of this.
Michael Büsch, Bjorn Helgaas and Matt Fleming contributed feedback
towards finding the best solution to this problem.
The following should be a comprehensive list of affected models:
iMac13,1 2012 21.5" [Root Port 00:1c.3 = 8086:1e16]
iMac13,2 2012 27" [Root Port 00:1c.3 = 8086:1e16]
Macmini5,1 2011 i5 2.3 GHz [Root Port 00:1c.1 = 8086:1c12]
Macmini5,2 2011 i5 2.5 GHz [Root Port 00:1c.1 = 8086:1c12]
Macmini5,3 2011 i7 2.0 GHz [Root Port 00:1c.1 = 8086:1c12]
Macmini6,1 2012 i5 2.5 GHz [Root Port 00:1c.1 = 8086:1e12]
Macmini6,2 2012 i7 2.3 GHz [Root Port 00:1c.1 = 8086:1e12]
MacBookPro8,1 2011 13" [Root Port 00:1c.1 = 8086:1c12]
MacBookPro8,2 2011 15" [Root Port 00:1c.1 = 8086:1c12]
MacBookPro8,3 2011 17" [Root Port 00:1c.1 = 8086:1c12]
MacBookPro9,1 2012 15" [Root Port 00:1c.1 = 8086:1e12]
MacBookPro9,2 2012 13" [Root Port 00:1c.1 = 8086:1e12]
MacBookPro10,1 2012 15" [Root Port 00:1c.1 = 8086:1e12]
MacBookPro10,2 2012 13" [Root Port 00:1c.1 = 8086:1e12]
For posterity, spurious interrupts caused by the Broadcom 4331 wireless
card resulted in splats like this (stacktrace omitted):
irq 17: nobody cared (try booting with the "irqpoll" option)
handlers:
[<ffffffff81374370>] pcie_isr
[<ffffffffc0704550>] sdhci_irq [sdhci] threaded [<ffffffffc07013c0>] sdhci_thread_irq [sdhci]
[<ffffffffc0a0b960>] azx_interrupt [snd_hda_codec]
Disabling IRQ #17
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79301
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=111781
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=728916
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=895951#c16
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1009819
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1098621
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1149632#c5
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1279130
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1332732
Tested-by: Konstantin Simanov <k.simanov@stlk.ru> # [MacBookPro8,1]
Tested-by: Lukas Wunner <lukas@wunner.de> # [MacBookPro9,1]
Tested-by: Bryan Paradis <bryan.paradis@gmail.com> # [MacBookPro9,2]
Tested-by: Andrew Worsley <amworsley@gmail.com> # [MacBookPro10,1]
Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com> # [MacBookPro10,2]
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Chris Milsted <cmilsted@redhat.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Michael Buesch <m@bues.ch>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: b43-dev@lists.infradead.org
Cc: linux-pci@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Link: http://lkml.kernel.org/r/48d0972ac82a53d460e5fce77a07b2560db95203.1465690253.git.lukas@wunner.de
[ Did minor readability edits. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
[bwh: Backported to 3.2:
- early_ioremap() is declared in <asm/io.h> not <asm/early_ioremap.h>
- Add definition of BCMA_RESET_ST
- Adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
arch/x86/kernel/early-quirks.c | 64 ++++++++++++++++++++++++++++++++++++++++++
drivers/bcma/bcma_private.h | 2 --
include/linux/bcma/bcma.h | 1 +
3 files changed, 65 insertions(+), 2 deletions(-)
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -11,13 +11,20 @@
#include <linux/pci.h>
#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/dmi.h>
#include <linux/pci_ids.h>
+#include <linux/bcma/bcma.h>
+#include <linux/bcma/bcma_regs.h>
#include <asm/pci-direct.h>
#include <asm/dma.h>
#include <asm/io_apic.h>
#include <asm/apic.h>
#include <asm/iommu.h>
#include <asm/gart.h>
+#include <asm/io.h>
+
+#define dev_err(msg) pr_err("pci 0000:%02x:%02x.%d: %s", bus, slot, func, msg)
static void __init fix_hypertransport_config(int num, int slot, int func)
{
@@ -199,6 +206,62 @@ static void __init ati_bugs_contd(int nu
}
#endif
+#define BCM4331_MMIO_SIZE 16384
+#define BCM4331_PM_CAP 0x40
+#define bcma_aread32(reg) ioread32(mmio + 1 * BCMA_CORE_SIZE + reg)
+#define bcma_awrite32(reg, val) iowrite32(val, mmio + 1 * BCMA_CORE_SIZE + reg)
+
+static void __init apple_airport_reset(int bus, int slot, int func)
+{
+ void __iomem *mmio;
+ u16 pmcsr;
+ u64 addr;
+ int i;
+
+ if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc."))
+ return;
+
+ /* Card may have been put into PCI_D3hot by grub quirk */
+ pmcsr = read_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL);
+
+ if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
+ pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
+ write_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL, pmcsr);
+ mdelay(10);
+
+ pmcsr = read_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL);
+ if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
+ dev_err("Cannot power up Apple AirPort card\n");
+ return;
+ }
+ }
+
+ addr = read_pci_config(bus, slot, func, PCI_BASE_ADDRESS_0);
+ addr |= (u64)read_pci_config(bus, slot, func, PCI_BASE_ADDRESS_1) << 32;
+ addr &= PCI_BASE_ADDRESS_MEM_MASK;
+
+ mmio = early_ioremap(addr, BCM4331_MMIO_SIZE);
+ if (!mmio) {
+ dev_err("Cannot iomap Apple AirPort card\n");
+ return;
+ }
+
+ pr_info("Resetting Apple AirPort card (left enabled by EFI)\n");
+
+ for (i = 0; bcma_aread32(BCMA_RESET_ST) && i < 30; i++)
+ udelay(10);
+
+ bcma_awrite32(BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
+ bcma_aread32(BCMA_RESET_CTL);
+ udelay(1);
+
+ bcma_awrite32(BCMA_RESET_CTL, 0);
+ bcma_aread32(BCMA_RESET_CTL);
+ udelay(10);
+
+ early_iounmap(mmio, BCM4331_MMIO_SIZE);
+}
+
#define QFLAG_APPLY_ONCE 0x1
#define QFLAG_APPLIED 0x2
#define QFLAG_DONE (QFLAG_APPLY_ONCE|QFLAG_APPLIED)
@@ -222,6 +285,8 @@ static struct chipset early_qrk[] __init
PCI_CLASS_SERIAL_SMBUS, PCI_ANY_ID, 0, ati_bugs },
{ PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_SBX00_SMBUS,
PCI_CLASS_SERIAL_SMBUS, PCI_ANY_ID, 0, ati_bugs_contd },
+ { PCI_VENDOR_ID_BROADCOM, 0x4331,
+ PCI_CLASS_NETWORK_OTHER, PCI_ANY_ID, 0, apple_airport_reset},
{}
};
--- a/drivers/bcma/bcma_private.h
+++ b/drivers/bcma/bcma_private.h
@@ -8,8 +8,6 @@
#include <linux/bcma/bcma.h>
#include <linux/delay.h>
-#define BCMA_CORE_SIZE 0x1000
-
struct bcma_bus;
/* main.c */
--- a/include/linux/bcma/bcma.h
+++ b/include/linux/bcma/bcma.h
@@ -124,6 +124,7 @@ struct bcma_host_ops {
#define BCMA_CORE_DEFAULT 0xFFF
#define BCMA_MAX_NR_CORES 16
+#define BCMA_CORE_SIZE 0x1000
struct bcma_device {
struct bcma_bus *bus;
--- a/include/linux/bcma/bcma_regs.h
+++ b/include/linux/bcma/bcma_regs.h
@@ -35,6 +35,7 @@
#define BCMA_IOST_BIST_DONE 0x8000
#define BCMA_RESET_CTL 0x0800
#define BCMA_RESET_CTL_RESET 0x0001
+#define BCMA_RESET_ST 0x0804
/* BCMA PCI config space registers. */
#define BCMA_PCI_PMCSR 0x44
^ permalink raw reply
* [PATCH 3.16 057/346] x86/quirks: Add early quirk to reset Apple AirPort card
From: Ben Hutchings @ 2016-11-14 0:14 UTC (permalink / raw)
To: linux-kernel, stable
Cc: akpm, Denys Vlasenko, Peter Zijlstra, Brian Gerst, Michael Buesch,
Lukas Wunner, Chris Milsted, Yinghai Lu, Linus Torvalds,
H. Peter Anvin, Matthew Garrett, Rafał Miłecki,
Thomas Gleixner, linux-pci, Ingo Molnar, linux-wireless,
Borislav Petkov, Josh Poimboeuf, b43-dev, Matt Fleming,
Konstantin Simanov, Andy Lutomirski, Andrew Worsley,
Bryan Paradis, Chris Bainbridge, Bjorn Helgaas
In-Reply-To: <lsq.1479082458.755945576@decadent.org.uk>
3.16.39-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Lukas Wunner <lukas@wunner.de>
commit abb2bafd295fe962bbadc329dbfb2146457283ac upstream.
The EFI firmware on Macs contains a full-fledged network stack for
downloading OS X images from osrecovery.apple.com. Unfortunately
on Macs introduced 2011 and 2012, EFI brings up the Broadcom 4331
wireless card on every boot and leaves it enabled even after
ExitBootServices has been called. The card continues to assert its IRQ
line, causing spurious interrupts if the IRQ is shared. It also corrupts
memory by DMAing received packets, allowing for remote code execution
over the air. This only stops when a driver is loaded for the wireless
card, which may be never if the driver is not installed or blacklisted.
The issue seems to be constrained to the Broadcom 4331. Chris Milsted
has verified that the newer Broadcom 4360 built into the MacBookPro11,3
(2013/2014) does not exhibit this behaviour. The chances that Apple will
ever supply a firmware fix for the older machines appear to be zero.
The solution is to reset the card on boot by writing to a reset bit in
its mmio space. This must be done as an early quirk and not as a plain
vanilla PCI quirk to successfully combat memory corruption by DMAed
packets: Matthew Garrett found out in 2012 that the packets are written
to EfiBootServicesData memory (http://mjg59.dreamwidth.org/11235.html).
This type of memory is made available to the page allocator by
efi_free_boot_services(). Plain vanilla PCI quirks run much later, in
subsys initcall level. In-between a time window would be open for memory
corruption. Random crashes occurring in this time window and attributed
to DMAed packets have indeed been observed in the wild by Chris
Bainbridge.
When Matthew Garrett analyzed the memory corruption issue in 2012, he
sought to fix it with a grub quirk which transitions the card to D3hot:
http://git.savannah.gnu.org/cgit/grub.git/commit/?id=9d34bb85da56
This approach does not help users with other bootloaders and while it
may prevent DMAed packets, it does not cure the spurious interrupts
emanating from the card. Unfortunately the card's mmio space is
inaccessible in D3hot, so to reset it, we have to undo the effect of
Matthew's grub patch and transition the card back to D0.
Note that the quirk takes a few shortcuts to reduce the amount of code:
The size of BAR 0 and the location of the PM capability is identical
on all affected machines and therefore hardcoded. Only the address of
BAR 0 differs between models. Also, it is assumed that the BCMA core
currently mapped is the 802.11 core. The EFI driver seems to always take
care of this.
Michael Büsch, Bjorn Helgaas and Matt Fleming contributed feedback
towards finding the best solution to this problem.
The following should be a comprehensive list of affected models:
iMac13,1 2012 21.5" [Root Port 00:1c.3 = 8086:1e16]
iMac13,2 2012 27" [Root Port 00:1c.3 = 8086:1e16]
Macmini5,1 2011 i5 2.3 GHz [Root Port 00:1c.1 = 8086:1c12]
Macmini5,2 2011 i5 2.5 GHz [Root Port 00:1c.1 = 8086:1c12]
Macmini5,3 2011 i7 2.0 GHz [Root Port 00:1c.1 = 8086:1c12]
Macmini6,1 2012 i5 2.5 GHz [Root Port 00:1c.1 = 8086:1e12]
Macmini6,2 2012 i7 2.3 GHz [Root Port 00:1c.1 = 8086:1e12]
MacBookPro8,1 2011 13" [Root Port 00:1c.1 = 8086:1c12]
MacBookPro8,2 2011 15" [Root Port 00:1c.1 = 8086:1c12]
MacBookPro8,3 2011 17" [Root Port 00:1c.1 = 8086:1c12]
MacBookPro9,1 2012 15" [Root Port 00:1c.1 = 8086:1e12]
MacBookPro9,2 2012 13" [Root Port 00:1c.1 = 8086:1e12]
MacBookPro10,1 2012 15" [Root Port 00:1c.1 = 8086:1e12]
MacBookPro10,2 2012 13" [Root Port 00:1c.1 = 8086:1e12]
For posterity, spurious interrupts caused by the Broadcom 4331 wireless
card resulted in splats like this (stacktrace omitted):
irq 17: nobody cared (try booting with the "irqpoll" option)
handlers:
[<ffffffff81374370>] pcie_isr
[<ffffffffc0704550>] sdhci_irq [sdhci] threaded [<ffffffffc07013c0>] sdhci_thread_irq [sdhci]
[<ffffffffc0a0b960>] azx_interrupt [snd_hda_codec]
Disabling IRQ #17
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79301
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=111781
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=728916
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=895951#c16
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1009819
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1098621
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1149632#c5
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1279130
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1332732
Tested-by: Konstantin Simanov <k.simanov@stlk.ru> # [MacBookPro8,1]
Tested-by: Lukas Wunner <lukas@wunner.de> # [MacBookPro9,1]
Tested-by: Bryan Paradis <bryan.paradis@gmail.com> # [MacBookPro9,2]
Tested-by: Andrew Worsley <amworsley@gmail.com> # [MacBookPro10,1]
Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com> # [MacBookPro10,2]
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Rafał Miłecki <zajec5@gmail.com>
Acked-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Chris Milsted <cmilsted@redhat.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Michael Buesch <m@bues.ch>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: b43-dev@lists.infradead.org
Cc: linux-pci@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Link: http://lkml.kernel.org/r/48d0972ac82a53d460e5fce77a07b2560db95203.1465690253.git.lukas@wunner.de
[ Did minor readability edits. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
arch/x86/kernel/early-quirks.c | 64 ++++++++++++++++++++++++++++++++++++++++++
drivers/bcma/bcma_private.h | 2 --
include/linux/bcma/bcma.h | 1 +
3 files changed, 65 insertions(+), 2 deletions(-)
--- a/arch/x86/kernel/early-quirks.c
+++ b/arch/x86/kernel/early-quirks.c
@@ -11,7 +11,11 @@
#include <linux/pci.h>
#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/dmi.h>
#include <linux/pci_ids.h>
+#include <linux/bcma/bcma.h>
+#include <linux/bcma/bcma_regs.h>
#include <drm/i915_drm.h>
#include <asm/pci-direct.h>
#include <asm/dma.h>
@@ -21,6 +25,9 @@
#include <asm/iommu.h>
#include <asm/gart.h>
#include <asm/irq_remapping.h>
+#include <asm/early_ioremap.h>
+
+#define dev_err(msg) pr_err("pci 0000:%02x:%02x.%d: %s", bus, slot, func, msg)
static void __init fix_hypertransport_config(int num, int slot, int func)
{
@@ -572,6 +579,61 @@ static void __init force_disable_hpet(in
#endif
}
+#define BCM4331_MMIO_SIZE 16384
+#define BCM4331_PM_CAP 0x40
+#define bcma_aread32(reg) ioread32(mmio + 1 * BCMA_CORE_SIZE + reg)
+#define bcma_awrite32(reg, val) iowrite32(val, mmio + 1 * BCMA_CORE_SIZE + reg)
+
+static void __init apple_airport_reset(int bus, int slot, int func)
+{
+ void __iomem *mmio;
+ u16 pmcsr;
+ u64 addr;
+ int i;
+
+ if (!dmi_match(DMI_SYS_VENDOR, "Apple Inc."))
+ return;
+
+ /* Card may have been put into PCI_D3hot by grub quirk */
+ pmcsr = read_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL);
+
+ if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
+ pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
+ write_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL, pmcsr);
+ mdelay(10);
+
+ pmcsr = read_pci_config_16(bus, slot, func, BCM4331_PM_CAP + PCI_PM_CTRL);
+ if ((pmcsr & PCI_PM_CTRL_STATE_MASK) != PCI_D0) {
+ dev_err("Cannot power up Apple AirPort card\n");
+ return;
+ }
+ }
+
+ addr = read_pci_config(bus, slot, func, PCI_BASE_ADDRESS_0);
+ addr |= (u64)read_pci_config(bus, slot, func, PCI_BASE_ADDRESS_1) << 32;
+ addr &= PCI_BASE_ADDRESS_MEM_MASK;
+
+ mmio = early_ioremap(addr, BCM4331_MMIO_SIZE);
+ if (!mmio) {
+ dev_err("Cannot iomap Apple AirPort card\n");
+ return;
+ }
+
+ pr_info("Resetting Apple AirPort card (left enabled by EFI)\n");
+
+ for (i = 0; bcma_aread32(BCMA_RESET_ST) && i < 30; i++)
+ udelay(10);
+
+ bcma_awrite32(BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
+ bcma_aread32(BCMA_RESET_CTL);
+ udelay(1);
+
+ bcma_awrite32(BCMA_RESET_CTL, 0);
+ bcma_aread32(BCMA_RESET_CTL);
+ udelay(10);
+
+ early_iounmap(mmio, BCM4331_MMIO_SIZE);
+}
#define QFLAG_APPLY_ONCE 0x1
#define QFLAG_APPLIED 0x2
@@ -610,6 +672,8 @@ static struct chipset early_qrk[] __init
*/
{ PCI_VENDOR_ID_INTEL, 0x0f00,
PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, force_disable_hpet},
+ { PCI_VENDOR_ID_BROADCOM, 0x4331,
+ PCI_CLASS_NETWORK_OTHER, PCI_ANY_ID, 0, apple_airport_reset},
{}
};
--- a/drivers/bcma/bcma_private.h
+++ b/drivers/bcma/bcma_private.h
@@ -8,8 +8,6 @@
#include <linux/bcma/bcma.h>
#include <linux/delay.h>
-#define BCMA_CORE_SIZE 0x1000
-
#define bcma_err(bus, fmt, ...) \
pr_err("bus%d: " fmt, (bus)->num, ##__VA_ARGS__)
#define bcma_warn(bus, fmt, ...) \
--- a/include/linux/bcma/bcma.h
+++ b/include/linux/bcma/bcma.h
@@ -153,6 +153,7 @@ struct bcma_host_ops {
#define BCMA_CORE_DEFAULT 0xFFF
#define BCMA_MAX_NR_CORES 16
+#define BCMA_CORE_SIZE 0x1000
/* Chip IDs of PCIe devices */
#define BCMA_CHIP_ID_BCM4313 0x4313
^ permalink raw reply
* RE: Problems getting mwifiex with sd8887 to work
From: Amitkumar Karwar @ 2016-11-14 5:01 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-wireless@vger.kernel.org, Nishant Sarmukadam, Kalle Valo
In-Reply-To: <20161113141348.GA32086@katana>
> From: Wolfram Sang [mailto:wsa@the-dreams.de]
> Sent: Sunday, November 13, 2016 7:44 PM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Nishant Sarmukadam; Kalle Valo
> Subject: Re: Problems getting mwifiex with sd8887 to work
>
> > 0x242 command failure is expected for sd8887, as It's not supported.
>
> Is the same true for 0x10f?
>
> mwifiex_sdio mmc2:0001:1: CMD_RESP: cmd 0x10f error, result=0x2
Yes. Error code 2 here means command not supported by firmware.
Regards,
Amitkumar
^ permalink raw reply
* Re: Outdated wl12xx firmwares
From: Luca Coelho @ 2016-11-14 7:37 UTC (permalink / raw)
To: Gary Bisson, linux-firmware, Maxim Altshul; +Cc: linux-wireless
In-Reply-To: <CAAMH-yvdCMMqMc5HBGtPTEYFS5s48hyRaBtQdEiM-GCDwbFcrA@mail.gmail.com>
Hi,
I'm CCing Maxim, since he seems to be the last developer from TI to
post patches for the WiLink drivers here.
On Thu, 2016-10-27 at 12:45 +0200, Gary Bisson wrote:
> Hi,
>
> Adding linux-wireless in CC, hoping to get more traction.
>
> Just checked the MAINTAINERS file of this driver and it is listed as
> 'Orphan', so is it ok if a non-TI person submits the up-to-date
> firmware files?
AFAICT, the license text in the github repository you pointed to is not
exactly the same as the one in linux-firmware.git. So, not being a
lawyer, I don't know how you could handle that, since the license in
github explicitly says that you need to reproduce that license.
> Regards,
> Gary
>
> On Fri, Oct 14, 2016 at 7:32 PM, Gary Bisson
> <gary.bisson@boundarydevices.com> wrote:
> > Hi,
> >
> > I am wondering why the w127x and wl128x firmware files have not been
> > updated to their latest version (since Jan 2014)?
> >
> > The current version in the linux-firmware repo are:
> > - 6.3.10.0.133 for single-role
> > - 6.5.7.0.42 for multi-role
> >
> > Looking at TI's forum[1], they recommend using the latest one to avoid
> > issues like firmware reload:
> > - 6.3.10.0.139 for single-role
> > - 6.5.7.0.47 for multi-role
> >
> > The up-to-date firmware files can be found in the following repo:
> > https://github.com/TI-OpenLink/ti-utils/tree/master/hw/firmware
> >
> > What is the procedure to update them, does it need to be a TI employee
> > that sends the patch? The former wl12xx maintainer (Luciano Coelho)
> > seems to work for Intel now.
Right, I'm not working for TI anymore, so there's nothing that I can do
for you. If I were you, I'd keep poking at current TI developers to
see if they can send these new FW versions to linux-firmware.git. They
probably even have newer versions than the ones published in github.
> > Regards,
> > Gary
> >
> > [1] https://e2e.ti.com/support/wireless_connectivity/wilink_wifi_bluetooth/f/307/p/350832/1236099#1236099
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE
From: Stanislaw Gruszka @ 2016-11-14 8:42 UTC (permalink / raw)
To: Mathias Kresin; +Cc: linux-wireless, Helmut Schaa
In-Reply-To: <48e4790b-7cb3-170e-eda1-11bce9934141@kresin.me>
On Sat, Nov 05, 2016 at 01:55:14PM +0100, Mathias Kresin wrote:
> 02.11.2016 15:10, Stanislaw Gruszka:
> >We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on
> >rt2800_init_registers().
> >
> >Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> >---
> > drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> >
> >diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >index feceb13..9ecdc4c 100644
> >--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> >@@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev)
> > rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, ®);
> > rt2x00_set_field32(®, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1);
> > rt2x00_set_field32(®, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1);
> >- rt2x00_set_field32(®, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2);
>
> More a notice than a potential issue since I don't have much
> knowledge about the driver/chip internals.
>
> But WPDMA_GLO_CFG_WP_DMA_BURST_SIZE in rt2800_init_registers() is
> set conditionally for rt2x00_is_usb(rt2x00dev), where this one is
> set unconditionally. Not sure if this change has side effects.
Default HW setting of WP_DMA_BURTS_SIZE is 2, hence patch does not
change behaviour on PCI devices. However I looked at RT3290 and
RT5592 PCI vendor drivers and there this value is initialized to 3.
So I think we can remove is_usb condition in rt2800_init_registers().
Stanislaw
^ permalink raw reply
* Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor
From: Stanislaw Gruszka @ 2016-11-14 8:45 UTC (permalink / raw)
To: Mathias Kresin; +Cc: linux-wireless, Helmut Schaa
In-Reply-To: <ebc4779b-a68b-f3b3-c8fc-87db9401a603@kresin.me>
On Sat, Nov 05, 2016 at 01:56:58PM +0100, Mathias Kresin wrote:
> 02.11.2016 15:11, Stanislaw Gruszka:
> >
> >- sta_priv = sta_to_rt2x00_sta(sta);
> >+ txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
> > txdesc->u.ht.wcid = sta_priv->wcid;
> >+
> >+ if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
> >+ ba_size = IEEE80211_MIN_AMPDU_BUF;
> >+ ba_size <<= sta->ht_cap.ampdu_factor;
> >+ ba_size = min_t(int, 63, ba_size - 1);
> >+ }
> > }
> >
> > /*
> >@@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
> > return;
> > }
> >
> >- txdesc->u.ht.ba_size = 7; /* FIXME: What value is needed? */
> >+ txdesc->u.ht.ba_size = ba_size;
> >
> > /*
> > * Only one STBC stream is supported for now.
> >
>
> Having this patch applied, the throughput on a vgv7510kw22 (RT3062F)
> in AP mode using LEDE head is decreased by somewhat around 10
> Mbits/sec. I'm using iperf3 for throughput tests and having this
> patch reverted the throughout is back to 80 Mbits/sec.
>
> When bringing down the wifi interface the following messages are
> logged with the patch applied:
>
> [ 281.738373] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
> Queue 2 failed to flush
> [ 281.906380] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
> Queue 2 failed to flush
Could you check below patch and see if it helps? If it does not,
could you printk sta->ht_cap.ampdu_density and ba_size values
and provide them here.
Thanks
Stanislaw
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 2515702..72c7948 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -4707,7 +4707,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
rt2x00_rt_rev_lt(rt2x00dev, RT3070, REV_RT3070E))
rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 2);
else
- rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 1);
+ rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 3;
rt2x00_set_field32(®, MAX_LEN_CFG_MIN_PSDU, 0);
rt2x00_set_field32(®, MAX_LEN_CFG_MIN_MPDU, 0);
rt2800_register_write(rt2x00dev, MAX_LEN_CFG, reg);
^ permalink raw reply related
* Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
From: Johannes Berg @ 2016-11-14 9:40 UTC (permalink / raw)
To: IgorMitsyanko
Cc: linux-wireless, btherthala, hwang, smaksimenko, dlebed,
Igor Mitsyanko, Kamlesh Rath, Sergey Matyukevich, Avinash Patil,
Ben Hutchings, Kyle McMartin
In-Reply-To: <a5075f50-25c6-18a1-bd5b-309927dacdab@quantenna.com>
On Mon, 2016-11-14 at 11:26 +0300, IgorMitsyanko wrote:
> Thanks, Johannes.
>
> To clarify with you and Kalle, as persons involved with linux-
> wireless: is my understanding correct that submitting firmware into
> linux-fimware repository is a prerequisite to accepting new driver
> into linux-wireless?
I personally don't think it needs to be quite that strict, but I don't
think I've heard anyone else express an opinion either way.
Frankly, so far I only commented on the firmware patch because I
haven't had time to go over the driver code at all.
johannes
^ permalink raw reply
* Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor
From: Julian Calaby @ 2016-11-14 9:41 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Mathias Kresin, linux-wireless, Helmut Schaa
In-Reply-To: <20161114084536.GB12372@redhat.com>
Hi Stainslaw,
On Mon, Nov 14, 2016 at 7:45 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Sat, Nov 05, 2016 at 01:56:58PM +0100, Mathias Kresin wrote:
>> 02.11.2016 15:11, Stanislaw Gruszka:
>> >
>> >- sta_priv = sta_to_rt2x00_sta(sta);
>> >+ txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
>> > txdesc->u.ht.wcid = sta_priv->wcid;
>> >+
>> >+ if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
>> >+ ba_size = IEEE80211_MIN_AMPDU_BUF;
>> >+ ba_size <<= sta->ht_cap.ampdu_factor;
>> >+ ba_size = min_t(int, 63, ba_size - 1);
>> >+ }
>> > }
>> >
>> > /*
>> >@@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
>> > return;
>> > }
>> >
>> >- txdesc->u.ht.ba_size = 7; /* FIXME: What value is needed? */
>> >+ txdesc->u.ht.ba_size = ba_size;
>> >
>> > /*
>> > * Only one STBC stream is supported for now.
>> >
>>
>> Having this patch applied, the throughput on a vgv7510kw22 (RT3062F)
>> in AP mode using LEDE head is decreased by somewhat around 10
>> Mbits/sec. I'm using iperf3 for throughput tests and having this
>> patch reverted the throughout is back to 80 Mbits/sec.
>>
>> When bringing down the wifi interface the following messages are
>> logged with the patch applied:
>>
>> [ 281.738373] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
>> Queue 2 failed to flush
>> [ 281.906380] ieee80211 phy0: rt2x00queue_flush_queue: Warning -
>> Queue 2 failed to flush
>
> Could you check below patch and see if it helps? If it does not,
> could you printk sta->ht_cap.ampdu_density and ba_size values
> and provide them here.
>
> Thanks
> Stanislaw
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index 2515702..72c7948 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -4707,7 +4707,7 @@ static int rt2800_init_registers(struct rt2x00_dev *rt2x00dev)
> rt2x00_rt_rev_lt(rt2x00dev, RT3070, REV_RT3070E))
> rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 2);
> else
> - rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 1);
> + rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 3;
You're missing a closing parenthesis, so it isn't going to work unless
it's added back in.
> rt2x00_set_field32(®, MAX_LEN_CFG_MIN_PSDU, 0);
> rt2x00_set_field32(®, MAX_LEN_CFG_MIN_MPDU, 0);
> rt2800_register_write(rt2x00dev, MAX_LEN_CFG, reg);
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
^ permalink raw reply
* Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor
From: Stanislaw Gruszka @ 2016-11-14 9:44 UTC (permalink / raw)
To: Julian Calaby; +Cc: Mathias Kresin, linux-wireless, Helmut Schaa
In-Reply-To: <CAGRGNgU=3tRrdgzoiGjoGr6QguyYnQ2kadiaX+HWxm0jiKqaqw@mail.gmail.com>
Hi
On Mon, Nov 14, 2016 at 08:41:57PM +1100, Julian Calaby wrote:
> > - rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 1);
> > + rt2x00_set_field32(®, MAX_LEN_CFG_MAX_PSDU, 3;
>
> You're missing a closing parenthesis, so it isn't going to work unless
> it's added back in.
Thanks for the notice, hopefully Mathias can fix that.
Stanislaw
^ permalink raw reply
* Re: [PATCH V3] qtnfmac: announcement of new FullMAC driver for Quantenna chipsets
From: Johannes Berg @ 2016-11-14 9:52 UTC (permalink / raw)
To: igor.mitsyanko.os, linux-wireless
Cc: btherthala, hwang, smaksimenko, dlebed, Sergey Matyukevich,
Kamlesh Rath, Avinash Patil
In-Reply-To: <1478700000-11624-2-git-send-email-igor.mitsyanko.os@quantenna.com>
> +++ b/drivers/net/wireless/quantenna/qtnfmac/Kconfig
> @@ -0,0 +1,23 @@
> +config QTNFMAC
> + tristate "Quantenna WiFi FullMAC WLAN driver"
> + default n
No need to state that default explicitly, but it also doesn't really
matter.
> + it as a module, it will be called qtnfmac_pearl_pcie.ko.
Where did the "pearl" come from? You didn't mention that in the commit
log.
Also, are you planning to add any other things to this soon? It seems
to me that perhaps the PCIe option should be the only one that's user-
visible, selecting the other one internally?
> + struct qtnf_bus_ops *bus_ops;
You should make that const and actually make all instances const, it's
better for security :)
> +/* Supported crypto cipher suits to be advertised to cfg80211 */
> +static const u32 qtnf_cipher_suites[] = {
> + WLAN_CIPHER_SUITE_TKIP,
> + WLAN_CIPHER_SUITE_CCMP,
> + WLAN_CIPHER_SUITE_AES_CMAC,
> +};
I'm surprised - no WEP? No CMAC and GCMP/GMAC?
> +static int
> +qtnf_change_virtual_intf(struct wiphy *wiphy,
> + struct net_device *dev,
> + enum nl80211_iftype type, u32 *flags,
> + struct vif_params *params)
> +{
> + struct qtnf_vif *vif;
> + u8 *mac_addr;
> +
> + vif = qtnf_netdev_get_priv(dev);
> +
> + if (params)
> + mac_addr = params->macaddr;
> + else
> + mac_addr = NULL;
> +
> + if (qtnf_cmd_send_change_intf_type(vif, type, mac_addr)) {
> + pr_err("failed to change interface type\n");
> + return -EFAULT;
Maybe -EIO would be better.
> +struct wireless_dev *qtnf_add_virtual_intf(struct wiphy *wiphy,
> + const char *name,
> + unsigned char
> name_assign_type,
> + enum nl80211_iftype type,
> + u32 *flags,
> + struct vif_params
> *params)
> +{
> + struct qtnf_wmac *mac;
> + struct qtnf_vif *vif;
> + u8 *mac_addr = NULL;
> +
> + mac = wiphy_priv(wiphy);
> +
> + if (!mac)
> + return ERR_PTR(-EFAULT);
> +
> + switch (type) {
> + case NL80211_IFTYPE_STATION:
> + case NL80211_IFTYPE_AP:
> + vif = qtnf_get_free_vif(mac);
> + if (!vif) {
> + pr_err("could not get free private
> structure\n");
> + return ERR_PTR(-EFAULT);
> + }
This is probably fairly natural to do, and it's not the first driver to
do this (brcmfmac also does), but some users may assume that they can
add interfaces as much as they want, until they bring them up (netdev
open).
It's probably worth having a discussion about this behaviour difference
- not necessarily in the context of this driver submission though.
> + if (qtnf_cmd_send_add_intf(vif, type, mac_addr)) {
> + vif->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
> + pr_err("failed to send add_intf command\n");
> + return ERR_PTR(-EFAULT);
I can't really tell, does this leak "vif"?
OK I need to find another way to review this patch now, it's too big
for my email client to work responsively...
johannes
^ permalink raw reply
* Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
From: IgorMitsyanko @ 2016-11-14 8:26 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless, btherthala, hwang, smaksimenko, dlebed,
Igor Mitsyanko, Kamlesh Rath, Sergey Matyukevich, Avinash Patil,
Ben Hutchings, Kyle McMartin
In-Reply-To: <1478864146.4129.4.camel@sipsolutions.net>
Thanks, Johannes.
To clarify with you and Kalle, as persons involved with linux-wireless:
is my understanding correct that submitting firmware into linux-fimware
repository is a prerequisite to accepting new driver into linux-wireless?
There is an option to start Quantenna device from internal flash memory,
no external binary files involved. If we will introduce this
functionality and remove code handling external firmware for now (until
firmware problem resolved), would that allow driver to be reviewed/accepted?
On 11/11/2016 02:35 PM, Johannes Berg wrote:
> Adding linux-firmware people to Cc, since presumably they don't
> necessarily read linux-wireless...
>
>> Johannes, from that perspective, who are the "redistributors"?
>> Specifically, is linux-firmware git repository considered a
>> redistributor or its just hosting files? I mean, at what moment
>> someone else other then Quantenna will start to be legally obliged to
>> make GPL code used in firmware available for others?
> Look, I don't know. I'd assume people who ship it, like any regular
> distro, would be (re)distributors thereof. "Normal" (non-GPL) firmware
> images come with a redistribution license, but that obviously can't
> work here.
>
> There's some info from Ben here regarding the carl9170 case:
> http://lkml.iu.edu/hypermail/linux/kernel/1605.3/01176.html
>
>> Personally I still hope that linux-firmware itself is not legally
>> concerned with what is the content of firmware its hosting, but looks
>> like there already was a precedent case with carl9170 driver and
>> we have to somehow deal with it.
> That's really all I wanted to bring up. I'm not involved with the
> linux-firmware git tree.
>
>> There still may be a difference though: Quantenna is semiconductor
>> company only, software
>> used on actual products based on Quantenna chipsets is released by
>> other
>> companies.
>> I just want to present our legal team with a clear case (and position
>> of
>> Linux maintainers) so that they can
>> work with it and make decision on how to proceed.
>>
>> From technical perspective, as I mentioned, SDK is quite huge and
>> include a lot of opensource
>> components including full Linux, I don't think its reasonable to have
>> it
>> inside linux-firmware tree.
>> What are the options to share it other then providing it on request
>> basis:
>> - git repository
>> - store tarball somewhere on official website
> Clearly that wasn't deemed appropriate for carl9170, so I don't see why
> it'd be different here.
>
> johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox