[11/34] brcmfmac: Split brcmf_sdiod_buffrw function up.

Message ID	20170726202557.15632-12-ian@mnementh.co.uk (mailing list archive)
State	Changes Requested
Delegated to:	Kalle Valo
Headers	show Return-Path: <linux-wireless-owner@kernel.org> From: Ian Molton <ian@mnementh.co.uk> To: linux-wireless@vger.kernel.org Cc: arend.vanspriel@broadcom.com, franky.lin@broadcom.com, hante.meuleman@broadcom.com Subject: [PATCH 11/34] brcmfmac: Split brcmf_sdiod_buffrw function up. Date: Wed, 26 Jul 2017 21:25:34 +0100 Message-Id: <20170726202557.15632-12-ian@mnementh.co.uk> In-Reply-To: <20170726202557.15632-1-ian@mnementh.co.uk> References: <20170726202557.15632-1-ian@mnementh.co.uk> X-Spam_score: -2.9 X-Spam_score_int: -28 X-Spam_bar: -- X-Spam_report: Spam detection software, running on the system "mnementh.co.uk", has identified this incoming email as possible spam. The original message has been attached to this so you can view it (if it isn't spam) or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: This function needs to be split up into separate read / write variants for clarity. Signed-off-by: Ian Molton <ian@mnementh.co.uk> # Conflicts: # drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c --- .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c \| 69 +++++++++++++++------- 1 file changed, 47 insertions(+), 22 deletions(-) [...] Content analysis details: (-2.9 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP 0.0 TVD_RCVD_IP Message was received from an IP address 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: mnementh.co.uk] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk

Message ID

20170726202557.15632-12-ian@mnementh.co.uk (mailing list archive)

State

Changes Requested

Delegated to:

Kalle Valo

Headers

From: Ian Molton <ian@mnementh.co.uk>
To: linux-wireless@vger.kernel.org
Cc: arend.vanspriel@broadcom.com, franky.lin@broadcom.com,
	hante.meuleman@broadcom.com
Subject: [PATCH 11/34] brcmfmac: Split brcmf_sdiod_buffrw function up.
Date: Wed, 26 Jul 2017 21:25:34 +0100
Message-Id: <20170726202557.15632-12-ian@mnementh.co.uk>
In-Reply-To: <20170726202557.15632-1-ian@mnementh.co.uk>
References: <20170726202557.15632-1-ian@mnementh.co.uk>
X-Spam_score: -2.9
X-Spam_score_int: -28
X-Spam_bar: --
X-Spam_report: Spam detection software,
	running on the system "mnementh.co.uk", has
	identified this incoming email as possible spam. The original message
	has been attached to this so you can view it (if it isn't spam) or
	label similar future email.  If you have any questions, see
	the administrator of that system for details.
	Content preview: This function needs to be split up into separate
	read / write
	variants for clarity. Signed-off-by: Ian Molton <ian@mnementh.co.uk>
	# Conflicts:
	# drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c ---
	.../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
	| 69 +++++++++++++++------- 1 file changed, 47 insertions(+),
	22 deletions(-) [...] 
	Content analysis details:   (-2.9 points, 5.0 required)
	pts rule name              description
	---- ----------------------
	--------------------------------------------------
	-1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP
	0.0 TVD_RCVD_IP            Message was received from an IP address
	0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was
	blocked. See
	http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block
	for more information. [URIs: mnementh.co.uk]
	-1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%
	[score: 0.0000]
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org
X-Virus-Scanned: ClamAV using ClamSMTP

Commit Message

Ian Molton July 26, 2017, 8:25 p.m. UTC

This function needs to be split up into separate read / write variants
for clarity.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>

# Conflicts:
#	drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 69 +++++++++++++++-------
 1 file changed, 47 insertions(+), 22 deletions(-)

Comments

Arend van Spriel Aug. 7, 2017, 11:26 a.m. UTC | #1

On 7/26/2017 10:25 PM, Ian Molton wrote:
> This function needs to be split up into separate read / write variants
> for clarity.

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Ian Molton <ian@mnementh.co.uk>

more comments below...

> # Conflicts:
> #	drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> ---
>   .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 69 +++++++++++++++-------
>   1 file changed, 47 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> index 9d5716d0ad73..858ad2d8706b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c

[...]

> @@ -706,10 +725,10 @@ int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
>   	err = brcmf_sdiod_addrprep(sdiodev, &addr);
>
>   	if (!err)
> -		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true, addr,
> -					 mypkt);
> +		err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_2, addr, mypkt);
>
>   	brcmu_pkt_buf_free_skb(mypkt);
> +

You are keen on sprinkling whitespace here and there. Could you please 
use separate patches for that as much as possible. Not sure why you 
added this one...

>   	return err;
>

...and kept this one.

>   }

Ian Molton Aug. 19, 2017, 9:41 p.m. UTC | #2

On 07/08/17 12:26, Arend van Spriel wrote:

>> +        err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_2, addr, mypkt);
>>
>>       brcmu_pkt_buf_free_skb(mypkt);
>> +
> 
> You are keen on sprinkling whitespace here and there. Could you please
> use separate patches for that as much as possible. Not sure why you
> added this one...
> 
>>       return err;
>>
> 
> ...and kept this one.
> 
>>   }
> 

Good catch. There's a couple of others - I'll break them out. That
latter one is clearly an error on my part.

-Ian

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 9d5716d0ad73..858ad2d8706b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -413,8 +413,8 @@  void brcmf_sdiod_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr,
 		*ret = retval;
 }
 
-static int brcmf_sdiod_buffrw(struct brcmf_sdio_dev *sdiodev, uint fn,
-			     bool write, u32 addr, struct sk_buff *pkt)
+static int brcmf_sdiod_buff_read(struct brcmf_sdio_dev *sdiodev, uint fn,
+				 u32 addr, struct sk_buff *pkt)
 {
 	unsigned int req_sz;
 	int err;
@@ -423,18 +423,36 @@  static int brcmf_sdiod_buffrw(struct brcmf_sdio_dev *sdiodev, uint fn,
 	req_sz = pkt->len + 3;
 	req_sz &= (uint)~3;
 
-	if (write)
-		err = sdio_memcpy_toio(sdiodev->func[fn], addr,
-				       ((u8 *)(pkt->data)), req_sz);
-	else if (fn == 1)
-		err = sdio_memcpy_fromio(sdiodev->func[fn], ((u8 *)(pkt->data)),
-					 addr, req_sz);
+	if (fn == 1)
+		err = sdio_memcpy_fromio(sdiodev->func[fn],
+					 ((u8 *)(pkt->data)), addr, req_sz);
 	else
 		/* function 2 read is FIFO operation */
-		err = sdio_readsb(sdiodev->func[fn], ((u8 *)(pkt->data)), addr,
-				  req_sz);
+		err = sdio_readsb(sdiodev->func[fn],
+				  ((u8 *)(pkt->data)), addr, req_sz);
+
+	if (err == -ENOMEDIUM)
+		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
+
+	return err;
+}
+
+static int brcmf_sdiod_buff_write(struct brcmf_sdio_dev *sdiodev, uint fn,
+				  u32 addr, struct sk_buff *pkt)
+{
+	unsigned int req_sz;
+	int err;
+
+	/* Single skb use the standard mmc interface */
+	req_sz = pkt->len + 3;
+	req_sz &= (uint)~3;
+
+	err = sdio_memcpy_toio(sdiodev->func[fn], addr,
+			       ((u8 *)(pkt->data)), req_sz);
+
 	if (err == -ENOMEDIUM)
 		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
+
 	return err;
 }
 
@@ -642,7 +660,7 @@  int brcmf_sdiod_recv_pkt(struct brcmf_sdio_dev *sdiodev, struct sk_buff *pkt)
 	if (err)
 		goto done;
 
-	err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr, pkt);
+	err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_2, addr, pkt);
 
 done:
 	return err;
@@ -664,14 +682,14 @@  int brcmf_sdiod_recv_chain(struct brcmf_sdio_dev *sdiodev,
 		goto done;
 
 	if (pktq->qlen == 1)
-		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr,
-					 pktq->next);
+		err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_2, addr,
+					    pktq->next);
 	else if (!sdiodev->sg_support) {
 		glom_skb = brcmu_pkt_buf_get_skb(totlen);
 		if (!glom_skb)
 			return -ENOMEM;
-		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr,
-					 glom_skb);
+		err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_2, addr,
+					    glom_skb);
 		if (err)
 			goto done;
 
@@ -695,6 +713,7 @@  int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 	int err;
 
 	mypkt = brcmu_pkt_buf_get_skb(nbytes);
+
 	if (!mypkt) {
 		brcmf_err("brcmu_pkt_buf_get_skb failed: len %d\n",
 			  nbytes);
@@ -706,10 +725,10 @@  int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 	err = brcmf_sdiod_addrprep(sdiodev, &addr);
 
 	if (!err)
-		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true, addr,
-					 mypkt);
+		err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_2, addr, mypkt);
 
 	brcmu_pkt_buf_free_skb(mypkt);
+
 	return err;
 
 }
@@ -729,8 +748,8 @@  int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 
 	if (pktq->qlen == 1 || !sdiodev->sg_support)
 		skb_queue_walk(pktq, skb) {
-			err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true,
-						 addr, skb);
+			err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_2,
+						     addr, skb);
 			if (err)
 				break;
 		}
@@ -782,10 +801,16 @@  brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 		sdaddr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
 
 		skb_put(pkt, dsize);
-		if (write)
+
+		if (write) {
 			memcpy(pkt->data, data, dsize);
-		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_1, write, sdaddr,
-					 pkt);
+			err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_1,
+						     sdaddr, pkt);
+		} else {
+			err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_1,
+						    sdaddr, pkt);
+		}
+
 		if (err) {
 			brcmf_err("membytes transfer failed\n");
 			break;

[11/34] brcmfmac: Split brcmf_sdiod_buffrw function up.

Commit Message

Comments

Patch