2012-08-24 14:17:41

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: [RFC 1/3] ath9k: Fix BTCOEX timer triggering comparision

From: Mohammed Shafi Shajakhan <[email protected]>

Its safer to convert btcoex_period to 'us' while
comparing with btcoex_no_stomp which is in 'us'.
Did not find any functionality issues being fixed,
as the generic hardware timer triggers are usually
refreshed with the newer duty cycle.

Cc: Rajkumar Manoharan <[email protected]>
Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 2 +-
drivers/net/wireless/ath/ath9k/gpio.c | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 7373e4b..2bb89b1 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -473,7 +473,7 @@ struct ath_btcoex {
unsigned long op_flags;
int bt_stomp_type; /* Types of BT stomping */
u32 btcoex_no_stomp; /* in usec */
- u32 btcoex_period; /* in usec */
+ u32 btcoex_period; /* in msec */
u32 btscan_no_stomp; /* in usec */
u32 duty_cycle;
u32 bt_wait_time;
diff --git a/drivers/net/wireless/ath/ath9k/gpio.c b/drivers/net/wireless/ath/ath9k/gpio.c
index bacdb8f..273eb67 100644
--- a/drivers/net/wireless/ath/ath9k/gpio.c
+++ b/drivers/net/wireless/ath/ath9k/gpio.c
@@ -228,7 +228,12 @@ static void ath_btcoex_period_timer(unsigned long data)
ath9k_hw_btcoex_enable(ah);
spin_unlock_bh(&btcoex->btcoex_lock);

- if (btcoex->btcoex_period != btcoex->btcoex_no_stomp) {
+ /*
+ * btcoex_period is in msec while (btocex/btscan_)no_stomp are in usec,
+ * ensure that we properly convert btcoex_period to usec
+ * for any comparision with (btcoex/btscan_)no_stomp.
+ */
+ if (btcoex->btcoex_period * 1000 != btcoex->btcoex_no_stomp) {
if (btcoex->hw_timer_enabled)
ath9k_gen_timer_stop(ah, btcoex->no_stomp_timer);

--
1.7.0.4



2012-08-24 14:18:02

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: [RFC 3/3] ath9k_htc: Cancel BTCOEX related work before disabling BTCOEX

From: Mohammed Shafi Shajakhan <[email protected]>

Before disabling BTCOEX in the h/w cancel all BTCOEX related
works. This is similar to the commit in ath9k(c32cdbd8)
ath9k: Stop the BTCOEX timers before disabling BTCOEX

Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
drivers/net/wireless/ath/ath9k/htc_drv_gpio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c b/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
index fe4836b..8fd64a6 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
@@ -173,9 +173,9 @@ void ath9k_htc_stop_btcoex(struct ath9k_htc_priv *priv)

if (ah->btcoex_hw.enabled &&
ath9k_hw_get_btcoex_scheme(ah) != ATH_BTCOEX_CFG_NONE) {
- ath9k_hw_btcoex_disable(ah);
if (ah->btcoex_hw.scheme == ATH_BTCOEX_CFG_3WIRE)
ath_htc_cancel_btcoex_work(priv);
+ ath9k_hw_btcoex_disable(ah);
}
}

--
1.7.0.4


2012-08-27 10:51:40

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [RFC 1/3] ath9k: Fix BTCOEX timer triggering comparision

On Mon, Aug 27, 2012 at 03:00:07PM +0530, Mohammed Shafi Shajakhan wrote:
> Hi Raj,
>
>
> On Friday 24 August 2012 10:28 PM, Manoharan, Rajkumar wrote:
> >On Fri, Aug 24, 2012 at 07:47:29PM +0530, Mohammed Shafi Shajakhan wrote:
> >>From: Mohammed Shafi Shajakhan <[email protected]>
> >>
> >>Its safer to convert btcoex_period to 'us' while
> >>comparing with btcoex_no_stomp which is in 'us'.
> >>Did not find any functionality issues being fixed,
> >>as the generic hardware timer triggers are usually
> >>refreshed with the newer duty cycle.
> >>
> >In which way it is safer? What does the patch fix? It was intentionally
> >converted to msec by "ath9k: keep btcoex period in milliseconds".
>
> we got btcoex_period is in 'us' while 'btcoex_no_stomp' in 'ms'
> when we are going to compare , both of them had to been in same time
> units. Rather then mentioning it as 'safer' the commit log should say
> its 'more correct' :-) previously the comparison happened with same
> timeunits(ms).
>
> I could not find any functionality issue being fixed,
> but i saw the difference, when putting debug prints for
> btcoex->btcoex_period, btcoex->btcoex_no_stomp), there was so
> much difference. Yet the check itself
>
> if (btcoex->btcoex_period != btcoex->btcoex_no_stomp)
>
> seems to be true for almost all the cases. So fine with changing the
> commit log ?
>
> thanks a lot for the review!
>
Oops. Its a regression. Can you please include the commit "ath9k: keep btcoex
period in milliseconds" that causes this regression in your commit log.

-Rajkumar

2012-08-24 14:17:52

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: [RFC 2/3] ath9k_htc: Add a modparam to enable BTCOEX rather than default

From: Mohammed Shafi Shajakhan <[email protected]>

Enable BTCOEX for WB193(which seems to be the only supported
ath9k_htc BTCOEX chipset)only when it is enabled via modparam,
rather than enabling it by default.

Cc: Vivek Natarajan <[email protected]>
Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
drivers/net/wireless/ath/ath9k/htc_drv_gpio.c | 9 +++++++++
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 5 +++++
2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c b/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
index 07df279..fe4836b 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_gpio.c
@@ -182,8 +182,17 @@ void ath9k_htc_stop_btcoex(struct ath9k_htc_priv *priv)
void ath9k_htc_init_btcoex(struct ath9k_htc_priv *priv, char *product)
{
struct ath_hw *ah = priv->ah;
+ struct ath_common *common = ath9k_hw_common(ah);
int qnum;

+ /*
+ * Check if BTCOEX is globally disabled.
+ */
+ if (!common->btcoex_enabled) {
+ ah->btcoex_hw.scheme = ATH_BTCOEX_CFG_NONE;
+ return;
+ }
+
if (product && strncmp(product, ATH_HTC_BTCOEX_PRODUCT_ID, 5) == 0) {
ah->btcoex_hw.scheme = ATH_BTCOEX_CFG_3WIRE;
}
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index a035a38..d98255e 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -30,6 +30,10 @@ int htc_modparam_nohwcrypt;
module_param_named(nohwcrypt, htc_modparam_nohwcrypt, int, 0444);
MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption");

+static int ath9k_htc_btcoex_enable;
+module_param_named(btcoex_enable, ath9k_htc_btcoex_enable, int, 0444);
+MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
+
#define CHAN2G(_freq, _idx) { \
.center_freq = (_freq), \
.hw_value = (_idx), \
@@ -635,6 +639,7 @@ static int ath9k_init_priv(struct ath9k_htc_priv *priv,
common->hw = priv->hw;
common->priv = priv;
common->debug_mask = ath9k_debug;
+ common->btcoex_enabled = ath9k_htc_btcoex_enable == 1;

spin_lock_init(&priv->beacon_lock);
spin_lock_init(&priv->tx.tx_lock);
--
1.7.0.4


2012-08-27 12:38:59

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC 1/3] ath9k: Fix BTCOEX timer triggering comparision

Hi Raj,

On Monday 27 August 2012 04:23 PM, Rajkumar Manoharan wrote:
> On Mon, Aug 27, 2012 at 03:00:07PM +0530, Mohammed Shafi Shajakhan wrote:
>> Hi Raj,
>>
>>
>> On Friday 24 August 2012 10:28 PM, Manoharan, Rajkumar wrote:
>>> On Fri, Aug 24, 2012 at 07:47:29PM +0530, Mohammed Shafi Shajakhan wrote:
>>>> From: Mohammed Shafi Shajakhan <[email protected]>
>>>>
>>>> Its safer to convert btcoex_period to 'us' while
>>>> comparing with btcoex_no_stomp which is in 'us'.
>>>> Did not find any functionality issues being fixed,
>>>> as the generic hardware timer triggers are usually
>>>> refreshed with the newer duty cycle.
>>>>
>>> In which way it is safer? What does the patch fix? It was intentionally
>>> converted to msec by "ath9k: keep btcoex period in milliseconds".
>>
>> we got btcoex_period is in 'us' while 'btcoex_no_stomp' in 'ms'
>> when we are going to compare , both of them had to been in same time
>> units. Rather then mentioning it as 'safer' the commit log should say
>> its 'more correct' :-) previously the comparison happened with same
>> timeunits(ms).
>>
>> I could not find any functionality issue being fixed,
>> but i saw the difference, when putting debug prints for
>> btcoex->btcoex_period, btcoex->btcoex_no_stomp), there was so
>> much difference. Yet the check itself
>>
>> if (btcoex->btcoex_period != btcoex->btcoex_no_stomp)
>>
>> seems to be true for almost all the cases. So fine with changing the
>> commit log ?
>>
>> thanks a lot for the review!
>>
> Oops. Its a regression. Can you please include the commit "ath9k: keep btcoex
> period in milliseconds" that causes this regression in your commit log.
>

sure Raj, i would do it, it did not make any difference :-)
the h/w timers are usually stopped and started.
again, thanks for you kind review!


--
thanks,
shafi