Messages in this thread | | | Date | Sun, 2 Aug 2009 19:49:52 +0200 | Subject | Re: [PATCH v3] regulator: Add GPIO enable control to fixed voltage regulator driver | From | pHilipp Zabel <> |
| |
On Sun, Aug 2, 2009 at 6:56 PM, Roger Quadros<quadros.roger@gmail.com> wrote: > From: Roger Quadros <ext-roger.quadros@nokia.com> > > Now fixed regulators that have their enable pin connected to a GPIO line > can use the fixed regulator driver for regulator enable/disable control. > The GPIO number and polarity information is passed through platform data. > GPIO enable control is achieved using gpiolib. > > Signed-off-by: Roger Quadros <ext-roger.quadros@nokia.com> > --- > drivers/regulator/fixed.c | 75 +++++++++++++++++++++++++++++++++++++- > include/linux/regulator/fixed.h | 21 +++++++++++ > 2 files changed, 94 insertions(+), 2 deletions(-) > > diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c > index cdc674f..0888cb7 100644 > --- a/drivers/regulator/fixed.c > +++ b/drivers/regulator/fixed.c [...] > @@ -85,12 +111,51 @@ static int regulator_fixed_voltage_probe(struct platform_device *pdev) > drvdata->desc.n_voltages = 1; > > drvdata->microvolts = config->microvolts; > + drvdata->gpio = config->gpio; > + > + if (gpio_is_valid(config->gpio)) { > + drvdata->enable_high = config->enable_high; > + > + /* FIXME: Remove this print warning */ > + if (!config->gpio) > + dev_warn(&pdev->dev, > + "using GPIO 0 for regulator enable control\n"); > + > + ret = gpio_request(config->gpio, config->supply_name); > + if (ret) { > + dev_err(&pdev->dev, > + "Could not obtain regulator enable GPIO %d: %d\n", > + config->gpio, ret); > + goto err_name; > + } > + > + /* set output direction without changing state > + * to prevent glitch > + */ > + drvdata->is_enabled = config->enabled_at_boot; > + if (!config->enable_high) > + drvdata->is_enabled = !drvdata->is_enabled;
Assume .enabled_at_boot = 1, .enable_high = 0. In this case we end up with .is_enabled = 0, which does not represent the real state after the following call:
> + ret = gpio_direction_output(config->gpio, drvdata->is_enabled);
Maybe use a local variable here or (drvdata->is_enabled ? config->enable_high : !config->enable_high).
regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |