diff mbox series

[RFC,v2,2/6] dt-bindings: mfd: bd96801 PMIC core

Message ID ea49494429528cf8e60fa984ae1f523ddacd850c.1712920132.git.mazziesaccount@gmail.com
State New
Headers show
Series Support ROHM BD96801 scalable PMIC | expand

Commit Message

Matti Vaittinen April 12, 2024, 11:21 a.m. UTC
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

Comments

Krzysztof Kozlowski April 13, 2024, 9:36 p.m. UTC | #1
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
Matti Vaittinen April 15, 2024, 6:24 a.m. UTC | #2
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
Krzysztof Kozlowski April 15, 2024, 3:25 p.m. UTC | #3
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
Krzysztof Kozlowski April 18, 2024, 5:28 p.m. UTC | #4
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
Matti Vaittinen April 19, 2024, 5:48 a.m. UTC | #5
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 mbox series

Patch

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>;
+                };
+            };
+        };
+    };