2017-11-20 12:54:32

by Venkat Prashanth B U

Subject: [PATCH v1] rtlwifi:rtl8723ae: Fix embedded function names with __func__

From: Venkat Prashanth B U <[email protected]>

Prefer and make it generic by using %s and __func__ to print functions name
instead of embedding functions name in print statements

Signed-off-by: Venkat Prashanth B U <[email protected]>

---
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
index f713c72..bd56bb6 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
@@ -389,7 +389,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,

if (pci_dma_mapping_error(rtlpci->pdev, mapping)) {
RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
- "DMA mapping error\n");
+ "%s():DMA Mapping Error", __func__);
return;
}
if (mac->opmode == NL80211_IFTYPE_STATION) {
@@ -498,7 +498,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
if (ieee80211_is_data_qos(fc)) {
if (mac->rdg_en) {
RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
- "Enable RDG function.\n");
+ "%s():Enable RDG function.\n", __func__);
SET_TX_DESC_RDG_ENABLE(pdesc, 1);
SET_TX_DESC_HTC(pdesc, 1);
}
@@ -537,7 +537,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
SET_TX_DESC_BMC(pdesc, 1);
}

- RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE, "\n");
+ RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE, "%s():\n", __func__);
}

void rtl8723e_tx_fill_cmddesc(struct ieee80211_hw *hw,
@@ -557,7 +557,7 @@ void rtl8723e_tx_fill_cmddesc(struct ieee80211_hw *hw,

if (pci_dma_mapping_error(rtlpci->pdev, mapping)) {
RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
- "DMA mapping error\n");
+ "%s():DMA Mapping Error", __func__);
return;
}
CLEAR_PCI_TX_DESC_CONTENT(pdesc, TX_DESC_SIZE);
--
1.9.1

2017-11-20 17:04:19

by Larry Finger

Subject: Re: [PATCH v1] rtlwifi:rtl8723ae: Fix embedded function names with __func__

On 11/20/2017 06:53 AM, [email protected] wrote:
> From: Venkat Prashanth B U <[email protected]>
>
> Prefer and make it generic by using %s and __func__ to print functions name
> instead of embedding functions name in print statements
>
> Signed-off-by: Venkat Prashanth B U <[email protected]>

What changed between the first one you sent, and the second that you labeled
"V1". The reviewers should not have to discover that for themselves.

Certainly adding the function name to the printouts might be useful; however,
your commit message says "instead of embedding functions name". That is
obviously not true. None of the changes ever included the function name in the
message.

There are other comments shown below.

>
> ---
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
> index f713c72..bd56bb6 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/trx.c
> @@ -389,7 +389,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
>
> if (pci_dma_mapping_error(rtlpci->pdev, mapping)) {
> RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
> - "DMA mapping error\n");
> + "%s():DMA Mapping Error", __func__);
> return;
> }
> if (mac->opmode == NL80211_IFTYPE_STATION) {
> @@ -498,7 +498,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
> if (ieee80211_is_data_qos(fc)) {
> if (mac->rdg_en) {
> RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
> - "Enable RDG function.\n");
> + "%s():Enable RDG function.\n", __func__);

The above line fails to align the continuation statement. Always run
scripts/checkpatch.pl on all your submissions.

> SET_TX_DESC_RDG_ENABLE(pdesc, 1);
> SET_TX_DESC_HTC(pdesc, 1);
> }
> @@ -537,7 +537,7 @@ void rtl8723e_tx_fill_desc(struct ieee80211_hw *hw,
> SET_TX_DESC_BMC(pdesc, 1);
> }
>
> - RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE, "\n");
> + RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE, "%s():\n", __func__);

This one is NOT an error condition. Why would the function name be needed here?
If the user has DBG_TRACE turned on, then the function where a COMP_SEND message
is emitted will be known. The code is bloated enough already.

> }
>
> void rtl8723e_tx_fill_cmddesc(struct ieee80211_hw *hw,
> @@ -557,7 +557,7 @@ void rtl8723e_tx_fill_cmddesc(struct ieee80211_hw *hw,
>
> if (pci_dma_mapping_error(rtlpci->pdev, mapping)) {
> RT_TRACE(rtlpriv, COMP_SEND, DBG_TRACE,
> - "DMA mapping error\n");
> + "%s():DMA Mapping Error", __func__);
> return;
> }
> CLEAR_PCI_TX_DESC_CONTENT(pdesc, TX_DESC_SIZE);
>

The patch needs more work. NACK.

Larry