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

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

[Zev Weiss]

Hello,

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 extends the pmbus core's regulator support with an
implementation of the .get_error_flags() operation, mapping PMBus
status flags to REGULATOR_ERROR_* flags where possible, and the second
patch adds regulator support for the lm25066 driver.  These two
patches are essentially independent of the power-efuse driver (and
each other) and could potentially be applicable individually, but are
necessary for the power-efuse driver to be useful on the AHE-50DC.

The third and fourth patches add dt-bindings and the implementation of
the power-efuse driver, respectively.  The driver is fairly simple; it
merely provides a sysfs interface to enable, disable, and retrieve
error flags from an underlying regulator.

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/

Zev Weiss (4):
  hwmon: (pmbus) Add get_error_flags support to regulator ops
  hwmon: (pmbus) lm25066: Add regulator support
  dt-bindings: Add power-efuse binding
  misc: Add power-efuse driver

 .../devicetree/bindings/misc/power-efuse.yaml |  37 +++
 MAINTAINERS                                   |   5 +
 drivers/hwmon/pmbus/Kconfig                   |   7 +
 drivers/hwmon/pmbus/lm25066.c                 |  14 ++
 drivers/hwmon/pmbus/pmbus_core.c              |  97 ++++++++
 drivers/misc/Kconfig                          |  15 ++
 drivers/misc/Makefile                         |   1 +
 drivers/misc/power-efuse.c                    | 221 ++++++++++++++++++
 8 files changed, 397 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
 create mode 100644 drivers/misc/power-efuse.c

-- 
2.35.1

[PATCH 3/4] 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 | 37 +++++++++++++++++++
 1 file changed, 37 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..cadce15d2ce7
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
@@ -0,0 +1,37 @@
+# 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>
+
+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

Re: [PATCH 3/4] dt-bindings: Add power-efuse binding

[Rob Herring]

On Thu, 17 Feb 2022 02:44:43 -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 | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/misc/power-efuse.yaml:10:2: [warning] wrong indentation: expected 2 but found 1 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1594124

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 0/4] misc: Add power-efuse driver

[Guenter Roeck]

On 2/17/22 02:44, Zev Weiss wrote:
> Hello,
> 
> 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 extends the pmbus core's regulator support with an
> implementation of the .get_error_flags() operation, mapping PMBus
> status flags to REGULATOR_ERROR_* flags where possible, and the second
> patch adds regulator support for the lm25066 driver.  These two
> patches are essentially independent of the power-efuse driver (and
> each other) and could potentially be applicable individually, but are
> necessary for the power-efuse driver to be useful on the AHE-50DC.
> 

Nevertheless, the first two patches are orthogonal to the remaining
two patches and should be separate.

Guenter

> The third and fourth patches add dt-bindings and the implementation of
> the power-efuse driver, respectively.  The driver is fairly simple; it
> merely provides a sysfs interface to enable, disable, and retrieve
> error flags from an underlying regulator.
> 
> 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/
> 
> Zev Weiss (4):
>    hwmon: (pmbus) Add get_error_flags support to regulator ops
>    hwmon: (pmbus) lm25066: Add regulator support
>    dt-bindings: Add power-efuse binding
>    misc: Add power-efuse driver
> 
>   .../devicetree/bindings/misc/power-efuse.yaml |  37 +++
>   MAINTAINERS                                   |   5 +
>   drivers/hwmon/pmbus/Kconfig                   |   7 +
>   drivers/hwmon/pmbus/lm25066.c                 |  14 ++
>   drivers/hwmon/pmbus/pmbus_core.c              |  97 ++++++++
>   drivers/misc/Kconfig                          |  15 ++
>   drivers/misc/Makefile                         |   1 +
>   drivers/misc/power-efuse.c                    | 221 ++++++++++++++++++
>   8 files changed, 397 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/misc/power-efuse.yaml
>   create mode 100644 drivers/misc/power-efuse.c
> 

Re: [PATCH 3/4] dt-bindings: Add power-efuse binding

[Rob Herring]

On Thu, Feb 17, 2022 at 02:44:43AM -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 | 37 +++++++++++++++++++
>  1 file changed, 37 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..cadce15d2ce7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
> @@ -0,0 +1,37 @@
> +# 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

No idea what this is, but I doubt any such generic device exists. This 
needs sufficient description to be convincing that it is indeed generic.

> +
> +maintainers:
> + - Zev Weiss <zev@bewilderbeest.net>
> +
> +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.

What are 'error flags'? Not something I've heard with respect to 
regulators.

> +
> +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 3/4] dt-bindings: Add power-efuse binding

[Zev Weiss]

On Thu, Feb 17, 2022 at 02:35:12PM PST, Rob Herring wrote:
>On Thu, Feb 17, 2022 at 02:44:43AM -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 | 37 +++++++++++++++++++
>>  1 file changed, 37 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..cadce15d2ce7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/power-efuse.yaml
>> @@ -0,0 +1,37 @@
>> +# 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
>
>No idea what this is, but I doubt any such generic device exists. This
>needs sufficient description to be convincing that it is indeed generic.
>

I was struggling a bit to come up with a reasonably concise title; this 
admittedly isn't great.

Would a description like the following clarify it adequately?

   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/


>> +
>> +maintainers:
>> + - Zev Weiss <zev@bewilderbeest.net>
>> +
>> +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.
>
>What are 'error flags'? Not something I've heard with respect to
>regulators.
>

That refers to the REGULATOR_ERROR_* flags in 
include/linux/regulator/consumer.h, in whatever "physical" form those 
ultimately take -- for example, in the PMBus-based case I'm currently 
aiming to support, they'd map to the flags returned by PMBus STATUS_* 
commands.  


Thanks for the review,
Zev