Message ID | 20240309-net-v11-4-eb99b76e4a21@outlook.com (mailing list archive) |
---|---|
State | Deferred |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: hisi-femac: add support for Hi3798MV200, remove unmaintained compatibles | expand |
On 3/9/2024 8:26 PM, Yang Xiwen via B4 Relay wrote: > From: Yang Xiwen <forbidden405@outlook.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@linaro.org> > Signed-off-by: Yang Xiwen <forbidden405@outlook.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@foxmail.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@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.
On Sat, 9 Mar 2024 20:30:21 +0800 Yang Xiwen wrote: > > + 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. You are responding to yourself 4 min after posting? What is the purpose of your comments?
On 3/12/2024 12:03 AM, Jakub Kicinski wrote: > On Sat, 9 Mar 2024 20:30:21 +0800 Yang Xiwen wrote: >>> + 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. > You are responding to yourself 4 min after posting? > What is the purpose of your comments? Just to remind others or myself this can be improved. But i think it's ready to be applied. There won't be similar design in mainline soon i think.
On Tue, 12 Mar 2024 08:49:21 +0800 Yang Xiwen wrote: > >> 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. > > You are responding to yourself 4 min after posting? > > What is the purpose of your comments? > > Just to remind others or myself this can be improved. But i think it's > ready to be applied. There won't be similar design in mainline soon i think. It's a fairly uncommon thing to do, normally such notes should either be comments in the code or notes in the commit message. In any case - the merge window for v6.9 has started, we won't be able to merge these changes for the next 2 weeks :(
On 3/12/2024 9:08 AM, Jakub Kicinski wrote: > On Tue, 12 Mar 2024 08:49:21 +0800 Yang Xiwen wrote: >>>> 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. >>> You are responding to yourself 4 min after posting? >>> What is the purpose of your comments? >> Just to remind others or myself this can be improved. But i think it's >> ready to be applied. There won't be similar design in mainline soon i think. > It's a fairly uncommon thing to do, normally such notes should either > be comments in the code or notes in the commit message. Yeah. Maybe i can try to improve this in the next 6 weeks. But without the hardware to test, i can hardly say my implementation is correct or not. > > In any case - the merge window for v6.9 has started, we won't be able > to merge these changes for the next 2 weeks :( It's sad to wait for another 6 weeks till v6.10. But i'm okay with it, since i still have a lot of other patches pending to be applied for Hi3798MV200 SoC.
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@foxmail.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 + + resets: + items: + - description: MAC reset signal + - description: PHY reset signal + + 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 + +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>; - };