All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings
@ 2017-06-12 10:46 Fabian Wolff
  2017-06-12 10:46 ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch series fixes all errors and warnings generated by
checkpatch.pl for wifi_regd.c.

Fabian Wolff (7):
  staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  staging: rtl8723bs: wifi_regd.c: fix checkpatch.pl warning 'Statements
    should start on a tabstop'
  staging: rtl8723bs: wifi_regd.c: fix comment formatting
  staging: rtl8723bs: wifi_regd.c: remove superfluous braces
  staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from
    pointer arguments
  staging: rtl8723bs: wifi_regd.c: adjust alignment to match open
    parenthesis
  staging: rtl8723bs: wifi_regd.c: insert blank line after declarations

 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 50 ++++++++++++++--------------
 1 file changed, 25 insertions(+), 25 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
@ 2017-06-12 10:46 ` Fabian Wolff
  2017-06-13 12:58   ` Greg KH
  2017-06-13 13:36   ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Ian W MORRISON
  2017-06-12 10:46 ` [PATCH 2/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch.pl warning 'Statements should start on a tabstop' Fabian Wolff
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch adds spaces around the binary operators '-' and '+', which
is the preferred style, and reformats a pointer argument declaration
to fix a false positive "spaces preferred around that '*'" message
reported by checkpatch.pl.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 9c61125..803eab0 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -20,7 +20,7 @@
 
 /* 2G chan 01 - chan 11 */
 #define RTW_2GHZ_CH01_11	\
-	REG_RULE(2412-10, 2462+10, 40, 0, 20, 0)
+	REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0)
 
 /*
  *We enable active scan on these a case
@@ -29,12 +29,12 @@
 
 /* 2G chan 12 - chan 13, PASSIV SCAN */
 #define RTW_2GHZ_CH12_13	\
-	REG_RULE(2467-10, 2472+10, 40, 0, 20,	\
+	REG_RULE(2467 - 10, 2472 + 10, 40, 0, 20,	\
 	NL80211_RRF_PASSIVE_SCAN)
 
 /* 2G chan 14, PASSIVS SCAN, NO OFDM (B only) */
 #define RTW_2GHZ_CH14	\
-	REG_RULE(2484-10, 2484+10, 40, 0, 20,	\
+	REG_RULE(2484 - 10, 2484 + 10, 40, 0, 20,	\
 	NL80211_RRF_PASSIVE_SCAN | NL80211_RRF_NO_OFDM)
 
 static const struct ieee80211_regdomain rtw_regdom_rd = {
@@ -116,8 +116,8 @@ static int _rtw_reg_notifier_apply(struct wiphy *wiphy,
 }
 
 static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
-							       rtw_regulatory
-							       *reg)
+							       rtw_regulatory *
+							       reg)
 {
 	return &rtw_regdom_rd;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch.pl warning 'Statements should start on a tabstop'
  2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
  2017-06-12 10:46 ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
@ 2017-06-12 10:46 ` Fabian Wolff
  2017-06-12 10:46 ` [PATCH 3/7] staging: rtl8723bs: wifi_regd.c: fix comment formatting Fabian Wolff
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch fixes the checkpatch.pl warning 'Statements should start on
a tabstop' by reformatting the affected lines.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 803eab0..00c7b0d 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -41,9 +41,9 @@ static const struct ieee80211_regdomain rtw_regdom_rd = {
 	.n_reg_rules = 3,
 	.alpha2 = "99",
 	.reg_rules = {
-		      RTW_2GHZ_CH01_11,
-		      RTW_2GHZ_CH12_13,
-		      }
+		RTW_2GHZ_CH01_11,
+		RTW_2GHZ_CH12_13,
+	}
 };
 
 static int rtw_ieee80211_channel_to_frequency(int chan, int band)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/7] staging: rtl8723bs: wifi_regd.c: fix comment formatting
  2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
  2017-06-12 10:46 ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
  2017-06-12 10:46 ` [PATCH 2/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch.pl warning 'Statements should start on a tabstop' Fabian Wolff
@ 2017-06-12 10:46 ` Fabian Wolff
  2017-06-12 10:46 ` [PATCH 4/7] staging: rtl8723bs: wifi_regd.c: remove superfluous braces Fabian Wolff
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch improves the formatting of block comments and removes one
commented-out line of code entirely (keeping it would be redundant
thanks to version control).

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 00c7b0d..043b34e 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -14,8 +14,8 @@
  */
 
 /*
- *Only these channels all allow active
- *scan on all world regulatory domains
+ * Only these channels all allow active
+ * scan on all world regulatory domains
  */
 
 /* 2G chan 01 - chan 11 */
@@ -23,8 +23,8 @@
 	REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0)
 
 /*
- *We enable active scan on these a case
- *by case basis by regulatory domain
+ * We enable active scan on these a case
+ * by case basis by regulatory domain
  */
 
 /* 2G chan 12 - chan 13, PASSIV SCAN */
@@ -49,7 +49,8 @@ static const struct ieee80211_regdomain rtw_regdom_rd = {
 static int rtw_ieee80211_channel_to_frequency(int chan, int band)
 {
 	/* see 802.11 17.3.8.3.2 and Annex J
-	 * there are overlapping channel numbers in 5GHz and 2GHz bands */
+	 * there are overlapping channel numbers in 5GHz and 2GHz bands
+	 */
 
 	/* NL80211_BAND_2GHZ */
 	if (chan == 14)
@@ -73,7 +74,7 @@ static void _rtw_reg_apply_flags(struct wiphy *wiphy)
 	u16 channel;
 	u32 freq;
 
-	/*  all channels disable */
+	/* all channels disable */
 	for (i = 0; i < NUM_NL80211_BANDS; i++) {
 		sband = wiphy->bands[i];
 
@@ -87,7 +88,7 @@ static void _rtw_reg_apply_flags(struct wiphy *wiphy)
 		}
 	}
 
-	/*  channels apply by channel plans. */
+	/* channels apply by channel plans. */
 	for (i = 0; i < max_chan_nums; i++) {
 		channel = channel_set[i].ChannelNum;
 		freq =
@@ -147,7 +148,6 @@ int rtw_regd_init(struct adapter *padapter,
 		  void (*reg_notifier) (struct wiphy * wiphy,
 				       struct regulatory_request *request))
 {
-	/* struct registry_priv  *registrypriv = &padapter->registrypriv; */
 	struct wiphy *wiphy = padapter->rtw_wdev->wiphy;
 	_rtw_regd_init_wiphy(NULL, wiphy, reg_notifier);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/7] staging: rtl8723bs: wifi_regd.c: remove superfluous braces
  2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
                   ` (2 preceding siblings ...)
  2017-06-12 10:46 ` [PATCH 3/7] staging: rtl8723bs: wifi_regd.c: fix comment formatting Fabian Wolff
@ 2017-06-12 10:46 ` Fabian Wolff
  2017-06-12 10:46 ` [PATCH 5/7] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments Fabian Wolff
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch removes unnecessary braces in an if/else-construct, thereby
fixing both a checkpatch.pl warning about superfluous braces and an
error about an ill-placed closing brace preceding the "else" keyword.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 043b34e..5e01846 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -97,12 +97,10 @@ static void _rtw_reg_apply_flags(struct wiphy *wiphy)
 
 		ch = ieee80211_get_channel(wiphy, freq);
 		if (ch) {
-			if (channel_set[i].ScanType == SCAN_PASSIVE) {
+			if (channel_set[i].ScanType == SCAN_PASSIVE)
 				ch->flags = IEEE80211_CHAN_NO_IR;
-			}
-			else {
+			else
 				ch->flags = 0;
-			}
 		}
 	}
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 5/7] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments
  2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
                   ` (3 preceding siblings ...)
  2017-06-12 10:46 ` [PATCH 4/7] staging: rtl8723bs: wifi_regd.c: remove superfluous braces Fabian Wolff
@ 2017-06-12 10:46 ` Fabian Wolff
  2017-06-12 10:46 ` [PATCH 6/7] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis Fabian Wolff
  2017-06-12 10:46 ` [PATCH 7/7] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations Fabian Wolff
  6 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch implements the suggestions of checkpatch.pl to remove
unnecessary spaces before function pointer arguments as well as in
statements of the form "foo * bar" (which should be "foo *bar").

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 5e01846..55d444d 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -123,7 +123,7 @@ static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
 
 static void _rtw_regd_init_wiphy(struct rtw_regulatory *reg,
 				struct wiphy *wiphy,
-				void (*reg_notifier) (struct wiphy * wiphy,
+				void (*reg_notifier)(struct wiphy *wiphy,
 						     struct regulatory_request *
 						     request))
 {
@@ -143,7 +143,7 @@ static void _rtw_regd_init_wiphy(struct rtw_regulatory *reg,
 }
 
 int rtw_regd_init(struct adapter *padapter,
-		  void (*reg_notifier) (struct wiphy * wiphy,
+		  void (*reg_notifier)(struct wiphy *wiphy,
 				       struct regulatory_request *request))
 {
 	struct wiphy *wiphy = padapter->rtw_wdev->wiphy;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 6/7] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis
  2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
                   ` (4 preceding siblings ...)
  2017-06-12 10:46 ` [PATCH 5/7] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments Fabian Wolff
@ 2017-06-12 10:46 ` Fabian Wolff
  2017-06-12 10:46 ` [PATCH 7/7] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations Fabian Wolff
  6 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch adjusts the alignment of several lines to match their
respective opening parenthesis.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 55d444d..c8945da 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -122,10 +122,11 @@ static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
 }
 
 static void _rtw_regd_init_wiphy(struct rtw_regulatory *reg,
-				struct wiphy *wiphy,
-				void (*reg_notifier)(struct wiphy *wiphy,
-						     struct regulatory_request *
-						     request))
+				 struct wiphy *wiphy,
+				 void (*reg_notifier)(struct wiphy *wiphy,
+						      struct
+						      regulatory_request *
+						      request))
 {
 	const struct ieee80211_regdomain *regd;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 7/7] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations
  2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
                   ` (5 preceding siblings ...)
  2017-06-12 10:46 ` [PATCH 6/7] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis Fabian Wolff
@ 2017-06-12 10:46 ` Fabian Wolff
  6 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-12 10:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel

This patch inserts a missing blank line after variable declarations.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index c8945da..0f446e7 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -148,6 +148,7 @@ int rtw_regd_init(struct adapter *padapter,
 				       struct regulatory_request *request))
 {
 	struct wiphy *wiphy = padapter->rtw_wdev->wiphy;
+
 	_rtw_regd_init_wiphy(NULL, wiphy, reg_notifier);
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-12 10:46 ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
@ 2017-06-13 12:58   ` Greg KH
  2017-06-13 13:11     ` Greg KH
  2017-06-13 13:36   ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Ian W MORRISON
  1 sibling, 1 reply; 21+ messages in thread
From: Greg KH @ 2017-06-13 12:58 UTC (permalink / raw)
  To: Fabian Wolff; +Cc: linux-kernel, horvatmate, hdegoede, devel, linux-kernel

On Mon, Jun 12, 2017 at 12:46:10PM +0200, Fabian Wolff wrote:
> This patch adds spaces around the binary operators '-' and '+', which
> is the preferred style, and reformats a pointer argument declaration
> to fix a false positive "spaces preferred around that '*'" message
> reported by checkpatch.pl.
> 
> Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
> Signed-off-by: Mate Horvath <horvatmate@gmail.com>
> ---
>  drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> index 9c61125..803eab0 100644
> --- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> +++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> @@ -20,7 +20,7 @@
>  
>  /* 2G chan 01 - chan 11 */
>  #define RTW_2GHZ_CH01_11	\
> -	REG_RULE(2412-10, 2462+10, 40, 0, 20, 0)
> +	REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0)
>  
>  /*
>   *We enable active scan on these a case
> @@ -29,12 +29,12 @@
>  
>  /* 2G chan 12 - chan 13, PASSIV SCAN */
>  #define RTW_2GHZ_CH12_13	\
> -	REG_RULE(2467-10, 2472+10, 40, 0, 20,	\
> +	REG_RULE(2467 - 10, 2472 + 10, 40, 0, 20,	\
>  	NL80211_RRF_PASSIVE_SCAN)
>  
>  /* 2G chan 14, PASSIVS SCAN, NO OFDM (B only) */
>  #define RTW_2GHZ_CH14	\
> -	REG_RULE(2484-10, 2484+10, 40, 0, 20,	\
> +	REG_RULE(2484 - 10, 2484 + 10, 40, 0, 20,	\
>  	NL80211_RRF_PASSIVE_SCAN | NL80211_RRF_NO_OFDM)
>  
>  static const struct ieee80211_regdomain rtw_regdom_rd = {
> @@ -116,8 +116,8 @@ static int _rtw_reg_notifier_apply(struct wiphy *wiphy,
>  }
>  
>  static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
> -							       rtw_regulatory
> -							       *reg)
> +							       rtw_regulatory *
> +							       reg)

Why make this last change?  It's not an arithmatic one :(

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-13 12:58   ` Greg KH
@ 2017-06-13 13:11     ` Greg KH
  2017-06-13 21:01       ` [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
  0 siblings, 1 reply; 21+ messages in thread
From: Greg KH @ 2017-06-13 13:11 UTC (permalink / raw)
  To: Fabian Wolff; +Cc: horvatmate, hdegoede, devel, linux-kernel, linux-kernel

On Tue, Jun 13, 2017 at 02:58:43PM +0200, Greg KH wrote:
> On Mon, Jun 12, 2017 at 12:46:10PM +0200, Fabian Wolff wrote:
> > This patch adds spaces around the binary operators '-' and '+', which
> > is the preferred style, and reformats a pointer argument declaration
> > to fix a false positive "spaces preferred around that '*'" message
> > reported by checkpatch.pl.
> > 
> > Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
> > Signed-off-by: Mate Horvath <horvatmate@gmail.com>
> > ---
> >  drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> > index 9c61125..803eab0 100644
> > --- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> > @@ -20,7 +20,7 @@
> >  
> >  /* 2G chan 01 - chan 11 */
> >  #define RTW_2GHZ_CH01_11	\
> > -	REG_RULE(2412-10, 2462+10, 40, 0, 20, 0)
> > +	REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0)
> >  
> >  /*
> >   *We enable active scan on these a case
> > @@ -29,12 +29,12 @@
> >  
> >  /* 2G chan 12 - chan 13, PASSIV SCAN */
> >  #define RTW_2GHZ_CH12_13	\
> > -	REG_RULE(2467-10, 2472+10, 40, 0, 20,	\
> > +	REG_RULE(2467 - 10, 2472 + 10, 40, 0, 20,	\
> >  	NL80211_RRF_PASSIVE_SCAN)
> >  
> >  /* 2G chan 14, PASSIVS SCAN, NO OFDM (B only) */
> >  #define RTW_2GHZ_CH14	\
> > -	REG_RULE(2484-10, 2484+10, 40, 0, 20,	\
> > +	REG_RULE(2484 - 10, 2484 + 10, 40, 0, 20,	\
> >  	NL80211_RRF_PASSIVE_SCAN | NL80211_RRF_NO_OFDM)
> >  
> >  static const struct ieee80211_regdomain rtw_regdom_rd = {
> > @@ -116,8 +116,8 @@ static int _rtw_reg_notifier_apply(struct wiphy *wiphy,
> >  }
> >  
> >  static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
> > -							       rtw_regulatory
> > -							       *reg)
> > +							       rtw_regulatory *
> > +							       reg)
> 
> Why make this last change?  It's not an arithmatic one :(

And can you refresh this series and resend the remaining ones?  They all
didn't apply :(

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-12 10:46 ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
  2017-06-13 12:58   ` Greg KH
@ 2017-06-13 13:36   ` Ian W MORRISON
  2017-06-13 14:36     ` Dan Carpenter
  1 sibling, 1 reply; 21+ messages in thread
From: Ian W MORRISON @ 2017-06-13 13:36 UTC (permalink / raw)
  To: Fabian Wolff
  Cc: linux-kernel, horvatmate, devel, Greg KH, linux-kernel, Hans de Goede

On 12 June 2017 at 20:46, Fabian Wolff <fabian.wolff@fau.de> wrote:
> This patch adds spaces around the binary operators '-' and '+', which
> is the preferred style, and reformats a pointer argument declaration
> to fix a false positive "spaces preferred around that '*'" message
> reported by checkpatch.pl.
>
> Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
> Signed-off-by: Mate Horvath <horvatmate@gmail.com>
> ---
>  drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> index 9c61125..803eab0 100644
> --- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> +++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
> @@ -20,7 +20,7 @@
>
>  /* 2G chan 01 - chan 11 */
>  #define RTW_2GHZ_CH01_11       \
> -       REG_RULE(2412-10, 2462+10, 40, 0, 20, 0)
> +       REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0)
>
>  /*
>   *We enable active scan on these a case
> @@ -29,12 +29,12 @@
>
>  /* 2G chan 12 - chan 13, PASSIV SCAN */
>  #define RTW_2GHZ_CH12_13       \
> -       REG_RULE(2467-10, 2472+10, 40, 0, 20,   \
> +       REG_RULE(2467 - 10, 2472 + 10, 40, 0, 20,       \
>         NL80211_RRF_PASSIVE_SCAN)
>
>  /* 2G chan 14, PASSIVS SCAN, NO OFDM (B only) */
>  #define RTW_2GHZ_CH14  \
> -       REG_RULE(2484-10, 2484+10, 40, 0, 20,   \
> +       REG_RULE(2484 - 10, 2484 + 10, 40, 0, 20,       \
>         NL80211_RRF_PASSIVE_SCAN | NL80211_RRF_NO_OFDM)
>

The construct 'freq_start-10, freq_end+10' is consistent with other drivers e.g.

drivers/net/wireless/mac80211_hwsim.c
drivers/net/wireless/broadcom/brcm80211/brcmsmac/channel.c:#define
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
drivers/net/wireless/realtek/rtlwifi/regd.c
drivers/net/wireless/ath/regd.c

and looks better (to me) without the inserted spaces.

If the change is required will the other drivers be 'corrected'?

>  static const struct ieee80211_regdomain rtw_regdom_rd = {
> @@ -116,8 +116,8 @@ static int _rtw_reg_notifier_apply(struct wiphy *wiphy,
>  }
>
>  static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
> -                                                              rtw_regulatory
> -                                                              *reg)
> +                                                              rtw_regulatory *
> +                                                              reg)
>  {
>         return &rtw_regdom_rd;
>  }
> --
> 2.7.4
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-13 13:36   ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Ian W MORRISON
@ 2017-06-13 14:36     ` Dan Carpenter
  2017-06-14 11:49       ` Ian W MORRISON
  0 siblings, 1 reply; 21+ messages in thread
From: Dan Carpenter @ 2017-06-13 14:36 UTC (permalink / raw)
  To: Ian W MORRISON
  Cc: Fabian Wolff, horvatmate, devel, linux-kernel, Greg KH,
	linux-kernel, Hans de Goede

Kernel style is to have spaces around the operators.  This is staging
code so we do all the style fixes.  We don't always update older drivers
but sometimes we do.  No one is planning to change those drivers though
so I guess the answer is no we're not going to update those unless you
are?

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings
  2017-06-13 13:11     ` Greg KH
@ 2017-06-13 21:01       ` Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 1/5] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
                           ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-13 21:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel,
	ianwmorrison, dan.carpenter

v1->v2: Refresh patches and restrict the first patch to arithmetic operators

This patch series fixes all errors and warnings generated by
checkpatch.pl for wifi_regd.c.

Fabian Wolff (5):
  staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  staging: rtl8723bs: wifi_regd.c: fix comment formatting
  staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from
    pointer arguments
  staging: rtl8723bs: wifi_regd.c: adjust alignment to match open
    parenthesis
  staging: rtl8723bs: wifi_regd.c: insert blank line after declarations

 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 34 +++++++++++++++-------------
 1 file changed, 18 insertions(+), 16 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/5] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-13 21:01       ` [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
@ 2017-06-13 21:01         ` Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 2/5] staging: rtl8723bs: wifi_regd.c: fix comment formatting Fabian Wolff
                           ` (3 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-13 21:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel,
	ianwmorrison, dan.carpenter

This patch adds spaces around the binary operators '-' and '+'.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
	v1->v2: apply changes only to arithmetic operators

 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 115ba66..5bcf968 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -20,7 +20,7 @@
 
 /* 2G chan 01 - chan 11 */
 #define RTW_2GHZ_CH01_11	\
-	REG_RULE(2412-10, 2462+10, 40, 0, 20, 0)
+	REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0)
 
 /*
  *We enable active scan on these a case
@@ -29,12 +29,12 @@
 
 /* 2G chan 12 - chan 13, PASSIV SCAN */
 #define RTW_2GHZ_CH12_13	\
-	REG_RULE(2467-10, 2472+10, 40, 0, 20,	\
+	REG_RULE(2467 - 10, 2472 + 10, 40, 0, 20,	\
 	NL80211_RRF_PASSIVE_SCAN)
 
 /* 2G chan 14, PASSIVS SCAN, NO OFDM (B only) */
 #define RTW_2GHZ_CH14	\
-	REG_RULE(2484-10, 2484+10, 40, 0, 20,	\
+	REG_RULE(2484 - 10, 2484 + 10, 40, 0, 20,	\
 	NL80211_RRF_PASSIVE_SCAN | NL80211_RRF_NO_OFDM)
 
 static const struct ieee80211_regdomain rtw_regdom_rd = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/5] staging: rtl8723bs: wifi_regd.c: fix comment formatting
  2017-06-13 21:01       ` [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 1/5] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
@ 2017-06-13 21:01         ` Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 3/5] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments Fabian Wolff
                           ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-13 21:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel,
	ianwmorrison, dan.carpenter

This patch improves the formatting of block comments and removes one
commented-out line of code entirely (keeping it would be redundant
thanks to version control).

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
	v1->v2: refresh patch

 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 5bcf968..53ff2ce 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -14,8 +14,8 @@
  */
 
 /*
- *Only these channels all allow active
- *scan on all world regulatory domains
+ * Only these channels all allow active
+ * scan on all world regulatory domains
  */
 
 /* 2G chan 01 - chan 11 */
@@ -23,8 +23,8 @@
 	REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0)
 
 /*
- *We enable active scan on these a case
- *by case basis by regulatory domain
+ * We enable active scan on these a case
+ * by case basis by regulatory domain
  */
 
 /* 2G chan 12 - chan 13, PASSIV SCAN */
@@ -49,7 +49,8 @@ static const struct ieee80211_regdomain rtw_regdom_rd = {
 static int rtw_ieee80211_channel_to_frequency(int chan, int band)
 {
 	/* see 802.11 17.3.8.3.2 and Annex J
-	 * there are overlapping channel numbers in 5GHz and 2GHz bands */
+	 * there are overlapping channel numbers in 5GHz and 2GHz bands
+	 */
 
 	/* NL80211_BAND_2GHZ */
 	if (chan == 14)
@@ -73,7 +74,7 @@ static void _rtw_reg_apply_flags(struct wiphy *wiphy)
 	u16 channel;
 	u32 freq;
 
-	/*  all channels disable */
+	/* all channels disable */
 	for (i = 0; i < NUM_NL80211_BANDS; i++) {
 		sband = wiphy->bands[i];
 
@@ -87,7 +88,7 @@ static void _rtw_reg_apply_flags(struct wiphy *wiphy)
 		}
 	}
 
-	/*  channels apply by channel plans. */
+	/* channels apply by channel plans. */
 	for (i = 0; i < max_chan_nums; i++) {
 		channel = channel_set[i].ChannelNum;
 		freq =
@@ -145,7 +146,6 @@ int rtw_regd_init(struct adapter *padapter,
 		  void (*reg_notifier) (struct wiphy * wiphy,
 				       struct regulatory_request *request))
 {
-	/* struct registry_priv  *registrypriv = &padapter->registrypriv; */
 	struct wiphy *wiphy = padapter->rtw_wdev->wiphy;
 	_rtw_regd_init_wiphy(NULL, wiphy, reg_notifier);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 3/5] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments
  2017-06-13 21:01       ` [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 1/5] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 2/5] staging: rtl8723bs: wifi_regd.c: fix comment formatting Fabian Wolff
@ 2017-06-13 21:01         ` Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 4/5] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 5/5] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations Fabian Wolff
  4 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-13 21:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel,
	ianwmorrison, dan.carpenter

This patch implements the suggestions of checkpatch.pl to remove
unnecessary spaces before function pointer arguments as well as in
statements of the form "foo * bar" (which should be "foo *bar").

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
	v1->v2: refresh patch

 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 53ff2ce..6c8d475 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -123,7 +123,7 @@ static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
 
 static void _rtw_regd_init_wiphy(struct rtw_regulatory *reg,
 				struct wiphy *wiphy,
-				void (*reg_notifier) (struct wiphy * wiphy,
+				void (*reg_notifier)(struct wiphy *wiphy,
 						     struct regulatory_request *
 						     request))
 {
@@ -143,7 +143,7 @@ static void _rtw_regd_init_wiphy(struct rtw_regulatory *reg,
 }
 
 int rtw_regd_init(struct adapter *padapter,
-		  void (*reg_notifier) (struct wiphy * wiphy,
+		  void (*reg_notifier)(struct wiphy *wiphy,
 				       struct regulatory_request *request))
 {
 	struct wiphy *wiphy = padapter->rtw_wdev->wiphy;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 4/5] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis
  2017-06-13 21:01       ` [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
                           ` (2 preceding siblings ...)
  2017-06-13 21:01         ` [PATCH v2 3/5] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments Fabian Wolff
@ 2017-06-13 21:01         ` Fabian Wolff
  2017-06-13 21:01         ` [PATCH v2 5/5] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations Fabian Wolff
  4 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-13 21:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel,
	ianwmorrison, dan.carpenter

This patch adjusts the alignment of several lines to match their
respective opening parenthesis.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
	v1->v2: refresh patch

 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 6c8d475..68890b4 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -122,10 +122,11 @@ static const struct ieee80211_regdomain *_rtw_regdomain_select(struct
 }
 
 static void _rtw_regd_init_wiphy(struct rtw_regulatory *reg,
-				struct wiphy *wiphy,
-				void (*reg_notifier)(struct wiphy *wiphy,
-						     struct regulatory_request *
-						     request))
+				 struct wiphy *wiphy,
+				 void (*reg_notifier)(struct wiphy *wiphy,
+						      struct
+						      regulatory_request *
+						      request))
 {
 	const struct ieee80211_regdomain *regd;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 5/5] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations
  2017-06-13 21:01       ` [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
                           ` (3 preceding siblings ...)
  2017-06-13 21:01         ` [PATCH v2 4/5] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis Fabian Wolff
@ 2017-06-13 21:01         ` Fabian Wolff
  4 siblings, 0 replies; 21+ messages in thread
From: Fabian Wolff @ 2017-06-13 21:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: gregkh, horvatmate, fabian.wolff, hdegoede, devel, linux-kernel,
	ianwmorrison, dan.carpenter

This patch inserts a missing blank line after variable declarations.

Signed-off-by: Fabian Wolff <fabian.wolff@fau.de>
Signed-off-by: Mate Horvath <horvatmate@gmail.com>
---
	v1->v2: refresh patch

 drivers/staging/rtl8723bs/os_dep/wifi_regd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
index 68890b4..305e88a 100644
--- a/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
+++ b/drivers/staging/rtl8723bs/os_dep/wifi_regd.c
@@ -148,6 +148,7 @@ int rtw_regd_init(struct adapter *padapter,
 				       struct regulatory_request *request))
 {
 	struct wiphy *wiphy = padapter->rtw_wdev->wiphy;
+
 	_rtw_regd_init_wiphy(NULL, wiphy, reg_notifier);
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-13 14:36     ` Dan Carpenter
@ 2017-06-14 11:49       ` Ian W MORRISON
  2017-06-14 12:19         ` Dan Carpenter
  0 siblings, 1 reply; 21+ messages in thread
From: Ian W MORRISON @ 2017-06-14 11:49 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Fabian Wolff, Máté Horváth, devel, linux-kernel,
	Greg KH, linux-kernel, Hans de Goede, Ian W Morrison

On 14 June 2017 at 00:36, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Kernel style is to have spaces around the operators.  This is staging
> code so we do all the style fixes.  We don't always update older drivers
> but sometimes we do.  No one is planning to change those drivers though
> so I guess the answer is no we're not going to update those unless you
> are?
>

Thanks for the explanation. I assume submitting changes for the
drivers I identified would only be seen as minor corrections to 'the
chaff' resulting in unnecessary churn. If however it is expected that
corrections should be made when identified then I'm willing to prepare
a patch set. I'm happy to take advice either way.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-14 11:49       ` Ian W MORRISON
@ 2017-06-14 12:19         ` Dan Carpenter
  2017-06-14 13:25           ` Joe Perches
  0 siblings, 1 reply; 21+ messages in thread
From: Dan Carpenter @ 2017-06-14 12:19 UTC (permalink / raw)
  To: Ian W MORRISON
  Cc: Fabian Wolff, Máté Horváth, devel, linux-kernel,
	Greg KH, linux-kernel, Hans de Goede

On Wed, Jun 14, 2017 at 09:49:10PM +1000, Ian W MORRISON wrote:
> On 14 June 2017 at 00:36, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Kernel style is to have spaces around the operators.  This is staging
> > code so we do all the style fixes.  We don't always update older drivers
> > but sometimes we do.  No one is planning to change those drivers though
> > so I guess the answer is no we're not going to update those unless you
> > are?
> >
> 
> Thanks for the explanation. I assume submitting changes for the
> drivers I identified would only be seen as minor corrections to 'the
> chaff' resulting in unnecessary churn. If however it is expected that
> corrections should be made when identified then I'm willing to prepare
> a patch set. I'm happy to take advice either way.

I would just leave the old drivers as-is.

Having spaces around operators has always been kernel style, but it's
only fairly recently that checkpatch.pl started to complain.  We keep
making checkpatch.pl more stict as time goes on.  I think that's good
because some reviewers will make you redo patches for style issues so
having checkpatch.pl complain early on means you don't have redo the
patch.  But it also means that old code will never be checkpatch.pl
clean because we keep adding new checkpatch warnings.

And it's fine that old code has checkpatch warnings.  The point of
checkpatch is to check new patches not to churn through old code.  As a
reviewer, I find that checkpatch saves my time because I can often tell
people to run it instead of listing all the style complaints.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators
  2017-06-14 12:19         ` Dan Carpenter
@ 2017-06-14 13:25           ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2017-06-14 13:25 UTC (permalink / raw)
  To: Dan Carpenter, Ian W MORRISON
  Cc: Fabian Wolff, Máté Horváth, devel, linux-kernel,
	Greg KH, linux-kernel, Hans de Goede

On Wed, 2017-06-14 at 15:19 +0300, Dan Carpenter wrote:
> On Wed, Jun 14, 2017 at 09:49:10PM +1000, Ian W MORRISON wrote:
> > On 14 June 2017 at 00:36, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > > Kernel style is to have spaces around the operators.  This is staging
> > > code so we do all the style fixes.  We don't always update older drivers
> > > but sometimes we do.  No one is planning to change those drivers though
> > > so I guess the answer is no we're not going to update those unless you
> > > are?
> > > 
> > 
> > Thanks for the explanation. I assume submitting changes for the
> > drivers I identified would only be seen as minor corrections to 'the
> > chaff' resulting in unnecessary churn. If however it is expected that
> > corrections should be made when identified then I'm willing to prepare
> > a patch set. I'm happy to take advice either way.
> 
> I would just leave the old drivers as-is.

There would be some value in converting them, if only to
note what functions are identical across multiple drivers
and could be consolidated.

> Having spaces around operators has always been kernel style, but it's
> only fairly recently that checkpatch.pl started to complain.

Not really.  Linux is a relatively old project.

The CodingStyle for spaces around operators was added about
ten years ago.  Bit shifts were and still are not mentioned.

Messages for spaces around arithmetic were not emitted because
so much old code had various styles and it would have been a
lot of churn to update with many complaints from various
maintainers.  checkpatch today would emit about a quarter
million error messages for spacing on the kernel source tree.
That's a lot of whitespace churn.

There is a lot of code in drivers and a few other areas that
hasn't been touched in those ten years that doesn't follow
today's commonly accepted coding styles.

CodingStyle still doesn't mention a lot of style nits and bits.

About spacing around bit shifts:

This is just a --strict preference that several people had
expressed a desire to warn about spaces around arithmetic
and bit operators.  

It was added a couple years ago to checkpatch but not to
CodingStyle.

The --strict test applies by default to net/and drivers/net
because DaveM is pretty style conscious and asks for rework
when patches don't have the style he prefers.

And the --strict test applies to drivers/staging as well
because it gives more opportunities for first-time patch
submitters to send cleanup style patches (and GregKH can
be a stickler too).

> We keep making checkpatch.pl more stict as time goes on.

I try to add things to checkpatch only when there seems to
be a general consensus about a style or when a significant
percentage of code throughout the kernel already follows a 
specific style.

Actively developed Kernel code now has a fairly uniform
style and additions to checkpatch seem less necessary.

One area where checkpatch could use some enhancing is in
tracking indentation better.

checkpatch doesn't emit warnings with indentation like:

	statement(1);
		statement(2);

where the statements should be aligned on the same tabstop.

I'm playing with it but patches welcome...

> I think that's good
> because some reviewers will make you redo patches for style issues so
> having checkpatch.pl complain early on means you don't have redo the
> patch.  But it also means that old code will never be checkpatch.pl
> clean because we keep adding new checkpatch warnings.
> 
> And it's fine that old code has checkpatch warnings.  The point of
> checkpatch is to check new patches not to churn through old code.  As a
> reviewer, I find that checkpatch saves my time because I can often tell
> people to run it instead of listing all the style complaints.

All very true.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-06-14 13:25 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-12 10:46 [PATCH 0/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
2017-06-12 10:46 ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
2017-06-13 12:58   ` Greg KH
2017-06-13 13:11     ` Greg KH
2017-06-13 21:01       ` [PATCH v2 0/5] staging: rtl8723bs: wifi_regd.c: fix checkpatch errors and warnings Fabian Wolff
2017-06-13 21:01         ` [PATCH v2 1/5] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Fabian Wolff
2017-06-13 21:01         ` [PATCH v2 2/5] staging: rtl8723bs: wifi_regd.c: fix comment formatting Fabian Wolff
2017-06-13 21:01         ` [PATCH v2 3/5] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments Fabian Wolff
2017-06-13 21:01         ` [PATCH v2 4/5] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis Fabian Wolff
2017-06-13 21:01         ` [PATCH v2 5/5] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations Fabian Wolff
2017-06-13 13:36   ` [PATCH 1/7] staging: rtl8723bs: wifi_regd.c: put spaces around binary operators Ian W MORRISON
2017-06-13 14:36     ` Dan Carpenter
2017-06-14 11:49       ` Ian W MORRISON
2017-06-14 12:19         ` Dan Carpenter
2017-06-14 13:25           ` Joe Perches
2017-06-12 10:46 ` [PATCH 2/7] staging: rtl8723bs: wifi_regd.c: fix checkpatch.pl warning 'Statements should start on a tabstop' Fabian Wolff
2017-06-12 10:46 ` [PATCH 3/7] staging: rtl8723bs: wifi_regd.c: fix comment formatting Fabian Wolff
2017-06-12 10:46 ` [PATCH 4/7] staging: rtl8723bs: wifi_regd.c: remove superfluous braces Fabian Wolff
2017-06-12 10:46 ` [PATCH 5/7] staging: rtl8723bs: wifi_regd.c: remove superfluous spaces from pointer arguments Fabian Wolff
2017-06-12 10:46 ` [PATCH 6/7] staging: rtl8723bs: wifi_regd.c: adjust alignment to match open parenthesis Fabian Wolff
2017-06-12 10:46 ` [PATCH 7/7] staging: rtl8723bs: wifi_regd.c: insert blank line after declarations Fabian Wolff

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.