On Mon, 16 Aug 2021 at 08:01, Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx> wrote: > > On Monday, August 16, 2021 8:55:06 AM CEST Fabio M. De Francesco wrote: > > On Monday, August 16, 2021 1:05:18 AM CEST Phillip Potter wrote: > > > Remove set but unused variable init_rate from rtl8188e_Add_RateATid > > > function in hal/rtl8188e_cmd.c, as it fixes a kernel test robot warning. > > > Removing the call to get_highest_rate_idx has no side effects here so is > > > safe. > > > > > > Also remove the DBG_88E macro call in this function, as it is not > > > particularly clear in my opinion. Additionally, rename variable > > > shortGIrate to short_gi_rate to conform to kernel camel case rules, > > > and improve general spacing around operators, some of which triggers > > > checkpatch 'CHECK' messages. These are not related to the test robot > > > warning. > > > > > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > > Signed-off-by: Phillip Potter <phil@xxxxxxxxxxxxxxxx> > > > --- > > > > > > drivers/staging/r8188eu/hal/rtl8188e_cmd.c | 22 +++++++--------------- > > > 1 file changed, 7 insertions(+), 15 deletions(-) > > > > Dear Philip, > > > > I'm sorry but, although every change here is fine, I cannot ack your patch > as- > > is. It shouldn't address so many different issues all at once, according to > > the best practices in patching and the kernel development rules. > > > > I understand that you think that, while you are at the removal of > "init_rate", > > why shouldn't I address all other trivial issues at once? > > > > Even if the patch is short and it probably doesn't require particular hard > > effort to review it, that mix-up of different works shouldn't be done, > mainly > > because this attitude could potentially lead you to add more and more > > different work in future patches. Where is the limit? Why not add some more > > different works next time you find some more problems into the same file/ > > directory? > > > > If I were you I'd, at least, prepare a series of two or three patches: > > > > 1/3 - Remove init_rate as reported by KTR; > > 2/3 - Remove unneeded DBG_88E macro; > > 3/3 - Do some clean-up of rtl8188e_cmd.c; > > > > Perhaps patches 2/3 and 3/3 could be merged into one, but I'm not really > sure. > > > > Thanks, > > > > Fabio > > Furthermore, I forgot to say that the "Subject" should summarize with few > words the whole work you do and in this case it is not what it does. > > Fabio > > > Dear Fabio, Thank you for your feedback, I shall prepare a v2 series. Regards, Phil
