On Tue, Jul 20, 2021 at 3:58 AM Joakim Zhang <qiangqing.zhang@xxxxxxx> wrote: > > > 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 What I wrote for you is the same thing, just more concise. > > > > > > > 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 Personally, I'd do 'enum: [ 1, 2, 3 ]' instead. > > > > > > + > > > + 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 You either need to document all those deprecated properties or update the dts files. I'd prefer the latter. Rob
