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

2012-11-27 Thread Christopher Heiny

On 11/27/2012 01:29 AM, Dmitry Torokhov wrote:

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

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

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

> >>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
>

> >

> >>
> >>
> >>Signed-off-by: Christopher Heiny
> >>
> >>Cc: Dmitry Torokhov
> >>Cc: Linus Walleij
> >>Cc: Naveen Kumar Gaddipati
> >>Cc: Joeri de Gram
> >>
> >>
> >>---
> >>
> >>  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 000..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]
>

> >>+/**
> >>+ * @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.


OK, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-27 Thread Dmitry Torokhov
On Mon, Nov 26, 2012 at 02:31:27PM -0800, Christopher Heiny wrote:
> On 11/26/2012 01:40 AM, Dmitry Torokhov wrote:
> >Hi Christopher,
> >
> >On Fri, Nov 16, 2012 at 07:58:53PM -0800, Christopher Heiny wrote:
> >>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
> 
> >
> >>
> >>
> >>Signed-off-by: Christopher Heiny 
> >>
> >>Cc: Dmitry Torokhov 
> >>Cc: Linus Walleij 
> >>Cc: Naveen Kumar Gaddipati 
> >>Cc: Joeri de Gram 
> >>
> >>
> >>---
> >>
> >>  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 000..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]
> 
> >>+/**
> >>+ * @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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-27 Thread Dmitry Torokhov
On Mon, Nov 26, 2012 at 02:31:27PM -0800, Christopher Heiny wrote:
 On 11/26/2012 01:40 AM, Dmitry Torokhov wrote:
 Hi Christopher,
 
 On Fri, Nov 16, 2012 at 07:58:53PM -0800, Christopher Heiny wrote:
 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
 
 
 
 
 Signed-off-by: Christopher Heiny che...@synaptics.com
 
 Cc: Dmitry Torokhov dmitry.torok...@gmail.com
 Cc: Linus Walleij linus.wall...@stericsson.com
 Cc: Naveen Kumar Gaddipati naveen.gaddip...@stericsson.com
 Cc: Joeri de Gram j.de.g...@gmail.com
 
 
 ---
 
   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 000..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]
 
 +/**
 + * @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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-27 Thread Christopher Heiny

On 11/27/2012 01:29 AM, Dmitry Torokhov wrote:

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

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

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

 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


 

 
 
 Signed-off-by: Christopher Heinyche...@synaptics.com
 
 Cc: Dmitry Torokhovdmitry.torok...@gmail.com
 Cc: Linus Walleijlinus.wall...@stericsson.com
 Cc: Naveen Kumar Gaddipatinaveen.gaddip...@stericsson.com
 Cc: Joeri de Gramj.de.g...@gmail.com
 
 
 ---
 
   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 000..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]


 +/**
 + * @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.


OK, thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-26 Thread Christopher Heiny

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

Hi Christopher,

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

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






Signed-off-by: Christopher Heiny 

Cc: Dmitry Torokhov 
Cc: Linus Walleij 
Cc: Naveen Kumar Gaddipati 
Cc: Joeri de Gram 


---

  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 000..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]


+/**
+ * @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.


[snip]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-26 Thread Christopher Heiny

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

Hi Christopher,

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

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






Signed-off-by: Christopher Heiny che...@synaptics.com

Cc: Dmitry Torokhov dmitry.torok...@gmail.com
Cc: Linus Walleij linus.wall...@stericsson.com
Cc: Naveen Kumar Gaddipati naveen.gaddip...@stericsson.com
Cc: Joeri de Gram j.de.g...@gmail.com


---

  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 000..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]


+/**
+ * @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.


[snip]
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-17 Thread Linus Walleij
On Sat, Nov 17, 2012 at 4:58 AM, Christopher Heiny  wrote:

> 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).
>
>
> Signed-off-by: Christopher Heiny 
>

Cut blank line.

> Cc: Dmitry Torokhov 
> Cc: Linus Walleij 
> Cc: Naveen Kumar Gaddipati 
> Cc: Joeri de Gram 

All other comments I've had appear to be fixed.
Reviewed-by: Linus Walleij 

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-17 Thread Linus Walleij
On Sat, Nov 17, 2012 at 4:58 AM, Christopher Heiny che...@synaptics.com wrote:

 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).


 Signed-off-by: Christopher Heiny che...@synaptics.com


Cut blank line.

 Cc: Dmitry Torokhov dmitry.torok...@gmail.com
 Cc: Linus Walleij linus.wall...@stericsson.com
 Cc: Naveen Kumar Gaddipati naveen.gaddip...@stericsson.com
 Cc: Joeri de Gram j.de.g...@gmail.com

All other comments I've had appear to be fixed.
Reviewed-by: Linus Walleij linus.wall...@linaro.org

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-11-16 Thread Christopher Heiny
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).


Signed-off-by: Christopher Heiny 

Cc: Dmitry Torokhov 
Cc: Linus Walleij 
Cc: Naveen Kumar Gaddipati 
Cc: Joeri de Gram 


---

 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 000..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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "rmi_driver.h"
+#include "rmi_f01.h"
+
+#define FUNCTION_NUMBER 0x01
+
+/**
+ * @reset - set this bit to force a firmware reset of the sensor.
+ */
+struct f01_device_commands {
+   bool reset:1;
+   u8 reserved:7;
+};
+
+/**
+ * @ctrl0 - see documentation in rmi_f01.h.
+ * @interrupt_enable - A mask of per-function interrupts on the touch sensor.
+ * @doze_interval - controls the interval between checks for finger presence
+ * when the touch sensor is in doze mode, in units of 10ms.
+ * @wakeup_threshold - controls the capacitance threshold at which the touch
+ * sensor will decide to wake up from that low power state.
+ * @doze_holdoff - controls how long the touch sensor waits after the last
+ * finger lifts before entering the doze state, in units of 100ms.
+ */
+struct f01_device_control {
+   struct f01_device_control_0 ctrl0;
+   u8 *interrupt_enable;
+   u8 doze_interval;
+   u8 wakeup_threshold;
+   u8 doze_holdoff;
+};
+
+/**
+ * @has_ds4_queries - if true, the query registers relating to Design Studio 4
+ * features are present.
+ * @has_multi_phy - if true, multiple physical communications interfaces are
+ * supported.
+ * @has_guest - if true, a "guest" device is supported.
+ */
+struct f01_query_42 {
+   bool has_ds4_queries:1;
+   bool has_multi_phy:1;
+   bool has_guest:1;
+   u8 reserved:5;
+} __attribute__((__packed__));
+
+/**
+ * @length - the length of the remaining Query43.* register block, not
+ * including the first register.
+ * @has_package_id_query -  the package ID query data will be accessible from
+ * inside the ProductID query registers.
+ * @has_packrat_query -  the packrat query data will be accessible from inside
+ * the ProductID query registers.
+ * @has_reset_query - the reset pin related registers are valid.
+ * @has_maskrev_query - the silicon mask revision number will be reported.
+ * @has_i2c_control - the register F01_RMI_Ctrl6 will exist.
+ * @has_spi_control - the register F01_RMI_Ctrl7 will exist.
+ * @has_attn_control - the register F01_RMI_Ctrl8 will exist.
+ * @reset_enabled - the hardware reset pin functionality has been enabled
+ * for this device.
+ * @reset_polarity - If this bit reports as ‘0’, it means that the reset state
+ * is active low. A ‘1’ means that the reset state is active high.
+ * @pullup_enabled - If set, it indicates that a built-in weak pull up has
+ * been enabled on the Reset pin; clear means that no pull-up is present.
+ * @reset_pin_number - This field represents which GPIO pin number has been
+ * assigned the reset functionality.
+ */
+struct f01_ds4_queries {
+   u8 length:4;
+   u8 reserved_1:4;
+
+   bool has_package_id_query:1;
+   bool has_packrat_query:1;
+   bool has_reset_query:1;
+   bool has_maskrev_query:1;
+   u8 reserved_2:4;
+
+   bool has_i2c_control:1;
+   bool has_spi_control:1;
+   bool has_attn_control:1;
+   u8 reserved_3:5;
+
+   bool reset_enabled:1;
+   bool reset_polarity:1;
+   bool pullup_enabled:1;
+   u8 reserved_4:1;
+   u8 reset_pin_number:4;
+} __attribute__((__packed__));
+
+struct f01_data {
+   struct f01_device_control device_control;
+   struct f01_basic_queries basic_queries;
+   struct f01_device_status device_status;
+   u8 product_id[RMI_PRODUCT_ID_LENGTH+1];
+
+   

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

2012-11-16 Thread Christopher Heiny
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).


Signed-off-by: Christopher Heiny che...@synaptics.com

Cc: Dmitry Torokhov dmitry.torok...@gmail.com
Cc: Linus Walleij linus.wall...@stericsson.com
Cc: Naveen Kumar Gaddipati naveen.gaddip...@stericsson.com
Cc: Joeri de Gram j.de.g...@gmail.com


---

 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 000..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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include linux/kernel.h
+#include linux/debugfs.h
+#include linux/kconfig.h
+#include linux/rmi.h
+#include linux/slab.h
+#include linux/uaccess.h
+#include rmi_driver.h
+#include rmi_f01.h
+
+#define FUNCTION_NUMBER 0x01
+
+/**
+ * @reset - set this bit to force a firmware reset of the sensor.
+ */
+struct f01_device_commands {
+   bool reset:1;
+   u8 reserved:7;
+};
+
+/**
+ * @ctrl0 - see documentation in rmi_f01.h.
+ * @interrupt_enable - A mask of per-function interrupts on the touch sensor.
+ * @doze_interval - controls the interval between checks for finger presence
+ * when the touch sensor is in doze mode, in units of 10ms.
+ * @wakeup_threshold - controls the capacitance threshold at which the touch
+ * sensor will decide to wake up from that low power state.
+ * @doze_holdoff - controls how long the touch sensor waits after the last
+ * finger lifts before entering the doze state, in units of 100ms.
+ */
+struct f01_device_control {
+   struct f01_device_control_0 ctrl0;
+   u8 *interrupt_enable;
+   u8 doze_interval;
+   u8 wakeup_threshold;
+   u8 doze_holdoff;
+};
+
+/**
+ * @has_ds4_queries - if true, the query registers relating to Design Studio 4
+ * features are present.
+ * @has_multi_phy - if true, multiple physical communications interfaces are
+ * supported.
+ * @has_guest - if true, a guest device is supported.
+ */
+struct f01_query_42 {
+   bool has_ds4_queries:1;
+   bool has_multi_phy:1;
+   bool has_guest:1;
+   u8 reserved:5;
+} __attribute__((__packed__));
+
+/**
+ * @length - the length of the remaining Query43.* register block, not
+ * including the first register.
+ * @has_package_id_query -  the package ID query data will be accessible from
+ * inside the ProductID query registers.
+ * @has_packrat_query -  the packrat query data will be accessible from inside
+ * the ProductID query registers.
+ * @has_reset_query - the reset pin related registers are valid.
+ * @has_maskrev_query - the silicon mask revision number will be reported.
+ * @has_i2c_control - the register F01_RMI_Ctrl6 will exist.
+ * @has_spi_control - the register F01_RMI_Ctrl7 will exist.
+ * @has_attn_control - the register F01_RMI_Ctrl8 will exist.
+ * @reset_enabled - the hardware reset pin functionality has been enabled
+ * for this device.
+ * @reset_polarity - If this bit reports as ‘0’, it means that the reset state
+ * is active low. A ‘1’ means that the reset state is active high.
+ * @pullup_enabled - If set, it indicates that a built-in weak pull up has
+ * been enabled on the Reset pin; clear means that no pull-up is present.
+ * @reset_pin_number - This field represents which GPIO pin number has been
+ * assigned the reset functionality.
+ */
+struct f01_ds4_queries {
+   u8 length:4;
+   u8 reserved_1:4;
+
+   bool has_package_id_query:1;
+   bool has_packrat_query:1;
+   bool has_reset_query:1;
+   bool has_maskrev_query:1;
+   u8 reserved_2:4;
+
+   bool has_i2c_control:1;
+   bool has_spi_control:1;
+   bool has_attn_control:1;
+   u8 reserved_3:5;
+
+   bool reset_enabled:1;
+   bool reset_polarity:1;
+   bool pullup_enabled:1;
+   u8 reserved_4:1;
+   u8 reset_pin_number:4;
+} __attribute__((__packed__));
+
+struct f01_data {
+  

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

2012-10-10 Thread Christopher Heiny
Linus Walleij wrote:
> On Sat, Oct 6, 2012 at 6:10 AM, Christopher Heiny  
> wrote:
> > RMI Function 01 implements basic device control and power management
> > behaviors for the RMI4 sensor.  Since the last patch, we've decoupled
> > rmi_f01.c implementation from rmi_driver.c, so rmi_f01.c acts as a
> > standard driver module to handle F01 devices on the RMI bus.
> > 
> > Like other modules, a number of attributes have been moved from sysfs to
> > debugfs, depending on their expected use.
> > 
> > 
> > rmi_f01.h exports definitions that we expect to be used by other
> > functionality in the future (such as firmware reflash).
> > 
> > 
> > Signed-off-by: Christopher Heiny 
> > 
> > Cc: Dmitry Torokhov 
> > Cc: Linus Walleij 
> > Cc: Naveen Kumar Gaddipati 
> > Cc: Joeri de Gram 
> > 
> > ---
> 
> There is liberal whitespacing above. (No big deal, but anyway.)
> 
> (...)
> 
> > +/**
> > + * @reset - set this bit to force a firmware reset of the sensor.
> > + */
> > +union f01_device_commands {
> > +   struct {
> > +   bool reset:1;
> > +   u8 reserved:7;
> > +   } __attribute__((__packed__));
> > +   u8 reg;
> > +};
> 
> I'm still scared by these unions. I see what you're doing but my
> preferred style of driver writing is to use a simple u8 if you just treat
> it the right way with some |= and &= ...
> 
> #include 
> 
> #define F01_RESET BIT(0)
> 
> u8 my_command = F01_RESET;
> 
> send(_command);
> 
> I will not insist on this because it's a bit about programming style.
> For memory-mapped devices we usually do it my way, but this
> is more like some protocol and I know protocols like to do things
> with structs and stuff so no big deal.

That's a good summary of what we're trying to do.  Our original version did 
more of the traditional mask+shift approach to manipulating the fields in the 
various registers, but in the case of complicated functions such as F11 this 
rapidly became unreadable.  We found the unions worked a lot better - the code 
was more readable and less error prone.  For consistency we decided to apply 
them throughout the code.

> 
> > +#ifdef CONFIG_RMI4_DEBUG
> > +struct f01_debugfs_data {
> > +   bool done;
> > +   struct rmi_function_container *fc;
> > +};
> > +
> > +static int f01_debug_open(struct inode *inodep, struct file *filp)
> > +{
> > +   struct f01_debugfs_data *data;
> > +   struct rmi_function_container *fc = inodep->i_private;
> > +
> > +   data = devm_kzalloc(>dev, sizeof(struct f01_debugfs_data),
> > +   GFP_KERNEL);
> 
> Wait, you probably did this because I requested it, but I was maybe
> wrong?
> 
> Will this not re-allocate a chunk every time you look at a debugfs
> file? So it leaks memory?
> 
> In that case common kzalloc() and kfree() is the way to go, as it
> is for dynamic buffers. Sorry for screwing things up for you.

No problem - we'll fix it.  Or unfix it.  Or something like that. :-)


> 
> > +   for (i = 0; i < f01->irq_count && *local_buf != 0;
> > +i++, local_buf += 2) {
> > +   int irq_shift;
> > +   int interrupt_enable;
> > +   int result;
> > +
> > +   irq_reg = i / 8;
> > +   irq_shift = i % 8;
> 
> Please stop doing these arithmetics-turned-maths things.
> 
> irq_reg = i >> 8;
> irq_shift = i & 0xFF;

See note on this in a previous email.

> 
> (...)
> 
> > +static ssize_t rmi_fn_01_interrupt_enable_show(struct device *dev,
> > +   struct device_attribute *attr, char *buf)
> > +{
> > +   struct rmi_function_container *fc;
> > +   struct f01_data *data;
> > +   int i, len, total_len = 0;
> > +   char *current_buf = buf;
> > +
> > +   fc = to_rmi_function_container(dev);
> > +   data = fc->data;
> > +   /* loop through each irq value and copy its
> > +* string representation into buf */
> > +   for (i = 0; i < data->irq_count; i++) {
> > +   int irq_reg;
> > +   int irq_shift;
> > +   int interrupt_enable;
> > +
> > +   irq_reg = i / 8;
> > +   irq_shift = i % 8;
> 
> Dito.
> 
> (...)
> 
> > +static int f01_probe(struct device *dev);
> 
> Do you really need to forward-declare this?

It's a leftover from the process of eliminating roll-your-own bus 
implementation, and move the other code around as well.  (same applies for 
similar code in rmi_f11.c).

> 
> (...)
> 
> > +static struct rmi_function_handler function_handler = {
> > +   .driver = {
> > +   .owner = THIS_MODULE,
> > +   .name = "rmi_f01",
> > +   .bus = _bus_type,
> > +   .probe = f01_probe,
> > +   .remove = f01_remove_device,
> > +   },
> > +   .func = 0x01,
> > +   .config = rmi_f01_config,
> > +   .attention = rmi_f01_attention,
> > +
> > +#ifdef CONFIG_PM
> > +   .suspend = rmi_f01_suspend,
> > +   .resume = rmi_f01_resume,
> 

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

2012-10-10 Thread Christopher Heiny
Linus Walleij wrote:
 On Sat, Oct 6, 2012 at 6:10 AM, Christopher Heiny che...@synaptics.com 
 wrote:
  RMI Function 01 implements basic device control and power management
  behaviors for the RMI4 sensor.  Since the last patch, we've decoupled
  rmi_f01.c implementation from rmi_driver.c, so rmi_f01.c acts as a
  standard driver module to handle F01 devices on the RMI bus.
  
  Like other modules, a number of attributes have been moved from sysfs to
  debugfs, depending on their expected use.
  
  
  rmi_f01.h exports definitions that we expect to be used by other
  functionality in the future (such as firmware reflash).
  
  
  Signed-off-by: Christopher Heiny che...@synaptics.com
  
  Cc: Dmitry Torokhov dmitry.torok...@gmail.com
  Cc: Linus Walleij linus.wall...@stericsson.com
  Cc: Naveen Kumar Gaddipati naveen.gaddip...@stericsson.com
  Cc: Joeri de Gram j.de.g...@gmail.com
  
  ---
 
 There is liberal whitespacing above. (No big deal, but anyway.)
 
 (...)
 
  +/**
  + * @reset - set this bit to force a firmware reset of the sensor.
  + */
  +union f01_device_commands {
  +   struct {
  +   bool reset:1;
  +   u8 reserved:7;
  +   } __attribute__((__packed__));
  +   u8 reg;
  +};
 
 I'm still scared by these unions. I see what you're doing but my
 preferred style of driver writing is to use a simple u8 if you just treat
 it the right way with some |= and = ...
 
 #include linux/bitops.h
 
 #define F01_RESET BIT(0)
 
 u8 my_command = F01_RESET;
 
 send(my_command);
 
 I will not insist on this because it's a bit about programming style.
 For memory-mapped devices we usually do it my way, but this
 is more like some protocol and I know protocols like to do things
 with structs and stuff so no big deal.

That's a good summary of what we're trying to do.  Our original version did 
more of the traditional mask+shift approach to manipulating the fields in the 
various registers, but in the case of complicated functions such as F11 this 
rapidly became unreadable.  We found the unions worked a lot better - the code 
was more readable and less error prone.  For consistency we decided to apply 
them throughout the code.

 
  +#ifdef CONFIG_RMI4_DEBUG
  +struct f01_debugfs_data {
  +   bool done;
  +   struct rmi_function_container *fc;
  +};
  +
  +static int f01_debug_open(struct inode *inodep, struct file *filp)
  +{
  +   struct f01_debugfs_data *data;
  +   struct rmi_function_container *fc = inodep-i_private;
  +
  +   data = devm_kzalloc(fc-dev, sizeof(struct f01_debugfs_data),
  +   GFP_KERNEL);
 
 Wait, you probably did this because I requested it, but I was maybe
 wrong?
 
 Will this not re-allocate a chunk every time you look at a debugfs
 file? So it leaks memory?
 
 In that case common kzalloc() and kfree() is the way to go, as it
 is for dynamic buffers. Sorry for screwing things up for you.

No problem - we'll fix it.  Or unfix it.  Or something like that. :-)


 
  +   for (i = 0; i  f01-irq_count  *local_buf != 0;
  +i++, local_buf += 2) {
  +   int irq_shift;
  +   int interrupt_enable;
  +   int result;
  +
  +   irq_reg = i / 8;
  +   irq_shift = i % 8;
 
 Please stop doing these arithmetics-turned-maths things.
 
 irq_reg = i  8;
 irq_shift = i  0xFF;

See note on this in a previous email.

 
 (...)
 
  +static ssize_t rmi_fn_01_interrupt_enable_show(struct device *dev,
  +   struct device_attribute *attr, char *buf)
  +{
  +   struct rmi_function_container *fc;
  +   struct f01_data *data;
  +   int i, len, total_len = 0;
  +   char *current_buf = buf;
  +
  +   fc = to_rmi_function_container(dev);
  +   data = fc-data;
  +   /* loop through each irq value and copy its
  +* string representation into buf */
  +   for (i = 0; i  data-irq_count; i++) {
  +   int irq_reg;
  +   int irq_shift;
  +   int interrupt_enable;
  +
  +   irq_reg = i / 8;
  +   irq_shift = i % 8;
 
 Dito.
 
 (...)
 
  +static int f01_probe(struct device *dev);
 
 Do you really need to forward-declare this?

It's a leftover from the process of eliminating roll-your-own bus 
implementation, and move the other code around as well.  (same applies for 
similar code in rmi_f11.c).

 
 (...)
 
  +static struct rmi_function_handler function_handler = {
  +   .driver = {
  +   .owner = THIS_MODULE,
  +   .name = rmi_f01,
  +   .bus = rmi_bus_type,
  +   .probe = f01_probe,
  +   .remove = f01_remove_device,
  +   },
  +   .func = 0x01,
  +   .config = rmi_f01_config,
  +   .attention = rmi_f01_attention,
  +
  +#ifdef CONFIG_PM
  +   .suspend = rmi_f01_suspend,
  +   .resume = rmi_f01_resume,
  +#endif  /* CONFIG_PM */
  +};
 
 Just move this block of struct below the 

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

2012-10-09 Thread Linus Walleij
On Sat, Oct 6, 2012 at 6:10 AM, Christopher Heiny  wrote:

> RMI Function 01 implements basic device control and power management
> behaviors for the RMI4 sensor.  Since the last patch, we've decoupled 
> rmi_f01.c
> implementation from rmi_driver.c, so rmi_f01.c acts as a standard driver
> module to handle F01 devices on the RMI bus.
>
> Like other modules, a number of attributes have been moved from sysfs to
> debugfs, depending on their expected use.
>
>
> rmi_f01.h exports definitions that we expect to be used by other functionality
> in the future (such as firmware reflash).
>
>
> Signed-off-by: Christopher Heiny 
>
> Cc: Dmitry Torokhov 
> Cc: Linus Walleij 
> Cc: Naveen Kumar Gaddipati 
> Cc: Joeri de Gram 
>
> ---

There is liberal whitespacing above. (No big deal, but anyway.)

(...)
> +/**
> + * @reset - set this bit to force a firmware reset of the sensor.
> + */
> +union f01_device_commands {
> +   struct {
> +   bool reset:1;
> +   u8 reserved:7;
> +   } __attribute__((__packed__));
> +   u8 reg;
> +};

I'm still scared by these unions. I see what you're doing but my
preferred style of driver writing is to use a simple u8 if you just treat
it the right way with some |= and &= ...

#include 

#define F01_RESET BIT(0)

u8 my_command = F01_RESET;

send(_command);

I will not insist on this because it's a bit about programming style.
For memory-mapped devices we usually do it my way, but this
is more like some protocol and I know protocols like to do things
with structs and stuff so no big deal.

> +#ifdef CONFIG_RMI4_DEBUG
> +struct f01_debugfs_data {
> +   bool done;
> +   struct rmi_function_container *fc;
> +};
> +
> +static int f01_debug_open(struct inode *inodep, struct file *filp)
> +{
> +   struct f01_debugfs_data *data;
> +   struct rmi_function_container *fc = inodep->i_private;
> +
> +   data = devm_kzalloc(>dev, sizeof(struct f01_debugfs_data),
> +   GFP_KERNEL);

Wait, you probably did this because I requested it, but I was maybe
wrong?

Will this not re-allocate a chunk every time you look at a debugfs
file? So it leaks memory?

In that case common kzalloc() and kfree() is the way to go, as it
is for dynamic buffers. Sorry for screwing things up for you. :-(

> +   for (i = 0; i < f01->irq_count && *local_buf != 0;
> +i++, local_buf += 2) {
> +   int irq_shift;
> +   int interrupt_enable;
> +   int result;
> +
> +   irq_reg = i / 8;
> +   irq_shift = i % 8;

Please stop doing these arithmetics-turned-maths things.

irq_reg = i >> 8;
irq_shift = i & 0xFF;

(...)
> +static ssize_t rmi_fn_01_interrupt_enable_show(struct device *dev,
> +   struct device_attribute *attr, char *buf)
> +{
> +   struct rmi_function_container *fc;
> +   struct f01_data *data;
> +   int i, len, total_len = 0;
> +   char *current_buf = buf;
> +
> +   fc = to_rmi_function_container(dev);
> +   data = fc->data;
> +   /* loop through each irq value and copy its
> +* string representation into buf */
> +   for (i = 0; i < data->irq_count; i++) {
> +   int irq_reg;
> +   int irq_shift;
> +   int interrupt_enable;
> +
> +   irq_reg = i / 8;
> +   irq_shift = i % 8;

Dito.

(...)
> +static int f01_probe(struct device *dev);

Do you really need to forward-declare this?

(...)
> +static struct rmi_function_handler function_handler = {
> +   .driver = {
> +   .owner = THIS_MODULE,
> +   .name = "rmi_f01",
> +   .bus = _bus_type,
> +   .probe = f01_probe,
> +   .remove = f01_remove_device,
> +   },
> +   .func = 0x01,
> +   .config = rmi_f01_config,
> +   .attention = rmi_f01_attention,
> +
> +#ifdef CONFIG_PM
> +   .suspend = rmi_f01_suspend,
> +   .resume = rmi_f01_resume,
> +#endif  /* CONFIG_PM */
> +};

Just move this block of struct *below* the probe function...

> +static __devinit int f01_probe(struct device *dev)

Header file looks OK (if these unions is the way to go...)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-10-09 Thread Linus Walleij
On Sat, Oct 6, 2012 at 6:10 AM, Christopher Heiny che...@synaptics.com wrote:

 RMI Function 01 implements basic device control and power management
 behaviors for the RMI4 sensor.  Since the last patch, we've decoupled 
 rmi_f01.c
 implementation from rmi_driver.c, so rmi_f01.c acts as a standard driver
 module to handle F01 devices on the RMI bus.

 Like other modules, a number of attributes have been moved from sysfs to
 debugfs, depending on their expected use.


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


 Signed-off-by: Christopher Heiny che...@synaptics.com

 Cc: Dmitry Torokhov dmitry.torok...@gmail.com
 Cc: Linus Walleij linus.wall...@stericsson.com
 Cc: Naveen Kumar Gaddipati naveen.gaddip...@stericsson.com
 Cc: Joeri de Gram j.de.g...@gmail.com

 ---

There is liberal whitespacing above. (No big deal, but anyway.)

(...)
 +/**
 + * @reset - set this bit to force a firmware reset of the sensor.
 + */
 +union f01_device_commands {
 +   struct {
 +   bool reset:1;
 +   u8 reserved:7;
 +   } __attribute__((__packed__));
 +   u8 reg;
 +};

I'm still scared by these unions. I see what you're doing but my
preferred style of driver writing is to use a simple u8 if you just treat
it the right way with some |= and = ...

#include linux/bitops.h

#define F01_RESET BIT(0)

u8 my_command = F01_RESET;

send(my_command);

I will not insist on this because it's a bit about programming style.
For memory-mapped devices we usually do it my way, but this
is more like some protocol and I know protocols like to do things
with structs and stuff so no big deal.

 +#ifdef CONFIG_RMI4_DEBUG
 +struct f01_debugfs_data {
 +   bool done;
 +   struct rmi_function_container *fc;
 +};
 +
 +static int f01_debug_open(struct inode *inodep, struct file *filp)
 +{
 +   struct f01_debugfs_data *data;
 +   struct rmi_function_container *fc = inodep-i_private;
 +
 +   data = devm_kzalloc(fc-dev, sizeof(struct f01_debugfs_data),
 +   GFP_KERNEL);

Wait, you probably did this because I requested it, but I was maybe
wrong?

Will this not re-allocate a chunk every time you look at a debugfs
file? So it leaks memory?

In that case common kzalloc() and kfree() is the way to go, as it
is for dynamic buffers. Sorry for screwing things up for you. :-(

 +   for (i = 0; i  f01-irq_count  *local_buf != 0;
 +i++, local_buf += 2) {
 +   int irq_shift;
 +   int interrupt_enable;
 +   int result;
 +
 +   irq_reg = i / 8;
 +   irq_shift = i % 8;

Please stop doing these arithmetics-turned-maths things.

irq_reg = i  8;
irq_shift = i  0xFF;

(...)
 +static ssize_t rmi_fn_01_interrupt_enable_show(struct device *dev,
 +   struct device_attribute *attr, char *buf)
 +{
 +   struct rmi_function_container *fc;
 +   struct f01_data *data;
 +   int i, len, total_len = 0;
 +   char *current_buf = buf;
 +
 +   fc = to_rmi_function_container(dev);
 +   data = fc-data;
 +   /* loop through each irq value and copy its
 +* string representation into buf */
 +   for (i = 0; i  data-irq_count; i++) {
 +   int irq_reg;
 +   int irq_shift;
 +   int interrupt_enable;
 +
 +   irq_reg = i / 8;
 +   irq_shift = i % 8;

Dito.

(...)
 +static int f01_probe(struct device *dev);

Do you really need to forward-declare this?

(...)
 +static struct rmi_function_handler function_handler = {
 +   .driver = {
 +   .owner = THIS_MODULE,
 +   .name = rmi_f01,
 +   .bus = rmi_bus_type,
 +   .probe = f01_probe,
 +   .remove = f01_remove_device,
 +   },
 +   .func = 0x01,
 +   .config = rmi_f01_config,
 +   .attention = rmi_f01_attention,
 +
 +#ifdef CONFIG_PM
 +   .suspend = rmi_f01_suspend,
 +   .resume = rmi_f01_resume,
 +#endif  /* CONFIG_PM */
 +};

Just move this block of struct *below* the probe function...

 +static __devinit int f01_probe(struct device *dev)

Header file looks OK (if these unions is the way to go...)

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-10-05 Thread Christopher Heiny
RMI Function 01 implements basic device control and power management
behaviors for the RMI4 sensor.  Since the last patch, we've decoupled rmi_f01.c
implementation from rmi_driver.c, so rmi_f01.c acts as a standard driver
module to handle F01 devices on the RMI bus.

Like other modules, a number of attributes have been moved from sysfs to
debugfs, depending on their expected use.


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


Signed-off-by: Christopher Heiny 

Cc: Dmitry Torokhov 
Cc: Linus Walleij 
Cc: Naveen Kumar Gaddipati 
Cc: Joeri de Gram 

---

 drivers/input/rmi4/rmi_f01.c | 1463 ++
 drivers/input/rmi4/rmi_f01.h |  136 
 2 files changed, 1599 insertions(+), 0 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
new file mode 100644
index 000..d734f46
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -0,0 +1,1463 @@
+/*
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "rmi_driver.h"
+#include "rmi_f01.h"
+
+/**
+ * @reset - set this bit to force a firmware reset of the sensor.
+ */
+union f01_device_commands {
+   struct {
+   bool reset:1;
+   u8 reserved:7;
+   } __attribute__((__packed__));
+   u8 reg;
+};
+
+/**
+ * @ctrl0 - see documentation in rmi_f01.h.
+ * @interrupt_enable - A mask of per-function interrupts on the touch sensor.
+ * @doze_interval - controls the interval between checks for finger presence
+ * when the touch sensor is in doze mode, in units of 10ms.
+ * @wakeup_threshold - controls the capacitance threshold at which the touch
+ * sensor will decide to wake up from that low power state.
+ * @doze_holdoff - controls how long the touch sensor waits after the last
+ * finger lifts before entering the doze state, in units of 100ms.
+ */
+struct f01_device_control {
+   union f01_device_control_0 ctrl0;
+   u8 *interrupt_enable;
+   u8 doze_interval;
+   u8 wakeup_threshold;
+   u8 doze_holdoff;
+};
+
+/**
+ * @has_ds4_queries - if true, the query registers relating to Design Studio 4
+ * features are present.
+ * @has_multi_phy - if true, multiple physical communications interfaces are
+ * supported.
+ * @has_guest - if true, a "guest" device is supported.
+ */
+union f01_query_42 {
+   struct {
+   bool has_ds4_queries:1;
+   bool has_multi_phy:1;
+   bool has_guest:1;
+   u8 reserved:5;
+   } __attribute__((__packed__));
+   u8 regs[1];
+};
+
+/**
+ * @length - the length of the remaining Query43.* register block, not
+ * including the first register.
+ * @has_package_id_query -  the package ID query data will be accessible from
+ * inside the ProductID query registers.
+ * @has_packrat_query -  the packrat query data will be accessible from inside
+ * the ProductID query registers.
+ * @has_reset_query - the reset pin related registers are valid.
+ * @has_maskrev_query - the silicon mask revision number will be reported.
+ * @has_i2c_control - the register F01_RMI_Ctrl6 will exist.
+ * @has_spi_control - the register F01_RMI_Ctrl7 will exist.
+ * @has_attn_control - the register F01_RMI_Ctrl8 will exist.
+ * @reset_enabled - the hardware reset pin functionality has been enabled
+ * for this device.
+ * @reset_polarity - If this bit reports as ‘0’, it means that the reset state
+ * is active low. A ‘1’ means that the reset state is active high.
+ * @pullup_enabled - If set, it indicates that a built-in weak pull up has
+ * been enabled on the Reset pin; clear means that no pull-up is present.
+ * @reset_pin_number - This field represents which GPIO pin number has been
+ * assigned the reset functionality.
+ */
+union f01_ds4_queries {
+   struct {
+   u8 length:4;
+   u8 reserved_1:4;
+
+   bool has_package_id_query:1;
+   bool has_packrat_query:1;
+   bool has_reset_query:1;
+   bool has_maskrev_query:1;
+   u8 reserved_2:4;
+
+   bool has_i2c_control:1;
+   bool 

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

2012-10-05 Thread Christopher Heiny
RMI Function 01 implements basic device control and power management
behaviors for the RMI4 sensor.  Since the last patch, we've decoupled rmi_f01.c
implementation from rmi_driver.c, so rmi_f01.c acts as a standard driver
module to handle F01 devices on the RMI bus.

Like other modules, a number of attributes have been moved from sysfs to
debugfs, depending on their expected use.


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


Signed-off-by: Christopher Heiny che...@synaptics.com

Cc: Dmitry Torokhov dmitry.torok...@gmail.com
Cc: Linus Walleij linus.wall...@stericsson.com
Cc: Naveen Kumar Gaddipati naveen.gaddip...@stericsson.com
Cc: Joeri de Gram j.de.g...@gmail.com

---

 drivers/input/rmi4/rmi_f01.c | 1463 ++
 drivers/input/rmi4/rmi_f01.h |  136 
 2 files changed, 1599 insertions(+), 0 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
new file mode 100644
index 000..d734f46
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -0,0 +1,1463 @@
+/*
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include linux/kernel.h
+#include linux/debugfs.h
+#include linux/kconfig.h
+#include linux/rmi.h
+#include linux/slab.h
+#include linux/uaccess.h
+#include rmi_driver.h
+#include rmi_f01.h
+
+/**
+ * @reset - set this bit to force a firmware reset of the sensor.
+ */
+union f01_device_commands {
+   struct {
+   bool reset:1;
+   u8 reserved:7;
+   } __attribute__((__packed__));
+   u8 reg;
+};
+
+/**
+ * @ctrl0 - see documentation in rmi_f01.h.
+ * @interrupt_enable - A mask of per-function interrupts on the touch sensor.
+ * @doze_interval - controls the interval between checks for finger presence
+ * when the touch sensor is in doze mode, in units of 10ms.
+ * @wakeup_threshold - controls the capacitance threshold at which the touch
+ * sensor will decide to wake up from that low power state.
+ * @doze_holdoff - controls how long the touch sensor waits after the last
+ * finger lifts before entering the doze state, in units of 100ms.
+ */
+struct f01_device_control {
+   union f01_device_control_0 ctrl0;
+   u8 *interrupt_enable;
+   u8 doze_interval;
+   u8 wakeup_threshold;
+   u8 doze_holdoff;
+};
+
+/**
+ * @has_ds4_queries - if true, the query registers relating to Design Studio 4
+ * features are present.
+ * @has_multi_phy - if true, multiple physical communications interfaces are
+ * supported.
+ * @has_guest - if true, a guest device is supported.
+ */
+union f01_query_42 {
+   struct {
+   bool has_ds4_queries:1;
+   bool has_multi_phy:1;
+   bool has_guest:1;
+   u8 reserved:5;
+   } __attribute__((__packed__));
+   u8 regs[1];
+};
+
+/**
+ * @length - the length of the remaining Query43.* register block, not
+ * including the first register.
+ * @has_package_id_query -  the package ID query data will be accessible from
+ * inside the ProductID query registers.
+ * @has_packrat_query -  the packrat query data will be accessible from inside
+ * the ProductID query registers.
+ * @has_reset_query - the reset pin related registers are valid.
+ * @has_maskrev_query - the silicon mask revision number will be reported.
+ * @has_i2c_control - the register F01_RMI_Ctrl6 will exist.
+ * @has_spi_control - the register F01_RMI_Ctrl7 will exist.
+ * @has_attn_control - the register F01_RMI_Ctrl8 will exist.
+ * @reset_enabled - the hardware reset pin functionality has been enabled
+ * for this device.
+ * @reset_polarity - If this bit reports as ‘0’, it means that the reset state
+ * is active low. A ‘1’ means that the reset state is active high.
+ * @pullup_enabled - If set, it indicates that a built-in weak pull up has
+ * been enabled on the Reset pin; clear means that no pull-up is present.
+ * @reset_pin_number - This field represents which GPIO pin number has been
+ * assigned the reset functionality.
+ */
+union f01_ds4_queries {
+   struct {
+   u8 length:4;
+   u8 reserved_1:4;
+
+   bool has_package_id_query:1;
+   bool