Re: [PATCH] staging: rts5208: rtsx.c: Fix checkpatch warnings.

[Sudip Mukherjee]

On Thu, Aug 25, 2016 at 09:59:46PM +0800, Ming-Chia Chung wrote:
> On Tue, Aug 23, 2016 at 12:01:36PM +0530, Sudip Mukherjee wrote:
> > On Tue, Aug 23, 2016 at 08:19:46AM +0800, Ming-Chia Chung wrote:
> > > On Mon, Aug 22, 2016 at 10:51:27PM +0530, Sudip Mukherjee wrote:
> > > > On Tue, Aug 23, 2016 at 12:54:17AM +0800, MingChia Chung wrote:
> > > > >   This patch fixes a minor checkpatch warnings:
> > > > > 
> > > > >   "WARNING: Block comments use a trailing */ on a separate line"
> > > > > 
> > > > >   Signed-off-by: Ming-Chia Chung 
> > > > > ---
> > > > >  drivers/staging/rts5208/rtsx.c | 42 
> > > > > --
> > > > >  1 file changed, 28 insertions(+), 14 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/staging/rts5208/rtsx.c 
> > > > > b/drivers/staging/rts5208/rtsx.c
> > > > > index e992e03..a880411 100644
> > > > > --- a/drivers/staging/rts5208/rtsx.c
> > > > > +++ b/drivers/staging/rts5208/rtsx.c
> > > > > @@ -81,14 +81,16 @@ static int slave_alloc(struct scsi_device *sdev)
> > > > >  
> > > > >  static int slave_configure(struct scsi_device *sdev)
> > > > >  {
> > > > > - /* Scatter-gather buffers (all but the last) must have a length
> > > > > + /*
> > > > > +   * Scatter-gather buffers (all but the last) must have a length
> > > > 
> > > > The alignment doesnot look correct here. Also in some more places later
> > > > in this patch.
> > > > 
> > > > regards
> > > > Sudip
> > > 
> > > I sent this patch by following docs ([1] and [2]). The style for long
> > > comments needs the begining and ending almost-blank lines. There is no
> > > the begining almost-blank line only for the files in net/ and
> > > drivers/net/.
> > > Please let me know what I misunderstood.
> > 
> > After applying the patch the file was like:
> > /*
> >* Scatter-gather buffers (all but the last) must have a length
> >  * divisible by the bulk maxpacket size.  Otherwise a data packet
> >  * would end up being short, causing a premature end to the data
> >  * transfer.  Since high-speed bulk pipes have a maxpacket size
> > 
> > See the alignment of the second line. Please use tab instead of space. And 
> > also
> > checkpatch should have warned you not to use space before the Signed-off-by.
> > 
> 
> Thank you for detailed explanation. I enabled "expandtab" in vim so that
> I didn't realize the alignment is wrong. Now, I install 'linuxsty.vim'
> to view the code. But checkpatch.pl didn't show the warning of space.
> Here is my command:
> `kernels/staging$ perl scripts/checkpatch.pl -f 
> drivers/staging/rts5208/rtsx.c`

Please run checkpatch on your patch and it will show you the warning
about space before Signed-off-by. But it missed the place where space
was used inplace of tab. Maybe you can send a patch to fix checkpatch.

Added Cc: to Joe and Andy.

regards
sudip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rts5208: rtsx.c: Fix checkpatch warnings.

[Ming-Chia Chung]

On Tue, Aug 23, 2016 at 12:01:36PM +0530, Sudip Mukherjee wrote:
> On Tue, Aug 23, 2016 at 08:19:46AM +0800, Ming-Chia Chung wrote:
> > On Mon, Aug 22, 2016 at 10:51:27PM +0530, Sudip Mukherjee wrote:
> > > On Tue, Aug 23, 2016 at 12:54:17AM +0800, MingChia Chung wrote:
> > > >   This patch fixes a minor checkpatch warnings:
> > > > 
> > > >   "WARNING: Block comments use a trailing */ on a separate line"
> > > > 
> > > >   Signed-off-by: Ming-Chia Chung 
> > > > ---
> > > >  drivers/staging/rts5208/rtsx.c | 42 
> > > > --
> > > >  1 file changed, 28 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/rts5208/rtsx.c 
> > > > b/drivers/staging/rts5208/rtsx.c
> > > > index e992e03..a880411 100644
> > > > --- a/drivers/staging/rts5208/rtsx.c
> > > > +++ b/drivers/staging/rts5208/rtsx.c
> > > > @@ -81,14 +81,16 @@ static int slave_alloc(struct scsi_device *sdev)
> > > >  
> > > >  static int slave_configure(struct scsi_device *sdev)
> > > >  {
> > > > -   /* Scatter-gather buffers (all but the last) must have a length
> > > > +   /*
> > > > +   * Scatter-gather buffers (all but the last) must have a length
> > > 
> > > The alignment doesnot look correct here. Also in some more places later
> > > in this patch.
> > > 
> > > regards
> > > Sudip
> > 
> > I sent this patch by following docs ([1] and [2]). The style for long
> > comments needs the begining and ending almost-blank lines. There is no
> > the begining almost-blank line only for the files in net/ and
> > drivers/net/.
> > Please let me know what I misunderstood.
> 
> After applying the patch the file was like:
> /*
>* Scatter-gather buffers (all but the last) must have a length
>  * divisible by the bulk maxpacket size.  Otherwise a data packet
>  * would end up being short, causing a premature end to the data
>  * transfer.  Since high-speed bulk pipes have a maxpacket size
> 
> See the alignment of the second line. Please use tab instead of space. And 
> also
> checkpatch should have warned you not to use space before the Signed-off-by.
> 
> regards
> sudip

Thank you for detailed explanation. I enabled "expandtab" in vim so that
I didn't realize the alignment is wrong. Now, I install 'linuxsty.vim'
to view the code. But checkpatch.pl didn't show the warning of space.
Here is my command:
`kernels/staging$ perl scripts/checkpatch.pl -f drivers/staging/rts5208/rtsx.c`

Thanks,
MingChia
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rts5208: rtsx.c: Fix checkpatch warnings.

[Sudip Mukherjee]

On Tue, Aug 23, 2016 at 08:19:46AM +0800, Ming-Chia Chung wrote:
> On Mon, Aug 22, 2016 at 10:51:27PM +0530, Sudip Mukherjee wrote:
> > On Tue, Aug 23, 2016 at 12:54:17AM +0800, MingChia Chung wrote:
> > >   This patch fixes a minor checkpatch warnings:
> > > 
> > >   "WARNING: Block comments use a trailing */ on a separate line"
> > > 
> > >   Signed-off-by: Ming-Chia Chung 
> > > ---
> > >  drivers/staging/rts5208/rtsx.c | 42 
> > > --
> > >  1 file changed, 28 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/staging/rts5208/rtsx.c 
> > > b/drivers/staging/rts5208/rtsx.c
> > > index e992e03..a880411 100644
> > > --- a/drivers/staging/rts5208/rtsx.c
> > > +++ b/drivers/staging/rts5208/rtsx.c
> > > @@ -81,14 +81,16 @@ static int slave_alloc(struct scsi_device *sdev)
> > >  
> > >  static int slave_configure(struct scsi_device *sdev)
> > >  {
> > > - /* Scatter-gather buffers (all but the last) must have a length
> > > + /*
> > > +   * Scatter-gather buffers (all but the last) must have a length
> > 
> > The alignment doesnot look correct here. Also in some more places later
> > in this patch.
> > 
> > regards
> > Sudip
> 
> I sent this patch by following docs ([1] and [2]). The style for long
> comments needs the begining and ending almost-blank lines. There is no
> the begining almost-blank line only for the files in net/ and
> drivers/net/.
> Please let me know what I misunderstood.

After applying the patch the file was like:
/*
   * Scatter-gather buffers (all but the last) must have a length
 * divisible by the bulk maxpacket size.  Otherwise a data packet
 * would end up being short, causing a premature end to the data
 * transfer.  Since high-speed bulk pipes have a maxpacket size

See the alignment of the second line. Please use tab instead of space. And also
checkpatch should have warned you not to use space before the Signed-off-by.

regards
sudip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rts5208: rtsx.c: Fix checkpatch warnings.

[Ming-Chia Chung]

On Mon, Aug 22, 2016 at 10:51:27PM +0530, Sudip Mukherjee wrote:
> On Tue, Aug 23, 2016 at 12:54:17AM +0800, MingChia Chung wrote:
> >   This patch fixes a minor checkpatch warnings:
> > 
> >   "WARNING: Block comments use a trailing */ on a separate line"
> > 
> >   Signed-off-by: Ming-Chia Chung 
> > ---
> >  drivers/staging/rts5208/rtsx.c | 42 
> > --
> >  1 file changed, 28 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
> > index e992e03..a880411 100644
> > --- a/drivers/staging/rts5208/rtsx.c
> > +++ b/drivers/staging/rts5208/rtsx.c
> > @@ -81,14 +81,16 @@ static int slave_alloc(struct scsi_device *sdev)
> >  
> >  static int slave_configure(struct scsi_device *sdev)
> >  {
> > -   /* Scatter-gather buffers (all but the last) must have a length
> > +   /*
> > +   * Scatter-gather buffers (all but the last) must have a length
> 
> The alignment doesnot look correct here. Also in some more places later
> in this patch.
> 
> regards
> Sudip

I sent this patch by following docs ([1] and [2]). The style for long
comments needs the begining and ending almost-blank lines. There is no
the begining almost-blank line only for the files in net/ and
drivers/net/.
Please let me know what I misunderstood.

[1]: Linux kernel coding style, ch. 8 Commenting
https://www.kernel.org/doc/Documentation/CodingStyle
[2]: Linus Torvalds on comment styles
https://lkml.org/lkml/2016/7/8/625

regards,
MingChia
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rts5208: rtsx.c: Fix checkpatch warnings.

[Sudip Mukherjee]

On Tue, Aug 23, 2016 at 12:54:17AM +0800, MingChia Chung wrote:
>   This patch fixes a minor checkpatch warnings:
> 
>   "WARNING: Block comments use a trailing */ on a separate line"
> 
>   Signed-off-by: Ming-Chia Chung 
> ---
>  drivers/staging/rts5208/rtsx.c | 42 
> --
>  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
> index e992e03..a880411 100644
> --- a/drivers/staging/rts5208/rtsx.c
> +++ b/drivers/staging/rts5208/rtsx.c
> @@ -81,14 +81,16 @@ static int slave_alloc(struct scsi_device *sdev)
>  
>  static int slave_configure(struct scsi_device *sdev)
>  {
> - /* Scatter-gather buffers (all but the last) must have a length
> + /*
> +   * Scatter-gather buffers (all but the last) must have a length

The alignment doesnot look correct here. Also in some more places later
in this patch.

regards
Sudip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel