2023-03-16 15:52:17

by Sumitra Sharma

[permalink] [raw]
Subject: [PATCH v2] Staging: octeon: Fix line ending with '('

Adhere to coding-style.

Checkpatch has reported code style warning:
CHECK: Lines should not end with a '('.

Enhance the design of a function header:
Align the function parameters immediately after '(' in a single line.

Signed-off-by: Sumitra Sharma <[email protected]>
---

v2: Change patch subject and description.

drivers/staging/octeon/octeon-stubs.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/octeon/octeon-stubs.h b/drivers/staging/octeon/octeon-stubs.h
index 7a02e59e283f..3e7b92cd2e35 100644
--- a/drivers/staging/octeon/octeon-stubs.h
+++ b/drivers/staging/octeon/octeon-stubs.h
@@ -1372,9 +1372,7 @@ static inline void cvmx_fau_async_fetch_and_add32(uint64_t scraddr,
int32_t value)
{ }

-static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(
- int interface,
- int port)
+static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(int interface, int port)
{
union cvmx_gmxx_rxx_rx_inbnd r;

--
2.25.1



2023-03-16 16:10:30

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v2] Staging: octeon: Fix line ending with '('

Hi--

On 3/16/23 08:52, Sumitra Sharma wrote:
> Adhere to coding-style.
>
> Checkpatch has reported code style warning:
> CHECK: Lines should not end with a '('.
>

Not to complain about the patch, but where is this documented
in coding-style? I searched for it and cannot find it.


Thanks.

> Enhance the design of a function header:
> Align the function parameters immediately after '(' in a single line.
>
> Signed-off-by: Sumitra Sharma <[email protected]>
> ---
>
> v2: Change patch subject and description.
>
> drivers/staging/octeon/octeon-stubs.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/octeon/octeon-stubs.h b/drivers/staging/octeon/octeon-stubs.h
> index 7a02e59e283f..3e7b92cd2e35 100644
> --- a/drivers/staging/octeon/octeon-stubs.h
> +++ b/drivers/staging/octeon/octeon-stubs.h
> @@ -1372,9 +1372,7 @@ static inline void cvmx_fau_async_fetch_and_add32(uint64_t scraddr,
> int32_t value)
> { }
>
> -static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(
> - int interface,
> - int port)
> +static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(int interface, int port)
> {
> union cvmx_gmxx_rxx_rx_inbnd r;
>

--
~Randy

2023-03-16 16:16:43

by Deepak R Varma

[permalink] [raw]
Subject: Re: [PATCH v2] Staging: octeon: Fix line ending with '('

On Thu, Mar 16, 2023 at 08:52:02AM -0700, Sumitra Sharma wrote:
> Adhere to coding-style.

Hi Sumitra,
Quick background: Linux Kernel coding style required developers to limit the
code within 80 columns per line. This resulted in splitting the long lines into
more than one lines. Some splits were unwillingly poor causing checkpatch error
[e.g. this one that you are attempting to address].
Recently, the 80 column limit was relaxed to 100 columns. This now will allow to
merge some poorly split lines resulting in improved readability and avoid
checkpath complaints.

Now, can you use this information to convert your live above a little more
descriptive. Remember a well written "why" explanation is almost always
necessary.

>
> Checkpatch has reported code style warning:
> CHECK: Lines should not end with a '('.
>
> Enhance the design of a function header:
> Align the function parameters immediately after '(' in a single line.

This is the "what" part and not so useful for such a simple change. You need not
write this. However for complex changes or tricky code optimization, the "what"
part will be very useful as well.

Hope this helps.

Deepak.

>
> Signed-off-by: Sumitra Sharma <[email protected]>
> ---
>
> v2: Change patch subject and description.
>
> drivers/staging/octeon/octeon-stubs.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/octeon/octeon-stubs.h b/drivers/staging/octeon/octeon-stubs.h
> index 7a02e59e283f..3e7b92cd2e35 100644
> --- a/drivers/staging/octeon/octeon-stubs.h
> +++ b/drivers/staging/octeon/octeon-stubs.h
> @@ -1372,9 +1372,7 @@ static inline void cvmx_fau_async_fetch_and_add32(uint64_t scraddr,
> int32_t value)
> { }
>
> -static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(
> - int interface,
> - int port)
> +static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(int interface, int port)
> {
> union cvmx_gmxx_rxx_rx_inbnd r;
>
> --
> 2.25.1
>
>



2023-03-16 16:40:33

by Deepak R Varma

[permalink] [raw]
Subject: Re: [PATCH v2] Staging: octeon: Fix line ending with '('

On Thu, Mar 16, 2023 at 09:24:42AM -0700, Sumitra Sharma wrote:
> On Thu, Mar 16, 2023 at 09:45:13PM +0530, Deepak R Varma wrote:
> > On Thu, Mar 16, 2023 at 08:52:02AM -0700, Sumitra Sharma wrote:
> > > Adhere to coding-style.
> >
> > Hi Sumitra,
> > Quick background: Linux Kernel coding style required developers to limit the
> > code within 80 columns per line. This resulted in splitting the long lines into
> > more than one lines. Some splits were unwillingly poor causing checkpatch error
> > [e.g. this one that you are attempting to address].
> > Recently, the 80 column limit was relaxed to 100 columns. This now will allow to
> > merge some poorly split lines resulting in improved readability and avoid
> > checkpath complaints.
> >
> Hi Deepak
>
> Since, you mentioned that the 80 column limit has been relaxed to 100
> columns. So will then this patch be useful??

Yes, that change from 80 to 100 makes way for your patch.

Also always do a "reply-all". You can add more recipients to the email if
necessary, but you should not remove anyone.

>
> > Now, can you use this information to convert your live above a little more
> > descriptive. Remember a well written "why" explanation is almost always
> > necessary.
>
> Okay!
>
> >
> > >
> > > Checkpatch has reported code style warning:
> > > CHECK: Lines should not end with a '('.
> > >
> > > Enhance the design of a function header:
> > > Align the function parameters immediately after '(' in a single line.
> >
> > This is the "what" part and not so useful for such a simple change. You need not
> > write this. However for complex changes or tricky code optimization, the "what"
> > part will be very useful as well.
> >
>
> I will keep this in mind.
>
> > Hope this helps.
> >
> > Deepak.
> >
> > >
>
> Regards
>
> SUmitra
> > > Signed-off-by: Sumitra Sharma <[email protected]>
> > > ---
> > >
> > > v2: Change patch subject and description.
> > >
> > > drivers/staging/octeon/octeon-stubs.h | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/staging/octeon/octeon-stubs.h b/drivers/staging/octeon/octeon-stubs.h
> > > index 7a02e59e283f..3e7b92cd2e35 100644
> > > --- a/drivers/staging/octeon/octeon-stubs.h
> > > +++ b/drivers/staging/octeon/octeon-stubs.h
> > > @@ -1372,9 +1372,7 @@ static inline void cvmx_fau_async_fetch_and_add32(uint64_t scraddr,
> > > int32_t value)
> > > { }
> > >
> > > -static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(
> > > - int interface,
> > > - int port)
> > > +static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(int interface, int port)
> > > {
> > > union cvmx_gmxx_rxx_rx_inbnd r;
> > >
> > > --
> > > 2.25.1
> > >
> > >
> >
> >



2023-03-16 16:56:12

by Sumitra Sharma

[permalink] [raw]
Subject: Re: [PATCH v2] Staging: octeon: Fix line ending with '('

On Thu, Mar 16, 2023 at 10:07:57PM +0530, Deepak R Varma wrote:
> On Thu, Mar 16, 2023 at 09:24:42AM -0700, Sumitra Sharma wrote:
> > On Thu, Mar 16, 2023 at 09:45:13PM +0530, Deepak R Varma wrote:
> > > On Thu, Mar 16, 2023 at 08:52:02AM -0700, Sumitra Sharma wrote:
> > > > Adhere to coding-style.
> > >
> > > Hi Sumitra,
> > > Quick background: Linux Kernel coding style required developers to limit the
> > > code within 80 columns per line. This resulted in splitting the long lines into
> > > more than one lines. Some splits were unwillingly poor causing checkpatch error
> > > [e.g. this one that you are attempting to address].
> > > Recently, the 80 column limit was relaxed to 100 columns. This now will allow to
> > > merge some poorly split lines resulting in improved readability and avoid
> > > checkpath complaints.
> > >
> > Hi Deepak
> >
> > Since, you mentioned that the 80 column limit has been relaxed to 100
> > columns. So will then this patch be useful??
>
> Yes, that change from 80 to 100 makes way for your patch.
>
> Also always do a "reply-all". You can add more recipients to the email if
> necessary, but you should not remove anyone.
>

Okay! Thank you deepak

Regards,

Sumitra

> >
> > > Now, can you use this information to convert your live above a little more
> > > descriptive. Remember a well written "why" explanation is almost always
> > > necessary.
> >
> > Okay!
> >
> > >
> > > >
> > > > Checkpatch has reported code style warning:
> > > > CHECK: Lines should not end with a '('.
> > > >
> > > > Enhance the design of a function header:
> > > > Align the function parameters immediately after '(' in a single line.
> > >
> > > This is the "what" part and not so useful for such a simple change. You need not
> > > write this. However for complex changes or tricky code optimization, the "what"
> > > part will be very useful as well.
> > >
> >
> > I will keep this in mind.
> >
> > > Hope this helps.
> > >
> > > Deepak.
> > >
> > > >
> >
> > Regards
> >
> > SUmitra
> > > > Signed-off-by: Sumitra Sharma <[email protected]>
> > > > ---
> > > >
> > > > v2: Change patch subject and description.
> > > >
> > > > drivers/staging/octeon/octeon-stubs.h | 4 +---
> > > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/octeon/octeon-stubs.h b/drivers/staging/octeon/octeon-stubs.h
> > > > index 7a02e59e283f..3e7b92cd2e35 100644
> > > > --- a/drivers/staging/octeon/octeon-stubs.h
> > > > +++ b/drivers/staging/octeon/octeon-stubs.h
> > > > @@ -1372,9 +1372,7 @@ static inline void cvmx_fau_async_fetch_and_add32(uint64_t scraddr,
> > > > int32_t value)
> > > > { }
> > > >
> > > > -static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(
> > > > - int interface,
> > > > - int port)
> > > > +static inline union cvmx_gmxx_rxx_rx_inbnd cvmx_spi4000_check_speed(int interface, int port)
> > > > {
> > > > union cvmx_gmxx_rxx_rx_inbnd r;
> > > >
> > > > --
> > > > 2.25.1
> > > >
> > > >
> > >
> > >
>
>