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

[Mark Brown <broonie@xxxxxxxxxx>]

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.

Attachment: signature.asc
Description: PGP signature