Re: [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains

From: Fabio M. De Francesco
Date: Mon Sep 20 2021 - 09:45:22 EST


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
>