[PATCH v4 0/4] regulator: output-supply DT support

[PATCH v4 0/4] regulator: output-supply DT support

[Naresh Solanki]

v4:
 - Add 9elements as vendor.
 - Update dt binding.

Add devicetree support of the output-supply driver.
The supply names for this driver is provided through DT
regulator-names & regulator handle can be acquired.
Driver events can be received from sysfs.

Laxman Dewangan (1):
  regulator: output-supply: Add devicetree support

Naresh Solanki (2):
  dt-bindings: vendor-prefixes: add 9elements
  dt-bindings: regulator: add bindings for output-supply

Patrick Rudolph (1):
  regulator: output-supply: Add Notification support

 .../regulator/9elements,output-supply.yaml    | 63 ++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 drivers/regulator/userspace-consumer.c        | 97 ++++++++++++++++++-
 3 files changed, 160 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml


base-commit: 9f09069cde34dcd86f5ecf3a3139fd752020812f
prerequisite-patch-id: 0000000000000000000000000000000000000000
-- 
2.35.3

[PATCH v4 1/4] dt-bindings: vendor-prefixes: add 9elements

[Naresh Solanki]

The vendor prefix for 9elements GmbH [1].

[1] https://9elements.com/

Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 0496773a3c4d..2b3c929b1332 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -29,6 +29,8 @@ patternProperties:
     description: 70mai Co., Ltd.
   "^8dev,.*":
     description: 8devices, UAB
+  "^9elements,.*":
+    description: 9elements GmbH
   "^abb,.*":
     description: ABB
   "^abilis,.*":
-- 
2.35.3

[PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Naresh Solanki]

Add a devicetree binding for the 9elements,output-supply driver.
Example is also provided.

Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 .../regulator/9elements,output-supply.yaml    | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml

diff --git a/Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml b/Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml
new file mode 100644
index 000000000000..e4b67bc0794f
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/userspace-consumer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulator output-supply driver
+
+maintainers:
+  - Patrick Rudolph <patrick.rudolph@9elements.com>
+
+description: |
+  Regulator output-supply driver helps interact with regulator.
+  Provides regulator events & sysfs notify to capture regulator events
+  in realtime for userspace application.
+  This enables userspace application to monitor events in regulator(s) &
+  handle them appropriately.
+
+
+properties:
+  compatible:
+    enum:
+      - 9elements,output-supply
+
+  regulator-name:
+    description: Name of the consumer line
+
+  regulator-boot-on:
+    description: Enable regulator during boot
+
+  regulator-supplies:
+    description: Supply names for this regulator. This can be multiple strings
+
+patternProperties:
+  ".*-supply$":
+    description: |
+      Input supply phandle(s) for this node. There should be
+      <supply-name>-supply to pass regulators handle
+
+required:
+  - compatible
+  - regulator-supplies
+  - ".*-supply$"
+
+additionalProperties: false
+
+examples:
+  - |
+    p12v_b_consumer {
+        compatible = "9elements,output-supply";
+        regulator-name = "BCM1";
+        regulator-supplies = "vbus";
+        vbus-supply = <&p12v_b>;
+    };
+
+    ssb_rssd32 {
+        compatible = "9elements,output-supply";
+        regulator-name = "ssb_rssd32";
+        regulator-supplies = "sw0", "sw1";
+        sw0-supply = <&sw0_ssb_rssd32>;
+        sw1-supply = <&sw1_ssb_rssd32>;
+    };
+...
-- 
2.35.3

[PATCH v4 3/4] regulator: output-supply: Add devicetree support

[Naresh Solanki]

From: Laxman Dewangan <ldewangan@nvidia.com>

Add DT support of the regulator driver output-supply.
The supply names for this driver is provided through DT properties
so that proper regulator(s) handle can be acquired.

Board DT can configure the output-supply driver to
* set regulator name
* turn on at boot time
* specific regulator handles that needs to be controlled by the driver.

Post boot facilitate control of regulator(s) from sysfs.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/regulator/userspace-consumer.c | 47 ++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/regulator/userspace-consumer.c b/drivers/regulator/userspace-consumer.c
index 8ca28664776e..b424a0ddf582 100644
--- a/drivers/regulator/userspace-consumer.c
+++ b/drivers/regulator/userspace-consumer.c
@@ -18,6 +18,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/regulator/userspace-consumer.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 struct userspace_consumer_data {
 	const char *name;
@@ -100,6 +101,40 @@ static const struct attribute_group attr_group = {
 	.attrs	= attributes,
 };
 
+static struct regulator_userspace_consumer_data *get_pdata_from_dt_node(
+		struct platform_device *pdev)
+{
+	struct regulator_userspace_consumer_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
+	struct property *prop;
+	const char *supply;
+	int num_supplies;
+	int count = 0;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	pdata->name = of_get_property(np, "regulator-name", NULL);
+	pdata->init_on = of_property_read_bool(np, "regulator-boot-on");
+	num_supplies = of_property_count_strings(np, "regulator-supplies");
+	if (num_supplies < 0) {
+		dev_err(&pdev->dev,
+			"could not parse property regulator-supplies\n");
+		return ERR_PTR(-EINVAL);
+	}
+	pdata->num_supplies = num_supplies;
+	pdata->supplies = devm_kzalloc(&pdev->dev, num_supplies *
+				sizeof(*pdata->supplies), GFP_KERNEL);
+	if (!pdata->supplies)
+		return ERR_PTR(-ENOMEM);
+
+	of_property_for_each_string(np, "regulator-supplies", prop, supply)
+		pdata->supplies[count++].supply = supply;
+
+	return pdata;
+}
+
 static int regulator_userspace_consumer_probe(struct platform_device *pdev)
 {
 	struct regulator_userspace_consumer_data *pdata;
@@ -107,6 +142,11 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
 	int ret;
 
 	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata && pdev->dev.of_node) {
+		pdata = get_pdata_from_dt_node(pdev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
 	if (!pdata)
 		return -EINVAL;
 
@@ -166,11 +206,18 @@ static int regulator_userspace_consumer_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id regulator_userspace_consumer_of_match[] = {
+	{ .compatible = "9elements,output-supply", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, regulator_userspace_consumer_of_match);
+
 static struct platform_driver regulator_userspace_consumer_driver = {
 	.probe		= regulator_userspace_consumer_probe,
 	.remove		= regulator_userspace_consumer_remove,
 	.driver		= {
 		.name		= "reg-userspace-consumer",
+		.of_match_table = regulator_userspace_consumer_of_match,
 	},
 };
 
-- 
2.35.3

[PATCH v4 4/4] regulator: output-supply: Add Notification support

[Naresh Solanki]

From: Patrick Rudolph <patrick.rudolph@9elements.com>

Add notification support to output-supply driver to receive regulator
events.

During runtime, regulator may encounter events like over/under voltage,
over current etc. & its useful to report to userspace to take necessary
corrective action.

If sysfs notification is desired then that can be enabled using
regulator-notify-enable property.
This is very useful in situation wherein immediate responsive action
needs to be taken upon receiving events & delayed response is not
acceptable depending on criticality of the received event.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/regulator/userspace-consumer.c | 50 ++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/userspace-consumer.c b/drivers/regulator/userspace-consumer.c
index b424a0ddf582..0cf2724b5c37 100644
--- a/drivers/regulator/userspace-consumer.c
+++ b/drivers/regulator/userspace-consumer.c
@@ -22,14 +22,33 @@
 
 struct userspace_consumer_data {
 	const char *name;
-
+	bool notify_enable;
+	struct notifier_block nb;
 	struct mutex lock;
 	bool enabled;
+	unsigned long events;
+	struct kobject *kobj;
 
 	int num_supplies;
 	struct regulator_bulk_data *supplies;
 };
 
+static DEFINE_MUTEX(events_lock);
+
+static ssize_t events_show(struct device *dev,
+			  struct device_attribute *attr, char *buf)
+{
+	struct userspace_consumer_data *data = dev_get_drvdata(dev);
+	unsigned long e;
+
+	mutex_lock(&events_lock);
+	e = data->events;
+	data->events = 0;
+	mutex_unlock(&events_lock);
+
+	return sprintf(buf, "0x%lx\n", e);
+}
+
 static ssize_t name_show(struct device *dev,
 			 struct device_attribute *attr, char *buf)
 {
@@ -88,12 +107,14 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static DEVICE_ATTR_RO(events);
 static DEVICE_ATTR_RO(name);
 static DEVICE_ATTR_RW(state);
 
 static struct attribute *attributes[] = {
 	&dev_attr_name.attr,
 	&dev_attr_state.attr,
+	&dev_attr_events.attr,
 	NULL,
 };
 
@@ -101,6 +122,22 @@ static const struct attribute_group attr_group = {
 	.attrs	= attributes,
 };
 
+static int regulator_userspace_notify(struct notifier_block *nb,
+				      unsigned long event,
+				      void *ignored)
+{
+	struct userspace_consumer_data *data =
+		container_of(nb, struct userspace_consumer_data, nb);
+
+	mutex_lock(&events_lock);
+	data->events |= event;
+	mutex_unlock(&events_lock);
+
+	sysfs_notify(data->kobj, NULL, dev_attr_events.attr.name);
+
+	return NOTIFY_OK;
+}
+
 static struct regulator_userspace_consumer_data *get_pdata_from_dt_node(
 		struct platform_device *pdev)
 {
@@ -139,7 +176,7 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
 {
 	struct regulator_userspace_consumer_data *pdata;
 	struct userspace_consumer_data *drvdata;
-	int ret;
+	int ret, i;
 
 	pdata = dev_get_platdata(&pdev->dev);
 	if (!pdata && pdev->dev.of_node) {
@@ -159,6 +196,7 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
 	drvdata->name = pdata->name;
 	drvdata->num_supplies = pdata->num_supplies;
 	drvdata->supplies = pdata->supplies;
+	drvdata->kobj = &pdev->dev.kobj;
 
 	mutex_init(&drvdata->lock);
 
@@ -183,7 +221,15 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
 		}
 	}
 
+	drvdata->nb.notifier_call = regulator_userspace_notify;
+	for (i = 0; i < drvdata->num_supplies; i++) {
+		ret = devm_regulator_register_notifier(drvdata->supplies[i].consumer, &drvdata->nb);
+		if (ret)
+			goto err_enable;
+	}
+
 	drvdata->enabled = pdata->init_on;
+
 	platform_set_drvdata(pdev, drvdata);
 
 	return 0;
-- 
2.35.3

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

On Thu, Jul 07, 2022 at 10:18:24AM +0200, Naresh Solanki wrote:
> Add a devicetree binding for the 9elements,output-supply driver.
> Example is also provided.

It's fine this time but in general if you're sending a patch series you
should always ensure that at least the cover letter goes to all the
relevant maintainers so that they know what's going on with
dependencies.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 0/4] regulator: output-supply DT support

[Mark Brown]

On Thu, 7 Jul 2022 10:18:22 +0200, Naresh Solanki wrote:
> v4:
>  - Add 9elements as vendor.
>  - Update dt binding.
> 
> Add devicetree support of the output-supply driver.
> The supply names for this driver is provided through DT
> regulator-names & regulator handle can be acquired.
> Driver events can be received from sysfs.
> 
> [...]

Applied to

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

Thanks!

[1/4] dt-bindings: vendor-prefixes: add 9elements
      commit: bd9b7998b4816b3d604253a774d83b6736474283
[2/4] dt-bindings: regulator: add bindings for output-supply
      commit: df9c96b69a1d2629519a5d8a9dc4b39f775ebe2d
[3/4] regulator: output-supply: Add devicetree support
      commit: 34e5700e1e64077ede50eb60d04e7604dc4f508a
[4/4] regulator: output-supply: Add Notification support
      commit: 490a15324ce6d55b950fce0eb9e95c793fac0dff

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

Re: [PATCH v4 1/4] dt-bindings: vendor-prefixes: add 9elements

[Rob Herring]

On Thu, Jul 7, 2022 at 2:19 AM Naresh Solanki
<naresh.solanki@9elements.com> wrote:
>
> The vendor prefix for 9elements GmbH [1].

Use get_maintainers.pl to send to the right lists if you want this
reviewed and/or applied.

>
> [1] https://9elements.com/
>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 0496773a3c4d..2b3c929b1332 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -29,6 +29,8 @@ patternProperties:
>      description: 70mai Co., Ltd.
>    "^8dev,.*":
>      description: 8devices, UAB
> +  "^9elements,.*":
> +    description: 9elements GmbH
>    "^abb,.*":
>      description: ABB
>    "^abilis,.*":
> --
> 2.35.3
>

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Rob Herring]

On Thu, Jul 07, 2022 at 10:18:24AM +0200, Naresh Solanki wrote:
> Add a devicetree binding for the 9elements,output-supply driver.
> Example is also provided.
> 
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> ---
>  .../regulator/9elements,output-supply.yaml    | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml

Now failing in linux-next. Please send an incremental patch to fix (and 
test it yourself):

/builds/robherring/linux-dt/Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml: required:2: '.*-supply$' does not match '^([a-zA-Z#][a-zA-Z0-9,+\\-._@]{0,63}|\\$nodename)$'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
./Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/regulator/9elements,output-supply.yaml#

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Rob Herring]

On Thu, Jul 07, 2022 at 10:18:24AM +0200, Naresh Solanki wrote:
> Add a devicetree binding for the 9elements,output-supply driver.

Bindings are for h/w, not drivers.

> Example is also provided.
> 
> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> ---
>  .../regulator/9elements,output-supply.yaml    | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml b/Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml
> new file mode 100644
> index 000000000000..e4b67bc0794f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/9elements,output-supply.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/userspace-consumer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Regulator output-supply driver
> +
> +maintainers:
> +  - Patrick Rudolph <patrick.rudolph@9elements.com>
> +
> +description: |
> +  Regulator output-supply driver helps interact with regulator.
> +  Provides regulator events & sysfs notify to capture regulator events
> +  in realtime for userspace application.
> +  This enables userspace application to monitor events in regulator(s) &
> +  handle them appropriately.

What does userspace have to do with hardware description?

> +
> +
> +properties:
> +  compatible:
> +    enum:
> +      - 9elements,output-supply

Why does this have a vendor prefix when it appears to be a s/w 
construct?

> +
> +  regulator-name:
> +    description: Name of the consumer line
> +
> +  regulator-boot-on:
> +    description: Enable regulator during boot
> +
> +  regulator-supplies:
> +    description: Supply names for this regulator. This can be multiple strings

What's the type for this? 

The supply names are the prefix on -supply properties.

> +
> +patternProperties:
> +  ".*-supply$":
> +    description: |
> +      Input supply phandle(s) for this node. There should be
> +      <supply-name>-supply to pass regulators handle
> +
> +required:
> +  - compatible
> +  - regulator-supplies
> +  - ".*-supply$"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    p12v_b_consumer {
> +        compatible = "9elements,output-supply";
> +        regulator-name = "BCM1";
> +        regulator-supplies = "vbus";
> +        vbus-supply = <&p12v_b>;
> +    };
> +
> +    ssb_rssd32 {
> +        compatible = "9elements,output-supply";
> +        regulator-name = "ssb_rssd32";
> +        regulator-supplies = "sw0", "sw1";
> +        sw0-supply = <&sw0_ssb_rssd32>;
> +        sw1-supply = <&sw1_ssb_rssd32>;
> +    };
> +...
> -- 
> 2.35.3
> 
> 

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On Thu, Jul 14, 2022 at 08:10:41AM -0600, Rob Herring wrote:
> On Thu, Jul 07, 2022 at 10:18:24AM +0200, Naresh Solanki wrote:
> > Add a devicetree binding for the 9elements,output-supply driver.

> > +properties:
> > +  compatible:
> > +    enum:
> > +      - 9elements,output-supply

> Why does this have a vendor prefix when it appears to be a s/w 
> construct?

It's a description of a power supply output from their system (system as
a whole, not power provisioned within the system).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 0/4] regulator: output-supply DT support

[Rob Herring]

On Mon, Jul 11, 2022 at 08:11:02PM +0100, Mark Brown wrote:
> On Thu, 7 Jul 2022 10:18:22 +0200, Naresh Solanki wrote:
> > v4:
> >  - Add 9elements as vendor.
> >  - Update dt binding.
> > 
> > Add devicetree support of the output-supply driver.
> > The supply names for this driver is provided through DT
> > regulator-names & regulator handle can be acquired.
> > Driver events can be received from sysfs.
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
> 
> Thanks!
> 
> [1/4] dt-bindings: vendor-prefixes: add 9elements
>       commit: bd9b7998b4816b3d604253a774d83b6736474283
> [2/4] dt-bindings: regulator: add bindings for output-supply
>       commit: df9c96b69a1d2629519a5d8a9dc4b39f775ebe2d
> [3/4] regulator: output-supply: Add devicetree support
>       commit: 34e5700e1e64077ede50eb60d04e7604dc4f508a
> [4/4] regulator: output-supply: Add Notification support
>       commit: 490a15324ce6d55b950fce0eb9e95c793fac0dff

Please revert this. The vendor prefix is not even appropriate here and 
the binding has lots of issues. The first being whether it should even 
be in DT. It's not been reviewed as none of it has gone to the DT list 
and the only thing I got was '[PATCH v4 1/4] dt-bindings: 
vendor-prefixes: add 9elements'.

Rob

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Rob Herring]

On Thu, Jul 14, 2022 at 8:14 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Jul 14, 2022 at 08:10:41AM -0600, Rob Herring wrote:
> > On Thu, Jul 07, 2022 at 10:18:24AM +0200, Naresh Solanki wrote:
> > > Add a devicetree binding for the 9elements,output-supply driver.
>
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - 9elements,output-supply
>
> > Why does this have a vendor prefix when it appears to be a s/w
> > construct?
>
> It's a description of a power supply output from their system (system as
> a whole, not power provisioned within the system).

Well, that's a better commit message than the original, but I still
don't understand.

Rob

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

On Thu, Jul 14, 2022 at 08:23:39AM -0600, Rob Herring wrote:
> On Thu, Jul 14, 2022 at 8:14 AM Mark Brown <broonie@kernel.org> wrote:

> > It's a description of a power supply output from their system (system as
> > a whole, not power provisioned within the system).

> Well, that's a better commit message than the original, but I still
> don't understand.

Consider for example a BMC (IIRC that's what their specific product is),
a bench supply or some automated test equipment.  Part of the function
for these systems is to provide power to other systems which would be
represented as a root or wall supply in the description of the system
that actually uses the supply if it were described using DT.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Rob Herring]

On Thu, Jul 14, 2022 at 8:43 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Jul 14, 2022 at 08:23:39AM -0600, Rob Herring wrote:
> > On Thu, Jul 14, 2022 at 8:14 AM Mark Brown <broonie@kernel.org> wrote:
>
> > > It's a description of a power supply output from their system (system as
> > > a whole, not power provisioned within the system).
>
> > Well, that's a better commit message than the original, but I still
> > don't understand.
>
> Consider for example a BMC (IIRC that's what their specific product is),
> a bench supply or some automated test equipment.  Part of the function
> for these systems is to provide power to other systems which would be
> represented as a root or wall supply in the description of the system
> that actually uses the supply if it were described using DT.

Didn't someone else have a similar use recently? Controlling some
supply external to the system. I can't seem to find it now.

In any case, it's not for you to describe, but Naresh, and in the
binding and commit messages. But first we need to overcome proper
usage of get_maintainers.pl. In response, to my first reply on v4, I
have a second v4 sent privately today (and still only the vendor
prefix). Sigh. AFAICT, for v1-v3, the only thing that made it to the
list was the cover letters. Bottom line is this series has multiple
problems and shouldn't have been applied yet.

Rob

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

On Thu, Jul 14, 2022 at 09:07:49AM -0600, Rob Herring wrote:
> On Thu, Jul 14, 2022 at 8:43 AM Mark Brown <broonie@kernel.org> wrote:

> > Consider for example a BMC (IIRC that's what their specific product is),
> > a bench supply or some automated test equipment.  Part of the function
> > for these systems is to provide power to other systems which would be
> > represented as a root or wall supply in the description of the system
> > that actually uses the supply if it were described using DT.

> Didn't someone else have a similar use recently? Controlling some
> supply external to the system. I can't seem to find it now.

IIRC that was an earlier iteration of the same thing - it's been round
the houses a bit.  extcon seemed like it might be a home since these are
external connections from the system but in the end people didn't think
it looked like a good fit.

> In any case, it's not for you to describe, but Naresh, and in the
> binding and commit messages. But first we need to overcome proper
> usage of get_maintainers.pl. In response, to my first reply on v4, I
> have a second v4 sent privately today (and still only the vendor
> prefix). Sigh. AFAICT, for v1-v3, the only thing that made it to the
> list was the cover letters. Bottom line is this series has multiple
> problems and shouldn't have been applied yet.

I can drop it but I do think it's reasonable to be adding a vendor
binding for this, we don't seem to have enough people engaged to scope
out a generic binding confidently and TBH I've got a feeling we might
want multiple application specific generic bindings when we do have one.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Rob Herring]

+Zev

On Thu, Jul 14, 2022 at 9:54 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Jul 14, 2022 at 09:07:49AM -0600, Rob Herring wrote:
> > On Thu, Jul 14, 2022 at 8:43 AM Mark Brown <broonie@kernel.org> wrote:
>
> > > Consider for example a BMC (IIRC that's what their specific product is),
> > > a bench supply or some automated test equipment.  Part of the function
> > > for these systems is to provide power to other systems which would be
> > > represented as a root or wall supply in the description of the system
> > > that actually uses the supply if it were described using DT.
>
> > Didn't someone else have a similar use recently? Controlling some
> > supply external to the system. I can't seem to find it now.
>
> IIRC that was an earlier iteration of the same thing - it's been round
> the houses a bit.  extcon seemed like it might be a home since these are
> external connections from the system but in the end people didn't think
> it looked like a good fit.

Found it:

v1: https://lore.kernel.org/all/20220504065252.6955-2-zev@bewilderbeest.net/
v2: https://lore.kernel.org/all/20220505232557.10936-1-zev@bewilderbeest.net/

v2 was the using extcon version. v1 looks pretty similar to this one
though anything that's just a compatible plus supplies would.

But AFAICT these 2 submissions are completely independent.

> > In any case, it's not for you to describe, but Naresh, and in the
> > binding and commit messages. But first we need to overcome proper
> > usage of get_maintainers.pl. In response, to my first reply on v4, I
> > have a second v4 sent privately today (and still only the vendor
> > prefix). Sigh. AFAICT, for v1-v3, the only thing that made it to the
> > list was the cover letters. Bottom line is this series has multiple
> > problems and shouldn't have been applied yet.
>
> I can drop it but I do think it's reasonable to be adding a vendor
> binding for this, we don't seem to have enough people engaged to scope
> out a generic binding confidently and TBH I've got a feeling we might
> want multiple application specific generic bindings when we do have one.

I don't mind the vendor prefix. I mind the vendor prefix without any
description of the vendor's h/w.

Rob

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]

On Thu, Jul 14, 2022 at 10:59:48AM -0600, Rob Herring wrote:
> On Thu, Jul 14, 2022 at 9:54 AM Mark Brown <broonie@kernel.org> wrote:
> > On Thu, Jul 14, 2022 at 09:07:49AM -0600, Rob Herring wrote:
> > > On Thu, Jul 14, 2022 at 8:43 AM Mark Brown <broonie@kernel.org> wrote:

> > IIRC that was an earlier iteration of the same thing - it's been round
> > the houses a bit.  extcon seemed like it might be a home since these are
> > external connections from the system but in the end people didn't think
> > it looked like a good fit.

> Found it:

> v1: https://lore.kernel.org/all/20220504065252.6955-2-zev@bewilderbeest.net/
> v2: https://lore.kernel.org/all/20220505232557.10936-1-zev@bewilderbeest.net/

> v2 was the using extcon version. v1 looks pretty similar to this one
> though anything that's just a compatible plus supplies would.

> But AFAICT these 2 submissions are completely independent.

Yes, I think so - I'd misremembered as being the same thing due to the
serialisation.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 0/4] regulator: output-supply DT support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

On Thu, Jul 14, 2022 at 08:17:12AM -0600, Rob Herring wrote:

> Please revert this. The vendor prefix is not even appropriate here and 
> the binding has lots of issues. The first being whether it should even 
> be in DT. It's not been reviewed as none of it has gone to the DT list 
> and the only thing I got was '[PATCH v4 1/4] dt-bindings: 
> vendor-prefixes: add 9elements'.

As discussed on the other subthread while some of this might not be an
actual problem others are causing serious concern for Rob so I've
dropped this series, it was the top commit so I just reverted the merge.
Sorry about the hassle here, hopefully the issues Rob has raised can be
addressed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Zev Weiss]

On Thu, Jul 14, 2022 at 09:59:48AM PDT, Rob Herring wrote:
>+Zev
>
>On Thu, Jul 14, 2022 at 9:54 AM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Thu, Jul 14, 2022 at 09:07:49AM -0600, Rob Herring wrote:
>> > On Thu, Jul 14, 2022 at 8:43 AM Mark Brown <broonie@kernel.org> wrote:
>>
>> > > Consider for example a BMC (IIRC that's what their specific product is),
>> > > a bench supply or some automated test equipment.  Part of the function
>> > > for these systems is to provide power to other systems which would be
>> > > represented as a root or wall supply in the description of the system
>> > > that actually uses the supply if it were described using DT.
>>
>> > Didn't someone else have a similar use recently? Controlling some
>> > supply external to the system. I can't seem to find it now.
>>
>> IIRC that was an earlier iteration of the same thing - it's been round
>> the houses a bit.  extcon seemed like it might be a home since these are
>> external connections from the system but in the end people didn't think
>> it looked like a good fit.
>
>Found it:
>
>v1: https://lore.kernel.org/all/20220504065252.6955-2-zev@bewilderbeest.net/
>v2: https://lore.kernel.org/all/20220505232557.10936-1-zev@bewilderbeest.net/
>
>v2 was the using extcon version. v1 looks pretty similar to this one
>though anything that's just a compatible plus supplies would.
>
>But AFAICT these 2 submissions are completely independent.
>

Thanks for looping me in here Rob, I hadn't been aware of this series.

Naresh, I'd appreciate it if you could CC me on any subsequent 
iterations; as linked above I've made some sporadic attempts at getting 
support for this functionality merged, and am certainly interested in 
any other efforts on that front.

A question for Mark though -- in one of my earliest stabs at this I 
floated the idea of using reg-userspace-consumer for it, but was told in 
no uncertain terms that that driver was for testing only and should 
under no circumstances ever be instantiated in a production system.  Has 
the thinking on its usage changed in the last year or so such that this 
approach was deemed okay?


Thanks,
Zev

Re: [PATCH v4 2/4] dt-bindings: regulator: add bindings for output-supply

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

On Thu, Jul 21, 2022 at 01:33:20AM -0700, Zev Weiss wrote:

> A question for Mark though -- in one of my earliest stabs at this I floated
> the idea of using reg-userspace-consumer for it, but was told in no
> uncertain terms that that driver was for testing only and should under no
> circumstances ever be instantiated in a production system.  Has the thinking
> on its usage changed in the last year or so such that this approach was
> deemed okay?

I was talked around after some discussion of the use case.  It's still
not a great API but for at least some of these users it'll do.  It's
important that it not appear directly in bindings though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]