| 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 |
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),
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.
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).
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.
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
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.
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/
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 --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),
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(-)