[PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c

[PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c

[Christopher Heiny]

This patch implements changes to the synaptics-rmi4 branch of
Dmitry's input tree.  The base for the patch is commit
8ca01dc61a42b6f7bcba052a8c084000f7057a34.

This patch primarily reorders the various declarations in rmi_bus.c in order to
group related elements together, along with some typo fixes.  The code is still
horribly broken, but this change should make the following fixes easier to
 review.

Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jean Delvare <khali@linux-fr.org>
Cc: Linus Walleij <linus.walleij@stericsson.com>
Cc: Joerie de Gram <j.de.gram@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---
 drivers/input/rmi4/rmi_bus.c | 189 +++++++++++++++++++++----------------------
 1 file changed, 90 insertions(+), 99 deletions(-)

diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 88f60ca..d9c450b 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012 Synaptics Incorporated
+ * Copyright (c) 2011-2013 Synaptics Incorporated
  * Copyright (c) 2011 Unixphere
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,77 +19,20 @@
 #include "rmi_bus.h"
 #include "rmi_driver.h"
 
-static int rmi_function_match(struct device *dev, struct device_driver *drv)
-{
-	struct rmi_function_handler *handler = to_rmi_function_handler(drv);
-	struct rmi_function *fn = to_rmi_function(dev);
-
-	return fn->fd.function_number == handler->func;
-}
-
-static int rmi_bus_match(struct device *dev, struct device_driver *drv)
-{
-	bool physical = rmi_is_physical_device(dev);
-
-	/* First see if types are not compatible */
-	if (physical != rmi_is_physical_driver(drv))
-		return 0;
-
-	return physical || rmi_function_match(dev, drv);
-}
-
-struct bus_type rmi_bus_type = {
-	.match		= rmi_bus_match,
-	.name		= "rmi",
-};
-
-#ifdef CONFIG_RMI4_DEBUG
-
-static struct dentry *rmi_debugfs_root;
-
-static void rmi_bus_setup_debugfs(void)
-{
-	rmi_debugfs_root = debugfs_create_dir(rmi_bus_type.name, NULL);
-	if (!rmi_debugfs_root)
-		pr_err("%s: Failed to create debugfs root\n",
-		       __func__);
-}
-
-static void rmi_bus_teardown_debugfs(void)
-{
-	if (rmi_debugfs_root)
-		debugfs_remove_recursive(rmi_debugfs_root);
-}
-
-#else
-
-static void rmi_bus_setup_debugfs(void)
-{
-}
-
-static void rmi_bus_teardown_debugfs(void)
-{
-}
-
-#endif
-
-
 /*
  * RMI Physical devices
  *
  * Physical RMI device consists of several functions serving particular
- * purpose. For example F11 is a 2D touch sensor while F10 is a generic
+ * purpose. For example F11 is a 2D touch sensor while F01 is a generic
  * function present in every RMI device.
  */
 
 static void rmi_release_device(struct device *dev)
 {
 	struct rmi_device *rmi_dev = to_rmi_device(dev);
-
 	kfree(rmi_dev);
 }
 
-/* Device type for physical RMI devices */
 struct device_type rmi_device_type = {
 	.name		= "rmi_sensor",
 	.release	= rmi_release_device,
@@ -118,7 +61,7 @@ static void rmi_physical_teardown_debugfs(struct rmi_device *rmi_dev)
 
 #else
 
-static void rmi_physocal_setup_debugfs(struct rmi_device *rmi_dev)
+static void rmi_physical_setup_debugfs(struct rmi_device *rmi_dev)
 {
 }
 
@@ -128,7 +71,6 @@ static void rmi_physical_teardown_debugfs(struct rmi_device *rmi_dev)
 
 #endif
 
-
 /**
  * rmi_register_physical_device - register a physical device connection on the RMI
  * bus.  Physical drivers provide communication from the devices on the bus to
@@ -174,7 +116,7 @@ int rmi_register_physical_device(struct rmi_phys_device *phys)
 
 	return 0;
 }
-EXPORT_SYMBOL(rmi_register_physical_device);
+EXPORT_SYMBOL_GPL(rmi_register_physical_device);
 
 /**
  * rmi_unregister_physical_device - unregister a physical device connection
@@ -191,18 +133,14 @@ void rmi_unregister_physical_device(struct rmi_phys_device *phys)
 EXPORT_SYMBOL(rmi_unregister_physical_device);
 
 
-/*
- * RMI Function devices and their handlers
- */
+/* Function specific stuff */
 
 static void rmi_release_function(struct device *dev)
 {
 	struct rmi_function *fn = to_rmi_function(dev);
-
 	kfree(fn);
 }
 
-/* Device type for RMI Function devices */
 struct device_type rmi_function_type = {
 	.name		= "rmi_function",
 	.release	= rmi_release_function,
@@ -244,35 +182,12 @@ static void rmi_function_teardown_debugfs(struct rmi_function *fn)
 
 #endif
 
-int rmi_register_function(struct rmi_function *fn)
+static int rmi_function_match(struct device *dev, struct device_driver *drv)
 {
-	struct rmi_device *rmi_dev = fn->rmi_dev;
-	int error;
-
-	dev_set_name(&fn->dev, "%s.fn%02x",
-		     dev_name(&rmi_dev->dev), fn->fd.function_number);
-
-	fn->dev.parent = &rmi_dev->dev;
-	fn->dev.type = &rmi_function_type;
-	fn->dev.bus = &rmi_bus_type;
-
-	error = device_register(&fn->dev);
-	if (error) {
-		dev_err(&rmi_dev->dev,
-			"Failed device_register function device %s\n",
-			dev_name(&fn->dev));
-	}
-
-	dev_dbg(&rmi_dev->dev, "Registered F%02X.\n", fn->fd.function_number);
-
-	rmi_function_setup_debugfs(fn);
-	return 0;
-}
+	struct rmi_function_handler *handler = to_rmi_function_handler(drv);
+	struct rmi_function *fn = to_rmi_function(dev);
 
-void rmi_unregister_function(struct rmi_function *fn)
-{
-	rmi_function_teardown_debugfs(fn);
-	device_unregister(&fn->dev);
+	return fn->fd.function_number == handler->func;
 }
 
 static int rmi_function_probe(struct device *dev)
@@ -302,6 +217,38 @@ static int rmi_function_remove(struct device *dev)
 	return 0;
 }
 
+int rmi_register_function(struct rmi_function *fn)
+{
+	struct rmi_device *rmi_dev = fn->rmi_dev;
+	int error;
+
+	dev_set_name(&fn->dev, "%s.fn%02x",
+		     dev_name(&rmi_dev->dev), fn->fd.function_number);
+
+	fn->dev.parent = &rmi_dev->dev;
+	fn->dev.type = &rmi_function_type;
+	fn->dev.bus = &rmi_bus_type;
+
+	error = device_register(&fn->dev);
+	if (error) {
+		dev_err(&rmi_dev->dev,
+			"Failed device_register function device %s\n",
+			dev_name(&fn->dev));
+		return error;
+	}
+
+	dev_dbg(&rmi_dev->dev, "Registered F%02X.\n", fn->fd.function_number);
+
+	rmi_function_setup_debugfs(fn);
+	return 0;
+}
+
+void rmi_unregister_function(struct rmi_function *fn)
+{
+	rmi_function_teardown_debugfs(fn);
+	device_unregister(&fn->dev);
+}
+
 /**
  * rmi_register_function_handler - register a handler for an RMI function
  * @handler: RMI handler that should be registered.
@@ -334,7 +281,7 @@ int __rmi_register_function_handler(struct rmi_function_handler *handler,
 
 	return 0;
 }
-EXPORT_SYMBOL(__rmi_register_function_handler);
+EXPORT_SYMBOL_GPL(__rmi_register_function_handler);
 
 /**
  * rmi_unregister_function_handler - unregister given RMI function handler
@@ -347,11 +294,55 @@ void rmi_unregister_function_handler(struct rmi_function_handler *handler)
 {
 	driver_unregister(&handler->driver);
 }
-EXPORT_SYMBOL(rmi_unregister_function_handler);
+EXPORT_SYMBOL_GPL(rmi_unregister_function_handler);
 
-/*
- * Bus registration and tear-down
- */
+/* Bus specific stuff */
+
+static int rmi_bus_match(struct device *dev, struct device_driver *drv)
+{
+	bool physical = rmi_is_physical_device(dev);
+
+	/* First see if types are not compatible */
+	if (physical != rmi_is_physical_driver(drv))
+		return 0;
+
+	return physical || rmi_function_match(dev, drv);
+}
+
+struct bus_type rmi_bus_type = {
+	.match		= rmi_bus_match,
+	.name		= "rmi",
+};
+
+#ifdef CONFIG_RMI4_DEBUG
+
+static struct dentry *rmi_debugfs_root;
+
+static void rmi_bus_setup_debugfs(void)
+{
+	rmi_debugfs_root = debugfs_create_dir(rmi_bus_type.name, NULL);
+	if (!rmi_debugfs_root)
+		pr_err("%s: Failed to create debugfs root\n",
+		       __func__);
+}
+
+static void rmi_bus_teardown_debugfs(void)
+{
+	if (rmi_debugfs_root)
+		debugfs_remove_recursive(rmi_debugfs_root);
+}
+
+#else
+
+static void rmi_bus_setup_debugfs(void)
+{
+}
+
+static void rmi_bus_teardown_debugfs(void)
+{
+}
+
+#endif
 
 static int __init rmi_bus_init(void)
 {

Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c

[Benjamin Tissoires]

Hi Chris,

On 05/12/13 19:29, Christopher Heiny wrote:
> This patch implements changes to the synaptics-rmi4 branch of
> Dmitry's input tree.  The base for the patch is commit
> 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
> 
> This patch primarily reorders the various declarations in rmi_bus.c in order to
> group related elements together, along with some typo fixes.  The code is still
> horribly broken, but this change should make the following fixes easier to
>  review.
> 
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Linus Walleij <linus.walleij@stericsson.com>
> Cc: Joerie de Gram <j.de.gram@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> ---

FWIW, I made a review of the patch.
The patches does not only reorder the functions, but also fix some few
things I will detail later (plus fixes of whitespace/comments issues).
It also changes the exported functions as GPL.

Dmitry, given the current state of the driver (which does not work at
all if I understood correctly), maybe you can pick this one in its
current state.

>  drivers/input/rmi4/rmi_bus.c | 189 +++++++++++++++++++++----------------------
>  1 file changed, 90 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index 88f60ca..d9c450b 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2011, 2012 Synaptics Incorporated
> + * Copyright (c) 2011-2013 Synaptics Incorporated
>   * Copyright (c) 2011 Unixphere
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -19,77 +19,20 @@
>  #include "rmi_bus.h"
>  #include "rmi_driver.h"
>  
> -static int rmi_function_match(struct device *dev, struct device_driver *drv)
> -{
> -	struct rmi_function_handler *handler = to_rmi_function_handler(drv);
> -	struct rmi_function *fn = to_rmi_function(dev);
> -
> -	return fn->fd.function_number == handler->func;
> -}
> -
> -static int rmi_bus_match(struct device *dev, struct device_driver *drv)
> -{
> -	bool physical = rmi_is_physical_device(dev);
> -
> -	/* First see if types are not compatible */
> -	if (physical != rmi_is_physical_driver(drv))
> -		return 0;
> -
> -	return physical || rmi_function_match(dev, drv);
> -}
> -
> -struct bus_type rmi_bus_type = {
> -	.match		= rmi_bus_match,
> -	.name		= "rmi",
> -};
> -
> -#ifdef CONFIG_RMI4_DEBUG
> -
> -static struct dentry *rmi_debugfs_root;
> -
> -static void rmi_bus_setup_debugfs(void)
> -{
> -	rmi_debugfs_root = debugfs_create_dir(rmi_bus_type.name, NULL);
> -	if (!rmi_debugfs_root)
> -		pr_err("%s: Failed to create debugfs root\n",
> -		       __func__);
> -}
> -
> -static void rmi_bus_teardown_debugfs(void)
> -{
> -	if (rmi_debugfs_root)
> -		debugfs_remove_recursive(rmi_debugfs_root);
> -}
> -
> -#else
> -
> -static void rmi_bus_setup_debugfs(void)
> -{
> -}
> -
> -static void rmi_bus_teardown_debugfs(void)
> -{
> -}
> -
> -#endif
> -
> -
>  /*
>   * RMI Physical devices
>   *
>   * Physical RMI device consists of several functions serving particular
> - * purpose. For example F11 is a 2D touch sensor while F10 is a generic
> + * purpose. For example F11 is a 2D touch sensor while F01 is a generic
>   * function present in every RMI device.
>   */
>  
>  static void rmi_release_device(struct device *dev)
>  {
>  	struct rmi_device *rmi_dev = to_rmi_device(dev);
> -
>  	kfree(rmi_dev);
>  }
>  
> -/* Device type for physical RMI devices */
>  struct device_type rmi_device_type = {
>  	.name		= "rmi_sensor",
>  	.release	= rmi_release_device,
> @@ -118,7 +61,7 @@ static void rmi_physical_teardown_debugfs(struct rmi_device *rmi_dev)
>  
>  #else
>  
> -static void rmi_physocal_setup_debugfs(struct rmi_device *rmi_dev)
> +static void rmi_physical_setup_debugfs(struct rmi_device *rmi_dev)

This typo makes me wonder how nobody saw this before. This will not
compile without CONFIG_RMI4_DEBUG... :(

>  {
>  }
>  
> @@ -128,7 +71,6 @@ static void rmi_physical_teardown_debugfs(struct rmi_device *rmi_dev)
>  
>  #endif
>  
> -
>  /**
>   * rmi_register_physical_device - register a physical device connection on the RMI
>   * bus.  Physical drivers provide communication from the devices on the bus to
> @@ -174,7 +116,7 @@ int rmi_register_physical_device(struct rmi_phys_device *phys)
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(rmi_register_physical_device);
> +EXPORT_SYMBOL_GPL(rmi_register_physical_device);
>  
>  /**
>   * rmi_unregister_physical_device - unregister a physical device connection
> @@ -191,18 +133,14 @@ void rmi_unregister_physical_device(struct rmi_phys_device *phys)
>  EXPORT_SYMBOL(rmi_unregister_physical_device);
>  
>  
> -/*
> - * RMI Function devices and their handlers
> - */
> +/* Function specific stuff */
>  
>  static void rmi_release_function(struct device *dev)
>  {
>  	struct rmi_function *fn = to_rmi_function(dev);
> -
>  	kfree(fn);
>  }
>  
> -/* Device type for RMI Function devices */
>  struct device_type rmi_function_type = {
>  	.name		= "rmi_function",
>  	.release	= rmi_release_function,
> @@ -244,35 +182,12 @@ static void rmi_function_teardown_debugfs(struct rmi_function *fn)
>  
>  #endif
>  
> -int rmi_register_function(struct rmi_function *fn)
> +static int rmi_function_match(struct device *dev, struct device_driver *drv)
>  {
> -	struct rmi_device *rmi_dev = fn->rmi_dev;
> -	int error;
> -
> -	dev_set_name(&fn->dev, "%s.fn%02x",
> -		     dev_name(&rmi_dev->dev), fn->fd.function_number);
> -
> -	fn->dev.parent = &rmi_dev->dev;
> -	fn->dev.type = &rmi_function_type;
> -	fn->dev.bus = &rmi_bus_type;
> -
> -	error = device_register(&fn->dev);
> -	if (error) {
> -		dev_err(&rmi_dev->dev,
> -			"Failed device_register function device %s\n",
> -			dev_name(&fn->dev));
> -	}
> -
> -	dev_dbg(&rmi_dev->dev, "Registered F%02X.\n", fn->fd.function_number);
> -
> -	rmi_function_setup_debugfs(fn);
> -	return 0;
> -}
> +	struct rmi_function_handler *handler = to_rmi_function_handler(drv);
> +	struct rmi_function *fn = to_rmi_function(dev);
>  
> -void rmi_unregister_function(struct rmi_function *fn)
> -{
> -	rmi_function_teardown_debugfs(fn);
> -	device_unregister(&fn->dev);
> +	return fn->fd.function_number == handler->func;
>  }
>  
>  static int rmi_function_probe(struct device *dev)
> @@ -302,6 +217,38 @@ static int rmi_function_remove(struct device *dev)
>  	return 0;
>  }
>  
> +int rmi_register_function(struct rmi_function *fn)
> +{
> +	struct rmi_device *rmi_dev = fn->rmi_dev;
> +	int error;
> +
> +	dev_set_name(&fn->dev, "%s.fn%02x",
> +		     dev_name(&rmi_dev->dev), fn->fd.function_number);
> +
> +	fn->dev.parent = &rmi_dev->dev;
> +	fn->dev.type = &rmi_function_type;
> +	fn->dev.bus = &rmi_bus_type;
> +
> +	error = device_register(&fn->dev);
> +	if (error) {
> +		dev_err(&rmi_dev->dev,
> +			"Failed device_register function device %s\n",
> +			dev_name(&fn->dev));
> +		return error;

this return statement was not in the previous version of rmi_bus.c, but
it looks obviously correct.

So to sum up:
Reviewed-by: benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> +	}
> +
> +	dev_dbg(&rmi_dev->dev, "Registered F%02X.\n", fn->fd.function_number);
> +
> +	rmi_function_setup_debugfs(fn);
> +	return 0;
> +}
> +
> +void rmi_unregister_function(struct rmi_function *fn)
> +{
> +	rmi_function_teardown_debugfs(fn);
> +	device_unregister(&fn->dev);
> +}
> +
>  /**
>   * rmi_register_function_handler - register a handler for an RMI function
>   * @handler: RMI handler that should be registered.
> @@ -334,7 +281,7 @@ int __rmi_register_function_handler(struct rmi_function_handler *handler,
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(__rmi_register_function_handler);
> +EXPORT_SYMBOL_GPL(__rmi_register_function_handler);
>  
>  /**
>   * rmi_unregister_function_handler - unregister given RMI function handler
> @@ -347,11 +294,55 @@ void rmi_unregister_function_handler(struct rmi_function_handler *handler)
>  {
>  	driver_unregister(&handler->driver);
>  }
> -EXPORT_SYMBOL(rmi_unregister_function_handler);
> +EXPORT_SYMBOL_GPL(rmi_unregister_function_handler);
>  
> -/*
> - * Bus registration and tear-down
> - */
> +/* Bus specific stuff */
> +
> +static int rmi_bus_match(struct device *dev, struct device_driver *drv)
> +{
> +	bool physical = rmi_is_physical_device(dev);
> +
> +	/* First see if types are not compatible */
> +	if (physical != rmi_is_physical_driver(drv))
> +		return 0;
> +
> +	return physical || rmi_function_match(dev, drv);
> +}
> +
> +struct bus_type rmi_bus_type = {
> +	.match		= rmi_bus_match,
> +	.name		= "rmi",
> +};
> +
> +#ifdef CONFIG_RMI4_DEBUG
> +
> +static struct dentry *rmi_debugfs_root;
> +
> +static void rmi_bus_setup_debugfs(void)
> +{
> +	rmi_debugfs_root = debugfs_create_dir(rmi_bus_type.name, NULL);
> +	if (!rmi_debugfs_root)
> +		pr_err("%s: Failed to create debugfs root\n",
> +		       __func__);
> +}
> +
> +static void rmi_bus_teardown_debugfs(void)
> +{
> +	if (rmi_debugfs_root)
> +		debugfs_remove_recursive(rmi_debugfs_root);
> +}
> +
> +#else
> +
> +static void rmi_bus_setup_debugfs(void)
> +{
> +}
> +
> +static void rmi_bus_teardown_debugfs(void)
> +{
> +}
> +
> +#endif
>  
>  static int __init rmi_bus_init(void)
>  {
> 

Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c

[Dmitry Torokhov]

On Mon, Dec 09, 2013 at 03:14:26PM -0500, Benjamin Tissoires wrote:
> Hi Chris,
> 
> On 05/12/13 19:29, Christopher Heiny wrote:
> > This patch implements changes to the synaptics-rmi4 branch of
> > Dmitry's input tree.  The base for the patch is commit
> > 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
> > 
> > This patch primarily reorders the various declarations in rmi_bus.c in order to
> > group related elements together, along with some typo fixes.  The code is still
> > horribly broken, but this change should make the following fixes easier to
> >  review.
> > 
> > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Jean Delvare <khali@linux-fr.org>
> > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > Cc: Joerie de Gram <j.de.gram@gmail.com>
> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > 
> > ---
> 
> FWIW, I made a review of the patch.
> The patches does not only reorder the functions, but also fix some few
> things I will detail later (plus fixes of whitespace/comments issues).
> It also changes the exported functions as GPL.
> 
> Dmitry, given the current state of the driver (which does not work at
> all if I understood correctly), maybe you can pick this one in its
> current state.

Applied, thank you.

-- 
Dmitry

Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c

[Dmitry Torokhov]

On Mon, Dec 09, 2013 at 10:39:14PM -0800, Dmitry Torokhov wrote:
> On Mon, Dec 09, 2013 at 03:14:26PM -0500, Benjamin Tissoires wrote:
> > Hi Chris,
> > 
> > On 05/12/13 19:29, Christopher Heiny wrote:
> > > This patch implements changes to the synaptics-rmi4 branch of
> > > Dmitry's input tree.  The base for the patch is commit
> > > 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
> > > 
> > > This patch primarily reorders the various declarations in rmi_bus.c in order to
> > > group related elements together, along with some typo fixes.  The code is still
> > > horribly broken, but this change should make the following fixes easier to
> > >  review.
> > > 
> > > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Cc: Jean Delvare <khali@linux-fr.org>
> > > Cc: Linus Walleij <linus.walleij@stericsson.com>
> > > Cc: Joerie de Gram <j.de.gram@gmail.com>
> > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > 
> > > ---
> > 
> > FWIW, I made a review of the patch.
> > The patches does not only reorder the functions, but also fix some few
> > things I will detail later (plus fixes of whitespace/comments issues).
> > It also changes the exported functions as GPL.
> > 
> > Dmitry, given the current state of the driver (which does not work at
> > all if I understood correctly), maybe you can pick this one in its
> > current state.
> 
> Applied, thank you.

Well, I had to pull up rmi_debugfs_root declaration to avoid:

  CC [M]  drivers/input/rmi4/rmi_driver.o
drivers/input/rmi4/rmi_bus.c: In function ‘rmi_physical_setup_debugfs’:
drivers/input/rmi4/rmi_bus.c:51:10: error: ‘rmi_debugfs_root’ undeclared
(first use in this function)
          rmi_debugfs_root);

Guys, a bit better compile coverage would be appreciated.

Thanks.

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

Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c

[Benjamin Tissoires]

On Tue, Dec 10, 2013 at 1:46 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Dec 09, 2013 at 10:39:14PM -0800, Dmitry Torokhov wrote:
>> On Mon, Dec 09, 2013 at 03:14:26PM -0500, Benjamin Tissoires wrote:
>> > Hi Chris,
>> >
>> > On 05/12/13 19:29, Christopher Heiny wrote:
>> > > This patch implements changes to the synaptics-rmi4 branch of
>> > > Dmitry's input tree.  The base for the patch is commit
>> > > 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
>> > >
>> > > This patch primarily reorders the various declarations in rmi_bus.c in order to
>> > > group related elements together, along with some typo fixes.  The code is still
>> > > horribly broken, but this change should make the following fixes easier to
>> > >  review.
>> > >
>> > > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
>> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > > Cc: Jean Delvare <khali@linux-fr.org>
>> > > Cc: Linus Walleij <linus.walleij@stericsson.com>
>> > > Cc: Joerie de Gram <j.de.gram@gmail.com>
>> > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> > >
>> > > ---
>> >
>> > FWIW, I made a review of the patch.
>> > The patches does not only reorder the functions, but also fix some few
>> > things I will detail later (plus fixes of whitespace/comments issues).
>> > It also changes the exported functions as GPL.
>> >
>> > Dmitry, given the current state of the driver (which does not work at
>> > all if I understood correctly), maybe you can pick this one in its
>> > current state.
>>
>> Applied, thank you.
>
> Well, I had to pull up rmi_debugfs_root declaration to avoid:
>
>   CC [M]  drivers/input/rmi4/rmi_driver.o
> drivers/input/rmi4/rmi_bus.c: In function ‘rmi_physical_setup_debugfs’:
> drivers/input/rmi4/rmi_bus.c:51:10: error: ‘rmi_debugfs_root’ undeclared
> (first use in this function)
>           rmi_debugfs_root);
>
> Guys, a bit better compile coverage would be appreciated.

oops, sorry for not having spot this one. My mistake.
Thanks for fixing it.

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

Re: [PATCH V2] input synaptics-rmi4: Reorder declarations in rmi_bus.c

[Christopher Heiny]

On 12/09/2013 10:46 PM, Dmitry Torokhov wrote:
> On Mon, Dec 09, 2013 at 10:39:14PM -0800, Dmitry Torokhov wrote:
>> On Mon, Dec 09, 2013 at 03:14:26PM -0500, Benjamin Tissoires wrote:
>>> Hi Chris,
>>>
>>> On 05/12/13 19:29, Christopher Heiny wrote:
>>>> This patch implements changes to the synaptics-rmi4 branch of
>>>> Dmitry's input tree.  The base for the patch is commit
>>>> 8ca01dc61a42b6f7bcba052a8c084000f7057a34.
>>>>
>>>> This patch primarily reorders the various declarations in rmi_bus.c in order to
>>>> group related elements together, along with some typo fixes.  The code is still
>>>> horribly broken, but this change should make the following fixes easier to
>>>>  review.
>>>>
>>>> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
>>>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>>> Cc: Jean Delvare <khali@linux-fr.org>
>>>> Cc: Linus Walleij <linus.walleij@stericsson.com>
>>>> Cc: Joerie de Gram <j.de.gram@gmail.com>
>>>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>>>
>>>> ---
>>>
>>> FWIW, I made a review of the patch.
>>> The patches does not only reorder the functions, but also fix some few
>>> things I will detail later (plus fixes of whitespace/comments issues).
>>> It also changes the exported functions as GPL.
>>>
>>> Dmitry, given the current state of the driver (which does not work at
>>> all if I understood correctly), maybe you can pick this one in its
>>> current state.
>>
>> Applied, thank you.
> 
> Well, I had to pull up rmi_debugfs_root declaration to avoid:
> 
>   CC [M]  drivers/input/rmi4/rmi_driver.o
> drivers/input/rmi4/rmi_bus.c: In function ‘rmi_physical_setup_debugfs’:
> drivers/input/rmi4/rmi_bus.c:51:10: error: ‘rmi_debugfs_root’ undeclared
> (first use in this function)
>           rmi_debugfs_root);
> 
> Guys, a bit better compile coverage would be appreciated.

Um, well, OK.  As mentioned in a previous patch, the code is still
horribly broken, although it's getting better.  I should have included
that previous patch's disclaimer with this one.

There's a bunch of debugfs problems in the branch as it stands, such as
failure to compile in certain configurations, kernel panics in others,
problematic initialization sequence, and so on. I was planning to fix
all that in a separate patch following this one, rather than putting
changes in piece meal, or piggybacking the changes onto another patch.

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