* [PATCH v2 0/2] misc: Add power-efuse driver @ 2022-03-08 1:18 Zev Weiss 2022-03-08 1:18 ` [PATCH v2 1/2] dt-bindings: Add power-efuse binding Zev Weiss 2022-03-08 1:18 ` [PATCH v2 2/2] misc: Add power-efuse driver Zev Weiss 0 siblings, 2 replies; 14+ messages in thread From: Zev Weiss @ 2022-03-08 1:18 UTC (permalink / raw) To: linux-kernel, Greg Kroah-Hartman, Arnd Bergmann Cc: devicetree, Zev Weiss, openbmc, Liam Girdwood, Rob Herring, Mark Brown 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] dt-bindings: Add power-efuse binding 2022-03-08 1:18 [PATCH v2 0/2] misc: Add power-efuse driver Zev Weiss @ 2022-03-08 1:18 ` Zev Weiss 2022-03-11 15:24 ` Rob Herring 2022-03-08 1:18 ` [PATCH v2 2/2] misc: Add power-efuse driver Zev Weiss 1 sibling, 1 reply; 14+ messages in thread From: Zev Weiss @ 2022-03-08 1:18 UTC (permalink / raw) To: Rob Herring, devicetree Cc: Zev Weiss, Arnd Bergmann, Greg Kroah-Hartman, openbmc, linux-kernel, Liam Girdwood, Mark Brown 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding 2022-03-08 1:18 ` [PATCH v2 1/2] dt-bindings: Add power-efuse binding Zev Weiss @ 2022-03-11 15:24 ` Rob Herring 2022-03-11 21:48 ` Zev Weiss 0 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2022-03-11 15:24 UTC (permalink / raw) To: Zev Weiss Cc: devicetree, Arnd Bergmann, Greg Kroah-Hartman, openbmc, linux-kernel, Liam Girdwood, Mark Brown 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 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding 2022-03-11 15:24 ` Rob Herring @ 2022-03-11 21:48 ` Zev Weiss 2022-03-12 0:08 ` Rob Herring 0 siblings, 1 reply; 14+ messages in thread From: Zev Weiss @ 2022-03-11 21:48 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Arnd Bergmann, Greg Kroah-Hartman, openbmc, linux-kernel, Liam Girdwood, Mark Brown 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 >> >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding 2022-03-11 21:48 ` Zev Weiss @ 2022-03-12 0:08 ` Rob Herring 2022-03-12 0:39 ` Zev Weiss 0 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2022-03-12 0:08 UTC (permalink / raw) To: Zev Weiss Cc: devicetree, Arnd Bergmann, Greg Kroah-Hartman, openbmc, Liam Girdwood, linux-kernel, Mark Brown 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: Add power-efuse binding 2022-03-12 0:08 ` Rob Herring @ 2022-03-12 0:39 ` Zev Weiss 0 siblings, 0 replies; 14+ messages in thread From: Zev Weiss @ 2022-03-12 0:39 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Arnd Bergmann, Greg Kroah-Hartman, openbmc, Liam Girdwood, linux-kernel, Mark Brown 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] misc: Add power-efuse driver 2022-03-08 1:18 [PATCH v2 0/2] misc: Add power-efuse driver Zev Weiss 2022-03-08 1:18 ` [PATCH v2 1/2] dt-bindings: Add power-efuse binding Zev Weiss @ 2022-03-08 1:18 ` Zev Weiss 2022-03-08 7:06 ` Greg Kroah-Hartman 2022-03-08 7:07 ` Greg Kroah-Hartman 1 sibling, 2 replies; 14+ messages in thread From: Zev Weiss @ 2022-03-08 1:18 UTC (permalink / raw) To: Greg Kroah-Hartman, Arnd Bergmann Cc: openbmc, Mark Brown, linux-kernel, Zev Weiss, Liam Girdwood 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] misc: Add power-efuse driver 2022-03-08 1:18 ` [PATCH v2 2/2] misc: Add power-efuse driver Zev Weiss @ 2022-03-08 7:06 ` Greg Kroah-Hartman 2022-03-08 7:45 ` Zev Weiss 2022-03-08 7:07 ` Greg Kroah-Hartman 1 sibling, 1 reply; 14+ messages in thread From: Greg Kroah-Hartman @ 2022-03-08 7:06 UTC (permalink / raw) To: Zev Weiss; +Cc: openbmc, Mark Brown, linux-kernel, Arnd Bergmann, Liam Girdwood 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 :( ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] misc: Add power-efuse driver 2022-03-08 7:06 ` Greg Kroah-Hartman @ 2022-03-08 7:45 ` Zev Weiss 0 siblings, 0 replies; 14+ messages in thread From: Zev Weiss @ 2022-03-08 7:45 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Mark Brown, linux-kernel, Arnd Bergmann, Liam Girdwood 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] misc: Add power-efuse driver 2022-03-08 1:18 ` [PATCH v2 2/2] misc: Add power-efuse driver Zev Weiss 2022-03-08 7:06 ` Greg Kroah-Hartman @ 2022-03-08 7:07 ` Greg Kroah-Hartman 2022-03-08 8:23 ` Zev Weiss 1 sibling, 1 reply; 14+ messages in thread From: Greg Kroah-Hartman @ 2022-03-08 7:07 UTC (permalink / raw) To: Zev Weiss; +Cc: openbmc, Mark Brown, linux-kernel, Arnd Bergmann, Liam Girdwood 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] misc: Add power-efuse driver 2022-03-08 7:07 ` Greg Kroah-Hartman @ 2022-03-08 8:23 ` Zev Weiss 2022-04-11 22:09 ` Zev Weiss 0 siblings, 1 reply; 14+ messages in thread From: Zev Weiss @ 2022-03-08 8:23 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Mark Brown, linux-kernel, Arnd Bergmann, Liam Girdwood 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] misc: Add power-efuse driver 2022-03-08 8:23 ` Zev Weiss @ 2022-04-11 22:09 ` Zev Weiss 2022-04-12 4:53 ` Greg Kroah-Hartman 0 siblings, 1 reply; 14+ messages in thread From: Zev Weiss @ 2022-04-11 22:09 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Mark Brown, linux-kernel, Arnd Bergmann, Liam Girdwood 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] misc: Add power-efuse driver 2022-04-11 22:09 ` Zev Weiss @ 2022-04-12 4:53 ` Greg Kroah-Hartman 2022-04-13 9:03 ` Zev Weiss 0 siblings, 1 reply; 14+ messages in thread From: Greg Kroah-Hartman @ 2022-04-12 4:53 UTC (permalink / raw) To: Zev Weiss; +Cc: openbmc, Mark Brown, linux-kernel, Arnd Bergmann, Liam Girdwood 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] misc: Add power-efuse driver 2022-04-12 4:53 ` Greg Kroah-Hartman @ 2022-04-13 9:03 ` Zev Weiss 0 siblings, 0 replies; 14+ messages in thread From: Zev Weiss @ 2022-04-13 9:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Mark Brown, linux-kernel, Arnd Bergmann, Liam Girdwood 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-04-13 9:04 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-08 1:18 [PATCH v2 0/2] misc: Add power-efuse driver Zev Weiss 2022-03-08 1:18 ` [PATCH v2 1/2] dt-bindings: Add power-efuse binding Zev Weiss 2022-03-11 15:24 ` Rob Herring 2022-03-11 21:48 ` Zev Weiss 2022-03-12 0:08 ` Rob Herring 2022-03-12 0:39 ` Zev Weiss 2022-03-08 1:18 ` [PATCH v2 2/2] misc: Add power-efuse driver Zev Weiss 2022-03-08 7:06 ` Greg Kroah-Hartman 2022-03-08 7:45 ` Zev Weiss 2022-03-08 7:07 ` Greg Kroah-Hartman 2022-03-08 8:23 ` Zev Weiss 2022-04-11 22:09 ` Zev Weiss 2022-04-12 4:53 ` Greg Kroah-Hartman 2022-04-13 9:03 ` Zev Weiss
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).