LinuxLists.cc - [PATCH 3.4 0/2] brcm80211: two fullmac bug fix for 3.4

2012-04-20 21:28:05

Subject: [PATCH 3.4 0/2] brcm80211: two fullmac bug fix for 3.4

Hi John,

Here are two bug fix patches for brcmfmac aim for 3.4. One corrects a wrong SDIO function register write access behavior. The other one fixes an issue than dpc could miss completion events in heavy loading system.

Thanks,
Franky

Franky Lin (2):
brcm80211: fmac: fix SDIO function 0 register r/w issue
brcm80211: fmac: fix missing completion events issue

.../net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c | 6 ++-
drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c | 63 ++++++++++++++++----
2 files changed, 56 insertions(+), 13 deletions(-)

--
1.7.5.4

2012-04-23 21:25:15

by Franky Lin

[permalink] [raw]

Subject: [PATCH 3.4 V2 0/2] brcm80211: two fullmac bug fix for 3.4

Here are two bug fix patches for brcmfmac aim for 3.4. One corrects a wrong SDIO function register write access behavior. The other one fixes an issue than dpc could miss completion events in heavy loading system.

V2: Rework "brcm80211: fmac: fix SDIO function 0 register r/w issue" based on remark from Florian and Johannes.

Thanks,
Franky

Franky Lin (2):
brcm80211: fmac: fix SDIO function 0 register r/w issue
brcm80211: fmac: fix missing completion events issue

.../net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c | 8 ++-
drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c | 63 ++++++++++++++++----
2 files changed, 58 insertions(+), 13 deletions(-)

--
1.7.5.4

2012-04-20 21:28:05

by Franky Lin

[permalink] [raw]

Subject: [PATCH 3.4 2/2] brcm80211: fmac: fix missing completion events issue

dpc takes care of all data packets transmissions for sdio function
2. It is possible that it misses some completion events when the
traffic is heavy or it's running on a slow cpu. A linked list is
introduced to make sure dpc is invoked whenever needed.

Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c | 63 ++++++++++++++++----
1 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index 2bf5dda..eb3829b 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -574,6 +574,8 @@ struct brcmf_sdio {

struct task_struct *dpc_tsk;
struct completion dpc_wait;
+ struct list_head dpc_tsklst;
+ spinlock_t dpc_tl_lock;

struct semaphore sdsem;

@@ -2594,29 +2596,58 @@ clkwait:
return resched;
}

+static inline void brcmf_sdbrcm_adddpctsk(struct brcmf_sdio *bus)
+{
+ struct list_head *new_hd;
+ unsigned long flags;
+
+ if (in_interrupt())
+ new_hd = kzalloc(sizeof(struct list_head), GFP_ATOMIC);
+ else
+ new_hd = kzalloc(sizeof(struct list_head), GFP_KERNEL);
+ if (new_hd == NULL)
+ return;
+
+ spin_lock_irqsave(&bus->dpc_tl_lock, flags);
+ list_add_tail(new_hd, &bus->dpc_tsklst);
+ spin_unlock_irqrestore(&bus->dpc_tl_lock, flags);
+}
+
static int brcmf_sdbrcm_dpc_thread(void *data)
{
struct brcmf_sdio *bus = (struct brcmf_sdio *) data;
+ struct list_head *cur_hd, *tmp_hd;
+ unsigned long flags;

allow_signal(SIGTERM);
/* Run until signal received */
while (1) {
if (kthread_should_stop())
break;
- if (!wait_for_completion_interruptible(&bus->dpc_wait)) {
- /* Call bus dpc unless it indicated down
- (then clean stop) */
- if (bus->sdiodev->bus_if->state != BRCMF_BUS_DOWN) {
- if (brcmf_sdbrcm_dpc(bus))
- complete(&bus->dpc_wait);
- } else {
+
+ if (list_empty(&bus->dpc_tsklst))
+ if (wait_for_completion_interruptible(&bus->dpc_wait))
+ break;
+
+ spin_lock_irqsave(&bus->dpc_tl_lock, flags);
+ list_for_each_safe(cur_hd, tmp_hd, &bus->dpc_tsklst) {
+ spin_unlock_irqrestore(&bus->dpc_tl_lock, flags);
+
+ if (bus->sdiodev->bus_if->state == BRCMF_BUS_DOWN) {
/* after stopping the bus, exit thread */
brcmf_sdbrcm_bus_stop(bus->sdiodev->dev);
bus->dpc_tsk = NULL;
break;
}
- } else
- break;
+
+ if (brcmf_sdbrcm_dpc(bus))
+ brcmf_sdbrcm_adddpctsk(bus);
+
+ spin_lock_irqsave(&bus->dpc_tl_lock, flags);
+ list_del(cur_hd);
+ kfree(cur_hd);
+ }
+ spin_unlock_irqrestore(&bus->dpc_tl_lock, flags);
}
return 0;
}
@@ -2669,8 +2700,10 @@ static int brcmf_sdbrcm_bus_txdata(struct device *dev, struct sk_buff *pkt)
/* Schedule DPC if needed to send queued packet(s) */
if (!bus->dpc_sched) {
bus->dpc_sched = true;
- if (bus->dpc_tsk)
+ if (bus->dpc_tsk) {
+ brcmf_sdbrcm_adddpctsk(bus);
complete(&bus->dpc_wait);
+ }
}

return ret;
@@ -3514,8 +3547,10 @@ void brcmf_sdbrcm_isr(void *arg)
brcmf_dbg(ERROR, "isr w/o interrupt configured!\n");

bus->dpc_sched = true;
- if (bus->dpc_tsk)
+ if (bus->dpc_tsk) {
+ brcmf_sdbrcm_adddpctsk(bus);
complete(&bus->dpc_wait);
+ }
}

static bool brcmf_sdbrcm_bus_watchdog(struct brcmf_sdio *bus)
@@ -3559,8 +3594,10 @@ static bool brcmf_sdbrcm_bus_watchdog(struct brcmf_sdio *bus)
bus->ipend = true;

bus->dpc_sched = true;
- if (bus->dpc_tsk)
+ if (bus->dpc_tsk) {
+ brcmf_sdbrcm_adddpctsk(bus);
complete(&bus->dpc_wait);
+ }
}
}

@@ -3897,6 +3934,8 @@ void *brcmf_sdbrcm_probe(u32 regsva, struct brcmf_sdio_dev *sdiodev)
}
/* Initialize DPC thread */
init_completion(&bus->dpc_wait);
+ INIT_LIST_HEAD(&bus->dpc_tsklst);
+ spin_lock_init(&bus->dpc_tl_lock);
bus->dpc_tsk = kthread_run(brcmf_sdbrcm_dpc_thread,
bus, "brcmf_dpc");
if (IS_ERR(bus->dpc_tsk)) {
--
1.7.5.4

2012-04-20 21:28:05

by Franky Lin

[permalink] [raw]

Subject: [PATCH 3.4 1/2] brcm80211: fmac: fix SDIO function 0 register r/w issue

SDIO stack doesn't have a structure for function 0. The structure
pointer stored in card->sdio_func[0] is actually for function 1.
With current implementation the register read/write is applied to
function 1. This pathch fixes the issue.

Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Arend van Spriel <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
index 4688904..6989916 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
@@ -108,9 +108,13 @@ static inline int brcmf_sdioh_f0_write_byte(struct brcmf_sdio_dev *sdiodev,
sdio_release_host(sdfunc);
}
} else if (regaddr == SDIO_CCCR_ABORT) {
+ sdfunc = kzalloc(sizeof(struct sdio_func), GFP_KERNEL);
+ memcpy(sdfunc, sdiodev->func[0], sizeof(struct sdio_func));
+ sdfunc->num = 0;
sdio_claim_host(sdfunc);
sdio_writeb(sdfunc, *byte, regaddr, &err_ret);
sdio_release_host(sdfunc);
+ kfree(sdfunc);
} else if (regaddr < 0xF0) {
brcmf_dbg(ERROR, "F0 Wr:0x%02x: write disallowed\n", regaddr);
err_ret = -EPERM;
@@ -486,7 +490,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
kfree(bus_if);
return -ENOMEM;
}
- sdiodev->func[0] = func->card->sdio_func[0];
+ sdiodev->func[0] = func;
sdiodev->func[1] = func;
sdiodev->bus_if = bus_if;
bus_if->bus_priv.sdio = sdiodev;
--
1.7.5.4

2012-04-23 17:45:24

by Franky Lin

[permalink] [raw]

Subject: Re: [PATCH 3.4 1/2] brcm80211: fmac: fix SDIO function 0 register r/w issue

On 04/23/2012 01:50 AM, Johannes Berg wrote:
> On Mon, 2012-04-23 at 10:45 +0200, Florian Fainelli wrote:
>
>>> } else if (regaddr == SDIO_CCCR_ABORT) {
>>> + sdfunc = kzalloc(sizeof(struct sdio_func), GFP_KERNEL);
>>
>> You are not catching a kzalloc() possible failure here.
>>
>>> + memcpy(sdfunc, sdiodev->func[0], sizeof(struct sdio_func));
>
> and it should probably be kmemdup anyway?
>
> johannes
>

Thx Florian and Johannes, I will send out a V2.

Franky

2012-04-23 08:50:17

by Johannes Berg

[permalink] [raw]

Subject: Re: [PATCH 3.4 1/2] brcm80211: fmac: fix SDIO function 0 register r/w issue

On Mon, 2012-04-23 at 10:45 +0200, Florian Fainelli wrote:

> > } else if (regaddr == SDIO_CCCR_ABORT) {
> > + sdfunc = kzalloc(sizeof(struct sdio_func), GFP_KERNEL);
>
> You are not catching a kzalloc() possible failure here.
>
> > + memcpy(sdfunc, sdiodev->func[0], sizeof(struct sdio_func));

and it should probably be kmemdup anyway?

johannes

2012-04-23 08:47:14

by Florian Fainelli

[permalink] [raw]

Subject: Re: [PATCH 3.4 1/2] brcm80211: fmac: fix SDIO function 0 register r/w issue

Hi Franky,

Le 04/20/12 23:27, Franky Lin a écrit :
> SDIO stack doesn't have a structure for function 0. The structure
> pointer stored in card->sdio_func[0] is actually for function 1.
> With current implementation the register read/write is applied to
> function 1. This pathch fixes the issue.
>
> Reviewed-by: Pieter-Paul Giesberts<[email protected]>
> Reviewed-by: Arend van Spriel<[email protected]>
> Signed-off-by: Franky Lin<[email protected]>
> ---
> .../net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
> index 4688904..6989916 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
> @@ -108,9 +108,13 @@ static inline int brcmf_sdioh_f0_write_byte(struct brcmf_sdio_dev *sdiodev,
> sdio_release_host(sdfunc);
> }
> } else if (regaddr == SDIO_CCCR_ABORT) {
> + sdfunc = kzalloc(sizeof(struct sdio_func), GFP_KERNEL);

You are not catching a kzalloc() possible failure here.

> + memcpy(sdfunc, sdiodev->func[0], sizeof(struct sdio_func));
> + sdfunc->num = 0;
> sdio_claim_host(sdfunc);
> sdio_writeb(sdfunc, *byte, regaddr,&err_ret);
> sdio_release_host(sdfunc);
> + kfree(sdfunc);
> } else if (regaddr< 0xF0) {
> brcmf_dbg(ERROR, "F0 Wr:0x%02x: write disallowed\n", regaddr);
> err_ret = -EPERM;
> @@ -486,7 +490,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
> kfree(bus_if);
> return -ENOMEM;
> }
> - sdiodev->func[0] = func->card->sdio_func[0];
> + sdiodev->func[0] = func;
> sdiodev->func[1] = func;
> sdiodev->bus_if = bus_if;
> bus_if->bus_priv.sdio = sdiodev;

2012-04-23 21:25:15

by Franky Lin

[permalink] [raw]

Subject: [PATCH 3.4 V2 2/2] brcm80211: fmac: fix missing completion events issue

2012-04-23 21:25:15

by Franky Lin

[permalink] [raw]

Subject: [PATCH 3.4 V2 1/2] brcm80211: fmac: fix SDIO function 0 register r/w issue

SDIO stack doesn't have a structure for function 0. The structure
pointer stored in card->sdio_func[0] is actually for function 1.
With current implementation the register read/write is applied to
function 1. This pathch fixes the issue.

Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Arend van Spriel <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Reviewed-by: Johannes Berg <[email protected]>
Signed-off-by: Franky Lin <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
index 4688904..758c115 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
@@ -108,9 +108,15 @@ static inline int brcmf_sdioh_f0_write_byte(struct brcmf_sdio_dev *sdiodev,
sdio_release_host(sdfunc);
}
} else if (regaddr == SDIO_CCCR_ABORT) {
+ sdfunc = kmemdup(sdiodev->func[0], sizeof(struct sdio_func),
+ GFP_KERNEL);
+ if (!sdfunc)
+ return -ENOMEM;
+ sdfunc->num = 0;
sdio_claim_host(sdfunc);
sdio_writeb(sdfunc, *byte, regaddr, &err_ret);
sdio_release_host(sdfunc);
+ kfree(sdfunc);
} else if (regaddr < 0xF0) {
brcmf_dbg(ERROR, "F0 Wr:0x%02x: write disallowed\n", regaddr);
err_ret = -EPERM;
@@ -486,7 +492,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
kfree(bus_if);
return -ENOMEM;
}
- sdiodev->func[0] = func->card->sdio_func[0];
+ sdiodev->func[0] = func;
sdiodev->func[1] = func;
sdiodev->bus_if = bus_if;
bus_if->bus_priv.sdio = sdiodev;
--
1.7.5.4