From: Jonathan Cameron <jic23@kernel.org>
To: Tejun Heo <tj@kernel.org>
Cc: Xie XiuQi <xiexiuqi@huawei.com>,
Jonathan Cameron <jic23@cam.ac.uk>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Dmitry Torokhov <dtor@mail.ru>,
Thomas Gleixner <tglx@linutronix.de>,
Andres Salomon <dilinger@queued.net>, Chris Ball <cjb@laptop.org>,
Jon Nettleton <jon.nettleton@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Nadia Derbey <Nadia.Derbey@bull.net>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Li Bin <huawei.libin@huawei.com>,
"Hennerich, Michael" <Michael.Hennerich@analog.com>,
Lars-Peter Clausen <lars@metafoo.de>
Subject: Re: [PATCH 1/5] iio: adc: remove unnecessary work pending test
Date: Sat, 07 Sep 2013 21:44:44 +0100 [thread overview]
Message-ID: <522B903C.7040604@kernel.org> (raw)
In-Reply-To: <20130906152028.GA22763@mtj.dyndns.org>
ccing Michael Hennerich and Lars-Peter Clausen,
On 09/06/13 16:20, Tejun Heo wrote:
> On Fri, Sep 06, 2013 at 07:02:34PM +0800, Xie XiuQi wrote:
>> Remove unnecessary work pending test before calling schedule_work().
>> It has been tested in queue_work_on() already. No functional changed.
>>
>> Signed-off-by: Xie XiuQi <xiexiuqi@huawei.com>
>> Cc: Tejun Heo <tj@kernel.org>
>
> Reviewed-by: Tejun Heo <tj@kernel.org>
>
> One nit below tho.
>
>> - if (iio_buffer_enabled(indio_dev)) {
>> - if (!work_pending(&st->poll_work))
>> - schedule_work(&st->poll_work);
>> - } else {
>> + if (iio_buffer_enabled(indio_dev))
>> + schedule_work(&st->poll_work);
>> + else {
>> st->done = true;
>> wake_up_interruptible(&st->wq_data_avail);
>> }
>
> Please don't drop the parentheses. The convention is to have either
> both or none on if/else.
I'll fix this up on applying if everyone is happy (can't see why the
won't be, but best to check!) Michael is clearly given as the author of driver
so should probably have been in the cc list. Whilst I might apply this directly
as maintainer I much prefer if I get an ack from the driver author as if nothing
else it gives me a warm fuzzy feeling ;)
Also patch title should mention that a) this is a staging driver, b) which adc driver it
applies to. I'll fix that up as well on applying it.
Thanks for the patch though, it would probably never have been noticed otherwise!
>
> Thanks.
>
next prev parent reply other threads:[~2013-09-07 19:44 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-06 11:01 [PATCH 0/5] remove unnecessary work pending test Xie XiuQi
2013-09-06 11:02 ` [PATCH 1/5] iio: adc: " Xie XiuQi
2013-09-06 15:20 ` Tejun Heo
2013-09-07 20:44 ` Jonathan Cameron [this message]
[not found] ` <522DE7F4.1050202@analog.com>
2013-09-09 21:14 ` Jonathan Cameron
2013-09-06 11:03 ` [PATCH 2/5] input: " Xie XiuQi
2013-09-06 15:21 ` Tejun Heo
2013-09-06 11:05 ` [PATCH 3/5] lib: " Xie XiuQi
2013-09-06 15:22 ` Tejun Heo
2013-09-06 11:06 ` [PATCH 4/5] staging: olpc_dcon: " Xie XiuQi
2013-09-06 15:24 ` Tejun Heo
2013-09-06 11:07 ` [PATCH 5/5] ipc: " Xie XiuQi
2013-09-06 15:25 ` Tejun Heo
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=522B903C.7040604@kernel.org \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=Nadia.Derbey@bull.net \
--cc=akpm@linux-foundation.org \
--cc=cjb@laptop.org \
--cc=dilinger@queued.net \
--cc=dmitry.torokhov@gmail.com \
--cc=dtor@mail.ru \
--cc=huawei.libin@huawei.com \
--cc=jic23@cam.ac.uk \
--cc=jon.nettleton@gmail.com \
--cc=lars@metafoo.de \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=xiexiuqi@huawei.com \
/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