> -----Original Message-----
> From: Kalle Valo [mailto:kv...@codeaurora.org]
> Sent: Sunday, October 14, 2018 1:48 AM
> To: Tony Chuang
> Cc: Johannes Berg; larry.fin...@lwfinger.net; Pkshih; Andy Huang;
> sgrus...@redhat.com; linux-wireless@vger.kernel.org
> Subject: Re: [RFC v3 01/12] rtw88: main files
>
> Tony Chuang <yhchu...@realtek.com> writes:
>
> >> > +static void rtw_watch_dog_work(struct work_struct *work)
> >> > +{
> >> > + struct rtw_dev *rtwdev = container_of(work, struct rtw_dev,
> >> > + watch_dog_work.work);
> >> > + struct rtw_vif *rtwvif;
> >> > +
> >> > + if (!rtw_flag_check(rtwdev, RTW_FLAG_RUNNING))
> >> > + return;
> >> > +
> >> > + ieee80211_queue_delayed_work(rtwdev->hw,
> >> &rtwdev->watch_dog_work,
> >> > + RTW_WATCH_DOG_DELAY_TIME);
> >>
> >> You're aware of the power cost of waking up every 2 seconds? That's a
> >> really bad idea, in general, at the very least you should use a more
> >> power efficient scheduling here to combine with other wakeups
> >> (round_jiffies_relative, or so).
> >
> > Yeah I knew it, but so far we can only work like this...
> > Will use round_jiffies_relative to combine the CPU wakeups.
>
> Can you elaborate more why this horrible timer is needed? And it
> definitely needs a comment in the code explaining the reason.
>
The watchdog timer is required for our devices to enhance the performance.
It does a lot of tx/rx statistics processing for the hardware.
Those information process routines help the devices to adapt to the environment.
However, status polling every two seconds is not a good solution.
But it makes drive simpler to be implemented.
We will try to change it to interrupt mode.
But it will take a lot of time to work on it.
So, before it's done, I think we can leave the timer here.
Yan-Hsuan Chuang
