diff mbox series

[v3,01/10] dt-bindings: phy: qcom,qmp-usb3-dp: Add DP phy information

Message ID 20200910004902.2252694-2-swboyd@chromium.org
State Superseded
Headers show
Series Support qcom USB3+DP combo phy (or type-c phy) | expand

Commit Message

Stephen Boyd Sept. 10, 2020, 12:48 a.m. UTC
This binding only describes the USB phy inside the USB3 + DP "combo"
phy. Add information for the DP phy and describe the sub-nodes that
represent the DP and USB3 phys that exist inside the combo wrapper.
Remove reg-names from required properties because it isn't required nor
used by the kernel driver.

Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Cc: Chandan Uddaraju <chandanu@codeaurora.org>
Cc: Vara Reddy <varar@codeaurora.org>
Cc: Tanmay Shah <tanmay@codeaurora.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Manu Gautam <mgautam@codeaurora.org>
Cc: Sandeep Maheswaram <sanm@codeaurora.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Jonathan Marek <jonathan@marek.ca>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Rob Clark <robdclark@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 .../bindings/phy/qcom,qmp-usb3-dp-phy.yaml    | 91 +++++++++++++++++--
 1 file changed, 81 insertions(+), 10 deletions(-)

Comments

Stephen Boyd Sept. 10, 2020, 6:48 a.m. UTC | #1
Quoting Stephen Boyd (2020-09-09 17:48:53)
> This binding only describes the USB phy inside the USB3 + DP "combo"
> phy. Add information for the DP phy and describe the sub-nodes that
> represent the DP and USB3 phys that exist inside the combo wrapper.
> Remove reg-names from required properties because it isn't required nor
> used by the kernel driver.
> 
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Cc: Chandan Uddaraju <chandanu@codeaurora.org>
> Cc: Vara Reddy <varar@codeaurora.org>
> Cc: Tanmay Shah <tanmay@codeaurora.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Manu Gautam <mgautam@codeaurora.org>
> Cc: Sandeep Maheswaram <sanm@codeaurora.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Jonathan Marek <jonathan@marek.ca>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Rob Clark <robdclark@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../bindings/phy/qcom,qmp-usb3-dp-phy.yaml    | 91 +++++++++++++++++--
>  1 file changed, 81 insertions(+), 10 deletions(-)

I noticed that I didn't document the new compatible string I'm using,
qcom,sc7180-qmp-usb3-dp-phy, ugh.

Should I copy the whole file over and make a new document for the new
compatible string? That feels like the better solution vs. making this
binding have min/max stuff where it fails to enforce the DP part of the
phy. We can delete this binding once the kernel tree isn't using it,
right?

Rob H?

> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> index ef8ae9f73092..4154f5748d39 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
> @@ -17,13 +17,15 @@ properties:
>        - qcom,sdm845-qmp-usb3-phy
>    reg:
Rob Herring Sept. 15, 2020, 8:24 p.m. UTC | #2
On Wed, Sep 09, 2020 at 11:48:21PM -0700, Stephen Boyd wrote:
> Quoting Stephen Boyd (2020-09-09 17:48:53)
> > This binding only describes the USB phy inside the USB3 + DP "combo"
> > phy. Add information for the DP phy and describe the sub-nodes that
> > represent the DP and USB3 phys that exist inside the combo wrapper.
> > Remove reg-names from required properties because it isn't required nor
> > used by the kernel driver.
> > 
> > Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> > Cc: Chandan Uddaraju <chandanu@codeaurora.org>
> > Cc: Vara Reddy <varar@codeaurora.org>
> > Cc: Tanmay Shah <tanmay@codeaurora.org>
> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Cc: Manu Gautam <mgautam@codeaurora.org>
> > Cc: Sandeep Maheswaram <sanm@codeaurora.org>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Jonathan Marek <jonathan@marek.ca>
> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Cc: <devicetree@vger.kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Rob Clark <robdclark@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >  .../bindings/phy/qcom,qmp-usb3-dp-phy.yaml    | 91 +++++++++++++++++--
> >  1 file changed, 81 insertions(+), 10 deletions(-)
> 
> I noticed that I didn't document the new compatible string I'm using,
> qcom,sc7180-qmp-usb3-dp-phy, ugh.
> 
> Should I copy the whole file over and make a new document for the new
> compatible string? That feels like the better solution vs. making this
> binding have min/max stuff where it fails to enforce the DP part of the
> phy. We can delete this binding once the kernel tree isn't using it,
> right?

It generally depends on how much if/then schema you have (or should 
have) vs. how much is common, but it's a judgement call. It looks 
like you are just extending the binding for the most part. If there's 
dtb warnings until the existing stuff gets updated, that's fine.

Rob
Stephen Boyd Sept. 16, 2020, 7:42 p.m. UTC | #3
Quoting Rob Herring (2020-09-15 13:24:32)
> On Wed, Sep 09, 2020 at 11:48:21PM -0700, Stephen Boyd wrote:
> > Quoting Stephen Boyd (2020-09-09 17:48:53)
> > > This binding only describes the USB phy inside the USB3 + DP "combo"
> > > phy. Add information for the DP phy and describe the sub-nodes that
> > > represent the DP and USB3 phys that exist inside the combo wrapper.
> > > Remove reg-names from required properties because it isn't required nor
> > > used by the kernel driver.
> > > 
> > > Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> > > Cc: Chandan Uddaraju <chandanu@codeaurora.org>
> > > Cc: Vara Reddy <varar@codeaurora.org>
> > > Cc: Tanmay Shah <tanmay@codeaurora.org>
> > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > Cc: Manu Gautam <mgautam@codeaurora.org>
> > > Cc: Sandeep Maheswaram <sanm@codeaurora.org>
> > > Cc: Douglas Anderson <dianders@chromium.org>
> > > Cc: Sean Paul <seanpaul@chromium.org>
> > > Cc: Jonathan Marek <jonathan@marek.ca>
> > > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Cc: <devicetree@vger.kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Rob Clark <robdclark@chromium.org>
> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > > ---
> > >  .../bindings/phy/qcom,qmp-usb3-dp-phy.yaml    | 91 +++++++++++++++++--
> > >  1 file changed, 81 insertions(+), 10 deletions(-)
> > 
> > I noticed that I didn't document the new compatible string I'm using,
> > qcom,sc7180-qmp-usb3-dp-phy, ugh.
> > 
> > Should I copy the whole file over and make a new document for the new
> > compatible string? That feels like the better solution vs. making this
> > binding have min/max stuff where it fails to enforce the DP part of the
> > phy. We can delete this binding once the kernel tree isn't using it,
> > right?
> 
> It generally depends on how much if/then schema you have (or should 
> have) vs. how much is common, but it's a judgement call. It looks 
> like you are just extending the binding for the most part. If there's 
> dtb warnings until the existing stuff gets updated, that's fine.
> 

Ok. I'll just add in the extra compatible and then we can drop the older
ones at some point.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
index ef8ae9f73092..4154f5748d39 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qmp-usb3-dp-phy.yaml
@@ -17,13 +17,15 @@  properties:
       - qcom,sdm845-qmp-usb3-phy
   reg:
     items:
-      - description: Address and length of PHY's common serdes block.
+      - description: Address and length of PHY's USB serdes block.
       - description: Address and length of the DP_COM control block.
+      - description: Address and length of PHY's DP serdes block.
 
   reg-names:
     items:
-      - const: reg-base
+      - const: usb
       - const: dp_com
+      - const: dp
 
   "#clock-cells":
     enum: [ 1, 2 ]
@@ -74,16 +76,74 @@  properties:
 
 #Required nodes:
 patternProperties:
-  "^phy@[0-9a-f]+$":
+  "^usb3-phy@[0-9a-f]+$":
     type: object
     description:
-      Each device node of QMP phy is required to have as many child nodes as
-      the number of lanes the PHY has.
+      The USB3 PHY.
+
+    properties:
+      reg:
+        items:
+          - description: Address and length of TX.
+          - description: Address and length of RX.
+          - description: Address and length of PCS.
+          - description: Address and length of TX2.
+          - description: Address and length of RX2.
+          - description: Address and length of pcs_misc.
+
+      clocks:
+        items:
+          - description: pipe clock
+
+      clock-names:
+        items:
+          - const: pipe0
+
+      clock-output-names:
+        items:
+          - const: usb3_phy_pipe_clk_src
+
+      '#clock-cells':
+        const: 0
+
+      '#phy-cells':
+        const: 0
+
+    required:
+      - reg
+      - clocks
+      - clock-names
+      - '#clock-cells'
+      - '#phy-cells'
+
+  "^dp-phy@[0-9a-f]+$":
+    type: object
+    description:
+      The DP PHY.
+
+    properties:
+      reg:
+        items:
+          - description: Address and length of TX.
+          - description: Address and length of RX.
+          - description: Address and length of PCS.
+          - description: Address and length of TX2.
+          - description: Address and length of RX2.
+
+      '#clock-cells':
+        const: 1
+
+      '#phy-cells':
+        const: 0
+
+    required:
+      - reg
+      - '#clock-cells'
+      - '#phy-cells'
 
 required:
   - compatible
   - reg
-  - reg-names
   - "#clock-cells"
   - "#address-cells"
   - "#size-cells"
@@ -103,12 +163,13 @@  examples:
     usb_1_qmpphy: phy-wrapper@88e9000 {
         compatible = "qcom,sdm845-qmp-usb3-phy";
         reg = <0x088e9000 0x18c>,
-              <0x088e8000 0x10>;
-        reg-names = "reg-base", "dp_com";
+              <0x088e8000 0x10>,
+              <0x088ea000 0x40>;
+        reg-names = "usb", "dp_com", "dp";
         #clock-cells = <1>;
         #address-cells = <1>;
         #size-cells = <1>;
-        ranges = <0x0 0x088e9000 0x1000>;
+        ranges = <0x0 0x088e9000 0x2000>;
 
         clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
                  <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
@@ -123,7 +184,7 @@  examples:
         vdda-phy-supply = <&vdda_usb2_ss_1p2>;
         vdda-pll-supply = <&vdda_usb2_ss_core>;
 
-        phy@200 {
+        usb3-phy@200 {
             reg = <0x200 0x128>,
                   <0x400 0x200>,
                   <0xc00 0x218>,
@@ -136,4 +197,14 @@  examples:
             clock-names = "pipe0";
             clock-output-names = "usb3_phy_pipe_clk_src";
         };
+
+        dp-phy@88ea200 {
+            reg = <0xa200 0x200>,
+                  <0xa400 0x200>,
+                  <0xaa00 0x200>,
+                  <0xa600 0x200>,
+                  <0xa800 0x200>;
+            #clock-cells = <1>;
+            #phy-cells = <0>;
+        };
     };