Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 11387C433CB
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Tony Chuang <yhchuang@realtek.com>
Cc:     Johannes Berg <johannes@sipsolutions.net>,
        "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
References: <20200313065114.23433-1-yhchuang@realtek.com>
        <87eetwo87q.fsf@kamboji.qca.qualcomm.com>
        <2e492e530d744713871f885e324106ef@realtek.com>
        <87eetrlanb.fsf@kamboji.qca.qualcomm.com>
        <ce990869ebf0478d98cd7e8416b36289@realtek.com>
Date:   Tue, 17 Mar 2020 17:40:58 +0200
In-Reply-To: <ce990869ebf0478d98cd7e8416b36289@realtek.com> (Tony Chuang's
        message of "Tue, 17 Mar 2020 10:32:58 +0000")
Message-ID: <875zf3kn05.fsf@kamboji.qca.qualcomm.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Tony Chuang <yhchuang@realtek.com> writes:

> // 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.

Either use an existing nl80211 command or add a new one if needed. For
me most important is that we don't add hacks to debugfs just for
avoiding using nl80211.

> 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).

_Why_ is NL80211_CMD_SET_TX_BITRATE_MASK not suitable for you? You keep
saying that but I have still figured out why exactly you think so.
Please clarify this in detail. 

> 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.

What kind of great impact are you talking about? Please be specific so
that we don't need to guess.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches