linux-kernel - Re: [PATCH v3 4/4] regulator: core: Balance coupled regulators voltages

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:   Tue, 12 Dec 2017 11:54:27 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Maciej Purski <m.purski@...sung.com>
Cc:     linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: [PATCH v3 4/4] regulator: core: Balance coupled regulators
 voltages

On Thu, Dec 07, 2017 at 10:46:15AM +0100, Maciej Purski wrote:

> @@ -2447,10 +2482,9 @@ static int _regulator_is_enabled(struct regulator_dev *rdev)
>  	return rdev->desc->ops->is_enabled(rdev);
>  }
>  
> -static int _regulator_list_voltage(struct regulator *regulator,
> -				    unsigned selector, int lock)
> +static int _regulator_list_voltage(struct regulator_dev *rdev,
> +				   unsigned selector, int lock)
>  {

Please split this refactoring of _list_voltage() into a separate patch
for ease of review.  It can go in separately which will make this change
smaller and easier to review.

> @@ -2928,6 +2961,35 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
>  	if (ret < 0)
>  		goto out2;
>  
> +	/*
> +	 * If the regulator is not coupled just set voltage normally, else
> +	 * return after changing consumer demands without changing voltage.
> +	 * This will be handled outside the function
> +	 * by regulator_balance_coupled()
> +	 */
> +	if (!rdev->coupling_desc) {
> +		ret = regulator_set_voltage_rdev(regulator->rdev,
> +						 min_uV, max_uV);
> +		if (ret < 0)
> +			goto out2;
> +	}

As I think I said on the previous version I'm not enthusiastic about
having two separate code paths for setting the voltage, it makes it much
more likely that things will break especially given how rare coupled
regulators are.  It would be cleaner to make uncoupled regulators just
be a special case of coupled regulators, that way more of the code is
shared.  To that end I'd adjust the code so that we always have a
coupling descriptor and then handle the case where there's only one
regulator described in there.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)