Re: [PATCH v3 1/3] dt-bindings: phy: rockchip: Add Naneng combo PHY bindings

Johan Jonker
2021-11-14T11:32:36Z

Hi Yifeng,

Driver and documents don't match.
Add a separate patch before this one to add to syscon.yaml:

"rockchip,rk3568-pipe-phy-grf", "syscon"
"rockchip,rk3568-pipe-grf", "syscon"

On 10/25/21 10:06 AM, Yifeng Zhao wrote:
> Add the compatible strings for the Naneng combo PHY found on rockchip SoC.
>
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Fix dtschema/dtc warnings/errors
>
> .../phy/phy-rockchip-naneng-combphy.yaml | 98 +++++++++++++++++++
> 1 file changed, 98 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 000000000000..55ad33d902ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml
> @@ -0,0 +1,98 @@
> +# 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:

> + minItems: 1

Remove.
DT node has 3 clocks and not 1.

> + items:
> + - description: reference clock
> + - description: apb clock
> + - description: pipe clock
> +
> + clock-names:

> + minItems: 1

dito

> + items:
> + - const: ref
> + - const: apb
> + - const: pipe
> +
> + '#phy-cells':
> + const: 1
> +
> + resets:

> + minItems: 1

Remove.
DT node has 2 resets and not 1.

> + items:
> + - description: exclusive apb reset line
> + - description: exclusive PHY reset line
> +
> + reset-names:

> + minItems: 1

dito

There are 2 resets. When the reset order does matter then use
devm_reset_control_array_get() to get the resets.
The use of reset-names is then not needed.

> + items:
> + - const: combphy-apb
> + - const: combphy
> +

Missing properties.

rockchip,dis-u3otg0-port

rockchip,dis-u3otg1-port

rockchip,enable-ssc

rockchip,ext-refclk

rockchip,sgmii-mac-sel

> + rockchip,pipe-grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:

> + Some additional phy settings are access through GRF regs.

are accessed

> +
> + rockchip,pipe-phy-grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:

> + Some additional pipe settings are access through GRF regs.

are accessed

> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - '#phy-cells'
> + - resets

> + - reset-names

dito

> + - rockchip,pipe-grf
> + - rockchip,pipe-phy-grf
> +
> +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_us: phy@fe820000 {
> + compatible = "rockchip,rk3568-naneng-combphy";
> + reg = <0xfe820000 0x100>;
> + #phy-cells = <1>;
> + 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>;

> + reset-names = "combphy-apb", "combphy";

dito

> + rockchip,pipe-grf = <&pipegrf>;
> + rockchip,pipe-phy-grf = <&pipe_phy_grf0>;
> + };