Message ID | 20200429095656.19315-6-yhchuang@realtek.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Kalle Valo |
Headers | show |
Series | rtw88: 8723d: add BB related routines | expand |
On 2020-04-29 17:56:53 [+0800], yhchuang@realtek.com wrote: > --- a/drivers/net/wireless/realtek/rtw88/main.c > +++ b/drivers/net/wireless/realtek/rtw88/main.c > @@ -933,7 +933,7 @@ static void rtw_init_ht_cap(struct rtw_dev *rtwdev, > ht_cap->cap = 0; > ht_cap->cap |= IEEE80211_HT_CAP_SGI_20 | > IEEE80211_HT_CAP_MAX_AMSDU | > - IEEE80211_HT_CAP_LDPC_CODING | > + (rtw_chip_wcpu_11ac(rtwdev) ? IEEE80211_HT_CAP_LDPC_CODING : 0) | > (1 << IEEE80211_HT_CAP_RX_STBC_SHIFT); What about ht_cap->cap = IEEE80211_HT_CAP_SGI_20 | IEEE80211_HT_CAP_MAX_AMSDU | (1 << IEEE80211_HT_CAP_RX_STBC_SHIFT); if (rtw_chip_wcpu_11ac(rtwdev)) ht_cap->cap |= IEEE80211_HT_CAP_LDPC_CODING; instead? > if (efuse->hw_cap.bw & BIT(RTW_CHANNEL_WIDTH_40)) > ht_cap->cap |= IEEE80211_HT_CAP_SUP_WIDTH_20_40 | Sebastian
Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes: > On 2020-04-29 17:56:53 [+0800], yhchuang@realtek.com wrote: >> --- a/drivers/net/wireless/realtek/rtw88/main.c >> +++ b/drivers/net/wireless/realtek/rtw88/main.c >> @@ -933,7 +933,7 @@ static void rtw_init_ht_cap(struct rtw_dev *rtwdev, >> ht_cap->cap = 0; >> ht_cap->cap |= IEEE80211_HT_CAP_SGI_20 | >> IEEE80211_HT_CAP_MAX_AMSDU | >> - IEEE80211_HT_CAP_LDPC_CODING | >> + (rtw_chip_wcpu_11ac(rtwdev) ? IEEE80211_HT_CAP_LDPC_CODING : 0) | >> (1 << IEEE80211_HT_CAP_RX_STBC_SHIFT); > > What about > > ht_cap->cap = IEEE80211_HT_CAP_SGI_20 | > IEEE80211_HT_CAP_MAX_AMSDU | > (1 << IEEE80211_HT_CAP_RX_STBC_SHIFT); > if (rtw_chip_wcpu_11ac(rtwdev)) > ht_cap->cap |= IEEE80211_HT_CAP_LDPC_CODING; > instead? Yes, that's much better. I even missed the '?' operator in my own review as it was not that visible.
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes: > > > On 2020-04-29 17:56:53 [+0800], yhchuang@realtek.com wrote: > >> --- a/drivers/net/wireless/realtek/rtw88/main.c > >> +++ b/drivers/net/wireless/realtek/rtw88/main.c > >> @@ -933,7 +933,7 @@ static void rtw_init_ht_cap(struct rtw_dev > *rtwdev, > >> ht_cap->cap = 0; > >> ht_cap->cap |= IEEE80211_HT_CAP_SGI_20 | > >> IEEE80211_HT_CAP_MAX_AMSDU | > >> - IEEE80211_HT_CAP_LDPC_CODING | > >> + (rtw_chip_wcpu_11ac(rtwdev) ? > IEEE80211_HT_CAP_LDPC_CODING : 0) | > >> (1 << IEEE80211_HT_CAP_RX_STBC_SHIFT); > > > > What about > > > > ht_cap->cap = IEEE80211_HT_CAP_SGI_20 | > > IEEE80211_HT_CAP_MAX_AMSDU | > > (1 << IEEE80211_HT_CAP_RX_STBC_SHIFT); > > if (rtw_chip_wcpu_11ac(rtwdev)) > > ht_cap->cap |= IEEE80211_HT_CAP_LDPC_CODING; > > instead? > > Yes, that's much better. I even missed the '?' operator in my own review > as it was not that visible. > Will fix that in v4, thanks. Yen-Hsuan
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c index b0dadff0dc7b..21371576a6b2 100644 --- a/drivers/net/wireless/realtek/rtw88/main.c +++ b/drivers/net/wireless/realtek/rtw88/main.c @@ -933,7 +933,7 @@ static void rtw_init_ht_cap(struct rtw_dev *rtwdev, ht_cap->cap = 0; ht_cap->cap |= IEEE80211_HT_CAP_SGI_20 | IEEE80211_HT_CAP_MAX_AMSDU | - IEEE80211_HT_CAP_LDPC_CODING | + (rtw_chip_wcpu_11ac(rtwdev) ? IEEE80211_HT_CAP_LDPC_CODING : 0) | (1 << IEEE80211_HT_CAP_RX_STBC_SHIFT); if (efuse->hw_cap.bw & BIT(RTW_CHANNEL_WIDTH_40)) ht_cap->cap |= IEEE80211_HT_CAP_SUP_WIDTH_20_40 |