[ath9k-devel] [RFC] ath9k: Add a debug entry to stop/start ANI

[Mohammed Shafi <mshajakhan@atheros.com>]

On Tuesday 17 May 2011 05:08 PM, Mohammed Shajakhan wrote:
> On Tuesday 17 May 2011 04:43 PM, Vivek Natarajan wrote:
>> On Tue, May 17, 2011 at 3:59 PM, Mohammed Shafi Shajakhan
>> <mshajakhan@atheros.com>   wrote:
>>> From: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>>
>>> this requires complete testing, disabling seems to work fine but
>>> renabling does seems to be complete
>>>
>>> Signed-off-by: Mohammed Shafi Shajakhan<mshajakhan@atheros.com>
>>> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
>>> index 7afe332..6c9f6bb 100644
>>> --- a/drivers/net/wireless/ath/ath9k/debug.c
>>> +++ b/drivers/net/wireless/ath/ath9k/debug.c
>>> @@ -176,6 +176,56 @@ static const struct file_operations fops_rx_chainmask = {
>>>          .llseek = default_llseek,
>>>    };
>>>
>>> +static ssize_t read_file_disable_ani(struct file *file, char __user *user_buf,
>>> +                            size_t count, loff_t *ppos)
>>> +{
>>> +       struct ath_softc *sc = file->private_data;
>>> +       struct ath_common *common = ath9k_hw_common(sc->sc_ah);
>>> +       char buf[32];
>>> +       unsigned int len;
>>> +
>>> +       len = sprintf(buf, "%1d\n", common->disable_ani);
>>> +       return simple_read_from_buffer(user_buf, count, ppos, buf, len);
>>> +}
>>> +
>>> +static ssize_t write_file_disable_ani(struct file *file,
>>> +               const char __user *user_buf, size_t count, loff_t *ppos)
>>> +{
>>> +       struct ath_softc *sc = file->private_data;
>>> +       struct ath_common *common = ath9k_hw_common(sc->sc_ah);
>>> +       unsigned long disable_ani;
>>> +       char buf[32];
>>> +       ssize_t len;
>>> +
>>> +       len = min(count, sizeof(buf) - 1);
>>> +       if (copy_from_user(buf, user_buf, len))
>>> +               return -EFAULT;
>>> +
>>> +       buf[len] = '\0';
>>> +       if (strict_strtoul(buf, 0,&disable_ani))
>>> +               return -EINVAL;
>>> +
>>> +       common->disable_ani = !!(disable_ani);
>>> +
>>> +       if (disable_ani) {
>>> +               sc->sc_flags&= ~SC_OP_ANI_RUN;
>>> +               del_timer_sync(&common->ani.timer);
>>> +       }
>>> +
>>> +       else
>>> +               setup_timer(&common->ani.timer, ath_ani_calibrate,
>>> +                                               (unsigned long)sc);
>>
>> For restarting the timer, you need to do mod_timer and not setup_timer.
>
> Thanks! I will change that
>
>>
>>> +
>>> +       return count;
>>> +}
>>> +
>>> +static const struct file_operations fops_disable_ani = {
>>> +       .read = read_file_disable_ani,
>>> +       .write = write_file_disable_ani,
>>> +       .open = ath9k_debugfs_open,
>>> +       .owner = THIS_MODULE,
>>> +       .llseek = default_llseek,
>>> +};
>>>
>>>    static ssize_t read_file_dma(struct file *file, char __user *user_buf,
>>>                               size_t count, loff_t *ppos)
>>> @@ -1160,6 +1210,8 @@ int ath9k_init_debug(struct ath_hw *ah)
>>>                              sc->debug.debugfs_phy, sc,&fops_rx_chainmask);
>>>          debugfs_create_file("tx_chainmask", S_IRUSR | S_IWUSR,
>>>                              sc->debug.debugfs_phy, sc,&fops_tx_chainmask);
>>> +       debugfs_create_file("disable_ani", S_IRUSR | S_IWUSR,
>>> +                           sc->debug.debugfs_phy, sc,&fops_disable_ani);
>>>          debugfs_create_file("regidx", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
>>>                              sc,&fops_regidx);
>>>          debugfs_create_file("regval", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
>>> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
>>> index b172d15..bf8ac4e 100644
>>> --- a/drivers/net/wireless/ath/ath9k/init.c
>>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>>> @@ -519,8 +519,9 @@ static void ath9k_init_misc(struct ath_softc *sc)
>>>    {
>>>          struct ath_common *common = ath9k_hw_common(sc->sc_ah);
>>>          int i = 0;
>>> -
>>> -       setup_timer(&common->ani.timer, ath_ani_calibrate, (unsigned long)sc);
>>> +       if (!(common->disable_ani))
>>> +               setup_timer(&common->ani.timer,
>>> +                               ath_ani_calibrate, (unsigned long)sc);
>>
>> This need not depend on disable_ani since you can stop/restart using
>> del_timer and mod_timer.
>
> incase of disabling ANI completely right from the start by hardcoding
> disable_ani true in driver initialization, I thought this might help..
> if this looks redundant I will revert it back.
>
>>
>>>
>>>          sc->config.txpowlimit = ATH_TXPOWER_MAX;
>>>
>>> @@ -585,6 +586,7 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc, u16 subsysid,
>>>          common->priv = sc;
>>>          common->debug_mask = ath9k_debug;
>>>          common->btcoex_enabled = ath9k_btcoex_enable == 1;
>>> +       common->disable_ani = false;
>>>          spin_lock_init(&common->cc_lock);
>>>
>>>          spin_lock_init(&sc->sc_serial_rw);
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>> index 17ebdf1..1ac7c67 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -235,7 +235,8 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
>>>
>>>          sc->hw_busy_count = 0;
>>>
>>> -       del_timer_sync(&common->ani.timer);
>>> +       if (!(common->disable_ani))
>>> +               del_timer_sync(&common->ani.timer);
>>>          cancel_work_sync(&sc->paprd_work);
>>>          cancel_work_sync(&sc->hw_check_work);
>>>          cancel_delayed_work_sync(&sc->tx_complete_work);
>>> @@ -302,7 +303,8 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
>>>                          ath_set_beacon(sc);
>>>                  ieee80211_queue_delayed_work(sc->hw,&sc->tx_complete_work, 0);
>>>                  ieee80211_queue_delayed_work(sc->hw,&sc->hw_pll_work, HZ/2);
>>> -               ath_start_ani(common);
>>> +               if (!(common->disable_ani))
>>> +                       ath_start_ani(common);
>>
>> The same applies here. If the requirement is just to stop/restart
>> dynamically during runtime, ani initialisation need not depend on
>> disable_ani.
>
> as you had later suggested, this looks fine.
>
>
> Thanks a lot for the review! I will change the setup_timer to mod_timer
> and this will fix the ANI getting restarted properly.
>
>>

did not help in perfectly re starting the ANI, still I am missing 
something, i need to look into it more

>> Vivek.
>> .
>>
> .
>