[PATCH v2 0/2] misc: Add power-efuse driver

[PATCH v2 0/2] misc: Add power-efuse driver

[Zev Weiss]

Hello,

This is a second revision of the v1 patch series posted previously
[2].

Changes since v1:
 - dropped pmbus/lm25066 patches, submitted separately via hwmon tree [Guenter]
 - fixed indentation in power-efuse.yaml [Rob's bot]
 - added more detailed description in power-efuse.yaml [Rob]
 - added sysfs ABI documentation [Greg]
 - replaced manual struct attribute_group and sysfs_create_group()
   with ATTRIBUTE_GROUPS() and driver.dev_groups [Greg]

A lightly edited version of the previous cover letter follows.


This patch series is another incarnation of some previous efforts [0]
at enabling userspace access to the OPERATION state (and now status
flags) of PMBus devices, specifically with respect to efuses
protecting general-purpose power outputs.  This functionality is an
important component enabling a port of OpenBMC to the Delta AHE-50DC
Open19 power shelf [1].

The first patch adds dt-bindings, and the second adds the
implementation of the power-efuse driver.  The driver is fairly
simple; it merely provides a sysfs interface to enable, disable, and
retrieve error flags from an underlying regulator (which in the
expected usage will most likely be a PMBus device).

There is one aspect of its usage of the regulator API I'm a bit
uncertain about, however: this driver seems like a case where an
exclusive 'get' of the regulator (i.e. devm_regulator_get_exclusive()
instead of devm_regulator_get() in efuse_probe()) would be
appropriate, since in the intended usage no other device should be
using an efuse's regulator.  With an exclusive get though, the
regulator's use_count and the consumer's enable_count don't balance
out properly to allow the enable/disable operations to work properly
(the former ending up one more than the latter, leading to
enable_count underflows on attempts to disable the regulator).  So at
least for now it's using a non-exclusive get -- I'd be happy to hear
any pointers on a way to allow an exclusive get to work here, though.


Thanks,
Zev

[0] https://lore.kernel.org/openbmc/YGLepYLvtlO6Ikzs@hatter.bewilderbeest.net/
[1] https://www.open19.org/marketplace/delta-16kw-power-shelf/
[2] https://lore.kernel.org/openbmc/20220217104444.7695-1-zev@bewilderbeest.net/

Zev Weiss (2):
  dt-bindings: Add power-efuse binding
  misc: Add power-efuse driver

 .../ABI/testing/sysfs-driver-power-efuse      |  32 +++
 .../devicetree/bindings/misc/power-efuse.yaml |  49 ++++
 MAINTAINERS                                   |   5 +
 drivers/misc/Kconfig                          |  15 ++
 drivers/misc/Makefile                         |   1 +
 drivers/misc/power-efuse.c                    | 212 ++++++++++++++++++
 6 files changed, 314 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-power-efuse
 create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
 create mode 100644 drivers/misc/power-efuse.c

-- 
2.35.1

[PATCH v2 1/2] dt-bindings: Add power-efuse binding

[Zev Weiss]

This can be used to describe a power output supplied by a regulator
device that the system controls.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 .../devicetree/bindings/misc/power-efuse.yaml | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml

diff --git a/Documentation/devicetree/bindings/misc/power-efuse.yaml b/Documentation/devicetree/bindings/misc/power-efuse.yaml
new file mode 100644
index 000000000000..5f8f0b21af0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/power-efuse.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic power efuse device
+
+maintainers:
+  - Zev Weiss <zev@bewilderbeest.net>
+
+description: |
+  This binding describes a physical power output supplied by a
+  regulator providing efuse functionality (manual on/off control, and
+  auto-shutoff if current, voltage, or thermal limits are exceeded).
+
+  These may be found on systems such as "smart" network PDUs, and
+  typically supply power to devices entirely separate from the system
+  described by the device-tree by way of an external connector such as
+  an Open19 power cable:
+
+  https://www.open19.org/marketplace/coolpower-cable-assembly-8ru/
+
+properties:
+  compatible:
+    const: power-efuse
+
+  vout-supply:
+    description:
+      phandle to the regulator providing power for the efuse
+
+  error-flags-cache-ttl-ms:
+    description:
+      The number of milliseconds the vout-supply regulator's error
+      flags should be cached before re-fetching them.
+
+required:
+  - compatible
+  - vout-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    efuse {
+        compatible = "power-efuse";
+        vout-supply = <&efuse_reg>;
+        error-flags-cache-ttl-ms = <500>;
+    };
-- 
2.35.1

[PATCH v2 2/2] misc: Add power-efuse driver

[Zev Weiss]

This driver provides a sysfs interface to access the on/off state and
error flags of a regulator supplying a power output controlled by the
system.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 .../ABI/testing/sysfs-driver-power-efuse      |  32 +++
 MAINTAINERS                                   |   5 +
 drivers/misc/Kconfig                          |  15 ++
 drivers/misc/Makefile                         |   1 +
 drivers/misc/power-efuse.c                    | 212 ++++++++++++++++++
 5 files changed, 265 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-power-efuse
 create mode 100644 drivers/misc/power-efuse.c

diff --git a/Documentation/ABI/testing/sysfs-driver-power-efuse b/Documentation/ABI/testing/sysfs-driver-power-efuse
new file mode 100644
index 000000000000..ff9bb53e012a
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-power-efuse
@@ -0,0 +1,32 @@
+What:		/sys/bus/platform/drivers/power-efuse/\*/operstate
+Date:		February 2022
+KernelVersion:	5.18
+Contact:	Zev Weiss <zev@bewilderbeest.net
+Description:	The operating state of the efuse as either "on" or "off"; either
+		value can be written to set the operating state.
+Users:		OpenBMC <openbmc@lists.ozlabs.org>
+
+What:		/sys/bus/platform/drivers/power-efuse/\*/fail
+Date:		February 2022
+KernelVersion:	5.18
+Contact:	Zev Weiss <zev@bewilderbeest.net
+Description:	0 if the efuse is operating normally, or 1 if it has tripped
+		(operating state set to "on" but the hardware has shut off the
+		output due to a fault).
+Users:		OpenBMC <openbmc@lists.ozlabs.org>
+
+What:		/sys/bus/platform/drivers/power-efuse/\*/{under_voltage,over_current,regulation_out,over_temp}
+Date:		February 2022
+KernelVersion:	5.18
+Contact:	Zev Weiss <zev@bewilderbeest.net
+Description:	1 if the underlying regulator reports a fault of the type
+		indicated by the name of the file, 0 otherwise.
+Users:		OpenBMC <openbmc@lists.ozlabs.org>
+
+What:		/sys/bus/platform/drivers/power-efuse/\*/{under_voltage,over_current,over_voltage,over_temp}_warn
+Date:		February 2022
+KernelVersion:	5.18
+Contact:	Zev Weiss <zev@bewilderbeest.net
+Description:	1 if the underlying regulator reports a warning of the type
+		indicated by the name of the file, 0 otherwise.
+Users:		OpenBMC <openbmc@lists.ozlabs.org>
diff --git a/MAINTAINERS b/MAINTAINERS
index 05fd080b82f3..6490b09b43d4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7058,6 +7058,11 @@ S:	Orphan
 W:	http://aeschi.ch.eu.org/efs/
 F:	fs/efs/
 
+POWER EFUSE DRIVER
+M:	Zev Weiss <zev@bewilderbeest.net>
+S:	Maintained
+F:	drivers/misc/power-efuse.c
+
 EHEA (IBM pSeries eHEA 10Gb ethernet adapter) DRIVER
 M:	Douglas Miller <dougmill@linux.ibm.com>
 L:	netdev@vger.kernel.org
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 0f5a49fc7c9e..45fc3e8ad35d 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -470,6 +470,21 @@ config HISI_HIKEY_USB
 	  switching between the dual-role USB-C port and the USB-A host ports
 	  using only one USB controller.
 
+config POWER_EFUSE
+	tristate "Power efuse driver support"
+	depends on OF && REGULATOR
+	help
+	  This driver supports a regulator device functioning as a
+	  power efuse, with status bits and an on/off switch available
+	  via sysfs.
+
+	  A typical use for this would be for an efuse controlling a
+	  generic power output for supplying power to devices external
+	  to the system running this driver (such as in the management
+	  controller of a "smart" PDU or similar), allowing the
+	  operator to manually turn the output on and off, check if
+	  the efuse has tripped due to overload, etc.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index a086197af544..7bd784b89ef8 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -59,3 +59,4 @@ obj-$(CONFIG_UACCE)		+= uacce/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
 obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
 obj-$(CONFIG_HI6421V600_IRQ)	+= hi6421v600-irq.o
+obj-$(CONFIG_POWER_EFUSE)	+= power-efuse.o
diff --git a/drivers/misc/power-efuse.c b/drivers/misc/power-efuse.c
new file mode 100644
index 000000000000..e9d2feb668f2
--- /dev/null
+++ b/drivers/misc/power-efuse.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This module provides a thin wrapper around a regulator device that exposes
+ * status bits and on/off state via sysfs.
+ *
+ * Copyright (C) 2022 Zev Weiss <zev@bewilderbeest.net>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct efuse {
+	struct regulator *reg;
+	struct {
+		unsigned int cache;
+		unsigned long ttl;
+		unsigned long fetch_time;
+		struct mutex lock;
+	} error_flags;
+};
+
+/* Ensure that the next error_flags access fetches them from the device */
+static void efuse_invalidate_error_flags(struct efuse *efuse)
+{
+	mutex_lock(&efuse->error_flags.lock);
+	efuse->error_flags.fetch_time = 0;
+	mutex_unlock(&efuse->error_flags.lock);
+}
+
+static ssize_t efuse_show_operstate(struct device *dev, struct device_attribute *attr,
+				    char *buf)
+{
+	struct efuse *efuse = dev_get_drvdata(dev);
+	int status = regulator_is_enabled(efuse->reg);
+
+	if (status < 0)
+		return status;
+
+	return sysfs_emit(buf, "%s\n", status ? "on" : "off");
+}
+
+static ssize_t efuse_set_operstate(struct device *dev, struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int status, wantstate;
+	struct efuse *efuse = dev_get_drvdata(dev);
+	struct regulator *reg = efuse->reg;
+
+	if (sysfs_streq(buf, "on"))
+		wantstate = 1;
+	else if (sysfs_streq(buf, "off"))
+		wantstate = 0;
+	else
+		return -EINVAL;
+
+	status = regulator_is_enabled(reg);
+
+	/*
+	 * We need to ensure our enable/disable calls don't get imbalanced, so
+	 * bail if we can't determine the current state.
+	 */
+	if (status < 0)
+		return status;
+
+	/* Return early if we're already in the desired state */
+	if (!!status == wantstate)
+		return count;
+
+	if (wantstate)
+		status = regulator_enable(reg);
+	else
+		status = regulator_disable(reg);
+
+	/*
+	 * Toggling operstate can reset latched status flags, so invalidate
+	 * the cached value.
+	 */
+	efuse_invalidate_error_flags(efuse);
+
+	if (!status && regulator_is_enabled(reg) != wantstate) {
+		/*
+		 * We could do
+		 *
+		 *   if (!wantstate)
+		 *     regulator_force_disable(reg);
+		 *
+		 * here, but it's likely to leave it such that it can't then
+		 * be re-enabled, so we'll just report the error and leave it
+		 * as it is (and hopefully as long as our enable/disable calls
+		 * remain balanced and nobody registers another consumer for
+		 * the same supply we won't end up in this situation anyway).
+		 */
+		dev_err(dev, "regulator_%sable() didn't take effect\n", wantstate ? "en" : "dis");
+		status = -EIO;
+	}
+
+	return status ? : count;
+}
+
+static int efuse_update_error_flags(struct efuse *efuse)
+{
+	int status = 0;
+	unsigned long cache_expiry;
+
+	mutex_lock(&efuse->error_flags.lock);
+
+	cache_expiry = efuse->error_flags.fetch_time + efuse->error_flags.ttl;
+
+	if (!efuse->error_flags.ttl || !efuse->error_flags.fetch_time ||
+	    time_after(jiffies, cache_expiry)) {
+		status = regulator_get_error_flags(efuse->reg, &efuse->error_flags.cache);
+		if (!status)
+			efuse->error_flags.fetch_time = jiffies;
+	}
+
+	mutex_unlock(&efuse->error_flags.lock);
+
+	return status;
+}
+
+static DEVICE_ATTR(operstate, 0644, efuse_show_operstate, efuse_set_operstate);
+
+#define EFUSE_ERROR_ATTR(name, bit)							    \
+	static ssize_t efuse_show_##name(struct device *dev, struct device_attribute *attr, \
+					 char *buf)                                         \
+	{                                                                                   \
+		struct efuse *efuse = dev_get_drvdata(dev);                                 \
+		int status = efuse_update_error_flags(efuse);                               \
+		if (status)                                                                 \
+			return status;                                                      \
+		return sysfs_emit(buf, "%d\n", !!(efuse->error_flags.cache & bit));         \
+	}                                                                                   \
+	static DEVICE_ATTR(name, 0444, efuse_show_##name, NULL)
+
+EFUSE_ERROR_ATTR(under_voltage, REGULATOR_ERROR_UNDER_VOLTAGE);
+EFUSE_ERROR_ATTR(over_current, REGULATOR_ERROR_OVER_CURRENT);
+EFUSE_ERROR_ATTR(regulation_out, REGULATOR_ERROR_REGULATION_OUT);
+EFUSE_ERROR_ATTR(fail, REGULATOR_ERROR_FAIL);
+EFUSE_ERROR_ATTR(over_temp, REGULATOR_ERROR_OVER_TEMP);
+EFUSE_ERROR_ATTR(under_voltage_warn, REGULATOR_ERROR_UNDER_VOLTAGE_WARN);
+EFUSE_ERROR_ATTR(over_current_warn, REGULATOR_ERROR_OVER_CURRENT_WARN);
+EFUSE_ERROR_ATTR(over_voltage_warn, REGULATOR_ERROR_OVER_VOLTAGE_WARN);
+EFUSE_ERROR_ATTR(over_temp_warn, REGULATOR_ERROR_OVER_TEMP_WARN);
+
+static struct attribute *efuse_attrs[] = {
+	&dev_attr_operstate.attr,
+	&dev_attr_under_voltage.attr,
+	&dev_attr_over_current.attr,
+	&dev_attr_regulation_out.attr,
+	&dev_attr_fail.attr,
+	&dev_attr_over_temp.attr,
+	&dev_attr_under_voltage_warn.attr,
+	&dev_attr_over_current_warn.attr,
+	&dev_attr_over_voltage_warn.attr,
+	&dev_attr_over_temp_warn.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(efuse);
+
+static int efuse_probe(struct platform_device *pdev)
+{
+	int status;
+	struct regulator *reg;
+	struct efuse *efuse;
+	u32 cache_ttl_ms;
+
+	reg = devm_regulator_get(&pdev->dev, "vout");
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	status = regulator_enable(reg);
+	if (status) {
+		dev_err(&pdev->dev, "failed to enable regulator\n");
+		return status;
+	}
+
+	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+	if (!efuse)
+		return -ENOMEM;
+
+	efuse->reg = reg;
+	mutex_init(&efuse->error_flags.lock);
+
+	if (!of_property_read_u32(pdev->dev.of_node, "error-flags-cache-ttl-ms", &cache_ttl_ms))
+		efuse->error_flags.ttl = msecs_to_jiffies(cache_ttl_ms);
+
+	platform_set_drvdata(pdev, efuse);
+
+	return 0;
+}
+
+static const struct of_device_id efuse_of_match_table[] = {
+	{ .compatible = "power-efuse" },
+	{ },
+};
+
+static struct platform_driver efuse_driver = {
+	.driver = {
+		.name = "power-efuse",
+		.of_match_table = efuse_of_match_table,
+		.dev_groups = efuse_groups,
+	},
+	.probe = efuse_probe,
+};
+module_platform_driver(efuse_driver);
+
+MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Power efuse driver");
-- 
2.35.1

Re: [PATCH v2 2/2] misc: Add power-efuse driver

[Greg Kroah-Hartman]

On Mon, Mar 07, 2022 at 05:18:10PM -0800, Zev Weiss wrote:
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7058,6 +7058,11 @@ S:	Orphan
>  W:	http://aeschi.ch.eu.org/efs/
>  F:	fs/efs/
>  
> +POWER EFUSE DRIVER
> +M:	Zev Weiss <zev@bewilderbeest.net>
> +S:	Maintained
> +F:	drivers/misc/power-efuse.c
> +
>  EHEA (IBM pSeries eHEA 10Gb ethernet adapter) DRIVER
>  M:	Douglas Miller <dougmill@linux.ibm.com>
>  L:	netdev@vger.kernel.org

Entries need to be in sorted order :(

Re: [PATCH v2 2/2] misc: Add power-efuse driver

[Greg Kroah-Hartman]

On Mon, Mar 07, 2022 at 05:18:10PM -0800, Zev Weiss wrote:
> +static DEVICE_ATTR(operstate, 0644, efuse_show_operstate, efuse_set_operstate);

DEVICE_ATTR_RW()?

> +
> +#define EFUSE_ERROR_ATTR(name, bit)							    \
> +	static ssize_t efuse_show_##name(struct device *dev, struct device_attribute *attr, \
> +					 char *buf)                                         \
> +	{                                                                                   \
> +		struct efuse *efuse = dev_get_drvdata(dev);                                 \
> +		int status = efuse_update_error_flags(efuse);                               \
> +		if (status)                                                                 \
> +			return status;                                                      \
> +		return sysfs_emit(buf, "%d\n", !!(efuse->error_flags.cache & bit));         \
> +	}                                                                                   \
> +	static DEVICE_ATTR(name, 0444, efuse_show_##name, NULL)

DEVICE_ATTR_RO()?

> +EFUSE_ERROR_ATTR(under_voltage, REGULATOR_ERROR_UNDER_VOLTAGE);
> +EFUSE_ERROR_ATTR(over_current, REGULATOR_ERROR_OVER_CURRENT);
> +EFUSE_ERROR_ATTR(regulation_out, REGULATOR_ERROR_REGULATION_OUT);
> +EFUSE_ERROR_ATTR(fail, REGULATOR_ERROR_FAIL);
> +EFUSE_ERROR_ATTR(over_temp, REGULATOR_ERROR_OVER_TEMP);
> +EFUSE_ERROR_ATTR(under_voltage_warn, REGULATOR_ERROR_UNDER_VOLTAGE_WARN);
> +EFUSE_ERROR_ATTR(over_current_warn, REGULATOR_ERROR_OVER_CURRENT_WARN);
> +EFUSE_ERROR_ATTR(over_voltage_warn, REGULATOR_ERROR_OVER_VOLTAGE_WARN);
> +EFUSE_ERROR_ATTR(over_temp_warn, REGULATOR_ERROR_OVER_TEMP_WARN);
> +
> +static struct attribute *efuse_attrs[] = {
> +	&dev_attr_operstate.attr,
> +	&dev_attr_under_voltage.attr,
> +	&dev_attr_over_current.attr,
> +	&dev_attr_regulation_out.attr,
> +	&dev_attr_fail.attr,
> +	&dev_attr_over_temp.attr,
> +	&dev_attr_under_voltage_warn.attr,
> +	&dev_attr_over_current_warn.attr,
> +	&dev_attr_over_voltage_warn.attr,
> +	&dev_attr_over_temp_warn.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(efuse);

Shouldn't these all just be what all regulator drivers report?  Or power
drivers?  I find it odd that this would be the first driver that would
need to export these types of attributes.  Surely there's already a
class for this?

thanks,

greg k-h

Re: [PATCH v2 2/2] misc: Add power-efuse driver

[Zev Weiss]

On Mon, Mar 07, 2022 at 11:06:01PM PST, Greg Kroah-Hartman wrote:
>On Mon, Mar 07, 2022 at 05:18:10PM -0800, Zev Weiss wrote:
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7058,6 +7058,11 @@ S:	Orphan
>>  W:	http://aeschi.ch.eu.org/efs/
>>  F:	fs/efs/
>>
>> +POWER EFUSE DRIVER
>> +M:	Zev Weiss <zev@bewilderbeest.net>
>> +S:	Maintained
>> +F:	drivers/misc/power-efuse.c
>> +
>>  EHEA (IBM pSeries eHEA 10Gb ethernet adapter) DRIVER
>>  M:	Douglas Miller <dougmill@linux.ibm.com>
>>  L:	netdev@vger.kernel.org
>
>Entries need to be in sorted order :(
>

Ack, forgot to move it when I renamed an earlier incarnation from 
"efuse" to "power-efuse"...will fix.


Zev

Re: [PATCH v2 2/2] misc: Add power-efuse driver

[Zev Weiss]

On Mon, Mar 07, 2022 at 11:07:57PM PST, Greg Kroah-Hartman wrote:
>On Mon, Mar 07, 2022 at 05:18:10PM -0800, Zev Weiss wrote:
>> +static DEVICE_ATTR(operstate, 0644, efuse_show_operstate, efuse_set_operstate);
>
>DEVICE_ATTR_RW()?
>

Ack.

>> +
>> +#define EFUSE_ERROR_ATTR(name, bit)							    \
>> +	static ssize_t efuse_show_##name(struct device *dev, struct device_attribute *attr, \
>> +					 char *buf)                                         \
>> +	{                                                                                   \
>> +		struct efuse *efuse = dev_get_drvdata(dev);                                 \
>> +		int status = efuse_update_error_flags(efuse);                               \
>> +		if (status)                                                                 \
>> +			return status;                                                      \
>> +		return sysfs_emit(buf, "%d\n", !!(efuse->error_flags.cache & bit));         \
>> +	}                                                                                   \
>> +	static DEVICE_ATTR(name, 0444, efuse_show_##name, NULL)
>
>DEVICE_ATTR_RO()?
>

Ack.

>> +EFUSE_ERROR_ATTR(under_voltage, REGULATOR_ERROR_UNDER_VOLTAGE);
>> +EFUSE_ERROR_ATTR(over_current, REGULATOR_ERROR_OVER_CURRENT);
>> +EFUSE_ERROR_ATTR(regulation_out, REGULATOR_ERROR_REGULATION_OUT);
>> +EFUSE_ERROR_ATTR(fail, REGULATOR_ERROR_FAIL);
>> +EFUSE_ERROR_ATTR(over_temp, REGULATOR_ERROR_OVER_TEMP);
>> +EFUSE_ERROR_ATTR(under_voltage_warn, REGULATOR_ERROR_UNDER_VOLTAGE_WARN);
>> +EFUSE_ERROR_ATTR(over_current_warn, REGULATOR_ERROR_OVER_CURRENT_WARN);
>> +EFUSE_ERROR_ATTR(over_voltage_warn, REGULATOR_ERROR_OVER_VOLTAGE_WARN);
>> +EFUSE_ERROR_ATTR(over_temp_warn, REGULATOR_ERROR_OVER_TEMP_WARN);
>> +
>> +static struct attribute *efuse_attrs[] = {
>> +	&dev_attr_operstate.attr,
>> +	&dev_attr_under_voltage.attr,
>> +	&dev_attr_over_current.attr,
>> +	&dev_attr_regulation_out.attr,
>> +	&dev_attr_fail.attr,
>> +	&dev_attr_over_temp.attr,
>> +	&dev_attr_under_voltage_warn.attr,
>> +	&dev_attr_over_current_warn.attr,
>> +	&dev_attr_over_voltage_warn.attr,
>> +	&dev_attr_over_temp_warn.attr,
>> +	NULL,
>> +};
>> +ATTRIBUTE_GROUPS(efuse);
>
>Shouldn't these all just be what all regulator drivers report?  Or power
>drivers?  I find it odd that this would be the first driver that would
>need to export these types of attributes.  Surely there's already a
>class for this?
>

The attributes available from the underlying regulator device don't 
include the error flags, and while they do include its state 
('operstate' here), it's a read-only attribute, and from previous 
discussions with Mark I gathered that was unlikely to change (whereas it 
being read-write is a critical part of this driver's functionality).

Given his input on the first stab at this I took a while back, I've been 
hoping to hear from Mark as to whether this looked more like something 
he'd find palatable; perhaps he could chime in on this too?  (And/or on 
the regulator API question in the cover letter.)


Thanks,
Zev

Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding

[Rob Herring]

On Mon, Mar 07, 2022 at 05:18:09PM -0800, Zev Weiss wrote:
> This can be used to describe a power output supplied by a regulator
> device that the system controls.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
>  .../devicetree/bindings/misc/power-efuse.yaml | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
> 
> diff --git a/Documentation/devicetree/bindings/misc/power-efuse.yaml b/Documentation/devicetree/bindings/misc/power-efuse.yaml
> new file mode 100644
> index 000000000000..5f8f0b21af0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/power-efuse.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic power efuse device
> +
> +maintainers:
> +  - Zev Weiss <zev@bewilderbeest.net>
> +
> +description: |
> +  This binding describes a physical power output supplied by a
> +  regulator providing efuse functionality (manual on/off control, and
> +  auto-shutoff if current, voltage, or thermal limits are exceeded).
> +
> +  These may be found on systems such as "smart" network PDUs, and
> +  typically supply power to devices entirely separate from the system
> +  described by the device-tree by way of an external connector such as
> +  an Open19 power cable:
> +
> +  https://www.open19.org/marketplace/coolpower-cable-assembly-8ru/

Not really a helpful link...

I still don't understand what the h/w looks like here. At least I now 
understand we're talking a fuse on power rail, not efuses in an SoC 
used as OTP bits or feature disables.

> +
> +properties:
> +  compatible:
> +    const: power-efuse
> +
> +  vout-supply:
> +    description:
> +      phandle to the regulator providing power for the efuse

Vout is a supply to the efuse and not the rail being fused? 

Sorry, I know nothing about how an efuse is implemented so you are going 
to have to explain or draw it.

> +
> +  error-flags-cache-ttl-ms:
> +    description:
> +      The number of milliseconds the vout-supply regulator's error
> +      flags should be cached before re-fetching them.

How does one fetch/read? the error flags?

> +
> +required:
> +  - compatible
> +  - vout-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    efuse {
> +        compatible = "power-efuse";
> +        vout-supply = <&efuse_reg>;
> +        error-flags-cache-ttl-ms = <500>;
> +    };
> -- 
> 2.35.1
> 
> 

Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding

[Zev Weiss]

On Fri, Mar 11, 2022 at 07:24:41AM PST, Rob Herring wrote:
>On Mon, Mar 07, 2022 at 05:18:09PM -0800, Zev Weiss wrote:
>> This can be used to describe a power output supplied by a regulator
>> device that the system controls.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>>  .../devicetree/bindings/misc/power-efuse.yaml | 49 +++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/misc/power-efuse.yaml b/Documentation/devicetree/bindings/misc/power-efuse.yaml
>> new file mode 100644
>> index 000000000000..5f8f0b21af0e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
>> @@ -0,0 +1,49 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/misc/power-efuse.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Generic power efuse device
>> +
>> +maintainers:
>> +  - Zev Weiss <zev@bewilderbeest.net>
>> +
>> +description: |
>> +  This binding describes a physical power output supplied by a
>> +  regulator providing efuse functionality (manual on/off control, and
>> +  auto-shutoff if current, voltage, or thermal limits are exceeded).
>> +
>> +  These may be found on systems such as "smart" network PDUs, and
>> +  typically supply power to devices entirely separate from the system
>> +  described by the device-tree by way of an external connector such as
>> +  an Open19 power cable:
>> +
>> +  https://www.open19.org/marketplace/coolpower-cable-assembly-8ru/
>
>Not really a helpful link...
>
>I still don't understand what the h/w looks like here. At least I now
>understand we're talking a fuse on power rail, not efuses in an SoC
>used as OTP bits or feature disables.
>

The systems this would actually be used for would be things like these:
  - https://www.open19.org/marketplace/delta-16kw-power-shelf/
  - https://www.open19.org/marketplace/inspur-open19-power-shelf-ob19200l1/

The rightmost pictures on those pages show the four black connectors 
where the cable assembly linked in the patch plugs in, each of which 
provides the outputs from 12 such efuses, on 12 pairs of ground and 
+12VDC pins.  (There are also two more single outputs off to the side.)

It essentially just amounts to an external power output supplied by a 
regulator, with the regulator providing an on/off switch, overcurrent 
protection, etc.

And yes, the ambiguity of the "efuse" terminology is unfortunate (the 
"power-" prefix was an attempt to clarify it slightly).  That's the term 
used in the documentation for the hardware and hence is what I've called 
it here, but I'd be open to using a different name if that would help.

>> +
>> +properties:
>> +  compatible:
>> +    const: power-efuse
>> +
>> +  vout-supply:
>> +    description:
>> +      phandle to the regulator providing power for the efuse
>
>Vout is a supply to the efuse and not the rail being fused?

Yeah, that was a fairly muddled description -- it's really the latter.  
Perhaps:

   phandle to the regulator providing power for the output rail
   controlled by the efuse

?

>
>Sorry, I know nothing about how an efuse is implemented so you are going
>to have to explain or draw it.
>
>> +
>> +  error-flags-cache-ttl-ms:
>> +    description:
>> +      The number of milliseconds the vout-supply regulator's error
>> +      flags should be cached before re-fetching them.
>
>How does one fetch/read? the error flags?
>

In the specific case I'm dealing with, via PMBus STATUS_* commands, 
though I was aiming to keep this more generic so it could potentially be 
used to describe non-PMBus arrangements (in the Linux case, via whatever 
mechanism the implementation of the regulator's .get_error_flags() 
function uses).

>> +
>> +required:
>> +  - compatible
>> +  - vout-supply
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    efuse {
>> +        compatible = "power-efuse";
>> +        vout-supply = <&efuse_reg>;
>> +        error-flags-cache-ttl-ms = <500>;
>> +    };
>> --
>> 2.35.1
>>
>>

Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding

[Rob Herring]

On Fri, Mar 11, 2022 at 01:48:22PM -0800, Zev Weiss wrote:
> On Fri, Mar 11, 2022 at 07:24:41AM PST, Rob Herring wrote:
> > On Mon, Mar 07, 2022 at 05:18:09PM -0800, Zev Weiss wrote:
> > > This can be used to describe a power output supplied by a regulator
> > > device that the system controls.
> > > 
> > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> > > ---
> > >  .../devicetree/bindings/misc/power-efuse.yaml | 49 +++++++++++++++++++
> > >  1 file changed, 49 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/misc/power-efuse.yaml b/Documentation/devicetree/bindings/misc/power-efuse.yaml
> > > new file mode 100644
> > > index 000000000000..5f8f0b21af0e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
> > > @@ -0,0 +1,49 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/misc/power-efuse.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Generic power efuse device
> > > +
> > > +maintainers:
> > > +  - Zev Weiss <zev@bewilderbeest.net>
> > > +
> > > +description: |
> > > +  This binding describes a physical power output supplied by a
> > > +  regulator providing efuse functionality (manual on/off control, and
> > > +  auto-shutoff if current, voltage, or thermal limits are exceeded).
> > > +
> > > +  These may be found on systems such as "smart" network PDUs, and
> > > +  typically supply power to devices entirely separate from the system
> > > +  described by the device-tree by way of an external connector such as
> > > +  an Open19 power cable:
> > > +
> > > +  https://www.open19.org/marketplace/coolpower-cable-assembly-8ru/
> > 
> > Not really a helpful link...
> > 
> > I still don't understand what the h/w looks like here. At least I now
> > understand we're talking a fuse on power rail, not efuses in an SoC
> > used as OTP bits or feature disables.
> > 
> 
> The systems this would actually be used for would be things like these:
>  - https://www.open19.org/marketplace/delta-16kw-power-shelf/
>  - https://www.open19.org/marketplace/inspur-open19-power-shelf-ob19200l1/

Those still don't help show me what the h/w looks like. High level 
schematics is what I'm looking for.


> The rightmost pictures on those pages show the four black connectors where
> the cable assembly linked in the patch plugs in, each of which provides the
> outputs from 12 such efuses, on 12 pairs of ground and +12VDC pins.  (There
> are also two more single outputs off to the side.)
> 
> It essentially just amounts to an external power output supplied by a
> regulator, with the regulator providing an on/off switch, overcurrent
> protection, etc.
> 
> And yes, the ambiguity of the "efuse" terminology is unfortunate (the
> "power-" prefix was an attempt to clarify it slightly).  That's the term
> used in the documentation for the hardware and hence is what I've called it
> here, but I'd be open to using a different name if that would help.
> 
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: power-efuse
> > > +
> > > +  vout-supply:
> > > +    description:
> > > +      phandle to the regulator providing power for the efuse
> > 
> > Vout is a supply to the efuse and not the rail being fused?
> 
> Yeah, that was a fairly muddled description -- it's really the latter.
> Perhaps:
> 
>   phandle to the regulator providing power for the output rail
>   controlled by the efuse
> 
> ?
> 
> > 
> > Sorry, I know nothing about how an efuse is implemented so you are going
> > to have to explain or draw it.
> > 
> > > +
> > > +  error-flags-cache-ttl-ms:
> > > +    description:
> > > +      The number of milliseconds the vout-supply regulator's error
> > > +      flags should be cached before re-fetching them.
> > 
> > How does one fetch/read? the error flags?
> > 
> 
> In the specific case I'm dealing with, via PMBus STATUS_* commands, though I
> was aiming to keep this more generic so it could potentially be used to
> describe non-PMBus arrangements (in the Linux case, via whatever mechanism
> the implementation of the regulator's .get_error_flags() function uses).

PMBus is I2C (subset). What device(s) is on the PMBus?

Here's what I've got for connections so far:

Vout(regulator)-->|efuse|-->12V

Host-->PMbus--->????

Rob

Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding

[Zev Weiss]

On Fri, Mar 11, 2022 at 04:08:47PM PST, Rob Herring wrote:
>On Fri, Mar 11, 2022 at 01:48:22PM -0800, Zev Weiss wrote:
>> On Fri, Mar 11, 2022 at 07:24:41AM PST, Rob Herring wrote:
>> > On Mon, Mar 07, 2022 at 05:18:09PM -0800, Zev Weiss wrote:
>> > > This can be used to describe a power output supplied by a regulator
>> > > device that the system controls.
>> > >
>> > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> > > ---
>> > >  .../devicetree/bindings/misc/power-efuse.yaml | 49 +++++++++++++++++++
>> > >  1 file changed, 49 insertions(+)
>> > >  create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
>> > >
>> > > diff --git a/Documentation/devicetree/bindings/misc/power-efuse.yaml b/Documentation/devicetree/bindings/misc/power-efuse.yaml
>> > > new file mode 100644
>> > > index 000000000000..5f8f0b21af0e
>> > > --- /dev/null
>> > > +++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
>> > > @@ -0,0 +1,49 @@
>> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> > > +%YAML 1.2
>> > > +---
>> > > +$id: http://devicetree.org/schemas/misc/power-efuse.yaml#
>> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > > +
>> > > +title: Generic power efuse device
>> > > +
>> > > +maintainers:
>> > > +  - Zev Weiss <zev@bewilderbeest.net>
>> > > +
>> > > +description: |
>> > > +  This binding describes a physical power output supplied by a
>> > > +  regulator providing efuse functionality (manual on/off control, and
>> > > +  auto-shutoff if current, voltage, or thermal limits are exceeded).
>> > > +
>> > > +  These may be found on systems such as "smart" network PDUs, and
>> > > +  typically supply power to devices entirely separate from the system
>> > > +  described by the device-tree by way of an external connector such as
>> > > +  an Open19 power cable:
>> > > +
>> > > +  https://www.open19.org/marketplace/coolpower-cable-assembly-8ru/
>> >
>> > Not really a helpful link...
>> >
>> > I still don't understand what the h/w looks like here. At least I now
>> > understand we're talking a fuse on power rail, not efuses in an SoC
>> > used as OTP bits or feature disables.
>> >
>>
>> The systems this would actually be used for would be things like these:
>>  - https://www.open19.org/marketplace/delta-16kw-power-shelf/
>>  - https://www.open19.org/marketplace/inspur-open19-power-shelf-ob19200l1/
>
>Those still don't help show me what the h/w looks like. High level
>schematics is what I'm looking for.
>
>
>> The rightmost pictures on those pages show the four black connectors where
>> the cable assembly linked in the patch plugs in, each of which provides the
>> outputs from 12 such efuses, on 12 pairs of ground and +12VDC pins.  (There
>> are also two more single outputs off to the side.)
>>
>> It essentially just amounts to an external power output supplied by a
>> regulator, with the regulator providing an on/off switch, overcurrent
>> protection, etc.
>>
>> And yes, the ambiguity of the "efuse" terminology is unfortunate (the
>> "power-" prefix was an attempt to clarify it slightly).  That's the term
>> used in the documentation for the hardware and hence is what I've called it
>> here, but I'd be open to using a different name if that would help.
>>
>> > > +
>> > > +properties:
>> > > +  compatible:
>> > > +    const: power-efuse
>> > > +
>> > > +  vout-supply:
>> > > +    description:
>> > > +      phandle to the regulator providing power for the efuse
>> >
>> > Vout is a supply to the efuse and not the rail being fused?
>>
>> Yeah, that was a fairly muddled description -- it's really the latter.
>> Perhaps:
>>
>>   phandle to the regulator providing power for the output rail
>>   controlled by the efuse
>>
>> ?
>>
>> >
>> > Sorry, I know nothing about how an efuse is implemented so you are going
>> > to have to explain or draw it.
>> >
>> > > +
>> > > +  error-flags-cache-ttl-ms:
>> > > +    description:
>> > > +      The number of milliseconds the vout-supply regulator's error
>> > > +      flags should be cached before re-fetching them.
>> >
>> > How does one fetch/read? the error flags?
>> >
>>
>> In the specific case I'm dealing with, via PMBus STATUS_* commands, though I
>> was aiming to keep this more generic so it could potentially be used to
>> describe non-PMBus arrangements (in the Linux case, via whatever mechanism
>> the implementation of the regulator's .get_error_flags() function uses).
>
>PMBus is I2C (subset). What device(s) is on the PMBus?
>
>Here's what I've got for connections so far:
>
>Vout(regulator)-->|efuse|-->12V
>
>Host-->PMbus--->????
>

On the hardware I'm currently working with (the Delta unit linked 
above), the PMBus device is a TI LM25066 with various other components 
(MOSFET, thermal diode, sense resistor, etc.) in the surrounding 
circuitry.  My understanding is that "efuse" as used by the manufacturer 
refers to the combined circuit, including the LM25066.

Is that a sufficient "high level schematic", or is there additional 
information you're looking for?  I do have access to a detailed 
schematic of the circuit, but unfortunately I don't think I'm at liberty 
to share it.

(I don't know exactly what the Inspur unit uses because I haven't dealt 
with that one first-hand, but I'd guess it's probably broadly similar.)


Thanks,
Zev

Re: [PATCH v2 2/2] misc: Add power-efuse driver

[Zev Weiss]

On Tue, Mar 08, 2022 at 12:23:44AM PST, Zev Weiss wrote:
>On Mon, Mar 07, 2022 at 11:07:57PM PST, Greg Kroah-Hartman wrote:
>
>>>+EFUSE_ERROR_ATTR(under_voltage, REGULATOR_ERROR_UNDER_VOLTAGE);
>>>+EFUSE_ERROR_ATTR(over_current, REGULATOR_ERROR_OVER_CURRENT);
>>>+EFUSE_ERROR_ATTR(regulation_out, REGULATOR_ERROR_REGULATION_OUT);
>>>+EFUSE_ERROR_ATTR(fail, REGULATOR_ERROR_FAIL);
>>>+EFUSE_ERROR_ATTR(over_temp, REGULATOR_ERROR_OVER_TEMP);
>>>+EFUSE_ERROR_ATTR(under_voltage_warn, REGULATOR_ERROR_UNDER_VOLTAGE_WARN);
>>>+EFUSE_ERROR_ATTR(over_current_warn, REGULATOR_ERROR_OVER_CURRENT_WARN);
>>>+EFUSE_ERROR_ATTR(over_voltage_warn, REGULATOR_ERROR_OVER_VOLTAGE_WARN);
>>>+EFUSE_ERROR_ATTR(over_temp_warn, REGULATOR_ERROR_OVER_TEMP_WARN);
>>>+
>>>+static struct attribute *efuse_attrs[] = {
>>>+	&dev_attr_operstate.attr,
>>>+	&dev_attr_under_voltage.attr,
>>>+	&dev_attr_over_current.attr,
>>>+	&dev_attr_regulation_out.attr,
>>>+	&dev_attr_fail.attr,
>>>+	&dev_attr_over_temp.attr,
>>>+	&dev_attr_under_voltage_warn.attr,
>>>+	&dev_attr_over_current_warn.attr,
>>>+	&dev_attr_over_voltage_warn.attr,
>>>+	&dev_attr_over_temp_warn.attr,
>>>+	NULL,
>>>+};
>>>+ATTRIBUTE_GROUPS(efuse);
>>
>>Shouldn't these all just be what all regulator drivers report?  Or power
>>drivers?  I find it odd that this would be the first driver that would
>>need to export these types of attributes.  Surely there's already a
>>class for this?
>>
>
>The attributes available from the underlying regulator device don't 
>include the error flags, and while they do include its state 
>('operstate' here), it's a read-only attribute, and from previous 
>discussions with Mark I gathered that was unlikely to change (whereas 
>it being read-write is a critical part of this driver's 
>functionality).
>
>Given his input on the first stab at this I took a while back, I've 
>been hoping to hear from Mark as to whether this looked more like 
>something he'd find palatable; perhaps he could chime in on this too?  
>(And/or on the regulator API question in the cover letter.)
>

Ping...Mark (or Liam?), any thoughts on an appropriate path forward on 
this?


Thanks,
Zev

Re: [PATCH v2 2/2] misc: Add power-efuse driver

[Greg Kroah-Hartman]

On Mon, Apr 11, 2022 at 03:09:53PM -0700, Zev Weiss wrote:
> On Tue, Mar 08, 2022 at 12:23:44AM PST, Zev Weiss wrote:
> > On Mon, Mar 07, 2022 at 11:07:57PM PST, Greg Kroah-Hartman wrote:
> > 
> > > > +EFUSE_ERROR_ATTR(under_voltage, REGULATOR_ERROR_UNDER_VOLTAGE);
> > > > +EFUSE_ERROR_ATTR(over_current, REGULATOR_ERROR_OVER_CURRENT);
> > > > +EFUSE_ERROR_ATTR(regulation_out, REGULATOR_ERROR_REGULATION_OUT);
> > > > +EFUSE_ERROR_ATTR(fail, REGULATOR_ERROR_FAIL);
> > > > +EFUSE_ERROR_ATTR(over_temp, REGULATOR_ERROR_OVER_TEMP);
> > > > +EFUSE_ERROR_ATTR(under_voltage_warn, REGULATOR_ERROR_UNDER_VOLTAGE_WARN);
> > > > +EFUSE_ERROR_ATTR(over_current_warn, REGULATOR_ERROR_OVER_CURRENT_WARN);
> > > > +EFUSE_ERROR_ATTR(over_voltage_warn, REGULATOR_ERROR_OVER_VOLTAGE_WARN);
> > > > +EFUSE_ERROR_ATTR(over_temp_warn, REGULATOR_ERROR_OVER_TEMP_WARN);
> > > > +
> > > > +static struct attribute *efuse_attrs[] = {
> > > > +	&dev_attr_operstate.attr,
> > > > +	&dev_attr_under_voltage.attr,
> > > > +	&dev_attr_over_current.attr,
> > > > +	&dev_attr_regulation_out.attr,
> > > > +	&dev_attr_fail.attr,
> > > > +	&dev_attr_over_temp.attr,
> > > > +	&dev_attr_under_voltage_warn.attr,
> > > > +	&dev_attr_over_current_warn.attr,
> > > > +	&dev_attr_over_voltage_warn.attr,
> > > > +	&dev_attr_over_temp_warn.attr,
> > > > +	NULL,
> > > > +};
> > > > +ATTRIBUTE_GROUPS(efuse);
> > > 
> > > Shouldn't these all just be what all regulator drivers report?  Or power
> > > drivers?  I find it odd that this would be the first driver that would
> > > need to export these types of attributes.  Surely there's already a
> > > class for this?
> > > 
> > 
> > The attributes available from the underlying regulator device don't
> > include the error flags, and while they do include its state
> > ('operstate' here), it's a read-only attribute, and from previous
> > discussions with Mark I gathered that was unlikely to change (whereas it
> > being read-write is a critical part of this driver's functionality).
> > 
> > Given his input on the first stab at this I took a while back, I've been
> > hoping to hear from Mark as to whether this looked more like something
> > he'd find palatable; perhaps he could chime in on this too?  (And/or on
> > the regulator API question in the cover letter.)
> > 
> 
> Ping...Mark (or Liam?), any thoughts on an appropriate path forward on this?

Make it a regular regulator driver please.

thanks,

greg k-h

Re: [PATCH v2 2/2] misc: Add power-efuse driver

[Zev Weiss]

On Mon, Apr 11, 2022 at 09:53:14PM PDT, Greg Kroah-Hartman wrote:
>On Mon, Apr 11, 2022 at 03:09:53PM -0700, Zev Weiss wrote:
>>
>> Ping...Mark (or Liam?), any thoughts on an appropriate path forward on this?
>
>Make it a regular regulator driver please.
>

The existing userspace-consumer regulator driver does provide some of 
the functionality I'm looking for (the on/off switch), and I think would 
be pretty easy to extend to provide the rest of it as well.  When I 
previously proposed using it as such though, Mark stated quite firmly 
that that wasn't going to fly [0]; this approach was my attempt at 
implementing a generic, abstracted mechanism as he had suggested later 
in that thread, though I haven't gotten any further feedback from him on 
what he thought of it.

If he's had a change of heart and would be open to the 
userspace-consumer driver taking on a bit of new functionality I'd be 
happy to go that route though.

[0] https://lore.kernel.org/openbmc/20210330174221.GJ4976@sirena.org.uk/


Thanks,
Zev