linux-kernel - Re: [PATCH 1/4] dt-bindings: mfd: qcom,qca639x: add binding for QCA639x defvice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJpp7cJO9Dej3uicPA0+BccqVjs=VphDmGSj05t7SeypAfQ@mail.gmail.com>
Date:   Sun, 3 Jan 2021 06:41:24 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Rob Herring <robh@...nel.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Lee Jones <lee.jones@...aro.org>,
        "open list:DRM DRIVER FOR MSM ADRENO GPU" 
        <linux-arm-msm@...r.kernel.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] dt-bindings: mfd: qcom,qca639x: add binding for
 QCA639x defvice

Hello,

On Fri, 1 Jan 2021 at 01:50, Rob Herring <robh@...nel.org> wrote:
>
> On Sun, Dec 20, 2020 at 07:58:42PM +0300, Dmitry Baryshkov wrote:
> > Qualcomm QCA639x is a family of WiFi + Bluetooth SoCs, with BT part
> > being controlled through the UART and WiFi being present on PCIe bus.
> > Both blocks share common power sources. Add binding to describe power
> > sequencing required to power up this device.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > ---
> >  .../devicetree/bindings/mfd/qcom,qca639x.yaml | 84 +++++++++++++++++++
> >  1 file changed, 84 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/qcom,qca639x.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/qcom,qca639x.yaml b/Documentation/devicetree/bindings/mfd/qcom,qca639x.yaml
> > new file mode 100644
> > index 000000000000..d43c75da136f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/qcom,qca639x.yaml
> > @@ -0,0 +1,84 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/mfd/qcom,qca639x.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Qualcomm QCA639x WiFi + Bluetoot SoC bindings
> > +
> > +maintainers:
> > +  - Andy Gross <agross@...nel.org>
> > +  - Bjorn Andersson <bjorn.andersson@...aro.org>
> > +
> > +description: |
> > +  This binding describes thes Qualcomm QCA6390 or QCA6391 power supplies and
> > +  enablement pins.
>
> Humm, this should really be for the whole device. For BT/WiFi chips
> we've gotten away with 2 nodes for each interface. If that doesn't work
> here, then I think this needs to be 1 node for all, not 3 as it seems
> you are doing.

2 nodes: one for common power sequencer and one for bluetooth part.
WiFi part doesn't need a separate node, but see below.

>
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,qca639x
>
> List each device, we don't do wildcards in compatible strings.

Ack. I will change this to qca6390, as 6391 should be fully compatible
from the power sequence point of view.

>
> > +
> > +  '#power-domain-cells':
> > +    const: 0
> > +
> > +  pinctrl-0: true
> > +  pinctrl-1: true
> > +
> > +  pinctrl-names:
> > +    items:
> > +      - const: default
> > +      - const: active
> > +
> > +  vddaon-supply:
> > +    description:
> > +      0.95V always-on LDO power input
> > +
> > +  vddpmu-supply:
> > +    description:
> > +      0.95V LDO power input to PMU
> > +
> > +  vddrfa1-supply:
> > +    description:
> > +      0.95V LDO power input to RFA
> > +
> > +  vddrfa2-supply:
> > +    description:
> > +      1.25V LDO power input to RFA
> > +
> > +  vddrfa3-supply:
> > +    description:
> > +      2V LDO power input to RFA
> > +
> > +  vddpcie1-supply:
> > +    description:
> > +      1.25V LDO power input to PCIe part
> > +
> > +  vddpcie2-supply:
> > +    description:
> > +      2V LDO power input to PCIe part
>
> Do the PCIe supplies have to be on if only the BT part is used?

Good question. The documentation just tells us to power up all rails.
There are further internal voltage regulators taking care of current
qca639x mode

>
> Supplies are refcounted, so I'd suggest just duplicating the supplies in
> both the BT and PCIe nodes.

While for BT it would be easy, for PCIe it is not that easy. We have
to make sure that the chip is powered up before the respective PCIe
bus is probed (basically before the PCIe controller driver is probed).
I ended up putting a reference to the PCIe PHY device node, making
sure that qca6391 is powered up before the PCIe PHY driver is probed.
PCIe device node itself has its own power-domains entry (PCIE_0_GDSC).

>
> > +
> > +  vddio-supply:
> > +    description:
> > +      1.8V VIO input
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    qca639x: qca639x {
> > +      compatible = "qcom,qca639x";
> > +      #power-domain-cells = <0>;
> > +
> > +      vddaon-supply = <&vreg_s6a_0p95>;
> > +      vddpmu-supply = <&vreg_s2f_0p95>;
> > +      vddrfa1-supply = <&vreg_s2f_0p95>;
> > +      vddrfa2-supply = <&vreg_s8c_1p3>;
> > +      vddrfa3-supply = <&vreg_s5a_1p9>;
> > +      vddpcie1-supply = <&vreg_s8c_1p3>;
> > +      vddpcie2-supply = <&vreg_s5a_1p9>;
> > +      vddio-supply = <&vreg_s4a_1p8>;
> > +      pinctrl-names = "default", "active";
> > +      pinctrl-0 = <&wlan_default_state &bt_default_state>;
> > +      pinctrl-1 = <&wlan_active_state &bt_active_state>;
> > +    };
> > +...
> > --
> > 2.29.2
> >



-- 
With best wishes
Dmitry