diff mbox series

[11/13] dt-bindings: phy: qcom,qmp-usb: fix sc8280xp binding

Message ID 20221024100632.20549-12-johan+linaro@kernel.org
State New
Headers
Series phy: qcom-qmp-usb: fix sc8280xp binding |

Commit Message

Johan Hovold Oct. 24, 2022, 10:06 a.m. UTC 
  The current QMP USB PHY bindings are based on the original MSM8996 PCIe
PHY binding which provided multiple PHYs per IP block and these in turn
were described by child nodes.

The QMP USB PHY block only provide a single PHY and the remnant child
node does not really reflect the hardware.

The original MSM8996 binding also ended up describing the individual
register blocks as belonging to either the wrapper node or the PHY child
nodes.

This is an unnecessary level of detail which has lead to problems when
later IP blocks using different register layouts have been forced to fit
the original mould rather than updating the binding. The bindings are
arguable also incomplete as they only the describe register blocks used
by the current Linux drivers (e.g. does not include the per lane PCS
registers).

Note that PCS_USB region is also not described by the current bindings
despite being used by the driver and this has led to people increasing
the size of the PCS region in the devicetree so that it includes PCS_USB
registers even though other regions like TX and RX may lie in between.

Add a new binding for the QMP USB PHYs found on SC8280XP which further
bindings can be based on.

Note that this also fixes the SC8280XP "phy_phy" reset name.

Also note that the current binding is simply removed instead of being
deprecated as it was only recently merged and support for SC8280XP is
still under development. And, specifically, there is no support in
mainline for the multiport controller that uses these PHYs.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../phy/qcom,msm8996-qmp-usb-phy.yaml         |  13 ---
 .../phy/qcom,sc8280xp-qmp-usb-phy.yaml        | 105 ++++++++++++++++++
 2 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml

Comments

Krzysztof Kozlowski Oct. 28, 2022, 2:15 a.m. UTC | #1 
On 24/10/2022 06:06, Johan Hovold wrote:
> The current QMP USB PHY bindings are based on the original MSM8996 PCIe
> PHY binding which provided multiple PHYs per IP block and these in turn
> were described by child nodes.
> 
> The QMP USB PHY block only provide a single PHY and the remnant child
> node does not really reflect the hardware.
> 
> The original MSM8996 binding also ended up describing the individual
> register blocks as belonging to either the wrapper node or the PHY child
> nodes.
> 

(...)

>      then:
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
> new file mode 100644
> index 000000000000..95ee81d782f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml

Filename based on compatible, so in this case
"qcom,sc8280xp-qmp-usb3-uni-phy.yaml", unless it's like the PCI case?


Best regards,
Krzysztof
Johan Hovold Oct. 28, 2022, 6:09 a.m. UTC | #2 
On Thu, Oct 27, 2022 at 10:15:45PM -0400, Krzysztof Kozlowski wrote:
> On 24/10/2022 06:06, Johan Hovold wrote:
> > The current QMP USB PHY bindings are based on the original MSM8996 PCIe
> > PHY binding which provided multiple PHYs per IP block and these in turn
> > were described by child nodes.
> > 
> > The QMP USB PHY block only provide a single PHY and the remnant child
> > node does not really reflect the hardware.
> > 
> > The original MSM8996 binding also ended up describing the individual
> > register blocks as belonging to either the wrapper node or the PHY child
> > nodes.
> > 
> 
> (...)
> 
> >      then:
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
> > new file mode 100644
> > index 000000000000..95ee81d782f9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
> 
> Filename based on compatible, so in this case
> "qcom,sc8280xp-qmp-usb3-uni-phy.yaml", unless it's like the PCI case?

Yes, you're right. Thanks for catching that. This should be the only
sc8280xp USB PHY compatible so the file name should be updates as you
suggested.

I'll wait a bit and see if Vinod has any comments before respinning this
series.

Johan
Johan Hovold Oct. 28, 2022, 6:13 a.m. UTC | #3 
On Fri, Oct 28, 2022 at 08:09:11AM +0200, Johan Hovold wrote:
> On Thu, Oct 27, 2022 at 10:15:45PM -0400, Krzysztof Kozlowski wrote:
> > On 24/10/2022 06:06, Johan Hovold wrote:
> > > The current QMP USB PHY bindings are based on the original MSM8996 PCIe
> > > PHY binding which provided multiple PHYs per IP block and these in turn
> > > were described by child nodes.
> > > 
> > > The QMP USB PHY block only provide a single PHY and the remnant child
> > > node does not really reflect the hardware.
> > > 
> > > The original MSM8996 binding also ended up describing the individual
> > > register blocks as belonging to either the wrapper node or the PHY child
> > > nodes.
> > > 
> > 
> > (...)
> > 
> > >      then:
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
> > > new file mode 100644
> > > index 000000000000..95ee81d782f9
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
> > 
> > Filename based on compatible, so in this case
> > "qcom,sc8280xp-qmp-usb3-uni-phy.yaml", unless it's like the PCI case?
> 
> Yes, you're right. Thanks for catching that. This should be the only
> sc8280xp USB PHY compatible so the file name should be updates as you
> suggested.

Looks like I missed the '3' in 'usb3' in the previous patch as well.

Johan
Vinod Koul Oct. 28, 2022, 1:05 p.m. UTC | #4 
On 28-10-22, 08:09, Johan Hovold wrote:
> On Thu, Oct 27, 2022 at 10:15:45PM -0400, Krzysztof Kozlowski wrote:
> > On 24/10/2022 06:06, Johan Hovold wrote:
> > > The current QMP USB PHY bindings are based on the original MSM8996 PCIe
> > > PHY binding which provided multiple PHYs per IP block and these in turn
> > > were described by child nodes.
> > > 
> > > The QMP USB PHY block only provide a single PHY and the remnant child
> > > node does not really reflect the hardware.
> > > 
> > > The original MSM8996 binding also ended up describing the individual
> > > register blocks as belonging to either the wrapper node or the PHY child
> > > nodes.
> > > 
> > 
> > (...)
> > 
> > >      then:
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
> > > new file mode 100644
> > > index 000000000000..95ee81d782f9
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
> > 
> > Filename based on compatible, so in this case
> > "qcom,sc8280xp-qmp-usb3-uni-phy.yaml", unless it's like the PCI case?
> 
> Yes, you're right. Thanks for catching that. This should be the only
> sc8280xp USB PHY compatible so the file name should be updates as you
> suggested.
> 
> I'll wait a bit and see if Vinod has any comments before respinning this
> series.

lgtm, pls respin
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb-phy.yaml
index a72e96f241d1..874c32b75428 100644
--- a/Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb-phy.yaml
@@ -26,7 +26,6 @@  properties:
       - qcom,qcm2290-qmp-usb3-phy
       - qcom,sc7180-qmp-usb3-phy
       - qcom,sc8180x-qmp-usb3-phy
-      - qcom,sc8280xp-qmp-usb3-uni-phy
       - qcom,sdm845-qmp-usb3-phy
       - qcom,sdm845-qmp-usb3-uni-phy
       - qcom,sdx55-qmp-usb3-uni-phy
@@ -204,7 +203,6 @@  allOf:
         compatible:
           contains:
             enum:
-              - qcom,sc8280xp-qmp-usb3-uni-phy
               - qcom,sm8150-qmp-usb3-phy
               - qcom,sm8150-qmp-usb3-uni-phy
               - qcom,sm8250-qmp-usb3-uni-phy
@@ -271,16 +269,6 @@  allOf:
             - const: phy_phy
             - const: phy
 
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,sc8280xp-qmp-usb3-uni-phy
-    then:
-      required:
-        - power-domains
-
   - if:
       properties:
         compatible:
@@ -352,7 +340,6 @@  allOf:
           contains:
             enum:
               - qcom,msm8996-qmp-usb3-phy
-              - qcom,sc8280xp-qmp-usb3-uni-phy
               - qcom,sm8250-qmp-usb3-uni-phy
               - qcom,sm8350-qmp-usb3-uni-phy
     then:
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
new file mode 100644
index 000000000000..95ee81d782f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb-phy.yaml
@@ -0,0 +1,105 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-usb-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QMP PHY controller (USB, SC8280XP)
+
+maintainers:
+  - Vinod Koul <vkoul@kernel.org>
+
+description:
+  The QMP PHY controller supports physical layer functionality for a number of
+  controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sc8280xp-qmp-usb3-uni-phy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 5
+
+  clock-names:
+    items:
+      - const: aux
+      - const: ref_clk_src
+      - const: ref
+      - const: com_aux
+      - const: pipe
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: phy
+      - const: phy_phy
+
+  vdda-phy-supply: true
+
+  vdda-pll-supply: true
+
+  "#clock-cells":
+    const: 0
+
+  clock-output-names:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+  - reset-names
+  - vdda-phy-supply
+  - vdda-pll-supply
+  - "#clock-cells"
+  - clock-output-names
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+
+    phy@88ef000 {
+      compatible = "qcom,sc8280xp-qmp-usb3-uni-phy";
+      reg = <0x088ef000 0x2000>;
+
+      clocks = <&gcc GCC_USB3_MP_PHY_AUX_CLK>,
+               <&rpmhcc RPMH_CXO_CLK>,
+               <&gcc GCC_USB3_MP0_CLKREF_CLK>,
+               <&gcc GCC_USB3_MP_PHY_COM_AUX_CLK>,
+               <&gcc GCC_USB3_MP_PHY_PIPE_0_CLK>;
+      clock-names = "aux", "ref_clk_src", "ref", "com_aux",
+                    "pipe";
+
+      power-domains = <&gcc USB30_MP_GDSC>;
+
+      resets = <&gcc GCC_USB3_UNIPHY_MP0_BCR>,
+               <&gcc GCC_USB3UNIPHY_PHY_MP0_BCR>;
+      reset-names = "phy", "phy_phy";
+
+      vdda-phy-supply = <&vreg_l3a>;
+      vdda-pll-supply = <&vreg_l5a>;
+
+      #clock-cells = <0>;
+      clock-output-names = "usb2_phy0_pipe_clk";
+
+      #phy-cells = <0>;
+    };