[PATCH 0/6] brcmfmac: PCIe changes and NVRAM support

[PATCH 0/6] brcmfmac: PCIe changes and NVRAM support

[Arend van Spriel]

This series for the brcmfmac driver provides:

* cleanup and new feature support in PCIe code.
* alternative nvram loading for router support.

The series is intended for v4.2 kernel and apply to the master
branch of the wireless-drivers-next repository.

Arend van Spriel (1):
  brcmfmac: allow device tree node without 'interrupts' property

Franky Lin (3):
  brcmfmac: remove pci shared structure rev4 support
  brcmfmac: remove dummy cache flush/invalidate function
  brcmfmac: add support for dma indices feature

Hante Meuleman (2):
  brcmfmac: Improve throughput by scheduling msbug flow worker.
  brcmfmac: Add support for host platform NVRAM loading.

 .../net/wireless/brcm80211/brcmfmac/commonring.c   |  18 ---
 drivers/net/wireless/brcm80211/brcmfmac/firmware.c | 107 +++++++++-----
 drivers/net/wireless/brcm80211/brcmfmac/flowring.c |   5 +-
 drivers/net/wireless/brcm80211/brcmfmac/flowring.h |   4 +-
 drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c   |  20 +--
 drivers/net/wireless/brcm80211/brcmfmac/of.c       |  11 +-
 drivers/net/wireless/brcm80211/brcmfmac/pcie.c     | 157 ++++++++++++++++-----
 7 files changed, 209 insertions(+), 113 deletions(-)

-- 
1.9.1

[PATCH 1/6] brcmfmac: allow device tree node without 'interrupts' property

[Arend van Spriel]

As described in the device tree bindings for 'brcm,bcm4329-fmac'
nodes, the interrupts property is optional. So adding a check
for the presence of this property before attempting to parse
and map the interrupt. If not present or parsing fails return
and fallback to in-band sdio interrupt.

Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/of.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/of.c b/drivers/net/wireless/brcm80211/brcmfmac/of.c
index c824570..03f35e0 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/of.c
@@ -39,10 +39,16 @@ void brcmf_of_probe(struct brcmf_sdio_dev *sdiodev)
 	if (!sdiodev->pdata)
 		return;
 
+	if (of_property_read_u32(np, "brcm,drive-strength", &val) == 0)
+		sdiodev->pdata->drive_strength = val;
+
+	/* make sure there are interrupts defined in the node */
+	if (!of_find_property(np, "interrupts", NULL))
+		return;
+
 	irq = irq_of_parse_and_map(np, 0);
 	if (!irq) {
 		brcmf_err("interrupt could not be mapped\n");
-		devm_kfree(dev, sdiodev->pdata);
 		return;
 	}
 	irqf = irqd_get_trigger_type(irq_get_irq_data(irq));
@@ -50,7 +56,4 @@ void brcmf_of_probe(struct brcmf_sdio_dev *sdiodev)
 	sdiodev->pdata->oob_irq_supported = true;
 	sdiodev->pdata->oob_irq_nr = irq;
 	sdiodev->pdata->oob_irq_flags = irqf;
-
-	if (of_property_read_u32(np, "brcm,drive-strength", &val) == 0)
-		sdiodev->pdata->drive_strength = val;
 }
-- 
1.9.1

[PATCH 2/6] brcmfmac: Improve throughput by scheduling msbug flow worker.

[Arend van Spriel]

From: Hante Meuleman <meuleman@broadcom.com>

The tx flow worker in msgbuf gets scheduled at tx till a certain
threshold has been reached. Then the tx completes will take over
the scheduling. When amsdu and ampdu is used the frames are
transferred wireless in a very bulky fashion, in combination
with this scheduling algorithm and buffer limiters in the stack
this can result in limited throughput. This change causes the
flow worker to be scheduled more frequently from tx.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/flowring.c | 5 +++--
 drivers/net/wireless/brcm80211/brcmfmac/flowring.h | 4 ++--
 drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c   | 9 ++++++---
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/brcm80211/brcmfmac/flowring.c
index eb13253..5944063 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/flowring.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/flowring.c
@@ -249,8 +249,8 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u8 flowid)
 }
 
 
-void brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
-			    struct sk_buff *skb)
+u32 brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
+			   struct sk_buff *skb)
 {
 	struct brcmf_flowring_ring *ring;
 
@@ -271,6 +271,7 @@ void brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
 		if (skb_queue_len(&ring->skblist) < BRCMF_FLOWRING_LOW)
 			brcmf_flowring_block(flow, flowid, false);
 	}
+	return skb_queue_len(&ring->skblist);
 }
 
 
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/flowring.h b/drivers/net/wireless/brcm80211/brcmfmac/flowring.h
index a34cd39..5551861 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/flowring.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/flowring.h
@@ -64,8 +64,8 @@ u32 brcmf_flowring_create(struct brcmf_flowring *flow, u8 da[ETH_ALEN],
 void brcmf_flowring_delete(struct brcmf_flowring *flow, u8 flowid);
 void brcmf_flowring_open(struct brcmf_flowring *flow, u8 flowid);
 u8 brcmf_flowring_tid(struct brcmf_flowring *flow, u8 flowid);
-void brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
-			    struct sk_buff *skb);
+u32 brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
+			   struct sk_buff *skb);
 struct sk_buff *brcmf_flowring_dequeue(struct brcmf_flowring *flow, u8 flowid);
 void brcmf_flowring_reinsert(struct brcmf_flowring *flow, u8 flowid,
 			     struct sk_buff *skb);
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c
index 4ec9811..b182f53 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c
@@ -73,7 +73,7 @@
 #define BRCMF_MSGBUF_TX_FLUSH_CNT1		32
 #define BRCMF_MSGBUF_TX_FLUSH_CNT2		96
 
-#define BRCMF_MSGBUF_DELAY_TXWORKER_THRS	64
+#define BRCMF_MSGBUF_DELAY_TXWORKER_THRS	96
 #define BRCMF_MSGBUF_TRICKLE_TXWORKER_THRS	32
 
 struct msgbuf_common_hdr {
@@ -797,6 +797,8 @@ static int brcmf_msgbuf_txdata(struct brcmf_pub *drvr, int ifidx,
 	struct brcmf_flowring *flow = msgbuf->flow;
 	struct ethhdr *eh = (struct ethhdr *)(skb->data);
 	u32 flowid;
+	u32 queue_count;
+	bool force;
 
 	flowid = brcmf_flowring_lookup(flow, eh->h_dest, skb->priority, ifidx);
 	if (flowid == BRCMF_FLOWRING_INVALID_ID) {
@@ -804,8 +806,9 @@ static int brcmf_msgbuf_txdata(struct brcmf_pub *drvr, int ifidx,
 		if (flowid == BRCMF_FLOWRING_INVALID_ID)
 			return -ENOMEM;
 	}
-	brcmf_flowring_enqueue(flow, flowid, skb);
-	brcmf_msgbuf_schedule_txdata(msgbuf, flowid, false);
+	queue_count = brcmf_flowring_enqueue(flow, flowid, skb);
+	force = ((queue_count % BRCMF_MSGBUF_TRICKLE_TXWORKER_THRS) == 0);
+	brcmf_msgbuf_schedule_txdata(msgbuf, flowid, force);
 
 	return 0;
 }
-- 
1.9.1

[PATCH 3/6] brcmfmac: remove pci shared structure rev4 support

[Arend van Spriel]

From: Franky Lin <frankyl@broadcom.com>

All pcie full dongle chips supported by fmac are using rev 5+ shared
structure. This patch removes the rev4 related code.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Signed-off-by: Franky Lin <frankyl@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/pcie.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
index 79ca24e..6ca3037 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
@@ -112,10 +112,9 @@ enum brcmf_pcie_state {
 						 BRCMF_PCIE_MB_INT_D2H3_DB0 | \
 						 BRCMF_PCIE_MB_INT_D2H3_DB1)
 
-#define BRCMF_PCIE_MIN_SHARED_VERSION		4
+#define BRCMF_PCIE_MIN_SHARED_VERSION		5
 #define BRCMF_PCIE_MAX_SHARED_VERSION		5
 #define BRCMF_PCIE_SHARED_VERSION_MASK		0x00FF
-#define BRCMF_PCIE_SHARED_TXPUSH_SUPPORT	0x4000
 
 #define BRCMF_PCIE_FLAGS_HTOD_SPLIT		0x4000
 #define BRCMF_PCIE_FLAGS_DTOH_SPLIT		0x8000
@@ -1280,11 +1279,6 @@ brcmf_pcie_init_share_ram_info(struct brcmf_pciedev_info *devinfo,
 		brcmf_err("Unsupported PCIE version %d\n", version);
 		return -EINVAL;
 	}
-	if (shared->flags & BRCMF_PCIE_SHARED_TXPUSH_SUPPORT) {
-		brcmf_err("Unsupported legacy TX mode 0x%x\n",
-			  shared->flags & BRCMF_PCIE_SHARED_TXPUSH_SUPPORT);
-		return -EINVAL;
-	}
 
 	addr = sharedram_addr + BRCMF_SHARED_MAX_RXBUFPOST_OFFSET;
 	shared->max_rxbufpost = brcmf_pcie_read_tcm16(devinfo, addr);
-- 
1.9.1

[PATCH 4/6] brcmfmac: remove dummy cache flush/invalidate function

[Arend van Spriel]

From: Franky Lin <frankyl@broadcom.com>

brcmf_dma_flush and brcmf_dma_invalidate_cache are not necessary and
have never been implemented.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Signed-off-by: Franky Lin <frankyl@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/commonring.c | 18 ------------------
 drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c     | 11 -----------
 drivers/net/wireless/brcm80211/brcmfmac/pcie.c       | 11 -----------
 3 files changed, 40 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/commonring.c b/drivers/net/wireless/brcm80211/brcmfmac/commonring.c
index 77656c7..26c6587 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/commonring.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/commonring.c
@@ -22,17 +22,6 @@
 #include "core.h"
 #include "commonring.h"
 
-
-/* dma flushing needs implementation for mips and arm platforms. Should
- * be put in util. Note, this is not real flushing. It is virtual non
- * cached memory. Only write buffers should have to be drained. Though
- * this may be different depending on platform......
- * SEE ALSO msgbuf.c
- */
-#define brcmf_dma_flush(addr, len)
-#define brcmf_dma_invalidate_cache(addr, len)
-
-
 void brcmf_commonring_register_cb(struct brcmf_commonring *commonring,
 				  int (*cr_ring_bell)(void *ctx),
 				  int (*cr_update_rptr)(void *ctx),
@@ -206,14 +195,9 @@ int brcmf_commonring_write_complete(struct brcmf_commonring *commonring)
 	address = commonring->buf_addr;
 	address += (commonring->f_ptr * commonring->item_len);
 	if (commonring->f_ptr > commonring->w_ptr) {
-		brcmf_dma_flush(address,
-				(commonring->depth - commonring->f_ptr) *
-				commonring->item_len);
 		address = commonring->buf_addr;
 		commonring->f_ptr = 0;
 	}
-	brcmf_dma_flush(address, (commonring->w_ptr - commonring->f_ptr) *
-			commonring->item_len);
 
 	commonring->f_ptr = commonring->w_ptr;
 
@@ -258,8 +242,6 @@ void *brcmf_commonring_get_read_ptr(struct brcmf_commonring *commonring,
 	if (commonring->r_ptr == commonring->depth)
 		commonring->r_ptr = 0;
 
-	brcmf_dma_invalidate_cache(ret_addr, *n_ items * commonring->item_len);
-
 	return ret_addr;
 }
 
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c
index b182f53..a932e45 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c
@@ -278,16 +278,6 @@ struct brcmf_msgbuf_pktids {
 	struct brcmf_msgbuf_pktid *array;
 };
 
-
-/* dma flushing needs implementation for mips and arm platforms. Should
- * be put in util. Note, this is not real flushing. It is virtual non
- * cached memory. Only write buffers should have to be drained. Though
- * this may be different depending on platform......
- */
-#define brcmf_dma_flush(addr, len)
-#define brcmf_dma_invalidate_cache(addr, len)
-
-
 static void brcmf_msgbuf_rxbuf_ioctlresp_post(struct brcmf_msgbuf *msgbuf);
 
 
@@ -462,7 +452,6 @@ static int brcmf_msgbuf_tx_ioctl(struct brcmf_pub *drvr, int ifidx,
 		memcpy(msgbuf->ioctbuf, buf, buf_len);
 	else
 		memset(msgbuf->ioctbuf, 0, buf_len);
-	brcmf_dma_flush(ioctl_buf, buf_len);
 
 	err = brcmf_commonring_write_complete(commonring);
 	brcmf_commonring_unlock(commonring);
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
index 6ca3037..2bc24a4 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
@@ -276,15 +276,6 @@ static const u32 brcmf_ring_itemsize[BRCMF_NROF_COMMON_MSGRINGS] = {
 };
 
 
-/* dma flushing needs implementation for mips and arm platforms. Should
- * be put in util. Note, this is not real flushing. It is virtual non
- * cached memory. Only write buffers should have to be drained. Though
- * this may be different depending on platform......
- */
-#define brcmf_dma_flush(addr, len)
-#define brcmf_dma_invalidate_cache(addr, len)
-
-
 static u32
 brcmf_pcie_read_reg32(struct brcmf_pciedev_info *devinfo, u32 reg_offset)
 {
@@ -1174,7 +1165,6 @@ static int brcmf_pcie_init_scratchbuffers(struct brcmf_pciedev_info *devinfo)
 		goto fail;
 
 	memset(devinfo->shared.scratch, 0, BRCMF_DMA_D2H_SCRATCH_BUF_LEN);
-	brcmf_dma_flush(devinfo->shared.scratch, BRCMF_DMA_D2H_SCRATCH_BUF_LEN);
 
 	addr = devinfo->shared.tcm_base_address +
 	       BRCMF_SHARED_DMA_SCRATCH_ADDR_OFFSET;
@@ -1192,7 +1182,6 @@ static int brcmf_pcie_init_scratchbuffers(struct brcmf_pciedev_info *devinfo)
 		goto fail;
 
 	memset(devinfo->shared.ringupd, 0, BRCMF_DMA_D2H_RINGUPD_BUF_LEN);
-	brcmf_dma_flush(devinfo->shared.ringupd, BRCMF_DMA_D2H_RINGUPD_BUF_LEN);
 
 	addr = devinfo->shared.tcm_base_address +
 	       BRCMF_SHARED_DMA_RINGUPD_ADDR_OFFSET;
-- 
1.9.1

[PATCH 5/6] brcmfmac: add support for dma indices feature

[Arend van Spriel]

From: Franky Lin <frankyl@broadcom.com>

PCIe full dongle firmware can support a dma indices feature with which
firmware can update/fetch the read/write indices of message buffer
rings on both host to dongle and dongle to host directions. The support is
announced by firmware through shared flags.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Franky Lin <frankyl@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/pcie.c | 140 +++++++++++++++++++++----
 1 file changed, 119 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
index 2bc24a4..37a2624 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c
@@ -115,6 +115,8 @@ enum brcmf_pcie_state {
 #define BRCMF_PCIE_MIN_SHARED_VERSION		5
 #define BRCMF_PCIE_MAX_SHARED_VERSION		5
 #define BRCMF_PCIE_SHARED_VERSION_MASK		0x00FF
+#define BRCMF_PCIE_SHARED_DMA_INDEX		0x10000
+#define BRCMF_PCIE_SHARED_DMA_2B_IDX		0x100000
 
 #define BRCMF_PCIE_FLAGS_HTOD_SPLIT		0x4000
 #define BRCMF_PCIE_FLAGS_DTOH_SPLIT		0x8000
@@ -146,6 +148,10 @@ enum brcmf_pcie_state {
 #define BRCMF_SHARED_RING_H2D_R_IDX_PTR_OFFSET	8
 #define BRCMF_SHARED_RING_D2H_W_IDX_PTR_OFFSET	12
 #define BRCMF_SHARED_RING_D2H_R_IDX_PTR_OFFSET	16
+#define BRCMF_SHARED_RING_H2D_WP_HADDR_OFFSET	20
+#define BRCMF_SHARED_RING_H2D_RP_HADDR_OFFSET	28
+#define BRCMF_SHARED_RING_D2H_WP_HADDR_OFFSET	36
+#define BRCMF_SHARED_RING_D2H_RP_HADDR_OFFSET	44
 #define BRCMF_SHARED_RING_TCM_MEMLOC_OFFSET	0
 #define BRCMF_SHARED_RING_MAX_SUB_QUEUES	52
 
@@ -247,6 +253,13 @@ struct brcmf_pciedev_info {
 	bool mbdata_completed;
 	bool irq_allocated;
 	bool wowl_enabled;
+	u8 dma_idx_sz;
+	void *idxbuf;
+	u32 idxbuf_sz;
+	dma_addr_t idxbuf_dmahandle;
+	u16 (*read_ptr)(struct brcmf_pciedev_info *devinfo, u32 mem_offset);
+	void (*write_ptr)(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
+			  u16 value);
 };
 
 struct brcmf_pcie_ringbuf {
@@ -323,6 +336,25 @@ brcmf_pcie_write_tcm16(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
 }
 
 
+static u16
+brcmf_pcie_read_idx(struct brcmf_pciedev_info *devinfo, u32 mem_offset)
+{
+	u16 *address = devinfo->idxbuf + mem_offset;
+
+	return (*(address));
+}
+
+
+static void
+brcmf_pcie_write_idx(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
+		     u16 value)
+{
+	u16 *address = devinfo->idxbuf + mem_offset;
+
+	*(address) = value;
+}
+
+
 static u32
 brcmf_pcie_read_tcm32(struct brcmf_pciedev_info *devinfo, u32 mem_offset)
 {
@@ -868,7 +900,7 @@ static int brcmf_pcie_ring_mb_write_rptr(void *ctx)
 	brcmf_dbg(PCIE, "W r_ptr %d (%d), ring %d\n", commonring->r_ptr,
 		  commonring->w_ptr, ring->id);
 
-	brcmf_pcie_write_tcm16(devinfo, ring->r_idx_addr, commonring->r_ptr);
+	devinfo->write_ptr(devinfo, ring->r_idx_addr, commonring->r_ptr);
 
 	return 0;
 }
@@ -886,7 +918,7 @@ static int brcmf_pcie_ring_mb_write_wptr(void *ctx)
 	brcmf_dbg(PCIE, "W w_ptr %d (%d), ring %d\n", commonring->w_ptr,
 		  commonring->r_ptr, ring->id);
 
-	brcmf_pcie_write_tcm16(devinfo, ring->w_idx_addr, commonring->w_ptr);
+	devinfo->write_ptr(devinfo, ring->w_idx_addr, commonring->w_ptr);
 
 	return 0;
 }
@@ -915,7 +947,7 @@ static int brcmf_pcie_ring_mb_update_rptr(void *ctx)
 	if (devinfo->state != BRCMFMAC_PCIE_STATE_UP)
 		return -EIO;
 
-	commonring->r_ptr = brcmf_pcie_read_tcm16(devinfo, ring->r_idx_addr);
+	commonring->r_ptr = devinfo->read_ptr(devinfo, ring->r_idx_addr);
 
 	brcmf_dbg(PCIE, "R r_ptr %d (%d), ring %d\n", commonring->r_ptr,
 		  commonring->w_ptr, ring->id);
@@ -933,7 +965,7 @@ static int brcmf_pcie_ring_mb_update_wptr(void *ctx)
 	if (devinfo->state != BRCMFMAC_PCIE_STATE_UP)
 		return -EIO;
 
-	commonring->w_ptr = brcmf_pcie_read_tcm16(devinfo, ring->w_idx_addr);
+	commonring->w_ptr = devinfo->read_ptr(devinfo, ring->w_idx_addr);
 
 	brcmf_dbg(PCIE, "R w_ptr %d (%d), ring %d\n", commonring->w_ptr,
 		  commonring->r_ptr, ring->id);
@@ -1038,6 +1070,13 @@ static void brcmf_pcie_release_ringbuffers(struct brcmf_pciedev_info *devinfo)
 	}
 	kfree(devinfo->shared.flowrings);
 	devinfo->shared.flowrings = NULL;
+	if (devinfo->idxbuf) {
+		dma_free_coherent(&devinfo->pdev->dev,
+				  devinfo->idxbuf_sz,
+				  devinfo->idxbuf,
+				  devinfo->idxbuf_dmahandle);
+		devinfo->idxbuf = NULL;
+	}
 }
 
 
@@ -1053,19 +1092,72 @@ static int brcmf_pcie_init_ringbuffers(struct brcmf_pciedev_info *devinfo)
 	u32 addr;
 	u32 ring_mem_ptr;
 	u32 i;
+	u64 address;
+	u32 bufsz;
 	u16 max_sub_queues;
+	u8 idx_offset;
 
 	ring_addr = devinfo->shared.ring_info_addr;
 	brcmf_dbg(PCIE, "Base ring addr = 0x%08x\n", ring_addr);
+	addr = ring_addr + BRCMF_SHARED_RING_MAX_SUB_QUEUES;
+	max_sub_queues = brcmf_pcie_read_tcm16(devinfo, addr);
+
+	if (devinfo->dma_idx_sz != 0) {
+		bufsz = (BRCMF_NROF_D2H_COMMON_MSGRINGS + max_sub_queues) *
+			devinfo->dma_idx_sz * 2;
+		devinfo->idxbuf = dma_alloc_coherent(&devinfo->pdev->dev, bufsz,
+						     &devinfo->idxbuf_dmahandle,
+						     GFP_KERNEL);
+		if (!devinfo->idxbuf)
+			devinfo->dma_idx_sz = 0;
+	}
 
-	addr = ring_addr + BRCMF_SHARED_RING_D2H_W_IDX_PTR_OFFSET;
-	d2h_w_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
-	addr = ring_addr + BRCMF_SHARED_RING_D2H_R_IDX_PTR_OFFSET;
-	d2h_r_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
-	addr = ring_addr + BRCMF_SHARED_RING_H2D_W_IDX_PTR_OFFSET;
-	h2d_w_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
-	addr = ring_addr + BRCMF_SHARED_RING_H2D_R_IDX_PTR_OFFSET;
-	h2d_r_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
+	if (devinfo->dma_idx_sz == 0) {
+		addr = ring_addr + BRCMF_SHARED_RING_D2H_W_IDX_PTR_OFFSET;
+		d2h_w_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
+		addr = ring_addr + BRCMF_SHARED_RING_D2H_R_IDX_PTR_OFFSET;
+		d2h_r_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
+		addr = ring_addr + BRCMF_SHARED_RING_H2D_W_IDX_PTR_OFFSET;
+		h2d_w_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
+		addr = ring_addr + BRCMF_SHARED_RING_H2D_R_IDX_PTR_OFFSET;
+		h2d_r_idx_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
+		idx_offset = sizeof(u32);
+		devinfo->write_ptr = brcmf_pcie_write_tcm16;
+		devinfo->read_ptr = brcmf_pcie_read_tcm16;
+		brcmf_dbg(PCIE, "Using TCM indices\n");
+	} else {
+		memset(devinfo->idxbuf, 0, bufsz);
+		devinfo->idxbuf_sz = bufsz;
+		idx_offset = devinfo->dma_idx_sz;
+		devinfo->write_ptr = brcmf_pcie_write_idx;
+		devinfo->read_ptr = brcmf_pcie_read_idx;
+
+		h2d_w_idx_ptr = 0;
+		addr = ring_addr + BRCMF_SHARED_RING_H2D_WP_HADDR_OFFSET;
+		address = (u64)devinfo->idxbuf_dmahandle;
+		brcmf_pcie_write_tcm32(devinfo, addr, address & 0xffffffff);
+		brcmf_pcie_write_tcm32(devinfo, addr + 4, address >> 32);
+
+		h2d_r_idx_ptr = h2d_w_idx_ptr + max_sub_queues * idx_offset;
+		addr = ring_addr + BRCMF_SHARED_RING_H2D_RP_HADDR_OFFSET;
+		address += max_sub_queues * idx_offset;
+		brcmf_pcie_write_tcm32(devinfo, addr, address & 0xffffffff);
+		brcmf_pcie_write_tcm32(devinfo, addr + 4, address >> 32);
+
+		d2h_w_idx_ptr = h2d_r_idx_ptr + max_sub_queues * idx_offset;
+		addr = ring_addr + BRCMF_SHARED_RING_D2H_WP_HADDR_OFFSET;
+		address += max_sub_queues * idx_offset;
+		brcmf_pcie_write_tcm32(devinfo, addr, address & 0xffffffff);
+		brcmf_pcie_write_tcm32(devinfo, addr + 4, address >> 32);
+
+		d2h_r_idx_ptr = d2h_w_idx_ptr +
+				BRCMF_NROF_D2H_COMMON_MSGRINGS * idx_offset;
+		addr = ring_addr + BRCMF_SHARED_RING_D2H_RP_HADDR_OFFSET;
+		address += BRCMF_NROF_D2H_COMMON_MSGRINGS * idx_offset;
+		brcmf_pcie_write_tcm32(devinfo, addr, address & 0xffffffff);
+		brcmf_pcie_write_tcm32(devinfo, addr + 4, address >> 32);
+		brcmf_dbg(PCIE, "Using host memory indices\n");
+	}
 
 	addr = ring_addr + BRCMF_SHARED_RING_TCM_MEMLOC_OFFSET;
 	ring_mem_ptr = brcmf_pcie_read_tcm32(devinfo, addr);
@@ -1079,8 +1171,8 @@ static int brcmf_pcie_init_ringbuffers(struct brcmf_pciedev_info *devinfo)
 		ring->id = i;
 		devinfo->shared.commonrings[i] = ring;
 
-		h2d_w_idx_ptr += sizeof(u32);
-		h2d_r_idx_ptr += sizeof(u32);
+		h2d_w_idx_ptr += idx_offset;
+		h2d_r_idx_ptr += idx_offset;
 		ring_mem_ptr += BRCMF_RING_MEM_SZ;
 	}
 
@@ -1094,13 +1186,11 @@ static int brcmf_pcie_init_ringbuffers(struct brcmf_pciedev_info *devinfo)
 		ring->id = i;
 		devinfo->shared.commonrings[i] = ring;
 
-		d2h_w_idx_ptr += sizeof(u32);
-		d2h_r_idx_ptr += sizeof(u32);
+		d2h_w_idx_ptr += idx_offset;
+		d2h_r_idx_ptr += idx_offset;
 		ring_mem_ptr += BRCMF_RING_MEM_SZ;
 	}
 
-	addr = ring_addr + BRCMF_SHARED_RING_MAX_SUB_QUEUES;
-	max_sub_queues = brcmf_pcie_read_tcm16(devinfo, addr);
 	devinfo->shared.nrof_flowrings =
 			max_sub_queues - BRCMF_NROF_H2D_COMMON_MSGRINGS;
 	rings = kcalloc(devinfo->shared.nrof_flowrings, sizeof(*ring),
@@ -1124,15 +1214,15 @@ static int brcmf_pcie_init_ringbuffers(struct brcmf_pciedev_info *devinfo)
 					     ring);
 		ring->w_idx_addr = h2d_w_idx_ptr;
 		ring->r_idx_addr = h2d_r_idx_ptr;
-		h2d_w_idx_ptr += sizeof(u32);
-		h2d_r_idx_ptr += sizeof(u32);
+		h2d_w_idx_ptr += idx_offset;
+		h2d_r_idx_ptr += idx_offset;
 	}
 	devinfo->shared.flowrings = rings;
 
 	return 0;
 
 fail:
-	brcmf_err("Allocating commonring buffers failed\n");
+	brcmf_err("Allocating ring buffers failed\n");
 	brcmf_pcie_release_ringbuffers(devinfo);
 	return -ENOMEM;
 }
@@ -1269,6 +1359,14 @@ brcmf_pcie_init_share_ram_info(struct brcmf_pciedev_info *devinfo,
 		return -EINVAL;
 	}
 
+	/* check firmware support dma indicies */
+	if (shared->flags & BRCMF_PCIE_SHARED_DMA_INDEX) {
+		if (shared->flags & BRCMF_PCIE_SHARED_DMA_2B_IDX)
+			devinfo->dma_idx_sz = sizeof(u16);
+		else
+			devinfo->dma_idx_sz = sizeof(u32);
+	}
+
 	addr = sharedram_addr + BRCMF_SHARED_MAX_RXBUFPOST_OFFSET;
 	shared->max_rxbufpost = brcmf_pcie_read_tcm16(devinfo, addr);
 	if (shared->max_rxbufpost == 0)
-- 
1.9.1

[PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Arend van Spriel]

From: Hante Meuleman <meuleman@broadcom.com>

Host platforms such as routers supported by OpenWRT can
support NVRAM reading directly from internal NVRAM store.
With this patch the nvram load routines will fall back to
this method when there is no nvram file and support is
available in the kernel.

Cc: linux-mips@linux-mips.org
Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
Hi Kalle,

This patch relies on a change which has been submitted to the
linux-mips maintainer [1]. However, the brcmfmac code that relies on
it is under CONFIG_BCM47XX flag. Still need to know whether that
patch will be accepted or not before applying this one.

Regards,
Arend

[1] http://mid.gmane.org/1432122655-3224-1-git-send-email-arend@broadcom.com
---
 drivers/net/wireless/brcm80211/brcmfmac/firmware.c | 107 ++++++++++++++-------
 1 file changed, 72 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c
index 8ff31ff..bf8928d 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c
@@ -19,6 +19,9 @@
 #include <linux/device.h>
 #include <linux/firmware.h>
 #include <linux/module.h>
+#if IS_ENABLED(CONFIG_BCM47XX)
+#include <linux/bcm47xx_nvram.h>
+#endif
 
 #include "debug.h"
 #include "firmware.h"
@@ -43,7 +46,8 @@ enum nvram_parser_state {
  * struct nvram_parser - internal info for parser.
  *
  * @state: current parser state.
- * @fwnv: input buffer being parsed.
+ * @data: input buffer data pointer.
+ * @data_len : len of input buffer.
  * @nvram: output buffer with parse result.
  * @nvram_len: lenght of parse result.
  * @line: current line.
@@ -55,7 +59,8 @@ enum nvram_parser_state {
  */
 struct nvram_parser {
 	enum nvram_parser_state state;
-	const struct firmware *fwnv;
+	u8 *data;
+	u32 data_len;
 	u8 *nvram;
 	u32 nvram_len;
 	u32 line;
@@ -66,6 +71,27 @@ struct nvram_parser {
 	bool multi_dev_v2;
 };
 
+#if IS_ENABLED(CONFIG_BCM47XX)
+static char *brcmf_nvram_get_contents(size_t *nvram_size)
+{
+	return bcm47xx_nvram_get_contents(nvram_size);
+}
+
+static void brcmf_nvram_release_contents(char *nvram)
+{
+	bcm47xx_nvram_release_contents(nvram);
+}
+#else
+static char *brcmf_nvram_get_contents(size_t *nvram_size)
+{
+	return NULL;
+}
+
+static void brcmf_nvram_release_contents(char *nvram)
+{
+}
+#endif
+
 static bool is_nvram_char(char c)
 {
 	/* comment marker excluded */
@@ -85,7 +111,7 @@ static enum nvram_parser_state brcmf_nvram_handle_idle(struct nvram_parser *nvp)
 {
 	char c;
 
-	c = nvp->fwnv->data[nvp->pos];
+	c = nvp->data[nvp->pos];
 	if (c == '\n')
 		return COMMENT;
 	if (is_whitespace(c))
@@ -109,16 +135,16 @@ static enum nvram_parser_state brcmf_nvram_handle_key(struct nvram_parser *nvp)
 	enum nvram_parser_state st = nvp->state;
 	char c;
 
-	c = nvp->fwnv->data[nvp->pos];
+	c = nvp->data[nvp->pos];
 	if (c == '=') {
 		/* ignore RAW1 by treating as comment */
-		if (strncmp(&nvp->fwnv->data[nvp->entry], "RAW1", 4) == 0)
+		if (strncmp(&nvp->data[nvp->entry], "RAW1", 4) == 0)
 			st = COMMENT;
 		else
 			st = VALUE;
-		if (strncmp(&nvp->fwnv->data[nvp->entry], "devpath", 7) == 0)
+		if (strncmp(&nvp->data[nvp->entry], "devpath", 7) == 0)
 			nvp->multi_dev_v1 = true;
-		if (strncmp(&nvp->fwnv->data[nvp->entry], "pcie/", 5) == 0)
+		if (strncmp(&nvp->data[nvp->entry], "pcie/", 5) == 0)
 			nvp->multi_dev_v2 = true;
 	} else if (!is_nvram_char(c)) {
 		brcmf_dbg(INFO, "warning: ln=%d:col=%d: '=' expected, skip invalid key entry\n",
@@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp)
 	char *ekv;
 	u32 cplen;
 
-	c = nvp->fwnv->data[nvp->pos];
-	if (!is_nvram_char(c)) {
+	c = nvp->data[nvp->pos];
+	if (!is_nvram_char(c) && (c != ' ')) {
 		/* key,value pair complete */
-		ekv = (u8 *)&nvp->fwnv->data[nvp->pos];
-		skv = (u8 *)&nvp->fwnv->data[nvp->entry];
+		ekv = (u8 *)&nvp->data[nvp->pos];
+		skv = (u8 *)&nvp->data[nvp->entry];
 		cplen = ekv - skv;
 		if (nvp->nvram_len + cplen + 1 >= BRCMF_FW_MAX_NVRAM_SIZE)
 			return END;
@@ -164,7 +190,7 @@ brcmf_nvram_handle_comment(struct nvram_parser *nvp)
 {
 	char *eol, *sol;
 
-	sol = (char *)&nvp->fwnv->data[nvp->pos];
+	sol = (char *)&nvp->data[nvp->pos];
 	eol = strchr(sol, '\n');
 	if (eol == NULL)
 		return END;
@@ -191,18 +217,20 @@ static enum nvram_parser_state
 	brcmf_nvram_handle_end
 };
 
-static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
-				   const struct firmware *nv)
+static int brcmf_init_nvram_parser(struct nvram_parser *nvp, u8 *data,
+				   u32 data_len)
 {
 	size_t size;
 
 	memset(nvp, 0, sizeof(*nvp));
-	nvp->fwnv = nv;
+	nvp->data = data;
+	nvp->data_len = data_len;
+
 	/* Limit size to MAX_NVRAM_SIZE, some files contain lot of comment */
-	if (nv->size > BRCMF_FW_MAX_NVRAM_SIZE)
+	if (data_len > BRCMF_FW_MAX_NVRAM_SIZE)
 		size = BRCMF_FW_MAX_NVRAM_SIZE;
 	else
-		size = nv->size;
+		size = data_len;
 	/* Alloc for extra 0 byte + roundup by 4 + length field */
 	size += 1 + 3 + sizeof(u32);
 	nvp->nvram = kzalloc(size, GFP_KERNEL);
@@ -342,7 +370,7 @@ fail:
  * and converts newlines to NULs. Shortens buffer as needed and pads with NULs.
  * End of buffer is completed with token identifying length of buffer.
  */
-static void *brcmf_fw_nvram_strip(const struct firmware *nv, u32 *new_length,
+static void *brcmf_fw_nvram_strip(u8 *data, u32 data_len, u32 *new_length,
 				  u16 domain_nr, u16 bus_nr)
 {
 	struct nvram_parser nvp;
@@ -350,10 +378,10 @@ static void *brcmf_fw_nvram_strip(const struct firmware *nv, u32 *new_length,
 	u32 token;
 	__le32 token_le;
 
-	if (brcmf_init_nvram_parser(&nvp, nv) < 0)
+	if (brcmf_init_nvram_parser(&nvp, data, data_len) < 0)
 		return NULL;
 
-	while (nvp.pos < nv->size) {
+	while (nvp.pos < data_len) {
 		nvp.state = nv_parser_states[nvp.state](&nvp);
 		if (nvp.state == END)
 			break;
@@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
 	struct brcmf_fw *fwctx = ctx;
 	u32 nvram_length = 0;
 	void *nvram = NULL;
+	u8 *data = NULL;
+	size_t data_len;
+	bool raw_nvram;
 
 	brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
-	if (!fw && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
-		goto fail;
+	if ((fw) && (fw->data)) {
+		data = (u8 *)fw->data;
+		data_len = fw->size;
+		raw_nvram = false;
+	} else {
+		data = brcmf_nvram_get_contents(&data_len);
+		if (!data && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
+			goto fail;
+		raw_nvram = true;
+	}
 
-	if (fw) {
-		nvram = brcmf_fw_nvram_strip(fw, &nvram_length,
+	if (data) {
+		nvram = brcmf_fw_nvram_strip(data, (u32)data_len, &nvram_length,
 					     fwctx->domain_nr, fwctx->bus_nr);
-		release_firmware(fw);
-		if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
-			goto fail;
+		if (raw_nvram)
+			brcmf_nvram_release_contents(data);
 	}
 
+	if (fw)
+		release_firmware(fw);
+	if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
+		goto fail;
+
 	fwctx->done(fwctx->dev, fwctx->code, nvram, nvram_length);
 	kfree(fwctx);
 	return;
@@ -453,15 +496,9 @@ static void brcmf_fw_request_code_done(const struct firmware *fw, void *ctx)
 	if (!ret)
 		return;
 
-	/* when nvram is optional call .done() callback here */
-	if (fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL) {
-		fwctx->done(fwctx->dev, fw, NULL, 0);
-		kfree(fwctx);
-		return;
-	}
+	brcmf_fw_request_nvram_done(NULL, fwctx);
+	return;
 
-	/* failed nvram request */
-	release_firmware(fw);
 fail:
 	brcmf_dbg(TRACE, "failed: dev=%s\n", dev_name(fwctx->dev));
 	device_release_driver(fwctx->dev);
-- 
1.9.1

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Rafał Miłecki]

On 20 May 2015 at 14:09, Arend van Spriel <arend@broadcom.com> wrote:
> From: Hante Meuleman <meuleman@broadcom.com>
>
> Host platforms such as routers supported by OpenWRT can
> support NVRAM reading directly from internal NVRAM store.
> With this patch the nvram load routines will fall back to
> this method when there is no nvram file and support is
> available in the kernel.

FWIW it's OpenWrt :)


> This patch relies on a change which has been submitted to the
> linux-mips maintainer [1]. However, the brcmfmac code that relies on
> it is under CONFIG_BCM47XX flag. Still need to know whether that
> patch will be accepted or not before applying this one.

Yeah, lets give this patch a few days at least, so we can be sure
it'll be properly synced with MIPS work.
I'm OK with brcmfmac internal changes (you may send them now as
separated patch if you want to), but lets be careful with
bcm47xx_nvram.h part.


> @@ -19,6 +19,9 @@
>  #include <linux/device.h>
>  #include <linux/firmware.h>
>  #include <linux/module.h>
> +#if IS_ENABLED(CONFIG_BCM47XX)
> +#include <linux/bcm47xx_nvram.h>
> +#endif

This header is safe to include on any arch, as well as all functions
are. Drop the #if please.


> @@ -66,6 +71,27 @@ struct nvram_parser {
>         bool multi_dev_v2;
>  };
>
> +#if IS_ENABLED(CONFIG_BCM47XX)
> +static char *brcmf_nvram_get_contents(size_t *nvram_size)
> +{
> +       return bcm47xx_nvram_get_contents(nvram_size);
> +}
> +
> +static void brcmf_nvram_release_contents(char *nvram)
> +{
> +       bcm47xx_nvram_release_contents(nvram);
> +}
> +#else
> +static char *brcmf_nvram_get_contents(size_t *nvram_size)
> +{
> +       return NULL;
> +}
> +
> +static void brcmf_nvram_release_contents(char *nvram)
> +{
> +}
> +#endif

Everything you put in #else simply re-implements what you sent for
MIPS tree. We don't want to duplicate that code.
Also applying above code will break building wireless-drivers-next on
MIPS, we can't push this patch.

I can understand you are looking for a way to get this patch into
current -next and to somehow sync work across trees. I'm against
anything like merging MIPS tree to wireless-driver-next, but I may
have some idea.
I think the best way for achieving this is to rework your patch to
modify include/linux/bcm47xx_nvram.h. You could modify it the same way
you did in your patch for MIPS tree, except for
bcm47xx_nvram_get_contents. Don't implement this function for real (in
.c file), but instead make in dummy inline in a bcm47xx_nvram.h like:
static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
{
        /* TODO: Implement in .c file */
        return NULL;
}

-- 
Rafał

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Rafał Miłecki]

On 20 May 2015 at 14:09, Arend van Spriel <arend@broadcom.com> wrote:
> @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp)
>         char *ekv;
>         u32 cplen;
>
> -       c = nvp->fwnv->data[nvp->pos];
> -       if (!is_nvram_char(c)) {
> +       c = nvp->data[nvp->pos];
> +       if (!is_nvram_char(c) && (c != ' ')) {

Don't smuggle behavior changes in patches doing something else!


> @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
>         struct brcmf_fw *fwctx = ctx;
>         u32 nvram_length = 0;
>         void *nvram = NULL;
> +       u8 *data = NULL;
> +       size_t data_len;
> +       bool raw_nvram;
>
>         brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
> -       if (!fw && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
> -               goto fail;
> +       if ((fw) && (fw->data)) {

if (fw && fw->data)
will work just fine, I'm surprised checkpatch doesn't complain.

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Rafał Miłecki]

On 20 May 2015 at 16:33, Rafał Miłecki <zajec5@gmail.com> wrote:
> I think the best way for achieving this is to rework your patch to
> modify include/linux/bcm47xx_nvram.h. You could modify it the same way
> you did in your patch for MIPS tree, except for
> bcm47xx_nvram_get_contents. Don't implement this function for real (in
> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like:
> static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
> {
>         /* TODO: Implement in .c file */
>         return NULL;
> }

One more note.
This of course will lead to conflict at some point, but I believe
Linus will handle it.

-- 
Rafał

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Arend van Spriel]

On 05/21/15 10:28, Rafał Miłecki wrote:
> On 20 May 2015 at 16:33, Rafał Miłecki<zajec5@gmail.com>  wrote:
>> I think the best way for achieving this is to rework your patch to
>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way
>> you did in your patch for MIPS tree, except for
>> bcm47xx_nvram_get_contents. Don't implement this function for real (in
>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like:
>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
>> {
>>          /* TODO: Implement in .c file */
>>          return NULL;
>> }
>
> One more note.
> This of course will lead to conflict at some point, but I believe
> Linus will handle it.

I prefer to avoid tricks so I will ask to drop this patch and wait for 
it to land in the next kernel, ie. 4.2, and resubmit this patch for 4.3. 
I am not in a hurry.

Regards,
Arend

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Arend van Spriel]

On 05/21/15 11:28, Arend van Spriel wrote:
> On 05/21/15 10:28, Rafał Miłecki wrote:
>> On 20 May 2015 at 16:33, Rafał Miłecki<zajec5@gmail.com> wrote:
>>> I think the best way for achieving this is to rework your patch to
>>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way
>>> you did in your patch for MIPS tree, except for
>>> bcm47xx_nvram_get_contents. Don't implement this function for real (in
>>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like:
>>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
>>> {
>>> /* TODO: Implement in .c file */
>>> return NULL;
>>> }
>>
>> One more note.
>> This of course will lead to conflict at some point, but I believe
>> Linus will handle it.
>
> I prefer to avoid tricks so I will ask to drop this patch and wait for
> it to land in the next kernel, ie. 4.2, and resubmit this patch for 4.3.
> I am not in a hurry.

The 'it' in 'wait for it to land' being the mips patch providing the new 
api function. I will submit a v2 for that one.

Regards,
Arend

> Regards,
> Arend

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Rafał Miłecki]

On 21 May 2015 at 11:28, Arend van Spriel <arend@broadcom.com> wrote:
> On 05/21/15 10:28, Rafał Miłecki wrote:
>>
>> On 20 May 2015 at 16:33, Rafał Miłecki<zajec5@gmail.com>  wrote:
>>>
>>> I think the best way for achieving this is to rework your patch to
>>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way
>>> you did in your patch for MIPS tree, except for
>>> bcm47xx_nvram_get_contents. Don't implement this function for real (in
>>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like:
>>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
>>> {
>>>          /* TODO: Implement in .c file */
>>>          return NULL;
>>> }
>>
>>
>> One more note.
>> This of course will lead to conflict at some point, but I believe
>> Linus will handle it.
>
>
> I prefer to avoid tricks so I will ask to drop this patch and wait for it to
> land in the next kernel, ie. 4.2, and resubmit this patch for 4.3. I am not
> in a hurry.

OK :)

-- 
Rafał

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Kalle Valo]

Arend van Spriel <arend@broadcom.com> writes:

> On 05/21/15 10:28, Rafał Miłecki wrote:
>> On 20 May 2015 at 16:33, Rafał Miłecki<zajec5@gmail.com>  wrote:
>>> I think the best way for achieving this is to rework your patch to
>>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way
>>> you did in your patch for MIPS tree, except for
>>> bcm47xx_nvram_get_contents. Don't implement this function for real (in
>>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like:
>>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
>>> {
>>>          /* TODO: Implement in .c file */
>>>          return NULL;
>>> }
>>
>> One more note.
>> This of course will lead to conflict at some point, but I believe
>> Linus will handle it.
>
> I prefer to avoid tricks so I will ask to drop this patch and wait for
> it to land in the next kernel, ie. 4.2, and resubmit this patch for
> 4.3. I am not in a hurry.

Yes, please :) Let's try to avoid conflicts as much as possible.

-- 
Kalle Valo

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Arend van Spriel]

On 05/21/15 12:16, Kalle Valo wrote:
> Arend van Spriel<arend@broadcom.com>  writes:
>
>> On 05/21/15 10:28, Rafał Miłecki wrote:
>>> On 20 May 2015 at 16:33, Rafał Miłecki<zajec5@gmail.com>   wrote:
>>>> I think the best way for achieving this is to rework your patch to
>>>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way
>>>> you did in your patch for MIPS tree, except for
>>>> bcm47xx_nvram_get_contents. Don't implement this function for real (in
>>>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like:
>>>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len)
>>>> {
>>>>           /* TODO: Implement in .c file */
>>>>           return NULL;
>>>> }
>>>
>>> One more note.
>>> This of course will lead to conflict at some point, but I believe
>>> Linus will handle it.
>>
>> I prefer to avoid tricks so I will ask to drop this patch and wait for
>> it to land in the next kernel, ie. 4.2, and resubmit this patch for
>> 4.3. I am not in a hurry.
>
> Yes, please :) Let's try to avoid conflicts as much as possible.

Ok. Let's make it official. Kalle, can you drop patch 6 from the series 
(if not already done so ;-) ).

Regards,
Arend

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Kalle Valo]

Arend van Spriel <arend@broadcom.com> writes:

>>> I prefer to avoid tricks so I will ask to drop this patch and wait for
>>> it to land in the next kernel, ie. 4.2, and resubmit this patch for
>>> 4.3. I am not in a hurry.
>>
>> Yes, please :) Let's try to avoid conflicts as much as possible.
>
> Ok. Let's make it official. Kalle, can you drop patch 6 from the
> series (if not already done so ;-) ).

Ok, dropped.

-- 
Kalle Valo

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Arend van Spriel]

The patch is dropped, but I still owe you a response to this. I missed 
it because you sent 2 or 3 replies confusing me.

On 05/20/15 17:02, Rafał Miłecki wrote:
> On 20 May 2015 at 14:09, Arend van Spriel<arend@broadcom.com>  wrote:
>> @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp)
>>          char *ekv;
>>          u32 cplen;
>>
>> -       c = nvp->fwnv->data[nvp->pos];
>> -       if (!is_nvram_char(c)) {
>> +       c = nvp->data[nvp->pos];
>> +       if (!is_nvram_char(c)&&  (c != ' ')) {
>
> Don't smuggle behavior changes in patches doing something else!

The subject is "Add support for host platform NVRAM loading" and guess 
what. That type of NVRAM turned out to have spaces in the entries so in 
my opinion it is related to this patch. I can split it up if you feel 
strongly about this.

>> @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
>>          struct brcmf_fw *fwctx = ctx;
>>          u32 nvram_length = 0;
>>          void *nvram = NULL;
>> +       u8 *data = NULL;
>> +       size_t data_len;
>> +       bool raw_nvram;
>>
>>          brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
>> -       if (!fw&&  !(fwctx->flags&  BRCMF_FW_REQ_NV_OPTIONAL))
>> -               goto fail;
>> +       if ((fw)&&  (fw->data)) {
>
> if (fw&&  fw->data)
> will work just fine, I'm surprised checkpatch doesn't complain.

I ran checkpatch.pl --strict and did not get complaint about this change.

Regards,
Arend

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Rafał Miłecki]

On 22 May 2015 at 10:31, Arend van Spriel <arend@broadcom.com> wrote:
> On 05/20/15 17:02, Rafał Miłecki wrote:
>>
>> On 20 May 2015 at 14:09, Arend van Spriel<arend@broadcom.com>  wrote:
>>>
>>> @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp)
>>>          char *ekv;
>>>          u32 cplen;
>>>
>>> -       c = nvp->fwnv->data[nvp->pos];
>>> -       if (!is_nvram_char(c)) {
>>> +       c = nvp->data[nvp->pos];
>>> +       if (!is_nvram_char(c)&&  (c != ' ')) {
>>
>>
>> Don't smuggle behavior changes in patches doing something else!
>
>
> The subject is "Add support for host platform NVRAM loading" and guess what.
> That type of NVRAM turned out to have spaces in the entries so in my opinion
> it is related to this patch. I can split it up if you feel strongly about
> this.

I'd expect such patch to just implement *loading* from different
source and nothing else. If there are additional changes needed, I
think they should go in separated patch if possible.

I noticed the same problem with parsing NVRAM values and sent
[PATCH] brcmfmac: allow NVRAM values to contain space and '#' chars
, so you should be able to drop this patch of your patch anyway.
You may give me an Ack if you have a moment :)


>>> @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const
>>> struct firmware *fw, void *ctx)
>>>          struct brcmf_fw *fwctx = ctx;
>>>          u32 nvram_length = 0;
>>>          void *nvram = NULL;
>>> +       u8 *data = NULL;
>>> +       size_t data_len;
>>> +       bool raw_nvram;
>>>
>>>          brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
>>> -       if (!fw&&  !(fwctx->flags&  BRCMF_FW_REQ_NV_OPTIONAL))
>>> -               goto fail;
>>> +       if ((fw)&&  (fw->data)) {
>>
>>
>> if (fw&&  fw->data)
>> will work just fine, I'm surprised checkpatch doesn't complain.
>
> I ran checkpatch.pl --strict and did not get complaint about this change.

I know, it's weird. Maybe I'll report this an improvement idea to
checkpatch maintainer.

-- 
Rafał

Re: [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading.

[Arend van Spriel]

On 05/22/15 11:05, Rafał Miłecki wrote:
> On 22 May 2015 at 10:31, Arend van Spriel<arend@broadcom.com>  wrote:
>> On 05/20/15 17:02, Rafał Miłecki wrote:
>>>
>>> On 20 May 2015 at 14:09, Arend van Spriel<arend@broadcom.com>   wrote:
>>>>
>>>> @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp)
>>>>           char *ekv;
>>>>           u32 cplen;
>>>>
>>>> -       c = nvp->fwnv->data[nvp->pos];
>>>> -       if (!is_nvram_char(c)) {
>>>> +       c = nvp->data[nvp->pos];
>>>> +       if (!is_nvram_char(c)&&   (c != ' ')) {
>>>
>>>
>>> Don't smuggle behavior changes in patches doing something else!
>>
>>
>> The subject is "Add support for host platform NVRAM loading" and guess what.
>> That type of NVRAM turned out to have spaces in the entries so in my opinion
>> it is related to this patch. I can split it up if you feel strongly about
>> this.
>
> I'd expect such patch to just implement *loading* from different
> source and nothing else. If there are additional changes needed, I
> think they should go in separated patch if possible.
>
> I noticed the same problem with parsing NVRAM values and sent
> [PATCH] brcmfmac: allow NVRAM values to contain space and '#' chars
> , so you should be able to drop this patch of your patch anyway.
> You may give me an Ack if you have a moment :)

Whoops. I did not :-p I don't want to deal with '#' in value field as it 
is either invalid or irrelevant to firmware on the device.

Regards,
Arend

>>>> @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const
>>>> struct firmware *fw, void *ctx)
>>>>           struct brcmf_fw *fwctx = ctx;
>>>>           u32 nvram_length = 0;
>>>>           void *nvram = NULL;
>>>> +       u8 *data = NULL;
>>>> +       size_t data_len;
>>>> +       bool raw_nvram;
>>>>
>>>>           brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
>>>> -       if (!fw&&   !(fwctx->flags&   BRCMF_FW_REQ_NV_OPTIONAL))
>>>> -               goto fail;
>>>> +       if ((fw)&&   (fw->data)) {
>>>
>>>
>>> if (fw&&   fw->data)
>>> will work just fine, I'm surprised checkpatch doesn't complain.
>>
>> I ran checkpatch.pl --strict and did not get complaint about this change.
>
> I know, it's weird. Maybe I'll report this an improvement idea to
> checkpatch maintainer.
>

Re: [1/6] brcmfmac: allow device tree node without 'interrupts' property

[Kalle Valo]

> As described in the device tree bindings for 'brcm,bcm4329-fmac'
> nodes, the interrupts property is optional. So adding a check
> for the presence of this property before attempting to parse
> and map the interrupt. If not present or parsing fails return
> and fallback to in-band sdio interrupt.
> 
> Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>

Thanks, 5 patches applied to wireless-drivers-next.git:

f58001fa9570 brcmfmac: allow device tree node without 'interrupts' property
464a5f3f0429 brcmfmac: Improve throughput by scheduling msbug flow worker.
fd5e8cb8178a brcmfmac: remove pci shared structure rev4 support
c2d4182edc05 brcmfmac: remove dummy cache flush/invalidate function
f3550aeb7fcd brcmfmac: add support for dma indices feature

Kalle Valo