linux-kernel - Re: [RFC PATCH v2 2/3] regulator: core: Parse coupled regulators properties

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]

Message-ID: <20171129162541.dfmed64koyu3ihls@sirena.org.uk>
Date:   Wed, 29 Nov 2017 16:25:41 +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: [RFC PATCH v2 2/3] regulator: core: Parse coupled regulators
 properties

On Wed, Oct 18, 2017 at 02:47:01PM +0200, Maciej Purski wrote:

This looks mostly good, a few things below that I think should be
relatively easy to addresss.

> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3953,6 +3953,198 @@ static int regulator_register_resolve_supply(struct device *dev, void *data)
>  	return 0;
>  }
>  
> +/* Function returns regulator coupled with the given regulator_dev */
> +static struct regulator_dev *parse_coupled_regulator(struct regulator_dev *rdev,
> +						     int index)
> +{
> +	struct device_node *node = rdev->dev.of_node;

This is DT parsing code directly in the regulator core - we need to work
with other firmware interfaces and with board files.  This code should
be in of_regulator.c to ensure that the infrastructure works for non-DT
users.  The code's pretty much there, it mostly needs moving around.
The coupling voltage needs to go into the constraints data.

> +		/* Regulator can't be found */
> +		if (j == n_coupled && tmp_rdev != c_rdev)
> +			return -1;

Please return error codes and print error messages, it'll help users
debug stuff.

> @@ -4116,6 +4308,8 @@ regulator_register(const struct regulator_desc *regulator_desc,
>  	dev_set_drvdata(&rdev->dev, rdev);
>  	rdev_init_debugfs(rdev);
>  
> +	regulator_resolve_coupling(rdev);
> +
>  	/* try to resolve regulators supply since a new one was registered */
>  	class_for_each_device(&regulator_class, NULL, NULL,
>  			      regulator_register_resolve_supply);
> @@ -4129,6 +4323,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
>  wash:
>  	kfree(rdev->constraints);
>  	mutex_lock(&regulator_list_mutex);
> +	regulator_clean_coupling(rdev);
>  	regulator_ena_gpio_free(rdev);
>  	mutex_unlock(&regulator_list_mutex);
>  clean:

Shouldn't we be resolving the coupling while we hold the list lock?
We're still working with multiple regulators at this point.

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