diff mbox series

[RFC,v4,2/4] dt-bindings: phy: rockchip: Add Naneng combo PHY bindings

Message ID 20211208185449.16763-3-jbx6244@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add Naneng combo PHY support for RK3568 | expand

Commit Message

Johan Jonker Dec. 8, 2021, 6:54 p.m. UTC
From: Yifeng Zhao <yifeng.zhao@rock-chips.com>

Add the compatible strings for the Naneng combo PHY found on rockchip SoC.

Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---

Changed V4:
  restyle
  remove some minItems
  add more properties
  remove reset-names
  move #phy-cells
  add rockchip,rk3568-pipe-grf
  add rockchip,rk3568-pipe-phy-grf
---
 .../phy/phy-rockchip-naneng-combphy.yaml      | 127 ++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml

Comments

Rob Herring (Arm) Dec. 10, 2021, 10:05 p.m. UTC | #1
On Wed, 08 Dec 2021 19:54:47 +0100, Johan Jonker wrote:
> From: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> 
> Add the compatible strings for the Naneng combo PHY found on rockchip SoC.
> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> ---
> 
> Changed V4:
>   restyle
>   remove some minItems
>   add more properties
>   remove reset-names
>   move #phy-cells
>   add rockchip,rk3568-pipe-grf
>   add rockchip,rk3568-pipe-phy-grf
> ---
>  .../phy/phy-rockchip-naneng-combphy.yaml      | 127 ++++++++++++++++++
>  1 file changed, 127 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Vinod Koul Dec. 14, 2021, 8:58 a.m. UTC | #2
On 08-12-21, 19:54, Johan Jonker wrote:
> From: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> 
> Add the compatible strings for the Naneng combo PHY found on rockchip SoC.

Why is this series still tagged RFC..?

> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> ---
> 
> Changed V4:
>   restyle
>   remove some minItems
>   add more properties
>   remove reset-names
>   move #phy-cells
>   add rockchip,rk3568-pipe-grf
>   add rockchip,rk3568-pipe-phy-grf
> ---
>  .../phy/phy-rockchip-naneng-combphy.yaml      | 127 ++++++++++++++++++
>  1 file changed, 127 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> new file mode 100644
> index 000000000..d309e2008
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> @@ -0,0 +1,127 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/phy-rockchip-naneng-combphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip SoC Naneng Combo Phy Device Tree Bindings
> +
> +maintainers:
> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - rockchip,rk3568-naneng-combphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: reference clock
> +      - description: apb clock
> +      - description: pipe clock

no maxItems or minItems for this?

> +
> +  clock-names:
> +    items:
> +      - const: ref
> +      - const: apb
> +      - const: pipe
> +
> +  resets:
> +    items:
> +      - description: exclusive apb reset line
> +      - description: exclusive PHY reset line

Ditto?

> +
> +  rockchip,dis-u3otg0-port:
> +    type: boolean
> +    description:
> +      Disable the u3otg0 port.

why not make it explicit and say rockchip,disable-u3otg0-port

Also why should this port be disabled?

> +
> +  rockchip,dis-u3otg1-port:
> +    type: boolean
> +    description:
> +      Disable the u3otg1 port.

ditto

> +
> +  rockchip,enable-ssc:
> +    type: boolean
> +    description:
> +      In U3 and SATA mode the SSC option is already disabled by default.
> +      In PCIE mode the option SSC can be enabled.
> +      If Spread Spectrum Clocking (SSC) is used it is
> +      required that a common reference clock is used by the link partners.
> +      Most commercially available platforms with PCIe backplanes use
> +      SSC to reduce EMI.
> +
> +  rockchip,ext-refclk:
> +    type: boolean
> +    description:
> +      Many PCIe connections, especially backplane connections,
> +      require a synchronous reference clock between the two link partners.
> +      To achieve this a common clock source, referred to as REFCLK in
> +      the PCI Express Card Electromechanical Specification,
> +      should be used by both ends of the PCIe link.
> +      The PCIe PHY provides 100MHz differential clock output
> +      (optional with SSC) in RC mode for system applications.
> +
> +  rockchip,pipe-grf:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Some additional phy settings are accessed through GRF regs.
> +
> +  rockchip,pipe-phy-grf:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Some additional pipe settings are accessed through GRF regs.
> +
> +  rockchip,sgmii-mac-sel:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    default: 0
> +    description:
> +      Select gmac0 or gmac1 to be used as SGMII controller.
> +
> +  "#phy-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - resets
> +  - rockchip,pipe-grf
> +  - rockchip,pipe-phy-grf
> +  - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rk3568-cru.h>
> +
> +    pipegrf: syscon@fdc50000 {
> +      compatible = "rockchip,rk3568-pipe-grf", "syscon";
> +      reg = <0xfdc50000 0x1000>;
> +    };
> +
> +    pipe_phy_grf0: syscon@fdc70000 {
> +      compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
> +      reg = <0xfdc70000 0x1000>;
> +    };
> +
> +    combphy0: phy@fe820000 {
> +      compatible = "rockchip,rk3568-naneng-combphy";
> +      reg = <0xfe820000 0x100>;
> +      clocks = <&pmucru CLK_PCIEPHY0_REF>,
> +               <&cru PCLK_PIPEPHY0>,
> +               <&cru PCLK_PIPE>;
> +      clock-names = "ref", "apb", "pipe";
> +      assigned-clocks = <&pmucru CLK_PCIEPHY0_REF>;
> +      assigned-clock-rates = <100000000>;
> +      resets = <&cru SRST_P_PIPEPHY0>, <&cru SRST_PIPEPHY0>;
> +      rockchip,pipe-grf = <&pipegrf>;
> +      rockchip,pipe-phy-grf = <&pipe_phy_grf0>;
> +      #phy-cells = <1>;
> +    };
> -- 
> 2.20.1
Johan Jonker Dec. 14, 2021, 12:20 p.m. UTC | #3
Hi,

On 12/14/21 9:58 AM, Vinod Koul wrote:
> On 08-12-21, 19:54, Johan Jonker wrote:
>> From: Yifeng Zhao <yifeng.zhao@rock-chips.com>
>>
>> Add the compatible strings for the Naneng combo PHY found on rockchip SoC.
> 

> Why is this series still tagged RFC..?

The phy DT nodes are urgent in need for other USB3, SATA and PCIe follow
up series. When the author doesn't respond for some time I can kick the
can a bit if it's for 'little' YAML, C style or DT changes. For larger
changes it's better to have the hardware tested as well, so I
carefully/politely marked it RFC as I don't know the author's intentions.

> 
>>
>> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>> ---
>>
>> Changed V4:
>>   restyle
>>   remove some minItems
>>   add more properties
>>   remove reset-names
>>   move #phy-cells
>>   add rockchip,rk3568-pipe-grf
>>   add rockchip,rk3568-pipe-phy-grf
>> ---
>>  .../phy/phy-rockchip-naneng-combphy.yaml      | 127 ++++++++++++++++++
>>  1 file changed, 127 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
>> new file mode 100644
>> index 000000000..d309e2008
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
>> @@ -0,0 +1,127 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/phy-rockchip-naneng-combphy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip SoC Naneng Combo Phy Device Tree Bindings
>> +
>> +maintainers:
>> +  - Heiko Stuebner <heiko@sntech.de>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - rockchip,rk3568-naneng-combphy
>> +
>> +  reg:
>> +    maxItems: 1
>> +

>> +  clocks:
>> +    items:
>> +      - description: reference clock
>> +      - description: apb clock
>> +      - description: pipe clock
> 
> no maxItems or minItems for this?

Documentation/devicetree/bindings/processed-schema.json

      "clocks": {
        "items": [
          {},
          {},
          {}
        ],
        "type": "array",
        "minItems": 3,
        "maxItems": 3,
        "additionalItems": false
      },

With 3 items the properties minItems and maxItems are automatically
added. Only when the number of clocks varies for example between 1 and 3
one should add minItems.

> 
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ref
>> +      - const: apb
>> +      - const: pipe
>> +
>> +  resets:
>> +    items:
>> +      - description: exclusive apb reset line
>> +      - description: exclusive PHY reset line
> 
> Ditto?
> 
>> +

>> +  rockchip,dis-u3otg0-port:
>> +    type: boolean
>> +    description:
>> +      Disable the u3otg0 port.
> 
> why not make it explicit and say rockchip,disable-u3otg0-port
> 
> Also why should this port be disabled?

From Rockchip RK3568 Datasheet V1.0-20201210 page 16-17:

Multi-PHY0 support one of the following interfaces
USB3.0 OTG
SATA0

Multi-PHY1 support one of the following interfaces
USB3.0 Host
SATA1
QSGMII/SGMII

Multi-PHY2 support one of the following interfaces
PCIe2.1
SATA2
QSGMII/SGMII

===

Rockchip RK3568 TRM Part1 V1.0-20210111
page 233-234

PIPE_GRF_USB3OTG0_CON1
Address: Operational Base + offset (0x0104)

usb3otg0_host_u3_port_disable
USB 3.0 SS Port Disable control.
1'b0: Port Enabled
1'b1: Port Disabled

page 235-236

PIPE_GRF_USB3OTG1_CON1
Address: Operational Base + offset (0x0144)

usb3otg1_host_u3_port_disable
USB 3.0 SS Port Disable control.
1'b0: Port Enabled
1'b1: Port Disabled

===

https://www.cnx-software.com/2020/12/01/rockchip-rk3568-processor-to-power-edge-computing-and-nvr-applications/
https://eji4evk5kxx.exactdn.com/wp-content/uploads/2020/12/RK3568-multiplexed-sata-usb-3.0-pcie.jpg?lossy=1&ssl=1

===

USB3.0 OTG, USB3.0 HOT, SATA3.0, PCIE2.1, QSGMII are all multiplexed via
three Serdes lanes.
The driver in it's current state doesn't keep track of which phy[0-2]
node it probes I think. Nodes can be probed in random order, so it's not
able to tell if usb3otg0_host_u3_port_disable or
usb3otg1_host_u3_port_disable should be used. That why the author
probably choose to use a property.

Please advise if we need more complex logic, state locking, etc.
(Any example from the kernel source for that?)

(with more complexity I sould better pass this serie to somebody else)

Johan


> 
>> +
>> +  rockchip,dis-u3otg1-port:
>> +    type: boolean
>> +    description:
>> +      Disable the u3otg1 port.
> 
> ditto
> 
>> +
>> +  rockchip,enable-ssc:
>> +    type: boolean
>> +    description:
>> +      In U3 and SATA mode the SSC option is already disabled by default.
>> +      In PCIE mode the option SSC can be enabled.
>> +      If Spread Spectrum Clocking (SSC) is used it is
>> +      required that a common reference clock is used by the link partners.
>> +      Most commercially available platforms with PCIe backplanes use
>> +      SSC to reduce EMI.
>> +
>> +  rockchip,ext-refclk:
>> +    type: boolean
>> +    description:
>> +      Many PCIe connections, especially backplane connections,
>> +      require a synchronous reference clock between the two link partners.
>> +      To achieve this a common clock source, referred to as REFCLK in
>> +      the PCI Express Card Electromechanical Specification,
>> +      should be used by both ends of the PCIe link.
>> +      The PCIe PHY provides 100MHz differential clock output
>> +      (optional with SSC) in RC mode for system applications.
>> +
>> +  rockchip,pipe-grf:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      Some additional phy settings are accessed through GRF regs.
>> +
>> +  rockchip,pipe-phy-grf:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      Some additional pipe settings are accessed through GRF regs.
>> +
>> +  rockchip,sgmii-mac-sel:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1]
>> +    default: 0
>> +    description:
>> +      Select gmac0 or gmac1 to be used as SGMII controller.
>> +
>> +  "#phy-cells":
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - rockchip,pipe-grf
>> +  - rockchip,pipe-phy-grf
>> +  - "#phy-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/rk3568-cru.h>
>> +
>> +    pipegrf: syscon@fdc50000 {
>> +      compatible = "rockchip,rk3568-pipe-grf", "syscon";
>> +      reg = <0xfdc50000 0x1000>;
>> +    };
>> +
>> +    pipe_phy_grf0: syscon@fdc70000 {
>> +      compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
>> +      reg = <0xfdc70000 0x1000>;
>> +    };
>> +
>> +    combphy0: phy@fe820000 {
>> +      compatible = "rockchip,rk3568-naneng-combphy";
>> +      reg = <0xfe820000 0x100>;
>> +      clocks = <&pmucru CLK_PCIEPHY0_REF>,
>> +               <&cru PCLK_PIPEPHY0>,
>> +               <&cru PCLK_PIPE>;
>> +      clock-names = "ref", "apb", "pipe";
>> +      assigned-clocks = <&pmucru CLK_PCIEPHY0_REF>;
>> +      assigned-clock-rates = <100000000>;
>> +      resets = <&cru SRST_P_PIPEPHY0>, <&cru SRST_PIPEPHY0>;
>> +      rockchip,pipe-grf = <&pipegrf>;
>> +      rockchip,pipe-phy-grf = <&pipe_phy_grf0>;
>> +      #phy-cells = <1>;
>> +    };
>> -- 
>> 2.20.1
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
new file mode 100644
index 000000000..d309e2008
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
@@ -0,0 +1,127 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/phy-rockchip-naneng-combphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip SoC Naneng Combo Phy Device Tree Bindings
+
+maintainers:
+  - Heiko Stuebner <heiko@sntech.de>
+
+properties:
+  compatible:
+    enum:
+      - rockchip,rk3568-naneng-combphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: reference clock
+      - description: apb clock
+      - description: pipe clock
+
+  clock-names:
+    items:
+      - const: ref
+      - const: apb
+      - const: pipe
+
+  resets:
+    items:
+      - description: exclusive apb reset line
+      - description: exclusive PHY reset line
+
+  rockchip,dis-u3otg0-port:
+    type: boolean
+    description:
+      Disable the u3otg0 port.
+
+  rockchip,dis-u3otg1-port:
+    type: boolean
+    description:
+      Disable the u3otg1 port.
+
+  rockchip,enable-ssc:
+    type: boolean
+    description:
+      In U3 and SATA mode the SSC option is already disabled by default.
+      In PCIE mode the option SSC can be enabled.
+      If Spread Spectrum Clocking (SSC) is used it is
+      required that a common reference clock is used by the link partners.
+      Most commercially available platforms with PCIe backplanes use
+      SSC to reduce EMI.
+
+  rockchip,ext-refclk:
+    type: boolean
+    description:
+      Many PCIe connections, especially backplane connections,
+      require a synchronous reference clock between the two link partners.
+      To achieve this a common clock source, referred to as REFCLK in
+      the PCI Express Card Electromechanical Specification,
+      should be used by both ends of the PCIe link.
+      The PCIe PHY provides 100MHz differential clock output
+      (optional with SSC) in RC mode for system applications.
+
+  rockchip,pipe-grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Some additional phy settings are accessed through GRF regs.
+
+  rockchip,pipe-phy-grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Some additional pipe settings are accessed through GRF regs.
+
+  rockchip,sgmii-mac-sel:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    default: 0
+    description:
+      Select gmac0 or gmac1 to be used as SGMII controller.
+
+  "#phy-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - rockchip,pipe-grf
+  - rockchip,pipe-phy-grf
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3568-cru.h>
+
+    pipegrf: syscon@fdc50000 {
+      compatible = "rockchip,rk3568-pipe-grf", "syscon";
+      reg = <0xfdc50000 0x1000>;
+    };
+
+    pipe_phy_grf0: syscon@fdc70000 {
+      compatible = "rockchip,rk3568-pipe-phy-grf", "syscon";
+      reg = <0xfdc70000 0x1000>;
+    };
+
+    combphy0: phy@fe820000 {
+      compatible = "rockchip,rk3568-naneng-combphy";
+      reg = <0xfe820000 0x100>;
+      clocks = <&pmucru CLK_PCIEPHY0_REF>,
+               <&cru PCLK_PIPEPHY0>,
+               <&cru PCLK_PIPE>;
+      clock-names = "ref", "apb", "pipe";
+      assigned-clocks = <&pmucru CLK_PCIEPHY0_REF>;
+      assigned-clock-rates = <100000000>;
+      resets = <&cru SRST_P_PIPEPHY0>, <&cru SRST_PIPEPHY0>;
+      rockchip,pipe-grf = <&pipegrf>;
+      rockchip,pipe-phy-grf = <&pipe_phy_grf0>;
+      #phy-cells = <1>;
+    };