| Message ID | 1511182437-5870-1-git-send-email-venkat.prashanth2498@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 11/20/2017 06:53 AM, venkat.prashanth2498@gmail.com wrote: > From: Venkat Prashanth B U <venkat.prashanth2498@gmail.com> > > 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 <venkat.prashanth2498@gmail.com> 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
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);