diff mbox series

[v2,2/4] dt-bindings: net: Add support for Sophgo SG2044 dwmac

Message ID 20241025011000.244350-3-inochiama@gmail.com (mailing list archive)
State New
Headers show
Series riscv: sophgo: Add ethernet support for SG2044 | expand

Commit Message

Inochi Amaoto Oct. 25, 2024, 1:09 a.m. UTC
The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
with some extra clock.

Add necessary compatible string for this device.

Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
 .../bindings/net/sophgo,sg2044-dwmac.yaml     | 124 ++++++++++++++++++
 2 files changed, 125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml

Comments

Krzysztof Kozlowski Oct. 27, 2024, 8:38 p.m. UTC | #1
On Fri, Oct 25, 2024 at 09:09:58AM +0800, Inochi Amaoto wrote:
> The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> with some extra clock.
> 
> Add necessary compatible string for this device.
> 
> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> ---

This should be squashed with a corrected previous patch (why do you need
to select snps,dwmac-5.30a?), so we clearly see which the fallback and
specific compatibles being added.

>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>  .../bindings/net/sophgo,sg2044-dwmac.yaml     | 124 ++++++++++++++++++
>  2 files changed, 125 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml

Best regards,
Krzysztof
Inochi Amaoto Oct. 27, 2024, 11:32 p.m. UTC | #2
On Sun, Oct 27, 2024 at 09:38:00PM +0100, Krzysztof Kozlowski wrote:
> On Fri, Oct 25, 2024 at 09:09:58AM +0800, Inochi Amaoto wrote:
> > The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> > with some extra clock.
> > 
> > Add necessary compatible string for this device.
> > 
> > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > ---
> 
> This should be squashed with a corrected previous patch 

Good, I will.

> (why do you need to select snps,dwmac-5.30a?), 

The is because the driver use the fallback versioned compatible 
string to set up some common arguments. (This is what the patch
3 does). It is also better to have a accurate fallback compatible
if we already know what it is.

Regards,
Inochi
Krzysztof Kozlowski Oct. 28, 2024, 7:06 a.m. UTC | #3
On 28/10/2024 00:32, Inochi Amaoto wrote:
> On Sun, Oct 27, 2024 at 09:38:00PM +0100, Krzysztof Kozlowski wrote:
>> On Fri, Oct 25, 2024 at 09:09:58AM +0800, Inochi Amaoto wrote:
>>> The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
>>> with some extra clock.
>>>
>>> Add necessary compatible string for this device.
>>>
>>> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
>>> ---
>>
>> This should be squashed with a corrected previous patch 
> 
> Good, I will.
> 
>> (why do you need to select snps,dwmac-5.30a?), 
> 
> The is because the driver use the fallback versioned compatible 
> string to set up some common arguments. (This is what the patch

Nope. Driver never relies on schema doing select. That's just incorrect.

> 3 does). It is also better to have a accurate fallback compatible
> if we already know what it is.

Best regards,
Krzysztof
Inochi Amaoto Oct. 28, 2024, 7:16 a.m. UTC | #4
On Mon, Oct 28, 2024 at 08:06:25AM +0100, Krzysztof Kozlowski wrote:
> On 28/10/2024 00:32, Inochi Amaoto wrote:
> > On Sun, Oct 27, 2024 at 09:38:00PM +0100, Krzysztof Kozlowski wrote:
> >> On Fri, Oct 25, 2024 at 09:09:58AM +0800, Inochi Amaoto wrote:
> >>> The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> >>> with some extra clock.
> >>>
> >>> Add necessary compatible string for this device.
> >>>
> >>> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> >>> ---
> >>
> >> This should be squashed with a corrected previous patch 
> > 
> > Good, I will.
> > 
> >> (why do you need to select snps,dwmac-5.30a?), 
> > 
> > The is because the driver use the fallback versioned compatible 
> > string to set up some common arguments. (This is what the patch
> 
> Nope. Driver never relies on schema doing select. That's just incorrect.
> 

Yeah, I make a mistake on understanding you. For me, I just followed
what others do. But there is a comment before this select.

"""
Select every compatible, including the deprecated ones. This way, we
will be able to report a warning when we have that compatible, since
we will validate the node thanks to the select, but won't report it
as a valid value in the compatible property description
"""

By reading this, I think there may be some historical reason? Maybe
someone can explain this.

Regards,
Inochi
Krzysztof Kozlowski Oct. 29, 2024, 1:27 p.m. UTC | #5
On 28/10/2024 08:16, Inochi Amaoto wrote:
> On Mon, Oct 28, 2024 at 08:06:25AM +0100, Krzysztof Kozlowski wrote:
>> On 28/10/2024 00:32, Inochi Amaoto wrote:
>>> On Sun, Oct 27, 2024 at 09:38:00PM +0100, Krzysztof Kozlowski wrote:
>>>> On Fri, Oct 25, 2024 at 09:09:58AM +0800, Inochi Amaoto wrote:
>>>>> The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
>>>>> with some extra clock.
>>>>>
>>>>> Add necessary compatible string for this device.
>>>>>
>>>>> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
>>>>> ---
>>>>
>>>> This should be squashed with a corrected previous patch 
>>>
>>> Good, I will.
>>>
>>>> (why do you need to select snps,dwmac-5.30a?), 
>>>
>>> The is because the driver use the fallback versioned compatible 
>>> string to set up some common arguments. (This is what the patch
>>
>> Nope. Driver never relies on schema doing select. That's just incorrect.
>>
> 
> Yeah, I make a mistake on understanding you. For me, I just followed
> what others do. But there is a comment before this select.
> 
> """
> Select every compatible, including the deprecated ones. This way, we
> will be able to report a warning when we have that compatible, since
> we will validate the node thanks to the select, but won't report it
> as a valid value in the compatible property description
> """
> 
> By reading this, I think there may be some historical reason? Maybe
> someone can explain this.

I think this is left-over from older times before all specific
compatibles were added here and in their bindings. This binding has been
waiting for some cleanup for a while now, so this is fine.

I still think the patches should be merged, though.

Best regards,
Krzysztof
Inochi Amaoto Oct. 30, 2024, 12:43 a.m. UTC | #6
On Tue, Oct 29, 2024 at 02:27:20PM +0100, Krzysztof Kozlowski wrote:
> On 28/10/2024 08:16, Inochi Amaoto wrote:
> > On Mon, Oct 28, 2024 at 08:06:25AM +0100, Krzysztof Kozlowski wrote:
> >> On 28/10/2024 00:32, Inochi Amaoto wrote:
> >>> On Sun, Oct 27, 2024 at 09:38:00PM +0100, Krzysztof Kozlowski wrote:
> >>>> On Fri, Oct 25, 2024 at 09:09:58AM +0800, Inochi Amaoto wrote:
> >>>>> The GMAC IP on SG2044 is almost a standard Synopsys DesignWare MAC
> >>>>> with some extra clock.
> >>>>>
> >>>>> Add necessary compatible string for this device.
> >>>>>
> >>>>> Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> >>>>> ---
> >>>>
> >>>> This should be squashed with a corrected previous patch 
> >>>
> >>> Good, I will.
> >>>
> >>>> (why do you need to select snps,dwmac-5.30a?), 
> >>>
> >>> The is because the driver use the fallback versioned compatible 
> >>> string to set up some common arguments. (This is what the patch
> >>
> >> Nope. Driver never relies on schema doing select. That's just incorrect.
> >>
> > 
> > Yeah, I make a mistake on understanding you. For me, I just followed
> > what others do. But there is a comment before this select.
> > 
> > """
> > Select every compatible, including the deprecated ones. This way, we
> > will be able to report a warning when we have that compatible, since
> > we will validate the node thanks to the select, but won't report it
> > as a valid value in the compatible property description
> > """
> > 
> > By reading this, I think there may be some historical reason? Maybe
> > someone can explain this.
> 
> I think this is left-over from older times before all specific
> compatibles were added here and in their bindings. This binding has been
> waiting for some cleanup for a while now, so this is fine.
> 
> I still think the patches should be merged, though.
> 
> Best regards,
> Krzysztof
> 

Thanks, I will squash this patch in the next version

Regards,
Inochi
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 3c4007cb65f8..69f6bb36970b 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -99,6 +99,7 @@  properties:
         - snps,dwmac-5.30a
         - snps,dwxgmac
         - snps,dwxgmac-2.10
+        - sophgo,sg2044-dwmac
         - starfive,jh7100-dwmac
         - starfive,jh7110-dwmac
 
diff --git a/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
new file mode 100644
index 000000000000..b7e4216ea45a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/sophgo,sg2044-dwmac.yaml
@@ -0,0 +1,124 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/sophgo,sg2044-dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2044 DWMAC glue layer
+
+maintainers:
+  - Inochi Amaoto <inochiama@gmail.com>
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - sophgo,sg2044-dwmac
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - const: sophgo,sg2044-dwmac
+      - const: snps,dwmac-5.30a
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: GMAC main clock
+      - description: PTP clock
+      - description: TX clock
+
+  clock-names:
+    items:
+      - const: stmmaceth
+      - const: ptp_ref
+      - const: tx
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: stmmaceth
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - interrupt-names
+  - resets
+  - reset-names
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    ethernet@30006000 {
+      compatible = "sophgo,sg2044-dwmac", "snps,dwmac-5.30a";
+      reg = <0x30006000 0x4000>;
+      clocks = <&clk 151>, <&clk 152>, <&clk 154>;
+      clock-names = "stmmaceth", "ptp_ref", "tx";
+      interrupt-parent = <&intc>;
+      interrupts = <296 IRQ_TYPE_LEVEL_HIGH>;
+      interrupt-names = "macirq";
+      resets = <&rst 30>;
+      reset-names = "stmmaceth";
+      snps,multicast-filter-bins = <0>;
+      snps,perfect-filter-entries = <1>;
+      snps,aal;
+      snps,tso;
+      snps,txpbl = <32>;
+      snps,rxpbl = <32>;
+      snps,mtl-rx-config = <&gmac0_mtl_rx_setup>;
+      snps,mtl-tx-config = <&gmac0_mtl_tx_setup>;
+      snps,axi-config = <&gmac0_stmmac_axi_setup>;
+      status = "disabled";
+
+      gmac0_mtl_rx_setup: rx-queues-config {
+        snps,rx-queues-to-use = <8>;
+        snps,rx-sched-wsp;
+        queue0 {};
+        queue1 {};
+        queue2 {};
+        queue3 {};
+        queue4 {};
+        queue5 {};
+        queue6 {};
+        queue7 {};
+      };
+
+      gmac0_mtl_tx_setup: tx-queues-config {
+        snps,tx-queues-to-use = <8>;
+        queue0 {};
+        queue1 {};
+        queue2 {};
+        queue3 {};
+        queue4 {};
+        queue5 {};
+        queue6 {};
+        queue7 {};
+      };
+
+      gmac0_stmmac_axi_setup: stmmac-axi-config {
+        snps,blen = <16 8 4 0 0 0 0>;
+        snps,wr_osr_lmt = <1>;
+        snps,rd_osr_lmt = <2>;
+      };
+    };