| Message ID | 1362207641-25152-1-git-send-email-broonie@opensource.wolfsonmicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -- 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
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...
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)
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(-)