| Message ID | 20250317025922.1526937-4-jacky_chou@aspeedtech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add AST2600 RGMII delay into ftgmac100 | expand |
On Mon, 17 Mar 2025 10:59:21 +0800, Jacky Chou wrote: > Add tx-internal-delay-ps and rx-internal-delay-ps to > configure the RGMII delay for MAC. According to > ethernet-controller.yaml, they use for RGMII TX and RX delay. > > In Aspeed desgin, the RGMII delay is a number of ps as unit to > set delay, do not use one ps as unit. The values are different > from each MAC. So, here describes the property values > as index to configure corresponding scu register. > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > --- > .../bindings/net/faraday,ftgmac100.yaml | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml:71:8: [warning] wrong indentation: expected 6 but found 7 (indentation) ./Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml:78:8: [warning] wrong indentation: expected 6 but found 7 (indentation) ./Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml:119:7: [error] no new line character at the end of file (new-line-at-end-of-file) dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250317025922.1526937-4-jacky_chou@aspeedtech.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 17/03/2025 03:59, Jacky Chou wrote: > Add tx-internal-delay-ps and rx-internal-delay-ps to Please wrap code according to the preferred limit expressed in Kernel coding style (checkpatch is not a coding style description, but only a tool). However don't wrap blindly (see Kernel coding style). > configure the RGMII delay for MAC. According to > ethernet-controller.yaml, they use for RGMII TX and RX delay. > > In Aspeed desgin, the RGMII delay is a number of ps as unit to > set delay, do not use one ps as unit. The values are different > from each MAC. So, here describes the property values > as index to configure corresponding scu register. > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > --- ... > mdio: > $ref: /schemas/net/mdio.yaml# > > @@ -102,4 +116,4 @@ examples: > reg = <1>; > }; > }; > - }; > + }; > \ No newline at end of file This was neither tested nor reviewed by you before sending. Best regards, Krzysztof
> In Aspeed desgin, the RGMII delay is a number of ps as unit to > set delay, do not use one ps as unit. The values are different > from each MAC. So, here describes the property values > as index to configure corresponding scu register. > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > --- > .../bindings/net/faraday,ftgmac100.yaml | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml > index 55d6a8379025..c5904aa84e05 100644 > --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml > +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml > @@ -66,6 +66,20 @@ properties: > type: boolean > deprecated: true > > + rx-internal-delay-ps: > + description: > + Setting this property to a non-zero number sets the RX internal delay > + for the MAC. Use this property value as a index not a ps unit to > + configure the corresponding delay register field. And the index range is > + 0 to 63. You have to use picoseconds here. As with all DT binding, you use SI units, and the driver then converts them to whatever value you need to poke into the register. Andrew --- pw-bot: cr
On 17/03/2025 13:44, Andrew Lunn wrote: >> diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml >> index 55d6a8379025..c5904aa84e05 100644 >> --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml >> +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml >> @@ -66,6 +66,20 @@ properties: >> type: boolean >> deprecated: true >> >> + rx-internal-delay-ps: >> + description: >> + Setting this property to a non-zero number sets the RX internal delay >> + for the MAC. Use this property value as a index not a ps unit to >> + configure the corresponding delay register field. And the index range is >> + 0 to 63. > > You have to use picoseconds here. As with all DT binding, you use SI > units, and the driver then converts them to whatever value you need to > poke into the register. Ykes, I did notice that when skimming through the patch. Such a sneaky way to pretend you conform to the bindings but eh, not really, I will do it my way. I think reviewing Aspeed code takes a lot, a lot of our time. It's not only about this patchset but several others. Maybe it is time for Aspeed to perform intensive internal review, before they post to the mailing lists? Several other companies do it, more or less. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml index 55d6a8379025..c5904aa84e05 100644 --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml @@ -66,6 +66,20 @@ properties: type: boolean deprecated: true + rx-internal-delay-ps: + description: + Setting this property to a non-zero number sets the RX internal delay + for the MAC. Use this property value as a index not a ps unit to + configure the corresponding delay register field. And the index range is + 0 to 63. + + tx-internal-delay-ps: + description: + Setting this property to a non-zero number sets the TX internal delay + for the MAC. Use this property value as a index not a ps unit to + configure the corresponding delay register field. And the index range is + 0 to 63. + mdio: $ref: /schemas/net/mdio.yaml# @@ -102,4 +116,4 @@ examples: reg = <1>; }; }; - }; + }; \ No newline at end of file
Add tx-internal-delay-ps and rx-internal-delay-ps to configure the RGMII delay for MAC. According to ethernet-controller.yaml, they use for RGMII TX and RX delay. In Aspeed desgin, the RGMII delay is a number of ps as unit to set delay, do not use one ps as unit. The values are different from each MAC. So, here describes the property values as index to configure corresponding scu register. Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> --- .../bindings/net/faraday,ftgmac100.yaml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)