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
>
next prev parent 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).