Add documentation to describe T-HEAD dwmac.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 1 +
.../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++
2 files changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index b196c5de2061..73821f86a609 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -96,6 +96,7 @@ properties:
- snps,dwxgmac
- snps,dwxgmac-2.10
- starfive,jh7110-dwmac
+ - thead,th1520-dwmac
reg:
minItems: 1
diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml
new file mode 100644
index 000000000000..bf8ec8ca2753
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/thead,dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: T-HEAD DWMAC Ethernet controller
+
+maintainers:
+ - Jisheng Zhang <jszhang@kernel.org>
+
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - thead,th1520-dwmac
+ required:
+ - compatible
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - thead,th1520-dwmac
+ - const: snps,dwmac-3.70a
+
+ reg:
+ maxItems: 1
+
+ thead,gmacapb:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ The phandle to the syscon node that control ethernet
+ interface and timing delay.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - interrupt-names
+ - phy-mode
+ - thead,gmacapb
+
+allOf:
+ - $ref: snps,dwmac.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ gmac0: ethernet@e7070000 {
+ compatible = "thead,th1520-dwmac", "snps,dwmac-3.70a";
+ reg = <0xe7070000 0x2000>;
+ clocks = <&clk 1>, <&clk 2>;
+ clock-names = "stmmaceth", "pclk";
+ interrupts = <66>;
+ interrupt-names = "macirq";
+ phy-mode = "rgmii-id";
+ snps,fixed-burst;
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,pbl = <32>;
+ thead,gmacapb = <&gmacapb_syscon>;
+ phy-handle = <&phy0>;
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "snps,dwmac-mdio";
+
+ phy0: ethernet-phy@0 {
+ reg = <0>;
+ };
+ };
+ };
--
2.40.1On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote:
> Add documentation to describe T-HEAD dwmac.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++
> 2 files changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index b196c5de2061..73821f86a609 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -96,6 +96,7 @@ properties:
> - snps,dwxgmac
> - snps,dwxgmac-2.10
> - starfive,jh7110-dwmac
> + - thead,th1520-dwmac
>
> reg:
> minItems: 1
> diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml
> new file mode 100644
> index 000000000000..bf8ec8ca2753
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: T-HEAD DWMAC Ethernet controller
> +
> +maintainers:
> + - Jisheng Zhang <jszhang@kernel.org>
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - thead,th1520-dwmac
> + required:
> + - compatible
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - thead,th1520-dwmac
> + - const: snps,dwmac-3.70a
> +
> + reg:
> + maxItems: 1
> +
> + thead,gmacapb:
BTW what is a point in having the "apb" prefix in the name?
The property name like "thead,gmac-syscon" looks much more suitable
since it refers to the actual property content.
-Serge(y)
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + The phandle to the syscon node that control ethernet
> + interface and timing delay.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - interrupts
> + - interrupt-names
> + - phy-mode
> + - thead,gmacapb
> +
> +allOf:
> + - $ref: snps,dwmac.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + gmac0: ethernet@e7070000 {
> + compatible = "thead,th1520-dwmac", "snps,dwmac-3.70a";
> + reg = <0xe7070000 0x2000>;
> + clocks = <&clk 1>, <&clk 2>;
> + clock-names = "stmmaceth", "pclk";
> + interrupts = <66>;
> + interrupt-names = "macirq";
> + phy-mode = "rgmii-id";
> + snps,fixed-burst;
> + snps,axi-config = <&stmmac_axi_setup>;
> + snps,pbl = <32>;
> + thead,gmacapb = <&gmacapb_syscon>;
> + phy-handle = <&phy0>;
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> +
> + phy0: ethernet-phy@0 {
> + reg = <0>;
> + };
> + };
> + };
> --
> 2.40.1
>
>
On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote:
> Add documentation to describe T-HEAD dwmac.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++
> 2 files changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index b196c5de2061..73821f86a609 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -96,6 +96,7 @@ properties:
> - snps,dwxgmac
> - snps,dwxgmac-2.10
> - starfive,jh7110-dwmac
> + - thead,th1520-dwmac
>
> reg:
> minItems: 1
> diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml
> new file mode 100644
> index 000000000000..bf8ec8ca2753
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml
see further regarding using dwmac in the names here.
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: T-HEAD DWMAC Ethernet controller
Additionally would be nice to have a brief controller "description:"
having the next info: the SoCs the controllers can be found on, the DW
(G)MAC IP-core version the ethernet controller is based on and some
data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA
FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters
availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload
engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE
1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC
Management counters (MMC). In addition to that for DW QoS
ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues
and DMA channels, MTL queues capabilities (QoS-related), TSO
availability, SPO availability.
Note DMA FIFO sizes can be also constrained in the properties
"rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes -
in "snps,perfect-filter-entries" and "snps,multicast-filter-bins".
> +
> +maintainers:
> + - Jisheng Zhang <jszhang@kernel.org>
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - thead,th1520-dwmac
Referring to the DW IP-core in the compatible string isn't very
much useful especially seeing you have a generic fallback compatible.
Name like "thead,th1520-gmac" looks more informative indicating its
speed capability.
> + required:
> + - compatible
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - thead,th1520-dwmac
ditto.
-Serge(y)
> + - const: snps,dwmac-3.70a
> +
> + reg:
> + maxItems: 1
> +
> + thead,gmacapb:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + The phandle to the syscon node that control ethernet
> + interface and timing delay.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - interrupts
> + - interrupt-names
> + - phy-mode
> + - thead,gmacapb
> +
> +allOf:
> + - $ref: snps,dwmac.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + gmac0: ethernet@e7070000 {
> + compatible = "thead,th1520-dwmac", "snps,dwmac-3.70a";
> + reg = <0xe7070000 0x2000>;
> + clocks = <&clk 1>, <&clk 2>;
> + clock-names = "stmmaceth", "pclk";
> + interrupts = <66>;
> + interrupt-names = "macirq";
> + phy-mode = "rgmii-id";
> + snps,fixed-burst;
> + snps,axi-config = <&stmmac_axi_setup>;
> + snps,pbl = <32>;
> + thead,gmacapb = <&gmacapb_syscon>;
> + phy-handle = <&phy0>;
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> +
> + phy0: ethernet-phy@0 {
> + reg = <0>;
> + };
> + };
> + };
> --
> 2.40.1
>
>
On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > > Add documentation to describe T-HEAD dwmac. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > > 2 files changed, 78 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > index b196c5de2061..73821f86a609 100644 > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > @@ -96,6 +96,7 @@ properties: > > - snps,dwxgmac > > - snps,dwxgmac-2.10 > > - starfive,jh7110-dwmac > > + - thead,th1520-dwmac > > > > reg: > > minItems: 1 > > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > new file mode 100644 > > index 000000000000..bf8ec8ca2753 > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > see further regarding using dwmac in the names here. > > > @@ -0,0 +1,77 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > > +title: T-HEAD DWMAC Ethernet controller > > Additionally would be nice to have a brief controller "description:" > having the next info: the SoCs the controllers can be found on, the DW > (G)MAC IP-core version the ethernet controller is based on and some > data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > Management counters (MMC). In addition to that for DW QoS > ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > and DMA channels, MTL queues capabilities (QoS-related), TSO > availability, SPO availability. > > Note DMA FIFO sizes can be also constrained in the properties > "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". Hi Serge, Thank you for your code review. I have different views here: If we only support the gmac controller in one specific SoC, these detailed information is nice to have, but what about if the driver/dt-binding supports the gmac controller in different SoCs? These detailed information will be outdated. what's more, I think the purpose of dt-binding is different from the one of documentation. So I prefer to put these GMAC IP related detailed information into the SoC's dtsi commit msg rather than polluting the dt-binding. > > > + > > +maintainers: > > + - Jisheng Zhang <jszhang@kernel.org> > > + > > +select: > > + properties: > > + compatible: > > + contains: > > + enum: > > > + - thead,th1520-dwmac > > Referring to the DW IP-core in the compatible string isn't very > much useful especially seeing you have a generic fallback compatible. > Name like "thead,th1520-gmac" looks more informative indicating its > speed capability. This is just to follow the common style as those dwmac-* does. I'm not sure which is better, but personally, I'd like to keep current common style.
On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: > On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > > On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > > > Add documentation to describe T-HEAD dwmac. > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > > > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > > > 2 files changed, 78 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > index b196c5de2061..73821f86a609 100644 > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > @@ -96,6 +96,7 @@ properties: > > > - snps,dwxgmac > > > - snps,dwxgmac-2.10 > > > - starfive,jh7110-dwmac > > > + - thead,th1520-dwmac > > > > > > reg: > > > minItems: 1 > > > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > new file mode 100644 > > > index 000000000000..bf8ec8ca2753 > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > see further regarding using dwmac in the names here. > > > > > @@ -0,0 +1,77 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > > > +title: T-HEAD DWMAC Ethernet controller > > > > Additionally would be nice to have a brief controller "description:" > > having the next info: the SoCs the controllers can be found on, the DW > > (G)MAC IP-core version the ethernet controller is based on and some > > data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > > FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > > availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > > engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > > 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > > Management counters (MMC). In addition to that for DW QoS > > ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > > and DMA channels, MTL queues capabilities (QoS-related), TSO > > availability, SPO availability. > > > > Note DMA FIFO sizes can be also constrained in the properties > > "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > > in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". BTW plus to this you may wish to add the "rx-internal-delay-ps" and "tx-internal-delay-ps" properties constraints seeing they device supports internal Tx/Rx delays. > > Hi Serge, > > Thank you for your code review. I have different views here: If we > only support the gmac controller in one specific SoC, these detailed > information is nice to have, but what about if the driver/dt-binding > supports the gmac controller in different SoCs? These detailed > information will be outdated. First they won't. Second then you can either add more info to the description for instance in a separate paragraph or create a dedicated DT-bindings. Such information would be very much useful for the generic STMMAC driver code maintenance. > > what's more, I think the purpose of dt-binding is different from > the one of documentation. The purpose of the DT-bindings is a hardware "description". The info I listed describes your hardware. > > So I prefer to put these GMAC IP related detailed information into > the SoC's dtsi commit msg rather than polluting the dt-binding. > > > > > + > > > +maintainers: > > > + - Jisheng Zhang <jszhang@kernel.org> > > > + > > > +select: > > > + properties: > > > + compatible: > > > + contains: > > > + enum: > > > > > + - thead,th1520-dwmac > > > > Referring to the DW IP-core in the compatible string isn't very > > much useful especially seeing you have a generic fallback compatible. > > Name like "thead,th1520-gmac" looks more informative indicating its > > speed capability. > > This is just to follow the common style as those dwmac-* does. > I'm not sure which is better, but personally, I'd like to keep current > common style. It's not that common. Half the compatible strings use the notation suggested by me and it has more sense then a dwmac suffix. It's ok to use the suffix in the STMMAC driver-related things because the glue code is supposed to work with the DW *MAC generic code. Using it in the compatible string especially together with the generic fallback compatible just useless. -Serge(y)
On 28/08/2023 17:51, Serge Semin wrote: > On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: >> On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: >>> On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: >>>> Add documentation to describe T-HEAD dwmac. >>>> >>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> >>>> --- >>>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + >>>> .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ >>>> 2 files changed, 78 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> index b196c5de2061..73821f86a609 100644 >>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> @@ -96,6 +96,7 @@ properties: >>>> - snps,dwxgmac >>>> - snps,dwxgmac-2.10 >>>> - starfive,jh7110-dwmac >>>> + - thead,th1520-dwmac >>>> >>>> reg: >>>> minItems: 1 >>>> diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml >>>> new file mode 100644 >>>> index 000000000000..bf8ec8ca2753 >>>> --- /dev/null >>> >>>> +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml >>> >>> see further regarding using dwmac in the names here. >>> >>>> @@ -0,0 +1,77 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>> >>>> +title: T-HEAD DWMAC Ethernet controller >>> >>> Additionally would be nice to have a brief controller "description:" >>> having the next info: the SoCs the controllers can be found on, the DW >>> (G)MAC IP-core version the ethernet controller is based on and some >>> data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA >>> FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters >>> availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload >>> engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE >>> 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC >>> Management counters (MMC). In addition to that for DW QoS >>> ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues >>> and DMA channels, MTL queues capabilities (QoS-related), TSO >>> availability, SPO availability. >>> > >>> Note DMA FIFO sizes can be also constrained in the properties >>> "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - >>> in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". > > BTW plus to this you may wish to add the "rx-internal-delay-ps" and > "tx-internal-delay-ps" properties constraints seeing they device > supports internal Tx/Rx delays. > >> >> Hi Serge, >> > >> Thank you for your code review. I have different views here: If we >> only support the gmac controller in one specific SoC, these detailed >> information is nice to have, but what about if the driver/dt-binding >> supports the gmac controller in different SoCs? These detailed >> information will be outdated. > > First they won't. Second then you can either add more info to the > description for instance in a separate paragraph or create a dedicated > DT-bindings. Such information would be very much useful for the > generic STMMAC driver code maintenance. > >> >> what's more, I think the purpose of dt-binding is different from >> the one of documentation. > > The purpose of the DT-bindings is a hardware "description". The info I > listed describes your hardware. > >> >> So I prefer to put these GMAC IP related detailed information into >> the SoC's dtsi commit msg rather than polluting the dt-binding. >>> >>>> + >>>> +maintainers: >>>> + - Jisheng Zhang <jszhang@kernel.org> >>>> + >>>> +select: >>>> + properties: >>>> + compatible: >>>> + contains: >>>> + enum: >>> >>>> + - thead,th1520-dwmac >>> >>> Referring to the DW IP-core in the compatible string isn't very >>> much useful especially seeing you have a generic fallback compatible. >>> Name like "thead,th1520-gmac" looks more informative indicating its >>> speed capability. >> > >> This is just to follow the common style as those dwmac-* does. >> I'm not sure which is better, but personally, I'd like to keep current >> common style. > > It's not that common. Half the compatible strings use the notation > suggested by me and it has more sense then a dwmac suffix. It's ok to > use the suffix in the STMMAC driver-related things because the glue > code is supposed to work with the DW *MAC generic code. Using it in > the compatible string especially together with the generic fallback > compatible just useless. THEAD did not make dwmac here, but a gmac. dwmac does not exist in the context of Thead and Th1520, so the naming suggested by Serge makes sense. Best regards, Krzysztof
On Mon, Aug 28, 2023 at 07:55:44PM +0200, Krzysztof Kozlowski wrote: > On 28/08/2023 17:51, Serge Semin wrote: > > On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: > >> On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > >>> On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > >>>> Add documentation to describe T-HEAD dwmac. > >>>> > >>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > >>>> --- > >>>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > >>>> .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > >>>> 2 files changed, 78 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > >>>> index b196c5de2061..73821f86a609 100644 > >>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > >>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > >>>> @@ -96,6 +96,7 @@ properties: > >>>> - snps,dwxgmac > >>>> - snps,dwxgmac-2.10 > >>>> - starfive,jh7110-dwmac > >>>> + - thead,th1520-dwmac > >>>> > >>>> reg: > >>>> minItems: 1 > >>>> diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > >>>> new file mode 100644 > >>>> index 000000000000..bf8ec8ca2753 > >>>> --- /dev/null > >>> > >>>> +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > >>> > >>> see further regarding using dwmac in the names here. > >>> > >>>> @@ -0,0 +1,77 @@ > >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>> +%YAML 1.2 > >>>> +--- > >>>> +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>> + > >>> > >>>> +title: T-HEAD DWMAC Ethernet controller > >>> > >>> Additionally would be nice to have a brief controller "description:" > >>> having the next info: the SoCs the controllers can be found on, the DW > >>> (G)MAC IP-core version the ethernet controller is based on and some > >>> data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > >>> FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > >>> availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > >>> engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > >>> 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > >>> Management counters (MMC). In addition to that for DW QoS > >>> ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > >>> and DMA channels, MTL queues capabilities (QoS-related), TSO > >>> availability, SPO availability. > >>> > > > >>> Note DMA FIFO sizes can be also constrained in the properties > >>> "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > >>> in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". > > > > BTW plus to this you may wish to add the "rx-internal-delay-ps" and > > "tx-internal-delay-ps" properties constraints seeing they device > > supports internal Tx/Rx delays. > > > >> > >> Hi Serge, > >> > > > >> Thank you for your code review. I have different views here: If we > >> only support the gmac controller in one specific SoC, these detailed > >> information is nice to have, but what about if the driver/dt-binding > >> supports the gmac controller in different SoCs? These detailed > >> information will be outdated. > > > > First they won't. Second then you can either add more info to the > > description for instance in a separate paragraph or create a dedicated > > DT-bindings. Such information would be very much useful for the > > generic STMMAC driver code maintenance. > > > >> > >> what's more, I think the purpose of dt-binding is different from > >> the one of documentation. > > > > The purpose of the DT-bindings is a hardware "description". The info I > > listed describes your hardware. > > > >> > >> So I prefer to put these GMAC IP related detailed information into > >> the SoC's dtsi commit msg rather than polluting the dt-binding. > >>> > >>>> + > >>>> +maintainers: > >>>> + - Jisheng Zhang <jszhang@kernel.org> > >>>> + > >>>> +select: > >>>> + properties: > >>>> + compatible: > >>>> + contains: > >>>> + enum: > >>> > >>>> + - thead,th1520-dwmac > >>> > >>> Referring to the DW IP-core in the compatible string isn't very > >>> much useful especially seeing you have a generic fallback compatible. > >>> Name like "thead,th1520-gmac" looks more informative indicating its > >>> speed capability. > >> > > > >> This is just to follow the common style as those dwmac-* does. > >> I'm not sure which is better, but personally, I'd like to keep current > >> common style. > > > > It's not that common. Half the compatible strings use the notation > > suggested by me and it has more sense then a dwmac suffix. It's ok to > > use the suffix in the STMMAC driver-related things because the glue > > code is supposed to work with the DW *MAC generic code. Using it in > > the compatible string especially together with the generic fallback > > compatible just useless. > > THEAD did not make dwmac here, but a gmac. dwmac does not exist in the > context of Thead and Th1520, so the naming suggested by Serge makes sense. > I have no preference. But just want to confirm: the th1520 ethernet controller doesn't always function as GMAC, but can act as MII, so "thead,th1520-gmac" is still OK?
On Mon, Aug 28, 2023 at 06:51:49PM +0300, Serge Semin wrote: > On Mon, Aug 28, 2023 at 11:17:36PM +0800, Jisheng Zhang wrote: > > On Mon, Aug 28, 2023 at 04:13:00PM +0300, Serge Semin wrote: > > > On Sun, Aug 27, 2023 at 05:17:09PM +0800, Jisheng Zhang wrote: > > > > Add documentation to describe T-HEAD dwmac. > > > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > > --- > > > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + > > > > .../devicetree/bindings/net/thead,dwmac.yaml | 77 +++++++++++++++++++ > > > > 2 files changed, 78 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > > index b196c5de2061..73821f86a609 100644 > > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > > @@ -96,6 +96,7 @@ properties: > > > > - snps,dwxgmac > > > > - snps,dwxgmac-2.10 > > > > - starfive,jh7110-dwmac > > > > + - thead,th1520-dwmac > > > > > > > > reg: > > > > minItems: 1 > > > > diff --git a/Documentation/devicetree/bindings/net/thead,dwmac.yaml b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > new file mode 100644 > > > > index 000000000000..bf8ec8ca2753 > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/net/thead,dwmac.yaml > > > > > > see further regarding using dwmac in the names here. > > > > > > > @@ -0,0 +1,77 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/net/thead,dwmac.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > > > > +title: T-HEAD DWMAC Ethernet controller > > > > > > Additionally would be nice to have a brief controller "description:" > > > having the next info: the SoCs the controllers can be found on, the DW > > > (G)MAC IP-core version the ethernet controller is based on and some > > > data about the synthesize parameters: SMA (MDIO-bus), Tx/Rx COE, DMA > > > FIFOs size, perfect and hash MAC-filters size, L3L4 frame filters > > > availability, VLAN hash filter, SA/VLAN-tag insertion, ARP offload > > > engine, PHY interfaces (MII, RMII, RGMII, etc), EEE support, IEEE > > > 1588(-2008) Timestamping support, PMT and Wake-up frame support, MAC > > > Management counters (MMC). In addition to that for DW QoS > > > ETH/XGMAC/XLGMAC the next info would be useful: number of MTL Queues > > > and DMA channels, MTL queues capabilities (QoS-related), TSO > > > availability, SPO availability. > > > > > > > Note DMA FIFO sizes can be also constrained in the properties > > > "rx-fifo-depth" and "tx-fifo-depth"; perfect and hash MAC-filter sizes - > > > in "snps,perfect-filter-entries" and "snps,multicast-filter-bins". > > BTW plus to this you may wish to add the "rx-internal-delay-ps" and > "tx-internal-delay-ps" properties constraints seeing they device > supports internal Tx/Rx delays. > > > > > Hi Serge, > > > > > Thank you for your code review. I have different views here: If we > > only support the gmac controller in one specific SoC, these detailed > > information is nice to have, but what about if the driver/dt-binding > > supports the gmac controller in different SoCs? These detailed > > information will be outdated. > > First they won't. Second then you can either add more info to the > description for instance in a separate paragraph or create a dedicated > DT-bindings. Such information would be very much useful for the > generic STMMAC driver code maintenance. > > > > > what's more, I think the purpose of dt-binding is different from > > the one of documentation. > > The purpose of the DT-bindings is a hardware "description". The info I > listed describes your hardware. dt-binding VS. dts(i), they are different things. Part of what you listed belong dts(i), that's the reason why I prefer to put those into dts(i) commit msg. The HW description is in dts(i) itself rather than dt-binding. Anyway I will add generic decriptions to the dt-binding. > > > > > So I prefer to put these GMAC IP related detailed information into > > the SoC's dtsi commit msg rather than polluting the dt-binding. > > > > > > > + > > > > +maintainers: > > > > + - Jisheng Zhang <jszhang@kernel.org> > > > > + > > > > +select: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > > > > + - thead,th1520-dwmac > > > > > > Referring to the DW IP-core in the compatible string isn't very > > > much useful especially seeing you have a generic fallback compatible. > > > Name like "thead,th1520-gmac" looks more informative indicating its > > > speed capability. > > > > > This is just to follow the common style as those dwmac-* does. > > I'm not sure which is better, but personally, I'd like to keep current > > common style. > > It's not that common. Half the compatible strings use the notation > suggested by me and it has more sense then a dwmac suffix. It's ok to > use the suffix in the STMMAC driver-related things because the glue > code is supposed to work with the DW *MAC generic code. Using it in > the compatible string especially together with the generic fallback > compatible just useless. > > -Serge(y) >
© 2016 - 2025 Red Hat, Inc.