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

[Mark Brown]

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.

signature.asc
Description: PGP signature