Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Nov 2022 18:51:20 +0100 (CET)
From:   Julia Lawall <julia.lawall@inria.fr>
To:     Tanju Brunostar <tanjubrunostar0@gmail.com>
cc:     gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, outreachy@lists.linux.dev
Subject: Re: [PATCH] staging: vt6655: change the function name
 s_vFillRTSHead
In-Reply-To: <CAHJEyKWBTxvP_mkSaWw7duhm1_-_AE27FhrnZYOXsTy1Qt_c=g@mail.gmail.com>
Message-ID: <9eef21fa-c7bc-9fba-67ae-2a532ee6c3fa@inria.fr>
References: <Y2OPStg4jXtWpJcd@elroy-temp-vm.gaiao0uenmiufjlowqgp5yxwdh.gvxx.internal.cloudapp.net> <d0d6d613-174b-f7dd-7d45-45eecd26a850@inria.fr> <CAHJEyKUA4FG468S5bMsx+XGUR3pB3kKm_bx=LNPNJ6n9yPxCrQ@mail.gmail.com> <612f7cf-43db-a684-7a98-8469275b43fd@inria.fr>
 <CAHJEyKXL=u=7tH2O3tdeSBZHyXLAdP9JcfzKHy6-bc7Bju5NMQ@mail.gmail.com> <CAHJEyKWBTxvP_mkSaWw7duhm1_-_AE27FhrnZYOXsTy1Qt_c=g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk


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