2018-11-12 07:29:26

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 0/8] brcmfmac: chip related changes

This patch series includes various chip-related changes:
* 43012 support
* 4373 saverestore support
* SDIO bus settings
* 4354 raw chipid

Changelog:
V2:
- Update commit message for patch 1.
- Update comments for patch 2.
- Remove CY_4373_F1_MESBUSYCTRL from patch 3.
- Collapse patch 6 (43102 sr support) in 4 (43012 support). Add helper functions.
- Remove sr_eng_en variable from patch 8.
- Collapse patch 10 and 11 in 9 (sdio_aos disable).

Chi-Hsien Lin (3):
brcmfmac: add support for CYW43012 SDIO chipset
brcmfmac: allow GCI core enumuration
brcmfmac: 4373 save-restore support

Madhan Mohan R (1):
brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373

Naveen Gupta (1):
brcmfmac: update 43012 F2 watermark setting to fix DMA Error during
UDP RX Traffic

Winnie Chang (1):
brcmfmac: add 4354 raw pcie device id

Wright Feng (2):
brcmfmac: set F2 watermark to 256 for 4373
brcmfmac: disable command decode in sdio_aos

.../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 +
.../wireless/broadcom/brcm80211/brcmfmac/chip.c | 33 ++++--
.../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 +
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 119 ++++++++++++++++++---
.../wireless/broadcom/brcm80211/brcmfmac/sdio.h | 3 +
.../broadcom/brcm80211/include/brcm_hw_ids.h | 2 +
include/linux/mmc/sdio_ids.h | 1 +
7 files changed, 139 insertions(+), 21 deletions(-)

--
2.1.0



2018-11-12 07:29:29

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 1/8] brcmfmac: add 4354 raw pcie device id

From: Winnie Chang <[email protected]>

Add the raw 4354 PCIe device ID for unprogrammed Cypress boards.

Reviewed-by: Arend Van Spriel <[email protected]>
Signed-off-by: Winnie Chang <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 +
drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 1 +
2 files changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 5dea569d63ed..8887bbc0084f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -2018,6 +2018,7 @@ static const struct dev_pm_ops brcmf_pciedrvr_pm = {
static const struct pci_device_id brcmf_pcie_devid_table[] = {
BRCMF_PCIE_DEVICE(BRCM_PCIE_4350_DEVICE_ID),
BRCMF_PCIE_DEVICE_SUB(0x4355, BRCM_PCIE_VENDOR_ID_BROADCOM, 0x4355),
+ BRCMF_PCIE_DEVICE(BRCM_PCIE_4354_RAW_DEVICE_ID),
BRCMF_PCIE_DEVICE(BRCM_PCIE_4356_DEVICE_ID),
BRCMF_PCIE_DEVICE(BRCM_PCIE_43567_DEVICE_ID),
BRCMF_PCIE_DEVICE(BRCM_PCIE_43570_DEVICE_ID),
diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
index 686f7a85a045..acb87238922f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
@@ -74,6 +74,7 @@
/* PCIE Device IDs */
#define BRCM_PCIE_4350_DEVICE_ID 0x43a3
#define BRCM_PCIE_4354_DEVICE_ID 0x43df
+#define BRCM_PCIE_4354_RAW_DEVICE_ID 0x4354
#define BRCM_PCIE_4356_DEVICE_ID 0x43ec
#define BRCM_PCIE_43567_DEVICE_ID 0x43d3
#define BRCM_PCIE_43570_DEVICE_ID 0x43d9
--
2.1.0


2018-11-12 07:29:34

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 2/8] brcmfmac: set F2 watermark to 256 for 4373

From: Wright Feng <[email protected]>

We got SDIO_CRC_ERROR with 4373 on SDR104 when doing bi-directional
throughput test. Enable watermark to 256 to guarantee the operation
stability.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Wright Feng <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
---
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 26 ++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index b2e1ab5adb64..05b8cfea5f9f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -49,6 +49,10 @@
#define DCMD_RESP_TIMEOUT msecs_to_jiffies(2500)
#define CTL_DONE_TIMEOUT msecs_to_jiffies(2500)

+/* watermark expressed in number of words */
+#define DEFAULT_F2_WATERMARK 0x8
+#define CY_4373_F2_WATERMARK 0x40
+
#ifdef DEBUG

#define BRCMF_TRAP_INFO_SIZE 80
@@ -138,6 +142,8 @@ struct rte_console {
/* 1: isolate internal sdio signals, put external pads in tri-state; requires
* sdio bus power cycle to clear (rev 9) */
#define SBSDIO_DEVCTL_PADS_ISO 0x08
+/* 1: enable F2 Watermark */
+#define SBSDIO_DEVCTL_F2WM_ENAB 0x10
/* Force SD->SB reset mapping (rev 11) */
#define SBSDIO_DEVCTL_SB_RST_CTL 0x30
/* Determined by CoreControl bit */
@@ -4046,6 +4052,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
void *nvram;
u32 nvram_len;
u8 saveclk;
+ u8 devctl;

brcmf_dbg(TRACE, "Enter: dev=%s, err=%d\n", dev_name(dev), err);

@@ -4101,8 +4108,23 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
brcmf_sdiod_writel(sdiod, core->base + SD_REG(hostintmask),
bus->hostintmask, NULL);

-
- brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK, 8, &err);
+ switch (sdiod->func1->device) {
+ case SDIO_DEVICE_ID_CYPRESS_4373:
+ brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n",
+ CY_4373_F2_WATERMARK);
+ brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK,
+ CY_4373_F2_WATERMARK, &err);
+ devctl = brcmf_sdiod_readb(sdiod, SBSDIO_DEVICE_CTL,
+ &err);
+ devctl |= SBSDIO_DEVCTL_F2WM_ENAB;
+ brcmf_sdiod_writeb(sdiod, SBSDIO_DEVICE_CTL, devctl,
+ &err);
+ break;
+ default:
+ brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK,
+ DEFAULT_F2_WATERMARK, &err);
+ break;
+ }
} else {
/* Disable F2 again */
sdio_disable_func(sdiod->func2);
--
2.1.0


2018-11-12 07:29:41

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 3/8] brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373

From: Madhan Mohan R <[email protected]>

Along with F2 watermark (existing) configuration, F1 MesBusyCtrl
should be enabled & configured to avoid overflow errors.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Madhan Mohan R <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 3 +++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h | 3 +++
2 files changed, 6 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 05b8cfea5f9f..b9ec40cc7d6b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4119,6 +4119,9 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
devctl |= SBSDIO_DEVCTL_F2WM_ENAB;
brcmf_sdiod_writeb(sdiod, SBSDIO_DEVICE_CTL, devctl,
&err);
+ brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_MESBUSYCTRL,
+ CY_4373_F2_WATERMARK |
+ SBSDIO_MESBUSYCTRL_ENAB, &err);
break;
default:
brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index 7faed831f07d..8aaabca1eb0e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -104,6 +104,9 @@
#define SBSDIO_FUNC1_RFRAMEBCHI 0x1001C
/* MesBusyCtl (rev 11) */
#define SBSDIO_FUNC1_MESBUSYCTRL 0x1001D
+/* Enable busy capability for MES access */
+#define SBSDIO_MESBUSYCTRL_ENAB 0x80
+
/* Sdio Core Rev 12 */
#define SBSDIO_FUNC1_WAKEUPCTRL 0x1001E
#define SBSDIO_FUNC1_WCTRL_ALPWAIT_MASK 0x1
--
2.1.0


2018-11-12 07:29:48

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 4/8] brcmfmac: add support for CYW43012 SDIO chipset

CYW43012 is a 1x1 802.11a/b/g/n Dual-Band HT20, 256-QAM/Turbo QAM. It
is an Ultra Low Power WLAN+BT combo chip.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
Signed-off-by: Praveen Babu C <[email protected]>
---
.../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 +
.../wireless/broadcom/brcm80211/brcmfmac/chip.c | 14 +++-
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 74 ++++++++++++++++++----
.../broadcom/brcm80211/include/brcm_hw_ids.h | 1 +
include/linux/mmc/sdio_ids.h | 1 +
5 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 3e37c8cf82c6..c1d4f93f7347 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -972,6 +972,7 @@ static const struct sdio_device_id brcmf_sdmmc_ids[] = {
BRCMF_SDIO_DEVICE(SDIO_DEVICE_ID_BROADCOM_4354),
BRCMF_SDIO_DEVICE(SDIO_DEVICE_ID_BROADCOM_4356),
BRCMF_SDIO_DEVICE(SDIO_DEVICE_ID_CYPRESS_4373),
+ BRCMF_SDIO_DEVICE(SDIO_DEVICE_ID_CYPRESS_43012),
{ /* end: all zeroes */ }
};
MODULE_DEVICE_TABLE(sdio, brcmf_sdmmc_ids);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 927d62b3d41b..a3c857721446 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -165,6 +165,7 @@ struct sbconfig {
#define SRCI_LSS_MASK 0x00f00000
#define SRCI_LSS_SHIFT 20
#define SRCI_SRNB_MASK 0xf0
+#define SRCI_SRNB_MASK_EXT 0x100
#define SRCI_SRNB_SHIFT 4
#define SRCI_SRBSZ_MASK 0xf
#define SRCI_SRBSZ_SHIFT 0
@@ -592,7 +593,13 @@ static void brcmf_chip_socram_ramsize(struct brcmf_core_priv *sr, u32 *ramsize,
if (lss != 0)
*ramsize += (1 << ((lss - 1) + SR_BSZ_BASE));
} else {
- nb = (coreinfo & SRCI_SRNB_MASK) >> SRCI_SRNB_SHIFT;
+ /* length of SRAM Banks increased for corerev greater than 23 */
+ if (sr->pub.rev >= 23) {
+ nb = (coreinfo & (SRCI_SRNB_MASK | SRCI_SRNB_MASK_EXT))
+ >> SRCI_SRNB_SHIFT;
+ } else {
+ nb = (coreinfo & SRCI_SRNB_MASK) >> SRCI_SRNB_SHIFT;
+ }
for (i = 0; i < nb; i++) {
retent = brcmf_chip_socram_banksize(sr, i, &banksize);
*ramsize += banksize;
@@ -1356,6 +1363,11 @@ bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
addr = CORE_CC_REG(base, sr_control1);
reg = chip->ops->read32(chip->ctx, addr);
return reg != 0;
+ case CY_CC_43012_CHIP_ID:
+ addr = CORE_CC_REG(pmu->base, retention_ctl);
+ reg = chip->ops->read32(chip->ctx, addr);
+ return (reg & (PMU_RCTL_MACPHY_DISABLE_MASK |
+ PMU_RCTL_LOGIC_DISABLE_MASK)) == 0;
default:
addr = CORE_CC_REG(pmu->base, pmucapabilities_ext);
reg = chip->ops->read32(chip->ctx, addr);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index b9ec40cc7d6b..7707b0169c21 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -624,6 +624,7 @@ BRCMF_FW_DEF(43455, "brcmfmac43455-sdio");
BRCMF_FW_DEF(4354, "brcmfmac4354-sdio");
BRCMF_FW_DEF(4356, "brcmfmac4356-sdio");
BRCMF_FW_DEF(4373, "brcmfmac4373-sdio");
+BRCMF_FW_DEF(43012, "brcmfmac43012-sdio");

static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
BRCMF_FW_ENTRY(BRCM_CC_43143_CHIP_ID, 0xFFFFFFFF, 43143),
@@ -643,7 +644,8 @@ static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
BRCMF_FW_ENTRY(BRCM_CC_4345_CHIP_ID, 0xFFFFFFC0, 43455),
BRCMF_FW_ENTRY(BRCM_CC_4354_CHIP_ID, 0xFFFFFFFF, 4354),
BRCMF_FW_ENTRY(BRCM_CC_4356_CHIP_ID, 0xFFFFFFFF, 4356),
- BRCMF_FW_ENTRY(CY_CC_4373_CHIP_ID, 0xFFFFFFFF, 4373)
+ BRCMF_FW_ENTRY(CY_CC_4373_CHIP_ID, 0xFFFFFFFF, 4373),
+ BRCMF_FW_ENTRY(CY_CC_43012_CHIP_ID, 0xFFFFFFFF, 43012)
};

static void pkt_align(struct sk_buff *p, int len, int align)
@@ -677,6 +679,14 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
/* 1st KSO write goes to AOS wake up core if device is asleep */
brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val, &err);

+ /* In case of 43012 chip, the chip could go down immediately after
+ * KSO bit is cleared. So the further reads of KSO register could
+ * fail. Thereby just bailing out immediately after clearing KSO
+ * bit, to avoid polling of KSO bit.
+ */
+ if (!on && bus->ci->chip == CY_CC_43012_CHIP_ID)
+ return err;
+
if (on) {
/* device WAKEUP through KSO:
* write bit 0 & read back until
@@ -2402,6 +2412,14 @@ static int brcmf_sdio_tx_ctrlframe(struct brcmf_sdio *bus, u8 *frame, u16 len)
return ret;
}

+static bool brcmf_chip_is_ulp(struct brcmf_chip *ci)
+{
+ if (ci->chip == CY_CC_43012_CHIP_ID)
+ return true;
+ else
+ return false;
+}
+
static void brcmf_sdio_bus_stop(struct device *dev)
{
struct brcmf_bus *bus_if = dev_get_drvdata(dev);
@@ -2409,7 +2427,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
struct brcmf_sdio *bus = sdiodev->bus;
struct brcmf_core *core = bus->sdio_core;
u32 local_hostintmask;
- u8 saveclk;
+ u8 saveclk, bpreq;
int err;

brcmf_dbg(TRACE, "Enter\n");
@@ -2436,9 +2454,14 @@ static void brcmf_sdio_bus_stop(struct device *dev)
/* Force backplane clocks to assure F2 interrupt propagates */
saveclk = brcmf_sdiod_readb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
&err);
- if (!err)
- brcmf_sdiod_writeb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
- (saveclk | SBSDIO_FORCE_HT), &err);
+ if (!err) {
+ bpreq = saveclk;
+ bpreq |= brcmf_chip_is_ulp(bus->ci) ?
+ SBSDIO_HT_AVAIL_REQ : SBSDIO_FORCE_HT;
+ brcmf_sdiod_writeb(sdiodev,
+ SBSDIO_FUNC1_CHIPCLKCSR,
+ bpreq, &err);
+ }
if (err)
brcmf_err("Failed to force clock for F2: err %d\n",
err);
@@ -3328,20 +3351,45 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,
return bcmerror;
}

+static bool brcmf_sdio_aos_no_decode(struct brcmf_sdio *bus)
+{
+ if (bus->ci->chip == CY_CC_43012_CHIP_ID)
+ return true;
+ else
+ return false;
+}
+
static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)
{
int err = 0;
u8 val;
+ u8 wakeupctrl;
+ u8 cardcap;
+ u8 chipclkcsr;

brcmf_dbg(TRACE, "Enter\n");

+ if (brcmf_chip_is_ulp(bus->ci)) {
+ wakeupctrl = SBSDIO_FUNC1_WCTRL_ALPWAIT_SHIFT;
+ chipclkcsr = SBSDIO_HT_AVAIL_REQ;
+ } else {
+ wakeupctrl = SBSDIO_FUNC1_WCTRL_HTWAIT_SHIFT;
+ chipclkcsr = SBSDIO_FORCE_HT;
+ }
+
+ if (brcmf_sdio_aos_no_decode(bus)) {
+ cardcap = SDIO_CCCR_BRCM_CARDCAP_CMD_NODEC;
+ } else {
+ cardcap = (SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT |
+ SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT);
+ }
+
val = brcmf_sdiod_readb(bus->sdiodev, SBSDIO_FUNC1_WAKEUPCTRL, &err);
if (err) {
brcmf_err("error reading SBSDIO_FUNC1_WAKEUPCTRL\n");
return;
}
-
- val |= 1 << SBSDIO_FUNC1_WCTRL_HTWAIT_SHIFT;
+ val |= 1 << wakeupctrl;
brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_WAKEUPCTRL, val, &err);
if (err) {
brcmf_err("error writing SBSDIO_FUNC1_WAKEUPCTRL\n");
@@ -3350,8 +3398,7 @@ static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)

/* Add CMD14 Support */
brcmf_sdiod_func0_wb(bus->sdiodev, SDIO_CCCR_BRCM_CARDCAP,
- (SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT |
- SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT),
+ cardcap,
&err);
if (err) {
brcmf_err("error writing SDIO_CCCR_BRCM_CARDCAP\n");
@@ -3359,7 +3406,7 @@ static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)
}

brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
- SBSDIO_FORCE_HT, &err);
+ chipclkcsr, &err);
if (err) {
brcmf_err("error writing SBSDIO_FUNC1_CHIPCLKCSR\n");
return;
@@ -4051,7 +4098,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
const struct firmware *code;
void *nvram;
u32 nvram_len;
- u8 saveclk;
+ u8 saveclk, bpreq;
u8 devctl;

brcmf_dbg(TRACE, "Enter: dev=%s, err=%d\n", dev_name(dev), err);
@@ -4085,8 +4132,11 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
/* Force clocks on backplane to be sure F2 interrupt propagates */
saveclk = brcmf_sdiod_readb(sdiod, SBSDIO_FUNC1_CHIPCLKCSR, &err);
if (!err) {
+ bpreq = saveclk;
+ bpreq |= brcmf_chip_is_ulp(bus->ci) ?
+ SBSDIO_HT_AVAIL_REQ : SBSDIO_FORCE_HT;
brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_CHIPCLKCSR,
- (saveclk | SBSDIO_FORCE_HT), &err);
+ bpreq, &err);
}
if (err) {
brcmf_err("Failed to force clock for F2: err %d\n", err);
diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
index acb87238922f..839980da9643 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
@@ -60,6 +60,7 @@
#define BRCM_CC_43664_CHIP_ID 43664
#define BRCM_CC_4371_CHIP_ID 0x4371
#define CY_CC_4373_CHIP_ID 0x4373
+#define CY_CC_43012_CHIP_ID 43012

/* USB Device IDs */
#define BRCM_USB_43143_DEVICE_ID 0xbd1e
diff --git a/include/linux/mmc/sdio_ids.h b/include/linux/mmc/sdio_ids.h
index 4224902a8e22..4332199c71c2 100644
--- a/include/linux/mmc/sdio_ids.h
+++ b/include/linux/mmc/sdio_ids.h
@@ -42,6 +42,7 @@
#define SDIO_DEVICE_ID_BROADCOM_4354 0x4354
#define SDIO_DEVICE_ID_BROADCOM_4356 0x4356
#define SDIO_DEVICE_ID_CYPRESS_4373 0x4373
+#define SDIO_DEVICE_ID_CYPRESS_43012 43012

#define SDIO_VENDOR_ID_INTEL 0x0089
#define SDIO_DEVICE_ID_INTEL_IWMC3200WIMAX 0x1402
--
2.1.0


2018-11-12 07:29:50

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 5/8] brcmfmac: allow GCI core enumuration

GCI core is needed for ULP operation. Allow GCI core enumuration with
below changes:
- Allow GCI to be added to core list even when it doesn't have a wrapper.
- Allow 8K address space size.
- Don't overwrite the address value when an additional size descriptor
is in place.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index a3c857721446..a8d3b96b727f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -786,7 +786,7 @@ static int brcmf_chip_dmp_get_regaddr(struct brcmf_chip_priv *ci, u32 *eromaddr,
u32 *regbase, u32 *wrapbase)
{
u8 desc;
- u32 val;
+ u32 val, szdesc;
u8 mpnum = 0;
u8 stype, sztype, wraptype;

@@ -832,14 +832,15 @@ static int brcmf_chip_dmp_get_regaddr(struct brcmf_chip_priv *ci, u32 *eromaddr,

/* next size descriptor can be skipped */
if (sztype == DMP_SLAVE_SIZE_DESC) {
- val = brcmf_chip_dmp_get_desc(ci, eromaddr, NULL);
+ szdesc = brcmf_chip_dmp_get_desc(ci, eromaddr, NULL);
/* skip upper size descriptor if present */
- if (val & DMP_DESC_ADDRSIZE_GT32)
+ if (szdesc & DMP_DESC_ADDRSIZE_GT32)
brcmf_chip_dmp_get_desc(ci, eromaddr, NULL);
}

- /* only look for 4K register regions */
- if (sztype != DMP_SLAVE_SIZE_4K)
+ /* look for 4K or 8K register regions */
+ if (sztype != DMP_SLAVE_SIZE_4K &&
+ sztype != DMP_SLAVE_SIZE_8K)
continue;

stype = (val & DMP_SLAVE_TYPE) >> DMP_SLAVE_TYPE_S;
@@ -896,7 +897,8 @@ int brcmf_chip_dmp_erom_scan(struct brcmf_chip_priv *ci)

/* need core with ports */
if (nmw + nsw == 0 &&
- id != BCMA_CORE_PMU)
+ id != BCMA_CORE_PMU &&
+ id != BCMA_CORE_GCI)
continue;

/* try to obtain register address info */
--
2.1.0


2018-11-12 07:29:55

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 6/8] brcmfmac: update 43012 F2 watermark setting to fix DMA Error during UDP RX Traffic

From: Naveen Gupta <[email protected]>

The number of words that the read FIFO has to contain except
the end of frame before sends data back to the host.
Max watermark = (512B - 2* (BurstLength))/4 =
(512 - 128)/4 = 384/4 = 0x60
so if burst length (i.e. BurstLength = 64) is increased,
watermark has to be reduced. This is the optimal setting for this chip.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Naveen Gupta <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 7707b0169c21..e1708e297d07 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -52,6 +52,7 @@
/* watermark expressed in number of words */
#define DEFAULT_F2_WATERMARK 0x8
#define CY_4373_F2_WATERMARK 0x40
+#define CY_43012_F2_WATERMARK 0x60

#ifdef DEBUG

@@ -4173,6 +4174,17 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
CY_4373_F2_WATERMARK |
SBSDIO_MESBUSYCTRL_ENAB, &err);
break;
+ case SDIO_DEVICE_ID_CYPRESS_43012:
+ brcmf_dbg(INFO, "set F2 watermark to 0x%x*4 bytes\n",
+ CY_43012_F2_WATERMARK);
+ brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK,
+ CY_43012_F2_WATERMARK, &err);
+ devctl = brcmf_sdiod_readb(sdiod, SBSDIO_DEVICE_CTL,
+ &err);
+ devctl |= SBSDIO_DEVCTL_F2WM_ENAB;
+ brcmf_sdiod_writeb(sdiod, SBSDIO_DEVICE_CTL, devctl,
+ &err);
+ break;
default:
brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK,
DEFAULT_F2_WATERMARK, &err);
--
2.1.0


2018-11-12 07:30:00

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 7/8] brcmfmac: 4373 save-restore support

Use sr_eng_en bit to check 4373 sr support.

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index a8d3b96b727f..08d5173d000c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -1365,6 +1365,11 @@ bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
addr = CORE_CC_REG(base, sr_control1);
reg = chip->ops->read32(chip->ctx, addr);
return reg != 0;
+ case CY_CC_4373_CHIP_ID:
+ /* explicitly check SR engine enable bit */
+ addr = CORE_CC_REG(base, sr_control0);
+ reg = chip->ops->read32(chip->ctx, addr);
+ return (reg & BIT(0)) != 0;
case CY_CC_43012_CHIP_ID:
addr = CORE_CC_REG(pmu->base, retention_ctl);
reg = chip->ops->read32(chip->ctx, addr);
--
2.1.0


2018-11-12 07:30:07

by Chi-Hsien Lin

[permalink] [raw]
Subject: [PATCH V2 8/8] brcmfmac: disable command decode in sdio_aos

From: Wright Feng <[email protected]>

AOS is a part of the SDIOD core that becomes active when the rest of
SDIOD is sleeping to keep SDIO bus alive responding to reduced set of
commands.

Transaction between AOS and SDIOD is not protected, and if cmd 52 is
received in AOS and in the middle of response state changed from AOS to
SDIOD, response is corrupted and it causes to SDIO Host controller to
hang.

Command decode for below chips are disabled in this commit:
- 4339
- 4345
- 4354
- 4373

Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Wright Feng <[email protected]>
Signed-off-by: Double Lo <[email protected]>
Signed-off-by: Madhan Mohan R <[email protected]>
Signed-off-by: Chi-Hsien Lin <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index e1708e297d07..e5f487b37c5a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3354,7 +3354,11 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio *bus,

static bool brcmf_sdio_aos_no_decode(struct brcmf_sdio *bus)
{
- if (bus->ci->chip == CY_CC_43012_CHIP_ID)
+ if (bus->ci->chip == CY_CC_43012_CHIP_ID ||
+ bus->ci->chip == CY_CC_4373_CHIP_ID ||
+ bus->ci->chip == BRCM_CC_4339_CHIP_ID ||
+ bus->ci->chip == BRCM_CC_4345_CHIP_ID ||
+ bus->ci->chip == BRCM_CC_4354_CHIP_ID)
return true;
else
return false;
--
2.1.0


2018-11-12 09:13:12

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 3/8] brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373

On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
> From: Madhan Mohan R <[email protected]>
>
> Along with F2 watermark (existing) configuration, F1 MesBusyCtrl
> should be enabled & configured to avoid overflow errors.

I am a bit confused. Why is it necessary to program the watermark in
both SBSDIO_WATERMARK (0x10008) and SDSDIO_FUNC1_MESBUSYCTRL (0x1001D).
Looks suspicious to me.

Regards,
Arend

2018-11-12 09:26:04

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 6/8] brcmfmac: update 43012 F2 watermark setting to fix DMA Error during UDP RX Traffic

On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
> From: Naveen Gupta <[email protected]>
>
> The number of words that the read FIFO has to contain except
> the end of frame before sends data back to the host.
> Max watermark = (512B - 2* (BurstLength))/4 =
> (512 - 128)/4 = 384/4 = 0x60
> so if burst length (i.e. BurstLength = 64) is increased,
> watermark has to be reduced. This is the optimal setting for this chip.
>
> Reviewed-by: Arend van Spriel <[email protected]>
> Signed-off-by: Naveen Gupta <[email protected]>
> Signed-off-by: Chi-Hsien Lin <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index 7707b0169c21..e1708e297d07 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -52,6 +52,7 @@
> /* watermark expressed in number of words */
> #define DEFAULT_F2_WATERMARK 0x8
> #define CY_4373_F2_WATERMARK 0x40
> +#define CY_43012_F2_WATERMARK 0x60

So basically you increase queuing in firmware rx path. How does this
affect TCP latency. The DMA error obviously needs fixing, but why go
from a watermark of 32 bytes to the maximum of 384 bytes. Same question
for 4373.

Regards,
Arend

2018-11-12 10:11:27

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 1/8] brcmfmac: add 4354 raw pcie device id

On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
> From: Winnie Chang <[email protected]>
>
> Add the raw 4354 PCIe device ID for unprogrammed Cypress boards.

Already have my review tag so this is fine as is.

> Reviewed-by: Arend Van Spriel <[email protected]>
> Signed-off-by: Winnie Chang <[email protected]>
> Signed-off-by: Chi-Hsien Lin <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 +
> drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 1 +
> 2 files changed, 2 insertions(+)


2018-11-12 10:12:36

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 2/8] brcmfmac: set F2 watermark to 256 for 4373

On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
> From: Wright Feng <[email protected]>
>
> We got SDIO_CRC_ERROR with 4373 on SDR104 when doing bi-directional
> throughput test. Enable watermark to 256 to guarantee the operation
> stability.

Please see my comment on patch 6/8 regarding introducing queuing latency
with this change.

Regards,
Arend

> Reviewed-by: Arend van Spriel <[email protected]>
> Signed-off-by: Wright Feng <[email protected]>
> Signed-off-by: Chi-Hsien Lin <[email protected]>
> ---
> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 26 ++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)


2018-11-12 10:16:53

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 5/8] brcmfmac: allow GCI core enumuration

On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
> GCI core is needed for ULP operation. Allow GCI core enumuration with
> below changes:
> - Allow GCI to be added to core list even when it doesn't have a wrapper.
> - Allow 8K address space size.
> - Don't overwrite the address value when an additional size descriptor
> is in place.

One question. This only assures the GCI core is listed. So does the
driver need to access it for ULP operation?

Regards,
Arend

> Reviewed-by: Arend van Spriel <[email protected]>
> Signed-off-by: Chi-Hsien Lin <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> index a3c857721446..a8d3b96b727f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> @@ -786,7 +786,7 @@ static int brcmf_chip_dmp_get_regaddr(struct brcmf_chip_priv *ci, u32 *eromaddr,
> u32 *regbase, u32 *wrapbase)
> {
> u8 desc;
> - u32 val;
> + u32 val, szdesc;
> u8 mpnum = 0;
> u8 stype, sztype, wraptype;
>
> @@ -832,14 +832,15 @@ static int brcmf_chip_dmp_get_regaddr(struct brcmf_chip_priv *ci, u32 *eromaddr,
>
> /* next size descriptor can be skipped */
> if (sztype == DMP_SLAVE_SIZE_DESC) {
> - val = brcmf_chip_dmp_get_desc(ci, eromaddr, NULL);
> + szdesc = brcmf_chip_dmp_get_desc(ci, eromaddr, NULL);
> /* skip upper size descriptor if present */
> - if (val & DMP_DESC_ADDRSIZE_GT32)
> + if (szdesc & DMP_DESC_ADDRSIZE_GT32)
> brcmf_chip_dmp_get_desc(ci, eromaddr, NULL);
> }
>
> - /* only look for 4K register regions */
> - if (sztype != DMP_SLAVE_SIZE_4K)
> + /* look for 4K or 8K register regions */
> + if (sztype != DMP_SLAVE_SIZE_4K &&
> + sztype != DMP_SLAVE_SIZE_8K)
> continue;
>
> stype = (val & DMP_SLAVE_TYPE) >> DMP_SLAVE_TYPE_S;
> @@ -896,7 +897,8 @@ int brcmf_chip_dmp_erom_scan(struct brcmf_chip_priv *ci)
>
> /* need core with ports */
> if (nmw + nsw == 0 &&
> - id != BCMA_CORE_PMU)
> + id != BCMA_CORE_PMU &&
> + id != BCMA_CORE_GCI)
> continue;
>
> /* try to obtain register address info */
>


2018-11-12 10:24:30

by Chi-Hsien Lin

[permalink] [raw]
Subject: Re: [PATCH V2 5/8] brcmfmac: allow GCI core enumuration



On 11/12/2018 6:16, Arend van Spriel wrote:
> On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
>> GCI core is needed for ULP operation. Allow GCI core enumuration with
>> below changes:
>> ?- Allow GCI to be added to core list even when it doesn't have a
>> wrapper.
>> ?- Allow 8K address space size.
>> ?- Don't overwrite the address value when an additional size descriptor
>> ?? is in place.
>
> One question. This only assures the GCI core is listed. So does the
> driver need to access it for ULP operation?

Yes, the GCI core registers are accessed when entering/exiting ULP sleep
modes. There will be other patches for the ULP support.

Regards,
Chi-hsien Lin

>
> Regards,
> Arend
>
>> Reviewed-by: Arend van Spriel <[email protected]>
>> Signed-off-by: Chi-Hsien Lin <[email protected]>
>> ---
>> ?drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 14
>> ++++++++------
>> ?1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> index a3c857721446..a8d3b96b727f 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> @@ -786,7 +786,7 @@ static int brcmf_chip_dmp_get_regaddr(struct
>> brcmf_chip_priv *ci, u32 *eromaddr,
>> ??????????????????????????????????? u32 *regbase, u32 *wrapbase)
>> ?{
>> ????? u8 desc;
>> -???? u32 val;
>> +???? u32 val, szdesc;
>> ????? u8 mpnum = 0;
>> ????? u8 stype, sztype, wraptype;
>>
>> @@ -832,14 +832,15 @@ static int brcmf_chip_dmp_get_regaddr(struct
>> brcmf_chip_priv *ci, u32 *eromaddr,
>>
>> ????????????? /* next size descriptor can be skipped */
>> ????????????? if (sztype == DMP_SLAVE_SIZE_DESC) {
>> -???????????????????? val = brcmf_chip_dmp_get_desc(ci, eromaddr, NULL);
>> +???????????????????? szdesc = brcmf_chip_dmp_get_desc(ci, eromaddr,
>> NULL);
>> ????????????????????? /* skip upper size descriptor if present */
>> -???????????????????? if (val & DMP_DESC_ADDRSIZE_GT32)
>> +???????????????????? if (szdesc & DMP_DESC_ADDRSIZE_GT32)
>> ????????????????????????????? brcmf_chip_dmp_get_desc(ci, eromaddr,
>> NULL);
>> ????????????? }
>>
>> -???????????? /* only look for 4K register regions */
>> -???????????? if (sztype != DMP_SLAVE_SIZE_4K)
>> +???????????? /* look for 4K or 8K register regions */
>> +???????????? if (sztype != DMP_SLAVE_SIZE_4K &&
>> +???????????????? sztype != DMP_SLAVE_SIZE_8K)
>> ????????????????????? continue;
>>
>> ????????????? stype = (val & DMP_SLAVE_TYPE) >> DMP_SLAVE_TYPE_S;
>> @@ -896,7 +897,8 @@ int brcmf_chip_dmp_erom_scan(struct
>> brcmf_chip_priv *ci)
>>
>> ????????????? /* need core with ports */
>> ????????????? if (nmw + nsw == 0 &&
>> -???????????????? id != BCMA_CORE_PMU)
>> +???????????????? id != BCMA_CORE_PMU &&
>> +???????????????? id != BCMA_CORE_GCI)
>> ????????????????????? continue;
>>
>> ????????????? /* try to obtain register address info */
>>
>
>
>
> ----------
>
> You're receiving this message because you're a member of the
> brcm80211-dev-list group.
> .
>

2018-11-12 10:27:05

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 5/8] brcmfmac: allow GCI core enumuration

On 11/12/2018 11:24 AM, Chi-Hsien Lin wrote:
>
>
> On 11/12/2018 6:16, Arend van Spriel wrote:
>> On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
>>> GCI core is needed for ULP operation. Allow GCI core enumuration with
>>> below changes:
>>> - Allow GCI to be added to core list even when it doesn't have a
>>> wrapper.
>>> - Allow 8K address space size.
>>> - Don't overwrite the address value when an additional size descriptor
>>> is in place.
>>
>> One question. This only assures the GCI core is listed. So does the
>> driver need to access it for ULP operation?
>
> Yes, the GCI core registers are accessed when entering/exiting ULP sleep
> modes. There will be other patches for the ULP support.

Yeah. I suspected such. It would have been better to send this patch
with that ULP series, but let's not make it an issue now. The patch is fine.

Regards,
Arend


2018-11-12 10:29:52

by Chi-Hsien Lin

[permalink] [raw]
Subject: Re: [PATCH V2 5/8] brcmfmac: allow GCI core enumuration



On 11/12/2018 6:27, Arend van Spriel wrote:
> On 11/12/2018 11:24 AM, Chi-Hsien Lin wrote:
>>
>>
>> On 11/12/2018 6:16, Arend van Spriel wrote:
>>> On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
>>>> GCI core is needed for ULP operation. Allow GCI core enumuration with
>>>> below changes:
>>>> ?- Allow GCI to be added to core list even when it doesn't have a
>>>> wrapper.
>>>> ?- Allow 8K address space size.
>>>> ?- Don't overwrite the address value when an additional size descriptor
>>>> ?? is in place.
>>>
>>> One question. This only assures the GCI core is listed. So does the
>>> driver need to access it for ULP operation?
>>
>> Yes, the GCI core registers are accessed when entering/exiting ULP sleep
>> modes. There will be other patches for the ULP support.
>
> Yeah. I suspected such. It would have been better to send this patch
> with that ULP series, but let's not make it an issue now. The patch is
> fine.

Thanks a lot. :)

>
> Regards,
> Arend
>
> .
>

2018-11-12 10:30:23

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 7/8] brcmfmac: 4373 save-restore support

On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
> Use sr_eng_en bit to check 4373 sr support.
>
> Reviewed-by: Arend van Spriel <[email protected]>
> Signed-off-by: Chi-Hsien Lin <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> index a8d3b96b727f..08d5173d000c 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> @@ -1365,6 +1365,11 @@ bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
> addr = CORE_CC_REG(base, sr_control1);
> reg = chip->ops->read32(chip->ctx, addr);
> return reg != 0;
> + case CY_CC_4373_CHIP_ID:
> + /* explicitly check SR engine enable bit */
> + addr = CORE_CC_REG(base, sr_control0);
> + reg = chip->ops->read32(chip->ctx, addr);
> + return (reg & BIT(0)) != 0;

Sorry for not saying it earlier, but maybe it is good to add define of
SR engine enable bit in brcm80211/include/chipcommon.h.

Regards,
Arend

2018-11-12 10:32:03

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 8/8] brcmfmac: disable command decode in sdio_aos

On 11/12/2018 8:30 AM, Chi-Hsien Lin wrote:
> From: Wright Feng <[email protected]>
>
> AOS is a part of the SDIOD core that becomes active when the rest of
> SDIOD is sleeping to keep SDIO bus alive responding to reduced set of
> commands.
>
> Transaction between AOS and SDIOD is not protected, and if cmd 52 is
> received in AOS and in the middle of response state changed from AOS to
> SDIOD, response is corrupted and it causes to SDIO Host controller to
> hang.
>
> Command decode for below chips are disabled in this commit:
> - 4339
> - 4345
> - 4354
> - 4373

Already have my review tag so this is fine as is.

> Reviewed-by: Arend van Spriel <[email protected]>
> Signed-off-by: Wright Feng <[email protected]>
> Signed-off-by: Double Lo <[email protected]>
> Signed-off-by: Madhan Mohan R <[email protected]>
> Signed-off-by: Chi-Hsien Lin <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)


2018-11-12 10:34:04

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 8/8] brcmfmac: disable command decode in sdio_aos

On 11/12/2018 8:30 AM, Chi-Hsien Lin wrote:
> From: Wright Feng <[email protected]>
>
> AOS is a part of the SDIOD core that becomes active when the rest of
> SDIOD is sleeping to keep SDIO bus alive responding to reduced set of
> commands.
>
> Transaction between AOS and SDIOD is not protected, and if cmd 52 is
> received in AOS and in the middle of response state changed from AOS to
> SDIOD, response is corrupted and it causes to SDIO Host controller to
> hang.

Just one question. The above sound pretty generic so does it apply to
any SDIO chip with AOS logic?

Regards,
Arend


2018-11-15 08:08:56

by Wright Feng

[permalink] [raw]
Subject: Re: [PATCH V2 8/8] brcmfmac: disable command decode in sdio_aos



On 2018/11/12 下午 06:33, Arend van Spriel wrote:
> On 11/12/2018 8:30 AM, Chi-Hsien Lin wrote:
>> From: Wright Feng <[email protected]>
>>
>> AOS is a part of the SDIOD core that becomes active when the rest of
>> SDIOD is sleeping to keep SDIO bus alive responding to reduced set of
>> commands.
>>
>> Transaction between AOS and SDIOD is not protected, and if cmd 52 is
>> received in AOS and in the middle of response state changed from AOS to
>> SDIOD, response is corrupted and it causes to SDIO Host controller to
>> hang.
>
> Just one question. The above sound pretty generic so does it apply to
> any SDIO chip with AOS logic?
>
We found this issue when verifying SR feature with some SDIO cards(what
we had), not sure whether every SDIO card has same problem. So we
only change those chip's wake-up mechanism to noCmdDecode mode and let
SDIOD_AOS just generates a wake-up request without responding.

-Wright
> Regards,
> Arend
>
>
>

2018-11-20 08:51:34

by Madhan Mohan R

[permalink] [raw]
Subject: Re: [PATCH V2 3/8] brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373

On Mon, Nov 12, 2018 at 10:13:07AM +0100, Arend van Spriel wrote:
> On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
> >From: Madhan Mohan R <[email protected]>
> >
> >Along with F2 watermark (existing) configuration, F1 MesBusyCtrl
> >should be enabled & configured to avoid overflow errors.
>
> I am a bit confused. Why is it necessary to program the watermark in
> both SBSDIO_WATERMARK (0x10008) and SDSDIO_FUNC1_MESBUSYCTRL
> (0x1001D).
> Looks suspicious to me.
>
> Regards,
> Arend

Hi Arend,

Register SDSDIO_FUNC1_MESBUSYCTRL(0x1001D) Bits [6:0] is programmed with
SDIO RX path watermark whereas the register SBSDIO_WATERMARK(0x10008) is
for the SDIO TX path.

Regards,
Madhan

2018-11-20 09:14:42

by Chi-Hsien Lin

[permalink] [raw]
Subject: Re: [PATCH V2 6/8] brcmfmac: update 43012 F2 watermark setting to fix DMA Error during UDP RX Traffic

(+Madhan)

On 11/12/2018 5:25, Arend van Spriel wrote:
> On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
>> From: Naveen Gupta <[email protected]>
>>
>> The number of words that the read FIFO has to contain except
>> the end of frame before sends data back to the host.
>> Max watermark = (512B - 2* (BurstLength))/4 =
>> (512 - 128)/4 = 384/4 = 0x60
>> so if burst length (i.e. BurstLength = 64) is increased,
>> watermark has to be reduced. This is the optimal setting for this chip.
>>
>> Reviewed-by: Arend van Spriel <[email protected]>
>> Signed-off-by: Naveen Gupta <[email protected]>
>> Signed-off-by: Chi-Hsien Lin <[email protected]>
>> ---
>> ?drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12
>> ++++++++++++
>> ?1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> index 7707b0169c21..e1708e297d07 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> @@ -52,6 +52,7 @@
>> ?/* watermark expressed in number of words */
>> ?#define DEFAULT_F2_WATERMARK??? 0x8
>> ?#define CY_4373_F2_WATERMARK??? 0x40
>> +#define CY_43012_F2_WATERMARK??? 0x60
>
> So basically you increase queuing in firmware rx path. How does this
> affect TCP latency. The DMA error obviously needs fixing, but why go
> from a watermark of 32 bytes to the maximum of 384 bytes. Same question
> for 4373.

This is to answer 2/8 and 6/8 -

For current chips like 4373 and 43012, we are configuring F2 block size
as 256 and it is recommended to program the F2 watermark (SDIO TX path -
SDIO Device to host traffic) to at least equal or greater than the block
size to make sure that at least >= 1 Block size of data is in FIFO
before sending on the SDIO bus to avoid underflow. We haven't observed
excess TCP latency with this setting.

Regards,
Chi-hsien Lin

>
> Regards,
> Arend

2018-11-20 09:15:19

by Chi-Hsien Lin

[permalink] [raw]
Subject: Re: [PATCH V2 7/8] brcmfmac: 4373 save-restore support



On 11/12/2018 6:30, Arend van Spriel wrote:
> On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
>> Use sr_eng_en bit to check 4373 sr support.
>>
>> Reviewed-by: Arend van Spriel <[email protected]>
>> Signed-off-by: Chi-Hsien Lin <[email protected]>
>> ---
>> ?drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 5 +++++
>> ?1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> index a8d3b96b727f..08d5173d000c 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
>> @@ -1365,6 +1365,11 @@ bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
>> ???????? addr = CORE_CC_REG(base, sr_control1);
>> ???????? reg = chip->ops->read32(chip->ctx, addr);
>> ???????? return reg != 0;
>> +??? case CY_CC_4373_CHIP_ID:
>> +??????? /* explicitly check SR engine enable bit */
>> +??????? addr = CORE_CC_REG(base, sr_control0);
>> +??????? reg = chip->ops->read32(chip->ctx, addr);
>> +??????? return (reg & BIT(0)) != 0;
>
> Sorry for not saying it earlier, but maybe it is good to add define of
> SR engine enable bit in brcm80211/include/chipcommon.h.

No problem. I'll make this change and submit V3 shortly.

>
> Regards,
> Arend
> .
>

2018-11-20 09:54:38

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH V2 3/8] brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373

On 11/20/2018 9:50 AM, Madhan Mohan R wrote:
> On Mon, Nov 12, 2018 at 10:13:07AM +0100, Arend van Spriel wrote:
>> On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote:
>>> From: Madhan Mohan R <[email protected]>
>>>
>>> Along with F2 watermark (existing) configuration, F1 MesBusyCtrl
>>> should be enabled & configured to avoid overflow errors.
>>
>> I am a bit confused. Why is it necessary to program the watermark in
>> both SBSDIO_WATERMARK (0x10008) and SDSDIO_FUNC1_MESBUSYCTRL
>> (0x1001D).
>> Looks suspicious to me.
>>
>> Regards,
>> Arend
>
> Hi Arend,
>
> Register SDSDIO_FUNC1_MESBUSYCTRL(0x1001D) Bits [6:0] is programmed with
> SDIO RX path watermark whereas the register SBSDIO_WATERMARK(0x10008) is
> for the SDIO TX path.

Thanks, Madhan

That makes sense. Maybe it would be good to express the tx vs rx detail
in the code.

Regards,
Arend