RE: [PATCH] rtw88: add debugfs to fix tx rate

From: Tony Chuang <yhchuang@realtek.com>
To: Kalle Valo <kvalo@codeaurora.org>,
	Johannes Berg <johannes@sipsolutions.net>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"briannorris@chromium.org" <briannorris@chromium.org>
Subject: RE: [PATCH] rtw88: add debugfs to fix tx rate
Date: Tue, 17 Mar 2020 10:32:58 +0000	[thread overview]
Message-ID: <ce990869ebf0478d98cd7e8416b36289@realtek.com> (raw)
In-Reply-To: <87eetrlanb.fsf@kamboji.qca.qualcomm.com>

// Add Johannes for commenting on adding another nl80211 commands

Kalle Valo <kvalo@codeaurora.org> writes:> 

> Tony Chuang <yhchuang@realtek.com> writes:
> 
> > Kalle Valo <kvalo@codeaurora.org> writes:
> >
> >> <yhchuang@realtek.com> writes:
> >>
> >> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >> >
> >> > It is useful to fix the bit rate of TX packets. For example, if
> >> > someone is measuring the TX power, or debugging with the issues
> >> > of the TX throughput on the field.
> >> >
> >> > To set the value of fixed rate, one should input corresponding
> >> > desc rate index (ex, 0x0b for DESC_RATE54M to fix at 54 Mbps).
> >> > Set a value larger than DESC_RATE_MAX will disable fix rate, so
> >> > the rate adaptive mechanism can resume to work.
> >> >
> >> > Example,
> >> >   To fix rate at MCS 1:
> >> >   echo 0x0d > /sys/kernel/debug/ieee80211/phy0/rtw88/fix_rate
> >> >
> >> >   To not to fix rate:
> >> >   echo 0xff > /sys/kernel/debug/ieee80211/phy0/rtw88/fix_rate
> >> >
> >> >   To know which rate was fixed at:
> >> >   cat /sys/kernel/debug/ieee80211/phy0/rtw88/fix_rate
> >> >
> >> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >>
> >> No way, debugfs is not a method for working around nl80211 and doing
> >> whatever idea you come up with. The goal is that we have a generic
> >> nl80211 command for all generic actions, like this one. And I think we
> >> already have an nl80211 command for fixing the tx rate, right?
> >>
> >
> > No, as I can see, there's no suitable nl80211 command that can achieve
> > what I want. If you are saying about
> NL80211_CMD_SET_TX_BITRATE_MASK,
> > it's used to allow some rates. But actually the firmware has its own rate
> > adaptive mechanism, so mask out the other rates does not mean the rate
> > left will be chosen. Moreover, the hardware will choose a lower bit rate
> > when retry, then the TX rate is not fixed at all. So the debugfs can disable
> > the firmware's RA mechanism, also disable the TX rate fall back when retry.
> > Both of them cannot be done by setting TX bitrate mask.
> 
> I'm confused, here you talk about firmware implementation etc but I'm
> just talking about replacing the fix_rate debugfs file to an nl80211
> command (for providing the fix_rate value). Can you clarify more why you
> think nl80211 is not suitable?

Oops, I thought that you wanted me to use the existing nl80211 command.
Now I know that you think we can add a new nl80211 command to help drivers
to fix the TX bitrate if necessary. If adding another nl80211 command for that
is acceptable, I can work on this. But I need Johannes's comment if it's better
to add a new nl80211 command or to expand the existing command
(ex. NL80211_CMD_SET_TX_BITRATE_MASK). It looks like that adding a new
nl80211 command will be better for me as expanding the existing one would
have great impact on the already distributed drivers/user-tools.

> 
> > I am sorry I need to add another debugfs for it, but to actually fix the TX
> > bitrate, we really need another debugfs or module parameter. Because
> > according to the design of the device there is not a good enough general
> > command I can use to fix the TX rate. If there is a command that can fix
> > the TX bitrate for me, please let me know, I can switch to it.
> 
> Sorry, but I'm not yet convinced that a debugfs file is justified.
> Fixing a transmit bitrate sounds like a very generic command, not
> something which should be in debugfs.
> 

Thanks.
Yen-Hsuan