2021-07-16 10:29:50

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V1 0/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

This patch set intends to convert fec binding into scheme, and fixes
when do dtbs_check on ARCH arm.

One notice is that there are below logs for some dts when do dtbs_check:
ethernet@2188000: More than one condition true in oneOf schema:
We found that fec node in these dts all have "interrupts-extended"
property, and schema default is:
'oneOf': [{'required': ['interrupts']},
{'required': ['interrupts-extended']}],
so we don't know if it is a common issue or need be fixed in specific
bindings.

Joakim Zhang (3):
dt-bindings: net: fec: convert fsl,*fec bindings to yaml
ARM: dts: imx35: correct node name for FEC
ARM: dts: imx7-mba7: remove un-used "phy-reset-delay" property

.../devicetree/bindings/net/fsl,fec.yaml | 213 ++++++++++++++++++
.../devicetree/bindings/net/fsl-fec.txt | 95 --------
arch/arm/boot/dts/imx35.dtsi | 2 +-
arch/arm/boot/dts/imx7-mba7.dtsi | 1 -
arch/arm/boot/dts/imx7d-mba7.dts | 1 -
5 files changed, 214 insertions(+), 98 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/fsl,fec.yaml
delete mode 100644 Documentation/devicetree/bindings/net/fsl-fec.txt

--
2.17.1


2021-07-16 10:30:41

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

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 <[email protected]>
---
.../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

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: http://devicetree.org/schemas/net/fsl,fec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Fast Ethernet Controller (FEC)
+
+maintainers:
+ - Joakim Zhang <[email protected]>
+
+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. 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).
+
+ 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
+
+ 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.
+
+ 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.
+
+ 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>.
+ 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
+
+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 = <&reg_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 = <&reg_fec_supply>;
+ phy-handle = <&ethphy0>;
+
+ 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 = <&reg_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 = <&reg_fec_supply>;
- phy-handle = <&ethphy>;
- mdio {
- clock-frequency = <5000000>;
- ethphy: ethernet-phy@6 {
- compatible = "ethernet-phy-ieee802.3-c22";
- reg = <6>;
- max-speed = <100>;
- };
- };
-};
--
2.17.1

2021-07-16 10:31:22

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V1 3/3] ARM: dts: imx7-mba7: remove un-used "phy-reset-delay" property

Remove un-used "phy-reset-delay" property which found when do dtbs_check
(set additionalProperties: false in fsl,fec.yaml).
Double check current driver and commit history, "phy-reset-delay" never comes
up, so it should be safe to remove it.

$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
arch/arm/boot/dts/imx7d-mba7.dt.yaml: ethernet@30be0000: 'phy-reset-delay' does not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm/boot/dts/imx7d-mba7.dt.yaml: ethernet@30bf0000: 'phy-reset-delay' does not match any of the regexes: 'pinctrl-[0-9]+'
/arch/arm/boot/dts/imx7s-mba7.dt.yaml: ethernet@30be0000: 'phy-reset-delay' does not match any of the regexes: 'pinctrl-[0-9]+'

Signed-off-by: Joakim Zhang <[email protected]>
---
arch/arm/boot/dts/imx7-mba7.dtsi | 1 -
arch/arm/boot/dts/imx7d-mba7.dts | 1 -
2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx7-mba7.dtsi b/arch/arm/boot/dts/imx7-mba7.dtsi
index c6d1c63f7905..5e6bef230dc7 100644
--- a/arch/arm/boot/dts/imx7-mba7.dtsi
+++ b/arch/arm/boot/dts/imx7-mba7.dtsi
@@ -216,7 +216,6 @@
phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio7 15 GPIO_ACTIVE_LOW>;
phy-reset-duration = <1>;
- phy-reset-delay = <1>;
phy-supply = <&reg_fec1_pwdn>;
phy-handle = <&ethphy1_0>;
fsl,magic-packet;
diff --git a/arch/arm/boot/dts/imx7d-mba7.dts b/arch/arm/boot/dts/imx7d-mba7.dts
index 23856a8d4b8c..36ef6a3cdb0b 100644
--- a/arch/arm/boot/dts/imx7d-mba7.dts
+++ b/arch/arm/boot/dts/imx7d-mba7.dts
@@ -23,7 +23,6 @@
phy-mode = "rgmii-id";
phy-reset-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>;
phy-reset-duration = <1>;
- phy-reset-delay = <1>;
phy-supply = <&reg_fec2_pwdn>;
phy-handle = <&ethphy2_0>;
fsl,magic-packet;
--
2.17.1

2021-07-16 10:32:03

by Joakim Zhang

[permalink] [raw]
Subject: [PATCH V1 2/3] ARM: dts: imx35: correct node name for FEC

Correct node name for FEC which found when do dtbs_check.

$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/fsl,fec.yaml
arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard.dt.yaml: fec@50038000: $nodename:0: 'fec@50038000' does not match '^ethernet(@.*)?$'
arch/arm/boot/dts/imx35-pdk.dt.yaml: fec@50038000: $nodename:0: 'fec@50038000' does not match '^ethernet(@.*)?$'

Signed-off-by: Joakim Zhang <[email protected]>
---
arch/arm/boot/dts/imx35.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 98ccc81ca6d9..8e41c8b7bd70 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -189,7 +189,7 @@
status = "disabled";
};

- fec: fec@50038000 {
+ fec: ethernet@50038000 {
compatible = "fsl,imx35-fec", "fsl,imx27-fec";
reg = <0x50038000 0x4000>;
clocks = <&clks 46>, <&clks 8>;
--
2.17.1

2021-07-16 19:41:25

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH V1 0/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 16 Jul 2021 18:29:08 +0800 you wrote:
> This patch set intends to convert fec binding into scheme, and fixes
> when do dtbs_check on ARCH arm.
>
> One notice is that there are below logs for some dts when do dtbs_check:
> ethernet@2188000: More than one condition true in oneOf schema:
> We found that fec node in these dts all have "interrupts-extended"
> property, and schema default is:
> 'oneOf': [{'required': ['interrupts']},
> {'required': ['interrupts-extended']}],
> so we don't know if it is a common issue or need be fixed in specific
> bindings.
>
> [...]

Here is the summary with links:
- [V1,1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml
https://git.kernel.org/netdev/net-next/c/96e4781b3d93
- [V1,2/3] ARM: dts: imx35: correct node name for FEC
https://git.kernel.org/netdev/net-next/c/95740a9a3ad9
- [V1,3/3] ARM: dts: imx7-mba7: remove un-used "phy-reset-delay" property
https://git.kernel.org/netdev/net-next/c/86a176f485b5

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


2021-07-20 00:18:44

by Rob Herring (Arm)

[permalink] [raw]
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 <[email protected]>
> ---
> .../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...

>
> 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: http://devicetree.org/schemas/net/fsl,fec.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Fast Ethernet Controller (FEC)
> +
> +maintainers:
> + - Joakim Zhang <[email protected]>
> +
> +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



> 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/


> +
> + 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?

> +
> + 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?

> +
> + 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

> + 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?

> +
> +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 = <&reg_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 = <&reg_fec_supply>;
> + phy-handle = <&ethphy0>;
> +
> + 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 = <&reg_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 = <&reg_fec_supply>;
> - phy-handle = <&ethphy>;
> - mdio {
> - clock-frequency = <5000000>;
> - ethphy: ethernet-phy@6 {
> - compatible = "ethernet-phy-ieee802.3-c22";
> - reg = <6>;
> - max-speed = <100>;
> - };
> - };
> -};
> --
> 2.17.1
>
>

2021-07-20 10:01:17

by Joakim Zhang

[permalink] [raw]
Subject: RE: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml


Hi Rob,

> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: 2021??7??20?? 6:45
> To: Joakim Zhang <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> > ---
> > .../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&amp;data=04%7C01
> %7Cqia
> >
> +ngqing.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686
> ea1d3b
> >
> +c2b4c6fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknow
> n%7CTWFp
> >
> +bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> I6M
> >
> +n0%3D%7C1000&amp;sdata=MG%2F9aKVQ8quuLiYxtC%2F7zqZTY7mIHur4G
> D7tsPtfq%
> > +2Bg%3D&amp;reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cqia
> ngqing
> >
> +.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686ea1d3b
> c2b4c6
> >
> +fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknown%7CT
> WFpbGZsb3
> >
> +d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%
> >
> +7C1000&amp;sdata=7vf8UYQ7c%2BlaJxzAzhzvkPPvbaX3UWVowCtDtPNO9p
> Q%3D&amp
> > +;reserved=0
> > +
> > +title: Freescale Fast Ethernet Controller (FEC)
> > +
> > +maintainers:
> > + - Joakim Zhang <[email protected]>
> > +
> > +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 = <&reg_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 = <&reg_fec_supply>;
> > + phy-handle = <&ethphy0>;
> > +
> > + 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 = <&reg_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 = <&reg_fec_supply>;
> > - phy-handle = <&ethphy>;
> > - mdio {
> > - clock-frequency = <5000000>;
> > - ethphy: ethernet-phy@6 {
> > - compatible = "ethernet-phy-ieee802.3-c22";
> > - reg = <6>;
> > - max-speed = <100>;
> > - };
> > - };
> > -};
> > --
> > 2.17.1
> >
> >

2021-07-20 16:38:25

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V1 1/3] dt-bindings: net: fec: convert fsl,*fec bindings to yaml

On Tue, Jul 20, 2021 at 3:58 AM Joakim Zhang <[email protected]> wrote:
>
>
> Hi Rob,
>
> > -----Original Message-----
> > From: Rob Herring <[email protected]>
> > Sent: 2021年7月20日 6:45
> > To: Joakim Zhang <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; dl-linux-imx <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]; [email protected]
> > 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 <[email protected]>
> > > ---
> > > .../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&amp;data=04%7C01
> > %7Cqia
> > >
> > +ngqing.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686
> > ea1d3b
> > >
> > +c2b4c6fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknow
> > n%7CTWFp
> > >
> > +bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> > I6M
> > >
> > +n0%3D%7C1000&amp;sdata=MG%2F9aKVQ8quuLiYxtC%2F7zqZTY7mIHur4G
> > D7tsPtfq%
> > > +2Bg%3D&amp;reserved=0
> > > +$schema:
> > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > >
> > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cqia
> > ngqing
> > >
> > +.zhang%40nxp.com%7Cf54454be267b426e60d008d94b06dfc7%7C686ea1d3b
> > c2b4c6
> > >
> > +fa92cd99c5c301635%7C0%7C0%7C637623315180063755%7CUnknown%7CT
> > WFpbGZsb3
> > >
> > +d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> > %3D%
> > >
> > +7C1000&amp;sdata=7vf8UYQ7c%2BlaJxzAzhzvkPPvbaX3UWVowCtDtPNO9p
> > Q%3D&amp
> > > +;reserved=0
> > > +
> > > +title: Freescale Fast Ethernet Controller (FEC)
> > > +
> > > +maintainers:
> > > + - Joakim Zhang <[email protected]>
> > > +
> > > +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