Re: [PATCH net-next v11 4/9] dt-bindings: net: convert hisi-femac.txt to YAML

[Yang Xiwen]

On 3/9/2024 8:26 PM, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@xxxxxxxxxxx>
>
> Convert the old text binding to new YAML.
>
> While at it, make some changes to the binding:
> - The version numbers are not documented publicly. The version also does
> not change programming interface. Remove it until it's really needed.
> - A few clocks are missing in old binding file. Add them to match the real
> hardware.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Signed-off-by: Yang Xiwen <forbidden405@xxxxxxxxxxx>
> ---
>   .../bindings/net/hisilicon,hisi-femac.yaml         | 87 ++++++++++++++++++++++
>   .../devicetree/bindings/net/hisilicon-femac.txt    | 41 ----------
>   2 files changed, 87 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml
> new file mode 100644
> index 000000000000..3344d3bfefb8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/hisilicon,hisi-femac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Hisilicon Fast Ethernet MAC controller
> +
> +maintainers:
> +  - Yang Xiwen <forbidden405@xxxxxxxxxxx>
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml
> +
> +properties:
> +  compatible:
> +    enum:
> +      - hisilicon,hi3516cv300-femac
> +
> +  reg:
> +    items:
> +      - description: The first region is the MAC core register base and size.
> +      - description: The second region is the global MAC control register.
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: MAC main clock
> +      - description: MAC bus interface clock
> +      - description: PHY clock
> +
> +  clock-names:
> +    items:
> +      - const: mac
> +      - const: macif
> +      - const: phy


Still not very correct here. In downstream the core can also have an 
external PHY. The internal phy is also optional. So maybe this clock 
should be optional.


> +
> +  resets:
> +    items:
> +      - description: MAC reset signal
> +      - description: PHY reset signal


Same here


> +
> +  reset-names:
> +    items:
> +      - const: mac
> +      - const: phy
> +
> +  hisilicon,phy-reset-delays-us:
> +    description: PHY reset timing requirement (in micro seconds).
> +      The integrated PHY usually have a special reset timing sequence and must
> +      interact with MAC controller to accomplish the entire reset procedure. So
> +      these properties belong to MAC controller, not PHY.
> +    items:
> +      - description: pre-reset delay for PHY
> +      - description: reset pulse for PHY
> +      - description: post-reset delay for PHY


And here.


> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - resets
> +  - reset-names
> +  - phy-mode
> +  - phy-handle
> +  - hisilicon,phy-reset-delays-us
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    ethernet@10090000 {
> +        compatible = "hisilicon,hi3516cv300-femac";
> +        reg = <0x10090000 0x1000>, <0x10091300 0x200>;
> +        interrupts = <12>;
> +        clocks = <&clk_femac>, <&clk_femacif>, <&clk_fephy>;
> +        clock-names = "mac", "macif", "phy";
> +        resets = <&crg 0xec 0>, <&crg 0xec 3>;
> +        reset-names = "mac", "phy";
> +        mac-address = [00 00 00 00 00 00];
> +        phy-mode = "mii";
> +        phy-handle = <&fephy>;
> +        hisilicon,phy-reset-delays-us = <10000 20000 20000>;
> +    };
> diff --git a/Documentation/devicetree/bindings/net/hisilicon-femac.txt b/Documentation/devicetree/bindings/net/hisilicon-femac.txt
> deleted file mode 100644
> index 5f96976f3cea..000000000000
> --- a/Documentation/devicetree/bindings/net/hisilicon-femac.txt
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -Hisilicon Fast Ethernet MAC controller
> -
> -Required properties:
> -- compatible: should contain one of the following version strings:
> -	* "hisilicon,hisi-femac-v1"
> -	* "hisilicon,hisi-femac-v2"
> -	and the soc string "hisilicon,hi3516cv300-femac".
> -- reg: specifies base physical address(s) and size of the device registers.
> -  The first region is the MAC core register base and size.
> -  The second region is the global MAC control register.
> -- interrupts: should contain the MAC interrupt.
> -- clocks: A phandle to the MAC main clock.
> -- resets: should contain the phandle to the MAC reset signal(required) and
> -	the PHY reset signal(optional).
> -- reset-names: should contain the reset signal name "mac"(required)
> -	and "phy"(optional).
> -- phy-mode: see ethernet.txt [1].
> -- phy-handle: see ethernet.txt [1].
> -- hisilicon,phy-reset-delays-us: triplet of delays if PHY reset signal given.
> -	The 1st cell is reset pre-delay in micro seconds.
> -	The 2nd cell is reset pulse in micro seconds.
> -	The 3rd cell is reset post-delay in micro seconds.
> -
> -The MAC address will be determined using the optional properties
> -defined in ethernet.txt[1].
> -
> -[1] Documentation/devicetree/bindings/net/ethernet.txt
> -
> -Example:
> -	hisi_femac: ethernet@10090000 {
> -		compatible = "hisilicon,hi3516cv300-femac","hisilicon,hisi-femac-v2";
> -		reg = <0x10090000 0x1000>,<0x10091300 0x200>;
> -		interrupts = <12>;
> -		clocks = <&crg HI3518EV200_ETH_CLK>;
> -		resets = <&crg 0xec 0>,<&crg 0xec 3>;
> -		reset-names = "mac","phy";
> -		mac-address = [00 00 00 00 00 00];
> -		phy-mode = "mii";
> -		phy-handle = <&phy0>;
> -		hisilicon,phy-reset-delays-us = <10000 20000 20000>;
> -	};

Currently both binding and driver assume internal PHY is always present. 
This should be fixed in the future.

--
Regards,
Yang Xiwen