Messages in this thread | | | Subject | Re: [PATCH v2 1/4] Docs: usb: update usb_bulk_msg receiving example | From | Philipp Hortmann <> | Date | Sun, 5 Dec 2021 18:04:48 +0100 |
| |
On 12/5/21 12:00 PM, Greg KH wrote: > On Sat, Dec 04, 2021 at 05:35:11PM +0100, Philipp Hortmann wrote: >> Clarification that this example is not in the driver template anymore. >> Update code example so that it fits best to usb-skeleton.c >> Update format of function names >> >> Signed-off-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> >> --- >> V1 -> V2: Added "Update format of function names" to patch description >> Corrected format of function names like the following example: >> "`usb_bulk_msg` function" to "usb_bulk_msg()" >> --- >> .../driver-api/usb/writing_usb_driver.rst | 32 +++++++++---------- >> 1 file changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/Documentation/driver-api/usb/writing_usb_driver.rst b/Documentation/driver-api/usb/writing_usb_driver.rst >> index b43e1ce49f0e..ed11398837e5 100644 >> --- a/Documentation/driver-api/usb/writing_usb_driver.rst >> +++ b/Documentation/driver-api/usb/writing_usb_driver.rst >> @@ -218,36 +218,36 @@ do very much processing at that time. Our implementation of >> ``skel_write_bulk_callback`` merely reports if the urb was completed >> successfully or not and then returns. >> >> -The read function works a bit differently from the write function in >> +This read function works a bit differently from the write function in >> that we do not use an urb to transfer data from the device to the >> -driver. Instead we call the :c:func:`usb_bulk_msg` function, which can be used >> +driver. Instead we call usb_bulk_msg(), which can be used >> to send or receive data from a device without having to create urbs and >> -handle urb completion callback functions. We call the :c:func:`usb_bulk_msg` >> -function, giving it a buffer into which to place any data received from >> +handle urb completion callback functions. We call usb_bulk_msg(), >> +giving it a buffer into which to place any data received from >> the device and a timeout value. If the timeout period expires without >> receiving any data from the device, the function will fail and return an >> error message. This can be shown with the following code:: >> >> /* do an immediate bulk read to get data from the device */ >> - retval = usb_bulk_msg (skel->dev, >> - usb_rcvbulkpipe (skel->dev, >> - skel->bulk_in_endpointAddr), >> - skel->bulk_in_buffer, >> - skel->bulk_in_size, >> - &count, 5000); >> + rv = usb_bulk_msg(dev->udev, > > Why are you changing the varible name? That seems unnecessary. Reason is that retval does not exist in skel_read(). > >> + usb_rcvbulkpipe (dev->udev, >> + dev->bulk_in_endpointAddr), >> + dev->bulk_in_buffer, >> + dev->bulk_in_size, >> + &len, 5000); >> /* if the read was successful, copy the data to user space */ >> - if (!retval) { >> - if (copy_to_user (buffer, skel->bulk_in_buffer, count)) >> - retval = -EFAULT; >> + if (!rv) { >> + if (copy_to_user (buffer, dev->bulk_in_buffer, len)) >> + rv = -EFAULT; >> else >> - retval = count; >> + rv = len; >> } > > Leaving the variable name alone keeps this patch much smaller. Will leave the variable name in the next patch. > >> >> >> -The :c:func:`usb_bulk_msg` function can be very useful for doing single reads >> +usb_bulk_msg() can be very useful for doing single reads > > You are doing different things in this patch, one is converting the > function style and one is updating the text. How about doing just the > function name stuff first, all in one patch, and then the updates, as > that would make it much easier to read. Will be changed. > > Also, any reason you aren't cc:ing the USB maintainer on these changes? :) According to: perl scripts/get_maintainer.pl --separator , --nokeywords --nogit --nogit-fallback --norolestats -f Documentation/driver-api/usb/writing_usb_driver.rst Jonathan Corbet <corbet@lwn.net>,linux-doc@vger.kernel.org,linux-kernel@vger.kernel.org you are not in charge. thanks, Philipp Hortmann > thanks, > > greg k-h >
| |