2022-10-31 23:49:04

by Zev Weiss

[permalink] [raw]
Subject: [PATCH v2 0/3] regulator: Add DT support for regulator-output connectors

Hello,

This is v2 of my recent series adding support for userspace-controlled
regulator-supplied power outputs [2]. This is an important feature
for some kinds of BMC (baseboard management controller) systems where
the BMC provides an operator with manual control of a set of DC power
outputs.

As in a broadly similar patchset that was recently almost merged [0],
this takes the approach of providing support by extending the existing
userspace-consumer regulator driver. A couple questions about the
userspace-consumer driver came up along the way, however.

First, how (if at all) is it currently being used? It appears the
last in-tree use of it was removed a bit over two years ago in commit
9d3239147d6d ("ARM: pxa: remove Compulab pxa2xx boards"). Aside from
just adding DT support I've made a couple small tweaks to the driver
in patch 3 that I hope are compatible with any current usage, but
without any extant examples to look at it's kind of hard to say.

Second, how critical is its support for controlling multiple
regulators? (i.e. its use of regulator_bulk_data and friends instead
of a single struct regulator.) As far as I can see every in-tree use
of it that's ever existed has used num_supplies = 1. If it's not
important to retain, patch 1 of this series could be supplanted by one
that instead simplifies the driver slightly by removing that
functionality.

The DT binding added in patch 2 is essentially identical to one I
posted in a previous patchset that had an R-B from Rob [1], but has
had some minor rewording and been moved from the extcon subsystem to
the regulator subsystem.

Changes since v1 [2]:
- removed 'regulator-leave-on' DT property
- added .is_visible method to hide sysfs 'name' property when it's
NULL


Thanks,
Zev

[0] https://lore.kernel.org/all/[email protected]/
[1] https://lore.kernel.org/linux-kernel/[email protected]/
[2] https://lore.kernel.org/openbmc/[email protected]/

Zev Weiss (3):
regulator: devres: Add devm_regulator_bulk_get_exclusive()
dt-bindings: regulator: Add regulator-output binding
regulator: userspace-consumer: Handle regulator-output DT nodes

.../bindings/regulator/regulator-output.yaml | 39 +++++++++++
drivers/regulator/core.c | 42 +++++++-----
drivers/regulator/devres.c | 66 ++++++++++++++-----
drivers/regulator/internal.h | 2 +
drivers/regulator/userspace-consumer.c | 60 +++++++++++++++--
include/linux/regulator/consumer.h | 2 +
include/linux/regulator/userspace-consumer.h | 1 +
7 files changed, 169 insertions(+), 43 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/regulator-output.yaml

--
2.38.1



2022-10-31 23:49:11

by Zev Weiss

[permalink] [raw]
Subject: [PATCH v2 1/3] regulator: devres: Add devm_regulator_bulk_get_exclusive()

We had an exclusive variant of the devm_regulator_get() API, but no
corresponding variant for the bulk API; let's add one now. We add a
generalized version of the existing regulator_bulk_get() function that
additionally takes a get_type parameter and redefine
regulator_bulk_get() in terms of it, then do similarly with
devm_regulator_bulk_get(), and finally add the new
devm_regulator_bulk_get_exclusive().

Signed-off-by: Zev Weiss <[email protected]>
---
drivers/regulator/core.c | 42 +++++++++++--------
drivers/regulator/devres.c | 66 ++++++++++++++++++++++--------
drivers/regulator/internal.h | 2 +
include/linux/regulator/consumer.h | 2 +
4 files changed, 76 insertions(+), 36 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index bcccad8f7516..704f91720dfe 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4778,22 +4778,8 @@ static int _notifier_call_chain(struct regulator_dev *rdev,
return blocking_notifier_call_chain(&rdev->notifier, event, data);
}

-/**
- * regulator_bulk_get - get multiple regulator consumers
- *
- * @dev: Device to supply
- * @num_consumers: Number of consumers to register
- * @consumers: Configuration of consumers; clients are stored here.
- *
- * @return 0 on success, an errno on failure.
- *
- * This helper function allows drivers to get several regulator
- * consumers in one operation. If any of the regulators cannot be
- * acquired then any regulators that were allocated will be freed
- * before returning to the caller.
- */
-int regulator_bulk_get(struct device *dev, int num_consumers,
- struct regulator_bulk_data *consumers)
+int _regulator_bulk_get(struct device *dev, int num_consumers,
+ struct regulator_bulk_data *consumers, enum regulator_get_type get_type)
{
int i;
int ret;
@@ -4802,8 +4788,8 @@ int regulator_bulk_get(struct device *dev, int num_consumers,
consumers[i].consumer = NULL;

for (i = 0; i < num_consumers; i++) {
- consumers[i].consumer = regulator_get(dev,
- consumers[i].supply);
+ consumers[i].consumer = _regulator_get(dev,
+ consumers[i].supply, get_type);
if (IS_ERR(consumers[i].consumer)) {
ret = dev_err_probe(dev, PTR_ERR(consumers[i].consumer),
"Failed to get supply '%s'",
@@ -4830,6 +4816,26 @@ int regulator_bulk_get(struct device *dev, int num_consumers,

return ret;
}
+
+/**
+ * regulator_bulk_get - get multiple regulator consumers
+ *
+ * @dev: Device to supply
+ * @num_consumers: Number of consumers to register
+ * @consumers: Configuration of consumers; clients are stored here.
+ *
+ * @return 0 on success, an errno on failure.
+ *
+ * This helper function allows drivers to get several regulator
+ * consumers in one operation. If any of the regulators cannot be
+ * acquired then any regulators that were allocated will be freed
+ * before returning to the caller.
+ */
+int regulator_bulk_get(struct device *dev, int num_consumers,
+ struct regulator_bulk_data *consumers)
+{
+ return _regulator_bulk_get(dev, num_consumers, consumers, NORMAL_GET);
+}
EXPORT_SYMBOL_GPL(regulator_bulk_get);

static void regulator_bulk_enable_async(void *data, async_cookie_t cookie)
diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 3265e75e97ab..fec0398d98b0 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -186,23 +186,9 @@ static void devm_regulator_bulk_release(struct device *dev, void *res)
regulator_bulk_free(devres->num_consumers, devres->consumers);
}

-/**
- * devm_regulator_bulk_get - managed get multiple regulator consumers
- *
- * @dev: device to supply
- * @num_consumers: number of consumers to register
- * @consumers: configuration of consumers; clients are stored here.
- *
- * @return 0 on success, an errno on failure.
- *
- * This helper function allows drivers to get several regulator
- * consumers in one operation with management, the regulators will
- * automatically be freed when the device is unbound. If any of the
- * regulators cannot be acquired then any regulators that were
- * allocated will be freed before returning to the caller.
- */
-int devm_regulator_bulk_get(struct device *dev, int num_consumers,
- struct regulator_bulk_data *consumers)
+static int _devm_regulator_bulk_get(struct device *dev, int num_consumers,
+ struct regulator_bulk_data *consumers,
+ enum regulator_get_type get_type)
{
struct regulator_bulk_devres *devres;
int ret;
@@ -212,7 +198,7 @@ int devm_regulator_bulk_get(struct device *dev, int num_consumers,
if (!devres)
return -ENOMEM;

- ret = regulator_bulk_get(dev, num_consumers, consumers);
+ ret = _regulator_bulk_get(dev, num_consumers, consumers, get_type);
if (!ret) {
devres->consumers = consumers;
devres->num_consumers = num_consumers;
@@ -223,8 +209,52 @@ int devm_regulator_bulk_get(struct device *dev, int num_consumers,

return ret;
}
+
+/**
+ * devm_regulator_bulk_get - managed get multiple regulator consumers
+ *
+ * @dev: device to supply
+ * @num_consumers: number of consumers to register
+ * @consumers: configuration of consumers; clients are stored here.
+ *
+ * @return 0 on success, an errno on failure.
+ *
+ * This helper function allows drivers to get several regulator
+ * consumers in one operation with management, the regulators will
+ * automatically be freed when the device is unbound. If any of the
+ * regulators cannot be acquired then any regulators that were
+ * allocated will be freed before returning to the caller.
+ */
+int devm_regulator_bulk_get(struct device *dev, int num_consumers,
+ struct regulator_bulk_data *consumers)
+{
+ return _devm_regulator_bulk_get(dev, num_consumers, consumers, NORMAL_GET);
+}
EXPORT_SYMBOL_GPL(devm_regulator_bulk_get);

+/**
+ * devm_regulator_bulk_get_exclusive - managed exclusive get of multiple
+ * regulator consumers
+ *
+ * @dev: device to supply
+ * @num_consumers: number of consumers to register
+ * @consumers: configuration of consumers; clients are stored here.
+ *
+ * @return 0 on success, an errno on failure.
+ *
+ * This helper function allows drivers to exclusively get several
+ * regulator consumers in one operation with management, the regulators
+ * will automatically be freed when the device is unbound. If any of
+ * the regulators cannot be acquired then any regulators that were
+ * allocated will be freed before returning to the caller.
+ */
+int devm_regulator_bulk_get_exclusive(struct device *dev, int num_consumers,
+ struct regulator_bulk_data *consumers)
+{
+ return _devm_regulator_bulk_get(dev, num_consumers, consumers, EXCLUSIVE_GET);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_bulk_get_exclusive);
+
/**
* devm_regulator_bulk_get_const - devm_regulator_bulk_get() w/ const data
*
diff --git a/drivers/regulator/internal.h b/drivers/regulator/internal.h
index 1e9c71642143..fb4433068d29 100644
--- a/drivers/regulator/internal.h
+++ b/drivers/regulator/internal.h
@@ -122,4 +122,6 @@ enum regulator_get_type {

struct regulator *_regulator_get(struct device *dev, const char *id,
enum regulator_get_type get_type);
+int _regulator_bulk_get(struct device *dev, int num_consumers,
+ struct regulator_bulk_data *consumers, enum regulator_get_type get_type);
#endif
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index ee3b4a014611..628a52b8e63f 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -247,6 +247,8 @@ int __must_check regulator_bulk_get(struct device *dev, int num_consumers,
int __must_check devm_regulator_bulk_get(struct device *dev, int num_consumers,
struct regulator_bulk_data *consumers);
void devm_regulator_bulk_put(struct regulator_bulk_data *consumers);
+int __must_check devm_regulator_bulk_get_exclusive(struct device *dev, int num_consumers,
+ struct regulator_bulk_data *consumers);
int __must_check devm_regulator_bulk_get_const(
struct device *dev, int num_consumers,
const struct regulator_bulk_data *in_consumers,
--
2.38.1


2022-11-03 16:26:31

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] regulator: Add DT support for regulator-output connectors

On Mon, 31 Oct 2022 16:37:01 -0700, Zev Weiss wrote:
> This is v2 of my recent series adding support for userspace-controlled
> regulator-supplied power outputs [2]. This is an important feature
> for some kinds of BMC (baseboard management controller) systems where
> the BMC provides an operator with manual control of a set of DC power
> outputs.
>
> As in a broadly similar patchset that was recently almost merged [0],
> this takes the approach of providing support by extending the existing
> userspace-consumer regulator driver. A couple questions about the
> userspace-consumer driver came up along the way, however.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] regulator: devres: Add devm_regulator_bulk_get_exclusive()
commit: fd1845069711cdf1b1aaaa0f22311b7736396331
[2/3] dt-bindings: regulator: Add regulator-output binding
commit: 14b8ad4c2580231fc45c2313ef822a15bb12f63f
[3/3] regulator: userspace-consumer: Handle regulator-output DT nodes
commit: 5c51d4afcf3fd36159713556402e16cfab794ae9

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark