* [PATCH] Input: mms114 - Fix regulator enable and disable paths
@ 2013-03-02 7:00 Mark Brown
2013-03-02 8:11 ` Joonyoung Shim
0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2013-03-02 7:00 UTC (permalink / raw)
To: Dmitry Torokhov, Joonyoung Shim; +Cc: linux-input, Mark Brown
When it uses regulators the mms114 driver checks to see if it managed to
acquire regulators and ignores errors. This is not the intended usage and
not great style in general.
Since the driver already refuses to probe if it fails to allocate the
regulators simply make the enable and disable calls unconditional and
add appropriate error handling.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
drivers/input/touchscreen/mms114.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 4a29ddf..bb95c89 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -314,10 +314,21 @@ static int mms114_start(struct mms114_data *data)
struct i2c_client *client = data->client;
int error;
- if (data->core_reg)
- regulator_enable(data->core_reg);
- if (data->io_reg)
- regulator_enable(data->io_reg);
+ error = regulator_enable(data->core_reg);
+ if (error != 0) {
+ dev_err(&client->dev, "Failed to enable avdd: %d\n",
+ error);
+ return error;
+ }
+
+ error = regulator_enable(data->io_reg);
+ if (error != 0) {
+ dev_err(&client->dev, "Failed to enable vdd: %d\n",
+ error);
+ regulator_disable(data->core_reg);
+ return error;
+ }
+
mdelay(MMS114_POWERON_DELAY);
error = mms114_setup_regs(data);
@@ -335,16 +346,20 @@ static int mms114_start(struct mms114_data *data)
static void mms114_stop(struct mms114_data *data)
{
struct i2c_client *client = data->client;
+ int error;
disable_irq(client->irq);
if (data->pdata->cfg_pin)
data->pdata->cfg_pin(false);
- if (data->io_reg)
- regulator_disable(data->io_reg);
- if (data->core_reg)
- regulator_disable(data->core_reg);
+ error = regulator_disable(data->io_reg);
+ if (error != 0)
+ dev_warn(&client->dev, "Failed to disable vdd: %d\n", error);
+
+ error = regulator_disable(data->core_reg);
+ if (error != 0)
+ dev_warn(&client->dev, "Failed to disable avdd: %d\n", error);
}
static int mms114_input_open(struct input_dev *dev)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Input: mms114 - Fix regulator enable and disable paths
2013-03-02 7:00 [PATCH] Input: mms114 - Fix regulator enable and disable paths Mark Brown
@ 2013-03-02 8:11 ` Joonyoung Shim
2013-03-02 8:18 ` Mark Brown
0 siblings, 1 reply; 5+ messages in thread
From: Joonyoung Shim @ 2013-03-02 8:11 UTC (permalink / raw)
To: Mark Brown; +Cc: Dmitry Torokhov, linux-input
Hi Mark,
On 03/02/2013 04:00 PM, Mark Brown wrote:
> When it uses regulators the mms114 driver checks to see if it managed to
> acquire regulators and ignores errors. This is not the intended usage and
> not great style in general.
>
> Since the driver already refuses to probe if it fails to allocate the
> regulators simply make the enable and disable calls unconditional and
> add appropriate error handling.
Right.
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> drivers/input/touchscreen/mms114.c | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 4a29ddf..bb95c89 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -314,10 +314,21 @@ static int mms114_start(struct mms114_data *data)
> struct i2c_client *client = data->client;
> int error;
>
> - if (data->core_reg)
> - regulator_enable(data->core_reg);
> - if (data->io_reg)
> - regulator_enable(data->io_reg);
> + error = regulator_enable(data->core_reg);
> + if (error != 0) {
> + dev_err(&client->dev, "Failed to enable avdd: %d\n",
> + error);
Could you make this to one line?
> + return error;
> + }
> +
> + error = regulator_enable(data->io_reg);
> + if (error != 0) {
> + dev_err(&client->dev, "Failed to enable vdd: %d\n",
> + error);
Ditto.
> + regulator_disable(data->core_reg);
> + return error;
> + }
> +
> mdelay(MMS114_POWERON_DELAY);
>
> error = mms114_setup_regs(data);
If error, we will have to disable regulators here.
> @@ -335,16 +346,20 @@ static int mms114_start(struct mms114_data *data)
> static void mms114_stop(struct mms114_data *data)
> {
> struct i2c_client *client = data->client;
> + int error;
>
> disable_irq(client->irq);
>
> if (data->pdata->cfg_pin)
> data->pdata->cfg_pin(false);
>
> - if (data->io_reg)
> - regulator_disable(data->io_reg);
> - if (data->core_reg)
> - regulator_disable(data->core_reg);
> + error = regulator_disable(data->io_reg);
> + if (error != 0)
> + dev_warn(&client->dev, "Failed to disable vdd: %d\n", error);
> +
> + error = regulator_disable(data->core_reg);
> + if (error != 0)
> + dev_warn(&client->dev, "Failed to disable avdd: %d\n", error);
> }
>
> static int mms114_input_open(struct input_dev *dev)
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Input: mms114 - Fix regulator enable and disable paths
2013-03-02 8:11 ` Joonyoung Shim
@ 2013-03-02 8:18 ` Mark Brown
0 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2013-03-02 8:18 UTC (permalink / raw)
To: Joonyoung Shim; +Cc: Dmitry Torokhov, linux-input
[-- Attachment #1: Type: text/plain, Size: 280 bytes --]
On Sat, Mar 02, 2013 at 05:11:00PM +0900, Joonyoung Shim wrote:
> On 03/02/2013 04:00 PM, Mark Brown wrote:
> > mdelay(MMS114_POWERON_DELAY);
> > error = mms114_setup_regs(data);
> If error, we will have to disable regulators here.
That's a preexisting error, of course...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Input: mms114 - Fix regulator enable and disable paths
@ 2013-03-02 8:20 Mark Brown
2013-03-02 8:32 ` Joonyoung Shim
0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2013-03-02 8:20 UTC (permalink / raw)
To: Joonyoung Shim, Dmitry Torokhov; +Cc: linux-input, Mark Brown
When it uses regulators the mms114 driver checks to see if it managed to
acquire regulators and ignores errors. This is not the intended usage and
not great style in general.
Since the driver already refuses to probe if it fails to allocate the
regulators simply make the enable and disable calls unconditional and
add appropriate error handling, including adding cleanup of the
regulators if setup_reg() fails.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
drivers/input/touchscreen/mms114.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 4a29ddf..662e34b 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -314,15 +314,27 @@ static int mms114_start(struct mms114_data *data)
struct i2c_client *client = data->client;
int error;
- if (data->core_reg)
- regulator_enable(data->core_reg);
- if (data->io_reg)
- regulator_enable(data->io_reg);
+ error = regulator_enable(data->core_reg);
+ if (error != 0) {
+ dev_err(&client->dev, "Failed to enable avdd: %d\n", error);
+ return error;
+ }
+
+ error = regulator_enable(data->io_reg);
+ if (error != 0) {
+ dev_err(&client->dev, "Failed to enable vdd: %d\n", error);
+ regulator_disable(data->core_reg);
+ return error;
+ }
+
mdelay(MMS114_POWERON_DELAY);
error = mms114_setup_regs(data);
- if (error < 0)
+ if (error < 0) {
+ regulator_disable(data->io_reg);
+ regulator_disable(data->core_reg);
return error;
+ }
if (data->pdata->cfg_pin)
data->pdata->cfg_pin(true);
@@ -335,16 +347,20 @@ static int mms114_start(struct mms114_data *data)
static void mms114_stop(struct mms114_data *data)
{
struct i2c_client *client = data->client;
+ int error;
disable_irq(client->irq);
if (data->pdata->cfg_pin)
data->pdata->cfg_pin(false);
- if (data->io_reg)
- regulator_disable(data->io_reg);
- if (data->core_reg)
- regulator_disable(data->core_reg);
+ error = regulator_disable(data->io_reg);
+ if (error != 0)
+ dev_warn(&client->dev, "Failed to disable vdd: %d\n", error);
+
+ error = regulator_disable(data->core_reg);
+ if (error != 0)
+ dev_warn(&client->dev, "Failed to disable avdd: %d\n", error);
}
static int mms114_input_open(struct input_dev *dev)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Input: mms114 - Fix regulator enable and disable paths
2013-03-02 8:20 Mark Brown
@ 2013-03-02 8:32 ` Joonyoung Shim
0 siblings, 0 replies; 5+ messages in thread
From: Joonyoung Shim @ 2013-03-02 8:32 UTC (permalink / raw)
To: Mark Brown; +Cc: Dmitry Torokhov, linux-input
On 03/02/2013 05:20 PM, Mark Brown wrote:
> When it uses regulators the mms114 driver checks to see if it managed to
> acquire regulators and ignores errors. This is not the intended usage and
> not great style in general.
>
> Since the driver already refuses to probe if it fails to allocate the
> regulators simply make the enable and disable calls unconditional and
> add appropriate error handling, including adding cleanup of the
> regulators if setup_reg() fails.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> drivers/input/touchscreen/mms114.c | 34 +++++++++++++++++++++++++---------
> 1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 4a29ddf..662e34b 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -314,15 +314,27 @@ static int mms114_start(struct mms114_data *data)
> struct i2c_client *client = data->client;
> int error;
>
> - if (data->core_reg)
> - regulator_enable(data->core_reg);
> - if (data->io_reg)
> - regulator_enable(data->io_reg);
> + error = regulator_enable(data->core_reg);
> + if (error != 0) {
> + dev_err(&client->dev, "Failed to enable avdd: %d\n", error);
> + return error;
> + }
> +
> + error = regulator_enable(data->io_reg);
> + if (error != 0) {
> + dev_err(&client->dev, "Failed to enable vdd: %d\n", error);
> + regulator_disable(data->core_reg);
> + return error;
> + }
> +
> mdelay(MMS114_POWERON_DELAY);
>
> error = mms114_setup_regs(data);
> - if (error < 0)
> + if (error < 0) {
> + regulator_disable(data->io_reg);
> + regulator_disable(data->core_reg);
> return error;
> + }
>
> if (data->pdata->cfg_pin)
> data->pdata->cfg_pin(true);
> @@ -335,16 +347,20 @@ static int mms114_start(struct mms114_data *data)
> static void mms114_stop(struct mms114_data *data)
> {
> struct i2c_client *client = data->client;
> + int error;
>
> disable_irq(client->irq);
>
> if (data->pdata->cfg_pin)
> data->pdata->cfg_pin(false);
>
> - if (data->io_reg)
> - regulator_disable(data->io_reg);
> - if (data->core_reg)
> - regulator_disable(data->core_reg);
> + error = regulator_disable(data->io_reg);
> + if (error != 0)
> + dev_warn(&client->dev, "Failed to disable vdd: %d\n", error);
> +
> + error = regulator_disable(data->core_reg);
> + if (error != 0)
> + dev_warn(&client->dev, "Failed to disable avdd: %d\n", error);
> }
>
> static int mms114_input_open(struct input_dev *dev)
Thanks for fixing it.
Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-03-02 8:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-02 7:00 [PATCH] Input: mms114 - Fix regulator enable and disable paths Mark Brown
2013-03-02 8:11 ` Joonyoung Shim
2013-03-02 8:18 ` Mark Brown
-- strict thread matches above, loose matches on Subject: below --
2013-03-02 8:20 Mark Brown
2013-03-02 8:32 ` Joonyoung Shim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).