> -----Original Message----- > From: Kalle Valo [mailto:kvalo@xxxxxxxxxxxxxx] > Sent: Sunday, October 14, 2018 1:48 AM > To: Tony Chuang > Cc: Johannes Berg; Larry.Finger@xxxxxxxxxxxx; Pkshih; Andy Huang; > sgruszka@xxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx > Subject: Re: [RFC v3 01/12] rtw88: main files > > Tony Chuang <yhchuang@xxxxxxxxxxx> 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
