linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Darren Hart <dvhart@infradead.org>
To: Lyude <lyude@redhat.com>
Cc: ibm-acpi-devel@lists.freedesktop.org,
	Daniel Martin <consume.noise@gmail.com>,
	Henrique de Moraes Holschuh <ibm-acpi@hmh.eng.br>,
	ibm-acpi-devel@lists.sourceforge.net,
	platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] thinkpad_acpi: Move tablet detection into separate function
Date: Sat, 5 Nov 2016 12:30:22 -0700	[thread overview]
Message-ID: <20161105193022.GH100528@f23x64.localdomain> (raw)
In-Reply-To: <1477958200-23883-1-git-send-email-lyude@redhat.com>

On Mon, Oct 31, 2016 at 07:56:40PM -0400, Lyude wrote:
> Suggested by Daniel Martin <consume.noise@gmail.com>
> 
> Lenovo's having a bit of fun randomly changing what hotkey events and
> acpi handles they use for reporting tablet mode, so unfortunately this
> means we're definitely going to need to probe for multiple types of
> tablet mode support. Since the hotkey_init() is already a lot larger
> then it should be, let's split up this detection into it's own function
> to make things a little easier to read.
> 
> As well, since we're going to have multiple types of tablet modes, make
> hotkey_tablet into an enum so we can also use it to indicate the type of
> tablet mode reporting the machine supports.
> 
> Changes since v1:
> - Don't use bool for in_tablet_mode (fixes complaints from kbuild test
>   robot)
> 

This series doesn't apply cleanly now (simple fuzz).

Once we hear back from Henrique on his enum preference and thoughts on the
refactoring (which looks reasonable to me), please resubmit this series and
review Documentation/SubmittingPatches for the changelog (below ---) and please
be consistent in your placement of "v2" in the subject [PATCH vX N/M] prefix.

Most all of my feedback here is minor, but there were enough little things that
added up and I'd like to see this resubmitted as a series with those addressed
that applies cleanly - largely to make sure I haven't missed context and somehow
merged the wrong bits.

Thanks Lyude,

-- 
Darren Hart
Intel Open Source Technology Center

  reply	other threads:[~2016-11-05 19:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-31 22:55 [PATCH 0/3] Add support for X1 Yoga (2016) Tablet Mode + refactors Lyude
2016-10-31 22:55 ` [PATCH 1/3] thinkpad_acpi: Move tablet detection into separate function Lyude
2016-11-05 19:17   ` Darren Hart
2016-10-31 22:55 ` [PATCH 2/3] thinkpad_acpi: Don't repeat ourselves in hotkey_init_tablet_mode() Lyude
2016-10-31 23:20   ` kbuild test robot
2016-10-31 23:49   ` kbuild test robot
2016-10-31 23:56     ` [PATCH v2 1/3] thinkpad_acpi: Move tablet detection into separate function Lyude
2016-11-05 19:30       ` Darren Hart [this message]
2016-11-06  3:19         ` Henrique de Moraes Holschuh
2016-10-31 22:55 ` [PATCH 3/3 v3] thinkpad_acpi: Add support for X1 Yoga (2016) Tablet Mode Lyude
2016-11-05 19:25   ` Darren Hart
2016-11-01 17:25 ` [PATCH 0/3] Add support for X1 Yoga (2016) Tablet Mode + refactors Lyude Paul

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=20161105193022.GH100528@f23x64.localdomain \
    --to=dvhart@infradead.org \
    --cc=consume.noise@gmail.com \
    --cc=ibm-acpi-devel@lists.freedesktop.org \
    --cc=ibm-acpi-devel@lists.sourceforge.net \
    --cc=ibm-acpi@hmh.eng.br \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --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;
as well as URLs for NNTP newsgroup(s).