[PATCH] input: synaptics-rmi4 - cleanup rmi_i2c_probe()

[PATCH] input: synaptics-rmi4 - cleanup rmi_i2c_probe()

[Christopher Heiny]

Moves i2c_check_functionality to occur before the gpio_config() call.  This 
can catch some issues that would otherwise result in mysterious problems
in gpio_config().

Reduces debugging output; updates remaining output to be more accurate.

Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

 drivers/input/rmi4/rmi_i2c.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index 43b0e53..b4bd8ae 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -274,26 +274,24 @@ static int rmi_i2c_probe(struct i2c_client *client,
 		dev_err(&client->dev, "no platform data\n");
 		return -EINVAL;
 	}
-	dev_info(&client->dev, "Probing %s at %#02x (IRQ %d).\n",
+	dev_dbg(&client->dev, "Probing %s at %#02x (GPIO %d).\n",
 		pdata->sensor_name ? pdata->sensor_name : "-no name-",
 		client->addr, pdata->attn_gpio);
 
+	retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
+	if (!retval) {
+		dev_err(&client->dev, "i2c_check_functionality error %d.\n",
+			retval);
+		return retval;
+	}
+
 	if (pdata->gpio_config) {
-		dev_dbg(&client->dev, "Configuring GPIOs.\n");
 		retval = pdata->gpio_config(pdata->gpio_data, true);
 		if (retval < 0) {
 			dev_err(&client->dev, "Failed to configure GPIOs, code: %d.\n",
 				retval);
 			return retval;
 		}
-		dev_info(&client->dev, "Done with GPIO configuration.\n");
-	}
-
-	retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
-	if (!retval) {
-		dev_err(&client->dev, "i2c_check_functionality error %d.\n",
-			retval);
-		return retval;
 	}
 
 	xport = devm_kzalloc(&client->dev, sizeof(struct rmi_transport_dev),

Re: [PATCH] input: synaptics-rmi4 - cleanup rmi_i2c_probe()

[Dmitry Torokhov]

On Wed, Jan 08, 2014 at 02:11:32PM -0800, Christopher Heiny wrote:
> Moves i2c_check_functionality to occur before the gpio_config() call.  This 
> can catch some issues that would otherwise result in mysterious problems
> in gpio_config().
> 
> Reduces debugging output; updates remaining output to be more accurate.

What kind of gpio config is there? Did not we add proper configuration
of attn_gpio to the core? Do you really need these callbacks? They will
hurt you when you will try to move to devicetree-based setups.

Thanks.

-- 
Dmitry

Re: [PATCH] input: synaptics-rmi4 - cleanup rmi_i2c_probe()

[Christopher Heiny]

On 01/08/2014 02:46 PM, Dmitry Torokhov wrote:
> On Wed, Jan 08, 2014 at 02:11:32PM -0800, Christopher Heiny wrote:
>> Moves i2c_check_functionality to occur before the gpio_config() call.  This
>> can catch some issues that would otherwise result in mysterious problems
>> in gpio_config().
>>
>> Reduces debugging output; updates remaining output to be more accurate.
>
> What kind of gpio config is there? Did not we add proper configuration
> of attn_gpio to the core? Do you really need these callbacks? They will
> hurt you when you will try to move to devicetree-based setups.

This is a gpio_config() does the platform specific gpio setup (power 
configuration, voltage levels, hardware reset lines, and so on).  This 
is used on every production platform using the driver that I know of, 
and the setup is different on every platform (sometimes even between 
revs of that platform).  We'd go crazy trying to handle that in 
rmi_i2c.c or rmi_driver.c, so it's pushed to the platform.

And yeah, I figure it'll hurt when we move to devicetree.  Right now 
we're just trying to get the current state of the code in the 
synaptics-rmi4 branch from "horribly broken" to "it lives!".  We're 
willing to put off that devicetree pain in order to get the code on its 
feet.

Re: [PATCH] input: synaptics-rmi4 - cleanup rmi_i2c_probe()

[Dmitry Torokhov]

On Wed, Jan 08, 2014 at 02:11:32PM -0800, Christopher Heiny wrote:
> Moves i2c_check_functionality to occur before the gpio_config() call.  This 
> can catch some issues that would otherwise result in mysterious problems
> in gpio_config().
> 
> Reduces debugging output; updates remaining output to be more accurate.
> 
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Applied, thank you.

> 
> ---
> 
>  drivers/input/rmi4/rmi_i2c.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index 43b0e53..b4bd8ae 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -274,26 +274,24 @@ static int rmi_i2c_probe(struct i2c_client *client,
>  		dev_err(&client->dev, "no platform data\n");
>  		return -EINVAL;
>  	}
> -	dev_info(&client->dev, "Probing %s at %#02x (IRQ %d).\n",
> +	dev_dbg(&client->dev, "Probing %s at %#02x (GPIO %d).\n",
>  		pdata->sensor_name ? pdata->sensor_name : "-no name-",
>  		client->addr, pdata->attn_gpio);
>  
> +	retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
> +	if (!retval) {
> +		dev_err(&client->dev, "i2c_check_functionality error %d.\n",
> +			retval);
> +		return retval;
> +	}
> +
>  	if (pdata->gpio_config) {
> -		dev_dbg(&client->dev, "Configuring GPIOs.\n");
>  		retval = pdata->gpio_config(pdata->gpio_data, true);
>  		if (retval < 0) {
>  			dev_err(&client->dev, "Failed to configure GPIOs, code: %d.\n",
>  				retval);
>  			return retval;
>  		}
> -		dev_info(&client->dev, "Done with GPIO configuration.\n");
> -	}
> -
> -	retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
> -	if (!retval) {
> -		dev_err(&client->dev, "i2c_check_functionality error %d.\n",
> -			retval);
> -		return retval;
>  	}
>  
>  	xport = devm_kzalloc(&client->dev, sizeof(struct rmi_transport_dev),

-- 
Dmitry