| Message ID | 20220514115946.8858-2-linux@fw-web.de |
|---|---|
| State | New |
| Headers | show |
| Series | RK3568 PCIe V3 support | expand |
On Sun, May 15, 2022 at 01:49:47PM +0200, Frank Wunderlich wrote: > Hi > > > Gesendet: Sonntag, 15. Mai 2022 um 01:14 Uhr > > Von: "Rob Herring" <robh@kernel.org> > > > On Sat, 14 May 2022 13:59:42 +0200, Frank Wunderlich wrote: > > > From: Frank Wunderlich <frank-w@public-files.de> > > > > > > Add a new binding file for Rockchip PCIe v3 phy driver. > > > > > > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > > > > > > --- > > > v3: > > > - drop quotes > > > - drop rk3588 > > > - make clockcount fixed to 3 > > > - full path for binding header file > > > - drop phy-mode and its header and add lane-map > > > > > > v2: > > > dt-bindings: rename yaml for PCIe v3 > > > rockchip-pcie3-phy.yaml => rockchip,pcie3-phy.yaml > > > > > > changes in pcie3 phy yaml > > > - change clock names to ordered const list > > > - extend pcie30-phymode description > > > - add phy-cells to required properties > > > - drop unevaluatedProperties > > > - example with 1 clock each line > > > - use default property instead of text describing it > > > - update license > > > --- > > > .../bindings/phy/rockchip,pcie3-phy.yaml | 82 +++++++++++++++++++ > > > 1 file changed, 82 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml > > > > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml: properties:clock-names: 'oneOf' conditional failed, one must be fixed: > > [{'const': 'refclk_m'}, {'const': 'refclk_n'}, {'const': 'pclk'}] is too long > > [{'const': 'refclk_m'}, {'const': 'refclk_n'}, {'const': 'pclk'}] is too short > > False schema does not allow 3 > > 1 was expected > > 3 is greater than the maximum of 2 > > hint: "minItems" is only needed if less than the "items" list length > > from schema $id: http://devicetree.org/meta-schemas/items.yaml# > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml: ignoring, error in schema: properties: clock-names > > Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.example.dtb:0:0: /example-0/phy@fe8c0000: failed to match any schema with compatible: ['rockchip,rk3568-pcie3-phy'] > > seems this is fixed when i remove the "minItems: 3" from clock names > (which is already fixed length because of the list). Yes. > needed to change type of lane-map to this: > > $ref: /schemas/types.yaml#/definitions/uint8-array Why? That's not a standard property though, so needs a 'rockchip' prefix. Though maybe a common property would be appropriate here. > then it looks clean for it.... > > -m causes many errors unrelated to this schema-file even if i pass > DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml The fix is fixing the remaining 40 or so '-m' errors. Rob
Am 16. Mai 2022 19:35:37 MESZ schrieb Rob Herring <robh@kernel.org>: >On Sun, May 15, 2022 at 01:49:47PM +0200, Frank Wunderlich wrote: >> Hi >> >> > Gesendet: Sonntag, 15. Mai 2022 um 01:14 Uhr >> > Von: "Rob Herring" <robh@kernel.org> >> >> > On Sat, 14 May 2022 13:59:42 +0200, Frank Wunderlich wrote: >Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml >> > > >> > >> > My bot found errors running 'make DT_CHECKER_FLAGS=-m >dt_binding_check' >> > on your patch (DT_CHECKER_FLAGS is new in v5.13): >> > >> > yamllint warnings/errors: >> > >> > dtschema/dtc warnings/errors: >> > >/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml: >properties:clock-names: 'oneOf' conditional failed, one must be fixed: >> > [{'const': 'refclk_m'}, {'const': 'refclk_n'}, {'const': 'pclk'}] >is too long >> > [{'const': 'refclk_m'}, {'const': 'refclk_n'}, {'const': 'pclk'}] >is too short >> > False schema does not allow 3 >> > 1 was expected >> > 3 is greater than the maximum of 2 >> > hint: "minItems" is only needed if less than the "items" list >length >> > from schema $id: http://devicetree.org/meta-schemas/items.yaml# >> > >/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml: >ignoring, error in schema: properties: clock-names >> > >Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.example.dtb:0:0: >/example-0/phy@fe8c0000: failed to match any schema with compatible: >['rockchip,rk3568-pcie3-phy'] >> >> seems this is fixed when i remove the "minItems: 3" from clock names >> (which is already fixed length because of the list). > >Yes. > >> needed to change type of lane-map to this: >> >> $ref: /schemas/types.yaml#/definitions/uint8-array > >Why? That's not a standard property though, so needs a 'rockchip' >prefix. Though maybe a common property would be appropriate here. Originally it was a bool property named "rockchip,bifurcation" and we changed it (after comments) to be a more generic property "lane-map" that can be re-used on other vendors/controllers/phys. Driver reads as u8 array and range is small enough for u8 even if used for larger controllers (e.g. PCIe x16). >> then it looks clean for it.... >> >> -m causes many errors unrelated to this schema-file even if i pass >> >DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml > >The fix is fixing the remaining 40 or so '-m' errors. So now clean for you(r bot), too? Did only get a bunch of other unrelated messages. >Rob regards Frank
> Gesendet: Freitag, 20. Mai 2022 um 13:50 Uhr > Von: "Frank Wunderlich" <frank-w@public-files.de> > An: "Rob Herring" <robh@kernel.org> > Hi, > > fixed reg-error by using 32bit-address in example, in my test output is clean. > > +++ b/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml > @@ -68,7 +68,7 @@ examples: > #include <dt-bindings/clock/rk3568-cru.h> > pcie30phy: phy@fe8c0000 { > compatible = "rockchip,rk3568-pcie3-phy"; > - reg = <0x0 0xfe8c0000 0x0 0x20000>; > + reg = <0xfe8c0000 0x20000>; > > > i hope yours is clean too have you tried it? > regarding data-lanes instead of own lane-map, Peter and me only find this in special > bindings outside the phy-"namespace" like this. > > https://elixir.bootlin.com/linux/v5.18-rc7/source/Documentation/devicetree/bindings/media/video-interfaces.yaml#L157 > > do you mean converting this binding and add it there and base out binding on it? > > https://elixir.bootlin.com/linux/v5.18-rc7/source/Documentation/devicetree/bindings/phy/phy-bindings.txt is this the right binding to add the data-lanes or do you refer another one (have not found phy-provider)? regards Frank
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..ac82f551bbfb --- /dev/null +++ b/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml @@ -0,0 +1,82 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%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@sntech.de> + +properties: + compatible: + enum: + - rockchip,rk3568-pcie3-phy + + reg: + maxItems: 1 + + clocks: + minItems: 3 + maxItems: 3 + + clock-names: + items: + - const: refclk_m + - const: refclk_n + - const: pclk + + minItems: 3 + + lane-map: + description: which lanes (by position) should be mapped to which + controller (value). 0 means lane unused, higher value means used. + (controller-number +1 ) + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 2 + maxItems: 16 + items: + minimum: 0 + maximum: 16 + + "#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" + +required: + - compatible + - reg + - rockchip,phy-grf + - "#phy-cells" + +additionalProperties: false + +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>; + clock-names = "refclk_m", "refclk_n", "pclk"; + resets = <&cru SRST_PCIE30PHY>; + reset-names = "phy"; + rockchip,phy-grf = <&pcie30_phy_grf>; + };