From: Frank Wunderlich <[email protected]>
Add a new binding file for Rockchip PCIe v3 phy driver.
Signed-off-by: Frank Wunderlich <[email protected]>
---
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
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 <[email protected]>
+
+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>;
+ };
--
2.25.1
On 14/05/2022 13:59, Frank Wunderlich wrote:
> From: Frank Wunderlich <[email protected]>
>
> Add a new binding file for Rockchip PCIe v3 phy driver.
>
> Signed-off-by: Frank Wunderlich <[email protected]>
>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Sat, 14 May 2022 13:59:42 +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <[email protected]>
>
> Add a new binding file for Rockchip PCIe v3 phy driver.
>
> Signed-off-by: Frank Wunderlich <[email protected]>
>
> ---
> 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.
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" <[email protected]>
>
> > On Sat, 14 May 2022 13:59:42 +0200, Frank Wunderlich wrote:
> > > From: Frank Wunderlich <[email protected]>
> > >
> > > Add a new binding file for Rockchip PCIe v3 phy driver.
> > >
> > > Signed-off-by: Frank Wunderlich <[email protected]>
> > >
> > > ---
> > > 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
Hi
> Gesendet: Sonntag, 15. Mai 2022 um 01:14 Uhr
> Von: "Rob Herring" <[email protected]>
> On Sat, 14 May 2022 13:59:42 +0200, Frank Wunderlich wrote:
> > From: Frank Wunderlich <[email protected]>
> >
> > Add a new binding file for Rockchip PCIe v3 phy driver.
> >
> > Signed-off-by: Frank Wunderlich <[email protected]>
> >
> > ---
> > 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
Am 16. Mai 2022 19:35:37 MESZ schrieb Rob Herring <[email protected]>:
>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" <[email protected]>
>>
>> > 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 <[email protected]>:
> >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" <[email protected]>
> >>
> >> > 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" <[email protected]>
> An: "Rob Herring" <[email protected]>
> 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