| Message ID | 20230814112539.70453-2-sriranjani.p@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/4] dt-bindings: net: Add FSD EQoS device tree bindings | expand |
On Mon, 14 Aug 2023 16:55:36 +0530, Sriranjani P wrote: > Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD > Ethernet YAML schema to enable the DT validation. > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > Signed-off-by: Ravi Patel <ravi.patel@samsung.com> > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > --- > .../devicetree/bindings/net/snps,dwmac.yaml | 5 +- > .../devicetree/bindings/net/tesla,ethqos.yaml | 114 ++++++++++++++++++ > 2 files changed, 117 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/tesla,ethqos.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/dt-review-ci/linux/Documentation/devicetree/bindings/net/tesla,ethqos.yaml: properties:clock-names: {'minItems': 5, 'maxItems': 10, 'items': [{'const': 'ptp_ref'}, {'const': 'master_bus'}, {'const': 'slave_bus'}, {'const': 'tx'}, {'const': 'rx'}, {'const': 'master2_bus'}, {'const': 'slave2_bus'}, {'const': 'eqos_rxclk_mux'}, {'const': 'eqos_phyrxclk'}, {'const': 'dout_peric_rgmii_clk'}]} should not be valid under {'required': ['maxItems']} hint: "maxItems" is not needed with an "items" list from schema $id: http://devicetree.org/meta-schemas/items.yaml# Documentation/devicetree/bindings/net/tesla,ethqos.example.dtb: /example-0/ethernet@14300000: failed to match any schema with compatible: ['tesla,dwc-qos-ethernet-4.21'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230814112539.70453-2-sriranjani.p@samsung.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. 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 after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 14/08/2023 13:25, Sriranjani P wrote: > Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD > Ethernet YAML schema to enable the DT validation. > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > Signed-off-by: Ravi Patel <ravi.patel@samsung.com> > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > --- > .../devicetree/bindings/net/snps,dwmac.yaml | 5 +- > .../devicetree/bindings/net/tesla,ethqos.yaml | 114 ++++++++++++++++++ > 2 files changed, 117 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/tesla,ethqos.yaml > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > index ddf9522a5dc2..0ced7901e644 100644 > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > @@ -96,6 +96,7 @@ properties: > - snps,dwxgmac > - snps,dwxgmac-2.10 > - starfive,jh7110-dwmac > + - tesla,fsd-ethqos-4.21 I don't think one given SoC - and I was told FSD is strictly defined one specific SoC - can have different versions of the same block, so drop the block versioning. > > reg: > minItems: 1 > @@ -117,7 +118,7 @@ properties: > > clocks: > minItems: 1 > - maxItems: 8 > + maxItems: 10 > additionalItems: true > items: > - description: GMAC main clock > @@ -129,7 +130,7 @@ properties: > > clock-names: > minItems: 1 > - maxItems: 8 > + maxItems: 10 > additionalItems: true > contains: > enum: > diff --git a/Documentation/devicetree/bindings/net/tesla,ethqos.yaml b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml > new file mode 100644 > index 000000000000..b78829246364 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml > @@ -0,0 +1,114 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/tesla,ethqos.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: FSD Ethernet Quality of Service > + > +maintainers: > + - Sriranjani P <sriranjani.p@samsung.com> > + - Swathi K S <swathi.ks@samsung.com> > + > +description: > + dwmmac based tesla ethernet devices which support Gigabit > + ethernet. > + > +allOf: > + - $ref: snps,dwmac.yaml# > + > +properties: > + compatible: > + const: tesla,fsd-ethqos-4.21.yaml ? > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + minItems: 5 Why? I expect it to be specific. > + maxItems: 10 > + > + clock-names: > + minItems: 5 > + maxItems: 10 > + items: > + - const: ptp_ref > + - const: master_bus > + - const: slave_bus > + - const: tx > + - const: rx > + - const: master2_bus > + - const: slave2_bus > + - const: eqos_rxclk_mux > + - const: eqos_phyrxclk > + - const: dout_peric_rgmii_clk > + > + fsd-rx-clock-skew: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + - items: > + - description: phandle to the syscon node > + - description: offset of the control register > + description: > + Should be phandle/offset pair. The phandle to the syscon node. > + > + iommus: > + maxItems: 1 > + > + phy-mode: > + $ref: ethernet-controller.yaml#/properties/phy-connection-type > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - rx-clock-skew Eee? Isn't it fsd-rx-clock-skew which anyway is not correct? > + - iommus > + - phy-mode > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/fsd-clk.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + ethernet_1: ethernet@14300000 { > + compatible = "tesla,dwc-qos-ethernet-4.21"; Three different compatibles for the same. No, please test your patches before sending. I am not even checking if previous feedback was applied... Did you really go through it? Best regards, Krzysztof
> + fsd-rx-clock-skew: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + - items: > + - description: phandle to the syscon node > + - description: offset of the control register > + description: > + Should be phandle/offset pair. The phandle to the syscon node. What clock are you skew-ing here? And why? > + ethernet_1: ethernet@14300000 { > + compatible = "tesla,dwc-qos-ethernet-4.21"; > + reg = <0x0 0x14300000 0x0 0x10000>; > + interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_CLK_PTP_REF_I>, > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_ACLK_I>, > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_HCLK_I>, > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_RGMII_CLK_I>, > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_CLK_RX_I>, > + <&clock_peric PERIC_BUS_D_PERIC_IPCLKPORT_EQOSCLK>, > + <&clock_peric PERIC_BUS_P_PERIC_IPCLKPORT_EQOSCLK>, > + <&clock_peric PERIC_EQOS_PHYRXCLK_MUX>, > + <&clock_peric PERIC_EQOS_PHYRXCLK>, > + <&clock_peric PERIC_DOUT_RGMII_CLK>; > + clock-names = "ptp_ref", > + "master_bus", > + "slave_bus", > + "tx", > + "rx", > + "master2_bus", > + "slave2_bus", > + "eqos_rxclk_mux", > + "eqos_phyrxclk", > + "dout_peric_rgmii_clk"; > + pinctrl-names = "default"; > + pinctrl-0 = <ð1_tx_clk>, <ð1_tx_data>, <ð1_tx_ctrl>, > + <ð1_phy_intr>, <ð1_rx_clk>, <ð1_rx_data>, > + <ð1_rx_ctrl>, <ð1_mdio>; > + fsd-rx-clock-skew = <&sysreg_peric 0x10>; > + iommus = <&smmu_peric 0x0 0x1>; > + phy-mode = "rgmii"; I know it is just an example, but "rgmii" is generally wrong. "rgmii-id" is generally what you need. And when i do see "rgmii", it starts ringing alarm bells for me, it could mean your RGMII delays are being handled wrongly. Andrew
> -----Original Message----- > From: Rob Herring [mailto:robh@kernel.org] > Sent: 14 August 2023 19:03 > To: Sriranjani P <sriranjani.p@samsung.com> > Cc: edumazet@google.com; linux-kernel@vger.kernel.org; > alexandre.torgue@foss.st.com; ravi.patel@samsung.com; > alim.akhtar@samsung.com; linux-samsung-soc@vger.kernel.org; linux- > fsd@tesla.com; conor+dt@kernel.org; mcoquelin.stm32@gmail.com; > kuba@kernel.org; netdev@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; pabeni@redhat.com; robh+dt@kernel.org; > pankaj.dubey@samsung.com; richardcochran@gmail.com; > krzysztof.kozlowski+dt@linaro.org; joabreu@synopsys.com; > devicetree@vger.kernel.org; davem@davemloft.net; > swathi.ks@samsung.com > Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree > bindings > > > On Mon, 14 Aug 2023 16:55:36 +0530, Sriranjani P wrote: > > Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD > > Ethernet YAML schema to enable the DT validation. > > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > > Signed-off-by: Ravi Patel <ravi.patel@samsung.com> > > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > > --- > > .../devicetree/bindings/net/snps,dwmac.yaml | 5 +- > > .../devicetree/bindings/net/tesla,ethqos.yaml | 114 > > ++++++++++++++++++ > > 2 files changed, 117 insertions(+), 2 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/net/tesla,ethqos.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/dt-review- > ci/linux/Documentation/devicetree/bindings/net/tesla,ethqos.yaml: > properties:clock-names: {'minItems': 5, 'maxItems': 10, 'items': [{'const': > 'ptp_ref'}, {'const': 'master_bus'}, {'const': 'slave_bus'}, {'const': 'tx'}, {'const': > 'rx'}, {'const': 'master2_bus'}, {'const': 'slave2_bus'}, {'const': > 'eqos_rxclk_mux'}, {'const': 'eqos_phyrxclk'}, {'const': > 'dout_peric_rgmii_clk'}]} should not be valid under {'required': ['maxItems']} > hint: "maxItems" is not needed with an "items" list > from schema $id: https://protect2.fireeye.com/v1/url?k=f50e335d- > aa950a44-f50fb812-000babff3793-de26ea17ef025418&q=1&e=897786e4- > 5f9b-40d8-8a7f-399cb69c7ee8&u=http%3A%2F%2Fdevicetree.org%2Fmeta- > schemas%2Fitems.yaml%23 > Documentation/devicetree/bindings/net/tesla,ethqos.example.dtb: > /example-0/ethernet@14300000: failed to match any schema with > compatible: ['tesla,dwc-qos-ethernet-4.21'] > Thanks for review. Will fix this in v4. > doc reference errors (make refcheckdocs): > > See https://protect2.fireeye.com/v1/url?k=ccb7f6d0-932ccfc9-ccb67d9f- > 000babff3793-2137ac63fe6ddef8&q=1&e=897786e4-5f9b-40d8-8a7f- > 399cb69c7ee8&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fdev > icetree-bindings%2Fpatch%2F20230814112539.70453-2- > sriranjani.p%40samsung.com > > The base for the series is generally the latest rc1. A different dependency > should be noted in *this* patch. > Sorry, I could not get this comment, can you elaborate this. > 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 > Sure will cross check. > Please check and re-submit after running the above command yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your > schema.
On 16/08/2023 07:36, Sriranjani P wrote: > > >> -----Original Message----- >> From: Rob Herring [mailto:robh@kernel.org] >> Sent: 14 August 2023 19:03 >> To: Sriranjani P <sriranjani.p@samsung.com> >> Cc: edumazet@google.com; linux-kernel@vger.kernel.org; >> alexandre.torgue@foss.st.com; ravi.patel@samsung.com; >> alim.akhtar@samsung.com; linux-samsung-soc@vger.kernel.org; linux- >> fsd@tesla.com; conor+dt@kernel.org; mcoquelin.stm32@gmail.com; >> kuba@kernel.org; netdev@vger.kernel.org; linux-arm- >> kernel@lists.infradead.org; pabeni@redhat.com; robh+dt@kernel.org; >> pankaj.dubey@samsung.com; richardcochran@gmail.com; >> krzysztof.kozlowski+dt@linaro.org; joabreu@synopsys.com; >> devicetree@vger.kernel.org; davem@davemloft.net; >> swathi.ks@samsung.com >> Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree >> bindings >> >> >> On Mon, 14 Aug 2023 16:55:36 +0530, Sriranjani P wrote: >>> Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD >>> Ethernet YAML schema to enable the DT validation. >>> >>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> >>> Signed-off-by: Ravi Patel <ravi.patel@samsung.com> >>> Signed-off-by: Swathi K S <swathi.ks@samsung.com> >>> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> >>> --- >>> .../devicetree/bindings/net/snps,dwmac.yaml | 5 +- >>> .../devicetree/bindings/net/tesla,ethqos.yaml | 114 >>> ++++++++++++++++++ >>> 2 files changed, 117 insertions(+), 2 deletions(-) create mode >>> 100644 Documentation/devicetree/bindings/net/tesla,ethqos.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/dt-review- >> ci/linux/Documentation/devicetree/bindings/net/tesla,ethqos.yaml: >> properties:clock-names: {'minItems': 5, 'maxItems': 10, 'items': [{'const': >> 'ptp_ref'}, {'const': 'master_bus'}, {'const': 'slave_bus'}, {'const': 'tx'}, {'const': >> 'rx'}, {'const': 'master2_bus'}, {'const': 'slave2_bus'}, {'const': >> 'eqos_rxclk_mux'}, {'const': 'eqos_phyrxclk'}, {'const': >> 'dout_peric_rgmii_clk'}]} should not be valid under {'required': ['maxItems']} >> hint: "maxItems" is not needed with an "items" list >> from schema $id: https://protect2.fireeye.com/v1/url?k=f50e335d- >> aa950a44-f50fb812-000babff3793-de26ea17ef025418&q=1&e=897786e4- >> 5f9b-40d8-8a7f-399cb69c7ee8&u=http%3A%2F%2Fdevicetree.org%2Fmeta- >> schemas%2Fitems.yaml%23 >> Documentation/devicetree/bindings/net/tesla,ethqos.example.dtb: >> /example-0/ethernet@14300000: failed to match any schema with >> compatible: ['tesla,dwc-qos-ethernet-4.21'] >> > > Thanks for review. Will fix this in v4. Test the patches before sending them. > >> doc reference errors (make refcheckdocs): >> >> See https://protect2.fireeye.com/v1/url?k=ccb7f6d0-932ccfc9-ccb67d9f- >> 000babff3793-2137ac63fe6ddef8&q=1&e=897786e4-5f9b-40d8-8a7f- >> 399cb69c7ee8&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fdev >> icetree-bindings%2Fpatch%2F20230814112539.70453-2- >> sriranjani.p%40samsung.com >> >> The base for the series is generally the latest rc1. A different dependency >> should be noted in *this* patch. >> > > Sorry, I could not get this comment, can you elaborate this. What else to say? You did no stated any dependency here. The base is explained. > >> 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 >> > Sure will cross check. Why do you ask/comment to bot? Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 15 August 2023 01:10 > To: Sriranjani P <sriranjani.p@samsung.com>; davem@davemloft.net; > edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > conor+dt@kernel.org; richardcochran@gmail.com; > alexandre.torgue@foss.st.com; joabreu@synopsys.com; > mcoquelin.stm32@gmail.com; alim.akhtar@samsung.com; linux- > fsd@tesla.com; pankaj.dubey@samsung.com; swathi.ks@samsung.com; > ravi.patel@samsung.com > Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree > bindings > > On 14/08/2023 13:25, Sriranjani P wrote: > > Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD > > Ethernet YAML schema to enable the DT validation. > > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > > Signed-off-by: Ravi Patel <ravi.patel@samsung.com> > > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > > --- > > .../devicetree/bindings/net/snps,dwmac.yaml | 5 +- > > .../devicetree/bindings/net/tesla,ethqos.yaml | 114 > > ++++++++++++++++++ > > 2 files changed, 117 insertions(+), 2 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/net/tesla,ethqos.yaml > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > index ddf9522a5dc2..0ced7901e644 100644 > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > @@ -96,6 +96,7 @@ properties: > > - snps,dwxgmac > > - snps,dwxgmac-2.10 > > - starfive,jh7110-dwmac > > + - tesla,fsd-ethqos-4.21 > > I don't think one given SoC - and I was told FSD is strictly defined one specific > SoC - can have different versions of the same block, so drop the block > versioning. > Ok, will remove versioning. > > > > reg: > > minItems: 1 > > @@ -117,7 +118,7 @@ properties: > > > > clocks: > > minItems: 1 > > - maxItems: 8 > > + maxItems: 10 > > additionalItems: true > > items: > > - description: GMAC main clock > > @@ -129,7 +130,7 @@ properties: > > > > clock-names: > > minItems: 1 > > - maxItems: 8 > > + maxItems: 10 > > additionalItems: true > > contains: > > enum: > > diff --git a/Documentation/devicetree/bindings/net/tesla,ethqos.yaml > > b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml > > new file mode 100644 > > index 000000000000..b78829246364 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml > > @@ -0,0 +1,114 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2 > > +--- > > +$id: > > +https://protect2.fireeye.com/v1/url?k=0d5f9085-6cd485b3-0d5e1bca- > 74fe > > +485cbff1-9835d59b137d73e5&q=1&e=93f28da2-6d86-4cc2-a07a- > 9be1380616cc& > > > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fnet%2Ftesla%2Cethqos. > yaml%2 > > +3 > > +$schema: > > +https://protect2.fireeye.com/v1/url?k=67d3522a-0658471c-67d2d965- > 74fe > > +485cbff1-b9570dbbedf33f81&q=1&e=93f28da2-6d86-4cc2-a07a- > 9be1380616cc& > > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23 > > + > > +title: FSD Ethernet Quality of Service > > + > > +maintainers: > > + - Sriranjani P <sriranjani.p@samsung.com> > > + - Swathi K S <swathi.ks@samsung.com> > > + > > +description: > > + dwmmac based tesla ethernet devices which support Gigabit > > + ethernet. > > + > > +allOf: > > + - $ref: snps,dwmac.yaml# > > + > > +properties: > > + compatible: > > + const: tesla,fsd-ethqos-4.21.yaml > > ? Will fix this to tesla,fsd-ethqos.yaml > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + minItems: 5 > > Why? I expect it to be specific. Sorry, I could not understood this comment. In FSD we have two instances of EQoS IP, one in PERIC block, which requires total 10 clocks to be configured and another instance exist in FSYS0 block which needs 5 clocks to be configured, so we kept minItems as 5 and maxItems as 10, but looks like latest items schema do not need maxItems entry so we will drop maxItems entry. In my understanding minItems still required so it should be kept with minimum number of clock requirements. > > > + maxItems: 10 > > + > > + clock-names: > > + minItems: 5 > > + maxItems: 10 > > + items: > > + - const: ptp_ref > > + - const: master_bus > > + - const: slave_bus > > + - const: tx > > + - const: rx > > + - const: master2_bus > > + - const: slave2_bus > > + - const: eqos_rxclk_mux > > + - const: eqos_phyrxclk > > + - const: dout_peric_rgmii_clk > > + > > + fsd-rx-clock-skew: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + items: > > + - items: > > + - description: phandle to the syscon node > > + - description: offset of the control register > > + description: > > + Should be phandle/offset pair. The phandle to the syscon node. > > + > > + iommus: > > + maxItems: 1 > > + > > + phy-mode: > > + $ref: ethernet-controller.yaml#/properties/phy-connection-type > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - clock-names > > + - rx-clock-skew > > Eee? Isn't it fsd-rx-clock-skew which anyway is not correct? Sorry, I missed to change this in DT schema before posting, I will make this to fsd-rx-clock-skew. > > > + - iommus > > + - phy-mode > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/fsd-clk.h> > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + ethernet_1: ethernet@14300000 { > > + compatible = "tesla,dwc-qos-ethernet-4.21"; > > Three different compatibles for the same. > > No, please test your patches before sending. > > I am not even checking if previous feedback was applied... Did you really go > through it? > Sorry, I missed this will take care. > Best regards, > Krzysztof
On 16/08/2023 07:58, Sriranjani P wrote: >>> + >>> +allOf: >>> + - $ref: snps,dwmac.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: tesla,fsd-ethqos-4.21.yaml >> >> ? > > Will fix this to tesla,fsd-ethqos.yaml Test your patches before sending. REALLY TEST. > >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + clocks: >>> + minItems: 5 >> >> Why? I expect it to be specific. > > Sorry, I could not understood this comment. In FSD we have two instances of EQoS IP, one in PERIC block, which requires total 10 clocks to be configured and another instance exist in FSYS0 block which needs 5 clocks to be configured, so we kept minItems as 5 and maxItems as 10, but looks like latest items schema do not need maxItems entry so we will drop maxItems entry. In my understanding minItems still required so it should be kept with minimum number of clock requirements. No, the code is fine then. > >> >>> + maxItems: 10 >>> + >>> + clock-names: >>> + minItems: 5 >>> + maxItems: 10 >>> + items: >>> + - const: ptp_ref >>> + - const: master_bus >>> + - const: slave_bus >>> + - const: tx >>> + - const: rx >>> + - const: master2_bus >>> + - const: slave2_bus >>> + - const: eqos_rxclk_mux >>> + - const: eqos_phyrxclk >>> + - const: dout_peric_rgmii_clk >>> + >>> + fsd-rx-clock-skew: >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>> + items: >>> + - items: >>> + - description: phandle to the syscon node >>> + - description: offset of the control register >>> + description: >>> + Should be phandle/offset pair. The phandle to the syscon node. >>> + >>> + iommus: >>> + maxItems: 1 >>> + >>> + phy-mode: >>> + $ref: ethernet-controller.yaml#/properties/phy-connection-type >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + - clocks >>> + - clock-names >>> + - rx-clock-skew >> >> Eee? Isn't it fsd-rx-clock-skew which anyway is not correct? > > Sorry, I missed to change this in DT schema before posting, I will make this to fsd-rx-clock-skew. Remember about vendor prefixes for every custom property. Best regards, Krzysztof
On Wed, Aug 16, 2023 at 11:06:51AM +0530, Sriranjani P wrote: > > > > -----Original Message----- > > From: Rob Herring [mailto:robh@kernel.org] > > Sent: 14 August 2023 19:03 > > To: Sriranjani P <sriranjani.p@samsung.com> > > Cc: edumazet@google.com; linux-kernel@vger.kernel.org; > > alexandre.torgue@foss.st.com; ravi.patel@samsung.com; > > alim.akhtar@samsung.com; linux-samsung-soc@vger.kernel.org; linux- > > fsd@tesla.com; conor+dt@kernel.org; mcoquelin.stm32@gmail.com; > > kuba@kernel.org; netdev@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org; pabeni@redhat.com; robh+dt@kernel.org; > > pankaj.dubey@samsung.com; richardcochran@gmail.com; > > krzysztof.kozlowski+dt@linaro.org; joabreu@synopsys.com; > > devicetree@vger.kernel.org; davem@davemloft.net; > > swathi.ks@samsung.com > > Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree > > bindings > > > > > > On Mon, 14 Aug 2023 16:55:36 +0530, Sriranjani P wrote: > > > Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD > > > Ethernet YAML schema to enable the DT validation. > > > > > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > > > Signed-off-by: Ravi Patel <ravi.patel@samsung.com> > > > Signed-off-by: Swathi K S <swathi.ks@samsung.com> > > > Signed-off-by: Sriranjani P <sriranjani.p@samsung.com> > > > --- > > > .../devicetree/bindings/net/snps,dwmac.yaml | 5 +- > > > .../devicetree/bindings/net/tesla,ethqos.yaml | 114 > > > ++++++++++++++++++ > > > 2 files changed, 117 insertions(+), 2 deletions(-) create mode > > > 100644 Documentation/devicetree/bindings/net/tesla,ethqos.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/dt-review- > > ci/linux/Documentation/devicetree/bindings/net/tesla,ethqos.yaml: > > properties:clock-names: {'minItems': 5, 'maxItems': 10, 'items': [{'const': > > 'ptp_ref'}, {'const': 'master_bus'}, {'const': 'slave_bus'}, {'const': 'tx'}, {'const': > > 'rx'}, {'const': 'master2_bus'}, {'const': 'slave2_bus'}, {'const': > > 'eqos_rxclk_mux'}, {'const': 'eqos_phyrxclk'}, {'const': > > 'dout_peric_rgmii_clk'}]} should not be valid under {'required': ['maxItems']} > > hint: "maxItems" is not needed with an "items" list > > from schema $id: https://protect2.fireeye.com/v1/url?k=f50e335d- > > aa950a44-f50fb812-000babff3793-de26ea17ef025418&q=1&e=897786e4- > > 5f9b-40d8-8a7f-399cb69c7ee8&u=http%3A%2F%2Fdevicetree.org%2Fmeta- > > schemas%2Fitems.yaml%23 > > Documentation/devicetree/bindings/net/tesla,ethqos.example.dtb: > > /example-0/ethernet@14300000: failed to match any schema with > > compatible: ['tesla,dwc-qos-ethernet-4.21'] > > > > Thanks for review. Will fix this in v4. It's not a review. It's an automated reply running what you should have run yourself... > > > doc reference errors (make refcheckdocs): > > > > See https://protect2.fireeye.com/v1/url?k=ccb7f6d0-932ccfc9-ccb67d9f- > > 000babff3793-2137ac63fe6ddef8&q=1&e=897786e4-5f9b-40d8-8a7f- > > 399cb69c7ee8&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fdev > > icetree-bindings%2Fpatch%2F20230814112539.70453-2- > > sriranjani.p%40samsung.com > > > > The base for the series is generally the latest rc1. A different dependency > > should be noted in *this* patch. > > > > Sorry, I could not get this comment, can you elaborate this. The automated tests apply patches to the latest rc1 tag. Patches which apply, but have some other dependency may have warnings. If you have such a dependency, you should note it in the patch (below the '---'). Rob
Hi Andrew, Sorry for the delay in response. Starting now, I will be taking over this task. I have gone through your comments and feedback and will be implementing them in v4 of this patch. > -----Original Message----- > From: Andrew Lunn [mailto:andrew@lunn.ch] > Sent: 15 August 2023 02:10 > To: Sriranjani P <sriranjani.p@samsung.com> > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; > richardcochran@gmail.com; alexandre.torgue@foss.st.com; > joabreu@synopsys.com; mcoquelin.stm32@gmail.com; > alim.akhtar@samsung.com; linux-fsd@tesla.com; > pankaj.dubey@samsung.com; swathi.ks@samsung.com; > ravi.patel@samsung.com; netdev@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- > soc@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree > bindings > > > + fsd-rx-clock-skew: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + items: > > + - items: > > + - description: phandle to the syscon node > > + - description: offset of the control register > > + description: > > + Should be phandle/offset pair. The phandle to the syscon node. > > What clock are you skew-ing here? And why? As per customer's requirement, we need 2ns delay in fsys block both in TX and RX path. > > > + ethernet_1: ethernet@14300000 { > > + compatible = "tesla,dwc-qos-ethernet-4.21"; > > + reg = <0x0 0x14300000 0x0 0x10000>; > > + interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clock_peric > PERIC_EQOS_TOP_IPCLKPORT_CLK_PTP_REF_I>, > > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_ACLK_I>, > > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_HCLK_I>, > > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_RGMII_CLK_I>, > > + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_CLK_RX_I>, > > + <&clock_peric PERIC_BUS_D_PERIC_IPCLKPORT_EQOSCLK>, > > + <&clock_peric PERIC_BUS_P_PERIC_IPCLKPORT_EQOSCLK>, > > + <&clock_peric PERIC_EQOS_PHYRXCLK_MUX>, > > + <&clock_peric PERIC_EQOS_PHYRXCLK>, > > + <&clock_peric PERIC_DOUT_RGMII_CLK>; > > + clock-names = "ptp_ref", > > + "master_bus", > > + "slave_bus", > > + "tx", > > + "rx", > > + "master2_bus", > > + "slave2_bus", > > + "eqos_rxclk_mux", > > + "eqos_phyrxclk", > > + "dout_peric_rgmii_clk"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <ð1_tx_clk>, <ð1_tx_data>, <ð1_tx_ctrl>, > > + <ð1_phy_intr>, <ð1_rx_clk>, <ð1_rx_data>, > > + <ð1_rx_ctrl>, <ð1_mdio>; > > + fsd-rx-clock-skew = <&sysreg_peric 0x10>; > > + iommus = <&smmu_peric 0x0 0x1>; > > + phy-mode = "rgmii"; > > I know it is just an example, but "rgmii" is generally wrong. "rgmii-id" is > generally what you need. And when i do see "rgmii", it starts ringing alarm > bells for me, it could mean your RGMII delays are being handled wrongly. Thanks for bringing this to our notice. Will correct this in v4 as rgmii-id. > > Andrew Regards, Swathi
Hi Krzysztof, Sorry for the delay in response. Starting now, I will be taking over this task. I have gone through your comments and feedback and will be implementing them in v4 of this patch. > -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 16 August 2023 11:48 > To: Sriranjani P <sriranjani.p@samsung.com>; davem@davemloft.net; > edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > conor+dt@kernel.org; richardcochran@gmail.com; > alexandre.torgue@foss.st.com; joabreu@synopsys.com; > mcoquelin.stm32@gmail.com; alim.akhtar@samsung.com; linux- > fsd@tesla.com; pankaj.dubey@samsung.com; swathi.ks@samsung.com; > ravi.patel@samsung.com > Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree > bindings > > On 16/08/2023 07:58, Sriranjani P wrote: > >>> + > >>> +allOf: > >>> + - $ref: snps,dwmac.yaml# > >>> + > >>> +properties: > >>> + compatible: > >>> + const: tesla,fsd-ethqos-4.21.yaml > >> > >> ? > > > > Will fix this to tesla,fsd-ethqos.yaml > > Test your patches before sending. REALLY TEST. Sure. Will fix this to tesla,fsd-ethqos.yaml and test the same. > > > > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + interrupts: > >>> + maxItems: 1 > >>> + > >>> + clocks: > >>> + minItems: 5 > >> > >> Why? I expect it to be specific. > > > > Sorry, I could not understood this comment. In FSD we have two instances > of EQoS IP, one in PERIC block, which requires total 10 clocks to be > configured and another instance exist in FSYS0 block which needs 5 clocks to > be configured, so we kept minItems as 5 and maxItems as 10, but looks like > latest items schema do not need maxItems entry so we will drop maxItems > entry. In my understanding minItems still required so it should be kept with > minimum number of clock requirements. > > No, the code is fine then. > > > > >> > >>> + maxItems: 10 > >>> + > >>> + clock-names: > >>> + minItems: 5 > >>> + maxItems: 10 > >>> + items: > >>> + - const: ptp_ref > >>> + - const: master_bus > >>> + - const: slave_bus > >>> + - const: tx > >>> + - const: rx > >>> + - const: master2_bus > >>> + - const: slave2_bus > >>> + - const: eqos_rxclk_mux > >>> + - const: eqos_phyrxclk > >>> + - const: dout_peric_rgmii_clk > >>> + > >>> + fsd-rx-clock-skew: > >>> + $ref: /schemas/types.yaml#/definitions/phandle-array > >>> + items: > >>> + - items: > >>> + - description: phandle to the syscon node > >>> + - description: offset of the control register > >>> + description: > >>> + Should be phandle/offset pair. The phandle to the syscon node. > >>> + > >>> + iommus: > >>> + maxItems: 1 > >>> + > >>> + phy-mode: > >>> + $ref: ethernet-controller.yaml#/properties/phy-connection-type > >>> + > >>> +required: > >>> + - compatible > >>> + - reg > >>> + - interrupts > >>> + - clocks > >>> + - clock-names > >>> + - rx-clock-skew > >> > >> Eee? Isn't it fsd-rx-clock-skew which anyway is not correct? > > > > Sorry, I missed to change this in DT schema before posting, I will make this > to fsd-rx-clock-skew. > > Remember about vendor prefixes for every custom property. Sure, will fix this in v4. > > > Best regards, > Krzysztof Regards, Swathi
> > > + fsd-rx-clock-skew: > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > > + items: > > > + - items: > > > + - description: phandle to the syscon node > > > + - description: offset of the control register > > > + description: > > > + Should be phandle/offset pair. The phandle to the syscon node. > > > > What clock are you skew-ing here? And why? > > As per customer's requirement, we need 2ns delay in fsys block both in TX > and RX path. Lots of people get RGMII delays wrong. Please look back at the mailing list where there is plenty of discussion about this. I don't want to have to repeat myself yet again... Andrew
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: 06 June 2024 18:56 > To: Swathi K S <swathi.ks@samsung.com> > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; > conor+dt@kernel.org; richardcochran@gmail.com; > alexandre.torgue@foss.st.com; joabreu@synopsys.com; > mcoquelin.stm32@gmail.com; alim.akhtar@samsung.com; linux- > fsd@tesla.com; pankaj.dubey@samsung.com; ravi.patel@samsung.com; > netdev@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree bindings > > > > > + fsd-rx-clock-skew: > > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > > > + items: > > > > + - items: > > > > + - description: phandle to the syscon node > > > > + - description: offset of the control register > > > > + description: > > > > + Should be phandle/offset pair. The phandle to the syscon node. > > > > > > What clock are you skew-ing here? And why? > > > > As per customer's requirement, we need 2ns delay in fsys block both in > > TX and RX path. > > Lots of people get RGMII delays wrong. Please look back at the mailing list > where there is plenty of discussion about this. I don't want to have to repeat > myself yet again... Sorry for the delay. We took time to confirm with the board designer regarding this delay. This is not a mandatory one to have and hence we are dropping clock-skewing here. > > Andrew Thanks, Swathi
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index ddf9522a5dc2..0ced7901e644 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml @@ -96,6 +96,7 @@ properties: - snps,dwxgmac - snps,dwxgmac-2.10 - starfive,jh7110-dwmac + - tesla,fsd-ethqos-4.21 reg: minItems: 1 @@ -117,7 +118,7 @@ properties: clocks: minItems: 1 - maxItems: 8 + maxItems: 10 additionalItems: true items: - description: GMAC main clock @@ -129,7 +130,7 @@ properties: clock-names: minItems: 1 - maxItems: 8 + maxItems: 10 additionalItems: true contains: enum: diff --git a/Documentation/devicetree/bindings/net/tesla,ethqos.yaml b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml new file mode 100644 index 000000000000..b78829246364 --- /dev/null +++ b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml @@ -0,0 +1,114 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/tesla,ethqos.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: FSD Ethernet Quality of Service + +maintainers: + - Sriranjani P <sriranjani.p@samsung.com> + - Swathi K S <swathi.ks@samsung.com> + +description: + dwmmac based tesla ethernet devices which support Gigabit + ethernet. + +allOf: + - $ref: snps,dwmac.yaml# + +properties: + compatible: + const: tesla,fsd-ethqos-4.21.yaml + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + minItems: 5 + maxItems: 10 + + clock-names: + minItems: 5 + maxItems: 10 + items: + - const: ptp_ref + - const: master_bus + - const: slave_bus + - const: tx + - const: rx + - const: master2_bus + - const: slave2_bus + - const: eqos_rxclk_mux + - const: eqos_phyrxclk + - const: dout_peric_rgmii_clk + + fsd-rx-clock-skew: + $ref: /schemas/types.yaml#/definitions/phandle-array + items: + - items: + - description: phandle to the syscon node + - description: offset of the control register + description: + Should be phandle/offset pair. The phandle to the syscon node. + + iommus: + maxItems: 1 + + phy-mode: + $ref: ethernet-controller.yaml#/properties/phy-connection-type + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - rx-clock-skew + - iommus + - phy-mode + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/fsd-clk.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + ethernet_1: ethernet@14300000 { + compatible = "tesla,dwc-qos-ethernet-4.21"; + reg = <0x0 0x14300000 0x0 0x10000>; + interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_CLK_PTP_REF_I>, + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_ACLK_I>, + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_HCLK_I>, + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_RGMII_CLK_I>, + <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_CLK_RX_I>, + <&clock_peric PERIC_BUS_D_PERIC_IPCLKPORT_EQOSCLK>, + <&clock_peric PERIC_BUS_P_PERIC_IPCLKPORT_EQOSCLK>, + <&clock_peric PERIC_EQOS_PHYRXCLK_MUX>, + <&clock_peric PERIC_EQOS_PHYRXCLK>, + <&clock_peric PERIC_DOUT_RGMII_CLK>; + clock-names = "ptp_ref", + "master_bus", + "slave_bus", + "tx", + "rx", + "master2_bus", + "slave2_bus", + "eqos_rxclk_mux", + "eqos_phyrxclk", + "dout_peric_rgmii_clk"; + pinctrl-names = "default"; + pinctrl-0 = <ð1_tx_clk>, <ð1_tx_data>, <ð1_tx_ctrl>, + <ð1_phy_intr>, <ð1_rx_clk>, <ð1_rx_data>, + <ð1_rx_ctrl>, <ð1_mdio>; + fsd-rx-clock-skew = <&sysreg_peric 0x10>; + iommus = <&smmu_peric 0x0 0x1>; + phy-mode = "rgmii"; + }; + +...