diff mbox series

[2/6] rtlwifi: Remove unnecessary parenthese in rtl_dbg uses

Message ID 9b2eaedb7ea123ea766a379459b20a9486d1cd41.1595706420.git.joe@perches.com (mailing list archive)
State Rejected
Delegated to: Kalle Valo
Headers show
Series rtlwifi: Convert RT_TRACE to rtl_dbg and neatening | expand

Commit Message

Joe Perches July 25, 2020, 7:55 p.m. UTC
Make these statements a little simpler.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/wireless/realtek/rtlwifi/base.c   | 14 +++++------
 .../rtlwifi/btcoexist/halbtc8192e2ant.c       | 23 ++++++++++---------
 .../rtlwifi/btcoexist/halbtc8821a2ant.c       | 12 +++++-----
 .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c  |  9 ++++----
 drivers/net/wireless/realtek/rtlwifi/pci.c    |  2 +-
 5 files changed, 30 insertions(+), 30 deletions(-)

Comments

Ping-Ke Shih July 27, 2020, 6:07 a.m. UTC | #1
On Sat, 2020-07-25 at 12:55 -0700, Joe Perches wrote:
> Make these statements a little simpler.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/net/wireless/realtek/rtlwifi/base.c   | 14 +++++------
>  .../rtlwifi/btcoexist/halbtc8192e2ant.c       | 23 ++++++++++---------
>  .../rtlwifi/btcoexist/halbtc8821a2ant.c       | 12 +++++-----
>  .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c  |  9 ++++----
>  drivers/net/wireless/realtek/rtlwifi/pci.c    |  2 +-
>  5 files changed, 30 insertions(+), 30 deletions(-)
> 
> 

[...]

> diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> index 8d28c68f083e..f9a2d8a6730c 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -874,11 +874,10 @@ static void halbtc_display_wifi_status(struct
> btc_coexist *btcoexist,
>  	seq_printf(m, "\n %-35s = %s / %s/ %s/ AP=%d ",
>  		   "Wifi freq/ bw/ traffic",
>  		   gl_btc_wifi_freq_string[wifi_freq],
> -		   ((wifi_under_b_mode) ? "11b" :
> -		    gl_btc_wifi_bw_string[wifi_bw]),
> -		   ((!wifi_busy) ? "idle" : ((BTC_WIFI_TRAFFIC_TX ==
> -					      wifi_traffic_dir) ? "uplink" :
> -					     "downlink")),
> +		   wifi_under_b_mode ? "11b" : gl_btc_wifi_bw_string[wifi_bw],
> +		   (!wifi_busy ? "idle" :
> +		    wifi_traffic_dir == BTC_WIFI_TRAFFIC_TX ? "uplink" :
> +		    "downlink"),

I think this would be better

+		   !wifi_busy ? "idle" :
+		   (wifi_traffic_dir == BTC_WIFI_TRAFFIC_TX ? "uplink" :
+		    "downlink"),



>  		   ap_num);
>  
>  	/* power status	 */
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c
> b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index 1d0af72ee780..3189d1c50d52 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -557,7 +557,7 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int
> prio)
>  		if (rtlpriv->rtlhal.earlymode_enable)
>  			skb_pull(skb, EM_HDR_LEN);
>  
> -		rtl_dbg(rtlpriv, (COMP_INTR | COMP_SEND), DBG_TRACE,
> +		rtl_dbg(rtlpriv, COMP_INTR | COMP_SEND, DBG_TRACE,
>  			"new ring->idx:%d, free: skb_queue_len:%d, free:
> seq:%x\n",
>  			ring->idx,
>  			skb_queue_len(&ring->queue),
Joe Perches July 27, 2020, 8:27 a.m. UTC | #2
On Mon, 2020-07-27 at 06:07 +0000, Pkshih wrote:
> On Sat, 2020-07-25 at 12:55 -0700, Joe Perches wrote:
> > Make these statements a little simpler.
[]
> > diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
[]
> > @@ -874,11 +874,10 @@ static void halbtc_display_wifi_status(struct
> > btc_coexist *btcoexist,
> >  	seq_printf(m, "\n %-35s = %s / %s/ %s/ AP=%d ",
> >  		   "Wifi freq/ bw/ traffic",
> >  		   gl_btc_wifi_freq_string[wifi_freq],
> > -		   ((wifi_under_b_mode) ? "11b" :
> > -		    gl_btc_wifi_bw_string[wifi_bw]),
> > -		   ((!wifi_busy) ? "idle" : ((BTC_WIFI_TRAFFIC_TX ==
> > -					      wifi_traffic_dir) ? "uplink" :
> > -					     "downlink")),
> > +		   wifi_under_b_mode ? "11b" : gl_btc_wifi_bw_string[wifi_bw],
> > +		   (!wifi_busy ? "idle" :
> > +		    wifi_traffic_dir == BTC_WIFI_TRAFFIC_TX ? "uplink" :
> > +		    "downlink"),
> 
> I think this would be better
> 
> +		   !wifi_busy ? "idle" :
> +		   (wifi_traffic_dir == BTC_WIFI_TRAFFIC_TX ? "uplink" :
> +		    "downlink"),

It seems most repeated test1 ? : test2 ? : test3 ?:
uses do not have the style you suggest in the kernel.
Ping-Ke Shih July 27, 2020, 9:04 a.m. UTC | #3
On Mon, 2020-07-27 at 01:27 -0700, Joe Perches wrote:
> On Mon, 2020-07-27 at 06:07 +0000, Pkshih wrote:
> > On Sat, 2020-07-25 at 12:55 -0700, Joe Perches wrote:
> > > Make these statements a little simpler.
> []
> > > diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> []
> > > @@ -874,11 +874,10 @@ static void halbtc_display_wifi_status(struct
> > > btc_coexist *btcoexist,
> > >  	seq_printf(m, "\n %-35s = %s / %s/ %s/ AP=%d ",
> > >  		   "Wifi freq/ bw/ traffic",
> > >  		   gl_btc_wifi_freq_string[wifi_freq],
> > > -		   ((wifi_under_b_mode) ? "11b" :
> > > -		    gl_btc_wifi_bw_string[wifi_bw]),
> > > -		   ((!wifi_busy) ? "idle" : ((BTC_WIFI_TRAFFIC_TX ==
> > > -					      wifi_traffic_dir) ?
> "uplink" :
> > > -					     "downlink")),
> > > +		   wifi_under_b_mode ? "11b" :
> gl_btc_wifi_bw_string[wifi_bw],
> > > +		   (!wifi_busy ? "idle" :
> > > +		    wifi_traffic_dir == BTC_WIFI_TRAFFIC_TX ? "uplink" :
> > > +		    "downlink"),
> > 
> > I think this would be better
> > 
> > +		   !wifi_busy ? "idle" :
> > +		   (wifi_traffic_dir == BTC_WIFI_TRAFFIC_TX ? "uplink" :
> > +		    "downlink"),
> 
> It seems most repeated test1 ? : test2 ? : test3 ?:
> uses do not have the style you suggest in the kernel.
> 

Your change is 
(test1 ? : test2 ? :)

So, I think you would like to have parenthesis intentionally.
If so, 
test1 ? : (test2 ? :)
would be better.


If not,
test1 ? : test2 ? :
may be what you want (without any parenthesis).
Joe Perches July 27, 2020, 2:52 p.m. UTC | #4
On Mon, 2020-07-27 at 09:04 +0000, Pkshih wrote:
> So, I think you would like to have parenthesis intentionally.
> If so, 
> test1 ? : (test2 ? :)
> would be better.
> 
> 
> If not,
> test1 ? : test2 ? :
> may be what you want (without any parenthesis).

Use whatever style you like, it's unimportant to me
and it's not worth spending any real time on it.
Larry Finger July 27, 2020, 4:25 p.m. UTC | #5
On 7/27/20 9:52 AM, Joe Perches wrote:
> On Mon, 2020-07-27 at 09:04 +0000, Pkshih wrote:
>> So, I think you would like to have parenthesis intentionally.
>> If so,
>> test1 ? : (test2 ? :)
>> would be better.
>>
>>
>> If not,
>> test1 ? : test2 ? :
>> may be what you want (without any parenthesis).
> 
> Use whatever style you like, it's unimportant to me
> and it's not worth spending any real time on it.

If you are so busy, why did you jump in with patches that you knew I was already 
working on? You knew because you critiqued my first submission.

@Kalle: Please drop my contributions in the sequence "PATCH v2 00/15] rtlwifi: 
Change RT_TRACE into rtl_dbg for all drivers".

Larry
Joe Perches July 27, 2020, 4:33 p.m. UTC | #6
On Mon, 2020-07-27 at 11:25 -0500, Larry Finger wrote:
> On 7/27/20 9:52 AM, Joe Perches wrote:
> > On Mon, 2020-07-27 at 09:04 +0000, Pkshih wrote:
> > > So, I think you would like to have parenthesis intentionally.
> > > If so,
> > > test1 ? : (test2 ? :)
> > > would be better.
> > > 
> > > 
> > > If not,
> > > test1 ? : test2 ? :
> > > may be what you want (without any parenthesis).
> > 
> > Use whatever style you like, it's unimportant to me
> > and it's not worth spending any real time on it.
> 
> If you are so busy, why did you jump in with patches that you knew I was already 
> working on? You knew because you critiqued my first submission.

Because it was over a week and you didn't reply
to my original message nor did you cc me on any
changes you made to your patch?

I can't read every patch I'm not cc'd on.
Kalle Valo Aug. 27, 2020, 9:27 a.m. UTC | #7
Larry Finger <Larry.Finger@lwfinger.net> writes:

> On 7/27/20 9:52 AM, Joe Perches wrote:
>> On Mon, 2020-07-27 at 09:04 +0000, Pkshih wrote:
>>> So, I think you would like to have parenthesis intentionally.
>>> If so,
>>> test1 ? : (test2 ? :)
>>> would be better.
>>>
>>>
>>> If not,
>>> test1 ? : test2 ? :
>>> may be what you want (without any parenthesis).
>>
>> Use whatever style you like, it's unimportant to me
>> and it's not worth spending any real time on it.
>
> If you are so busy, why did you jump in with patches that you knew I
> was already working on? You knew because you critiqued my first
> submission.

Yeah, I don't understand this either. First stepping on Larry's work and
when after getting review comments claiming being busy and not caring is
contradicting.

> @Kalle: Please drop my contributions in the sequence "PATCH v2 00/15]
> rtlwifi: Change RT_TRACE into rtl_dbg for all drivers".

Is there a technical reason for that? I prefer that patchset more,
nicely split in smaller patches and it's fully available from patchwork.

Patch 15 had a build problem but I can drop that for now, it can be
resent separately:

https://patchwork.kernel.org/patch/11681621/
Joe Perches Aug. 27, 2020, 10:52 a.m. UTC | #8
On Thu, 2020-08-27 at 12:27 +0300, Kalle Valo wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
> > On 7/27/20 9:52 AM, Joe Perches wrote:
> > > On Mon, 2020-07-27 at 09:04 +0000, Pkshih wrote:
> > > > So, I think you would like to have parenthesis intentionally.
> > > > If so,
> > > > test1 ? : (test2 ? :)
> > > > would be better.
> > > > 
> > > > 
> > > > If not,
> > > > test1 ? : test2 ? :
> > > > may be what you want (without any parenthesis).
> > > 
> > > Use whatever style you like, it's unimportant to me
> > > and it's not worth spending any real time on it.
> > 
> > If you are so busy, why did you jump in with patches that you knew I
> > was already working on? You knew because you critiqued my first
> > submission.
> 
> Yeah, I don't understand this either. First stepping on Larry's work and
> when after getting review comments claiming being busy and not caring is
> contradicting.

I didn't say I was busy, I said I didn't care.

And it's not stepping on anyone's work, it's
a trivial contribution.  Take it or not.
It's been months btw, the pace of the work isn't
particularly fast here.
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
index 270aea0f841b..b8d184950dac 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -1385,7 +1385,7 @@  bool rtl_action_proc(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx)
 			if (mac->act_scanning)
 				return false;
 
-			rtl_dbg(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+			rtl_dbg(rtlpriv, COMP_SEND | COMP_RECV, DBG_DMESG,
 				"%s ACT_ADDBAREQ From :%pM\n",
 				is_tx ? "Tx" : "Rx", hdr->addr2);
 			RT_PRINT_DATA(rtlpriv, COMP_INIT, DBG_DMESG, "req\n",
@@ -1428,12 +1428,12 @@  bool rtl_action_proc(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx)
 			}
 			break;
 		case ACT_ADDBARSP:
-			rtl_dbg(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+			rtl_dbg(rtlpriv, COMP_SEND | COMP_RECV, DBG_DMESG,
 				"%s ACT_ADDBARSP From :%pM\n",
 				is_tx ? "Tx" : "Rx", hdr->addr2);
 			break;
 		case ACT_DELBA:
-			rtl_dbg(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+			rtl_dbg(rtlpriv, COMP_SEND | COMP_RECV, DBG_DMESG,
 				"ACT_ADDBADEL From :%pM\n", hdr->addr2);
 			break;
 		}
@@ -1519,9 +1519,9 @@  u8 rtl_is_special_data(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx,
 				/* 68 : UDP BOOTP client
 				 * 67 : UDP BOOTP server
 				 */
-				rtl_dbg(rtlpriv, (COMP_SEND | COMP_RECV),
+				rtl_dbg(rtlpriv, COMP_SEND | COMP_RECV,
 					DBG_DMESG, "dhcp %s !!\n",
-					(is_tx) ? "Tx" : "Rx");
+					is_tx ? "Tx" : "Rx");
 
 				if (is_tx)
 					setup_special_tx(rtlpriv, ppsc,
@@ -1540,8 +1540,8 @@  u8 rtl_is_special_data(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx,
 		rtlpriv->btcoexist.btc_info.in_4way = true;
 		rtlpriv->btcoexist.btc_info.in_4way_ts = jiffies;
 
-		rtl_dbg(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
-			"802.1X %s EAPOL pkt!!\n", (is_tx) ? "Tx" : "Rx");
+		rtl_dbg(rtlpriv, COMP_SEND | COMP_RECV, DBG_DMESG,
+			"802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
 
 		if (is_tx) {
 			rtlpriv->ra.is_special_data = true;
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c
index 4989fd3bae15..30c782d61d70 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c
@@ -801,8 +801,8 @@  static void btc8192e2ant_bt_auto_report(struct btc_coexist *btcoexist,
 
 	rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
 		"[BTCoex], %s BT Auto report = %s\n",
-		(force_exec ? "force to" : ""),
-		((enable_auto_report) ? "Enabled" : "Disabled"));
+		force_exec ? "force to" : "",
+		enable_auto_report ? "Enabled" : "Disabled");
 	coex_dm->cur_bt_auto_report = enable_auto_report;
 
 	if (!force_exec) {
@@ -878,9 +878,9 @@  static void btc8192e2ant_rf_shrink(struct btc_coexist *btcoexist,
 	struct rtl_priv *rtlpriv = btcoexist->adapter;
 
 	rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
-		"[BTCoex], %s turn Rx RF Shrink = %s\n",
-		(force_exec ? "force to" : ""),
-		((rx_rf_shrink_on) ? "ON" : "OFF"));
+		"[BTCoex], %sturn Rx RF Shrink = %s\n",
+		force_exec ? "force to " : "",
+		rx_rf_shrink_on ? "ON" : "OFF");
 	coex_dm->cur_rf_rx_lpf_shrink = rx_rf_shrink_on;
 
 	if (!force_exec) {
@@ -927,9 +927,10 @@  static void btc8192e2ant_dac_swing(struct btc_coexist *btcoexist,
 	struct rtl_priv *rtlpriv = btcoexist->adapter;
 
 	rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
-		"[BTCoex], %s turn DacSwing=%s, dac_swing_lvl = 0x%x\n",
-		(force_exec ? "force to" : ""),
-		((dac_swing_on) ? "ON" : "OFF"), dac_swing_lvl);
+		"[BTCoex], %sturn DacSwing=%s, dac_swing_lvl = 0x%x\n",
+		force_exec ? "force to " : "",
+		dac_swing_on ? "ON" : "OFF",
+		dac_swing_lvl);
 	coex_dm->cur_dac_swing_on = dac_swing_on;
 	coex_dm->cur_dac_swing_lvl = dac_swing_lvl;
 
@@ -987,9 +988,9 @@  static void btc8192e2ant_agc_table(struct btc_coexist *btcoexist,
 	struct rtl_priv *rtlpriv = btcoexist->adapter;
 
 	rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
-		"[BTCoex], %s %s Agc Table\n",
-		(force_exec ? "force to" : ""),
-		((agc_table_en) ? "Enable" : "Disable"));
+		"[BTCoex], %s%s Agc Table\n",
+		force_exec ? "force to " : "",
+		agc_table_en ? "Enable" : "Disable");
 	coex_dm->cur_agc_table_en = agc_table_en;
 
 	if (!force_exec) {
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c
index d2f4287da9a5..43bd52a62c4f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c
@@ -732,9 +732,9 @@  static void btc8821a2ant_low_penalty_ra(struct btc_coexist *btcoexist,
 	struct rtl_priv *rtlpriv = btcoexist->adapter;
 
 	rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
-		"[BTCoex], %s turn LowPenaltyRA = %s\n",
-		(force_exec ? "force to" : ""),
-		((low_penalty_ra) ? "ON" : "OFF"));
+		"[BTCoex], %sturn LowPenaltyRA = %s\n",
+		force_exec ? "force to " : "",
+		low_penalty_ra ? "ON" : "OFF");
 	coex_dm->cur_low_penalty_ra = low_penalty_ra;
 
 	if (!force_exec) {
@@ -780,9 +780,9 @@  static void btc8821a2ant_dac_swing(struct btc_coexist *btcoexist,
 	struct rtl_priv *rtlpriv = btcoexist->adapter;
 
 	rtl_dbg(rtlpriv, COMP_BT_COEXIST, DBG_LOUD,
-		"[BTCoex], %s turn DacSwing = %s, dac_swing_lvl = 0x%x\n",
-		(force_exec ? "force to" : ""),
-		((dac_swing_on) ? "ON" : "OFF"),
+		"[BTCoex], %sturn DacSwing = %s, dac_swing_lvl = 0x%x\n",
+		force_exec ? "force to " : "",
+		dac_swing_on ? "ON" : "OFF",
 		dac_swing_lvl);
 	coex_dm->cur_dac_swing_on = dac_swing_on;
 	coex_dm->cur_dac_swing_lvl = dac_swing_lvl;
diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
index 8d28c68f083e..f9a2d8a6730c 100644
--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
@@ -874,11 +874,10 @@  static void halbtc_display_wifi_status(struct btc_coexist *btcoexist,
 	seq_printf(m, "\n %-35s = %s / %s/ %s/ AP=%d ",
 		   "Wifi freq/ bw/ traffic",
 		   gl_btc_wifi_freq_string[wifi_freq],
-		   ((wifi_under_b_mode) ? "11b" :
-		    gl_btc_wifi_bw_string[wifi_bw]),
-		   ((!wifi_busy) ? "idle" : ((BTC_WIFI_TRAFFIC_TX ==
-					      wifi_traffic_dir) ? "uplink" :
-					     "downlink")),
+		   wifi_under_b_mode ? "11b" : gl_btc_wifi_bw_string[wifi_bw],
+		   (!wifi_busy ? "idle" :
+		    wifi_traffic_dir == BTC_WIFI_TRAFFIC_TX ? "uplink" :
+		    "downlink"),
 		   ap_num);
 
 	/* power status	 */
diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index 1d0af72ee780..3189d1c50d52 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -557,7 +557,7 @@  static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int prio)
 		if (rtlpriv->rtlhal.earlymode_enable)
 			skb_pull(skb, EM_HDR_LEN);
 
-		rtl_dbg(rtlpriv, (COMP_INTR | COMP_SEND), DBG_TRACE,
+		rtl_dbg(rtlpriv, COMP_INTR | COMP_SEND, DBG_TRACE,
 			"new ring->idx:%d, free: skb_queue_len:%d, free: seq:%x\n",
 			ring->idx,
 			skb_queue_len(&ring->queue),