Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CGME20181201154428epcas4p3a1a5c9c576027b1c56f8dd1510b32187@epcas4p3.samsung.com>
 <20181201154151.14890-1-linus.walleij@linaro.org> <d6b5f384-51aa-0e99-7e88-a1debcf5eca3@samsung.com>
In-Reply-To: <d6b5f384-51aa-0e99-7e88-a1debcf5eca3@samsung.com>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Tue, 4 Dec 2018 10:31:19 +0100
Message-ID: <CACRpkdYQ5cy1-8wu--deF0kdzrRGVJ_teNh2LRF7tr9vyh+oHw@mail.gmail.com>
Subject: Re: [PATCH 00/13 v2] Regulator ena_gpiod fixups
To:     Marek Szyprowski <m.szyprowski@samsung.com>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Charles Keepax <ckeepax@opensource.cirrus.com>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Marek,

first, thanks a *lot* for testing this, it is is much, much appreciated!

On Mon, Dec 3, 2018 at 3:35 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:

> The idea is good imho, but it looks that there are some missing cases in
> the code. Here are some logs from the boards I have access to:

OK let's fix this!

> Artik5 evaluation board (arch/arm/boot/dts/exynos3250-artik5-eval.dtb):
> s2mps11-pmic s2mps14-regulator: Failed to get control GPIO for 11/LDO12

Question: this is supposed to fail, right? It is something
like a probe deferral or nonexisting GPIO controller?

I look in the upstream tree:
arch/arm/boot/dts/exynos3250-artik5.dtsi
where s2mps14 is defined:

ldo12_reg: LDO12 {
    /* VDD72 ~ VDD73 */
    regulator-name = "VLDO12_2.8V";
    regulator-min-microvolt = <2800000>;
    regulator-max-microvolt = <2800000>;
    samsung,ext-control-gpios = <&gpk0 2 GPIO_ACTIVE_HIGH>;
};

I didn't really change anything about this, so this missing
GPIO descriptor looks worrysome.

Anyways what happens is this:

gpio[reg] = devm_gpiod_get_from_of_node(...)
if (IS_ERR(gpio[reg]))
(...)
            continue;

So this IS_ERR descriptor is left around. So we should
probably handle erronoeus or NULL descriptors in
gpiod_unhinge().

If you add this on top, does it start working?

diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c
index 5864e758d7f2..e35751bf0ea8 100644
--- a/drivers/gpio/gpiolib-devres.c
+++ b/drivers/gpio/gpiolib-devres.c
@@ -332,6 +332,8 @@ EXPORT_SYMBOL(devm_gpiod_put);

 void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc)
 {
+       if (IS_ERR_OR_NULL(desc))
+               return;
        WARN_ON(devres_destroy(dev, devm_gpiod_release,
                               devm_gpiod_match, desc));
 }

Yours,
Linus Walleij