linux-kernel - Re: [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <157ee66fd0e3304c238e7ad8123277892e0d1132.camel@perches.com>
Date:   Wed, 20 Oct 2021 07:09:51 -0700
From:   Joe Perches <joe@...ches.com>
To:     Karolina Drobnik <karolinadrobnik@...il.com>,
        outreachy-kernel@...glegroups.com
Cc:     gregkh@...uxfoundation.org, forest@...ttletooquiet.net,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: vt6655: Rename `dwAL2230InitTable` array

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++) {