[PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Benjamin Tissoires]

Hi Dmitry,

As mentioned in the thread on linux-i2c[1], I am making good progress on the
RMI4 SMBus implementation [2].
I am still missing the PS/2 pass-through function but I have an intern (Chandler,
CC-ed) working on that. Also, I believe forcepads could used RMI4 but we will
have to implement the ForcePad feature first :)

Instead of waiting for both the i2c SMBus feature to be validated and the driver
to be feature equivalent (greater actually because it tracks 5 fingers instead
of 1 and a half), I am sending those first fixes to you for the branch
synaptics-rmi4.

Andrew, Christopher, I am not 100% sure regarding the Signed-off and acked-by
for the implementation of function 30 and the SMBus transport layer in my tree
(I smashed a bunch of commits so the result differs from what you saw last time).
Could you validate this?

Cheers,
Benjamin


[1] http://www.spinics.net/lists/linux-i2c/msg20241.html
[2] https://github.com/bentiss/linux branch synaptics-rmi4-smbus-v4.1-15-06-23

Benjamin Tissoires (11):
  Input: synaptics-rmi4 - embed the function modules in rmi_core
  Input: synaptics-rmi4 - add a common input device in rmi_driver
  Input: synaptics-rmi4 - explicitly request polling when needed
  Input: synaptics-rmi4 - prevent oopses when irq arrives while the
    device is not bound
  Input: synaptics-rmi4 - call rmi_driver_process_config_requests in
    enable_sensor
  Input: synaptics-rmi4 - add a reset callback
  Input: synaptics-rmi4 - f11: fix bitmap irq check
  Input: synaptics-rmi4 - f11: use the unified input node if available
  Input: synaptics-rmi4 - f11: clean up rmi_f11_finger_handler
  Input: synaptics-rmi4 - f11: allow the top software button property to
    be set
  Input: synaptics-rmi4 - f11: add support for kernel tracking

 drivers/input/rmi4/Kconfig      |   5 +-
 drivers/input/rmi4/Makefile     |   2 +-
 drivers/input/rmi4/rmi_bus.c    |  11 +-
 drivers/input/rmi4/rmi_bus.h    |   1 +
 drivers/input/rmi4/rmi_driver.c | 146 +++++++++++++++-------
 drivers/input/rmi4/rmi_driver.h |  17 +++
 drivers/input/rmi4/rmi_f01.c    |   7 ++
 drivers/input/rmi4/rmi_f11.c    | 262 ++++++++++++++++++++++++++--------------
 include/linux/rmi.h             |  22 +++-
 9 files changed, 326 insertions(+), 147 deletions(-)

-- 
2.4.3

[PATCH 01/11] Input: synaptics-rmi4 - embed the function modules in rmi_core

[Benjamin Tissoires]

the function modules can not be auto-loaded by udev. So at boot, the
functions are not there and the device is not properly populated.
Force the functions to be embedded in rmi_core so that when the touchpad
is there, the functions are there too.

There is not much use of having the functions separate anyway

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/Kconfig      |  5 +----
 drivers/input/rmi4/Makefile     |  2 +-
 drivers/input/rmi4/rmi_bus.c    | 11 ++++++++++-
 drivers/input/rmi4/rmi_driver.h |  8 ++++++++
 drivers/input/rmi4/rmi_f11.c    | 10 +++++++++-
 5 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index d0c7b6e..5e3890e 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -37,7 +37,7 @@ config RMI4_I2C
 	  This feature is not currently available as a loadable module.
 
 config RMI4_F11
-	tristate "RMI4 Function 11 (2D pointing)"
+	bool "RMI4 Function 11 (2D pointing)"
 	depends on RMI4_CORE
 	help
 	  Say Y here if you want to add support for RMI4 function 11.
@@ -46,9 +46,6 @@ config RMI4_F11
 	  touchpads.  For sensors that support relative pointing, F11 also
 	  provides mouse input.
 
-	  To compile this driver as a module, choose M here: the
-	  module will be called rmi-f11.
-
 config RMI4_F11_PEN
 	bool "RMI4 F11 Pen Support"
 	depends on RMI4_F11
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 5c6bad5..63bc595 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -2,7 +2,7 @@ obj-$(CONFIG_RMI4_CORE) += rmi_core.o
 rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
 
 # Function drivers
-obj-$(CONFIG_RMI4_F11) += rmi_f11.o
+rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
 
 # Transports
 obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 6e0454a..ef2078a 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -383,15 +383,24 @@ static int __init rmi_bus_init(void)
 		goto err_unregister_bus;
 	}
 
+	error = rmi_register_f11_handler();
+	if (error) {
+		pr_err("%s: error registering the RMI F11 handler: %d\n",
+			__func__, error);
+		goto err_unregister_f01;
+	}
+
 	error = rmi_register_physical_driver();
 	if (error) {
 		pr_err("%s: error registering the RMI physical driver: %d\n",
 			__func__, error);
-		goto err_unregister_f01;
+		goto err_unregister_f11;
 	}
 
 	return 0;
 
+err_unregister_f11:
+	rmi_unregister_f11_handler();
 err_unregister_f01:
 	rmi_unregister_f01_handler();
 err_unregister_bus:
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 34f7a7d..dda564f 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -113,4 +113,12 @@ void rmi_unregister_physical_driver(void);
 int rmi_register_f01_handler(void);
 void rmi_unregister_f01_handler(void);
 
+#ifdef CONFIG_RMI4_F11
+int rmi_register_f11_handler(void);
+void rmi_unregister_f11_handler(void);
+#else
+static inline int rmi_register_f11_handler(void) { return 0; }
+static inline void rmi_unregister_f11_handler(void) {}
+#endif
+
 #endif
diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 7af4f68..4bdf4a5 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -1537,7 +1537,15 @@ static struct rmi_function_handler rmi_f11_handler = {
 	.attention	= rmi_f11_attention,
 };
 
-module_rmi_driver(rmi_f11_handler);
+int __init rmi_register_f11_handler(void)
+{
+	return rmi_register_function_handler(&rmi_f11_handler);
+}
+
+void rmi_unregister_f11_handler(void)
+{
+	rmi_unregister_function_handler(&rmi_f11_handler);
+}
 
 MODULE_AUTHOR("Christopher Heiny <cheiny@synaptics.com");
 MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com");
-- 
2.4.3

Re: [PATCH 01/11] Input: synaptics-rmi4 - embed the function modules in rmi_core

[Paul Bolle]

On Tue, 2015-06-23 at 15:17 -0400, Benjamin Tissoires wrote:
>  drivers/input/rmi4/Kconfig      |  5 +----
>  drivers/input/rmi4/Makefile     |  2 +-
>  drivers/input/rmi4/rmi_bus.c    | 11 ++++++++++-
>  drivers/input/rmi4/rmi_driver.h |  8 ++++++++
>  drivers/input/rmi4/rmi_f11.c    | 10 +++++++++-
>  5 files changed, 29 insertions(+), 7 deletions(-)

(These files are neither in v4.1 nor in current linux-next. I have no
idea which tree this is for.)

> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
 
>  config RMI4_F11
> -	tristate "RMI4 Function 11 (2D pointing)"
> +	bool "RMI4 Function 11 (2D pointing)"
>  	depends on RMI4_CORE
>  	help
>  	  Say Y here if you want to add support for RMI4 function 
> 11.

(Upgrading to Fedora 22 gave me evolution 3.16. And evolution now
somehow manages to screw up replies to long lines in patches!)

> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1537,7 +1537,15 @@ static struct rmi_function_handler 
> rmi_f11_handler = {
>  	.attention	= rmi_f11_attention,
>  };
>  
> -module_rmi_driver(rmi_f11_handler);
> +int __init rmi_register_f11_handler(void)
> +{
> +	return rmi_register_function_handler(&rmi_f11_handler);
> +}
> +
> +void rmi_unregister_f11_handler(void)
> +{
> +	rmi_unregister_function_handler(&rmi_f11_handler);
> +}
>  
>  MODULE_AUTHOR("Christopher Heiny <cheiny@synaptics.com");
>  MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com");

You can remove these macros too. And whatever module specific code or
macros there still might be left in this file.

Thanks,


Paul Bolle

Re: [PATCH 01/11] Input: synaptics-rmi4 - embed the function modules in rmi_core

[Benjamin Tissoires]

On Jun 24 2015 or thereabouts, Paul Bolle wrote:
> On Tue, 2015-06-23 at 15:17 -0400, Benjamin Tissoires wrote:
> >  drivers/input/rmi4/Kconfig      |  5 +----
> >  drivers/input/rmi4/Makefile     |  2 +-
> >  drivers/input/rmi4/rmi_bus.c    | 11 ++++++++++-
> >  drivers/input/rmi4/rmi_driver.h |  8 ++++++++
> >  drivers/input/rmi4/rmi_f11.c    | 10 +++++++++-
> >  5 files changed, 29 insertions(+), 7 deletions(-)
> 
> (These files are neither in v4.1 nor in current linux-next. I have no
> idea which tree this is for.)

Apologies. I should have mentioned that these patches are against
Dmitri's synaptics-rmi4 branch.

https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/log/?h=synaptics-rmi4

Cheers,
Benjamin

Re: [PATCH 01/11] Input: synaptics-rmi4 - embed the function modules in rmi_core

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> the function modules can not be auto-loaded by udev. So at boot, the
> functions are not there and the device is not properly populated.
> Force the functions to be embedded in rmi_core so that when the touchpad
> is there, the functions are there too.
>
> There is not much use of having the functions separate anyway
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

I'm ok with embedding the function drivers, at least for now. 
Eventually, there may be lots of different function drivers and it might 
be desirable to load only the functions which are needed by a particular 
device. We experimented with loading the modules by calling 
request_module() while scanning the PDT. But, loading modules at runtime 
adds complexity and overhead which isn't really justified when there are 
only a couple function drivers. I am fine with just including them in 
the core until there is a critical mass of function drivers which 
justifies doing dynamic loading.

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/Kconfig      |  5 +----
>   drivers/input/rmi4/Makefile     |  2 +-
>   drivers/input/rmi4/rmi_bus.c    | 11 ++++++++++-
>   drivers/input/rmi4/rmi_driver.h |  8 ++++++++
>   drivers/input/rmi4/rmi_f11.c    | 10 +++++++++-
>   5 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> index d0c7b6e..5e3890e 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -37,7 +37,7 @@ config RMI4_I2C
>   	  This feature is not currently available as a loadable module.
>   
>   config RMI4_F11
> -	tristate "RMI4 Function 11 (2D pointing)"
> +	bool "RMI4 Function 11 (2D pointing)"
>   	depends on RMI4_CORE
>   	help
>   	  Say Y here if you want to add support for RMI4 function 11.
> @@ -46,9 +46,6 @@ config RMI4_F11
>   	  touchpads.  For sensors that support relative pointing, F11 also
>   	  provides mouse input.
>   
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called rmi-f11.
> -
>   config RMI4_F11_PEN
>   	bool "RMI4 F11 Pen Support"
>   	depends on RMI4_F11
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> index 5c6bad5..63bc595 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -2,7 +2,7 @@ obj-$(CONFIG_RMI4_CORE) += rmi_core.o
>   rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
>   
>   # Function drivers
> -obj-$(CONFIG_RMI4_F11) += rmi_f11.o
> +rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
>   
>   # Transports
>   obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index 6e0454a..ef2078a 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -383,15 +383,24 @@ static int __init rmi_bus_init(void)
>   		goto err_unregister_bus;
>   	}
>   
> +	error = rmi_register_f11_handler();
> +	if (error) {
> +		pr_err("%s: error registering the RMI F11 handler: %d\n",
> +			__func__, error);
> +		goto err_unregister_f01;
> +	}
> +
>   	error = rmi_register_physical_driver();
>   	if (error) {
>   		pr_err("%s: error registering the RMI physical driver: %d\n",
>   			__func__, error);
> -		goto err_unregister_f01;
> +		goto err_unregister_f11;
>   	}
>   
>   	return 0;
>   
> +err_unregister_f11:
> +	rmi_unregister_f11_handler();
>   err_unregister_f01:
>   	rmi_unregister_f01_handler();
>   err_unregister_bus:
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 34f7a7d..dda564f 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -113,4 +113,12 @@ void rmi_unregister_physical_driver(void);
>   int rmi_register_f01_handler(void);
>   void rmi_unregister_f01_handler(void);
>   
> +#ifdef CONFIG_RMI4_F11
> +int rmi_register_f11_handler(void);
> +void rmi_unregister_f11_handler(void);
> +#else
> +static inline int rmi_register_f11_handler(void) { return 0; }
> +static inline void rmi_unregister_f11_handler(void) {}
> +#endif
> +
>   #endif
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 7af4f68..4bdf4a5 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1537,7 +1537,15 @@ static struct rmi_function_handler rmi_f11_handler = {
>   	.attention	= rmi_f11_attention,
>   };
>   
> -module_rmi_driver(rmi_f11_handler);
> +int __init rmi_register_f11_handler(void)
> +{
> +	return rmi_register_function_handler(&rmi_f11_handler);
> +}
> +
> +void rmi_unregister_f11_handler(void)
> +{
> +	rmi_unregister_function_handler(&rmi_f11_handler);
> +}
>   
>   MODULE_AUTHOR("Christopher Heiny <cheiny@synaptics.com");
>   MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com");

Re: [PATCH 01/11] Input: synaptics-rmi4 - embed the function modules in rmi_core

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> the function modules can not be auto-loaded by udev. So at boot, the
> functions are not there and the device is not properly populated.
> Force the functions to be embedded in rmi_core so that when the touchpad
> is there, the functions are there too.
>
> There is not much use of having the functions separate anyway
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

I'm ok with embedding the function drivers, at least for now. 
Eventually, there may be lots of different function drivers and it might 
be desirable to load only the functions which are needed by a particular 
device. We experimented with loading the modules by calling 
request_module() while scanning the PDT. But, loading modules at runtime 
adds complexity and overhead which isn't really justified when there are 
only a couple function drivers. I am fine with just including them in 
the core until there is a critical mass of function drivers which 
justifies doing dynamic loading.

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/Kconfig      |  5 +----
>   drivers/input/rmi4/Makefile     |  2 +-
>   drivers/input/rmi4/rmi_bus.c    | 11 ++++++++++-
>   drivers/input/rmi4/rmi_driver.h |  8 ++++++++
>   drivers/input/rmi4/rmi_f11.c    | 10 +++++++++-
>   5 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> index d0c7b6e..5e3890e 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -37,7 +37,7 @@ config RMI4_I2C
>   	  This feature is not currently available as a loadable module.
>   
>   config RMI4_F11
> -	tristate "RMI4 Function 11 (2D pointing)"
> +	bool "RMI4 Function 11 (2D pointing)"
>   	depends on RMI4_CORE
>   	help
>   	  Say Y here if you want to add support for RMI4 function 11.
> @@ -46,9 +46,6 @@ config RMI4_F11
>   	  touchpads.  For sensors that support relative pointing, F11 also
>   	  provides mouse input.
>   
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called rmi-f11.
> -
>   config RMI4_F11_PEN
>   	bool "RMI4 F11 Pen Support"
>   	depends on RMI4_F11
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> index 5c6bad5..63bc595 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -2,7 +2,7 @@ obj-$(CONFIG_RMI4_CORE) += rmi_core.o
>   rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
>   
>   # Function drivers
> -obj-$(CONFIG_RMI4_F11) += rmi_f11.o
> +rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o
>   
>   # Transports
>   obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index 6e0454a..ef2078a 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -383,15 +383,24 @@ static int __init rmi_bus_init(void)
>   		goto err_unregister_bus;
>   	}
>   
> +	error = rmi_register_f11_handler();
> +	if (error) {
> +		pr_err("%s: error registering the RMI F11 handler: %d\n",
> +			__func__, error);
> +		goto err_unregister_f01;
> +	}
> +
>   	error = rmi_register_physical_driver();
>   	if (error) {
>   		pr_err("%s: error registering the RMI physical driver: %d\n",
>   			__func__, error);
> -		goto err_unregister_f01;
> +		goto err_unregister_f11;
>   	}
>   
>   	return 0;
>   
> +err_unregister_f11:
> +	rmi_unregister_f11_handler();
>   err_unregister_f01:
>   	rmi_unregister_f01_handler();
>   err_unregister_bus:
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 34f7a7d..dda564f 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -113,4 +113,12 @@ void rmi_unregister_physical_driver(void);
>   int rmi_register_f01_handler(void);
>   void rmi_unregister_f01_handler(void);
>   
> +#ifdef CONFIG_RMI4_F11
> +int rmi_register_f11_handler(void);
> +void rmi_unregister_f11_handler(void);
> +#else
> +static inline int rmi_register_f11_handler(void) { return 0; }
> +static inline void rmi_unregister_f11_handler(void) {}
> +#endif
> +
>   #endif
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 7af4f68..4bdf4a5 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1537,7 +1537,15 @@ static struct rmi_function_handler rmi_f11_handler = {
>   	.attention	= rmi_f11_attention,
>   };
>   
> -module_rmi_driver(rmi_f11_handler);
> +int __init rmi_register_f11_handler(void)
> +{
> +	return rmi_register_function_handler(&rmi_f11_handler);
> +}
> +
> +void rmi_unregister_f11_handler(void)
> +{
> +	rmi_unregister_function_handler(&rmi_f11_handler);
> +}
>   
>   MODULE_AUTHOR("Christopher Heiny <cheiny@synaptics.com");
>   MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com");

[PATCH 02/11] Input: synaptics-rmi4 - add a common input device in rmi_driver

[Benjamin Tissoires]

When .unified_input is set to true in the platform data, the
functions should rely on the common input node created by rmi_driver
to forward events instead of having their own input node.

This node is named "Synaptics PRODUCT_ID" to be able to
differentiate the various models.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 43 +++++++++++++++++++++++++++++++++++++++++
 drivers/input/rmi4/rmi_driver.h |  6 ++++++
 drivers/input/rmi4/rmi_f01.c    |  7 +++++++
 include/linux/rmi.h             |  2 ++
 4 files changed, 58 insertions(+)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index b9db709..95f9386 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -310,6 +310,9 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev)
 		if (entry->irq_mask)
 			process_one_interrupt(data, entry);
 
+	if (data->input)
+		input_sync(data->input);
+
 	return 0;
 }
 
@@ -330,6 +333,25 @@ static int rmi_driver_set_input_params(struct rmi_device *rmi_dev,
 	return 0;
 }
 
+static void rmi_driver_set_input_name(struct rmi_device *rmi_dev,
+				struct input_dev *input)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
+	char *device_name = rmi_f01_get_product_ID(data->f01_container);
+	char *name;
+
+	if (!device_name)
+		return;
+
+	name = devm_kasprintf(&rmi_dev->dev, GFP_KERNEL,
+			      "Synaptics %s", device_name);
+	if (!name)
+		return;
+
+	input->name = name;
+}
+
+
 static int rmi_driver_set_irq_bits(struct rmi_device *rmi_dev,
 				   unsigned long *mask)
 {
@@ -720,6 +742,8 @@ static int rmi_driver_remove(struct device *dev)
 	const struct rmi_device_platform_data *pdata =
 					rmi_get_platform_data(rmi_dev);
 
+	if (data->input)
+		input_unregister_device(data->input);
 	disable_sensor(rmi_dev);
 	rmi_free_function_list(rmi_dev);
 
@@ -832,6 +856,15 @@ static int rmi_driver_probe(struct device *dev)
 	data->current_irq_mask	= irq_memory + size * 2;
 	data->new_irq_mask	= irq_memory + size * 3;
 
+	if (pdata->unified_input) {
+		data->input = input_allocate_device();
+		if (data->input) {
+			rmi_driver_set_input_params(rmi_dev, data->input);
+			sprintf(data->input_phys, "%s/input0", dev_name(dev));
+			data->input->phys = data->input_phys;
+		}
+	}
+
 	irq_count = 0;
 	dev_dbg(dev, "Creating functions.");
 	retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
@@ -866,6 +899,15 @@ static int rmi_driver_probe(struct device *dev)
 		mutex_init(&data->suspend_mutex);
 	}
 
+	if (data->input) {
+		rmi_driver_set_input_name(rmi_dev, data->input);
+		if (input_register_device(data->input)) {
+			dev_err(dev, "%s: Failed to register input device.\n",
+				__func__);
+			goto err_destroy_functions;
+		}
+	}
+
 	if (gpio_is_valid(pdata->attn_gpio)) {
 		static const char GPIO_LABEL[] = "attn";
 		unsigned long gpio_flags = GPIOF_DIR_IN;
@@ -921,6 +963,7 @@ static int rmi_driver_probe(struct device *dev)
 	return 0;
 
 err_destroy_functions:
+	input_free_device(data->input);
 	rmi_free_function_list(rmi_dev);
 	kfree(irq_memory);
 err_free_mem:
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index dda564f..36ca34b 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -13,6 +13,7 @@
 #include <linux/ctype.h>
 #include <linux/hrtimer.h>
 #include <linux/ktime.h>
+#include <linux/input.h>
 #include "rmi_bus.h"
 
 #define RMI_DRIVER_VERSION "1.6"
@@ -29,6 +30,8 @@
 
 #define RMI_PDT_PROPS_HAS_BSR 0x02
 
+#define NAME_BUFFER_SIZE 256
+
 struct rmi_driver_data {
 	struct list_head function_list;
 
@@ -49,6 +52,8 @@ struct rmi_driver_data {
 	unsigned long *current_irq_mask;
 	unsigned long *new_irq_mask;
 	struct mutex irq_mutex;
+	struct input_dev *input;
+	char input_phys[NAME_BUFFER_SIZE];
 
 	/* Following are used when polling. */
 	struct hrtimer poll_timer;
@@ -112,6 +117,7 @@ void rmi_unregister_physical_driver(void);
 
 int rmi_register_f01_handler(void);
 void rmi_unregister_f01_handler(void);
+char *rmi_f01_get_product_ID(struct rmi_function *fn);
 
 #ifdef CONFIG_RMI4_F11
 int rmi_register_f11_handler(void);
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index ee5f4a1..2d72dc8 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -176,6 +176,13 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
 	return 0;
 }
 
+char *rmi_f01_get_product_ID(struct rmi_function *fn)
+{
+	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
+
+	return f01->properties.product_id;
+}
+
 static int rmi_f01_probe(struct rmi_function *fn)
 {
 	struct rmi_device *rmi_dev = fn->rmi_dev;
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index ca35b2f..1d22985 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -277,6 +277,8 @@ struct rmi_device_platform_data {
 	struct rmi_f30_gpioled_map *gpioled_map;
 	struct rmi_button_map *f41_button_map;
 
+	bool unified_input;
+
 #ifdef CONFIG_RMI4_FWLIB
 	char *firmware_name;
 #endif
-- 
2.4.3

Re: [PATCH 02/11] Input: synaptics-rmi4 - add a common input device in rmi_driver

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> When .unified_input is set to true in the platform data, the
> functions should rely on the common input node created by rmi_driver
> to forward events instead of having their own input node.
>
> This node is named "Synaptics PRODUCT_ID" to be able to
> differentiate the various models.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 43 +++++++++++++++++++++++++++++++++++++++++
>   drivers/input/rmi4/rmi_driver.h |  6 ++++++
>   drivers/input/rmi4/rmi_f01.c    |  7 +++++++
>   include/linux/rmi.h             |  2 ++
>   4 files changed, 58 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index b9db709..95f9386 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -310,6 +310,9 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev)
>   		if (entry->irq_mask)
>   			process_one_interrupt(data, entry);
>   
> +	if (data->input)
> +		input_sync(data->input);
> +
>   	return 0;
>   }
>   
> @@ -330,6 +333,25 @@ static int rmi_driver_set_input_params(struct rmi_device *rmi_dev,
>   	return 0;
>   }
>   
> +static void rmi_driver_set_input_name(struct rmi_device *rmi_dev,
> +				struct input_dev *input)
> +{
> +	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> +	char *device_name = rmi_f01_get_product_ID(data->f01_container);
> +	char *name;
> +
> +	if (!device_name)
> +		return;
> +
> +	name = devm_kasprintf(&rmi_dev->dev, GFP_KERNEL,
> +			      "Synaptics %s", device_name);
> +	if (!name)
> +		return;
> +
> +	input->name = name;
> +}
> +
> +
>   static int rmi_driver_set_irq_bits(struct rmi_device *rmi_dev,
>   				   unsigned long *mask)
>   {
> @@ -720,6 +742,8 @@ static int rmi_driver_remove(struct device *dev)
>   	const struct rmi_device_platform_data *pdata =
>   					rmi_get_platform_data(rmi_dev);
>   
> +	if (data->input)
> +		input_unregister_device(data->input);
>   	disable_sensor(rmi_dev);
>   	rmi_free_function_list(rmi_dev);
>   
> @@ -832,6 +856,15 @@ static int rmi_driver_probe(struct device *dev)
>   	data->current_irq_mask	= irq_memory + size * 2;
>   	data->new_irq_mask	= irq_memory + size * 3;
>   
> +	if (pdata->unified_input) {
> +		data->input = input_allocate_device();
> +		if (data->input) {
> +			rmi_driver_set_input_params(rmi_dev, data->input);
> +			sprintf(data->input_phys, "%s/input0", dev_name(dev));
> +			data->input->phys = data->input_phys;
> +		}
> +	}
> +
>   	irq_count = 0;
>   	dev_dbg(dev, "Creating functions.");
>   	retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
> @@ -866,6 +899,15 @@ static int rmi_driver_probe(struct device *dev)
>   		mutex_init(&data->suspend_mutex);
>   	}
>   
> +	if (data->input) {
> +		rmi_driver_set_input_name(rmi_dev, data->input);
> +		if (input_register_device(data->input)) {
> +			dev_err(dev, "%s: Failed to register input device.\n",
> +				__func__);
> +			goto err_destroy_functions;
> +		}
> +	}
> +
>   	if (gpio_is_valid(pdata->attn_gpio)) {
>   		static const char GPIO_LABEL[] = "attn";
>   		unsigned long gpio_flags = GPIOF_DIR_IN;
> @@ -921,6 +963,7 @@ static int rmi_driver_probe(struct device *dev)
>   	return 0;
>   
>   err_destroy_functions:
> +	input_free_device(data->input);
>   	rmi_free_function_list(rmi_dev);
>   	kfree(irq_memory);
>   err_free_mem:
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index dda564f..36ca34b 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -13,6 +13,7 @@
>   #include <linux/ctype.h>
>   #include <linux/hrtimer.h>
>   #include <linux/ktime.h>
> +#include <linux/input.h>
>   #include "rmi_bus.h"
>   
>   #define RMI_DRIVER_VERSION "1.6"
> @@ -29,6 +30,8 @@
>   
>   #define RMI_PDT_PROPS_HAS_BSR 0x02
>   
> +#define NAME_BUFFER_SIZE 256
> +
>   struct rmi_driver_data {
>   	struct list_head function_list;
>   
> @@ -49,6 +52,8 @@ struct rmi_driver_data {
>   	unsigned long *current_irq_mask;
>   	unsigned long *new_irq_mask;
>   	struct mutex irq_mutex;
> +	struct input_dev *input;
> +	char input_phys[NAME_BUFFER_SIZE];
>   
>   	/* Following are used when polling. */
>   	struct hrtimer poll_timer;
> @@ -112,6 +117,7 @@ void rmi_unregister_physical_driver(void);
>   
>   int rmi_register_f01_handler(void);
>   void rmi_unregister_f01_handler(void);
> +char *rmi_f01_get_product_ID(struct rmi_function *fn);
>   
>   #ifdef CONFIG_RMI4_F11
>   int rmi_register_f11_handler(void);
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index ee5f4a1..2d72dc8 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -176,6 +176,13 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
>   	return 0;
>   }
>   
> +char *rmi_f01_get_product_ID(struct rmi_function *fn)
> +{
> +	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> +
> +	return f01->properties.product_id;
> +}
> +
>   static int rmi_f01_probe(struct rmi_function *fn)
>   {
>   	struct rmi_device *rmi_dev = fn->rmi_dev;
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index ca35b2f..1d22985 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -277,6 +277,8 @@ struct rmi_device_platform_data {
>   	struct rmi_f30_gpioled_map *gpioled_map;
>   	struct rmi_button_map *f41_button_map;
>   
> +	bool unified_input;
> +
>   #ifdef CONFIG_RMI4_FWLIB
>   	char *firmware_name;
>   #endif

Re: [PATCH 02/11] Input: synaptics-rmi4 - add a common input device in rmi_driver

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> When .unified_input is set to true in the platform data, the
> functions should rely on the common input node created by rmi_driver
> to forward events instead of having their own input node.
>
> This node is named "Synaptics PRODUCT_ID" to be able to
> differentiate the various models.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 43 +++++++++++++++++++++++++++++++++++++++++
>   drivers/input/rmi4/rmi_driver.h |  6 ++++++
>   drivers/input/rmi4/rmi_f01.c    |  7 +++++++
>   include/linux/rmi.h             |  2 ++
>   4 files changed, 58 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index b9db709..95f9386 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -310,6 +310,9 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev)
>   		if (entry->irq_mask)
>   			process_one_interrupt(data, entry);
>   
> +	if (data->input)
> +		input_sync(data->input);
> +
>   	return 0;
>   }
>   
> @@ -330,6 +333,25 @@ static int rmi_driver_set_input_params(struct rmi_device *rmi_dev,
>   	return 0;
>   }
>   
> +static void rmi_driver_set_input_name(struct rmi_device *rmi_dev,
> +				struct input_dev *input)
> +{
> +	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> +	char *device_name = rmi_f01_get_product_ID(data->f01_container);
> +	char *name;
> +
> +	if (!device_name)
> +		return;
> +
> +	name = devm_kasprintf(&rmi_dev->dev, GFP_KERNEL,
> +			      "Synaptics %s", device_name);
> +	if (!name)
> +		return;
> +
> +	input->name = name;
> +}
> +
> +
>   static int rmi_driver_set_irq_bits(struct rmi_device *rmi_dev,
>   				   unsigned long *mask)
>   {
> @@ -720,6 +742,8 @@ static int rmi_driver_remove(struct device *dev)
>   	const struct rmi_device_platform_data *pdata =
>   					rmi_get_platform_data(rmi_dev);
>   
> +	if (data->input)
> +		input_unregister_device(data->input);
>   	disable_sensor(rmi_dev);
>   	rmi_free_function_list(rmi_dev);
>   
> @@ -832,6 +856,15 @@ static int rmi_driver_probe(struct device *dev)
>   	data->current_irq_mask	= irq_memory + size * 2;
>   	data->new_irq_mask	= irq_memory + size * 3;
>   
> +	if (pdata->unified_input) {
> +		data->input = input_allocate_device();
> +		if (data->input) {
> +			rmi_driver_set_input_params(rmi_dev, data->input);
> +			sprintf(data->input_phys, "%s/input0", dev_name(dev));
> +			data->input->phys = data->input_phys;
> +		}
> +	}
> +
>   	irq_count = 0;
>   	dev_dbg(dev, "Creating functions.");
>   	retval = rmi_scan_pdt(rmi_dev, &irq_count, rmi_create_function);
> @@ -866,6 +899,15 @@ static int rmi_driver_probe(struct device *dev)
>   		mutex_init(&data->suspend_mutex);
>   	}
>   
> +	if (data->input) {
> +		rmi_driver_set_input_name(rmi_dev, data->input);
> +		if (input_register_device(data->input)) {
> +			dev_err(dev, "%s: Failed to register input device.\n",
> +				__func__);
> +			goto err_destroy_functions;
> +		}
> +	}
> +
>   	if (gpio_is_valid(pdata->attn_gpio)) {
>   		static const char GPIO_LABEL[] = "attn";
>   		unsigned long gpio_flags = GPIOF_DIR_IN;
> @@ -921,6 +963,7 @@ static int rmi_driver_probe(struct device *dev)
>   	return 0;
>   
>   err_destroy_functions:
> +	input_free_device(data->input);
>   	rmi_free_function_list(rmi_dev);
>   	kfree(irq_memory);
>   err_free_mem:
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index dda564f..36ca34b 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -13,6 +13,7 @@
>   #include <linux/ctype.h>
>   #include <linux/hrtimer.h>
>   #include <linux/ktime.h>
> +#include <linux/input.h>
>   #include "rmi_bus.h"
>   
>   #define RMI_DRIVER_VERSION "1.6"
> @@ -29,6 +30,8 @@
>   
>   #define RMI_PDT_PROPS_HAS_BSR 0x02
>   
> +#define NAME_BUFFER_SIZE 256
> +
>   struct rmi_driver_data {
>   	struct list_head function_list;
>   
> @@ -49,6 +52,8 @@ struct rmi_driver_data {
>   	unsigned long *current_irq_mask;
>   	unsigned long *new_irq_mask;
>   	struct mutex irq_mutex;
> +	struct input_dev *input;
> +	char input_phys[NAME_BUFFER_SIZE];
>   
>   	/* Following are used when polling. */
>   	struct hrtimer poll_timer;
> @@ -112,6 +117,7 @@ void rmi_unregister_physical_driver(void);
>   
>   int rmi_register_f01_handler(void);
>   void rmi_unregister_f01_handler(void);
> +char *rmi_f01_get_product_ID(struct rmi_function *fn);
>   
>   #ifdef CONFIG_RMI4_F11
>   int rmi_register_f11_handler(void);
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index ee5f4a1..2d72dc8 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -176,6 +176,13 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
>   	return 0;
>   }
>   
> +char *rmi_f01_get_product_ID(struct rmi_function *fn)
> +{
> +	struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> +
> +	return f01->properties.product_id;
> +}
> +
>   static int rmi_f01_probe(struct rmi_function *fn)
>   {
>   	struct rmi_device *rmi_dev = fn->rmi_dev;
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index ca35b2f..1d22985 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -277,6 +277,8 @@ struct rmi_device_platform_data {
>   	struct rmi_f30_gpioled_map *gpioled_map;
>   	struct rmi_button_map *f41_button_map;
>   
> +	bool unified_input;
> +
>   #ifdef CONFIG_RMI4_FWLIB
>   	char *firmware_name;
>   #endif

[PATCH 03/11] Input: synaptics-rmi4 - explicitly request polling when needed

[Benjamin Tissoires]

Host Notify does not work with neither IRQ nor polling. Allow a RMI4 driver
to request or not polling depending on the attn_gpio.

When neither the internal IRQ or polling systems are used, the
transport driver can call rmi_process_interrupt_requests() to
trigger a process of the alert.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 18 +++++++++---------
 drivers/input/rmi4/rmi_driver.h |  3 +++
 include/linux/rmi.h             |  3 +++
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 95f9386..2fdc7e8 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -67,15 +67,13 @@ static irqreturn_t rmi_irq_thread(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static int process_interrupt_requests(struct rmi_device *rmi_dev);
-
 static void rmi_poll_work(struct work_struct *work)
 {
 	struct rmi_driver_data *data =
 			container_of(work, struct rmi_driver_data, poll_work);
 	struct rmi_device *rmi_dev = data->rmi_dev;
 
-	process_interrupt_requests(rmi_dev);
+	rmi_process_interrupt_requests(rmi_dev);
 }
 
 /*
@@ -124,7 +122,7 @@ static void disable_sensor(struct rmi_device *rmi_dev)
 	if (!data->enabled)
 		return;
 
-	if (!data->irq)
+	if (data->polling)
 		disable_polling(rmi_dev);
 
 	if (rmi_dev->xport->ops->disable_device)
@@ -163,7 +161,7 @@ static int enable_sensor(struct rmi_device *rmi_dev)
 				dev_name(&rmi_dev->dev), xport);
 		if (retval)
 			return retval;
-	} else {
+	} else if (data->polling) {
 		retval = enable_polling(rmi_dev);
 		if (retval < 0)
 			return retval;
@@ -171,7 +169,7 @@ static int enable_sensor(struct rmi_device *rmi_dev)
 
 	data->enabled = true;
 
-	return process_interrupt_requests(rmi_dev);
+	return rmi_process_interrupt_requests(rmi_dev);
 }
 
 static void rmi_free_function_list(struct rmi_device *rmi_dev)
@@ -274,7 +272,7 @@ static void process_one_interrupt(struct rmi_driver_data *data,
 	}
 }
 
-static int process_interrupt_requests(struct rmi_device *rmi_dev)
+int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 {
 	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
 	struct device *dev = &rmi_dev->dev;
@@ -315,6 +313,7 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
 
 /**
  * rmi_driver_set_input_params - set input device id and other data.
@@ -421,7 +420,7 @@ static int rmi_driver_irq_handler(struct rmi_device *rmi_dev, int irq)
 		return 0;
 	}
 
-	return process_interrupt_requests(rmi_dev);
+	return rmi_process_interrupt_requests(rmi_dev);
 }
 
 static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
@@ -949,10 +948,11 @@ static int rmi_driver_probe(struct device *dev)
 				}
 			}
 		}
-	} else {
+	} else if (pdata->attn_gpio == RMI_POLLING) {
 		data->poll_interval = ktime_set(0,
 			(pdata->poll_interval_ms ? pdata->poll_interval_ms :
 			DEFAULT_POLL_INTERVAL_MS) * 1000 * 1000);
+		data->polling = true;
 	}
 
 	if (data->f01_container->dev.driver) {
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 36ca34b..8a2d91a 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -43,6 +43,7 @@ struct rmi_driver_data {
 	u32 attn_count;
 	u32 irq_debug;	/* Should be bool, but debugfs wants u32 */
 	bool gpio_held;
+	bool polling;
 	int irq;
 	int irq_flags;
 	int num_of_irq_regs;
@@ -115,6 +116,8 @@ bool rmi_is_physical_driver(struct device_driver *);
 int rmi_register_physical_driver(void);
 void rmi_unregister_physical_driver(void);
 
+int rmi_process_interrupt_requests(struct rmi_device *rmi_dev);
+
 int rmi_register_f01_handler(void);
 void rmi_unregister_f01_handler(void);
 char *rmi_f01_get_product_ID(struct rmi_function *fn);
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 1d22985..b771f41 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -23,6 +23,9 @@
 #include <linux/wait.h>
 #include <linux/debugfs.h>
 
+#define RMI_POLLING	-1
+#define RMI_CUSTOM_IRQ	-2
+
 enum rmi_attn_polarity {
 	RMI_ATTN_ACTIVE_LOW = 0,
 	RMI_ATTN_ACTIVE_HIGH = 1
-- 
2.4.3

Re: [PATCH 03/11] Input: synaptics-rmi4 - explicitly request polling when needed

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Host Notify does not work with neither IRQ nor polling. Allow a RMI4 driver
> to request or not polling depending on the attn_gpio.
>
> When neither the internal IRQ or polling systems are used, the
> transport driver can call rmi_process_interrupt_requests() to
> trigger a process of the alert.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

This will also be needed by hid-rmi when it is eventually converted into 
a rmi transport driver.

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 18 +++++++++---------
>   drivers/input/rmi4/rmi_driver.h |  3 +++
>   include/linux/rmi.h             |  3 +++
>   3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 95f9386..2fdc7e8 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -67,15 +67,13 @@ static irqreturn_t rmi_irq_thread(int irq, void *p)
>   	return IRQ_HANDLED;
>   }
>   
> -static int process_interrupt_requests(struct rmi_device *rmi_dev);
> -
>   static void rmi_poll_work(struct work_struct *work)
>   {
>   	struct rmi_driver_data *data =
>   			container_of(work, struct rmi_driver_data, poll_work);
>   	struct rmi_device *rmi_dev = data->rmi_dev;
>   
> -	process_interrupt_requests(rmi_dev);
> +	rmi_process_interrupt_requests(rmi_dev);
>   }
>   
>   /*
> @@ -124,7 +122,7 @@ static void disable_sensor(struct rmi_device *rmi_dev)
>   	if (!data->enabled)
>   		return;
>   
> -	if (!data->irq)
> +	if (data->polling)
>   		disable_polling(rmi_dev);
>   
>   	if (rmi_dev->xport->ops->disable_device)
> @@ -163,7 +161,7 @@ static int enable_sensor(struct rmi_device *rmi_dev)
>   				dev_name(&rmi_dev->dev), xport);
>   		if (retval)
>   			return retval;
> -	} else {
> +	} else if (data->polling) {
>   		retval = enable_polling(rmi_dev);
>   		if (retval < 0)
>   			return retval;
> @@ -171,7 +169,7 @@ static int enable_sensor(struct rmi_device *rmi_dev)
>   
>   	data->enabled = true;
>   
> -	return process_interrupt_requests(rmi_dev);
> +	return rmi_process_interrupt_requests(rmi_dev);
>   }
>   
>   static void rmi_free_function_list(struct rmi_device *rmi_dev)
> @@ -274,7 +272,7 @@ static void process_one_interrupt(struct rmi_driver_data *data,
>   	}
>   }
>   
> -static int process_interrupt_requests(struct rmi_device *rmi_dev)
> +int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>   {
>   	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
>   	struct device *dev = &rmi_dev->dev;
> @@ -315,6 +313,7 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev)
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
>   
>   /**
>    * rmi_driver_set_input_params - set input device id and other data.
> @@ -421,7 +420,7 @@ static int rmi_driver_irq_handler(struct rmi_device *rmi_dev, int irq)
>   		return 0;
>   	}
>   
> -	return process_interrupt_requests(rmi_dev);
> +	return rmi_process_interrupt_requests(rmi_dev);
>   }
>   
>   static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
> @@ -949,10 +948,11 @@ static int rmi_driver_probe(struct device *dev)
>   				}
>   			}
>   		}
> -	} else {
> +	} else if (pdata->attn_gpio == RMI_POLLING) {
>   		data->poll_interval = ktime_set(0,
>   			(pdata->poll_interval_ms ? pdata->poll_interval_ms :
>   			DEFAULT_POLL_INTERVAL_MS) * 1000 * 1000);
> +		data->polling = true;
>   	}
>   
>   	if (data->f01_container->dev.driver) {
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 36ca34b..8a2d91a 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -43,6 +43,7 @@ struct rmi_driver_data {
>   	u32 attn_count;
>   	u32 irq_debug;	/* Should be bool, but debugfs wants u32 */
>   	bool gpio_held;
> +	bool polling;
>   	int irq;
>   	int irq_flags;
>   	int num_of_irq_regs;
> @@ -115,6 +116,8 @@ bool rmi_is_physical_driver(struct device_driver *);
>   int rmi_register_physical_driver(void);
>   void rmi_unregister_physical_driver(void);
>   
> +int rmi_process_interrupt_requests(struct rmi_device *rmi_dev);
> +
>   int rmi_register_f01_handler(void);
>   void rmi_unregister_f01_handler(void);
>   char *rmi_f01_get_product_ID(struct rmi_function *fn);
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index 1d22985..b771f41 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -23,6 +23,9 @@
>   #include <linux/wait.h>
>   #include <linux/debugfs.h>
>   
> +#define RMI_POLLING	-1
> +#define RMI_CUSTOM_IRQ	-2
> +
>   enum rmi_attn_polarity {
>   	RMI_ATTN_ACTIVE_LOW = 0,
>   	RMI_ATTN_ACTIVE_HIGH = 1

Re: [PATCH 03/11] Input: synaptics-rmi4 - explicitly request polling when needed

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Host Notify does not work with neither IRQ nor polling. Allow a RMI4 driver
> to request or not polling depending on the attn_gpio.
>
> When neither the internal IRQ or polling systems are used, the
> transport driver can call rmi_process_interrupt_requests() to
> trigger a process of the alert.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

This will also be needed by hid-rmi when it is eventually converted into 
a rmi transport driver.

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 18 +++++++++---------
>   drivers/input/rmi4/rmi_driver.h |  3 +++
>   include/linux/rmi.h             |  3 +++
>   3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 95f9386..2fdc7e8 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -67,15 +67,13 @@ static irqreturn_t rmi_irq_thread(int irq, void *p)
>   	return IRQ_HANDLED;
>   }
>   
> -static int process_interrupt_requests(struct rmi_device *rmi_dev);
> -
>   static void rmi_poll_work(struct work_struct *work)
>   {
>   	struct rmi_driver_data *data =
>   			container_of(work, struct rmi_driver_data, poll_work);
>   	struct rmi_device *rmi_dev = data->rmi_dev;
>   
> -	process_interrupt_requests(rmi_dev);
> +	rmi_process_interrupt_requests(rmi_dev);
>   }
>   
>   /*
> @@ -124,7 +122,7 @@ static void disable_sensor(struct rmi_device *rmi_dev)
>   	if (!data->enabled)
>   		return;
>   
> -	if (!data->irq)
> +	if (data->polling)
>   		disable_polling(rmi_dev);
>   
>   	if (rmi_dev->xport->ops->disable_device)
> @@ -163,7 +161,7 @@ static int enable_sensor(struct rmi_device *rmi_dev)
>   				dev_name(&rmi_dev->dev), xport);
>   		if (retval)
>   			return retval;
> -	} else {
> +	} else if (data->polling) {
>   		retval = enable_polling(rmi_dev);
>   		if (retval < 0)
>   			return retval;
> @@ -171,7 +169,7 @@ static int enable_sensor(struct rmi_device *rmi_dev)
>   
>   	data->enabled = true;
>   
> -	return process_interrupt_requests(rmi_dev);
> +	return rmi_process_interrupt_requests(rmi_dev);
>   }
>   
>   static void rmi_free_function_list(struct rmi_device *rmi_dev)
> @@ -274,7 +272,7 @@ static void process_one_interrupt(struct rmi_driver_data *data,
>   	}
>   }
>   
> -static int process_interrupt_requests(struct rmi_device *rmi_dev)
> +int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>   {
>   	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
>   	struct device *dev = &rmi_dev->dev;
> @@ -315,6 +313,7 @@ static int process_interrupt_requests(struct rmi_device *rmi_dev)
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
>   
>   /**
>    * rmi_driver_set_input_params - set input device id and other data.
> @@ -421,7 +420,7 @@ static int rmi_driver_irq_handler(struct rmi_device *rmi_dev, int irq)
>   		return 0;
>   	}
>   
> -	return process_interrupt_requests(rmi_dev);
> +	return rmi_process_interrupt_requests(rmi_dev);
>   }
>   
>   static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
> @@ -949,10 +948,11 @@ static int rmi_driver_probe(struct device *dev)
>   				}
>   			}
>   		}
> -	} else {
> +	} else if (pdata->attn_gpio == RMI_POLLING) {
>   		data->poll_interval = ktime_set(0,
>   			(pdata->poll_interval_ms ? pdata->poll_interval_ms :
>   			DEFAULT_POLL_INTERVAL_MS) * 1000 * 1000);
> +		data->polling = true;
>   	}
>   
>   	if (data->f01_container->dev.driver) {
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 36ca34b..8a2d91a 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -43,6 +43,7 @@ struct rmi_driver_data {
>   	u32 attn_count;
>   	u32 irq_debug;	/* Should be bool, but debugfs wants u32 */
>   	bool gpio_held;
> +	bool polling;
>   	int irq;
>   	int irq_flags;
>   	int num_of_irq_regs;
> @@ -115,6 +116,8 @@ bool rmi_is_physical_driver(struct device_driver *);
>   int rmi_register_physical_driver(void);
>   void rmi_unregister_physical_driver(void);
>   
> +int rmi_process_interrupt_requests(struct rmi_device *rmi_dev);
> +
>   int rmi_register_f01_handler(void);
>   void rmi_unregister_f01_handler(void);
>   char *rmi_f01_get_product_ID(struct rmi_function *fn);
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index 1d22985..b771f41 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -23,6 +23,9 @@
>   #include <linux/wait.h>
>   #include <linux/debugfs.h>
>   
> +#define RMI_POLLING	-1
> +#define RMI_CUSTOM_IRQ	-2
> +
>   enum rmi_attn_polarity {
>   	RMI_ATTN_ACTIVE_LOW = 0,
>   	RMI_ATTN_ACTIVE_HIGH = 1

[PATCH 04/11] Input: synaptics-rmi4 - prevent oopses when irq arrives while the device is not bound

[Benjamin Tissoires]

If the device has been registered but is not populated, we should not
process any incoming interrupt.
Make sure the pointers we are following are valid.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 2fdc7e8..fe5f2f9 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -279,6 +279,9 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 	struct rmi_function *entry;
 	int error;
 
+	if (!data || !data->f01_container || !data->irq_status)
+		return 0;
+
 	error = rmi_read_block(rmi_dev,
 				data->f01_container->fd.data_base_addr + 1,
 				data->irq_status, data->num_of_irq_regs);
-- 
2.4.3

Re: [PATCH 04/11] Input: synaptics-rmi4 - prevent oopses when irq arrives while the device is not bound

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> If the device has been registered but is not populated, we should not
> process any incoming interrupt.
> Make sure the pointers we are following are valid.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 2fdc7e8..fe5f2f9 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -279,6 +279,9 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>   	struct rmi_function *entry;
>   	int error;
>   
> +	if (!data || !data->f01_container || !data->irq_status)
> +		return 0;
> +
>   	error = rmi_read_block(rmi_dev,
>   				data->f01_container->fd.data_base_addr + 1,
>   				data->irq_status, data->num_of_irq_regs);

Re: [PATCH 04/11] Input: synaptics-rmi4 - prevent oopses when irq arrives while the device is not bound

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> If the device has been registered but is not populated, we should not
> process any incoming interrupt.
> Make sure the pointers we are following are valid.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 2fdc7e8..fe5f2f9 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -279,6 +279,9 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>   	struct rmi_function *entry;
>   	int error;
>   
> +	if (!data || !data->f01_container || !data->irq_status)
> +		return 0;
> +
>   	error = rmi_read_block(rmi_dev,
>   				data->f01_container->fd.data_base_addr + 1,
>   				data->irq_status, data->num_of_irq_regs);

[PATCH 05/11] Input: synaptics-rmi4 - call rmi_driver_process_config_requests in enable_sensor

[Benjamin Tissoires]

The SMBus devices have their IRQs disabled after a reset. We need to
configure them by calling rmi_driver_process_config_request() when
enabling the sensor.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_driver.c | 78 ++++++++++++++++++++++-------------------
 drivers/input/rmi4/rmi_f11.c    |  4 +++
 2 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index fe5f2f9..264bc62 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -136,42 +136,6 @@ static void disable_sensor(struct rmi_device *rmi_dev)
 	data->enabled = false;
 }
 
-static int enable_sensor(struct rmi_device *rmi_dev)
-{
-	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
-	struct rmi_transport_dev *xport;
-	int retval = 0;
-
-	if (data->enabled)
-		return 0;
-
-	if (rmi_dev->xport->ops->enable_device) {
-		retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
-		if (retval)
-			return retval;
-	}
-
-	xport = rmi_dev->xport;
-	if (data->irq) {
-		retval = request_threaded_irq(data->irq,
-				xport->hard_irq ? xport->hard_irq : NULL,
-				xport->irq_thread ?
-					xport->irq_thread : rmi_irq_thread,
-				data->irq_flags,
-				dev_name(&rmi_dev->dev), xport);
-		if (retval)
-			return retval;
-	} else if (data->polling) {
-		retval = enable_polling(rmi_dev);
-		if (retval < 0)
-			return retval;
-	}
-
-	data->enabled = true;
-
-	return rmi_process_interrupt_requests(rmi_dev);
-}
-
 static void rmi_free_function_list(struct rmi_device *rmi_dev)
 {
 	struct rmi_function *fn, *tmp;
@@ -318,6 +282,46 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 }
 EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
 
+static int enable_sensor(struct rmi_device *rmi_dev)
+{
+	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
+	struct rmi_transport_dev *xport;
+	int retval = 0;
+
+	if (data->enabled)
+		return 0;
+
+	if (rmi_dev->xport->ops->enable_device) {
+		retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
+		if (retval)
+			return retval;
+	}
+
+	retval = rmi_driver_process_config_requests(rmi_dev);
+	if (retval < 0)
+		return retval;
+
+	xport = rmi_dev->xport;
+	if (data->irq) {
+		retval = request_threaded_irq(data->irq,
+				xport->hard_irq ? xport->hard_irq : NULL,
+				xport->irq_thread ?
+					xport->irq_thread : rmi_irq_thread,
+				data->irq_flags,
+				dev_name(&rmi_dev->dev), xport);
+		if (retval)
+			return retval;
+	} else if (data->polling) {
+		retval = enable_polling(rmi_dev);
+		if (retval < 0)
+			return retval;
+	}
+
+	data->enabled = true;
+
+	return rmi_process_interrupt_requests(rmi_dev);
+}
+
 /**
  * rmi_driver_set_input_params - set input device id and other data.
  *
@@ -960,7 +964,7 @@ static int rmi_driver_probe(struct device *dev)
 
 	if (data->f01_container->dev.driver) {
 		/* Driver already bound, so enable ATTN now. */
-		enable_sensor(rmi_dev);
+		return enable_sensor(rmi_dev);
 	}
 
 	return 0;
diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 4bdf4a5..99d6181 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -1440,9 +1440,13 @@ static int rmi_f11_config(struct rmi_function *fn)
 
 	if (!sensor->report_abs)
 		drv->clear_irq_bits(fn->rmi_dev, f11->abs_mask);
+	else
+		drv->set_irq_bits(fn->rmi_dev, f11->abs_mask);
 
 	if (!sensor->report_rel)
 		drv->clear_irq_bits(fn->rmi_dev, f11->rel_mask);
+	else
+		drv->set_irq_bits(fn->rmi_dev, f11->rel_mask);
 
 	rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
 			   &f11->dev_controls, fn->fd.query_base_addr);
-- 
2.4.3

Re: [PATCH 05/11] Input: synaptics-rmi4 - call rmi_driver_process_config_requests in enable_sensor

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> The SMBus devices have their IRQs disabled after a reset. We need to
> configure them by calling rmi_driver_process_config_request() when
> enabling the sensor.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 78 ++++++++++++++++++++++-------------------
>   drivers/input/rmi4/rmi_f11.c    |  4 +++
>   2 files changed, 45 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index fe5f2f9..264bc62 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -136,42 +136,6 @@ static void disable_sensor(struct rmi_device *rmi_dev)
>   	data->enabled = false;
>   }
>   
> -static int enable_sensor(struct rmi_device *rmi_dev)
> -{
> -	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> -	struct rmi_transport_dev *xport;
> -	int retval = 0;
> -
> -	if (data->enabled)
> -		return 0;
> -
> -	if (rmi_dev->xport->ops->enable_device) {
> -		retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
> -		if (retval)
> -			return retval;
> -	}
> -
> -	xport = rmi_dev->xport;
> -	if (data->irq) {
> -		retval = request_threaded_irq(data->irq,
> -				xport->hard_irq ? xport->hard_irq : NULL,
> -				xport->irq_thread ?
> -					xport->irq_thread : rmi_irq_thread,
> -				data->irq_flags,
> -				dev_name(&rmi_dev->dev), xport);
> -		if (retval)
> -			return retval;
> -	} else if (data->polling) {
> -		retval = enable_polling(rmi_dev);
> -		if (retval < 0)
> -			return retval;
> -	}
> -
> -	data->enabled = true;
> -
> -	return rmi_process_interrupt_requests(rmi_dev);
> -}
> -
>   static void rmi_free_function_list(struct rmi_device *rmi_dev)
>   {
>   	struct rmi_function *fn, *tmp;
> @@ -318,6 +282,46 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>   }
>   EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
>   
> +static int enable_sensor(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> +	struct rmi_transport_dev *xport;
> +	int retval = 0;
> +
> +	if (data->enabled)
> +		return 0;
> +
> +	if (rmi_dev->xport->ops->enable_device) {
> +		retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
> +		if (retval)
> +			return retval;
> +	}
> +
> +	retval = rmi_driver_process_config_requests(rmi_dev);
> +	if (retval < 0)
> +		return retval;
> +
> +	xport = rmi_dev->xport;
> +	if (data->irq) {
> +		retval = request_threaded_irq(data->irq,
> +				xport->hard_irq ? xport->hard_irq : NULL,
> +				xport->irq_thread ?
> +					xport->irq_thread : rmi_irq_thread,
> +				data->irq_flags,
> +				dev_name(&rmi_dev->dev), xport);
> +		if (retval)
> +			return retval;
> +	} else if (data->polling) {
> +		retval = enable_polling(rmi_dev);
> +		if (retval < 0)
> +			return retval;
> +	}
> +
> +	data->enabled = true;
> +
> +	return rmi_process_interrupt_requests(rmi_dev);
> +}
> +
>   /**
>    * rmi_driver_set_input_params - set input device id and other data.
>    *
> @@ -960,7 +964,7 @@ static int rmi_driver_probe(struct device *dev)
>   
>   	if (data->f01_container->dev.driver) {
>   		/* Driver already bound, so enable ATTN now. */
> -		enable_sensor(rmi_dev);
> +		return enable_sensor(rmi_dev);
>   	}
>   
>   	return 0;
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 4bdf4a5..99d6181 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1440,9 +1440,13 @@ static int rmi_f11_config(struct rmi_function *fn)
>   
>   	if (!sensor->report_abs)
>   		drv->clear_irq_bits(fn->rmi_dev, f11->abs_mask);
> +	else
> +		drv->set_irq_bits(fn->rmi_dev, f11->abs_mask);
>   
>   	if (!sensor->report_rel)
>   		drv->clear_irq_bits(fn->rmi_dev, f11->rel_mask);
> +	else
> +		drv->set_irq_bits(fn->rmi_dev, f11->rel_mask);
>   
>   	rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
>   			   &f11->dev_controls, fn->fd.query_base_addr);

Re: [PATCH 05/11] Input: synaptics-rmi4 - call rmi_driver_process_config_requests in enable_sensor

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> The SMBus devices have their IRQs disabled after a reset. We need to
> configure them by calling rmi_driver_process_config_request() when
> enabling the sensor.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_driver.c | 78 ++++++++++++++++++++++-------------------
>   drivers/input/rmi4/rmi_f11.c    |  4 +++
>   2 files changed, 45 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index fe5f2f9..264bc62 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -136,42 +136,6 @@ static void disable_sensor(struct rmi_device *rmi_dev)
>   	data->enabled = false;
>   }
>   
> -static int enable_sensor(struct rmi_device *rmi_dev)
> -{
> -	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> -	struct rmi_transport_dev *xport;
> -	int retval = 0;
> -
> -	if (data->enabled)
> -		return 0;
> -
> -	if (rmi_dev->xport->ops->enable_device) {
> -		retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
> -		if (retval)
> -			return retval;
> -	}
> -
> -	xport = rmi_dev->xport;
> -	if (data->irq) {
> -		retval = request_threaded_irq(data->irq,
> -				xport->hard_irq ? xport->hard_irq : NULL,
> -				xport->irq_thread ?
> -					xport->irq_thread : rmi_irq_thread,
> -				data->irq_flags,
> -				dev_name(&rmi_dev->dev), xport);
> -		if (retval)
> -			return retval;
> -	} else if (data->polling) {
> -		retval = enable_polling(rmi_dev);
> -		if (retval < 0)
> -			return retval;
> -	}
> -
> -	data->enabled = true;
> -
> -	return rmi_process_interrupt_requests(rmi_dev);
> -}
> -
>   static void rmi_free_function_list(struct rmi_device *rmi_dev)
>   {
>   	struct rmi_function *fn, *tmp;
> @@ -318,6 +282,46 @@ int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>   }
>   EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
>   
> +static int enable_sensor(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> +	struct rmi_transport_dev *xport;
> +	int retval = 0;
> +
> +	if (data->enabled)
> +		return 0;
> +
> +	if (rmi_dev->xport->ops->enable_device) {
> +		retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
> +		if (retval)
> +			return retval;
> +	}
> +
> +	retval = rmi_driver_process_config_requests(rmi_dev);
> +	if (retval < 0)
> +		return retval;
> +
> +	xport = rmi_dev->xport;
> +	if (data->irq) {
> +		retval = request_threaded_irq(data->irq,
> +				xport->hard_irq ? xport->hard_irq : NULL,
> +				xport->irq_thread ?
> +					xport->irq_thread : rmi_irq_thread,
> +				data->irq_flags,
> +				dev_name(&rmi_dev->dev), xport);
> +		if (retval)
> +			return retval;
> +	} else if (data->polling) {
> +		retval = enable_polling(rmi_dev);
> +		if (retval < 0)
> +			return retval;
> +	}
> +
> +	data->enabled = true;
> +
> +	return rmi_process_interrupt_requests(rmi_dev);
> +}
> +
>   /**
>    * rmi_driver_set_input_params - set input device id and other data.
>    *
> @@ -960,7 +964,7 @@ static int rmi_driver_probe(struct device *dev)
>   
>   	if (data->f01_container->dev.driver) {
>   		/* Driver already bound, so enable ATTN now. */
> -		enable_sensor(rmi_dev);
> +		return enable_sensor(rmi_dev);
>   	}
>   
>   	return 0;
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 4bdf4a5..99d6181 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1440,9 +1440,13 @@ static int rmi_f11_config(struct rmi_function *fn)
>   
>   	if (!sensor->report_abs)
>   		drv->clear_irq_bits(fn->rmi_dev, f11->abs_mask);
> +	else
> +		drv->set_irq_bits(fn->rmi_dev, f11->abs_mask);
>   
>   	if (!sensor->report_rel)
>   		drv->clear_irq_bits(fn->rmi_dev, f11->rel_mask);
> +	else
> +		drv->set_irq_bits(fn->rmi_dev, f11->rel_mask);
>   
>   	rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
>   			   &f11->dev_controls, fn->fd.query_base_addr);

[PATCH 06/11] Input: synaptics-rmi4 - add a reset callback

[Benjamin Tissoires]

When a device is physically reset, the transport layer may need to reset
its state too and also do extra work to make the device accessible on the
bus.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_bus.h    | 1 +
 drivers/input/rmi4/rmi_driver.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index d4cfc85..41d6c3d 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -197,6 +197,7 @@ struct rmi_transport_ops {
 
 	int (*enable_device) (struct rmi_transport_dev *xport);
 	void (*disable_device) (struct rmi_transport_dev *xport);
+	int (*reset)(struct rmi_transport_dev *xport, u16 reset_addr);
 };
 
 /**
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 264bc62..81e7c55 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -611,6 +611,14 @@ static int rmi_initial_reset(struct rmi_device *rmi_dev,
 		const struct rmi_device_platform_data *pdata =
 				rmi_get_platform_data(rmi_dev);
 
+		if (rmi_dev->xport->ops->reset) {
+			if (rmi_dev->xport->ops->reset(rmi_dev->xport,
+						       cmd_addr))
+				return error;
+
+			return RMI_SCAN_DONE;
+		}
+
 		error = rmi_write_block(rmi_dev, cmd_addr, &cmd_buf, 1);
 		if (error) {
 			dev_err(&rmi_dev->dev,
-- 
2.4.3

[PATCH 07/11] Input: synaptics-rmi4 - f11: fix bitmap irq check

[Benjamin Tissoires]

num_irq_regs is the count of registers of 1 bytes that we use to check
the irqs. bitmap_and() expects the number of bits.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_f11.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 99d6181..c3b757b 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -726,12 +726,12 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
 		}
 
 		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
-				      num_irq_regs);
+				      num_irq_regs * 8);
 		if (abs_bits)
 			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
 
 		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
-				      num_irq_regs);
+				      num_irq_regs * 8);
 		if (rel_bits)
 			rmi_f11_rel_pos_report(sensor, i);
 	}
-- 
2.4.3

Re: [PATCH 07/11] Input: synaptics-rmi4 - f11: fix bitmap irq check

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> num_irq_regs is the count of registers of 1 bytes that we use to check
> the irqs. bitmap_and() expects the number of bits.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 99d6181..c3b757b 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -726,12 +726,12 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   		}
>   
>   		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
> -				      num_irq_regs);
> +				      num_irq_regs * 8);
>   		if (abs_bits)
>   			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
>   
>   		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
> -				      num_irq_regs);
> +				      num_irq_regs * 8);
>   		if (rel_bits)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}

Re: [PATCH 07/11] Input: synaptics-rmi4 - f11: fix bitmap irq check

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> num_irq_regs is the count of registers of 1 bytes that we use to check
> the irqs. bitmap_and() expects the number of bits.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 99d6181..c3b757b 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -726,12 +726,12 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   		}
>   
>   		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
> -				      num_irq_regs);
> +				      num_irq_regs * 8);
>   		if (abs_bits)
>   			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
>   
>   		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
> -				      num_irq_regs);
> +				      num_irq_regs * 8);
>   		if (rel_bits)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}

[PATCH 08/11] Input: synaptics-rmi4 - f11: use the unified input node if available

[Benjamin Tissoires]

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_f11.c | 60 ++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index c3b757b..061530a 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -34,7 +34,6 @@
 #define DEFAULT_MAX_ABS_MT_ORIENTATION 1
 #define DEFAULT_MIN_ABS_MT_TRACKING_ID 1
 #define DEFAULT_MAX_ABS_MT_TRACKING_ID 10
-#define NAME_BUFFER_SIZE 256
 #define FUNCTION_NUMBER 0x11
 
 /** A note about RMI4 F11 register structure.
@@ -518,6 +517,7 @@ struct f11_2d_sensor {
 	u32 type_a;	/* boolean but debugfs API requires u32 */
 	enum rmi_f11_sensor_type sensor_type;
 	struct input_dev *input;
+	bool unified_input;
 	struct rmi_function *fn;
 	char input_phys[NAME_BUFFER_SIZE];
 	u8 report_abs;
@@ -737,7 +737,8 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
 	}
 
 	input_mt_sync_frame(sensor->input);
-	input_sync(sensor->input);
+	if (!sensor->unified_input)
+		input_sync(sensor->input);
 }
 
 static int f11_2d_construct_data(struct f11_2d_sensor *sensor)
@@ -1373,35 +1374,42 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 static int rmi_f11_register_devices(struct rmi_function *fn)
 {
 	struct rmi_device *rmi_dev = fn->rmi_dev;
+	struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
 	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
 	struct input_dev *input_dev;
 	struct rmi_driver *driver = rmi_dev->driver;
 	struct f11_2d_sensor *sensor = &f11->sensor;
 	int rc;
 
-	input_dev = input_allocate_device();
+	if (!drv_data->input) {
+		input_dev = input_allocate_device();
+	} else {
+		input_dev = drv_data->input;
+		sensor->unified_input = true;
+	}
 	if (!input_dev) {
 		rc = -ENOMEM;
 		goto error_unregister;
 	}
 
 	sensor->input = input_dev;
-	if (driver->set_input_params) {
-		rc = driver->set_input_params(rmi_dev, input_dev);
-		if (rc < 0) {
-			dev_err(&fn->dev,
-				"%s: Error in setting input device.\n",
-				__func__);
-			goto error_unregister;
+
+	if (!sensor->unified_input) {
+		if (driver->set_input_params) {
+			rc = driver->set_input_params(rmi_dev, input_dev);
+			if (rc < 0) {
+				dev_err(&fn->dev,
+					"%s: Error in setting input device.\n",
+					__func__);
+				goto error_unregister;
+			}
 		}
+		sprintf(sensor->input_phys, "%s.abs/input0",
+			dev_name(&fn->dev));
+		input_dev->phys = sensor->input_phys;
+		input_dev->dev.parent = &rmi_dev->dev;
 	}
-	sprintf(sensor->input_phys, "%s.abs/input0",
-		dev_name(&fn->dev));
-	input_dev->phys = sensor->input_phys;
-	input_dev->dev.parent = &rmi_dev->dev;
-	input_set_drvdata(input_dev, f11);
 
-	set_bit(EV_SYN, input_dev->evbit);
 	set_bit(EV_ABS, input_dev->evbit);
 	input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
 
@@ -1413,19 +1421,21 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
 		set_bit(REL_X, input_dev->relbit);
 		set_bit(REL_Y, input_dev->relbit);
 	}
-	rc = input_register_device(input_dev);
-	if (rc) {
-		input_free_device(input_dev);
-		sensor->input = NULL;
-		goto error_unregister;
+	if (!sensor->unified_input) {
+		rc = input_register_device(input_dev);
+		if (rc) {
+			input_free_device(input_dev);
+			sensor->input = NULL;
+			goto error_unregister;
+		}
 	}
 
 	return 0;
 
 error_unregister:
-	if (f11->sensor.input) {
-		input_unregister_device(f11->sensor.input);
-		f11->sensor.input = NULL;
+	if (!sensor->unified_input && sensor->input) {
+		input_unregister_device(sensor->input);
+		sensor->input = NULL;
 	}
 
 	return rc;
@@ -1525,7 +1535,7 @@ static void rmi_f11_remove(struct rmi_function *fn)
 {
 	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
 
-	if (f11->sensor.input)
+	if (!f11->sensor.unified_input && f11->sensor.input)
 		input_unregister_device(f11->sensor.input);
 }
 
-- 
2.4.3

Re: [PATCH 08/11] Input: synaptics-rmi4 - f11: use the unified input node if available

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 60 ++++++++++++++++++++++++++------------------
>   1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index c3b757b..061530a 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -34,7 +34,6 @@
>   #define DEFAULT_MAX_ABS_MT_ORIENTATION 1
>   #define DEFAULT_MIN_ABS_MT_TRACKING_ID 1
>   #define DEFAULT_MAX_ABS_MT_TRACKING_ID 10
> -#define NAME_BUFFER_SIZE 256
>   #define FUNCTION_NUMBER 0x11
>   
>   /** A note about RMI4 F11 register structure.
> @@ -518,6 +517,7 @@ struct f11_2d_sensor {
>   	u32 type_a;	/* boolean but debugfs API requires u32 */
>   	enum rmi_f11_sensor_type sensor_type;
>   	struct input_dev *input;
> +	bool unified_input;
>   	struct rmi_function *fn;
>   	char input_phys[NAME_BUFFER_SIZE];
>   	u8 report_abs;
> @@ -737,7 +737,8 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   	}
>   
>   	input_mt_sync_frame(sensor->input);
> -	input_sync(sensor->input);
> +	if (!sensor->unified_input)
> +		input_sync(sensor->input);
>   }
>   
>   static int f11_2d_construct_data(struct f11_2d_sensor *sensor)
> @@ -1373,35 +1374,42 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   static int rmi_f11_register_devices(struct rmi_function *fn)
>   {
>   	struct rmi_device *rmi_dev = fn->rmi_dev;
> +	struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
>   	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
>   	struct input_dev *input_dev;
>   	struct rmi_driver *driver = rmi_dev->driver;
>   	struct f11_2d_sensor *sensor = &f11->sensor;
>   	int rc;
>   
> -	input_dev = input_allocate_device();
> +	if (!drv_data->input) {
> +		input_dev = input_allocate_device();
> +	} else {
> +		input_dev = drv_data->input;
> +		sensor->unified_input = true;
> +	}
>   	if (!input_dev) {
>   		rc = -ENOMEM;
>   		goto error_unregister;
>   	}
>   
>   	sensor->input = input_dev;
> -	if (driver->set_input_params) {
> -		rc = driver->set_input_params(rmi_dev, input_dev);
> -		if (rc < 0) {
> -			dev_err(&fn->dev,
> -				"%s: Error in setting input device.\n",
> -				__func__);
> -			goto error_unregister;
> +
> +	if (!sensor->unified_input) {
> +		if (driver->set_input_params) {
> +			rc = driver->set_input_params(rmi_dev, input_dev);
> +			if (rc < 0) {
> +				dev_err(&fn->dev,
> +					"%s: Error in setting input device.\n",
> +					__func__);
> +				goto error_unregister;
> +			}
>   		}
> +		sprintf(sensor->input_phys, "%s.abs/input0",
> +			dev_name(&fn->dev));
> +		input_dev->phys = sensor->input_phys;
> +		input_dev->dev.parent = &rmi_dev->dev;
>   	}
> -	sprintf(sensor->input_phys, "%s.abs/input0",
> -		dev_name(&fn->dev));
> -	input_dev->phys = sensor->input_phys;
> -	input_dev->dev.parent = &rmi_dev->dev;
> -	input_set_drvdata(input_dev, f11);
>   
> -	set_bit(EV_SYN, input_dev->evbit);
>   	set_bit(EV_ABS, input_dev->evbit);
>   	input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>   
> @@ -1413,19 +1421,21 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
>   		set_bit(REL_X, input_dev->relbit);
>   		set_bit(REL_Y, input_dev->relbit);
>   	}
> -	rc = input_register_device(input_dev);
> -	if (rc) {
> -		input_free_device(input_dev);
> -		sensor->input = NULL;
> -		goto error_unregister;
> +	if (!sensor->unified_input) {
> +		rc = input_register_device(input_dev);
> +		if (rc) {
> +			input_free_device(input_dev);
> +			sensor->input = NULL;
> +			goto error_unregister;
> +		}
>   	}
>   
>   	return 0;
>   
>   error_unregister:
> -	if (f11->sensor.input) {
> -		input_unregister_device(f11->sensor.input);
> -		f11->sensor.input = NULL;
> +	if (!sensor->unified_input && sensor->input) {
> +		input_unregister_device(sensor->input);
> +		sensor->input = NULL;
>   	}
>   
>   	return rc;
> @@ -1525,7 +1535,7 @@ static void rmi_f11_remove(struct rmi_function *fn)
>   {
>   	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
>   
> -	if (f11->sensor.input)
> +	if (!f11->sensor.unified_input && f11->sensor.input)
>   		input_unregister_device(f11->sensor.input);
>   }
>   

Re: [PATCH 08/11] Input: synaptics-rmi4 - f11: use the unified input node if available

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 60 ++++++++++++++++++++++++++------------------
>   1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index c3b757b..061530a 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -34,7 +34,6 @@
>   #define DEFAULT_MAX_ABS_MT_ORIENTATION 1
>   #define DEFAULT_MIN_ABS_MT_TRACKING_ID 1
>   #define DEFAULT_MAX_ABS_MT_TRACKING_ID 10
> -#define NAME_BUFFER_SIZE 256
>   #define FUNCTION_NUMBER 0x11
>   
>   /** A note about RMI4 F11 register structure.
> @@ -518,6 +517,7 @@ struct f11_2d_sensor {
>   	u32 type_a;	/* boolean but debugfs API requires u32 */
>   	enum rmi_f11_sensor_type sensor_type;
>   	struct input_dev *input;
> +	bool unified_input;
>   	struct rmi_function *fn;
>   	char input_phys[NAME_BUFFER_SIZE];
>   	u8 report_abs;
> @@ -737,7 +737,8 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   	}
>   
>   	input_mt_sync_frame(sensor->input);
> -	input_sync(sensor->input);
> +	if (!sensor->unified_input)
> +		input_sync(sensor->input);
>   }
>   
>   static int f11_2d_construct_data(struct f11_2d_sensor *sensor)
> @@ -1373,35 +1374,42 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   static int rmi_f11_register_devices(struct rmi_function *fn)
>   {
>   	struct rmi_device *rmi_dev = fn->rmi_dev;
> +	struct rmi_driver_data *drv_data = dev_get_drvdata(&rmi_dev->dev);
>   	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
>   	struct input_dev *input_dev;
>   	struct rmi_driver *driver = rmi_dev->driver;
>   	struct f11_2d_sensor *sensor = &f11->sensor;
>   	int rc;
>   
> -	input_dev = input_allocate_device();
> +	if (!drv_data->input) {
> +		input_dev = input_allocate_device();
> +	} else {
> +		input_dev = drv_data->input;
> +		sensor->unified_input = true;
> +	}
>   	if (!input_dev) {
>   		rc = -ENOMEM;
>   		goto error_unregister;
>   	}
>   
>   	sensor->input = input_dev;
> -	if (driver->set_input_params) {
> -		rc = driver->set_input_params(rmi_dev, input_dev);
> -		if (rc < 0) {
> -			dev_err(&fn->dev,
> -				"%s: Error in setting input device.\n",
> -				__func__);
> -			goto error_unregister;
> +
> +	if (!sensor->unified_input) {
> +		if (driver->set_input_params) {
> +			rc = driver->set_input_params(rmi_dev, input_dev);
> +			if (rc < 0) {
> +				dev_err(&fn->dev,
> +					"%s: Error in setting input device.\n",
> +					__func__);
> +				goto error_unregister;
> +			}
>   		}
> +		sprintf(sensor->input_phys, "%s.abs/input0",
> +			dev_name(&fn->dev));
> +		input_dev->phys = sensor->input_phys;
> +		input_dev->dev.parent = &rmi_dev->dev;
>   	}
> -	sprintf(sensor->input_phys, "%s.abs/input0",
> -		dev_name(&fn->dev));
> -	input_dev->phys = sensor->input_phys;
> -	input_dev->dev.parent = &rmi_dev->dev;
> -	input_set_drvdata(input_dev, f11);
>   
> -	set_bit(EV_SYN, input_dev->evbit);
>   	set_bit(EV_ABS, input_dev->evbit);
>   	input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>   
> @@ -1413,19 +1421,21 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
>   		set_bit(REL_X, input_dev->relbit);
>   		set_bit(REL_Y, input_dev->relbit);
>   	}
> -	rc = input_register_device(input_dev);
> -	if (rc) {
> -		input_free_device(input_dev);
> -		sensor->input = NULL;
> -		goto error_unregister;
> +	if (!sensor->unified_input) {
> +		rc = input_register_device(input_dev);
> +		if (rc) {
> +			input_free_device(input_dev);
> +			sensor->input = NULL;
> +			goto error_unregister;
> +		}
>   	}
>   
>   	return 0;
>   
>   error_unregister:
> -	if (f11->sensor.input) {
> -		input_unregister_device(f11->sensor.input);
> -		f11->sensor.input = NULL;
> +	if (!sensor->unified_input && sensor->input) {
> +		input_unregister_device(sensor->input);
> +		sensor->input = NULL;
>   	}
>   
>   	return rc;
> @@ -1525,7 +1535,7 @@ static void rmi_f11_remove(struct rmi_function *fn)
>   {
>   	struct f11_data *f11 = dev_get_drvdata(&fn->dev);
>   
> -	if (f11->sensor.input)
> +	if (!f11->sensor.unified_input && f11->sensor.input)
>   		input_unregister_device(f11->sensor.input);
>   }
>   

[PATCH 09/11] Input: synaptics-rmi4 - f11: clean up rmi_f11_finger_handler

[Benjamin Tissoires]

abs_bit and rel_bits should not be computed at each iteration of the for
loop.
finger_press_count is unused.
Break out rmi_f11_parse_finger_state in its own function.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_f11.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 061530a..77a18aa 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -695,43 +695,36 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
 		input_mt_sync(input);
 }
 
+static inline u8 rmi_f11_parse_finger_state(const u8 *f_state, u8 n_finger)
+{
+	return (f_state[n_finger / 4] >> (2 * (n_finger % 4))) &
+							FINGER_STATE_MASK;
+}
+
 static void rmi_f11_finger_handler(struct f11_data *f11,
 				   struct f11_2d_sensor *sensor,
 				   unsigned long *irq_bits, int num_irq_regs)
 {
 	const u8 *f_state = sensor->data.f_state;
 	u8 finger_state;
-	u8 finger_pressed_count;
 	u8 i;
 
-	int rel_bits;
-	int abs_bits;
+	int abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
+				  num_irq_regs * 8);
+	int rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
+				  num_irq_regs * 8);
 
-	for (i = 0, finger_pressed_count = 0; i < sensor->nbr_fingers; i++) {
+	for (i = 0; i < sensor->nbr_fingers; i++) {
 		/* Possible of having 4 fingers per f_statet register */
-		finger_state = (f_state[i / 4] >> (2 * (i % 4))) &
-					FINGER_STATE_MASK;
-		switch (finger_state) {
-		case F11_RESERVED:
+		finger_state = rmi_f11_parse_finger_state(f_state, i);
+		if (finger_state == F11_RESERVED) {
 			pr_err("Invalid finger state[%d]: 0x%02x", i, finger_state);
 			continue;
-
-		case F11_PRESENT:
-		case F11_INACCURATE:
-			finger_pressed_count++;
-			break;
-
-		case F11_NO_FINGER:
-			break;
 		}
 
-		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
-				      num_irq_regs * 8);
 		if (abs_bits)
 			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
 
-		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
-				      num_irq_regs * 8);
 		if (rel_bits)
 			rmi_f11_rel_pos_report(sensor, i);
 	}
-- 
2.4.3

Re: [PATCH 09/11] Input: synaptics-rmi4 - f11: clean up rmi_f11_finger_handler

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> abs_bit and rel_bits should not be computed at each iteration of the for
> loop.
> finger_press_count is unused.
> Break out rmi_f11_parse_finger_state in its own function.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 33 +++++++++++++--------------------
>   1 file changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 061530a..77a18aa 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -695,43 +695,36 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   		input_mt_sync(input);
>   }
>   
> +static inline u8 rmi_f11_parse_finger_state(const u8 *f_state, u8 n_finger)
> +{
> +	return (f_state[n_finger / 4] >> (2 * (n_finger % 4))) &
> +							FINGER_STATE_MASK;
> +}
> +
>   static void rmi_f11_finger_handler(struct f11_data *f11,
>   				   struct f11_2d_sensor *sensor,
>   				   unsigned long *irq_bits, int num_irq_regs)
>   {
>   	const u8 *f_state = sensor->data.f_state;
>   	u8 finger_state;
> -	u8 finger_pressed_count;
>   	u8 i;
>   
> -	int rel_bits;
> -	int abs_bits;
> +	int abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
> +				  num_irq_regs * 8);
> +	int rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
> +				  num_irq_regs * 8);
>   
> -	for (i = 0, finger_pressed_count = 0; i < sensor->nbr_fingers; i++) {
> +	for (i = 0; i < sensor->nbr_fingers; i++) {
>   		/* Possible of having 4 fingers per f_statet register */
> -		finger_state = (f_state[i / 4] >> (2 * (i % 4))) &
> -					FINGER_STATE_MASK;
> -		switch (finger_state) {
> -		case F11_RESERVED:
> +		finger_state = rmi_f11_parse_finger_state(f_state, i);
> +		if (finger_state == F11_RESERVED) {
>   			pr_err("Invalid finger state[%d]: 0x%02x", i, finger_state);
>   			continue;
> -
> -		case F11_PRESENT:
> -		case F11_INACCURATE:
> -			finger_pressed_count++;
> -			break;
> -
> -		case F11_NO_FINGER:
> -			break;
>   		}
>   
> -		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
> -				      num_irq_regs * 8);
>   		if (abs_bits)
>   			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
>   
> -		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
> -				      num_irq_regs * 8);
>   		if (rel_bits)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}

Re: [PATCH 09/11] Input: synaptics-rmi4 - f11: clean up rmi_f11_finger_handler

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> abs_bit and rel_bits should not be computed at each iteration of the for
> loop.
> finger_press_count is unused.
> Break out rmi_f11_parse_finger_state in its own function.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 33 +++++++++++++--------------------
>   1 file changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 061530a..77a18aa 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -695,43 +695,36 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   		input_mt_sync(input);
>   }
>   
> +static inline u8 rmi_f11_parse_finger_state(const u8 *f_state, u8 n_finger)
> +{
> +	return (f_state[n_finger / 4] >> (2 * (n_finger % 4))) &
> +							FINGER_STATE_MASK;
> +}
> +
>   static void rmi_f11_finger_handler(struct f11_data *f11,
>   				   struct f11_2d_sensor *sensor,
>   				   unsigned long *irq_bits, int num_irq_regs)
>   {
>   	const u8 *f_state = sensor->data.f_state;
>   	u8 finger_state;
> -	u8 finger_pressed_count;
>   	u8 i;
>   
> -	int rel_bits;
> -	int abs_bits;
> +	int abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
> +				  num_irq_regs * 8);
> +	int rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
> +				  num_irq_regs * 8);
>   
> -	for (i = 0, finger_pressed_count = 0; i < sensor->nbr_fingers; i++) {
> +	for (i = 0; i < sensor->nbr_fingers; i++) {
>   		/* Possible of having 4 fingers per f_statet register */
> -		finger_state = (f_state[i / 4] >> (2 * (i % 4))) &
> -					FINGER_STATE_MASK;
> -		switch (finger_state) {
> -		case F11_RESERVED:
> +		finger_state = rmi_f11_parse_finger_state(f_state, i);
> +		if (finger_state == F11_RESERVED) {
>   			pr_err("Invalid finger state[%d]: 0x%02x", i, finger_state);
>   			continue;
> -
> -		case F11_PRESENT:
> -		case F11_INACCURATE:
> -			finger_pressed_count++;
> -			break;
> -
> -		case F11_NO_FINGER:
> -			break;
>   		}
>   
> -		abs_bits = bitmap_and(f11->result_bits, irq_bits, f11->abs_mask,
> -				      num_irq_regs * 8);
>   		if (abs_bits)
>   			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
>   
> -		rel_bits = bitmap_and(f11->result_bits, irq_bits, f11->rel_mask,
> -				      num_irq_regs * 8);
>   		if (rel_bits)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}

[PATCH 10/11] Input: synaptics-rmi4 - f11: allow the top software button property to be set

[Benjamin Tissoires]

Currently, in PS/2 we only have the PNPIds list to detect the property.
Unfortunately, it looks like the information is not embeded in the RMI4
protocol either, so allow the Top software buttons property to be set
in the platform data.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_f11.c | 5 +++++
 include/linux/rmi.h          | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 77a18aa..50df7a1 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -515,6 +515,7 @@ struct f11_2d_sensor {
 	int pkt_size;
 	u8 sensor_index;
 	u32 type_a;	/* boolean but debugfs API requires u32 */
+	bool topbuttonpad;
 	enum rmi_f11_sensor_type sensor_type;
 	struct input_dev *input;
 	bool unified_input;
@@ -1285,6 +1286,7 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 		sensor->axis_align =
 			pdata->f11_sensor_data->axis_align;
 		sensor->type_a = pdata->f11_sensor_data->type_a;
+		sensor->topbuttonpad = pdata->f11_sensor_data->topbuttonpad;
 
 		if (sensor->sens_query.has_physical_props) {
 			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
@@ -1409,6 +1411,9 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
 	if (sensor->report_abs)
 		f11_set_abs_params(fn, f11);
 
+	if (sensor->topbuttonpad)
+		set_bit(INPUT_PROP_TOPBUTTONPAD, input_dev->propbit);
+
 	if (sensor->report_rel) {
 		set_bit(EV_REL, input_dev->evbit);
 		set_bit(REL_X, input_dev->relbit);
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index b771f41..4ffe9fe 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -93,6 +93,8 @@ enum rmi_f11_sensor_type {
  * available.
  * @disable_report_mask - Force data to not be reported even if it is supported
  * by the firware.
+ * @topbuttonpad - Used with the "5 buttons touchpads" found on the Lenovo 40
+ * series
  */
 struct rmi_f11_sensor_data {
 	struct rmi_f11_2d_axis_alignment axis_align;
@@ -101,6 +103,7 @@ struct rmi_f11_sensor_data {
 	int x_mm;
 	int y_mm;
 	int disable_report_mask;
+	bool topbuttonpad;
 };
 
 /**
-- 
2.4.3

Re: [PATCH 10/11] Input: synaptics-rmi4 - f11: allow the top software button property to be set

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Currently, in PS/2 we only have the PNPIds list to detect the property.
> Unfortunately, it looks like the information is not embeded in the RMI4
> protocol either, so allow the Top software buttons property to be set
> in the platform data.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 5 +++++
>   include/linux/rmi.h          | 3 +++
>   2 files changed, 8 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 77a18aa..50df7a1 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -515,6 +515,7 @@ struct f11_2d_sensor {
>   	int pkt_size;
>   	u8 sensor_index;
>   	u32 type_a;	/* boolean but debugfs API requires u32 */
> +	bool topbuttonpad;
>   	enum rmi_f11_sensor_type sensor_type;
>   	struct input_dev *input;
>   	bool unified_input;
> @@ -1285,6 +1286,7 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   		sensor->axis_align =
>   			pdata->f11_sensor_data->axis_align;
>   		sensor->type_a = pdata->f11_sensor_data->type_a;
> +		sensor->topbuttonpad = pdata->f11_sensor_data->topbuttonpad;
>   
>   		if (sensor->sens_query.has_physical_props) {
>   			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
> @@ -1409,6 +1411,9 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
>   	if (sensor->report_abs)
>   		f11_set_abs_params(fn, f11);
>   
> +	if (sensor->topbuttonpad)
> +		set_bit(INPUT_PROP_TOPBUTTONPAD, input_dev->propbit);
> +
>   	if (sensor->report_rel) {
>   		set_bit(EV_REL, input_dev->evbit);
>   		set_bit(REL_X, input_dev->relbit);
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index b771f41..4ffe9fe 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -93,6 +93,8 @@ enum rmi_f11_sensor_type {
>    * available.
>    * @disable_report_mask - Force data to not be reported even if it is supported
>    * by the firware.
> + * @topbuttonpad - Used with the "5 buttons touchpads" found on the Lenovo 40
> + * series
>    */
>   struct rmi_f11_sensor_data {
>   	struct rmi_f11_2d_axis_alignment axis_align;
> @@ -101,6 +103,7 @@ struct rmi_f11_sensor_data {
>   	int x_mm;
>   	int y_mm;
>   	int disable_report_mask;
> +	bool topbuttonpad;
>   };
>   
>   /**

Re: [PATCH 10/11] Input: synaptics-rmi4 - f11: allow the top software button property to be set

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Currently, in PS/2 we only have the PNPIds list to detect the property.
> Unfortunately, it looks like the information is not embeded in the RMI4
> protocol either, so allow the Top software buttons property to be set
> in the platform data.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 5 +++++
>   include/linux/rmi.h          | 3 +++
>   2 files changed, 8 insertions(+)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 77a18aa..50df7a1 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -515,6 +515,7 @@ struct f11_2d_sensor {
>   	int pkt_size;
>   	u8 sensor_index;
>   	u32 type_a;	/* boolean but debugfs API requires u32 */
> +	bool topbuttonpad;
>   	enum rmi_f11_sensor_type sensor_type;
>   	struct input_dev *input;
>   	bool unified_input;
> @@ -1285,6 +1286,7 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   		sensor->axis_align =
>   			pdata->f11_sensor_data->axis_align;
>   		sensor->type_a = pdata->f11_sensor_data->type_a;
> +		sensor->topbuttonpad = pdata->f11_sensor_data->topbuttonpad;
>   
>   		if (sensor->sens_query.has_physical_props) {
>   			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
> @@ -1409,6 +1411,9 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
>   	if (sensor->report_abs)
>   		f11_set_abs_params(fn, f11);
>   
> +	if (sensor->topbuttonpad)
> +		set_bit(INPUT_PROP_TOPBUTTONPAD, input_dev->propbit);
> +
>   	if (sensor->report_rel) {
>   		set_bit(EV_REL, input_dev->evbit);
>   		set_bit(REL_X, input_dev->relbit);
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index b771f41..4ffe9fe 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -93,6 +93,8 @@ enum rmi_f11_sensor_type {
>    * available.
>    * @disable_report_mask - Force data to not be reported even if it is supported
>    * by the firware.
> + * @topbuttonpad - Used with the "5 buttons touchpads" found on the Lenovo 40
> + * series
>    */
>   struct rmi_f11_sensor_data {
>   	struct rmi_f11_2d_axis_alignment axis_align;
> @@ -101,6 +103,7 @@ struct rmi_f11_sensor_data {
>   	int x_mm;
>   	int y_mm;
>   	int disable_report_mask;
> +	bool topbuttonpad;
>   };
>   
>   /**

[PATCH 11/11] Input: synaptics-rmi4 - f11: add support for kernel tracking

[Benjamin Tissoires]

Kernel tracking is used in 2 use cases:
- filter out jumps when the sensor is not relieable enough
- provide a MT protocol B when the sensor is providing MT protocol A data

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_f11.c | 152 +++++++++++++++++++++++++++++--------------
 include/linux/rmi.h          |  14 ++--
 2 files changed, 114 insertions(+), 52 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 50df7a1..8bccc8a 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -65,6 +65,9 @@
  * devices currently in the field.
  */
 
+/* maximum ABS_MT_POSITION displacement (in mm) */
+#define DMAX 10
+
 /**
  * @rezero - writing this to the F11 command register will cause the sensor to
  * calibrate to the current capacitive state.
@@ -497,9 +500,6 @@ struct f11_2d_data {
  * @data_pkt - buffer for data reported by this sensor.
  * @pkt_size - number of bytes in that buffer.
  * @sensor_index - identifies this particular 2D touch sensor
- * @type_a - some early RMI4 2D sensors do not reliably track the finger
- * position when two fingers are on the device.  When this is true, we
- * assume we have one of those sensors and report events appropriately.
  * @sensor_type - indicates whether we're touchscreen or touchpad.
  * @input - input device for absolute pointing stream
  * @input_phys - buffer for the absolute phys name for this sensor.
@@ -508,13 +508,16 @@ struct f11_2d_sensor {
 	struct rmi_f11_2d_axis_alignment axis_align;
 	struct f11_2d_sensor_queries sens_query;
 	struct f11_2d_data data;
+	struct input_mt_pos *tracking_pos;
+	int *tracking_slots;
+	bool kernel_tracking;
+	int dmax;
 	u16 max_x;
 	u16 max_y;
 	u8 nbr_fingers;
 	u8 *data_pkt;
 	int pkt_size;
 	u8 sensor_index;
-	u32 type_a;	/* boolean but debugfs API requires u32 */
 	bool topbuttonpad;
 	enum rmi_f11_sensor_type sensor_type;
 	struct input_dev *input;
@@ -604,6 +607,53 @@ static void rmi_f11_rel_pos_report(struct f11_2d_sensor *sensor, u8 n_finger)
 	}
 }
 
+static void rmi_f11_abs_parse_xy(struct f11_data *f11,
+				 struct f11_2d_sensor *sensor,
+				 enum f11_finger_state finger_state,
+				 u8 n_finger)
+{
+	struct f11_2d_data *data = &sensor->data;
+	struct rmi_f11_2d_axis_alignment *axis_align = &sensor->axis_align;
+	u8 *pos_data = &data->abs_pos[n_finger * RMI_F11_ABS_BYTES];
+	u16 x, y;
+
+	/* we keep the previous values if the finger is released */
+	if (!finger_state)
+		return;
+
+	x = (pos_data[0] << 4) | (pos_data[2] & 0x0F);
+	y = (pos_data[1] << 4) | (pos_data[2] >> 4);
+
+	if (axis_align->swap_axes)
+		swap(x, y);
+
+	if (axis_align->flip_x)
+		x = max(sensor->max_x - x, 0);
+
+	if (axis_align->flip_y)
+		y = max(sensor->max_y - y, 0);
+
+	/*
+	 * Here checking if X offset or y offset are specified is
+	 * redundant. We just add the offsets or clip the values.
+	 *
+	 * Note: offsets need to be applied before clipping occurs,
+	 * or we could get funny values that are outside of
+	 * clipping boundaries.
+	 */
+	x += axis_align->offset_x;
+	y += axis_align->offset_y;
+	x =  max(axis_align->clip_x_low, x);
+	y =  max(axis_align->clip_y_low, y);
+	if (axis_align->clip_x_high)
+		x = min(axis_align->clip_x_high, x);
+	if (axis_align->clip_y_high)
+		y =  min(axis_align->clip_y_high, y);
+
+	sensor->tracking_pos[n_finger].x = x;
+	sensor->tracking_pos[n_finger].y = y;
+}
+
 static void rmi_f11_abs_pos_report(struct f11_data *f11,
 				   struct f11_2d_sensor *sensor,
 				   enum f11_finger_state finger_state,
@@ -617,44 +667,16 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
 	int w_x, w_y, w_max, w_min, orient;
 	int tool_type = rmi_f11_get_tool_type(sensor, finger_state);
 
-	if (sensor->type_a) {
-		input_report_abs(input, ABS_MT_TRACKING_ID, n_finger);
-		input_report_abs(input, ABS_MT_TOOL_TYPE, tool_type);
-	} else {
+	if (sensor->kernel_tracking)
+		input_mt_slot(input, sensor->tracking_slots[n_finger]);
+	else
 		input_mt_slot(input, n_finger);
-		input_mt_report_slot_state(input, tool_type,
-					   finger_state != F11_NO_FINGER);
-	}
+	input_mt_report_slot_state(input, tool_type,
+				   finger_state != F11_NO_FINGER);
 
 	if (finger_state) {
-		x = (pos_data[0] << 4) | (pos_data[2] & 0x0F);
-		y = (pos_data[1] << 4) | (pos_data[2] >> 4);
-
-		if (axis_align->swap_axes)
-			swap(x, y);
-
-		if (axis_align->flip_x)
-			x = max(sensor->max_x - x, 0);
-
-		if (axis_align->flip_y)
-			y = max(sensor->max_y - y, 0);
-
-		/*
-		 * Here checking if X offset or y offset are specified is
-		 * redundant. We just add the offsets or clip the values.
-		 *
-		 * Note: offsets need to be applied before clipping occurs,
-		 * or we could get funny values that are outside of
-		 * clipping boundaries.
-		 */
-		x += axis_align->offset_x;
-		y += axis_align->offset_y;
-		x =  max(axis_align->clip_x_low, x);
-		y =  max(axis_align->clip_y_low, y);
-		if (axis_align->clip_x_high)
-			x = min(axis_align->clip_x_high, x);
-		if (axis_align->clip_y_high)
-			y =  min(axis_align->clip_y_high, y);
+		x = sensor->tracking_pos[n_finger].x;
+		y = sensor->tracking_pos[n_finger].y;
 
 		w_x = pos_data[3] & 0x0f;
 		w_y = pos_data[3] >> 4;
@@ -690,10 +712,6 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
 			"finger[%d]:%d - x:%d y:%d z:%d w_max:%d w_min:%d\n",
 			n_finger, finger_state, x, y, z, w_max, w_min);
 	}
-
-	/* MT sync between fingers */
-	if (sensor->type_a)
-		input_mt_sync(input);
 }
 
 static inline u8 rmi_f11_parse_finger_state(const u8 *f_state, u8 n_finger)
@@ -724,13 +742,36 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
 		}
 
 		if (abs_bits)
-			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
+			rmi_f11_abs_parse_xy(f11, sensor, finger_state, i);
 
 		if (rel_bits)
 			rmi_f11_rel_pos_report(sensor, i);
 	}
 
-	input_mt_sync_frame(sensor->input);
+	if (abs_bits) {
+		/*
+		 * the absolute part is made in 2 parts to allow the kernel
+		 * tracking to take place.
+		 */
+		if (sensor->kernel_tracking)
+			input_mt_assign_slots(sensor->input,
+					      sensor->tracking_slots,
+					      sensor->tracking_pos,
+					      sensor->nbr_fingers,
+					      sensor->dmax);
+
+		for (i = 0; i < sensor->nbr_fingers; i++) {
+			finger_state = rmi_f11_parse_finger_state(f_state, i);
+			if (finger_state == F11_RESERVED)
+				/* no need to send twice the error */
+				continue;
+
+			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
+		}
+
+		input_mt_sync_frame(sensor->input);
+	}
+
 	if (!sensor->unified_input)
 		input_sync(sensor->input);
 }
@@ -1138,6 +1179,9 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
 	else
 		input_flags = INPUT_MT_DIRECT;
 
+	if (sensor->kernel_tracking)
+		input_flags |= INPUT_MT_TRACK;
+
 	if (sensor->axis_align.swap_axes) {
 		int temp = device_x_max;
 		device_x_max = device_y_max;
@@ -1189,10 +1233,12 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
 
 		input_abs_set_res(input, ABS_MT_POSITION_X, res_x);
 		input_abs_set_res(input, ABS_MT_POSITION_Y, res_y);
+
+		if (!sensor->dmax)
+			sensor->dmax = DMAX * res_x;
 	}
 
-	if (!sensor->type_a)
-		input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
+	input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
 	if (IS_ENABLED(CONFIG_RMI4_F11_PEN) && sensor->sens_query.has_pen)
 		input_set_abs_params(input, ABS_MT_TOOL_TYPE,
 				     0, MT_TOOL_MAX, 0, 0);
@@ -1285,8 +1331,9 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 	if (pdata->f11_sensor_data) {
 		sensor->axis_align =
 			pdata->f11_sensor_data->axis_align;
-		sensor->type_a = pdata->f11_sensor_data->type_a;
 		sensor->topbuttonpad = pdata->f11_sensor_data->topbuttonpad;
+		sensor->kernel_tracking = pdata->f11_sensor_data->kernel_tracking;
+		sensor->dmax = pdata->f11_sensor_data->dmax;
 
 		if (sensor->sens_query.has_physical_props) {
 			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
@@ -1336,6 +1383,15 @@ static int rmi_f11_initialize(struct rmi_function *fn)
 	if (rc < 0)
 		return rc;
 
+	/* allocate the in-kernel tracking buffers */
+	sensor->tracking_pos = devm_kzalloc(&fn->dev,
+			sizeof(struct input_mt_pos) * sensor->nbr_fingers,
+			GFP_KERNEL);
+	sensor->tracking_slots = devm_kzalloc(&fn->dev,
+			sizeof(int) * sensor->nbr_fingers, GFP_KERNEL);
+	if (!sensor->tracking_pos || !sensor->tracking_slots)
+		return -ENOMEM;
+
 	ctrl = &f11->dev_controls;
 	if (sensor->axis_align.delta_x_threshold) {
 		ctrl->ctrl0_9[RMI_F11_DELTA_X_THRESHOLD] =
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index 4ffe9fe..f270ff9 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -84,9 +84,6 @@ enum rmi_f11_sensor_type {
 /**
  * struct rmi_f11_sensor_data - overrides defaults for a single F11 2D sensor.
  * @axis_align - provides axis alignment overrides (see above).
- * @type_a - all modern RMI F11 firmwares implement Multifinger Type B
- * protocol.  Set this to true to force MF Type A behavior, in case you find
- * an older sensor.
  * @sensor_type - Forces the driver to treat the sensor as an indirect
  * pointing device (touchpad) rather than a direct pointing device
  * (touchscreen).  This is useful when F11_2D_QUERY14 register is not
@@ -95,15 +92,24 @@ enum rmi_f11_sensor_type {
  * by the firware.
  * @topbuttonpad - Used with the "5 buttons touchpads" found on the Lenovo 40
  * series
+ * @kernel_tracking - most moderns RMI f11 firmwares implement Multifinger
+ * Type B protocol. However, there are some corner cases where the user
+ * triggers some jumps by tapping with two fingers on the touchpad.
+ * Use this setting and dmax to filter out these jumps.
+ * Also, when using an old sensor using MF Type A behavior, set to true to
+ * report an actual MT protocol B.
+ * @dmax - the maximum distance (in sensor units) the kernel tracking allows two
+ * distincts fingers to be considered the same.
  */
 struct rmi_f11_sensor_data {
 	struct rmi_f11_2d_axis_alignment axis_align;
-	bool type_a;
 	enum rmi_f11_sensor_type sensor_type;
 	int x_mm;
 	int y_mm;
 	int disable_report_mask;
 	bool topbuttonpad;
+	bool kernel_tracking;
+	int dmax;
 };
 
 /**
-- 
2.4.3

Re: [PATCH 11/11] Input: synaptics-rmi4 - f11: add support for kernel tracking

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Kernel tracking is used in 2 use cases:
> - filter out jumps when the sensor is not relieable enough
> - provide a MT protocol B when the sensor is providing MT protocol A data
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 152 +++++++++++++++++++++++++++++--------------
>   include/linux/rmi.h          |  14 ++--
>   2 files changed, 114 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 50df7a1..8bccc8a 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -65,6 +65,9 @@
>    * devices currently in the field.
>    */
>   
> +/* maximum ABS_MT_POSITION displacement (in mm) */
> +#define DMAX 10
> +
>   /**
>    * @rezero - writing this to the F11 command register will cause the sensor to
>    * calibrate to the current capacitive state.
> @@ -497,9 +500,6 @@ struct f11_2d_data {
>    * @data_pkt - buffer for data reported by this sensor.
>    * @pkt_size - number of bytes in that buffer.
>    * @sensor_index - identifies this particular 2D touch sensor
> - * @type_a - some early RMI4 2D sensors do not reliably track the finger
> - * position when two fingers are on the device.  When this is true, we
> - * assume we have one of those sensors and report events appropriately.
>    * @sensor_type - indicates whether we're touchscreen or touchpad.
>    * @input - input device for absolute pointing stream
>    * @input_phys - buffer for the absolute phys name for this sensor.
> @@ -508,13 +508,16 @@ struct f11_2d_sensor {
>   	struct rmi_f11_2d_axis_alignment axis_align;
>   	struct f11_2d_sensor_queries sens_query;
>   	struct f11_2d_data data;
> +	struct input_mt_pos *tracking_pos;
> +	int *tracking_slots;
> +	bool kernel_tracking;
> +	int dmax;
>   	u16 max_x;
>   	u16 max_y;
>   	u8 nbr_fingers;
>   	u8 *data_pkt;
>   	int pkt_size;
>   	u8 sensor_index;
> -	u32 type_a;	/* boolean but debugfs API requires u32 */
>   	bool topbuttonpad;
>   	enum rmi_f11_sensor_type sensor_type;
>   	struct input_dev *input;
> @@ -604,6 +607,53 @@ static void rmi_f11_rel_pos_report(struct f11_2d_sensor *sensor, u8 n_finger)
>   	}
>   }
>   
> +static void rmi_f11_abs_parse_xy(struct f11_data *f11,
> +				 struct f11_2d_sensor *sensor,
> +				 enum f11_finger_state finger_state,
> +				 u8 n_finger)
> +{
> +	struct f11_2d_data *data = &sensor->data;
> +	struct rmi_f11_2d_axis_alignment *axis_align = &sensor->axis_align;
> +	u8 *pos_data = &data->abs_pos[n_finger * RMI_F11_ABS_BYTES];
> +	u16 x, y;
> +
> +	/* we keep the previous values if the finger is released */
> +	if (!finger_state)
> +		return;
> +
> +	x = (pos_data[0] << 4) | (pos_data[2] & 0x0F);
> +	y = (pos_data[1] << 4) | (pos_data[2] >> 4);
> +
> +	if (axis_align->swap_axes)
> +		swap(x, y);
> +
> +	if (axis_align->flip_x)
> +		x = max(sensor->max_x - x, 0);
> +
> +	if (axis_align->flip_y)
> +		y = max(sensor->max_y - y, 0);
> +
> +	/*
> +	 * Here checking if X offset or y offset are specified is
> +	 * redundant. We just add the offsets or clip the values.
> +	 *
> +	 * Note: offsets need to be applied before clipping occurs,
> +	 * or we could get funny values that are outside of
> +	 * clipping boundaries.
> +	 */
> +	x += axis_align->offset_x;
> +	y += axis_align->offset_y;
> +	x =  max(axis_align->clip_x_low, x);
> +	y =  max(axis_align->clip_y_low, y);
> +	if (axis_align->clip_x_high)
> +		x = min(axis_align->clip_x_high, x);
> +	if (axis_align->clip_y_high)
> +		y =  min(axis_align->clip_y_high, y);
> +
> +	sensor->tracking_pos[n_finger].x = x;
> +	sensor->tracking_pos[n_finger].y = y;
> +}
> +
>   static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   				   struct f11_2d_sensor *sensor,
>   				   enum f11_finger_state finger_state,
> @@ -617,44 +667,16 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   	int w_x, w_y, w_max, w_min, orient;
>   	int tool_type = rmi_f11_get_tool_type(sensor, finger_state);
>   
> -	if (sensor->type_a) {
> -		input_report_abs(input, ABS_MT_TRACKING_ID, n_finger);
> -		input_report_abs(input, ABS_MT_TOOL_TYPE, tool_type);
> -	} else {
> +	if (sensor->kernel_tracking)
> +		input_mt_slot(input, sensor->tracking_slots[n_finger]);
> +	else
>   		input_mt_slot(input, n_finger);
> -		input_mt_report_slot_state(input, tool_type,
> -					   finger_state != F11_NO_FINGER);
> -	}
> +	input_mt_report_slot_state(input, tool_type,
> +				   finger_state != F11_NO_FINGER);
>   
>   	if (finger_state) {
> -		x = (pos_data[0] << 4) | (pos_data[2] & 0x0F);
> -		y = (pos_data[1] << 4) | (pos_data[2] >> 4);
> -
> -		if (axis_align->swap_axes)
> -			swap(x, y);
> -
> -		if (axis_align->flip_x)
> -			x = max(sensor->max_x - x, 0);
> -
> -		if (axis_align->flip_y)
> -			y = max(sensor->max_y - y, 0);
> -
> -		/*
> -		 * Here checking if X offset or y offset are specified is
> -		 * redundant. We just add the offsets or clip the values.
> -		 *
> -		 * Note: offsets need to be applied before clipping occurs,
> -		 * or we could get funny values that are outside of
> -		 * clipping boundaries.
> -		 */
> -		x += axis_align->offset_x;
> -		y += axis_align->offset_y;
> -		x =  max(axis_align->clip_x_low, x);
> -		y =  max(axis_align->clip_y_low, y);
> -		if (axis_align->clip_x_high)
> -			x = min(axis_align->clip_x_high, x);
> -		if (axis_align->clip_y_high)
> -			y =  min(axis_align->clip_y_high, y);
> +		x = sensor->tracking_pos[n_finger].x;
> +		y = sensor->tracking_pos[n_finger].y;
>   
>   		w_x = pos_data[3] & 0x0f;
>   		w_y = pos_data[3] >> 4;
> @@ -690,10 +712,6 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   			"finger[%d]:%d - x:%d y:%d z:%d w_max:%d w_min:%d\n",
>   			n_finger, finger_state, x, y, z, w_max, w_min);
>   	}
> -
> -	/* MT sync between fingers */
> -	if (sensor->type_a)
> -		input_mt_sync(input);
>   }
>   
>   static inline u8 rmi_f11_parse_finger_state(const u8 *f_state, u8 n_finger)
> @@ -724,13 +742,36 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   		}
>   
>   		if (abs_bits)
> -			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
> +			rmi_f11_abs_parse_xy(f11, sensor, finger_state, i);
>   
>   		if (rel_bits)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}
>   
> -	input_mt_sync_frame(sensor->input);
> +	if (abs_bits) {
> +		/*
> +		 * the absolute part is made in 2 parts to allow the kernel
> +		 * tracking to take place.
> +		 */
> +		if (sensor->kernel_tracking)
> +			input_mt_assign_slots(sensor->input,
> +					      sensor->tracking_slots,
> +					      sensor->tracking_pos,
> +					      sensor->nbr_fingers,
> +					      sensor->dmax);
> +
> +		for (i = 0; i < sensor->nbr_fingers; i++) {
> +			finger_state = rmi_f11_parse_finger_state(f_state, i);
> +			if (finger_state == F11_RESERVED)
> +				/* no need to send twice the error */
> +				continue;
> +
> +			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
> +		}
> +
> +		input_mt_sync_frame(sensor->input);
> +	}
> +
>   	if (!sensor->unified_input)
>   		input_sync(sensor->input);
>   }
> @@ -1138,6 +1179,9 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
>   	else
>   		input_flags = INPUT_MT_DIRECT;
>   
> +	if (sensor->kernel_tracking)
> +		input_flags |= INPUT_MT_TRACK;
> +
>   	if (sensor->axis_align.swap_axes) {
>   		int temp = device_x_max;
>   		device_x_max = device_y_max;
> @@ -1189,10 +1233,12 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
>   
>   		input_abs_set_res(input, ABS_MT_POSITION_X, res_x);
>   		input_abs_set_res(input, ABS_MT_POSITION_Y, res_y);
> +
> +		if (!sensor->dmax)
> +			sensor->dmax = DMAX * res_x;
>   	}
>   
> -	if (!sensor->type_a)
> -		input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
> +	input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
>   	if (IS_ENABLED(CONFIG_RMI4_F11_PEN) && sensor->sens_query.has_pen)
>   		input_set_abs_params(input, ABS_MT_TOOL_TYPE,
>   				     0, MT_TOOL_MAX, 0, 0);
> @@ -1285,8 +1331,9 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   	if (pdata->f11_sensor_data) {
>   		sensor->axis_align =
>   			pdata->f11_sensor_data->axis_align;
> -		sensor->type_a = pdata->f11_sensor_data->type_a;
>   		sensor->topbuttonpad = pdata->f11_sensor_data->topbuttonpad;
> +		sensor->kernel_tracking = pdata->f11_sensor_data->kernel_tracking;
> +		sensor->dmax = pdata->f11_sensor_data->dmax;
>   
>   		if (sensor->sens_query.has_physical_props) {
>   			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
> @@ -1336,6 +1383,15 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   	if (rc < 0)
>   		return rc;
>   
> +	/* allocate the in-kernel tracking buffers */
> +	sensor->tracking_pos = devm_kzalloc(&fn->dev,
> +			sizeof(struct input_mt_pos) * sensor->nbr_fingers,
> +			GFP_KERNEL);
> +	sensor->tracking_slots = devm_kzalloc(&fn->dev,
> +			sizeof(int) * sensor->nbr_fingers, GFP_KERNEL);
> +	if (!sensor->tracking_pos || !sensor->tracking_slots)
> +		return -ENOMEM;
> +
>   	ctrl = &f11->dev_controls;
>   	if (sensor->axis_align.delta_x_threshold) {
>   		ctrl->ctrl0_9[RMI_F11_DELTA_X_THRESHOLD] =
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index 4ffe9fe..f270ff9 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -84,9 +84,6 @@ enum rmi_f11_sensor_type {
>   /**
>    * struct rmi_f11_sensor_data - overrides defaults for a single F11 2D sensor.
>    * @axis_align - provides axis alignment overrides (see above).
> - * @type_a - all modern RMI F11 firmwares implement Multifinger Type B
> - * protocol.  Set this to true to force MF Type A behavior, in case you find
> - * an older sensor.
>    * @sensor_type - Forces the driver to treat the sensor as an indirect
>    * pointing device (touchpad) rather than a direct pointing device
>    * (touchscreen).  This is useful when F11_2D_QUERY14 register is not
> @@ -95,15 +92,24 @@ enum rmi_f11_sensor_type {
>    * by the firware.
>    * @topbuttonpad - Used with the "5 buttons touchpads" found on the Lenovo 40
>    * series
> + * @kernel_tracking - most moderns RMI f11 firmwares implement Multifinger
> + * Type B protocol. However, there are some corner cases where the user
> + * triggers some jumps by tapping with two fingers on the touchpad.
> + * Use this setting and dmax to filter out these jumps.
> + * Also, when using an old sensor using MF Type A behavior, set to true to
> + * report an actual MT protocol B.
> + * @dmax - the maximum distance (in sensor units) the kernel tracking allows two
> + * distincts fingers to be considered the same.
>    */
>   struct rmi_f11_sensor_data {
>   	struct rmi_f11_2d_axis_alignment axis_align;
> -	bool type_a;
>   	enum rmi_f11_sensor_type sensor_type;
>   	int x_mm;
>   	int y_mm;
>   	int disable_report_mask;
>   	bool topbuttonpad;
> +	bool kernel_tracking;
> +	int dmax;
>   };
>   
>   /**

Re: [PATCH 11/11] Input: synaptics-rmi4 - f11: add support for kernel tracking

[Andrew Duggan]

On 06/23/2015 12:17 PM, Benjamin Tissoires wrote:
> Kernel tracking is used in 2 use cases:
> - filter out jumps when the sensor is not relieable enough
> - provide a MT protocol B when the sensor is providing MT protocol A data
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---

Reviewed-by: Andrew Duggan <aduggan@synaptics.com>

>   drivers/input/rmi4/rmi_f11.c | 152 +++++++++++++++++++++++++++++--------------
>   include/linux/rmi.h          |  14 ++--
>   2 files changed, 114 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 50df7a1..8bccc8a 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -65,6 +65,9 @@
>    * devices currently in the field.
>    */
>   
> +/* maximum ABS_MT_POSITION displacement (in mm) */
> +#define DMAX 10
> +
>   /**
>    * @rezero - writing this to the F11 command register will cause the sensor to
>    * calibrate to the current capacitive state.
> @@ -497,9 +500,6 @@ struct f11_2d_data {
>    * @data_pkt - buffer for data reported by this sensor.
>    * @pkt_size - number of bytes in that buffer.
>    * @sensor_index - identifies this particular 2D touch sensor
> - * @type_a - some early RMI4 2D sensors do not reliably track the finger
> - * position when two fingers are on the device.  When this is true, we
> - * assume we have one of those sensors and report events appropriately.
>    * @sensor_type - indicates whether we're touchscreen or touchpad.
>    * @input - input device for absolute pointing stream
>    * @input_phys - buffer for the absolute phys name for this sensor.
> @@ -508,13 +508,16 @@ struct f11_2d_sensor {
>   	struct rmi_f11_2d_axis_alignment axis_align;
>   	struct f11_2d_sensor_queries sens_query;
>   	struct f11_2d_data data;
> +	struct input_mt_pos *tracking_pos;
> +	int *tracking_slots;
> +	bool kernel_tracking;
> +	int dmax;
>   	u16 max_x;
>   	u16 max_y;
>   	u8 nbr_fingers;
>   	u8 *data_pkt;
>   	int pkt_size;
>   	u8 sensor_index;
> -	u32 type_a;	/* boolean but debugfs API requires u32 */
>   	bool topbuttonpad;
>   	enum rmi_f11_sensor_type sensor_type;
>   	struct input_dev *input;
> @@ -604,6 +607,53 @@ static void rmi_f11_rel_pos_report(struct f11_2d_sensor *sensor, u8 n_finger)
>   	}
>   }
>   
> +static void rmi_f11_abs_parse_xy(struct f11_data *f11,
> +				 struct f11_2d_sensor *sensor,
> +				 enum f11_finger_state finger_state,
> +				 u8 n_finger)
> +{
> +	struct f11_2d_data *data = &sensor->data;
> +	struct rmi_f11_2d_axis_alignment *axis_align = &sensor->axis_align;
> +	u8 *pos_data = &data->abs_pos[n_finger * RMI_F11_ABS_BYTES];
> +	u16 x, y;
> +
> +	/* we keep the previous values if the finger is released */
> +	if (!finger_state)
> +		return;
> +
> +	x = (pos_data[0] << 4) | (pos_data[2] & 0x0F);
> +	y = (pos_data[1] << 4) | (pos_data[2] >> 4);
> +
> +	if (axis_align->swap_axes)
> +		swap(x, y);
> +
> +	if (axis_align->flip_x)
> +		x = max(sensor->max_x - x, 0);
> +
> +	if (axis_align->flip_y)
> +		y = max(sensor->max_y - y, 0);
> +
> +	/*
> +	 * Here checking if X offset or y offset are specified is
> +	 * redundant. We just add the offsets or clip the values.
> +	 *
> +	 * Note: offsets need to be applied before clipping occurs,
> +	 * or we could get funny values that are outside of
> +	 * clipping boundaries.
> +	 */
> +	x += axis_align->offset_x;
> +	y += axis_align->offset_y;
> +	x =  max(axis_align->clip_x_low, x);
> +	y =  max(axis_align->clip_y_low, y);
> +	if (axis_align->clip_x_high)
> +		x = min(axis_align->clip_x_high, x);
> +	if (axis_align->clip_y_high)
> +		y =  min(axis_align->clip_y_high, y);
> +
> +	sensor->tracking_pos[n_finger].x = x;
> +	sensor->tracking_pos[n_finger].y = y;
> +}
> +
>   static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   				   struct f11_2d_sensor *sensor,
>   				   enum f11_finger_state finger_state,
> @@ -617,44 +667,16 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   	int w_x, w_y, w_max, w_min, orient;
>   	int tool_type = rmi_f11_get_tool_type(sensor, finger_state);
>   
> -	if (sensor->type_a) {
> -		input_report_abs(input, ABS_MT_TRACKING_ID, n_finger);
> -		input_report_abs(input, ABS_MT_TOOL_TYPE, tool_type);
> -	} else {
> +	if (sensor->kernel_tracking)
> +		input_mt_slot(input, sensor->tracking_slots[n_finger]);
> +	else
>   		input_mt_slot(input, n_finger);
> -		input_mt_report_slot_state(input, tool_type,
> -					   finger_state != F11_NO_FINGER);
> -	}
> +	input_mt_report_slot_state(input, tool_type,
> +				   finger_state != F11_NO_FINGER);
>   
>   	if (finger_state) {
> -		x = (pos_data[0] << 4) | (pos_data[2] & 0x0F);
> -		y = (pos_data[1] << 4) | (pos_data[2] >> 4);
> -
> -		if (axis_align->swap_axes)
> -			swap(x, y);
> -
> -		if (axis_align->flip_x)
> -			x = max(sensor->max_x - x, 0);
> -
> -		if (axis_align->flip_y)
> -			y = max(sensor->max_y - y, 0);
> -
> -		/*
> -		 * Here checking if X offset or y offset are specified is
> -		 * redundant. We just add the offsets or clip the values.
> -		 *
> -		 * Note: offsets need to be applied before clipping occurs,
> -		 * or we could get funny values that are outside of
> -		 * clipping boundaries.
> -		 */
> -		x += axis_align->offset_x;
> -		y += axis_align->offset_y;
> -		x =  max(axis_align->clip_x_low, x);
> -		y =  max(axis_align->clip_y_low, y);
> -		if (axis_align->clip_x_high)
> -			x = min(axis_align->clip_x_high, x);
> -		if (axis_align->clip_y_high)
> -			y =  min(axis_align->clip_y_high, y);
> +		x = sensor->tracking_pos[n_finger].x;
> +		y = sensor->tracking_pos[n_finger].y;
>   
>   		w_x = pos_data[3] & 0x0f;
>   		w_y = pos_data[3] >> 4;
> @@ -690,10 +712,6 @@ static void rmi_f11_abs_pos_report(struct f11_data *f11,
>   			"finger[%d]:%d - x:%d y:%d z:%d w_max:%d w_min:%d\n",
>   			n_finger, finger_state, x, y, z, w_max, w_min);
>   	}
> -
> -	/* MT sync between fingers */
> -	if (sensor->type_a)
> -		input_mt_sync(input);
>   }
>   
>   static inline u8 rmi_f11_parse_finger_state(const u8 *f_state, u8 n_finger)
> @@ -724,13 +742,36 @@ static void rmi_f11_finger_handler(struct f11_data *f11,
>   		}
>   
>   		if (abs_bits)
> -			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
> +			rmi_f11_abs_parse_xy(f11, sensor, finger_state, i);
>   
>   		if (rel_bits)
>   			rmi_f11_rel_pos_report(sensor, i);
>   	}
>   
> -	input_mt_sync_frame(sensor->input);
> +	if (abs_bits) {
> +		/*
> +		 * the absolute part is made in 2 parts to allow the kernel
> +		 * tracking to take place.
> +		 */
> +		if (sensor->kernel_tracking)
> +			input_mt_assign_slots(sensor->input,
> +					      sensor->tracking_slots,
> +					      sensor->tracking_pos,
> +					      sensor->nbr_fingers,
> +					      sensor->dmax);
> +
> +		for (i = 0; i < sensor->nbr_fingers; i++) {
> +			finger_state = rmi_f11_parse_finger_state(f_state, i);
> +			if (finger_state == F11_RESERVED)
> +				/* no need to send twice the error */
> +				continue;
> +
> +			rmi_f11_abs_pos_report(f11, sensor, finger_state, i);
> +		}
> +
> +		input_mt_sync_frame(sensor->input);
> +	}
> +
>   	if (!sensor->unified_input)
>   		input_sync(sensor->input);
>   }
> @@ -1138,6 +1179,9 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
>   	else
>   		input_flags = INPUT_MT_DIRECT;
>   
> +	if (sensor->kernel_tracking)
> +		input_flags |= INPUT_MT_TRACK;
> +
>   	if (sensor->axis_align.swap_axes) {
>   		int temp = device_x_max;
>   		device_x_max = device_y_max;
> @@ -1189,10 +1233,12 @@ static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
>   
>   		input_abs_set_res(input, ABS_MT_POSITION_X, res_x);
>   		input_abs_set_res(input, ABS_MT_POSITION_Y, res_y);
> +
> +		if (!sensor->dmax)
> +			sensor->dmax = DMAX * res_x;
>   	}
>   
> -	if (!sensor->type_a)
> -		input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
> +	input_mt_init_slots(input, sensor->nbr_fingers, input_flags);
>   	if (IS_ENABLED(CONFIG_RMI4_F11_PEN) && sensor->sens_query.has_pen)
>   		input_set_abs_params(input, ABS_MT_TOOL_TYPE,
>   				     0, MT_TOOL_MAX, 0, 0);
> @@ -1285,8 +1331,9 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   	if (pdata->f11_sensor_data) {
>   		sensor->axis_align =
>   			pdata->f11_sensor_data->axis_align;
> -		sensor->type_a = pdata->f11_sensor_data->type_a;
>   		sensor->topbuttonpad = pdata->f11_sensor_data->topbuttonpad;
> +		sensor->kernel_tracking = pdata->f11_sensor_data->kernel_tracking;
> +		sensor->dmax = pdata->f11_sensor_data->dmax;
>   
>   		if (sensor->sens_query.has_physical_props) {
>   			sensor->x_mm = sensor->sens_query.x_sensor_size_mm;
> @@ -1336,6 +1383,15 @@ static int rmi_f11_initialize(struct rmi_function *fn)
>   	if (rc < 0)
>   		return rc;
>   
> +	/* allocate the in-kernel tracking buffers */
> +	sensor->tracking_pos = devm_kzalloc(&fn->dev,
> +			sizeof(struct input_mt_pos) * sensor->nbr_fingers,
> +			GFP_KERNEL);
> +	sensor->tracking_slots = devm_kzalloc(&fn->dev,
> +			sizeof(int) * sensor->nbr_fingers, GFP_KERNEL);
> +	if (!sensor->tracking_pos || !sensor->tracking_slots)
> +		return -ENOMEM;
> +
>   	ctrl = &f11->dev_controls;
>   	if (sensor->axis_align.delta_x_threshold) {
>   		ctrl->ctrl0_9[RMI_F11_DELTA_X_THRESHOLD] =
> diff --git a/include/linux/rmi.h b/include/linux/rmi.h
> index 4ffe9fe..f270ff9 100644
> --- a/include/linux/rmi.h
> +++ b/include/linux/rmi.h
> @@ -84,9 +84,6 @@ enum rmi_f11_sensor_type {
>   /**
>    * struct rmi_f11_sensor_data - overrides defaults for a single F11 2D sensor.
>    * @axis_align - provides axis alignment overrides (see above).
> - * @type_a - all modern RMI F11 firmwares implement Multifinger Type B
> - * protocol.  Set this to true to force MF Type A behavior, in case you find
> - * an older sensor.
>    * @sensor_type - Forces the driver to treat the sensor as an indirect
>    * pointing device (touchpad) rather than a direct pointing device
>    * (touchscreen).  This is useful when F11_2D_QUERY14 register is not
> @@ -95,15 +92,24 @@ enum rmi_f11_sensor_type {
>    * by the firware.
>    * @topbuttonpad - Used with the "5 buttons touchpads" found on the Lenovo 40
>    * series
> + * @kernel_tracking - most moderns RMI f11 firmwares implement Multifinger
> + * Type B protocol. However, there are some corner cases where the user
> + * triggers some jumps by tapping with two fingers on the touchpad.
> + * Use this setting and dmax to filter out these jumps.
> + * Also, when using an old sensor using MF Type A behavior, set to true to
> + * report an actual MT protocol B.
> + * @dmax - the maximum distance (in sensor units) the kernel tracking allows two
> + * distincts fingers to be considered the same.
>    */
>   struct rmi_f11_sensor_data {
>   	struct rmi_f11_2d_axis_alignment axis_align;
> -	bool type_a;
>   	enum rmi_f11_sensor_type sensor_type;
>   	int x_mm;
>   	int y_mm;
>   	int disable_report_mask;
>   	bool topbuttonpad;
> +	bool kernel_tracking;
> +	int dmax;
>   };
>   
>   /**

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Benjamin Tissoires]

On Jun 23 2015 or thereabouts, Benjamin Tissoires wrote:
> Hi Dmitry,
> 
> As mentioned in the thread on linux-i2c[1], I am making good progress on the
> RMI4 SMBus implementation [2].
> I am still missing the PS/2 pass-through function but I have an intern (Chandler,
> CC-ed) working on that. Also, I believe forcepads could used RMI4 but we will
> have to implement the ForcePad feature first :)
> 
> Instead of waiting for both the i2c SMBus feature to be validated and the driver
> to be feature equivalent (greater actually because it tracks 5 fingers instead
> of 1 and a half), I am sending those first fixes to you for the branch
> synaptics-rmi4.
> 
> Andrew, Christopher, I am not 100% sure regarding the Signed-off and acked-by
> for the implementation of function 30 and the SMBus transport layer in my tree
> (I smashed a bunch of commits so the result differs from what you saw last time).
> Could you validate this?
> 
> Cheers,
> Benjamin

Dmitry, the Sony guys are currently working on upstreaming their work.
Their Xperia are using a Synaptics I2C touchscreen. They are able to
boot with an upstream kernel a lot of parts now, and they will
eventually need the upstream touch controller.

So, can you consider merging these patches in your synaptics-rmi4 branch
so they have the same features we are working on?

Right now we will be missing for them the device tree implementation in
rmi_i2c.c, but that is something we can work around.

Thanks and sorry for being pushing.

Cheers,
Benjamin

> 
> 
> [1] http://www.spinics.net/lists/linux-i2c/msg20241.html
> [2] https://github.com/bentiss/linux branch synaptics-rmi4-smbus-v4.1-15-06-23
> 
> Benjamin Tissoires (11):
>   Input: synaptics-rmi4 - embed the function modules in rmi_core
>   Input: synaptics-rmi4 - add a common input device in rmi_driver
>   Input: synaptics-rmi4 - explicitly request polling when needed
>   Input: synaptics-rmi4 - prevent oopses when irq arrives while the
>     device is not bound
>   Input: synaptics-rmi4 - call rmi_driver_process_config_requests in
>     enable_sensor
>   Input: synaptics-rmi4 - add a reset callback
>   Input: synaptics-rmi4 - f11: fix bitmap irq check
>   Input: synaptics-rmi4 - f11: use the unified input node if available
>   Input: synaptics-rmi4 - f11: clean up rmi_f11_finger_handler
>   Input: synaptics-rmi4 - f11: allow the top software button property to
>     be set
>   Input: synaptics-rmi4 - f11: add support for kernel tracking
> 
>  drivers/input/rmi4/Kconfig      |   5 +-
>  drivers/input/rmi4/Makefile     |   2 +-
>  drivers/input/rmi4/rmi_bus.c    |  11 +-
>  drivers/input/rmi4/rmi_bus.h    |   1 +
>  drivers/input/rmi4/rmi_driver.c | 146 +++++++++++++++-------
>  drivers/input/rmi4/rmi_driver.h |  17 +++
>  drivers/input/rmi4/rmi_f01.c    |   7 ++
>  drivers/input/rmi4/rmi_f11.c    | 262 ++++++++++++++++++++++++++--------------
>  include/linux/rmi.h             |  22 +++-
>  9 files changed, 326 insertions(+), 147 deletions(-)
> 
> -- 
> 2.4.3
> 

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Linus Walleij]

On Thu, Jul 23, 2015 at 7:10 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:

> Dmitry, the Sony guys are currently working on upstreaming their work.
> Their Xperia are using a Synaptics I2C touchscreen. They are able to
> boot with an upstream kernel a lot of parts now, and they will
> eventually need the upstream touch controller.

I have a Synaptics touchscreen thing since 2010 waiting in
drivers/staging/ste_rmi4.

Is there *anything* I can do to help further this?

It just seems like a massive body of code that doesn't get the
right love, Benjamin have you considered just merging this into
drivers/staging so it is simple for everyone to participate, or are there
infrastructural blockers?

Yours,
Linus Walleij

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Andrew Duggan]

On 10/31/2015 01:41 PM, Linus Walleij wrote:
> On Thu, Jul 23, 2015 at 7:10 PM, Benjamin Tissoires
> <benjamin.tissoires@gmail.com> wrote:
>
>> Dmitry, the Sony guys are currently working on upstreaming their work.
>> Their Xperia are using a Synaptics I2C touchscreen. They are able to
>> boot with an upstream kernel a lot of parts now, and they will
>> eventually need the upstream touch controller.
> I have a Synaptics touchscreen thing since 2010 waiting in
> drivers/staging/ste_rmi4.
>
> Is there *anything* I can do to help further this?
>
> It just seems like a massive body of code that doesn't get the
> right love, Benjamin have you considered just merging this into
> drivers/staging so it is simple for everyone to participate, or are there
> infrastructural blockers?
>
> Yours,
> Linus Walleij

I have been continuing to work on the synaptics-rmi4 driver and was just 
trying to figure out what the next step should be. I recently uploaded 
my changes here https://github.com/aduggan/linux. I've held off on 
posting more patches to the list since there are previous patches still 
outstanding and I didn't want to inundate the list. But, at this point I 
think I would appreciate someone volunteering to review it. I can post 
some or all of it to the list if that's the best place for that to happen.

I've added support for newer devices (Function 12), connected it up to 
hid-rmi to support HID touchpads, and reworked the SPI transport driver. 
I'm able to get basic support for touch working for a variety of devices 
RMI4 over I2C on a Nexus 4, RMI4 over SPI on a Nexus 9, and RMI4 over 
HID on various touchpads. With Benjamin's SMBus work it won't take much 
to also support SMBus touchpads.

I think the biggest blocker right now is just the fact that it is a 
large body of code and there hasn't been enough continuous activity to 
on it. So whenever a new patch is posted, reviewers have to go through 
the time consuming process of relearning the code. But, there are a lot 
of RMI4 devices out there without an upstreamed driver so I think it is 
worth doing.

Andrew

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Linus Walleij]

On Mon, Nov 2, 2015 at 11:14 PM, Andrew Duggan <aduggan@synaptics.com> wrote:

> I think the biggest blocker right now is just the fact that it is a large
> body of code and there hasn't been enough continuous activity to on it. So
> whenever a new patch is posted, reviewers have to go through the time
> consuming process of relearning the code. But, there are a lot of RMI4
> devices out there without an upstreamed driver so I think it is worth doing.

OK I've offloaded Dmitry with reviews of this code in the past and I
can do it again.

I'd like to know what to review though and it needs to be posted to
the mailing list with a cover letter.

In the meantime I can clone your git and see if I can get it working
on my Ux500 TVK with the RMI4 Synaptics touchscreen.

Yours,
Linus Walleij

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Linus Walleij]

On Mon, Nov 2, 2015 at 11:14 PM, Andrew Duggan <aduggan@synaptics.com> wrote:

> I have been continuing to work on the synaptics-rmi4 driver and was just
> trying to figure out what the next step should be. I recently uploaded my
> changes here https://github.com/aduggan/linux.

I just tested this patch set on the Ux500 with a TVK UI board.

I added this:

        i2c@80110000 {
            synaptics@4b {
                /* Synaptics RMI4 TM1217 touchscreen */
                compatible = "syna,rmi-i2c";
                #address-cells = <1>;
                #size-cells = <0>;
                reg = <0x4b>;
                pinctrl-names = "default";
                pinctrl-0 = <&synaptics_tvk_mode>;
                interrupt-parent = <&gpio2>;
                interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
                syna,sensor-name="TM1217";

                rmi-f01@1 {
                    reg = <0x1>;
                    syna,nosleep = <1>;
                };
                rmi-f11@11 {
                    reg = <0x11>;
                    syna,f11-flip-x = <1>;
                    syna,sensor-type = <1>;
                };
            };
        };

Bootlog:
[    2.143127] rmi_f01 sensor00.fn01: found RMI device, manufacturer:
Synaptics, product: TM1217
[    2.155242] input: Synaptics RMI4 Touch Sensor as
/devices/sensor00/input/input2
[    2.165466] rmi_i2c 3-004b: registered rmi i2c driver at 0x4b.

Doing cat /dev/input/event2 gives noise on screen, yay.

Some quick questions I see immediately:

- The DT examples in Documentation/devicetree/bindings/input/*
  omit
  #address-cells = <1>;
  #size-cells = <0>;
  for the I2C device children (i.e. the function nodes), it needs to look
  like my example above to work.

- All things boolean like syna,nosleep and syna,f11-flip-x
  should just be like:
  syna,nosleep;
  syna,f11-flip-x;
  in the device tree. Use of_property_read_bool() for these.

- syna,sensor-name = "FOO";
  Why?
  The bootlog clearly states that f01 can autodetect the sensor
  type. And f01 is always compiled in, right? So just cut this
  binding and handling, if the two don't match it's just
  super-confusing.

- /proc/interrupts say this:
 206:        114          0  nmk2-64-95  20 Edge      sensor00
 sensor00? Unhelpful. Why can't it say "TM1217", give take
 an instance number, with the detected sensor name?

But to me the code seems overall pretty mature. It just works.
I'll be happy to give a more detailed review if you post it.

Yours,
Linus Walleij

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Andrew Duggan]

On 11/03/2015 06:01 AM, Linus Walleij wrote:
> On Mon, Nov 2, 2015 at 11:14 PM, Andrew Duggan <aduggan@synaptics.com> wrote:
>
>> I have been continuing to work on the synaptics-rmi4 driver and was just
>> trying to figure out what the next step should be. I recently uploaded my
>> changes here https://github.com/aduggan/linux.
> I just tested this patch set on the Ux500 with a TVK UI board.
>
> I added this:
>
>          i2c@80110000 {
>              synaptics@4b {
>                  /* Synaptics RMI4 TM1217 touchscreen */
>                  compatible = "syna,rmi-i2c";
>                  #address-cells = <1>;
>                  #size-cells = <0>;
>                  reg = <0x4b>;
>                  pinctrl-names = "default";
>                  pinctrl-0 = <&synaptics_tvk_mode>;
>                  interrupt-parent = <&gpio2>;
>                  interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
>                  syna,sensor-name="TM1217";
>
>                  rmi-f01@1 {
>                      reg = <0x1>;
>                      syna,nosleep = <1>;
>                  };
>                  rmi-f11@11 {
>                      reg = <0x11>;
>                      syna,f11-flip-x = <1>;
>                      syna,sensor-type = <1>;
>                  };
>              };
>          };
>
> Bootlog:
> [    2.143127] rmi_f01 sensor00.fn01: found RMI device, manufacturer:
> Synaptics, product: TM1217
> [    2.155242] input: Synaptics RMI4 Touch Sensor as
> /devices/sensor00/input/input2
> [    2.165466] rmi_i2c 3-004b: registered rmi i2c driver at 0x4b.
>
> Doing cat /dev/input/event2 gives noise on screen, yay.
>
> Some quick questions I see immediately:
>
> - The DT examples in Documentation/devicetree/bindings/input/*
>    omit
>    #address-cells = <1>;
>    #size-cells = <0>;
>    for the I2C device children (i.e. the function nodes), it needs to look
>    like my example above to work.

The devices which I have tested on haven't needed #address-cells or 
#size-cells. It looks like they are inheriting values defined at higher 
levels. I can add them to the example for completeness and update the 
documentation to say that they may be needed.

> - All things boolean like syna,nosleep and syna,f11-flip-x
>    should just be like:
>    syna,nosleep;
>    syna,f11-flip-x;
>    in the device tree. Use of_property_read_bool() for these.

That makes sense. I will switch them to booleans.

> - syna,sensor-name = "FOO";
>    Why?
>    The bootlog clearly states that f01 can autodetect the sensor
>    type. And f01 is always compiled in, right? So just cut this
>    binding and handling, if the two don't match it's just
>    super-confusing.

The sensor name in the platform data is used by some debug messages 
before F01 is loaded. But, I agree it can be confusing having multiple 
names for the device. Especially, when the sensor name is the same as 
the product id. I'll see if it makes sense to use another name like the 
transport device's name in the logs.

> - /proc/interrupts say this:
>   206:        114          0  nmk2-64-95  20 Edge      sensor00
>   sensor00? Unhelpful. Why can't it say "TM1217", give take
>   an instance number, with the detected sensor name?

Currently, rmi_dev's device name is also being set before F01 is loaded. 
I'll see if we can simply set it later when we have the product id from F01.

> But to me the code seems overall pretty mature. It just works.
> I'll be happy to give a more detailed review if you post it.

Great! I'll look into the issues you highlighted above and start posting 
patches soon. Thanks for reviewing!

Andrew

> Yours,
> Linus Walleij

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Benjamin Tissoires]

On Wed, Nov 4, 2015 at 1:38 AM, Andrew Duggan <aduggan@synaptics.com> wrote:
> On 11/03/2015 06:01 AM, Linus Walleij wrote:
>>
>> On Mon, Nov 2, 2015 at 11:14 PM, Andrew Duggan <aduggan@synaptics.com>
>> wrote:
>>
>>> I have been continuing to work on the synaptics-rmi4 driver and was just
>>> trying to figure out what the next step should be. I recently uploaded my
>>> changes here https://github.com/aduggan/linux.
>>
>> I just tested this patch set on the Ux500 with a TVK UI board.
>>
>> I added this:
>>
>>          i2c@80110000 {
>>              synaptics@4b {
>>                  /* Synaptics RMI4 TM1217 touchscreen */
>>                  compatible = "syna,rmi-i2c";
>>                  #address-cells = <1>;
>>                  #size-cells = <0>;
>>                  reg = <0x4b>;
>>                  pinctrl-names = "default";
>>                  pinctrl-0 = <&synaptics_tvk_mode>;
>>                  interrupt-parent = <&gpio2>;
>>                  interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
>>                  syna,sensor-name="TM1217";
>>
>>                  rmi-f01@1 {
>>                      reg = <0x1>;
>>                      syna,nosleep = <1>;
>>                  };
>>                  rmi-f11@11 {
>>                      reg = <0x11>;
>>                      syna,f11-flip-x = <1>;
>>                      syna,sensor-type = <1>;
>>                  };
>>              };
>>          };
>>
>> Bootlog:
>> [    2.143127] rmi_f01 sensor00.fn01: found RMI device, manufacturer:
>> Synaptics, product: TM1217
>> [    2.155242] input: Synaptics RMI4 Touch Sensor as
>> /devices/sensor00/input/input2
>> [    2.165466] rmi_i2c 3-004b: registered rmi i2c driver at 0x4b.
>>
>> Doing cat /dev/input/event2 gives noise on screen, yay.
>>
>> Some quick questions I see immediately:
>>
>> - The DT examples in Documentation/devicetree/bindings/input/*
>>    omit
>>    #address-cells = <1>;
>>    #size-cells = <0>;
>>    for the I2C device children (i.e. the function nodes), it needs to look
>>    like my example above to work.
>
>
> The devices which I have tested on haven't needed #address-cells or
> #size-cells. It looks like they are inheriting values defined at higher
> levels. I can add them to the example for completeness and update the
> documentation to say that they may be needed.
>
>> - All things boolean like syna,nosleep and syna,f11-flip-x
>>    should just be like:
>>    syna,nosleep;
>>    syna,f11-flip-x;
>>    in the device tree. Use of_property_read_bool() for these.
>
>
> That makes sense. I will switch them to booleans.
>
>> - syna,sensor-name = "FOO";
>>    Why?
>>    The bootlog clearly states that f01 can autodetect the sensor
>>    type. And f01 is always compiled in, right? So just cut this
>>    binding and handling, if the two don't match it's just
>>    super-confusing.
>
>
> The sensor name in the platform data is used by some debug messages before
> F01 is loaded. But, I agree it can be confusing having multiple names for
> the device. Especially, when the sensor name is the same as the product id.
> I'll see if it makes sense to use another name like the transport device's
> name in the logs.
>
>> - /proc/interrupts say this:
>>   206:        114          0  nmk2-64-95  20 Edge      sensor00
>>   sensor00? Unhelpful. Why can't it say "TM1217", give take
>>   an instance number, with the detected sensor name?
>
>
> Currently, rmi_dev's device name is also being set before F01 is loaded.
> I'll see if we can simply set it later when we have the product id from F01.
>
>> But to me the code seems overall pretty mature. It just works.
>> I'll be happy to give a more detailed review if you post it.
>
>
> Great! I'll look into the issues you highlighted above and start posting
> patches soon. Thanks for reviewing!
>

Guys,

sorry for being a little bit unresponsive on this matter. I just moved
across the Atlantic (again :-P ) and am in a rough setup for a few
days (hopefully not weeks). I don't currently have any Synaptics RMI4
over SMBus device with me, but Lyude (AKA Chandler) does have access
to the ones we have in Westford.

In addition to the patches I published in this series (the ones Andrew
has), our latest tree
(https://github.com/bentiss/linux/commits/synaptics-rmi4-smbus-v4.3-rc6%2B)
contains the SMBus support and some attempts to fix suspend/resume
with SMBus touchpads.

The problem we have with suspend/resume on SMBus is that the device is
both seen as a PS/2 device (which we unbind) and the SMBus one.
Problem is, during resume, the serial port is reset, which disable the
SMBus part.

Our solution currently consists in disabling the PM resume function
from the rmi_driver code, and export it through an internal API. Once
the low level driver receive the resume callback (either from the PM
code or the serial driver which just busted our init through its own
reset), this low level driver calls the rmi_driver resume function
which does all the RMI4 resume handling.

I think this should be needed for i2c-hid devices too given that most
of them also enumerates as PS/2.

I always refrained to send this resume fixes because we were not able
to test the rmi4_i2c driver as it doesn't register itself with the PM
subsystem.

These are hopefully the only changes that needs to be done on the
rmi4_core part for having rmi_smbus working.

Cheers,
Benjamin

Re: [PATCH 00/11] Input: synaptics-rmi4: various fixes for the existing rmi4 branch

[Linus Walleij]

On Wed, Nov 4, 2015 at 1:38 AM, Andrew Duggan <aduggan@synaptics.com> wrote:

> Great! I'll look into the issues you highlighted above and start posting
> patches soon. Thanks for reviewing!

I think you should also squash all fixes into the logical steps in the previous
patch series or just squash the entire history and send it out in totally
new, logical chunks of functionality.

The credits can be preserved by just stacking on some signed-off-by
in the end of the patch. Author: is usually set to whoever wrote the
majority of the code, but I suspect few care too much as long as
they're mentioned.

Yours,
Linus Walleij