Skip to contentSkip to conversation

staging: rtl8723au:core

PATCH In review Paul McQuade 4 participants 8 messages Oct 26, 2014 – Oct 29, 2014 Staging

lore
Share & tools
lore

Paul McQuade

author
#0 lore raw

PATCH 2/2 staging: rtl8723au:core

ERROR: spaces required around that ':' (ctx:VxE)

Signed-off-by Paul McQuade <[email protected]>
---
 drivers/staging/rtl8723au/core/rtw_ieee80211.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/staging/rtl8723au/core/rtw_ieee80211.c +8 -8
index 6274cb3..41006cf 100644
@@ -794,28 +794,28 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40,
 
 	if (rf_type == RF_1T1R) {
 		if (mcs->rx_mask[0] & BIT(7))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350) :
 				((short_GI_20)?722:650);
 		else if (mcs->rx_mask[0] & BIT(6))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215) :
 				((short_GI_20)?650:585);
 		else if (mcs->rx_mask[0] & BIT(5))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080) :
 				((short_GI_20)?578:520);
 		else if (mcs->rx_mask[0] & BIT(4))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?900:810):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?900:810) :
 				((short_GI_20)?433:390);
 		else if (mcs->rx_mask[0] & BIT(3))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?600:540):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?600:540) :
 				((short_GI_20)?289:260);
 		else if (mcs->rx_mask[0] & BIT(2))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?450:405):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?450:405) :
 				((short_GI_20)?217:195);
 		else if (mcs->rx_mask[0] & BIT(1))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?300:270):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?300:270) :
 				((short_GI_20)?144:130);
 		else if (mcs->rx_mask[0] & BIT(0))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?150:135):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?150:135) :
 				((short_GI_20)?72:65);
 	} else {
 		if (mcs->rx_mask[1]) {
Signature · 1 line
1.9.1
#1 lore raw
Quoted 36 lines · Paul McQuade

ERROR: spaces required around that ':' (ctx:VxE)

Signed-off-by Paul McQuade <[email protected]>
---
 drivers/staging/rtl8723au/core/rtw_ieee80211.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/staging/rtl8723au/core/rtw_ieee80211.c +5 -6
index 6274cb3..41006cf 100644
@@ -794,28 +794,28 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40,
 
 	if (rf_type == RF_1T1R) {
 		if (mcs->rx_mask[0] & BIT(7))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350) :
 				((short_GI_20)?722:650);
 		else if (mcs->rx_mask[0] & BIT(6))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215) :
 				((short_GI_20)?650:585);
 		else if (mcs->rx_mask[0] & BIT(5))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080) :
 				((short_GI_20)?578:520);
 		else if (mcs->rx_mask[0] & BIT(4))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?900:810):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?900:810) :
 				((short_GI_20)?433:390);
 		else if (mcs->rx_mask[0] & BIT(3))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?600:540):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?600:540) :
 				((short_GI_20)?289:260);
 		else if (mcs->rx_mask[0] & BIT(2))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?450:405):

drivers/staging/rtl8723au/core/rtw_ieee80211.c u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40, · line 819

Quoted 10 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +3 -2
+			max_rate = (bw_40MHz) ? ((short_GI_40)?450:405) :
 				((short_GI_20)?217:195);
 		else if (mcs->rx_mask[0] & BIT(1))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?300:270):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?300:270) :
 				((short_GI_20)?144:130);
 		else if (mcs->rx_mask[0] & BIT(0))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?150:135):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?150:135) :
 				((short_GI_20)?72:65);

A macro could help intelligibility here - maybe something like:

#define get_max_rate(r1, r2, r3, r4)					\
	(bw_40MHz ? (short_GI_40 ? r1 : r2) : (short_GI_20 ? r3 : r4))
and:
 		if (mcs->rx_mask[0] & BIT(7))
			max_rate = get_max_rate(1500, 1350, 722, 650);
		else if (mcs->rx_mask[0] & BIT(6))
			max_rate = get_max_rate(1350, 1215, 650, 585);
		else if (mcs->rx_mask[0] & BIT(5))
			max_rate = get_max_rate(1200, 1080, 578, 520);
		else if (mcs->rx_mask[0] & BIT(4))
			max_rate = get_max_rate(900, 810, 433, 390);
		else if (mcs->rx_mask[0] & BIT(3))
			max_rate = get_max_rate(600, 540, 289, 260);
		else if (mcs->rx_mask[0] & BIT(2))
			max_rate = get_max_rate(450, 405, 217, 195);
		else if (mcs->rx_mask[0] & BIT(1))
			max_rate = get_max_rate(300, 270, 144, 130);
		else if (mcs->rx_mask[0] & BIT(0))
			max_rate = get_max_rate(150, 135, 72, 65);
#2 lore raw
Quoted 50 lines · Joe Perches
Quoted 36 lines · Paul McQuade

ERROR: spaces required around that ':' (ctx:VxE)

Signed-off-by Paul McQuade <[email protected]>
---
 drivers/staging/rtl8723au/core/rtw_ieee80211.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/staging/rtl8723au/core/rtw_ieee80211.c +5 -6
index 6274cb3..41006cf 100644
@@ -794,28 +794,28 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40,
 
 	if (rf_type == RF_1T1R) {
 		if (mcs->rx_mask[0] & BIT(7))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350) :
 				((short_GI_20)?722:650);
 		else if (mcs->rx_mask[0] & BIT(6))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215) :
 				((short_GI_20)?650:585);
 		else if (mcs->rx_mask[0] & BIT(5))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080) :
 				((short_GI_20)?578:520);
 		else if (mcs->rx_mask[0] & BIT(4))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?900:810):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?900:810) :
 				((short_GI_20)?433:390);
 		else if (mcs->rx_mask[0] & BIT(3))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?600:540):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?600:540) :
 				((short_GI_20)?289:260);
 		else if (mcs->rx_mask[0] & BIT(2))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?450:405):

drivers/staging/rtl8723au/core/rtw_ieee80211.c u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40, · line 819

Quoted 10 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +3 -2
+			max_rate = (bw_40MHz) ? ((short_GI_40)?450:405) :
 				((short_GI_20)?217:195);
 		else if (mcs->rx_mask[0] & BIT(1))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?300:270):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?300:270) :
 				((short_GI_20)?144:130);
 		else if (mcs->rx_mask[0] & BIT(0))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?150:135):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?150:135) :
 				((short_GI_20)?72:65);

A macro could help intelligibility here - maybe something like:

Quoted 5 lines
#define get_max_rate(r1, r2, r3, r4)					\
	(bw_40MHz ? (short_GI_40 ? r1 : r2) : (short_GI_20 ? r3 : r4))
and:
 		if (mcs->rx_mask[0] & BIT(7))
			max_rate = get_max_rate(1500, 1350, 722, 650);

Hiding a parameter to a macro like that is bad coding practice, so don't do that please!

Jes

#3 lore raw
Quoted 3 lines · Jes Sorensen
Quoted 2 lines · Joe Perches
Quoted 1 line · Paul McQuade

ERROR: spaces required around that ':' (ctx:VxE)

[]

Quoted 1 line
Quoted 1 line

drivers/staging/rtl8723au/core/rtw_ieee80211.c

Quoted 1 line
drivers/staging/rtl8723au/core/rtw_ieee80211.c +0 -0

[]

Quoted 6 lines
Quoted 6 lines

drivers/staging/rtl8723au/core/rtw_ieee80211.c u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40, · line 797

Quoted 6 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +1 -1
@@ -794,28 +794,28 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40,
 
 	if (rf_type == RF_1T1R) {
 		if (mcs->rx_mask[0] & BIT(7))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350) :

[]

Quoted 11 lines
Quoted 8 lines

A macro could help intelligibility here - maybe something like:

#define get_max_rate(r1, r2, r3, r4)					\
	(bw_40MHz ? (short_GI_40 ? r1 : r2) : (short_GI_20 ? r3 : r4))
and:
 		if (mcs->rx_mask[0] & BIT(7))
			max_rate = get_max_rate(1500, 1350, 722, 650);

Hiding a parameter to a macro like that is bad coding practice, so don't do that please!

Yes and no.

Adding the other 3 arguments to the macro doesn't help legibility.

Keeping the macro definition local to the place that it's used can help avoid typos.

#4 lore raw
Quoted 27 lines · Joe Perches
Quoted 3 lines · Jes Sorensen
Quoted 2 lines · Joe Perches
Quoted 1 line · Paul McQuade

ERROR: spaces required around that ':' (ctx:VxE)

[]

Quoted 2 lines
Quoted 2 lines

drivers/staging/rtl8723au/core/rtw_ieee80211.c

Quoted 2 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +0 -0

[]

Quoted 6 lines
Quoted 6 lines

drivers/staging/rtl8723au/core/rtw_ieee80211.c u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40, · line 797

Quoted 6 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +1 -1
@@ -794,28 +794,28 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40,
 
 	if (rf_type == RF_1T1R) {
 		if (mcs->rx_mask[0] & BIT(7))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350) :

[]

Quoted 11 lines
Quoted 8 lines

A macro could help intelligibility here - maybe something like:

#define get_max_rate(r1, r2, r3, r4)					\
	(bw_40MHz ? (short_GI_40 ? r1 : r2) : (short_GI_20 ? r3 : r4))
and:
 		if (mcs->rx_mask[0] & BIT(7))
			max_rate = get_max_rate(1500, 1350, 722, 650);

Hiding a parameter to a macro like that is bad coding practice, so don't do that please!

Quoted 6 lines

Yes and no.

Adding the other 3 arguments to the macro doesn't help legibility.

Keeping the macro definition local to the place that it's used can help avoid typos.

It's wrong, so just don't do it here!

Thank you,
Jes

#5 lore raw
Quoted 28 lines · Jes Sorensen
Quoted 27 lines · Joe Perches
Quoted 3 lines · Jes Sorensen
Quoted 2 lines · Joe Perches
Quoted 1 line · Paul McQuade

ERROR: spaces required around that ':' (ctx:VxE)

[]

Quoted 2 lines
Quoted 2 lines

drivers/staging/rtl8723au/core/rtw_ieee80211.c

Quoted 2 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +0 -0

[]

Quoted 6 lines
Quoted 6 lines

drivers/staging/rtl8723au/core/rtw_ieee80211.c u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40, · line 797

Quoted 6 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +1 -1
@@ -794,28 +794,28 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40,
 
 	if (rf_type == RF_1T1R) {
 		if (mcs->rx_mask[0] & BIT(7))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350) :

[]

Quoted 11 lines
Quoted 8 lines

A macro could help intelligibility here - maybe something like:

#define get_max_rate(r1, r2, r3, r4)					\
	(bw_40MHz ? (short_GI_40 ? r1 : r2) : (short_GI_20 ? r3 : r4))
and:
 		if (mcs->rx_mask[0] & BIT(7))
			max_rate = get_max_rate(1500, 1350, 722, 650);

Hiding a parameter to a macro like that is bad coding practice, so don't do that please!

Quoted 8 lines
Quoted 6 lines

Yes and no.

Adding the other 3 arguments to the macro doesn't help legibility.

Keeping the macro definition local to the place that it's used can help avoid typos.

It's wrong, so just don't do it here!

It's not "wrong", you just don't like it.

#6 lore raw
Quoted 31 lines · Joe Perches
Quoted 30 lines · Jes Sorensen
Quoted 29 lines · Joe Perches
Quoted 3 lines · Jes Sorensen
Quoted 2 lines · Joe Perches
Quoted 1 line · Paul McQuade

ERROR: spaces required around that ':' (ctx:VxE)

[]

Quoted 2 lines
Quoted 2 lines

drivers/staging/rtl8723au/core/rtw_ieee80211.c

Quoted 2 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +0 -0

[]

Quoted 6 lines
Quoted 6 lines

drivers/staging/rtl8723au/core/rtw_ieee80211.c u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40, · line 797

Quoted 6 lines
drivers/staging/rtl8723au/core/rtw_ieee80211.c +1 -1
@@ -794,28 +794,28 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 short_GI_20, u8 short_GI_40,
 
 	if (rf_type == RF_1T1R) {
 		if (mcs->rx_mask[0] & BIT(7))
-			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
+			max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350) :

[]

Quoted 11 lines
Quoted 8 lines

A macro could help intelligibility here - maybe something like:

#define get_max_rate(r1, r2, r3, r4)					\
	(bw_40MHz ? (short_GI_40 ? r1 : r2) : (short_GI_20 ? r3 : r4))
and:
 		if (mcs->rx_mask[0] & BIT(7))
			max_rate = get_max_rate(1500, 1350, 722, 650);

Hiding a parameter to a macro like that is bad coding practice, so don't do that please!

Yes and no.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

Adding the other 3 arguments to the macro doesn't help legibility.

Keeping the macro definition local to the place that it's used can help avoid typos.

It's wrong, so just don't do it here!

It's not "wrong", you just don't like it.

I consider it wrong, and I am the co-maintainer of this driver .... and I NACK it.

There's a gazillion real issues to fix in this driver, adding a bad macro isn't on that list.

Jes