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>
Subject: Re: [PATCH v7 16/19] staging: r8188eu: clean up rtw_read*() and rtw_write*()
Date: Sat, 18 Sep 2021 13:41:48 +0200 [thread overview]
Message-ID: <2291806.MQVBmByTbm@localhost.localdomain> (raw)
In-Reply-To: <YUSqCYKOulGjk1lZ@kroah.com>
On Friday, September 17, 2021 4:45:29 PM CEST Greg Kroah-Hartman wrote:
> On Fri, Sep 17, 2021 at 09:18:34AM +0200, Fabio M. De Francesco wrote:
> > Clean up rtw_read{8,16,32}() and rtw_write{8,16,32,N}() in
usb_ops_linux.c.
> >
> > 1) Rename variables:
> > length => len
> > pio_priv => io_priv
> > pintfhdl => intfhdl
> > wvalue => address.
>
> Wait, why are you changing wvalue? Isn't that the USB name for this
> variable in the USB message sent to the device? Check the USB spec
> before changing this, that is a common field and probably should not be
> changed.
Oh, sorry. This was due to my very limited knowledge of the USB subsystems
and its Core API. So I misunderstood the semantics of this "wvalue" argument
and we'll change it to "value" (just to remove that unnecessary 'w', that I
guess is for "word").
I had thought that the mere knowledge of C and OS kernels (from a theoretical
perspective) would suffice to work on the code that we change in our patches.
Now I understand that such a naive approach is clearly wrong.
I have been too lazy to open your LDD 3rd ed. for reading but now I have
decided that it is time to do it. Furthermore, I have found one more book
about Linux device drivers and I'm about to read also its "Linux USB device
drivers" chapter.
I think that by this evening I'll have some basic knowledge of the USB
subsystem, at least of what is needed to avoid future mistakes like the one
you noticed. :)
Thank you very much for the time you spent reviewing our code and for taking
the first 14 patches,
Fabio
>
> thanks,
>
> greg k-h
next prev parent reply other threads:[~2021-09-18 11:41 UTC|newest]
Thread overview: 36+ 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
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 [this message]
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 14:54 ` Pavel Skripkin
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=2291806.MQVBmByTbm@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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.