linux-kernel - Re: [PATCH 05/18 v2] regulator: max77686: Pass descriptor instead of GPIO number

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 23 Apr 2018 08:46:11 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>, linux-kernel@...r.kernel.org,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: [PATCH 05/18 v2] regulator: max77686: Pass descriptor instead of
 GPIO number

On Mon, Apr 23, 2018 at 1:07 AM, Linus Walleij <linus.walleij@...aro.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@...sung.com>
> Cc: Krzysztof Kozlowski <krzk@...nel.org>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> Signed-off-by: Linus Walleij <linus.walleij@...aro.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
>