mbox series

[00/25] net: stmmac: Fix clocks/reset-related procedures

Message ID 20201214091616.13545-1-Sergey.Semin@baikalelectronics.ru
Headers show
Series net: stmmac: Fix clocks/reset-related procedures | expand

Message

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

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

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

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

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

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

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

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

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

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

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

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

Comments

Serge Semin Dec. 15, 2020, 8:54 a.m. UTC | #1
Hello Rob,

On Mon, Dec 14, 2020 at 08:30:06AM -0600, Rob Herring wrote:
> On Mon, Dec 14, 2020 at 12:15:54PM +0300, Serge Semin wrote:

> > Currently the "snps,axi-config", "snps,mtl-rx-config" and

> > "snps,mtl-tx-config" properties are declared as a single phandle reference

> > to a node with corresponding parameters defined. That's not good for

> > several reasons. First of all scattering around a device tree some

> > particular device-specific configs with no visual relation to that device

> > isn't suitable from maintainability point of view. That leads to a

> > disturbed representation of the actual device tree mixing actual device

> > nodes and some vendor-specific configs. Secondly using the same configs

> > set for several device nodes doesn't represent well the devices structure,

> > since the interfaces these configs describe in hardware belong to

> > different devices and may actually differ. In the later case having the

> > configs node separated from the corresponding device nodes gets to be

> > even unjustified.

> > 

> > So instead of having a separate DW *MAC configs nodes we suggest to

> > define them as sub-nodes of the device nodes, which interfaces they

> > actually describe. By doing so we'll make the DW *MAC nodes visually

> > correct describing all the aspects of the IP-core configuration. Thus

> > we'll be able to describe the configs sub-nodes bindings right in the

> > snps,dwmac.yaml file.

> > 

> > Note the former "snps,axi-config", "snps,mtl-rx-config" and

> > "snps,mtl-tx-config" bindings have been marked as deprecated.

> > 

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

> > 

> > ---

> > 

> > Note the current DT schema tool requires the vendor-specific properties to be

> > defined in accordance with the schema: dtschema/meta-schemas/vendor-props.yaml

> > It means the property can be;

> > - boolean,

> > - string,

> > - defined with $ref and additional constraints,

> > - defined with allOf: [ $ref ] and additional constraints.

> > 

> > The modification provided by this commit needs to extend that definition to

> > make the DT schema tool correctly parse this schema. That is we need to let

> > the vendors-specific properties to also accept the oneOf-based combined

> > sub-schema. Like this:

> > 

> > --- a/dtschema/meta-schemas/vendor-props.yaml

> > +++ b/dtschema/meta-schemas/vendor-props.yaml

> > @@ -48,15 +48,24 @@

> >        - properties:   # A property with a type and additional constraints

> >            $ref:

> >              pattern: "types.yaml#[\/]{0,1}definitions\/.*"

> > -          allOf:

> > -            items:

> > -              - properties:

> > +

> > +        if:

> > +          not:

> > +            required:

> > +              - $ref

> > +        then:

> > +          patternProperties:

> > +            "^(all|one)Of$":

> > +              contains:

> > +                properties:

> >                    $ref:

> >                      pattern: "types.yaml#[\/]{0,1}definitions\/.*"

> >                  required:

> >                    - $ref

> > -        oneOf:

> > +

> > +        anyOf:

> >            - required: [ $ref ]

> >            - required: [ allOf ]

> > +          - required: [ oneOf ]

> > 

> >  ...

> > ---

> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 380 +++++++++++++-----

> >  1 file changed, 288 insertions(+), 92 deletions(-)

> > 

> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > index 0dd543c6c08e..44aa88151cba 100644

> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > @@ -150,69 +150,251 @@ properties:

> >        in a different mode than the PHY in order to function.

> >  

> >    snps,axi-config:

> > -    $ref: /schemas/types.yaml#definitions/phandle

> > -    description:

> > -      AXI BUS Mode parameters. Phandle to a node that can contain the

> > -      following properties

> > -        * snps,lpi_en, enable Low Power Interface

> > -        * snps,xit_frm, unlock on WoL

> > -        * snps,wr_osr_lmt, max write outstanding req. limit

> > -        * snps,rd_osr_lmt, max read outstanding req. limit

> > -        * snps,kbbe, do not cross 1KiB boundary.

> > -        * snps,blen, this is a vector of supported burst length.

> > -        * snps,fb, fixed-burst

> > -        * snps,mb, mixed-burst

> > -        * snps,rb, rebuild INCRx Burst

> > +    description: AXI BUS Mode parameters

> > +    oneOf:

> > +      - deprecated: true

> > +        $ref: /schemas/types.yaml#definitions/phandle

> > +      - type: object

> > +        properties:

> 


> Anywhere have have the same node/property string meaning 2 different 

> things is a pain, let's not create another one. 


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

> Just define a new node 

> 'axi-config'. Or just put all the properties into the node directly. 

> Grouping them has little purpose.


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

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

-Sergey

> 

> > +          snps,lpi_en:

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description: Enable Low Power Interface

> > +

> > +          snps,xit_frm:

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description: Unlock on WoL

> > +

> > +          snps,wr_osr_lmt:

> > +            $ref: /schemas/types.yaml#definitions/uint32

> > +            description: Max write outstanding req. limit

> > +            default: 1

> > +            minimum: 0

> > +            maximum: 15

> > +

> > +          snps,rd_osr_lmt:

> > +            $ref: /schemas/types.yaml#definitions/uint32

> > +            description: Max read outstanding req. limit

> > +            default: 1

> > +            minimum: 0

> > +            maximum: 15

> > +

> > +          snps,kbbe:

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description: Do not cross 1KiB boundary

> > +

> > +          snps,blen:

> > +            $ref: /schemas/types.yaml#definitions/uint32-array

> > +            description: A vector of supported burst lengths

> > +            minItems: 7

> > +            maxItems: 7

> > +            items:

> > +              enum: [256, 128, 64, 32, 16, 8, 4, 0]

> > +

> > +          snps,fb:

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description: Fixed-burst

> > +

> > +          snps,mb:

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description: Mixed-burst

> > +

> > +          snps,rb:

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description: Rebuild INCRx Burst

> > +

> > +        additionalProperties: false

> >  

> >    snps,mtl-rx-config:

> > -    $ref: /schemas/types.yaml#definitions/phandle

> >      description:

> > -      Multiple RX Queues parameters. Phandle to a node that can

> > -      contain the following properties

> > -        * snps,rx-queues-to-use, number of RX queues to be used in the

> > -          driver

> > -        * Choose one of these RX scheduling algorithms

> > -          * snps,rx-sched-sp, Strict priority

> > -          * snps,rx-sched-wsp, Weighted Strict priority

> > -        * For each RX queue

> > -          * Choose one of these modes

> > -            * snps,dcb-algorithm, Queue to be enabled as DCB

> > -            * snps,avb-algorithm, Queue to be enabled as AVB

> > -          * snps,map-to-dma-channel, Channel to map

> > -          * Specifiy specific packet routing

> > -            * snps,route-avcp, AV Untagged Control packets

> > -            * snps,route-ptp, PTP Packets

> > -            * snps,route-dcbcp, DCB Control Packets

> > -            * snps,route-up, Untagged Packets

> > -            * snps,route-multi-broad, Multicast & Broadcast Packets

> > -          * snps,priority, RX queue priority (Range 0x0 to 0xF)

> > +      Multiple RX Queues parameters

> > +    oneOf:

> > +      - deprecated: true

> > +        $ref: /schemas/types.yaml#definitions/phandle

> > +      - type: object

> > +        properties:

> > +          snps,rx-queues-to-use:

> > +            $ref: /schemas/types.yaml#definitions/uint32

> > +            description: Number of RX queues to be used in the driver

> > +            default: 1

> > +            minimum: 1

> > +

> > +        patternProperties:

> > +          "^snps,rx-sched-(sp|wsp)$":

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description: Strict/Weighted Strict RX scheduling priority

> > +

> > +          "^queue[0-9]$":

> > +            type: object

> > +            description: Each RX Queue parameters

> > +

> > +            properties:

> > +              snps,map-to-dma-channel:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: DMA channel to map

> > +

> > +              snps,priority:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: RX queue priority

> > +                minimum: 0

> > +                maximum: 15

> > +

> > +            patternProperties:

> > +              "^snps,(dcb|avb)-algorithm$":

> > +                $ref: /schemas/types.yaml#definitions/flag

> > +                description: Enable Queue as DCB/AVB

> > +

> > +              "^snps,route-(avcp|ptp|dcbcp|up|multi-broad)$":

> > +                $ref: /schemas/types.yaml#definitions/flag

> > +                description:

> > +                  AV Untagged/PTP/DCB Control/Untagged/Multicast & Broadcast

> > +                  packets routing respectively.

> > +

> > +            additionalProperties: false

> > +

> > +            # Choose only one of the Queue modes and the packets routing

> > +            allOf:

> > +              - not:

> > +                  required:

> > +                    - snps,dcb-algorithm

> > +                    - snps,avb-algorithm

> > +              - oneOf:

> > +                  - required:

> > +                      - snps,route-avcp

> > +                  - required:

> > +                      - snps,route-ptp

> > +                  - required:

> > +                      - snps,route-dcbcp

> > +                  - required:

> > +                      - snps,route-up

> > +                  - required:

> > +                      - snps,route-multi-broad

> > +                  - not:

> > +                      anyOf:

> > +                        - required:

> > +                            - snps,route-avcp

> > +                        - required:

> > +                            - snps,route-ptp

> > +                        - required:

> > +                            - snps,route-dcbcp

> > +                        - required:

> > +                            - snps,route-up

> > +                        - required:

> > +                            - snps,route-multi-broad

> > +

> > +        additionalProperties: false

> > +

> > +        # Choose one of the RX scheduling algorithms

> > +        not:

> > +          required:

> > +            - snps,rx-sched-sp

> > +            - snps,rx-sched-wsp

> >  

> >    snps,mtl-tx-config:

> > -    $ref: /schemas/types.yaml#definitions/phandle

> >      description:

> > -      Multiple TX Queues parameters. Phandle to a node that can

> > -      contain the following properties

> > -        * snps,tx-queues-to-use, number of TX queues to be used in the

> > -          driver

> > -        * Choose one of these TX scheduling algorithms

> > -          * snps,tx-sched-wrr, Weighted Round Robin

> > -          * snps,tx-sched-wfq, Weighted Fair Queuing

> > -          * snps,tx-sched-dwrr, Deficit Weighted Round Robin

> > -          * snps,tx-sched-sp, Strict priority

> > -        * For each TX queue

> > -          * snps,weight, TX queue weight (if using a DCB weight

> > -            algorithm)

> > -          * Choose one of these modes

> > -            * snps,dcb-algorithm, TX queue will be working in DCB

> > -            * snps,avb-algorithm, TX queue will be working in AVB

> > -              [Attention] Queue 0 is reserved for legacy traffic

> > -                          and so no AVB is available in this queue.

> > -          * Configure Credit Base Shaper (if AVB Mode selected)

> > -            * snps,send_slope, enable Low Power Interface

> > -            * snps,idle_slope, unlock on WoL

> > -            * snps,high_credit, max write outstanding req. limit

> > -            * snps,low_credit, max read outstanding req. limit

> > -          * snps,priority, TX queue priority (Range 0x0 to 0xF)

> > +      Multiple TX Queues parameters

> > +    oneOf:

> > +      - deprecated: true

> > +        $ref: /schemas/types.yaml#definitions/phandle

> > +      - type: object

> > +        properties:

> > +          snps,tx-queues-to-use:

> > +            $ref: /schemas/types.yaml#definitions/uint32

> > +            description: Number of TX queues to be used in the driver

> > +            default: 1

> > +            minimum: 1

> > +

> > +        patternProperties:

> > +          "^snps,tx-sched-(wrr|wfq|dwrr|sp)$":

> > +            $ref: /schemas/types.yaml#definitions/flag

> > +            description:

> > +              Weighted Round Robin, Weighted Fair Queuing,

> > +              Deficit Weighted Round Robin or Strict TX scheduling priority.

> > +

> > +          "^queue[0-9]$":

> > +            type: object

> > +            description: Each TX Queue parameters

> > +

> > +            properties:

> > +              snps,priority:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: TX queue priority

> > +                minimum: 0

> > +                maximum: 15

> > +

> > +              snps,weight:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: TX queue weight (if using a DCB weight algorithm)

> > +                minimum: 0

> > +                maximum: 0x1FFFFF

> > +

> > +              snps,send_slope:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: Enable Low Power Interface

> > +                minimum: 0

> > +                maximum: 0x3FFF

> > +

> > +              snps,idle_slope:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: Unlock on WoL

> > +                minimum: 0

> > +                maximum: 0x1FFFFF

> > +

> > +              snps,high_credit:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: Max write outstanding req. limit

> > +                minimum: 0

> > +                maximum: 0x1FFFFFFF

> > +

> > +              snps,low_credit:

> > +                $ref: /schemas/types.yaml#definitions/uint32

> > +                description: Max read outstanding req. limit

> > +                minimum: 0

> > +                maximum: 0x1FFFFFFF

> > +

> > +            patternProperties:

> > +              "^snps,(dcb|avb)-algorithm$":

> > +                $ref: /schemas/types.yaml#definitions/flag

> > +                description:

> > +                  Enable Queue as DCB/AVB. Note Queue 0 is reserved for legacy

> > +                  traffic and so no AVB is available in this queue.

> > +

> > +            additionalProperties: false

> > +

> > +            # Choose only one of the Queue modes

> > +            not:

> > +              required:

> > +                - snps,dcb-algorithm

> > +                - snps,avb-algorithm

> > +

> > +            # Credit Base Shaper is configurable for AVB Mode only

> > +            dependencies:

> > +              snps,send_slope: ["snps,avb-algorithm"]

> > +              snps,idle_slope: ["snps,avb-algorithm"]

> > +              snps,high_credit: ["snps,avb-algorithm"]

> > +              snps,low_credit: ["snps,avb-algorithm"]

> > +

> > +        additionalProperties: false

> > +

> > +        # Choose one of the TX scheduling algorithms

> > +        oneOf:

> > +          - required:

> > +              - snps,tx-sched-wrr

> > +          - required:

> > +              - snps,tx-sched-wfq

> > +          - required:

> > +              - snps,tx-sched-dwrr

> > +          - required:

> > +              - snps,tx-sched-sp

> > +          - not:

> > +              anyOf:

> > +                - required:

> > +                    - snps,tx-sched-wrr

> > +                - required:

> > +                    - snps,tx-sched-wfq

> > +                - required:

> > +                    - snps,tx-sched-dwrr

> > +                - required:

> > +                    - snps,tx-sched-sp

> >  

> >    snps,reset-gpio:

> >      deprecated: true

> > @@ -342,41 +524,6 @@ additionalProperties: true

> >  

> >  examples:

> >    - |

> > -    stmmac_axi_setup: stmmac-axi-config {

> > -        snps,wr_osr_lmt = <0xf>;

> > -        snps,rd_osr_lmt = <0xf>;

> > -        snps,blen = <256 128 64 32 0 0 0>;

> > -    };

> > -

> > -    mtl_rx_setup: rx-queues-config {

> > -        snps,rx-queues-to-use = <1>;

> > -        snps,rx-sched-sp;

> > -        queue0 {

> > -            snps,dcb-algorithm;

> > -            snps,map-to-dma-channel = <0x0>;

> > -            snps,priority = <0x0>;

> > -        };

> > -    };

> > -

> > -    mtl_tx_setup: tx-queues-config {

> > -        snps,tx-queues-to-use = <2>;

> > -        snps,tx-sched-wrr;

> > -        queue0 {

> > -            snps,weight = <0x10>;

> > -            snps,dcb-algorithm;

> > -            snps,priority = <0x0>;

> > -        };

> > -

> > -        queue1 {

> > -            snps,avb-algorithm;

> > -            snps,send_slope = <0x1000>;

> > -            snps,idle_slope = <0x1000>;

> > -            snps,high_credit = <0x3E800>;

> > -            snps,low_credit = <0xFFC18000>;

> > -            snps,priority = <0x1>;

> > -        };

> > -    };

> > -

> >      gmac0: ethernet@e0800000 {

> >          compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";

> >          reg = <0xe0800000 0x8000>;

> > @@ -404,6 +551,55 @@ examples:

> >              };

> >          };

> >      };

> > +  - |

> > +    gmac1: ethernet@f8010000 {

> > +        compatible = "snps,dwmac-4.10a", "snps,dwmac";

> > +        reg = <0xf8010000 0x4000>;

> > +        interrupts = <0 98 4>;

> > +        interrupt-names = "macirq";

> > +        clock-names = "stmmaceth", "ptp_ref";

> > +        clocks = <&clock 4>, <&clock 5>;

> > +        phy-mode = "rgmii";

> > +        snps,txpbl = <8>;

> > +        snps,rxpbl = <2>;

> > +        snps,aal;

> > +        snps,tso;

> > +

> > +        snps,axi-config {

> > +            snps,wr_osr_lmt = <0xf>;

> > +            snps,rd_osr_lmt = <0xf>;

> > +            snps,blen = <256 128 64 32 0 0 0>;

> > +        };

> > +

> > +        snps,mtl-rx-config {

> > +            snps,rx-queues-to-use = <1>;

> > +            snps,rx-sched-sp;

> > +            queue0 {

> > +               snps,dcb-algorithm;

> > +               snps,map-to-dma-channel = <0x0>;

> > +               snps,priority = <0x0>;

> > +            };

> > +        };

> > +

> > +        snps,mtl-tx-config {

> > +            snps,tx-queues-to-use = <2>;

> > +            snps,tx-sched-wrr;

> > +            queue0 {

> > +                snps,weight = <0x10>;

> > +                snps,dcb-algorithm;

> > +                snps,priority = <0x0>;

> > +            };

> > +

> > +            queue1 {

> > +                snps,avb-algorithm;

> > +                snps,send_slope = <0x1000>;

> > +                snps,idle_slope = <0x1000>;

> > +                snps,high_credit = <0x3E800>;

> > +                snps,low_credit = <0xFFC18000>;

> > +                snps,priority = <0x1>;

> > +            };

> > +        };

> > +    };

> >  

> >  # FIXME: We should set it, but it would report all the generic

> >  # properties as additional properties.

> > -- 

> > 2.29.2

> >
Rob Herring (Arm) Dec. 15, 2020, 2:08 p.m. UTC | #2
On Tue, Dec 15, 2020 at 2:54 AM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
>

> Hello Rob,

>

> On Mon, Dec 14, 2020 at 08:30:06AM -0600, Rob Herring wrote:

> > On Mon, Dec 14, 2020 at 12:15:54PM +0300, Serge Semin wrote:

> > > Currently the "snps,axi-config", "snps,mtl-rx-config" and

> > > "snps,mtl-tx-config" properties are declared as a single phandle reference

> > > to a node with corresponding parameters defined. That's not good for

> > > several reasons. First of all scattering around a device tree some

> > > particular device-specific configs with no visual relation to that device

> > > isn't suitable from maintainability point of view. That leads to a

> > > disturbed representation of the actual device tree mixing actual device

> > > nodes and some vendor-specific configs. Secondly using the same configs

> > > set for several device nodes doesn't represent well the devices structure,

> > > since the interfaces these configs describe in hardware belong to

> > > different devices and may actually differ. In the later case having the

> > > configs node separated from the corresponding device nodes gets to be

> > > even unjustified.

> > >

> > > So instead of having a separate DW *MAC configs nodes we suggest to

> > > define them as sub-nodes of the device nodes, which interfaces they

> > > actually describe. By doing so we'll make the DW *MAC nodes visually

> > > correct describing all the aspects of the IP-core configuration. Thus

> > > we'll be able to describe the configs sub-nodes bindings right in the

> > > snps,dwmac.yaml file.

> > >

> > > Note the former "snps,axi-config", "snps,mtl-rx-config" and

> > > "snps,mtl-tx-config" bindings have been marked as deprecated.

> > >

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

> > >

> > > ---

> > >

> > > Note the current DT schema tool requires the vendor-specific properties to be

> > > defined in accordance with the schema: dtschema/meta-schemas/vendor-props.yaml

> > > It means the property can be;

> > > - boolean,

> > > - string,

> > > - defined with $ref and additional constraints,

> > > - defined with allOf: [ $ref ] and additional constraints.

> > >

> > > The modification provided by this commit needs to extend that definition to

> > > make the DT schema tool correctly parse this schema. That is we need to let

> > > the vendors-specific properties to also accept the oneOf-based combined

> > > sub-schema. Like this:

> > >

> > > --- a/dtschema/meta-schemas/vendor-props.yaml

> > > +++ b/dtschema/meta-schemas/vendor-props.yaml

> > > @@ -48,15 +48,24 @@

> > >        - properties:   # A property with a type and additional constraints

> > >            $ref:

> > >              pattern: "types.yaml#[\/]{0,1}definitions\/.*"

> > > -          allOf:

> > > -            items:

> > > -              - properties:

> > > +

> > > +        if:

> > > +          not:

> > > +            required:

> > > +              - $ref

> > > +        then:

> > > +          patternProperties:

> > > +            "^(all|one)Of$":

> > > +              contains:

> > > +                properties:

> > >                    $ref:

> > >                      pattern: "types.yaml#[\/]{0,1}definitions\/.*"

> > >                  required:

> > >                    - $ref

> > > -        oneOf:

> > > +

> > > +        anyOf:

> > >            - required: [ $ref ]

> > >            - required: [ allOf ]

> > > +          - required: [ oneOf ]

> > >

> > >  ...

> > > ---

> > >  .../devicetree/bindings/net/snps,dwmac.yaml   | 380 +++++++++++++-----

> > >  1 file changed, 288 insertions(+), 92 deletions(-)

> > >

> > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > > index 0dd543c6c08e..44aa88151cba 100644

> > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > > @@ -150,69 +150,251 @@ properties:

> > >        in a different mode than the PHY in order to function.

> > >

> > >    snps,axi-config:

> > > -    $ref: /schemas/types.yaml#definitions/phandle

> > > -    description:

> > > -      AXI BUS Mode parameters. Phandle to a node that can contain the

> > > -      following properties

> > > -        * snps,lpi_en, enable Low Power Interface

> > > -        * snps,xit_frm, unlock on WoL

> > > -        * snps,wr_osr_lmt, max write outstanding req. limit

> > > -        * snps,rd_osr_lmt, max read outstanding req. limit

> > > -        * snps,kbbe, do not cross 1KiB boundary.

> > > -        * snps,blen, this is a vector of supported burst length.

> > > -        * snps,fb, fixed-burst

> > > -        * snps,mb, mixed-burst

> > > -        * snps,rb, rebuild INCRx Burst

> > > +    description: AXI BUS Mode parameters

> > > +    oneOf:

> > > +      - deprecated: true

> > > +        $ref: /schemas/types.yaml#definitions/phandle

> > > +      - type: object

> > > +        properties:

> >

>

> > Anywhere have have the same node/property string meaning 2 different

> > things is a pain, let's not create another one.

>

> IIUC you meant that having a node and property with the same name

> isn't ok. Right? If so could you explain why not? especially seeing

> the property is expected to be set with phandle reference to that

> node. That seemed like a perfect solution to me. We wouldn't need to

> introduce a new property/node name, but just deprecate the

> corresponding name to be a property.


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

> > Just define a new node

> > 'axi-config'. Or just put all the properties into the node directly.

> > Grouping them has little purpose.

>

> Hm, you suggest to remove the vendor prefix, right?


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

> If so what about

> the rest of the changes introduced here in this patch? They concern

> "snps,mtl-tx-config" and "snps,mtl-rx-config" properties (please note

> these changes are a bit more complicated than once connected with

> "snps,axi-config"). Should I remove the vendor-prefix from them too?


Yes.

> Anyway that seems a bit questionable, because all the "snps,*-config"

> properties/nodes seems more vendor-specific than generic. Am I wrong

> in that matter?

>

> If you think they are generic, then the "{axi,mtl-rx,mtl-tx}-config"

> nodes most likely should be described in the dedicated DT schema...

>

> -Sergey

>
Rob Herring (Arm) Dec. 15, 2020, 5:22 p.m. UTC | #3
On Mon, Dec 14, 2020 at 12:15:53PM +0300, Serge Semin wrote:
> Indeed the STMMAC driver doesn't take the vendor-specific compatible

> string into account to parse the "snps,tso" boolean property. It just

> makes sure the node is compatible with DW MAC 4.x, 5.x and DW xGMAC

> IP-cores. Fix the conditional statement so the TSO-property would be

> evaluated for the compatibles having the corresponding IP-core version.

> 

> While at it move the whole allOf-block from the tail of the binding file

> to the head of it, as it's normally done in the most of the DT schemas.

> 

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

> 

> ---

> 

> Note this won't break the bindings description, since the "snps,tso"

> property isn't parsed by the Allwinner SunX GMAC glue driver, but only

> by the generic platform DT-parser.


But still should be valid for Allwinner?

> ---

>  .../devicetree/bindings/net/snps,dwmac.yaml   | 52 +++++++++----------

>  1 file changed, 24 insertions(+), 28 deletions(-)

> 

> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> index e084fbbf976e..0dd543c6c08e 100644

> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> @@ -37,6 +37,30 @@ select:

>    required:

>      - compatible

>  

> +allOf:

> +  - $ref: "ethernet-controller.yaml#"

> +  - if:

> +      properties:

> +        compatible:

> +          contains:

> +            enum:

> +              - snps,dwmac-4.00

> +              - snps,dwmac-4.10a

> +              - snps,dwmac-4.20a

> +              - snps,dwmac-5.10a

> +              - snps,dwxgmac

> +              - snps,dwxgmac-2.10

> +

> +      required:

> +        - compatible

> +    then:

> +      properties:

> +        snps,tso:

> +          $ref: /schemas/types.yaml#definitions/flag

> +          description:

> +            Enables the TSO feature otherwise it will be managed by

> +            MAC HW capability register.


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

> +

>  properties:

>  

>    # We need to include all the compatibles from schemas that will

> @@ -314,34 +338,6 @@ dependencies:

>    snps,reset-active-low: ["snps,reset-gpio"]

>    snps,reset-delay-us: ["snps,reset-gpio"]

>  

> -allOf:

> -  - $ref: "ethernet-controller.yaml#"

> -  - if:

> -      properties:

> -        compatible:

> -          contains:

> -            enum:

> -              - allwinner,sun7i-a20-gmac


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

> -              - allwinner,sun8i-a83t-emac

> -              - allwinner,sun8i-h3-emac

> -              - allwinner,sun8i-r40-emac

> -              - allwinner,sun8i-v3s-emac

> -              - allwinner,sun50i-a64-emac

> -              - snps,dwmac-4.00

> -              - snps,dwmac-4.10a

> -              - snps,dwmac-4.20a

> -              - snps,dwxgmac

> -              - snps,dwxgmac-2.10

> -              - st,spear600-gmac

> -

> -    then:

> -      properties:

> -        snps,tso:

> -          $ref: /schemas/types.yaml#definitions/flag

> -          description:

> -            Enables the TSO feature otherwise it will be managed by

> -            MAC HW capability register.

> -

>  additionalProperties: true

>  

>  examples:

> -- 

> 2.29.2

>
Rob Herring (Arm) Dec. 15, 2020, 5:50 p.m. UTC | #4
On Mon, Dec 14, 2020 at 12:15:57PM +0300, Serge Semin wrote:
> Currently the snps,dwmac.yaml DT bindings file is used for both DT nodes

> describing generic DW MAC devices and as DT schema with common properties

> to be evaluated against a vendor-specific DW MAC IP-cores. Due to such

> dual-purpose design the "compatible" property of the common DW MAC schema

> needs to contain the vendor-specific strings to successfully pass the

> schema evaluation in case if it's referenced from the vendor-specific DT

> bindings. That's a bad design from maintainability point of view, since

> adding/removing any DW MAC-based device bindings requires the common

> schema modification. In order to fix that let's detach the schema which

> provides the generic DW MAC DT nodes evaluation into a dedicated DT

> bindings file preserving the common DW MAC properties declaration in the

> snps,dwmac.yaml file. By doing so we'll still provide a common properties

> evaluation for each vendor-specific MAC bindings which refer to the

> common bindings file, while the generic DW MAC DT nodes will be checked

> against the new snps,dwmac-generic.yaml DT schema.


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

> 

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

> ---

>  .../bindings/net/snps,dwmac-generic.yaml      | 148 ++++++++++++++++++

>  .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------

>  2 files changed, 149 insertions(+), 138 deletions(-)

>  create mode 100644 Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

> 

> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

> new file mode 100644

> index 000000000000..f1b387911390

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

> @@ -0,0 +1,148 @@

> +# SPDX-License-Identifier: GPL-2.0

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/net/snps,dwmac-generic.yaml#

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

> +

> +title: Synopsys DesignWare Generic MAC Device Tree Bindings

> +

> +maintainers:

> +  - Alexandre Torgue <alexandre.torgue@st.com>

> +  - Giuseppe Cavallaro <peppe.cavallaro@st.com>

> +  - Jose Abreu <joabreu@synopsys.com>

> +

> +# Select the DT nodes, which have got compatible strings either as just a

> +# single string with IP-core name optionally followed by the IP version or

> +# two strings: one with IP-core name plus the IP version, another as just

> +# the IP-core name.

> +select:

> +  properties:

> +    compatible:

> +      oneOf:

> +        - items:

> +            - pattern: "^snps,dw(xg)+mac(-[0-9]+\\.[0-9]+a?)?$"

> +        - items:

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

> +            - const: snps,dwmac

> +        - items:

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

> +            - const: snps,dwxgmac

> +

> +  required:

> +    - compatible

> +

> +allOf:

> +  - $ref: snps,dwmac.yaml#

> +

> +properties:

> +  compatible:

> +    oneOf:

> +      - description: Generic Synopsys DW MAC

> +        oneOf:

> +          - items:

> +              - enum:

> +                  - snps,dwmac-3.50a

> +                  - snps,dwmac-3.610

> +                  - snps,dwmac-3.70a

> +                  - snps,dwmac-3.710

> +                  - snps,dwmac-4.00

> +                  - snps,dwmac-4.10a

> +                  - snps,dwmac-4.20a

> +              - const: snps,dwmac

> +          - const: snps,dwmac

> +      - description: Generic Synopsys DW xGMAC

> +        oneOf:

> +          - items:

> +              - enum:

> +                  - snps,dwxgmac-2.10

> +              - const: snps,dwxgmac

> +          - const: snps,dwxgmac

> +      - description: ST SPEAr SoC Family GMAC

> +        deprecated: true

> +        const: st,spear600-gmac

> +

> +  reg:

> +    maxItems: 1

> +

> +unevaluatedProperties: false

> +

> +examples:

> +  - |

> +    gmac0: ethernet@e0800000 {

> +      compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";

> +      reg = <0xe0800000 0x8000>;

> +      interrupt-parent = <&vic1>;

> +      interrupts = <24 23 22>;

> +      interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";

> +      mac-address = [000000000000]; /* Filled in by U-Boot */

> +      max-frame-size = <3800>;

> +      phy-mode = "gmii";

> +      snps,multicast-filter-bins = <256>;

> +      snps,perfect-filter-entries = <128>;

> +      rx-fifo-depth = <16384>;

> +      tx-fifo-depth = <16384>;

> +      clocks = <&clock>;

> +      clock-names = "stmmaceth";

> +      snps,axi-config = <&stmmac_axi_setup>;

> +      snps,mtl-rx-config = <&mtl_rx_setup>;

> +      snps,mtl-tx-config = <&mtl_tx_setup>;

> +      mdio0 {

> +        #address-cells = <1>;

> +        #size-cells = <0>;

> +        compatible = "snps,dwmac-mdio";

> +        phy1: ethernet-phy@0 {

> +          reg = <0>;

> +        };

> +      };

> +    };

> +  - |

> +    gmac1: ethernet@f8010000 {

> +      compatible = "snps,dwmac-4.10a", "snps,dwmac";

> +      reg = <0xf8010000 0x4000>;

> +      interrupts = <0 98 4>;

> +      interrupt-names = "macirq";

> +      clock-names = "stmmaceth", "ptp_ref";

> +      clocks = <&clock 4>, <&clock 5>;

> +      phy-mode = "rgmii";

> +      snps,txpbl = <8>;

> +      snps,rxpbl = <2>;

> +      snps,aal;

> +      snps,tso;

> +

> +      snps,axi-config {

> +        snps,wr_osr_lmt = <0xf>;

> +        snps,rd_osr_lmt = <0xf>;

> +        snps,blen = <256 128 64 32 0 0 0>;

> +      };

> +

> +      snps,mtl-rx-config {

> +        snps,rx-queues-to-use = <1>;

> +        snps,rx-sched-sp;

> +        queue0 {

> +          snps,dcb-algorithm;

> +          snps,map-to-dma-channel = <0x0>;

> +          snps,priority = <0x0>;

> +        };

> +      };

> +

> +      snps,mtl-tx-config {

> +        snps,tx-queues-to-use = <2>;

> +        snps,tx-sched-wrr;

> +

> +        queue0 {

> +          snps,weight = <0x10>;

> +          snps,dcb-algorithm;

> +          snps,priority = <0x0>;

> +        };

> +

> +        queue1 {

> +          snps,avb-algorithm;

> +          snps,send_slope = <0x1000>;

> +          snps,idle_slope = <0x1000>;

> +          snps,high_credit = <0x3E800>;

> +          snps,low_credit = <0x1FC18000>;

> +          snps,priority = <0x1>;

> +        };

> +      };

> +    };

> +...

> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> index 74820f491346..72b58f86bc41 100644

> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> @@ -11,31 +11,7 @@ maintainers:

>    - Giuseppe Cavallaro <peppe.cavallaro@st.com>

>    - Jose Abreu <joabreu@synopsys.com>

>  

> -# Select every compatible, including the deprecated ones. This way, we

> -# will be able to report a warning when we have that compatible, since

> -# we will validate the node thanks to the select, but won't report it

> -# as a valid value in the compatible property description

> -select:

> -  properties:

> -    compatible:

> -      contains:

> -        enum:

> -          - snps,dwmac

> -          - snps,dwmac-3.50a

> -          - snps,dwmac-3.610

> -          - snps,dwmac-3.70a

> -          - snps,dwmac-3.710

> -          - snps,dwmac-4.00

> -          - snps,dwmac-4.10a

> -          - snps,dwmac-4.20a

> -          - snps,dwxgmac

> -          - snps,dwxgmac-2.10

> -

> -          # Deprecated

> -          - st,spear600-gmac

> -

> -  required:

> -    - compatible

> +select: false

>  

>  allOf:

>    - $ref: "ethernet-controller.yaml#"

> @@ -62,35 +38,6 @@ allOf:

>              MAC HW capability register.

>  

>  properties:

> -

> -  # We need to include all the compatibles from schemas that will

> -  # include that schemas, otherwise compatible won't validate for

> -  # those.

> -  compatible:

> -    contains:

> -      enum:

> -        - allwinner,sun7i-a20-gmac

> -        - allwinner,sun8i-a83t-emac

> -        - allwinner,sun8i-h3-emac

> -        - allwinner,sun8i-r40-emac

> -        - allwinner,sun8i-v3s-emac

> -        - allwinner,sun50i-a64-emac

> -        - amlogic,meson6-dwmac

> -        - amlogic,meson8b-dwmac

> -        - amlogic,meson8m2-dwmac

> -        - amlogic,meson-gxbb-dwmac

> -        - amlogic,meson-axg-dwmac

> -        - snps,dwmac

> -        - snps,dwmac-3.50a

> -        - snps,dwmac-3.610

> -        - snps,dwmac-3.70a

> -        - snps,dwmac-3.710

> -        - snps,dwmac-4.00

> -        - snps,dwmac-4.10a

> -        - snps,dwmac-4.20a

> -        - snps,dwxgmac

> -        - snps,dwxgmac-2.10

> -

>    reg:

>      minItems: 1

>      maxItems: 2

> @@ -543,88 +490,4 @@ dependencies:

>    snps,reset-delay-us: ["snps,reset-gpio"]

>  

>  additionalProperties: true

> -

> -examples:

> -  - |

> -    gmac0: ethernet@e0800000 {

> -        compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";

> -        reg = <0xe0800000 0x8000>;

> -        interrupt-parent = <&vic1>;

> -        interrupts = <24 23 22>;

> -        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";

> -        mac-address = [000000000000]; /* Filled in by U-Boot */

> -        max-frame-size = <3800>;

> -        phy-mode = "gmii";

> -        snps,multicast-filter-bins = <256>;

> -        snps,perfect-filter-entries = <128>;

> -        rx-fifo-depth = <16384>;

> -        tx-fifo-depth = <16384>;

> -        clocks = <&clock>;

> -        clock-names = "stmmaceth";

> -        snps,axi-config = <&stmmac_axi_setup>;

> -        snps,mtl-rx-config = <&mtl_rx_setup>;

> -        snps,mtl-tx-config = <&mtl_tx_setup>;

> -        mdio0 {

> -            #address-cells = <1>;

> -            #size-cells = <0>;

> -            compatible = "snps,dwmac-mdio";

> -            phy1: ethernet-phy@0 {

> -                reg = <0>;

> -            };

> -        };

> -    };

> -  - |

> -    gmac1: ethernet@f8010000 {

> -        compatible = "snps,dwmac-4.10a", "snps,dwmac";

> -        reg = <0xf8010000 0x4000>;

> -        interrupts = <0 98 4>;

> -        interrupt-names = "macirq";

> -        clock-names = "stmmaceth", "ptp_ref";

> -        clocks = <&clock 4>, <&clock 5>;

> -        phy-mode = "rgmii";

> -        snps,txpbl = <8>;

> -        snps,rxpbl = <2>;

> -        snps,aal;

> -        snps,tso;

> -

> -        snps,axi-config {

> -            snps,wr_osr_lmt = <0xf>;

> -            snps,rd_osr_lmt = <0xf>;

> -            snps,blen = <256 128 64 32 0 0 0>;

> -        };

> -

> -        snps,mtl-rx-config {

> -            snps,rx-queues-to-use = <1>;

> -            snps,rx-sched-sp;

> -            queue0 {

> -               snps,dcb-algorithm;

> -               snps,map-to-dma-channel = <0x0>;

> -               snps,priority = <0x0>;

> -            };

> -        };

> -

> -        snps,mtl-tx-config {

> -            snps,tx-queues-to-use = <2>;

> -            snps,tx-sched-wrr;

> -            queue0 {

> -                snps,weight = <0x10>;

> -                snps,dcb-algorithm;

> -                snps,priority = <0x0>;

> -            };

> -

> -            queue1 {

> -                snps,avb-algorithm;

> -                snps,send_slope = <0x1000>;

> -                snps,idle_slope = <0x1000>;

> -                snps,high_credit = <0x3E800>;

> -                snps,low_credit = <0xFFC18000>;

> -                snps,priority = <0x1>;

> -            };

> -        };

> -    };

> -

> -# FIXME: We should set it, but it would report all the generic

> -# properties as additional properties.

> -# additionalProperties: false

> -

>  ...

> -- 

> 2.29.2

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

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

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

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

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

Ok.

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

> Yes.

Ok.

-Sergey

> 
> > Anyway that seems a bit questionable, because all the "snps,*-config"
> > properties/nodes seems more vendor-specific than generic. Am I wrong
> > in that matter?
> >
> > If you think they are generic, then the "{axi,mtl-rx,mtl-tx}-config"
> > nodes most likely should be described in the dedicated DT schema...
> >
> > -Sergey
> >
Serge Semin Dec. 16, 2020, 8:59 a.m. UTC | #6
On Tue, Dec 15, 2020 at 11:22:40AM -0600, Rob Herring wrote:
> On Mon, Dec 14, 2020 at 12:15:53PM +0300, Serge Semin wrote:

> > Indeed the STMMAC driver doesn't take the vendor-specific compatible

> > string into account to parse the "snps,tso" boolean property. It just

> > makes sure the node is compatible with DW MAC 4.x, 5.x and DW xGMAC

> > IP-cores. Fix the conditional statement so the TSO-property would be

> > evaluated for the compatibles having the corresponding IP-core version.

> > 

> > While at it move the whole allOf-block from the tail of the binding file

> > to the head of it, as it's normally done in the most of the DT schemas.

> > 

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

> > 

> > ---

> > 

> > Note this won't break the bindings description, since the "snps,tso"

> > property isn't parsed by the Allwinner SunX GMAC glue driver, but only

> > by the generic platform DT-parser.

> 


> But still should be valid for Allwinner?


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

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

> 

> > ---

> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 52 +++++++++----------

> >  1 file changed, 24 insertions(+), 28 deletions(-)

> > 

> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > index e084fbbf976e..0dd543c6c08e 100644

> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > @@ -37,6 +37,30 @@ select:

> >    required:

> >      - compatible

> >  

> > +allOf:

> > +  - $ref: "ethernet-controller.yaml#"

> > +  - if:

> > +      properties:

> > +        compatible:

> > +          contains:

> > +            enum:

> > +              - snps,dwmac-4.00

> > +              - snps,dwmac-4.10a

> > +              - snps,dwmac-4.20a

> > +              - snps,dwmac-5.10a

> > +              - snps,dwxgmac

> > +              - snps,dwxgmac-2.10

> > +

> > +      required:

> > +        - compatible

> > +    then:

> > +      properties:

> > +        snps,tso:

> > +          $ref: /schemas/types.yaml#definitions/flag

> > +          description:

> > +            Enables the TSO feature otherwise it will be managed by

> > +            MAC HW capability register.

> 


> BTW, I prefer that properties are defined unconditionally, and then 

> restricted in conditional schemas (or ones that include this schema).


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

> 

> > +

> >  properties:

> >  

> >    # We need to include all the compatibles from schemas that will

> > @@ -314,34 +338,6 @@ dependencies:

> >    snps,reset-active-low: ["snps,reset-gpio"]

> >    snps,reset-delay-us: ["snps,reset-gpio"]

> >  

> > -allOf:

> > -  - $ref: "ethernet-controller.yaml#"

> > -  - if:

> > -      properties:

> > -        compatible:

> > -          contains:

> > -            enum:

> > -              - allwinner,sun7i-a20-gmac

> 


> This does not have a fallback, so snps,tso is no longer validated. I 

> didn't check the rest.


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

-Sergey

> 

> > -              - allwinner,sun8i-a83t-emac

> > -              - allwinner,sun8i-h3-emac

> > -              - allwinner,sun8i-r40-emac

> > -              - allwinner,sun8i-v3s-emac

> > -              - allwinner,sun50i-a64-emac

> > -              - snps,dwmac-4.00

> > -              - snps,dwmac-4.10a

> > -              - snps,dwmac-4.20a

> > -              - snps,dwxgmac

> > -              - snps,dwxgmac-2.10

> > -              - st,spear600-gmac

> > -

> > -    then:

> > -      properties:

> > -        snps,tso:

> > -          $ref: /schemas/types.yaml#definitions/flag

> > -          description:

> > -            Enables the TSO feature otherwise it will be managed by

> > -            MAC HW capability register.

> > -

> >  additionalProperties: true

> >  

> >  examples:

> > -- 

> > 2.29.2

> >
Serge Semin Dec. 16, 2020, 9:10 a.m. UTC | #7
On Tue, Dec 15, 2020 at 11:50:02AM -0600, Rob Herring wrote:
> On Mon, Dec 14, 2020 at 12:15:57PM +0300, Serge Semin wrote:

> > Currently the snps,dwmac.yaml DT bindings file is used for both DT nodes

> > describing generic DW MAC devices and as DT schema with common properties

> > to be evaluated against a vendor-specific DW MAC IP-cores. Due to such

> > dual-purpose design the "compatible" property of the common DW MAC schema

> > needs to contain the vendor-specific strings to successfully pass the

> > schema evaluation in case if it's referenced from the vendor-specific DT

> > bindings. That's a bad design from maintainability point of view, since

> > adding/removing any DW MAC-based device bindings requires the common

> > schema modification. In order to fix that let's detach the schema which

> > provides the generic DW MAC DT nodes evaluation into a dedicated DT

> > bindings file preserving the common DW MAC properties declaration in the

> > snps,dwmac.yaml file. By doing so we'll still provide a common properties

> > evaluation for each vendor-specific MAC bindings which refer to the

> > common bindings file, while the generic DW MAC DT nodes will be checked

> > against the new snps,dwmac-generic.yaml DT schema.

> 


> I'm okay with the change, but it needs a big fat note that 

> snps,dwmac-generic.yaml should not have new users. New users should have 

> an SoC specific compatible. History has shown that even IP versions are 

> not enough to handle all the integration crap vendors do.


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

-Sergey

> 

> > 

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

> > ---

> >  .../bindings/net/snps,dwmac-generic.yaml      | 148 ++++++++++++++++++

> >  .../devicetree/bindings/net/snps,dwmac.yaml   | 139 +---------------

> >  2 files changed, 149 insertions(+), 138 deletions(-)

> >  create mode 100644 Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

> > 

> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

> > new file mode 100644

> > index 000000000000..f1b387911390

> > --- /dev/null

> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac-generic.yaml

> > @@ -0,0 +1,148 @@

> > +# SPDX-License-Identifier: GPL-2.0

> > +%YAML 1.2

> > +---

> > +$id: http://devicetree.org/schemas/net/snps,dwmac-generic.yaml#

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

> > +

> > +title: Synopsys DesignWare Generic MAC Device Tree Bindings

> > +

> > +maintainers:

> > +  - Alexandre Torgue <alexandre.torgue@st.com>

> > +  - Giuseppe Cavallaro <peppe.cavallaro@st.com>

> > +  - Jose Abreu <joabreu@synopsys.com>

> > +

> > +# Select the DT nodes, which have got compatible strings either as just a

> > +# single string with IP-core name optionally followed by the IP version or

> > +# two strings: one with IP-core name plus the IP version, another as just

> > +# the IP-core name.

> > +select:

> > +  properties:

> > +    compatible:

> > +      oneOf:

> > +        - items:

> > +            - pattern: "^snps,dw(xg)+mac(-[0-9]+\\.[0-9]+a?)?$"

> > +        - items:

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

> > +            - const: snps,dwmac

> > +        - items:

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

> > +            - const: snps,dwxgmac

> > +

> > +  required:

> > +    - compatible

> > +

> > +allOf:

> > +  - $ref: snps,dwmac.yaml#

> > +

> > +properties:

> > +  compatible:

> > +    oneOf:

> > +      - description: Generic Synopsys DW MAC

> > +        oneOf:

> > +          - items:

> > +              - enum:

> > +                  - snps,dwmac-3.50a

> > +                  - snps,dwmac-3.610

> > +                  - snps,dwmac-3.70a

> > +                  - snps,dwmac-3.710

> > +                  - snps,dwmac-4.00

> > +                  - snps,dwmac-4.10a

> > +                  - snps,dwmac-4.20a

> > +              - const: snps,dwmac

> > +          - const: snps,dwmac

> > +      - description: Generic Synopsys DW xGMAC

> > +        oneOf:

> > +          - items:

> > +              - enum:

> > +                  - snps,dwxgmac-2.10

> > +              - const: snps,dwxgmac

> > +          - const: snps,dwxgmac

> > +      - description: ST SPEAr SoC Family GMAC

> > +        deprecated: true

> > +        const: st,spear600-gmac

> > +

> > +  reg:

> > +    maxItems: 1

> > +

> > +unevaluatedProperties: false

> > +

> > +examples:

> > +  - |

> > +    gmac0: ethernet@e0800000 {

> > +      compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";

> > +      reg = <0xe0800000 0x8000>;

> > +      interrupt-parent = <&vic1>;

> > +      interrupts = <24 23 22>;

> > +      interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";

> > +      mac-address = [000000000000]; /* Filled in by U-Boot */

> > +      max-frame-size = <3800>;

> > +      phy-mode = "gmii";

> > +      snps,multicast-filter-bins = <256>;

> > +      snps,perfect-filter-entries = <128>;

> > +      rx-fifo-depth = <16384>;

> > +      tx-fifo-depth = <16384>;

> > +      clocks = <&clock>;

> > +      clock-names = "stmmaceth";

> > +      snps,axi-config = <&stmmac_axi_setup>;

> > +      snps,mtl-rx-config = <&mtl_rx_setup>;

> > +      snps,mtl-tx-config = <&mtl_tx_setup>;

> > +      mdio0 {

> > +        #address-cells = <1>;

> > +        #size-cells = <0>;

> > +        compatible = "snps,dwmac-mdio";

> > +        phy1: ethernet-phy@0 {

> > +          reg = <0>;

> > +        };

> > +      };

> > +    };

> > +  - |

> > +    gmac1: ethernet@f8010000 {

> > +      compatible = "snps,dwmac-4.10a", "snps,dwmac";

> > +      reg = <0xf8010000 0x4000>;

> > +      interrupts = <0 98 4>;

> > +      interrupt-names = "macirq";

> > +      clock-names = "stmmaceth", "ptp_ref";

> > +      clocks = <&clock 4>, <&clock 5>;

> > +      phy-mode = "rgmii";

> > +      snps,txpbl = <8>;

> > +      snps,rxpbl = <2>;

> > +      snps,aal;

> > +      snps,tso;

> > +

> > +      snps,axi-config {

> > +        snps,wr_osr_lmt = <0xf>;

> > +        snps,rd_osr_lmt = <0xf>;

> > +        snps,blen = <256 128 64 32 0 0 0>;

> > +      };

> > +

> > +      snps,mtl-rx-config {

> > +        snps,rx-queues-to-use = <1>;

> > +        snps,rx-sched-sp;

> > +        queue0 {

> > +          snps,dcb-algorithm;

> > +          snps,map-to-dma-channel = <0x0>;

> > +          snps,priority = <0x0>;

> > +        };

> > +      };

> > +

> > +      snps,mtl-tx-config {

> > +        snps,tx-queues-to-use = <2>;

> > +        snps,tx-sched-wrr;

> > +

> > +        queue0 {

> > +          snps,weight = <0x10>;

> > +          snps,dcb-algorithm;

> > +          snps,priority = <0x0>;

> > +        };

> > +

> > +        queue1 {

> > +          snps,avb-algorithm;

> > +          snps,send_slope = <0x1000>;

> > +          snps,idle_slope = <0x1000>;

> > +          snps,high_credit = <0x3E800>;

> > +          snps,low_credit = <0x1FC18000>;

> > +          snps,priority = <0x1>;

> > +        };

> > +      };

> > +    };

> > +...

> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > index 74820f491346..72b58f86bc41 100644

> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

> > @@ -11,31 +11,7 @@ maintainers:

> >    - Giuseppe Cavallaro <peppe.cavallaro@st.com>

> >    - Jose Abreu <joabreu@synopsys.com>

> >  

> > -# Select every compatible, including the deprecated ones. This way, we

> > -# will be able to report a warning when we have that compatible, since

> > -# we will validate the node thanks to the select, but won't report it

> > -# as a valid value in the compatible property description

> > -select:

> > -  properties:

> > -    compatible:

> > -      contains:

> > -        enum:

> > -          - snps,dwmac

> > -          - snps,dwmac-3.50a

> > -          - snps,dwmac-3.610

> > -          - snps,dwmac-3.70a

> > -          - snps,dwmac-3.710

> > -          - snps,dwmac-4.00

> > -          - snps,dwmac-4.10a

> > -          - snps,dwmac-4.20a

> > -          - snps,dwxgmac

> > -          - snps,dwxgmac-2.10

> > -

> > -          # Deprecated

> > -          - st,spear600-gmac

> > -

> > -  required:

> > -    - compatible

> > +select: false

> >  

> >  allOf:

> >    - $ref: "ethernet-controller.yaml#"

> > @@ -62,35 +38,6 @@ allOf:

> >              MAC HW capability register.

> >  

> >  properties:

> > -

> > -  # We need to include all the compatibles from schemas that will

> > -  # include that schemas, otherwise compatible won't validate for

> > -  # those.

> > -  compatible:

> > -    contains:

> > -      enum:

> > -        - allwinner,sun7i-a20-gmac

> > -        - allwinner,sun8i-a83t-emac

> > -        - allwinner,sun8i-h3-emac

> > -        - allwinner,sun8i-r40-emac

> > -        - allwinner,sun8i-v3s-emac

> > -        - allwinner,sun50i-a64-emac

> > -        - amlogic,meson6-dwmac

> > -        - amlogic,meson8b-dwmac

> > -        - amlogic,meson8m2-dwmac

> > -        - amlogic,meson-gxbb-dwmac

> > -        - amlogic,meson-axg-dwmac

> > -        - snps,dwmac

> > -        - snps,dwmac-3.50a

> > -        - snps,dwmac-3.610

> > -        - snps,dwmac-3.70a

> > -        - snps,dwmac-3.710

> > -        - snps,dwmac-4.00

> > -        - snps,dwmac-4.10a

> > -        - snps,dwmac-4.20a

> > -        - snps,dwxgmac

> > -        - snps,dwxgmac-2.10

> > -

> >    reg:

> >      minItems: 1

> >      maxItems: 2

> > @@ -543,88 +490,4 @@ dependencies:

> >    snps,reset-delay-us: ["snps,reset-gpio"]

> >  

> >  additionalProperties: true

> > -

> > -examples:

> > -  - |

> > -    gmac0: ethernet@e0800000 {

> > -        compatible = "snps,dwxgmac-2.10", "snps,dwxgmac";

> > -        reg = <0xe0800000 0x8000>;

> > -        interrupt-parent = <&vic1>;

> > -        interrupts = <24 23 22>;

> > -        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";

> > -        mac-address = [000000000000]; /* Filled in by U-Boot */

> > -        max-frame-size = <3800>;

> > -        phy-mode = "gmii";

> > -        snps,multicast-filter-bins = <256>;

> > -        snps,perfect-filter-entries = <128>;

> > -        rx-fifo-depth = <16384>;

> > -        tx-fifo-depth = <16384>;

> > -        clocks = <&clock>;

> > -        clock-names = "stmmaceth";

> > -        snps,axi-config = <&stmmac_axi_setup>;

> > -        snps,mtl-rx-config = <&mtl_rx_setup>;

> > -        snps,mtl-tx-config = <&mtl_tx_setup>;

> > -        mdio0 {

> > -            #address-cells = <1>;

> > -            #size-cells = <0>;

> > -            compatible = "snps,dwmac-mdio";

> > -            phy1: ethernet-phy@0 {

> > -                reg = <0>;

> > -            };

> > -        };

> > -    };

> > -  - |

> > -    gmac1: ethernet@f8010000 {

> > -        compatible = "snps,dwmac-4.10a", "snps,dwmac";

> > -        reg = <0xf8010000 0x4000>;

> > -        interrupts = <0 98 4>;

> > -        interrupt-names = "macirq";

> > -        clock-names = "stmmaceth", "ptp_ref";

> > -        clocks = <&clock 4>, <&clock 5>;

> > -        phy-mode = "rgmii";

> > -        snps,txpbl = <8>;

> > -        snps,rxpbl = <2>;

> > -        snps,aal;

> > -        snps,tso;

> > -

> > -        snps,axi-config {

> > -            snps,wr_osr_lmt = <0xf>;

> > -            snps,rd_osr_lmt = <0xf>;

> > -            snps,blen = <256 128 64 32 0 0 0>;

> > -        };

> > -

> > -        snps,mtl-rx-config {

> > -            snps,rx-queues-to-use = <1>;

> > -            snps,rx-sched-sp;

> > -            queue0 {

> > -               snps,dcb-algorithm;

> > -               snps,map-to-dma-channel = <0x0>;

> > -               snps,priority = <0x0>;

> > -            };

> > -        };

> > -

> > -        snps,mtl-tx-config {

> > -            snps,tx-queues-to-use = <2>;

> > -            snps,tx-sched-wrr;

> > -            queue0 {

> > -                snps,weight = <0x10>;

> > -                snps,dcb-algorithm;

> > -                snps,priority = <0x0>;

> > -            };

> > -

> > -            queue1 {

> > -                snps,avb-algorithm;

> > -                snps,send_slope = <0x1000>;

> > -                snps,idle_slope = <0x1000>;

> > -                snps,high_credit = <0x3E800>;

> > -                snps,low_credit = <0xFFC18000>;

> > -                snps,priority = <0x1>;

> > -            };

> > -        };

> > -    };

> > -

> > -# FIXME: We should set it, but it would report all the generic

> > -# properties as additional properties.

> > -# additionalProperties: false

> > -

> >  ...

> > -- 

> > 2.29.2

> >