* [PATCH 01/11] regulator: rename variable rd to rdev
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:15 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev Sascha Hauer
` (9 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
the struct regulator_dev * variable is mostly named 'rdev', but
sometimes 'rd' is used. Rename to 'rdev' consistently.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/core.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 97e76b0db9..e9b5b82dbe 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -166,13 +166,13 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
return 0;
}
-static struct regulator_internal * __regulator_register(struct regulator_dev *rd, const char *name)
+static struct regulator_internal * __regulator_register(struct regulator_dev *rdev, const char *name)
{
struct regulator_internal *ri;
int ret;
ri = xzalloc(sizeof(*ri));
- ri->rdev = rd;
+ ri->rdev = rdev;
INIT_LIST_HEAD(&ri->consumer_list);
@@ -181,7 +181,7 @@ static struct regulator_internal * __regulator_register(struct regulator_dev *rd
if (name)
ri->name = xstrdup(name);
- if (rd->boot_on || rd->always_on) {
+ if (rdev->boot_on || rdev->always_on) {
ret = regulator_resolve_supply(ri->rdev);
if (ret < 0)
goto err;
@@ -204,38 +204,38 @@ static struct regulator_internal * __regulator_register(struct regulator_dev *rd
#ifdef CONFIG_OFDEVICE
/*
* of_regulator_register - register a regulator corresponding to a device_node
- * @rd: the regulator device providing the ops
+ * rdev: the regulator device providing the ops
* @node: the device_node this regulator corresponds to
*
* Return: 0 for success or a negative error code
*/
-int of_regulator_register(struct regulator_dev *rd, struct device_node *node)
+int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
{
struct regulator_internal *ri;
const char *name;
- if (!rd || !node)
+ if (!rdev || !node)
return -EINVAL;
- rd->boot_on = of_property_read_bool(node, "regulator-boot-on");
- rd->always_on = of_property_read_bool(node, "regulator-always-on");
+ rdev->boot_on = of_property_read_bool(node, "regulator-boot-on");
+ rdev->always_on = of_property_read_bool(node, "regulator-always-on");
name = of_get_property(node, "regulator-name", NULL);
if (!name)
name = node->name;
- ri = __regulator_register(rd, name);
+ ri = __regulator_register(rdev, name);
if (IS_ERR(ri))
return PTR_ERR(ri);
ri->node = node;
- node->dev = rd->dev;
+ node->dev = rdev->dev;
- if (rd->desc->off_on_delay)
- ri->enable_time_us = rd->desc->off_on_delay;
+ if (rdev->desc->off_on_delay)
+ ri->enable_time_us = rdev->desc->off_on_delay;
- if (rd->desc->fixed_uV && rd->desc->n_voltages == 1)
- ri->min_uv = ri->max_uv = rd->desc->fixed_uV;
+ if (rdev->desc->fixed_uV && rdev->desc->n_voltages == 1)
+ ri->min_uv = ri->max_uv = rdev->desc->fixed_uV;
of_property_read_u32(node, "regulator-enable-ramp-delay",
&ri->enable_time_us);
@@ -333,11 +333,11 @@ static struct regulator_internal *of_regulator_get(struct device *dev,
}
#endif
-int dev_regulator_register(struct regulator_dev *rd, const char * name, const char* supply)
+int dev_regulator_register(struct regulator_dev *rdev, const char * name, const char* supply)
{
struct regulator_internal *ri;
- ri = __regulator_register(rd, name);
+ ri = __regulator_register(rdev, name);
ri->supply = supply;
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 01/11] regulator: rename variable rd to rdev
2023-09-20 10:33 ` [PATCH 01/11] regulator: rename variable rd to rdev Sascha Hauer
@ 2023-09-20 11:15 ` Marco Felsch
0 siblings, 0 replies; 26+ messages in thread
From: Marco Felsch @ 2023-09-20 11:15 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
On 23-09-20, Sascha Hauer wrote:
> the struct regulator_dev * variable is mostly named 'rdev', but
> sometimes 'rd' is used. Rename to 'rdev' consistently.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> drivers/regulator/core.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 97e76b0db9..e9b5b82dbe 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -166,13 +166,13 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
> return 0;
> }
>
> -static struct regulator_internal * __regulator_register(struct regulator_dev *rd, const char *name)
> +static struct regulator_internal * __regulator_register(struct regulator_dev *rdev, const char *name)
> {
> struct regulator_internal *ri;
> int ret;
>
> ri = xzalloc(sizeof(*ri));
> - ri->rdev = rd;
> + ri->rdev = rdev;
>
> INIT_LIST_HEAD(&ri->consumer_list);
>
> @@ -181,7 +181,7 @@ static struct regulator_internal * __regulator_register(struct regulator_dev *rd
> if (name)
> ri->name = xstrdup(name);
>
> - if (rd->boot_on || rd->always_on) {
> + if (rdev->boot_on || rdev->always_on) {
> ret = regulator_resolve_supply(ri->rdev);
> if (ret < 0)
> goto err;
> @@ -204,38 +204,38 @@ static struct regulator_internal * __regulator_register(struct regulator_dev *rd
> #ifdef CONFIG_OFDEVICE
> /*
> * of_regulator_register - register a regulator corresponding to a device_node
> - * @rd: the regulator device providing the ops
> + * rdev: the regulator device providing the ops
> * @node: the device_node this regulator corresponds to
> *
> * Return: 0 for success or a negative error code
> */
> -int of_regulator_register(struct regulator_dev *rd, struct device_node *node)
> +int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
> {
> struct regulator_internal *ri;
> const char *name;
>
> - if (!rd || !node)
> + if (!rdev || !node)
> return -EINVAL;
>
> - rd->boot_on = of_property_read_bool(node, "regulator-boot-on");
> - rd->always_on = of_property_read_bool(node, "regulator-always-on");
> + rdev->boot_on = of_property_read_bool(node, "regulator-boot-on");
> + rdev->always_on = of_property_read_bool(node, "regulator-always-on");
>
> name = of_get_property(node, "regulator-name", NULL);
> if (!name)
> name = node->name;
>
> - ri = __regulator_register(rd, name);
> + ri = __regulator_register(rdev, name);
> if (IS_ERR(ri))
> return PTR_ERR(ri);
>
> ri->node = node;
> - node->dev = rd->dev;
> + node->dev = rdev->dev;
>
> - if (rd->desc->off_on_delay)
> - ri->enable_time_us = rd->desc->off_on_delay;
> + if (rdev->desc->off_on_delay)
> + ri->enable_time_us = rdev->desc->off_on_delay;
>
> - if (rd->desc->fixed_uV && rd->desc->n_voltages == 1)
> - ri->min_uv = ri->max_uv = rd->desc->fixed_uV;
> + if (rdev->desc->fixed_uV && rdev->desc->n_voltages == 1)
> + ri->min_uv = ri->max_uv = rdev->desc->fixed_uV;
>
> of_property_read_u32(node, "regulator-enable-ramp-delay",
> &ri->enable_time_us);
> @@ -333,11 +333,11 @@ static struct regulator_internal *of_regulator_get(struct device *dev,
> }
> #endif
>
> -int dev_regulator_register(struct regulator_dev *rd, const char * name, const char* supply)
> +int dev_regulator_register(struct regulator_dev *rdev, const char * name, const char* supply)
> {
> struct regulator_internal *ri;
>
> - ri = __regulator_register(rd, name);
> + ri = __regulator_register(rdev, name);
>
> ri->supply = supply;
>
> --
> 2.39.2
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
2023-09-20 10:33 ` [PATCH 01/11] regulator: rename variable rd to rdev Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 10:52 ` Marco Felsch
2023-09-20 11:20 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 03/11] regulator: introduce regulator logging functions Sascha Hauer
` (8 subsequent siblings)
10 siblings, 2 replies; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
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@pengutronix.de>
---
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);
- 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev
2023-09-20 10:33 ` [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev Sascha Hauer
@ 2023-09-20 10:52 ` Marco Felsch
2023-09-20 11:07 ` Sascha Hauer
2023-09-20 11:20 ` Marco Felsch
1 sibling, 1 reply; 26+ messages in thread
From: Marco Felsch @ 2023-09-20 10:52 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
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@pengutronix.de>
> ---
> 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
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev
2023-09-20 10:52 ` Marco Felsch
@ 2023-09-20 11:07 ` Sascha Hauer
2023-09-20 11:13 ` Marco Felsch
0 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 11:07 UTC (permalink / raw)
To: Marco Felsch; +Cc: Barebox List
On Wed, Sep 20, 2023 at 12:52:03PM +0200, Marco Felsch wrote:
> 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@pengutronix.de>
> > ---
> > 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?
Yes, the rest of this patch ;)
> > 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;
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev
2023-09-20 11:07 ` Sascha Hauer
@ 2023-09-20 11:13 ` Marco Felsch
0 siblings, 0 replies; 26+ messages in thread
From: Marco Felsch @ 2023-09-20 11:13 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
On 23-09-20, Sascha Hauer wrote:
> On Wed, Sep 20, 2023 at 12:52:03PM +0200, Marco Felsch wrote:
> > 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@pengutronix.de>
> > > ---
> > > 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?
>
> Yes, the rest of this patch ;)
Argh.. sorry for the noise.
Regards,
Marco
> > > 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;
>
> Sascha
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev
2023-09-20 10:33 ` [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev Sascha Hauer
2023-09-20 10:52 ` Marco Felsch
@ 2023-09-20 11:20 ` Marco Felsch
1 sibling, 0 replies; 26+ messages in thread
From: Marco Felsch @ 2023-09-20 11:20 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
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.
This also aligns it with Linux :)
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> 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);
>
> - 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
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 03/11] regulator: introduce regulator logging functions.
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
2023-09-20 10:33 ` [PATCH 01/11] regulator: rename variable rd to rdev Sascha Hauer
2023-09-20 10:33 ` [PATCH 02/11] regulator: merge struct regulator_internal fields into struct regulator_dev Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:22 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 04/11] regulator: add regulator_get_voltage_internal() Sascha Hauer
` (7 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
dev_* functions only print the struct device * as context, but often
a single struct device * implements multiple regulators. Add rdev_*
logging functions which allow to to print one specific regulator as
context.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/core.c | 12 ++++++++++--
include/regulator.h | 13 +++++++++++++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 41a3378ac8..8ef5a2372c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -19,6 +19,14 @@ struct regulator {
struct device *dev;
};
+const char *rdev_get_name(struct regulator_dev *rdev)
+{
+ if (rdev->name)
+ return rdev->name;
+
+ return "";
+}
+
static int regulator_map_voltage(struct regulator_dev *rdev, int min_uV,
int max_uV)
{
@@ -125,7 +133,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
if (!supply_name)
return 0;
- dev_dbg(rdev->dev, "resolving %s\n", supply_name);
+ rdev_dbg(rdev, "resolving %s\n", supply_name);
supply = regulator_get(rdev->dev, supply_name);
if (IS_ERR(supply)) {
@@ -141,7 +149,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
* we couldn't. If you want to get rid of this warning, consider
* migrating your platform to have deep probe support.
*/
- dev_warn(rdev->dev, "Failed to get '%s' regulator (ignored).\n",
+ rdev_warn(rdev, "Failed to get '%s' regulator (ignored).\n",
supply_name);
return 0;
}
diff --git a/include/regulator.h b/include/regulator.h
index 5eb236e602..d02ea8ffd0 100644
--- a/include/regulator.h
+++ b/include/regulator.h
@@ -159,6 +159,19 @@ int dev_regulator_register(struct regulator_dev *rd, const char * name,
void regulators_print(void);
+const char *rdev_get_name(struct regulator_dev *rdev);
+
+#define rdev_crit(rdev, fmt, ...) \
+ pr_crit("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
+#define rdev_err(rdev, fmt, ...) \
+ pr_err("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
+#define rdev_warn(rdev, fmt, ...) \
+ pr_warn("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
+#define rdev_info(rdev, fmt, ...) \
+ pr_info("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
+#define rdev_dbg(rdev, fmt, ...) \
+ pr_debug("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
+
#ifdef CONFIG_REGULATOR
struct regulator *regulator_get(struct device *, const char *);
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 03/11] regulator: introduce regulator logging functions.
2023-09-20 10:33 ` [PATCH 03/11] regulator: introduce regulator logging functions Sascha Hauer
@ 2023-09-20 11:22 ` Marco Felsch
0 siblings, 0 replies; 26+ messages in thread
From: Marco Felsch @ 2023-09-20 11:22 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
On 23-09-20, Sascha Hauer wrote:
> dev_* functions only print the struct device * as context, but often
> a single struct device * implements multiple regulators. Add rdev_*
> logging functions which allow to to print one specific regulator as
> context.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> drivers/regulator/core.c | 12 ++++++++++--
> include/regulator.h | 13 +++++++++++++
> 2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 41a3378ac8..8ef5a2372c 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -19,6 +19,14 @@ struct regulator {
> struct device *dev;
> };
>
> +const char *rdev_get_name(struct regulator_dev *rdev)
> +{
> + if (rdev->name)
> + return rdev->name;
> +
> + return "";
> +}
> +
> static int regulator_map_voltage(struct regulator_dev *rdev, int min_uV,
> int max_uV)
> {
> @@ -125,7 +133,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
> if (!supply_name)
> return 0;
>
> - dev_dbg(rdev->dev, "resolving %s\n", supply_name);
> + rdev_dbg(rdev, "resolving %s\n", supply_name);
>
> supply = regulator_get(rdev->dev, supply_name);
> if (IS_ERR(supply)) {
> @@ -141,7 +149,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
> * we couldn't. If you want to get rid of this warning, consider
> * migrating your platform to have deep probe support.
> */
> - dev_warn(rdev->dev, "Failed to get '%s' regulator (ignored).\n",
> + rdev_warn(rdev, "Failed to get '%s' regulator (ignored).\n",
> supply_name);
> return 0;
> }
> diff --git a/include/regulator.h b/include/regulator.h
> index 5eb236e602..d02ea8ffd0 100644
> --- a/include/regulator.h
> +++ b/include/regulator.h
> @@ -159,6 +159,19 @@ int dev_regulator_register(struct regulator_dev *rd, const char * name,
>
> void regulators_print(void);
>
> +const char *rdev_get_name(struct regulator_dev *rdev);
> +
> +#define rdev_crit(rdev, fmt, ...) \
> + pr_crit("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
> +#define rdev_err(rdev, fmt, ...) \
> + pr_err("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
> +#define rdev_warn(rdev, fmt, ...) \
> + pr_warn("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
> +#define rdev_info(rdev, fmt, ...) \
> + pr_info("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
> +#define rdev_dbg(rdev, fmt, ...) \
> + pr_debug("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
> +
> #ifdef CONFIG_REGULATOR
>
> struct regulator *regulator_get(struct device *, const char *);
> --
> 2.39.2
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 04/11] regulator: add regulator_get_voltage_internal()
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (2 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 03/11] regulator: introduce regulator logging functions Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:25 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 05/11] regulator: Add missing cases in regulator_map_voltage() Sascha Hauer
` (6 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
regulator_get_voltage() works on struct regulator * which may not always
be available internally, so add a regulator_get_voltage_internal() and
use it from regulator_get_voltage().
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/core.c | 52 +++++++++++++++++++++-------------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 8ef5a2372c..5693fa9634 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -121,6 +121,32 @@ static int regulator_set_voltage_internal(struct regulator_dev *rdev,
return -ENOSYS;
}
+static int regulator_get_voltage_internal(struct regulator_dev *rdev)
+{
+ int sel, ret;
+
+ if (rdev->desc->ops->get_voltage_sel) {
+ sel = rdev->desc->ops->get_voltage_sel(rdev);
+ if (sel < 0)
+ return sel;
+ ret = rdev->desc->ops->list_voltage(rdev, sel);
+ } else if (rdev->desc->ops->get_voltage) {
+ ret = rdev->desc->ops->get_voltage(rdev);
+ } else if (rdev->desc->ops->list_voltage) {
+ 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 (rdev->min_uv && rdev->min_uv == rdev->max_uv) {
+ ret = rdev->min_uv;
+ } else if (rdev->supply) {
+ ret = regulator_get_voltage(rdev->supply);
+ } else {
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
static int regulator_resolve_supply(struct regulator_dev *rdev)
{
struct regulator *supply;
@@ -622,34 +648,10 @@ EXPORT_SYMBOL_GPL(regulator_bulk_free);
int regulator_get_voltage(struct regulator *regulator)
{
- struct regulator_dev *rdev;
- int sel, ret;
-
if (!regulator)
return -EINVAL;
- rdev = regulator->rdev;
-
- if (rdev->desc->ops->get_voltage_sel) {
- sel = rdev->desc->ops->get_voltage_sel(rdev);
- if (sel < 0)
- return sel;
- ret = rdev->desc->ops->list_voltage(rdev, sel);
- } else if (rdev->desc->ops->get_voltage) {
- ret = rdev->desc->ops->get_voltage(rdev);
- } else if (rdev->desc->ops->list_voltage) {
- 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 (rdev->min_uv && rdev->min_uv == rdev->max_uv) {
- ret = rdev->min_uv;
- } else if (rdev->supply) {
- ret = regulator_get_voltage(rdev->supply);
- } else {
- return -EINVAL;
- }
-
- return ret;
+ return regulator_get_voltage_internal(regulator->rdev);
}
EXPORT_SYMBOL_GPL(regulator_get_voltage);
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 04/11] regulator: add regulator_get_voltage_internal()
2023-09-20 10:33 ` [PATCH 04/11] regulator: add regulator_get_voltage_internal() Sascha Hauer
@ 2023-09-20 11:25 ` Marco Felsch
2023-09-20 11:45 ` Sascha Hauer
0 siblings, 1 reply; 26+ messages in thread
From: Marco Felsch @ 2023-09-20 11:25 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
On 23-09-20, Sascha Hauer wrote:
> regulator_get_voltage() works on struct regulator * which may not always
> be available internally, so add a regulator_get_voltage_internal() and
> use it from regulator_get_voltage().
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/regulator/core.c | 52 +++++++++++++++++++++-------------------
> 1 file changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 8ef5a2372c..5693fa9634 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -121,6 +121,32 @@ static int regulator_set_voltage_internal(struct regulator_dev *rdev,
> return -ENOSYS;
> }
>
> +static int regulator_get_voltage_internal(struct regulator_dev *rdev)
Can we name this regulator_get_voltage_rdev()? This would align the code
with Linux.
Regards,
Marco
> +{
> + int sel, ret;
> +
> + if (rdev->desc->ops->get_voltage_sel) {
> + sel = rdev->desc->ops->get_voltage_sel(rdev);
> + if (sel < 0)
> + return sel;
> + ret = rdev->desc->ops->list_voltage(rdev, sel);
> + } else if (rdev->desc->ops->get_voltage) {
> + ret = rdev->desc->ops->get_voltage(rdev);
> + } else if (rdev->desc->ops->list_voltage) {
> + 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 (rdev->min_uv && rdev->min_uv == rdev->max_uv) {
> + ret = rdev->min_uv;
> + } else if (rdev->supply) {
> + ret = regulator_get_voltage(rdev->supply);
> + } else {
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> static int regulator_resolve_supply(struct regulator_dev *rdev)
> {
> struct regulator *supply;
> @@ -622,34 +648,10 @@ EXPORT_SYMBOL_GPL(regulator_bulk_free);
>
> int regulator_get_voltage(struct regulator *regulator)
> {
> - struct regulator_dev *rdev;
> - int sel, ret;
> -
> if (!regulator)
> return -EINVAL;
>
> - rdev = regulator->rdev;
> -
> - if (rdev->desc->ops->get_voltage_sel) {
> - sel = rdev->desc->ops->get_voltage_sel(rdev);
> - if (sel < 0)
> - return sel;
> - ret = rdev->desc->ops->list_voltage(rdev, sel);
> - } else if (rdev->desc->ops->get_voltage) {
> - ret = rdev->desc->ops->get_voltage(rdev);
> - } else if (rdev->desc->ops->list_voltage) {
> - 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 (rdev->min_uv && rdev->min_uv == rdev->max_uv) {
> - ret = rdev->min_uv;
> - } else if (rdev->supply) {
> - ret = regulator_get_voltage(rdev->supply);
> - } else {
> - return -EINVAL;
> - }
> -
> - return ret;
> + return regulator_get_voltage_internal(regulator->rdev);
> }
> EXPORT_SYMBOL_GPL(regulator_get_voltage);
>
> --
> 2.39.2
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 04/11] regulator: add regulator_get_voltage_internal()
2023-09-20 11:25 ` Marco Felsch
@ 2023-09-20 11:45 ` Sascha Hauer
0 siblings, 0 replies; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 11:45 UTC (permalink / raw)
To: Marco Felsch; +Cc: Barebox List
On Wed, Sep 20, 2023 at 01:25:31PM +0200, Marco Felsch wrote:
> On 23-09-20, Sascha Hauer wrote:
> > regulator_get_voltage() works on struct regulator * which may not always
> > be available internally, so add a regulator_get_voltage_internal() and
> > use it from regulator_get_voltage().
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > drivers/regulator/core.c | 52 +++++++++++++++++++++-------------------
> > 1 file changed, 27 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> > index 8ef5a2372c..5693fa9634 100644
> > --- a/drivers/regulator/core.c
> > +++ b/drivers/regulator/core.c
> > @@ -121,6 +121,32 @@ static int regulator_set_voltage_internal(struct regulator_dev *rdev,
> > return -ENOSYS;
> > }
> >
> > +static int regulator_get_voltage_internal(struct regulator_dev *rdev)
>
> Can we name this regulator_get_voltage_rdev()? This would align the code
> with Linux.
Yes. The _internal suffix currently aligns with the other functions we
have in barebox, so I'll keep it here in this patch and add a patch to
this series changing the names for all functions.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 05/11] regulator: Add missing cases in regulator_map_voltage()
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (3 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 04/11] regulator: add regulator_get_voltage_internal() Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:28 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 06/11] regulator: stpmic1: add .get_voltage_sel Sascha Hauer
` (5 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
regulator_map_voltage() misses to handle some cases, sync this with the
kernel.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5693fa9634..b9a97a784f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -33,7 +33,10 @@ static int regulator_map_voltage(struct regulator_dev *rdev, int min_uV,
if (rdev->desc->ops->list_voltage == regulator_list_voltage_linear)
return regulator_map_voltage_linear(rdev, min_uV, max_uV);
- return -ENOSYS;
+ if (rdev->desc->ops->list_voltage == regulator_list_voltage_linear_range)
+ return regulator_map_voltage_linear_range(rdev, min_uV, max_uV);
+
+ return regulator_map_voltage_iterate(rdev, min_uV, max_uV);
}
static int regulator_enable_internal(struct regulator_dev *rdev)
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 06/11] regulator: stpmic1: add .get_voltage_sel
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (4 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 05/11] regulator: Add missing cases in regulator_map_voltage() Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:29 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 07/11] regulator: stpmic1: add .supply_name Sascha Hauer
` (4 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
To get the current voltage from the regulators the .get_voltage_sel
callback is needed. Initialize it to the correct function.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/stpmic1_regulator.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/regulator/stpmic1_regulator.c b/drivers/regulator/stpmic1_regulator.c
index c8b56e3a1a..ef8cc7f145 100644
--- a/drivers/regulator/stpmic1_regulator.c
+++ b/drivers/regulator/stpmic1_regulator.c
@@ -126,6 +126,7 @@ static const struct regulator_ops stpmic1_ldo_ops = {
.is_enabled = regulator_is_enabled_regmap,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
};
@@ -135,6 +136,7 @@ static const struct regulator_ops stpmic1_ldo3_ops = {
.is_enabled = regulator_is_enabled_regmap,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
};
@@ -150,6 +152,7 @@ static const struct regulator_ops stpmic1_buck_ops = {
.is_enabled = regulator_is_enabled_regmap,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
};
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 07/11] regulator: stpmic1: add .supply_name
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (5 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 06/11] regulator: stpmic1: add .get_voltage_sel Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:36 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 08/11] regulator: register regulator as last step in of_regulator_register() Sascha Hauer
` (3 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
Add .supply_name to the regulator descriptors, otherwise the supplies
are never enabled.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/stpmic1_regulator.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/regulator/stpmic1_regulator.c b/drivers/regulator/stpmic1_regulator.c
index ef8cc7f145..1e8c1b23ac 100644
--- a/drivers/regulator/stpmic1_regulator.c
+++ b/drivers/regulator/stpmic1_regulator.c
@@ -185,6 +185,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = LDO_ENABLE_MASK, \
.enable_val = 1, \
.disable_val = 0, \
+ .supply_name = #base, \
}
#define REG_LDO3(ids, base) { \
@@ -198,6 +199,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = LDO_ENABLE_MASK, \
.enable_val = 1, \
.disable_val = 0, \
+ .supply_name = #base, \
}
#define REG_LDO4(ids, base) { \
@@ -208,6 +210,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = LDO_ENABLE_MASK, \
.enable_val = 1, \
.disable_val = 0, \
+ .supply_name = #base, \
}
#define REG_BUCK(ids, base) { \
@@ -221,6 +224,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = BUCK_ENABLE_MASK, \
.enable_val = 1, \
.disable_val = 0, \
+ .supply_name = #base, \
}
#define REG_VREF_DDR(ids, base) { \
@@ -231,6 +235,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = BUCK_ENABLE_MASK, \
.enable_val = 1, \
.disable_val = 0, \
+ .supply_name = #base, \
}
#define REG_BOOST(ids, base) { \
@@ -241,6 +246,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = BOOST_ENABLED, \
.enable_val = BOOST_ENABLED, \
.disable_val = 0, \
+ .supply_name = #base, \
}
#define REG_VBUS_OTG(ids, base) { \
@@ -251,6 +257,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = USBSW_OTG_SWITCH_ENABLED, \
.enable_val = USBSW_OTG_SWITCH_ENABLED, \
.disable_val = 0, \
+ .supply_name = #base, \
}
#define REG_SW_OUT(ids, base) { \
@@ -261,6 +268,7 @@ static const struct regulator_ops stpmic1_switch_regul_ops = {
.enable_mask = SWIN_SWOUT_ENABLED, \
.enable_val = SWIN_SWOUT_ENABLED, \
.disable_val = 0, \
+ .supply_name = #base, \
}
static struct stpmic1_regulator_cfg stpmic1_regulator_cfgs[] = {
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 08/11] regulator: register regulator as last step in of_regulator_register()
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (6 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 07/11] regulator: stpmic1: add .supply_name Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:39 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 09/11] regulator: Set initial voltage Sascha Hauer
` (2 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
In of_regulator_register() call __regulator_register as last step after
all fields have been initialized. This was not possible before as
__regulator_register() returned the struct regulator_internal * which
contained the remaining fields. Now that struct regulator_internal is
gone we can restore the natural initialization order.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b9a97a784f..d08df1dc68 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -240,10 +240,6 @@ int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
if (!name)
name = node->name;
- ret = __regulator_register(rdev, name);
- if (ret)
- return ret;
-
rdev->node = node;
node->dev = rdev->dev;
@@ -260,6 +256,10 @@ int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
of_property_read_u32(node, "regulator-max-microvolt",
&rdev->max_uv);
+ ret = __regulator_register(rdev, name);
+ if (ret)
+ return ret;
+
return 0;
}
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 08/11] regulator: register regulator as last step in of_regulator_register()
2023-09-20 10:33 ` [PATCH 08/11] regulator: register regulator as last step in of_regulator_register() Sascha Hauer
@ 2023-09-20 11:39 ` Marco Felsch
0 siblings, 0 replies; 26+ messages in thread
From: Marco Felsch @ 2023-09-20 11:39 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Barebox List
On 23-09-20, Sascha Hauer wrote:
> In of_regulator_register() call __regulator_register as last step after
> all fields have been initialized. This was not possible before as
> __regulator_register() returned the struct regulator_internal * which
> contained the remaining fields. Now that struct regulator_internal is
> gone we can restore the natural initialization order.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/regulator/core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index b9a97a784f..d08df1dc68 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -240,10 +240,6 @@ int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
> if (!name)
> name = node->name;
>
> - ret = __regulator_register(rdev, name);
> - if (ret)
> - return ret;
> -
> rdev->node = node;
> node->dev = rdev->dev;
>
> @@ -260,6 +256,10 @@ int of_regulator_register(struct regulator_dev *rdev, struct device_node *node)
> of_property_read_u32(node, "regulator-max-microvolt",
> &rdev->max_uv);
>
> + ret = __regulator_register(rdev, name);
return __regulator_register(rdev, name);
Regards,
Marco
> + if (ret)
> + return ret;
> +
> return 0;
> }
>
> --
> 2.39.2
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 09/11] regulator: Set initial voltage
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (7 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 08/11] regulator: register regulator as last step in of_regulator_register() Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:51 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 10/11] regulator: drop struct regulator_dev::supply_name Sascha Hauer
2023-09-20 10:33 ` [PATCH 11/11] regulator: print regulator tree Sascha Hauer
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
When voltage constraints are given in the device tree then we should
set the regulator to a valid voltage before enabling it. Without it
we can end up with invalid voltages.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/core.c | 60 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d08df1dc68..00a2aefce7 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -187,6 +187,62 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
return 0;
}
+static int regulator_init_voltage(struct regulator_dev *rdev)
+{
+ int target_min, target_max, current_uV, ret;
+
+ if (!rdev->min_uv || !rdev->max_uv)
+ return 0;
+
+ current_uV = regulator_get_voltage_internal(rdev);
+ if (current_uV < 0) {
+ /* This regulator can't be read and must be initialized */
+ rdev_info(rdev, "Setting %d-%duV\n", rdev->min_uv, rdev->max_uv);
+ regulator_set_voltage_internal(rdev, rdev->min_uv, rdev->max_uv);
+ current_uV = regulator_get_voltage_internal(rdev);
+ }
+
+ if (current_uV < 0) {
+ if (current_uV != -EPROBE_DEFER)
+ rdev_err(rdev,
+ "failed to get the current voltage: %pe\n",
+ ERR_PTR(current_uV));
+ return current_uV;
+ }
+
+ /*
+ * If we're below the minimum voltage move up to the
+ * minimum voltage, if we're above the maximum voltage
+ * then move down to the maximum.
+ */
+ target_min = current_uV;
+ target_max = current_uV;
+
+ if (current_uV < rdev->min_uv) {
+ target_min = rdev->min_uv;
+ target_max = rdev->min_uv;
+ }
+
+ if (current_uV > rdev->max_uv) {
+ target_min = rdev->max_uv;
+ target_max = rdev->max_uv;
+ }
+
+ if (target_min != current_uV || target_max != current_uV) {
+ rdev_info(rdev, "Bringing %duV into %d-%duV\n",
+ current_uV, target_min, target_max);
+ ret = regulator_set_voltage_internal(rdev, target_min, target_max);
+ if (ret < 0) {
+ rdev_err(rdev,
+ "failed to apply %d-%duV constraint: %pe\n",
+ target_min, target_max, ERR_PTR(ret));
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static int __regulator_register(struct regulator_dev *rdev, const char *name)
{
int ret;
@@ -198,6 +254,10 @@ static int __regulator_register(struct regulator_dev *rdev, const char *name)
if (name)
rdev->name = xstrdup(name);
+ ret = regulator_init_voltage(rdev);
+ if (ret)
+ goto err;
+
if (rdev->boot_on || rdev->always_on) {
ret = regulator_resolve_supply(rdev);
if (ret < 0)
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 10/11] regulator: drop struct regulator_dev::supply_name
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (8 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 09/11] regulator: Set initial voltage Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
2023-09-20 11:51 ` Marco Felsch
2023-09-20 10:33 ` [PATCH 11/11] regulator: print regulator tree Sascha Hauer
10 siblings, 1 reply; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
The supply name can be set in struct regulator_desc, no need to have
it in struct regulator_dev also, so drop it there.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/bcm2835.c | 2 +-
drivers/regulator/core.c | 8 +++-----
include/regulator.h | 4 +---
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/regulator/bcm2835.c b/drivers/regulator/bcm2835.c
index 34e0429dfd..fa9fc47207 100644
--- a/drivers/regulator/bcm2835.c
+++ b/drivers/regulator/bcm2835.c
@@ -122,7 +122,7 @@ static int regulator_bcm2835_probe(struct device *dev)
rb->rdev.desc = &rb->rdesc;
rb->rdev.dev = dev;
- ret = dev_regulator_register(&rb->rdev, rb->devname, NULL);
+ ret = dev_regulator_register(&rb->rdev, rb->devname);
if (ret)
return ret;
}
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 00a2aefce7..7fe264a133 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -409,7 +409,7 @@ static struct regulator_dev *of_regulator_get(struct device *dev,
}
#endif
-int dev_regulator_register(struct regulator_dev *rdev, const char * name, const char* supply)
+int dev_regulator_register(struct regulator_dev *rdev, const char *name)
{
int ret;
@@ -417,8 +417,6 @@ int dev_regulator_register(struct regulator_dev *rdev, const char * name, const
if (ret)
return ret;
- rdev->supply_name = supply;
-
return 0;
}
@@ -437,8 +435,8 @@ static struct regulator_dev *dev_regulator_get(struct device *dev,
continue;
match += 2;
}
- if (rdev->supply_name) {
- if (!supply || strcmp(rdev->supply_name, supply))
+ if (rdev->desc->supply_name) {
+ if (!supply || strcmp(rdev->desc->supply_name, supply))
continue;
match += 1;
}
diff --git a/include/regulator.h b/include/regulator.h
index d02ea8ffd0..c5b405dfcd 100644
--- a/include/regulator.h
+++ b/include/regulator.h
@@ -92,7 +92,6 @@ struct regulator_dev {
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;
@@ -154,8 +153,7 @@ static inline int of_regulator_register(struct regulator_dev *rd,
return -ENOSYS;
}
#endif
-int dev_regulator_register(struct regulator_dev *rd, const char * name,
- const char* supply);
+int dev_regulator_register(struct regulator_dev *rd, const char *name);
void regulators_print(void);
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 11/11] regulator: print regulator tree
2023-09-20 10:33 [PATCH 00/11] regulator updates Sascha Hauer
` (9 preceding siblings ...)
2023-09-20 10:33 ` [PATCH 10/11] regulator: drop struct regulator_dev::supply_name Sascha Hauer
@ 2023-09-20 10:33 ` Sascha Hauer
10 siblings, 0 replies; 26+ messages in thread
From: Sascha Hauer @ 2023-09-20 10:33 UTC (permalink / raw)
To: Barebox List
Regulators have a tree structure, so print them as a tree to show the
users their dependencies.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/regulator/core.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7fe264a133..a861d14cd2 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -15,6 +15,7 @@ static LIST_HEAD(regulator_list);
struct regulator {
struct regulator_dev *rdev;
+ struct regulator_dev *rdev_consumer;
struct list_head list;
struct device *dev;
};
@@ -183,7 +184,11 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
return 0;
}
+ if (supply)
+ supply->rdev_consumer = rdev;
+
rdev->supply = supply;
+
return 0;
}
@@ -716,17 +721,22 @@ int regulator_get_voltage(struct regulator *regulator)
}
EXPORT_SYMBOL_GPL(regulator_get_voltage);
-static void regulator_print_one(struct regulator_dev *rdev)
+static void regulator_print_one(struct regulator_dev *rdev, int level)
{
struct regulator *r;
- printf("%-20s %6d %10d %10d\n", rdev->name, rdev->enable_count, rdev->min_uv, rdev->max_uv);
+ if (!rdev)
+ return;
- if (!list_empty(&rdev->consumer_list)) {
- printf(" consumers:\n");
+ printf("%*s%-*s %6d %10d %10d\n", level * 3, "",
+ 30 - level * 3,
+ rdev->name, rdev->enable_count, rdev->min_uv, rdev->max_uv);
- list_for_each_entry(r, &rdev->consumer_list, list)
- printf(" %s\n", r->dev ? dev_name(r->dev) : "none");
+ list_for_each_entry(r, &rdev->consumer_list, list) {
+ if (r->rdev_consumer)
+ regulator_print_one(r->rdev_consumer, level + 1);
+ else
+ printf("%*s%s\n", (level + 1) * 3, "", r->dev ? dev_name(r->dev) : "none");
}
}
@@ -737,7 +747,9 @@ void regulators_print(void)
{
struct regulator_dev *rdev;
- printf("%-20s %6s %10s %10s\n", "name", "enable", "min_uv", "max_uv");
- list_for_each_entry(rdev, ®ulator_list, list)
- regulator_print_one(rdev);
+ printf("%-30s %6s %10s %10s\n", "name", "enable", "min_uv", "max_uv");
+ list_for_each_entry(rdev, ®ulator_list, list) {
+ if (!rdev->supply)
+ regulator_print_one(rdev, 0);
+ }
}
--
2.39.2
^ permalink raw reply [flat|nested] 26+ messages in thread