DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A2EB12083A
Authenticated-By: 
From:   Tony Chuang <yhchuang@realtek.com>
To:     Kalle Valo <kvalo@codeaurora.org>
CC:     Johannes Berg <johannes@sipsolutions.net>,
        "Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>,
        Pkshih <pkshih@realtek.com>, Andy Huang <tehuang@realtek.com>,
        "sgruszka@redhat.com" <sgruszka@redhat.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: RE: [RFC v3 01/12] rtw88: main files
Thread-Topic: [RFC v3 01/12] rtw88: main files
Thread-Index: AQHUYxzYH/Zu3N4AK0aJXCSsksiUEqUqq1yg
Date:   Mon, 22 Oct 2018 03:40:27 +0000
Message-ID: <F7CD281DE3E379468C6D07993EA72F84D1734F60@RTITMBSVM01.realtek.com.tw>
References: <1538565659-29530-1-git-send-email-yhchuang@realtek.com>
        <1538565659-29530-2-git-send-email-yhchuang@realtek.com>
        <201810081447.w98ElQfu018110@rtits1.realtek.com.tw>
        <F7CD281DE3E379468C6D07993EA72F84D172DCB6@RTITMBSVM01.realtek.com.tw>
 <87lg7120xq.fsf@kamboji.qca.qualcomm.com>
In-Reply-To: <87lg7120xq.fsf@kamboji.qca.qualcomm.com>
Accept-Language: zh-TW, en-US
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk


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