linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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>,
	Martin Kaiser <martin@kaiser.cx>
Subject: Re: [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains
Date: Mon, 20 Sep 2021 15:44:53 +0200	[thread overview]
Message-ID: <17555679.WE2PrEGejU@localhost.localdomain> (raw)
In-Reply-To: <YUh2vtxNxNBmpKHQ@kroah.com>

On Monday, September 20, 2021 1:55:42 PM CEST Greg Kroah-Hartman wrote:
> On Mon, Sep 20, 2021 at 01:53:37AM +0200, Fabio M. De Francesco wrote:
> > --- Preface ---
> > 
> > This is v8 of "shorten and simplify calls chain". The first 14 patches
> > have already been applied to staging-testing, so we have been requested
> > to reset the numbering of the remaining patches to 01/19, while 
discarding
> > from this new submission the above-mentioned 14 patches (otherwise we 
would 
> > have submitted a series containing 33 patches).
> > 
> > The following commit message is provided as it was in v7, both for the 
> > purpose of presenting the whole picture to Maintainers, Reviewers, and to 
> > anybody else who may be interested in knowing the entire design and the
> > evolution since v1 to the current v8.
> > 
> > --- Commit message ---
> > 
> > io_ops abstraction is useless in this driver, since there is only one ops
> > registration. Without io_ops we can get rid of indirect calls mess and
> > shorten the calls chain.
> > 
> > Shorten the calls chain of rtw_read8/16/32() down to the actual reads.
> > For this purpose unify the three usb_read8/16/32 into the new
> > usb_read(); make the latter parameterizable with 'size'; embed most of
> > the code of usbctrl_vendorreq() into usb_read() and use in it the new
> > usb_control_msg_recv() API of USB Core.
> > 
> > Shorten the calls chain of rtw_write8/16/32() down to the actual writes.
> > For this purpose unify the four usb_write8/16/32/N() into the new
> > usb_write(); make the latter parameterizable with 'size'; embed most of
> > the code of usbctrl_vendorreq() into usb_write() and use in it the new
> > usb_control_msg_send() API of USB Core.
> > 
> > The code with the modifications was thoroughly tested by Pavel Skripkin
> > using a TP-Link TL-WN722N v2 / v3 [Realtek RTL8188EUS] and by Fabio M.
> > De Francesco using a ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 
Nano].
> > 
> > --- Changelog ---
> > 
> > v7->v8 (old numbering):
> > 	- 1-14:
> > 		Patches applied to staging-testing, so they are 
dropped
> > 		from the current v8;
> > 
> > 	- 15-19:
> > 		Split into 19 patches. Numbering reset to 01. After 
this
> > 		reset, 15-19/19 become 01-19/19 (so we have a total of 
33
> > 		patches in this series. 
> 
> Better, still needs a bit more work.  I took 2 of these to shorten your
> load a bit :)
>
Thanks, Greg. I've already seen that you took those other 2 from the series. 
We are about half way to destination: 14 + 2 already taken. :)

I've not replied to the other messages of yours because they are easy to 
understand, fix or change commit messages, and resend as v9.

Instead I'm replying to this particular message because I'm not able to see 
where "a bit more work" is needed.

Maybe that you don't like that in paragraph "v7->v8 (old numbering" I only  
said that the old 15-19/19 (4 patches) are now split into 19 without further 
information about each logical group within those 19?

Please notice that, immediately after the above-mentioned section there is 
also the "v7->v8 (new numbering for the old 15-19/19):" new section where 
everything is detailed to a level that we considered sufficient.

For your convenience I copy-paste the lines that immediately follow the "old 
numbering" section:

v7->v8 (new numbering for the old 15-19/19):
        - 1-15:
                Old 15/19 and 16/19 are split into 15 patches (as it is 
                required by Greg Kroah-Hartman) in order to make them
                more easily reviewable and for checking that the logic
                is not broken; no significant changes to the resulting
                code;
        - 16-17:
                Old 17/19 and 18/19 become 16/19 and 17/19: There are 
                no significant changes to the code, with the sole
                exception of moving the acquiring of a mutex before
                the test for "bSurpriseRemoved" being 'true';
        - 18-19:
                Old 19/19 is split into new 18/19 and 19/19. The changes
                are split into a first patch that remove a shared buffer
                and a second that remove the mutex protecting the receiving 
                (in usb_read()) and sending (in usb_write()) of the
                USB control messages. 

If it is not what you require, can you please provide some more hints about 
what you would consider the "perfect" approach to rework this changelog?

It would help a lot, because at the moment I cannot see what I'm missing and 
I've been unable to reach Pavel and ask him if he saw something wrong that I 
can't see.

Thanks,

Fabio
> 
> thanks,
> 
> greg k-h
> 





  reply	other threads:[~2021-09-20 13:45 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-19 23:53 [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 01/19] staging: r8188eu: clean up symbols usbctrl_vendorreq() Fabio M. De Francesco
2021-09-20 11:46   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 02/19] staging: r8188eu: reorder declarations in usbctrl_vendorreq() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 03/19] staging: r8188eu: remove unnecessary test " Fabio M. De Francesco
2021-09-20 11:47   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 04/19] staging: r8188eu: reorder comments " Fabio M. De Francesco
2021-09-20 11:48   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 05/19] staging: r8188eu: remove unnedeed parentheses " Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 06/19] staging: r8188eu: remove unnecessary space " Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 07/19] staging: r8188eu: remove unnecessary comment " Fabio M. De Francesco
2021-09-20 11:48   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 08/19] staging: r8188eu: fix grammar mistake " Fabio M. De Francesco
2021-09-20 11:49   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 09/19] staging: r8188eu: remove unnecessary braces " Fabio M. De Francesco
2021-09-20 11:49   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 10/19] staging: r8188eu: rename symbols in rtw_read*() and rtw_write*() Fabio M. De Francesco
2021-09-20 11:50   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 11/19] staging: r8188eu: remove unnecessary casts from rtw_{read,write}*() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 12/19] staging: r8188eu: change the type of a variable in rtw_write16() Fabio M. De Francesco
2021-09-20 11:50   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 13/19] staging: r8188eu: remove an unneeded buffer from rtw_writeN() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 14/19] staging: r8188eu: remove an unnecessary bit AND " Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 15/19] staging: r8188eu: change the type of a variable in rtw_read16() Fabio M. De Francesco
2021-09-20 11:51   ` Greg Kroah-Hartman
2021-09-20 11:56   ` Dan Carpenter
2021-09-20 13:03     ` Fabio M. De Francesco
2021-09-20 13:10       ` Dan Carpenter
2021-09-20 16:17         ` Fabio M. De Francesco
2021-09-20 19:01           ` Dan Carpenter
2021-09-20 19:54             ` Fabio M. De Francesco
2021-09-21  5:35               ` Dan Carpenter
2021-09-19 23:53 ` [PATCH v8 16/19] staging: r8188eu: call the new usb_read() from rtw_read{8,16,32}() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 17/19] staging: r8188eu: call the new usb_write() from rtw_write{8,16,32,N}() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 18/19] staging: r8188eu: remove shared buffer for USB requests Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 19/19] staging: r8188eu: remove usb_vendor_req_mutex Fabio M. De Francesco
2021-09-20 11:55 ` [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains Greg Kroah-Hartman
2021-09-20 13:44   ` Fabio M. De Francesco [this message]
2021-09-20 14:06     ` Greg Kroah-Hartman

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=17555679.WE2PrEGejU@localhost.localdomain \
    --to=fmdefrancesco@gmail.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=dan.carpenter@oracle.com \
    --cc=david.Laight@aculab.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=martin@kaiser.cx \
    --cc=paskripkin@gmail.com \
    --cc=phil@philpotter.co.uk \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).