linux-kernel - Re: [PATCH] regulator: reorder initialization steps in regulator

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]

Message-ID: <20160412123916.54a0428e@bbrezillon>
Date:	Tue, 12 Apr 2016 12:39:16 +0200
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Mark Brown <broonie@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>
Cc:	linux-kernel@...r.kernel.org, Caesar Wang <wxt@...k-chips.com>,
	Heiko Stübner <heiko@...ech.de>,
	Stephen Barber <smbarber@...omium.org>,
	Douglas Anderson <dianders@...omium.org>,
	Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH] regulator: reorder initialization steps in
 regulator_register()

On Tue, 12 Apr 2016 12:31:00 +0200
Boris Brezillon <boris.brezillon@...e-electrons.com> wrote:

> device_register() is calling ->get_voltage() as part of is sysfs attribute

							  ^ its

> initialization process, and this functions might need to know the regulator
> constraints to return a valid value.
> This is at least true for the pwm regulator driver (when operating in
> continuous mode) which needs to know the minimum and maximum voltage values
> to calculate the current voltage:
> 
> min_uV + (((max_uV - min_uV) * dutycycle) / 100);
> 
> Move device_register() after set_machine_constraints() to make sure those
> constraints are correctly initialized when ->get_voltage() is called.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@...e-electrons.com>
> Reported-by: Stephen Barber <smbarber@...omium.org>
> ---
>  drivers/regulator/core.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index e0b7642..8258568 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3950,13 +3950,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
>  	rdev->dev.parent = dev;
>  	dev_set_name(&rdev->dev, "regulator.%lu",
>  		    (unsigned long) atomic_inc_return(&regulator_no));
> -	ret = device_register(&rdev->dev);
> -	if (ret != 0) {
> -		put_device(&rdev->dev);
> -		goto wash;
> -	}
> -
> -	dev_set_drvdata(&rdev->dev, rdev);
>  
>  	/* set regulator constraints */
>  	if (init_data)
> @@ -3964,7 +3957,15 @@ regulator_register(const struct regulator_desc *regulator_desc,
>  
>  	ret = set_machine_constraints(rdev, constraints);
>  	if (ret < 0)
> -		goto scrub;
> +		goto wash;
> +
> +	ret = device_register(&rdev->dev);
> +	if (ret != 0) {
> +		put_device(&rdev->dev);
> +		goto wash;
> +	}
> +
> +	dev_set_drvdata(&rdev->dev, rdev);
>  
>  	if (init_data && init_data->supply_regulator)
>  		rdev->supply_name = init_data->supply_regulator;
> @@ -3993,8 +3994,6 @@ out:
>  
>  unset_supplies:
>  	unset_regulator_supplies(rdev);
> -
> -scrub:
>  	regulator_ena_gpio_free(rdev);
>  	device_unregister(&rdev->dev);
>  	/* device core frees rdev */
> @@ -4002,6 +4001,7 @@ scrub:
>  	goto out;
>  
>  wash:
> +	kfree(rdev->constraints);
>  	regulator_ena_gpio_free(rdev);
>  clean:
>  	kfree(rdev);



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com