From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Subject: Re: [PATCH 08/21] regulator: max77686: Pass descriptor instead of GPIO number Date: Mon, 12 Feb 2018 14:37:11 +0100 Message-ID: References: <20180212131717.27193-1-linus.walleij@linaro.org> <20180212131717.27193-9-linus.walleij@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: Received: from mail.kernel.org ([198.145.29.99]:50050 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932944AbeBLNhO (ORCPT ); Mon, 12 Feb 2018 08:37:14 -0500 In-Reply-To: <20180212131717.27193-9-linus.walleij@linaro.org> Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Linus Walleij Cc: Liam Girdwood , Mark Brown , linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, Chanwoo Choi , Bartlomiej Zolnierkiewicz On Mon, Feb 12, 2018 at 2:17 PM, Linus Walleij wrote: > Instead of passing a global GPIO number, pass a descriptor looked > up from the device tree configuration node. > > Cc: Chanwoo Choi > Cc: Krzysztof Kozlowski > Cc: Bartlomiej Zolnierkiewicz > Signed-off-by: Linus Walleij > --- > 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 > #include > #include > -#include > -#include > +#include > #include > #include > #include > @@ -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"); This is also for bucks so how about naming it "max77686-regulator"? > + if (IS_ERR(config->ena_gpiod)) > + return PTR_ERR(config->ena_gpiod); This is not equivalent (although I did not look at previous patches in the series). In case of of_get_named_gpio() failure, old code would just set ena_gpio to invalid value and the ena-gpio method would not be used. All other init data from Device Tree would be parsed. In new code, failure of devm_gpiod_get_from_of_node() would cause error of entire regulator_of_get_init_data() therefore using default driver's init data. This is not a huge difference in practice because both are result of bad DT. However discarding entire init data on one property error seems quite drastic. On the other hand, what would happen on PROBE_DEFER? 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 >