Re: [RFC PATCH 05/06] input/rmi4: F01 - device control

On Mon, Nov 26, 2012 at 02:31:27PM -0800, Christopher Heiny wrote:

On 11/26/2012 01:40 AM, Dmitry Torokhov wrote:

quoted

Hi Christopher,

On Fri, Nov 16, 2012 at 07:58:53PM -0800, Christopher Heiny wrote:

quoted

RMI Function 01 implements basic device control and power management
behaviors for the RMI4 sensor.

rmi_f01.h exports definitions that we expect to be used by other functionality
in the future (such as firmware reflash).

Please see my comments below.

Hi Dmitry,

Thanks for the feedback and the patch.  I've got just one question,
included below, with a bunch of snipping).

				Chris

quoted

quoted

Signed-off-by: Christopher Heiny <redacted>

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Linus Walleij <redacted>
Cc: Naveen Kumar Gaddipati <redacted>
Cc: Joeri de Gram <redacted>


---

 drivers/input/rmi4/rmi_f01.c | 1348 ++++++++++++++++++++++++++++++++++++++++++
 drivers/input/rmi4/rmi_f01.h |  160 +++++
 2 files changed, 1508 insertions(+), 0 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
new file mode 100644
index 0000000..038266c
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f01.c

@@ -0,0 +1,1348 @@
+/*
+ * Copyright (c) 2011-2012 Synaptics Incorporated
+ * Copyright (c) 2011 Unixphere
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.

[snip]

quoted

quoted

+/**
+ * @reset - set this bit to force a firmware reset of the sensor.
+ */
+struct f01_device_commands {
+	bool reset:1;
+	u8 reserved:7;

When specifying bitwise fields please use u8, u16, etc only.

Um, OK.  Previously patch feedback suggested to use bool instead of
u8 for single bit fields (see here:
http://www.spinics.net/lists/linux-input/msg22198.html).  So I'm a
little confused.  It's no big deal to change it back, but I'd like
confirmation that it is really what we should do.

I believe that it is better to specify exact bitness of the base type of
the bitfield so you do not surprised by the alignment.

Thanks.

-- 
Dmitry