linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array
@ 2021-10-20 13:28 Karolina Drobnik
  2021-10-20 13:44 ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Karolina Drobnik @ 2021-10-20 13:28 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: gregkh, forest, linux-staging, linux-kernel, Karolina Drobnik

Drop Hungarian notation prefix in `dwAL2230InitTable` array.
Change it to use snake case.

Fix issue detected by checkpatch.pl:
  CHECK: Avoid CamelCase: <dwAL2230InitTable>

Signed-off-by: Karolina Drobnik <karolinadrobnik@gmail.com>
---
 drivers/staging/vt6655/rf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
index a6f17162d017..6bdacf8fbc27 100644
--- a/drivers/staging/vt6655/rf.c
+++ b/drivers/staging/vt6655/rf.c
@@ -33,7 +33,7 @@
 #define SWITCH_CHANNEL_DELAY_AL7230 200 /* us */
 #define AL7230_PWR_IDX_LEN    64
 
-static const unsigned long dwAL2230InitTable[CB_AL2230_INIT_SEQ] = {
+static const unsigned long al2230_init_table[CB_AL2230_INIT_SEQ] = {
 	0x03F79000 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
 	0x03333100 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
 	0x01A00200 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
@@ -545,7 +545,7 @@ static bool RFbAL2230Init(struct vnt_private *priv)
 	IFRFbWriteEmbedded(priv, (0x07168700 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW));
 
 	for (ii = 0; ii < CB_AL2230_INIT_SEQ; ii++)
-		ret &= IFRFbWriteEmbedded(priv, dwAL2230InitTable[ii]);
+		ret &= IFRFbWriteEmbedded(priv, al2230_init_table[ii]);
 	MACvTimer0MicroSDelay(priv, 30); /* delay 30 us */
 
 	/* PLL On */
@@ -557,7 +557,7 @@ static bool RFbAL2230Init(struct vnt_private *priv)
 	ret &= IFRFbWriteEmbedded(priv, (0x00780f00 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW));
 	MACvTimer0MicroSDelay(priv, 30);/* 30us */
 	ret &= IFRFbWriteEmbedded(priv,
-				  dwAL2230InitTable[CB_AL2230_INIT_SEQ - 1]);
+				  al2230_init_table[CB_AL2230_INIT_SEQ - 1]);
 
 	MACvWordRegBitsOn(iobase, MAC_REG_SOFTPWRCTL, (SOFTPWRCTL_SWPE3    |
 							 SOFTPWRCTL_SWPE2    |
@@ -699,7 +699,7 @@ bool RFvWriteWakeProgSyn(struct vnt_private *priv, unsigned char byRFType,
 			return false;
 
 		for (ii = 0; ii < CB_AL2230_INIT_SEQ; ii++)
-			MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230InitTable[ii]);
+			MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), al2230_init_table[ii]);
 
 		MACvSetMISCFifo(priv, (unsigned short)(MISCFIFO_SYNDATA_IDX + ii), dwAL2230ChannelTable0[uChannel - 1]);
 		ii++;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array
  2021-10-20 13:28 [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array Karolina Drobnik
@ 2021-10-20 13:44 ` Joe Perches
  2021-10-20 14:09   ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2021-10-20 13:44 UTC (permalink / raw)
  To: Karolina Drobnik, outreachy-kernel
  Cc: gregkh, forest, linux-staging, linux-kernel

On Wed, 2021-10-20 at 14:28 +0100, Karolina Drobnik wrote:
> Drop Hungarian notation prefix in `dwAL2230InitTable` array.
> Change it to use snake case.
> 
> Fix issue detected by checkpatch.pl:
>   CHECK: Avoid CamelCase: <dwAL2230InitTable>

Seems fine.

trivial suggestion:

> diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
[]
> @@ -33,7 +33,7 @@
>  #define SWITCH_CHANNEL_DELAY_AL7230 200 /* us */
>  #define AL7230_PWR_IDX_LEN    64
>  
> -static const unsigned long dwAL2230InitTable[CB_AL2230_INIT_SEQ] = {
> +static const unsigned long al2230_init_table[CB_AL2230_INIT_SEQ] = {
>  	0x03F79000 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
>  	0x03333100 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
>  	0x01A00200 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,

In this file there are more than 100 uses of

	(BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW

Maybe add a define for it and substitute the uses for the define.



^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array
  2021-10-20 13:44 ` Joe Perches
@ 2021-10-20 14:09   ` Joe Perches
  2021-10-20 14:16     ` [Outreachy kernel] " Julia Lawall
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2021-10-20 14:09 UTC (permalink / raw)
  To: Karolina Drobnik, outreachy-kernel
  Cc: gregkh, forest, linux-staging, linux-kernel

On Wed, 2021-10-20 at 06:44 -0700, Joe Perches wrote:
> trivial suggestion:
> 
> > diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
> []
> > @@ -33,7 +33,7 @@
> >  #define SWITCH_CHANNEL_DELAY_AL7230 200 /* us */
> >  #define AL7230_PWR_IDX_LEN    64
> >  
> > -static const unsigned long dwAL2230InitTable[CB_AL2230_INIT_SEQ] = {
> > +static const unsigned long al2230_init_table[CB_AL2230_INIT_SEQ] = {
> >  	0x03F79000 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
> >  	0x03333100 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
> >  	0x01A00200 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
> 
> In this file there are more than 100 uses of
> 
> 	(BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW
> 
> Maybe add a define for it and substitute the uses for the define.

Look at the code too.

It looks as if every use of IFRFbWriteEmbedded() has this added to
the 2nd argument and that the 2nd argument isn't used anywhere else.

Maybe remove it altogether and add it to IFRFbWriteEmbedded().

And it looks as if the + uses for these should logically be |

Something like:

diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
index 0dae593c6944f..26803f6f9a27b 100644
--- a/drivers/staging/vt6655/rf.c
+++ b/drivers/staging/vt6655/rf.c
@@ -498,7 +498,8 @@ bool IFRFbWriteEmbedded(struct vnt_private *priv, unsigned long dwData)
        unsigned short ww;
        unsigned long dwValue;
 
-       VNSvOutPortD(iobase + MAC_REG_IFREGCTL, dwData);
+       VNSvOutPortD(iobase + MAC_REG_IFREGCTL,
+                    dwData | (BY_AL2230_REG_LEN << 3) | IFREGCTL_REGW);
 
        /* W_MAX_TIMEOUT is the timeout period */
        for (ww = 0; ww < W_MAX_TIMEOUT; ww++) {



^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array
  2021-10-20 14:09   ` Joe Perches
@ 2021-10-20 14:16     ` Julia Lawall
  2021-10-20 15:56       ` Karolina Drobnik
  0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2021-10-20 14:16 UTC (permalink / raw)
  To: Joe Perches
  Cc: Karolina Drobnik, outreachy-kernel, gregkh, forest,
	linux-staging, linux-kernel



On Wed, 20 Oct 2021, Joe Perches wrote:

> On Wed, 2021-10-20 at 06:44 -0700, Joe Perches wrote:
> > trivial suggestion:
> >
> > > diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
> > []
> > > @@ -33,7 +33,7 @@
> > >  #define SWITCH_CHANNEL_DELAY_AL7230 200 /* us */
> > >  #define AL7230_PWR_IDX_LEN    64
> > >
> > > -static const unsigned long dwAL2230InitTable[CB_AL2230_INIT_SEQ] = {
> > > +static const unsigned long al2230_init_table[CB_AL2230_INIT_SEQ] = {
> > >  	0x03F79000 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
> > >  	0x03333100 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
> > >  	0x01A00200 + (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW,
> >
> > In this file there are more than 100 uses of
> >
> > 	(BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW
> >
> > Maybe add a define for it and substitute the uses for the define.
>
> Look at the code too.
>
> It looks as if every use of IFRFbWriteEmbedded() has this added to
> the 2nd argument and that the 2nd argument isn't used anywhere else.
>
> Maybe remove it altogether and add it to IFRFbWriteEmbedded().
>
> And it looks as if the + uses for these should logically be |
>
> Something like:
>
> diff --git a/drivers/staging/vt6655/rf.c b/drivers/staging/vt6655/rf.c
> index 0dae593c6944f..26803f6f9a27b 100644
> --- a/drivers/staging/vt6655/rf.c
> +++ b/drivers/staging/vt6655/rf.c
> @@ -498,7 +498,8 @@ bool IFRFbWriteEmbedded(struct vnt_private *priv, unsigned long dwData)
>         unsigned short ww;
>         unsigned long dwValue;
>
> -       VNSvOutPortD(iobase + MAC_REG_IFREGCTL, dwData);
> +       VNSvOutPortD(iobase + MAC_REG_IFREGCTL,
> +                    dwData | (BY_AL2230_REG_LEN << 3) | IFREGCTL_REGW);
>
>         /* W_MAX_TIMEOUT is the timeout period */
>         for (ww = 0; ww < W_MAX_TIMEOUT; ww++) {

Karolina,

If more than a hundred such changes are necessary, you may find Coccinelle
useful:  https://coccinelle.gitlabpages.inria.fr/website/

julia


>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/157ee66fd0e3304c238e7ad8123277892e0d1132.camel%40perches.com.
>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array
  2021-10-20 14:16     ` [Outreachy kernel] " Julia Lawall
@ 2021-10-20 15:56       ` Karolina Drobnik
  0 siblings, 0 replies; 5+ messages in thread
From: Karolina Drobnik @ 2021-10-20 15:56 UTC (permalink / raw)
  To: Julia Lawall, Joe Perches
  Cc: outreachy-kernel, gregkh, forest, linux-staging, linux-kernel

On Wed, 2021-10-20 at 07:09 -0700, Joe Perches wrote:
> On Wed, 2021-10-20 at 06:44 -0700, Joe Perches wrote:
> > In this file there are more than 100 uses of
> > 
> >         (BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW
> > 
> > Maybe add a define for it and substitute the uses for the define.
> 
> Look at the code too.
> 
> It looks as if every use of IFRFbWriteEmbedded() has this added to
> the 2nd argument and that the 2nd argument isn't used anywhere else.
> 
> Maybe remove it altogether and add it to IFRFbWriteEmbedded().
> 
> And it looks as if the + uses for these should logically be |

Sounds like a very good idea. I'll add it to my to-do list, thanks :)

On Wed, 2021-10-20 at 16:16 +0200, Julia Lawall wrote:
> Karolina,
> 
> If more than a hundred such changes are necessary, you may find
> Coccinelle
> useful:  https://coccinelle.gitlabpages.inria.fr/website/

Hmm, I'll check this out, it may come in handy.


Thanks,
Karolina


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-20 15:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 13:28 [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array Karolina Drobnik
2021-10-20 13:44 ` Joe Perches
2021-10-20 14:09   ` Joe Perches
2021-10-20 14:16     ` [Outreachy kernel] " Julia Lawall
2021-10-20 15:56       ` Karolina Drobnik

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).