diff mbox

[4.12] brcmfmac: put chip into passive mode (when attaching) just once

Message ID 20170224131027.29880-1-zajec5@gmail.com (mailing list archive)
State Rejected
Delegated to: Kalle Valo
Headers show

Commit Message

Rafał Miłecki Feb. 24, 2017, 1:10 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

It avoids some unnecessary work. Most likely there wasn't much sense in
doing this before bus reset anyway, as reset is supposed to put chip
into default state. In PCIe code (only bus implementing reset) we e.g.
use watchdog to perform a chip "reboot".

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Arend van Spriel Feb. 24, 2017, 1:31 p.m. UTC | #1
On 24-2-2017 14:10, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> It avoids some unnecessary work. Most likely there wasn't much sense in
> doing this before bus reset anyway, as reset is supposed to put chip
> into default state. In PCIe code (only bus implementing reset) we e.g.
> use watchdog to perform a chip "reboot".

Sorry, but removing the fisrt passive call will cause chip lockups for
sure on certain systems. We learned the hard way :-p

Regards,
Arend

> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> index 05f22ff81d60..670f2f50f9e5 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> @@ -967,16 +967,14 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
>  	if (ret)
>  		return ret;
>  
> -	/* assure chip is passive for core access */
> -	brcmf_chip_set_passive(&ci->pub);
> -
>  	/* Call bus specific reset function now. Cores have been determined
>  	 * but further access may require a chip specific reset at this point.
>  	 */
> -	if (ci->ops->reset) {
> +	if (ci->ops->reset)
>  		ci->ops->reset(ci->ctx, &ci->pub);
> -		brcmf_chip_set_passive(&ci->pub);
> -	}
> +
> +	/* assure chip is passive for core access */
> +	brcmf_chip_set_passive(&ci->pub);
>  
>  	return brcmf_chip_get_raminfo(ci);
>  }
>
Rafał Miłecki Feb. 24, 2017, 1:47 p.m. UTC | #2
On 2017-02-24 14:31, Arend Van Spriel wrote:
> On 24-2-2017 14:10, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> It avoids some unnecessary work. Most likely there wasn't much sense 
>> in
>> doing this before bus reset anyway, as reset is supposed to put chip
>> into default state. In PCIe code (only bus implementing reset) we e.g.
>> use watchdog to perform a chip "reboot".
> 
> Sorry, but removing the fisrt passive call will cause chip lockups for
> sure on certain systems. We learned the hard way :-p

OK, acknowledged! ;) I just tested it on my BCM43602 doing warm reboots 
(they
were causing problems earlier) and it worked fine.

It also seems I don't use SDK recent enough as my reference :)

Kalle please drop this patch.
diff mbox

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 05f22ff81d60..670f2f50f9e5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -967,16 +967,14 @@  static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
 	if (ret)
 		return ret;
 
-	/* assure chip is passive for core access */
-	brcmf_chip_set_passive(&ci->pub);
-
 	/* Call bus specific reset function now. Cores have been determined
 	 * but further access may require a chip specific reset at this point.
 	 */
-	if (ci->ops->reset) {
+	if (ci->ops->reset)
 		ci->ops->reset(ci->ctx, &ci->pub);
-		brcmf_chip_set_passive(&ci->pub);
-	}
+
+	/* assure chip is passive for core access */
+	brcmf_chip_set_passive(&ci->pub);
 
 	return brcmf_chip_get_raminfo(ci);
 }