* [PATCH 2/2] staging: rtl8723au:core @ 2014-10-26 16:18 Paul McQuade 2014-10-26 19:04 ` Joe Perches 2014-10-29 9:35 ` Greg KH 0 siblings, 2 replies; 8+ messages in thread From: Paul McQuade @ 2014-10-26 16:18 UTC (permalink / raw) To: paulmcquad Cc: linux-kernel, devel, linux-wireless, gregkh, Jes.Sorensen, Larry.Finger ERROR: spaces required around that ':' (ctx:VxE) Signed-off-by: Paul McQuade <paulmcquad@gmail.com> --- drivers/staging/rtl8723au/core/rtw_ieee80211.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c b/drivers/staging/rtl8723au/core/rtw_ieee80211.c index 6274cb3..41006cf 100644 --- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c +++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c @@ -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]) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] staging: rtl8723au:core 2014-10-26 16:18 [PATCH 2/2] staging: rtl8723au:core Paul McQuade @ 2014-10-26 19:04 ` Joe Perches 2014-10-27 5:45 ` Jes Sorensen 2014-10-29 9:35 ` Greg KH 1 sibling, 1 reply; 8+ messages in thread From: Joe Perches @ 2014-10-26 19:04 UTC (permalink / raw) To: Paul McQuade Cc: linux-kernel, devel, linux-wireless, gregkh, Jes.Sorensen, Larry.Finger On Sun, 2014-10-26 at 16:18 +0000, Paul McQuade wrote: > ERROR: spaces required around that ':' (ctx:VxE) > > Signed-off-by: Paul McQuade <paulmcquad@gmail.com> > --- > drivers/staging/rtl8723au/core/rtw_ieee80211.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c b/drivers/staging/rtl8723au/core/rtw_ieee80211.c > index 6274cb3..41006cf 100644 > --- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c > +++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c > @@ -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); 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); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] staging: rtl8723au:core 2014-10-26 19:04 ` Joe Perches @ 2014-10-27 5:45 ` Jes Sorensen 2014-10-27 5:51 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: Jes Sorensen @ 2014-10-27 5:45 UTC (permalink / raw) To: Joe Perches Cc: Paul McQuade, linux-kernel, devel, linux-wireless, gregkh, Larry.Finger Joe Perches <joe@perches.com> writes: > On Sun, 2014-10-26 at 16:18 +0000, Paul McQuade wrote: >> ERROR: spaces required around that ':' (ctx:VxE) >> >> Signed-off-by: Paul McQuade <paulmcquad@gmail.com> >> --- >> drivers/staging/rtl8723au/core/rtw_ieee80211.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c b/drivers/staging/rtl8723au/core/rtw_ieee80211.c >> index 6274cb3..41006cf 100644 >> --- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c >> +++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c >> @@ -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); > > 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! Jes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] staging: rtl8723au:core 2014-10-27 5:45 ` Jes Sorensen @ 2014-10-27 5:51 ` Joe Perches 2014-10-27 6:16 ` Jes Sorensen 0 siblings, 1 reply; 8+ messages in thread From: Joe Perches @ 2014-10-27 5:51 UTC (permalink / raw) To: Jes Sorensen Cc: Paul McQuade, linux-kernel, devel, linux-wireless, gregkh, Larry.Finger On Mon, 2014-10-27 at 06:45 +0100, Jes Sorensen wrote: > Joe Perches <joe@perches.com> writes: > > On Sun, 2014-10-26 at 16:18 +0000, Paul McQuade wrote: > >> ERROR: spaces required around that ':' (ctx:VxE) [] > >> diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c b/drivers/staging/rtl8723au/core/rtw_ieee80211.c [] > >> @@ -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) : [] > > > > 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] staging: rtl8723au:core 2014-10-27 5:51 ` Joe Perches @ 2014-10-27 6:16 ` Jes Sorensen 2014-10-27 6:23 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: Jes Sorensen @ 2014-10-27 6:16 UTC (permalink / raw) To: Joe Perches Cc: Paul McQuade, linux-kernel, devel, linux-wireless, gregkh, Larry.Finger Joe Perches <joe@perches.com> writes: > On Mon, 2014-10-27 at 06:45 +0100, Jes Sorensen wrote: >> Joe Perches <joe@perches.com> writes: >> > On Sun, 2014-10-26 at 16:18 +0000, Paul McQuade wrote: >> >> ERROR: spaces required around that ':' (ctx:VxE) > [] >> >> diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c >> >> b/drivers/staging/rtl8723au/core/rtw_ieee80211.c > [] >> >> @@ -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) : > [] >> > >> > 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. It's wrong, so just don't do it here! Thank you, Jes ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] staging: rtl8723au:core 2014-10-27 6:16 ` Jes Sorensen @ 2014-10-27 6:23 ` Joe Perches 2014-10-27 6:44 ` Jes Sorensen 0 siblings, 1 reply; 8+ messages in thread From: Joe Perches @ 2014-10-27 6:23 UTC (permalink / raw) To: Jes Sorensen Cc: Paul McQuade, linux-kernel, devel, linux-wireless, gregkh, Larry.Finger On Mon, 2014-10-27 at 07:16 +0100, Jes Sorensen wrote: > Joe Perches <joe@perches.com> writes: > > On Mon, 2014-10-27 at 06:45 +0100, Jes Sorensen wrote: > >> Joe Perches <joe@perches.com> writes: > >> > On Sun, 2014-10-26 at 16:18 +0000, Paul McQuade wrote: > >> >> ERROR: spaces required around that ':' (ctx:VxE) > > [] > >> >> diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c > >> >> b/drivers/staging/rtl8723au/core/rtw_ieee80211.c > > [] > >> >> @@ -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) : > > [] > >> > > >> > 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. > > It's wrong, so just don't do it here! It's not "wrong", you just don't like it. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] staging: rtl8723au:core 2014-10-27 6:23 ` Joe Perches @ 2014-10-27 6:44 ` Jes Sorensen 0 siblings, 0 replies; 8+ messages in thread From: Jes Sorensen @ 2014-10-27 6:44 UTC (permalink / raw) To: Joe Perches Cc: Paul McQuade, linux-kernel, devel, linux-wireless, gregkh, Larry.Finger Joe Perches <joe@perches.com> writes: > On Mon, 2014-10-27 at 07:16 +0100, Jes Sorensen wrote: >> Joe Perches <joe@perches.com> writes: >> > On Mon, 2014-10-27 at 06:45 +0100, Jes Sorensen wrote: >> >> Joe Perches <joe@perches.com> writes: >> >> > On Sun, 2014-10-26 at 16:18 +0000, Paul McQuade wrote: >> >> >> ERROR: spaces required around that ':' (ctx:VxE) >> > [] >> >> >> diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c >> >> >> b/drivers/staging/rtl8723au/core/rtw_ieee80211.c >> > [] >> >> >> @@ -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) : >> > [] >> >> > >> >> > 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. >> >> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] staging: rtl8723au:core 2014-10-26 16:18 [PATCH 2/2] staging: rtl8723au:core Paul McQuade 2014-10-26 19:04 ` Joe Perches @ 2014-10-29 9:35 ` Greg KH 1 sibling, 0 replies; 8+ messages in thread From: Greg KH @ 2014-10-29 9:35 UTC (permalink / raw) To: Paul McQuade Cc: devel, linux-wireless, linux-kernel, Jes.Sorensen, Larry.Finger On Sun, Oct 26, 2014 at 04:18:52PM +0000, Paul McQuade wrote: > ERROR: spaces required around that ':' (ctx:VxE) > > Signed-off-by: Paul McQuade <paulmcquad@gmail.com> > --- > drivers/staging/rtl8723au/core/rtw_ieee80211.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Your Subject: makes no sense :( ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-10-29 15:42 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-26 16:18 [PATCH 2/2] staging: rtl8723au:core Paul McQuade 2014-10-26 19:04 ` Joe Perches 2014-10-27 5:45 ` Jes Sorensen 2014-10-27 5:51 ` Joe Perches 2014-10-27 6:16 ` Jes Sorensen 2014-10-27 6:23 ` Joe Perches 2014-10-27 6:44 ` Jes Sorensen 2014-10-29 9:35 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).