[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SEZPR06MB6959090F2C45C3E5D6B3F9F496262@SEZPR06MB6959.apcprd06.prod.outlook.com>
Date: Sat, 9 Mar 2024 20:30:21 +0800
From: Yang Xiwen <forbidden405@...look.com>
To: Yisen Zhuang <yisen.zhuang@...wei.com>,
Salil Mehta <salil.mehta@...wei.com>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH net-next v11 4/9] dt-bindings: net: convert hisi-femac.txt
to YAML
On 3/9/2024 8:26 PM, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@...look.com>
>
> 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@...aro.org>
> Signed-off-by: Yang Xiwen <forbidden405@...look.com>
> ---
> .../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@...mail.com>
> +
> +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@...90000 {
> + 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@...90000 {
> - 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
Powered by blists - more mailing lists