| Message ID | 20220514115946.8858-2-linux@fw-web.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | RK3568 PCIe V3 support | expand |
On 14/05/2022 13:59, 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> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
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'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
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). needed to change type of lane-map to this: $ref: /schemas/types.yaml#/definitions/uint8-array 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 will wait a bit for other comments (for driver) till i send another version. maybe you can confirm my changes are the right way to fix. regards Frank
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
On Mon, May 16, 2022 at 09:21:31PM +0200, Frank Wunderlich wrote: > 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. Fair enough. The type needs to be defined in a common binding though. phy/phy-provider.yaml in dtschema probably. We already have clock-lanes and data-lanes for other serdes interfaces. Maybe data-lanes works here? > Driver reads as u8 array and range is small enough for u8 even if > used for larger controllers (e.g. PCIe x16). Not arguing that it shouldn't be, just confused how the type was related to warnings. > > >> 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. No, the bot does a baseline build and extracts the diff in warnings. Still too many warnings popping up frequently... :( Rob
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
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
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>; + };