From: Jonathan Woithe <jwoithe@just42.net>
To: Micha?? K??pie?? <kernel@kempniu.pl>
Cc: Darren Hart <dvhart@infradead.org>,
Andy Shevchenko <andy@infradead.org>,
platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/8] fujitsu-laptop: use sparse keymaps for input event handling
Date: Wed, 29 Mar 2017 09:30:29 +1030 [thread overview]
Message-ID: <20170328230029.GA29146@marvin.atrad.com.au> (raw)
In-Reply-To: <20170328061618.GA3694@ozzy.nask.waw.pl>
On Tue, Mar 28, 2017 at 08:16:18AM +0200, Micha?? K??pie?? wrote:
> > On Fri, Mar 24, 2017 at 09:19:59PM +1030, Jonathan Woithe wrote:
> > > On Mon, Mar 20, 2017 at 10:32:16AM +0100, Micha?? K??pie?? wrote:
> > > > This series simplifies handling of both brightness key and hotkey input
> > > > events on Fujitsu laptops by making use of sparse keymaps. This not
> > > > only makes the driver shorter and, hopefully, cleaner, but also enables
> > > > us to get rid of the keycodeX fields inside struct fujitsu_bl, which
> > > > facilitates further cleanups. Also, to simplify error handling, input
> > > > devices registered by fujitsu-laptop are migrated to the devres API
> > > > along the way.
> > > > :
> > I have completed my initial review of this patch series. Aside from the
> > single recommendation about patch 7/8 (posted separately) it looks good.
> > I await your thoughts regarding patch 7/8 so we can finalise and sign off on
> > this series.
>
> Thanks for the review, Jonathan. I agree with your remark regarding the
> potentially confusing name of the variable holding the S64x0 keymap.
>
> As in the past, we have at least two options: I can either post v2 of
> all eight patches with three characters changed or you can provide your
> Reviewed-by for v1, in which case I will kindly ask the maintainers to
> run:
>
> sed -i 's|s6400|s64x0|;' drivers/platform/x86/fujitsu-laptop.c
>
> after applying patch 7/8. Given that this series does a bit more than
> the cleanup series I posted previously, I sense it might be a good idea
> to defer submitting v2 until after subsystem maintainers review v1, just
> in case they find more issues. If that happens, I will post v2 to avoid
> confusion. If not, then v1 can be applied with the one-liner above
> taken into account (or I can post v2 anyway if that would be preferred
> by Darren and Andy).
>
> In other words, I am happy to follow whatever route you and the
> subsystem maintainers suggest and I just want to avoid spamming the
> mailing list.
I would be interested to hear what Darren and Andy's preference would be,
and have no issue with following that. My personal thought is that it's
best to have a patch series v2 submitted with the keymap fix since I think
that reduces the potential confusion now and in the future. It makes it
clear what exactly is being signed off on in any Reviewed-By tags. This
approach also removes the need for Darren or Andy to special case the
eventual merge of the series (having to remember to do the replacement),
which in turn reduces the chances of little errors creeping in. However, if
Darren and Andy are happy with an alternative then we can go with that.
As to whether v2 is held until Darren or Andy do their own review, I guess
it makes sense in case they have other suggestions which are similarly
trivial to implement.
Regards
jonathan
next prev parent reply other threads:[~2017-03-28 23:01 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-20 9:32 Michał Kępień
2017-03-20 9:32 ` [PATCH 1/8] platform/x86: fujitsu-laptop: move backlight input device setup to a separate function Michał Kępień
2017-03-29 19:54 ` Darren Hart
2017-03-30 4:04 ` Jonathan Woithe
2017-03-20 9:32 ` [PATCH 2/8] platform/x86: fujitsu-laptop: switch to a managed backlight input device Michał Kępień
2017-03-29 19:58 ` Darren Hart
2017-03-20 9:32 ` [PATCH 3/8] platform/x86: fujitsu-laptop: use a sparse keymap for brightness key event generation Michał Kępień
2017-03-20 9:32 ` [PATCH 4/8] platform/x86: fujitsu-laptop: move hotkey input device setup to a separate function Michał Kępień
2017-03-29 20:12 ` Darren Hart
2017-03-20 9:32 ` [PATCH 5/8] platform/x86: fujitsu-laptop: switch to a managed hotkey input device Michał Kępień
2017-03-20 9:32 ` [PATCH 6/8] platform/x86: fujitsu-laptop: use a sparse keymap for hotkey event generation Michał Kępień
2017-03-31 11:22 ` Michał Kępień
2017-04-01 20:00 ` Darren Hart
2017-04-02 9:01 ` Jonathan Woithe
2017-03-20 9:32 ` [PATCH 7/8] platform/x86: fujitsu-laptop: model-dependent sparse keymap overrides Michał Kępień
2017-03-27 0:40 ` Jonathan Woithe
2017-03-20 9:32 ` [PATCH 8/8] platform/x86: fujitsu-laptop: remove keycode fields from struct fujitsu_bl Michał Kępień
2017-03-24 10:49 ` [PATCH 0/8] fujitsu-laptop: use sparse keymaps for input event handling Jonathan Woithe
2017-03-27 23:57 ` Jonathan Woithe
2017-03-28 6:16 ` Michał Kępień
2017-03-28 23:00 ` Jonathan Woithe [this message]
2017-03-29 7:19 ` Michał Kępień
2017-03-29 16:35 ` Andy Shevchenko
2017-03-30 3:36 ` Darren Hart
2017-03-30 3:56 ` Jonathan Woithe
2017-03-30 5:04 ` Darren Hart
2017-03-30 6:41 ` Michał Kępień
2017-03-30 22:25 ` Jonathan Woithe
2017-03-29 19:28 ` Darren Hart
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=20170328230029.GA29146@marvin.atrad.com.au \
--to=jwoithe@just42.net \
--cc=andy@infradead.org \
--cc=dvhart@infradead.org \
--cc=kernel@kempniu.pl \
--cc=linux-kernel@vger.kernel.org \
--cc=platform-driver-x86@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®