[PATCH v2 00/24] net: stmmac: Fix clocks/reset-related procedures

[PATCH v2 00/24] net: stmmac: Fix clocks/reset-related procedures

[Serge Semin]

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.

Link: https://lore.kernel.org/netdev/20201214091616.13545-1-Sergey.Semin@baikalelectronics.ru
Changelog v2:
- Discard "snps" vendor-prefix from the new AXI/MTL-Tx/Rx config
  sub-nodes.
- Add the new sub-nodes "axi-config", "mtl-rx-config" and "mtl-tx-config"
  to the DW *MAC bindings describing the nodes the now deprecated
  config-properties were supposed to refer to.
- Fix invalid identation in the "snps,route-*" property settings.
- Discard the patch
  [PATCH 15/25] net: stmmac: Use optional clock request method to get pclk
  since the corresponding functionality has already been integrated into
  the driver.
- Rebase on top of the kernel 5.11-rc7.

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 (24):
  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 {axi,mtl-rx,mtl-tx}-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 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      | 154 +++++
 .../devicetree/bindings/net/snps,dwmac.yaml   | 578 ++++++++++--------
 .../stmicro/stmmac/dwmac-dwc-qos-eth.c        |  90 +--
 .../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 |  85 ++-
 include/linux/stmmac.h                        |   2 +
 11 files changed, 616 insertions(+), 355 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

-- 
2.29.2

[PATCH v2 01/24] dt-bindings: net: dwmac: Validate PBL for all IP-cores

[Serge Semin]

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>
Reviewed-by: Rob Herring <robh@kernel.org>

---

Changelog v2:
- Use correct syntax of the JSON pointers, so the later would begin
  with a '/' after the '#'.
---
 .../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 0642b0f59491..40a002770441 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -265,6 +265,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:
@@ -290,49 +316,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

[PATCH v2 02/24] dt-bindings: net: dwmac: Extend number of PBL values

[Serge Semin]

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 result 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. 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>
Reviewed-by: Rob Herring <robh@kernel.org>

Changelog v2:
- Use correct syntax of the JSON pointers, so the later would begin
  with a '/' after the '#'.
---
 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 40a002770441..cb68a8dcafd7 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -267,23 +267,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

[PATCH v2 03/24] dt-bindings: net: dwmac: Fix the TSO property declaration

[Serge Semin]

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. The original allwinner sunXi bindings file also didn't have the
TSO-related property declared. Taking all of that into account 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.

Changelog v2:
- Use correct syntax of the JSON pointers, so the later would begin
  with a '/' after the '#'.
---
 .../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 cb68a8dcafd7..03d58bf9965f 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
@@ -317,34 +341,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

[PATCH v2 04/24] dt-bindings: net: dwmac: Refactor snps,*-config properties

[Serge Semin]

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" properties have been marked as deprecated in favor of
the added by this commit "axi-config", "mtl-rx-config" and "mtl-tx-config"
sub-nodes respectively.

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

---

Note this change will work only if DT-schema tool is fixed like this:

--- a/meta-schemas/nodes.yaml	2021-02-08 14:20:56.732447780 +0300
+++ b/meta-schemas/nodes.yaml	2021-02-08 14:21:00.736492245 +0300
@@ -22,6 +22,7 @@
     - unevaluatedProperties
     - deprecated
     - required
+    - not
     - allOf
     - anyOf
     - oneOf

So a property with name "not" would be allowed and the "not-required"
pattern would work.

Changelog v2:
- Add the new sub-nodes "axi-config", "mtl-rx-config" and "mtl-tx-config"
  describing the nodes now deprecated properties were supposed to
  refer to.
- Fix invalid identation in the "snps,route-*" property settings.
- Use correct syntax of the JSON pointers, so the later would begin
  with a '/' after the '#'.
---
 .../devicetree/bindings/net/snps,dwmac.yaml   | 389 +++++++++++++-----
 1 file changed, 297 insertions(+), 92 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 03d58bf9965f..4dda9ffa822c 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -150,73 +150,264 @@ properties:
       in a different mode than the PHY in order to function.
 
   snps,axi-config:
+    deprecated: true
     $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
+      AXI BUS Mode parameters. Phandle to a node that contains the properties
+      described in the 'axi-config' sub-node.
+
+  axi-config:
+    type: object
+    description: AXI BUS Mode parameters
+
+    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:
+    deprecated: true
     $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, bitmask of the tagged frames priorities assigned to
-            the queue
+      Multiple RX Queues parameters. Phandle to a node that contains the
+      properties described in the 'mtl-rx-config' sub-node.
+
+  mtl-rx-config:
+    type: object
+    description: Multiple RX Queues parameters
+
+    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:
+    deprecated: true
     $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, bitmask of the priorities assigned to the queue.
-            When a PFC frame is received with priorities matching the bitmask,
-            the queue is blocked from transmitting for the pause time specified
-            in the PFC frame.
+      Multiple TX Queues parameters. Phandle to a node that contains the
+      properties described in the 'mtl-tx-config' sub-node.
+
+  mtl-tx-config:
+    type: object
+    description: Multiple TX Queues parameters
+
+    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
@@ -345,41 +536,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>;
@@ -407,6 +563,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;
+
+        axi-config {
+            snps,wr_osr_lmt = <0xf>;
+            snps,rd_osr_lmt = <0xf>;
+            snps,blen = <256 128 64 32 0 0 0>;
+        };
+
+        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>;
+            };
+        };
+
+        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

[PATCH v2 05/24] dt-bindings: net: dwmac: Elaborate stmmaceth/pclk description

[Serge Semin]

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>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../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 4dda9ffa822c..21e53427551c 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

[PATCH v2 06/24] dt-bindings: net: dwmac: Add Tx/Rx clock sources

[Serge Semin]

Generic DW *MAC can be connected to an external Transmit 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>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../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 21e53427551c..56baf8e6bf17 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

[PATCH v2 07/24] dt-bindings: net: dwmac: Detach Generic DW MAC bindings

[Serge Semin]

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>

---

Changelog v2:
- Add a note to the snps,dwmac-generic.yaml bindings file description of
  a requirement to create a new DT bindings file for the vendor-specific
  versions of the DW MAC.
---
 .../bindings/net/snps,dwmac-generic.yaml      | 154 ++++++++++++++++++
 .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------
 2 files changed, 155 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..6c3bf5b2f890
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
@@ -0,0 +1,154 @@
+# 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>
+
+description:
+  The primary purpose of this bindings file is to validate the Generic
+  Synopsys Desginware MAC DT nodes defined in accordance with the select
+  schema. All new vendor-specific versions of the DW *MAC IP-cores must
+  be described in a dedicated DT bindings file.
+
+# 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;
+
+      axi-config {
+        snps,wr_osr_lmt = <0xf>;
+        snps,rd_osr_lmt = <0xf>;
+        snps,blen = <256 128 64 32 0 0 0>;
+      };
+
+      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>;
+        };
+      };
+
+      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 56baf8e6bf17..bdc437b14878 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
@@ -555,88 +502,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;
-
-        axi-config {
-            snps,wr_osr_lmt = <0xf>;
-            snps,rd_osr_lmt = <0xf>;
-            snps,blen = <256 128 64 32 0 0 0>;
-        };
-
-        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>;
-            };
-        };
-
-        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

[PATCH v2 08/24] net: stmmac: Add {axi,mtl-rx,mtl-tx}-config sub-nodes support

[Serge Semin]

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 sub-nodes to the DW
MAC device DT node with vendor-prefixless names. Make sure the STMMAC
driver supports them.

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

---

Changelog v2:
- Discard "snps" vendor-prefix from the new AXI/MTL Tx/Rx config
  sub-nodes.
---
 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 6dc9f10414e4..1815fe36b62f 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, "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, "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, "mtl-tx-config");
 	if (!tx_node) {
 		of_node_put(rx_node);
 		return ret;
-- 
2.29.2

[PATCH v2 09/24] net: stmmac: dwmac-rk: Cleanup STMMAC DT-config in remove cb

[Serge Semin]

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

[PATCH v2 10/24] net: stmmac: dwmac-sti: Cleanup STMMAC DT-config in remove cb

[Serge Semin]

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

[PATCH v2 11/24] net: stmmac: dwmac-stm32: Cleanup STMMAC DT-config in remove cb

[Serge Semin]

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

[PATCH v2 12/24] net: stmmac: Directly call reverse methods in stmmac_probe_config_dt()

[Serge Semin]

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 | 23 ++++++++++---------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 1815fe36b62f..c9feac70ca77 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -402,7 +402,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	struct device_node *np = pdev->dev.of_node;
 	struct plat_stmmacenet_data *plat;
 	struct stmmac_dma_cfg *dma_cfg;
-	void *ret;
 	int rc;
 
 	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
@@ -458,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);
 
@@ -536,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;
 
@@ -564,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")) {
@@ -582,7 +579,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 
 	plat->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
 	if (IS_ERR(plat->pclk)) {
-		ret = plat->pclk;
+		rc = PTR_ERR(plat->pclk);
 		goto error_pclk_get;
 	}
 	clk_prepare_enable(plat->pclk);
@@ -601,7 +598,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	plat->stmmac_rst = devm_reset_control_get_optional(&pdev->dev,
 							   STMMAC_RESOURCE_NAME);
 	if (IS_ERR(plat->stmmac_rst)) {
-		ret = plat->stmmac_rst;
+		rc = PTR_ERR(plat->stmmac_rst);
 		goto error_hw_init;
 	}
 
@@ -611,8 +608,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 ret;
+	return ERR_PTR(rc);
 }
 
 /**
-- 
2.29.2

[PATCH v2 13/24] net: stmmac: Fix clocks left enabled on glue-probes failure

[Serge Semin]

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 103d2448e9e0..56b914b5527a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -665,6 +665,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 26b971cd4da5..b371842d9337 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5157,8 +5157,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 c9feac70ca77..ff66c470f07f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -621,11 +621,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

[PATCH v2 14/24] net: stmmac: Use optional clock request method to get stmmaceth

[Serge Semin]

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 | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index ff66c470f07f..a66467baf30a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -566,16 +566,17 @@ 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 may use the platform
+	 * clocks pointers to preserve their own clock-descriptors.
+	 */
+	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_optional(&pdev->dev, "pclk");
 	if (IS_ERR(plat->pclk)) {
-- 
2.29.2

[PATCH v2 15/24] net: stmmac: Use optional clock request method to get ptp_clk

[Serge Semin]

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 b371842d9337..4f1bf8f6538b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5289,8 +5289,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 a66467baf30a..9a7c94622c36 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -586,10 +586,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

[PATCH v2 16/24] net: stmmac: Use optional reset control API to work with stmmaceth

[Serge Semin]

Since commit bb3222f71b57 ("net: stmmac: platform: use optional clk/reset
get APIs") a manual implementation of the optional device reset control
functionality has been replaced with using the
devm_reset_control_get_optional() method. But for some reason the optional
reset control handler usage hasn't been fixed and preserved the
NULL-checking statements. There is no need in that 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.

Fixes: bb3222f71b57 ("net: stmmac: platform: use optional clk/reset get APIs")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4f1bf8f6538b..a8dec219c295 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4935,15 +4935,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);
@@ -5155,8 +5153,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);
-- 
2.29.2

[PATCH v2 17/24] net: stmmac: dwc-qos: Cleanup STMMAC platform data clock pointers

[Serge Semin]

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 27254b27d7ed..20b3696fb776 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 int 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 PTR_ERR(plat_dat->stmmac_clk);
+		return PTR_ERR(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;
 	}
 
-	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 0;
 
 disable:
 	clk_disable_unprepare(plat_dat->stmmac_clk);
+	plat_dat->stmmac_clk = NULL;
+
 	return 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;
 }
@@ -301,12 +315,12 @@ static int 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);
@@ -377,6 +391,7 @@ static int 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:
@@ -385,6 +400,7 @@ static int 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);
@@ -394,6 +410,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

[PATCH v2 18/24] net: stmmac: dwc-qos: Use dev_err_probe() for probe errors handling

[Serge Semin]

There is a very handy dev_err_probe() method to handle the deferred probe
error number. It reduces the code size and identations, 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 | 6 ++----
 1 file changed, 2 insertions(+), 4 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 20b3696fb776..b71f0c3faebe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -466,10 +466,8 @@ static int dwc_eth_dwmac_probe(struct platform_device *pdev)
 
 	ret = data->probe(pdev, plat_dat, &stmmac_res);
 	if (ret < 0) {
-		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

[PATCH v2 19/24] net: stmmac: Add Tx/Rx platform clocks support

[Serge Semin]

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 | 22 +++++++++++++++++++
 include/linux/stmmac.h                        |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 9a7c94622c36..a6e35c84e135 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -585,6 +585,22 @@ 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)) {
@@ -609,6 +625,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);
@@ -630,6 +650,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 15ca6b4167cc..cec970adaf2e 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -186,6 +186,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

[PATCH v2 20/24] net: stmmac: dwc-qos: Discard Tx/Rx clocks request

[Serge Semin]

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 b71f0c3faebe..f315ca395e12 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 int dwc_qos_probe(struct platform_device *pdev,
 		goto disable;
 	}
 
-	plat_dat->pclk = clk;
+	plat_dat->tx_clk = clk;
 
 	return 0;
 
@@ -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);
 }
@@ -299,6 +298,11 @@ static int 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);
@@ -321,30 +325,10 @@ static int 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);
@@ -385,10 +369,6 @@ static int 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;
@@ -405,8 +385,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

[PATCH v2 21/24] net: stmmac: dwmac-imx: Discard Tx clock request

[Serge Semin]

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 223f69da7e95..8b2c7f1ba745 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) {
@@ -251,7 +241,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

[PATCH v2 22/24] net: stmmac: Call stmmaceth clock as system clock in warn-message

[Serge Semin]

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. While at it add error message printout if
actual CSR clock failed to be requested.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index a6e35c84e135..7cbde9d99133 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;
 	}
 	clk_prepare_enable(plat->stmmac_clk);
@@ -581,6 +581,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	plat->pclk = devm_clk_get_optional(&pdev->dev, "pclk");
 	if (IS_ERR(plat->pclk)) {
 		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);
-- 
2.29.2

[PATCH v2 23/24] net: stmmac: Use pclk to set MDC clock frequency

[Serge Semin]

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 a8dec219c295..03acf14d76de 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

[PATCH v2 24/24] net: stmmac: dwc-qos: Save master/slave clocks in the plat-data

[Serge Semin]

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 f315ca395e12..bb2297638805 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c
@@ -313,6 +313,8 @@ static int 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);
@@ -323,7 +325,7 @@ static int 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)) {
@@ -371,9 +373,10 @@ static int 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:
 	return err;
 }
@@ -392,6 +395,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

Re: [PATCH v2 00/24] net: stmmac: Fix clocks/reset-related procedures

[Jakub Kicinski]

On Mon, 8 Feb 2021 16:55:44 +0300 Serge Semin wrote:
> 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.

Hi Serge!

You've submitted 60 patches at once, that's a lot of patches, in netdev
we limit submissions to 15 patches at a time to avoid overwhelming
reviewers. 

At a glance the patches seem to mix fixes which affect existing,
supported systems (eg. error patch reference leaks) with extensions
required to support your platform. Can the two be separated?
The fixes for existing bugs should be targeting net (Subject: 
[PATCH net]) and patches to support your platform net-next (Subject:
[PATCH net-next]).

Right now the patches are not tagged so our build bot tried applying
them to net-next and they failed to apply, so I need to toss them away.

Andrew, others, please chime in if I'm misreading the contents of the
series or if you have additional guidance!

Re: [PATCH v2 00/24] net: stmmac: Fix clocks/reset-related procedures

[Serge Semin]

On Mon, Feb 08, 2021 at 11:05:21AM -0800, Jakub Kicinski wrote:
> On Mon, 8 Feb 2021 16:55:44 +0300 Serge Semin wrote:
> > 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.
> 
> Hi Serge!
> 
> You've submitted 60 patches at once, that's a lot of patches, in netdev
> we limit submissions to 15 patches at a time to avoid overwhelming
> reviewers. 
> 
> At a glance the patches seem to mix fixes which affect existing,
> supported systems (eg. error patch reference leaks) with extensions
> required to support your platform. Can the two be separated?
> The fixes for existing bugs should be targeting net (Subject: 
> [PATCH net]) and patches to support your platform net-next (Subject:
> [PATCH net-next]).
> 
> Right now the patches are not tagged so our build bot tried applying
> them to net-next and they failed to apply, so I need to toss them away.
> 
> Andrew, others, please chime in if I'm misreading the contents of the
> series or if you have additional guidance!

Hi Jakub,
Of course I know about the "too-big-series-to-review" rule. That's why I've
split all my work up into three patchsets. Actually this one is the very
first patchset, which I've submitted over two months ago but noone except
Rob gave me any review comment. So I've decided to post all the work,
to so called depict a scale of the work, which needs to be reviewed.

Anyway I thought it would be obvious from the cover-letters which patchsets
should be applied first, which next. This one states that it's a very
first series. The rest of the patchsets contains a link to the
previous one they were supposed to be applied to. Perhaps I should have
stated that in a clearer way.

Regarding having over 15 patches in this series. Normally it's not that
strict rule. There are even bigger series can be found submitted,
reviewed and accepted in the kernel. Of course sometimes it's hard to
review even 15 patches due to complexity of the changes. But most of
the alterations posted here are trivial and shouldn't be difficult to
review. That's why I felt free to post it as is.

Of course I agree with you. It would be too much to review over 60
patches at once. Let's review one series at a time then. This one is
the very first one. Please start with it.

Regarding splitting this series up. Well, normally there is no such
requirement to split the fixes and feature into different series.
(Though I am not surprised to get such request from net-subsystem. You
always prefer to do things in your special way. ^_^) So in this series
the fixes have been structured together and have been submitted first
in the order, but after DT-bindings related patches. Anyway if you
want it, I'll split the patchset up into two. The first one will be
targeted to "net" and will have all the fixes. The second one will
contain the bindings-related modifications and the clocks-related feature
implementation. It will be sent to net-next. I'll do that in v2. But
at the current stage could you start reviewing this series the way
it is? I'll take into account your comments and add your tags in the
split v2 patchsets if any.

Thanks
-Sergey

Re: [PATCH v2 03/24] dt-bindings: net: dwmac: Fix the TSO property declaration

[Rob Herring]

On Mon, 08 Feb 2021 16:55:47 +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. The original allwinner sunXi bindings file also didn't have the
> TSO-related property declared. Taking all of that into account 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.
> 
> Changelog v2:
> - Use correct syntax of the JSON pointers, so the later would begin
>   with a '/' after the '#'.
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   | 52 +++++++++----------
>  1 file changed, 24 insertions(+), 28 deletions(-)
> 

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

Re: [PATCH v2 04/24] dt-bindings: net: dwmac: Refactor snps,*-config properties

[Rob Herring]

On Mon, Feb 08, 2021 at 04:55:48PM +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" properties have been marked as deprecated in favor of
> the added by this commit "axi-config", "mtl-rx-config" and "mtl-tx-config"
> sub-nodes respectively.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> ---
> 
> Note this change will work only if DT-schema tool is fixed like this:
> 
> --- a/meta-schemas/nodes.yaml	2021-02-08 14:20:56.732447780 +0300
> +++ b/meta-schemas/nodes.yaml	2021-02-08 14:21:00.736492245 +0300
> @@ -22,6 +22,7 @@
>      - unevaluatedProperties
>      - deprecated
>      - required
> +    - not
>      - allOf
>      - anyOf
>      - oneOf

Can you send me a patch or GH PR. There is another way to express. More 
below.

> 
> So a property with name "not" would be allowed and the "not-required"
> pattern would work.
> 
> Changelog v2:
> - Add the new sub-nodes "axi-config", "mtl-rx-config" and "mtl-tx-config"
>   describing the nodes now deprecated properties were supposed to
>   refer to.
> - Fix invalid identation in the "snps,route-*" property settings.
> - Use correct syntax of the JSON pointers, so the later would begin
>   with a '/' after the '#'.
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   | 389 +++++++++++++-----
>  1 file changed, 297 insertions(+), 92 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 03d58bf9965f..4dda9ffa822c 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -150,73 +150,264 @@ properties:
>        in a different mode than the PHY in order to function.
>  
>    snps,axi-config:
> +    deprecated: true
>      $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
> +      AXI BUS Mode parameters. Phandle to a node that contains the properties
> +      described in the 'axi-config' sub-node.
> +
> +  axi-config:
> +    type: object
> +    description: AXI BUS Mode parameters
> +
> +    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:

You could keep these pointing to child nodes to avoid driver changes.

> +    deprecated: true
>      $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, bitmask of the tagged frames priorities assigned to
> -            the queue
> +      Multiple RX Queues parameters. Phandle to a node that contains the
> +      properties described in the 'mtl-rx-config' sub-node.
> +
> +  mtl-rx-config:
> +    type: object
> +    description: Multiple RX Queues parameters
> +
> +    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

This 'not: ..." could be:

properties:
  snps,route-avcp: false
  snps,route-ptp: false
  snps,route-dcbcp: false
  snps,route-up: false
  snps,route-multi-broad: false

Not sure which one is better. Using required everywhere or more 
concise...

(Really, 'route' should have taken a value and the schema would be 
greatly simplified. Oh well.)

> +
> +    additionalProperties: false
> +
> +    # Choose one of the RX scheduling algorithms
> +    not:
> +      required:
> +        - snps,rx-sched-sp
> +        - snps,rx-sched-wsp

I guess this is the problematic one. The rest should be hidden behind 
conditionals (a common loophole in meta-schema checks). You could do 
that here:

allOf:
  - not:
      ...

But why not just make one of the 2 properties required? You're already 
changing things. 

Rob

Re: [PATCH v2 07/24] dt-bindings: net: dwmac: Detach Generic DW MAC bindings

[Rob Herring]

On Mon, Feb 08, 2021 at 04:55:51PM +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.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> ---
> 
> Changelog v2:
> - Add a note to the snps,dwmac-generic.yaml bindings file description of
>   a requirement to create a new DT bindings file for the vendor-specific
>   versions of the DW MAC.
> ---
>  .../bindings/net/snps,dwmac-generic.yaml      | 154 ++++++++++++++++++
>  .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------
>  2 files changed, 155 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..6c3bf5b2f890
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> @@ -0,0 +1,154 @@
> +# 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>
> +
> +description:
> +  The primary purpose of this bindings file is to validate the Generic
> +  Synopsys Desginware MAC DT nodes defined in accordance with the select
> +  schema. All new vendor-specific versions of the DW *MAC IP-cores must
> +  be described in a dedicated DT bindings file.
> +
> +# 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?)?$"

Use '' instead of "" and you can skip the double \.

With that,

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

> +        - items:
> +            - pattern: "^snps,dwmac-[0-9]+\\.[0-9]+a?$"
> +            - const: snps,dwmac
> +        - items:
> +            - pattern: "^snps,dwxgmac-[0-9]+\\.[0-9]+a?$"
> +            - const: snps,dwxgmac

Re: [PATCH v2 16/24] net: stmmac: Use optional reset control API to work with stmmaceth

[Jisheng Zhang]

Hi,

On Mon, 8 Feb 2021 16:56:00 +0300 Serge Semin wrote:


> 
> Since commit bb3222f71b57 ("net: stmmac: platform: use optional clk/reset
> get APIs") a manual implementation of the optional device reset control
> functionality has been replaced with using the
> devm_reset_control_get_optional() method. But for some reason the optional
> reset control handler usage hasn't been fixed and preserved the
> NULL-checking statements. There is no need in that 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.
> 
> Fixes: bb3222f71b57 ("net: stmmac: platform: use optional clk/reset get APIs")

The patch itself looks good, but the Fix tag isn't necessary since the
patch is a clean up rather than a bug fix. Can you please drop it in next
version?

Thanks

> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 4f1bf8f6538b..a8dec219c295 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4935,15 +4935,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);
> @@ -5155,8 +5153,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);
> --
> 2.29.2
> 

Re: [PATCH v2 04/24] dt-bindings: net: dwmac: Refactor snps,*-config properties

[Serge Semin]

On Tue, Feb 09, 2021 at 04:26:08PM -0600, Rob Herring wrote:
> On Mon, Feb 08, 2021 at 04:55:48PM +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" properties have been marked as deprecated in favor of
> > the added by this commit "axi-config", "mtl-rx-config" and "mtl-tx-config"
> > sub-nodes respectively.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > ---
> > 
> > Note this change will work only if DT-schema tool is fixed like this:
> > 
> > --- a/meta-schemas/nodes.yaml	2021-02-08 14:20:56.732447780 +0300
> > +++ b/meta-schemas/nodes.yaml	2021-02-08 14:21:00.736492245 +0300
> > @@ -22,6 +22,7 @@
> >      - unevaluatedProperties
> >      - deprecated
> >      - required
> > +    - not
> >      - allOf
> >      - anyOf
> >      - oneOf
> 

> Can you send me a patch or GH PR. There is another way to express. More 
> below.

Ok. I'll send a patch. To what email and mailing lists shall I send it
to?

> 
> > 
> > So a property with name "not" would be allowed and the "not-required"
> > pattern would work.
> > 
> > Changelog v2:
> > - Add the new sub-nodes "axi-config", "mtl-rx-config" and "mtl-tx-config"
> >   describing the nodes now deprecated properties were supposed to
> >   refer to.
> > - Fix invalid identation in the "snps,route-*" property settings.
> > - Use correct syntax of the JSON pointers, so the later would begin
> >   with a '/' after the '#'.
> > ---
> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 389 +++++++++++++-----
> >  1 file changed, 297 insertions(+), 92 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index 03d58bf9965f..4dda9ffa822c 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -150,73 +150,264 @@ properties:
> >        in a different mode than the PHY in order to function.
> >  
> >    snps,axi-config:
> > +    deprecated: true
> >      $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
> > +      AXI BUS Mode parameters. Phandle to a node that contains the properties
> > +      described in the 'axi-config' sub-node.
> > +
> > +  axi-config:
> > +    type: object
> > +    description: AXI BUS Mode parameters
> > +
> > +    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:
> 

> You could keep these pointing to child nodes to avoid driver changes.

Right, but I'd prefer having the AXI/MTL-config nodes directly found
because they are supposed to be defined as sub-nodes anyway. Having
the snps-prefixed AXI/MTL-config properties with phandle reference are
going to be marked as deprecated. By doing so we'll discourage
defining the DW MAC-related configs scattered around the new dts-es. 

> 
> > +    deprecated: true
> >      $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, bitmask of the tagged frames priorities assigned to
> > -            the queue
> > +      Multiple RX Queues parameters. Phandle to a node that contains the
> > +      properties described in the 'mtl-rx-config' sub-node.
> > +
> > +  mtl-rx-config:
> > +    type: object
> > +    description: Multiple RX Queues parameters
> > +
> > +    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
> 

> This 'not: ..." could be:
> 
> properties:
>   snps,route-avcp: false
>   snps,route-ptp: false
>   snps,route-dcbcp: false
>   snps,route-up: false
>   snps,route-multi-broad: false
> 
> Not sure which one is better. Using required everywhere or more 
> concise...

Thanks for suggesting an alternative. I didn't figure out such option
myself. Though in this case since we need to use the required property
anyway, I'd prefer to have it used in the 'not' sub-schema too. At
least for uniformity and to simplify the conditional statement
readability, even if it causes a bit of the concise loss.

> 
> (Really, 'route' should have taken a value and the schema would be 
> greatly simplified. Oh well.)

Yeah, I had the same thought in mind when first saw that
boolean-properties hell. There are few more properties in this
bindings file which should have been also defined as taking values
instead of being booleans...

> 
> > +
> > +    additionalProperties: false
> > +
> > +    # Choose one of the RX scheduling algorithms
> > +    not:
> > +      required:
> > +        - snps,rx-sched-sp
> > +        - snps,rx-sched-wsp
> 

> I guess this is the problematic one. The rest should be hidden behind 
> conditionals (a common loophole in meta-schema checks). You could do 
> that here:
> 
> allOf:
>   - not:
>       ...

Oh, thanks. I can't believe I've missed that option. Though fixing the
dt-schema tool would be better than using the combining schema
keywords as a workaround.

> 
> But why not just make one of the 2 properties required? You're already 
> changing things. 

First of all the driver permits omitting all of them in the
corresponding nodes and implicitly using one of the properties by
default (the same thing is for the MTL Tx-queues). If we made some of
them being required we would have broken the driver dts-contract,
which is not good. Secondly I'd have to fix the
arch/arm/boot/dts/artpec6.dtsi dts file too, which would have been an
additional patch in the series, additional work, additional review
from the platform maintainer, additional merge path, etc. So to speak
that would cause more troubles, than just using the "not:" statement
here.)

-Sergey

> 
> Rob

Re: [PATCH v2 07/24] dt-bindings: net: dwmac: Detach Generic DW MAC bindings

[Serge Semin]

On Tue, Feb 09, 2021 at 04:32:58PM -0600, Rob Herring wrote:
> On Mon, Feb 08, 2021 at 04:55:51PM +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.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > ---
> > 
> > Changelog v2:
> > - Add a note to the snps,dwmac-generic.yaml bindings file description of
> >   a requirement to create a new DT bindings file for the vendor-specific
> >   versions of the DW MAC.
> > ---
> >  .../bindings/net/snps,dwmac-generic.yaml      | 154 ++++++++++++++++++
> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------
> >  2 files changed, 155 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..6c3bf5b2f890
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml
> > @@ -0,0 +1,154 @@
> > +# 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>
> > +
> > +description:
> > +  The primary purpose of this bindings file is to validate the Generic
> > +  Synopsys Desginware MAC DT nodes defined in accordance with the select
> > +  schema. All new vendor-specific versions of the DW *MAC IP-cores must
> > +  be described in a dedicated DT bindings file.
> > +
> > +# 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?)?$"
> 

> Use '' instead of "" and you can skip the double \.
> 
> With that,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Got it. Thanks.

-Sergey

> 
> > +        - items:
> > +            - pattern: "^snps,dwmac-[0-9]+\\.[0-9]+a?$"
> > +            - const: snps,dwmac
> > +        - items:
> > +            - pattern: "^snps,dwxgmac-[0-9]+\\.[0-9]+a?$"
> > +            - const: snps,dwxgmac

Re: [PATCH v2 16/24] net: stmmac: Use optional reset control API to work with stmmaceth

[Serge Semin]

On Wed, Feb 10, 2021 at 02:49:24PM +0800, Jisheng Zhang wrote:
> Hi,
> 
> On Mon, 8 Feb 2021 16:56:00 +0300 Serge Semin wrote:
> 
> 
> > 
> > Since commit bb3222f71b57 ("net: stmmac: platform: use optional clk/reset
> > get APIs") a manual implementation of the optional device reset control
> > functionality has been replaced with using the
> > devm_reset_control_get_optional() method. But for some reason the optional
> > reset control handler usage hasn't been fixed and preserved the
> > NULL-checking statements. There is no need in that 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.
> > 
> > Fixes: bb3222f71b57 ("net: stmmac: platform: use optional clk/reset get APIs")
> 

> The patch itself looks good, but the Fix tag isn't necessary since the
> patch is a clean up rather than a bug fix. Can you please drop it in next
> version?

Ok. I'll remove it.

-Sergey

> 
> Thanks
> 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++-----------
> >  1 file changed, 8 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 4f1bf8f6538b..a8dec219c295 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -4935,15 +4935,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);
> > @@ -5155,8 +5153,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);
> > --
> > 2.29.2
> > 
> 

Re: [PATCH v2 04/24] dt-bindings: net: dwmac: Refactor snps,*-config properties

[Serge Semin]

On Thu, Feb 11, 2021 at 12:58:00AM +0300, Serge Semin wrote:
> On Tue, Feb 09, 2021 at 04:26:08PM -0600, Rob Herring wrote:
> > On Mon, Feb 08, 2021 at 04:55:48PM +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" properties have been marked as deprecated in favor of
> > > the added by this commit "axi-config", "mtl-rx-config" and "mtl-tx-config"
> > > sub-nodes respectively.
> > > 
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > 
> > > ---
> > > 
> > > Note this change will work only if DT-schema tool is fixed like this:
> > > 
> > > --- a/meta-schemas/nodes.yaml	2021-02-08 14:20:56.732447780 +0300
> > > +++ b/meta-schemas/nodes.yaml	2021-02-08 14:21:00.736492245 +0300
> > > @@ -22,6 +22,7 @@
> > >      - unevaluatedProperties
> > >      - deprecated
> > >      - required
> > > +    - not
> > >      - allOf
> > >      - anyOf
> > >      - oneOf
> > 
> 
> > Can you send me a patch or GH PR. There is another way to express. More 
> > below.
> 
> Ok. I'll send a patch. To what email and mailing lists shall I send it
> to?

Rob, any comments on my questions above and below?

-Sergey

> 
> > 
> > > 
> > > So a property with name "not" would be allowed and the "not-required"
> > > pattern would work.
> > > 
> > > Changelog v2:
> > > - Add the new sub-nodes "axi-config", "mtl-rx-config" and "mtl-tx-config"
> > >   describing the nodes now deprecated properties were supposed to
> > >   refer to.
> > > - Fix invalid identation in the "snps,route-*" property settings.
> > > - Use correct syntax of the JSON pointers, so the later would begin
> > >   with a '/' after the '#'.
> > > ---
> > >  .../devicetree/bindings/net/snps,dwmac.yaml   | 389 +++++++++++++-----
> > >  1 file changed, 297 insertions(+), 92 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > index 03d58bf9965f..4dda9ffa822c 100644
> > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > @@ -150,73 +150,264 @@ properties:
> > >        in a different mode than the PHY in order to function.
> > >  
> > >    snps,axi-config:
> > > +    deprecated: true
> > >      $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
> > > +      AXI BUS Mode parameters. Phandle to a node that contains the properties
> > > +      described in the 'axi-config' sub-node.
> > > +
> > > +  axi-config:
> > > +    type: object
> > > +    description: AXI BUS Mode parameters
> > > +
> > > +    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:
> > 
> 
> > You could keep these pointing to child nodes to avoid driver changes.
> 
> Right, but I'd prefer having the AXI/MTL-config nodes directly found
> because they are supposed to be defined as sub-nodes anyway. Having
> the snps-prefixed AXI/MTL-config properties with phandle reference are
> going to be marked as deprecated. By doing so we'll discourage
> defining the DW MAC-related configs scattered around the new dts-es. 
> 
> > 
> > > +    deprecated: true
> > >      $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, bitmask of the tagged frames priorities assigned to
> > > -            the queue
> > > +      Multiple RX Queues parameters. Phandle to a node that contains the
> > > +      properties described in the 'mtl-rx-config' sub-node.
> > > +
> > > +  mtl-rx-config:
> > > +    type: object
> > > +    description: Multiple RX Queues parameters
> > > +
> > > +    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
> > 
> 
> > This 'not: ..." could be:
> > 
> > properties:
> >   snps,route-avcp: false
> >   snps,route-ptp: false
> >   snps,route-dcbcp: false
> >   snps,route-up: false
> >   snps,route-multi-broad: false
> > 
> > Not sure which one is better. Using required everywhere or more 
> > concise...
> 
> Thanks for suggesting an alternative. I didn't figure out such option
> myself. Though in this case since we need to use the required property
> anyway, I'd prefer to have it used in the 'not' sub-schema too. At
> least for uniformity and to simplify the conditional statement
> readability, even if it causes a bit of the concise loss.
> 
> > 
> > (Really, 'route' should have taken a value and the schema would be 
> > greatly simplified. Oh well.)
> 
> Yeah, I had the same thought in mind when first saw that
> boolean-properties hell. There are few more properties in this
> bindings file which should have been also defined as taking values
> instead of being booleans...
> 
> > 
> > > +
> > > +    additionalProperties: false
> > > +
> > > +    # Choose one of the RX scheduling algorithms
> > > +    not:
> > > +      required:
> > > +        - snps,rx-sched-sp
> > > +        - snps,rx-sched-wsp
> > 
> 
> > I guess this is the problematic one. The rest should be hidden behind 
> > conditionals (a common loophole in meta-schema checks). You could do 
> > that here:
> > 
> > allOf:
> >   - not:
> >       ...
> 
> Oh, thanks. I can't believe I've missed that option. Though fixing the
> dt-schema tool would be better than using the combining schema
> keywords as a workaround.
> 
> > 
> > But why not just make one of the 2 properties required? You're already 
> > changing things. 
> 
> First of all the driver permits omitting all of them in the
> corresponding nodes and implicitly using one of the properties by
> default (the same thing is for the MTL Tx-queues). If we made some of
> them being required we would have broken the driver dts-contract,
> which is not good. Secondly I'd have to fix the
> arch/arm/boot/dts/artpec6.dtsi dts file too, which would have been an
> additional patch in the series, additional work, additional review
> from the platform maintainer, additional merge path, etc. So to speak
> that would cause more troubles, than just using the "not:" statement
> here.)
> 
> -Sergey
> 
> > 
> > Rob