[PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[Naveen Kumar G]

From: Naveen Kumar Gaddipati <naveen.gaddipati@stericsson.com>

Added the regulator support in ROHM BU21013 touch
panel driver.

Signed-off-by: Naveen Kumar Gaddipati <naveen.gaddipati@stericsson.com>
---
 drivers/input/touchscreen/bu21013_ts.c |   27 +++++++++++++++++++++++++++
 include/linux/input/bu21013.h          |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/bu21013_ts.c b/drivers/input/touchscreen/bu21013_ts.c
index f7fa9ef..531e982 100644
--- a/drivers/input/touchscreen/bu21013_ts.c
+++ b/drivers/input/touchscreen/bu21013_ts.c
@@ -12,6 +12,7 @@
 #include <linux/input.h>
 #include <linux/input/bu21013.h>
 #include <linux/slab.h>
+#include <linux/regulator/consumer.h>
 
 #define PEN_DOWN_INTR	0
 #define MAX_FINGERS	2
@@ -139,6 +140,7 @@
  * @chip: pointer to the touch panel controller
  * @in_dev: pointer to the input device structure
  * @intr_pin: interrupt pin value
+ * @regulator: pointer to the Regulator used for touch screen
  *
  * Touch panel device data structure
  */
@@ -149,6 +151,7 @@ struct bu21013_ts_data {
 	const struct bu21013_platform_device *chip;
 	struct input_dev *in_dev;
 	unsigned int intr_pin;
+	struct regulator *regulator;
 };
 
 /**
@@ -456,6 +459,16 @@ static int __devinit bu21013_probe(struct i2c_client *client,
 	bu21013_data->in_dev = in_dev;
 	bu21013_data->chip = pdata;
 	bu21013_data->client = client;
+	dev_set_name(&client->dev, pdata->name);
+
+	bu21013_data->regulator = regulator_get(&client->dev, "V-TOUCH");
+	if (IS_ERR(bu21013_data->regulator)) {
+		dev_warn(&client->dev, "regulator_get failed\n");
+		bu21013_data->regulator = NULL;
+	}
+	if (bu21013_data->regulator)
+		regulator_enable(bu21013_data->regulator);
+
 	bu21013_data->touch_stopped = false;
 	init_waitqueue_head(&bu21013_data->wait);
 
@@ -514,6 +527,10 @@ err_free_irq:
 err_cs_disable:
 	pdata->cs_dis(pdata->cs_pin);
 err_free_mem:
+	if (bu21013_data->regulator) {
+		regulator_disable(bu21013_data->regulator);
+		regulator_put(bu21013_data->regulator);
+	}
 	input_free_device(in_dev);
 	kfree(bu21013_data);
 
@@ -535,6 +552,10 @@ static int __devexit bu21013_remove(struct i2c_client *client)
 	bu21013_data->chip->cs_dis(bu21013_data->chip->cs_pin);
 
 	input_unregister_device(bu21013_data->in_dev);
+	if (bu21013_data->regulator) {
+		regulator_disable(bu21013_data->regulator);
+		regulator_put(bu21013_data->regulator);
+	}
 	kfree(bu21013_data);
 
 	device_init_wakeup(&client->dev, false);
@@ -561,6 +582,9 @@ static int bu21013_suspend(struct device *dev)
 	else
 		disable_irq(bu21013_data->chip->irq);
 
+	if (bu21013_data->regulator)
+		regulator_disable(bu21013_data->regulator);
+
 	return 0;
 }
 
@@ -577,6 +601,9 @@ static int bu21013_resume(struct device *dev)
 	struct i2c_client *client = bu21013_data->client;
 	int retval;
 
+	if (bu21013_data->regulator)
+		regulator_enable(bu21013_data->regulator);
+
 	retval = bu21013_init_chip(bu21013_data);
 	if (retval < 0) {
 		dev_err(&client->dev, "bu21013 controller config failed\n");
diff --git a/include/linux/input/bu21013.h b/include/linux/input/bu21013.h
index e470d38..ac9ed7b 100644
--- a/include/linux/input/bu21013.h
+++ b/include/linux/input/bu21013.h
@@ -9,6 +9,7 @@
 
 /**
  * struct bu21013_platform_device - Handle the platform data
+ * @name:	name of the touch panel device
  * @cs_en:	pointer to the cs enable function
  * @cs_dis:	pointer to the cs disable function
  * @irq_read_val:    pointer to read the pen irq value function
@@ -26,6 +27,7 @@
  * This is used to handle the platform data
  */
 struct bu21013_platform_device {
+	const char *name;
 	int (*cs_en)(int reset_pin);
 	int (*cs_dis)(int reset_pin);
 	int (*irq_read_val)(void);
-- 
1.7.2.dirty

Re: [PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[Dmitry Torokhov]

Hi Naveen,

On Wed, Jan 12, 2011 at 12:05:26PM +0530, Naveen Kumar G wrote:
> @@ -456,6 +459,16 @@ static int __devinit bu21013_probe(struct i2c_client *client,
>  	bu21013_data->in_dev = in_dev;
>  	bu21013_data->chip = pdata;
>  	bu21013_data->client = client;
> +	dev_set_name(&client->dev, pdata->name);

Hmm, why is this needed?


> +
> +	bu21013_data->regulator = regulator_get(&client->dev, "V-TOUCH");
> +	if (IS_ERR(bu21013_data->regulator)) {
> +		dev_warn(&client->dev, "regulator_get failed\n");
> +		bu21013_data->regulator = NULL;
> +	}
> +	if (bu21013_data->regulator)
> +		regulator_enable(bu21013_data->regulator);
> +

I believe Mark prefers to fail initialization if regulator isn't
found/available and use dummy regulators on platforms that do not have
real one. Mark?

-- 
Dmitry

RE: [PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[Naveen Kumar GADDIPATI]

Hi Dmitry,


>On Wed, Jan 12, 2011 at 12:05:26PM +0530, Naveen Kumar G wrote:
>> @@ -456,6 +459,16 @@ static int __devinit bu21013_probe(struct
>i2c_client *client,
>>  	bu21013_data->in_dev = in_dev;
>>  	bu21013_data->chip = pdata;
>>  	bu21013_data->client = client;
>> +	dev_set_name(&client->dev, pdata->name);
>
>Hmm, why is this needed?
>
Actual name of the client device is 3-005c, which is not clearly understand.
So, in our project line we are setting the device name from platform data.

Thanks & Regards,
Naveen

Re: [PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[Dmitry Torokhov]

On Wed, Jan 12, 2011 at 10:05:07AM +0100, Naveen Kumar GADDIPATI wrote:
> Hi Dmitry,
> 
> 
> >On Wed, Jan 12, 2011 at 12:05:26PM +0530, Naveen Kumar G wrote:
> >> @@ -456,6 +459,16 @@ static int __devinit bu21013_probe(struct
> >i2c_client *client,
> >>  	bu21013_data->in_dev = in_dev;
> >>  	bu21013_data->chip = pdata;
> >>  	bu21013_data->client = client;
> >> +	dev_set_name(&client->dev, pdata->name);
> >
> >Hmm, why is this needed?
> >
> Actual name of the client device is 3-005c, which is not clearly understand.
> So, in our project line we are setting the device name from platform data.
> 

So who/what cares about I2C device name? I do not see how this help
anything, but it does make device's name differ from its sysfs name.

I believe that driver should only alter names of objects they create and
leave other objects alone.

-- 
Dmitry

RE: [PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[Naveen Kumar GADDIPATI]

Hi Dmitry,

>>
>> >On Wed, Jan 12, 2011 at 12:05:26PM +0530, Naveen Kumar G wrote:
>> >> @@ -456,6 +459,16 @@ static int __devinit bu21013_probe(struct
>> >i2c_client *client,
>> >>  	bu21013_data->in_dev = in_dev;
>> >>  	bu21013_data->chip = pdata;
>> >>  	bu21013_data->client = client;
>> >> +	dev_set_name(&client->dev, pdata->name);
>> >
>> >Hmm, why is this needed?
>> >
>> Actual name of the client device is 3-005c, which is not clearly
>understand.
>> So, in our project line we are setting the device name from platform
>data.
>>
>
>So who/what cares about I2C device name? I do not see how this help
>anything, but it does make device's name differ from its sysfs name.
>
>I believe that driver should only alter names of objects they create and
>leave other objects alone.
>

In our regulator driver platform data, we used to differentiate with device name,
which is more readable than the client device name. That is why we are sending it through platform data.
If you are not preferable to use this, we could change it to client device name.

Thanks & Regards,
Naveen

Re: [PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[Mark Brown]

On Wed, Jan 12, 2011 at 11:04:10AM +0100, Naveen Kumar GADDIPATI wrote:
> >> >On Wed, Jan 12, 2011 at 12:05:26PM +0530, Naveen Kumar G wrote:

> >So who/what cares about I2C device name? I do not see how this help
> >anything, but it does make device's name differ from its sysfs name.

> >I believe that driver should only alter names of objects they create and
> >leave other objects alone.

> In our regulator driver platform data, we used to differentiate with device name,
> which is more readable than the client device name. That is why we are sending it through platform data.
> If you are not preferable to use this, we could change it to client device name.

This is not what the regulator API is expecting you to do.  The
regulator API is expecting you to use whatever the device name the
bus/device model ends up giving you by default is.  If you want to
assign other device names for readability or similar reasons that's
another issue which should be taken up with the device core guys.

Re: [PATCH 1/2] input:Regulator support in ROHM BU21013 touch panel

[Mark Brown]

On Wed, Jan 12, 2011 at 12:05:26PM +0530, Naveen Kumar G wrote:

> +	bu21013_data->regulator = regulator_get(&client->dev, "V-TOUCH");
> +	if (IS_ERR(bu21013_data->regulator)) {
> +		dev_warn(&client->dev, "regulator_get failed\n");
> +		bu21013_data->regulator = NULL;
> +	}
> +	if (bu21013_data->regulator)
> +		regulator_enable(bu21013_data->regulator);
> +

If you fail to acquire the regulator just return an error, there is
support in the regulator core for automatically providing a dummy
regulator (including in cases where some regulators are in use) so no
need to replicate this code in each driver or have lots of conditionals
for the regulator.

You're also not checking the result of regulator_enable().

>   */
>  struct bu21013_platform_device {
> +	const char *name;

As Dimitry said you should just use the dev_name() you get by default.