[PATCH] staging: greybus: fix line length issue

[PATCH] staging: greybus: fix line length issue

[ErickKaranja]

Fix line length exceeding 100 columns in arche-platform.c. The code now
follows Linux kernel coding style guidelines by keeping lines under the
maximum allowed length of 100 characters.

Reported by checkpatch:

CHECK: line length of 101 exceeds 100 columns

Signed-off-by: ErickKaranja <karanja99erick@gmail.com>
---
 drivers/staging/greybus/arche-platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
index d48464390f58..1a82a7a3991c 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -179,8 +179,8 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
 				 */
 				if (arche_pdata->wake_detect_state !=
 						WD_STATE_COLDBOOT_START) {
-					arche_platform_set_wake_detect_state(arche_pdata,
-									     WD_STATE_COLDBOOT_TRIG);
+					arche_platform_set_wake_detect_state
+						(arche_pdata, WD_STATE_COLDBOOT_TRIG);
 					spin_unlock_irqrestore(&arche_pdata->wake_lock,
 							       flags);
 					return IRQ_WAKE_THREAD;
-- 
2.43.0

Re: [PATCH] staging: greybus: fix line length issue

[Julia Lawall]

On Fri, 21 Mar 2025, ErickKaranja wrote:

> Fix line length exceeding 100 columns in arche-platform.c. The code now
> follows Linux kernel coding style guidelines by keeping lines under the
> maximum allowed length of 100 characters.

Thanks for the patch.

In your log message you can try to avoid the word "Fix".  Probably most
patches fix something, so it doesn't give much information.  And it
doesn't say anything about how you fixed it.  Here it could be "Reduce a
line that exceeds 100 columns".  You don't have to mention the file name,
because one can easily see that from looking at the patch.

>
> Reported by checkpatch:
>
> CHECK: line length of 101 exceeds 100 columns
>
> Signed-off-by: ErickKaranja <karanja99erick@gmail.com>

Here you should have Erick Karanja, not ErickKaranja.

thanks,
julia

> ---
>  drivers/staging/greybus/arche-platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
> index d48464390f58..1a82a7a3991c 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -179,8 +179,8 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
>  				 */
>  				if (arche_pdata->wake_detect_state !=
>  						WD_STATE_COLDBOOT_START) {
> -					arche_platform_set_wake_detect_state(arche_pdata,
> -									     WD_STATE_COLDBOOT_TRIG);
> +					arche_platform_set_wake_detect_state
> +						(arche_pdata, WD_STATE_COLDBOOT_TRIG);
>  					spin_unlock_irqrestore(&arche_pdata->wake_lock,
>  							       flags);
>  					return IRQ_WAKE_THREAD;
> --
> 2.43.0
>
>
>

Re: [PATCH] staging: greybus: fix line length issue

[Greg KH]

On Fri, Mar 21, 2025 at 02:55:45PM +0300, ErickKaranja wrote:
> Fix line length exceeding 100 columns in arche-platform.c. The code now
> follows Linux kernel coding style guidelines by keeping lines under the
> maximum allowed length of 100 characters.
> 
> Reported by checkpatch:
> 
> CHECK: line length of 101 exceeds 100 columns
> 
> Signed-off-by: ErickKaranja <karanja99erick@gmail.com>
> ---
>  drivers/staging/greybus/arche-platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
> index d48464390f58..1a82a7a3991c 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -179,8 +179,8 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
>  				 */
>  				if (arche_pdata->wake_detect_state !=
>  						WD_STATE_COLDBOOT_START) {
> -					arche_platform_set_wake_detect_state(arche_pdata,
> -									     WD_STATE_COLDBOOT_TRIG);
> +					arche_platform_set_wake_detect_state
> +						(arche_pdata, WD_STATE_COLDBOOT_TRIG);

With this change, you will now have added a different checkpatch
warning.  So the original should stay as-is, sorry.

thanks,

greg k-h

Re: [PATCH] staging: greybus: fix line length issue

[Alex Elder]

On 3/21/25 6:55 AM, ErickKaranja wrote:
> Fix line length exceeding 100 columns in arche-platform.c. The code now
> follows Linux kernel coding style guidelines by keeping lines under the
> maximum allowed length of 100 characters.
> 
> Reported by checkpatch:
> 
> CHECK: line length of 101 exceeds 100 columns
> 
> Signed-off-by: ErickKaranja <karanja99erick@gmail.com>

Sometimes a good strategy for reducing long lines (which, as in this
case, is partially due to indentation) is to define a helper function
to isolate the code (and reduce the indentation level).

Another approach can be to define local variables so that the arguments
don't make the line too long.  Here though, the name of the function
(arche_platform_set_wake_detect_state()) is pretty wide.

Ultimately the goal is readability and comprehension.  It's not always
cut and dried the best way to improve things.

					-Alex


> ---
>   drivers/staging/greybus/arche-platform.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
> index d48464390f58..1a82a7a3991c 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -179,8 +179,8 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
>   				 */
>   				if (arche_pdata->wake_detect_state !=
>   						WD_STATE_COLDBOOT_START) {
> -					arche_platform_set_wake_detect_state(arche_pdata,
> -									     WD_STATE_COLDBOOT_TRIG);
> +					arche_platform_set_wake_detect_state
> +						(arche_pdata, WD_STATE_COLDBOOT_TRIG);
>   					spin_unlock_irqrestore(&arche_pdata->wake_lock,
>   							       flags);
>   					return IRQ_WAKE_THREAD;

Re: [greybus-dev] [PATCH] staging: greybus: fix line length issue

[Jeff Johnson]

On 3/21/2025 4:55 AM, ErickKaranja wrote:
> Fix line length exceeding 100 columns in arche-platform.c. The code now
> follows Linux kernel coding style guidelines by keeping lines under the
> maximum allowed length of 100 characters.
> 
> Reported by checkpatch:
> 
> CHECK: line length of 101 exceeds 100 columns
> 
> Signed-off-by: ErickKaranja <karanja99erick@gmail.com>
> ---
>  drivers/staging/greybus/arche-platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
> index d48464390f58..1a82a7a3991c 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -179,8 +179,8 @@ static irqreturn_t arche_platform_wd_irq(int irq, void *devid)
>  				 */
>  				if (arche_pdata->wake_detect_state !=
>  						WD_STATE_COLDBOOT_START) {
> -					arche_platform_set_wake_detect_state(arche_pdata,
> -									     WD_STATE_COLDBOOT_TRIG);
> +					arche_platform_set_wake_detect_state
> +						(arche_pdata, WD_STATE_COLDBOOT_TRIG);
>  					spin_unlock_irqrestore(&arche_pdata->wake_lock,
>  							       flags);
>  					return IRQ_WAKE_THREAD;

I see you already received some good feedback. However I think the most
important feedback wasn't given, namely that checkpatch performs rigorous
enforcement of Coding Style guidelines that themselves are not always rigorous.

For line length the Coding Style still says the "preferred limit" is 80
columns. But whether it is 80 or 100, the Coding Style has this very important
caveat: Statements longer than [the limit] should be broken into sensible
chunks, unless exceeding [the limit] significantly increases readability.

<https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings>

Obviously this is subjective, but for me the original code is significantly
more readable than the patched code.

So the takeaway is that not every checkpatch issue should be "fixed".

/jeff