| Message ID | ea49494429528cf8e60fa984ae1f523ddacd850c.1712920132.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support ROHM BD96801 scalable PMIC | expand |
On 12/04/2024 13:21, Matti Vaittinen wrote: > ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce > DT bindings for the BD96801 core. > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > --- > Revision history: > RFCv1 => RFCv2: > - Document rohm,hw-timeout-ms > - Document rohm,wdg-action > --- > .../bindings/mfd/rohm,bd96801-pmic.yaml | 171 ++++++++++++++++++ > 1 file changed, 171 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml > new file mode 100644 > index 000000000000..31ef787d6a8a > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml > @@ -0,0 +1,171 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/rohm,bd96801-pmic.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ROHM BD96801 Scalable Power Management Integrated Circuit > + > +maintainers: > + - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > + > +description: | Do not need '|' unless you need to preserve formatting. > + BD96801 is an automotive grade single-chip power management IC. > + It integrates 4 buck converters and 3 LDOs with safety features like > + over-/under voltage and over current detection and a watchdog. > + > +properties: > + compatible: > + const: rohm,bd96801 > + > + reg: > + description: > + I2C slave address. Drop description, obvious. > + maxItems: 1 > + > + interrupts: > + description: > + The PMIC provides intb and errb IRQ lines. The errb IRQ line is used > + for fatal IRQs which will cause the PMIC to shut down power outputs. > + In many systems this will shut down the SoC contolling the PMIC and > + connecting/handling the errb can be omitted. However, there are cases > + where the SoC is not powered by the PMIC. In that case it may be > + useful to connect the errb and handle errb events. > + minItems: 1 > + maxItems: 2 > + > + interrupt-names: > + minItems: 1 > + items: > + - enum: [intb, errb] > + - const: errb > + > + rohm,hw-timeout-ms: > + description: > + Watchdog timeout value(s). First walue is timeout limit. Second value is > + optional value for 'too early' watchdog ping if window timeout mode is > + to be used. Standard property timeout-sec does not work for you? It should allow two items as well. > + minItems: 1 > + maxItems: 2 > + > + rohm,wdg-action: > + description: > + Whether the watchdog failure must turn off the regulator power outputs or > + just toggle the INTB line. > + enum: > + - prstb > + - intb-only This is second property controlling bite behavior. The other being: https://lore.kernel.org/all/e86812b3-a3aa-4bdb-9b32-a0339f0f76b5@kernel.org/ Probably we need common property in watchdog.yaml. > + > + regulators: > + $ref: ../regulator/rohm,bd96801-regulator.yaml Full path, so /schemas/regulator/.... > + description: > + List of child nodes that specify the regulators. > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-names > + - regulators > + Missing allOf and $ref to watchdog.yaml > +additionalProperties: false > + > +examples: Best regards, Krzysztof
On 13/04/2024 23:33, Krzysztof Kozlowski wrote: >> + rohm,wdg-action: >> + description: >> + Whether the watchdog failure must turn off the regulator power outputs or >> + just toggle the INTB line. >> + enum: >> + - prstb >> + - intb-only > > This is second property controlling bite behavior. The other being: > https://lore.kernel.org/all/e86812b3-a3aa-4bdb-9b32-a0339f0f76b5@kernel.org/ > > Probably we need common property in watchdog.yaml. No, that's not the same case. I mixed up topics. Best regards, Krzysztof
Morning Krzysztof, Thanks again for the review/help! On 4/14/24 00:33, Krzysztof Kozlowski wrote: > On 12/04/2024 13:21, Matti Vaittinen wrote >> + >> + rohm,hw-timeout-ms: >> + description: >> + Watchdog timeout value(s). First walue is timeout limit. Second value is >> + optional value for 'too early' watchdog ping if window timeout mode is >> + to be used. > > Standard property timeout-sec does not work for you? It should allow two > items as well. I don't think so. We need sub-second units. Furthermore, the timeout-sec (if I understand it correctly) updates the "timeout policy", which tells the expected ping-interval. This can be different from the "HW heart-beat" which tells the HW's ping expectation. Hence the "hw-" prefix. > Missing allOf This just about summarizes my feelings when I try write the bindings. XD I do feel completely lost. Hence I do really appreciate someone like you taking the time to help me through ^^; Enjoy the Seattle! Yours, -- Matti
On 4/15/24 08:50, Matti Vaittinen wrote: > Morning Krzysztof, > > Thanks again for the review/help! > > On 4/14/24 00:33, Krzysztof Kozlowski wrote: >> On 12/04/2024 13:21, Matti Vaittinen wrote >>> + >>> + rohm,hw-timeout-ms: >>> + description: >>> + Watchdog timeout value(s). First walue is timeout limit. >>> Second value is >>> + optional value for 'too early' watchdog ping if window timeout >>> mode is >>> + to be used. >> >> Standard property timeout-sec does not work for you? It should allow two >> items as well. > > I don't think so. We need sub-second units. Furthermore, the timeout-sec > (if I understand it correctly) updates the "timeout policy", which tells > the expected ping-interval. This can be different from the "HW > heart-beat" which tells the HW's ping expectation. Hence the "hw-" prefix. Oh, I just found out that this is an existing property. The ROHM BD9576/BD9573 do aleady use this. It seems I've had some discussion about it with Rob/Guenter when adding it. Frightening thing is that I didin't remember the discussion or that the property existed at all... Well, luckily we have lore :) https://lore.kernel.org/all/c390476e4279d8b75de53271e9fb8948d8854528.camel@fi.rohmeurope.com/#r (I don't see the final conclusion in this discussion, it has probably been done on some later version of the series). Yours, -- Matti
On 4/14/24 00:33, Krzysztof Kozlowski wrote: > On 12/04/2024 13:21, Matti Vaittinen wrote: >> ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce >> DT bindings for the BD96801 core. >> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> >> >> --- >> Revision history: >> RFCv1 => RFCv2: >> - Document rohm,hw-timeout-ms >> - Document rohm,wdg-action >> --- >> .../bindings/mfd/rohm,bd96801-pmic.yaml | 171 ++++++++++++++++++ >> 1 file changed, 171 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml >> >> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml ... > > Missing allOf and $ref to watchdog.yaml Huh. The watchdog.yaml contains: select: properties: $nodename: pattern: "^watchdog(@.*|-([0-9]|[1-9][0-9]+))?$" properties: $nodename: pattern: "^(timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$" This means the watchdog _must_ have own sub-node inside the PMIC node, right? Yours, -- Matti
On 15/04/2024 08:24, Matti Vaittinen wrote: > On 4/15/24 08:50, Matti Vaittinen wrote: >> Morning Krzysztof, >> >> Thanks again for the review/help! >> >> On 4/14/24 00:33, Krzysztof Kozlowski wrote: >>> On 12/04/2024 13:21, Matti Vaittinen wrote >>>> + >>>> + rohm,hw-timeout-ms: >>>> + description: >>>> + Watchdog timeout value(s). First walue is timeout limit. >>>> Second value is >>>> + optional value for 'too early' watchdog ping if window timeout >>>> mode is >>>> + to be used. >>> >>> Standard property timeout-sec does not work for you? It should allow two >>> items as well. >> >> I don't think so. We need sub-second units. Furthermore, the timeout-sec >> (if I understand it correctly) updates the "timeout policy", which tells >> the expected ping-interval. This can be different from the "HW >> heart-beat" which tells the HW's ping expectation. Hence the "hw-" prefix. > > Oh, I just found out that this is an existing property. The ROHM > BD9576/BD9573 do aleady use this. It seems I've had some discussion > about it with Rob/Guenter when adding it. Frightening thing is that I > didin't remember the discussion or that the property existed at all... > Well, luckily we have lore :) > > https://lore.kernel.org/all/c390476e4279d8b75de53271e9fb8948d8854528.camel@fi.rohmeurope.com/#r > > (I don't see the final conclusion in this discussion, it has probably > been done on some later version of the series). > Sure, it's fine then. Best regards, Krzysztof
On 15/04/2024 10:28, Matti Vaittinen wrote: >> >> Missing allOf and $ref to watchdog.yaml > > Huh. The watchdog.yaml contains: > > select: > properties: > $nodename: > pattern: "^watchdog(@.*|-([0-9]|[1-9][0-9]+))?$" > > properties: > $nodename: > pattern: "^(timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$" > > > This means the watchdog _must_ have own sub-node inside the PMIC node, Yes, that's a bit of a trouble. Then instead maybe just add timeout-sec with maxItems: 2. Best regards, Krzysztof
On 4/18/24 20:28, Krzysztof Kozlowski wrote: > On 15/04/2024 10:28, Matti Vaittinen wrote: >>> >>> Missing allOf and $ref to watchdog.yaml >> >> Huh. The watchdog.yaml contains: >> >> select: >> properties: >> $nodename: >> pattern: "^watchdog(@.*|-([0-9]|[1-9][0-9]+))?$" >> >> properties: >> $nodename: >> pattern: "^(timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$" >> >> >> This means the watchdog _must_ have own sub-node inside the PMIC node, > > Yes, that's a bit of a trouble. Agree. I'm not 100% sure why this requirement? I guess it is a bit of a problem for many MFD devices with a watchdog. > Then instead maybe just add timeout-sec > with maxItems: 2. Nice idea. Thanks for the suggestion, I'll do just that! Yours, -- Matti
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml new file mode 100644 index 000000000000..31ef787d6a8a --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml @@ -0,0 +1,171 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/rohm,bd96801-pmic.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ROHM BD96801 Scalable Power Management Integrated Circuit + +maintainers: + - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> + +description: | + BD96801 is an automotive grade single-chip power management IC. + It integrates 4 buck converters and 3 LDOs with safety features like + over-/under voltage and over current detection and a watchdog. + +properties: + compatible: + const: rohm,bd96801 + + reg: + description: + I2C slave address. + maxItems: 1 + + interrupts: + description: + The PMIC provides intb and errb IRQ lines. The errb IRQ line is used + for fatal IRQs which will cause the PMIC to shut down power outputs. + In many systems this will shut down the SoC contolling the PMIC and + connecting/handling the errb can be omitted. However, there are cases + where the SoC is not powered by the PMIC. In that case it may be + useful to connect the errb and handle errb events. + minItems: 1 + maxItems: 2 + + interrupt-names: + minItems: 1 + items: + - enum: [intb, errb] + - const: errb + + rohm,hw-timeout-ms: + description: + Watchdog timeout value(s). First walue is timeout limit. Second value is + optional value for 'too early' watchdog ping if window timeout mode is + to be used. + minItems: 1 + maxItems: 2 + + rohm,wdg-action: + description: + Whether the watchdog failure must turn off the regulator power outputs or + just toggle the INTB line. + enum: + - prstb + - intb-only + + regulators: + $ref: ../regulator/rohm,bd96801-regulator.yaml + description: + List of child nodes that specify the regulators. + +required: + - compatible + - reg + - interrupts + - interrupt-names + - regulators + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/leds/common.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + pmic: pmic@60 { + reg = <0x60>; + compatible = "rohm,bd96801"; + interrupt-parent = <&gpio1>; + interrupts = <29 IRQ_TYPE_LEVEL_LOW>, <6 IRQ_TYPE_LEVEL_LOW>; + interrupt-names = "intb", "errb"; + + regulators { + buck1: BUCK1 { + regulator-name = "buck1"; + regulator-ramp-delay = <1250>; + /* 0.5V min INITIAL - 150 mV tune */ + regulator-min-microvolt = <350000>; + /* 3.3V + 150mV tune */ + regulator-max-microvolt = <3450000>; + + /* These can be set only when PMIC is in STBY */ + rohm,initial-voltage-microvolt = <500000>; + regulator-ov-error-microvolt = <230000>; + regulator-uv-error-microvolt = <230000>; + regulator-temp-protection-kelvin = <1>; + regulator-temp-warn-kelvin = <0>; + }; + buck2: BUCK2 { + regulator-name = "buck2"; + regulator-min-microvolt = <350000>; + regulator-max-microvolt = <3450000>; + + rohm,initial-voltage-microvolt = <3000000>; + regulator-ov-error-microvolt = <18000>; + regulator-uv-error-microvolt = <18000>; + regulator-temp-protection-kelvin = <1>; + regulator-temp-warn-kelvin = <1>; + }; + buck3: BUCK3 { + regulator-name = "buck3"; + regulator-min-microvolt = <350000>; + regulator-max-microvolt = <3450000>; + + rohm,initial-voltage-microvolt = <600000>; + regulator-ov-warn-microvolt = <18000>; + regulator-uv-warn-microvolt = <18000>; + regulator-temp-protection-kelvin = <1>; + regulator-temp-error-kelvin = <0>; + }; + buck4: BUCK4 { + regulator-name = "buck4"; + regulator-min-microvolt = <350000>; + regulator-max-microvolt = <3450000>; + + rohm,initial-voltage-microvolt = <600000>; + regulator-ov-warn-microvolt = <18000>; + regulator-uv-warn-microvolt = <18000>; + regulator-temp-protection-kelvin = <1>; + regulator-temp-error-kelvin = <0>; + }; + ldo5: LDO5 { + regulator-name = "ldo5"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <3300000>; + + rohm,initial-voltage-microvolt = <500000>; + regulator-ov-error-microvolt = <36000>; + regulator-uv-error-microvolt = <34000>; + regulator-temp-protection-kelvin = <1>; + regulator-temp-warn-kelvin = <0>; + }; + ldo6: LDO6 { + regulator-name = "ldo6"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <3300000>; + + rohm,initial-voltage-microvolt = <300000>; + regulator-ov-error-microvolt = <36000>; + regulator-uv-error-microvolt = <34000>; + regulator-temp-protection-kelvin = <1>; + regulator-temp-warn-kelvin = <0>; + }; + ldo7: LDO7 { + regulator-name = "ldo7"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <3300000>; + + rohm,initial-voltage-microvolt = <500000>; + regulator-ov-error-microvolt = <36000>; + regulator-uv-error-microvolt = <34000>; + regulator-temp-protection-kelvin = <1>; + regulator-temp-warn-kelvin = <0>; + }; + }; + }; + };
ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce DT bindings for the BD96801 core. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- Revision history: RFCv1 => RFCv2: - Document rohm,hw-timeout-ms - Document rohm,wdg-action --- .../bindings/mfd/rohm,bd96801-pmic.yaml | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml