All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: vt6655: change the function name s_vFillRTSHead
@ 2022-11-03  9:52 Tanjuate Brunostar
  2022-11-03 12:38 ` Julia Lawall
  0 siblings, 1 reply; 8+ messages in thread
From: Tanjuate Brunostar @ 2022-11-03  9:52 UTC (permalink / raw)
  To: gregkh, linux-staging, linux-kernel, outreachy

Remove the use of Hungarian notation, which is not used in the Linux
kernel. Reported by checkpatch

Fix checkpatch error related to code line ends with a '(', by joining
some lines and indenting correctly. This improves visibility

Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
---
 drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 31ae99b3cb35..debc5d5daede 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -23,7 +23,7 @@
  *      s_uGetTxRsvTime- get frame reserved time
  *      s_vFillCTSHead- fulfill CTS ctl header
  *      s_vFillFragParameter- Set fragment ctl parameter.
- *      s_vFillRTSHead- fulfill RTS ctl header
+ *      fill_rts_header- fulfill RTS ctl header
  *      s_vFillTxKey- fulfill tx encrypt key
  *      s_vSWencryption- Software encrypt header
  *      vDMA0_tx_80211- tx 802.11 frame via dma0
@@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
 #define DATADUR_A_F1    13
 
 /*---------------------  Static Functions  --------------------------*/
-static void s_vFillRTSHead(struct vnt_private *pDevice,
-			   unsigned char byPktType,
-			   void *pvRTS,
-			   unsigned int	cbFrameLength,
-			   bool bNeedAck,
-			   bool bDisCRC,
-			   struct ieee80211_hdr *hdr,
-			   unsigned short wCurrentRate,
-			   unsigned char byFBOption);
+static void fill_rts_header(struct vnt_private *pDevice,
+			    unsigned char byPktType,
+			    void *pvRTS,
+			    unsigned int	cbFrameLength,
+			    bool bNeedAck,
+			    bool bDisCRC,
+			    struct ieee80211_hdr *hdr,
+			    unsigned short wCurrentRate,
+			    unsigned char byFBOption);
 
 static void s_vGenerateTxParameter(struct vnt_private *pDevice,
 				   unsigned char byPktType,
@@ -555,19 +555,15 @@ s_uFillDataHead(
 	return buf->duration;
 }
 
-static
-void
-s_vFillRTSHead(
-	struct vnt_private *pDevice,
-	unsigned char byPktType,
-	void *pvRTS,
-	unsigned int cbFrameLength,
-	bool bNeedAck,
-	bool bDisCRC,
-	struct ieee80211_hdr *hdr,
-	unsigned short wCurrentRate,
-	unsigned char byFBOption
-)
+static void fill_rts_header(struct vnt_private *pDevice,
+			    unsigned char byPktType,
+			    void *pvRTS,
+			    unsigned int cbFrameLength,
+			    bool bNeedAck,
+			    bool bDisCRC,
+			    struct ieee80211_hdr *hdr,
+			    unsigned short wCurrentRate,
+			    unsigned char byFBOption)
 {
 	unsigned int uRTSFrameLen = 20;
 
@@ -912,7 +908,7 @@ s_vGenerateTxParameter(
 			buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
 			buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
 
-			s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
+			fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
 		} else {/* RTS_needless, PCF mode */
 			struct vnt_rrv_time_cts *buf = pvRrvTime;
 
@@ -931,7 +927,7 @@ s_vGenerateTxParameter(
 			buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
 
 			/* Fill RTS */
-			s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
+			fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
 		} else if (!pvRTS) {/* RTS_needless, non PCF mode */
 			struct vnt_rrv_time_ab *buf = pvRrvTime;
 
@@ -945,7 +941,7 @@ s_vGenerateTxParameter(
 			buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
 
 			/* Fill RTS */
-			s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
+			fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
 		} else { /* RTS_needless, non PCF mode */
 			struct vnt_rrv_time_ab *buf = pvRrvTime;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vt6655: change the function name s_vFillRTSHead
  2022-11-03  9:52 [PATCH] staging: vt6655: change the function name s_vFillRTSHead Tanjuate Brunostar
@ 2022-11-03 12:38 ` Julia Lawall
  2022-11-03 15:49   ` Tanju Brunostar
  0 siblings, 1 reply; 8+ messages in thread
From: Julia Lawall @ 2022-11-03 12:38 UTC (permalink / raw)
  To: Tanjuate Brunostar; +Cc: gregkh, linux-staging, linux-kernel, outreachy



On Thu, 3 Nov 2022, Tanjuate Brunostar wrote:

> Remove the use of Hungarian notation, which is not used in the Linux
> kernel. Reported by checkpatch
>
> Fix checkpatch error related to code line ends with a '(', by joining
> some lines and indenting correctly. This improves visibility

I don't think it will be appreciated to do two things at once.

>
> Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> ---
>  drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> index 31ae99b3cb35..debc5d5daede 100644
> --- a/drivers/staging/vt6655/rxtx.c
> +++ b/drivers/staging/vt6655/rxtx.c
> @@ -23,7 +23,7 @@
>   *      s_uGetTxRsvTime- get frame reserved time
>   *      s_vFillCTSHead- fulfill CTS ctl header
>   *      s_vFillFragParameter- Set fragment ctl parameter.
> - *      s_vFillRTSHead- fulfill RTS ctl header
> + *      fill_rts_header- fulfill RTS ctl header
>   *      s_vFillTxKey- fulfill tx encrypt key
>   *      s_vSWencryption- Software encrypt header
>   *      vDMA0_tx_80211- tx 802.11 frame via dma0
> @@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
>  #define DATADUR_A_F1    13
>
>  /*---------------------  Static Functions  --------------------------*/
> -static void s_vFillRTSHead(struct vnt_private *pDevice,
> -			   unsigned char byPktType,
> -			   void *pvRTS,
> -			   unsigned int	cbFrameLength,
> -			   bool bNeedAck,
> -			   bool bDisCRC,
> -			   struct ieee80211_hdr *hdr,
> -			   unsigned short wCurrentRate,
> -			   unsigned char byFBOption);
> +static void fill_rts_header(struct vnt_private *pDevice,
> +			    unsigned char byPktType,
> +			    void *pvRTS,
> +			    unsigned int	cbFrameLength,

Strange spacing in the above parameter declaration.  Maybe there is a tab
that should be a space?

julia

> +			    bool bNeedAck,
> +			    bool bDisCRC,
> +			    struct ieee80211_hdr *hdr,
> +			    unsigned short wCurrentRate,
> +			    unsigned char byFBOption);
>
>  static void s_vGenerateTxParameter(struct vnt_private *pDevice,
>  				   unsigned char byPktType,
> @@ -555,19 +555,15 @@ s_uFillDataHead(
>  	return buf->duration;
>  }
>
> -static
> -void
> -s_vFillRTSHead(
> -	struct vnt_private *pDevice,
> -	unsigned char byPktType,
> -	void *pvRTS,
> -	unsigned int cbFrameLength,
> -	bool bNeedAck,
> -	bool bDisCRC,
> -	struct ieee80211_hdr *hdr,
> -	unsigned short wCurrentRate,
> -	unsigned char byFBOption
> -)
> +static void fill_rts_header(struct vnt_private *pDevice,
> +			    unsigned char byPktType,
> +			    void *pvRTS,
> +			    unsigned int cbFrameLength,
> +			    bool bNeedAck,
> +			    bool bDisCRC,
> +			    struct ieee80211_hdr *hdr,
> +			    unsigned short wCurrentRate,
> +			    unsigned char byFBOption)
>  {
>  	unsigned int uRTSFrameLen = 20;
>
> @@ -912,7 +908,7 @@ s_vGenerateTxParameter(
>  			buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
>  			buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
>
> -			s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> +			fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
>  		} else {/* RTS_needless, PCF mode */
>  			struct vnt_rrv_time_cts *buf = pvRrvTime;
>
> @@ -931,7 +927,7 @@ s_vGenerateTxParameter(
>  			buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
>
>  			/* Fill RTS */
> -			s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> +			fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
>  		} else if (!pvRTS) {/* RTS_needless, non PCF mode */
>  			struct vnt_rrv_time_ab *buf = pvRrvTime;
>
> @@ -945,7 +941,7 @@ s_vGenerateTxParameter(
>  			buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
>
>  			/* Fill RTS */
> -			s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> +			fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
>  		} else { /* RTS_needless, non PCF mode */
>  			struct vnt_rrv_time_ab *buf = pvRrvTime;
>
> --
> 2.34.1
>
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vt6655: change the function name s_vFillRTSHead
  2022-11-03 12:38 ` Julia Lawall
@ 2022-11-03 15:49   ` Tanju Brunostar
  2022-11-03 15:58     ` Julia Lawall
  0 siblings, 1 reply; 8+ messages in thread
From: Tanju Brunostar @ 2022-11-03 15:49 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, linux-staging, linux-kernel, outreachy

On Thu, Nov 3, 2022 at 1:38 PM Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>
> On Thu, 3 Nov 2022, Tanjuate Brunostar wrote:
>
> > Remove the use of Hungarian notation, which is not used in the Linux
> > kernel. Reported by checkpatch
> >
> > Fix checkpatch error related to code line ends with a '(', by joining
> > some lines and indenting correctly. This improves visibility
>
> I don't think it will be appreciated to do two things at once.
>

I sent it earlier as two patches but it seemed to break and fix the
kernel which was not appreciated by one of the mentors

> >
> > Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> > ---
> >  drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
> >  1 file changed, 22 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > index 31ae99b3cb35..debc5d5daede 100644
> > --- a/drivers/staging/vt6655/rxtx.c
> > +++ b/drivers/staging/vt6655/rxtx.c
> > @@ -23,7 +23,7 @@
> >   *      s_uGetTxRsvTime- get frame reserved time
> >   *      s_vFillCTSHead- fulfill CTS ctl header
> >   *      s_vFillFragParameter- Set fragment ctl parameter.
> > - *      s_vFillRTSHead- fulfill RTS ctl header
> > + *      fill_rts_header- fulfill RTS ctl header
> >   *      s_vFillTxKey- fulfill tx encrypt key
> >   *      s_vSWencryption- Software encrypt header
> >   *      vDMA0_tx_80211- tx 802.11 frame via dma0
> > @@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
> >  #define DATADUR_A_F1    13
> >
> >  /*---------------------  Static Functions  --------------------------*/
> > -static void s_vFillRTSHead(struct vnt_private *pDevice,
> > -                        unsigned char byPktType,
> > -                        void *pvRTS,
> > -                        unsigned int cbFrameLength,
> > -                        bool bNeedAck,
> > -                        bool bDisCRC,
> > -                        struct ieee80211_hdr *hdr,
> > -                        unsigned short wCurrentRate,
> > -                        unsigned char byFBOption);
> > +static void fill_rts_header(struct vnt_private *pDevice,
> > +                         unsigned char byPktType,
> > +                         void *pvRTS,
> > +                         unsigned int        cbFrameLength,
>
> Strange spacing in the above parameter declaration.  Maybe there is a tab
> that should be a space?
>
> julia
>

I was also shocked when I noticed that. It seemed like a tab but when
I removed the space, the two words were joined and when I added the
space, it became this lage space once again. checkpatch does not
complain about it though

> > +                         bool bNeedAck,
> > +                         bool bDisCRC,
> > +                         struct ieee80211_hdr *hdr,
> > +                         unsigned short wCurrentRate,
> > +                         unsigned char byFBOption);
> >
> >  static void s_vGenerateTxParameter(struct vnt_private *pDevice,
> >                                  unsigned char byPktType,
> > @@ -555,19 +555,15 @@ s_uFillDataHead(
> >       return buf->duration;
> >  }
> >
> > -static
> > -void
> > -s_vFillRTSHead(
> > -     struct vnt_private *pDevice,
> > -     unsigned char byPktType,
> > -     void *pvRTS,
> > -     unsigned int cbFrameLength,
> > -     bool bNeedAck,
> > -     bool bDisCRC,
> > -     struct ieee80211_hdr *hdr,
> > -     unsigned short wCurrentRate,
> > -     unsigned char byFBOption
> > -)
> > +static void fill_rts_header(struct vnt_private *pDevice,
> > +                         unsigned char byPktType,
> > +                         void *pvRTS,
> > +                         unsigned int cbFrameLength,
> > +                         bool bNeedAck,
> > +                         bool bDisCRC,
> > +                         struct ieee80211_hdr *hdr,
> > +                         unsigned short wCurrentRate,
> > +                         unsigned char byFBOption)
> >  {
> >       unsigned int uRTSFrameLen = 20;
> >
> > @@ -912,7 +908,7 @@ s_vGenerateTxParameter(
> >                       buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> >                       buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
> >
> > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> >               } else {/* RTS_needless, PCF mode */
> >                       struct vnt_rrv_time_cts *buf = pvRrvTime;
> >
> > @@ -931,7 +927,7 @@ s_vGenerateTxParameter(
> >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> >
> >                       /* Fill RTS */
> > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> >               } else if (!pvRTS) {/* RTS_needless, non PCF mode */
> >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> >
> > @@ -945,7 +941,7 @@ s_vGenerateTxParameter(
> >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
> >
> >                       /* Fill RTS */
> > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> >               } else { /* RTS_needless, non PCF mode */
> >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> >
> > --
> > 2.34.1
> >
> >
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vt6655: change the function name s_vFillRTSHead
  2022-11-03 15:49   ` Tanju Brunostar
@ 2022-11-03 15:58     ` Julia Lawall
  2022-11-03 16:04       ` Tanju Brunostar
  0 siblings, 1 reply; 8+ messages in thread
From: Julia Lawall @ 2022-11-03 15:58 UTC (permalink / raw)
  To: Tanju Brunostar; +Cc: gregkh, linux-staging, linux-kernel, outreachy



On Thu, 3 Nov 2022, Tanju Brunostar wrote:

> On Thu, Nov 3, 2022 at 1:38 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> >
> >
> >
> > On Thu, 3 Nov 2022, Tanjuate Brunostar wrote:
> >
> > > Remove the use of Hungarian notation, which is not used in the Linux
> > > kernel. Reported by checkpatch
> > >
> > > Fix checkpatch error related to code line ends with a '(', by joining
> > > some lines and indenting correctly. This improves visibility
> >
> > I don't think it will be appreciated to do two things at once.
> >
>
> I sent it earlier as two patches but it seemed to break and fix the
> kernel which was not appreciated by one of the mentors

OK, then it is a presentation problem.  You have presented the changes as
unrelated.  Rewrite the message to make clear what is being done.

julia

>
> > >
> > > Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> > > ---
> > >  drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
> > >  1 file changed, 22 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > > index 31ae99b3cb35..debc5d5daede 100644
> > > --- a/drivers/staging/vt6655/rxtx.c
> > > +++ b/drivers/staging/vt6655/rxtx.c
> > > @@ -23,7 +23,7 @@
> > >   *      s_uGetTxRsvTime- get frame reserved time
> > >   *      s_vFillCTSHead- fulfill CTS ctl header
> > >   *      s_vFillFragParameter- Set fragment ctl parameter.
> > > - *      s_vFillRTSHead- fulfill RTS ctl header
> > > + *      fill_rts_header- fulfill RTS ctl header
> > >   *      s_vFillTxKey- fulfill tx encrypt key
> > >   *      s_vSWencryption- Software encrypt header
> > >   *      vDMA0_tx_80211- tx 802.11 frame via dma0
> > > @@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
> > >  #define DATADUR_A_F1    13
> > >
> > >  /*---------------------  Static Functions  --------------------------*/
> > > -static void s_vFillRTSHead(struct vnt_private *pDevice,
> > > -                        unsigned char byPktType,
> > > -                        void *pvRTS,
> > > -                        unsigned int cbFrameLength,
> > > -                        bool bNeedAck,
> > > -                        bool bDisCRC,
> > > -                        struct ieee80211_hdr *hdr,
> > > -                        unsigned short wCurrentRate,
> > > -                        unsigned char byFBOption);
> > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > +                         unsigned char byPktType,
> > > +                         void *pvRTS,
> > > +                         unsigned int        cbFrameLength,
> >
> > Strange spacing in the above parameter declaration.  Maybe there is a tab
> > that should be a space?
> >
> > julia
> >
>
> I was also shocked when I noticed that. It seemed like a tab but when
> I removed the space, the two words were joined and when I added the
> space, it became this lage space once again. checkpatch does not
> complain about it though
>
> > > +                         bool bNeedAck,
> > > +                         bool bDisCRC,
> > > +                         struct ieee80211_hdr *hdr,
> > > +                         unsigned short wCurrentRate,
> > > +                         unsigned char byFBOption);
> > >
> > >  static void s_vGenerateTxParameter(struct vnt_private *pDevice,
> > >                                  unsigned char byPktType,
> > > @@ -555,19 +555,15 @@ s_uFillDataHead(
> > >       return buf->duration;
> > >  }
> > >
> > > -static
> > > -void
> > > -s_vFillRTSHead(
> > > -     struct vnt_private *pDevice,
> > > -     unsigned char byPktType,
> > > -     void *pvRTS,
> > > -     unsigned int cbFrameLength,
> > > -     bool bNeedAck,
> > > -     bool bDisCRC,
> > > -     struct ieee80211_hdr *hdr,
> > > -     unsigned short wCurrentRate,
> > > -     unsigned char byFBOption
> > > -)
> > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > +                         unsigned char byPktType,
> > > +                         void *pvRTS,
> > > +                         unsigned int cbFrameLength,
> > > +                         bool bNeedAck,
> > > +                         bool bDisCRC,
> > > +                         struct ieee80211_hdr *hdr,
> > > +                         unsigned short wCurrentRate,
> > > +                         unsigned char byFBOption)
> > >  {
> > >       unsigned int uRTSFrameLen = 20;
> > >
> > > @@ -912,7 +908,7 @@ s_vGenerateTxParameter(
> > >                       buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > >                       buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
> > >
> > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > >               } else {/* RTS_needless, PCF mode */
> > >                       struct vnt_rrv_time_cts *buf = pvRrvTime;
> > >
> > > @@ -931,7 +927,7 @@ s_vGenerateTxParameter(
> > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > >
> > >                       /* Fill RTS */
> > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > >               } else if (!pvRTS) {/* RTS_needless, non PCF mode */
> > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > >
> > > @@ -945,7 +941,7 @@ s_vGenerateTxParameter(
> > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
> > >
> > >                       /* Fill RTS */
> > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > >               } else { /* RTS_needless, non PCF mode */
> > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > >
> > > --
> > > 2.34.1
> > >
> > >
> > >
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vt6655: change the function name s_vFillRTSHead
  2022-11-03 15:58     ` Julia Lawall
@ 2022-11-03 16:04       ` Tanju Brunostar
  2022-11-03 17:38         ` Tanju Brunostar
  0 siblings, 1 reply; 8+ messages in thread
From: Tanju Brunostar @ 2022-11-03 16:04 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, linux-staging, linux-kernel, outreachy

On Thu, Nov 3, 2022 at 4:58 PM Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>
> On Thu, 3 Nov 2022, Tanju Brunostar wrote:
>
> > On Thu, Nov 3, 2022 at 1:38 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> > >
> > >
> > >
> > > On Thu, 3 Nov 2022, Tanjuate Brunostar wrote:
> > >
> > > > Remove the use of Hungarian notation, which is not used in the Linux
> > > > kernel. Reported by checkpatch
> > > >
> > > > Fix checkpatch error related to code line ends with a '(', by joining
> > > > some lines and indenting correctly. This improves visibility
> > >
> > > I don't think it will be appreciated to do two things at once.
> > >
> >
> > I sent it earlier as two patches but it seemed to break and fix the
> > kernel which was not appreciated by one of the mentors
>
> OK, then it is a presentation problem.  You have presented the changes as
> unrelated.  Rewrite the message to make clear what is being done.
>
> julia
>
okay
> >
> > > >
> > > > Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> > > > ---
> > > >  drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
> > > >  1 file changed, 22 insertions(+), 26 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > > > index 31ae99b3cb35..debc5d5daede 100644
> > > > --- a/drivers/staging/vt6655/rxtx.c
> > > > +++ b/drivers/staging/vt6655/rxtx.c
> > > > @@ -23,7 +23,7 @@
> > > >   *      s_uGetTxRsvTime- get frame reserved time
> > > >   *      s_vFillCTSHead- fulfill CTS ctl header
> > > >   *      s_vFillFragParameter- Set fragment ctl parameter.
> > > > - *      s_vFillRTSHead- fulfill RTS ctl header
> > > > + *      fill_rts_header- fulfill RTS ctl header
> > > >   *      s_vFillTxKey- fulfill tx encrypt key
> > > >   *      s_vSWencryption- Software encrypt header
> > > >   *      vDMA0_tx_80211- tx 802.11 frame via dma0
> > > > @@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
> > > >  #define DATADUR_A_F1    13
> > > >
> > > >  /*---------------------  Static Functions  --------------------------*/
> > > > -static void s_vFillRTSHead(struct vnt_private *pDevice,
> > > > -                        unsigned char byPktType,
> > > > -                        void *pvRTS,
> > > > -                        unsigned int cbFrameLength,
> > > > -                        bool bNeedAck,
> > > > -                        bool bDisCRC,
> > > > -                        struct ieee80211_hdr *hdr,
> > > > -                        unsigned short wCurrentRate,
> > > > -                        unsigned char byFBOption);
> > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > +                         unsigned char byPktType,
> > > > +                         void *pvRTS,
> > > > +                         unsigned int        cbFrameLength,
> > >
> > > Strange spacing in the above parameter declaration.  Maybe there is a tab
> > > that should be a space?
> > >
> > > julia
> > >
> >
> > I was also shocked when I noticed that. It seemed like a tab but when
> > I removed the space, the two words were joined and when I added the
> > space, it became this lage space once again. checkpatch does not
> > complain about it though
> >
> > > > +                         bool bNeedAck,
> > > > +                         bool bDisCRC,
> > > > +                         struct ieee80211_hdr *hdr,
> > > > +                         unsigned short wCurrentRate,
> > > > +                         unsigned char byFBOption);
> > > >
> > > >  static void s_vGenerateTxParameter(struct vnt_private *pDevice,
> > > >                                  unsigned char byPktType,
> > > > @@ -555,19 +555,15 @@ s_uFillDataHead(
> > > >       return buf->duration;
> > > >  }
> > > >
> > > > -static
> > > > -void
> > > > -s_vFillRTSHead(
> > > > -     struct vnt_private *pDevice,
> > > > -     unsigned char byPktType,
> > > > -     void *pvRTS,
> > > > -     unsigned int cbFrameLength,
> > > > -     bool bNeedAck,
> > > > -     bool bDisCRC,
> > > > -     struct ieee80211_hdr *hdr,
> > > > -     unsigned short wCurrentRate,
> > > > -     unsigned char byFBOption
> > > > -)
> > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > +                         unsigned char byPktType,
> > > > +                         void *pvRTS,
> > > > +                         unsigned int cbFrameLength,
> > > > +                         bool bNeedAck,
> > > > +                         bool bDisCRC,
> > > > +                         struct ieee80211_hdr *hdr,
> > > > +                         unsigned short wCurrentRate,
> > > > +                         unsigned char byFBOption)
> > > >  {
> > > >       unsigned int uRTSFrameLen = 20;
> > > >
> > > > @@ -912,7 +908,7 @@ s_vGenerateTxParameter(
> > > >                       buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > >                       buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
> > > >
> > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > >               } else {/* RTS_needless, PCF mode */
> > > >                       struct vnt_rrv_time_cts *buf = pvRrvTime;
> > > >
> > > > @@ -931,7 +927,7 @@ s_vGenerateTxParameter(
> > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > >
> > > >                       /* Fill RTS */
> > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > >               } else if (!pvRTS) {/* RTS_needless, non PCF mode */
> > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > >
> > > > @@ -945,7 +941,7 @@ s_vGenerateTxParameter(
> > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
> > > >
> > > >                       /* Fill RTS */
> > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > >               } else { /* RTS_needless, non PCF mode */
> > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > > >
> > > >
> >
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vt6655: change the function name s_vFillRTSHead
  2022-11-03 16:04       ` Tanju Brunostar
@ 2022-11-03 17:38         ` Tanju Brunostar
  2022-11-03 17:51           ` Julia Lawall
  0 siblings, 1 reply; 8+ messages in thread
From: Tanju Brunostar @ 2022-11-03 17:38 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, linux-staging, linux-kernel, outreachy

On Thu, Nov 3, 2022 at 5:04 PM Tanju Brunostar
<tanjubrunostar0@gmail.com> wrote:
>
> On Thu, Nov 3, 2022 at 4:58 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> >
> >
> >
> > On Thu, 3 Nov 2022, Tanju Brunostar wrote:
> >
> > > On Thu, Nov 3, 2022 at 1:38 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> > > >
> > > >
> > > >
> > > > On Thu, 3 Nov 2022, Tanjuate Brunostar wrote:
> > > >
> > > > > Remove the use of Hungarian notation, which is not used in the Linux
> > > > > kernel. Reported by checkpatch
> > > > >
> > > > > Fix checkpatch error related to code line ends with a '(', by joining
> > > > > some lines and indenting correctly. This improves visibility
> > > >
> > > > I don't think it will be appreciated to do two things at once.
> > > >
> > >
> > > I sent it earlier as two patches but it seemed to break and fix the
> > > kernel which was not appreciated by one of the mentors
> >
> > OK, then it is a presentation problem.  You have presented the changes as
> > unrelated.  Rewrite the message to make clear what is being done.
> >
> > julia
> >

Hi Julia,
I am having a hard time figuring out exactly how to write this, since
I made both changes in one patch and there is no way to send them in
separate patches. should I just report just one change and leave the
other? I would appreciate your help here
Thanks

tanju

> okay
> > >
> > > > >
> > > > > Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> > > > > ---
> > > > >  drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
> > > > >  1 file changed, 22 insertions(+), 26 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > > > > index 31ae99b3cb35..debc5d5daede 100644
> > > > > --- a/drivers/staging/vt6655/rxtx.c
> > > > > +++ b/drivers/staging/vt6655/rxtx.c
> > > > > @@ -23,7 +23,7 @@
> > > > >   *      s_uGetTxRsvTime- get frame reserved time
> > > > >   *      s_vFillCTSHead- fulfill CTS ctl header
> > > > >   *      s_vFillFragParameter- Set fragment ctl parameter.
> > > > > - *      s_vFillRTSHead- fulfill RTS ctl header
> > > > > + *      fill_rts_header- fulfill RTS ctl header
> > > > >   *      s_vFillTxKey- fulfill tx encrypt key
> > > > >   *      s_vSWencryption- Software encrypt header
> > > > >   *      vDMA0_tx_80211- tx 802.11 frame via dma0
> > > > > @@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
> > > > >  #define DATADUR_A_F1    13
> > > > >
> > > > >  /*---------------------  Static Functions  --------------------------*/
> > > > > -static void s_vFillRTSHead(struct vnt_private *pDevice,
> > > > > -                        unsigned char byPktType,
> > > > > -                        void *pvRTS,
> > > > > -                        unsigned int cbFrameLength,
> > > > > -                        bool bNeedAck,
> > > > > -                        bool bDisCRC,
> > > > > -                        struct ieee80211_hdr *hdr,
> > > > > -                        unsigned short wCurrentRate,
> > > > > -                        unsigned char byFBOption);
> > > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > > +                         unsigned char byPktType,
> > > > > +                         void *pvRTS,
> > > > > +                         unsigned int        cbFrameLength,
> > > >
> > > > Strange spacing in the above parameter declaration.  Maybe there is a tab
> > > > that should be a space?
> > > >
> > > > julia
> > > >
> > >
> > > I was also shocked when I noticed that. It seemed like a tab but when
> > > I removed the space, the two words were joined and when I added the
> > > space, it became this lage space once again. checkpatch does not
> > > complain about it though
> > >
> > > > > +                         bool bNeedAck,
> > > > > +                         bool bDisCRC,
> > > > > +                         struct ieee80211_hdr *hdr,
> > > > > +                         unsigned short wCurrentRate,
> > > > > +                         unsigned char byFBOption);
> > > > >
> > > > >  static void s_vGenerateTxParameter(struct vnt_private *pDevice,
> > > > >                                  unsigned char byPktType,
> > > > > @@ -555,19 +555,15 @@ s_uFillDataHead(
> > > > >       return buf->duration;
> > > > >  }
> > > > >
> > > > > -static
> > > > > -void
> > > > > -s_vFillRTSHead(
> > > > > -     struct vnt_private *pDevice,
> > > > > -     unsigned char byPktType,
> > > > > -     void *pvRTS,
> > > > > -     unsigned int cbFrameLength,
> > > > > -     bool bNeedAck,
> > > > > -     bool bDisCRC,
> > > > > -     struct ieee80211_hdr *hdr,
> > > > > -     unsigned short wCurrentRate,
> > > > > -     unsigned char byFBOption
> > > > > -)
> > > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > > +                         unsigned char byPktType,
> > > > > +                         void *pvRTS,
> > > > > +                         unsigned int cbFrameLength,
> > > > > +                         bool bNeedAck,
> > > > > +                         bool bDisCRC,
> > > > > +                         struct ieee80211_hdr *hdr,
> > > > > +                         unsigned short wCurrentRate,
> > > > > +                         unsigned char byFBOption)
> > > > >  {
> > > > >       unsigned int uRTSFrameLen = 20;
> > > > >
> > > > > @@ -912,7 +908,7 @@ s_vGenerateTxParameter(
> > > > >                       buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > > >                       buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
> > > > >
> > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > >               } else {/* RTS_needless, PCF mode */
> > > > >                       struct vnt_rrv_time_cts *buf = pvRrvTime;
> > > > >
> > > > > @@ -931,7 +927,7 @@ s_vGenerateTxParameter(
> > > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > > >
> > > > >                       /* Fill RTS */
> > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > >               } else if (!pvRTS) {/* RTS_needless, non PCF mode */
> > > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > > >
> > > > > @@ -945,7 +941,7 @@ s_vGenerateTxParameter(
> > > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
> > > > >
> > > > >                       /* Fill RTS */
> > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > >               } else { /* RTS_needless, non PCF mode */
> > > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > > >
> > > > > --
> > > > > 2.34.1
> > > > >
> > > > >
> > > > >
> > >
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vt6655: change the function name s_vFillRTSHead
  2022-11-03 17:38         ` Tanju Brunostar
@ 2022-11-03 17:51           ` Julia Lawall
  2022-11-03 17:58             ` Tanju Brunostar
  0 siblings, 1 reply; 8+ messages in thread
From: Julia Lawall @ 2022-11-03 17:51 UTC (permalink / raw)
  To: Tanju Brunostar; +Cc: gregkh, linux-staging, linux-kernel, outreachy



On Thu, 3 Nov 2022, Tanju Brunostar wrote:

> On Thu, Nov 3, 2022 at 5:04 PM Tanju Brunostar
> <tanjubrunostar0@gmail.com> wrote:
> >
> > On Thu, Nov 3, 2022 at 4:58 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> > >
> > >
> > >
> > > On Thu, 3 Nov 2022, Tanju Brunostar wrote:
> > >
> > > > On Thu, Nov 3, 2022 at 1:38 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Thu, 3 Nov 2022, Tanjuate Brunostar wrote:
> > > > >
> > > > > > Remove the use of Hungarian notation, which is not used in the Linux
> > > > > > kernel. Reported by checkpatch
> > > > > >
> > > > > > Fix checkpatch error related to code line ends with a '(', by joining
> > > > > > some lines and indenting correctly. This improves visibility
> > > > >
> > > > > I don't think it will be appreciated to do two things at once.
> > > > >
> > > >
> > > > I sent it earlier as two patches but it seemed to break and fix the
> > > > kernel which was not appreciated by one of the mentors
> > >
> > > OK, then it is a presentation problem.  You have presented the changes as
> > > unrelated.  Rewrite the message to make clear what is being done.
> > >
> > > julia
> > >
>
> Hi Julia,
> I am having a hard time figuring out exactly how to write this, since
> I made both changes in one patch and there is no way to send them in
> separate patches. should I just report just one change and leave the
> other? I would appreciate your help here

Add indentation to the affected function headers to follow the Linux
kernel coding style?

julia


> Thanks
>
> tanju
>
> > okay
> > > >
> > > > > >
> > > > > > Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> > > > > > ---
> > > > > >  drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
> > > > > >  1 file changed, 22 insertions(+), 26 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > > > > > index 31ae99b3cb35..debc5d5daede 100644
> > > > > > --- a/drivers/staging/vt6655/rxtx.c
> > > > > > +++ b/drivers/staging/vt6655/rxtx.c
> > > > > > @@ -23,7 +23,7 @@
> > > > > >   *      s_uGetTxRsvTime- get frame reserved time
> > > > > >   *      s_vFillCTSHead- fulfill CTS ctl header
> > > > > >   *      s_vFillFragParameter- Set fragment ctl parameter.
> > > > > > - *      s_vFillRTSHead- fulfill RTS ctl header
> > > > > > + *      fill_rts_header- fulfill RTS ctl header
> > > > > >   *      s_vFillTxKey- fulfill tx encrypt key
> > > > > >   *      s_vSWencryption- Software encrypt header
> > > > > >   *      vDMA0_tx_80211- tx 802.11 frame via dma0
> > > > > > @@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
> > > > > >  #define DATADUR_A_F1    13
> > > > > >
> > > > > >  /*---------------------  Static Functions  --------------------------*/
> > > > > > -static void s_vFillRTSHead(struct vnt_private *pDevice,
> > > > > > -                        unsigned char byPktType,
> > > > > > -                        void *pvRTS,
> > > > > > -                        unsigned int cbFrameLength,
> > > > > > -                        bool bNeedAck,
> > > > > > -                        bool bDisCRC,
> > > > > > -                        struct ieee80211_hdr *hdr,
> > > > > > -                        unsigned short wCurrentRate,
> > > > > > -                        unsigned char byFBOption);
> > > > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > > > +                         unsigned char byPktType,
> > > > > > +                         void *pvRTS,
> > > > > > +                         unsigned int        cbFrameLength,
> > > > >
> > > > > Strange spacing in the above parameter declaration.  Maybe there is a tab
> > > > > that should be a space?
> > > > >
> > > > > julia
> > > > >
> > > >
> > > > I was also shocked when I noticed that. It seemed like a tab but when
> > > > I removed the space, the two words were joined and when I added the
> > > > space, it became this lage space once again. checkpatch does not
> > > > complain about it though
> > > >
> > > > > > +                         bool bNeedAck,
> > > > > > +                         bool bDisCRC,
> > > > > > +                         struct ieee80211_hdr *hdr,
> > > > > > +                         unsigned short wCurrentRate,
> > > > > > +                         unsigned char byFBOption);
> > > > > >
> > > > > >  static void s_vGenerateTxParameter(struct vnt_private *pDevice,
> > > > > >                                  unsigned char byPktType,
> > > > > > @@ -555,19 +555,15 @@ s_uFillDataHead(
> > > > > >       return buf->duration;
> > > > > >  }
> > > > > >
> > > > > > -static
> > > > > > -void
> > > > > > -s_vFillRTSHead(
> > > > > > -     struct vnt_private *pDevice,
> > > > > > -     unsigned char byPktType,
> > > > > > -     void *pvRTS,
> > > > > > -     unsigned int cbFrameLength,
> > > > > > -     bool bNeedAck,
> > > > > > -     bool bDisCRC,
> > > > > > -     struct ieee80211_hdr *hdr,
> > > > > > -     unsigned short wCurrentRate,
> > > > > > -     unsigned char byFBOption
> > > > > > -)
> > > > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > > > +                         unsigned char byPktType,
> > > > > > +                         void *pvRTS,
> > > > > > +                         unsigned int cbFrameLength,
> > > > > > +                         bool bNeedAck,
> > > > > > +                         bool bDisCRC,
> > > > > > +                         struct ieee80211_hdr *hdr,
> > > > > > +                         unsigned short wCurrentRate,
> > > > > > +                         unsigned char byFBOption)
> > > > > >  {
> > > > > >       unsigned int uRTSFrameLen = 20;
> > > > > >
> > > > > > @@ -912,7 +908,7 @@ s_vGenerateTxParameter(
> > > > > >                       buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > > > >                       buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
> > > > > >
> > > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > >               } else {/* RTS_needless, PCF mode */
> > > > > >                       struct vnt_rrv_time_cts *buf = pvRrvTime;
> > > > > >
> > > > > > @@ -931,7 +927,7 @@ s_vGenerateTxParameter(
> > > > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > > > >
> > > > > >                       /* Fill RTS */
> > > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > >               } else if (!pvRTS) {/* RTS_needless, non PCF mode */
> > > > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > > > >
> > > > > > @@ -945,7 +941,7 @@ s_vGenerateTxParameter(
> > > > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
> > > > > >
> > > > > >                       /* Fill RTS */
> > > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > >               } else { /* RTS_needless, non PCF mode */
> > > > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > > > >
> > > > > > --
> > > > > > 2.34.1
> > > > > >
> > > > > >
> > > > > >
> > > >
> > >
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vt6655: change the function name s_vFillRTSHead
  2022-11-03 17:51           ` Julia Lawall
@ 2022-11-03 17:58             ` Tanju Brunostar
  0 siblings, 0 replies; 8+ messages in thread
From: Tanju Brunostar @ 2022-11-03 17:58 UTC (permalink / raw)
  To: Julia Lawall; +Cc: gregkh, linux-staging, linux-kernel, outreachy

On Thu, Nov 3, 2022 at 6:51 PM Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>
> On Thu, 3 Nov 2022, Tanju Brunostar wrote:
>
> > On Thu, Nov 3, 2022 at 5:04 PM Tanju Brunostar
> > <tanjubrunostar0@gmail.com> wrote:
> > >
> > > On Thu, Nov 3, 2022 at 4:58 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> > > >
> > > >
> > > >
> > > > On Thu, 3 Nov 2022, Tanju Brunostar wrote:
> > > >
> > > > > On Thu, Nov 3, 2022 at 1:38 PM Julia Lawall <julia.lawall@inria.fr> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Thu, 3 Nov 2022, Tanjuate Brunostar wrote:
> > > > > >
> > > > > > > Remove the use of Hungarian notation, which is not used in the Linux
> > > > > > > kernel. Reported by checkpatch
> > > > > > >
> > > > > > > Fix checkpatch error related to code line ends with a '(', by joining
> > > > > > > some lines and indenting correctly. This improves visibility
> > > > > >
> > > > > > I don't think it will be appreciated to do two things at once.
> > > > > >
> > > > >
> > > > > I sent it earlier as two patches but it seemed to break and fix the
> > > > > kernel which was not appreciated by one of the mentors
> > > >
> > > > OK, then it is a presentation problem.  You have presented the changes as
> > > > unrelated.  Rewrite the message to make clear what is being done.
> > > >
> > > > julia
> > > >
> >
> > Hi Julia,
> > I am having a hard time figuring out exactly how to write this, since
> > I made both changes in one patch and there is no way to send them in
> > separate patches. should I just report just one change and leave the
> > other? I would appreciate your help here
>
> Add indentation to the affected function headers to follow the Linux
> kernel coding style?
>
> julia
>

Thank you

>
> > Thanks
> >
> > tanju
> >
> > > okay
> > > > >
> > > > > > >
> > > > > > > Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> > > > > > > ---
> > > > > > >  drivers/staging/vt6655/rxtx.c | 48 ++++++++++++++++-------------------
> > > > > > >  1 file changed, 22 insertions(+), 26 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > > > > > > index 31ae99b3cb35..debc5d5daede 100644
> > > > > > > --- a/drivers/staging/vt6655/rxtx.c
> > > > > > > +++ b/drivers/staging/vt6655/rxtx.c
> > > > > > > @@ -23,7 +23,7 @@
> > > > > > >   *      s_uGetTxRsvTime- get frame reserved time
> > > > > > >   *      s_vFillCTSHead- fulfill CTS ctl header
> > > > > > >   *      s_vFillFragParameter- Set fragment ctl parameter.
> > > > > > > - *      s_vFillRTSHead- fulfill RTS ctl header
> > > > > > > + *      fill_rts_header- fulfill RTS ctl header
> > > > > > >   *      s_vFillTxKey- fulfill tx encrypt key
> > > > > > >   *      s_vSWencryption- Software encrypt header
> > > > > > >   *      vDMA0_tx_80211- tx 802.11 frame via dma0
> > > > > > > @@ -85,15 +85,15 @@ static const unsigned short fb_opt1[2][5] = {
> > > > > > >  #define DATADUR_A_F1    13
> > > > > > >
> > > > > > >  /*---------------------  Static Functions  --------------------------*/
> > > > > > > -static void s_vFillRTSHead(struct vnt_private *pDevice,
> > > > > > > -                        unsigned char byPktType,
> > > > > > > -                        void *pvRTS,
> > > > > > > -                        unsigned int cbFrameLength,
> > > > > > > -                        bool bNeedAck,
> > > > > > > -                        bool bDisCRC,
> > > > > > > -                        struct ieee80211_hdr *hdr,
> > > > > > > -                        unsigned short wCurrentRate,
> > > > > > > -                        unsigned char byFBOption);
> > > > > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > > > > +                         unsigned char byPktType,
> > > > > > > +                         void *pvRTS,
> > > > > > > +                         unsigned int        cbFrameLength,
> > > > > >
> > > > > > Strange spacing in the above parameter declaration.  Maybe there is a tab
> > > > > > that should be a space?
> > > > > >
> > > > > > julia
> > > > > >
> > > > >
> > > > > I was also shocked when I noticed that. It seemed like a tab but when
> > > > > I removed the space, the two words were joined and when I added the
> > > > > space, it became this lage space once again. checkpatch does not
> > > > > complain about it though
> > > > >
> > > > > > > +                         bool bNeedAck,
> > > > > > > +                         bool bDisCRC,
> > > > > > > +                         struct ieee80211_hdr *hdr,
> > > > > > > +                         unsigned short wCurrentRate,
> > > > > > > +                         unsigned char byFBOption);
> > > > > > >
> > > > > > >  static void s_vGenerateTxParameter(struct vnt_private *pDevice,
> > > > > > >                                  unsigned char byPktType,
> > > > > > > @@ -555,19 +555,15 @@ s_uFillDataHead(
> > > > > > >       return buf->duration;
> > > > > > >  }
> > > > > > >
> > > > > > > -static
> > > > > > > -void
> > > > > > > -s_vFillRTSHead(
> > > > > > > -     struct vnt_private *pDevice,
> > > > > > > -     unsigned char byPktType,
> > > > > > > -     void *pvRTS,
> > > > > > > -     unsigned int cbFrameLength,
> > > > > > > -     bool bNeedAck,
> > > > > > > -     bool bDisCRC,
> > > > > > > -     struct ieee80211_hdr *hdr,
> > > > > > > -     unsigned short wCurrentRate,
> > > > > > > -     unsigned char byFBOption
> > > > > > > -)
> > > > > > > +static void fill_rts_header(struct vnt_private *pDevice,
> > > > > > > +                         unsigned char byPktType,
> > > > > > > +                         void *pvRTS,
> > > > > > > +                         unsigned int cbFrameLength,
> > > > > > > +                         bool bNeedAck,
> > > > > > > +                         bool bDisCRC,
> > > > > > > +                         struct ieee80211_hdr *hdr,
> > > > > > > +                         unsigned short wCurrentRate,
> > > > > > > +                         unsigned char byFBOption)
> > > > > > >  {
> > > > > > >       unsigned int uRTSFrameLen = 20;
> > > > > > >
> > > > > > > @@ -912,7 +908,7 @@ s_vGenerateTxParameter(
> > > > > > >                       buf->rrv_time_a = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > > > > >                       buf->rrv_time_b = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, pDevice->byTopCCKBasicRate, bNeedACK);
> > > > > > >
> > > > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > >               } else {/* RTS_needless, PCF mode */
> > > > > > >                       struct vnt_rrv_time_cts *buf = pvRrvTime;
> > > > > > >
> > > > > > > @@ -931,7 +927,7 @@ s_vGenerateTxParameter(
> > > > > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, byPktType, cbFrameSize, wCurrentRate, bNeedACK);
> > > > > > >
> > > > > > >                       /* Fill RTS */
> > > > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > >               } else if (!pvRTS) {/* RTS_needless, non PCF mode */
> > > > > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > > > > >
> > > > > > > @@ -945,7 +941,7 @@ s_vGenerateTxParameter(
> > > > > > >                       buf->rrv_time = vnt_rxtx_rsvtime_le16(pDevice, PK_TYPE_11B, cbFrameSize, wCurrentRate, bNeedACK);
> > > > > > >
> > > > > > >                       /* Fill RTS */
> > > > > > > -                     s_vFillRTSHead(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > > +                     fill_rts_header(pDevice, byPktType, pvRTS, cbFrameSize, bNeedACK, bDisCRC, psEthHeader, wCurrentRate, byFBOption);
> > > > > > >               } else { /* RTS_needless, non PCF mode */
> > > > > > >                       struct vnt_rrv_time_ab *buf = pvRrvTime;
> > > > > > >
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > > >
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-03 17:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03  9:52 [PATCH] staging: vt6655: change the function name s_vFillRTSHead Tanjuate Brunostar
2022-11-03 12:38 ` Julia Lawall
2022-11-03 15:49   ` Tanju Brunostar
2022-11-03 15:58     ` Julia Lawall
2022-11-03 16:04       ` Tanju Brunostar
2022-11-03 17:38         ` Tanju Brunostar
2022-11-03 17:51           ` Julia Lawall
2022-11-03 17:58             ` Tanju Brunostar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.