Re: [PATCH v6 01/24] rtw89: add CAM files

[Kalle Valo <kvalo@codeaurora.org>]

Ping-Ke Shih <pkshih@realtek.com> writes:

> Three kinds of CAM, security, address and bssid, are implemented in the
> chip, and then we control them via H2C commands.
>
> Every vif contains one address and one bssid CAM entries, and both are
> allocated while vif is adding. Once any state is changed, driver notices
> firmware. For example, add/remove vif, sta assoc/disassoc, and changed
> security key.
>
> When connecting to an AP with security, a security CAM entry is allocated
> from security CAM pool and filled key values, and then attached to the
> address CAM that contains seven keys entries for unicast, group and BIP
> keys. Since the functions of these seven keys are predefined by firmware,
> we should attach key to proper position depends on security entry mode and
> key type.
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

[...]

> +int rtw89_cam_sec_key_add(struct rtw89_dev *rtwdev,
> +			  struct ieee80211_vif *vif,
> +			  struct ieee80211_sta *sta,
> +			  struct ieee80211_key_conf *key)
> +{
> +	u8 hw_key_type;
> +	bool ext_key = false;
> +	int ret;
> +
> +	switch (key->cipher) {
> +	case WLAN_CIPHER_SUITE_WEP40:
> +		hw_key_type = RTW89_SEC_KEY_TYPE_WEP40;
> +		break;
> +	case WLAN_CIPHER_SUITE_WEP104:
> +		hw_key_type = RTW89_SEC_KEY_TYPE_WEP104;
> +		break;
> +	case WLAN_CIPHER_SUITE_CCMP:
> +		hw_key_type = RTW89_SEC_KEY_TYPE_CCMP128;
> +		key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
> +		break;
> +	case WLAN_CIPHER_SUITE_CCMP_256:
> +		hw_key_type = RTW89_SEC_KEY_TYPE_CCMP256;
> +		key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
> +		ext_key = true;
> +		break;
> +	case WLAN_CIPHER_SUITE_GCMP:
> +		hw_key_type = RTW89_SEC_KEY_TYPE_GCMP128;
> +		key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
> +		break;
> +	case WLAN_CIPHER_SUITE_GCMP_256:
> +		hw_key_type = RTW89_SEC_KEY_TYPE_GCMP256;
> +		key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
> +		ext_key = true;
> +		break;
> +	case WLAN_CIPHER_SUITE_TKIP:
> +	case WLAN_CIPHER_SUITE_AES_CMAC:
> +	case WLAN_CIPHER_SUITE_BIP_CMAC_256:
> +	case WLAN_CIPHER_SUITE_BIP_GMAC_128:
> +	case WLAN_CIPHER_SUITE_BIP_GMAC_256:
> +		/* suppress error messages */

What error messages? I don't understand the comment.

> +		return -EOPNOTSUPP;
> +	default:
> +		return -ENOTSUPP;

I would merge default with other cases above and have just one return
call.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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