diff mbox series

[21/22] rtw89: Replace comments with C99 initializers

Message ID 20220326165909.506926-21-benni@stuerz.xyz (mailing list archive)
State Changes Requested
Headers show
Series [01/22] orion5x: Replace comments with C99 initializers | expand

Commit Message

Benjamin Stürz March 26, 2022, 4:59 p.m. UTC
This replaces comments with C99's designated
initializers because the kernel supports them now.

Signed-off-by: Benjamin Stürz <benni@stuerz.xyz>
---
 drivers/net/wireless/realtek/rtw89/coex.c | 40 +++++++++++------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Comments

Larry Finger March 26, 2022, 6:55 p.m. UTC | #1
On 3/26/22 11:59, Benjamin Stürz wrote:
> This replaces comments with C99's designated
> initializers because the kernel supports them now.
> 
> Signed-off-by: Benjamin Stürz <benni@stuerz.xyz>
> ---
>   drivers/net/wireless/realtek/rtw89/coex.c | 40 +++++++++++------------
>   1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/coex.c b/drivers/net/wireless/realtek/rtw89/coex.c
> index 684583955511..3c83a0bfb120 100644
> --- a/drivers/net/wireless/realtek/rtw89/coex.c
> +++ b/drivers/net/wireless/realtek/rtw89/coex.c
> @@ -97,26 +97,26 @@ static const struct rtw89_btc_fbtc_slot s_def[] = {
>   };
>   
>   static const u32 cxtbl[] = {
> -	0xffffffff, /* 0 */
> -	0xaaaaaaaa, /* 1 */
> -	0x55555555, /* 2 */
> -	0x66555555, /* 3 */
> -	0x66556655, /* 4 */
> -	0x5a5a5a5a, /* 5 */
> -	0x5a5a5aaa, /* 6 */
> -	0xaa5a5a5a, /* 7 */
> -	0x6a5a5a5a, /* 8 */
> -	0x6a5a5aaa, /* 9 */
> -	0x6a5a6a5a, /* 10 */
> -	0x6a5a6aaa, /* 11 */
> -	0x6afa5afa, /* 12 */
> -	0xaaaa5aaa, /* 13 */
> -	0xaaffffaa, /* 14 */
> -	0xaa5555aa, /* 15 */
> -	0xfafafafa, /* 16 */
> -	0xffffddff, /* 17 */
> -	0xdaffdaff, /* 18 */
> -	0xfafadafa  /* 19 */
> +	[0]  = 0xffffffff,
> +	[1]  = 0xaaaaaaaa,
> +	[2]  = 0x55555555,
> +	[3]  = 0x66555555,
> +	[4]  = 0x66556655,
> +	[5]  = 0x5a5a5a5a,
> +	[6]  = 0x5a5a5aaa,
> +	[7]  = 0xaa5a5a5a,
> +	[8]  = 0x6a5a5a5a,
> +	[9]  = 0x6a5a5aaa,
> +	[10] = 0x6a5a6a5a,
> +	[11] = 0x6a5a6aaa,
> +	[12] = 0x6afa5afa,
> +	[13] = 0xaaaa5aaa,
> +	[14] = 0xaaffffaa,
> +	[15] = 0xaa5555aa,
> +	[16] = 0xfafafafa,
> +	[17] = 0xffffddff,
> +	[18] = 0xdaffdaff,
> +	[19] = 0xfafadafa
>   };
>   
>   struct rtw89_btc_btf_tlv {


Is this change really necessary? Yes, the entries must be ordered; however, the 
comment carries that information at very few extra characters. To me, this patch 
looks like unneeded source churn. One other concern is that this driver is 
backported to older kernels and older compilers by several distros. Will this 
change require adding extra conditional statements to the source used in these 
applications?

Larry
Kalle Valo March 28, 2022, 9:28 a.m. UTC | #2
Larry Finger <Larry.Finger@lwfinger.net> writes:

> On 3/26/22 11:59, Benjamin Stürz wrote:
>> This replaces comments with C99's designated
>> initializers because the kernel supports them now.
>>
>> Signed-off-by: Benjamin Stürz <benni@stuerz.xyz>
>> ---
>>   drivers/net/wireless/realtek/rtw89/coex.c | 40 +++++++++++------------
>>   1 file changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw89/coex.c b/drivers/net/wireless/realtek/rtw89/coex.c
>> index 684583955511..3c83a0bfb120 100644
>> --- a/drivers/net/wireless/realtek/rtw89/coex.c
>> +++ b/drivers/net/wireless/realtek/rtw89/coex.c
>> @@ -97,26 +97,26 @@ static const struct rtw89_btc_fbtc_slot s_def[] = {
>>   };
>>     static const u32 cxtbl[] = {
>> -	0xffffffff, /* 0 */
>> -	0xaaaaaaaa, /* 1 */
>> -	0x55555555, /* 2 */
>> -	0x66555555, /* 3 */
>> -	0x66556655, /* 4 */
>> -	0x5a5a5a5a, /* 5 */
>> -	0x5a5a5aaa, /* 6 */
>> -	0xaa5a5a5a, /* 7 */
>> -	0x6a5a5a5a, /* 8 */
>> -	0x6a5a5aaa, /* 9 */
>> -	0x6a5a6a5a, /* 10 */
>> -	0x6a5a6aaa, /* 11 */
>> -	0x6afa5afa, /* 12 */
>> -	0xaaaa5aaa, /* 13 */
>> -	0xaaffffaa, /* 14 */
>> -	0xaa5555aa, /* 15 */
>> -	0xfafafafa, /* 16 */
>> -	0xffffddff, /* 17 */
>> -	0xdaffdaff, /* 18 */
>> -	0xfafadafa  /* 19 */
>> +	[0]  = 0xffffffff,
>> +	[1]  = 0xaaaaaaaa,
>> +	[2]  = 0x55555555,
>> +	[3]  = 0x66555555,
>> +	[4]  = 0x66556655,
>> +	[5]  = 0x5a5a5a5a,
>> +	[6]  = 0x5a5a5aaa,
>> +	[7]  = 0xaa5a5a5a,
>> +	[8]  = 0x6a5a5a5a,
>> +	[9]  = 0x6a5a5aaa,
>> +	[10] = 0x6a5a6a5a,
>> +	[11] = 0x6a5a6aaa,
>> +	[12] = 0x6afa5afa,
>> +	[13] = 0xaaaa5aaa,
>> +	[14] = 0xaaffffaa,
>> +	[15] = 0xaa5555aa,
>> +	[16] = 0xfafafafa,
>> +	[17] = 0xffffddff,
>> +	[18] = 0xdaffdaff,
>> +	[19] = 0xfafadafa
>>   };
>>     struct rtw89_btc_btf_tlv {
>
>
> Is this change really necessary? Yes, the entries must be ordered;
> however, the comment carries that information at very few extra
> characters. To me, this patch looks like unneeded source churn.

One small benefit I see is to avoid the comment index being wrong and
there would be no way to catch that. But otherwise I don't have any
opinion about this.
David Laight March 28, 2022, 12:21 p.m. UTC | #3
From: Kalle Valo
> Sent: 28 March 2022 10:29
> 
> Larry Finger <Larry.Finger@lwfinger.net> writes:
> 
> > On 3/26/22 11:59, Benjamin Stürz wrote:
> >> This replaces comments with C99's designated
> >> initializers because the kernel supports them now.
> >>
> >> Signed-off-by: Benjamin Stürz <benni@stuerz.xyz>
> >> ---
> >>   drivers/net/wireless/realtek/rtw89/coex.c | 40 +++++++++++------------
> >>   1 file changed, 20 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtw89/coex.c b/drivers/net/wireless/realtek/rtw89/coex.c
> >> index 684583955511..3c83a0bfb120 100644
> >> --- a/drivers/net/wireless/realtek/rtw89/coex.c
> >> +++ b/drivers/net/wireless/realtek/rtw89/coex.c
> >> @@ -97,26 +97,26 @@ static const struct rtw89_btc_fbtc_slot s_def[] = {
> >>   };
> >>     static const u32 cxtbl[] = {
> >> -	0xffffffff, /* 0 */
> >> -	0xaaaaaaaa, /* 1 */
> >> -	0x55555555, /* 2 */
> >> -	0x66555555, /* 3 */
> >> -	0x66556655, /* 4 */
> >> -	0x5a5a5a5a, /* 5 */
> >> -	0x5a5a5aaa, /* 6 */
> >> -	0xaa5a5a5a, /* 7 */
> >> -	0x6a5a5a5a, /* 8 */
> >> -	0x6a5a5aaa, /* 9 */
> >> -	0x6a5a6a5a, /* 10 */
> >> -	0x6a5a6aaa, /* 11 */
> >> -	0x6afa5afa, /* 12 */
> >> -	0xaaaa5aaa, /* 13 */
> >> -	0xaaffffaa, /* 14 */
> >> -	0xaa5555aa, /* 15 */
> >> -	0xfafafafa, /* 16 */
> >> -	0xffffddff, /* 17 */
> >> -	0xdaffdaff, /* 18 */
> >> -	0xfafadafa  /* 19 */
> >> +	[0]  = 0xffffffff,
> >> +	[1]  = 0xaaaaaaaa,
> >> +	[2]  = 0x55555555,
> >> +	[3]  = 0x66555555,
> >> +	[4]  = 0x66556655,
> >> +	[5]  = 0x5a5a5a5a,
> >> +	[6]  = 0x5a5a5aaa,
> >> +	[7]  = 0xaa5a5a5a,
> >> +	[8]  = 0x6a5a5a5a,
> >> +	[9]  = 0x6a5a5aaa,
> >> +	[10] = 0x6a5a6a5a,
> >> +	[11] = 0x6a5a6aaa,
> >> +	[12] = 0x6afa5afa,
> >> +	[13] = 0xaaaa5aaa,
> >> +	[14] = 0xaaffffaa,
> >> +	[15] = 0xaa5555aa,
> >> +	[16] = 0xfafafafa,
> >> +	[17] = 0xffffddff,
> >> +	[18] = 0xdaffdaff,
> >> +	[19] = 0xfafadafa
> >>   };
> >>     struct rtw89_btc_btf_tlv {
> >
> >
> > Is this change really necessary? Yes, the entries must be ordered;
> > however, the comment carries that information at very few extra
> > characters. To me, this patch looks like unneeded source churn.
> 
> One small benefit I see is to avoid the comment index being wrong and
> there would be no way to catch that. But otherwise I don't have any
> opinion about this.

If the [nn] are wrong the effect is probably worse.
You really don't want a gap!

Doesn't seem worth using C99 initialisers unless they are
#defines or enum values.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw89/coex.c b/drivers/net/wireless/realtek/rtw89/coex.c
index 684583955511..3c83a0bfb120 100644
--- a/drivers/net/wireless/realtek/rtw89/coex.c
+++ b/drivers/net/wireless/realtek/rtw89/coex.c
@@ -97,26 +97,26 @@  static const struct rtw89_btc_fbtc_slot s_def[] = {
 };
 
 static const u32 cxtbl[] = {
-	0xffffffff, /* 0 */
-	0xaaaaaaaa, /* 1 */
-	0x55555555, /* 2 */
-	0x66555555, /* 3 */
-	0x66556655, /* 4 */
-	0x5a5a5a5a, /* 5 */
-	0x5a5a5aaa, /* 6 */
-	0xaa5a5a5a, /* 7 */
-	0x6a5a5a5a, /* 8 */
-	0x6a5a5aaa, /* 9 */
-	0x6a5a6a5a, /* 10 */
-	0x6a5a6aaa, /* 11 */
-	0x6afa5afa, /* 12 */
-	0xaaaa5aaa, /* 13 */
-	0xaaffffaa, /* 14 */
-	0xaa5555aa, /* 15 */
-	0xfafafafa, /* 16 */
-	0xffffddff, /* 17 */
-	0xdaffdaff, /* 18 */
-	0xfafadafa  /* 19 */
+	[0]  = 0xffffffff,
+	[1]  = 0xaaaaaaaa,
+	[2]  = 0x55555555,
+	[3]  = 0x66555555,
+	[4]  = 0x66556655,
+	[5]  = 0x5a5a5a5a,
+	[6]  = 0x5a5a5aaa,
+	[7]  = 0xaa5a5a5a,
+	[8]  = 0x6a5a5a5a,
+	[9]  = 0x6a5a5aaa,
+	[10] = 0x6a5a6a5a,
+	[11] = 0x6a5a6aaa,
+	[12] = 0x6afa5afa,
+	[13] = 0xaaaa5aaa,
+	[14] = 0xaaffffaa,
+	[15] = 0xaa5555aa,
+	[16] = 0xfafafafa,
+	[17] = 0xffffddff,
+	[18] = 0xdaffdaff,
+	[19] = 0xfafadafa
 };
 
 struct rtw89_btc_btf_tlv {