| Message ID | 20240305-net-v8-4-166aaeea2107@outlook.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | net: hisi-femac: add support for Hi3798MV200, remove unmaintained compatibles | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 89 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 05/03/2024 08:51, 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. > > Signed-off-by: Yang Xiwen <forbidden405@outlook.com> > --- > .../bindings/net/hisilicon,hisi-femac.yaml | 89 ++++++++++++++++++++++ > .../devicetree/bindings/net/hisilicon-femac.txt | 41 ---------- > 2 files changed, 89 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..ba207f2c9ae4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml > @@ -0,0 +1,89 @@ > +# 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: > + items: > + - enum: > + - hisilicon,hi3516cv300-femac > + - const: hisilicon,hisi-femac Drop this fallback, your later driver change does not use it, so neither should have binding. Explain in commit msg, that old binding was incorrect (we discussed it a lot) thus you are making such change during conversion. Best regards, Krzysztof
On 3/6/2024 3:39 PM, Krzysztof Kozlowski wrote: > On 05/03/2024 08:51, 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. >> >> Signed-off-by: Yang Xiwen <forbidden405@outlook.com> >> --- >> .../bindings/net/hisilicon,hisi-femac.yaml | 89 ++++++++++++++++++++++ >> .../devicetree/bindings/net/hisilicon-femac.txt | 41 ---------- >> 2 files changed, 89 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..ba207f2c9ae4 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml >> @@ -0,0 +1,89 @@ >> +# 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: >> + items: >> + - enum: >> + - hisilicon,hi3516cv300-femac >> + - const: hisilicon,hisi-femac > Drop this fallback, your later driver change does not use it, so neither > should have binding. Explain in commit msg, that old binding was > incorrect (we discussed it a lot) thus you are making such change during > conversion. What about deprecating "hisilicon,hisi-femac-vn" compatibles and introduce a new generic compatible "hisilicon,hisi-femac" instead? This way, We can keep backward compatibility. > > Best regards, > Krzysztof >
On 06/03/2024 10:28, Yang Xiwen wrote: > On 3/6/2024 3:39 PM, Krzysztof Kozlowski wrote: >> On 05/03/2024 08:51, 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. >>> >>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com> >>> --- >>> .../bindings/net/hisilicon,hisi-femac.yaml | 89 ++++++++++++++++++++++ >>> .../devicetree/bindings/net/hisilicon-femac.txt | 41 ---------- >>> 2 files changed, 89 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..ba207f2c9ae4 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml >>> @@ -0,0 +1,89 @@ >>> +# 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: >>> + items: >>> + - enum: >>> + - hisilicon,hi3516cv300-femac >>> + - const: hisilicon,hisi-femac >> Drop this fallback, your later driver change does not use it, so neither >> should have binding. Explain in commit msg, that old binding was >> incorrect (we discussed it a lot) thus you are making such change during >> conversion. > > > What about deprecating "hisilicon,hisi-femac-vn" compatibles and > introduce a new generic compatible "hisilicon,hisi-femac" instead? This > way, We can keep backward compatibility. What backward compatibility? Didn't you say bindings and driver are broken and you are going to break ABI to fix them up? Best regards, Krzysztof
On 3/7/2024 3:48 PM, Krzysztof Kozlowski wrote: > On 06/03/2024 10:28, Yang Xiwen wrote: >> On 3/6/2024 3:39 PM, Krzysztof Kozlowski wrote: >>> On 05/03/2024 08:51, 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. >>>> >>>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com> >>>> --- >>>> .../bindings/net/hisilicon,hisi-femac.yaml | 89 ++++++++++++++++++++++ >>>> .../devicetree/bindings/net/hisilicon-femac.txt | 41 ---------- >>>> 2 files changed, 89 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..ba207f2c9ae4 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml >>>> @@ -0,0 +1,89 @@ >>>> +# 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: >>>> + items: >>>> + - enum: >>>> + - hisilicon,hi3516cv300-femac >>>> + - const: hisilicon,hisi-femac >>> Drop this fallback, your later driver change does not use it, so neither >>> should have binding. Explain in commit msg, that old binding was >>> incorrect (we discussed it a lot) thus you are making such change during >>> conversion. >> >> What about deprecating "hisilicon,hisi-femac-vn" compatibles and >> introduce a new generic compatible "hisilicon,hisi-femac" instead? This >> way, We can keep backward compatibility. > What backward compatibility? Didn't you say bindings and driver are > broken and you are going to break ABI to fix them up? Yes, indeed the old binding is broken. I'm just wondering why you say "hisilicon,hisi-femac" should be removed. The FEMAC core on both SoCs are compatible afaik. > > Best regards, > Krzysztof >
On 07/03/2024 11:33, Yang Xiwen wrote: > On 3/7/2024 3:48 PM, Krzysztof Kozlowski wrote: >> On 06/03/2024 10:28, Yang Xiwen wrote: >>> On 3/6/2024 3:39 PM, Krzysztof Kozlowski wrote: >>>> On 05/03/2024 08:51, 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. >>>>> >>>>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com> >>>>> --- >>>>> .../bindings/net/hisilicon,hisi-femac.yaml | 89 ++++++++++++++++++++++ >>>>> .../devicetree/bindings/net/hisilicon-femac.txt | 41 ---------- >>>>> 2 files changed, 89 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..ba207f2c9ae4 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml >>>>> @@ -0,0 +1,89 @@ >>>>> +# 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: >>>>> + items: >>>>> + - enum: >>>>> + - hisilicon,hi3516cv300-femac >>>>> + - const: hisilicon,hisi-femac >>>> Drop this fallback, your later driver change does not use it, so neither >>>> should have binding. Explain in commit msg, that old binding was >>>> incorrect (we discussed it a lot) thus you are making such change during >>>> conversion. >>> >>> What about deprecating "hisilicon,hisi-femac-vn" compatibles and >>> introduce a new generic compatible "hisilicon,hisi-femac" instead? This >>> way, We can keep backward compatibility. >> What backward compatibility? Didn't you say bindings and driver are >> broken and you are going to break ABI to fix them up? > > > Yes, indeed the old binding is broken. I'm just wondering why you say > "hisilicon,hisi-femac" should be removed. The FEMAC core on both SoCs > are compatible afaik. You don't use that fallback. What is the point of having it? You express compatibility via SoC compatibles. Best regards, Krzysztof
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..ba207f2c9ae4 --- /dev/null +++ b/Documentation/devicetree/bindings/net/hisilicon,hisi-femac.yaml @@ -0,0 +1,89 @@ +# 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: + items: + - enum: + - hisilicon,hi3516cv300-femac + - const: hisilicon,hisi-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", "hisilicon,hisi-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>; - };