All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.