ATH11K Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Rajkumar Manoharan <rmanohar@codeaurora.org>
To: John Crispin <john@phrozen.org>
Cc: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>,
	Srini Kode <skode@qti.qualcomm.com>,
	ath11k@lists.infradead.org, Kalle Valo <kvalo@codeaurora.org>,
	Shashidhar Lakkavalli <slakkavalli@datto.com>
Subject: Re: [PATCH V2 4/4] ath11k: add HE support
Date: Mon, 29 Apr 2019 22:41:35 -0700	[thread overview]
Message-ID: <e6083123cf28cb4c48814b45d27d43cc@codeaurora.org> (raw)
In-Reply-To: <20190425181740.1994-5-john@phrozen.org>

On 2019-04-25 11:17, John Crispin wrote:
> Add basic HE support to the driver. The sband_iftype data is generated 
> from
> the capabilities read from the FW.
> 
Nice.. :) Sorry for the late response.
> 
>  static inline enum wmi_phy_mode
> -chan_to_phymode(const struct cfg80211_chan_def *chandef)
> +chan_to_phymode_2ghz(const struct cfg80211_chan_def *chandef)
> +{
> +	enum wmi_phy_mode phymode = MODE_UNKNOWN;
> +
> +	switch (chandef->width) {
> +	case NL80211_CHAN_WIDTH_20_NOHT:
> +		if (chandef->chan->flags & IEEE80211_CHAN_NO_OFDM)
> +			phymode = MODE_11B;
> +		else
> +			phymode = MODE_11G;
> +		break;
> +	case NL80211_CHAN_WIDTH_20:
> +		phymode = MODE_11NG_HT20;
> +		break;
> +	case NL80211_CHAN_WIDTH_40:
> +		phymode = MODE_11NG_HT40;
> +		break;
> +	case NL80211_CHAN_WIDTH_5:
> +	case NL80211_CHAN_WIDTH_10:
> +	case NL80211_CHAN_WIDTH_80:
> +	case NL80211_CHAN_WIDTH_80P80:
> +	case NL80211_CHAN_WIDTH_160:
> +		phymode = MODE_UNKNOWN;
> +		break;
> +	}
> +
> +	return phymode;
> +}
> +
> +static inline enum wmi_phy_mode
> +chan_to_phymode_5ghz(const struct cfg80211_chan_def *chandef)
> +{
> +	enum wmi_phy_mode phymode = MODE_UNKNOWN;
> +
> +	switch (chandef->width) {
> +	case NL80211_CHAN_WIDTH_20_NOHT:
> +		phymode = MODE_11A;
> +		break;
> +	case NL80211_CHAN_WIDTH_20:
> +		phymode = MODE_11AC_VHT20;
> +		break;
> +	case NL80211_CHAN_WIDTH_40:
> +		phymode = MODE_11AC_VHT40;
> +		break;
> +	case NL80211_CHAN_WIDTH_80:
> +		phymode = MODE_11AC_VHT80;
> +		break;
> +	case NL80211_CHAN_WIDTH_160:
> +		phymode = MODE_11AC_VHT160;
> +		break;
> +	case NL80211_CHAN_WIDTH_80P80:
> +		phymode = MODE_11AC_VHT80_80;
> +		break;
> +	case NL80211_CHAN_WIDTH_5:
> +	case NL80211_CHAN_WIDTH_10:
> +		phymode = MODE_UNKNOWN;
> +		break;
> +	}
> +
> +	return phymode;
> +}
> +
> +static inline enum wmi_phy_mode
> +chan_to_phymode_2ghz_he(const struct cfg80211_chan_def *chandef)
> +{
> +	enum wmi_phy_mode phymode = MODE_UNKNOWN;
> +
> +	switch (chandef->width) {
> +	case NL80211_CHAN_WIDTH_20_NOHT:
> +		if (chandef->chan->flags & IEEE80211_CHAN_NO_OFDM)
> +			phymode = MODE_11B;
> +		else
> +			phymode = MODE_11G;
> +		break;
> +	case NL80211_CHAN_WIDTH_20:
> +		phymode = MODE_11AX_HE20_2G;
> +		break;
> +	case NL80211_CHAN_WIDTH_40:
> +		phymode = MODE_11AX_HE40_2G;
> +		break;
> +	case NL80211_CHAN_WIDTH_80:
> +		phymode = MODE_11AX_HE80_2G;
> +		break;
> +	case NL80211_CHAN_WIDTH_5:
> +	case NL80211_CHAN_WIDTH_10:
> +	case NL80211_CHAN_WIDTH_80P80:
> +	case NL80211_CHAN_WIDTH_160:
> +		phymode = MODE_UNKNOWN;
> +		break;
> +	}
> +
> +	return phymode;
> +}
> +
> +static inline enum wmi_phy_mode
> +chan_to_phymode_5ghz_he(const struct cfg80211_chan_def *chandef)
> +{
> +	enum wmi_phy_mode phymode = MODE_UNKNOWN;
> +
> +	switch (chandef->width) {
> +	case NL80211_CHAN_WIDTH_20_NOHT:
> +		phymode = MODE_11A;
> +		break;
> +	case NL80211_CHAN_WIDTH_20:
> +		phymode = MODE_11AX_HE20;
> +		break;
> +	case NL80211_CHAN_WIDTH_40:
> +		phymode = MODE_11AX_HE40;
> +		break;
> +	case NL80211_CHAN_WIDTH_80:
> +		phymode = MODE_11AX_HE80;
> +		break;
> +	case NL80211_CHAN_WIDTH_160:
> +		phymode = MODE_11AX_HE160;
> +		break;
> +	case NL80211_CHAN_WIDTH_80P80:
> +		phymode = MODE_11AX_HE80_80;
> +		break;
> +	case NL80211_CHAN_WIDTH_5:
> +	case NL80211_CHAN_WIDTH_10:
> +		phymode = MODE_UNKNOWN;
> +		break;
> +	}
> +
> +	return phymode;
> +}
> +
> +static inline enum wmi_phy_mode
> +chan_to_phymode(const struct cfg80211_chan_def *chandef, int 
> he_support)
>  {
>  	enum wmi_phy_mode phymode = MODE_UNKNOWN;
> 
>  	switch (chandef->chan->band) {
>  	case NL80211_BAND_2GHZ:
> -		switch (chandef->width) {
> -		case NL80211_CHAN_WIDTH_20_NOHT:
> -			if (chandef->chan->flags & IEEE80211_CHAN_NO_OFDM)
> -				phymode = MODE_11B;
> -			else
> -				phymode = MODE_11G;
> -			break;
> -		case NL80211_CHAN_WIDTH_20:
> -			phymode = MODE_11NG_HT20;
> -			break;
> -		case NL80211_CHAN_WIDTH_40:
> -			phymode = MODE_11NG_HT40;
> -			break;
> -		case NL80211_CHAN_WIDTH_5:
> -		case NL80211_CHAN_WIDTH_10:
> -		case NL80211_CHAN_WIDTH_80:
> -		case NL80211_CHAN_WIDTH_80P80:
> -		case NL80211_CHAN_WIDTH_160:
> -			phymode = MODE_UNKNOWN;
> -			break;
> -		}
> +		if (he_support)
> +			phymode = chan_to_phymode_2ghz_he(chandef);
> +		else
> +			phymode = chan_to_phymode_2ghz(chandef);
>  		break;
>  	case NL80211_BAND_5GHZ:
> -		switch (chandef->width) {
> -		case NL80211_CHAN_WIDTH_20_NOHT:
> -			phymode = MODE_11A;
> -			break;
> -		case NL80211_CHAN_WIDTH_20:
> -			phymode = MODE_11AC_VHT20;
> -			break;
> -		case NL80211_CHAN_WIDTH_40:
> -			phymode = MODE_11AC_VHT40;
> -			break;
> -		case NL80211_CHAN_WIDTH_80:
> -			phymode = MODE_11AC_VHT80;
> -			break;
> -		case NL80211_CHAN_WIDTH_160:
> -			phymode = MODE_11AC_VHT160;
> -			break;
> -		case NL80211_CHAN_WIDTH_80P80:
> -			phymode = MODE_11AC_VHT80_80;
> -			break;
> -		case NL80211_CHAN_WIDTH_5:
> -		case NL80211_CHAN_WIDTH_10:
> -			phymode = MODE_UNKNOWN;
> -			break;
> -		}
> +		if (he_support)
> +			phymode = chan_to_phymode_5ghz_he(chandef);
> +		else
> +			phymode = chan_to_phymode_5ghz(chandef);
> +
>  		break;
> 
The above repeated switch case routines can be simplified by simple 
array of modes. no?

               WIDTH_20_NOHT |   WIDTH_20    | WIDTH_40     | WIDTH_80    
   | WIDTH_160        | WIDTH_80P80

MODE_11G            11G       11NG_HT20       11NG_HT40       NA         
          NA            NA
MODE_11G_HE         11G       11AX_HE20_2G    11AX_HE40_2G   
11AX_HE80_2G         NA            NA
MODE_11A            11A       11NA_HT20       11NA_HT40      NA          
          NA            NA
MODE_11A_VHT        11A       11AC_VHT20      11AC_VHT40     11AC_VHT80  
          11AC_VHT160   11AC_VHT80_80
MODE_11A_HE         11A       11AX_HE20       11AX_HE40      11AX_HE80   
          11AX_HE160    11AX_HE80_80

[...]

> +static enum wmi_phy_mode ath11k_mac_get_phymode_he(struct ath11k *ar,
> +						   struct ieee80211_sta *sta)
> +{
> +	if (sta->bandwidth == IEEE80211_STA_RX_BW_160) {
> +		if (sta->he_cap.he_cap_elem.phy_cap_info[0] &
> +		     IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G)
> +			return MODE_11AX_HE160;
> +		else if (sta->he_cap.he_cap_elem.phy_cap_info[0] &
> +		     IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G)
> +			return MODE_11AX_HE80_80;
> +		/* not sure if this is a valid case? */
> +		return MODE_11AX_HE160;
> +	}
> +
> +	if (sta->bandwidth == IEEE80211_STA_RX_BW_80)
> +		return MODE_11AX_HE80;
> +
> +	if (sta->bandwidth == IEEE80211_STA_RX_BW_40)
> +		return MODE_11AX_HE40;
> +
> +	if (sta->bandwidth == IEEE80211_STA_RX_BW_20)
> +		return MODE_11AX_HE20;
> +
> +	return MODE_UNKNOWN;
> +}
> +

I think this function also be removed if we have MODE table as mentioned 
above.

[...]

> +static void ath11k_mac_copy_he_cap(struct ath11k_pdev_cap *cap,
> +				   struct ieee80211_sband_iftype_data *data,
> +				   int band)
> +{
> +	int i;
> +
> +	for (i = 0; i < 2; i++) {
> +		struct ieee80211_sta_he_cap *he_cap = &data[i].he_cap;
> +		struct ath11k_band_cap *band_cap = &cap->band[band];
> +		struct ieee80211_he_cap_elem *he_cap_elem =
> +				&he_cap->he_cap_elem;
> +
> +		he_cap->has_he = true;
> +		memcpy(he_cap_elem->mac_cap_info, band_cap->he_cap_info,
> +		       sizeof(he_cap_elem->mac_cap_info));
> +		memcpy(he_cap_elem->phy_cap_info, band_cap->he_cap_phy_info,
> +		       sizeof(he_cap_elem->phy_cap_info));
> +
> +		if (i) {
> +			data[i].types_mask = BIT(NL80211_IFTYPE_STATION);
> +			he_cap_elem->mac_cap_info[0] &=
> +				~IEEE80211_HE_MAC_CAP0_TWT_RES;
> +			he_cap_elem->mac_cap_info[0] |=
> +				IEEE80211_HE_MAC_CAP0_TWT_REQ;
> +			/* TODO - add further differences between AP and STA */
> +		} else {
> +			data[i].types_mask = BIT(NL80211_IFTYPE_AP);
> +		}
> 
Hmmm.. If you are running loop to extract he_cap, he_cap_extn, how will 
you extend HE support
for other interface types (Mesh)?

-Rajkumar

_______________________________________________
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

  reply	other threads:[~2019-04-30  5:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-25 18:17 [PATCH V2 0/4] ath11k: add HE support John Crispin
2019-04-25 18:17 ` [PATCH V2 1/4] ath11k: add HE handling to the debug code John Crispin
2019-04-25 18:17 ` [PATCH V2 2/4] ath11k: extend reading of FW capabilities John Crispin
2019-04-25 18:17 ` [PATCH V2 3/4] ath11k: handle rx status for HE frames John Crispin
2019-04-25 18:17 ` [PATCH V2 4/4] ath11k: add HE support John Crispin
2019-04-30  5:41   ` Rajkumar Manoharan [this message]
2019-04-30  7:05     ` John Crispin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e6083123cf28cb4c48814b45d27d43cc@codeaurora.org \
    --to=rmanohar@codeaurora.org \
    --cc=ath11k@lists.infradead.org \
    --cc=john@phrozen.org \
    --cc=kvalo@codeaurora.org \
    --cc=rmanohar@qti.qualcomm.com \
    --cc=skode@qti.qualcomm.com \
    --cc=slakkavalli@datto.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox