| Message ID | 20180422230742.3729-5-linus.walleij@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/18,v2] regulator: fixed: Convert to use GPIO descriptor only | expand |
On Mon, Apr 23, 2018 at 1:07 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > Instead of passing a global GPIO number, pass a descriptor looked > up from the device tree configuration node. > > Cc: Chanwoo Choi <cw00.choi@samsung.com> > Cc: Krzysztof Kozlowski <krzk@kernel.org> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - Rebase the patch on the other changes. > --- > drivers/regulator/max77686-regulator.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/regulator/max77686-regulator.c b/drivers/regulator/max77686-regulator.c > index c301f3733475..5ebd06f47e6a 100644 > --- a/drivers/regulator/max77686-regulator.c > +++ b/drivers/regulator/max77686-regulator.c > @@ -25,8 +25,7 @@ > #include <linux/kernel.h> > #include <linux/bug.h> > #include <linux/err.h> > -#include <linux/gpio.h> > -#include <linux/of_gpio.h> > +#include <linux/gpio/consumer.h> > #include <linux/slab.h> > #include <linux/platform_device.h> > #include <linux/regulator/driver.h> > @@ -90,6 +89,7 @@ enum max77686_ramp_rate { > }; > > struct max77686_data { > + struct device *dev; > DECLARE_BITMAP(gpio_enabled, MAX77686_REGULATORS); > > /* Array indexed by regulator id */ > @@ -269,16 +269,20 @@ static int max77686_of_parse_cb(struct device_node *np, > case MAX77686_BUCK8: > case MAX77686_BUCK9: > case MAX77686_LDO20 ... MAX77686_LDO22: > - config->ena_gpio = of_get_named_gpio(np, > - "maxim,ena-gpios", 0); > - config->ena_gpio_flags = GPIOF_OUT_INIT_HIGH; > - config->ena_gpio_initialized = true; > + config->ena_gpiod = devm_gpiod_get_from_of_node(max77686->dev, > + np, > + "maxim,ena", > + 0, > + GPIOD_OUT_HIGH, > + "max77686-LDO"); > + if (IS_ERR(config->ena_gpiod)) > + return PTR_ERR(config->ena_gpiod); Both my previous comments remain not addressed: 1. Name: This is also for bucks so how about naming it "max77686-regulator"? 2. Error path here is not equivalent to old code and results in using default driver's init data. Instead I would prefer to just ignore the gpio. Best regards, Krzysztof > break; > default: > return 0; > } > > - if (gpio_is_valid(config->ena_gpio)) { > + if (config->ena_gpiod) { > set_bit(desc->id, max77686->gpio_enabled); > > return regmap_update_bits(config->regmap, desc->enable_reg, > @@ -521,6 +525,7 @@ static int max77686_pmic_probe(struct platform_device *pdev) > if (!max77686) > return -ENOMEM; > > + max77686->dev = &pdev->dev; > config.dev = iodev->dev; > config.regmap = iodev->regmap; > config.driver_data = max77686; > -- > 2.14.3 >
On Mon, Apr 23, 2018 at 8:46 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote: > Both my previous comments remain not addressed: > 1. Name: This is also for bucks so how about naming it "max77686-regulator"? > 2. Error path here is not equivalent to old code and results in using > default driver's init data. Instead I would prefer to just ignore the > gpio. Sorry for missing these :( I fixed it up for v3. Yours, Linus Walleij
diff --git a/drivers/regulator/max77686-regulator.c b/drivers/regulator/max77686-regulator.c index c301f3733475..5ebd06f47e6a 100644 --- a/drivers/regulator/max77686-regulator.c +++ b/drivers/regulator/max77686-regulator.c @@ -25,8 +25,7 @@ #include <linux/kernel.h> #include <linux/bug.h> #include <linux/err.h> -#include <linux/gpio.h> -#include <linux/of_gpio.h> +#include <linux/gpio/consumer.h> #include <linux/slab.h> #include <linux/platform_device.h> #include <linux/regulator/driver.h> @@ -90,6 +89,7 @@ enum max77686_ramp_rate { }; struct max77686_data { + struct device *dev; DECLARE_BITMAP(gpio_enabled, MAX77686_REGULATORS); /* Array indexed by regulator id */ @@ -269,16 +269,20 @@ static int max77686_of_parse_cb(struct device_node *np, case MAX77686_BUCK8: case MAX77686_BUCK9: case MAX77686_LDO20 ... MAX77686_LDO22: - config->ena_gpio = of_get_named_gpio(np, - "maxim,ena-gpios", 0); - config->ena_gpio_flags = GPIOF_OUT_INIT_HIGH; - config->ena_gpio_initialized = true; + config->ena_gpiod = devm_gpiod_get_from_of_node(max77686->dev, + np, + "maxim,ena", + 0, + GPIOD_OUT_HIGH, + "max77686-LDO"); + if (IS_ERR(config->ena_gpiod)) + return PTR_ERR(config->ena_gpiod); break; default: return 0; } - if (gpio_is_valid(config->ena_gpio)) { + if (config->ena_gpiod) { set_bit(desc->id, max77686->gpio_enabled); return regmap_update_bits(config->regmap, desc->enable_reg, @@ -521,6 +525,7 @@ static int max77686_pmic_probe(struct platform_device *pdev) if (!max77686) return -ENOMEM; + max77686->dev = &pdev->dev; config.dev = iodev->dev; config.regmap = iodev->regmap; config.driver_data = max77686;
Instead of passing a global GPIO number, pass a descriptor looked up from the device tree configuration node. Cc: Chanwoo Choi <cw00.choi@samsung.com> Cc: Krzysztof Kozlowski <krzk@kernel.org> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - Rebase the patch on the other changes. --- drivers/regulator/max77686-regulator.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) -- 2.14.3