From: Jeff Johnson <quic_jjohnson@quicinc.com>
To: ErickKaranja <karanja99erick@gmail.com>,
<hvaibhav.linux@gmail.com>, <johan@kernel.org>,
<elder@kernel.org>, <gregkh@linuxfoundation.org>,
<outreachy@lists.linux.dev>
Cc: <greybus-dev@lists.linaro.org>, <linux-staging@lists.linux.dev>,
<linux-kernel@vger.kernel.org>
Subject: Re: [greybus-dev] [PATCH] staging: greybus: fix line length issue
Date: Fri, 21 Mar 2025 07:37:30 -0700 [thread overview]
Message-ID: <acbe5688-9be4-489b-8a9c-4950dc3b8133@quicinc.com> (raw)
In-Reply-To: <20250321115545.24468-1-karanja99erick@gmail.com>
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
prev parent reply other threads:[~2025-03-21 14:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-21 11:55 [PATCH] staging: greybus: fix line length issue ErickKaranja
2025-03-21 12:48 ` Julia Lawall
2025-03-21 12:52 ` Greg KH
2025-03-21 14:14 ` Alex Elder
2025-03-21 14:37 ` Jeff Johnson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acbe5688-9be4-489b-8a9c-4950dc3b8133@quicinc.com \
--to=quic_jjohnson@quicinc.com \
--cc=elder@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=hvaibhav.linux@gmail.com \
--cc=johan@kernel.org \
--cc=karanja99erick@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=outreachy@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox