Re: [PATCH 13/19] rtw89: extend role_maintain to support AP mode

[Kalle Valo <kvalo@kernel.org>]

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

> Fill mac_id and self_role depends on the operation mode.
>
> In AP mode, echo connected station has an unique mac_id, and each vif also
> has one mac_id to represent itself.
>
> The self_role is assigned to vif if the operation mode is decided, and
> RTW89_SELF_ROLE_AP_CLIENT is assigned to the connected STA in AP mode,
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
>  drivers/net/wireless/realtek/rtw89/fw.c  | 8 ++++++--
>  drivers/net/wireless/realtek/rtw89/fw.h  | 1 +
>  drivers/net/wireless/realtek/rtw89/mac.c | 4 ++--
>  3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
> index 5209813275676..4641aadea0386 100644
> --- a/drivers/net/wireless/realtek/rtw89/fw.c
> +++ b/drivers/net/wireless/realtek/rtw89/fw.c
> @@ -993,9 +993,13 @@ int rtw89_fw_h2c_update_beacon(struct rtw89_dev *rtwdev,
>  #define H2C_ROLE_MAINTAIN_LEN 4
>  int rtw89_fw_h2c_role_maintain(struct rtw89_dev *rtwdev,
>  			       struct rtw89_vif *rtwvif,
> +			       struct rtw89_sta *rtwsta,
>  			       enum rtw89_upd_mode upd_mode)
>  {
>  	struct sk_buff *skb;
> +	u8 mac_id = rtwsta ? rtwsta->mac_id : rtwvif->mac_id;
> +	u8 self_role = rtwvif->net_type == RTW89_NET_TYPE_AP_MODE && rtwsta ?
> +		       RTW89_SELF_ROLE_AP_CLIENT : rtwvif->self_role;

It seems you like '?' operator more than I do, and it's ok to use in
simple cases. But the latter statement is not really readable, something
like this is so much easier to read:

if (rtwvif->net_type == RTW89_NET_TYPE_AP_MODE && rtwsta)
    self_role = RTW89_SELF_ROLE_AP_CLIENT
else
    self_role = rtwvif->self_role;

But should there a parenthesis around the == operator? I cannot now
recall what's the preference in the kernel, can someone help on that?

Maybe I also move check for rtwsta first?

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

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