Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220309195759.1494-1-jelonek.jonas@gmail.com>
 <20220309195759.1494-2-jelonek.jonas@gmail.com> <bc398c48-b5b2-8037-a8c6-712b2e191202@candelatech.com>
 <8FC6B734-4D4A-4D91-A38E-DE36E415A36E@inet.tu-berlin.de> <22a43538-a2ad-f3a7-2909-5ef6a0d2ab27@candelatech.com>
In-Reply-To: <22a43538-a2ad-f3a7-2909-5ef6a0d2ab27@candelatech.com>
From:   Jonas Jelonek <jelonek.jonas@gmail.com>
Date:   Thu, 10 Mar 2022 18:27:55 +0100
Message-ID: <CAChE-vRdnLgjGt3wDrzDC_eKWnX-wi_bWAiBYcheZrSMcfDoyQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] mac80211: extend current rate control tx status API
To:     Ben Greear <greearb@candelatech.com>
Cc:     =?UTF-8?B?VGhvbWFzIEjDvGhu?= <thomas@inet.tu-berlin.de>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Kalle Valo <kvalo@kernel.org>, "nbd@nbd.name" <nbd@nbd.name>,
        johannes@sipsolutions.net
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On 3/10/22 16:43 UTC, Ben Greear wrote:
> >
> > Certain 802.11a/g/n Atheros chips provide a 0,5dB tx-power step granula=
rity, while Mediatek 802.11ac chips have 1dB or even 3dB step width. So I w=
ould argue that a 1dB step width is a good compromise as the common value f=
or new tpc algorithms.
>
> If you use 0.5db units for that struct, then it will support anything wit=
h that granularity or higher.
> But, fine with me if you want to just have it be 1db units.
>
using 0.5db is more appropriate for the already existing chips that
support this granularity, and is more future-proof.
1db units may be easier to handle for the API and/or TPC algorithms
but again limits existing hardware capabilities.

> > The ath9k chips I have used so far support a minimum tx-power of -5dBm =
(=3D0,32mW), Mediatek has a min of 0dBm (=3D1mW)=E2=80=A6 so I would argue =
to use 0dBm  (=3D1mW) as common minimum tx-power value, as the any possible=
 spatial reuse gain happens from 0dBm up to max tx-power.
>
> If a chip supports setting to txpower to -5, then why not let the API sup=
port that?  Have The value -128
> be 'do not set', and anything else will mean 'try to set the chip to this=
 power or the nearest thing to it
> that the chip supports'.

I agree with that, having -128 as value for 'not set' or 'invalid'
would leave the negative dBm for chips that support this.
Whether the TPC then actually makes use of this should not be the
reason to use 0 as default.

To your previous question:
retry_count =3D 1 is intended to be a single tx, so naming the struct
member 'try_count' would be more appropriate?

Besides this, I will add proper documentation for this in the
following patch version to clarify the units and meanings.