Hi Rob, > -----Original Message----- > From: Rob Herring <robh@xxxxxxxxxx> > Sent: 2021年7月20日 6:45 > To: Joakim Zhang <qiangqing.zhang@xxxxxxx> > Cc: davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; shawnguo@xxxxxxxxxx; > s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; > bruno.thomsen@xxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; > netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to > yaml > > On Fri, Jul 16, 2021 at 06:29:09PM +0800, Joakim Zhang wrote: > > In order to automate the verification of DT nodes convert fsl-fec.txt > > to fsl,fec.yaml, and pass binding check with below command. > > > > $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml > > DTEX Documentation/devicetree/bindings/net/fsl,fec.example.dts > > DTC > Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml > > CHECK > Documentation/devicetree/bindings/net/fsl,fec.example.dt.yaml > > > > Signed-off-by: Joakim Zhang <qiangqing.zhang@xxxxxxx> > > --- > > .../devicetree/bindings/net/fsl,fec.yaml | 213 ++++++++++++++++++ > > .../devicetree/bindings/net/fsl-fec.txt | 95 -------- > > 2 files changed, 213 insertions(+), 95 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/net/fsl,fec.yaml > > delete mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt > > Since the networking maintainers can't wait for actual binding reviews, you get > to send a follow-up patch... OK. > > > > diff --git a/Documentation/devicetree/bindings/net/fsl,fec.yaml > > b/Documentation/devicetree/bindings/net/fsl,fec.yaml > > new file mode 100644 > > index 000000000000..7fa11f6622b1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/fsl,fec.yaml > > @@ -0,0 +1,213 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > +cetree.org%2Fschemas%2Fnet%2Ffsl%2Cfec.yaml%23&data=04%7C01 > %7Cqia > > > +ngqing.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686 > ea1d3b > > > +c2b4c6fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknow > n%7CTWFp > > > +bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC > I6M > > > +n0%3D%7C1000&sdata=MG%2F9aKVQ8quuLiYxtC%2F7zqZTY7mIHur4G > D7tsPtfq% > > +2Bg%3D&reserved=0 > > +$schema: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=04%7C01%7Cqia > ngqing > > > +.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686ea1d3b > c2b4c6 > > > +fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknown%7CT > WFpbGZsb3 > > > +d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0 > %3D% > > > +7C1000&sdata=7vf8UYQ7c%2BlaJxzAzhzvkPPvbaX3UWVowCtDtPNO9p > Q%3D& > > +;reserved=0 > > + > > +title: Freescale Fast Ethernet Controller (FEC) > > + > > +maintainers: > > + - Joakim Zhang <qiangqing.zhang@xxxxxxx> > > + > > +allOf: > > + - $ref: ethernet-controller.yaml# > > + > > +properties: > > + compatible: > > + oneOf: > > + - enum: > > + - fsl,imx25-fec > > + - fsl,imx27-fec > > + - fsl,imx28-fec > > + - fsl,imx6q-fec > > + - fsl,mvf600-fec > > + - items: > > + - enum: > > + - fsl,imx53-fec > > + - fsl,imx6sl-fec > > + - const: fsl,imx25-fec > > + - items: > > + - enum: > > + - fsl,imx35-fec > > + - fsl,imx51-fec > > + - const: fsl,imx27-fec > > + - items: > > + - enum: > > + - fsl,imx6ul-fec > > + - fsl,imx6sx-fec > > + - const: fsl,imx6q-fec > > + - items: > > + - enum: > > + - fsl,imx7d-fec > > + - const: fsl,imx6sx-fec > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + minItems: 1 > > + maxItems: 4 > > + > > + interrupt-names: > > + description: > > + Names of the interrupts listed in interrupts property in the same > order. > > + The defaults if not specified are > > + __Number of interrupts__ __Default__ > > + 1 "int0" > > + 2 "int0", "pps" > > + 3 "int0", "int1", "int2" > > + 4 "int0", "int1", "int2", "pps" > > + The order may be changed as long as they correspond to the > interrupts > > + property. > > Why? None of the existing dts files do that and there is no reason to support > random order. You can do this: > > oneOf: > - minItems: 1 > items: > - const: int0 > - const: pps > - minItems: 3 > items: > - const: int0 > - const: int1 > - const: int2 > - const: pps How about below? oneOf: - items: - const: int0 - items: - const: int0 - const: pps - items: - const: int0 - const: int1 - const: int2 - items: - const: int0 - const: int1 - const: int2 - const: pps > > > > Currently, only i.mx7 uses "int1" and "int2". They correspond to > > Sounds like another constraint under an if/then schema. > > > + tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII > interrupts. > > + For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for > the pulse > > + per second interrupt associated with 1588 precision time > protocol(PTP). > > + > > + clocks: > > + minItems: 2 > > + maxItems: 5 > > + description: > > + The "ipg", for MAC ipg_clk_s, ipg_clk_mac_s that are for register > accessing. > > + The "ahb", for MAC ipg_clk, ipg_clk_mac that are bus clock. > > + The "ptp"(option), for IEEE1588 timer clock that requires the clock. > > + The "enet_clk_ref"(option), for MAC transmit/receiver reference > clock like > > + RGMII TXC clock or RMII reference clock. It depends on board > design, > > + the clock is required if RGMII TXC and RMII reference clock source > from > > + SOC internal PLL. > > + The "enet_out"(option), output clock for external device, like supply > clock > > + for PHY. The clock is required if PHY clock source from SOC. > > + > > + clock-names: > > + minItems: 2 > > + maxItems: 5 > > + contains: > > + enum: > > + - ipg > > + - ahb > > + - ptp > > + - enet_clk_ref > > + - enet_out > > This means clock-names contains one of these strings and then anything else is > valid. > > s/contains/items/ OK. > > > + > > + phy-mode: true > > + > > + phy-handle: true > > + > > + fixed-link: true > > + > > + local-mac-address: true > > + > > + mac-address: true > > + > > + phy-supply: > > + description: > > + Regulator that powers the Ethernet PHY. > > + > > + fsl,num-tx-queues: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + The property is valid for enet-avb IP, which supports hw multi > queues. > > + Should specify the tx queue number, otherwise set tx queue number > to 1. > > constraints? 2^32 queues are valid? Yes, the value should be limited to 1, 2, 3. Will add: default: 1 minimum: 1 maximum: 3 > > > + > > + fsl,num-rx-queues: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + The property is valid for enet-avb IP, which supports hw multi > queues. > > + Should specify the rx queue number, otherwise set rx queue number > to 1. > > constraints? Will add. > > > + > > + fsl,magic-packet: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > + If present, indicates that the hardware supports waking up via magic > packet. > > + > > + fsl,err006687-workaround-present: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > + If present indicates that the system has the hardware workaround > for > > + ERR006687 applied and does not need a software workaround. > > + > > + fsl,stop-mode: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + description: > > + Register bits of stop mode control, the format is <&gpr req_gpr > req_bit>. > > So, maxItems: 3 Why? I think maxItems: 1 > > > + gpr is the phandle to general purpose register node. > > + req_gpr is the gpr register offset for ENET stop request. > > + req_bit is the gpr bit offset for ENET stop request. > > + > > + mdio: > > + type: object > > + description: > > + Specifies the mdio bus in the FEC, used as a container for phy nodes. > > + > > + # Deprecated optional properties: > > + # To avoid these, create a phy node according to ethernet-phy.yaml > > + in the same # directory, and point the FEC's "phy-handle" property > > + to it. Then use # the phy's reset binding, again described by > ethernet-phy.yaml. > > + > > + phy-reset-gpios: > > + deprecated: true > > + description: > > + Should specify the gpio for phy reset. > > + > > + phy-reset-duration: > > + deprecated: true > > + description: > > + Reset duration in milliseconds. Should present only if property > > + "phy-reset-gpios" is available. Missing the property will have the > > + duration be 1 millisecond. Numbers greater than 1000 are invalid > > + and 1 millisecond will be used instead. > > + > > + phy-reset-active-high: > > + deprecated: true > > + description: > > + If present then the reset sequence using the GPIO specified in the > > + "phy-reset-gpios" property is reversed (H=reset state, L=operation > state). > > + > > + phy-reset-post-delay: > > + deprecated: true > > + description: > > + Post reset delay in milliseconds. If present then a delay of > phy-reset-post-delay > > + milliseconds will be observed after the phy-reset-gpios has been > toggled. > > + Can be omitted thus no delay is observed. Delay is in range of 1ms to > 1000ms. > > + Other delays are invalid. > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +# FIXME: We had better set additionalProperties to false to avoid > > +invalid or at # least undocumented properties. However, PHY may have > > +a deprecated option to # place PHY OF properties in the MAC node, > > +such as Micrel PHY, and we can find # these boards which is based on > i.MX6QDL. > > +additionalProperties: true > > Why can't the dts files be updated? Can you point me to an example .dts? Below dtsi under fec device node: arch/arm/boot/dts/imx6qdl-sabrelite.dtsi arch/arm/boot/dts/imx6qdl-nitrogen6_max.dtsi arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi .... Best Regards, Joakim Zhang > > + > > +examples: > > + - | > > + ethernet@83fec000 { > > + compatible = "fsl,imx51-fec", "fsl,imx27-fec"; > > + reg = <0x83fec000 0x4000>; > > + interrupts = <87>; > > + phy-mode = "mii"; > > + phy-reset-gpios = <&gpio2 14 0>; > > + phy-supply = <®_fec_supply>; > > + }; > > + > > + ethernet@83fed000 { > > + compatible = "fsl,imx51-fec", "fsl,imx27-fec"; > > + reg = <0x83fed000 0x4000>; > > + interrupts = <87>; > > + phy-mode = "mii"; > > + phy-reset-gpios = <&gpio2 14 0>; > > + phy-supply = <®_fec_supply>; > > + phy-handle = <ðphy0>; > > + > > + mdio { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + ethphy0: ethernet-phy@0 { > > + compatible = "ethernet-phy-ieee802.3-c22"; > > + reg = <0>; > > + }; > > + }; > > + }; > > diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt > > b/Documentation/devicetree/bindings/net/fsl-fec.txt > > deleted file mode 100644 > > index 9b543789cd52..000000000000 > > --- a/Documentation/devicetree/bindings/net/fsl-fec.txt > > +++ /dev/null > > @@ -1,95 +0,0 @@ > > -* Freescale Fast Ethernet Controller (FEC) > > - > > -Required properties: > > -- compatible : Should be "fsl,<soc>-fec" > > -- reg : Address and length of the register set for the device > > -- interrupts : Should contain fec interrupt > > -- phy-mode : See ethernet.txt file in the same directory > > - > > -Optional properties: > > -- phy-supply : regulator that powers the Ethernet PHY. > > -- phy-handle : phandle to the PHY device connected to this device. > > -- fixed-link : Assume a fixed link. See fixed-link.txt in the same directory. > > - Use instead of phy-handle. > > -- fsl,num-tx-queues : The property is valid for enet-avb IP, which > > supports > > - hw multi queues. Should specify the tx queue number, otherwise set > > tx queue > > - number to 1. > > -- fsl,num-rx-queues : The property is valid for enet-avb IP, which > > supports > > - hw multi queues. Should specify the rx queue number, otherwise set > > rx queue > > - number to 1. > > -- fsl,magic-packet : If present, indicates that the hardware supports > > waking > > - up via magic packet. > > -- fsl,err006687-workaround-present: If present indicates that the > > system has > > - the hardware workaround for ERR006687 applied and does not need a > > software > > - workaround. > > -- fsl,stop-mode: register bits of stop mode control, the format is > > - <&gpr req_gpr req_bit>. > > - gpr is the phandle to general purpose register node. > > - req_gpr is the gpr register offset for ENET stop request. > > - req_bit is the gpr bit offset for ENET stop request. > > - -interrupt-names: names of the interrupts listed in interrupts > > property in > > - the same order. The defaults if not specified are > > - __Number of interrupts__ __Default__ > > - 1 "int0" > > - 2 "int0", "pps" > > - 3 "int0", "int1", "int2" > > - 4 "int0", "int1", "int2", "pps" > > - The order may be changed as long as they correspond to the > > interrupts > > - property. Currently, only i.mx7 uses "int1" and "int2". They > > correspond to > > - tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII > interrupts. > > - For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for > > the pulse > > - per second interrupt associated with 1588 precision time protocol(PTP). > > - > > -Optional subnodes: > > -- mdio : specifies the mdio bus in the FEC, used as a container for > > phy nodes > > - according to phy.txt in the same directory > > - > > -Deprecated optional properties: > > - To avoid these, create a phy node according to phy.txt in the same > > - directory, and point the fec's "phy-handle" property to it. Then use > > - the phy's reset binding, again described by phy.txt. > > -- phy-reset-gpios : Should specify the gpio for phy reset > > -- phy-reset-duration : Reset duration in milliseconds. Should > > present > > - only if property "phy-reset-gpios" is available. Missing the > > property > > - will have the duration be 1 millisecond. Numbers greater than 1000 > > are > > - invalid and 1 millisecond will be used instead. > > -- phy-reset-active-high : If present then the reset sequence using > > the GPIO > > - specified in the "phy-reset-gpios" property is reversed (H=reset > > state, > > - L=operation state). > > -- phy-reset-post-delay : Post reset delay in milliseconds. If present > > then > > - a delay of phy-reset-post-delay milliseconds will be observed after > > the > > - phy-reset-gpios has been toggled. Can be omitted thus no delay is > > - observed. Delay is in range of 1ms to 1000ms. Other delays are invalid. > > - > > -Example: > > - > > -ethernet@83fec000 { > > - compatible = "fsl,imx51-fec", "fsl,imx27-fec"; > > - reg = <0x83fec000 0x4000>; > > - interrupts = <87>; > > - phy-mode = "mii"; > > - phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */ > > - local-mac-address = [00 04 9F 01 1B B9]; > > - phy-supply = <®_fec_supply>; > > -}; > > - > > -Example with phy specified: > > - > > -ethernet@83fec000 { > > - compatible = "fsl,imx51-fec", "fsl,imx27-fec"; > > - reg = <0x83fec000 0x4000>; > > - interrupts = <87>; > > - phy-mode = "mii"; > > - phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; /* GPIO2_14 */ > > - local-mac-address = [00 04 9F 01 1B B9]; > > - phy-supply = <®_fec_supply>; > > - phy-handle = <ðphy>; > > - mdio { > > - clock-frequency = <5000000>; > > - ethphy: ethernet-phy@6 { > > - compatible = "ethernet-phy-ieee802.3-c22"; > > - reg = <6>; > > - max-speed = <100>; > > - }; > > - }; > > -}; > > -- > > 2.17.1 > > > >
