Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1329851
| From | Joshua Clayton <stillcompiling@gmail.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v2 1/9] Staging: rts5208: rtsx_transport.c: Fix comment style warnings |
| Date | 2016-02-09 04:10 +0100 |
| Message-ID | <r06QF-6k0-3@gated-at.bofh.it> (permalink) |
| References | <qV1yi-4Nn-5@gated-at.bofh.it> <r05rA-5cr-5@gated-at.bofh.it> <r05rB-5cr-27@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hello Shaun,
/*
* Multiline comments (except in the net subsystem) should
* start with "/*" on a separate line. see Documentation/CodingStyle
*/
If you are going to fix the comments you should get both the beginning
and the end.
More comments inline.
On Monday, February 08, 2016 05:31:17 PM Shaun Ren wrote:
> This patch fixes all comment style warnings in rtsx_transport.c reported by
> checkpatch.pl:
>
> WARNING: Block comments use a trailing */ on a separate line
>
> Signed-off-by: Shaun Ren <shaun.ren@linux.com>
> ---
> drivers/staging/rts5208/rtsx_transport.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/rts5208/rtsx_transport.c b/drivers/staging/rts5208/rtsx_transport.c
> index f27491e..3e3f6fb 100644
> --- a/drivers/staging/rts5208/rtsx_transport.c
> +++ b/drivers/staging/rts5208/rtsx_transport.c
> @@ -36,7 +36,8 @@
> * For non-scatter-gather transfers, srb->request_buffer points to the
> * transfer buffer itself and srb->request_bufflen is the buffer's length.)
> * Update the *index and *offset variables so that the next copy will
> - * pick up from where this one left off. */
> + * pick up from where this one left off.
> + */
>
Fix the beginning too, as mentioned above.
> unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
> unsigned int buflen, struct scsi_cmnd *srb, unsigned int *index,
> @@ -45,7 +46,8 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
> unsigned int cnt;
>
> /* If not using scatter-gather, just transfer the data directly.
> - * Make certain it will fit in the available buffer space. */
> + * Make certain it will fit in the available buffer space.
> + */
Either fix the beginning... or better yet, get rid of the useless
(obvious) second line so it can be a single line comment.
> if (scsi_sg_count(srb) == 0) {
> if (*offset >= scsi_bufflen(srb))
> return 0;
> @@ -64,7 +66,8 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
> * in kernel-addressable memory then kmap() will return its address.
> * If the page is not directly accessible -- such as a user buffer
> * located in high memory -- then kmap() will map it to a temporary
> - * position in the kernel's virtual address space. */
> + * position in the kernel's virtual address space.
> + */
Fix the beginning of this comment as well.
Also, from "If the page is already in kernel -addressible memory..."
on is just a description of what kmap() does.
I'd get rid of those lines.
> } else {
> struct scatterlist *sg =
> (struct scatterlist *) scsi_sglist(srb)
> @@ -73,7 +76,8 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
> /* This loop handles a single s-g list entry, which may
> * include multiple pages. Find the initial page structure
> * and the starting offset within the page, and update
> - * the *offset and *index values for the next loop. */
> + * the *offset and *index values for the next loop.
> + */
Fix the beginning of this comment.
> cnt = 0;
> while (cnt < buflen && *index < scsi_sg_count(srb)) {
> struct page *page = sg_page(sg) +
> @@ -97,7 +101,8 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
>
> /* Transfer the data for all the pages in this
> * s-g entry. For each page: call kmap(), do the
> - * transfer, and call kunmap() immediately after. */
> + * transfer, and call kunmap() immediately after.
> + */
I'd get rid of this comment. It parrots what the code does, but does not
add any information.
> while (sglen > 0) {
> unsigned int plen = min(sglen, (unsigned int)
> PAGE_SIZE - poff);
> @@ -123,7 +128,8 @@ unsigned int rtsx_stor_access_xfer_buf(unsigned char *buffer,
> }
>
> /* Store the contents of buffer into srb's transfer buffer and set the
> -* SCSI residue. */
> + * SCSI residue.
> + */
Fix the beginning as well.
> void rtsx_stor_set_xfer_buf(unsigned char *buffer,
> unsigned int buflen, struct scsi_cmnd *srb)
> {
> @@ -196,7 +202,8 @@ void rtsx_invoke_transport(struct scsi_cmnd *srb, struct rtsx_chip *chip)
>
> /* Error and abort processing: try to resynchronize with the device
> * by issuing a port reset. If that fails, try a class-specific
> - * device reset. */
> + * device reset.
> + */
This comment describes something that does not
happen in this function. Perhaps it did at one point.
Regardless, It should be removed.
> Handle_Errors:
> return;
> }
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v2 0/9] Staging: rts5208: Coding style and dma mapping fixes Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
[PATCH v2 2/9] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
Re: [PATCH v2 2/9] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-02-10 06:30 +0100
[PATCH v2 7/9] Staging: rts5208: rtsx_transport.c: Remove unnecessary parentheses Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
Re: [PATCH v2 7/9] Staging: rts5208: rtsx_transport.c: Remove unnecessary parentheses Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-02-10 06:30 +0100
Re: [PATCH v2 7/9] Staging: rts5208: rtsx_transport.c: Remove unnecessary parentheses Shaun Ren <shaun.ren@linux.com> - 2016-02-10 19:40 +0100
[PATCH v2 3/9] Staging: rts5208: rtsx_transport.c: Remove spaces after casts Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
[PATCH v2 5/9] Staging: rts5208: rtsx_transport.c: Remove extra newlines Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
[PATCH v2 9/9] Staging: rts5208: Add missing dma_mapping_error Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
[PATCH v2 6/9] Staging: rts5208: rtsx_transport.c: Fix label naming convention Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
[PATCH v2 1/9] Staging: rts5208: rtsx_transport.c: Fix comment style warnings Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
Re: [PATCH v2 1/9] Staging: rts5208: rtsx_transport.c: Fix comment style warnings Joshua Clayton <stillcompiling@gmail.com> - 2016-02-09 04:10 +0100
[PATCH v2 8/9] Staging: rts5208: rtsx_transport.c: Fix comparisons to NULL Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
[PATCH v2 4/9] Staging: rts5208: rtsx_transport.c: Add spaces around - Shaun Ren <shaun.ren@linux.com> - 2016-02-09 02:40 +0100
[PATCH v3 3/9] Staging: rts5208: rtsx_transport.c: Remove spaces after casts Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v3 6/9] Staging: rts5208: rtsx_transport.c: Fix label naming convention Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v3 0/9] Staging: rts5208: Coding style and dma mapping fixes Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v3 8/9] Staging: rts5208: rtsx_transport.c: Fix comparisons to NULL Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v3 5/9] Staging: rts5208: rtsx_transport.c: Remove extra newlines Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v3 2/9] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
Re: [PATCH v3 2/9] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Joe Perches <joe@perches.com> - 2016-02-10 04:00 +0100
[PATCH v3 1/9] Staging: rts5208: rtsx_transport.c: Cleanup comments Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
Re: [PATCH v3 1/9] Staging: rts5208: rtsx_transport.c: Cleanup comments Joshua Clayton <stillcompiling@gmail.com> - 2016-02-10 04:30 +0100
[PATCH v3 7/9] Staging: rts5208: rtsx_transport.c: Remove unnecessary parentheses Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v3 4/9] Staging: rts5208: rtsx_transport.c: Add spaces around - Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v3 9/9] Staging: rts5208: Add missing dma_mapping_error Shaun Ren <shaun.ren@linux.com> - 2016-02-10 03:50 +0100
[PATCH v4 7/8] Staging: rts5208: rtsx_transport.c: Remove unnecessary parentheses Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v4 4/8] Staging: rts5208: rtsx_transport.c: Add spaces around - Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v4 6/8] Staging: rts5208: rtsx_transport.c: Fix label naming convention Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v4 0/8] Staging: rts5208: Fix coding style Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v4 1/8] Staging: rts5208: rtsx_transport.c: Cleanup comments Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v4 8/8] Staging: rts5208: rtsx_transport.c: Fix comparisons to NULL Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v4 2/8] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
Re: [PATCH v4 2/8] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Joe Perches <joe@perches.com> - 2016-02-12 05:40 +0100
[PATCH v5 2/8] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Shaun Ren <shaun.ren@linux.com> - 2016-02-12 08:00 +0100
Re: [PATCH v5 2/8] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Greg KH <gregkh@linuxfoundation.org> - 2016-02-15 01:50 +0100
[PATCH v4 3/8] Staging: rts5208: rtsx_transport.c: Remove spaces after casts Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v4 5/8] Staging: rts5208: rtsx_transport.c: Remove extra newlines Shaun Ren <shaun.ren@linux.com> - 2016-02-12 05:20 +0100
[PATCH v5 RESEND 3/8] Staging: rts5208: rtsx_transport.c: Remove spaces after casts Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:00 +0100
[PATCH v5 RESEND 7/8] Staging: rts5208: rtsx_transport.c: Remove unnecessary parentheses Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:00 +0100
[PATCH v5 RESEND 6/8] Staging: rts5208: rtsx_transport.c: Fix label naming convention Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:00 +0100
[PATCH v5 RESEND 8/8] Staging: rts5208: rtsx_transport.c: Fix comparisons to NULL Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:00 +0100
[PATCH v5 RESEND 0/8] Staging: rts5208: Fix coding style Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:00 +0100
[PATCH v5 RESEND 1/8] Staging: rts5208: rtsx_transport.c: Cleanup comments Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:00 +0100
[PATCH v5 RESEND 4/8] Staging: rts5208: rtsx_transport.c: Add spaces around - Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:10 +0100
[PATCH v5 RESEND 5/8] Staging: rts5208: rtsx_transport.c: Remove extra newlines Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:10 +0100
[PATCH v5 RESEND 2/8] Staging: rts5208: rtsx_transport.c: Align to open parenthesis Shaun Ren <shaun.ren@linux.com> - 2016-02-15 20:10 +0100
csiph-web