From: Aleksa Zdravkovic <alex.zdravkovic98@gmail.com>
To: Joe Perches <joe@perches.com>, Dan Carpenter <dan.carpenter@oracle.com>
Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: axis-fifo: Fix line over 80 characters error
Date: Thu, 25 Oct 2018 16:44:02 +0200 [thread overview]
Message-ID: <20181025144401.GA27089@zdravcePC> (raw)
In-Reply-To: <cc426006c1d69f9fd464afe96a62e9e745b2078b.camel@perches.com>
On Wed, Oct 24, 2018 at 11:18:17PM -0700, Joe Perches wrote:
> On Thu, 2018-10-25 at 09:05 +0300, Dan Carpenter wrote:
> > On Wed, Oct 24, 2018 at 05:05:53PM +0200, Aleksa Zdravkovic wrote:
> > > This patch fixes the checkpatch.pl warning:
> []
> > > diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> []
> > > @@ -482,10 +482,10 @@ static ssize_t axis_fifo_write(struct file *f, const char __user *buf,
> > > spin_lock_irq(&fifo->write_queue_lock);
> > > ret = wait_event_interruptible_lock_irq_timeout
> > > (fifo->write_queue,
> > > - ioread32(fifo->base_addr + XLLF_TDFV_OFFSET)
> > > + ioread32(fifo->base_addr + XLLF_TDFV_OFFSET)
> > > >= words_to_write,
> > > - fifo->write_queue_lock,
> > > - (write_timeout >= 0) ? msecs_to_jiffies(write_timeout) :
> > > + fifo->write_queue_lock,
> > > + (write_timeout >= 0) ? msecs_to_jiffies(write_timeout) :
> > > MAX_SCHEDULE_TIMEOUT);
> >
> > The original was fine. Just leave it.
> >
> > Checkpatch.pl is only useful if it improves the readability for humans.
>
> True, but I think the original is just OK.
>
> Any suggestion on how to make the thing better?
>
> wait_event_interruptible_lock_irq_timeout is a fairly long
> identifier with multiple long arguments.
>
> It's as if it should be written here as
>
> ret = wait_event_interruptible_lock_irq_timeout(fifo->write_queue,
> ioread32(fifo->base_addr + XLLF_TDFV_OFFSET) >= words_to_write,
> fifo->write_queue_lock,
> write_timeout >= 0 ? msecs_to_jiffies(write_timeout) : MAX_SCHEDULE_TIMEOUT);
>
> where the longest is way over 80 chars, (140?) but I simply don't care
> because it's just that much more readable for me.
>
>
Thank you Dan and Joe for your feedback.
I don't have any suggestion how to improve this code otherwise.
I will try to find a way to improve it. Maybe we can define some
macros but I don't think it would help much.
prev parent reply other threads:[~2018-10-25 14:44 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-24 15:05 [PATCH] staging: axis-fifo: Fix line over 80 characters error Aleksa Zdravkovic
2018-10-25 6:05 ` Dan Carpenter
2018-10-25 6:18 ` Joe Perches
2018-10-25 14:44 ` Aleksa Zdravkovic [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=20181025144401.GA27089@zdravcePC \
--to=alex.zdravkovic98@gmail.com \
--cc=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.