linux-kernel - Re: [PATCH v3] staging: vt6655: Macro with braces issue change to inline function

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]

Date:   Mon, 6 Feb 2023 00:39:08 +0100
From:   Guru Mehar Rachaputi <gurumeharrachaputi@...il.com>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     Forest Bond <forest@...ttletooquiet.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] staging: vt6655: Macro with braces issue change to
 inline function

On Sun, Feb 05, 2023 at 08:12:31PM +0100, Christophe JAILLET wrote:
> Le 05/02/2023 à 19:11, Guru Mehar Rachaputi a écrit :
> > This patch is to fix checkpatch warning: "Macro argument 'iobase' may be better
> > as '(iobase)' to avoid precedence issues" changed to inline function. In
> > relation to this, names of the callers of macro are also modified to call
> > this function.
> > 
> > Signed-off-by: Guru Mehar Rachaputi <gurumeharrachaputi@...il.com>
> 
> Hi,
> 
> this patch should be v4.
> You re-sent it with a modified commit message (the position of your S-o-b)
> 
> The idea behind patch versions is to help maintainer. With the way you did,
> now 2 patches stating v3 are available.
> Which one is the correct one?
> The maintainer would need to look at both, search for differences, maybe
> look at the date of the mails.
> A v4 would be much easier for him.
> 
> 
> Also, when you send an updated version of a patch, it should always be
> "complete". I mean that the patch below seems to need v2, and maybe even v1
> (which is apparently not on the linux-kernel mailing list).
> 
> A maintainer can't know by himself what is needed and what is not.
> 
> So you should resend a new patch.
> It should be a v4, and it should include what is needed from (v1?), v2 and
> v3 all together.
> 
> CJ
> 
> 
> > ---
> > Changes in v3:
> > 	- Whitespace error from checkpatch fixed
> > 
> > Changes in v2:
> > 	- Macros with one statement that is to call 'iowrite8' function changed
> > 	to inline function as reviewed by gregkh@...uxfoundation.org.
> > 	In relation to this, names of the callers of macro are also modified
> > 	to call this function.
> > ---
> >   drivers/staging/vt6655/card.c    | 3 +--
> >   drivers/staging/vt6655/channel.c | 2 +-
> >   drivers/staging/vt6655/mac.h     | 4 ++--
> >   3 files changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c
> > index a6ff496b01b6..d2d122dc16d8 100644
> > --- a/drivers/staging/vt6655/card.c
> > +++ b/drivers/staging/vt6655/card.c
> > @@ -643,8 +643,7 @@ void CARDvSetRSPINF(struct vnt_private *priv, u8 bb_type)
> >   				   &byRsvTime);
> >   	iowrite16(MAKEWORD(byTxRate, byRsvTime), priv->port_offset + MAC_REG_RSPINF_A_72);
> >   	/* Set to Page0 */
> > -        vt6655_mac_select_page0(priv->port_offset);
> > -
> > +	vt6655_mac_select_page0(priv->port_offset);
> >   	spin_unlock_irqrestore(&priv->lock, flags);
> >   }
> > diff --git a/drivers/staging/vt6655/channel.c b/drivers/staging/vt6655/channel.c
> > index e9a44bcebe32..60b445c38424 100644
> > --- a/drivers/staging/vt6655/channel.c
> > +++ b/drivers/staging/vt6655/channel.c
> > @@ -121,7 +121,7 @@ bool set_channel(struct vnt_private *priv, struct ieee80211_channel *ch)
> >   		iowrite8(priv->byCurPwr, priv->port_offset + MAC_REG_PWRCCK);
> >   		RFbSetPower(priv, RATE_6M, priv->byCurrentCh);
> >   		iowrite8(priv->byCurPwr, priv->port_offset + MAC_REG_PWROFDM);
> > -	        vt6655_mac_select_page0(priv->port_offset);
> > +		vt6655_mac_select_page0(priv->port_offset);
> >   		spin_unlock_irqrestore(&priv->lock, flags);
> >   	}
> > diff --git a/drivers/staging/vt6655/mac.h b/drivers/staging/vt6655/mac.h
> > index b9a7ca0fe604..ae3064303691 100644
> > --- a/drivers/staging/vt6655/mac.h
> > +++ b/drivers/staging/vt6655/mac.h
> > @@ -539,12 +539,12 @@
> >   static inline void vt6655_mac_select_page0(void __iomem *iobase)
> >   {
> > -        iowrite8(0, iobase + MAC_REG_PAGE1SEL);
> > +	iowrite8(0, iobase + MAC_REG_PAGE1SEL);
> >   }
> >   static inline void  vt6655_mac_select_page1(void __iomem *iobase)
> >   {
> > -        iowrite8(1, iobase + MAC_REG_PAGE1SEL);
> > +	iowrite8(1, iobase + MAC_REG_PAGE1SEL);
> >   }
> >   #define MAKEWORD(lb, hb) \
> 

Thanks for the explaination.
Since I amended commit message and thought as there is no new commit it
should still be the same patch.

Is it ok if I send a new patchset based on the previous conversations?
I have four commits now, 4th commit being just the commit message and
this patchset doesn't have s-o-b issue.

or

should I undo all the amends?

-- 
Thanks & Regards,
Guru