Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: Problems with mwifiex_pcie firmware activation
From: Stanislaw Gruszka @ 2016-08-12  8:13 UTC (permalink / raw)
  To: Amitkumar Karwar; +Cc: Nishant Sarmukadam, linux-wireless@vger.kernel.org
In-Reply-To: <fc70c75bfcab4e0e932274034b5b4493@SC-EXCH04.marvell.com>

On Fri, Aug 12, 2016 at 07:17:38AM +0000, Amitkumar Karwar wrote:
> The problem looks strange. The patch just splits mwifiex_check_fw_status() and increases poll count. It should not have any side-effects.
> Our code used to check winner status before this patch also.

Ok, I misread the patch. Anyway checking "winner status" seems
does not work well on some condition and prevent loading firmware
into device.

Stanislaw

^ permalink raw reply

* RE: Problems with mwifiex_pcie firmware activation
From: Amitkumar Karwar @ 2016-08-12  7:17 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Nishant Sarmukadam, linux-wireless@vger.kernel.org
In-Reply-To: <20160811122917.GB27059@redhat.com>

Hi Stanislaw,

> From: Stanislaw Gruszka [mailto:sgruszka@redhat.com]
> Sent: Thursday, August 11, 2016 5:59 PM
> To: Amitkumar Karwar
> Cc: Nishant Sarmukadam; linux-wireless@vger.kernel.org
> Subject: Re: Problems with mwifiex_pcie firmware activation
> 
> Hi
> 
> On Thu, Aug 11, 2016 at 10:21:58AM +0000, Amitkumar Karwar wrote:
> > > -----Original Message-----
> > > From: Stanislaw Gruszka [mailto:sgruszka@redhat.com]
> > > Sent: Thursday, August 11, 2016 2:31 PM
> > > To: Amitkumar Karwar
> > > Cc: Nishant Sarmukadam; linux-wireless@vger.kernel.org
> > > Subject: Re: Problems with mwifiex_pcie firmware activation
> > >
> > > On Thu, Jun 23, 2016 at 02:09:52PM +0200, Stanislaw Gruszka wrote:
> > > > On Thu, Jun 16, 2016 at 01:53:15PM +0200, Stanislaw Gruszka wrote:
> > > > > On Wed, Jun 15, 2016 at 01:47:53PM +0000, Amitkumar Karwar
> wrote:
> > > > > > Could you please share complete dmesg log for failure and
> > > successful cases?
> > > > >
> > > > > Dmesg from failure case is in attachment. I loose access to
> > > > > system where device initalize, I'll provide missed dmesg when I
> > > > > get back the access.
> > > >
> > > > Have the access now, dmesg is in attachment.
> > >
> > > Any news here ?
> >
> > Do you have below patch included? It increases poll count which may
> help in your case.
> > https://patchwork.kernel.org/patch/7973951/
> 
> Yes, I have that patch.
> 
> To clarify the issue: reverting above patch seems to helps with the
> problem. That what I basically did on first of my systems with mwifiex,
> that did not work similarly like second one. However I did not revert
> the patch but just comment out check_winner_status:
> 
> @@ -755,7 +755,7 @@ int mwifiex_dnld_fw(struct mwifiex_adapter *adapter,
>         }
> 
>         /* check if we are the winner for downloading FW */
> -       if (adapter->if_ops.check_winner_status) {
> +       if (false) {
>                 adapter->winner = 0;
>                 ret = adapter->if_ops.check_winner_status(adapter);
> 
> After running modified kernel once, the problem was gone on unmodified
> kernel! I suppose that once new firmware was uploaded to the device it
> somehow modify it (change it's internal firmware?) and make
> check_winner_status() works correctly. This is strange, but I can not
> find any other explanation and don't see any other changes in the first
> system that makes mwifiex suddenly work.
> 
> I could be wrong, but I suppose somebody who use pre 4.6 kernel (without
> the above patch committed as: 2fd5c6ed0b4fbf3c2d9314a24c82862ce5254d42
> "mwifiex: firmware download enhancements") with new enough mwifiex
> firmware, will not see the problem even after kernel update. But if
> somebody install 4.6 or newer kernel on pristine hardware, the device
> will not work.
> 

The problem looks strange. The patch just splits mwifiex_check_fw_status() and increases poll count. It should not have any side-effects.
Our code used to check winner status before this patch also.

Regards,
Amitkumar Karwar

^ permalink raw reply

* Re: [PATCH 1/4] cfg80211: rdev-ops: remove callback check from rdev_set_coalesce()
From: Johannes Berg @ 2016-08-12  5:59 UTC (permalink / raw)
  To: Arend Van Spriel; +Cc: linux-wireless
In-Reply-To: <ee2929c1-1f1b-ca1e-07d2-a3f43f8b73a8@broadcom.com>


> I was in doubt to raise the question first about getting this stuff
> consistent, ie. keep rdev-ops as flat as possible, but decided just
> to put it out there in patch format. My bad :-)

:-)

> If you want the rdev-ops to be safe against (future) callers not
> checking the callback, it seems you should add a check in all rdev-
> ops where the callback is optional.

Yeah, perhaps. I don't really mind hugely either way, but I think
removing them now would mostly be unwarranted churn.

johannes

^ permalink raw reply

* Re: [RESEND PATCH] nl80211: Allow GET_INTERFACE dumps to be filtered
From: Johannes Berg @ 2016-08-12  5:58 UTC (permalink / raw)
  To: Denis Kenzior, linux-wireless
In-Reply-To: <57ACC1F8.6070908@gmail.com>

On Thu, 2016-08-11 at 13:20 -0500, Denis Kenzior wrote:
> Hi Johannes,
> 
>  >> Speaking of indentation, can you point me to a doc of the rules I
> > 
> > > 
> > > should follow?
> > 
> > You've seen Documentation/CodingStyle?
> 
> Of course.  But that one doesn't discuss that you want your function 
> parameters to be aligned to the opening '('.  Is there a dialect 
> document specific to linux-wireless?

Sorry. I don't think this is specific to our part of the tree, and I'm
surprised it's not in there. But I see Arend also pointed you to
checkpatch.pl, which, I might add, you shouldn't always take as
authoritative since sometimes "fixing" things for it makes the code
look worse.

> The initial conditions are that:
> cb->args[0..2] == 0.
> 
> So on the first iteration we set filter_wiphy == -1 and check the
> filter attributes.  If set, we modify filter_wiphy accordingly.
> 
> Even if filter_wiphy is set to 0, the if statement should still never
> be entered afterwards since wp_start and if_start are incremented.
> 
> Is this what you're worried about? Do you see a fault in my logic?

No, I don't see a fault in the logic. I just think it's misleading. You
make the code look like it relies on filter_wiphy != 0, but then you go
and treat filter_wiphy==0 as a valid case.

In other places, like nl80211_prepare_wdev_dump(), we add 1 to the
wiphy and subtract it again later to avoid exactly this. Perhaps you
could do the same, and rely only on filter_wiphy instead of really
relying only on wp_start/if_start.

johannes

^ permalink raw reply

* Re: [PATCH] Staging: rtl8723au: os_intfs: fixed case statement is variable issue
From: sunbing @ 2016-08-11 22:50 UTC (permalink / raw)
  To: Jes Sorensen
  Cc: Larry.Finger, gregkh, linux-wireless, devel, linux-kernel,
	sunbing.linux
In-Reply-To: <wrfja8gje395.fsf@redhat.com>


On Aug 11, 2016, at 23:25, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:

> Bing Sun <sunbing@redflag-linux.com> writes:
>> Fixed sparse parse error:
>> Expected constant expression in case statement.
>> 
>> Signed-off-by: Bing Sun <sunbing@redflag-linux.com>
>> ---
>> drivers/staging/rtl8723au/os_dep/os_intfs.c | 11 +++++------
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/staging/rtl8723au/os_dep/os_intfs.c b/drivers/staging/rtl8723au/os_dep/os_intfs.c
>> index b8848c2..f30d5d2 100644
>> --- a/drivers/staging/rtl8723au/os_dep/os_intfs.c
>> +++ b/drivers/staging/rtl8723au/os_dep/os_intfs.c
>> @@ -283,14 +283,13 @@ static u32 rtw_classify8021d(struct sk_buff *skb)
>> 	 */
>> 	if (skb->priority >= 256 && skb->priority <= 263)
>> 		return skb->priority - 256;
>> -	switch (skb->protocol) {
>> -	case htons(ETH_P_IP):
>> +
>> +	if (skb->protocol == htons(ETH_P_IP)) {
>> 		dscp = ip_hdr(skb)->tos & 0xfc;
>> -		break;
>> -	default:
>> -		return 0;
>> +		return dscp >> 5;
>> 	}
>> -	return dscp >> 5;
>> +
>> +	return 0;
>> }
> 
> Pardon me here, but I find it really hard to see how this change is an
> improvement over the old code in any shape or form.
> 
> Jes


There is no functional improvement. 
But before this patch, when we do: make C=1 M=drivers/staging/rtl8723au/
An error output: 
drivers/staging/rtl8723au//os_dep/os_intfs.c:287:14: error: Expected constant expression in case statement
To avoid sparse parse error, a case statement converts to an if statement.
So we got this patch.




^ permalink raw reply

* Re: [PATCH 16/16] net: wireless: realtek: rtlwifi: usb: don't print error when allocating urb fails
From: Larry Finger @ 2016-08-11 21:35 UTC (permalink / raw)
  To: Wolfram Sang, linux-usb
  Cc: Chaoming Li, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <1470949988-25705-4-git-send-email-wsa-dev@sang-engineering.com>

On 08/11/2016 04:13 PM, Wolfram Sang wrote:
> kmalloc will print enough information in case of failure.
>
> Signed-off-by: Wolfram Sang <wsa-dev@sang-engineering.com>
> ---
>  drivers/net/wireless/realtek/rtlwifi/usb.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
> index 41617b7b082260..32aa5c1d070a07 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/usb.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
> @@ -739,11 +739,8 @@ static int _rtl_usb_receive(struct ieee80211_hw *hw)
>  	for (i = 0; i < rtlusb->rx_urb_num; i++) {
>  		err = -ENOMEM;
>  		urb = usb_alloc_urb(0, GFP_KERNEL);
> -		if (!urb) {
> -			RT_TRACE(rtlpriv, COMP_USB, DBG_EMERG,
> -				 "Failed to alloc URB!!\n");
> +		if (!urb)
>  			goto err_out;
> -		}
>
>  		err = _rtl_prep_rx_urb(hw, rtlusb, urb, GFP_KERNEL);
>  		if (err < 0) {
> @@ -907,15 +904,12 @@ static void _rtl_tx_complete(struct urb *urb)
>  static struct urb *_rtl_usb_tx_urb_setup(struct ieee80211_hw *hw,
>  				struct sk_buff *skb, u32 ep_num)
>  {
> -	struct rtl_priv *rtlpriv = rtl_priv(hw);
>  	struct rtl_usb *rtlusb = rtl_usbdev(rtl_usbpriv(hw));
>  	struct urb *_urb;
>
>  	WARN_ON(NULL == skb);
>  	_urb = usb_alloc_urb(0, GFP_ATOMIC);
>  	if (!_urb) {
> -		RT_TRACE(rtlpriv, COMP_USB, DBG_EMERG,
> -			 "Can't allocate URB for bulk out!\n");
>  		kfree_skb(skb);
>  		return NULL;
>  	}
>


^ permalink raw reply

* Re: [RESEND PATCH] nl80211: Allow GET_INTERFACE dumps to be filtered
From: Arend Van Spriel @ 2016-08-11 21:22 UTC (permalink / raw)
  To: Denis Kenzior; +Cc: Johannes Berg, linux-wireless
In-Reply-To: <57ACCC84.8090104@gmail.com>

On 11-8-2016 21:05, Denis Kenzior wrote:
> Hi Arend,
> 
>>
>> You can find it in checkpatch.pl [1] :-p
>>
> 
> Aha!  Will use that one next time.  Thanks.

There is a '--strict' command line option. Not sure if this indentation
one false into that category.

Regards,
Arend

> Regards,
> -Denis

^ permalink raw reply

* [PATCH 16/16] net: wireless: realtek: rtlwifi: usb: don't print error when allocating urb fails
From: Wolfram Sang @ 2016-08-11 21:13 UTC (permalink / raw)
  To: linux-usb
  Cc: Wolfram Sang, Larry Finger, Chaoming Li, Kalle Valo,
	linux-wireless, netdev, linux-kernel
In-Reply-To: <1470949988-25705-1-git-send-email-wsa-dev@sang-engineering.com>

kmalloc will print enough information in case of failure.

Signed-off-by: Wolfram Sang <wsa-dev@sang-engineering.com>
---
 drivers/net/wireless/realtek/rtlwifi/usb.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
index 41617b7b082260..32aa5c1d070a07 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -739,11 +739,8 @@ static int _rtl_usb_receive(struct ieee80211_hw *hw)
 	for (i = 0; i < rtlusb->rx_urb_num; i++) {
 		err = -ENOMEM;
 		urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (!urb) {
-			RT_TRACE(rtlpriv, COMP_USB, DBG_EMERG,
-				 "Failed to alloc URB!!\n");
+		if (!urb)
 			goto err_out;
-		}
 
 		err = _rtl_prep_rx_urb(hw, rtlusb, urb, GFP_KERNEL);
 		if (err < 0) {
@@ -907,15 +904,12 @@ static void _rtl_tx_complete(struct urb *urb)
 static struct urb *_rtl_usb_tx_urb_setup(struct ieee80211_hw *hw,
 				struct sk_buff *skb, u32 ep_num)
 {
-	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	struct rtl_usb *rtlusb = rtl_usbdev(rtl_usbpriv(hw));
 	struct urb *_urb;
 
 	WARN_ON(NULL == skb);
 	_urb = usb_alloc_urb(0, GFP_ATOMIC);
 	if (!_urb) {
-		RT_TRACE(rtlpriv, COMP_USB, DBG_EMERG,
-			 "Can't allocate URB for bulk out!\n");
 		kfree_skb(skb);
 		return NULL;
 	}
-- 
2.8.1


^ permalink raw reply related

* [PATCH 15/16] net: wireless: marvell: mwifiex: usb: don't print error when allocating urb fails
From: Wolfram Sang @ 2016-08-11 21:13 UTC (permalink / raw)
  To: linux-usb
  Cc: Wolfram Sang, Amitkumar Karwar, Nishant Sarmukadam, Kalle Valo,
	linux-wireless, netdev, linux-kernel
In-Reply-To: <1470949988-25705-1-git-send-email-wsa-dev@sang-engineering.com>

kmalloc will print enough information in case of failure.

Signed-off-by: Wolfram Sang <wsa-dev@sang-engineering.com>
---
 drivers/net/wireless/marvell/mwifiex/usb.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 0857575c5c39fe..3bd04f52f36988 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -657,11 +657,8 @@ static int mwifiex_usb_tx_init(struct mwifiex_adapter *adapter)
 	card->tx_cmd.ep = card->tx_cmd_ep;
 
 	card->tx_cmd.urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!card->tx_cmd.urb) {
-		mwifiex_dbg(adapter, ERROR,
-			    "tx_cmd.urb allocation failed\n");
+	if (!card->tx_cmd.urb)
 		return -ENOMEM;
-	}
 
 	for (i = 0; i < MWIFIEX_TX_DATA_PORT; i++) {
 		port = &card->port[i];
@@ -677,11 +674,8 @@ static int mwifiex_usb_tx_init(struct mwifiex_adapter *adapter)
 			port->tx_data_list[j].ep = port->tx_data_ep;
 			port->tx_data_list[j].urb =
 					usb_alloc_urb(0, GFP_KERNEL);
-			if (!port->tx_data_list[j].urb) {
-				mwifiex_dbg(adapter, ERROR,
-					    "urb allocation failed\n");
+			if (!port->tx_data_list[j].urb)
 				return -ENOMEM;
-			}
 		}
 	}
 
@@ -697,10 +691,8 @@ static int mwifiex_usb_rx_init(struct mwifiex_adapter *adapter)
 	card->rx_cmd.ep = card->rx_cmd_ep;
 
 	card->rx_cmd.urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!card->rx_cmd.urb) {
-		mwifiex_dbg(adapter, ERROR, "rx_cmd.urb allocation failed\n");
+	if (!card->rx_cmd.urb)
 		return -ENOMEM;
-	}
 
 	card->rx_cmd.skb = dev_alloc_skb(MWIFIEX_RX_CMD_BUF_SIZE);
 	if (!card->rx_cmd.skb)
@@ -714,11 +706,8 @@ static int mwifiex_usb_rx_init(struct mwifiex_adapter *adapter)
 		card->rx_data_list[i].ep = card->rx_data_ep;
 
 		card->rx_data_list[i].urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (!card->rx_data_list[i].urb) {
-			mwifiex_dbg(adapter, ERROR,
-				    "rx_data_list[] urb allocation failed\n");
+		if (!card->rx_data_list[i].urb)
 			return -1;
-		}
 		if (mwifiex_usb_submit_rx_urb(&card->rx_data_list[i],
 					      MWIFIEX_RX_DATA_BUF_SIZE))
 			return -1;
-- 
2.8.1


^ permalink raw reply related

* [PATCH 14/16] net: wireless: marvell: libertas_tf: if_usb: don't print error when allocating urb fails
From: Wolfram Sang @ 2016-08-11 21:13 UTC (permalink / raw)
  To: linux-usb; +Cc: Wolfram Sang, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <1470949988-25705-1-git-send-email-wsa-dev@sang-engineering.com>

kmalloc will print enough information in case of failure.

Signed-off-by: Wolfram Sang <wsa-dev@sang-engineering.com>
---
 drivers/net/wireless/marvell/libertas_tf/if_usb.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/marvell/libertas_tf/if_usb.c b/drivers/net/wireless/marvell/libertas_tf/if_usb.c
index 799a2efe579372..e0ade40d9497d7 100644
--- a/drivers/net/wireless/marvell/libertas_tf/if_usb.c
+++ b/drivers/net/wireless/marvell/libertas_tf/if_usb.c
@@ -198,22 +198,16 @@ static int if_usb_probe(struct usb_interface *intf,
 	}
 
 	cardp->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!cardp->rx_urb) {
-		lbtf_deb_usbd(&udev->dev, "Rx URB allocation failed\n");
+	if (!cardp->rx_urb)
 		goto dealloc;
-	}
 
 	cardp->tx_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!cardp->tx_urb) {
-		lbtf_deb_usbd(&udev->dev, "Tx URB allocation failed\n");
+	if (!cardp->tx_urb)
 		goto dealloc;
-	}
 
 	cardp->cmd_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!cardp->cmd_urb) {
-		lbtf_deb_usbd(&udev->dev, "Cmd URB allocation failed\n");
+	if (!cardp->cmd_urb)
 		goto dealloc;
-	}
 
 	cardp->ep_out_buf = kmalloc(MRVDRV_ETH_TX_PACKET_BUFFER_SIZE,
 				    GFP_KERNEL);
-- 
2.8.1


^ permalink raw reply related

* [PATCH 13/16] net: wireless: intersil: orinoco: orinoco_usb: don't print error when allocating urb fails
From: Wolfram Sang @ 2016-08-11 21:13 UTC (permalink / raw)
  To: linux-usb; +Cc: Wolfram Sang, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <1470949539-25392-1-git-send-email-wsa-dev@sang-engineering.com>

kmalloc will print enough information in case of failure.

Signed-off-by: Wolfram Sang <wsa-dev@sang-engineering.com>
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 56f109bc83945d..bca6935a94db9a 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1613,10 +1613,8 @@ static int ezusb_probe(struct usb_interface *interface,
 			}
 
 			upriv->read_urb = usb_alloc_urb(0, GFP_KERNEL);
-			if (!upriv->read_urb) {
-				err("No free urbs available");
+			if (!upriv->read_urb)
 				goto error;
-			}
 			if (le16_to_cpu(ep->wMaxPacketSize) != 64)
 				pr_warn("bulk in: wMaxPacketSize!= 64\n");
 			if (ep->bEndpointAddress != (2 | USB_DIR_IN))
-- 
2.8.1


^ permalink raw reply related

* [PATCH 12/16] net: wireless: broadcom: brcm80211: brcmfmac: usb: don't print error when allocating urb fails
From: Wolfram Sang @ 2016-08-11 21:05 UTC (permalink / raw)
  To: linux-usb
  Cc: Wolfram Sang, Arend van Spriel, Franky Lin, Hante Meuleman,
	Kalle Valo, linux-wireless, brcm80211-dev-list.pdl, netdev
In-Reply-To: <1470949539-25392-1-git-send-email-wsa-dev@sang-engineering.com>

kmalloc will print enough information in case of failure.

Signed-off-by: Wolfram Sang <wsa-dev@sang-engineering.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 98b15a9a2779f4..fa26619a7945d9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -1099,15 +1099,11 @@ struct brcmf_usbdev *brcmf_usb_attach(struct brcmf_usbdev_info *devinfo,
 	devinfo->tx_freecount = ntxq;
 
 	devinfo->ctl_urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!devinfo->ctl_urb) {
-		brcmf_err("usb_alloc_urb (ctl) failed\n");
+	if (!devinfo->ctl_urb)
 		goto error;
-	}
 	devinfo->bulk_urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!devinfo->bulk_urb) {
-		brcmf_err("usb_alloc_urb (bulk) failed\n");
+	if (!devinfo->bulk_urb)
 		goto error;
-	}
 
 	return &devinfo->bus_pub;
 
-- 
2.8.1


^ permalink raw reply related

* [PATCH 11/16] net: wireless: ath: ar5523: ar5523: don't print error when allocating urb fails
From: Wolfram Sang @ 2016-08-11 21:05 UTC (permalink / raw)
  To: linux-usb; +Cc: Wolfram Sang, Pontus Fuchs, Kalle Valo, linux-wireless, netdev
In-Reply-To: <1470949539-25392-1-git-send-email-wsa-dev@sang-engineering.com>

kmalloc will print enough information in case of failure.

Signed-off-by: Wolfram Sang <wsa-dev@sang-engineering.com>
---
 drivers/net/wireless/ath/ar5523/ar5523.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ar5523/ar5523.c b/drivers/net/wireless/ath/ar5523/ar5523.c
index 8aded24bcdf49e..7a60d2e652dad6 100644
--- a/drivers/net/wireless/ath/ar5523/ar5523.c
+++ b/drivers/net/wireless/ath/ar5523/ar5523.c
@@ -706,10 +706,8 @@ static int ar5523_alloc_rx_bufs(struct ar5523 *ar)
 
 		data->ar = ar;
 		data->urb = usb_alloc_urb(0, GFP_KERNEL);
-		if (!data->urb) {
-			ar5523_err(ar, "could not allocate rx data urb\n");
+		if (!data->urb)
 			goto err;
-		}
 		list_add_tail(&data->list, &ar->rx_data_free);
 		atomic_inc(&ar->rx_data_free_cnt);
 	}
@@ -824,7 +822,6 @@ static void ar5523_tx_work_locked(struct ar5523 *ar)
 
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (!urb) {
-			ar5523_err(ar, "Failed to allocate TX urb\n");
 			ieee80211_free_txskb(ar->hw, skb);
 			continue;
 		}
@@ -949,10 +946,8 @@ static int ar5523_alloc_tx_cmd(struct ar5523 *ar)
 	init_completion(&cmd->done);
 
 	cmd->urb_tx = usb_alloc_urb(0, GFP_KERNEL);
-	if (!cmd->urb_tx) {
-		ar5523_err(ar, "could not allocate urb\n");
+	if (!cmd->urb_tx)
 		return -ENOMEM;
-	}
 	cmd->buf_tx = usb_alloc_coherent(ar->dev, AR5523_MAX_TXCMDSZ,
 					 GFP_KERNEL,
 					 &cmd->urb_tx->transfer_dma);
-- 
2.8.1


^ permalink raw reply related

* [PATCH 00/16] net: don't print error when allocating urb fails
From: Wolfram Sang @ 2016-08-11 21:05 UTC (permalink / raw)
  To: linux-usb
  Cc: Wolfram Sang, brcm80211-dev-list.pdl, linux-can, linux-wireless,
	netdev

This per-subsystem series is part of a tree wide cleanup. usb_alloc_urb() uses
kmalloc which already prints enough information on failure. So, let's simply
remove those "allocation failed" messages from drivers like we did already for
other -ENOMEM cases. gkh acked this approach when we talked about it at LCJ in
Tokyo a few weeks ago.


Wolfram Sang (16):
  net: can: usb: ems_usb: don't print error when allocating urb fails
  net: can: usb: esd_usb2: don't print error when allocating urb fails
  net: can: usb: gs_usb: don't print error when allocating urb fails
  net: can: usb: kvaser_usb: don't print error when allocating urb fails
  net: can: usb: peak_usb: pcan_usb_core: don't print error when
    allocating urb fails
  net: can: usb: usb_8dev: don't print error when allocating urb fails
  net: usb: hso: don't print error when allocating urb fails
  net: usb: lan78xx: don't print error when allocating urb fails
  net: usb: usbnet: don't print error when allocating urb fails
  net: wimax: i2400m: usb-notif: don't print error when allocating urb
    fails
  net: wireless: ath: ar5523: ar5523: don't print error when allocating
    urb fails
  net: wireless: broadcom: brcm80211: brcmfmac: usb: don't print error
    when allocating urb fails
  net: wireless: intersil: orinoco: orinoco_usb: don't print error when
    allocating urb fails
  net: wireless: marvell: libertas_tf: if_usb: don't print error when
    allocating urb fails
  net: wireless: marvell: mwifiex: usb: don't print error when
    allocating urb fails
  net: wireless: realtek: rtlwifi: usb: don't print error when
    allocating urb fails

 drivers/net/can/usb/ems_usb.c                        |  9 ++-------
 drivers/net/can/usb/esd_usb2.c                       |  3 ---
 drivers/net/can/usb/gs_usb.c                         |  9 ++-------
 drivers/net/can/usb/kvaser_usb.c                     |  7 +------
 drivers/net/can/usb/peak_usb/pcan_usb_core.c         |  6 +-----
 drivers/net/can/usb/usb_8dev.c                       |  5 +----
 drivers/net/usb/hso.c                                | 20 +++++---------------
 drivers/net/usb/lan78xx.c                            |  4 +---
 drivers/net/usb/usbnet.c                             |  5 +----
 drivers/net/wimax/i2400m/usb-notif.c                 |  1 -
 drivers/net/wireless/ath/ar5523/ar5523.c             |  9 ++-------
 .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c   |  8 ++------
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c  |  4 +---
 drivers/net/wireless/marvell/libertas_tf/if_usb.c    | 12 +++---------
 drivers/net/wireless/marvell/mwifiex/usb.c           | 19 ++++---------------
 drivers/net/wireless/realtek/rtlwifi/usb.c           |  8 +-------
 16 files changed, 27 insertions(+), 102 deletions(-)

-- 
2.8.1


^ permalink raw reply

* Re: [PATCH 1/2] ath9k: use ieee80211_tx_status_noskb where possible
From: Felix Fietkau @ 2016-08-11 19:27 UTC (permalink / raw)
  To: Zefir Kurtisi, linux-wireless; +Cc: kvalo
In-Reply-To: <8aeb7913-d955-d894-d82c-4614e17d3ee4@neratec.com>

On 2016-08-11 18:05, Zefir Kurtisi wrote:
> On 08/04/2016 11:49 PM, Felix Fietkau wrote:
>> It removes the need for undoing the padding changes to skb->data and it
>> improves performance by eliminating one tx status lookup per MPDU in the
>> status path. It is also useful for preparing a follow-up fix to better
>> handle powersave filtering.
>> 
> 
> For me, this one introduces a regression to the statistics, e.g.
> 'dot11TransmittedFragmentCount' is now accounted differently since it is not
> updated from within ieee80211_tx_status_noskb().
Is this important? I guess it would be possible to make this more
accurate by extending the API, but I wonder if that's worth doing just
for these debugfs counters.

- Felix


^ permalink raw reply

* Re: [PATCH 1/4] cfg80211: rdev-ops: remove callback check from rdev_set_coalesce()
From: Arend Van Spriel @ 2016-08-11 19:00 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1470919713.12075.8.camel@sipsolutions.net>

On 11-8-2016 14:48, Johannes Berg wrote:
> On Wed, 2016-08-10 at 12:33 +0200, Arend van Spriel wrote:
>> The wrapper rdev_set_coalesce() checks whether the driver provides
>> the set_coalesce callback and returns -ENOTSUPP if not. However, this
>> check is already performed in nl80211_set_coalesce() resulting in
>> -EOPNOTSUPP. This patch removes check from rdev wrapper function.
> 
> What's the point though? Presumably the compiler will optimise it out,
> and it seems safer to have it this way? Same for all patches in this
> series.

I was in doubt to raise the question first about getting this stuff
consistent, ie. keep rdev-ops as flat as possible, but decided just to
put it out there in patch format. My bad :-)

If you want the rdev-ops to be safe against (future) callers not
checking the callback, it seems you should add a check in all rdev-ops
where the callback is optional.

Regards,
Arend

^ permalink raw reply

* Re: [RESEND PATCH] nl80211: Allow GET_INTERFACE dumps to be filtered
From: Denis Kenzior @ 2016-08-11 19:05 UTC (permalink / raw)
  To: Arend Van Spriel, Johannes Berg, linux-wireless
In-Reply-To: <f2fe3568-0b83-9db4-278e-be0a1ad08ae8@broadcom.com>

Hi Arend,

 >
> You can find it in checkpatch.pl [1] :-p
>

Aha!  Will use that one next time.  Thanks.

Regards,
-Denis

^ permalink raw reply

* default dtim period
From: Reinoud Koornstra @ 2016-08-11 19:03 UTC (permalink / raw)
  To: linux-wireless

Hi Everybody,

Maybe not the correct mailinglist to ask, but I'm going to give it a shot.
Any advises on the most preferred default dtim period?
2 or 3? Not every driver has the same defaults.

Same question about the listen interval.
0x0258 (600 beacon intervals) sounds decent?
Thanks,

Reinoud.

^ permalink raw reply

* Re: [RESEND PATCH] nl80211: Allow GET_INTERFACE dumps to be filtered
From: Arend Van Spriel @ 2016-08-11 18:58 UTC (permalink / raw)
  To: Denis Kenzior, Johannes Berg, linux-wireless
In-Reply-To: <57ACC1F8.6070908@gmail.com>

On 11-8-2016 20:20, Denis Kenzior wrote:
> Hi Johannes,
> 
>>> Speaking of indentation, can you point me to a doc of the rules I
>>> should follow?
>>
>> You've seen Documentation/CodingStyle?
> 
> Of course.  But that one doesn't discuss that you want your function
> parameters to be aligned to the opening '('.  Is there a dialect
> document specific to linux-wireless?

You can find it in checkpatch.pl [1] :-p

Regards,
Arend

[1] http://lxr.free-electrons.com/source/scripts/checkpatch.pl#L2855

^ permalink raw reply

* Re: [RESEND PATCH] nl80211: Allow GET_INTERFACE dumps to be filtered
From: Denis Kenzior @ 2016-08-11 18:20 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless
In-Reply-To: <1470938627.12075.25.camel@sipsolutions.net>

Hi Johannes,

 >> Speaking of indentation, can you point me to a doc of the rules I
>> should follow?
>
> You've seen Documentation/CodingStyle?

Of course.  But that one doesn't discuss that you want your function 
parameters to be aligned to the opening '('.  Is there a dialect 
document specific to linux-wireless?

>
>> I can confirm that I sanity checked this patch. Both ATTR_WIPHY,
>> ATTR_WDEV and wildcard dumps seemed to produce expected results.
>
> I think it probably works due to the other conditions?

The initial conditions are that:
cb->args[0..2] == 0.

So on the first iteration we set filter_wiphy == -1 and check the filter 
attributes.  If set, we modify filter_wiphy accordingly.

Even if filter_wiphy is set to 0, the if statement should still never be 
entered afterwards since wp_start and if_start are incremented.

Is this what you're worried about? Do you see a fault in my logic?

Regards,
-Denis

^ permalink raw reply

* Re: [RESEND PATCH] nl80211: Allow GET_INTERFACE dumps to be filtered
From: Johannes Berg @ 2016-08-11 18:03 UTC (permalink / raw)
  To: Denis Kenzior, linux-wireless
In-Reply-To: <57ACA9E9.4020507@gmail.com>

On Thu, 2016-08-11 at 11:38 -0500, Denis Kenzior wrote:
> Hi Johannes,
> 
> On 08/11/2016 07:47 AM, Johannes Berg wrote:
> > 
> > On Wed, 2016-08-03 at 17:02 -0500, Denis Kenzior wrote:
> > > 
> > > 
> > > +static int nl80211_dump_interface_parse(struct sk_buff *skb,
> > > +				    struct netlink_callback *cb,
> > > +				    int *filter_wiphy)
> > 
> > Wrong indentation :)
> 
> Sorry :)
> 
> Speaking of indentation, can you point me to a doc of the rules I
> should follow?

You've seen Documentation/CodingStyle?

> I can confirm that I sanity checked this patch. Both ATTR_WIPHY, 
> ATTR_WDEV and wildcard dumps seemed to produce expected results.

I think it probably works due to the other conditions?

> I noticed you applied this patch.  Is there a particular scenario
> where it goes wrong or did you convince yourself it is correct?
> 

No, that was a mistake :( I've removed it now.

johannes

^ permalink raw reply

* Re: [RESEND PATCH] nl80211: Allow GET_INTERFACE dumps to be filtered
From: Denis Kenzior @ 2016-08-11 16:38 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless
In-Reply-To: <1470919643.12075.7.camel@sipsolutions.net>

Hi Johannes,

On 08/11/2016 07:47 AM, Johannes Berg wrote:
> On Wed, 2016-08-03 at 17:02 -0500, Denis Kenzior wrote:
>>
>> +static int nl80211_dump_interface_parse(struct sk_buff *skb,
>> +				    struct netlink_callback *cb,
>> +				    int *filter_wiphy)
>
> Wrong indentation :)

Sorry :)

Speaking of indentation, can you point me to a doc of the rules I should 
follow?

>
>>   static int nl80211_dump_interface(struct sk_buff *skb, struct
>> netlink_callback *cb)
>>   {
>>   	int wp_idx = 0;
>>   	int if_idx = 0;
>>   	int wp_start = cb->args[0];
>>   	int if_start = cb->args[1];
>> +	int filter_wiphy = cb->args[2];
>>   	struct cfg80211_registered_device *rdev;
>>   	struct wireless_dev *wdev;
>>
>> +	if (!wp_start && !if_start && !filter_wiphy) {
>
> This seems incorrect - you're setting
>
>> +		int ret;
>> +
>> +		filter_wiphy = -1;
>> +
>> +		ret = nl80211_dump_interface_parse(skb, cb,
>> &filter_wiphy);
>
> it here, but it can take the value 0, so !filter_wiphy seems wrong?
>

I can confirm that I sanity checked this patch. Both ATTR_WIPHY, 
ATTR_WDEV and wildcard dumps seemed to produce expected results.

I noticed you applied this patch.  Is there a particular scenario where 
it goes wrong or did you convince yourself it is correct?

Regards,
-Denis

^ permalink raw reply

* Re: [PATCH v3 0/3] Improve wireless netdev detection
From: Denis Kenzior @ 2016-08-11 16:31 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless
In-Reply-To: <1470919457.12075.5.camel@sipsolutions.net>

Hi Johannes,

 >
> Applied. I squashed 1/2 since it's kinda pointless to have a patch
> introducing infrastructure that's not used.
>
> I also made some minor cleanups - please check (both that it's right,
> and for next time)
>

Your changes look fine to me.  Thanks for taking these.

Regards,
-Denis


^ permalink raw reply

* Re: [PATCH 1/2] ath9k: use ieee80211_tx_status_noskb where possible
From: Zefir Kurtisi @ 2016-08-11 16:05 UTC (permalink / raw)
  To: Felix Fietkau, linux-wireless; +Cc: kvalo
In-Reply-To: <20160804214940.78476-1-nbd@nbd.name>

On 08/04/2016 11:49 PM, Felix Fietkau wrote:
> It removes the need for undoing the padding changes to skb->data and it
> improves performance by eliminating one tx status lookup per MPDU in the
> status path. It is also useful for preparing a follow-up fix to better
> handle powersave filtering.
> 

For me, this one introduces a regression to the statistics, e.g.
'dot11TransmittedFragmentCount' is now accounted differently since it is not
updated from within ieee80211_tx_status_noskb().

Cheers,
Zefir

^ permalink raw reply

* RE: [PATCH v5] cfg80211: Provision to allow the support for different beacon intervals
From: Undekari, Sunil Dutt @ 2016-08-11 15:34 UTC (permalink / raw)
  To: Johannes Berg, Kushwaha, Purushottam
  Cc: linux-wireless@vger.kernel.org, Malinen, Jouni,
	Hullur Subramanyam, Amarnath, Kumar, Deepak (QCA)
In-Reply-To: <1470920916.12075.15.camel@sipsolutions.net>

PiBUaGlzIHNlZW1zIG9kZC4gU2hvdWxkbid0IGl0IHVzZSBjZmc4MDIxMV9pdGVyX2NvbWJpbmF0
aW9ucygpPw0KWWVzIC4gV2Ugc2hvdWxkIGhhdmUgcmV1c2VkIGNmZzgwMjExX2l0ZXJfY29tYmlu
YXRpb25zLiBUaGFua3MgYSBsb3QgZm9yIHlvdXIgc3VnZ2VzdGlvbi4gDQoNCj4gc2VlbXMgd2Ug
bW9zdGx5IG1vdmVkIHRoYXQgdG8gbWFjODAyMTEsIGJ1dCBmb3IgdGhpcyBjYXNlIGl0IHNob3Vs
ZCBwcm9iYWJseSBzdGF5IGluIGNmZzgwMjExPyBPciBzaG91bGQgaXQ/DQpUaGUgc2NvcGUgaGVy
ZSBpcyB0byBrbm93IHRoZSBhZHZlcnRpc2VkICJkaWZmX2JlYWNvbl9pbnRfZ2NkIiBmb3IgdGhl
IHJlc3BlY3RpdmUgbWF0Y2hpbmcgaW50ZXJmYWNlIGNvbWJpbmF0aW9uIGFuZCBmdXJ0aGVyIHZh
bGlkYXRlIGl0IHdpdGggdGhlIGNvbmZpZ3VyZWQgb25lLiANClRvIGFkZHJlc3MgdGhpcyAiIGNm
ZzgwMjExX3ZhbGlkYXRlX2JlYWNvbl9pbnQiIG5lZWRzIHRvIGdldCBlbmhhbmNlZCAsIGlzbid0
ID8NCg0KUmVnYXJkcywNClN1bmlsDQotLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTog
Sm9oYW5uZXMgQmVyZyBbbWFpbHRvOmpvaGFubmVzQHNpcHNvbHV0aW9ucy5uZXRdIA0KU2VudDog
VGh1cnNkYXksIEF1Z3VzdCAxMSwgMjAxNiA2OjM5IFBNDQpUbzogS3VzaHdhaGEsIFB1cnVzaG90
dGFtIDxwa3VzaHdhaEBxdGkucXVhbGNvbW0uY29tPg0KQ2M6IGxpbnV4LXdpcmVsZXNzQHZnZXIu
a2VybmVsLm9yZzsgTWFsaW5lbiwgSm91bmkgPGpvdW5pQHFjYS5xdWFsY29tbS5jb20+OyBVbmRl
a2FyaSwgU3VuaWwgRHV0dCA8dXNkdXR0QHF0aS5xdWFsY29tbS5jb20+OyBIdWxsdXIgU3VicmFt
YW55YW0sIEFtYXJuYXRoIDxhbWFybmF0aEBxY2EucXVhbGNvbW0uY29tPjsgS3VtYXIsIERlZXBh
ayAoUUNBKSA8ZGppbmRhbEBxdGkucXVhbGNvbW0uY29tPg0KU3ViamVjdDogUmU6IFtQQVRDSCB2
NV0gY2ZnODAyMTE6IFByb3Zpc2lvbiB0byBhbGxvdyB0aGUgc3VwcG9ydCBmb3IgZGlmZmVyZW50
IGJlYWNvbiBpbnRlcnZhbHMNCg0KDQo+ICtzdGF0aWMgYm9vbCBkaWZmX2JlYWNvbl9pbnRlcnZh
bF9zdXBwb3J0ZWQoc3RydWN0IHdpcGh5ICp3aXBoeSwgdTE2DQo+IHR5cGVzLA0KPiArCQkJCQnC
oMKgwqB1MzIgYmVhY29uX2ludCkNCj4gK3sNCj4gKwljb25zdCBzdHJ1Y3QgaWVlZTgwMjExX2lm
YWNlX2NvbWJpbmF0aW9uICpjOw0KPiArCXUxNiBhbGxfdHlwZXM7DQo+ICsJaW50IGksIGo7DQo+
ICsNCj4gKwlmb3IgKGkgPSAwOyBpIDwgd2lwaHktPm5faWZhY2VfY29tYmluYXRpb25zOyBpKysp
IHsNCj4gKwkJYyA9ICZ3aXBoeS0+aWZhY2VfY29tYmluYXRpb25zW2ldOw0KPiArDQo+ICsJCWlm
ICghYy0+ZGlmZl9iZWFjb25faW50X2djZCB8fA0KPiArCQnCoMKgwqDCoChiZWFjb25faW50ICUg
Yy0+ZGlmZl9iZWFjb25faW50X2djZCkpDQo+ICsJCQljb250aW51ZTsNCj4gKw0KPiArCQlhbGxf
dHlwZXMgPSAwOw0KPiArCQlmb3IgKGogPSAwOyBqIDwgYy0+bl9saW1pdHM7IGorKykNCj4gKwkJ
CWFsbF90eXBlcyB8PSBjLT5saW1pdHNbal0udHlwZXM7DQo+ICsNCj4gKwkJaWYgKGFsbF90eXBl
cyAmIHR5cGVzKQ0KPiArCQkJcmV0dXJuIHRydWU7DQo+ICsJfQ0KPiArDQo+ICsJcmV0dXJuIGZh
bHNlOw0KPiArfQ0KDQpUaGlzIHNlZW1zIG9kZC4gU2hvdWxkbid0IGl0IHVzZcKgY2ZnODAyMTFf
aXRlcl9jb21iaW5hdGlvbnMoKT8gTWlnaHQgbmVlZCBzb21lIGFkZGl0aW9uYWwgaGVscGVyIHRv
IHVuZGVyc3RhbmQgd2hpY2ggaW50ZXJmYWNlIHR5cGVzIGFyZSB1c2VkIHJpZ2h0IG5vdyAtIHNl
ZW1zIHdlIG1vc3RseSBtb3ZlZCB0aGF0IHRvIG1hYzgwMjExLCBidXQgZm9yIHRoaXMgY2FzZSBp
dCBzaG91bGQgcHJvYmFibHkgc3RheSBpbiBjZmc4MDIxMT8gT3Igc2hvdWxkIGl0Pw0KDQpqb2hh
bm5lcw0K

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox