diff mbox

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

Message ID 1435087050-11444-2-git-send-email-benjamin.tissoires@redhat.com
State New, archived
Headers show

Commit Message

Benjamin Tissoires June 23, 2015, 7:17 p.m. UTC
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(-)

Comments

Paul Bolle June 24, 2015, 9:51 a.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Tissoires June 24, 2015, 2:10 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Duggan July 2, 2015, 5:49 p.m. UTC | #3
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");

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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");