[PATCH v3] brcmfmac: fix CLM load error for legacy chips when user helper is enabled

[Wright Feng]

For legacy chips without CLM blob files, kernel with user helper function
returns -EAGAIN when we request_firmware(), and then driver got failed
when bringing up legacy chips. We expect the CLM blob file for legacy chip
is not existence in firmware path, but the -ENOENT error is transferred to
-EAGAIN in firmware_class.c with user helper.
Because of that, we continue with CLM data currently present in firmware
if getting error from doing request_firmware().

Signed-off-by: Wright Feng 
---
v2: remove retry from patch v1
v3: remove redundant log print
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 6a59d06..b0ef0e7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -182,12 +182,9 @@ static int brcmf_c_process_clm_blob(struct brcmf_if *ifp)
 
err = request_firmware(, clm_name, dev);
if (err) {
-   if (err == -ENOENT) {
-   brcmf_dbg(INFO, "continue with CLM data currently 
present in firmware\n");
-   return 0;
-   }
-   brcmf_err("request CLM blob file failed (%d)\n", err);
-   return err;
+   brcmf_info("no clm_blob available(%d), device may have limited 
channels available\n",
+  err);
+   return 0;
}
 
chunk_buf = kzalloc(sizeof(*chunk_buf) + MAX_CHUNK_LEN - 1, GFP_KERNEL);
-- 
1.9.1

Re: [PATCH] brcmfmac: Make sure CLM downloading is optional

[Bjorn Andersson]

On Mon 15 Jan 11:40 PST 2018, Arend van Spriel wrote:

> On 1/15/2018 6:10 PM, Bjorn Andersson wrote:
> > The presence of a CLM file is described as optional, but missing the clm
> > blob causes the preinit to return unsuccessfully. Fix this by ignoring
> > the return value of the brcmf_c_process_clm_blob().
> > 
> > Also remove the extra debug print, as brcmf_c_process_clm_blob() already
> > did print a useful error message before returning.
> > 
> > Fixes: fdd0bd88ceae ("brcmfmac: add CLM download support")
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Bjorn Andersson 
> > ---
> > 
> > This regression was introduced in v4.15-rc1, but I unfortunately didn't test
> > WiFi until now. Included a Cc to stable@ in case you choose to pick this up
> > after v4.15.
> 
> Hi Bjorn,
> 
> Thanks for looking into this. Actually there already have been a couple of
> fixes posted on the linux-wireless list. [1] was rejected, [2] is being
> discussed, and [2] was posted by me and I was awaiting response from the guy
> reporting it.
> 

Thanks for pointing me to these discussions, I did for some reason not
find them this morning. I don't see the need for the retry in [1], so I
think that's invalid.

I tested [2] and it works well for me, but I agree with Kalle that a
better description of why would be in order. Unfortunatley I can't find
it in my inbox...even though I'm subscribed to linux-wireless@.

The answer to Kalle's question should probably include a reference to
0542ad88fbdd ("firmware loader: Fix _request_firmware_load() return val
for fw load abort")

> Now the thing is that for old (Broadcom) firmware this is optional. Those
> firmwares don't even have CLM support and those that do have the CLM data
> embedded in firmware.

I don't know which applies to my device, but it at least doesn't come
with a dedicated clm blob - and the device won't receive any upgrades
and hence will never get a clm blob.

> However, Cypress wants to provide their customers with
> firmware without CLM data. For those devices it is not optional. I still
> prefer we add a mechanism to the driver to detect that, but we do not have
> that yet.
> 

That sounds reasonable, but I hope we can sort out the regression first
and then add such logic.

> Now regarding your patch some comments below ...
> 
> >   drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 8 ++--
> >   1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
> > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > index 6a59d0609d30..0c67ba6ae135 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > @@ -278,12 +278,8 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
> > }
> > ri->result = err;
> > 
> > -   /* Do any CLM downloading */
> > -   err = brcmf_c_process_clm_blob(ifp);
> > -   if (err < 0) {
> > -   brcmf_err("download CLM blob file failed, %d\n", err);
> > -   goto done;
> > -   }
> > +   /* Do any optional CLM downloading */
> > +   brcmf_c_process_clm_blob(ifp);
> 
> The error print is indeed redundant and should be removed here. However,
> brcmf_c_process_clm_blob() also returns -ENOMEM upon allocation failure and
> I would still fail the driver probe for that.
> 

Agreed, but as we want to let a few errors, specifically from the
firmware loader, slip by I believe it's better to check the return value
there instead. So I prefer Write's [2].

Regards,
Bjorn

Re: [PATCH for-4.15] ssb: Disable PCI host for PCI_DRIVERS_GENERIC

[Guenter Roeck]

On 01/15/2018 01:17 PM, James Hogan wrote:

Since commit d41e6858ba58 ("MIPS: Kconfig: Set default MIPS system type
as generic") changed the default MIPS platform to the "generic"
platform, which uses PCI_DRIVERS_GENERIC instead of PCI_DRIVERS_LEGACY,
various files in drivers/ssb/ have failed to build.

This is particularly due to the existence of struct pci_controller being
dependent on PCI_DRIVERS_LEGACY since commit c5611df96804 ("MIPS: PCI:
Introduce CONFIG_PCI_DRIVERS_LEGACY"), so add that dependency to Kconfig
to prevent these files being built for the "generic" platform including
all{yes,mod}config builds.

Fixes: c5611df96804 ("MIPS: PCI: Introduce CONFIG_PCI_DRIVERS_LEGACY")
Signed-off-by: James Hogan 
Cc: Michael Buesch 
Cc: Ralf Baechle 
Cc: Paul Burton 
Cc: Matt Redfearn 
Cc: Guenter Roeck 
Cc: linux-wireless@vger.kernel.org
Cc: linux-m...@linux-mips.org


Tested-by: Guenter Roeck 


---
  drivers/ssb/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index d8e4219c2324..71c73766ee22 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -32,7 +32,7 @@ config SSB_BLOCKIO
  
  config SSB_PCIHOST_POSSIBLE

bool
-   depends on SSB && (PCI = y || PCI = SSB)
+   depends on SSB && (PCI = y || PCI = SSB) && PCI_DRIVERS_LEGACY
default y
  
  config SSB_PCIHOST

Re: [PATCH v2] brcmfmac: fix CLM load error for legacy chips when user helper is enabled

[Wright Feng]

On 2018/1/16 上午 03:54, Arend van Spriel wrote:

On 1/15/2018 11:09 AM, Wright Feng wrote:




[...]


---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 12
++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 6a59d06..aaab0e6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -182,12 +182,12 @@ static int brcmf_c_process_clm_blob(struct
brcmf_if *ifp)

  err = request_firmware(, clm_name, dev);
  if (err) {
-if (err == -ENOENT) {
-brcmf_dbg(INFO, "continue with CLM data currently present
in firmware\n");
-return 0;
-}
-brcmf_err("request CLM blob file failed (%d)\n", err);
-return err;
+if (err == -ENOENT || err == -EAGAIN)
+brcmf_info("continue with CLM data in FW\n");
+else
+brcmf_err("request clm_blob failed(%d) continue with CLM
data in FW\n",
+  err);


Don't see much value in doing it this way. Either way we need to inform 
the user about the consequence of this, ie.:


 brcmf_info("no clm_blob available (%d). device may have limited 
channels available\n", err);

+return 0;
  }


Regards,
Arend

Thanks for the comment, I will post patch v3 with your suggestion later.
The patch will include one brcmf_info print and returning 0 regardless
of errors from request_firmware.

Regards,
Wright

Re: [PATCH v2 07/10] rtlwifi: btcoex: Add power_on_setting routine

[Pkshih]

On Mon, 2018-01-15 at 13:19 -0600, Larry Finger wrote:
> On 01/11/2018 01:09 AM, pks...@realtek.com wrote:
> > 
> > From: Ping-Ke Shih 
> > 
> > After mac power-on sequence, wifi will start to work so notify
> > btcoex the
> > event to configure registers especially related to antenna. This
> > will not
> > only help to assign antenna but also to yield better user
> > experience.
> > 
> > Signed-off-by: Ping-Ke Shih 
> > ---
> >   drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h | 1
> > +
> >   drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c  | 6
> > ++
> >   drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h  | 1
> > +
> >   drivers/net/wireless/realtek/rtlwifi/wifi.h   | 1
> > +
> >   4 files changed, 9 insertions(+)
> > 
> > diff --git
> > a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > index ea12b9d63a73..bc523af7ef88 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > @@ -608,6 +608,7 @@ extern struct btc_coexist gl_bt_coexist;
> >   
> >   bool exhalbtc_initlize_variables(void);
> >   bool exhalbtc_bind_bt_coex_withadapter(void *adapter);
> > +void exhalbtc_power_on_setting(struct btc_coexist *btcoexist);
> >   void exhalbtc_init_hw_config(struct btc_coexist *btcoexist, bool
> > wifi_only);
> >   void exhalbtc_init_coex_dm(struct btc_coexist *btcoexist);
> >   void exhalbtc_ips_notify(struct btc_coexist *btcoexist, u8 type);
> > diff --git
> > a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c
> > b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c
> > index 4d9e33078d4f..9e3623b0423c 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c
> > +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c
> > @@ -32,6 +32,7 @@
> >   static struct rtl_btc_ops rtl_btc_operation = {
> >     .btc_init_variables = rtl_btc_init_variables,
> >     .btc_init_hal_vars = rtl_btc_init_hal_vars,
> > +   .btc_power_on_setting = rtl_btc_power_on_setting,
> >     .btc_init_hw_config = rtl_btc_init_hw_config,
> >     .btc_ips_notify = rtl_btc_ips_notify,
> >     .btc_lps_notify = rtl_btc_lps_notify,
> > @@ -116,6 +117,11 @@ void rtl_btc_init_hal_vars(struct rtl_priv
> > *rtlpriv)
> >      */
> >   }
> >   
> > +void rtl_btc_power_on_setting(struct rtl_priv *rtlpriv)
> > +{
> > +   exhalbtc_power_on_setting(_bt_coexist);
> > +}
> > +
> >   void rtl_btc_init_hw_config(struct rtl_priv *rtlpriv)
> >   {
> >     u8 bt_exist;
> > diff --git
> > a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h
> > b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h
> > index 40f1ce8c8a06..9becfa59407d 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h
> > +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h
> > @@ -29,6 +29,7 @@
> >   
> >   void rtl_btc_init_variables(struct rtl_priv *rtlpriv);
> >   void rtl_btc_init_hal_vars(struct rtl_priv *rtlpriv);
> > +void rtl_btc_power_on_setting(struct rtl_priv *rtlpriv);
> >   void rtl_btc_init_hw_config(struct rtl_priv *rtlpriv);
> >   void rtl_btc_ips_notify(struct rtl_priv *rtlpriv, u8 type);
> >   void rtl_btc_lps_notify(struct rtl_priv *rtlpriv, u8 type);
> > diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h
> > b/drivers/net/wireless/realtek/rtlwifi/wifi.h
> > index 941694060f48..b27dbe10b163 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
> > +++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
> > @@ -2557,6 +2557,7 @@ struct bt_coexist_info {
> >   struct rtl_btc_ops {
> >     void (*btc_init_variables) (struct rtl_priv *rtlpriv);
> >     void (*btc_init_hal_vars) (struct rtl_priv *rtlpriv);
> > +   void (*btc_power_on_setting)(struct rtl_priv *rtlpriv);
> >     void (*btc_init_hw_config) (struct rtl_priv *rtlpriv);
> >     void (*btc_ips_notify) (struct rtl_priv *rtlpriv, u8
> > type);
> >     void (*btc_lps_notify)(struct rtl_priv *rtlpriv, u8
> > type);
> > 
> I do not see any place in the code where rtl_btc_power_on_setting()
> is called, 
> either directly or by callback. Did I miss it?
> 
This will be called by 8822be and 8723de during initializing hardware,
and originally 8723be will also need it (little ant_sel related) but
you had fixed it by the commits 6d6226928369 and a33fcba6ec01. I'll
check whether 8723be has to call this or not.

PK

Re: [PATCH v2 09/10] rtlwifi: btcoex: Add common function for qeurying BT information

[Pkshih]

On Mon, 2018-01-15 at 13:15 -0600, Larry Finger wrote:
> On 01/11/2018 01:09 AM, pks...@realtek.com wrote:
> > 
> > From: Ping-Ke Shih 
> > 
> > This commit implement the common function to sort old features, and
> > add
> > more new features that are get_supported_feature,
> > get_supported_version,
> > get_ant_det_val, ble_scan_type, ble_scan_para, bt_dev_info,
> > forbidden_slot_val, afh_map and etc.
> > 
> > Signed-off-by: Ping-Ke Shih 
> > ---
> >   .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c   | 309
> > ++---
> >   .../realtek/rtlwifi/btcoexist/halbtcoutsrc.h   |  70 +
> >   .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.c   |  48 +++-
> >   3 files changed, 394 insertions(+), 33 deletions(-)
> > 
> > diff --git
> > a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> > b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> > index 2be81fec789a..30d940cf3abf 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> > +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> > @@ -207,6 +207,102 @@ u8 rtl_get_hwpg_package_type(struct rtl_priv
> > *rtlpriv)
> >     return rtlhal->package_type;
> >   }
> >   
> > +static
> > +bool halbtc_is_hw_mailbox_exist(struct btc_coexist *btcoexist)
> > +{
> > +   if (IS_HARDWARE_TYPE_8812(btcoexist->adapter))
> > +   return false;
> > +   else
> > +   return true;
> Once you have returned false for 8812, you do not need the 'else'.
> Use a simple 
> ' return true;'. As this is the only objection in this patch,
> I will let it 
> pass. Some user with a suitable tool will change it.
> 
> > 
> > +}
> > +
> > +static
> > +bool halbtc_send_bt_mp_operation(struct btc_coexist *btcoexist, u8
> > op_code,
> > +    u8 *cmd, u32 len, unsigned long
> > wait_ms)
> --snip--
> 
> > 
> >   
> > diff --git
> > a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > index 5a7816ff6877..cbbf5e5a9c9b 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > @@ -278,6 +278,8 @@ enum btc_get_type {
> >     BTC_GET_U4_VENDOR,
> >     BTC_GET_U4_SUPPORTED_VERSION,
> >     BTC_GET_U4_SUPPORTED_FEATURE,
> > +   BTC_GET_U4_BT_DEVICE_INFO,
> > +   BTC_GET_U4_BT_FORBIDDEN_SLOT_VAL,
> >     BTC_GET_U4_WIFI_IQK_TOTAL,
> >     BTC_GET_U4_WIFI_IQK_OK,
> >     BTC_GET_U4_WIFI_IQK_FAIL,
> > @@ -459,6 +461,19 @@ typedefbool (*bfp_btc_get)(void
> > *btcoexist, u8 get_type, void *out_buf);
> >   
> >   typedef   bool (*bfp_btc_set)(void *btcoexist, u8 set_type,
> > void *in_buf);
> >   
> > +typedef u32 (*bfp_btc_get_bt_coex_supported_feature)(void
> > *btcoexist);
> > +
> > +typedef u32 (*bfp_btc_get_bt_coex_supported_version)(void
> > *btcoexist);
> > +
> > +typedef u8 (*bfp_btc_get_ant_det_val_from_bt)(void *btcoexist);
> > +
> > +typedef u8 (*bfp_btc_get_ble_scan_type_from_bt)(void *btcoexist);
> > +
> > +typedef u32 (*bfp_btc_get_ble_scan_para_from_bt)(void *btcoexist,
> > u8 scan_type);
> > +
> > +typedef bool (*bfp_btc_get_bt_afh_map_from_bt)(void *btcoexist, u8
> > map_type,
> > +      u8 *afh_map);
> > +
> >   typedef void (*bfp_btc_set_bt_reg)(void *btc_context, u8
> > reg_type, u32 offset,
> >        u32 value);
> I would prefer that you not add additional typedef statements, but I
> will let it 
> pass.
> 
Thank you for pointing out. I'll remove all typedef in next patchset.

> 
> Acked-by: Larry Finger 
> 
> --Please consider the environment before printing this e-mail.

Re: brcmfmac4329-sdio firmware load failed.

[Kyle Evans]

On Sat, Jan 13, 2018 at 10:19:19AM +0100, Arend van Spriel wrote:
> On 1/12/2018 9:18 PM, Kyle Evans wrote:
> > Thank you Arend. I updated to master again, v4.15-rc7+, and applied your
> > patch. All log snips are grabbed with dmesg|grep -E 'mmc0|brcm', as the
> > sdio device is on mmc0.
> 
> Could you follow the reply convention of not top-posting.
> 
> > Without patch [1], for reference...
> >
> > [0.00] Kernel command line: console=tty0 selinux=0
> > video=1280x800 root=/dev/mmcblk1p1 brcmfmac.bebug=0x2
> 
> You made a type in the kernel command line, ie. "bebug" should be 
> "debug". Anyway, that is not that important for now.
> 
> > [3.952561] mmc0: Invalid maximum block size, assuming 512 bytes
> > [4.010466] mmc0: SDHCI controller on c800.sdhci [c800.sdhci]
> > using ADMA
> > [4.338545] mmc0: queuing unknown CIS tuple 0x80 (50 bytes)
> > [4.347098] mmc0: queuing unknown CIS tuple 0x80 (7 bytes)
> > [4.350099] mmc0: queuing unknown CIS tuple 0x80 (7 bytes)
> > [4.378430] mmc0: queuing unknown CIS tuple 0x02 (1 bytes)
> > [4.388387] mmc0: new SDIO card at address 0001
> > [4.401773] brcmfmac: F1 signature read @0x1800=0x9934329
> > [4.407624] brcmfmac: brcmf_sdiod_regrw_helper: failed to read data
> > F1@0x080ac, err: -84
> > [4.410159] brcmfmac: brcmf_fw_map_chip_to_name: using
> > brcm/brcmfmac4329-sdio.bin for chip 0x004329(17193) rev 0x03
> > [4.781374] brcmfmac mmc0:0001:1: Direct firmware load for
> > brcm/brcmfmac4329-sdio.clm_blob failed with error -2
> > [4.781491] brcmfmac mmc0:0001:1: Falling back to user helper
> > [   69.601645] brcmfmac: brcmf_c_process_clm_blob: request CLM blob file
> > failed (-11)
> > [   69.601668] brcmfmac: brcmf_c_preinit_dcmds: download CLM blob file
> > failed, -11
> > [   69.601685] brcmfmac: brcmf_bus_started: failed: -11
> > [   69.601728] brcmfmac: brcmf_sdio_firmware_callback: dongle is not
> > responding
> >
> >
> > With patch [1]...
> >
> > [0.00] Kernel command line: console=tty0 selinux=0
> > video=1280x800 root=/dev/mmcblk1p1 brcmfmac.bebug=0x2
> > [3.954628] mmc0: Invalid maximum block size, assuming 512 bytes
> > [4.010608] mmc0: SDHCI controller on c800.sdhci [c800.sdhci]
> > using ADMA
> > [4.341727] mmc0: queuing unknown CIS tuple 0x80 (50 bytes)
> > [4.349595] mmc0: queuing unknown CIS tuple 0x80 (7 bytes)
> > [4.352695] mmc0: queuing unknown CIS tuple 0x80 (7 bytes)
> > [4.381292] mmc0: queuing unknown CIS tuple 0x02 (1 bytes)
> > [4.387377] mmc0: new SDIO card at address 0001
> > [4.399393] brcmfmac: F1 signature read @0x1800=0x9934329
> > [4.405883] brcmfmac: brcmf_sdiod_regrw_helper: failed to read data
> > F1@0x080ac, err: -84
> > [4.407207] brcmfmac: brcmf_fw_map_chip_to_name: using
> > brcm/brcmfmac4329-sdio.bin for chip 0x004329(17193) rev 0x03
> > [4.709436] brcmfmac mmc0:0001:1: Direct firmware load for
> > brcm/brcmfmac4329-sdio.clm_blob failed with error -2
> > [4.709517] brcmfmac mmc0:0001:1: Falling back to user helper
> > [   69.710122] brcmfmac mmc0:0001:1: Direct firmware load for
> > brcm/brcmfmac4329-sdio.clm_blob failed with error -2
> > [   69.710137] brcmfmac mmc0:0001:1: Falling back to user helper
> > [  131.054899] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0:
> > Sep  2 2011 14:48:19 version 4.220.48
> > [  131.072561] brcmfmac: brcmf_setup_wiphybands: rxchain error (-23)
> > [  131.388384] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  131.392390] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  131.920861] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  131.924183] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  132.593074] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  133.135973] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  133.138223] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  133.152149] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  134.311983] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  134.458577] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  134.461640] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> > [  134.555048] brcmfmac: _brcmf_set_multicast_list: Setting
> > BRCMF_C_SET_PROMISC failed, -23
> >
> >
> > I can now connect to an AP with the following caveats:
> >
> > 1) It takes about two minutes before the wlan device is available. For a
> > long while I didn't think it was working because I would check dmesg and
> > check ifconfig -a before the wlan would be present.
> 
> Those two minutes are a direct 

[PATCH for-4.15] ssb: Disable PCI host for PCI_DRIVERS_GENERIC

[James Hogan]

Since commit d41e6858ba58 ("MIPS: Kconfig: Set default MIPS system type
as generic") changed the default MIPS platform to the "generic"
platform, which uses PCI_DRIVERS_GENERIC instead of PCI_DRIVERS_LEGACY,
various files in drivers/ssb/ have failed to build.

This is particularly due to the existence of struct pci_controller being
dependent on PCI_DRIVERS_LEGACY since commit c5611df96804 ("MIPS: PCI:
Introduce CONFIG_PCI_DRIVERS_LEGACY"), so add that dependency to Kconfig
to prevent these files being built for the "generic" platform including
all{yes,mod}config builds.

Fixes: c5611df96804 ("MIPS: PCI: Introduce CONFIG_PCI_DRIVERS_LEGACY")
Signed-off-by: James Hogan 
Cc: Michael Buesch 
Cc: Ralf Baechle 
Cc: Paul Burton 
Cc: Matt Redfearn 
Cc: Guenter Roeck 
Cc: linux-wireless@vger.kernel.org
Cc: linux-m...@linux-mips.org
---
 drivers/ssb/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index d8e4219c2324..71c73766ee22 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -32,7 +32,7 @@ config SSB_BLOCKIO
 
 config SSB_PCIHOST_POSSIBLE
bool
-   depends on SSB && (PCI = y || PCI = SSB)
+   depends on SSB && (PCI = y || PCI = SSB) && PCI_DRIVERS_LEGACY
default y
 
 config SSB_PCIHOST
-- 
2.13.6

Re: [PATCH] bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets

[Guenter Roeck]

On 01/15/2018 12:30 PM, James Hogan wrote:

On Mon, Jan 15, 2018 at 12:05:48PM -0800, Guenter Roeck wrote:

On 01/15/2018 09:10 AM, Paul Burton wrote:

Hello,

On Mon, Jan 15, 2018 at 10:23:37AM +, James Hogan wrote:

On Sun, Jan 14, 2018 at 01:34:02PM -0800, Guenter Roeck wrote:

Mips builds with BCMA host mode enabled fail in mainline and -next
with:

In file included from include/linux/bcma/bcma.h:10:0,
   from drivers/bcma/bcma_private.h:9,
 from drivers/bcma/main.c:8:
include/linux/bcma/bcma_driver_pci.h:218:24: error:
field 'pci_controller' has incomplete type

Bisect points to commit d41e6858ba58c ("MIPS: Kconfig: Set default MIPS
system type as generic") as the culprit. Analysis shows that the commmit
changes PCI configuration and enables PCI_DRIVERS_GENERIC. This in turn
disables PCI_DRIVERS_LEGACY. 'struct pci_controller' is, however, only
defined if PCI_DRIVERS_LEGACY is enabled.

Ultimately that means that BCMA_DRIVER_PCI_HOSTMODE depends on
PCI_DRIVERS_LEGACY. Add the missing dependency.

Fixes: d41e6858ba58c ("MIPS: Kconfig: Set default MIPS system type as ...")


Well, technically I think commit c5611df96804 ("MIPS: PCI: Introduce
CONFIG_PCI_DRIVERS_LEGACY") is to blame (Cc'ing paul), and the first bad
commit would be commit eed0eabd12ef ("MIPS: generic: Introduce generic
DT-based board support") which selects PCI_DRIVERS_GENERIC and is the
only platform to do so. Both commits were first in v4.9-rc1 and I can
reproduce this problem at that latter commit with the appropriate
configuration.


Ah - yes if I recall correctly my assumption was that the MIPS-specific
struct pci_controller was only used by the MIPS-specific PCI drivers
under arch/mips/pci/, which are only built when configured for the
appropriate platform.

In this case use of that MIPS-specific struct pci_controller has spread
beyond arch/mips/ & the user can be configured in for platforms other
than the one that will actually use the driver, including the generic
platform which moves towards more generic PCI drivers in
drivers/pci/host/.


But yes clearly the mentioned commit does also expose that existing
problem more widely and to the default allmodconfig, and it looks like a
reasonable approach for now, so if some mention of the other two commits
is added:

Reviewed-by: James Hogan 


Likewise, with the "Fixes:" tag fixed:

  Reviewed-by: Paul Burton 



Unfortunately, that alone doesn't fix the problem. SSB driver dependencies
are also broken, and in much worse shape. I had to add dependencies in five
places to get it to build, and the result is so messy that I won't even try
to submit it.


Oh, thats interesting. When I tried this earlier I just added "&&
PCI_DRIVERS_LEGACY" to the SSB_PCIHOST_POSSIBLE dependencies, but I was
waiting for Paul's feedback before submitting a similar patch.



You are right, that is much more straightforward than my attempted fix,
and it works.


But that wasn't -next, it was mainline + mips fixes branch + individual
fixes:



Mine is mainline plus "MIPS: Fix CPS SMP NS16550 UART defaults"
which for some reason never made it into mainline. For the nightly builds,
I ended up modifying my buildripts to fix that up manually in the created
configuration file.


And if that is fixed, mips:allmodconfig still doesn't build -
the next error is an undefined reference to physical_memsize in
arch/mips/kernel/vpe-mt.o.


That one is fairly easy to fix properly, I'll hopefully submit something
this evening.



I wonder if I should just stop trying to build allmodconfig for mips.
Any thoughts ?


With a few fixes applied it should be buildable I think. Sorry its been
late in the cycle before we've been able to get fixes merged.


Ok, I'll wait a bit longer before giving up on it.

Thanks,
Guenter

Re: [PATCH] bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets

[James Hogan]

On Mon, Jan 15, 2018 at 12:05:48PM -0800, Guenter Roeck wrote:
> On 01/15/2018 09:10 AM, Paul Burton wrote:
> > Hello,
> > 
> > On Mon, Jan 15, 2018 at 10:23:37AM +, James Hogan wrote:
> >> On Sun, Jan 14, 2018 at 01:34:02PM -0800, Guenter Roeck wrote:
> >>> Mips builds with BCMA host mode enabled fail in mainline and -next
> >>> with:
> >>>
> >>> In file included from include/linux/bcma/bcma.h:10:0,
> >>>   from drivers/bcma/bcma_private.h:9,
> >>>from drivers/bcma/main.c:8:
> >>> include/linux/bcma/bcma_driver_pci.h:218:24: error:
> >>>   field 'pci_controller' has incomplete type
> >>>
> >>> Bisect points to commit d41e6858ba58c ("MIPS: Kconfig: Set default MIPS
> >>> system type as generic") as the culprit. Analysis shows that the commmit
> >>> changes PCI configuration and enables PCI_DRIVERS_GENERIC. This in turn
> >>> disables PCI_DRIVERS_LEGACY. 'struct pci_controller' is, however, only
> >>> defined if PCI_DRIVERS_LEGACY is enabled.
> >>>
> >>> Ultimately that means that BCMA_DRIVER_PCI_HOSTMODE depends on
> >>> PCI_DRIVERS_LEGACY. Add the missing dependency.
> >>>
> >>> Fixes: d41e6858ba58c ("MIPS: Kconfig: Set default MIPS system type as 
> >>> ...")
> >>
> >> Well, technically I think commit c5611df96804 ("MIPS: PCI: Introduce
> >> CONFIG_PCI_DRIVERS_LEGACY") is to blame (Cc'ing paul), and the first bad
> >> commit would be commit eed0eabd12ef ("MIPS: generic: Introduce generic
> >> DT-based board support") which selects PCI_DRIVERS_GENERIC and is the
> >> only platform to do so. Both commits were first in v4.9-rc1 and I can
> >> reproduce this problem at that latter commit with the appropriate
> >> configuration.
> > 
> > Ah - yes if I recall correctly my assumption was that the MIPS-specific
> > struct pci_controller was only used by the MIPS-specific PCI drivers
> > under arch/mips/pci/, which are only built when configured for the
> > appropriate platform.
> > 
> > In this case use of that MIPS-specific struct pci_controller has spread
> > beyond arch/mips/ & the user can be configured in for platforms other
> > than the one that will actually use the driver, including the generic
> > platform which moves towards more generic PCI drivers in
> > drivers/pci/host/.
> > 
> >> But yes clearly the mentioned commit does also expose that existing
> >> problem more widely and to the default allmodconfig, and it looks like a
> >> reasonable approach for now, so if some mention of the other two commits
> >> is added:
> >>
> >> Reviewed-by: James Hogan 
> > 
> > Likewise, with the "Fixes:" tag fixed:
> > 
> >  Reviewed-by: Paul Burton 
> > 
> 
> Unfortunately, that alone doesn't fix the problem. SSB driver dependencies
> are also broken, and in much worse shape. I had to add dependencies in five
> places to get it to build, and the result is so messy that I won't even try
> to submit it.

Oh, thats interesting. When I tried this earlier I just added "&&
PCI_DRIVERS_LEGACY" to the SSB_PCIHOST_POSSIBLE dependencies, but I was
waiting for Paul's feedback before submitting a similar patch.

But that wasn't -next, it was mainline + mips fixes branch + individual
fixes:

> And if that is fixed, mips:allmodconfig still doesn't build -
> the next error is an undefined reference to physical_memsize in
> arch/mips/kernel/vpe-mt.o.

That one is fairly easy to fix properly, I'll hopefully submit something
this evening.

> 
> I wonder if I should just stop trying to build allmodconfig for mips.
> Any thoughts ?

With a few fixes applied it should be buildable I think. Sorry its been
late in the cycle before we've been able to get fixes merged.

Cheers
James

> 
> Guenter
> 
> > Thanks,
> >  Paul
> > 
> >> Having it in 4.15 would be great.
> >>
> >> Cheers
> >> James
> >>
> >>> Cc: Matt Redfearn 
> >>> Cc: James Hogan 
> >>> Signed-off-by: Guenter Roeck 
> >>> ---
> >>> I am aware that this problem has been reported several times. I have
> >>> not been able to find a fix, but I may have missed it. If so, my
> >>> apologies for the noise.
> >>>
> >>> Also note that this is not the only fix required; commit d41e6858ba58c,
> >>> as simple as it looks like, does a pretty good job messing up
> >>> "mips:allmodconfig" builds.
> >>>
> >>>   drivers/bcma/Kconfig | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
> >>> index 02d78f6cecbb..ba8acca036df 100644
> >>> --- a/drivers/bcma/Kconfig
> >>> +++ b/drivers/bcma/Kconfig
> >>> @@ -55,7 +55,7 @@ config BCMA_DRIVER_PCI
> >>>   
> >>>   config BCMA_DRIVER_PCI_HOSTMODE
> >>>   bool "Driver for PCI core working in hostmode"
> >>> - depends on MIPS && BCMA_DRIVER_PCI
> >>> + depends on MIPS && BCMA_DRIVER_PCI && PCI_DRIVERS_LEGACY
> >>>   help
> >>> PCI core hostmode operation (external PCI bus).
> >>>   
> >>> -- 
> >>> 2.7.4
> 

Re: AP6335 with mainline kernel

[Fabio Estevam]

Hi Arend,

On Tue, Dec 5, 2017 at 12:58 PM, Vanessa Maegima
 wrote:

> Hi Arend,
>
> Sorry for this!
>
> I updated the folder on https://drive.google.com/drive/folders/1fosahjL
> N1KI5NKS59_aPZdHLpENPFHtK
>
> Thanks!

Any ideas, please?

Thanks

Re: [PATCH] bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets

[Guenter Roeck]

On 01/15/2018 09:10 AM, Paul Burton wrote:

Hello,

On Mon, Jan 15, 2018 at 10:23:37AM +, James Hogan wrote:

On Sun, Jan 14, 2018 at 01:34:02PM -0800, Guenter Roeck wrote:

Mips builds with BCMA host mode enabled fail in mainline and -next
with:

In file included from include/linux/bcma/bcma.h:10:0,
  from drivers/bcma/bcma_private.h:9,
 from drivers/bcma/main.c:8:
include/linux/bcma/bcma_driver_pci.h:218:24: error:
field 'pci_controller' has incomplete type

Bisect points to commit d41e6858ba58c ("MIPS: Kconfig: Set default MIPS
system type as generic") as the culprit. Analysis shows that the commmit
changes PCI configuration and enables PCI_DRIVERS_GENERIC. This in turn
disables PCI_DRIVERS_LEGACY. 'struct pci_controller' is, however, only
defined if PCI_DRIVERS_LEGACY is enabled.

Ultimately that means that BCMA_DRIVER_PCI_HOSTMODE depends on
PCI_DRIVERS_LEGACY. Add the missing dependency.

Fixes: d41e6858ba58c ("MIPS: Kconfig: Set default MIPS system type as ...")


Well, technically I think commit c5611df96804 ("MIPS: PCI: Introduce
CONFIG_PCI_DRIVERS_LEGACY") is to blame (Cc'ing paul), and the first bad
commit would be commit eed0eabd12ef ("MIPS: generic: Introduce generic
DT-based board support") which selects PCI_DRIVERS_GENERIC and is the
only platform to do so. Both commits were first in v4.9-rc1 and I can
reproduce this problem at that latter commit with the appropriate
configuration.


Ah - yes if I recall correctly my assumption was that the MIPS-specific
struct pci_controller was only used by the MIPS-specific PCI drivers
under arch/mips/pci/, which are only built when configured for the
appropriate platform.

In this case use of that MIPS-specific struct pci_controller has spread
beyond arch/mips/ & the user can be configured in for platforms other
than the one that will actually use the driver, including the generic
platform which moves towards more generic PCI drivers in
drivers/pci/host/.


But yes clearly the mentioned commit does also expose that existing
problem more widely and to the default allmodconfig, and it looks like a
reasonable approach for now, so if some mention of the other two commits
is added:

Reviewed-by: James Hogan 


Likewise, with the "Fixes:" tag fixed:

 Reviewed-by: Paul Burton 



Unfortunately, that alone doesn't fix the problem. SSB driver dependencies
are also broken, and in much worse shape. I had to add dependencies in five
places to get it to build, and the result is so messy that I won't even try
to submit it. And if that is fixed, mips:allmodconfig still doesn't build -
the next error is an undefined reference to physical_memsize in
arch/mips/kernel/vpe-mt.o.

I wonder if I should just stop trying to build allmodconfig for mips.
Any thoughts ?

Guenter


Thanks,
 Paul


Having it in 4.15 would be great.

Cheers
James


Cc: Matt Redfearn 
Cc: James Hogan 
Signed-off-by: Guenter Roeck 
---
I am aware that this problem has been reported several times. I have
not been able to find a fix, but I may have missed it. If so, my
apologies for the noise.

Also note that this is not the only fix required; commit d41e6858ba58c,
as simple as it looks like, does a pretty good job messing up
"mips:allmodconfig" builds.

  drivers/bcma/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 02d78f6cecbb..ba8acca036df 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -55,7 +55,7 @@ config BCMA_DRIVER_PCI
  
  config BCMA_DRIVER_PCI_HOSTMODE

bool "Driver for PCI core working in hostmode"
-   depends on MIPS && BCMA_DRIVER_PCI
+   depends on MIPS && BCMA_DRIVER_PCI && PCI_DRIVERS_LEGACY
help
  PCI core hostmode operation (external PCI bus).
  
--

2.7.4

Re: brcmfmac: set p2p_disc error on BCM4350

[William Dauchy]

Hello Arend,

On Mon, Jan 15, 2018 at 08:48:50PM +0100, Arend van Spriel wrote:
> Now the error is only an issue if you intend to setup a P2P connection with
> some other device. For running in station or AP mode you can ignore this. So
> if you have any specific issue and suspect this to be causing it, please
> elaborate on the specific issue you see.

no specific issue, just wanted to fix issue/error and avoid other users
to report it.

Thanks,
-- 
William

Re: [PATCH v2] brcmfmac: fix CLM load error for legacy chips when user helper is enabled

[Arend van Spriel]

On 1/15/2018 11:09 AM, Wright Feng wrote:




[...]


---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 12
++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 6a59d06..aaab0e6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -182,12 +182,12 @@ static int brcmf_c_process_clm_blob(struct
brcmf_if *ifp)

  err = request_firmware(, clm_name, dev);
  if (err) {
-if (err == -ENOENT) {
-brcmf_dbg(INFO, "continue with CLM data currently present
in firmware\n");
-return 0;
-}
-brcmf_err("request CLM blob file failed (%d)\n", err);
-return err;
+if (err == -ENOENT || err == -EAGAIN)
+brcmf_info("continue with CLM data in FW\n");
+else
+brcmf_err("request clm_blob failed(%d) continue with CLM
data in FW\n",
+  err);


Don't see much value in doing it this way. Either way we need to inform 
the user about the consequence of this, ie.:


	brcmf_info("no clm_blob available (%d). device may have limited 
channels available\n", err);

+return 0;
  }


Regards,
Arend

Re: brcmfmac: set p2p_disc error on BCM4350

[Arend van Spriel]

On 1/15/2018 11:29 AM, William Dauchy wrote:

Hello Arend,

Did you had the chance to look into this?


Thanks for the ping, but no not really. I just noticed the mac address 
in the p2p-dev create request is the null address and I recall that is 
handled, but I will need to look into that. I will get back to you if I 
have done so. Feel free to ping again when it is taking to long ;-)


Now the error is only an issue if you intend to setup a P2P connection 
with some other device. For running in station or AP mode you can ignore 
this. So if you have any specific issue and suspect this to be causing 
it, please elaborate on the specific issue you see.


Regards,
Arend


On Mon, Jan 08, 2018 at 11:30:12AM +0100, William Dauchy wrote:

Hello Arend,

Thanks for your quick answer.

On Mon, Jan 08, 2018 at 10:31:52AM +0100, Arend van Spriel wrote:

This has been reported before, but not been able to look into it. Diving
into firmware it seems that error can only occur if the mac address for the
p2pdev interface is already used. So if possible can you rebuild the driver
with the patch below applied and provide dmesg output and output of
'ifconfig -a'.

Regards,
Arend
---8<

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
b/drivers/n
index 2ee5413..f96ad37 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2065,6 +2065,7 @@ static struct wireless_dev
*brcmf_p2p_create_p2pdev(struc
 int err;
 u32 bsscfgidx;

+   brcmf_err("enter: mac=%pM\n", addr);
 if (p2p->bss_idx[P2PAPI_BSSCFG_DEVICE].vif)
 return ERR_PTR(-ENOSPC);




Here is my brcmfmac dmesg output with patch applied
(please tell me if you need complete dmesg)

brcmfmac: brcmf_fw_map_chip_to_name: using brcm/brcmfmac4350-pcie.bin for chip 
0x004350(17232) rev 0x08
brcmfmac :3a:00.0: Direct firmware load for brcm/brcmfmac4350-pcie.txt 
failed with error -2
brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Oct 22 2015 06:16:26 
version 7.35.180.119 (r594535) FWID 01-e791c176
brcmfmac :3a:00.0 wlp58s0: renamed from wlan0
brcmfmac: brcmf_p2p_create_p2pdev: enter: mac=00:00:00:00:00:00
brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlp58s0 type 10 failed: 
err=-16

ifconfig shows:

wlp58s0: flags=4163  mtu 1500
 inet 192.168.18.77  netmask 255.255.255.0  broadcast 192.168.18.255
 inet6 fe80::3517:e1be:5bc7:899  prefixlen 64  scopeid 0x20
 ether 30:52:cb:83:71:33  txqueuelen 1000  (Ethernet)
 RX packets 26557  bytes 34149730 (32.5 MiB)
 RX errors 0  dropped 0  overruns 0  frame 0
 TX packets 14965  bytes 2298777 (2.1 MiB)
 TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0


Best,

Re: pull-request: wireless-drivers-next 2018-01-13

[David Miller]

From: Kalle Valo 
Date: Sat, 13 Jan 2018 12:33:43 +0200

> this is a pull request to net-next tree for 4.16, more info in the
> signed tag below. I'm not expecting any problems but please let me know
> if you have any.

Pulled, thanks Kalle.

Re: [PATCH] brcmfmac: Make sure CLM downloading is optional

[Arend van Spriel]

On 1/15/2018 6:10 PM, Bjorn Andersson wrote:

The presence of a CLM file is described as optional, but missing the clm
blob causes the preinit to return unsuccessfully. Fix this by ignoring
the return value of the brcmf_c_process_clm_blob().

Also remove the extra debug print, as brcmf_c_process_clm_blob() already
did print a useful error message before returning.

Fixes: fdd0bd88ceae ("brcmfmac: add CLM download support")
Cc: sta...@vger.kernel.org
Signed-off-by: Bjorn Andersson 
---

This regression was introduced in v4.15-rc1, but I unfortunately didn't test
WiFi until now. Included a Cc to stable@ in case you choose to pick this up
after v4.15.


Hi Bjorn,

Thanks for looking into this. Actually there already have been a couple 
of fixes posted on the linux-wireless list. [1] was rejected, [2] is 
being discussed, and [2] was posted by me and I was awaiting response 
from the guy reporting it.


Now the thing is that for old (Broadcom) firmware this is optional. 
Those firmwares don't even have CLM support and those that do have the 
CLM data embedded in firmware. However, Cypress wants to provide their 
customers with firmware without CLM data. For those devices it is not 
optional. I still prefer we add a mechanism to the driver to detect 
that, but we do not have that yet.


Now regarding your patch some comments below ...


  drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 8 ++--
  1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 6a59d0609d30..0c67ba6ae135 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -278,12 +278,8 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
}
ri->result = err;

-   /* Do any CLM downloading */
-   err = brcmf_c_process_clm_blob(ifp);
-   if (err < 0) {
-   brcmf_err("download CLM blob file failed, %d\n", err);
-   goto done;
-   }
+   /* Do any optional CLM downloading */
+   brcmf_c_process_clm_blob(ifp);


The error print is indeed redundant and should be removed here. However, 
brcmf_c_process_clm_blob() also returns -ENOMEM upon allocation failure 
and I would still fail the driver probe for that.


Regards,
Arend

[1] https://patchwork.kernel.org/patch/10106571/
[2] https://patchwork.kernel.org/patch/10159731/
[3] https://patchwork.kernel.org/patch/10154595/

Re: [PATCH v2 07/10] rtlwifi: btcoex: Add power_on_setting routine

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Ping-Ke Shih 

After mac power-on sequence, wifi will start to work so notify btcoex the
event to configure registers especially related to antenna. This will not
only help to assign antenna but also to yield better user experience.

Signed-off-by: Ping-Ke Shih 
---
  drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h | 1 +
  drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c  | 6 ++
  drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h  | 1 +
  drivers/net/wireless/realtek/rtlwifi/wifi.h   | 1 +
  4 files changed, 9 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
index ea12b9d63a73..bc523af7ef88 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
@@ -608,6 +608,7 @@ extern struct btc_coexist gl_bt_coexist;
  
  bool exhalbtc_initlize_variables(void);

  bool exhalbtc_bind_bt_coex_withadapter(void *adapter);
+void exhalbtc_power_on_setting(struct btc_coexist *btcoexist);
  void exhalbtc_init_hw_config(struct btc_coexist *btcoexist, bool wifi_only);
  void exhalbtc_init_coex_dm(struct btc_coexist *btcoexist);
  void exhalbtc_ips_notify(struct btc_coexist *btcoexist, u8 type);
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c
index 4d9e33078d4f..9e3623b0423c 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c
@@ -32,6 +32,7 @@
  static struct rtl_btc_ops rtl_btc_operation = {
.btc_init_variables = rtl_btc_init_variables,
.btc_init_hal_vars = rtl_btc_init_hal_vars,
+   .btc_power_on_setting = rtl_btc_power_on_setting,
.btc_init_hw_config = rtl_btc_init_hw_config,
.btc_ips_notify = rtl_btc_ips_notify,
.btc_lps_notify = rtl_btc_lps_notify,
@@ -116,6 +117,11 @@ void rtl_btc_init_hal_vars(struct rtl_priv *rtlpriv)
 */
  }
  
+void rtl_btc_power_on_setting(struct rtl_priv *rtlpriv)

+{
+   exhalbtc_power_on_setting(_bt_coexist);
+}
+
  void rtl_btc_init_hw_config(struct rtl_priv *rtlpriv)
  {
u8 bt_exist;
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h
index 40f1ce8c8a06..9becfa59407d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.h
@@ -29,6 +29,7 @@
  
  void rtl_btc_init_variables(struct rtl_priv *rtlpriv);

  void rtl_btc_init_hal_vars(struct rtl_priv *rtlpriv);
+void rtl_btc_power_on_setting(struct rtl_priv *rtlpriv);
  void rtl_btc_init_hw_config(struct rtl_priv *rtlpriv);
  void rtl_btc_ips_notify(struct rtl_priv *rtlpriv, u8 type);
  void rtl_btc_lps_notify(struct rtl_priv *rtlpriv, u8 type);
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h 
b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 941694060f48..b27dbe10b163 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -2557,6 +2557,7 @@ struct bt_coexist_info {
  struct rtl_btc_ops {
void (*btc_init_variables) (struct rtl_priv *rtlpriv);
void (*btc_init_hal_vars) (struct rtl_priv *rtlpriv);
+   void (*btc_power_on_setting)(struct rtl_priv *rtlpriv);
void (*btc_init_hw_config) (struct rtl_priv *rtlpriv);
void (*btc_ips_notify) (struct rtl_priv *rtlpriv, u8 type);
void (*btc_lps_notify)(struct rtl_priv *rtlpriv, u8 type);



I do not see any place in the code where rtl_btc_power_on_setting() is called, 
either directly or by callback. Did I miss it?


Larry

Re: [PATCH v2 10/10] rtlwifi: btcoex: add rfe_type parameter to btcoex

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Ping-Ke Shih 

btcoex configure antenna by rfe_type that is RF type programmed in efuse.

Signed-off-by: Ping-Ke Shih 
---
  drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 10 ++
  drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h |  2 ++
  2 files changed, 12 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 30d940cf3abf..7b7099cbb1c7 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -207,6 +207,14 @@ u8 rtl_get_hwpg_package_type(struct rtl_priv *rtlpriv)
return rtlhal->package_type;
  }
  
+static

+u8 rtl_get_hwpg_rfe_type(struct rtl_priv *rtlpriv)
+{
+   struct rtl_hal *rtlhal = rtl_hal(rtlpriv);
+
+   return rtlhal->rfe_type;
+}
+
  static
  bool halbtc_is_hw_mailbox_exist(struct btc_coexist *btcoexist)
  {
@@ -1309,6 +1317,8 @@ bool exhalbtc_bind_bt_coex_withadapter(void *adapter)
RT_TRACE(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
 "[BTCoex], Package Type = Non-TFBGA\n");
  
+	btcoexist->board_info.rfe_type = rtl_get_hwpg_rfe_type(rtlpriv);

+
return true;
  }
  
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h

index cbbf5e5a9c9b..ee7bbbd87eae 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
@@ -151,6 +151,8 @@ struct btc_board_info {
u8 btdm_ant_pos;
u8 single_ant_path; /* current used for 8723b only, 1=>s0,  0=>s1 */
bool tfbga_package;
+
+   u8 rfe_type;
  };
  
  enum btc_dbg_opcode {


Acked-by: Larry Finger 

Of course, until HP starts encoding efuse correctly, the special ant_sel code is 
still needed.


Larry

Re: [PATCH v2 09/10] rtlwifi: btcoex: Add common function for qeurying BT information

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Ping-Ke Shih 

This commit implement the common function to sort old features, and add
more new features that are get_supported_feature, get_supported_version,
get_ant_det_val, ble_scan_type, ble_scan_para, bt_dev_info,
forbidden_slot_val, afh_map and etc.

Signed-off-by: Ping-Ke Shih 
---
  .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c   | 309 ++---
  .../realtek/rtlwifi/btcoexist/halbtcoutsrc.h   |  70 +
  .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.c   |  48 +++-
  3 files changed, 394 insertions(+), 33 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 2be81fec789a..30d940cf3abf 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -207,6 +207,102 @@ u8 rtl_get_hwpg_package_type(struct rtl_priv *rtlpriv)
return rtlhal->package_type;
  }
  
+static

+bool halbtc_is_hw_mailbox_exist(struct btc_coexist *btcoexist)
+{
+   if (IS_HARDWARE_TYPE_8812(btcoexist->adapter))
+   return false;
+   else
+   return true;


Once you have returned false for 8812, you do not need the 'else'. Use a simple 
'	return true;'. As this is the only objection in this patch, I will let it 
pass. Some user with a suitable tool will change it.



+}
+
+static
+bool halbtc_send_bt_mp_operation(struct btc_coexist *btcoexist, u8 op_code,
+u8 *cmd, u32 len, unsigned long wait_ms)

--snip--

  
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h

index 5a7816ff6877..cbbf5e5a9c9b 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
@@ -278,6 +278,8 @@ enum btc_get_type {
BTC_GET_U4_VENDOR,
BTC_GET_U4_SUPPORTED_VERSION,
BTC_GET_U4_SUPPORTED_FEATURE,
+   BTC_GET_U4_BT_DEVICE_INFO,
+   BTC_GET_U4_BT_FORBIDDEN_SLOT_VAL,
BTC_GET_U4_WIFI_IQK_TOTAL,
BTC_GET_U4_WIFI_IQK_OK,
BTC_GET_U4_WIFI_IQK_FAIL,
@@ -459,6 +461,19 @@ typedefbool (*bfp_btc_get)(void *btcoexist, u8 
get_type, void *out_buf);
  
  typedef	bool (*bfp_btc_set)(void *btcoexist, u8 set_type, void *in_buf);
  
+typedef u32 (*bfp_btc_get_bt_coex_supported_feature)(void *btcoexist);

+
+typedef u32 (*bfp_btc_get_bt_coex_supported_version)(void *btcoexist);
+
+typedef u8 (*bfp_btc_get_ant_det_val_from_bt)(void *btcoexist);
+
+typedef u8 (*bfp_btc_get_ble_scan_type_from_bt)(void *btcoexist);
+
+typedef u32 (*bfp_btc_get_ble_scan_para_from_bt)(void *btcoexist, u8 
scan_type);
+
+typedef bool (*bfp_btc_get_bt_afh_map_from_bt)(void *btcoexist, u8 map_type,
+  u8 *afh_map);
+
  typedef void (*bfp_btc_set_bt_reg)(void *btc_context, u8 reg_type, u32 offset,
   u32 value);


I would prefer that you not add additional typedef statements, but I will let it 
pass.



Acked-by: Larry Finger 

Re: [PATCH v2 08/10] rtlwifi: btcoex: Remove global variables from btcoex

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Ping-Ke Shih 

Remove global variables, so btcoexist can support multiple instances
simultaneously.

Signed-off-by: Ping-Ke Shih 
---
  .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c   |  79 -
  .../realtek/rtlwifi/btcoexist/halbtcoutsrc.h   |  21 +--
  .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.c   | 182 +
  .../wireless/realtek/rtlwifi/btcoexist/rtl_btc.h   |   3 +-
  drivers/net/wireless/realtek/rtlwifi/pci.c |   5 +-
  drivers/net/wireless/realtek/rtlwifi/wifi.h|   5 +-
  6 files changed, 217 insertions(+), 78 deletions(-)


I am very happy to see this change. Although a given box is unlikely to have 
more than one PCI wireless device, it is certainly possible for one of the PCI 
drivers and rtl8192cu to both be needed.


Acked-by: Larry Finger 



diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c 
b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 5f3eda31187a..2be81fec789a 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -25,14 +25,6 @@
  
  #include "halbt_precomp.h"
  
-/***

- * Global variables
- ***/
-
-struct btc_coexist gl_bt_coexist;
-
-u32 btc_dbg_type[BTC_MSG_MAX];
-
  /***
   *Debug related function
   ***/
@@ -971,9 +963,12 @@ bool halbtc_under_ips(struct btc_coexist *btcoexist)
  /*
   * Extern functions called by other module
   */
-bool exhalbtc_initlize_variables(void)
+bool exhalbtc_initlize_variables(struct rtl_priv *rtlpriv)
  {
-   struct btc_coexist *btcoexist = _bt_coexist;
+   struct btc_coexist *btcoexist = rtl_btc_coexist(rtlpriv);
+
+   if (!btcoexist)
+   return false;
  
  	halbtc_dbg_init();
  
@@ -1009,9 +1004,12 @@ bool exhalbtc_initlize_variables(void)
  
  bool exhalbtc_bind_bt_coex_withadapter(void *adapter)

  {
-   struct btc_coexist *btcoexist = _bt_coexist;
struct rtl_priv *rtlpriv = adapter;
-   u8 ant_num = 2, chip_type;
+   struct btc_coexist *btcoexist = rtl_btc_coexist(rtlpriv);
+   u8 ant_num = 2, chip_type, single_ant_path = 0;
+
+   if (!btcoexist)
+   return false;
  
  	if (btcoexist->binded)

return false;
@@ -1042,10 +1040,16 @@ bool exhalbtc_bind_bt_coex_withadapter(void *adapter)
btcoexist->bt_info.miracast_plus_bt = false;
  
  	chip_type = rtl_get_hwpg_bt_type(rtlpriv);

-   exhalbtc_set_chip_type(chip_type);
+   exhalbtc_set_chip_type(btcoexist, chip_type);
ant_num = rtl_get_hwpg_ant_num(rtlpriv);
exhalbtc_set_ant_num(rtlpriv, BT_COEX_ANT_TYPE_PG, ant_num);
  
+	/* set default antenna position to main  port */

+   btcoexist->board_info.btdm_ant_pos = BTC_ANTENNA_AT_MAIN_PORT;
+
+   single_ant_path = rtl_get_hwpg_single_ant_path(rtlpriv);
+   exhalbtc_set_single_ant_path(btcoexist, single_ant_path);
+
if (rtl_get_hwpg_package_type(rtlpriv) == 0)
btcoexist->board_info.tfbga_package = false;
else if (rtl_get_hwpg_package_type(rtlpriv) == 1)
@@ -1550,30 +1554,25 @@ void exhalbtc_stack_update_profile_info(void)
  {
  }
  
-void exhalbtc_update_min_bt_rssi(s8 bt_rssi)

+void exhalbtc_update_min_bt_rssi(struct btc_coexist *btcoexist, s8 bt_rssi)
  {
-   struct btc_coexist *btcoexist = _bt_coexist;
-
if (!halbtc_is_bt_coexist_available(btcoexist))
return;
  
  	btcoexist->stack_info.min_bt_rssi = bt_rssi;

  }
  
-void exhalbtc_set_hci_version(u16 hci_version)

+void exhalbtc_set_hci_version(struct btc_coexist *btcoexist, u16 hci_version)
  {
-   struct btc_coexist *btcoexist = _bt_coexist;
-
if (!halbtc_is_bt_coexist_available(btcoexist))
return;
  
  	btcoexist->stack_info.hci_version = hci_version;

  }
  
-void exhalbtc_set_bt_patch_version(u16 bt_hci_version, u16 bt_patch_version)

+void exhalbtc_set_bt_patch_version(struct btc_coexist *btcoexist,
+  u16 bt_hci_version, u16 bt_patch_version)
  {
-   struct btc_coexist *btcoexist = _bt_coexist;
-
if (!halbtc_is_bt_coexist_available(btcoexist))
return;
  
@@ -1581,7 +1580,7 @@ void exhalbtc_set_bt_patch_version(u16 bt_hci_version, u16 bt_patch_version)

btcoexist->bt_info.bt_hci_ver = bt_hci_version;
  }
  
-void exhalbtc_set_chip_type(u8 chip_type)

+void exhalbtc_set_chip_type(struct btc_coexist *btcoexist, u8 chip_type)
  {
switch (chip_type) {
default:

Re: [PATCH v2 06/10] rtlwifi: Support A-MSDU in A-MPDU capability

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Steven Ting 

Due to the fact that A-MSDU deaggregation is done in software,
we set this flag to support the A-MSDU in A-MPDU

Signed-off-by: Steven Ting 
Signed-off-by: Ping-Ke Shih 
---
  drivers/net/wireless/realtek/rtlwifi/base.c | 1 +
  1 file changed, 1 insertion(+)


Acked-by: Larry Finger 



diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c 
b/drivers/net/wireless/realtek/rtlwifi/base.c
index e902cd7adbfe..07b91bc9adf9 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -397,6 +397,7 @@ static void _rtl_init_mac80211(struct ieee80211_hw *hw)
ieee80211_hw_set(hw, REPORTS_TX_ACK_STATUS);
ieee80211_hw_set(hw, SUPPORTS_TX_FRAG);
ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
+   ieee80211_hw_set(hw, SUPPORTS_AMSDU_IN_AMPDU);
  
  	/* swlps or hwlps has been set in diff chip in init_sw_vars */

if (rtlpriv->psc.swctrl_lps) {

Re: [PATCH v2 05/10] rtlwifi: enable mac80211 fast-tx support

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Ping-Ke Shih 

To enable the mac80211 fast-tx feature, the hw/driver needs to support
dynamic power saving and fragmentation. Since our driver does not
need to fragment packet into smaller pieces, we just hook an empty
callback of set_frag_threshold to avoid fragmentation in mac80211.

After this, the mac80211 will not fragment the packets and can transmit
them faster by cache the header information.

Signed-off-by: Yan-Hsuan Chuang 
Signed-off-by: Ping-Ke Shih 
---
  drivers/net/wireless/realtek/rtlwifi/base.c | 2 ++
  drivers/net/wireless/realtek/rtlwifi/core.c | 6 ++
  2 files changed, 8 insertions(+)


Acked-by: Larry Finger 



diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c 
b/drivers/net/wireless/realtek/rtlwifi/base.c
index 89ec318598ea..e902cd7adbfe 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -395,6 +395,8 @@ static void _rtl_init_mac80211(struct ieee80211_hw *hw)
ieee80211_hw_set(hw, CONNECTION_MONITOR);
ieee80211_hw_set(hw, MFP_CAPABLE);
ieee80211_hw_set(hw, REPORTS_TX_ACK_STATUS);
+   ieee80211_hw_set(hw, SUPPORTS_TX_FRAG);
+   ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
  
  	/* swlps or hwlps has been set in diff chip in init_sw_vars */

if (rtlpriv->psc.swctrl_lps) {
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c 
b/drivers/net/wireless/realtek/rtlwifi/core.c
index 6c698123ac07..d454c38fc9cd 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -1001,6 +1001,11 @@ static void rtl_op_sta_statistics(struct ieee80211_hw 
*hw,
sinfo->filled = 0;
  }
  
+static int rtl_op_set_frag_threshold(struct ieee80211_hw *hw, u32 value)

+{
+   return -EOPNOTSUPP;
+}
+
  /*
   *for mac80211 VO = 0, VI = 1, BE = 2, BK = 3
   *for rtl819x  BE = 0, BK = 1, VI = 2, VO = 3
@@ -1906,6 +1911,7 @@ const struct ieee80211_ops rtl_ops = {
.configure_filter = rtl_op_configure_filter,
.set_key = rtl_op_set_key,
.sta_statistics = rtl_op_sta_statistics,
+   .set_frag_threshold = rtl_op_set_frag_threshold,
.conf_tx = rtl_op_conf_tx,
.bss_info_changed = rtl_op_bss_info_changed,
.get_tsf = rtl_op_get_tsf,

Re: [PATCH v2 04/10] rtlwifi: unlink bss when un-association

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Tsang-Shian Lin 

When AP change bandwidth setting from 20M to 40M, STA may use old 20M AP
information to association with AP. Driver unlink bss in the
.bss_info_changed of ieee80211_ops to make sure that later scan can get
correct AP bandwidth capability.

Signed-off-by: Tsang-Shian Lin 
Signed-off-by: Ping-Ke Shih 
---
  drivers/net/wireless/realtek/rtlwifi/core.c | 18 ++
  1 file changed, 18 insertions(+)


Acked-by: Larry Finger 




diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c 
b/drivers/net/wireless/realtek/rtlwifi/core.c
index ec639fa8095e..6c698123ac07 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -1171,6 +1171,8 @@ static void rtl_op_bss_info_changed(struct ieee80211_hw 
*hw,
RT_TRACE(rtlpriv, COMP_MAC80211, DBG_DMESG,
 "BSS_CHANGED_ASSOC\n");
} else {
+   struct cfg80211_bss *bss = NULL;
+
mstatus = RT_MEDIA_DISCONNECT;
  
  			if (mac->link_state == MAC80211_LINKED)

@@ -1178,6 +1180,22 @@ static void rtl_op_bss_info_changed(struct ieee80211_hw 
*hw,
if (ppsc->p2p_ps_info.p2p_ps_mode > P2P_PS_NONE)
rtl_p2p_ps_cmd(hw, P2P_PS_DISABLE);
mac->link_state = MAC80211_NOLINK;
+
+   bss = cfg80211_get_bss(hw->wiphy, NULL,
+  (u8 *)mac->bssid, NULL, 0,
+  IEEE80211_BSS_TYPE_ESS,
+  IEEE80211_PRIVACY_OFF);
+
+   RT_TRACE(rtlpriv, COMP_MAC80211, DBG_DMESG,
+"bssid = %pMF\n", mac->bssid);
+
+   if (bss) {
+   cfg80211_unlink_bss(hw->wiphy, bss);
+   cfg80211_put_bss(hw->wiphy, bss);
+   RT_TRACE(rtlpriv, COMP_MAC80211, DBG_DMESG,
+"cfg80211_unlink !!\n");
+   }
+
eth_zero_addr(mac->bssid);
mac->vendor = PEER_UNKNOWN;
mac->mode = 0;

Re: [PATCH v2 03/10] rtlwifi: Add sta_statistics of mac80211's op, and set filled=0 by default

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Ping-Ke Shih 

When using iwconfig to check wifi status, wext uses 'static struct' of
sinfo to get station info so sinfo->filled will be persistent. Since the
commit 2b9a7e1bac24 ("mac80211: allow drivers to provide most station
statistics") assumes driver initializes sinfo->filled to declare supported
fields, without initialization it will report wrong info. This commit
simply set 'filled' to be zero simply, and left sinfo to be filled by
mac80211.

Signed-off-by: Ping-Ke Shih 
---
  drivers/net/wireless/realtek/rtlwifi/core.c | 10 ++
  1 file changed, 10 insertions(+)


Acked-by: Larry Finger 




diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c 
b/drivers/net/wireless/realtek/rtlwifi/core.c
index a78b828f531a..ec639fa8095e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -992,6 +992,15 @@ static int _rtl_get_hal_qnum(u16 queue)
return qnum;
  }
  
+static void rtl_op_sta_statistics(struct ieee80211_hw *hw,

+ struct ieee80211_vif *vif,
+ struct ieee80211_sta *sta,
+ struct station_info *sinfo)
+{
+   /* nothing filled by driver, so mac80211 will update all info */
+   sinfo->filled = 0;
+}
+
  /*
   *for mac80211 VO = 0, VI = 1, BE = 2, BK = 3
   *for rtl819x  BE = 0, BK = 1, VI = 2, VO = 3
@@ -1878,6 +1887,7 @@ const struct ieee80211_ops rtl_ops = {
.config = rtl_op_config,
.configure_filter = rtl_op_configure_filter,
.set_key = rtl_op_set_key,
+   .sta_statistics = rtl_op_sta_statistics,
.conf_tx = rtl_op_conf_tx,
.bss_info_changed = rtl_op_bss_info_changed,
.get_tsf = rtl_op_get_tsf,

Re: [PATCH v2 02/10] rtlwifi: fix scan channel 1 fail after IPS

[Larry Finger]

On 01/11/2018 01:09 AM, pks...@realtek.com wrote:

From: Ping-Ke Shih 

If there is no connection, driver will enter IPS state. Meanwhile, it
fails to scan channel 1 by the command 'iw dev wlan0 scan freq 2412',
because hardware channel setting lose after IPS. Thus, restore channel
setting from hw->conf.channel set by last rtl_op_config().

Signed-off-by: Tim Lee 
Signed-off-by: Ping-Ke Shih 
---
  drivers/net/wireless/realtek/rtlwifi/ps.c | 5 +
  1 file changed, 5 insertions(+)


Acked-by: Larry Finger 




diff --git a/drivers/net/wireless/realtek/rtlwifi/ps.c 
b/drivers/net/wireless/realtek/rtlwifi/ps.c
index 6a4008845f49..71af24e2e051 100644
--- a/drivers/net/wireless/realtek/rtlwifi/ps.c
+++ b/drivers/net/wireless/realtek/rtlwifi/ps.c
@@ -51,6 +51,11 @@ bool rtl_ps_enable_nic(struct ieee80211_hw *hw)
>retry_long);
RT_CLEAR_PS_LEVEL(ppsc, RT_RF_OFF_LEVL_HALT_NIC);
  
+	rtlpriv->cfg->ops->switch_channel(hw);

+   rtlpriv->cfg->ops->set_channel_access(hw);
+   rtlpriv->cfg->ops->set_bw_mode(hw,
+   cfg80211_get_chandef_type(>conf.chandef));
+
/*<3> Enable Interrupt */
rtlpriv->cfg->ops->enable_interrupt(hw);
  

Re: [PATCH] bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets

[Paul Burton]

Hello,

On Mon, Jan 15, 2018 at 10:23:37AM +, James Hogan wrote:
> On Sun, Jan 14, 2018 at 01:34:02PM -0800, Guenter Roeck wrote:
> > Mips builds with BCMA host mode enabled fail in mainline and -next
> > with:
> > 
> > In file included from include/linux/bcma/bcma.h:10:0,
> >  from drivers/bcma/bcma_private.h:9,
> >  from drivers/bcma/main.c:8:
> > include/linux/bcma/bcma_driver_pci.h:218:24: error:
> > field 'pci_controller' has incomplete type
> > 
> > Bisect points to commit d41e6858ba58c ("MIPS: Kconfig: Set default MIPS
> > system type as generic") as the culprit. Analysis shows that the commmit
> > changes PCI configuration and enables PCI_DRIVERS_GENERIC. This in turn
> > disables PCI_DRIVERS_LEGACY. 'struct pci_controller' is, however, only
> > defined if PCI_DRIVERS_LEGACY is enabled.
> > 
> > Ultimately that means that BCMA_DRIVER_PCI_HOSTMODE depends on
> > PCI_DRIVERS_LEGACY. Add the missing dependency.
> > 
> > Fixes: d41e6858ba58c ("MIPS: Kconfig: Set default MIPS system type as ...")
> 
> Well, technically I think commit c5611df96804 ("MIPS: PCI: Introduce
> CONFIG_PCI_DRIVERS_LEGACY") is to blame (Cc'ing paul), and the first bad
> commit would be commit eed0eabd12ef ("MIPS: generic: Introduce generic
> DT-based board support") which selects PCI_DRIVERS_GENERIC and is the
> only platform to do so. Both commits were first in v4.9-rc1 and I can
> reproduce this problem at that latter commit with the appropriate
> configuration.

Ah - yes if I recall correctly my assumption was that the MIPS-specific
struct pci_controller was only used by the MIPS-specific PCI drivers
under arch/mips/pci/, which are only built when configured for the
appropriate platform.

In this case use of that MIPS-specific struct pci_controller has spread
beyond arch/mips/ & the user can be configured in for platforms other
than the one that will actually use the driver, including the generic
platform which moves towards more generic PCI drivers in
drivers/pci/host/.

> But yes clearly the mentioned commit does also expose that existing
> problem more widely and to the default allmodconfig, and it looks like a
> reasonable approach for now, so if some mention of the other two commits
> is added:
> 
> Reviewed-by: James Hogan 

Likewise, with the "Fixes:" tag fixed:

Reviewed-by: Paul Burton 

Thanks,
Paul

> Having it in 4.15 would be great.
> 
> Cheers
> James
> 
> > Cc: Matt Redfearn 
> > Cc: James Hogan 
> > Signed-off-by: Guenter Roeck 
> > ---
> > I am aware that this problem has been reported several times. I have
> > not been able to find a fix, but I may have missed it. If so, my
> > apologies for the noise.
> > 
> > Also note that this is not the only fix required; commit d41e6858ba58c,
> > as simple as it looks like, does a pretty good job messing up
> > "mips:allmodconfig" builds.
> > 
> >  drivers/bcma/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
> > index 02d78f6cecbb..ba8acca036df 100644
> > --- a/drivers/bcma/Kconfig
> > +++ b/drivers/bcma/Kconfig
> > @@ -55,7 +55,7 @@ config BCMA_DRIVER_PCI
> >  
> >  config BCMA_DRIVER_PCI_HOSTMODE
> > bool "Driver for PCI core working in hostmode"
> > -   depends on MIPS && BCMA_DRIVER_PCI
> > +   depends on MIPS && BCMA_DRIVER_PCI && PCI_DRIVERS_LEGACY
> > help
> >   PCI core hostmode operation (external PCI bus).
> >  
> > -- 
> > 2.7.4
> > 

[PATCH] brcmfmac: Make sure CLM downloading is optional

[Bjorn Andersson]

The presence of a CLM file is described as optional, but missing the clm
blob causes the preinit to return unsuccessfully. Fix this by ignoring
the return value of the brcmf_c_process_clm_blob().

Also remove the extra debug print, as brcmf_c_process_clm_blob() already
did print a useful error message before returning.

Fixes: fdd0bd88ceae ("brcmfmac: add CLM download support")
Cc: sta...@vger.kernel.org
Signed-off-by: Bjorn Andersson 
---

This regression was introduced in v4.15-rc1, but I unfortunately didn't test
WiFi until now. Included a Cc to stable@ in case you choose to pick this up
after v4.15.

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 6a59d0609d30..0c67ba6ae135 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -278,12 +278,8 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
}
ri->result = err;
 
-   /* Do any CLM downloading */
-   err = brcmf_c_process_clm_blob(ifp);
-   if (err < 0) {
-   brcmf_err("download CLM blob file failed, %d\n", err);
-   goto done;
-   }
+   /* Do any optional CLM downloading */
+   brcmf_c_process_clm_blob(ifp);
 
/* query for 'ver' to get version info from firmware */
memset(buf, 0, sizeof(buf));
-- 
2.15.0

Re: [RFC v2 4/5] nl80211: Implement TX of control port frames

[Denis Kenzior]

Hi Johannes,

On 01/15/2018 07:14 AM, Johannes Berg wrote:

Ok this is the interesting part :-)


+   int (*tx_control_port)(struct wiphy *wiphy,
+  struct net_device *dev,
+  const u8 *buf, size_t len,
+  const u8 *dest, const u16 proto,
+  const bool noencrypt);


(indentation seems off in both patchwork and my email, but whatever)


Yes, that was my fault.




+   wdev_lock(wdev);
+
+   switch (wdev->iftype) {
+   case NL80211_IFTYPE_STATION:
+   if (wdev->current_bss)


err, !current_bss?



Seems fine to me?  There's a break after that if statement.


+   buf = nla_data(info->attrs[NL80211_ATTR_FRAME]);
+   len = nla_len(info->attrs[NL80211_ATTR_FRAME]);
+   dest = nla_data(info->attrs[NL80211_ATTR_MAC]);
+   proto = nla_get_u16(info->attrs[NL80211_ATTR_CONTROL_PORT_ETHERTYPE]);
+   noencrypt =
+   nla_get_flag(info->attrs[NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT]);


So this is the data we support here.

Jouni and I were talking about this and were thinking we might need
"use old key" or something like that. That's rather difficult to do
though, I'm not even sure we keep the old key around?

Jouni? Do you see how this could work?


The other thing we thought about was that maybe we should have "open
port after this frame", but since it's an in-band mechanism now you
could do that also just before the frame.

FWIW, I'm checking with our guy on what other specialities we might
want to add into this mix as far as workarounds are concerned.



Sure, I'll hold off on re-spinning these until I get more feedback.

Regards,
-Denis

Re: [RFC v2 3/5] mac80211: Send control port frames over nl80211

[Denis Kenzior]

Hi Johannes,

On 01/15/2018 07:09 AM, Johannes Berg wrote:

On Wed, 2018-01-10 at 11:09 -0600, Denis Kenzior wrote:


Pre-authentication type frames (protocol: 0x88c7) are also forwarded
over nl80211.


Interesting - maybe userspace should be able to configure a(n) (list
of) ethertype(s)?



I'm not so sure.  Preauthentication is different from EAPoL frames and 
most of the special-case checks for control_port_ethertype inside 
mac80211 do not apply to it.  I think it might be safer to carry it as a 
special case to the special case :P



Signed-off-by: Denis Kenzior 
---
  net/mac80211/cfg.c |  2 ++
  net/mac80211/ieee80211_i.h |  1 +
  net/mac80211/mlme.c|  2 ++
  net/mac80211/rx.c  | 31 ---
  4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 46028e12e216..f53bfb27295f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -925,6 +925,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct 
net_device *dev,
 */
sdata->control_port_protocol = params->crypto.control_port_ethertype;
sdata->control_port_no_encrypt = params->crypto.control_port_no_encrypt;
+   sdata->control_port_over_nl80211 = false;


It's probably better to reset this at interface type switching, if it's
not done anyway (we zero a lot of memory there, but not sure precisely
what)

Otherwise we'll surely forget this in some place like mesh or OCB ...

Initially it must be false (0) anyway.



Where do I look?  Right now I'm mostly following the precedent of other 
control_port_* stuff.


Regards,
-Denis

Re: [PATCH v3 4/5] mac80211_hwsim: add permanent mac address option for new radios

[Benjamin Beichler]

Am 15.01.2018 um 13:14 schrieb Johannes Berg:
> On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
>>  
>> +if (info->attrs[HWSIM_ATTR_PERM_ADDR]) {
>> +if (nla_len(info->attrs[HWSIM_ATTR_PERM_ADDR]) != ETH_ALEN) {
>> +pr_debug("mac80211_hwsim: MAC has wrong size\n");
>> +return -EINVAL;
>> +}
> Oh, also - don't do this.
>
> 1) don't print, use NL_SET_ERR_MSG().
Ok, I only adapted the code already there, but I change this.
>
> 2) use the policy to validate lengths
Yeah, of course I change that, much easier -.-

>
>> +if (!is_local_ether_addr(
>> +nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
>> +pr_debug("mac80211_hwsim: MAC is not locally 
>> administered\n");
>> +return -EINVAL;
>> +}
> This doesn't really matter - it's purely virtual. I think we can avoid
> that.
>
>> +if (get_hwsim_data_ref_from_addr(
>> +nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
>> +pr_debug("mac80211_hwsim: perm MAC already in use\n");
>> +return -EINVAL;
>> +}
> This is racy afaict - remove it and return a clash later when you fail
> to insert the new radio.
Ehm yes, actually exactly this test is already in the rhashtable patch.
But maybe I should also change there the error print to a NL_ERR_MSG() ?

>
> johannes
>

-- 
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: benjamin.beich...@uni-rostock.de
www: http://www.imd.uni-rostock.de/

Re: [PATCH v3 4/5] mac80211_hwsim: add permanent mac address option for new radios

[Benjamin Beichler]

Am 15.01.2018 um 13:09 schrieb Johannes Berg:
> On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
>
>> To do not break the operation with legacy software using hwsim, the new
>> address is set twice. The problem here is, the netlink call backs use
>> wiphy->addresses[1] as identification of a radio and not the proposed
>> permanent address (wiphy->addresses[0]). This design decision is not
>> documented in the kernel repo, therefore this patch simply reproduces this,
>> but with the same address.
> It's weird to have the same address twice - perhaps we can XOR 0x40 or
> so?
I really don't know, why there were 2 addresses introduced in the first
place, and why the netlink operations for receiving only check against
the second address and not the first address (see
get_hwsim_data_ref_from_addr). My suggestion would be, only allocate one
mac address and check against this one. I don't know, whether really any
user of hwsim rely on the structure, that an adapter needs two mac
addresses. Since this is part of the initial commit, this decision is
not documented in git ...
> johannes
>
>

-- 
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: benjamin.beich...@uni-rostock.de
www: http://www.imd.uni-rostock.de/

Re: [PATCH v3 3/5] mac80211_hwsim: add generation count for netlink dump operation

[Benjamin Beichler]

Am 15.01.2018 um 13:08 schrieb Johannes Berg:
> On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
>>  Change the dump iteration to be independent from
>> increasing radio indices on radio list.
> You can't do that, data structures can be deleted while the dump is
> ongoing.
That's what I try to address with this patch. In the past there were
corner cases, which might have failed while deletion.

Nonetheless, the radio count is in the current version strictly
monotonic, maybe the description is misleading. :-)
So in the inner loop there is no deletion possible because of the
spinlock, and multiple calls of nl_dump check the generation count and
cancel if it have changed.

>
> johannes
>

--
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: benjamin.beich...@uni-rostock.de
www: http://www.imd.uni-rostock.de/

Re: [v2,6/9] wil6210: add support for headroom configuration

[merez]

On 2018-01-09 10:07, Kalle Valo wrote:

Maya Erez  wrote:


Add module parameter for configuring the headroom size
in the skb allocation.

Signed-off-by: Lazar Alexei 
Signed-off-by: Maya Erez 
Signed-off-by: Kalle Valo 


Why?


Some platforms have specific requirements on packet alignment.
I will upload an update of the commit text in the next set of 11ad 
patches.




https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_log_does_not_answer_why

And I'm a bit skeptic about this, controlling headroom via a module 
parameter

is not really making any sense to me.

--
Maya Erez
Qualcomm Israel, Inc. on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [RFC v2 5/5] mac80211: Add support for tx_control_port

[Johannes Berg]

This looks straight-forward.

Again, question about which ethertype.

And you should probably add a define for 0x88c7 (and yeah, I'm willing
to take that through my tree)

johannes

Re: [RFC v2 4/5] nl80211: Implement TX of control port frames

[Johannes Berg]

Ok this is the interesting part :-)

> + int (*tx_control_port)(struct wiphy *wiphy,
> +struct net_device *dev,
> +const u8 *buf, size_t len,
> +const u8 *dest, const u16 proto,
> +const bool noencrypt);

(indentation seems off in both patchwork and my email, but whatever)

> + wdev_lock(wdev);
> +
> + switch (wdev->iftype) {
> + case NL80211_IFTYPE_STATION:
> + if (wdev->current_bss)

err, !current_bss?

> + buf = nla_data(info->attrs[NL80211_ATTR_FRAME]);
> + len = nla_len(info->attrs[NL80211_ATTR_FRAME]);
> + dest = nla_data(info->attrs[NL80211_ATTR_MAC]);
> + proto = nla_get_u16(info->attrs[NL80211_ATTR_CONTROL_PORT_ETHERTYPE]);
> + noencrypt =
> + nla_get_flag(info->attrs[NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT]);

So this is the data we support here.

Jouni and I were talking about this and were thinking we might need
"use old key" or something like that. That's rather difficult to do
though, I'm not even sure we keep the old key around?

Jouni? Do you see how this could work?


The other thing we thought about was that maybe we should have "open
port after this frame", but since it's an in-band mechanism now you
could do that also just before the frame.

FWIW, I'm checking with our guy on what other specialities we might
want to add into this mix as far as workarounds are concerned.

johannes

Re: [RFC v2 3/5] mac80211: Send control port frames over nl80211

[Johannes Berg]

On Wed, 2018-01-10 at 11:09 -0600, Denis Kenzior wrote:

> Pre-authentication type frames (protocol: 0x88c7) are also forwarded
> over nl80211.

Interesting - maybe userspace should be able to configure a(n) (list
of) ethertype(s)?

> Signed-off-by: Denis Kenzior 
> ---
>  net/mac80211/cfg.c |  2 ++
>  net/mac80211/ieee80211_i.h |  1 +
>  net/mac80211/mlme.c|  2 ++
>  net/mac80211/rx.c  | 31 ---
>  4 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 46028e12e216..f53bfb27295f 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -925,6 +925,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct 
> net_device *dev,
>*/
>   sdata->control_port_protocol = params->crypto.control_port_ethertype;
>   sdata->control_port_no_encrypt = params->crypto.control_port_no_encrypt;
> + sdata->control_port_over_nl80211 = false;

It's probably better to reset this at interface type switching, if it's
not done anyway (we zero a lot of memory there, but not sure precisely
what)

Otherwise we'll surely forget this in some place like mesh or OCB ...

Initially it must be false (0) anyway.

>   sdata->control_port_protocol = req->crypto.control_port_ethertype;
>   sdata->control_port_no_encrypt = req->crypto.control_port_no_encrypt;
> + sdata->control_port_over_nl80211 =
> + req->crypto.control_port_over_nl80211;

Heh. One of the cases where the 80 cols line is just dumb :-)

>   sdata->encrypt_headroom = ieee80211_cs_headroom(local, >crypto,
>   sdata->vif.type);
>  
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index b3cff69bfd66..28b74ae1ee48 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -2326,16 +2326,41 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
>   }
>  #endif
>  
> - if (skb) {
> + if (!skb)
> + goto try_xmit;

Seems this could be better without the goto, perhaps pulling the code
into a helper function?

> + skb->protocol = eth_type_trans(skb, dev);
> + memset(skb->cb, 0, sizeof(skb->cb));
> +
> + if (unlikely((skb->protocol == sdata->control_port_protocol ||
> +   skb->protocol == cpu_to_be16(0x88c7)) &&
> +  sdata->control_port_over_nl80211)) {
> + struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> + bool noencrypt;
> +
> + ehdr = eth_hdr(skb);
> +
> + if (status->flag & RX_FLAG_DECRYPTED)
> + noencrypt = false;
> + else
> + noencrypt = true;

noencrypt = status->flags & RX_FLAG_DECRYPTED;
? It's a bool, after all, and you could even roll it into the
declaration.

johannes

Re: [RFC v2 2/5] nl80211: Add CMD_CONTROL_PORT_FRAME API

[Johannes Berg]

Can only nitpick on this patch ...

> +static int __nl80211_control_port(struct net_device *dev,
> +   const u8 *buf, size_t len,
> +   const u8 *addr, u16 proto,
> +   bool unencrypted, gfp_t gfp)

but that could use an "rx" in the function name :)

johannes

Re: [RFC v2 1/5] nl80211: Add CONTROL_PORT_OVER_NL80211 attribute

[Johannes Berg]

On Wed, 2018-01-10 at 11:09 -0600, Denis Kenzior wrote:
> 
> + WIPHY_FLAG_CONTROL_PORT_OVER_NL80211= BIT(25),

I think it'd be easier for everyone to just use an nl80211 extended
feature flag for this, then you also don't need this code:

> + if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211]) {
> + if (!info->attrs[NL80211_ATTR_SOCKET_OWNER])
> + return -EINVAL;
> +
> + if (!(rdev->wiphy.flags & WIPHY_FLAG_CONTROL_PORT_OVER_NL80211))
> + return -EOPNOTSUPP;
> +
> + settings->control_port_over_nl80211 = true;
> + } else {
> + settings->control_port_over_nl80211 = false;
> + }

Nit: you don't really need the else branch.

johannes

Re: [PATCH v2 1/3] cfg80211/nl80211: Optional authentication offload to userspace

[Johannes Berg]

On Thu, 2018-01-04 at 14:59 -0600, Denis Kenzior wrote:

> So this implies userspace must pre-register for authentication 
> management frames, correct?  And other applications could register to 
> receive these frames as well?  Would it not be easier (and more secure) 
> to simply forward these directly to the application that triggered 
> CMD_CONNECT instead?

Only one will be able to register, so I think it's OK. We could even
check that it's registered already at CONNECT time, I guess.

> > +   genlmsg_multicast_netns(_fam, wiphy_net(>wiphy), msg, 0,
> > +   NL80211_MCGRP_MLME, gfp);
> 
> Is there a reason this is being multicast and not unicast to the 
> application that triggered the CONNECT?  Who else besides the supplicant 
> daemon might find this information useful?

That's a good point, we should send this to the CONNECT owner (and thus
obviously also require that there is one).

johannes

Re: [PATCH v3 5/5] mac80211_hwsim: add hwsim_tx_rate_flags to netlink attributes

[Johannes Berg]

On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
> For correct interpretation of a tx rate, the corresponding rate flags are
> needed (e.g. whether a HT-MCS rate or a legacy rate) and moreover for more
> correct simulation the other infos of the flags are important (like
> short-GI). Keeping compatibility, the flags are not integrated into the
> existing hwsim_tx_rate, but transmitted as an additional netlink attribute.

Also applied.

johannes

Re: [PATCH v3 4/5] mac80211_hwsim: add permanent mac address option for new radios

[Johannes Berg]

On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
> 
>  
> + if (info->attrs[HWSIM_ATTR_PERM_ADDR]) {
> + if (nla_len(info->attrs[HWSIM_ATTR_PERM_ADDR]) != ETH_ALEN) {
> + pr_debug("mac80211_hwsim: MAC has wrong size\n");
> + return -EINVAL;
> + }

Oh, also - don't do this.

1) don't print, use NL_SET_ERR_MSG().

2) use the policy to validate lengths

> + if (!is_local_ether_addr(
> + nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
> + pr_debug("mac80211_hwsim: MAC is not locally 
> administered\n");
> + return -EINVAL;
> + }

This doesn't really matter - it's purely virtual. I think we can avoid
that.

> + if (get_hwsim_data_ref_from_addr(
> + nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
> + pr_debug("mac80211_hwsim: perm MAC already in use\n");
> + return -EINVAL;
> + }

This is racy afaict - remove it and return a clash later when you fail
to insert the new radio.

johannes

Re: WARNING in rfkill_alloc

[Dmitry Vyukov]

On Mon, Jan 15, 2018 at 1:01 PM, Johannes Berg
 wrote:
> On Mon, 2018-01-15 at 10:12 +0100, Dmitry Vyukov wrote:
>
>> However, there can be some surprising things, for example, executing
>> one ioctl/setsockopt with data meant for another one, or these
>> 0x are actually mean 0 (for involved reasons),
>
> I think those fff was actually what was throwing me off.
>
>> or we
>> can simply have bugs in these descriptions so they don't match C
>> structures and then all data is messed/shifted.
>
> No, I think this part was OK.
>
>> If this representation does not make sense to you right away, your
>> best bet is looking at/running the C reproducer where you can see true
>> data layout:
>>
>>
> [...]
> Yeah, good point, I should've just done that.
>
>> > Ah, then again, now I see the fault injection - I guess dev_set_name()
>> > just failed and we didn't check the return value, will fix that.
>>
>> Yes, it's highly likely the root cause. The raw.log file shows there
>> there was an immediately preceding fault in kmalloc in the same
>> process, in a close stack.
>
> Yep, I submitted the fix now (with the correct reported-by).
>
> Also for the other one, the wiphy_register() warning.

Thanks!

Re: [PATCH v3 4/5] mac80211_hwsim: add permanent mac address option for new radios

[Johannes Berg]

On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:

> To do not break the operation with legacy software using hwsim, the new
> address is set twice. The problem here is, the netlink call backs use
> wiphy->addresses[1] as identification of a radio and not the proposed
> permanent address (wiphy->addresses[0]). This design decision is not
> documented in the kernel repo, therefore this patch simply reproduces this,
> but with the same address.

It's weird to have the same address twice - perhaps we can XOR 0x40 or
so?

johannes

Re: [PATCH v3 3/5] mac80211_hwsim: add generation count for netlink dump operation

[Johannes Berg]

On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
>  Change the dump iteration to be independent from
> increasing radio indices on radio list.

You can't do that, data structures can be deleted while the dump is
ongoing.

johannes

Re: WARNING in rfkill_alloc

[Johannes Berg]

On Mon, 2018-01-15 at 10:12 +0100, Dmitry Vyukov wrote:

> However, there can be some surprising things, for example, executing
> one ioctl/setsockopt with data meant for another one, or these
> 0x are actually mean 0 (for involved reasons), 

I think those fff was actually what was throwing me off.

> or we
> can simply have bugs in these descriptions so they don't match C
> structures and then all data is messed/shifted.

No, I think this part was OK.

> If this representation does not make sense to you right away, your
> best bet is looking at/running the C reproducer where you can see true
> data layout:
> 
> 
[...]
Yeah, good point, I should've just done that.

> > Ah, then again, now I see the fault injection - I guess dev_set_name()
> > just failed and we didn't check the return value, will fix that.
> 
> Yes, it's highly likely the root cause. The raw.log file shows there
> there was an immediately preceding fault in kmalloc in the same
> process, in a close stack.

Yep, I submitted the fix now (with the correct reported-by).

Also for the other one, the wiphy_register() warning.

johannes

Re: [PATCH v3 2/5] mac80211_hwsim: add hashtable with mac address keys for faster lookup

[Johannes Berg]

On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
> This patch adds a rhastable for mac address lookup of hwsim radios. This
> especially improve the speed on reception of a netlink message with a new
> frame. Although redundant, we keep holding a normal list for all radios,
> since the rhashtable_walk interface adds a lot of overhead for iterating
> over all radios and the doc of rhashtable recommend a redundant structure
> for stable walks in such situations.
> 
> Since rhashtable is rcu protected we do not need a lock for delivering
> frames and thus improving this scenario.

Applied, but

> +static u32 hwsim_table_hash(const void *addr, u32 len, u32 seed)
> +{
> + /* Use last four bytes of hw addr as hash index */
> + return jhash_1word(*(u32 *)(addr + 2), seed);
> +}

I removed this, because that isn't actually safe - there's only 2-byte
alignment guaranteed... rhashtable will just do jhash() on the whole
thing, which should be fast enough.

johannes

Re: [PATCH v3 1/5] mac80211_hwsim: add workqueue to wait for deferred radio deletion on mod unload

[Johannes Berg]

On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
> When closing multiple wmediumd instances with many radios and try to
> unload the  mac80211_hwsim module, it may happen that the work items live
> longer than the module. To wait especially for this deletion work items,
> add a work queue, otherwise flush_scheduled_work would be necessary.
> 
Applied this to mac80211, thanks.

johannes

Re: [PATCH] bcma: Prevent build of PCI host features in module

[Matt Redfearn]

Hi Kalle,

On 15/01/18 10:07, Kalle Valo wrote:

Matt Redfearn  writes:


Attempting to build bcma.ko with BCMA_DRIVER_PCI_HOSTMODE=y results in
a build error due to use of symbols not exported from vmlinux:

ERROR: "pcibios_enable_device" [drivers/bcma/bcma.ko] undefined!
ERROR: "register_pci_controller" [drivers/bcma/bcma.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1

To prevent this, don't allow the host mode feature to be built if
CONFIG_BCMA=m

Signed-off-by: Matt Redfearn 

---

  drivers/bcma/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 02d78f6cecbb..4294784b9cf1 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -55,7 +55,7 @@ config BCMA_DRIVER_PCI
  
  config BCMA_DRIVER_PCI_HOSTMODE

bool "Driver for PCI core working in hostmode"
-   depends on MIPS && BCMA_DRIVER_PCI
+   depends on MIPS && BCMA_DRIVER_PCI && BCMA = y


Is this a new regression? Do you know the commit which broke this?


As far as I can see, pcibios_enable_device and register_pci_controller 
have never being exported symbols from vmlinux, and an allmodconfig 
build with CONFIG_MIPS_COBALT=y (MIPS cobalt platform) which attempts to 
put this this functionality into a module would never have linked 
successfully. As such it can be traced back to when this functionality 
was added, 49dc9577155576b10ff79f0c1486c816b01f58bf ("bcma: add PCIe 
host controller").




Is it somehow related to this:

bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets
https://patchwork.kernel.org/patch/10162839/

Or are these two separate issues?



Separate issues - that one fixes allmodconfig when CONFIG_MIPS_GENERIC=y 
(MIPS generic platform) which does not define the struct pci_controller 
- that error was really introduced when that struct definition was 
removed for the generic platform by 
c5611df968047fb0b38156497b4242730ef66108 ("MIPS: PCI: Introduce 
CONFIG_PCI_DRIVERS_LEGACY").


I think both patches are valid since they fix errors building 
allmodconfig on 2 separate MIPS platforms.


Thanks,
Matt

pull-request: mac80211 2018-01-15

[Johannes Berg]

Hi Dave,

I know this comes last minute, so if it doesn't make it then
I guess we can live with that, but I got the earliest of the
patches here on Wednesday last week, and that was the most
uninteresting one - the others didn't come in until Saturday.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 8155aedf512edd3f88ef19f7cacf476ace7d1322:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf (2018-01-14 
11:01:33 -0500)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2018-01-15

for you to fetch changes up to 59b179b48ce2a6076448a44531242ac2b3f6cef2:

  cfg80211: check dev_set_name() return value (2018-01-15 11:35:06 +0100)


More fixes:
 * hwsim:
- properly flush deletion works at module unload
- validate # of channels passed from userspace
 * cfg80211:
- fix RCU locking regression
- initialize on-stack channel data for nl80211 event
- check dev_set_name() return value


Benjamin Beichler (1):
  mac80211_hwsim: add workqueue to wait for deferred radio deletion on mod 
unload

Dominik Brodowski (1):
  nl80211: take RCU read lock when calling ieee80211_bss_get_ie()

Johannes Berg (3):
  cfg80211: fully initialize old channel for event
  mac80211_hwsim: validate number of different channels
  cfg80211: check dev_set_name() return value

 drivers/net/wireless/mac80211_hwsim.c | 17 +++--
 include/net/cfg80211.h|  2 ++
 net/wireless/core.c   |  8 +++-
 net/wireless/core.h   |  2 --
 net/wireless/nl80211.c| 11 +++
 net/wireless/reg.c|  3 +--
 6 files changed, 32 insertions(+), 11 deletions(-)

Re: brcmfmac: set p2p_disc error on BCM4350

[William Dauchy]

Hello Arend,

Did you had the chance to look into this?

On Mon, Jan 08, 2018 at 11:30:12AM +0100, William Dauchy wrote:
> Hello Arend,
> 
> Thanks for your quick answer.
> 
> On Mon, Jan 08, 2018 at 10:31:52AM +0100, Arend van Spriel wrote:
> > This has been reported before, but not been able to look into it. Diving
> > into firmware it seems that error can only occur if the mac address for the
> > p2pdev interface is already used. So if possible can you rebuild the driver
> > with the patch below applied and provide dmesg output and output of
> > 'ifconfig -a'.
> > 
> > Regards,
> > Arend
> > ---8<
> > 
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > b/drivers/n
> > index 2ee5413..f96ad37 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > @@ -2065,6 +2065,7 @@ static struct wireless_dev
> > *brcmf_p2p_create_p2pdev(struc
> > int err;
> > u32 bsscfgidx;
> > 
> > +   brcmf_err("enter: mac=%pM\n", addr);
> > if (p2p->bss_idx[P2PAPI_BSSCFG_DEVICE].vif)
> > return ERR_PTR(-ENOSPC);
> > 
> >
> 
> Here is my brcmfmac dmesg output with patch applied
> (please tell me if you need complete dmesg)
> 
> brcmfmac: brcmf_fw_map_chip_to_name: using brcm/brcmfmac4350-pcie.bin for 
> chip 0x004350(17232) rev 0x08
> brcmfmac :3a:00.0: Direct firmware load for brcm/brcmfmac4350-pcie.txt 
> failed with error -2
> brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Oct 22 2015 06:16:26 
> version 7.35.180.119 (r594535) FWID 01-e791c176
> brcmfmac :3a:00.0 wlp58s0: renamed from wlan0
> brcmfmac: brcmf_p2p_create_p2pdev: enter: mac=00:00:00:00:00:00
> brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
> brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlp58s0 type 10 failed: 
> err=-16
> 
> ifconfig shows:
> 
> wlp58s0: flags=4163  mtu 1500
> inet 192.168.18.77  netmask 255.255.255.0  broadcast 192.168.18.255
> inet6 fe80::3517:e1be:5bc7:899  prefixlen 64  scopeid 0x20
> ether 30:52:cb:83:71:33  txqueuelen 1000  (Ethernet)
> RX packets 26557  bytes 34149730 (32.5 MiB)
> RX errors 0  dropped 0  overruns 0  frame 0
> TX packets 14965  bytes 2298777 (2.1 MiB)
> TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

Best,
-- 
William

Re: [PATCH] bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets

[James Hogan]

On Sun, Jan 14, 2018 at 01:34:02PM -0800, Guenter Roeck wrote:
> Mips builds with BCMA host mode enabled fail in mainline and -next
> with:
> 
> In file included from include/linux/bcma/bcma.h:10:0,
>  from drivers/bcma/bcma_private.h:9,
>from drivers/bcma/main.c:8:
> include/linux/bcma/bcma_driver_pci.h:218:24: error:
>   field 'pci_controller' has incomplete type
> 
> Bisect points to commit d41e6858ba58c ("MIPS: Kconfig: Set default MIPS
> system type as generic") as the culprit. Analysis shows that the commmit
> changes PCI configuration and enables PCI_DRIVERS_GENERIC. This in turn
> disables PCI_DRIVERS_LEGACY. 'struct pci_controller' is, however, only
> defined if PCI_DRIVERS_LEGACY is enabled.
> 
> Ultimately that means that BCMA_DRIVER_PCI_HOSTMODE depends on
> PCI_DRIVERS_LEGACY. Add the missing dependency.
> 
> Fixes: d41e6858ba58c ("MIPS: Kconfig: Set default MIPS system type as ...")

Well, technically I think commit c5611df96804 ("MIPS: PCI: Introduce
CONFIG_PCI_DRIVERS_LEGACY") is to blame (Cc'ing paul), and the first bad
commit would be commit eed0eabd12ef ("MIPS: generic: Introduce generic
DT-based board support") which selects PCI_DRIVERS_GENERIC and is the
only platform to do so. Both commits were first in v4.9-rc1 and I can
reproduce this problem at that latter commit with the appropriate
configuration.

But yes clearly the mentioned commit does also expose that existing
problem more widely and to the default allmodconfig, and it looks like a
reasonable approach for now, so if some mention of the other two commits
is added:

Reviewed-by: James Hogan 

Having it in 4.15 would be great.

Cheers
James

> Cc: Matt Redfearn 
> Cc: James Hogan 
> Signed-off-by: Guenter Roeck 
> ---
> I am aware that this problem has been reported several times. I have
> not been able to find a fix, but I may have missed it. If so, my
> apologies for the noise.
> 
> Also note that this is not the only fix required; commit d41e6858ba58c,
> as simple as it looks like, does a pretty good job messing up
> "mips:allmodconfig" builds.
> 
>  drivers/bcma/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
> index 02d78f6cecbb..ba8acca036df 100644
> --- a/drivers/bcma/Kconfig
> +++ b/drivers/bcma/Kconfig
> @@ -55,7 +55,7 @@ config BCMA_DRIVER_PCI
>  
>  config BCMA_DRIVER_PCI_HOSTMODE
>   bool "Driver for PCI core working in hostmode"
> - depends on MIPS && BCMA_DRIVER_PCI
> + depends on MIPS && BCMA_DRIVER_PCI && PCI_DRIVERS_LEGACY
>   help
> PCI core hostmode operation (external PCI bus).
>  
> -- 
> 2.7.4
> 


signature.asc
Description: Digital signature

Re: [PATCH] bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets

[Kalle Valo]

Guenter Roeck  writes:

> [ copying linux-mips ]
>
> On 01/14/2018 01:34 PM, Guenter Roeck wrote:
>> Mips builds with BCMA host mode enabled fail in mainline and -next
>> with:
>>
>> In file included from include/linux/bcma/bcma.h:10:0,
>>   from drivers/bcma/bcma_private.h:9,
>>   from drivers/bcma/main.c:8:
>> include/linux/bcma/bcma_driver_pci.h:218:24: error:
>>  field 'pci_controller' has incomplete type
>>
>> Bisect points to commit d41e6858ba58c ("MIPS: Kconfig: Set default MIPS
>> system type as generic") as the culprit. Analysis shows that the commmit
>> changes PCI configuration and enables PCI_DRIVERS_GENERIC. This in turn
>> disables PCI_DRIVERS_LEGACY. 'struct pci_controller' is, however, only
>> defined if PCI_DRIVERS_LEGACY is enabled.
>>
>> Ultimately that means that BCMA_DRIVER_PCI_HOSTMODE depends on
>> PCI_DRIVERS_LEGACY. Add the missing dependency.
>>
>> Fixes: d41e6858ba58c ("MIPS: Kconfig: Set default MIPS system type as ...")
>> Cc: Matt Redfearn 
>> Cc: James Hogan 
>> Signed-off-by: Guenter Roeck 
>> ---
>> I am aware that this problem has been reported several times. I have
>> not been able to find a fix, but I may have missed it. If so, my
>> apologies for the noise.
>>
> I should have said "I have not been able to find a patch fixing it".
>
>> Also note that this is not the only fix required; commit d41e6858ba58c,
>> as simple as it looks like, does a pretty good job messing up
>> "mips:allmodconfig" builds.
>>
> ... nor did I find patch(es) fixing the other build problem(s) introduced
> by d41e6858ba58c.

As I forgot to cc linux-mips on my previous email: I'm planning to queue
this for v4.15.

Over the weeked I got this bcma patch, but don't know if it's related or
not:

bcma: Prevent build of PCI host features in module
https://patchwork.kernel.org/patch/10161087/

And Guenter's patch is:

https://patchwork.kernel.org/patch/10162839/

Which one should I take? Adding also Matt.

And Matt also submitted similar patch for ssb:

ssb: Prevent build of PCI host features in module
https://patchwork.kernel.org/patch/10161131/

-- 
Kalle Valo

Re: [PATCH v2] brcmfmac: fix CLM load error for legacy chips when user helper is enabled

[Wright Feng]

On 2018/1/12 下午 07:16, Kalle Valo wrote:

Arend van Spriel  writes:


On 1/12/2018 8:44 AM, Wright Feng wrote:

For legacy chips without CLM blob files, kernel with user helper function
returns -EAGAIN when we request_firmware() for blob file.


_Why_ is the -EAGAIN returned? Is it because of user space, due to
timing when loading the brcmfmac module or what? You should explain the
problem in detail in the commit log and why this is the right approach
to fix the problem.

Based on the commit log to me this still looks like a random attempt to
workaround a bug, not a proper fix.



It is not about the timing issue, it is about "the clm blob is not
existence in firmware path for legacy firmware with CLM data built-in" 
and the -ENOENT error is transferred to -EAGAIN in firmware_class.c.


Here is explanation of -EAGAIN returned in detail.
In drivers/base/firmware_class.c:__fw_state_wait_common, it returns
-ENOENT to indicate the clm_blob file is not found via user helper.
However, in drivers/base/firmware_class.c:_request_firmware_load, all
errors with fw status aborted are transferred to:

1. -ERESTARTSYS: The signal is pending and the task is interruptible.
Before 76098b36b5db ("firmware: send -EINTR on signal abort on fallback
mechanism"), all errors with fw status aborted are transferred to
-EAGAIN.

2. -EAGAIN: All others fw status aborted(include -ENOENT) except for
-ERESTARTSYS.

And that's why I handle -EAGAIN error to let driver using CLM data in
firmware.


In this case, brcmf_bus_started gets error and failed to bring up
legacy chips. Because of that, we should continue with CLM data
currently present in firmware if getting -EAGAIN when doing
request_firmware().

Signed-off-by: Wright Feng 
---
v2: remove retry from patch v1
---
   drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 6a59d06..0baab4c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -182,7 +182,7 @@ static int brcmf_c_process_clm_blob(struct brcmf_if *ifp)

err = request_firmware(, clm_name, dev);
if (err) {
-   if (err == -ENOENT) {
+   if (err == -ENOENT || err == -EAGAIN) {
brcmf_dbg(INFO, "continue with CLM data currently present in 
firmware\n");
return 0;
}


Why don't we just fall-back to "CLM in firmware" regardless of the
error code?


Indeed, I was thinking the same.


For the firmwares with CLM data built-in, it is okay to continue the
bringing-up flow.
But when users put clm_blob file to firmware path, the corresponding
firmware may not have CLM data built-in. If we fall-back to use empty
CLM data in firmware when hitting other errors like "-ENOMEM" or
"-EINTR", the country code revision will be null and users will meet
error when trying to connect to access point.
It is fine to fall-back to "CLM in firmware" regardless of the error
code, but it would be better to print error log to indicate the error if
the returned error codes are not what we expected.


Please let me know your opinion of below commit log and diff, if it is
okay to you, I will post Patch v3 later.



For legacy chips without CLM blob files, kernel with user helper
function returns -EAGAIN when we request_firmware() for blob file. So,
it got failed when bring up legacy chips. We expect the CLM blob files
is not existence in firmware path, however the -ENOENT error is
transferred to -EAGAIN in firmware_class.c.
Because of that, we continue with CLM data currently present in
firmware if getting error in doing request_firmware().

---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 12 
++--

 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c

index 6a59d06..aaab0e6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -182,12 +182,12 @@ static int brcmf_c_process_clm_blob(struct 
brcmf_if *ifp)


err = request_firmware(, clm_name, dev);
if (err) {
-   if (err == -ENOENT) {
-			brcmf_dbg(INFO, "continue with CLM data currently present in 
firmware\n");

-   return 0;
-   }
-   brcmf_err("request CLM blob file failed (%d)\n", err);
-   return err;
+   if (err == -ENOENT || err == -EAGAIN)
+   brcmf_info("continue with CLM data in FW\n");
+   else
+   brcmf_err("request clm_blob failed(%d) continue with CLM 
data in FW\n",
+   

Re: [PATCH] bcma: Prevent build of PCI host features in module

[Kalle Valo]

Matt Redfearn  writes:

> Attempting to build bcma.ko with BCMA_DRIVER_PCI_HOSTMODE=y results in
> a build error due to use of symbols not exported from vmlinux:
>
> ERROR: "pcibios_enable_device" [drivers/bcma/bcma.ko] undefined!
> ERROR: "register_pci_controller" [drivers/bcma/bcma.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
>
> To prevent this, don't allow the host mode feature to be built if
> CONFIG_BCMA=m
>
> Signed-off-by: Matt Redfearn 
>
> ---
>
>  drivers/bcma/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
> index 02d78f6cecbb..4294784b9cf1 100644
> --- a/drivers/bcma/Kconfig
> +++ b/drivers/bcma/Kconfig
> @@ -55,7 +55,7 @@ config BCMA_DRIVER_PCI
>  
>  config BCMA_DRIVER_PCI_HOSTMODE
>   bool "Driver for PCI core working in hostmode"
> - depends on MIPS && BCMA_DRIVER_PCI
> + depends on MIPS && BCMA_DRIVER_PCI && BCMA = y

Is this a new regression? Do you know the commit which broke this? 

Is it somehow related to this:

bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets
https://patchwork.kernel.org/patch/10162839/

Or are these two separate issues?

-- 
Kalle Valo

Re: [PATCH] bcma: Fix 'allmodconfig' and BCMA builds on MIPS targets

[Kalle Valo]

Guenter Roeck  writes:

> Mips builds with BCMA host mode enabled fail in mainline and -next
> with:
>
> In file included from include/linux/bcma/bcma.h:10:0,
>  from drivers/bcma/bcma_private.h:9,
>from drivers/bcma/main.c:8:
> include/linux/bcma/bcma_driver_pci.h:218:24: error:
>   field 'pci_controller' has incomplete type
>
> Bisect points to commit d41e6858ba58c ("MIPS: Kconfig: Set default MIPS
> system type as generic") as the culprit. Analysis shows that the commmit
> changes PCI configuration and enables PCI_DRIVERS_GENERIC. This in turn
> disables PCI_DRIVERS_LEGACY. 'struct pci_controller' is, however, only
> defined if PCI_DRIVERS_LEGACY is enabled.
>
> Ultimately that means that BCMA_DRIVER_PCI_HOSTMODE depends on
> PCI_DRIVERS_LEGACY. Add the missing dependency.
>
> Fixes: d41e6858ba58c ("MIPS: Kconfig: Set default MIPS system type as ...")

So the first release for this commit was v4.15-rc1 so this is a recent
regression.

> Cc: Matt Redfearn 
> Cc: James Hogan 
> Signed-off-by: Guenter Roeck 
> ---
> I am aware that this problem has been reported several times. I have
> not been able to find a fix, but I may have missed it. If so, my
> apologies for the noise.

As this fixes a build problem I'm planning to queue this for v4.15.

-- 
Kalle Valo

Re: [PATCH 02/10] qtnfmac: pass complete channel data between driver and firmware

[Sergey Matyukevich]

Hello Kalle,

> >> > +/**
> >> >   * struct qlink_chandef - qlink channel definition
> >> >   *
> >> > + * @chan: primary channel definition
> >> >   * @center_freq1: center frequency of first segment
> >> >   * @center_freq2: center frequency of second segment (80+80 only)
> >> >   * @width: channel width, one of @enum qlink_channel_width
> >> >   */
> >> >  struct qlink_chandef {
> >> > + struct qlink_channel chan;
> >> >   __le16 center_freq1;
> >> >   __le16 center_freq2;
> >> >   u8 width;
> >> > - u8 rsvd[3];
> >> > + u8 rsvd;
> >> >  } __packed;
> >>
> >> Doesn't this break backwards compatibility with the older firmware? The
> >> basic princinple is that old firmware images continue to work with newer
> >> driver (or there will be a firmware image with new name, eg. fw-2.bin).
> >> You can check how iwlwifi does that.
> >
> > Yes, it breaks. That is why we increment qlink protocol version in each
> > change affecting backwards compatibility. So driver is going to work only
> > with matching firmware. This is a very simplistic approach, but it looks
> > reasonable for current stage of development since we keep adding features.
> 
> Everyone are always adding new features, that's no excuse to break
> backwards compatibility with user space. In the future you really need
> to come up a way to handle the firmware interface breaks gracefully,
> just like iwlwifi does.
> 
> Related to this, any progress on getting the firmware image to
> linux-firmware?

Here is a brief status. In our case, one of the SoC cores runs Linux, so we
have to accompany firmware image with SDK containing all the GPL components.
SDK is not appropriate for linux-firmware repository. That is why the plan
is to make SDK accessible via Quantenna github or website, and then
to get the firmware image to linux-firmware repository. Actually, firmware
image can be submitted any time. But our understanding is that SDK should be
released prior to the next attempt to submit firmware image. So currently
the work is ongoing on making SDK fully GPL compliant, e.g. sorting out
licensing of 3rd party modules.

Regards,
Sergey

Re: WARNING in rfkill_alloc

[Dmitry Vyukov]

On Mon, Jan 15, 2018 at 9:57 AM, Johannes Berg
 wrote:
> Hi,
>
>> RIP: 0010:rfkill_alloc+0x2c0/0x380 net/rfkill/core.c:930
>
> This seems pretty obvious - there's no name given.
>
>>   wiphy_new_nm+0x159c/0x21d0 net/wireless/core.c:487
>>   ieee80211_alloc_hw_nm+0x4b4/0x2140 net/mac80211/main.c:531
>
> which is strange, because we try to validate the name here.
>
> Can you help me read this?
>
> sendmsg$nl_generic(r1, &(0x7fb3e000-0x38)={&(0x7fd4a000-
> 0xc)={0x10, 0x0, 0x0, 0x0}, 0xc,
> &(0x7f007000)={&(0x7f1ca000)={0x14, 0x1c, 0x109,
> 0x, 0x, {0x4, 0x0, 0x0}, []}, 0x14},
> 0x1, 0x0, 0x0, 0x0}, 0x0)
>
> I've reformatted it as
>
> sendmsg$nl_generic(
> r1,
> &(0x7fb3e000-0x38)={
> addr=   &(0x7fd4a000-0xc)={
> 0x10, 0x0, 0x0, 0x0
> },
> addrlen=0xc,
> vec=&(0x7f007000)={
> ptr=&(0x7f1ca000)={
> 0x14, 0x1c, 0x109, 0x,
> 0x, {0x4, 0x0, 0x0}, []
> },
> len=0x14
> },
> vlen=   0x1,
> ctrl=   0x0,
> ctrllen=0x0,
> f=  0x0
> },
> 0x0
> )
>
> but am still getting lost - what exactly is the *byte* sequence inside
> the (full) message (including headers)?

Hi,

I think you decoded it correctly. The netlink message is:

{0x14, 0x1c, 0x109, 0x, 0x, {0x4, 0x0, 0x0}, []}

0x14 length, 0x1c is type, etc

These numbers are input data for there descriptions:
https://github.com/google/syzkaller/blob/master/sys/linux/socket_netlink.txt
which generally match C structures as you expect.

However, there can be some surprising things, for example, executing
one ioctl/setsockopt with data meant for another one, or these
0x are actually mean 0 (for involved reasons), or we
can simply have bugs in these descriptions so they don't match C
structures and then all data is messed/shifted.

If this representation does not make sense to you right away, your
best bet is looking at/running the C reproducer where you can see true
data layout:

  *(uint64_t*)0x20b3dfc8 = 0x20d49ff4;
  *(uint32_t*)0x20b3dfd0 = 0xc;
  *(uint64_t*)0x20b3dfd8 = 0x20007000;
  *(uint64_t*)0x20b3dfe0 = 1;
  *(uint64_t*)0x20b3dfe8 = 0;
  *(uint64_t*)0x20b3dff0 = 0;
  *(uint32_t*)0x20b3dff8 = 0;
  *(uint16_t*)0x20d49ff4 = 0x10;
  *(uint16_t*)0x20d49ff6 = 0;
  *(uint32_t*)0x20d49ff8 = 0;
  *(uint32_t*)0x20d49ffc = 0;
  *(uint64_t*)0x20007000 = 0x201ca000;
  *(uint64_t*)0x20007008 = 0x14;
  *(uint32_t*)0x201ca000 = 0x14;
  *(uint16_t*)0x201ca004 = 0x1c;
  *(uint16_t*)0x201ca006 = 0x109;
  *(uint32_t*)0x201ca008 = 0;
  *(uint32_t*)0x201ca00c = 0;
  *(uint8_t*)0x201ca010 = 4;
  *(uint8_t*)0x201ca011 = 0;
  *(uint16_t*)0x201ca012 = 0;
  syscall(__NR_sendmsg, r[1], 0x20b3dfc8, 0);


> Ah, then again, now I see the fault injection - I guess dev_set_name()
> just failed and we didn't check the return value, will fix that.

Yes, it's highly likely the root cause. The raw.log file shows there
there was an immediately preceding fault in kmalloc in the same
process, in a close stack.

Re: WARNING in wiphy_register

[Dmitry Vyukov]

On Mon, Jan 15, 2018 at 9:22 AM, Johannes Berg
 wrote:
> Hi syzbot maintainers,
>
> Thanks for the report.
>
>>   hwsim_new_radio_nl+0x5b7/0x7c0 drivers/net/wireless/mac80211_hwsim.c:3152
>>   genl_family_rcv_msg+0x7b7/0xfb0 net/netlink/genetlink.c:599
>>   genl_rcv_msg+0xb2/0x140 net/netlink/genetlink.c:624
>
> You're getting into the kernel via generic netlink receive, so just as
> an FYI - the generic netlink numbers aren't stable across systems, so
> your reproducer has a quite good chance of not working without your
> kernel .config and (virt) hardware environment.

Hi Johannes,

Thanks for the feeback.
syzbot tests within a net namespace (which is free of eth0 and other
stuff) and does setup of devices in that namespace. For bugs, it first
tries to reproduce them in that environment and if that succeeds it
tries to simplify the reproducer by stripping namespace/device setup
(which is quite verbose), and if that succeeds it provides this
simplified reproducer.
In this case it decided that namespace setup is not important. .config
is still important, but it is provided.

Are you able to reproduce the WARNING with the provided config? If
not, we can look as to how to improve this.


> I'll take a look at this and the rfkill one, I assume that there are
> some sanity checks missing in hwsim generic netlink when it builds a
> radio struct.
>
> However, I can't really promise that I'll be able to validate the
> changes against your reproducer.
>
> johannes
>
> --
> You received this message because you are subscribed to the Google Groups 
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/syzkaller-bugs/1516004561.410.3.camel%40sipsolutions.net.
> For more options, visit https://groups.google.com/d/optout.

Re: WARNING in rfkill_alloc

[Johannes Berg]

Hi,

> RIP: 0010:rfkill_alloc+0x2c0/0x380 net/rfkill/core.c:930

This seems pretty obvious - there's no name given.

>   wiphy_new_nm+0x159c/0x21d0 net/wireless/core.c:487
>   ieee80211_alloc_hw_nm+0x4b4/0x2140 net/mac80211/main.c:531

which is strange, because we try to validate the name here.

Can you help me read this?

sendmsg$nl_generic(r1, &(0x7fb3e000-0x38)={&(0x7fd4a000-
0xc)={0x10, 0x0, 0x0, 0x0}, 0xc,
&(0x7f007000)={&(0x7f1ca000)={0x14, 0x1c, 0x109,
0x, 0x, {0x4, 0x0, 0x0}, []}, 0x14},
0x1, 0x0, 0x0, 0x0}, 0x0)

I've reformatted it as

sendmsg$nl_generic(
r1,
&(0x7fb3e000-0x38)={
addr=   &(0x7fd4a000-0xc)={
0x10, 0x0, 0x0, 0x0
},
addrlen=0xc,
vec=&(0x7f007000)={
ptr=&(0x7f1ca000)={
0x14, 0x1c, 0x109, 0x,
0x, {0x4, 0x0, 0x0}, []
},
len=0x14
},
vlen=   0x1,
ctrl=   0x0,
ctrllen=0x0,
f=  0x0
},
0x0
)

but am still getting lost - what exactly is the *byte* sequence inside
the (full) message (including headers)?


Ah, then again, now I see the fault injection - I guess dev_set_name()
just failed and we didn't check the return value, will fix that.

johannes

[PATCH] mac80211_hwsim: validate number of different channels

[Johannes Berg]

From: Johannes Berg 

When creating a new radio on the fly, hwsim allows this
to be done with an arbitrary number of channels, but
cfg80211 only supports a limited number of simultaneous
channels, leading to a warning.

Fix this by validating the number - this requires moving
the define for the maximum out to a visible header file.

Reported-by: syzbot+8dd9051ff19940290...@syzkaller.appspotmail.com
Fixes: b59ec8dd4394 ("mac80211_hwsim: fix number of channels in interface 
combinations")
Signed-off-by: Johannes Berg 
---
 drivers/net/wireless/mac80211_hwsim.c | 5 +
 include/net/cfg80211.h| 2 ++
 net/wireless/core.h   | 2 --
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c 
b/drivers/net/wireless/mac80211_hwsim.c
index ccd573e53c92..f6d4a50f1bdb 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3121,6 +3121,11 @@ static int hwsim_new_radio_nl(struct sk_buff *msg, 
struct genl_info *info)
if (info->attrs[HWSIM_ATTR_CHANNELS])
param.channels = nla_get_u32(info->attrs[HWSIM_ATTR_CHANNELS]);
 
+   if (param.channels > CFG80211_MAX_NUM_DIFFERENT_CHANNELS) {
+   GENL_SET_ERR_MSG(info, "too many channels specified");
+   return -EINVAL;
+   }
+
if (info->attrs[HWSIM_ATTR_NO_VIF])
param.no_vif = true;
 
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index cb4d92b79cd9..fb94a8bd8ab5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -815,6 +815,8 @@ struct cfg80211_csa_settings {
u8 count;
 };
 
+#define CFG80211_MAX_NUM_DIFFERENT_CHANNELS 10
+
 /**
  * struct iface_combination_params - input parameters for interface 
combinations
  *
diff --git a/net/wireless/core.h b/net/wireless/core.h
index d2f7e8b8a097..eaff636169c2 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -507,8 +507,6 @@ void cfg80211_stop_p2p_device(struct 
cfg80211_registered_device *rdev,
 void cfg80211_stop_nan(struct cfg80211_registered_device *rdev,
   struct wireless_dev *wdev);
 
-#define CFG80211_MAX_NUM_DIFFERENT_CHANNELS 10
-
 #ifdef CONFIG_CFG80211_DEVELOPER_WARNINGS
 #define CFG80211_DEV_WARN_ON(cond) WARN_ON(cond)
 #else
-- 
2.15.1

Re: WARNING in wiphy_register

[Johannes Berg]

Hi syzbot maintainers,

Thanks for the report.

>   hwsim_new_radio_nl+0x5b7/0x7c0 drivers/net/wireless/mac80211_hwsim.c:3152
>   genl_family_rcv_msg+0x7b7/0xfb0 net/netlink/genetlink.c:599
>   genl_rcv_msg+0xb2/0x140 net/netlink/genetlink.c:624

You're getting into the kernel via generic netlink receive, so just as
an FYI - the generic netlink numbers aren't stable across systems, so
your reproducer has a quite good chance of not working without your
kernel .config and (virt) hardware environment.

I'll take a look at this and the rfkill one, I assume that there are
some sanity checks missing in hwsim generic netlink when it builds a
radio struct.

However, I can't really promise that I'll be able to validate the
changes against your reproducer.

johannes

Re: [PATCH v3] nl80211: take RCU read lock when calling ieee80211_bss_get_ie()

[Johannes Berg]

On Mon, 2018-01-15 at 08:12 +0100, Dominik Brodowski wrote:
> As ieee80211_bss_get_ie() derefences an RCU to return ssid_ie, both
> the call to this function and any operation on this variable need
> protection by the RCU read lock.
> 
> Fixes: 44905265bc15 ("nl80211: don't expose wdev->ssid for most interfaces")
> Signed-off-by: Dominik Brodowski 
> ---
> 
> > but after, perhaps it's easier to just do
> > 
> > if (ssid_ie &&
> > nla_put(...)
> > goto nla_put_failure_rcu_locked;
> > 
> > and avoid the extra label (but yeah, it's getting late)
> 
> OK, done that (and updated the commit message), and testet it.
> 

Applied, thanks!

johannes

[PATCH] cfg80211: fully initialize old channel for event

[Johannes Berg]

From: Johannes Berg 

Paul reported that he got a report about undefined behaviour
that seems to me to originate in using uninitialized memory
when the channel structure here is used in the event code in
nl80211 later.

He never reported whether this fixed it, and I wasn't able
to trigger this so far, but we should do the right thing and
fully initialize the on-stack structure anyway.

Reported-by: Paul Menzel 
Signed-off-by: Johannes Berg 
---
 net/wireless/reg.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 78e71b0390be..7b42f0bacfd8 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1769,8 +1769,7 @@ static void handle_reg_beacon(struct wiphy *wiphy, 
unsigned int chan_idx,
if (wiphy->regulatory_flags & REGULATORY_DISABLE_BEACON_HINTS)
return;
 
-   chan_before.center_freq = chan->center_freq;
-   chan_before.flags = chan->flags;
+   chan_before = *chan;
 
if (chan->flags & IEEE80211_CHAN_NO_IR) {
chan->flags &= ~IEEE80211_CHAN_NO_IR;
-- 
2.15.1