2022-12-01 09:07:29

by yanhong wang

[permalink] [raw]
Subject: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.

Signed-off-by: Yanhong Wang <[email protected]>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 1 +
.../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++
MAINTAINERS | 5 +
3 files changed, 112 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index d8779d3de3d6..13c5928d7170 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -33,6 +33,7 @@ select:
- snps,dwmac-5.20
- snps,dwxgmac
- snps,dwxgmac-2.10
+ - starfive,dwmac

# Deprecated
- st,spear600-gmac
diff --git a/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
new file mode 100644
index 000000000000..561cf2a713ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 StarFive Technology Co., Ltd.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/net/dwmac-starfive.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: StarFive DWMAC glue layer
+
+maintainers:
+ - Yanhong Wang <[email protected]>
+
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - starfive,dwmac
+ required:
+ - compatible
+
+allOf:
+ - $ref: "snps,dwmac.yaml#"
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - starfive,dwmac
+ - const: snps,dwmac-5.20
+
+ clocks:
+ items:
+ - description: GMAC main clock
+ - description: GMAC AHB clock
+ - description: PTP clock
+ - description: TX clock
+ - description: GTXC clock
+ - description: GTX clock
+
+ clock-names:
+ contains:
+ enum:
+ - stmmaceth
+ - pclk
+ - ptp_ref
+ - tx
+ - gtxc
+ - gtx
+
+required:
+ - compatible
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/starfive-jh7110.h>
+ #include <dt-bindings/reset/starfive-jh7110.h>
+
+ stmmac_axi_setup: stmmac-axi-config {
+ snps,wr_osr_lmt = <4>;
+ snps,rd_osr_lmt = <4>;
+ snps,blen = <256 128 64 32 0 0 0>;
+ };
+
+ gmac0: ethernet@16030000 {
+ compatible = "starfive,dwmac", "snps,dwmac-5.20";
+ reg = <0x16030000 0x10000>;
+ clocks = <&aoncrg_clk JH7110_AONCLK_GMAC0_AXI>,
+ <&aoncrg_clk JH7110_AONCLK_GMAC0_AHB>,
+ <&syscrg_clk JH7110_SYSCLK_GMAC0_PTP>,
+ <&aoncrg_clk JH7110_AONCLK_GMAC0_TX>,
+ <&syscrg_clk JH7110_SYSCLK_GMAC0_GTXC>,
+ <&syscrg_clk JH7110_SYSCLK_GMAC0_GTXCLK>;
+ clock-names = "stmmaceth",
+ "pclk",
+ "ptp_ref",
+ "tx",
+ "gtxc",
+ "gtx";
+ resets = <&aoncrg JH7110_AONRST_GMAC0_AXI>,
+ <&aoncrg JH7110_AONRST_GMAC0_AHB>;
+ reset-names = "stmmaceth", "ahb";
+ interrupts = <7>, <6>, <5>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+ phy-mode = "rgmii-id";
+ snps,multicast-filter-bins = <64>;
+ snps,perfect-filter-entries = <8>;
+ rx-fifo-depth = <2048>;
+ tx-fifo-depth = <2048>;
+ snps,fixed-burst;
+ snps,no-pbl-x8;
+ snps,force_thresh_dma_mode;
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,tso;
+ snps,en-tx-lpi-clockgating;
+ snps,lpi_en;
+ snps,txpbl = <16>;
+ snps,rxpbl = <16>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index a70c1d0f303e..7eaaec8d3b96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19606,6 +19606,11 @@ F: Documentation/devicetree/bindings/clock/starfive*
F: drivers/clk/starfive/
F: include/dt-bindings/clock/starfive*

+STARFIVE DWMAC GLUE LAYER
+M: Yanhong Wang <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
+
STARFIVE PINCTRL DRIVER
M: Emil Renner Berthing <[email protected]>
M: Jianlong Huang <[email protected]>
--
2.17.1


2022-12-01 13:47:44

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac


On Thu, 01 Dec 2022 17:02:38 +0800, Yanhong Wang wrote:
> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>
> Signed-off-by: Yanhong Wang <[email protected]>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++
> MAINTAINERS | 5 +
> 3 files changed, 112 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml:30:16: [warning] wrong indentation: expected 14 but found 15 (indentation)

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/net/starfive,dwmac-plat.yaml#
Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
21 | #include <dt-bindings/clock/starfive-jh7110.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

2022-12-01 16:44:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

On 01/12/2022 10:02, Yanhong Wang wrote:
> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.

Subject: drop second, redundant "bindings".

>
> Signed-off-by: Yanhong Wang <[email protected]>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++
> MAINTAINERS | 5 +
> 3 files changed, 112 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index d8779d3de3d6..13c5928d7170 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -33,6 +33,7 @@ select:
> - snps,dwmac-5.20
> - snps,dwxgmac
> - snps,dwxgmac-2.10
> + - starfive,dwmac
>
> # Deprecated
> - st,spear600-gmac
> diff --git a/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> new file mode 100644
> index 000000000000..561cf2a713ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml

Filename should base on compatible.

> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/net/dwmac-starfive.yaml#"

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes from both lines.

> +
> +title: StarFive DWMAC glue layer
> +
> +maintainers:
> + - Yanhong Wang <[email protected]>
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - starfive,dwmac
> + required:
> + - compatible
> +
> +allOf:
> + - $ref: "snps,dwmac.yaml#"

Drop quotes.

> +
> +properties:
> + compatible:
> + oneOf:

Drop oneOf. You do not have more cases here.

> + - items:
> + - enum:
> + - starfive,dwmac

Wrong indentation.... kind of expected since you did not test the bindings.

> + - const: snps,dwmac-5.20
> +
> + clocks:
> + items:
> + - description: GMAC main clock
> + - description: GMAC AHB clock
> + - description: PTP clock
> + - description: TX clock
> + - description: GTXC clock
> + - description: GTX clock
> +
> + clock-names:
> + contains:
> + enum:
> + - stmmaceth
> + - pclk
> + - ptp_ref
> + - tx
> + - gtxc
> + - gtx

Names should be specific and with fixed order, just like clocks are.

> +
> +required:
> + - compatible
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> +
> +unevaluatedProperties: false
> +

Best regards,
Krzysztof

2022-12-01 17:04:28

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

Hey Yanhong,

On Thu, Dec 01, 2022 at 07:36:29AM -0600, Rob Herring wrote:
>
> On Thu, 01 Dec 2022 17:02:38 +0800, Yanhong Wang wrote:
> > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
> >
> > Signed-off-by: Yanhong Wang <[email protected]>
> > ---
> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++
> > MAINTAINERS | 5 +
> > 3 files changed, 112 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml:30:16: [warning] wrong indentation: expected 14 but found 15 (indentation)
>
> dtschema/dtc warnings/errors:
> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml: $id: relative path/filename doesn't match actual path or filename
> expected: http://devicetree.org/schemas/net/starfive,dwmac-plat.yaml#
> Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
> 21 | #include <dt-bindings/clock/starfive-jh7110.h>

Perhaps, rather than putting a long list of "prerequisite-patch-id" in
your cover letters etc, you drop the need for headers from your bindings
entirely? Otherwise, you need to wait for the clock bindings to be applied
before any of your other peripherals etc can have drivers/bindings upstream.

AFAIU, and Rob/Krzk please correct me, the example in a dt-binding
really is an *example* and there's no requirement for it to match the
jh7110 dts exactly. Because of that you can drop the header & just do
something like `clocks = <&clk 7>, <&clk 77>;` etc and the example is
still valid. Same goes for all of the other driver patchsets for new
StarFive stuff, like the pmu or crypto, that also have dt-bindings.

The only person who has to worry then about dependencies is me when I
apply the .dts patches :)

> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

2022-12-01 18:04:21

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
> On 01/12/2022 10:02, Yanhong Wang wrote:
> > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>
> Subject: drop second, redundant "bindings".
>
> >
> > Signed-off-by: Yanhong Wang <[email protected]>
> > ---
> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > +properties:
> > + compatible:
> > + oneOf:
>
> Drop oneOf. You do not have more cases here.
>
> > + - items:
> > + - enum:
> > + - starfive,dwmac
>
> Wrong indentation.... kind of expected since you did not test the bindings.
>
> > + - const: snps,dwmac-5.20

Disclaimer: no familiarity with the version info with DW stuff

Is it a bit foolish to call this binding "starfive,dwmac"? Could there
not be another StarFive SoC in the future that uses another DW mac IP
version & this would be better off as "starfive,jh7110-dwmac" or similar?

Thanks,
Conor.

2022-12-01 19:55:14

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

On Thu, Dec 01, 2022 at 05:35:37PM +0000, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
> > On 01/12/2022 10:02, Yanhong Wang wrote:
> > > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
> >
> > Subject: drop second, redundant "bindings".
> >
> > >
> > > Signed-off-by: Yanhong Wang <[email protected]>
> > > ---
> > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> >
> > Drop oneOf. You do not have more cases here.
> >
> > > + - items:
> > > + - enum:
> > > + - starfive,dwmac
> >
> > Wrong indentation.... kind of expected since you did not test the bindings.
> >
> > > + - const: snps,dwmac-5.20
>
> Disclaimer: no familiarity with the version info with DW stuff
>
> Is it a bit foolish to call this binding "starfive,dwmac"? Could there
> not be another StarFive SoC in the future that uses another DW mac IP
> version & this would be better off as "starfive,jh7110-dwmac" or similar?

Yes. Really, *only* "starfive,jh7110-dwmac" is enough IMO.

The question is what would the OS do with only understanding
"snps,dwmac-5.20"? The answer is typically nothing because it isn't
enough information to act on. So that compatible is not needed. Maybe
the driver can do some things based on version, but that can be implied
from the compatible (if not read from a register). And often, the exact
version is not known, so do you want to hardcode a guess in DT? For
these reasons, we've moved away from using these generic IP compatibles
(with or without versions).

Rob

2022-12-02 07:37:33

by yanhong wang

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac



On 2022/12/2 0:21, Krzysztof Kozlowski wrote:
> On 01/12/2022 10:02, Yanhong Wang wrote:
>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>
> Subject: drop second, redundant "bindings".
>

I'll fix the title in the next version.

>>
>> Signed-off-by: Yanhong Wang <[email protected]>
>> ---
>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
>> .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++
>> MAINTAINERS | 5 +
>> 3 files changed, 112 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index d8779d3de3d6..13c5928d7170 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -33,6 +33,7 @@ select:
>> - snps,dwmac-5.20
>> - snps,dwxgmac
>> - snps,dwxgmac-2.10
>> + - starfive,dwmac
>>
>> # Deprecated
>> - st,spear600-gmac
>> diff --git a/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>> new file mode 100644
>> index 000000000000..561cf2a713ab
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>
> Filename should base on compatible.
>

Will update file name.

>> @@ -0,0 +1,106 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/net/dwmac-starfive.yaml#"
>
> Does not look like you tested the bindings. Please run `make
> dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
>
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>
> Drop quotes from both lines.
>

Will remove quotes.

>> +
>> +title: StarFive DWMAC glue layer
>> +
>> +maintainers:
>> + - Yanhong Wang <[email protected]>
>> +
>> +select:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - starfive,dwmac
>> + required:
>> + - compatible
>> +
>> +allOf:
>> + - $ref: "snps,dwmac.yaml#"
>
> Drop quotes.
>

Will remove quotes.

>> +
>> +properties:
>> + compatible:
>> + oneOf:
>
> Drop oneOf. You do not have more cases here.
>

Will remove oneOf.

>> + - items:
>> + - enum:
>> + - starfive,dwmac
>
> Wrong indentation.... kind of expected since you did not test the bindings.
>

Will fix.

>> + - const: snps,dwmac-5.20
>> +
>> + clocks:
>> + items:
>> + - description: GMAC main clock
>> + - description: GMAC AHB clock
>> + - description: PTP clock
>> + - description: TX clock
>> + - description: GTXC clock
>> + - description: GTX clock
>> +
>> + clock-names:
>> + contains:
>> + enum:
>> + - stmmaceth
>> + - pclk
>> + - ptp_ref
>> + - tx
>> + - gtxc
>> + - gtx
>
> Names should be specific and with fixed order, just like clocks are.
>

Will fix.

>> +
>> +required:
>> + - compatible
>> + - clocks
>> + - clock-names
>> + - resets
>> + - reset-names
>> +
>> +unevaluatedProperties: false
>> +
>
> Best regards,
> Krzysztof
>

2022-12-02 07:39:26

by yanhong wang

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac



On 2022/12/2 1:35, Conor Dooley wrote:
> On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
>> On 01/12/2022 10:02, Yanhong Wang wrote:
>> > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>>
>> Subject: drop second, redundant "bindings".
>>
>> >
>> > Signed-off-by: Yanhong Wang <[email protected]>
>> > ---
>> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
>> > +properties:
>> > + compatible:
>> > + oneOf:
>>
>> Drop oneOf. You do not have more cases here.
>>
>> > + - items:
>> > + - enum:
>> > + - starfive,dwmac
>>
>> Wrong indentation.... kind of expected since you did not test the bindings.
>>
>> > + - const: snps,dwmac-5.20
>
> Disclaimer: no familiarity with the version info with DW stuff
>
> Is it a bit foolish to call this binding "starfive,dwmac"? Could there
> not be another StarFive SoC in the future that uses another DW mac IP
> version & this would be better off as "starfive,jh7110-dwmac" or similar?
>

The StarFive JH8100 SoC in the future that uses the same mac IP version, so call this binding "starfive,dwmac".

> Thanks,
> Conor.
>

2022-12-02 09:09:25

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

On 02/12/2022 08:16, yanhong wang wrote:
>
>
> On 2022/12/2 1:35, Conor Dooley wrote:
>> On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote:
>>> On 01/12/2022 10:02, Yanhong Wang wrote:
>>>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>>>
>>> Subject: drop second, redundant "bindings".
>>>
>>>>
>>>> Signed-off-by: Yanhong Wang <[email protected]>
>>>> ---
>>>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
>>>> +properties:
>>>> + compatible:
>>>> + oneOf:
>>>
>>> Drop oneOf. You do not have more cases here.
>>>
>>>> + - items:
>>>> + - enum:
>>>> + - starfive,dwmac
>>>
>>> Wrong indentation.... kind of expected since you did not test the bindings.
>>>
>>>> + - const: snps,dwmac-5.20
>>
>> Disclaimer: no familiarity with the version info with DW stuff
>>
>> Is it a bit foolish to call this binding "starfive,dwmac"? Could there
>> not be another StarFive SoC in the future that uses another DW mac IP
>> version & this would be better off as "starfive,jh7110-dwmac" or similar?
>>
>
> The StarFive JH8100 SoC in the future that uses the same mac IP version, so call this binding "starfive,dwmac".

It's not a reason to make compatible generic.

Best regards,
Krzysztof

2022-12-02 11:30:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1 3/7] dt-bindings: net: Add bindings for StarFive dwmac

On 01/12/2022 17:45, Conor Dooley wrote:
> Hey Yanhong,
>
> On Thu, Dec 01, 2022 at 07:36:29AM -0600, Rob Herring wrote:
>>
>> On Thu, 01 Dec 2022 17:02:38 +0800, Yanhong Wang wrote:
>>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs.
>>>
>>> Signed-off-by: Yanhong Wang <[email protected]>
>>> ---
>>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
>>> .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++
>>> MAINTAINERS | 5 +
>>> 3 files changed, 112 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml
>>>
>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml:30:16: [warning] wrong indentation: expected 14 but found 15 (indentation)
>>
>> dtschema/dtc warnings/errors:
>> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml: $id: relative path/filename doesn't match actual path or filename
>> expected: http://devicetree.org/schemas/net/starfive,dwmac-plat.yaml#
>> Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory
>> 21 | #include <dt-bindings/clock/starfive-jh7110.h>
>
> Perhaps, rather than putting a long list of "prerequisite-patch-id" in
> your cover letters etc, you drop the need for headers from your bindings
> entirely? Otherwise, you need to wait for the clock bindings to be applied
> before any of your other peripherals etc can have drivers/bindings upstream.
>
> AFAIU, and Rob/Krzk please correct me, the example in a dt-binding
> really is an *example* and there's no requirement for it to match the
> jh7110 dts exactly. Because of that you can drop the header & just do
> something like `clocks = <&clk 7>, <&clk 77>;` etc and the example is
> still valid. Same goes for all of the other driver patchsets for new
> StarFive stuff, like the pmu or crypto, that also have dt-bindings.
>
> The only person who has to worry then about dependencies is me when I
> apply the .dts patches :)

It's nice when example looks good and matching final DTS, but that's not
a requirement. Pretty often clocks/resets/domains are mocked for the
example.

Best regards,
Krzysztof