[PATCH 0/4] Add support for Qualcomm QCA639x chips family

[PATCH 0/4] Add support for Qualcomm QCA639x chips family

[Dmitry Baryshkov]

Qualcomm QCA639x is a family of WiFi + Bluetooth chips, with BT part
being controlled through the UART and WiFi being present on PCIe
bus. Both blocks share common power sources wich should be turned on
before either of devices can be probed. Declare common 'qca639x' driver
providing a power domain to be used by both BT and WiFi parts.

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

[Dmitry Baryshkov]

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@linaro.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@kernel.org>
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+description: |
+  This binding describes thes Qualcomm QCA6390 or QCA6391 power supplies and
+  enablement pins.
+
+properties:
+  compatible:
+    const: qcom,qca639x
+
+  '#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
+
+  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

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

[Rob Herring]

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@linaro.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@kernel.org>
> +  - Bjorn Andersson <bjorn.andersson@linaro.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.

> +
> +properties:
> +  compatible:
> +    const: qcom,qca639x

List each device, we don't do wildcards in compatible strings. 

> +
> +  '#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?

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

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

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

[Dmitry Baryshkov]

Hello,

On Fri, 1 Jan 2021 at 01:50, Rob Herring <robh@kernel.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@linaro.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@kernel.org>
> > +  - Bjorn Andersson <bjorn.andersson@linaro.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

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

[Rob Herring]

On Sat, Jan 2, 2021 at 9:41 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Hello,
>
> On Fri, 1 Jan 2021 at 01:50, Rob Herring <robh@kernel.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@linaro.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@kernel.org>
> > > +  - Bjorn Andersson <bjorn.andersson@linaro.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).

This is an abuse of the power-domains binding and a complete hack, so
no. The wifi part should be a child node on the PCI bus. That's the
only acceptable solution for DT.

Obviously there's a probe chicken and egg problem for Linux, but for
DT it doesn't matter. You have 2 options. You can fix PCIe to force
probe devices with a DT node (and lots of folks would appreciate it
because you aren't the only one needing it). If there's a DT node,
then you know there is a device there. This is what MDIO bus does. Or
you can keep your misc driver, but it needs to go find the PCIe child
node itself. IOW, you have to create the platform device yourself in
the initcall rather than rely on the DT code to create one.

Personally, I wouldn't accept the 2nd solution as I think it is still
a hack, but I won't object.

Rob

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

[Dmitry Baryshkov]

Hi Rob,

On Thu, 14 Jan 2021 at 17:33, Rob Herring <robh@kernel.org> wrote:
>
> On Sat, Jan 2, 2021 at 9:41 PM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > Hello,
> >
> > On Fri, 1 Jan 2021 at 01:50, Rob Herring <robh@kernel.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@linaro.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@kernel.org>
> > > > +  - Bjorn Andersson <bjorn.andersson@linaro.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).
>
> This is an abuse of the power-domains binding and a complete hack, so
> no. The wifi part should be a child node on the PCI bus. That's the
> only acceptable solution for DT.

I see your point here.

>
> Obviously there's a probe chicken and egg problem for Linux, but for
> DT it doesn't matter. You have 2 options. You can fix PCIe to force
> probe devices with a DT node (and lots of folks would appreciate it
> because you aren't the only one needing it). If there's a DT node,
> then you know there is a device there. This is what MDIO bus does. Or
> you can keep your misc driver, but it needs to go find the PCIe child
> node itself. IOW, you have to create the platform device yourself in
> the initcall rather than rely on the DT code to create one.
>
> Personally, I wouldn't accept the 2nd solution as I think it is still
> a hack, but I won't object.

To make things worse, if the device is not powered during the initial
PCIe host probe, the powerup sequence turns into PCIe hotplug story.
Even PCIehp driver is probed after initial link training. Would you
agree to one of the following variants:

1) Add extra power-domain to pcie host:

 pcie0: pci@1c00000 {
     compatible = "qcom,pcie-sm8250", "snps,dw-pcie";
     [....]
     power-domains = <&gcc PCIE_0_GDSC>, <&qca639x>;
     power-domain-names = "core", "child";
};

2) Add power domain to PCIe bridge device  (and handle that from the host code):

 pcie0: pci@1c00000 {
     compatible = "qcom,pcie-sm8250", "snps,dw-pcie";
     [....]
     power-domains = <&gcc PCIE_0_GDSC>;

     bridge@0,0 {
         compatible = "pci17cb,010b";
         [.....]
         power-domains = <qca639x>;
    };
};

Does any of those variants seem suitable for you?

Or you'd really insist on the following binding:

3)

pcie0: pci@1c00000 {
     compatible = "qcom,pcie-sm8250", "snps,dw-pcie";
     [....]
     power-domains = <&gcc PCIE_0_GDSC>;

     bridge@0,0 {
         compatible = "pci17cb,010b";
         [.....]
         wifi@1,0 {
             compatible = "pci17cb,1101";
             power-domains = <qca639x>;
          };
    };
};


-- 
With best wishes
Dmitry

[PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence

[Dmitry Baryshkov]

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. So add mfd device driver
handling power sequencing of QCA6390/1.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/mfd/Kconfig        |  12 +++
 drivers/mfd/Makefile       |   1 +
 drivers/mfd/qcom-qca639x.c | 168 +++++++++++++++++++++++++++++++++++++
 3 files changed, 181 insertions(+)
 create mode 100644 drivers/mfd/qcom-qca639x.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index bdfce7b15621..2fd6b9770ad0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1036,6 +1036,18 @@ config MFD_PM8XXX
 	  Say M here if you want to include support for PM8xxx chips as a
 	  module. This will build a module called "pm8xxx-core".
 
+config MFD_QCOM_QCA639X
+	tristate "Qualcomm QCA639x WiFi/Bluetooth module support"
+	depends on REGULATOR && PM_GENERIC_DOMAINS
+	help
+	  If you say yes to this option, support will be included for Qualcomm
+	  QCA639x family of WiFi and Bluetooth SoCs. Note, this driver supports
+	  only power control for this SoC, you still have to enable individual
+	  Bluetooth and WiFi drivers.
+
+	  Say M here if you want to include support for QCA639x chips as a
+	  module. This will build a module called "qcom-qca639x".
+
 config MFD_QCOM_RPM
 	tristate "Qualcomm Resource Power Manager (RPM)"
 	depends on ARCH_QCOM && OF
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 14fdb188af02..da5747508faf 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -202,6 +202,7 @@ obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
 obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
 obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
 obj-$(CONFIG_MFD_PM8XXX) 	+= qcom-pm8xxx.o ssbi.o
+obj-$(CONFIG_MFD_QCOM_QCA639X)	+= qcom-qca639x.o
 obj-$(CONFIG_MFD_QCOM_RPM)	+= qcom_rpm.o
 obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
diff --git a/drivers/mfd/qcom-qca639x.c b/drivers/mfd/qcom-qca639x.c
new file mode 100644
index 000000000000..1ecc2e2e5bfd
--- /dev/null
+++ b/drivers/mfd/qcom-qca639x.c
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/devinfo.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#define MAX_NUM_REGULATORS	8
+
+static struct vreg {
+	const char *name;
+	unsigned int load_uA;
+} vregs[MAX_NUM_REGULATORS] = {
+	/* 2.0 V */
+	{ "vddpcie2", 15000 },
+	{ "vddrfa3", 400000 },
+
+	/* 0.95 V */
+	{ "vddaon", 100000 },
+	{ "vddpmu", 1250000 },
+	{ "vddrfa1", 200000 },
+
+	/* 1.35 V */
+	{ "vddrfa2", 400000 },
+	{ "vddpcie1", 35000 },
+
+	/* 1.8 V */
+	{ "vddio", 20000 },
+};
+
+struct qca639x_data {
+	struct regulator_bulk_data regulators[MAX_NUM_REGULATORS];
+	size_t num_vregs;
+	struct device *dev;
+	struct pinctrl_state *active_state;
+	struct generic_pm_domain pd;
+};
+
+#define domain_to_data(domain) container_of(domain, struct qca639x_data, pd)
+
+static int qca639x_power_on(struct generic_pm_domain *domain)
+{
+	struct qca639x_data *data = domain_to_data(domain);
+	int ret;
+
+	dev_warn(&domain->dev, "DUMMY POWER ON\n");
+
+	ret = regulator_bulk_enable(data->num_vregs, data->regulators);
+	if (ret) {
+		dev_err(data->dev, "Failed to enable regulators");
+		return ret;
+	}
+
+	/* Wait for 1ms before toggling enable pins. */
+	usleep_range(1000, 2000);
+
+	ret = pinctrl_select_state(data->dev->pins->p, data->active_state);
+	if (ret) {
+		dev_err(data->dev, "Failed to select active state");
+		return ret;
+	}
+
+	/* Wait for all power levels to stabilize */
+	usleep_range(6000, 7000);
+
+	return 0;
+}
+
+static int qca639x_power_off(struct generic_pm_domain *domain)
+{
+	struct qca639x_data *data = domain_to_data(domain);
+
+	dev_warn(&domain->dev, "DUMMY POWER OFF\n");
+
+	pinctrl_select_default_state(data->dev);
+	regulator_bulk_disable(data->num_vregs, data->regulators);
+
+	return 0;
+}
+
+static int qca639x_probe(struct platform_device *pdev)
+{
+	struct qca639x_data *data;
+	struct device *dev = &pdev->dev;
+	int i, ret;
+
+	if (!dev->pins || IS_ERR_OR_NULL(dev->pins->default_state))
+		return -EINVAL;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = dev;
+	data->num_vregs = ARRAY_SIZE(vregs);
+
+	data->active_state = pinctrl_lookup_state(dev->pins->p, "active");
+	if (IS_ERR(data->active_state)) {
+		ret = PTR_ERR(data->active_state);
+		dev_err(dev, "Failed to get active_state: %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0; i < data->num_vregs; i++)
+		data->regulators[i].supply = vregs[i].name;
+	ret = devm_regulator_bulk_get(dev, data->num_vregs, data->regulators);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < data->num_vregs; i++) {
+		ret = regulator_set_load(data->regulators[i].consumer, vregs[i].load_uA);
+		if (ret)
+			return ret;
+	}
+
+	data->pd.name = dev_name(dev);
+	data->pd.power_on = qca639x_power_on;
+	data->pd.power_off = qca639x_power_off;
+
+	ret = pm_genpd_init(&data->pd, NULL, true);
+	if (ret < 0)
+		return ret;
+
+	ret = of_genpd_add_provider_simple(dev->of_node, &data->pd);
+	if (ret < 0) {
+		pm_genpd_remove(&data->pd);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, data);
+
+	return 0;
+}
+
+static int qca639x_remove(struct platform_device *pdev)
+{
+	struct qca639x_data *data = platform_get_drvdata(pdev);
+
+	pm_genpd_remove(&data->pd);
+
+	return 0;
+}
+
+static const struct of_device_id qca639x_of_match[] = {
+	{ .compatible = "qcom,qca639x" },
+};
+
+static struct platform_driver qca639x_driver = {
+	.probe = qca639x_probe,
+	.remove = qca639x_remove,
+	.driver = {
+		.name = "qca639x",
+		.of_match_table = qca639x_of_match,
+	},
+};
+
+module_platform_driver(qca639x_driver);
+MODULE_AUTHOR("Dmitry Baryshkov <dmitry.baryshkov@linaro.org>");
+MODULE_DESCRIPTION("Power control for Qualcomm QCA639x BT/WiFi chip");
+MODULE_LICENSE("GPL v2");
-- 
2.29.2

Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence

[Lee Jones]

On Sun, 20 Dec 2020, 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. So add mfd device driver
> handling power sequencing of QCA6390/1.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/mfd/Kconfig        |  12 +++
>  drivers/mfd/Makefile       |   1 +
>  drivers/mfd/qcom-qca639x.c | 168 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 181 insertions(+)
>  create mode 100644 drivers/mfd/qcom-qca639x.c

This is not an MFD, since it utilised neither the MFD API nor
of_platform_populate() to register child devices.

Suggest you use drivers/power or similar to handle shared power
sequencing requirements.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence

[Dmitry Baryshkov]

Hello,

On Mon, 21 Dec 2020 at 12:02, Lee Jones <lee.jones@linaro.org> wrote:
>
> On Sun, 20 Dec 2020, 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. So add mfd device driver
> > handling power sequencing of QCA6390/1.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/mfd/Kconfig        |  12 +++
> >  drivers/mfd/Makefile       |   1 +
> >  drivers/mfd/qcom-qca639x.c | 168 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 181 insertions(+)
> >  create mode 100644 drivers/mfd/qcom-qca639x.c
>
> This is not an MFD, since it utilised neither the MFD API nor
> of_platform_populate() to register child devices.

It would use them if the WiFi part was not on a discoverable bus.

> Suggest you use drivers/power or similar to handle shared power
> sequencing requirements.

What about drivers/soc/qcom? Or drivers/misc?

-- 
With best wishes
Dmitry

Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence

[Lee Jones]

On Mon, 21 Dec 2020, Dmitry Baryshkov wrote:

> Hello,
> 
> On Mon, 21 Dec 2020 at 12:02, Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Sun, 20 Dec 2020, 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. So add mfd device driver
> > > handling power sequencing of QCA6390/1.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/mfd/Kconfig        |  12 +++
> > >  drivers/mfd/Makefile       |   1 +
> > >  drivers/mfd/qcom-qca639x.c | 168 +++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 181 insertions(+)
> > >  create mode 100644 drivers/mfd/qcom-qca639x.c
> >
> > This is not an MFD, since it utilised neither the MFD API nor
> > of_platform_populate() to register child devices.
> 
> It would use them if the WiFi part was not on a discoverable bus.

This is a can of worms that I do not wish to open right now.

However the TL;DR is:

 MFD is currently only *meant* for non-discoverable platform devices.

> > Suggest you use drivers/power or similar to handle shared power
> > sequencing requirements.
> 
> What about drivers/soc/qcom? Or drivers/misc?

These are 2 other subsystems, just like MFD, which are commonly used
as dumping grounds for code which doesn't fit anywhere else.  However,
I implore you to please try to find a suitable subsystem for this to
fit into first.  If this driver only deals with power management,
place it is a power related subsystem *before* considering drivers/soc
or drivers/misc.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/4] mfd: qca639x: add support for QCA639x powerup sequence

[Rob Herring]

On Mon, Dec 21, 2020 at 05:08:44PM +0300, Dmitry Baryshkov wrote:
> Hello,
> 
> On Mon, 21 Dec 2020 at 12:02, Lee Jones <lee.jones@linaro.org> wrote:
> >
> > On Sun, 20 Dec 2020, 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. So add mfd device driver
> > > handling power sequencing of QCA6390/1.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/mfd/Kconfig        |  12 +++
> > >  drivers/mfd/Makefile       |   1 +
> > >  drivers/mfd/qcom-qca639x.c | 168 +++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 181 insertions(+)
> > >  create mode 100644 drivers/mfd/qcom-qca639x.c
> >
> > This is not an MFD, since it utilised neither the MFD API nor
> > of_platform_populate() to register child devices.
> 
> It would use them if the WiFi part was not on a discoverable bus.

PCI nodes have been supported in DT for forever. If you have 
non-discoverable additions to a PCI device, then the PCI device should 
be in DT.

Rob

[PATCH 3/4] arm64: dts: qcom: Add Bluetooth support on RB5

[Dmitry Baryshkov]

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Add Bluetooth support on RB5 using the onboard QCA6391 WLAN+BT chipset.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
[DB: added qca639x power domain]
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 97 ++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index 1be984d89f48..e6bab9960cea 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -17,6 +17,7 @@ / {
 	compatible = "qcom,qrb5165-rb5", "qcom,sm8250";
 
 	aliases {
+		hsuart0 = &uart6;
 		serial0 = &uart12;
 		sdhc2 = &sdhc_2;
 	};
@@ -118,6 +119,23 @@ vreg_s4a_1p8: vreg-s4a-1p8 {
 		regulator-max-microvolt = <1800000>;
 		regulator-always-on;
 	};
+
+	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>;
+	};
 };
 
 &apps_rsc {
@@ -131,6 +149,13 @@ pm8009-rpmh-regulators {
 		vdd-l5-l6-supply = <&vreg_bob>;
 		vdd-l7-supply = <&vreg_s4a_1p8>;
 
+		vreg_s2f_0p95: smps2 {
+			regulator-name = "vreg_s2f_0p95";
+			regulator-min-microvolt = <512000>;
+			regulator-max-microvolt = <1100000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+		};
+
 		vreg_l1f_1p1: ldo1 {
 			regulator-name = "vreg_l1f_1p1";
 			regulator-min-microvolt = <1104000>;
@@ -513,6 +538,26 @@ &pm8150_rtc {
 	status = "okay";
 };
 
+&qup_uart6_default {
+	ctsrx {
+		pins = "gpio16", "gpio19";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	rts {
+		pins = "gpio17";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	tx {
+		pins = "gpio18";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -738,6 +783,28 @@ &tlmm {
 		"HST_WLAN_UART_TX",
 		"HST_WLAN_UART_RX";
 
+	bt_default_state: bt-default-state {
+		bt-en {
+			pins = "gpio21";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-low;
+			bias-pull-up;
+		};
+	};
+
+	bt_active_state: bt-active-state {
+		bt-en {
+			pins = "gpio21";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-high;
+			bias-pull-up;
+		};
+	};
+
 	pcie0_default_state: pcie0-default {
 		clkreq {
 			pins = "gpio80";
@@ -838,6 +905,36 @@ sdc2_card_det_n: sd-card-det-n {
 		function = "gpio";
 		bias-pull-up;
 	};
+
+	wlan_default_state: wlan-default-state {
+		wlan-en {
+			pins = "gpio20";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-low;
+			bias-pull-up;
+		};
+	};
+
+	wlan_active_state: wlan-active-state {
+		wlan-en {
+			pins = "gpio20";
+			function = "gpio";
+
+			drive-strength = <16>;
+			output-high;
+			bias-pull-up;
+		};
+	};
+};
+
+&uart6 {
+	status = "okay";
+	bluetooth {
+		compatible = "qcom,qca6390-bt";
+		power-domains = <&qca639x>;
+	};
 };
 
 &uart12 {
-- 
2.29.2

[PATCH 4/4] arm64: dtb: qcom: qrb5165-rb5: add power domain to pcie0 phy

[Dmitry Baryshkov]

If QCA6391 chip (connected to PCIe0) is not powered at the PCIe probe
time, PCIe0 bus probe will timeout and the device will not be detected.
To ease device power up support, use qca639x as pcie0 phy power-domain.
This allows us to make sure that QCA6391 chip is powered on before PCIe0
probe happens.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index e6bab9960cea..9aa7793cd7c0 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -457,6 +457,9 @@ &pcie0_phy {
 	status = "okay";
 	vdda-phy-supply = <&vreg_l5a_0p88>;
 	vdda-pll-supply = <&vreg_l9a_1p2>;
+
+	/* Power on QCA639x chip, otherwise PCIe bus timeouts */
+	power-domains = <&qca639x>;
 };
 
 &pcie1 {
-- 
2.29.2