Hi Sascha,
On 23-09-20, Sascha Hauer wrote:
> Each struct regulator_dev instance has a struct regulator_internal
> associated with it. The idea was that core internal fields are seen
> by the core only. In the end this is more confusing than helpful. We
> have a ri->rdev link, but no rdev->ri link so that we can't get a
> rdev from a ri pointer. We could add that link, but instead make the
> whole stuff a bit easier by just merging everything from struct
> regulator_internal to struct regulator_dev.
>
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> ---
> drivers/regulator/core.c | 214 ++++++++++++++++++---------------------
> include/regulator.h | 9 ++
> 2 files changed, 106 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index e9b5b82dbe..41a3378ac8 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -13,21 +13,8 @@
>
> static LIST_HEAD(regulator_list);
>
> -struct regulator_internal {
> - struct list_head list;
> - struct device_node *node;
> - struct regulator_dev *rdev;
> - int enable_count;
> - int enable_time_us;
> - int min_uv;
> - int max_uv;
> - char *name;
> - const char *supply;
> - struct list_head consumer_list;
> -};
> -
> struct regulator {
> - struct regulator_internal *ri;
> + struct regulator_dev *rdev;
> struct list_head list;
> struct device *dev;
> };
> @@ -41,13 +28,12 @@ static int regulator_map_voltage(struct regulator_dev *rdev, int min_uV,
> return -ENOSYS;
> }
>
> -static int regulator_enable_internal(struct regulator_internal *ri)
> +static int regulator_enable_internal(struct regulator_dev *rdev)
> {
> - struct regulator_dev *rdev = ri->rdev;
> int ret;
>
> - if (ri->enable_count) {
> - ri->enable_count++;
> + if (rdev->enable_count) {
> + rdev->enable_count++;
> return 0;
> }
>
> @@ -59,30 +45,29 @@ static int regulator_enable_internal(struct regulator_internal *ri)
> if (ret)
> return ret;
>
> - ret = rdev->desc->ops->enable(ri->rdev);
> + ret = rdev->desc->ops->enable(rdev);
> if (ret) {
> regulator_disable(rdev->supply);
> return ret;
> }
>
> - if (ri->enable_time_us)
> - udelay(ri->enable_time_us);
> + if (rdev->enable_time_us)
> + udelay(rdev->enable_time_us);
regualtor_dev does not have a 'enable_time_us' or do I miss something?
Regards,
Marco
> - ri->enable_count++;
> + rdev->enable_count++;
>
> return 0;
> }
>
> -static int regulator_disable_internal(struct regulator_internal *ri)
> +static int regulator_disable_internal(struct regulator_dev *rdev)
> {
> - struct regulator_dev *rdev = ri->rdev;
> int ret;
>
> - if (!ri->enable_count)
> + if (!rdev->enable_count)
> return -EINVAL;
>
> - if (ri->enable_count > 1) {
> - ri->enable_count--;
> + if (rdev->enable_count > 1) {
> + rdev->enable_count--;
> return 0;
> }
>
> @@ -97,15 +82,14 @@ static int regulator_disable_internal(struct regulator_internal *ri)
> if (ret)
> return ret;
>
> - ri->enable_count--;
> + rdev->enable_count--;
>
> - return regulator_disable(ri->rdev->supply);
> + return regulator_disable(rdev->supply);
> }
>
> -static int regulator_set_voltage_internal(struct regulator_internal *ri,
> +static int regulator_set_voltage_internal(struct regulator_dev *rdev,
> int min_uV, int max_uV)
> {
> - struct regulator_dev *rdev = ri->rdev;
> const struct regulator_ops *ops = rdev->desc->ops;
> unsigned int selector;
> int best_val = 0;
> @@ -166,38 +150,33 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
> return 0;
> }
>
> -static struct regulator_internal * __regulator_register(struct regulator_dev *rdev, const char *name)
> +static int __regulator_register(struct regulator_dev *rdev, const char *name)
> {
> - struct regulator_internal *ri;
> int ret;
>
> - ri = xzalloc(sizeof(*ri));
> - ri->rdev = rdev;
> + INIT_LIST_HEAD(&rdev->consumer_list);
>
> - INIT_LIST_HEAD(&ri->consumer_list);
> -
> - list_add_tail(&ri->list, ®ulator_list);
> + list_add_tail(&rdev->list, ®ulator_list);
>
> if (name)
> - ri->name = xstrdup(name);
> + rdev->name = xstrdup(name);
>
> if (rdev->boot_on || rdev->always_on) {
> - ret = regulator_resolve_supply(ri->rdev);
> + ret = regulator_resolve_supply(rdev);
> if (ret < 0)
> goto err;
>
> - ret = regulator_enable_internal(ri);
> + ret = regulator_enable_internal(rdev);
> if (ret && ret != -ENOSYS)
> goto err;
> }
>
> - return ri;
> + return 0;
> err:
> - list_del(&ri->list);
> - free(ri->name);
> - free(ri);
> + list_del(&rdev->list);
> + free((char *)rdev->name);
>
> - return ERR_PTR(ret);
> + return ret;
> }
>
>
> @@ -211,8 +190,8 @@ static struct regulator_internal * __regulator_register(struct regulator_dev *rd
> */
> int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
> {
> - struct regulator_internal *ri;
> const char *name;
> + int ret;
>
> if (!rdev || !node)
> return -EINVAL;
> @@ -224,34 +203,34 @@ int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
> if (!name)
> name = node->name;
>
> - ri = __regulator_register(rdev, name);
> - if (IS_ERR(ri))
> - return PTR_ERR(ri);
> + ret = __regulator_register(rdev, name);
> + if (ret)
> + return ret;
>
> - ri->node = node;
> + rdev->node = node;
> node->dev = rdev->dev;
>
> if (rdev->desc->off_on_delay)
> - ri->enable_time_us = rdev->desc->off_on_delay;
> + rdev->enable_time_us = rdev->desc->off_on_delay;
>
> if (rdev->desc->fixed_uV && rdev->desc->n_voltages == 1)
> - ri->min_uv = ri->max_uv = rdev->desc->fixed_uV;
> + rdev->min_uv = rdev->max_uv = rdev->desc->fixed_uV;
>
> of_property_read_u32(node, "regulator-enable-ramp-delay",
> - &ri->enable_time_us);
> + &rdev->enable_time_us);
> of_property_read_u32(node, "regulator-min-microvolt",
> - &ri->min_uv);
> + &rdev->min_uv);
> of_property_read_u32(node, "regulator-max-microvolt",
> - &ri->max_uv);
> + &rdev->max_uv);
>
> return 0;
> }
>
> -static struct regulator_internal *of_regulator_get(struct device *dev,
> +static struct regulator_dev *of_regulator_get(struct device *dev,
> const char *supply)
> {
> char *propname;
> - struct regulator_internal *ri;
> + struct regulator_dev *rdev;
> struct device_node *node, *node_parent;
> int ret;
>
> @@ -270,7 +249,7 @@ static struct regulator_internal *of_regulator_get(struct device *dev,
> if (!of_get_property(dev->of_node, propname, NULL)) {
> dev_dbg(dev, "No %s-supply node found, using dummy regulator\n",
> supply);
> - ri = NULL;
> + rdev = NULL;
> goto out;
> }
>
> @@ -281,7 +260,7 @@ static struct regulator_internal *of_regulator_get(struct device *dev,
> node = of_parse_phandle(dev->of_node, propname, 0);
> if (!node) {
> dev_dbg(dev, "No %s node found\n", propname);
> - ri = ERR_PTR(-EINVAL);
> + rdev = ERR_PTR(-EINVAL);
> goto out;
> }
>
> @@ -298,16 +277,16 @@ static struct regulator_internal *of_regulator_get(struct device *dev,
> of_get_property(node_parent, "barebox,allow-dummy-supply", NULL)) {
> dev_dbg(dev, "Allow use of dummy regulator for " \
> "%s-supply\n", supply);
> - ri = NULL;
> + rdev = NULL;
> goto out;
> }
>
> - ri = ERR_PTR(ret);
> + rdev = ERR_PTR(ret);
> goto out;
> }
>
> - list_for_each_entry(ri, ®ulator_list, list) {
> - if (ri->node == node) {
> + list_for_each_entry(rdev, ®ulator_list, list) {
> + if (rdev->node == node) {
> dev_dbg(dev, "Using %s regulator from %pOF\n",
> propname, node);
> goto out;
> @@ -319,14 +298,14 @@ static struct regulator_internal *of_regulator_get(struct device *dev,
> * added in future initcalls, so, instead of reporting a
> * complete failure report probe deferral
> */
> - ri = ERR_PTR(-EPROBE_DEFER);
> + rdev = ERR_PTR(-EPROBE_DEFER);
> out:
> free(propname);
>
> - return ri;
> + return rdev;
> }
> #else
> -static struct regulator_internal *of_regulator_get(struct device *dev,
> +static struct regulator_dev *of_regulator_get(struct device *dev,
> const char *supply)
> {
> return NULL;
> @@ -335,38 +314,40 @@ static struct regulator_internal *of_regulator_get(struct device *dev,
>
> int dev_regulator_register(struct regulator_dev *rdev, const char * name, const char* supply)
> {
> - struct regulator_internal *ri;
> + int ret;
>
> - ri = __regulator_register(rdev, name);
> + ret = __regulator_register(rdev, name);
> + if (ret)
> + return ret;
>
> - ri->supply = supply;
> + rdev->supply_name = supply;
>
> return 0;
> }
>
> -static struct regulator_internal *dev_regulator_get(struct device *dev,
> - const char *supply)
> +static struct regulator_dev *dev_regulator_get(struct device *dev,
> + const char *supply)
> {
> - struct regulator_internal *ri;
> - struct regulator_internal *ret = NULL;
> + struct regulator_dev *rdev;
> + struct regulator_dev *ret = NULL;
> int match, best = 0;
> const char *dev_id = dev ? dev_name(dev) : NULL;
>
> - list_for_each_entry(ri, ®ulator_list, list) {
> + list_for_each_entry(rdev, ®ulator_list, list) {
> match = 0;
> - if (ri->name) {
> - if (!dev_id || strcmp(ri->name, dev_id))
> + if (rdev->name) {
> + if (!dev_id || strcmp(rdev->name, dev_id))
> continue;
> match += 2;
> }
> - if (ri->supply) {
> - if (!supply || strcmp(ri->supply, supply))
> + if (rdev->supply_name) {
> + if (!supply || strcmp(rdev->supply_name, supply))
> continue;
> match += 1;
> }
>
> if (match > best) {
> - ret = ri;
> + ret = rdev;
> if (match != 3)
> best = match;
> else
> @@ -389,62 +370,63 @@ static struct regulator_internal *dev_regulator_get(struct device *dev,
> */
> struct regulator *regulator_get(struct device *dev, const char *supply)
> {
> - struct regulator_internal *ri = NULL;
> + struct regulator_dev *rdev = NULL;
> struct regulator *r;
> int ret;
>
> if (dev->of_node && supply) {
> - ri = of_regulator_get(dev, supply);
> - if (IS_ERR(ri))
> - return ERR_CAST(ri);
> + rdev = of_regulator_get(dev, supply);
> + if (IS_ERR(rdev))
> + return ERR_CAST(rdev);
> }
>
> - if (!ri) {
> - ri = dev_regulator_get(dev, supply);
> - if (IS_ERR(ri))
> - return ERR_CAST(ri);
> + if (!rdev) {
> + rdev = dev_regulator_get(dev, supply);
> + if (IS_ERR(rdev))
> + return ERR_CAST(rdev);
> }
>
> - if (!ri)
> + if (!rdev)
> return NULL;
>
> - ret = regulator_resolve_supply(ri->rdev);
> + ret = regulator_resolve_supply(rdev);
> if (ret < 0)
> return ERR_PTR(ret);
>
> r = xzalloc(sizeof(*r));
> - r->ri = ri;
> + r->rdev = rdev;
> r->dev = dev;
>
> - list_add_tail(&r->list, &ri->consumer_list);
> + list_add_tail(&r->list, &rdev->consumer_list);
>
> return r;
> }
>
> -static struct regulator_internal *regulator_by_name(const char *name)
> +static struct regulator_dev *regulator_by_name(const char *name)
> {
> - struct regulator_internal *ri;
> + struct regulator_dev *rdev;
>
> - list_for_each_entry(ri, ®ulator_list, list)
> - if (ri->name && !strcmp(ri->name, name))
> - return ri;
> + list_for_each_entry(rdev, ®ulator_list, list) {
> + if (rdev->name && !strcmp(rdev->name, name))
> + return rdev;
> + }
>
> return NULL;
> }
>
> struct regulator *regulator_get_name(const char *name)
> {
> - struct regulator_internal *ri;
> + struct regulator_dev *rdev;
> struct regulator *r;
>
> - ri = regulator_by_name(name);
> - if (!ri)
> + rdev = regulator_by_name(name);
> + if (!rdev)
> return ERR_PTR(-ENODEV);
>
> r = xzalloc(sizeof(*r));
> - r->ri = ri;
> + r->rdev = rdev;
>
> - list_add_tail(&r->list, &ri->consumer_list);
> + list_add_tail(&r->list, &rdev->consumer_list);
>
> return r;
> }
> @@ -463,7 +445,7 @@ int regulator_enable(struct regulator *r)
> if (!r)
> return 0;
>
> - return regulator_enable_internal(r->ri);
> + return regulator_enable_internal(r->rdev);
> }
>
> /*
> @@ -480,7 +462,7 @@ int regulator_disable(struct regulator *r)
> if (!r)
> return 0;
>
> - return regulator_disable_internal(r->ri);
> + return regulator_disable_internal(r->rdev);
> }
>
> int regulator_set_voltage(struct regulator *r, int min_uV, int max_uV)
> @@ -488,7 +470,7 @@ int regulator_set_voltage(struct regulator *r, int min_uV, int max_uV)
> if (!r)
> return 0;
>
> - return regulator_set_voltage_internal(r->ri, min_uV, max_uV);
> + return regulator_set_voltage_internal(r->rdev, min_uV, max_uV);
> }
>
> /**
> @@ -632,15 +614,13 @@ EXPORT_SYMBOL_GPL(regulator_bulk_free);
>
> int regulator_get_voltage(struct regulator *regulator)
> {
> - struct regulator_internal *ri;
> struct regulator_dev *rdev;
> int sel, ret;
>
> if (!regulator)
> return -EINVAL;
>
> - ri = regulator->ri;
> - rdev = ri->rdev;
> + rdev = regulator->rdev;
>
> if (rdev->desc->ops->get_voltage_sel) {
> sel = rdev->desc->ops->get_voltage_sel(rdev);
> @@ -653,8 +633,8 @@ int regulator_get_voltage(struct regulator *regulator)
> ret = rdev->desc->ops->list_voltage(rdev, 0);
> } else if (rdev->desc->fixed_uV && (rdev->desc->n_voltages == 1)) {
> ret = rdev->desc->fixed_uV;
> - } else if (ri->min_uv && ri->min_uv == ri->max_uv) {
> - ret = ri->min_uv;
> + } else if (rdev->min_uv && rdev->min_uv == rdev->max_uv) {
> + ret = rdev->min_uv;
> } else if (rdev->supply) {
> ret = regulator_get_voltage(rdev->supply);
> } else {
> @@ -665,16 +645,16 @@ int regulator_get_voltage(struct regulator *regulator)
> }
> EXPORT_SYMBOL_GPL(regulator_get_voltage);
>
> -static void regulator_print_one(struct regulator_internal *ri)
> +static void regulator_print_one(struct regulator_dev *rdev)
> {
> struct regulator *r;
>
> - printf("%-20s %6d %10d %10d\n", ri->name, ri->enable_count, ri->min_uv, ri->max_uv);
> + printf("%-20s %6d %10d %10d\n", rdev->name, rdev->enable_count, rdev->min_uv, rdev->max_uv);
>
> - if (!list_empty(&ri->consumer_list)) {
> + if (!list_empty(&rdev->consumer_list)) {
> printf(" consumers:\n");
>
> - list_for_each_entry(r, &ri->consumer_list, list)
> + list_for_each_entry(r, &rdev->consumer_list, list)
> printf(" %s\n", r->dev ? dev_name(r->dev) : "none");
> }
> }
> @@ -684,9 +664,9 @@ static void regulator_print_one(struct regulator_internal *ri)
> */
> void regulators_print(void)
> {
> - struct regulator_internal *ri;
> + struct regulator_dev *rdev;
>
> printf("%-20s %6s %10s %10s\n", "name", "enable", "min_uv", "max_uv");
> - list_for_each_entry(ri, ®ulator_list, list)
> - regulator_print_one(ri);
> + list_for_each_entry(rdev, ®ulator_list, list)
> + regulator_print_one(rdev);
> }
> diff --git a/include/regulator.h b/include/regulator.h
> index b0a500434c..5eb236e602 100644
> --- a/include/regulator.h
> +++ b/include/regulator.h
> @@ -85,6 +85,15 @@ struct regulator_desc {
> };
>
> struct regulator_dev {
> + const char *name;
> + struct list_head list;
> + struct device_node *node;
> + int enable_count;
> + int enable_time_us;
> + int min_uv;
> + int max_uv;
> + const char *supply_name;
> + struct list_head consumer_list;
> const struct regulator_desc *desc;
> struct regmap *regmap;
> bool boot_on;
> --
> 2.39.2
>
>
>
