linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures
@ 2020-12-14  9:15 Serge Semin
  2020-12-14  9:15 ` [PATCH 01/25] dt-bindings: net: dwmac: Validate PBL for all IP-cores Serge Semin
                   ` (24 more replies)
  0 siblings, 25 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Baikal-T1 SoC is equipped with two Synopsys DesignWare GMAC v3.73a-based
ethernet interfaces with no internal Ethernet PHY attached. The IP-cores
are configured as GMAC-AXI with CSR interface clocked by a dedicated
signal. Each of which has got Rx/Tx FIFOs of 16KB, up to 8 MAC addresses
capability, no embedded filter hash table logic, EEE enabled, IEEE 1588
and 1588-2008 Advanced timestamping capabilities, power management with
remote wake-up, IP CSUM hardware acceleration, a single PHY interface -
RGMII with MDIO bus, 1xGPI and 1xGPO.

This is a very first series of patches with fixes we've found to be
required in order to make things working well for our setup. The series
has turned to be rather large, but most of the patches are trivial and
some of them are just cleanups, so it shouldn't be that hard to review.

The series starts with fixes of the PBL (Programmable DMA Burst length)
DT-property, which is supposed to be defined for each DW *MAC IP-core, but
not only for a Allwinner sun* GMAC and DW xGMAC. The number of possible
PBL values need to be also extended in accordance with the DW *MAC manual.
Then the TSO flag property should be also declared free of the
vendor-specific conditional schema, because the driver expects the
compatible string to have the IP-core version specified anyway and none of
the glue-drivers refer to the property directly.

Then we suggest to refactor the "snps,{axi,mtl-rx,mtl-tx}-config"
properties/nodes declaration, so the configs would be able to be defined
as the sub-nodes of the DW *MAC DT nodes. The reason is that the DW MAC
DT-schema doesn't validate them at the moment and having them defined as
separate from the DW MAC nodes isn't descriptive at all. (Please note the
patch log, since the DT-schema tool needs to be fixed in order to make the
change working).

Another big modification of the DW *MAC bindings file is the generic
DT-properties and generic DT-nodes schema splitting up. So in order to
improve the DW *MAC bindings maintainability we suggest to leave the
generic DW *MAC properties definition in the "snps,dwmac.yaml" file and
move the bindings for the generic DW *MAC DT-nodes validation in the
dedicated DT-schema "snps,dwmac-generic.yaml".

Another concern has been related with the System/CSR clocks. We have
discovered that currently the "stmmaceth" clocks are considered by the
driver as the combined system+CSR clocks, while in fact CSR interface can
be equipped with a dedicated clock source (this is our case). If so then
the clock with "pclk" can be used to define the later one. But neither
bindings are descriptive enough nor the DW *MAC driver is fixed to support
that feature. So first we suggest to elaborate stmmaceth/pclk description
in the bindings file and then fix the MDIO-bus clock selection procedure
so pclk would be used there if specified. The DW QoS Eth MAC driver is
also fixed in accordance with that modification.

The biggest part of the series concerns adding the generic Tx/Rx clocks
support to the DT-schema and to the DW MAC drivers and with fixed related
to that. It is really a good decision to add the generic Tx/Rx clocks,
because a lot of the glue-drivers expect them to be specified in the
DT-node. So first we add the "tx"/"rx" clocks declaration in the generic
DW MAC DT-schema. Then the glue-drivers like
dwmac-rk/dwmac-sti/dwmac-stm32 remove() callbacks need to be fixed to call
stmmac_remove_config_dt() otherwise the resources allocated in the
stmmac_probe_config_dt() won't be freed on the device removal. A small
modification needs to be provided for the cleanup-on-failure path of the
stmmac_probe_config_dt() method in order to improve its maintainability.
Then we've discovered that the "stmmaceth" and "pclk" clocks while being
acquired and enabled in the stmmac_probe_config_dt() method are disabled
in the stmmac_dvr_remove() function, which is erroneous for every
cleanup-on-failure path of the glue-driver probe methods. Finally before
adding the Tx/Rx clocks support we provide a set of optimizations of the
"stmmaceth"/"pclk"/"ptp_clk" clocks and the "stmmaceth" reset procedures
by removing the manual optional resources acquisition/enable/disable
implementation with the one provided by the corresponding subsystems.
Since the generic Tx/Rx clocks have been added we can freely remove the
similar clocks handling from the glue-drivers.

(Please note I have also discovered, but didn't try to fix the Allwinner
Sun8i cleanup-on-failure path implemented in the DW MAC probe() procedure.
It has been broken since don't know what time and it's a bit too
complicated to be fixed with no hardware at hands.)

That's it for now. The next series will concern the GPIOs support and
Baikal-T1 SoC specific bindings.

Fixes: d2ed0a7755fe ("net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks")
Fixes: f573c0b9c4e0 ("stmmac: move stmmac_clk, pclk, clk_ptp_ref and stmmac_rst to platform structure")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Vyacheslav Mitrofanov <Vyacheslav.Mitrofanov@baikalelectronics.ru>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (25):
  dt-bindings: net: dwmac: Validate PBL for all IP-cores
  dt-bindings: net: dwmac: Extend number of PBL values
  dt-bindings: net: dwmac: Fix the TSO property declaration
  dt-bindings: net: dwmac: Refactor snps,*-config properties
  dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description
  dt-bindings: net: dwmac: Add Tx/Rx clock sources
  dt-bindings: net: dwmac: Detach Generic DW MAC bindings
  net: stmmac: Add snps,*-config sub-nodes support
  net: stmmac: dwmac-rk: Cleanup STMMAC DT-config in remove cb
  net: stmmac: dwmac-sti: Cleanup STMMAC DT-config in remove cb
  net: stmmac: dwmac-stm32: Cleanup STMMAC DT-config in remove cb
  net: stmmac: Directly call reverse methods in stmmac_probe_config_dt()
  net: stmmac: Fix clocks left enabled on glue-probes failure
  net: stmmac: Use optional clock request method to get stmmaceth
  net: stmmac: Use optional clock request method to get pclk
  net: stmmac: Use optional clock request method to get ptp_clk
  net: stmmac: Use optional reset control API to work with stmmaceth
  net: stmmac: dwc-qos: Cleanup STMMAC platform data clock pointers
  net: stmmac: dwc-qos: Use dev_err_probe() for probe errors handling
  net: stmmac: Add Tx/Rx platform clocks support
  net: stmmac: dwc-qos: Discard Tx/Rx clocks request
  net: stmmac: dwmac-imx: Discard Tx clock request
  net: stmmac: Call stmmaceth clock as system clock in warn-message
  net: stmmac: Use pclk to set MDC clock frequency
  net: stmmac: dwc-qos: Save master/slave clocks in the plat-data

 .../bindings/net/snps,dwmac-generic.yaml      | 148 +++++
 .../devicetree/bindings/net/snps,dwmac.yaml   | 569 ++++++++++--------
 .../stmicro/stmmac/dwmac-dwc-qos-eth.c        |  91 +--
 .../net/ethernet/stmicro/stmmac/dwmac-imx.c   |  21 +-
 .../net/ethernet/stmicro/stmmac/dwmac-intel.c |   2 +
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    |   3 +
 .../net/ethernet/stmicro/stmmac/dwmac-sti.c   |   3 +
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c |   2 +
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  31 +-
 .../ethernet/stmicro/stmmac/stmmac_platform.c | 104 ++--
 include/linux/stmmac.h                        |   2 +
 11 files changed, 611 insertions(+), 365 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

-- 
2.29.2


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/25] dt-bindings: net: dwmac: Validate PBL for all IP-cores
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-15 17:14   ` Rob Herring
  2020-12-14  9:15 ` [PATCH 02/25] dt-bindings: net: dwmac: Extend number of PBL values Serge Semin
                   ` (23 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Indeed the maximum DMA burst length can be programmed not only for DW
xGMACs, Allwinner EMACs and Spear SoC GMAC, but in accordance with [1]
for Generic DW *MAC IP-cores. Moreover the STMMAC set of drivers parse
the property and then apply the configuration for all supported DW MAC
devices. All of that makes the property being available for all IP-cores
the bindings supports. Let's make sure the PBL-related properties are
validated for all of them by the common DW MAC DT schema.

[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
    October 2013, p. 380.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   | 69 +++++++------------
 1 file changed, 26 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 11a6fdb657c9..4b672499f20d 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -262,6 +262,32 @@ properties:
       is supported. For example, this is used in case of SGMII and
       MAC2MAC connection.
 
+  snps,pbl:
+    description:
+      Programmable Burst Length (tx and rx)
+    $ref: /schemas/types.yaml#definitions/uint32
+    enum: [2, 4, 8]
+
+  snps,txpbl:
+    description:
+      Tx Programmable Burst Length. If set, DMA tx will use this
+      value rather than snps,pbl.
+    $ref: /schemas/types.yaml#definitions/uint32
+    enum: [2, 4, 8]
+
+  snps,rxpbl:
+    description:
+      Rx Programmable Burst Length. If set, DMA rx will use this
+      value rather than snps,pbl.
+    $ref: /schemas/types.yaml#definitions/uint32
+    enum: [2, 4, 8]
+
+  snps,no-pbl-x8:
+    $ref: /schemas/types.yaml#definitions/flag
+    description:
+      Don\'t multiply the pbl/txpbl/rxpbl values by 8. For core
+      rev < 3.50, don\'t multiply the values by 4.
+
   mdio:
     type: object
     description:
@@ -287,49 +313,6 @@ dependencies:
 
 allOf:
   - $ref: "ethernet-controller.yaml#"
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - allwinner,sun7i-a20-gmac
-              - allwinner,sun8i-a83t-emac
-              - allwinner,sun8i-h3-emac
-              - allwinner,sun8i-r40-emac
-              - allwinner,sun8i-v3s-emac
-              - allwinner,sun50i-a64-emac
-              - snps,dwxgmac
-              - snps,dwxgmac-2.10
-              - st,spear600-gmac
-
-    then:
-      properties:
-        snps,pbl:
-          description:
-            Programmable Burst Length (tx and rx)
-          $ref: /schemas/types.yaml#definitions/uint32
-          enum: [2, 4, 8]
-
-        snps,txpbl:
-          description:
-            Tx Programmable Burst Length. If set, DMA tx will use this
-            value rather than snps,pbl.
-          $ref: /schemas/types.yaml#definitions/uint32
-          enum: [2, 4, 8]
-
-        snps,rxpbl:
-          description:
-            Rx Programmable Burst Length. If set, DMA rx will use this
-            value rather than snps,pbl.
-          $ref: /schemas/types.yaml#definitions/uint32
-          enum: [2, 4, 8]
-
-        snps,no-pbl-x8:
-          $ref: /schemas/types.yaml#definitions/flag
-          description:
-            Don\'t multiply the pbl/txpbl/rxpbl values by 8. For core
-            rev < 3.50, don\'t multiply the values by 4.
-
   - if:
       properties:
         compatible:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/25] dt-bindings: net: dwmac: Extend number of PBL values
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
  2020-12-14  9:15 ` [PATCH 01/25] dt-bindings: net: dwmac: Validate PBL for all IP-cores Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-15 17:14   ` Rob Herring
  2020-12-14  9:15 ` [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration Serge Semin
                   ` (22 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

In accordance with [1] the permitted PBL values can be set as one of
[1, 2, 4, 8, 16, 32]. The rest of the values results in undefined
behavior. At the same time some of the permitted values can be also
invalid depending on the controller FIFOs size and the data bus width.
Seeing due to having too many variables all the possible PBL property
constraints can't be implemented in the bindings schema, let's extend
the set of permitted PBL values to be as much as the configuration
register supports leaving the undefined behaviour cases for developers
to handle.

[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
    October 2013, p. 380.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 4b672499f20d..e084fbbf976e 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -264,23 +264,26 @@ properties:
 
   snps,pbl:
     description:
-      Programmable Burst Length (tx and rx)
+      Programmable Burst Length (tx and rx). Note some of these values
+      can be still invalid due to HW limitations connected with the data
+      bus width and the FIFOs depth, so a total length of a single DMA
+      burst shouldn't exceed half the FIFO depth.
     $ref: /schemas/types.yaml#definitions/uint32
-    enum: [2, 4, 8]
+    enum: [1, 2, 4, 8, 16, 32]
 
   snps,txpbl:
     description:
       Tx Programmable Burst Length. If set, DMA tx will use this
       value rather than snps,pbl.
     $ref: /schemas/types.yaml#definitions/uint32
-    enum: [2, 4, 8]
+    enum: [1, 2, 4, 8, 16, 32]
 
   snps,rxpbl:
     description:
       Rx Programmable Burst Length. If set, DMA rx will use this
       value rather than snps,pbl.
     $ref: /schemas/types.yaml#definitions/uint32
-    enum: [2, 4, 8]
+    enum: [1, 2, 4, 8, 16, 32]
 
   snps,no-pbl-x8:
     $ref: /schemas/types.yaml#definitions/flag
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
  2020-12-14  9:15 ` [PATCH 01/25] dt-bindings: net: dwmac: Validate PBL for all IP-cores Serge Semin
  2020-12-14  9:15 ` [PATCH 02/25] dt-bindings: net: dwmac: Extend number of PBL values Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-15 17:22   ` Rob Herring
  2020-12-14  9:15 ` [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties Serge Semin
                   ` (21 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Indeed the STMMAC driver doesn't take the vendor-specific compatible
string into account to parse the "snps,tso" boolean property. It just
makes sure the node is compatible with DW MAC 4.x, 5.x and DW xGMAC
IP-cores. Fix the conditional statement so the TSO-property would be
evaluated for the compatibles having the corresponding IP-core version.

While at it move the whole allOf-block from the tail of the binding file
to the head of it, as it's normally done in the most of the DT schemas.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Note this won't break the bindings description, since the "snps,tso"
property isn't parsed by the Allwinner SunX GMAC glue driver, but only
by the generic platform DT-parser.
---
 .../devicetree/bindings/net/snps,dwmac.yaml   | 52 +++++++++----------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e084fbbf976e..0dd543c6c08e 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -37,6 +37,30 @@ select:
   required:
     - compatible
 
+allOf:
+  - $ref: "ethernet-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - snps,dwmac-4.00
+              - snps,dwmac-4.10a
+              - snps,dwmac-4.20a
+              - snps,dwmac-5.10a
+              - snps,dwxgmac
+              - snps,dwxgmac-2.10
+
+      required:
+        - compatible
+    then:
+      properties:
+        snps,tso:
+          $ref: /schemas/types.yaml#definitions/flag
+          description:
+            Enables the TSO feature otherwise it will be managed by
+            MAC HW capability register.
+
 properties:
 
   # We need to include all the compatibles from schemas that will
@@ -314,34 +338,6 @@ dependencies:
   snps,reset-active-low: ["snps,reset-gpio"]
   snps,reset-delay-us: ["snps,reset-gpio"]
 
-allOf:
-  - $ref: "ethernet-controller.yaml#"
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - allwinner,sun7i-a20-gmac
-              - allwinner,sun8i-a83t-emac
-              - allwinner,sun8i-h3-emac
-              - allwinner,sun8i-r40-emac
-              - allwinner,sun8i-v3s-emac
-              - allwinner,sun50i-a64-emac
-              - snps,dwmac-4.00
-              - snps,dwmac-4.10a
-              - snps,dwmac-4.20a
-              - snps,dwxgmac
-              - snps,dwxgmac-2.10
-              - st,spear600-gmac
-
-    then:
-      properties:
-        snps,tso:
-          $ref: /schemas/types.yaml#definitions/flag
-          description:
-            Enables the TSO feature otherwise it will be managed by
-            MAC HW capability register.
-
 additionalProperties: true
 
 examples:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (2 preceding siblings ...)
  2020-12-14  9:15 ` [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-14 14:30   ` Rob Herring
  2020-12-14  9:15 ` [PATCH 05/25] dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description Serge Semin
                   ` (20 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Currently the "snps,axi-config", "snps,mtl-rx-config" and
"snps,mtl-tx-config" properties are declared as a single phandle reference
to a node with corresponding parameters defined. That's not good for
several reasons. First of all scattering around a device tree some
particular device-specific configs with no visual relation to that device
isn't suitable from maintainability point of view. That leads to a
disturbed representation of the actual device tree mixing actual device
nodes and some vendor-specific configs. Secondly using the same configs
set for several device nodes doesn't represent well the devices structure,
since the interfaces these configs describe in hardware belong to
different devices and may actually differ. In the later case having the
configs node separated from the corresponding device nodes gets to be
even unjustified.

So instead of having a separate DW *MAC configs nodes we suggest to
define them as sub-nodes of the device nodes, which interfaces they
actually describe. By doing so we'll make the DW *MAC nodes visually
correct describing all the aspects of the IP-core configuration. Thus
we'll be able to describe the configs sub-nodes bindings right in the
snps,dwmac.yaml file.

Note the former "snps,axi-config", "snps,mtl-rx-config" and
"snps,mtl-tx-config" bindings have been marked as deprecated.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

---

Note the current DT schema tool requires the vendor-specific properties to be
defined in accordance with the schema: dtschema/meta-schemas/vendor-props.yaml
It means the property can be;
- boolean,
- string,
- defined with $ref and additional constraints,
- defined with allOf: [ $ref ] and additional constraints.

The modification provided by this commit needs to extend that definition to
make the DT schema tool correctly parse this schema. That is we need to let
the vendors-specific properties to also accept the oneOf-based combined
sub-schema. Like this:

--- a/dtschema/meta-schemas/vendor-props.yaml
+++ b/dtschema/meta-schemas/vendor-props.yaml
@@ -48,15 +48,24 @@
       - properties:   # A property with a type and additional constraints
           $ref:
             pattern: "types.yaml#[\/]{0,1}definitions\/.*"
-          allOf:
-            items:
-              - properties:
+
+        if:
+          not:
+            required:
+              - $ref
+        then:
+          patternProperties:
+            "^(all|one)Of$":
+              contains:
+                properties:
                   $ref:
                     pattern: "types.yaml#[\/]{0,1}definitions\/.*"
                 required:
                   - $ref
-        oneOf:
+
+        anyOf:
           - required: [ $ref ]
           - required: [ allOf ]
+          - required: [ oneOf ]

 ...
---
 .../devicetree/bindings/net/snps,dwmac.yaml   | 380 +++++++++++++-----
 1 file changed, 288 insertions(+), 92 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 0dd543c6c08e..44aa88151cba 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -150,69 +150,251 @@ properties:
       in a different mode than the PHY in order to function.
 
   snps,axi-config:
-    $ref: /schemas/types.yaml#definitions/phandle
-    description:
-      AXI BUS Mode parameters. Phandle to a node that can contain the
-      following properties
-        * snps,lpi_en, enable Low Power Interface
-        * snps,xit_frm, unlock on WoL
-        * snps,wr_osr_lmt, max write outstanding req. limit
-        * snps,rd_osr_lmt, max read outstanding req. limit
-        * snps,kbbe, do not cross 1KiB boundary.
-        * snps,blen, this is a vector of supported burst length.
-        * snps,fb, fixed-burst
-        * snps,mb, mixed-burst
-        * snps,rb, rebuild INCRx Burst
+    description: AXI BUS Mode parameters
+    oneOf:
+      - deprecated: true
+        $ref: /schemas/types.yaml#definitions/phandle
+      - type: object
+        properties:
+          snps,lpi_en:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: Enable Low Power Interface
+
+          snps,xit_frm:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: Unlock on WoL
+
+          snps,wr_osr_lmt:
+            $ref: /schemas/types.yaml#definitions/uint32
+            description: Max write outstanding req. limit
+            default: 1
+            minimum: 0
+            maximum: 15
+
+          snps,rd_osr_lmt:
+            $ref: /schemas/types.yaml#definitions/uint32
+            description: Max read outstanding req. limit
+            default: 1
+            minimum: 0
+            maximum: 15
+
+          snps,kbbe:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: Do not cross 1KiB boundary
+
+          snps,blen:
+            $ref: /schemas/types.yaml#definitions/uint32-array
+            description: A vector of supported burst lengths
+            minItems: 7
+            maxItems: 7
+            items:
+              enum: [256, 128, 64, 32, 16, 8, 4, 0]
+
+          snps,fb:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: Fixed-burst
+
+          snps,mb:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: Mixed-burst
+
+          snps,rb:
+            $ref: /schemas/types.yaml#definitions/flag
+            description: Rebuild INCRx Burst
+
+        additionalProperties: false
 
   snps,mtl-rx-config:
-    $ref: /schemas/types.yaml#definitions/phandle
     description:
-      Multiple RX Queues parameters. Phandle to a node that can
-      contain the following properties
-        * snps,rx-queues-to-use, number of RX queues to be used in the
-          driver
-        * Choose one of these RX scheduling algorithms
-          * snps,rx-sched-sp, Strict priority
-          * snps,rx-sched-wsp, Weighted Strict priority
-        * For each RX queue
-          * Choose one of these modes
-            * snps,dcb-algorithm, Queue to be enabled as DCB
-            * snps,avb-algorithm, Queue to be enabled as AVB
-          * snps,map-to-dma-channel, Channel to map
-          * Specifiy specific packet routing
-            * snps,route-avcp, AV Untagged Control packets
-            * snps,route-ptp, PTP Packets
-            * snps,route-dcbcp, DCB Control Packets
-            * snps,route-up, Untagged Packets
-            * snps,route-multi-broad, Multicast & Broadcast Packets
-          * snps,priority, RX queue priority (Range 0x0 to 0xF)
+      Multiple RX Queues parameters
+    oneOf:
+      - deprecated: true
+        $ref: /schemas/types.yaml#definitions/phandle
+      - type: object
+        properties:
+          snps,rx-queues-to-use:
+            $ref: /schemas/types.yaml#definitions/uint32
+            description: Number of RX queues to be used in the driver
+            default: 1
+            minimum: 1
+
+        patternProperties:
+          "^snps,rx-sched-(sp|wsp)$":
+            $ref: /schemas/types.yaml#definitions/flag
+            description: Strict/Weighted Strict RX scheduling priority
+
+          "^queue[0-9]$":
+            type: object
+            description: Each RX Queue parameters
+
+            properties:
+              snps,map-to-dma-channel:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: DMA channel to map
+
+              snps,priority:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: RX queue priority
+                minimum: 0
+                maximum: 15
+
+            patternProperties:
+              "^snps,(dcb|avb)-algorithm$":
+                $ref: /schemas/types.yaml#definitions/flag
+                description: Enable Queue as DCB/AVB
+
+              "^snps,route-(avcp|ptp|dcbcp|up|multi-broad)$":
+                $ref: /schemas/types.yaml#definitions/flag
+                description:
+                  AV Untagged/PTP/DCB Control/Untagged/Multicast & Broadcast
+                  packets routing respectively.
+
+            additionalProperties: false
+
+            # Choose only one of the Queue modes and the packets routing
+            allOf:
+              - not:
+                  required:
+                    - snps,dcb-algorithm
+                    - snps,avb-algorithm
+              - oneOf:
+                  - required:
+                      - snps,route-avcp
+                  - required:
+                      - snps,route-ptp
+                  - required:
+                      - snps,route-dcbcp
+                  - required:
+                      - snps,route-up
+                  - required:
+                      - snps,route-multi-broad
+                  - not:
+                      anyOf:
+                        - required:
+                            - snps,route-avcp
+                        - required:
+                            - snps,route-ptp
+                        - required:
+                            - snps,route-dcbcp
+                        - required:
+                            - snps,route-up
+                        - required:
+                            - snps,route-multi-broad
+
+        additionalProperties: false
+
+        # Choose one of the RX scheduling algorithms
+        not:
+          required:
+            - snps,rx-sched-sp
+            - snps,rx-sched-wsp
 
   snps,mtl-tx-config:
-    $ref: /schemas/types.yaml#definitions/phandle
     description:
-      Multiple TX Queues parameters. Phandle to a node that can
-      contain the following properties
-        * snps,tx-queues-to-use, number of TX queues to be used in the
-          driver
-        * Choose one of these TX scheduling algorithms
-          * snps,tx-sched-wrr, Weighted Round Robin
-          * snps,tx-sched-wfq, Weighted Fair Queuing
-          * snps,tx-sched-dwrr, Deficit Weighted Round Robin
-          * snps,tx-sched-sp, Strict priority
-        * For each TX queue
-          * snps,weight, TX queue weight (if using a DCB weight
-            algorithm)
-          * Choose one of these modes
-            * snps,dcb-algorithm, TX queue will be working in DCB
-            * snps,avb-algorithm, TX queue will be working in AVB
-              [Attention] Queue 0 is reserved for legacy traffic
-                          and so no AVB is available in this queue.
-          * Configure Credit Base Shaper (if AVB Mode selected)
-            * snps,send_slope, enable Low Power Interface
-            * snps,idle_slope, unlock on WoL
-            * snps,high_credit, max write outstanding req. limit
-            * snps,low_credit, max read outstanding req. limit
-          * snps,priority, TX queue priority (Range 0x0 to 0xF)
+      Multiple TX Queues parameters
+    oneOf:
+      - deprecated: true
+        $ref: /schemas/types.yaml#definitions/phandle
+      - type: object
+        properties:
+          snps,tx-queues-to-use:
+            $ref: /schemas/types.yaml#definitions/uint32
+            description: Number of TX queues to be used in the driver
+            default: 1
+            minimum: 1
+
+        patternProperties:
+          "^snps,tx-sched-(wrr|wfq|dwrr|sp)$":
+            $ref: /schemas/types.yaml#definitions/flag
+            description:
+              Weighted Round Robin, Weighted Fair Queuing,
+              Deficit Weighted Round Robin or Strict TX scheduling priority.
+
+          "^queue[0-9]$":
+            type: object
+            description: Each TX Queue parameters
+
+            properties:
+              snps,priority:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: TX queue priority
+                minimum: 0
+                maximum: 15
+
+              snps,weight:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: TX queue weight (if using a DCB weight algorithm)
+                minimum: 0
+                maximum: 0x1FFFFF
+
+              snps,send_slope:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: Enable Low Power Interface
+                minimum: 0
+                maximum: 0x3FFF
+
+              snps,idle_slope:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: Unlock on WoL
+                minimum: 0
+                maximum: 0x1FFFFF
+
+              snps,high_credit:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: Max write outstanding req. limit
+                minimum: 0
+                maximum: 0x1FFFFFFF
+
+              snps,low_credit:
+                $ref: /schemas/types.yaml#definitions/uint32
+                description: Max read outstanding req. limit
+                minimum: 0
+                maximum: 0x1FFFFFFF
+
+            patternProperties:
+              "^snps,(dcb|avb)-algorithm$":
+                $ref: /schemas/types.yaml#definitions/flag
+                description:
+                  Enable Queue as DCB/AVB. Note Queue 0 is reserved for legacy
+                  traffic and so no AVB is available in this queue.
+
+            additionalProperties: false
+
+            # Choose only one of the Queue modes
+            not:
+              required:
+                - snps,dcb-algorithm
+                - snps,avb-algorithm
+
+            # Credit Base Shaper is configurable for AVB Mode only
+            dependencies:
+              snps,send_slope: ["snps,avb-algorithm"]
+              snps,idle_slope: ["snps,avb-algorithm"]
+              snps,high_credit: ["snps,avb-algorithm"]
+              snps,low_credit: ["snps,avb-algorithm"]
+
+        additionalProperties: false
+
+        # Choose one of the TX scheduling algorithms
+        oneOf:
+          - required:
+              - snps,tx-sched-wrr
+          - required:
+              - snps,tx-sched-wfq
+          - required:
+              - snps,tx-sched-dwrr
+          - required:
+              - snps,tx-sched-sp
+          - not:
+              anyOf:
+                - required:
+                    - snps,tx-sched-wrr
+                - required:
+                    - snps,tx-sched-wfq
+                - required:
+                    - snps,tx-sched-dwrr
+                - required:
+                    - snps,tx-sched-sp
 
   snps,reset-gpio:
     deprecated: true
@@ -342,41 +524,6 @@ additionalProperties: true
 
 examples:
   - |
-    stmmac_axi_setup: stmmac-axi-config {
-        snps,wr_osr_lmt = <0xf>;
-        snps,rd_osr_lmt = <0xf>;
-        snps,blen = <256 128 64 32 0 0 0>;
-    };
-
-    mtl_rx_setup: rx-queues-config {
-        snps,rx-queues-to-use = <1>;
-        snps,rx-sched-sp;
-        queue0 {
-            snps,dcb-algorithm;
-            snps,map-to-dma-channel = <0x0>;
-            snps,priority = <0x0>;
-        };
-    };
-
-    mtl_tx_setup: tx-queues-config {
-        snps,tx-queues-to-use = <2>;
-        snps,tx-sched-wrr;
-        queue0 {
-            snps,weight = <0x10>;
-            snps,dcb-algorithm;
-            snps,priority = <0x0>;
-        };
-
-        queue1 {
-            snps,avb-algorithm;
-            snps,send_slope = <0x1000>;
-            snps,idle_slope = <0x1000>;
-            snps,high_credit = <0x3E800>;
-            snps,low_credit = <0xFFC18000>;
-            snps,priority = <0x1>;
-        };
-    };
-
     gmac0: ethernet@e0800000 {
         compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
         reg = <0xe0800000 0x8000>;
@@ -404,6 +551,55 @@ examples:
             };
         };
     };
+  - |
+    gmac1: ethernet@f8010000 {
+        compatible = "snps,dwmac-4.10a", "snps,dwmac";
+        reg = <0xf8010000 0x4000>;
+        interrupts = <0 98 4>;
+        interrupt-names = "macirq";
+        clock-names = "stmmaceth", "ptp_ref";
+        clocks = <&clock 4>, <&clock 5>;
+        phy-mode = "rgmii";
+        snps,txpbl = <8>;
+        snps,rxpbl = <2>;
+        snps,aal;
+        snps,tso;
+
+        snps,axi-config {
+            snps,wr_osr_lmt = <0xf>;
+            snps,rd_osr_lmt = <0xf>;
+            snps,blen = <256 128 64 32 0 0 0>;
+        };
+
+        snps,mtl-rx-config {
+            snps,rx-queues-to-use = <1>;
+            snps,rx-sched-sp;
+            queue0 {
+               snps,dcb-algorithm;
+               snps,map-to-dma-channel = <0x0>;
+               snps,priority = <0x0>;
+            };
+        };
+
+        snps,mtl-tx-config {
+            snps,tx-queues-to-use = <2>;
+            snps,tx-sched-wrr;
+            queue0 {
+                snps,weight = <0x10>;
+                snps,dcb-algorithm;
+                snps,priority = <0x0>;
+            };
+
+            queue1 {
+                snps,avb-algorithm;
+                snps,send_slope = <0x1000>;
+                snps,idle_slope = <0x1000>;
+                snps,high_credit = <0x3E800>;
+                snps,low_credit = <0xFFC18000>;
+                snps,priority = <0x1>;
+            };
+        };
+    };
 
 # FIXME: We should set it, but it would report all the generic
 # properties as additional properties.
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/25] dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (3 preceding siblings ...)
  2020-12-14  9:15 ` [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-15 17:30   ` Rob Herring
  2020-12-14  9:15 ` [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources Serge Semin
                   ` (19 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Current clocks description doesn't provide a comprehensive notion about
what "stmmaceth" and "pclk" actually represent from the IP-core manual
point of view. The bindings file states:
stmmaceth - "GMAC main clock",
apb - "Peripheral registers interface clock".
It isn't that easy to understand what they actually mean especially seeing
the DW *MAC manual operates with clock definitions like Application,
System, Host, CSR, Transmit, Receive, etc clocks. Moreover the clocks
usage in the driver doesn't shade a full light on their essence. What
inferred from there is that the "stmmaceth" name has been assigned to the
common clock, which feeds both system and CSR interfaces. But what about
"apb"? The bindings defines it as the clock for "peripheral registers
interface". So it's close to the CSR clock in the IP-core manual notation.
If so then when "apb" clock is specified aside with the "stmmaceth", it
represents a case when the DW *MAC is synthesized with CSR_SLV_CLK=y
(separate system and CSR clocks). But even though the "apb" clock is
requested in the MAC driver, the driver doesn't actually use it as a
separate CSR clock where the IP-core manual requires. All of that makes me
thinking that the case of separate system and CSR clocks isn't correctly
implemented in the driver.

Let's start with elaborating the clocks description so anyone reading
the DW *MAC bindings file would understand that "stmmaceth" is the
system clock and "pclk" is actually the CSR clock. Indeed in accordance
with sheets depicted in [1]:
system/application clock can be either of: hclk_i, aclk_i, clk_app_i;
CSR clock can be either of: hclk_i, aclk_i, clk_app_i, clk_csr_i.
(Most likely the similar definitions present in the others IP-core
manuals.) So the CSR clock can be tied to the application clock
considering the later as the main clock, but not the other way around. In
case if there is only "stmmaceth" clock specified in a DT node, then it
will be considered as a source of clocks for both application and CSR. But
if "pclk" is also specified in the list of the device clocks, then it will
be perceived as the separate CSR clock.

[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
    October 2013, p. 564.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../devicetree/bindings/net/snps,dwmac.yaml          | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 44aa88151cba..e1ebe5c8b1da 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -116,8 +116,16 @@ properties:
     maxItems: 5
     additionalItems: true
     items:
-      - description: GMAC main clock
-      - description: Peripheral registers interface clock
+      - description:
+          GMAC main clock, also called as system/application clock.
+          This clock is used to provide a periodic signal for the DMA/MTL
+          interface and optionally for CSR, if the later isn't separately
+          clocked.
+      - description:
+          Peripheral registers interface clock, also called as CSR clock.
+          MCI, CSR and SMA interfaces run on this clock. If it's omitted,
+          the CSR interfaces are considered as synchronous to the system
+          clock domain.
       - description:
           PTP reference clock. This clock is used for programming the
           Timestamp Addend Register. If not passed then the system
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (4 preceding siblings ...)
  2020-12-14  9:15 ` [PATCH 05/25] dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-15 17:32   ` Rob Herring
  2020-12-15 17:32   ` Rob Herring
  2020-12-14  9:15 ` [PATCH 07/25] dt-bindings: net: dwmac: Detach Generic DW MAC bindings Serge Semin
                   ` (18 subsequent siblings)
  24 siblings, 2 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Generic DW *MAC can be connected to an external Tramit and Receive clock
generators. Add the corresponding clocks description and clock-names to
the generic bindings schema so new DW *MAC-based bindings wouldn't declare
its own names of the same clocks.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../devicetree/bindings/net/snps,dwmac.yaml        | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e1ebe5c8b1da..74820f491346 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -126,6 +126,18 @@ properties:
           MCI, CSR and SMA interfaces run on this clock. If it's omitted,
           the CSR interfaces are considered as synchronous to the system
           clock domain.
+      - description:
+          GMAC Tx clock or so called Transmit clock. The clock is supplied
+          by an external with respect to the DW MAC clock generator.
+          The clock source and its frequency depends on the DW MAC xMII mode.
+          In case if it's supplied by PHY/SerDes this property can be
+          omitted.
+      - description:
+          GMAC Rx clock or so called Receive clock. The clock is supplied
+          by an external with respect to the DW MAC clock generator.
+          The clock source and its frequency depends on the DW MAC xMII mode.
+          In case if it's supplied by PHY/SerDes or it's synchronous to
+          the Tx clock this property can be omitted.
       - description:
           PTP reference clock. This clock is used for programming the
           Timestamp Addend Register. If not passed then the system
@@ -139,6 +151,8 @@ properties:
       enum:
         - stmmaceth
         - pclk
+        - tx
+        - rx
         - ptp_ref
 
   resets:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/25] dt-bindings: net: dwmac: Detach Generic DW MAC bindings
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (5 preceding siblings ...)
  2020-12-14  9:15 ` [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-15 17:50   ` Rob Herring
  2020-12-14  9:15 ` [PATCH 08/25] net: stmmac: Add snps,*-config sub-nodes support Serge Semin
                   ` (17 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Currently the snps,dwmac.yaml DT bindings file is used for both DT nodes
describing generic DW MAC devices and as DT schema with common properties
to be evaluated against a vendor-specific DW MAC IP-cores. Due to such
dual-purpose design the "compatible" property of the common DW MAC schema
needs to contain the vendor-specific strings to successfully pass the
schema evaluation in case if it's referenced from the vendor-specific DT
bindings. That's a bad design from maintainability point of view, since
adding/removing any DW MAC-based device bindings requires the common
schema modification. In order to fix that let's detach the schema which
provides the generic DW MAC DT nodes evaluation into a dedicated DT
bindings file preserving the common DW MAC properties declaration in the
snps,dwmac.yaml file. By doing so we'll still provide a common properties
evaluation for each vendor-specific MAC bindings which refer to the
common bindings file, while the generic DW MAC DT nodes will be checked
against the new snps,dwmac-generic.yaml DT schema.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../bindings/net/snps,dwmac-generic.yaml      | 148 ++++++++++++++++++
 .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------
 2 files changed, 149 insertions(+), 138 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
new file mode 100644
index 000000000000..f1b387911390
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/snps,dwmac-generic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare Generic MAC Device Tree Bindings
+
+maintainers:
+  - Alexandre Torgue <alexandre.torgue@st.com>
+  - Giuseppe Cavallaro <peppe.cavallaro@st.com>
+  - Jose Abreu <joabreu@synopsys.com>
+
+# Select the DT nodes, which have got compatible strings either as just a
+# single string with IP-core name optionally followed by the IP version or
+# two strings: one with IP-core name plus the IP version, another as just
+# the IP-core name.
+select:
+  properties:
+    compatible:
+      oneOf:
+        - items:
+            - pattern: "^snps,dw(xg)+mac(-[0-9]+\\.[0-9]+a?)?$"
+        - items:
+            - pattern: "^snps,dwmac-[0-9]+\\.[0-9]+a?$"
+            - const: snps,dwmac
+        - items:
+            - pattern: "^snps,dwxgmac-[0-9]+\\.[0-9]+a?$"
+            - const: snps,dwxgmac
+
+  required:
+    - compatible
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - description: Generic Synopsys DW MAC
+        oneOf:
+          - items:
+              - enum:
+                  - snps,dwmac-3.50a
+                  - snps,dwmac-3.610
+                  - snps,dwmac-3.70a
+                  - snps,dwmac-3.710
+                  - snps,dwmac-4.00
+                  - snps,dwmac-4.10a
+                  - snps,dwmac-4.20a
+              - const: snps,dwmac
+          - const: snps,dwmac
+      - description: Generic Synopsys DW xGMAC
+        oneOf:
+          - items:
+              - enum:
+                  - snps,dwxgmac-2.10
+              - const: snps,dwxgmac
+          - const: snps,dwxgmac
+      - description: ST SPEAr SoC Family GMAC
+        deprecated: true
+        const: st,spear600-gmac
+
+  reg:
+    maxItems: 1
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    gmac0: ethernet@e0800000 {
+      compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
+      reg = <0xe0800000 0x8000>;
+      interrupt-parent = <&vic1>;
+      interrupts = <24 23 22>;
+      interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+      mac-address = [000000000000]; /* Filled in by U-Boot */
+      max-frame-size = <3800>;
+      phy-mode = "gmii";
+      snps,multicast-filter-bins = <256>;
+      snps,perfect-filter-entries = <128>;
+      rx-fifo-depth = <16384>;
+      tx-fifo-depth = <16384>;
+      clocks = <&clock>;
+      clock-names = "stmmaceth";
+      snps,axi-config = <&stmmac_axi_setup>;
+      snps,mtl-rx-config = <&mtl_rx_setup>;
+      snps,mtl-tx-config = <&mtl_tx_setup>;
+      mdio0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "snps,dwmac-mdio";
+        phy1: ethernet-phy@0 {
+          reg = <0>;
+        };
+      };
+    };
+  - |
+    gmac1: ethernet@f8010000 {
+      compatible = "snps,dwmac-4.10a", "snps,dwmac";
+      reg = <0xf8010000 0x4000>;
+      interrupts = <0 98 4>;
+      interrupt-names = "macirq";
+      clock-names = "stmmaceth", "ptp_ref";
+      clocks = <&clock 4>, <&clock 5>;
+      phy-mode = "rgmii";
+      snps,txpbl = <8>;
+      snps,rxpbl = <2>;
+      snps,aal;
+      snps,tso;
+
+      snps,axi-config {
+        snps,wr_osr_lmt = <0xf>;
+        snps,rd_osr_lmt = <0xf>;
+        snps,blen = <256 128 64 32 0 0 0>;
+      };
+
+      snps,mtl-rx-config {
+        snps,rx-queues-to-use = <1>;
+        snps,rx-sched-sp;
+        queue0 {
+          snps,dcb-algorithm;
+          snps,map-to-dma-channel = <0x0>;
+          snps,priority = <0x0>;
+        };
+      };
+
+      snps,mtl-tx-config {
+        snps,tx-queues-to-use = <2>;
+        snps,tx-sched-wrr;
+
+        queue0 {
+          snps,weight = <0x10>;
+          snps,dcb-algorithm;
+          snps,priority = <0x0>;
+        };
+
+        queue1 {
+          snps,avb-algorithm;
+          snps,send_slope = <0x1000>;
+          snps,idle_slope = <0x1000>;
+          snps,high_credit = <0x3E800>;
+          snps,low_credit = <0x1FC18000>;
+          snps,priority = <0x1>;
+        };
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 74820f491346..72b58f86bc41 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -11,31 +11,7 @@ maintainers:
   - Giuseppe Cavallaro <peppe.cavallaro@st.com>
   - Jose Abreu <joabreu@synopsys.com>
 
-# 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
-select:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - snps,dwmac
-          - snps,dwmac-3.50a
-          - snps,dwmac-3.610
-          - snps,dwmac-3.70a
-          - snps,dwmac-3.710
-          - snps,dwmac-4.00
-          - snps,dwmac-4.10a
-          - snps,dwmac-4.20a
-          - snps,dwxgmac
-          - snps,dwxgmac-2.10
-
-          # Deprecated
-          - st,spear600-gmac
-
-  required:
-    - compatible
+select: false
 
 allOf:
   - $ref: "ethernet-controller.yaml#"
@@ -62,35 +38,6 @@ allOf:
             MAC HW capability register.
 
 properties:
-
-  # We need to include all the compatibles from schemas that will
-  # include that schemas, otherwise compatible won't validate for
-  # those.
-  compatible:
-    contains:
-      enum:
-        - allwinner,sun7i-a20-gmac
-        - allwinner,sun8i-a83t-emac
-        - allwinner,sun8i-h3-emac
-        - allwinner,sun8i-r40-emac
-        - allwinner,sun8i-v3s-emac
-        - allwinner,sun50i-a64-emac
-        - amlogic,meson6-dwmac
-        - amlogic,meson8b-dwmac
-        - amlogic,meson8m2-dwmac
-        - amlogic,meson-gxbb-dwmac
-        - amlogic,meson-axg-dwmac
-        - snps,dwmac
-        - snps,dwmac-3.50a
-        - snps,dwmac-3.610
-        - snps,dwmac-3.70a
-        - snps,dwmac-3.710
-        - snps,dwmac-4.00
-        - snps,dwmac-4.10a
-        - snps,dwmac-4.20a
-        - snps,dwxgmac
-        - snps,dwxgmac-2.10
-
   reg:
     minItems: 1
     maxItems: 2
@@ -543,88 +490,4 @@ dependencies:
   snps,reset-delay-us: ["snps,reset-gpio"]
 
 additionalProperties: true
-
-examples:
-  - |
-    gmac0: ethernet@e0800000 {
-        compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
-        reg = <0xe0800000 0x8000>;
-        interrupt-parent = <&vic1>;
-        interrupts = <24 23 22>;
-        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
-        mac-address = [000000000000]; /* Filled in by U-Boot */
-        max-frame-size = <3800>;
-        phy-mode = "gmii";
-        snps,multicast-filter-bins = <256>;
-        snps,perfect-filter-entries = <128>;
-        rx-fifo-depth = <16384>;
-        tx-fifo-depth = <16384>;
-        clocks = <&clock>;
-        clock-names = "stmmaceth";
-        snps,axi-config = <&stmmac_axi_setup>;
-        snps,mtl-rx-config = <&mtl_rx_setup>;
-        snps,mtl-tx-config = <&mtl_tx_setup>;
-        mdio0 {
-            #address-cells = <1>;
-            #size-cells = <0>;
-            compatible = "snps,dwmac-mdio";
-            phy1: ethernet-phy@0 {
-                reg = <0>;
-            };
-        };
-    };
-  - |
-    gmac1: ethernet@f8010000 {
-        compatible = "snps,dwmac-4.10a", "snps,dwmac";
-        reg = <0xf8010000 0x4000>;
-        interrupts = <0 98 4>;
-        interrupt-names = "macirq";
-        clock-names = "stmmaceth", "ptp_ref";
-        clocks = <&clock 4>, <&clock 5>;
-        phy-mode = "rgmii";
-        snps,txpbl = <8>;
-        snps,rxpbl = <2>;
-        snps,aal;
-        snps,tso;
-
-        snps,axi-config {
-            snps,wr_osr_lmt = <0xf>;
-            snps,rd_osr_lmt = <0xf>;
-            snps,blen = <256 128 64 32 0 0 0>;
-        };
-
-        snps,mtl-rx-config {
-            snps,rx-queues-to-use = <1>;
-            snps,rx-sched-sp;
-            queue0 {
-               snps,dcb-algorithm;
-               snps,map-to-dma-channel = <0x0>;
-               snps,priority = <0x0>;
-            };
-        };
-
-        snps,mtl-tx-config {
-            snps,tx-queues-to-use = <2>;
-            snps,tx-sched-wrr;
-            queue0 {
-                snps,weight = <0x10>;
-                snps,dcb-algorithm;
-                snps,priority = <0x0>;
-            };
-
-            queue1 {
-                snps,avb-algorithm;
-                snps,send_slope = <0x1000>;
-                snps,idle_slope = <0x1000>;
-                snps,high_credit = <0x3E800>;
-                snps,low_credit = <0xFFC18000>;
-                snps,priority = <0x1>;
-            };
-        };
-    };
-
-# FIXME: We should set it, but it would report all the generic
-# properties as additional properties.
-# additionalProperties: false
-
 ...
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/25] net: stmmac: Add snps,*-config sub-nodes support
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (6 preceding siblings ...)
  2020-12-14  9:15 ` [PATCH 07/25] dt-bindings: net: dwmac: Detach Generic DW MAC bindings Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-14  9:15 ` [PATCH 09/25] net: stmmac: dwmac-rk: Cleanup STMMAC DT-config in remove cb Serge Semin
                   ` (16 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Currently the "snps,axi-config", "snps,mtl-rx-config" and
"snps,mtl-tx-config" DT node properties are marked as deprecated when
being defined as a phandle reference to a node with parameters. The new
way of defining the DW MAC interfaces config is to add an eponymous
sub-nodes to the DW MAC device DT node. Make sure the STMMAC driver
supports it.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index af34a4cadbb0..b4720e477d90 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -95,7 +95,8 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
 	struct device_node *np;
 	struct stmmac_axi *axi;
 
-	np = of_parse_phandle(pdev->dev.of_node, "snps,axi-config", 0);
+	np = of_parse_phandle(pdev->dev.of_node, "snps,axi-config", 0) ?:
+	     of_get_child_by_name(pdev->dev.of_node, "snps,axi-config");
 	if (!np)
 		return NULL;
 
@@ -150,11 +151,13 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
 	plat->rx_queues_cfg[0].mode_to_use = MTL_QUEUE_DCB;
 	plat->tx_queues_cfg[0].mode_to_use = MTL_QUEUE_DCB;
 
-	rx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-rx-config", 0);
+	rx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-rx-config", 0) ?:
+		  of_get_child_by_name(pdev->dev.of_node, "snps,mtl-rx-config");
 	if (!rx_node)
 		return ret;
 
-	tx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-tx-config", 0);
+	tx_node = of_parse_phandle(pdev->dev.of_node, "snps,mtl-tx-config", 0) ?:
+		  of_get_child_by_name(pdev->dev.of_node, "snps,mtl-tx-config");
 	if (!tx_node) {
 		of_node_put(rx_node);
 		return ret;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/25] net: stmmac: dwmac-rk: Cleanup STMMAC DT-config in remove cb
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (7 preceding siblings ...)
  2020-12-14  9:15 ` [PATCH 08/25] net: stmmac: Add snps,*-config sub-nodes support Serge Semin
@ 2020-12-14  9:15 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 10/25] net: stmmac: dwmac-sti: " Serge Semin
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:15 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

The stmmac_remove_config_dt() method needs to be called on the device
remove procedure otherwise for at least some of device-nodes will be left
requested.

Fixes: d2ed0a7755fe ("net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 6ef30252bfe0..01c10ca80f1a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1433,11 +1433,14 @@ static int rk_gmac_probe(struct platform_device *pdev)
 
 static int rk_gmac_remove(struct platform_device *pdev)
 {
+	struct stmmac_priv *priv = netdev_priv(platform_get_drvdata(pdev));
 	struct rk_priv_data *bsp_priv = get_stmmac_bsp_priv(&pdev->dev);
 	int ret = stmmac_dvr_remove(&pdev->dev);
 
 	rk_gmac_powerdown(bsp_priv);
 
+	stmmac_remove_config_dt(pdev, priv->plat);
+
 	return ret;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/25] net: stmmac: dwmac-sti: Cleanup STMMAC DT-config in remove cb
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (8 preceding siblings ...)
  2020-12-14  9:15 ` [PATCH 09/25] net: stmmac: dwmac-rk: Cleanup STMMAC DT-config in remove cb Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 11/25] net: stmmac: dwmac-stm32: " Serge Semin
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

The stmmac_remove_config_dt() method needs to be called on the device
remove procedure otherwise for at least some of device-nodes will be left
requested.

Fixes: d2ed0a7755fe ("net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
index e1b63df6f96f..3454c5eff822 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -370,11 +370,14 @@ static int sti_dwmac_probe(struct platform_device *pdev)
 
 static int sti_dwmac_remove(struct platform_device *pdev)
 {
+	struct stmmac_priv *priv = netdev_priv(platform_get_drvdata(pdev));
 	struct sti_dwmac *dwmac = get_stmmac_bsp_priv(&pdev->dev);
 	int ret = stmmac_dvr_remove(&pdev->dev);
 
 	clk_disable_unprepare(dwmac->clk);
 
+	stmmac_remove_config_dt(pdev, priv->plat);
+
 	return ret;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/25] net: stmmac: dwmac-stm32: Cleanup STMMAC DT-config in remove cb
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (9 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 10/25] net: stmmac: dwmac-sti: " Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 12/25] net: stmmac: Directly call reverse methods in stmmac_probe_config_dt() Serge Semin
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

The stmmac_remove_config_dt() method needs to be called on the device
remove procedure otherwise for at least some of device-nodes will be left
requested.

Fixes: d2ed0a7755fe ("net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 5d4df4c5254e..b45aab38c7b0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -426,6 +426,8 @@ static int stm32_dwmac_remove(struct platform_device *pdev)
 
 	stm32_dwmac_clk_disable(priv->plat->bsp_priv);
 
+	stmmac_remove_config_dt(pdev, priv->plat);
+
 	if (dwmac->irq_pwr_wakeup >= 0) {
 		dev_pm_clear_wake_irq(&pdev->dev);
 		device_init_wakeup(&pdev->dev, false);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/25] net: stmmac: Directly call reverse methods in stmmac_probe_config_dt()
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (10 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 11/25] net: stmmac: dwmac-stm32: " Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 13/25] net: stmmac: Fix clocks left enabled on glue-probes failure Serge Semin
                   ` (12 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Calling an antagonistic method from the corresponding protagonist isn't
good from maintainability point of view, since prevents us from directly
adding a functionality in the later, which needs to be reverted in the
former. Since that's what we are about to do in order to fix the commit
573c0b9c4e0 ("stmmac: move stmmac_clk, pclk, clk_ptp_ref and stmmac_rst to
platform structure"), let's replace the stmmac_remove_config_dt() method
invocation in stmmac_probe_config_dt() with direct reversal procedures.

Fixes: f573c0b9c4e0 ("stmmac: move stmmac_clk, pclk, clk_ptp_ref and stmmac_rst to platform structure")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../ethernet/stmicro/stmmac/stmmac_platform.c | 26 ++++++++++++-------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index b4720e477d90..5110545090d2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -457,7 +457,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	/* To Configure PHY by using all device-tree supported properties */
 	rc = stmmac_dt_phy(plat, np, &pdev->dev);
 	if (rc)
-		return ERR_PTR(rc);
+		goto error_dt_phy_parse;
 
 	of_property_read_u32(np, "tx-fifo-depth", &plat->tx_fifo_size);
 
@@ -535,8 +535,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
 			       GFP_KERNEL);
 	if (!dma_cfg) {
-		stmmac_remove_config_dt(pdev, plat);
-		return ERR_PTR(-ENOMEM);
+		rc = -ENOMEM;
+		goto error_dma_cfg_alloc;
 	}
 	plat->dma_cfg = dma_cfg;
 
@@ -563,10 +563,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	plat->axi = stmmac_axi_setup(pdev);
 
 	rc = stmmac_mtl_setup(pdev, plat);
-	if (rc) {
-		stmmac_remove_config_dt(pdev, plat);
-		return ERR_PTR(rc);
-	}
+	if (rc)
+		goto error_dma_cfg_alloc;
 
 	/* clock setup */
 	if (!of_device_is_compatible(np, "snps,dwc-qos-ethernet-4.10")) {
@@ -581,8 +579,10 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 
 	plat->pclk = devm_clk_get(&pdev->dev, "pclk");
 	if (IS_ERR(plat->pclk)) {
-		if (PTR_ERR(plat->pclk) == -EPROBE_DEFER)
+		if (PTR_ERR(plat->pclk) == -EPROBE_DEFER) {
+			rc = PTR_ERR(plat->pclk);
 			goto error_pclk_get;
+		}
 
 		plat->pclk = NULL;
 	}
@@ -602,8 +602,10 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	plat->stmmac_rst = devm_reset_control_get(&pdev->dev,
 						  STMMAC_RESOURCE_NAME);
 	if (IS_ERR(plat->stmmac_rst)) {
-		if (PTR_ERR(plat->stmmac_rst) == -EPROBE_DEFER)
+		if (PTR_ERR(plat->stmmac_rst) == -EPROBE_DEFER) {
+			rc = PTR_ERR(plat->stmmac_rst);
 			goto error_hw_init;
+		}
 
 		dev_info(&pdev->dev, "no reset control found\n");
 		plat->stmmac_rst = NULL;
@@ -615,8 +617,12 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	clk_disable_unprepare(plat->pclk);
 error_pclk_get:
 	clk_disable_unprepare(plat->stmmac_clk);
+error_dma_cfg_alloc:
+	of_node_put(plat->mdio_node);
+error_dt_phy_parse:
+	of_node_put(plat->phy_node);
 
-	return ERR_PTR(-EPROBE_DEFER);
+	return ERR_PTR(rc);
 }
 
 /**
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/25] net: stmmac: Fix clocks left enabled on glue-probes failure
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (11 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 12/25] net: stmmac: Directly call reverse methods in stmmac_probe_config_dt() Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 14/25] net: stmmac: Use optional clock request method to get stmmaceth Serge Semin
                   ` (11 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

The generic clocks request and preparation have been moved from
stmmac_dvr_probe()/stmmac_init_ptp() to the stmmac_probe_config_dt()
method in the framework of commit f573c0b9c4e0 ("stmmac: move stmmac_clk,
pclk, clk_ptp_ref and stmmac_rst to platform structure"). At the same time
the clocks disabling and reset assertion have been left in
stmmac_dvr_remove() instead of also being moved to the symmetric
antagonistic method - stmmac_remove_config_dt(). Due to that all the glue
drivers probe cleanup-on-failure paths don't perform the generic clocks
disable/unprepare procedure, which of course is wrong. Fix it by moving
the clocks disable/unprepare methods invocation to the
stmmac_remove_config_dt() function.

Fixes: f573c0b9c4e0 ("stmmac: move stmmac_clk, pclk, clk_ptp_ref and stmmac_rst to platform structure")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c     | 2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 2 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 +++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 81ee0a071b4e..884b8d661442 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -667,6 +667,8 @@ static void intel_eth_pci_remove(struct pci_dev *pdev)
 
 	pci_free_irq_vectors(pdev);
 
+	clk_disable_unprepare(priv->plat->stmmac_clk);
+
 	clk_unregister_fixed_rate(priv->plat->stmmac_clk);
 
 	pcim_iounmap_regions(pdev, BIT(0));
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d833908b660a..13681027dd09 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5103,8 +5103,6 @@ int stmmac_dvr_remove(struct device *dev)
 	phylink_destroy(priv->phylink);
 	if (priv->plat->stmmac_rst)
 		reset_control_assert(priv->plat->stmmac_rst);
-	clk_disable_unprepare(priv->plat->pclk);
-	clk_disable_unprepare(priv->plat->stmmac_clk);
 	if (priv->hw->pcs != STMMAC_PCS_TBI &&
 	    priv->hw->pcs != STMMAC_PCS_RTBI)
 		stmmac_mdio_unregister(ndev);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5110545090d2..56419f511a48 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -630,11 +630,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
  * @pdev: platform_device structure
  * @plat: driver data platform structure
  *
- * Release resources claimed by stmmac_probe_config_dt().
+ * Disable and release resources claimed by stmmac_probe_config_dt().
  */
 void stmmac_remove_config_dt(struct platform_device *pdev,
 			     struct plat_stmmacenet_data *plat)
 {
+	clk_disable_unprepare(plat->pclk);
+	clk_disable_unprepare(plat->stmmac_clk);
 	of_node_put(plat->phy_node);
 	of_node_put(plat->mdio_node);
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/25] net: stmmac: Use optional clock request method to get stmmaceth
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (12 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 13/25] net: stmmac: Fix clocks left enabled on glue-probes failure Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 15/25] net: stmmac: Use optional clock request method to get pclk Serge Semin
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

The "stmmaceth" clock is expected to be optional by the current driver
design, but there are several problems in the implementation. First if the
clock is specified, but failed to be requested due to an internal error or
due to not being ready yet for configuration, then the DT-probe procedure
will just proceed with further initializations. It is erroneous in both
cases. Secondly if we'd use the clock API, which expect the clock being
optional we wouldn't have needed to avoid the clock request procedure for
the "snps,dwc-qos-ethernet-4.10"-compatible devices to prevent the error
message from being printed. All of that can be fixed by using the
devm_clk_get_optional() method here provided by the common clock
framework.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../ethernet/stmicro/stmmac/stmmac_platform.c | 20 ++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 56419f511a48..e79b3e3351a9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -566,17 +566,19 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	if (rc)
 		goto error_dma_cfg_alloc;
 
-	/* clock setup */
-	if (!of_device_is_compatible(np, "snps,dwc-qos-ethernet-4.10")) {
-		plat->stmmac_clk = devm_clk_get(&pdev->dev,
-						STMMAC_RESOURCE_NAME);
-		if (IS_ERR(plat->stmmac_clk)) {
-			dev_warn(&pdev->dev, "Cannot get CSR clock\n");
-			plat->stmmac_clk = NULL;
-		}
-		clk_prepare_enable(plat->stmmac_clk);
+	/* All clocks are optional since the sub-drivers can have a specific
+	 * clocks set and their naming.
+	 */
+	plat->stmmac_clk = devm_clk_get_optional(&pdev->dev,
+						 STMMAC_RESOURCE_NAME);
+	if (IS_ERR(plat->stmmac_clk)) {
+		rc = PTR_ERR(plat->stmmac_clk);
+		dev_err_probe(&pdev->dev, rc, "Cannot get CSR clock\n");
+		goto error_dma_cfg_alloc;
 	}
 
+	clk_prepare_enable(plat->stmmac_clk);
+
 	plat->pclk = devm_clk_get(&pdev->dev, "pclk");
 	if (IS_ERR(plat->pclk)) {
 		if (PTR_ERR(plat->pclk) == -EPROBE_DEFER) {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 15/25] net: stmmac: Use optional clock request method to get pclk
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (13 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 14/25] net: stmmac: Use optional clock request method to get stmmaceth Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 16/25] net: stmmac: Use optional clock request method to get ptp_clk Serge Semin
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Let's replace the manual implementation of the optional "pclk"
functionality with using devm_clk_get_optional(). By doing so we'll
improve the code maintainability, and fix a hidden bug which will cause
problems if the "pclk" clock has been actually passed to the device, but
the clock framework failed to request it.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c    | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index e79b3e3351a9..3809b00d3077 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -579,15 +579,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 
 	clk_prepare_enable(plat->stmmac_clk);
 
-	plat->pclk = devm_clk_get(&pdev->dev, "pclk");
+	plat->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
 	if (IS_ERR(plat->pclk)) {
-		if (PTR_ERR(plat->pclk) == -EPROBE_DEFER) {
-			rc = PTR_ERR(plat->pclk);
-			goto error_pclk_get;
-		}
-
-		plat->pclk = NULL;
+		rc = PTR_ERR(plat->pclk);
+		dev_err_probe(&pdev->dev, rc, "Cannot get CSR clock\n");
+		goto error_pclk_get;
 	}
+
 	clk_prepare_enable(plat->pclk);
 
 	/* Fall-back to main clock in case of no PTP ref is passed */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 16/25] net: stmmac: Use optional clock request method to get ptp_clk
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (14 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 15/25] net: stmmac: Use optional clock request method to get pclk Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 17/25] net: stmmac: Use optional reset control API to work with stmmaceth Serge Semin
                   ` (8 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Let's replace the manual implementation of the optional ptp_clk
functionality with method devm_clk_get_optional() provided by the common
clock kernel framework. First of all it will be better from
maintainability point of view. Secondly by doing so we'll also fix a
potential problem, which will come out if the PTP clock has been actually
specified, but the clock framework failed to request it.

Note since we are switching the code to using the optional common clock
API, then there is no need in checking the clk_ptp_ref pointer for being
not NULL before calling the clk_prepare_enable() method. The later will
correctly handle it. So just discard the conditional statement of
priv->plat->clk_ptp_ref pointer value testing in the stmmac_resume()
method.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 3 +--
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 13681027dd09..e9003684efc8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5230,8 +5230,7 @@ int stmmac_resume(struct device *dev)
 		/* enable the clk previously disabled */
 		clk_prepare_enable(priv->plat->stmmac_clk);
 		clk_prepare_enable(priv->plat->pclk);
-		if (priv->plat->clk_ptp_ref)
-			clk_prepare_enable(priv->plat->clk_ptp_ref);
+		clk_prepare_enable(priv->plat->clk_ptp_ref);
 		/* reset the phy so that it's ready */
 		if (priv->mii)
 			stmmac_mdio_reset(priv->mii);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 3809b00d3077..367d1458d66d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -589,10 +589,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	clk_prepare_enable(plat->pclk);
 
 	/* Fall-back to main clock in case of no PTP ref is passed */
-	plat->clk_ptp_ref = devm_clk_get(&pdev->dev, "ptp_ref");
+	plat->clk_ptp_ref = devm_clk_get_optional(&pdev->dev, "ptp_ref");
 	if (IS_ERR(plat->clk_ptp_ref)) {
+		rc = PTR_ERR(plat->clk_ptp_ref);
+		dev_err_probe(&pdev->dev, rc, "Cannot get PTP clock\n");
+		goto error_hw_init;
+	} else if (!plat->clk_ptp_ref) {
 		plat->clk_ptp_rate = clk_get_rate(plat->stmmac_clk);
-		plat->clk_ptp_ref = NULL;
 		dev_info(&pdev->dev, "PTP uses main clock\n");
 	} else {
 		plat->clk_ptp_rate = clk_get_rate(plat->clk_ptp_ref);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 17/25] net: stmmac: Use optional reset control API to work with stmmaceth
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (15 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 16/25] net: stmmac: Use optional clock request method to get ptp_clk Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 18/25] net: stmmac: dwc-qos: Cleanup STMMAC platform data clock pointers Serge Semin
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Replace the manual implementation of the optional device reset control
functionality with using the devm_reset_control_get_optional() method in
order to improve the code maintainability and fix a potential bug. It
will come out if the reset control handler has been specified, but the
reset framework failed to request it.

Note there is no need in checking the priv->plat->stmmac_rst pointer for
being not NULL in order to perform the reset control assertion/deassertion
because the passed NULL will be considered by the reset framework as
absent optional reset control handler anyway.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++-----------
 .../ethernet/stmicro/stmmac/stmmac_platform.c | 14 +++++---------
 2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e9003684efc8..7f4d54d2fc72 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4889,15 +4889,13 @@ int stmmac_dvr_probe(struct device *device,
 	if ((phyaddr >= 0) && (phyaddr <= 31))
 		priv->plat->phy_addr = phyaddr;
 
-	if (priv->plat->stmmac_rst) {
-		ret = reset_control_assert(priv->plat->stmmac_rst);
-		reset_control_deassert(priv->plat->stmmac_rst);
-		/* Some reset controllers have only reset callback instead of
-		 * assert + deassert callbacks pair.
-		 */
-		if (ret == -ENOTSUPP)
-			reset_control_reset(priv->plat->stmmac_rst);
-	}
+	ret = reset_control_assert(priv->plat->stmmac_rst);
+	reset_control_deassert(priv->plat->stmmac_rst);
+	/* Some reset controllers have only reset callback instead of
+	 * assert + deassert callbacks pair.
+	 */
+	if (ret == -ENOTSUPP)
+		reset_control_reset(priv->plat->stmmac_rst);
 
 	/* Init MAC and get the capabilities */
 	ret = stmmac_hw_init(priv);
@@ -5101,8 +5099,7 @@ int stmmac_dvr_remove(struct device *dev)
 	stmmac_exit_fs(ndev);
 #endif
 	phylink_destroy(priv->phylink);
-	if (priv->plat->stmmac_rst)
-		reset_control_assert(priv->plat->stmmac_rst);
+	reset_control_assert(priv->plat->stmmac_rst);
 	if (priv->hw->pcs != STMMAC_PCS_TBI &&
 	    priv->hw->pcs != STMMAC_PCS_RTBI)
 		stmmac_mdio_unregister(ndev);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 367d1458d66d..38e8836861c4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -602,16 +602,12 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
 	}
 
-	plat->stmmac_rst = devm_reset_control_get(&pdev->dev,
-						  STMMAC_RESOURCE_NAME);
+	plat->stmmac_rst = devm_reset_control_get_optional(&pdev->dev,
+							   STMMAC_RESOURCE_NAME);
 	if (IS_ERR(plat->stmmac_rst)) {
-		if (PTR_ERR(plat->stmmac_rst) == -EPROBE_DEFER) {
-			rc = PTR_ERR(plat->stmmac_rst);
-			goto error_hw_init;
-		}
-
-		dev_info(&pdev->dev, "no reset control found\n");
-		plat->stmmac_rst = NULL;
+		rc = PTR_ERR(plat->stmmac_rst);
+		dev_err_probe(&pdev->dev, rc, "Cannot get reset control\n");
+		goto error_hw_init;
 	}
 
 	return plat;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 18/25] net: stmmac: dwc-qos: Cleanup STMMAC platform data clock pointers
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (16 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 17/25] net: stmmac: Use optional reset control API to work with stmmaceth Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 19/25] net: stmmac: dwc-qos: Use dev_err_probe() for probe errors handling Serge Semin
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

The pointers need to be nullified otherwise the stmmac_remove_config_dt()
method called after them being initialized will disable the clocks. That
then will cause a WARN() backtrace being printed since the clocks would be
also disabled in the locally defined remove method.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../stmicro/stmmac/dwmac-dwc-qos-eth.c        | 42 ++++++++++++++-----
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index 2342d497348e..31ca299a1cfd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -123,39 +123,46 @@ static void *dwc_qos_probe(struct platform_device *pdev,
 			   struct plat_stmmacenet_data *plat_dat,
 			   struct stmmac_resources *stmmac_res)
 {
+	struct clk *clk;
 	int err;
 
-	plat_dat->stmmac_clk = devm_clk_get(&pdev->dev, "apb_pclk");
-	if (IS_ERR(plat_dat->stmmac_clk)) {
+	clk = devm_clk_get(&pdev->dev, "apb_pclk");
+	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "apb_pclk clock not found.\n");
-		return ERR_CAST(plat_dat->stmmac_clk);
+		return ERR_CAST(clk);
 	}
 
-	err = clk_prepare_enable(plat_dat->stmmac_clk);
+	err = clk_prepare_enable(clk);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to enable apb_pclk clock: %d\n",
 			err);
 		return ERR_PTR(err);
 	}
 
-	plat_dat->pclk = devm_clk_get(&pdev->dev, "phy_ref_clk");
-	if (IS_ERR(plat_dat->pclk)) {
+	plat_dat->stmmac_clk = clk;
+
+	clk = devm_clk_get(&pdev->dev, "phy_ref_clk");
+	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
-		err = PTR_ERR(plat_dat->pclk);
+		err = PTR_ERR(clk);
 		goto disable;
 	}
 
-	err = clk_prepare_enable(plat_dat->pclk);
+	err = clk_prepare_enable(clk);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to enable phy_ref clock: %d\n",
 			err);
 		goto disable;
 	}
 
+	plat_dat->pclk = clk;
+
 	return NULL;
 
 disable:
 	clk_disable_unprepare(plat_dat->stmmac_clk);
+	plat_dat->stmmac_clk = NULL;
+
 	return ERR_PTR(err);
 }
 
@@ -164,8 +171,15 @@ static int dwc_qos_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
+	/* Cleanup the pointers to the clock handlers hidden in the platform
+	 * data so the stmmac_remove_config_dt() method wouldn't have disabled
+	 * the clocks too.
+	 */
 	clk_disable_unprepare(priv->plat->pclk);
+	priv->plat->pclk = NULL;
+
 	clk_disable_unprepare(priv->plat->stmmac_clk);
+	priv->plat->stmmac_clk = NULL;
 
 	return 0;
 }
@@ -303,12 +317,12 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 		goto disable_master;
 	}
 
-	data->stmmac_clk = eqos->clk_slave;
-
 	err = clk_prepare_enable(eqos->clk_slave);
 	if (err < 0)
 		goto disable_master;
 
+	data->stmmac_clk = eqos->clk_slave;
+
 	eqos->clk_rx = devm_clk_get(&pdev->dev, "rx");
 	if (IS_ERR(eqos->clk_rx)) {
 		err = PTR_ERR(eqos->clk_rx);
@@ -381,6 +395,7 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 	clk_disable_unprepare(eqos->clk_rx);
 disable_slave:
 	clk_disable_unprepare(eqos->clk_slave);
+	data->stmmac_clk = NULL;
 disable_master:
 	clk_disable_unprepare(eqos->clk_master);
 error:
@@ -390,6 +405,7 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 
 static int tegra_eqos_remove(struct platform_device *pdev)
 {
+	struct stmmac_priv *priv = netdev_priv(platform_get_drvdata(pdev));
 	struct tegra_eqos *eqos = get_stmmac_bsp_priv(&pdev->dev);
 
 	reset_control_assert(eqos->rst);
@@ -399,6 +415,12 @@ static int tegra_eqos_remove(struct platform_device *pdev)
 	clk_disable_unprepare(eqos->clk_slave);
 	clk_disable_unprepare(eqos->clk_master);
 
+	/* Cleanup the pointers to the clock handlers hidden in the platform
+	 * data so the stmmac_remove_config_dt() method wouldn't have disabled
+	 * the clocks too.
+	 */
+	priv->plat->stmmac_clk = NULL;
+
 	return 0;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 19/25] net: stmmac: dwc-qos: Use dev_err_probe() for probe errors handling
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (17 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 18/25] net: stmmac: dwc-qos: Cleanup STMMAC platform data clock pointers Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 20/25] net: stmmac: Add Tx/Rx platform clocks support Serge Semin
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel, Anson Huang

There is a very handy dev_err_probe() method to handle the deferred probe
error number. It reduces the code size, uniforms error handling, records
the defer probe reason, etc. Use it to print the probe callback error
message.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index 31ca299a1cfd..57f957898b60 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -473,11 +473,8 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
 	priv = data->probe(pdev, plat_dat, &stmmac_res);
 	if (IS_ERR(priv)) {
 		ret = PTR_ERR(priv);
-
-		if (ret != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "failed to probe subdriver: %d\n",
-				ret);
-
+		dev_err_probe(&pdev->dev, ret, "failed to probe subdriver: %d\n",
+			      ret);
 		goto remove_config;
 	}
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 20/25] net: stmmac: Add Tx/Rx platform clocks support
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (18 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 19/25] net: stmmac: dwc-qos: Use dev_err_probe() for probe errors handling Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 21/25] net: stmmac: dwc-qos: Discard Tx/Rx clocks request Serge Semin
                   ` (4 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Depending on the DW *MAC configuration it can be at least connected to an
external Transmit clock, but in some cases to an external Receive clock
generator. In order to simplify/unify the sub-drivers code and to prevent
having the same clocks named differently add the Tx/Rx clocks support to
the generic STMMAC DT-based platform data initialization method under the
names "tx" and "rx" respectively. The bindings schema has already been
altered in accordance with that.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../ethernet/stmicro/stmmac/stmmac_platform.c | 24 +++++++++++++++++++
 include/linux/stmmac.h                        |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 38e8836861c4..943498d57e3a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -588,6 +588,24 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 
 	clk_prepare_enable(plat->pclk);
 
+	plat->tx_clk = devm_clk_get_optional(&pdev->dev, "tx");
+	if (IS_ERR(plat->tx_clk)) {
+		rc = PTR_ERR(plat->tx_clk);
+		dev_err_probe(&pdev->dev, rc, "Cannot get Tx clock\n");
+		goto error_tx_clk_get;
+	}
+
+	clk_prepare_enable(plat->tx_clk);
+
+	plat->rx_clk = devm_clk_get_optional(&pdev->dev, "rx");
+	if (IS_ERR(plat->rx_clk)) {
+		rc = PTR_ERR(plat->rx_clk);
+		dev_err_probe(&pdev->dev, rc, "Cannot get Rx clock\n");
+		goto error_rx_clk_get;
+	}
+
+	clk_prepare_enable(plat->rx_clk);
+
 	/* Fall-back to main clock in case of no PTP ref is passed */
 	plat->clk_ptp_ref = devm_clk_get_optional(&pdev->dev, "ptp_ref");
 	if (IS_ERR(plat->clk_ptp_ref)) {
@@ -613,6 +631,10 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	return plat;
 
 error_hw_init:
+	clk_disable_unprepare(plat->rx_clk);
+error_rx_clk_get:
+	clk_disable_unprepare(plat->tx_clk);
+error_tx_clk_get:
 	clk_disable_unprepare(plat->pclk);
 error_pclk_get:
 	clk_disable_unprepare(plat->stmmac_clk);
@@ -634,6 +656,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 void stmmac_remove_config_dt(struct platform_device *pdev,
 			     struct plat_stmmacenet_data *plat)
 {
+	clk_disable_unprepare(plat->rx_clk);
+	clk_disable_unprepare(plat->tx_clk);
 	clk_disable_unprepare(plat->pclk);
 	clk_disable_unprepare(plat->stmmac_clk);
 	of_node_put(plat->phy_node);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 628e28903b8b..b75cf13d088c 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -185,6 +185,8 @@ struct plat_stmmacenet_data {
 	void *bsp_priv;
 	struct clk *stmmac_clk;
 	struct clk *pclk;
+	struct clk *tx_clk;
+	struct clk *rx_clk;
 	struct clk *clk_ptp_ref;
 	unsigned int clk_ptp_rate;
 	unsigned int clk_ref_rate;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 21/25] net: stmmac: dwc-qos: Discard Tx/Rx clocks request
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (19 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 20/25] net: stmmac: Add Tx/Rx platform clocks support Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 22/25] net: stmmac: dwmac-imx: Discard Tx clock request Serge Semin
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Since the Tx/Rx clocks with the same names are now requested and
enabled/disabled in the STMMAC DT-based platform config method, there is
no need in duplicating the same procedures in the DWC QoS Eth sub-driver.
Discard it then, but make sure the denoted clocks have been specified
for the platform.

Note also the deprecated clock "phy_ref_clk" have been defined as the Tx
clock in the DWC QoS Eth bindings. Let's use a pointer to the Tx clock
defined in the platform data then instead of the unrelated pclk pointer.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../stmicro/stmmac/dwmac-dwc-qos-eth.c        | 44 +++++--------------
 1 file changed, 11 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index 57f957898b60..f53a78448988 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -31,8 +31,6 @@ struct tegra_eqos {
 	struct reset_control *rst;
 	struct clk *clk_master;
 	struct clk *clk_slave;
-	struct clk *clk_tx;
-	struct clk *clk_rx;
 
 	struct gpio_desc *reset;
 };
@@ -155,7 +153,7 @@ static void *dwc_qos_probe(struct platform_device *pdev,
 		goto disable;
 	}
 
-	plat_dat->pclk = clk;
+	plat_dat->tx_clk = clk;
 
 	return NULL;
 
@@ -175,8 +173,8 @@ static int dwc_qos_remove(struct platform_device *pdev)
 	 * data so the stmmac_remove_config_dt() method wouldn't have disabled
 	 * the clocks too.
 	 */
-	clk_disable_unprepare(priv->plat->pclk);
-	priv->plat->pclk = NULL;
+	clk_disable_unprepare(priv->plat->tx_clk);
+	priv->plat->tx_clk = NULL;
 
 	clk_disable_unprepare(priv->plat->stmmac_clk);
 	priv->plat->stmmac_clk = NULL;
@@ -197,6 +195,7 @@ static int dwc_qos_remove(struct platform_device *pdev)
 static void tegra_eqos_fix_speed(void *priv, unsigned int speed)
 {
 	struct tegra_eqos *eqos = priv;
+	struct stmmac_priv *sp = netdev_priv(dev_get_drvdata(eqos->dev));
 	unsigned long rate = 125000000;
 	bool needs_calibration = false;
 	u32 value;
@@ -262,7 +261,7 @@ static void tegra_eqos_fix_speed(void *priv, unsigned int speed)
 		writel(value, eqos->regs + AUTO_CAL_CONFIG);
 	}
 
-	err = clk_set_rate(eqos->clk_tx, rate);
+	err = clk_set_rate(sp->plat->tx_clk, rate);
 	if (err < 0)
 		dev_err(eqos->dev, "failed to set TX rate: %d\n", err);
 }
@@ -301,6 +300,11 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 	if (!is_of_node(dev->fwnode))
 		goto bypass_clk_reset_gpio;
 
+	if (!data->tx_clk || !data->rx_clk) {
+		err = -EINVAL;
+		goto error;
+	}
+
 	eqos->clk_master = devm_clk_get(&pdev->dev, "master_bus");
 	if (IS_ERR(eqos->clk_master)) {
 		err = PTR_ERR(eqos->clk_master);
@@ -323,30 +327,10 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 
 	data->stmmac_clk = eqos->clk_slave;
 
-	eqos->clk_rx = devm_clk_get(&pdev->dev, "rx");
-	if (IS_ERR(eqos->clk_rx)) {
-		err = PTR_ERR(eqos->clk_rx);
-		goto disable_slave;
-	}
-
-	err = clk_prepare_enable(eqos->clk_rx);
-	if (err < 0)
-		goto disable_slave;
-
-	eqos->clk_tx = devm_clk_get(&pdev->dev, "tx");
-	if (IS_ERR(eqos->clk_tx)) {
-		err = PTR_ERR(eqos->clk_tx);
-		goto disable_rx;
-	}
-
-	err = clk_prepare_enable(eqos->clk_tx);
-	if (err < 0)
-		goto disable_rx;
-
 	eqos->reset = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(eqos->reset)) {
 		err = PTR_ERR(eqos->reset);
-		goto disable_tx;
+		goto disable_slave;
 	}
 
 	usleep_range(2000, 4000);
@@ -389,10 +373,6 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 	reset_control_assert(eqos->rst);
 reset_phy:
 	gpiod_set_value(eqos->reset, 1);
-disable_tx:
-	clk_disable_unprepare(eqos->clk_tx);
-disable_rx:
-	clk_disable_unprepare(eqos->clk_rx);
 disable_slave:
 	clk_disable_unprepare(eqos->clk_slave);
 	data->stmmac_clk = NULL;
@@ -410,8 +390,6 @@ static int tegra_eqos_remove(struct platform_device *pdev)
 
 	reset_control_assert(eqos->rst);
 	gpiod_set_value(eqos->reset, 1);
-	clk_disable_unprepare(eqos->clk_tx);
-	clk_disable_unprepare(eqos->clk_rx);
 	clk_disable_unprepare(eqos->clk_slave);
 	clk_disable_unprepare(eqos->clk_master);
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 22/25] net: stmmac: dwmac-imx: Discard Tx clock request
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (20 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 21/25] net: stmmac: dwc-qos: Discard Tx/Rx clocks request Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 23/25] net: stmmac: Call stmmaceth clock as system clock in warn-message Serge Semin
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Since the Tx clock is now requested and enabled/disabled in the STMMAC
DT-based platform config method, there is no need in duplicating the same
procedures in the DW MAC iMX sub-driver.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../net/ethernet/stmicro/stmmac/dwmac-imx.c   | 21 +++++--------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index efef5476a577..7b4590670b4e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -40,7 +40,6 @@ struct imx_dwmac_ops {
 
 struct imx_priv_data {
 	struct device *dev;
-	struct clk *clk_tx;
 	struct clk *clk_mem;
 	struct regmap *intf_regmap;
 	u32 intf_reg_off;
@@ -104,12 +103,6 @@ static int imx_dwmac_init(struct platform_device *pdev, void *priv)
 		return ret;
 	}
 
-	ret = clk_prepare_enable(dwmac->clk_tx);
-	if (ret) {
-		dev_err(&pdev->dev, "tx clock enable failed\n");
-		goto clk_tx_en_failed;
-	}
-
 	if (dwmac->ops->set_intf_mode) {
 		ret = dwmac->ops->set_intf_mode(plat_dat);
 		if (ret)
@@ -119,8 +112,6 @@ static int imx_dwmac_init(struct platform_device *pdev, void *priv)
 	return 0;
 
 intf_mode_failed:
-	clk_disable_unprepare(dwmac->clk_tx);
-clk_tx_en_failed:
 	clk_disable_unprepare(dwmac->clk_mem);
 	return ret;
 }
@@ -129,7 +120,6 @@ static void imx_dwmac_exit(struct platform_device *pdev, void *priv)
 {
 	struct imx_priv_data *dwmac = priv;
 
-	clk_disable_unprepare(dwmac->clk_tx);
 	clk_disable_unprepare(dwmac->clk_mem);
 }
 
@@ -162,7 +152,7 @@ static void imx_dwmac_fix_speed(void *priv, unsigned int speed)
 		return;
 	}
 
-	err = clk_set_rate(dwmac->clk_tx, rate);
+	err = clk_set_rate(plat_dat->tx_clk, rate);
 	if (err < 0)
 		dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
 }
@@ -176,10 +166,9 @@ imx_dwmac_parse_dt(struct imx_priv_data *dwmac, struct device *dev)
 	if (of_get_property(np, "snps,rmii_refclk_ext", NULL))
 		dwmac->rmii_refclk_ext = true;
 
-	dwmac->clk_tx = devm_clk_get(dev, "tx");
-	if (IS_ERR(dwmac->clk_tx)) {
-		dev_err(dev, "failed to get tx clock\n");
-		return PTR_ERR(dwmac->clk_tx);
+	if (!dwmac->plat_dat->tx_clk) {
+		dev_err(dev, "no tx clock found\n");
+		return -EINVAL;
 	}
 
 	dwmac->clk_mem = NULL;
@@ -239,6 +228,7 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 
 	dwmac->ops = data;
 	dwmac->dev = &pdev->dev;
+	dwmac->plat_dat = plat_dat;
 
 	ret = imx_dwmac_parse_dt(dwmac, &pdev->dev);
 	if (ret) {
@@ -257,7 +247,6 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 	plat_dat->exit = imx_dwmac_exit;
 	plat_dat->fix_mac_speed = imx_dwmac_fix_speed;
 	plat_dat->bsp_priv = dwmac;
-	dwmac->plat_dat = plat_dat;
 
 	ret = imx_dwmac_init(pdev, dwmac);
 	if (ret)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 23/25] net: stmmac: Call stmmaceth clock as system clock in warn-message
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (21 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 22/25] net: stmmac: dwmac-imx: Discard Tx clock request Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 24/25] net: stmmac: Use pclk to set MDC clock frequency Serge Semin
  2020-12-14  9:16 ` [PATCH 25/25] net: stmmac: dwc-qos: Save master/slave clocks in the plat-data Serge Semin
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

By all means of the stmmac_clk clock usage it isn't CSR clock, but the
system or application clock, which in particular cases can be used as a
clock source for the CSR interface. Make sure the warning message
correctly identify the clock.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 943498d57e3a..6e22036eab60 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -573,7 +573,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 						 STMMAC_RESOURCE_NAME);
 	if (IS_ERR(plat->stmmac_clk)) {
 		rc = PTR_ERR(plat->stmmac_clk);
-		dev_err_probe(&pdev->dev, rc, "Cannot get CSR clock\n");
+		dev_err_probe(&pdev->dev, rc, "Cannot get system clock\n");
 		goto error_dma_cfg_alloc;
 	}
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 24/25] net: stmmac: Use pclk to set MDC clock frequency
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (22 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 23/25] net: stmmac: Call stmmaceth clock as system clock in warn-message Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  2020-12-14  9:16 ` [PATCH 25/25] net: stmmac: dwc-qos: Save master/slave clocks in the plat-data Serge Semin
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

In accordance with [1] the MDC clock frequency is supposed to be selected
with respect to the CSR clock frequency. CSR clock can be either tied to
the DW MAC system clock (GMAC main clock) or supplied via a dedicated
clk_csr_i signal. Current MDC clock selection procedure handles the former
case while having no support of the later one. That's wrong for the
devices which have separate system and CSR clocks. Let's fix it by first
trying to get the synchro-signal rate from the "pclk" clock, if it hasn't
been specified then fall-back to the "stmmaceth" clock.

[1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
    October 2013, p. 424.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7f4d54d2fc72..719b00fd2a70 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -206,7 +206,12 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
 {
 	u32 clk_rate;
 
-	clk_rate = clk_get_rate(priv->plat->stmmac_clk);
+	/* If APB clock has been specified then it is supposed to be used
+	 * to select the CSR mode. Otherwise the application clock is the
+	 * source of the periodic signal for the CSR interface.
+	 */
+	clk_rate = clk_get_rate(priv->plat->pclk) ?:
+		   clk_get_rate(priv->plat->stmmac_clk);
 
 	/* Platform provided default clk_csr would be assumed valid
 	 * for all other cases except for the below mentioned ones.
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 25/25] net: stmmac: dwc-qos: Save master/slave clocks in the plat-data
  2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
                   ` (23 preceding siblings ...)
  2020-12-14  9:16 ` [PATCH 24/25] net: stmmac: Use pclk to set MDC clock frequency Serge Semin
@ 2020-12-14  9:16 ` Serge Semin
  24 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-14  9:16 UTC (permalink / raw)
  To: Rob Herring, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Maxime Coquelin
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, netdev, linux-stm32, linux-arm-kernel,
	devicetree, linux-kernel

Currently the "master_bus" clock of the DW QoS Eth controller isn't
preserved in the STMMAC platform data, while the "slave_bus" clock is
assigned to the stmmaceth clock pointer. It isn't correct from the
platform clock bindings point of view. The "stmmaceth" clock is supposed
to be the system clock, which is responsible for clocking the DMA
transfers from/to the controller FIFOs to/from the system memory and the
CSR interface if the later isn't separately clocked. If it's clocked
separately then the STMMAC platform code expects to also have "pclk"
specified. So in order to have the STMMAC platform data properly
initialized we need to set the "master_bus" clock handler to the
"stmmaceth" clock pointer, and the "slave_bus" clock handler to the "pclk"
clock pointer.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
index f53a78448988..b90d7e4c3d4a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -315,6 +315,8 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 	if (err < 0)
 		goto error;
 
+	data->stmmac_clk = eqos->clk_master;
+
 	eqos->clk_slave = devm_clk_get(&pdev->dev, "slave_bus");
 	if (IS_ERR(eqos->clk_slave)) {
 		err = PTR_ERR(eqos->clk_slave);
@@ -325,7 +327,7 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 	if (err < 0)
 		goto disable_master;
 
-	data->stmmac_clk = eqos->clk_slave;
+	data->pclk = eqos->clk_slave;
 
 	eqos->reset = devm_gpiod_get(&pdev->dev, "phy-reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(eqos->reset)) {
@@ -375,9 +377,10 @@ static void *tegra_eqos_probe(struct platform_device *pdev,
 	gpiod_set_value(eqos->reset, 1);
 disable_slave:
 	clk_disable_unprepare(eqos->clk_slave);
-	data->stmmac_clk = NULL;
+	data->pclk = NULL;
 disable_master:
 	clk_disable_unprepare(eqos->clk_master);
+	data->stmmac_clk = NULL;
 error:
 	eqos = ERR_PTR(err);
 	goto out;
@@ -397,6 +400,7 @@ static int tegra_eqos_remove(struct platform_device *pdev)
 	 * data so the stmmac_remove_config_dt() method wouldn't have disabled
 	 * the clocks too.
 	 */
+	priv->plat->pclk = NULL;
 	priv->plat->stmmac_clk = NULL;
 
 	return 0;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties
  2020-12-14  9:15 ` [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties Serge Semin
@ 2020-12-14 14:30   ` Rob Herring
  2020-12-15  8:54     ` Serge Semin
  0 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2020-12-14 14:30 UTC (permalink / raw)
  To: Serge Semin
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Serge Semin, Alexey Malahov,
	Pavel Parkhomenko, Vyacheslav Mitrofanov, Maxime Coquelin,
	netdev, linux-stm32, linux-arm-kernel, devicetree, linux-kernel

On Mon, Dec 14, 2020 at 12:15:54PM +0300, Serge Semin wrote:
> Currently the "snps,axi-config", "snps,mtl-rx-config" and
> "snps,mtl-tx-config" properties are declared as a single phandle reference
> to a node with corresponding parameters defined. That's not good for
> several reasons. First of all scattering around a device tree some
> particular device-specific configs with no visual relation to that device
> isn't suitable from maintainability point of view. That leads to a
> disturbed representation of the actual device tree mixing actual device
> nodes and some vendor-specific configs. Secondly using the same configs
> set for several device nodes doesn't represent well the devices structure,
> since the interfaces these configs describe in hardware belong to
> different devices and may actually differ. In the later case having the
> configs node separated from the corresponding device nodes gets to be
> even unjustified.
> 
> So instead of having a separate DW *MAC configs nodes we suggest to
> define them as sub-nodes of the device nodes, which interfaces they
> actually describe. By doing so we'll make the DW *MAC nodes visually
> correct describing all the aspects of the IP-core configuration. Thus
> we'll be able to describe the configs sub-nodes bindings right in the
> snps,dwmac.yaml file.
> 
> Note the former "snps,axi-config", "snps,mtl-rx-config" and
> "snps,mtl-tx-config" bindings have been marked as deprecated.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> ---
> 
> Note the current DT schema tool requires the vendor-specific properties to be
> defined in accordance with the schema: dtschema/meta-schemas/vendor-props.yaml
> It means the property can be;
> - boolean,
> - string,
> - defined with $ref and additional constraints,
> - defined with allOf: [ $ref ] and additional constraints.
> 
> The modification provided by this commit needs to extend that definition to
> make the DT schema tool correctly parse this schema. That is we need to let
> the vendors-specific properties to also accept the oneOf-based combined
> sub-schema. Like this:
> 
> --- a/dtschema/meta-schemas/vendor-props.yaml
> +++ b/dtschema/meta-schemas/vendor-props.yaml
> @@ -48,15 +48,24 @@
>        - properties:   # A property with a type and additional constraints
>            $ref:
>              pattern: "types.yaml#[\/]{0,1}definitions\/.*"
> -          allOf:
> -            items:
> -              - properties:
> +
> +        if:
> +          not:
> +            required:
> +              - $ref
> +        then:
> +          patternProperties:
> +            "^(all|one)Of$":
> +              contains:
> +                properties:
>                    $ref:
>                      pattern: "types.yaml#[\/]{0,1}definitions\/.*"
>                  required:
>                    - $ref
> -        oneOf:
> +
> +        anyOf:
>            - required: [ $ref ]
>            - required: [ allOf ]
> +          - required: [ oneOf ]
> 
>  ...
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   | 380 +++++++++++++-----
>  1 file changed, 288 insertions(+), 92 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 0dd543c6c08e..44aa88151cba 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -150,69 +150,251 @@ properties:
>        in a different mode than the PHY in order to function.
>  
>    snps,axi-config:
> -    $ref: /schemas/types.yaml#definitions/phandle
> -    description:
> -      AXI BUS Mode parameters. Phandle to a node that can contain the
> -      following properties
> -        * snps,lpi_en, enable Low Power Interface
> -        * snps,xit_frm, unlock on WoL
> -        * snps,wr_osr_lmt, max write outstanding req. limit
> -        * snps,rd_osr_lmt, max read outstanding req. limit
> -        * snps,kbbe, do not cross 1KiB boundary.
> -        * snps,blen, this is a vector of supported burst length.
> -        * snps,fb, fixed-burst
> -        * snps,mb, mixed-burst
> -        * snps,rb, rebuild INCRx Burst
> +    description: AXI BUS Mode parameters
> +    oneOf:
> +      - deprecated: true
> +        $ref: /schemas/types.yaml#definitions/phandle
> +      - type: object
> +        properties:

Anywhere have have the same node/property string meaning 2 different 
things is a pain, let's not create another one. Just define a new node 
'axi-config'. Or just put all the properties into the node directly. 
Grouping them has little purpose.

> +          snps,lpi_en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: Enable Low Power Interface
> +
> +          snps,xit_frm:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: Unlock on WoL
> +
> +          snps,wr_osr_lmt:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            description: Max write outstanding req. limit
> +            default: 1
> +            minimum: 0
> +            maximum: 15
> +
> +          snps,rd_osr_lmt:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            description: Max read outstanding req. limit
> +            default: 1
> +            minimum: 0
> +            maximum: 15
> +
> +          snps,kbbe:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: Do not cross 1KiB boundary
> +
> +          snps,blen:
> +            $ref: /schemas/types.yaml#definitions/uint32-array
> +            description: A vector of supported burst lengths
> +            minItems: 7
> +            maxItems: 7
> +            items:
> +              enum: [256, 128, 64, 32, 16, 8, 4, 0]
> +
> +          snps,fb:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: Fixed-burst
> +
> +          snps,mb:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: Mixed-burst
> +
> +          snps,rb:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: Rebuild INCRx Burst
> +
> +        additionalProperties: false
>  
>    snps,mtl-rx-config:
> -    $ref: /schemas/types.yaml#definitions/phandle
>      description:
> -      Multiple RX Queues parameters. Phandle to a node that can
> -      contain the following properties
> -        * snps,rx-queues-to-use, number of RX queues to be used in the
> -          driver
> -        * Choose one of these RX scheduling algorithms
> -          * snps,rx-sched-sp, Strict priority
> -          * snps,rx-sched-wsp, Weighted Strict priority
> -        * For each RX queue
> -          * Choose one of these modes
> -            * snps,dcb-algorithm, Queue to be enabled as DCB
> -            * snps,avb-algorithm, Queue to be enabled as AVB
> -          * snps,map-to-dma-channel, Channel to map
> -          * Specifiy specific packet routing
> -            * snps,route-avcp, AV Untagged Control packets
> -            * snps,route-ptp, PTP Packets
> -            * snps,route-dcbcp, DCB Control Packets
> -            * snps,route-up, Untagged Packets
> -            * snps,route-multi-broad, Multicast & Broadcast Packets
> -          * snps,priority, RX queue priority (Range 0x0 to 0xF)
> +      Multiple RX Queues parameters
> +    oneOf:
> +      - deprecated: true
> +        $ref: /schemas/types.yaml#definitions/phandle
> +      - type: object
> +        properties:
> +          snps,rx-queues-to-use:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            description: Number of RX queues to be used in the driver
> +            default: 1
> +            minimum: 1
> +
> +        patternProperties:
> +          "^snps,rx-sched-(sp|wsp)$":
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: Strict/Weighted Strict RX scheduling priority
> +
> +          "^queue[0-9]$":
> +            type: object
> +            description: Each RX Queue parameters
> +
> +            properties:
> +              snps,map-to-dma-channel:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: DMA channel to map
> +
> +              snps,priority:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: RX queue priority
> +                minimum: 0
> +                maximum: 15
> +
> +            patternProperties:
> +              "^snps,(dcb|avb)-algorithm$":
> +                $ref: /schemas/types.yaml#definitions/flag
> +                description: Enable Queue as DCB/AVB
> +
> +              "^snps,route-(avcp|ptp|dcbcp|up|multi-broad)$":
> +                $ref: /schemas/types.yaml#definitions/flag
> +                description:
> +                  AV Untagged/PTP/DCB Control/Untagged/Multicast & Broadcast
> +                  packets routing respectively.
> +
> +            additionalProperties: false
> +
> +            # Choose only one of the Queue modes and the packets routing
> +            allOf:
> +              - not:
> +                  required:
> +                    - snps,dcb-algorithm
> +                    - snps,avb-algorithm
> +              - oneOf:
> +                  - required:
> +                      - snps,route-avcp
> +                  - required:
> +                      - snps,route-ptp
> +                  - required:
> +                      - snps,route-dcbcp
> +                  - required:
> +                      - snps,route-up
> +                  - required:
> +                      - snps,route-multi-broad
> +                  - not:
> +                      anyOf:
> +                        - required:
> +                            - snps,route-avcp
> +                        - required:
> +                            - snps,route-ptp
> +                        - required:
> +                            - snps,route-dcbcp
> +                        - required:
> +                            - snps,route-up
> +                        - required:
> +                            - snps,route-multi-broad
> +
> +        additionalProperties: false
> +
> +        # Choose one of the RX scheduling algorithms
> +        not:
> +          required:
> +            - snps,rx-sched-sp
> +            - snps,rx-sched-wsp
>  
>    snps,mtl-tx-config:
> -    $ref: /schemas/types.yaml#definitions/phandle
>      description:
> -      Multiple TX Queues parameters. Phandle to a node that can
> -      contain the following properties
> -        * snps,tx-queues-to-use, number of TX queues to be used in the
> -          driver
> -        * Choose one of these TX scheduling algorithms
> -          * snps,tx-sched-wrr, Weighted Round Robin
> -          * snps,tx-sched-wfq, Weighted Fair Queuing
> -          * snps,tx-sched-dwrr, Deficit Weighted Round Robin
> -          * snps,tx-sched-sp, Strict priority
> -        * For each TX queue
> -          * snps,weight, TX queue weight (if using a DCB weight
> -            algorithm)
> -          * Choose one of these modes
> -            * snps,dcb-algorithm, TX queue will be working in DCB
> -            * snps,avb-algorithm, TX queue will be working in AVB
> -              [Attention] Queue 0 is reserved for legacy traffic
> -                          and so no AVB is available in this queue.
> -          * Configure Credit Base Shaper (if AVB Mode selected)
> -            * snps,send_slope, enable Low Power Interface
> -            * snps,idle_slope, unlock on WoL
> -            * snps,high_credit, max write outstanding req. limit
> -            * snps,low_credit, max read outstanding req. limit
> -          * snps,priority, TX queue priority (Range 0x0 to 0xF)
> +      Multiple TX Queues parameters
> +    oneOf:
> +      - deprecated: true
> +        $ref: /schemas/types.yaml#definitions/phandle
> +      - type: object
> +        properties:
> +          snps,tx-queues-to-use:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            description: Number of TX queues to be used in the driver
> +            default: 1
> +            minimum: 1
> +
> +        patternProperties:
> +          "^snps,tx-sched-(wrr|wfq|dwrr|sp)$":
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description:
> +              Weighted Round Robin, Weighted Fair Queuing,
> +              Deficit Weighted Round Robin or Strict TX scheduling priority.
> +
> +          "^queue[0-9]$":
> +            type: object
> +            description: Each TX Queue parameters
> +
> +            properties:
> +              snps,priority:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: TX queue priority
> +                minimum: 0
> +                maximum: 15
> +
> +              snps,weight:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: TX queue weight (if using a DCB weight algorithm)
> +                minimum: 0
> +                maximum: 0x1FFFFF
> +
> +              snps,send_slope:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: Enable Low Power Interface
> +                minimum: 0
> +                maximum: 0x3FFF
> +
> +              snps,idle_slope:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: Unlock on WoL
> +                minimum: 0
> +                maximum: 0x1FFFFF
> +
> +              snps,high_credit:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: Max write outstanding req. limit
> +                minimum: 0
> +                maximum: 0x1FFFFFFF
> +
> +              snps,low_credit:
> +                $ref: /schemas/types.yaml#definitions/uint32
> +                description: Max read outstanding req. limit
> +                minimum: 0
> +                maximum: 0x1FFFFFFF
> +
> +            patternProperties:
> +              "^snps,(dcb|avb)-algorithm$":
> +                $ref: /schemas/types.yaml#definitions/flag
> +                description:
> +                  Enable Queue as DCB/AVB. Note Queue 0 is reserved for legacy
> +                  traffic and so no AVB is available in this queue.
> +
> +            additionalProperties: false
> +
> +            # Choose only one of the Queue modes
> +            not:
> +              required:
> +                - snps,dcb-algorithm
> +                - snps,avb-algorithm
> +
> +            # Credit Base Shaper is configurable for AVB Mode only
> +            dependencies:
> +              snps,send_slope: ["snps,avb-algorithm"]
> +              snps,idle_slope: ["snps,avb-algorithm"]
> +              snps,high_credit: ["snps,avb-algorithm"]
> +              snps,low_credit: ["snps,avb-algorithm"]
> +
> +        additionalProperties: false
> +
> +        # Choose one of the TX scheduling algorithms
> +        oneOf:
> +          - required:
> +              - snps,tx-sched-wrr
> +          - required:
> +              - snps,tx-sched-wfq
> +          - required:
> +              - snps,tx-sched-dwrr
> +          - required:
> +              - snps,tx-sched-sp
> +          - not:
> +              anyOf:
> +                - required:
> +                    - snps,tx-sched-wrr
> +                - required:
> +                    - snps,tx-sched-wfq
> +                - required:
> +                    - snps,tx-sched-dwrr
> +                - required:
> +                    - snps,tx-sched-sp
>  
>    snps,reset-gpio:
>      deprecated: true
> @@ -342,41 +524,6 @@ additionalProperties: true
>  
>  examples:
>    - |
> -    stmmac_axi_setup: stmmac-axi-config {
> -        snps,wr_osr_lmt = <0xf>;
> -        snps,rd_osr_lmt = <0xf>;
> -        snps,blen = <256 128 64 32 0 0 0>;
> -    };
> -
> -    mtl_rx_setup: rx-queues-config {
> -        snps,rx-queues-to-use = <1>;
> -        snps,rx-sched-sp;
> -        queue0 {
> -            snps,dcb-algorithm;
> -            snps,map-to-dma-channel = <0x0>;
> -            snps,priority = <0x0>;
> -        };
> -    };
> -
> -    mtl_tx_setup: tx-queues-config {
> -        snps,tx-queues-to-use = <2>;
> -        snps,tx-sched-wrr;
> -        queue0 {
> -            snps,weight = <0x10>;
> -            snps,dcb-algorithm;
> -            snps,priority = <0x0>;
> -        };
> -
> -        queue1 {
> -            snps,avb-algorithm;
> -            snps,send_slope = <0x1000>;
> -            snps,idle_slope = <0x1000>;
> -            snps,high_credit = <0x3E800>;
> -            snps,low_credit = <0xFFC18000>;
> -            snps,priority = <0x1>;
> -        };
> -    };
> -
>      gmac0: ethernet@e0800000 {
>          compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
>          reg = <0xe0800000 0x8000>;
> @@ -404,6 +551,55 @@ examples:
>              };
>          };
>      };
> +  - |
> +    gmac1: ethernet@f8010000 {
> +        compatible = "snps,dwmac-4.10a", "snps,dwmac";
> +        reg = <0xf8010000 0x4000>;
> +        interrupts = <0 98 4>;
> +        interrupt-names = "macirq";
> +        clock-names = "stmmaceth", "ptp_ref";
> +        clocks = <&clock 4>, <&clock 5>;
> +        phy-mode = "rgmii";
> +        snps,txpbl = <8>;
> +        snps,rxpbl = <2>;
> +        snps,aal;
> +        snps,tso;
> +
> +        snps,axi-config {
> +            snps,wr_osr_lmt = <0xf>;
> +            snps,rd_osr_lmt = <0xf>;
> +            snps,blen = <256 128 64 32 0 0 0>;
> +        };
> +
> +        snps,mtl-rx-config {
> +            snps,rx-queues-to-use = <1>;
> +            snps,rx-sched-sp;
> +            queue0 {
> +               snps,dcb-algorithm;
> +               snps,map-to-dma-channel = <0x0>;
> +               snps,priority = <0x0>;
> +            };
> +        };
> +
> +        snps,mtl-tx-config {
> +            snps,tx-queues-to-use = <2>;
> +            snps,tx-sched-wrr;
> +            queue0 {
> +                snps,weight = <0x10>;
> +                snps,dcb-algorithm;
> +                snps,priority = <0x0>;
> +            };
> +
> +            queue1 {
> +                snps,avb-algorithm;
> +                snps,send_slope = <0x1000>;
> +                snps,idle_slope = <0x1000>;
> +                snps,high_credit = <0x3E800>;
> +                snps,low_credit = <0xFFC18000>;
> +                snps,priority = <0x1>;
> +            };
> +        };
> +    };
>  
>  # FIXME: We should set it, but it would report all the generic
>  # properties as additional properties.
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties
  2020-12-14 14:30   ` Rob Herring
@ 2020-12-15  8:54     ` Serge Semin
  2020-12-15 14:08       ` Rob Herring
  0 siblings, 1 reply; 40+ messages in thread
From: Serge Semin @ 2020-12-15  8:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

Hello Rob,

On Mon, Dec 14, 2020 at 08:30:06AM -0600, Rob Herring wrote:
> On Mon, Dec 14, 2020 at 12:15:54PM +0300, Serge Semin wrote:
> > Currently the "snps,axi-config", "snps,mtl-rx-config" and
> > "snps,mtl-tx-config" properties are declared as a single phandle reference
> > to a node with corresponding parameters defined. That's not good for
> > several reasons. First of all scattering around a device tree some
> > particular device-specific configs with no visual relation to that device
> > isn't suitable from maintainability point of view. That leads to a
> > disturbed representation of the actual device tree mixing actual device
> > nodes and some vendor-specific configs. Secondly using the same configs
> > set for several device nodes doesn't represent well the devices structure,
> > since the interfaces these configs describe in hardware belong to
> > different devices and may actually differ. In the later case having the
> > configs node separated from the corresponding device nodes gets to be
> > even unjustified.
> > 
> > So instead of having a separate DW *MAC configs nodes we suggest to
> > define them as sub-nodes of the device nodes, which interfaces they
> > actually describe. By doing so we'll make the DW *MAC nodes visually
> > correct describing all the aspects of the IP-core configuration. Thus
> > we'll be able to describe the configs sub-nodes bindings right in the
> > snps,dwmac.yaml file.
> > 
> > Note the former "snps,axi-config", "snps,mtl-rx-config" and
> > "snps,mtl-tx-config" bindings have been marked as deprecated.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > ---
> > 
> > Note the current DT schema tool requires the vendor-specific properties to be
> > defined in accordance with the schema: dtschema/meta-schemas/vendor-props.yaml
> > It means the property can be;
> > - boolean,
> > - string,
> > - defined with $ref and additional constraints,
> > - defined with allOf: [ $ref ] and additional constraints.
> > 
> > The modification provided by this commit needs to extend that definition to
> > make the DT schema tool correctly parse this schema. That is we need to let
> > the vendors-specific properties to also accept the oneOf-based combined
> > sub-schema. Like this:
> > 
> > --- a/dtschema/meta-schemas/vendor-props.yaml
> > +++ b/dtschema/meta-schemas/vendor-props.yaml
> > @@ -48,15 +48,24 @@
> >        - properties:   # A property with a type and additional constraints
> >            $ref:
> >              pattern: "types.yaml#[\/]{0,1}definitions\/.*"
> > -          allOf:
> > -            items:
> > -              - properties:
> > +
> > +        if:
> > +          not:
> > +            required:
> > +              - $ref
> > +        then:
> > +          patternProperties:
> > +            "^(all|one)Of$":
> > +              contains:
> > +                properties:
> >                    $ref:
> >                      pattern: "types.yaml#[\/]{0,1}definitions\/.*"
> >                  required:
> >                    - $ref
> > -        oneOf:
> > +
> > +        anyOf:
> >            - required: [ $ref ]
> >            - required: [ allOf ]
> > +          - required: [ oneOf ]
> > 
> >  ...
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 380 +++++++++++++-----
> >  1 file changed, 288 insertions(+), 92 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index 0dd543c6c08e..44aa88151cba 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -150,69 +150,251 @@ properties:
> >        in a different mode than the PHY in order to function.
> >  
> >    snps,axi-config:
> > -    $ref: /schemas/types.yaml#definitions/phandle
> > -    description:
> > -      AXI BUS Mode parameters. Phandle to a node that can contain the
> > -      following properties
> > -        * snps,lpi_en, enable Low Power Interface
> > -        * snps,xit_frm, unlock on WoL
> > -        * snps,wr_osr_lmt, max write outstanding req. limit
> > -        * snps,rd_osr_lmt, max read outstanding req. limit
> > -        * snps,kbbe, do not cross 1KiB boundary.
> > -        * snps,blen, this is a vector of supported burst length.
> > -        * snps,fb, fixed-burst
> > -        * snps,mb, mixed-burst
> > -        * snps,rb, rebuild INCRx Burst
> > +    description: AXI BUS Mode parameters
> > +    oneOf:
> > +      - deprecated: true
> > +        $ref: /schemas/types.yaml#definitions/phandle
> > +      - type: object
> > +        properties:
> 

> Anywhere have have the same node/property string meaning 2 different 
> things is a pain, let's not create another one. 

IIUC you meant that having a node and property with the same name
isn't ok. Right? If so could you explain why not? especially seeing
the property is expected to be set with phandle reference to that
node. That seemed like a perfect solution to me. We wouldn't need to
introduce a new property/node name, but just deprecate the
corresponding name to be a property.

> Just define a new node 
> 'axi-config'. Or just put all the properties into the node directly. 
> Grouping them has little purpose.

Hm, you suggest to remove the vendor prefix, right? If so what about
the rest of the changes introduced here in this patch? They concern
"snps,mtl-tx-config" and "snps,mtl-rx-config" properties (please note
these changes are a bit more complicated than once connected with
"snps,axi-config"). Should I remove the vendor-prefix from them too?
Anyway that seems a bit questionable, because all the "snps,*-config"
properties/nodes seems more vendor-specific than generic. Am I wrong
in that matter?

If you think they are generic, then the "{axi,mtl-rx,mtl-tx}-config"
nodes most likely should be described in the dedicated DT schema...

-Sergey

> 
> > +          snps,lpi_en:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: Enable Low Power Interface
> > +
> > +          snps,xit_frm:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: Unlock on WoL
> > +
> > +          snps,wr_osr_lmt:
> > +            $ref: /schemas/types.yaml#definitions/uint32
> > +            description: Max write outstanding req. limit
> > +            default: 1
> > +            minimum: 0
> > +            maximum: 15
> > +
> > +          snps,rd_osr_lmt:
> > +            $ref: /schemas/types.yaml#definitions/uint32
> > +            description: Max read outstanding req. limit
> > +            default: 1
> > +            minimum: 0
> > +            maximum: 15
> > +
> > +          snps,kbbe:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: Do not cross 1KiB boundary
> > +
> > +          snps,blen:
> > +            $ref: /schemas/types.yaml#definitions/uint32-array
> > +            description: A vector of supported burst lengths
> > +            minItems: 7
> > +            maxItems: 7
> > +            items:
> > +              enum: [256, 128, 64, 32, 16, 8, 4, 0]
> > +
> > +          snps,fb:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: Fixed-burst
> > +
> > +          snps,mb:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: Mixed-burst
> > +
> > +          snps,rb:
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: Rebuild INCRx Burst
> > +
> > +        additionalProperties: false
> >  
> >    snps,mtl-rx-config:
> > -    $ref: /schemas/types.yaml#definitions/phandle
> >      description:
> > -      Multiple RX Queues parameters. Phandle to a node that can
> > -      contain the following properties
> > -        * snps,rx-queues-to-use, number of RX queues to be used in the
> > -          driver
> > -        * Choose one of these RX scheduling algorithms
> > -          * snps,rx-sched-sp, Strict priority
> > -          * snps,rx-sched-wsp, Weighted Strict priority
> > -        * For each RX queue
> > -          * Choose one of these modes
> > -            * snps,dcb-algorithm, Queue to be enabled as DCB
> > -            * snps,avb-algorithm, Queue to be enabled as AVB
> > -          * snps,map-to-dma-channel, Channel to map
> > -          * Specifiy specific packet routing
> > -            * snps,route-avcp, AV Untagged Control packets
> > -            * snps,route-ptp, PTP Packets
> > -            * snps,route-dcbcp, DCB Control Packets
> > -            * snps,route-up, Untagged Packets
> > -            * snps,route-multi-broad, Multicast & Broadcast Packets
> > -          * snps,priority, RX queue priority (Range 0x0 to 0xF)
> > +      Multiple RX Queues parameters
> > +    oneOf:
> > +      - deprecated: true
> > +        $ref: /schemas/types.yaml#definitions/phandle
> > +      - type: object
> > +        properties:
> > +          snps,rx-queues-to-use:
> > +            $ref: /schemas/types.yaml#definitions/uint32
> > +            description: Number of RX queues to be used in the driver
> > +            default: 1
> > +            minimum: 1
> > +
> > +        patternProperties:
> > +          "^snps,rx-sched-(sp|wsp)$":
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description: Strict/Weighted Strict RX scheduling priority
> > +
> > +          "^queue[0-9]$":
> > +            type: object
> > +            description: Each RX Queue parameters
> > +
> > +            properties:
> > +              snps,map-to-dma-channel:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: DMA channel to map
> > +
> > +              snps,priority:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: RX queue priority
> > +                minimum: 0
> > +                maximum: 15
> > +
> > +            patternProperties:
> > +              "^snps,(dcb|avb)-algorithm$":
> > +                $ref: /schemas/types.yaml#definitions/flag
> > +                description: Enable Queue as DCB/AVB
> > +
> > +              "^snps,route-(avcp|ptp|dcbcp|up|multi-broad)$":
> > +                $ref: /schemas/types.yaml#definitions/flag
> > +                description:
> > +                  AV Untagged/PTP/DCB Control/Untagged/Multicast & Broadcast
> > +                  packets routing respectively.
> > +
> > +            additionalProperties: false
> > +
> > +            # Choose only one of the Queue modes and the packets routing
> > +            allOf:
> > +              - not:
> > +                  required:
> > +                    - snps,dcb-algorithm
> > +                    - snps,avb-algorithm
> > +              - oneOf:
> > +                  - required:
> > +                      - snps,route-avcp
> > +                  - required:
> > +                      - snps,route-ptp
> > +                  - required:
> > +                      - snps,route-dcbcp
> > +                  - required:
> > +                      - snps,route-up
> > +                  - required:
> > +                      - snps,route-multi-broad
> > +                  - not:
> > +                      anyOf:
> > +                        - required:
> > +                            - snps,route-avcp
> > +                        - required:
> > +                            - snps,route-ptp
> > +                        - required:
> > +                            - snps,route-dcbcp
> > +                        - required:
> > +                            - snps,route-up
> > +                        - required:
> > +                            - snps,route-multi-broad
> > +
> > +        additionalProperties: false
> > +
> > +        # Choose one of the RX scheduling algorithms
> > +        not:
> > +          required:
> > +            - snps,rx-sched-sp
> > +            - snps,rx-sched-wsp
> >  
> >    snps,mtl-tx-config:
> > -    $ref: /schemas/types.yaml#definitions/phandle
> >      description:
> > -      Multiple TX Queues parameters. Phandle to a node that can
> > -      contain the following properties
> > -        * snps,tx-queues-to-use, number of TX queues to be used in the
> > -          driver
> > -        * Choose one of these TX scheduling algorithms
> > -          * snps,tx-sched-wrr, Weighted Round Robin
> > -          * snps,tx-sched-wfq, Weighted Fair Queuing
> > -          * snps,tx-sched-dwrr, Deficit Weighted Round Robin
> > -          * snps,tx-sched-sp, Strict priority
> > -        * For each TX queue
> > -          * snps,weight, TX queue weight (if using a DCB weight
> > -            algorithm)
> > -          * Choose one of these modes
> > -            * snps,dcb-algorithm, TX queue will be working in DCB
> > -            * snps,avb-algorithm, TX queue will be working in AVB
> > -              [Attention] Queue 0 is reserved for legacy traffic
> > -                          and so no AVB is available in this queue.
> > -          * Configure Credit Base Shaper (if AVB Mode selected)
> > -            * snps,send_slope, enable Low Power Interface
> > -            * snps,idle_slope, unlock on WoL
> > -            * snps,high_credit, max write outstanding req. limit
> > -            * snps,low_credit, max read outstanding req. limit
> > -          * snps,priority, TX queue priority (Range 0x0 to 0xF)
> > +      Multiple TX Queues parameters
> > +    oneOf:
> > +      - deprecated: true
> > +        $ref: /schemas/types.yaml#definitions/phandle
> > +      - type: object
> > +        properties:
> > +          snps,tx-queues-to-use:
> > +            $ref: /schemas/types.yaml#definitions/uint32
> > +            description: Number of TX queues to be used in the driver
> > +            default: 1
> > +            minimum: 1
> > +
> > +        patternProperties:
> > +          "^snps,tx-sched-(wrr|wfq|dwrr|sp)$":
> > +            $ref: /schemas/types.yaml#definitions/flag
> > +            description:
> > +              Weighted Round Robin, Weighted Fair Queuing,
> > +              Deficit Weighted Round Robin or Strict TX scheduling priority.
> > +
> > +          "^queue[0-9]$":
> > +            type: object
> > +            description: Each TX Queue parameters
> > +
> > +            properties:
> > +              snps,priority:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: TX queue priority
> > +                minimum: 0
> > +                maximum: 15
> > +
> > +              snps,weight:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: TX queue weight (if using a DCB weight algorithm)
> > +                minimum: 0
> > +                maximum: 0x1FFFFF
> > +
> > +              snps,send_slope:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: Enable Low Power Interface
> > +                minimum: 0
> > +                maximum: 0x3FFF
> > +
> > +              snps,idle_slope:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: Unlock on WoL
> > +                minimum: 0
> > +                maximum: 0x1FFFFF
> > +
> > +              snps,high_credit:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: Max write outstanding req. limit
> > +                minimum: 0
> > +                maximum: 0x1FFFFFFF
> > +
> > +              snps,low_credit:
> > +                $ref: /schemas/types.yaml#definitions/uint32
> > +                description: Max read outstanding req. limit
> > +                minimum: 0
> > +                maximum: 0x1FFFFFFF
> > +
> > +            patternProperties:
> > +              "^snps,(dcb|avb)-algorithm$":
> > +                $ref: /schemas/types.yaml#definitions/flag
> > +                description:
> > +                  Enable Queue as DCB/AVB. Note Queue 0 is reserved for legacy
> > +                  traffic and so no AVB is available in this queue.
> > +
> > +            additionalProperties: false
> > +
> > +            # Choose only one of the Queue modes
> > +            not:
> > +              required:
> > +                - snps,dcb-algorithm
> > +                - snps,avb-algorithm
> > +
> > +            # Credit Base Shaper is configurable for AVB Mode only
> > +            dependencies:
> > +              snps,send_slope: ["snps,avb-algorithm"]
> > +              snps,idle_slope: ["snps,avb-algorithm"]
> > +              snps,high_credit: ["snps,avb-algorithm"]
> > +              snps,low_credit: ["snps,avb-algorithm"]
> > +
> > +        additionalProperties: false
> > +
> > +        # Choose one of the TX scheduling algorithms
> > +        oneOf:
> > +          - required:
> > +              - snps,tx-sched-wrr
> > +          - required:
> > +              - snps,tx-sched-wfq
> > +          - required:
> > +              - snps,tx-sched-dwrr
> > +          - required:
> > +              - snps,tx-sched-sp
> > +          - not:
> > +              anyOf:
> > +                - required:
> > +                    - snps,tx-sched-wrr
> > +                - required:
> > +                    - snps,tx-sched-wfq
> > +                - required:
> > +                    - snps,tx-sched-dwrr
> > +                - required:
> > +                    - snps,tx-sched-sp
> >  
> >    snps,reset-gpio:
> >      deprecated: true
> > @@ -342,41 +524,6 @@ additionalProperties: true
> >  
> >  examples:
> >    - |
> > -    stmmac_axi_setup: stmmac-axi-config {
> > -        snps,wr_osr_lmt = <0xf>;
> > -        snps,rd_osr_lmt = <0xf>;
> > -        snps,blen = <256 128 64 32 0 0 0>;
> > -    };
> > -
> > -    mtl_rx_setup: rx-queues-config {
> > -        snps,rx-queues-to-use = <1>;
> > -        snps,rx-sched-sp;
> > -        queue0 {
> > -            snps,dcb-algorithm;
> > -            snps,map-to-dma-channel = <0x0>;
> > -            snps,priority = <0x0>;
> > -        };
> > -    };
> > -
> > -    mtl_tx_setup: tx-queues-config {
> > -        snps,tx-queues-to-use = <2>;
> > -        snps,tx-sched-wrr;
> > -        queue0 {
> > -            snps,weight = <0x10>;
> > -            snps,dcb-algorithm;
> > -            snps,priority = <0x0>;
> > -        };
> > -
> > -        queue1 {
> > -            snps,avb-algorithm;
> > -            snps,send_slope = <0x1000>;
> > -            snps,idle_slope = <0x1000>;
> > -            snps,high_credit = <0x3E800>;
> > -            snps,low_credit = <0xFFC18000>;
> > -            snps,priority = <0x1>;
> > -        };
> > -    };
> > -
> >      gmac0: ethernet@e0800000 {
> >          compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
> >          reg = <0xe0800000 0x8000>;
> > @@ -404,6 +551,55 @@ examples:
> >              };
> >          };
> >      };
> > +  - |
> > +    gmac1: ethernet@f8010000 {
> > +        compatible = "snps,dwmac-4.10a", "snps,dwmac";
> > +        reg = <0xf8010000 0x4000>;
> > +        interrupts = <0 98 4>;
> > +        interrupt-names = "macirq";
> > +        clock-names = "stmmaceth", "ptp_ref";
> > +        clocks = <&clock 4>, <&clock 5>;
> > +        phy-mode = "rgmii";
> > +        snps,txpbl = <8>;
> > +        snps,rxpbl = <2>;
> > +        snps,aal;
> > +        snps,tso;
> > +
> > +        snps,axi-config {
> > +            snps,wr_osr_lmt = <0xf>;
> > +            snps,rd_osr_lmt = <0xf>;
> > +            snps,blen = <256 128 64 32 0 0 0>;
> > +        };
> > +
> > +        snps,mtl-rx-config {
> > +            snps,rx-queues-to-use = <1>;
> > +            snps,rx-sched-sp;
> > +            queue0 {
> > +               snps,dcb-algorithm;
> > +               snps,map-to-dma-channel = <0x0>;
> > +               snps,priority = <0x0>;
> > +            };
> > +        };
> > +
> > +        snps,mtl-tx-config {
> > +            snps,tx-queues-to-use = <2>;
> > +            snps,tx-sched-wrr;
> > +            queue0 {
> > +                snps,weight = <0x10>;
> > +                snps,dcb-algorithm;
> > +                snps,priority = <0x0>;
> > +            };
> > +
> > +            queue1 {
> > +                snps,avb-algorithm;
> > +                snps,send_slope = <0x1000>;
> > +                snps,idle_slope = <0x1000>;
> > +                snps,high_credit = <0x3E800>;
> > +                snps,low_credit = <0xFFC18000>;
> > +                snps,priority = <0x1>;
> > +            };
> > +        };
> > +    };
> >  
> >  # FIXME: We should set it, but it would report all the generic
> >  # properties as additional properties.
> > -- 
> > 2.29.2
> > 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties
  2020-12-15  8:54     ` Serge Semin
@ 2020-12-15 14:08       ` Rob Herring
  2020-12-16  6:24         ` Serge Semin
  0 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2020-12-15 14:08 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev,
	moderated list:ARM/STM32 ARCHITECTURE, linux-arm-kernel,
	devicetree, linux-kernel

On Tue, Dec 15, 2020 at 2:54 AM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>
> Hello Rob,
>
> On Mon, Dec 14, 2020 at 08:30:06AM -0600, Rob Herring wrote:
> > On Mon, Dec 14, 2020 at 12:15:54PM +0300, Serge Semin wrote:
> > > Currently the "snps,axi-config", "snps,mtl-rx-config" and
> > > "snps,mtl-tx-config" properties are declared as a single phandle reference
> > > to a node with corresponding parameters defined. That's not good for
> > > several reasons. First of all scattering around a device tree some
> > > particular device-specific configs with no visual relation to that device
> > > isn't suitable from maintainability point of view. That leads to a
> > > disturbed representation of the actual device tree mixing actual device
> > > nodes and some vendor-specific configs. Secondly using the same configs
> > > set for several device nodes doesn't represent well the devices structure,
> > > since the interfaces these configs describe in hardware belong to
> > > different devices and may actually differ. In the later case having the
> > > configs node separated from the corresponding device nodes gets to be
> > > even unjustified.
> > >
> > > So instead of having a separate DW *MAC configs nodes we suggest to
> > > define them as sub-nodes of the device nodes, which interfaces they
> > > actually describe. By doing so we'll make the DW *MAC nodes visually
> > > correct describing all the aspects of the IP-core configuration. Thus
> > > we'll be able to describe the configs sub-nodes bindings right in the
> > > snps,dwmac.yaml file.
> > >
> > > Note the former "snps,axi-config", "snps,mtl-rx-config" and
> > > "snps,mtl-tx-config" bindings have been marked as deprecated.
> > >
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > >
> > > ---
> > >
> > > Note the current DT schema tool requires the vendor-specific properties to be
> > > defined in accordance with the schema: dtschema/meta-schemas/vendor-props.yaml
> > > It means the property can be;
> > > - boolean,
> > > - string,
> > > - defined with $ref and additional constraints,
> > > - defined with allOf: [ $ref ] and additional constraints.
> > >
> > > The modification provided by this commit needs to extend that definition to
> > > make the DT schema tool correctly parse this schema. That is we need to let
> > > the vendors-specific properties to also accept the oneOf-based combined
> > > sub-schema. Like this:
> > >
> > > --- a/dtschema/meta-schemas/vendor-props.yaml
> > > +++ b/dtschema/meta-schemas/vendor-props.yaml
> > > @@ -48,15 +48,24 @@
> > >        - properties:   # A property with a type and additional constraints
> > >            $ref:
> > >              pattern: "types.yaml#[\/]{0,1}definitions\/.*"
> > > -          allOf:
> > > -            items:
> > > -              - properties:
> > > +
> > > +        if:
> > > +          not:
> > > +            required:
> > > +              - $ref
> > > +        then:
> > > +          patternProperties:
> > > +            "^(all|one)Of$":
> > > +              contains:
> > > +                properties:
> > >                    $ref:
> > >                      pattern: "types.yaml#[\/]{0,1}definitions\/.*"
> > >                  required:
> > >                    - $ref
> > > -        oneOf:
> > > +
> > > +        anyOf:
> > >            - required: [ $ref ]
> > >            - required: [ allOf ]
> > > +          - required: [ oneOf ]
> > >
> > >  ...
> > > ---
> > >  .../devicetree/bindings/net/snps,dwmac.yaml   | 380 +++++++++++++-----
> > >  1 file changed, 288 insertions(+), 92 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > index 0dd543c6c08e..44aa88151cba 100644
> > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > @@ -150,69 +150,251 @@ properties:
> > >        in a different mode than the PHY in order to function.
> > >
> > >    snps,axi-config:
> > > -    $ref: /schemas/types.yaml#definitions/phandle
> > > -    description:
> > > -      AXI BUS Mode parameters. Phandle to a node that can contain the
> > > -      following properties
> > > -        * snps,lpi_en, enable Low Power Interface
> > > -        * snps,xit_frm, unlock on WoL
> > > -        * snps,wr_osr_lmt, max write outstanding req. limit
> > > -        * snps,rd_osr_lmt, max read outstanding req. limit
> > > -        * snps,kbbe, do not cross 1KiB boundary.
> > > -        * snps,blen, this is a vector of supported burst length.
> > > -        * snps,fb, fixed-burst
> > > -        * snps,mb, mixed-burst
> > > -        * snps,rb, rebuild INCRx Burst
> > > +    description: AXI BUS Mode parameters
> > > +    oneOf:
> > > +      - deprecated: true
> > > +        $ref: /schemas/types.yaml#definitions/phandle
> > > +      - type: object
> > > +        properties:
> >
>
> > Anywhere have have the same node/property string meaning 2 different
> > things is a pain, let's not create another one.
>
> IIUC you meant that having a node and property with the same name
> isn't ok. Right? If so could you explain why not? especially seeing
> the property is expected to be set with phandle reference to that
> node. That seemed like a perfect solution to me. We wouldn't need to
> introduce a new property/node name, but just deprecate the
> corresponding name to be a property.

Right. It's also a property or node name having 2 different meanings.
I think your schema above demonstrates the problem in that it
unnecessarily complicates the schema. It's not such a problem here as
it is self contained. The worst example is 'ports'. That's a container
of graph port nodes, ethernet switch nodes or a property pointing to
DRM graphics pipelines. If there's multiple meanings, then we can't
apply a schema unconditionally. Or we can only check it matches one of
the 3 definitions.

> > Just define a new node
> > 'axi-config'. Or just put all the properties into the node directly.
> > Grouping them has little purpose.
>
> Hm, you suggest to remove the vendor prefix, right?

Yes, we don't do vendor prefixes on node names either.

> If so what about
> the rest of the changes introduced here in this patch? They concern
> "snps,mtl-tx-config" and "snps,mtl-rx-config" properties (please note
> these changes are a bit more complicated than once connected with
> "snps,axi-config"). Should I remove the vendor-prefix from them too?

Yes.

> Anyway that seems a bit questionable, because all the "snps,*-config"
> properties/nodes seems more vendor-specific than generic. Am I wrong
> in that matter?
>
> If you think they are generic, then the "{axi,mtl-rx,mtl-tx}-config"
> nodes most likely should be described in the dedicated DT schema...
>
> -Sergey
>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/25] dt-bindings: net: dwmac: Validate PBL for all IP-cores
  2020-12-14  9:15 ` [PATCH 01/25] dt-bindings: net: dwmac: Validate PBL for all IP-cores Serge Semin
@ 2020-12-15 17:14   ` Rob Herring
  0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-12-15 17:14 UTC (permalink / raw)
  To: Serge Semin
  Cc: Johan Hovold, Pavel Parkhomenko, netdev, Jose Abreu,
	Maxime Coquelin, Giuseppe Cavallaro, linux-stm32, Jakub Kicinski,
	linux-arm-kernel, linux-kernel, Serge Semin, Maxime Ripard,
	Vyacheslav Mitrofanov, devicetree, Alexandre Torgue,
	David S. Miller, Rob Herring, Alexey Malahov, Lars Persson,
	Joao Pinto

On Mon, 14 Dec 2020 12:15:51 +0300, Serge Semin wrote:
> Indeed the maximum DMA burst length can be programmed not only for DW
> xGMACs, Allwinner EMACs and Spear SoC GMAC, but in accordance with [1]
> for Generic DW *MAC IP-cores. Moreover the STMMAC set of drivers parse
> the property and then apply the configuration for all supported DW MAC
> devices. All of that makes the property being available for all IP-cores
> the bindings supports. Let's make sure the PBL-related properties are
> validated for all of them by the common DW MAC DT schema.
> 
> [1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
>     October 2013, p. 380.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   | 69 +++++++------------
>  1 file changed, 26 insertions(+), 43 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 02/25] dt-bindings: net: dwmac: Extend number of PBL values
  2020-12-14  9:15 ` [PATCH 02/25] dt-bindings: net: dwmac: Extend number of PBL values Serge Semin
@ 2020-12-15 17:14   ` Rob Herring
  0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-12-15 17:14 UTC (permalink / raw)
  To: Serge Semin
  Cc: netdev, linux-arm-kernel, David S. Miller, linux-kernel,
	Jose Abreu, Alexandre Torgue, Serge Semin, devicetree,
	Maxime Coquelin, Rob Herring, Jakub Kicinski,
	Vyacheslav Mitrofanov, Giuseppe Cavallaro, Pavel Parkhomenko,
	Joao Pinto, Lars Persson, Alexey Malahov, linux-stm32,
	Johan Hovold, Maxime Ripard

On Mon, 14 Dec 2020 12:15:52 +0300, Serge Semin wrote:
> In accordance with [1] the permitted PBL values can be set as one of
> [1, 2, 4, 8, 16, 32]. The rest of the values results in undefined
> behavior. At the same time some of the permitted values can be also
> invalid depending on the controller FIFOs size and the data bus width.
> Seeing due to having too many variables all the possible PBL property
> constraints can't be implemented in the bindings schema, let's extend
> the set of permitted PBL values to be as much as the configuration
> register supports leaving the undefined behaviour cases for developers
> to handle.
> 
> [1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
>     October 2013, p. 380.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration
  2020-12-14  9:15 ` [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration Serge Semin
@ 2020-12-15 17:22   ` Rob Herring
  2020-12-16  8:59     ` Serge Semin
  0 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2020-12-15 17:22 UTC (permalink / raw)
  To: Serge Semin
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Serge Semin, Alexey Malahov,
	Pavel Parkhomenko, Vyacheslav Mitrofanov, Maxime Coquelin,
	netdev, linux-stm32, linux-arm-kernel, devicetree, linux-kernel

On Mon, Dec 14, 2020 at 12:15:53PM +0300, Serge Semin wrote:
> Indeed the STMMAC driver doesn't take the vendor-specific compatible
> string into account to parse the "snps,tso" boolean property. It just
> makes sure the node is compatible with DW MAC 4.x, 5.x and DW xGMAC
> IP-cores. Fix the conditional statement so the TSO-property would be
> evaluated for the compatibles having the corresponding IP-core version.
> 
> While at it move the whole allOf-block from the tail of the binding file
> to the head of it, as it's normally done in the most of the DT schemas.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> ---
> 
> Note this won't break the bindings description, since the "snps,tso"
> property isn't parsed by the Allwinner SunX GMAC glue driver, but only
> by the generic platform DT-parser.

But still should be valid for Allwinner?

> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   | 52 +++++++++----------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index e084fbbf976e..0dd543c6c08e 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -37,6 +37,30 @@ select:
>    required:
>      - compatible
>  
> +allOf:
> +  - $ref: "ethernet-controller.yaml#"
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - snps,dwmac-4.00
> +              - snps,dwmac-4.10a
> +              - snps,dwmac-4.20a
> +              - snps,dwmac-5.10a
> +              - snps,dwxgmac
> +              - snps,dwxgmac-2.10
> +
> +      required:
> +        - compatible
> +    then:
> +      properties:
> +        snps,tso:
> +          $ref: /schemas/types.yaml#definitions/flag
> +          description:
> +            Enables the TSO feature otherwise it will be managed by
> +            MAC HW capability register.

BTW, I prefer that properties are defined unconditionally, and then 
restricted in conditional schemas (or ones that include this schema).

> +
>  properties:
>  
>    # We need to include all the compatibles from schemas that will
> @@ -314,34 +338,6 @@ dependencies:
>    snps,reset-active-low: ["snps,reset-gpio"]
>    snps,reset-delay-us: ["snps,reset-gpio"]
>  
> -allOf:
> -  - $ref: "ethernet-controller.yaml#"
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - allwinner,sun7i-a20-gmac

This does not have a fallback, so snps,tso is no longer validated. I 
didn't check the rest.

> -              - allwinner,sun8i-a83t-emac
> -              - allwinner,sun8i-h3-emac
> -              - allwinner,sun8i-r40-emac
> -              - allwinner,sun8i-v3s-emac
> -              - allwinner,sun50i-a64-emac
> -              - snps,dwmac-4.00
> -              - snps,dwmac-4.10a
> -              - snps,dwmac-4.20a
> -              - snps,dwxgmac
> -              - snps,dwxgmac-2.10
> -              - st,spear600-gmac
> -
> -    then:
> -      properties:
> -        snps,tso:
> -          $ref: /schemas/types.yaml#definitions/flag
> -          description:
> -            Enables the TSO feature otherwise it will be managed by
> -            MAC HW capability register.
> -
>  additionalProperties: true
>  
>  examples:
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 05/25] dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description
  2020-12-14  9:15 ` [PATCH 05/25] dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description Serge Semin
@ 2020-12-15 17:30   ` Rob Herring
  0 siblings, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-12-15 17:30 UTC (permalink / raw)
  To: Serge Semin
  Cc: Jakub Kicinski, Alexandre Torgue, netdev, linux-kernel,
	Serge Semin, Lars Persson, Pavel Parkhomenko, Giuseppe Cavallaro,
	devicetree, Maxime Coquelin, linux-arm-kernel, Alexey Malahov,
	Jose Abreu, David S. Miller, Johan Hovold, Maxime Ripard,
	Rob Herring, Joao Pinto, Vyacheslav Mitrofanov, linux-stm32

On Mon, 14 Dec 2020 12:15:55 +0300, Serge Semin wrote:
> Current clocks description doesn't provide a comprehensive notion about
> what "stmmaceth" and "pclk" actually represent from the IP-core manual
> point of view. The bindings file states:
> stmmaceth - "GMAC main clock",
> apb - "Peripheral registers interface clock".
> It isn't that easy to understand what they actually mean especially seeing
> the DW *MAC manual operates with clock definitions like Application,
> System, Host, CSR, Transmit, Receive, etc clocks. Moreover the clocks
> usage in the driver doesn't shade a full light on their essence. What
> inferred from there is that the "stmmaceth" name has been assigned to the
> common clock, which feeds both system and CSR interfaces. But what about
> "apb"? The bindings defines it as the clock for "peripheral registers
> interface". So it's close to the CSR clock in the IP-core manual notation.
> If so then when "apb" clock is specified aside with the "stmmaceth", it
> represents a case when the DW *MAC is synthesized with CSR_SLV_CLK=y
> (separate system and CSR clocks). But even though the "apb" clock is
> requested in the MAC driver, the driver doesn't actually use it as a
> separate CSR clock where the IP-core manual requires. All of that makes me
> thinking that the case of separate system and CSR clocks isn't correctly
> implemented in the driver.
> 
> Let's start with elaborating the clocks description so anyone reading
> the DW *MAC bindings file would understand that "stmmaceth" is the
> system clock and "pclk" is actually the CSR clock. Indeed in accordance
> with sheets depicted in [1]:
> system/application clock can be either of: hclk_i, aclk_i, clk_app_i;
> CSR clock can be either of: hclk_i, aclk_i, clk_app_i, clk_csr_i.
> (Most likely the similar definitions present in the others IP-core
> manuals.) So the CSR clock can be tied to the application clock
> considering the later as the main clock, but not the other way around. In
> case if there is only "stmmaceth" clock specified in a DT node, then it
> will be considered as a source of clocks for both application and CSR. But
> if "pclk" is also specified in the list of the device clocks, then it will
> be perceived as the separate CSR clock.
> 
> [1] DesignWare Cores Ethernet MAC Universal Databook, Revision 3.73a,
>     October 2013, p. 564.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml          | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources
  2020-12-14  9:15 ` [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources Serge Semin
@ 2020-12-15 17:32   ` Rob Herring
  2020-12-16  6:02     ` Serge Semin
  2020-12-15 17:32   ` Rob Herring
  1 sibling, 1 reply; 40+ messages in thread
From: Rob Herring @ 2020-12-15 17:32 UTC (permalink / raw)
  To: Serge Semin
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Serge Semin, Alexey Malahov,
	Pavel Parkhomenko, Vyacheslav Mitrofanov, Maxime Coquelin,
	netdev, linux-stm32, linux-arm-kernel, devicetree, linux-kernel

On Mon, Dec 14, 2020 at 12:15:56PM +0300, Serge Semin wrote:
> Generic DW *MAC can be connected to an external Tramit and Receive clock

s/Tramit/Transmit/

> generators. Add the corresponding clocks description and clock-names to
> the generic bindings schema so new DW *MAC-based bindings wouldn't declare
> its own names of the same clocks.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml        | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index e1ebe5c8b1da..74820f491346 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -126,6 +126,18 @@ properties:
>            MCI, CSR and SMA interfaces run on this clock. If it's omitted,
>            the CSR interfaces are considered as synchronous to the system
>            clock domain.
> +      - description:
> +          GMAC Tx clock or so called Transmit clock. The clock is supplied
> +          by an external with respect to the DW MAC clock generator.
> +          The clock source and its frequency depends on the DW MAC xMII mode.
> +          In case if it's supplied by PHY/SerDes this property can be
> +          omitted.
> +      - description:
> +          GMAC Rx clock or so called Receive clock. The clock is supplied
> +          by an external with respect to the DW MAC clock generator.
> +          The clock source and its frequency depends on the DW MAC xMII mode.
> +          In case if it's supplied by PHY/SerDes or it's synchronous to
> +          the Tx clock this property can be omitted.
>        - description:
>            PTP reference clock. This clock is used for programming the
>            Timestamp Addend Register. If not passed then the system
> @@ -139,6 +151,8 @@ properties:
>        enum:
>          - stmmaceth
>          - pclk
> +        - tx
> +        - rx
>          - ptp_ref
>  
>    resets:
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources
  2020-12-14  9:15 ` [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources Serge Semin
  2020-12-15 17:32   ` Rob Herring
@ 2020-12-15 17:32   ` Rob Herring
  1 sibling, 0 replies; 40+ messages in thread
From: Rob Herring @ 2020-12-15 17:32 UTC (permalink / raw)
  To: Serge Semin
  Cc: Joao Pinto, David S. Miller, linux-stm32, Maxime Coquelin,
	devicetree, Maxime Ripard, linux-kernel, Alexandre Torgue,
	Jose Abreu, Giuseppe Cavallaro, Serge Semin, Johan Hovold,
	netdev, Pavel Parkhomenko, Lars Persson, Alexey Malahov,
	linux-arm-kernel, Rob Herring, Jakub Kicinski,
	Vyacheslav Mitrofanov

On Mon, 14 Dec 2020 12:15:56 +0300, Serge Semin wrote:
> Generic DW *MAC can be connected to an external Tramit and Receive clock
> generators. Add the corresponding clocks description and clock-names to
> the generic bindings schema so new DW *MAC-based bindings wouldn't declare
> its own names of the same clocks.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml        | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 07/25] dt-bindings: net: dwmac: Detach Generic DW MAC bindings
  2020-12-14  9:15 ` [PATCH 07/25] dt-bindings: net: dwmac: Detach Generic DW MAC bindings Serge Semin
@ 2020-12-15 17:50   ` Rob Herring
  2020-12-16  9:10     ` Serge Semin
  0 siblings, 1 reply; 40+ messages in thread
From: Rob Herring @ 2020-12-15 17:50 UTC (permalink / raw)
  To: Serge Semin
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Serge Semin, Alexey Malahov,
	Pavel Parkhomenko, Vyacheslav Mitrofanov, Maxime Coquelin,
	netdev, linux-stm32, linux-arm-kernel, devicetree, linux-kernel

On Mon, Dec 14, 2020 at 12:15:57PM +0300, Serge Semin wrote:
> Currently the snps,dwmac.yaml DT bindings file is used for both DT nodes
> describing generic DW MAC devices and as DT schema with common properties
> to be evaluated against a vendor-specific DW MAC IP-cores. Due to such
> dual-purpose design the "compatible" property of the common DW MAC schema
> needs to contain the vendor-specific strings to successfully pass the
> schema evaluation in case if it's referenced from the vendor-specific DT
> bindings. That's a bad design from maintainability point of view, since
> adding/removing any DW MAC-based device bindings requires the common
> schema modification. In order to fix that let's detach the schema which
> provides the generic DW MAC DT nodes evaluation into a dedicated DT
> bindings file preserving the common DW MAC properties declaration in the
> snps,dwmac.yaml file. By doing so we'll still provide a common properties
> evaluation for each vendor-specific MAC bindings which refer to the
> common bindings file, while the generic DW MAC DT nodes will be checked
> against the new snps,dwmac-generic.yaml DT schema.

I'm okay with the change, but it needs a big fat note that 
snps,dwmac-generic.yaml should not have new users. New users should have 
an SoC specific compatible. History has shown that even IP versions are 
not enough to handle all the integration crap vendors do.

> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  .../bindings/net/snps,dwmac-generic.yaml      | 148 ++++++++++++++++++
>  .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------
>  2 files changed, 149 insertions(+), 138 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> new file mode 100644
> index 000000000000..f1b387911390
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> @@ -0,0 +1,148 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/snps,dwmac-generic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare Generic MAC Device Tree Bindings
> +
> +maintainers:
> +  - Alexandre Torgue <alexandre.torgue@st.com>
> +  - Giuseppe Cavallaro <peppe.cavallaro@st.com>
> +  - Jose Abreu <joabreu@synopsys.com>
> +
> +# Select the DT nodes, which have got compatible strings either as just a
> +# single string with IP-core name optionally followed by the IP version or
> +# two strings: one with IP-core name plus the IP version, another as just
> +# the IP-core name.
> +select:
> +  properties:
> +    compatible:
> +      oneOf:
> +        - items:
> +            - pattern: "^snps,dw(xg)+mac(-[0-9]+\\.[0-9]+a?)?$"
> +        - items:
> +            - pattern: "^snps,dwmac-[0-9]+\\.[0-9]+a?$"
> +            - const: snps,dwmac
> +        - items:
> +            - pattern: "^snps,dwxgmac-[0-9]+\\.[0-9]+a?$"
> +            - const: snps,dwxgmac
> +
> +  required:
> +    - compatible
> +
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: Generic Synopsys DW MAC
> +        oneOf:
> +          - items:
> +              - enum:
> +                  - snps,dwmac-3.50a
> +                  - snps,dwmac-3.610
> +                  - snps,dwmac-3.70a
> +                  - snps,dwmac-3.710
> +                  - snps,dwmac-4.00
> +                  - snps,dwmac-4.10a
> +                  - snps,dwmac-4.20a
> +              - const: snps,dwmac
> +          - const: snps,dwmac
> +      - description: Generic Synopsys DW xGMAC
> +        oneOf:
> +          - items:
> +              - enum:
> +                  - snps,dwxgmac-2.10
> +              - const: snps,dwxgmac
> +          - const: snps,dwxgmac
> +      - description: ST SPEAr SoC Family GMAC
> +        deprecated: true
> +        const: st,spear600-gmac
> +
> +  reg:
> +    maxItems: 1
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    gmac0: ethernet@e0800000 {
> +      compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
> +      reg = <0xe0800000 0x8000>;
> +      interrupt-parent = <&vic1>;
> +      interrupts = <24 23 22>;
> +      interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> +      mac-address = [000000000000]; /* Filled in by U-Boot */
> +      max-frame-size = <3800>;
> +      phy-mode = "gmii";
> +      snps,multicast-filter-bins = <256>;
> +      snps,perfect-filter-entries = <128>;
> +      rx-fifo-depth = <16384>;
> +      tx-fifo-depth = <16384>;
> +      clocks = <&clock>;
> +      clock-names = "stmmaceth";
> +      snps,axi-config = <&stmmac_axi_setup>;
> +      snps,mtl-rx-config = <&mtl_rx_setup>;
> +      snps,mtl-tx-config = <&mtl_tx_setup>;
> +      mdio0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        compatible = "snps,dwmac-mdio";
> +        phy1: ethernet-phy@0 {
> +          reg = <0>;
> +        };
> +      };
> +    };
> +  - |
> +    gmac1: ethernet@f8010000 {
> +      compatible = "snps,dwmac-4.10a", "snps,dwmac";
> +      reg = <0xf8010000 0x4000>;
> +      interrupts = <0 98 4>;
> +      interrupt-names = "macirq";
> +      clock-names = "stmmaceth", "ptp_ref";
> +      clocks = <&clock 4>, <&clock 5>;
> +      phy-mode = "rgmii";
> +      snps,txpbl = <8>;
> +      snps,rxpbl = <2>;
> +      snps,aal;
> +      snps,tso;
> +
> +      snps,axi-config {
> +        snps,wr_osr_lmt = <0xf>;
> +        snps,rd_osr_lmt = <0xf>;
> +        snps,blen = <256 128 64 32 0 0 0>;
> +      };
> +
> +      snps,mtl-rx-config {
> +        snps,rx-queues-to-use = <1>;
> +        snps,rx-sched-sp;
> +        queue0 {
> +          snps,dcb-algorithm;
> +          snps,map-to-dma-channel = <0x0>;
> +          snps,priority = <0x0>;
> +        };
> +      };
> +
> +      snps,mtl-tx-config {
> +        snps,tx-queues-to-use = <2>;
> +        snps,tx-sched-wrr;
> +
> +        queue0 {
> +          snps,weight = <0x10>;
> +          snps,dcb-algorithm;
> +          snps,priority = <0x0>;
> +        };
> +
> +        queue1 {
> +          snps,avb-algorithm;
> +          snps,send_slope = <0x1000>;
> +          snps,idle_slope = <0x1000>;
> +          snps,high_credit = <0x3E800>;
> +          snps,low_credit = <0x1FC18000>;
> +          snps,priority = <0x1>;
> +        };
> +      };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 74820f491346..72b58f86bc41 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -11,31 +11,7 @@ maintainers:
>    - Giuseppe Cavallaro <peppe.cavallaro@st.com>
>    - Jose Abreu <joabreu@synopsys.com>
>  
> -# 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
> -select:
> -  properties:
> -    compatible:
> -      contains:
> -        enum:
> -          - snps,dwmac
> -          - snps,dwmac-3.50a
> -          - snps,dwmac-3.610
> -          - snps,dwmac-3.70a
> -          - snps,dwmac-3.710
> -          - snps,dwmac-4.00
> -          - snps,dwmac-4.10a
> -          - snps,dwmac-4.20a
> -          - snps,dwxgmac
> -          - snps,dwxgmac-2.10
> -
> -          # Deprecated
> -          - st,spear600-gmac
> -
> -  required:
> -    - compatible
> +select: false
>  
>  allOf:
>    - $ref: "ethernet-controller.yaml#"
> @@ -62,35 +38,6 @@ allOf:
>              MAC HW capability register.
>  
>  properties:
> -
> -  # We need to include all the compatibles from schemas that will
> -  # include that schemas, otherwise compatible won't validate for
> -  # those.
> -  compatible:
> -    contains:
> -      enum:
> -        - allwinner,sun7i-a20-gmac
> -        - allwinner,sun8i-a83t-emac
> -        - allwinner,sun8i-h3-emac
> -        - allwinner,sun8i-r40-emac
> -        - allwinner,sun8i-v3s-emac
> -        - allwinner,sun50i-a64-emac
> -        - amlogic,meson6-dwmac
> -        - amlogic,meson8b-dwmac
> -        - amlogic,meson8m2-dwmac
> -        - amlogic,meson-gxbb-dwmac
> -        - amlogic,meson-axg-dwmac
> -        - snps,dwmac
> -        - snps,dwmac-3.50a
> -        - snps,dwmac-3.610
> -        - snps,dwmac-3.70a
> -        - snps,dwmac-3.710
> -        - snps,dwmac-4.00
> -        - snps,dwmac-4.10a
> -        - snps,dwmac-4.20a
> -        - snps,dwxgmac
> -        - snps,dwxgmac-2.10
> -
>    reg:
>      minItems: 1
>      maxItems: 2
> @@ -543,88 +490,4 @@ dependencies:
>    snps,reset-delay-us: ["snps,reset-gpio"]
>  
>  additionalProperties: true
> -
> -examples:
> -  - |
> -    gmac0: ethernet@e0800000 {
> -        compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
> -        reg = <0xe0800000 0x8000>;
> -        interrupt-parent = <&vic1>;
> -        interrupts = <24 23 22>;
> -        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> -        mac-address = [000000000000]; /* Filled in by U-Boot */
> -        max-frame-size = <3800>;
> -        phy-mode = "gmii";
> -        snps,multicast-filter-bins = <256>;
> -        snps,perfect-filter-entries = <128>;
> -        rx-fifo-depth = <16384>;
> -        tx-fifo-depth = <16384>;
> -        clocks = <&clock>;
> -        clock-names = "stmmaceth";
> -        snps,axi-config = <&stmmac_axi_setup>;
> -        snps,mtl-rx-config = <&mtl_rx_setup>;
> -        snps,mtl-tx-config = <&mtl_tx_setup>;
> -        mdio0 {
> -            #address-cells = <1>;
> -            #size-cells = <0>;
> -            compatible = "snps,dwmac-mdio";
> -            phy1: ethernet-phy@0 {
> -                reg = <0>;
> -            };
> -        };
> -    };
> -  - |
> -    gmac1: ethernet@f8010000 {
> -        compatible = "snps,dwmac-4.10a", "snps,dwmac";
> -        reg = <0xf8010000 0x4000>;
> -        interrupts = <0 98 4>;
> -        interrupt-names = "macirq";
> -        clock-names = "stmmaceth", "ptp_ref";
> -        clocks = <&clock 4>, <&clock 5>;
> -        phy-mode = "rgmii";
> -        snps,txpbl = <8>;
> -        snps,rxpbl = <2>;
> -        snps,aal;
> -        snps,tso;
> -
> -        snps,axi-config {
> -            snps,wr_osr_lmt = <0xf>;
> -            snps,rd_osr_lmt = <0xf>;
> -            snps,blen = <256 128 64 32 0 0 0>;
> -        };
> -
> -        snps,mtl-rx-config {
> -            snps,rx-queues-to-use = <1>;
> -            snps,rx-sched-sp;
> -            queue0 {
> -               snps,dcb-algorithm;
> -               snps,map-to-dma-channel = <0x0>;
> -               snps,priority = <0x0>;
> -            };
> -        };
> -
> -        snps,mtl-tx-config {
> -            snps,tx-queues-to-use = <2>;
> -            snps,tx-sched-wrr;
> -            queue0 {
> -                snps,weight = <0x10>;
> -                snps,dcb-algorithm;
> -                snps,priority = <0x0>;
> -            };
> -
> -            queue1 {
> -                snps,avb-algorithm;
> -                snps,send_slope = <0x1000>;
> -                snps,idle_slope = <0x1000>;
> -                snps,high_credit = <0x3E800>;
> -                snps,low_credit = <0xFFC18000>;
> -                snps,priority = <0x1>;
> -            };
> -        };
> -    };
> -
> -# FIXME: We should set it, but it would report all the generic
> -# properties as additional properties.
> -# additionalProperties: false
> -
>  ...
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources
  2020-12-15 17:32   ` Rob Herring
@ 2020-12-16  6:02     ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-16  6:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

On Tue, Dec 15, 2020 at 11:32:04AM -0600, Rob Herring wrote:
> On Mon, Dec 14, 2020 at 12:15:56PM +0300, Serge Semin wrote:
> > Generic DW *MAC can be connected to an external Tramit and Receive clock
> 

> s/Tramit/Transmit/

Thanks. I'll fix it in v2.

-Sergey

> 
> > generators. Add the corresponding clocks description and clock-names to
> > the generic bindings schema so new DW *MAC-based bindings wouldn't declare
> > its own names of the same clocks.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml        | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index e1ebe5c8b1da..74820f491346 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -126,6 +126,18 @@ properties:
> >            MCI, CSR and SMA interfaces run on this clock. If it's omitted,
> >            the CSR interfaces are considered as synchronous to the system
> >            clock domain.
> > +      - description:
> > +          GMAC Tx clock or so called Transmit clock. The clock is supplied
> > +          by an external with respect to the DW MAC clock generator.
> > +          The clock source and its frequency depends on the DW MAC xMII mode.
> > +          In case if it's supplied by PHY/SerDes this property can be
> > +          omitted.
> > +      - description:
> > +          GMAC Rx clock or so called Receive clock. The clock is supplied
> > +          by an external with respect to the DW MAC clock generator.
> > +          The clock source and its frequency depends on the DW MAC xMII mode.
> > +          In case if it's supplied by PHY/SerDes or it's synchronous to
> > +          the Tx clock this property can be omitted.
> >        - description:
> >            PTP reference clock. This clock is used for programming the
> >            Timestamp Addend Register. If not passed then the system
> > @@ -139,6 +151,8 @@ properties:
> >        enum:
> >          - stmmaceth
> >          - pclk
> > +        - tx
> > +        - rx
> >          - ptp_ref
> >  
> >    resets:
> > -- 
> > 2.29.2
> > 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties
  2020-12-15 14:08       ` Rob Herring
@ 2020-12-16  6:24         ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-16  6:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev,
	moderated list:ARM/STM32 ARCHITECTURE, linux-arm-kernel,
	devicetree, linux-kernel

On Tue, Dec 15, 2020 at 08:08:35AM -0600, Rob Herring wrote:
> On Tue, Dec 15, 2020 at 2:54 AM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> >
> > Hello Rob,
> >
> > On Mon, Dec 14, 2020 at 08:30:06AM -0600, Rob Herring wrote:
> > > On Mon, Dec 14, 2020 at 12:15:54PM +0300, Serge Semin wrote:
> > > > Currently the "snps,axi-config", "snps,mtl-rx-config" and
> > > > "snps,mtl-tx-config" properties are declared as a single phandle reference
> > > > to a node with corresponding parameters defined. That's not good for
> > > > several reasons. First of all scattering around a device tree some
> > > > particular device-specific configs with no visual relation to that device
> > > > isn't suitable from maintainability point of view. That leads to a
> > > > disturbed representation of the actual device tree mixing actual device
> > > > nodes and some vendor-specific configs. Secondly using the same configs
> > > > set for several device nodes doesn't represent well the devices structure,
> > > > since the interfaces these configs describe in hardware belong to
> > > > different devices and may actually differ. In the later case having the
> > > > configs node separated from the corresponding device nodes gets to be
> > > > even unjustified.
> > > >
> > > > So instead of having a separate DW *MAC configs nodes we suggest to
> > > > define them as sub-nodes of the device nodes, which interfaces they
> > > > actually describe. By doing so we'll make the DW *MAC nodes visually
> > > > correct describing all the aspects of the IP-core configuration. Thus
> > > > we'll be able to describe the configs sub-nodes bindings right in the
> > > > snps,dwmac.yaml file.
> > > >
> > > > Note the former "snps,axi-config", "snps,mtl-rx-config" and
> > > > "snps,mtl-tx-config" bindings have been marked as deprecated.
> > > >
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > >
> > > > ---
> > > >
> > > > Note the current DT schema tool requires the vendor-specific properties to be
> > > > defined in accordance with the schema: dtschema/meta-schemas/vendor-props.yaml
> > > > It means the property can be;
> > > > - boolean,
> > > > - string,
> > > > - defined with $ref and additional constraints,
> > > > - defined with allOf: [ $ref ] and additional constraints.
> > > >
> > > > The modification provided by this commit needs to extend that definition to
> > > > make the DT schema tool correctly parse this schema. That is we need to let
> > > > the vendors-specific properties to also accept the oneOf-based combined
> > > > sub-schema. Like this:
> > > >
> > > > --- a/dtschema/meta-schemas/vendor-props.yaml
> > > > +++ b/dtschema/meta-schemas/vendor-props.yaml
> > > > @@ -48,15 +48,24 @@
> > > >        - properties:   # A property with a type and additional constraints
> > > >            $ref:
> > > >              pattern: "types.yaml#[\/]{0,1}definitions\/.*"
> > > > -          allOf:
> > > > -            items:
> > > > -              - properties:
> > > > +
> > > > +        if:
> > > > +          not:
> > > > +            required:
> > > > +              - $ref
> > > > +        then:
> > > > +          patternProperties:
> > > > +            "^(all|one)Of$":
> > > > +              contains:
> > > > +                properties:
> > > >                    $ref:
> > > >                      pattern: "types.yaml#[\/]{0,1}definitions\/.*"
> > > >                  required:
> > > >                    - $ref
> > > > -        oneOf:
> > > > +
> > > > +        anyOf:
> > > >            - required: [ $ref ]
> > > >            - required: [ allOf ]
> > > > +          - required: [ oneOf ]
> > > >
> > > >  ...
> > > > ---
> > > >  .../devicetree/bindings/net/snps,dwmac.yaml   | 380 +++++++++++++-----
> > > >  1 file changed, 288 insertions(+), 92 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > index 0dd543c6c08e..44aa88151cba 100644
> > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > > @@ -150,69 +150,251 @@ properties:
> > > >        in a different mode than the PHY in order to function.
> > > >
> > > >    snps,axi-config:
> > > > -    $ref: /schemas/types.yaml#definitions/phandle
> > > > -    description:
> > > > -      AXI BUS Mode parameters. Phandle to a node that can contain the
> > > > -      following properties
> > > > -        * snps,lpi_en, enable Low Power Interface
> > > > -        * snps,xit_frm, unlock on WoL
> > > > -        * snps,wr_osr_lmt, max write outstanding req. limit
> > > > -        * snps,rd_osr_lmt, max read outstanding req. limit
> > > > -        * snps,kbbe, do not cross 1KiB boundary.
> > > > -        * snps,blen, this is a vector of supported burst length.
> > > > -        * snps,fb, fixed-burst
> > > > -        * snps,mb, mixed-burst
> > > > -        * snps,rb, rebuild INCRx Burst
> > > > +    description: AXI BUS Mode parameters
> > > > +    oneOf:
> > > > +      - deprecated: true
> > > > +        $ref: /schemas/types.yaml#definitions/phandle
> > > > +      - type: object
> > > > +        properties:
> > >
> >
> > > Anywhere have have the same node/property string meaning 2 different
> > > things is a pain, let's not create another one.
> >
> > IIUC you meant that having a node and property with the same name
> > isn't ok. Right? If so could you explain why not? especially seeing
> > the property is expected to be set with phandle reference to that
> > node. That seemed like a perfect solution to me. We wouldn't need to
> > introduce a new property/node name, but just deprecate the
> > corresponding name to be a property.
> 

> Right. It's also a property or node name having 2 different meanings.
> I think your schema above demonstrates the problem in that it
> unnecessarily complicates the schema. It's not such a problem here as
> it is self contained. The worst example is 'ports'. That's a container
> of graph port nodes, ethernet switch nodes or a property pointing to
> DRM graphics pipelines. If there's multiple meanings, then we can't
> apply a schema unconditionally. Or we can only check it matches one of
> the 3 definitions.

It turned out in case of this change having different meaning also luckily
fit with having different types (property vs node). Right, as you called
it's self contained. But in general, of course, having different meaning
of the same node indeed may cause problems with different schema validation.
So ok. Thanks for clarification. I'll just introduce a new sub-node with
the same name but no vendor-prefix.

> 
> > > Just define a new node
> > > 'axi-config'. Or just put all the properties into the node directly.
> > > Grouping them has little purpose.
> >
> > Hm, you suggest to remove the vendor prefix, right?
> 

> Yes, we don't do vendor prefixes on node names either.

Ok.

> 
> > If so what about
> > the rest of the changes introduced here in this patch? They concern
> > "snps,mtl-tx-config" and "snps,mtl-rx-config" properties (please note
> > these changes are a bit more complicated than once connected with
> > "snps,axi-config"). Should I remove the vendor-prefix from them too?
> 

> Yes.

Ok.

-Sergey

> 
> > Anyway that seems a bit questionable, because all the "snps,*-config"
> > properties/nodes seems more vendor-specific than generic. Am I wrong
> > in that matter?
> >
> > If you think they are generic, then the "{axi,mtl-rx,mtl-tx}-config"
> > nodes most likely should be described in the dedicated DT schema...
> >
> > -Sergey
> >

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration
  2020-12-15 17:22   ` Rob Herring
@ 2020-12-16  8:59     ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-16  8:59 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

On Tue, Dec 15, 2020 at 11:22:40AM -0600, Rob Herring wrote:
> On Mon, Dec 14, 2020 at 12:15:53PM +0300, Serge Semin wrote:
> > Indeed the STMMAC driver doesn't take the vendor-specific compatible
> > string into account to parse the "snps,tso" boolean property. It just
> > makes sure the node is compatible with DW MAC 4.x, 5.x and DW xGMAC
> > IP-cores. Fix the conditional statement so the TSO-property would be
> > evaluated for the compatibles having the corresponding IP-core version.
> > 
> > While at it move the whole allOf-block from the tail of the binding file
> > to the head of it, as it's normally done in the most of the DT schemas.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > ---
> > 
> > Note this won't break the bindings description, since the "snps,tso"
> > property isn't parsed by the Allwinner SunX GMAC glue driver, but only
> > by the generic platform DT-parser.
> 

> But still should be valid for Allwinner?

I don't know. It seems to me that even the original driver developer
didn't know what DW MAC IP has been used to create the Allwinner
EMAC, since in the cover letter to the original patch he said:
"During the development, it appeared that in fact the hardware was
a modified version of some dwmac." (See https://lwn.net/Articles/721459/)
Most likely Maxime Ripard also didn't know that when he was converting
the legacy bindings to the DT schema.

What I do know the TSO is supported by the driver only for IP-cores with
version higher than 4.00. (See the stmmac_probe_config_dt() method
implementation). Version is determined by checking whether the DT
device node compatible property having the "snps,dwmac-*" or
"snps,dwxgmac" strings. Allwinner EMAC nodes aren't defined with
those strings, so they won't have the TSO property parsed and set.

> 
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 52 +++++++++----------
> >  1 file changed, 24 insertions(+), 28 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index e084fbbf976e..0dd543c6c08e 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -37,6 +37,30 @@ select:
> >    required:
> >      - compatible
> >  
> > +allOf:
> > +  - $ref: "ethernet-controller.yaml#"
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - snps,dwmac-4.00
> > +              - snps,dwmac-4.10a
> > +              - snps,dwmac-4.20a
> > +              - snps,dwmac-5.10a
> > +              - snps,dwxgmac
> > +              - snps,dwxgmac-2.10
> > +
> > +      required:
> > +        - compatible
> > +    then:
> > +      properties:
> > +        snps,tso:
> > +          $ref: /schemas/types.yaml#definitions/flag
> > +          description:
> > +            Enables the TSO feature otherwise it will be managed by
> > +            MAC HW capability register.
> 

> BTW, I prefer that properties are defined unconditionally, and then 
> restricted in conditional schemas (or ones that include this schema).

Are you saying that it's ok to have all the properties unconditionally
defined in some generic schema and then being un-defined (like redefined
to a false-schema) in a schema including (allOf-ing) it?

> 
> > +
> >  properties:
> >  
> >    # We need to include all the compatibles from schemas that will
> > @@ -314,34 +338,6 @@ dependencies:
> >    snps,reset-active-low: ["snps,reset-gpio"]
> >    snps,reset-delay-us: ["snps,reset-gpio"]
> >  
> > -allOf:
> > -  - $ref: "ethernet-controller.yaml#"
> > -  - if:
> > -      properties:
> > -        compatible:
> > -          contains:
> > -            enum:
> > -              - allwinner,sun7i-a20-gmac
> 

> This does not have a fallback, so snps,tso is no longer validated. I 
> didn't check the rest.

Until the DT node is having a compatible string with the DW MAC
IP-core version the property won't be checked by the driver anyway.
AFAICS noone really knows what IP was that. So most likely the
allwinner emacs have been added to this conditional schema by
mistake...

-Sergey

> 
> > -              - allwinner,sun8i-a83t-emac
> > -              - allwinner,sun8i-h3-emac
> > -              - allwinner,sun8i-r40-emac
> > -              - allwinner,sun8i-v3s-emac
> > -              - allwinner,sun50i-a64-emac
> > -              - snps,dwmac-4.00
> > -              - snps,dwmac-4.10a
> > -              - snps,dwmac-4.20a
> > -              - snps,dwxgmac
> > -              - snps,dwxgmac-2.10
> > -              - st,spear600-gmac
> > -
> > -    then:
> > -      properties:
> > -        snps,tso:
> > -          $ref: /schemas/types.yaml#definitions/flag
> > -          description:
> > -            Enables the TSO feature otherwise it will be managed by
> > -            MAC HW capability register.
> > -
> >  additionalProperties: true
> >  
> >  examples:
> > -- 
> > 2.29.2
> > 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 07/25] dt-bindings: net: dwmac: Detach Generic DW MAC bindings
  2020-12-15 17:50   ` Rob Herring
@ 2020-12-16  9:10     ` Serge Semin
  0 siblings, 0 replies; 40+ messages in thread
From: Serge Semin @ 2020-12-16  9:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Johan Hovold, Maxime Ripard,
	Joao Pinto, Lars Persson, Alexey Malahov, Pavel Parkhomenko,
	Vyacheslav Mitrofanov, Maxime Coquelin, netdev, linux-stm32,
	linux-arm-kernel, devicetree, linux-kernel

On Tue, Dec 15, 2020 at 11:50:02AM -0600, Rob Herring wrote:
> On Mon, Dec 14, 2020 at 12:15:57PM +0300, Serge Semin wrote:
> > Currently the snps,dwmac.yaml DT bindings file is used for both DT nodes
> > describing generic DW MAC devices and as DT schema with common properties
> > to be evaluated against a vendor-specific DW MAC IP-cores. Due to such
> > dual-purpose design the "compatible" property of the common DW MAC schema
> > needs to contain the vendor-specific strings to successfully pass the
> > schema evaluation in case if it's referenced from the vendor-specific DT
> > bindings. That's a bad design from maintainability point of view, since
> > adding/removing any DW MAC-based device bindings requires the common
> > schema modification. In order to fix that let's detach the schema which
> > provides the generic DW MAC DT nodes evaluation into a dedicated DT
> > bindings file preserving the common DW MAC properties declaration in the
> > snps,dwmac.yaml file. By doing so we'll still provide a common properties
> > evaluation for each vendor-specific MAC bindings which refer to the
> > common bindings file, while the generic DW MAC DT nodes will be checked
> > against the new snps,dwmac-generic.yaml DT schema.
> 

> I'm okay with the change, but it needs a big fat note that 
> snps,dwmac-generic.yaml should not have new users. New users should have 
> an SoC specific compatible. History has shown that even IP versions are 
> not enough to handle all the integration crap vendors do.

Agreed. I'll add such note to the "description" text of the
snps,dwmac-generic.yaml schema. Is that ok?

-Sergey

> 
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  .../bindings/net/snps,dwmac-generic.yaml      | 148 ++++++++++++++++++
> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------
> >  2 files changed, 149 insertions(+), 138 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> > new file mode 100644
> > index 000000000000..f1b387911390
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> > @@ -0,0 +1,148 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/snps,dwmac-generic.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synopsys DesignWare Generic MAC Device Tree Bindings
> > +
> > +maintainers:
> > +  - Alexandre Torgue <alexandre.torgue@st.com>
> > +  - Giuseppe Cavallaro <peppe.cavallaro@st.com>
> > +  - Jose Abreu <joabreu@synopsys.com>
> > +
> > +# Select the DT nodes, which have got compatible strings either as just a
> > +# single string with IP-core name optionally followed by the IP version or
> > +# two strings: one with IP-core name plus the IP version, another as just
> > +# the IP-core name.
> > +select:
> > +  properties:
> > +    compatible:
> > +      oneOf:
> > +        - items:
> > +            - pattern: "^snps,dw(xg)+mac(-[0-9]+\\.[0-9]+a?)?$"
> > +        - items:
> > +            - pattern: "^snps,dwmac-[0-9]+\\.[0-9]+a?$"
> > +            - const: snps,dwmac
> > +        - items:
> > +            - pattern: "^snps,dwxgmac-[0-9]+\\.[0-9]+a?$"
> > +            - const: snps,dwxgmac
> > +
> > +  required:
> > +    - compatible
> > +
> > +allOf:
> > +  - $ref: snps,dwmac.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - description: Generic Synopsys DW MAC
> > +        oneOf:
> > +          - items:
> > +              - enum:
> > +                  - snps,dwmac-3.50a
> > +                  - snps,dwmac-3.610
> > +                  - snps,dwmac-3.70a
> > +                  - snps,dwmac-3.710
> > +                  - snps,dwmac-4.00
> > +                  - snps,dwmac-4.10a
> > +                  - snps,dwmac-4.20a
> > +              - const: snps,dwmac
> > +          - const: snps,dwmac
> > +      - description: Generic Synopsys DW xGMAC
> > +        oneOf:
> > +          - items:
> > +              - enum:
> > +                  - snps,dwxgmac-2.10
> > +              - const: snps,dwxgmac
> > +          - const: snps,dwxgmac
> > +      - description: ST SPEAr SoC Family GMAC
> > +        deprecated: true
> > +        const: st,spear600-gmac
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    gmac0: ethernet@e0800000 {
> > +      compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
> > +      reg = <0xe0800000 0x8000>;
> > +      interrupt-parent = <&vic1>;
> > +      interrupts = <24 23 22>;
> > +      interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> > +      mac-address = [000000000000]; /* Filled in by U-Boot */
> > +      max-frame-size = <3800>;
> > +      phy-mode = "gmii";
> > +      snps,multicast-filter-bins = <256>;
> > +      snps,perfect-filter-entries = <128>;
> > +      rx-fifo-depth = <16384>;
> > +      tx-fifo-depth = <16384>;
> > +      clocks = <&clock>;
> > +      clock-names = "stmmaceth";
> > +      snps,axi-config = <&stmmac_axi_setup>;
> > +      snps,mtl-rx-config = <&mtl_rx_setup>;
> > +      snps,mtl-tx-config = <&mtl_tx_setup>;
> > +      mdio0 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        compatible = "snps,dwmac-mdio";
> > +        phy1: ethernet-phy@0 {
> > +          reg = <0>;
> > +        };
> > +      };
> > +    };
> > +  - |
> > +    gmac1: ethernet@f8010000 {
> > +      compatible = "snps,dwmac-4.10a", "snps,dwmac";
> > +      reg = <0xf8010000 0x4000>;
> > +      interrupts = <0 98 4>;
> > +      interrupt-names = "macirq";
> > +      clock-names = "stmmaceth", "ptp_ref";
> > +      clocks = <&clock 4>, <&clock 5>;
> > +      phy-mode = "rgmii";
> > +      snps,txpbl = <8>;
> > +      snps,rxpbl = <2>;
> > +      snps,aal;
> > +      snps,tso;
> > +
> > +      snps,axi-config {
> > +        snps,wr_osr_lmt = <0xf>;
> > +        snps,rd_osr_lmt = <0xf>;
> > +        snps,blen = <256 128 64 32 0 0 0>;
> > +      };
> > +
> > +      snps,mtl-rx-config {
> > +        snps,rx-queues-to-use = <1>;
> > +        snps,rx-sched-sp;
> > +        queue0 {
> > +          snps,dcb-algorithm;
> > +          snps,map-to-dma-channel = <0x0>;
> > +          snps,priority = <0x0>;
> > +        };
> > +      };
> > +
> > +      snps,mtl-tx-config {
> > +        snps,tx-queues-to-use = <2>;
> > +        snps,tx-sched-wrr;
> > +
> > +        queue0 {
> > +          snps,weight = <0x10>;
> > +          snps,dcb-algorithm;
> > +          snps,priority = <0x0>;
> > +        };
> > +
> > +        queue1 {
> > +          snps,avb-algorithm;
> > +          snps,send_slope = <0x1000>;
> > +          snps,idle_slope = <0x1000>;
> > +          snps,high_credit = <0x3E800>;
> > +          snps,low_credit = <0x1FC18000>;
> > +          snps,priority = <0x1>;
> > +        };
> > +      };
> > +    };
> > +...
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index 74820f491346..72b58f86bc41 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -11,31 +11,7 @@ maintainers:
> >    - Giuseppe Cavallaro <peppe.cavallaro@st.com>
> >    - Jose Abreu <joabreu@synopsys.com>
> >  
> > -# 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
> > -select:
> > -  properties:
> > -    compatible:
> > -      contains:
> > -        enum:
> > -          - snps,dwmac
> > -          - snps,dwmac-3.50a
> > -          - snps,dwmac-3.610
> > -          - snps,dwmac-3.70a
> > -          - snps,dwmac-3.710
> > -          - snps,dwmac-4.00
> > -          - snps,dwmac-4.10a
> > -          - snps,dwmac-4.20a
> > -          - snps,dwxgmac
> > -          - snps,dwxgmac-2.10
> > -
> > -          # Deprecated
> > -          - st,spear600-gmac
> > -
> > -  required:
> > -    - compatible
> > +select: false
> >  
> >  allOf:
> >    - $ref: "ethernet-controller.yaml#"
> > @@ -62,35 +38,6 @@ allOf:
> >              MAC HW capability register.
> >  
> >  properties:
> > -
> > -  # We need to include all the compatibles from schemas that will
> > -  # include that schemas, otherwise compatible won't validate for
> > -  # those.
> > -  compatible:
> > -    contains:
> > -      enum:
> > -        - allwinner,sun7i-a20-gmac
> > -        - allwinner,sun8i-a83t-emac
> > -        - allwinner,sun8i-h3-emac
> > -        - allwinner,sun8i-r40-emac
> > -        - allwinner,sun8i-v3s-emac
> > -        - allwinner,sun50i-a64-emac
> > -        - amlogic,meson6-dwmac
> > -        - amlogic,meson8b-dwmac
> > -        - amlogic,meson8m2-dwmac
> > -        - amlogic,meson-gxbb-dwmac
> > -        - amlogic,meson-axg-dwmac
> > -        - snps,dwmac
> > -        - snps,dwmac-3.50a
> > -        - snps,dwmac-3.610
> > -        - snps,dwmac-3.70a
> > -        - snps,dwmac-3.710
> > -        - snps,dwmac-4.00
> > -        - snps,dwmac-4.10a
> > -        - snps,dwmac-4.20a
> > -        - snps,dwxgmac
> > -        - snps,dwxgmac-2.10
> > -
> >    reg:
> >      minItems: 1
> >      maxItems: 2
> > @@ -543,88 +490,4 @@ dependencies:
> >    snps,reset-delay-us: ["snps,reset-gpio"]
> >  
> >  additionalProperties: true
> > -
> > -examples:
> > -  - |
> > -    gmac0: ethernet@e0800000 {
> > -        compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";
> > -        reg = <0xe0800000 0x8000>;
> > -        interrupt-parent = <&vic1>;
> > -        interrupts = <24 23 22>;
> > -        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> > -        mac-address = [000000000000]; /* Filled in by U-Boot */
> > -        max-frame-size = <3800>;
> > -        phy-mode = "gmii";
> > -        snps,multicast-filter-bins = <256>;
> > -        snps,perfect-filter-entries = <128>;
> > -        rx-fifo-depth = <16384>;
> > -        tx-fifo-depth = <16384>;
> > -        clocks = <&clock>;
> > -        clock-names = "stmmaceth";
> > -        snps,axi-config = <&stmmac_axi_setup>;
> > -        snps,mtl-rx-config = <&mtl_rx_setup>;
> > -        snps,mtl-tx-config = <&mtl_tx_setup>;
> > -        mdio0 {
> > -            #address-cells = <1>;
> > -            #size-cells = <0>;
> > -            compatible = "snps,dwmac-mdio";
> > -            phy1: ethernet-phy@0 {
> > -                reg = <0>;
> > -            };
> > -        };
> > -    };
> > -  - |
> > -    gmac1: ethernet@f8010000 {
> > -        compatible = "snps,dwmac-4.10a", "snps,dwmac";
> > -        reg = <0xf8010000 0x4000>;
> > -        interrupts = <0 98 4>;
> > -        interrupt-names = "macirq";
> > -        clock-names = "stmmaceth", "ptp_ref";
> > -        clocks = <&clock 4>, <&clock 5>;
> > -        phy-mode = "rgmii";
> > -        snps,txpbl = <8>;
> > -        snps,rxpbl = <2>;
> > -        snps,aal;
> > -        snps,tso;
> > -
> > -        snps,axi-config {
> > -            snps,wr_osr_lmt = <0xf>;
> > -            snps,rd_osr_lmt = <0xf>;
> > -            snps,blen = <256 128 64 32 0 0 0>;
> > -        };
> > -
> > -        snps,mtl-rx-config {
> > -            snps,rx-queues-to-use = <1>;
> > -            snps,rx-sched-sp;
> > -            queue0 {
> > -               snps,dcb-algorithm;
> > -               snps,map-to-dma-channel = <0x0>;
> > -               snps,priority = <0x0>;
> > -            };
> > -        };
> > -
> > -        snps,mtl-tx-config {
> > -            snps,tx-queues-to-use = <2>;
> > -            snps,tx-sched-wrr;
> > -            queue0 {
> > -                snps,weight = <0x10>;
> > -                snps,dcb-algorithm;
> > -                snps,priority = <0x0>;
> > -            };
> > -
> > -            queue1 {
> > -                snps,avb-algorithm;
> > -                snps,send_slope = <0x1000>;
> > -                snps,idle_slope = <0x1000>;
> > -                snps,high_credit = <0x3E800>;
> > -                snps,low_credit = <0xFFC18000>;
> > -                snps,priority = <0x1>;
> > -            };
> > -        };
> > -    };
> > -
> > -# FIXME: We should set it, but it would report all the generic
> > -# properties as additional properties.
> > -# additionalProperties: false
> > -
> >  ...
> > -- 
> > 2.29.2
> > 

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2020-12-16  9:11 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14  9:15 [PATCH 00/25] net: stmmac: Fix clocks/reset-related procedures Serge Semin
2020-12-14  9:15 ` [PATCH 01/25] dt-bindings: net: dwmac: Validate PBL for all IP-cores Serge Semin
2020-12-15 17:14   ` Rob Herring
2020-12-14  9:15 ` [PATCH 02/25] dt-bindings: net: dwmac: Extend number of PBL values Serge Semin
2020-12-15 17:14   ` Rob Herring
2020-12-14  9:15 ` [PATCH 03/25] dt-bindings: net: dwmac: Fix the TSO property declaration Serge Semin
2020-12-15 17:22   ` Rob Herring
2020-12-16  8:59     ` Serge Semin
2020-12-14  9:15 ` [PATCH 04/25] dt-bindings: net: dwmac: Refactor snps,*-config properties Serge Semin
2020-12-14 14:30   ` Rob Herring
2020-12-15  8:54     ` Serge Semin
2020-12-15 14:08       ` Rob Herring
2020-12-16  6:24         ` Serge Semin
2020-12-14  9:15 ` [PATCH 05/25] dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description Serge Semin
2020-12-15 17:30   ` Rob Herring
2020-12-14  9:15 ` [PATCH 06/25] dt-bindings: net: dwmac: Add Tx/Rx clock sources Serge Semin
2020-12-15 17:32   ` Rob Herring
2020-12-16  6:02     ` Serge Semin
2020-12-15 17:32   ` Rob Herring
2020-12-14  9:15 ` [PATCH 07/25] dt-bindings: net: dwmac: Detach Generic DW MAC bindings Serge Semin
2020-12-15 17:50   ` Rob Herring
2020-12-16  9:10     ` Serge Semin
2020-12-14  9:15 ` [PATCH 08/25] net: stmmac: Add snps,*-config sub-nodes support Serge Semin
2020-12-14  9:15 ` [PATCH 09/25] net: stmmac: dwmac-rk: Cleanup STMMAC DT-config in remove cb Serge Semin
2020-12-14  9:16 ` [PATCH 10/25] net: stmmac: dwmac-sti: " Serge Semin
2020-12-14  9:16 ` [PATCH 11/25] net: stmmac: dwmac-stm32: " Serge Semin
2020-12-14  9:16 ` [PATCH 12/25] net: stmmac: Directly call reverse methods in stmmac_probe_config_dt() Serge Semin
2020-12-14  9:16 ` [PATCH 13/25] net: stmmac: Fix clocks left enabled on glue-probes failure Serge Semin
2020-12-14  9:16 ` [PATCH 14/25] net: stmmac: Use optional clock request method to get stmmaceth Serge Semin
2020-12-14  9:16 ` [PATCH 15/25] net: stmmac: Use optional clock request method to get pclk Serge Semin
2020-12-14  9:16 ` [PATCH 16/25] net: stmmac: Use optional clock request method to get ptp_clk Serge Semin
2020-12-14  9:16 ` [PATCH 17/25] net: stmmac: Use optional reset control API to work with stmmaceth Serge Semin
2020-12-14  9:16 ` [PATCH 18/25] net: stmmac: dwc-qos: Cleanup STMMAC platform data clock pointers Serge Semin
2020-12-14  9:16 ` [PATCH 19/25] net: stmmac: dwc-qos: Use dev_err_probe() for probe errors handling Serge Semin
2020-12-14  9:16 ` [PATCH 20/25] net: stmmac: Add Tx/Rx platform clocks support Serge Semin
2020-12-14  9:16 ` [PATCH 21/25] net: stmmac: dwc-qos: Discard Tx/Rx clocks request Serge Semin
2020-12-14  9:16 ` [PATCH 22/25] net: stmmac: dwmac-imx: Discard Tx clock request Serge Semin
2020-12-14  9:16 ` [PATCH 23/25] net: stmmac: Call stmmaceth clock as system clock in warn-message Serge Semin
2020-12-14  9:16 ` [PATCH 24/25] net: stmmac: Use pclk to set MDC clock frequency Serge Semin
2020-12-14  9:16 ` [PATCH 25/25] net: stmmac: dwc-qos: Save master/slave clocks in the plat-data Serge Semin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).