Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9412f6ff-1fd7-31b5-56a7-12bda174f7f6@linaro.org>
Date:   Wed, 30 Nov 2022 17:24:11 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH 1/8] dt-bindings: power: supply: Add DT schema for
 Qualcomm SMBCHG
Content-Language: en-US
To:     Yassine Oudjana <yassine.oudjana@gmail.com>,
        Sebastian Reichel <sre@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     Yassine Oudjana <y.oudjana@protonmail.com>,
        Alejandro Tafalla <atafalla@dnyon.com>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, phone-devel@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220808073459.396278-1-y.oudjana@protonmail.com>
 <20220808073459.396278-2-y.oudjana@protonmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20220808073459.396278-2-y.oudjana@protonmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 08/08/2022 09:34, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 


> +      See Documentation/devicetree/bindings/power/supply/battery.yaml
> +
> +  interrupts:
> +    items:
> +      - description: Charger error
> +      - description: Charger inhibited
> +      - description: Charger precharge safety timer timeout
> +      - description: Charger charge safety timer timeout
> +      - description: Charger pre to fast charging switch threshold reached
> +      - description: Charger recharge threshold reached
> +      - description: Charger taper threshold reached

After discussing with Dmitry, it's current design of the PMIC (parent
device) so it's fine. We would need to make bigger refactoring of
drivers and bindings to change it to match other typical MFD PMICs. In
current state it's the only way to express device interrupts.

Two more comments below.

> +      - description: Charger charge termination threshold reached
> +      - description: Battery hot
> +      - description: Battery warm
> +      - description: Battery cold
> +      - description: Battery cool
> +      - description: Battery overvoltage
> +      - description: Battery low
> +      - description: Battery missing
> +      - description: Battery thermistor missing # unconfirmed
> +      - description: USB input undervolt
> +      - description: USB input overvolt
> +      - description: USB input source detected
> +      - description: OTG regulator failure
> +      - description: OTG regulator overcurrent
> +      - description: Automatic input current limiting done
> +      - description: USB ID pin changed
> +      - description: DC input undervolt
> +      - description: DC input overvolt
> +      - description: Power OK
> +      - description: Temperature shutdown
> +      - description: Watchdog timeout
> +      - description: Flash failure
> +      - description: OTST2 # unknown
> +      - description: OTST3 # unknown
> +
> +  interrupt-names:
> +    items:
> +      - const: chg-error
> +      - const: chg-inhibit
> +      - const: chg-prechg-sft
> +      - const: chg-complete-chg-sft
> +      - const: chg-p2f-thr
> +      - const: chg-rechg-thr
> +      - const: chg-taper-thr
> +      - const: chg-tcc-thr
> +      - const: batt-hot
> +      - const: batt-warm
> +      - const: batt-cold
> +      - const: batt-cool
> +      - const: batt-ov
> +      - const: batt-low
> +      - const: batt-missing
> +      - const: batt-term-missing
> +      - const: usbin-uv
> +      - const: usbin-ov
> +      - const: usbin-src-det
> +      - const: otg-fail
> +      - const: otg-oc
> +      - const: aicl-done
> +      - const: usbid-change
> +      - const: dcin-uv
> +      - const: dcin-ov
> +      - const: power-ok
> +      - const: temp-shutdown
> +      - const: wdog-timeout
> +      - const: flash-fail
> +      - const: otst2
> +      - const: otst3
> +
> +  otg-vbus:
> +    type: object

I think I did not comment about this one - this looks like regulator.yaml.

> +
> +    description:
> +      OTG regulator subnode.
> +
> +required:
> +  - compatible
> +  - reg
> +  - monitored-battery
> +  - interrupts
> +  - interrupt-names
> +  - otg-vbus
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    battery: battery {
> +        compatible = "simple-battery";
> +
> +        charge-full-design-microamp-hours = <4070000>;
> +        charge-term-current-microamp = <100000>;
> +        voltage-min-design-microvolt = <3400000>;
> +        voltage-max-design-microvolt = <4400000>;
> +    };
> +
> +    pmic {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        charger@1000 {
> +            compatible = "qcom,pmi8996-smbchg";
> +            reg = <0x1000>;
> +
> +            interrupts = <0x2 0x10 0x0 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x10 0x1 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x10 0x2 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x10 0x3 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x10 0x4 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x10 0x5 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x10 0x6 IRQ_TYPE_EDGE_RISING>,
> +                         <0x2 0x10 0x7 IRQ_TYPE_EDGE_RISING>,
> +                         <0x2 0x12 0x0 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x12 0x1 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x12 0x2 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x12 0x3 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x12 0x4 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x12 0x5 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x12 0x6 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x12 0x7 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x13 0x0 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x13 0x1 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x13 0x2 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x13 0x3 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x13 0x4 IRQ_TYPE_EDGE_RISING>,
> +                         <0x2 0x13 0x5 IRQ_TYPE_EDGE_RISING>,
> +                         <0x2 0x13 0x6 IRQ_TYPE_EDGE_FALLING>,
> +                         <0x2 0x14 0x0 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x14 0x1 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x16 0x0 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x16 0x1 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x16 0x2 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x16 0x3 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x16 0x4 IRQ_TYPE_EDGE_BOTH>,
> +                         <0x2 0x16 0x5 IRQ_TYPE_EDGE_BOTH>;
> +            interrupt-names = "chg-error",
> +                              "chg-inhibit",
> +                              "chg-prechg-sft",
> +                              "chg-complete-chg-sft",
> +                              "chg-p2f-thr",
> +                              "chg-rechg-thr",
> +                              "chg-taper-thr",
> +                              "chg-tcc-thr",
> +                              "batt-hot",
> +                              "batt-warm",
> +                              "batt-cold",
> +                              "batt-cool",
> +                              "batt-ov",
> +                              "batt-low",
> +                              "batt-missing",
> +                              "batt-term-missing",
> +                              "usbin-uv",
> +                              "usbin-ov",
> +                              "usbin-src-det",
> +                              "otg-fail",
> +                              "otg-oc",
> +                              "aicl-done",
> +                              "usbid-change",
> +                              "dcin-uv",
> +                              "dcin-ov",
> +                              "power-ok",
> +                              "temp-shutdown",
> +                              "wdog-timeout",
> +                              "flash-fail",
> +                              "otst2",
> +                              "otst3";
> +
> +            monitored-battery = <&battery>;
> +
> +            otg-vbus { };

Why empty?

Best regards,
Krzysztof