Re: [PATCH 01/10] carl9170: mac80211: enable Short Guard Interval for 20 MHz

[Christian Lamparter <chunkeey@gmail.com>]

On 3/12/26 11:37 AM, Masi Osmani wrote:
> The AR9170 hardware supports Short Guard Interval (400ns) for both
> 20 MHz and 40 MHz channel widths.  SGI_40 was already advertised in
> the HT capabilities, but SGI_20 was missing.  This reduces the OFDM
> symbol duration from 800ns to 400ns on 20 MHz channels, increasing
> the maximum PHY rate from 130 Mbps to 144.4 Mbps (MCS 15, 2SS).
> 
> ath9k (the PCI sibling for the same AR9xxx chipset family) has
> always advertised both SGI_20 and SGI_40.
> 

Are you really, really sure about that?

Because based on what's in ath9k:
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ath/ath9k/hw.c?h=v7.0-rc4#n2622>

|        if (AR_SREV_9287_11_OR_LATER(ah) || AR_SREV_9271(ah))
|                pCap->hw_caps |= ATH9K_HW_CAP_SGI_20;

Whereas AR_SREV_9287_11_OR_LATER and AR_SREV_9271 are defined in reg.h @
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ath/ath9k/reg.h?h=v7.0-rc4#n861>

|#define AR_SREV_9287_11_OR_LATER(_ah) \
|        (((_ah)->hw_version.macVersion >= AR_SREV_VERSION_9287))
| [...]
|#define AR_SREV_9271(_ah) \
|    (((_ah))->hw_version.macVersion == AR_SREV_VERSION_9271)

What's strange here is that this isn't a PHY check, but MAC check...
And the AR9170 uses a modified ZyDAS MAC which is completely different
than what any of the ath9k siblings uses.

So based on that: No. I can't ACK that. The feature might work or not,
but that commit message is wrong.

That said, if you say have been successfully been using this and rewrite the
commit message to not include wrong information and add a module_parameter
like experimental that enables it, this would be OK!

Cheers,
Christian


> Signed-off-by: Masi Osmani <mas-i@hotmail.de>
> ---
>   drivers/net/wireless/ath/carl9170/main.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
> index a7a9345..aa7e481 100644
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -154,6 +154,7 @@ static struct ieee80211_channel carl9170_5ghz_chantable[] = {
>   	.cap		= IEEE80211_HT_CAP_MAX_AMSDU |			\
>   			  IEEE80211_HT_CAP_SUP_WIDTH_20_40 |		\
>   			  IEEE80211_HT_CAP_SGI_40 |			\
> +			  IEEE80211_HT_CAP_SGI_20 |			\
>   			  IEEE80211_HT_CAP_DSSSCCK40 |			\
>   			  IEEE80211_HT_CAP_SM_PS,			\
>   	.ampdu_factor	= IEEE80211_HT_MAX_AMPDU_64K,			\