LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Cc: Larry Finger <Larry.Finger@lwfinger.net>,
	Phillip Potter <phil@philpotter.co.uk>,
	Pavel Skripkin <paskripkin@gmail.com>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	David Laight <david.Laight@aculab.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH v7 15/19] staging: r8188eu: clean up usbctrl_vendorreq()
Date: Fri, 17 Sep 2021 16:44:17 +0200	[thread overview]
Message-ID: <YUSpwTneDgRtUBvM@kroah.com> (raw)
In-Reply-To: <20210917071837.10926-16-fmdefrancesco@gmail.com>

On Fri, Sep 17, 2021 at 09:18:33AM +0200, Fabio M. De Francesco wrote:
> Clean up usbctrl_vendoreq() in usb_ops_linux.c.
> 
> List of changes:
> 
> 1) Rename variables:
> 	pdata => data
>         pio_priv => io_priv
>         pintfhdl => intfhdl
>         wvalue => address.
> 2) Reorder variables declarations according to the "Reverse Xmas Tree"
>    style.
> 3) Remove unncecessary test for "!pIo_buf".
> 4) Move comments one line below code.
> 5) Remove unnecessary excess parentheses.
> 6) Remove unnecessary extra spaces.
> 7) Remove unnecessary comments.
> 8) Fix grammar errors (checksumed => checksummed).

When you find yourself listing all of the different things you have done
in a single commit, that is a HUGE hint that you need to break this up
into smaller pieces.

Please do so here, this should not be just one change, as it's almost
impossible to look at this and "know" it's all still the same logic
happening here.  But if you had broken this down into 8 different
changes, then it would have been obvious and I could easily have applied
the changes.

I've taken the first 14 patches in this series, it's great work, thank
you all for doing this.  But this, and the remaining patches in here
need to be split up more to make it obvious that the changes are correct
and should be accepted.  Please feel free to start the numbering of the
patch series over now, given that the first 14 are now merged into my
tree.

thanks,

greg k-h

  reply	other threads:[~2021-09-17 14:45 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-17  7:18 [PATCH v7 00/19] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 01/19] staging: r8188eu: remove usb_{read,write}_mem() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 02/19] staging: r8188eu: remove the helpers of rtw_read8() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 03/19] staging: r8188eu: remove the helpers of rtw_read16() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 04/19] staging: r8188eu: remove the helpers of rtw_read32() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 05/19] staging: r8188eu: remove the helpers of usb_write8() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 06/19] staging: r8188eu: remove the helpers of usb_write16() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 07/19] staging: r8188eu: remove the helpers of usb_write32() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 08/19] staging: r8188eu: remove the helpers of usb_writeN() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 09/19] staging: r8188eu: remove the helpers of usb_read_port() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 10/19] staging: r8188eu: remove the helpers of usb_write_port() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 11/19] staging: r8188eu: remove the helpers of usb_read_port_cancel() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 12/19] staging: r8188eu: remove the helpers of usb_write_port_cancel() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 13/19] staging: r8188eu: remove core/rtw_io.c Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 14/19] staging: r8188eu: remove struct _io_ops Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 15/19] staging: r8188eu: clean up usbctrl_vendorreq() Fabio M. De Francesco
2021-09-17 14:44   ` Greg Kroah-Hartman [this message]
2021-09-18 11:13     ` Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 16/19] staging: r8188eu: clean up rtw_read*() and rtw_write*() Fabio M. De Francesco
2021-09-17 14:44   ` Greg Kroah-Hartman
2021-09-17 14:45   ` Greg Kroah-Hartman
2021-09-18 11:41     ` Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 17/19] staging: r8188eu: shorten calls chain of rtw_read{8,16,32}() Fabio M. De Francesco
2021-09-17 14:50   ` Greg Kroah-Hartman
2021-09-17 15:01     ` David Laight
2021-09-17 15:17       ` 'Greg Kroah-Hartman'
2021-09-18 12:19     ` Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 18/19] staging: r8188eu: shorten calls chain of rtw_write{8,16,32,N}() Fabio M. De Francesco
2021-09-17  7:18 ` [PATCH v7 19/19] staging: r8188eu: remove shared buffer for usb requests Fabio M. De Francesco
2021-09-17 14:55   ` Greg Kroah-Hartman
2021-09-17 15:03     ` Pavel Skripkin
2021-09-17 15:06       ` Pavel Skripkin
2021-09-17 15:18       ` Greg Kroah-Hartman
2021-09-17 15:23         ` Pavel Skripkin
2021-09-17 11:07 ` [PATCH v7 00/19] staging: r8188eu: shorten and simplify calls chains Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YUSpwTneDgRtUBvM@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Larry.Finger@lwfinger.net \
    --cc=dan.carpenter@oracle.com \
    --cc=david.Laight@aculab.com \
    --cc=fmdefrancesco@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=paskripkin@gmail.com \
    --cc=phil@philpotter.co.uk \
    --subject='Re: [PATCH v7 15/19] staging: r8188eu: clean up usbctrl_vendorreq()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).