> Gesendet: Montag, 18. April 2022 um 17:52 Uhr > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@xxxxxxxxxx> > > diff --git a/Documentation/devicetree/bindings/phy/rockchip-pcie3-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip-pcie3-phy.yaml > > new file mode 100644 > > index 000000000000..58a8ce175f13 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/rockchip-pcie3-phy.yaml > > Filename: vendor,hardware > so for example "rockchip,pcie3-phy" although Rob proposed recently for > other bindings using compatible as a base: > https://lore.kernel.org/linux-devicetree/YlhkwvGdcf4ozTzG@xxxxxxxxxxxxxxxxxx/ ok, i rename > > @@ -0,0 +1,77 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/phy/rockchip-pcie3-phy.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Rockchip PCIe v3 phy > > + > > +maintainers: > > + - Heiko Stuebner <heiko@xxxxxxxxx> > > + > > +properties: > > + compatible: > > + enum: > > + - rockchip,rk3568-pcie3-phy > > + - rockchip,rk3588-pcie3-phy > > + > > + reg: > > + maxItems: 2 > > + > > + clocks: > > + minItems: 1 > > + maxItems: 3 > > + > > + clock-names: > > + contains: > > + anyOf: > > + - enum: [ refclk_m, refclk_n, pclk ] > > The list should be strictly ordered (defined), so: > items: > - const: ... > - const: ... > - const: ... > minItems: 1 > > However the question is - why the clocks have different amount? Is it > per different SoC implementation? i only know the rk3568, which needs the clocks defined here, don't know about rk3588 yet. in rk3568 TPM i have the pcie-part seems missing (at least the specific register definition), so i had used the driver as i got it from the downstream kernel. not yet looked if i find a rk3588 TPM and if this part is there as i cannot test it (one of the reasons this is a rfc/rft). > > + > > + "#phy-cells": > > + const: 0 > > + > > + resets: > > + maxItems: 1 > > + > > + reset-names: > > + const: phy > > + > > + rockchip,phy-grf: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: phandle to the syscon managing the phy "general register files" > > + > > + rockchip,pipe-grf: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: phandle to the syscon managing the pipe "general register files" > > + > > + rockchip,pcie30-phymode: > > + $ref: '/schemas/types.yaml#/definitions/uint32' > > + description: | > > + use PHY_MODE_PCIE_AGGREGATION if not defined > > I don't understand the description. Do you mean here a case when the > variable is missing? yes, if the property is not set, then value is PHY_MODE_PCIE_AGGREGATION = 4 > > + minimum: 0x0 > > + maximum: 0x4 > > Please explain these values. Register values should not be part of > bindings, but instead some logical behavior of hardware or its logic. it's a bitmask, so maybe description: | bit0: bifurcation for port 0 bit1: bifurcation for port 1 bit2: aggregation use PHY_MODE_PCIE_AGGREGATION (4) as default > > + > > + > > Just one blank line. > > > +required: > > + - compatible > > + - reg > > + - rockchip,phy-grf > > phy-cells as well > > > + > > +additionalProperties: false > > + > > +unevaluatedProperties: false > > Just one please, additionalProperties. ok > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/rk3568-cru.h> > > + pcie30phy: phy@fe8c0000 { > > + compatible = "rockchip,rk3568-pcie3-phy"; > > + reg = <0x0 0xfe8c0000 0x0 0x20000>; > > + #phy-cells = <0>; > > + clocks = <&pmucru CLK_PCIE30PHY_REF_M>, <&pmucru CLK_PCIE30PHY_REF_N>, > > + <&cru PCLK_PCIE30PHY>; > > Align the entry with opening '<'. Usually the most readable is one clock > per line. ok > > + clock-names = "refclk_m", "refclk_n", "pclk"; > > + resets = <&cru SRST_PCIE30PHY>; > > + reset-names = "phy"; > > + rockchip,phy-grf = <&pcie30_phy_grf>; > > + }; regards Frank
