linux-kernel - Re: [PATCH 06/15 v3] regulator: max8973: Let core handle GPIO descriptor

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 5 Dec 2018 13:43:45 +0000
From:   Charles Keepax <ckeepax@...nsource.cirrus.com>
To:     Linus Walleij <linus.walleij@...aro.org>
CC:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        <linux-kernel@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH 06/15 v3] regulator: max8973: Let core handle GPIO
 descriptor

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@...aro.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