| Message ID | 20230914040525.1170102-1-yunchuan@nfschina.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [wireless-next,1/9] wifi: ar5523: Remove unnecessary (void*) conversions | expand |
On 9/13/2023 9:05 PM, Wu Yunchuan wrote: > No need cast (void*) to (struct hal_rx_ppdu_end_user_stats *), > (struct ath12k_rx_desc_info *) or (struct hal_tx_msdu_ext_desc *). > > Signed-off-by: Wu Yunchuan <yunchuan@nfschina.com> > --- > drivers/net/wireless/ath/ath12k/dp_mon.c | 6 ++---- > drivers/net/wireless/ath/ath12k/dp_rx.c | 2 +- > drivers/net/wireless/ath/ath12k/dp_tx.c | 2 +- > 3 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath12k/dp_mon.c b/drivers/net/wireless/ath/ath12k/dp_mon.c > index f1e57e98bdc6..41cfe7bd865f 100644 > --- a/drivers/net/wireless/ath/ath12k/dp_mon.c > +++ b/drivers/net/wireless/ath/ath12k/dp_mon.c > @@ -13,8 +13,7 @@ > static void ath12k_dp_mon_rx_handle_ofdma_info(void *rx_tlv, > struct hal_rx_user_status *rx_user_status) > { > - struct hal_rx_ppdu_end_user_stats *ppdu_end_user = > - (struct hal_rx_ppdu_end_user_stats *)rx_tlv; > + struct hal_rx_ppdu_end_user_stats *ppdu_end_user = rx_tlv; > > rx_user_status->ul_ofdma_user_v0_word0 = > __le32_to_cpu(ppdu_end_user->usr_resp_ref); > @@ -26,8 +25,7 @@ static void > ath12k_dp_mon_rx_populate_byte_count(void *rx_tlv, void *ppduinfo, > struct hal_rx_user_status *rx_user_status) > { > - struct hal_rx_ppdu_end_user_stats *ppdu_end_user = > - (struct hal_rx_ppdu_end_user_stats *)rx_tlv; > + struct hal_rx_ppdu_end_user_stats *ppdu_end_user = rx_tlv; > u32 mpdu_ok_byte_count = __le32_to_cpu(ppdu_end_user->mpdu_ok_cnt); > u32 mpdu_err_byte_count = __le32_to_cpu(ppdu_end_user->mpdu_err_cnt); > for both of the above IMO the better solution is to change the prototype to replace void *rx_tlv with struct hal_rx_ppdu_end_user_stats *ppdu_end_user and to remove the local variable further, I think you can add const to that since the TLV is only read, not written this better describes that the function requires a specific flavor of RX TLV rather than handling any RX TLV > diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c > index e6e64d437c47..ff5f66ca7783 100644 > --- a/drivers/net/wireless/ath/ath12k/dp_rx.c > +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c > @@ -3730,7 +3730,7 @@ int ath12k_dp_rx_process_wbm_err(struct ath12k_base *ab, > continue; > } > > - desc_info = (struct ath12k_rx_desc_info *)err_info.rx_desc; > + desc_info = err_info.rx_desc; this is ok > > /* retry manual desc retrieval if hw cc is not done */ > if (!desc_info) { > diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c > index 8874c815d7fa..98ddf46b3bb9 100644 > --- a/drivers/net/wireless/ath/ath12k/dp_tx.c > +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c > @@ -109,7 +109,7 @@ static struct ath12k_tx_desc_info *ath12k_dp_tx_assign_buffer(struct ath12k_dp * > static void ath12k_hal_tx_cmd_ext_desc_setup(struct ath12k_base *ab, void *cmd, This function is only called from one place, and it is already passing a variable that is of type struct hal_tx_msdu_ext_desc * so IMO a better solution is to change the prototype to replace void *cmd with struct hal_tx_msdu_ext_desc *tcl_ext_desc and remove the local variable again this better describes that the function requires a specific payload rather than a generic opaque payload as a general rule, use void * when the payload has some level of opaqueness and use a specific struct * when the payload must be of a specific type > struct hal_tx_info *ti) > { > - struct hal_tx_msdu_ext_desc *tcl_ext_cmd = (struct hal_tx_msdu_ext_desc *)cmd; > + struct hal_tx_msdu_ext_desc *tcl_ext_cmd = cmd; > > tcl_ext_cmd->info0 = le32_encode_bits(ti->paddr, > HAL_TX_MSDU_EXT_INFO0_BUF_PTR_LO);
On 2023/9/15 01:06, Jeff Johnson wrote: > On 9/13/2023 9:05 PM, Wu Yunchuan wrote: >> No need cast (void*) to (struct hal_rx_ppdu_end_user_stats *), >> (struct ath12k_rx_desc_info *) or (struct hal_tx_msdu_ext_desc *). >> >> Signed-off-by: Wu Yunchuan <yunchuan@nfschina.com> >> --- >> drivers/net/wireless/ath/ath12k/dp_mon.c | 6 ++---- >> drivers/net/wireless/ath/ath12k/dp_rx.c | 2 +- >> drivers/net/wireless/ath/ath12k/dp_tx.c | 2 +- >> 3 files changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath12k/dp_mon.c >> b/drivers/net/wireless/ath/ath12k/dp_mon.c >> index f1e57e98bdc6..41cfe7bd865f 100644 >> --- a/drivers/net/wireless/ath/ath12k/dp_mon.c >> +++ b/drivers/net/wireless/ath/ath12k/dp_mon.c >> @@ -13,8 +13,7 @@ >> static void ath12k_dp_mon_rx_handle_ofdma_info(void *rx_tlv, >> struct hal_rx_user_status *rx_user_status) >> { >> - struct hal_rx_ppdu_end_user_stats *ppdu_end_user = >> - (struct hal_rx_ppdu_end_user_stats *)rx_tlv; >> + struct hal_rx_ppdu_end_user_stats *ppdu_end_user = rx_tlv; >> rx_user_status->ul_ofdma_user_v0_word0 = >> __le32_to_cpu(ppdu_end_user->usr_resp_ref); >> @@ -26,8 +25,7 @@ static void >> ath12k_dp_mon_rx_populate_byte_count(void *rx_tlv, void *ppduinfo, >> struct hal_rx_user_status *rx_user_status) >> { >> - struct hal_rx_ppdu_end_user_stats *ppdu_end_user = >> - (struct hal_rx_ppdu_end_user_stats *)rx_tlv; >> + struct hal_rx_ppdu_end_user_stats *ppdu_end_user = rx_tlv; >> u32 mpdu_ok_byte_count = >> __le32_to_cpu(ppdu_end_user->mpdu_ok_cnt); >> u32 mpdu_err_byte_count = >> __le32_to_cpu(ppdu_end_user->mpdu_err_cnt); > > for both of the above IMO the better solution is to change the > prototype to replace void *rx_tlv with struct > hal_rx_ppdu_end_user_stats *ppdu_end_user and to remove the local > variable > > further, I think you can add const to that since the TLV is only read, > not written > > this better describes that the function requires a specific flavor of > RX TLV rather than handling any RX TLV > Hi, Sounds like a good idea, I will check other patches for this situation. Thanks for your suggestions. Wu Yunchuan
diff --git a/drivers/net/wireless/ath/ath12k/dp_mon.c b/drivers/net/wireless/ath/ath12k/dp_mon.c index f1e57e98bdc6..41cfe7bd865f 100644 --- a/drivers/net/wireless/ath/ath12k/dp_mon.c +++ b/drivers/net/wireless/ath/ath12k/dp_mon.c @@ -13,8 +13,7 @@ static void ath12k_dp_mon_rx_handle_ofdma_info(void *rx_tlv, struct hal_rx_user_status *rx_user_status) { - struct hal_rx_ppdu_end_user_stats *ppdu_end_user = - (struct hal_rx_ppdu_end_user_stats *)rx_tlv; + struct hal_rx_ppdu_end_user_stats *ppdu_end_user = rx_tlv; rx_user_status->ul_ofdma_user_v0_word0 = __le32_to_cpu(ppdu_end_user->usr_resp_ref); @@ -26,8 +25,7 @@ static void ath12k_dp_mon_rx_populate_byte_count(void *rx_tlv, void *ppduinfo, struct hal_rx_user_status *rx_user_status) { - struct hal_rx_ppdu_end_user_stats *ppdu_end_user = - (struct hal_rx_ppdu_end_user_stats *)rx_tlv; + struct hal_rx_ppdu_end_user_stats *ppdu_end_user = rx_tlv; u32 mpdu_ok_byte_count = __le32_to_cpu(ppdu_end_user->mpdu_ok_cnt); u32 mpdu_err_byte_count = __le32_to_cpu(ppdu_end_user->mpdu_err_cnt); diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c index e6e64d437c47..ff5f66ca7783 100644 --- a/drivers/net/wireless/ath/ath12k/dp_rx.c +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c @@ -3730,7 +3730,7 @@ int ath12k_dp_rx_process_wbm_err(struct ath12k_base *ab, continue; } - desc_info = (struct ath12k_rx_desc_info *)err_info.rx_desc; + desc_info = err_info.rx_desc; /* retry manual desc retrieval if hw cc is not done */ if (!desc_info) { diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c index 8874c815d7fa..98ddf46b3bb9 100644 --- a/drivers/net/wireless/ath/ath12k/dp_tx.c +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c @@ -109,7 +109,7 @@ static struct ath12k_tx_desc_info *ath12k_dp_tx_assign_buffer(struct ath12k_dp * static void ath12k_hal_tx_cmd_ext_desc_setup(struct ath12k_base *ab, void *cmd, struct hal_tx_info *ti) { - struct hal_tx_msdu_ext_desc *tcl_ext_cmd = (struct hal_tx_msdu_ext_desc *)cmd; + struct hal_tx_msdu_ext_desc *tcl_ext_cmd = cmd; tcl_ext_cmd->info0 = le32_encode_bits(ti->paddr, HAL_TX_MSDU_EXT_INFO0_BUF_PTR_LO);
No need cast (void*) to (struct hal_rx_ppdu_end_user_stats *), (struct ath12k_rx_desc_info *) or (struct hal_tx_msdu_ext_desc *). Signed-off-by: Wu Yunchuan <yunchuan@nfschina.com> --- drivers/net/wireless/ath/ath12k/dp_mon.c | 6 ++---- drivers/net/wireless/ath/ath12k/dp_rx.c | 2 +- drivers/net/wireless/ath/ath12k/dp_tx.c | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-)