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;
Date:   Wed, 5 Dec 2018 13:43:45 +0000
From:   Charles Keepax <ckeepax@opensource.cirrus.com>
To:     Linus Walleij <linus.walleij@linaro.org>
CC:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        <linux-kernel@vger.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH 06/15 v3] regulator: max8973: Let core handle GPIO
 descriptor
Message-ID: <20181205134345.GF16508@imbe.wolfsonmicro.main>
References: <20181205124721.26624-1-linus.walleij@linaro.org>
 <20181205124721.26624-7-linus.walleij@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20181205124721.26624-7-linus.walleij@linaro.org>
User-Agent: Mutt/1.5.20 (2009-12-10)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Dec 05, 2018 at 01:47:12PM +0100, Linus Walleij wrote:
> Use the gpiod_get() rather than the devm_* version so that the
> regulator core can handle the lifecycle of these descriptors.
> 
> Fixes: e7d2be696faa ("regulator: max8973: Pass descriptor instead of GPIO number")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - Resending.
> ChangeLog v1->v2:
> - Drop the gpiod_put() on the errorpath after devm_regulator_register()
>   as this will be handled by the regulator core.
> - Fix the second case of devm_gpiod_get()
> - Put a comment in the code so maintainers knows not to
>   use managed resources (devm*)
> ---
> @@ -775,10 +779,13 @@ static int max8973_probe(struct i2c_client *client,
>  		/*
>  		 * We do not let the core switch this regulator on/off,
>  		 * we just leave it on.
> +		 *
> +		 * Do not use devm* here: the regulator core takes over the
> +		 * lifecycle management of the GPIO descriptor.
>  		 */
> -		gpiod = devm_gpiod_get_optional(&client->dev,
> -						"maxim,enable",
> -						GPIOD_OUT_HIGH);
> +		gpiod = gpiod_get_optional(&client->dev,
> +					   "maxim,enable",
> +					   GPIOD_OUT_HIGH);

My comment on v2 still stands here, the GPIO is not passed to
the regulator core on this path. Very sorry it took me so long
to review v2 (been one of those wonderful weeks at this end)
and then I managed to perfectly time reviewing it to the exact
second you sent v3.

I think apart from this the series looks good to me though.

Thanks,
Charles