[PATCH] staging: vt6655: rearrange lines exceeding 100 columns

[PATCH] staging: vt6655: rearrange lines exceeding 100 columns

[Deepak R Varma]

Rearrange lines that are longer than 100 columns width. Issue reported
by chckpatch script.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>

---

 drivers/staging/vt6655/device_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 09ab6d6f2429..4fc90b111f9d 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -461,7 +461,10 @@ static bool device_init_rings(struct vnt_private *priv)
 		priv->opts.rx_descs0 * sizeof(struct vnt_rx_desc);
 
 	priv->tx0_bufs = dma_alloc_coherent(&priv->pcid->dev,
-					    priv->opts.tx_descs[0] * PKT_BUF_SZ + priv->opts.tx_descs[1] * PKT_BUF_SZ + CB_BEACON_BUF_SIZE + CB_MAX_BUF_SIZE,
+					    priv->opts.tx_descs[0] * PKT_BUF_SZ +
+					    priv->opts.tx_descs[1] * PKT_BUF_SZ +
+					    CB_BEACON_BUF_SIZE +
+					    CB_MAX_BUF_SIZE,
 					    &priv->tx_bufs_dma0, GFP_ATOMIC);
 	if (!priv->tx0_bufs) {
 		dev_err(&priv->pcid->dev, "allocate buf dma memory failed\n");
@@ -1077,7 +1080,8 @@ static void vnt_interrupt_process(struct vnt_private *priv)
 			    priv->op_mode == NL80211_IFTYPE_ADHOC) &&
 			    priv->vif->bss_conf.enable_beacon) {
 				MACvOneShotTimer1MicroSec(priv,
-							  (priv->vif->bss_conf.beacon_int - MAKE_BEACON_RESERVED) << 10);
+							  (priv->vif->bss_conf.beacon_int -
+							   MAKE_BEACON_RESERVED) << 10);
 			}
 
 			/* TODO: adhoc PS mode */
-- 
2.25.1

Re: [Outreachy kernel] [PATCH] staging: vt6655: rearrange lines exceeding 100 columns

[Julia Lawall]

On Wed, 14 Oct 2020, Deepak R Varma wrote:

> Rearrange lines that are longer than 100 columns width. Issue reported
> by chckpatch script.
>
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>
> ---
>
>  drivers/staging/vt6655/device_main.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
> index 09ab6d6f2429..4fc90b111f9d 100644
> --- a/drivers/staging/vt6655/device_main.c
> +++ b/drivers/staging/vt6655/device_main.c
> @@ -461,7 +461,10 @@ static bool device_init_rings(struct vnt_private *priv)
>  		priv->opts.rx_descs0 * sizeof(struct vnt_rx_desc);
>
>  	priv->tx0_bufs = dma_alloc_coherent(&priv->pcid->dev,
> -					    priv->opts.tx_descs[0] * PKT_BUF_SZ + priv->opts.tx_descs[1] * PKT_BUF_SZ + CB_BEACON_BUF_SIZE + CB_MAX_BUF_SIZE,
> +					    priv->opts.tx_descs[0] * PKT_BUF_SZ +
> +					    priv->opts.tx_descs[1] * PKT_BUF_SZ +
> +					    CB_BEACON_BUF_SIZE +
> +					    CB_MAX_BUF_SIZE,
>  					    &priv->tx_bufs_dma0, GFP_ATOMIC);
>  	if (!priv->tx0_bufs) {
>  		dev_err(&priv->pcid->dev, "allocate buf dma memory failed\n");
> @@ -1077,7 +1080,8 @@ static void vnt_interrupt_process(struct vnt_private *priv)
>  			    priv->op_mode == NL80211_IFTYPE_ADHOC) &&
>  			    priv->vif->bss_conf.enable_beacon) {
>  				MACvOneShotTimer1MicroSec(priv,
> -							  (priv->vif->bss_conf.beacon_int - MAKE_BEACON_RESERVED) << 10);
> +							  (priv->vif->bss_conf.beacon_int -
> +							   MAKE_BEACON_RESERVED) << 10);

Maybe it would make more sense to shorten the function name?

Also the {} are not needed.

julia


>  			}
>
>  			/* TODO: adhoc PS mode */
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20201014154121.GA23584%40ubuntu204.
>

Re: [Outreachy kernel] [PATCH] staging: vt6655: rearrange lines exceeding 100 columns

[Deepak R Varma]

On Wed, Oct 14, 2020 at 06:01:39PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 14 Oct 2020, Deepak R Varma wrote:
> 
> > Rearrange lines that are longer than 100 columns width. Issue reported
> > by chckpatch script.
> >
> > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> >
> > ---
> >
> >  drivers/staging/vt6655/device_main.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
> > index 09ab6d6f2429..4fc90b111f9d 100644
> > --- a/drivers/staging/vt6655/device_main.c
> > +++ b/drivers/staging/vt6655/device_main.c
> > @@ -461,7 +461,10 @@ static bool device_init_rings(struct vnt_private *priv)
> >  		priv->opts.rx_descs0 * sizeof(struct vnt_rx_desc);
> >
> >  	priv->tx0_bufs = dma_alloc_coherent(&priv->pcid->dev,
> > -					    priv->opts.tx_descs[0] * PKT_BUF_SZ + priv->opts.tx_descs[1] * PKT_BUF_SZ + CB_BEACON_BUF_SIZE + CB_MAX_BUF_SIZE,
> > +					    priv->opts.tx_descs[0] * PKT_BUF_SZ +
> > +					    priv->opts.tx_descs[1] * PKT_BUF_SZ +
> > +					    CB_BEACON_BUF_SIZE +
> > +					    CB_MAX_BUF_SIZE,
> >  					    &priv->tx_bufs_dma0, GFP_ATOMIC);
> >  	if (!priv->tx0_bufs) {
> >  		dev_err(&priv->pcid->dev, "allocate buf dma memory failed\n");
> > @@ -1077,7 +1080,8 @@ static void vnt_interrupt_process(struct vnt_private *priv)
> >  			    priv->op_mode == NL80211_IFTYPE_ADHOC) &&
> >  			    priv->vif->bss_conf.enable_beacon) {
> >  				MACvOneShotTimer1MicroSec(priv,
> > -							  (priv->vif->bss_conf.beacon_int - MAKE_BEACON_RESERVED) << 10);
> > +							  (priv->vif->bss_conf.beacon_int -
> > +							   MAKE_BEACON_RESERVED) << 10);
> 
> Maybe it would make more sense to shorten the function name?
> 
Thank you Julia.
I do not think that will work. Even without the function name,
the line would still cross 100 columns if you try to fit it on a single
line. Also, shortening the function name would make it looks different
from rest of similar named functions.
Please suggest how would you do it?

> Also the {} are not needed.
> 
Agree if we can get the instruction on a single line. Else, I think it
is better to keep the {} to make it more readable.

Thank you.
Deepak.

> julia
> 
> 
> >  			}
> >
> >  			/* TODO: adhoc PS mode */
> > --
> > 2.25.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20201014154121.GA23584%40ubuntu204.
> >

Re: [Outreachy kernel] [PATCH] staging: vt6655: rearrange lines exceeding 100 columns

[Julia Lawall]

On Wed, 14 Oct 2020, Deepak R Varma wrote:

> On Wed, Oct 14, 2020 at 06:01:39PM +0200, Julia Lawall wrote:
> >
> >
> > On Wed, 14 Oct 2020, Deepak R Varma wrote:
> >
> > > Rearrange lines that are longer than 100 columns width. Issue reported
> > > by chckpatch script.
> > >
> > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > >
> > > ---
> > >
> > >  drivers/staging/vt6655/device_main.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
> > > index 09ab6d6f2429..4fc90b111f9d 100644
> > > --- a/drivers/staging/vt6655/device_main.c
> > > +++ b/drivers/staging/vt6655/device_main.c
> > > @@ -461,7 +461,10 @@ static bool device_init_rings(struct vnt_private *priv)
> > >  		priv->opts.rx_descs0 * sizeof(struct vnt_rx_desc);
> > >
> > >  	priv->tx0_bufs = dma_alloc_coherent(&priv->pcid->dev,
> > > -					    priv->opts.tx_descs[0] * PKT_BUF_SZ + priv->opts.tx_descs[1] * PKT_BUF_SZ + CB_BEACON_BUF_SIZE + CB_MAX_BUF_SIZE,
> > > +					    priv->opts.tx_descs[0] * PKT_BUF_SZ +
> > > +					    priv->opts.tx_descs[1] * PKT_BUF_SZ +
> > > +					    CB_BEACON_BUF_SIZE +
> > > +					    CB_MAX_BUF_SIZE,
> > >  					    &priv->tx_bufs_dma0, GFP_ATOMIC);
> > >  	if (!priv->tx0_bufs) {
> > >  		dev_err(&priv->pcid->dev, "allocate buf dma memory failed\n");
> > > @@ -1077,7 +1080,8 @@ static void vnt_interrupt_process(struct vnt_private *priv)
> > >  			    priv->op_mode == NL80211_IFTYPE_ADHOC) &&
> > >  			    priv->vif->bss_conf.enable_beacon) {
> > >  				MACvOneShotTimer1MicroSec(priv,
> > > -							  (priv->vif->bss_conf.beacon_int - MAKE_BEACON_RESERVED) << 10);
> > > +							  (priv->vif->bss_conf.beacon_int -
> > > +							   MAKE_BEACON_RESERVED) << 10);
> >
> > Maybe it would make more sense to shorten the function name?
> >
> Thank you Julia.
> I do not think that will work. Even without the function name,
> the line would still cross 100 columns if you try to fit it on a single
> line. Also, shortening the function name would make it looks different
> from rest of similar named functions.
> Please suggest how would you do it?

Perhaps the whole thing can't fit on one line, but if the function name
were shorter, then perhaps the newline would go in a different place.

All of the function names in that file do not follow the kernel coding
style.  Admittedly it would not be such a great idea to change only one of
them.

> > Also the {} are not needed.
> >
> Agree if we can get the instruction on a single line. Else, I think it
> is better to keep the {} to make it more readable.

Coding style doesn't say anything about that.  The only exception proposed
is that when one of the branches requires {}, then the other should use {}
as well.

linux/Documentation/process/coding-style.rst

julia

Re: [Outreachy kernel] [PATCH] staging: vt6655: rearrange lines exceeding 100 columns

[Deepak R Varma]

On Wed, Oct 14, 2020 at 06:46:09PM +0200, Julia Lawall wrote:
> 
> 
> On Wed, 14 Oct 2020, Deepak R Varma wrote:
> 
> > On Wed, Oct 14, 2020 at 06:01:39PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Wed, 14 Oct 2020, Deepak R Varma wrote:
> > >
> > > > Rearrange lines that are longer than 100 columns width. Issue reported
> > > > by chckpatch script.
> > > >
> > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > >
> > > > ---
> > > >
> > > >  drivers/staging/vt6655/device_main.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
> > > > index 09ab6d6f2429..4fc90b111f9d 100644
> > > > --- a/drivers/staging/vt6655/device_main.c
> > > > +++ b/drivers/staging/vt6655/device_main.c
> > > > @@ -461,7 +461,10 @@ static bool device_init_rings(struct vnt_private *priv)
> > > >  		priv->opts.rx_descs0 * sizeof(struct vnt_rx_desc);
> > > >
> > > >  	priv->tx0_bufs = dma_alloc_coherent(&priv->pcid->dev,
> > > > -					    priv->opts.tx_descs[0] * PKT_BUF_SZ + priv->opts.tx_descs[1] * PKT_BUF_SZ + CB_BEACON_BUF_SIZE + CB_MAX_BUF_SIZE,
> > > > +					    priv->opts.tx_descs[0] * PKT_BUF_SZ +
> > > > +					    priv->opts.tx_descs[1] * PKT_BUF_SZ +
> > > > +					    CB_BEACON_BUF_SIZE +
> > > > +					    CB_MAX_BUF_SIZE,
> > > >  					    &priv->tx_bufs_dma0, GFP_ATOMIC);
> > > >  	if (!priv->tx0_bufs) {
> > > >  		dev_err(&priv->pcid->dev, "allocate buf dma memory failed\n");
> > > > @@ -1077,7 +1080,8 @@ static void vnt_interrupt_process(struct vnt_private *priv)
> > > >  			    priv->op_mode == NL80211_IFTYPE_ADHOC) &&
> > > >  			    priv->vif->bss_conf.enable_beacon) {
> > > >  				MACvOneShotTimer1MicroSec(priv,
> > > > -							  (priv->vif->bss_conf.beacon_int - MAKE_BEACON_RESERVED) << 10);
> > > > +							  (priv->vif->bss_conf.beacon_int -
> > > > +							   MAKE_BEACON_RESERVED) << 10);
> > >
> > > Maybe it would make more sense to shorten the function name?
> > >
> > Thank you Julia.
> > I do not think that will work. Even without the function name,
> > the line would still cross 100 columns if you try to fit it on a single
> > line. Also, shortening the function name would make it looks different
> > from rest of similar named functions.
> > Please suggest how would you do it?
> 
> Perhaps the whole thing can't fit on one line, but if the function name
> were shorter, then perhaps the newline would go in a different place.
> 
> All of the function names in that file do not follow the kernel coding
> style.  Admittedly it would not be such a great idea to change only one of
> them.
> 
> > > Also the {} are not needed.
> > >
> > Agree if we can get the instruction on a single line. Else, I think it
> > is better to keep the {} to make it more readable.
> 
> Coding style doesn't say anything about that.  The only exception proposed
> is that when one of the branches requires {}, then the other should use {}
> as well.
> 
> linux/Documentation/process/coding-style.rst
> 
Yes, agreed. I will implement this feedback and resend v2.

Thank you again!
Deepak.

> julia