mbox series

[v6,net-next,00/13] net: ethernet: ti: introduce new cpsw switchdev based driver

Message ID 20191109151525.18651-1-grygorii.strashko@ti.com
Headers show
Series net: ethernet: ti: introduce new cpsw switchdev based driver | expand

Message

Grygorii Strashko Nov. 9, 2019, 3:15 p.m. UTC
Hi All,

Thank you All for review of v5.

The major change in this version is DT bindings conversation to json-schema, and
fixed other comments to v5. Also added patch to clean up ALE on init and netif
restart.

--- v5
The major part of work done in this iteration is rebasing on top of net-next
with XDP series from Ivan Khoronzhuk [3], and enable XDP support in the new
CPSW switchdev driver (it was little bit painful ;(). There are mostly no
functional changes in new CPSW driver, just few fixes, sync with old driver
and cleanups/optimizations. So, I've kept rest of cover letter unchanged.

---
This series originally based on work [1][2] done by
Ilias Apalodimas <ilias.apalodimas@linaro.org>.

This the RFC v5 which introduces new CPSW switchdev based driver which is 
operating in dual-emac mode by default, thus working as 2 individual
network interfaces. The Switch mode can be enabled by configuring devlink driver
parameter "switch_mode" to 1/true:
	devlink dev param set platform/48484000.switch \
	name switch_mode value 1 cmode runtime
This can be done regardless of the state of Port's netdev devices - UP/DOWN, but
Port's netdev devices have to be in UP before joining the bridge to avoid
overwriting of bridge configuration as CPSW switch driver completely reloads its
configuration when first Port changes its state to UP.
When the both interfaces joined the bridge - CPSW switch driver will start
marking packets with offload_fwd_mark flag unless "ale_bypass=0".
All configuration is implemented via switchdev API. 

The previous solution of tracking both Ports joined the bridge
(from netdevice_notifier) proved to be not correct as changing CPSW switch
driver mode required cleanup of ALE table and CPSW settings which happens
while second Port is joined bridge and as result configuration loaded
by bridge for the first Port became corrupted.

The introduction of the new CPSW switchdev based driver (cpsw_new.c) is split
on two parts: Part 1 - basic dual-emac driver; Part 2 switchdev support.
Such approach has simplified code development and testing alot. And, I hope, 
it will help with better review.

patches #1 - 5: preparation patches which also moves common code to cpsw_priv.c
patches #6 - 9: Introduce TI CPSW switch driver based on switchdev and new
 DT bindings
patch #10: new CPSW switchdev driver documentation
patch #11: adds DT nodes for new CPSW switchdev driver added for DRA7 SoC
patch #12: adds DT nodes for new cpsw switchdev driver for am571x-idk board
patch #13: enables build of TI CPSW driver

Most of the contents of the previous cover-letter have been added in
new driver documentation, so please refer to that for configuration,
testing and future work.

These patches can be found at:
 https://github.com/grygoriyS/linux.git
 branch: lkml-5.4-switch-tbd-v6

changes in v6:
 - DT bindings converted to json-schema
 - netdev initialization is split on creation and registration.
   The netdevs registration happens now at the end of the pobe.
 - reworked cpsw_set_pauseparam() to use PHYlib APIs.
 - other comments for v5 fixed

v5: https://patchwork.kernel.org/cover/11208785/
 - rebase on top of net-next with XDP series from Ivan Khoronzhuk [3],
   and enable XDP support in the new CPSW switchdev driver
   cpsw driver (tested XDP_DROP only)
 - sync with old cpsw driver
 - implement comments from  Ivan Khoronzhuk and Rob Herring
 - fixed "NETDEV WATCHDOG: .." warning after interface after interface UP/DOWN,
   missed TX wake in cpsw_adjust_link()

v4: https://patchwork.kernel.org/cover/11010523/
 - finished split of common CPSW code
 - added devlink support
 - changed CPSW mode configuration approach: from netdevice_notifier to devlink
   parameter
 - refactor and clean up ALE changes which allows to modify VLANs/MDBs entries
 - added missed support for port QDISC_CBS and QDISC_MQPRIO
 - the CPSW is split on two parts: basic dual_mac driver and switchdev support
 - added missed callback .ndo_get_port_parent_id()
 - reworked ingress frames marking in switch mode (offload_fwd_mark)
 - applied comments from Andrew Lunn

v3: https://lwn.net/Articles/786677/
Changes in v3:
- alot of work done to split properly common code between legacy and switchdev
  CPSW drivers and clean up code
- CPSW switchdev interface updated to the current LKML switchdev interface
- actually new CPSW switchdev based driver introduced
- optimized dual_mac mode in new driver. Main change is that in promiscuous
mode P0_UNI_FLOOD (both ports) is enabled in addition to ALLMULTI (current
port) instead of ALE_BYPASS.  So, port in non promiscuous mode will keep
possibility of mcast and vlan filtering.
- changed bridge join sequnce: now switch mode will be enabled only when
both ports joined the bridge. CPSW will be switched to dual_mac mode if any
port leave bridge. ALE table is completly cleared and then refiled while
switching to switch mode - this simplidies code a lot, but introduces some
limitation to bridge setup sequence:
 ip link add name br0 type bridge
 ip link set dev br0 type bridge ageing_time 1000
 ip link set dev br0 type bridge vlan_filtering 0 <- disable
 echo 0 > /sys/class/net/br0/bridge/default_vlan

 ip link set dev sw0p1 up <- add ports
 ip link set dev sw0p2 up
 ip link set dev sw0p1 master br0
 ip link set dev sw0p2 master br0

 echo 1 > /sys/class/net/br0/bridge/default_vlan <- enable
 ip link set dev br0 type bridge vlan_filtering 1
 bridge vlan add dev br0 vid 1 pvid untagged self
- STP tested with vlan_filtering 1/0. To make STP work I've had to set
  NO_SA_UPDATE for all slave ports (see comment in code). It also required to
  statically register STP mcast address {0x01, 0x80, 0xc2, 0x0, 0x0, 0x0};
- allowed build both TI_CPSW and TI_CPSW_SWITCHDEV drivers
- PTP can be enabled on both ports in dual_mac mode

[1] https://patchwork.ozlabs.org/cover/929367/
[2] https://patches.linaro.org/cover/136709/
[3] https://patchwork.kernel.org/cover/11035813/

Grygorii Strashko (9):
  net: ethernet: ti: ale: clean ale tbl on init and intf restart
  net: ethernet: ti: cpsw: allow untagged traffic on host port
  net: ethernet: ti: cpsw: resolve build deps of cpsw drivers
  net: ethernet: ti: cpsw: move set of common functions in cpsw_priv
  dt-bindings: net: ti: add new cpsw switch driver bindings
  phy: ti: phy-gmii-sel: dependency from ti cpsw-switchdev driver
  ARM: dts: dra7: add dt nodes for new cpsw switch dev driver
  ARM: dts: am571x-idk: enable for new cpsw switch dev driver
  arm: omap2plus_defconfig: enable new cpsw switchdev driver

Ilias Apalodimas (4):
  net: ethernet: ti: ale: modify vlan/mdb api for switchdev
  net: ethernet: ti: introduce cpsw  switchdev based driver part 1 -
    dual-emac
  net: ethernet: ti: introduce cpsw switchdev based driver part 2 -
    switch
  Documentation: networking: add cpsw switchdev based driver
    documentation

 .../bindings/net/ti,cpsw-switch.yaml          |  245 ++
 .../device_drivers/ti/cpsw_switchdev.txt      |  210 ++
 .../devlink-params-ti-cpsw-switch.txt         |   10 +
 arch/arm/boot/dts/am571x-idk.dts              |   27 +
 arch/arm/boot/dts/am572x-idk.dts              |    5 +
 arch/arm/boot/dts/am574x-idk.dts              |    5 +
 arch/arm/boot/dts/am57xx-idk-common.dtsi      |    5 -
 arch/arm/boot/dts/dra7-l4.dtsi                |   52 +
 arch/arm/configs/omap2plus_defconfig          |    1 +
 drivers/net/ethernet/ti/Kconfig               |   19 +-
 drivers/net/ethernet/ti/Makefile              |    2 +
 drivers/net/ethernet/ti/cpsw.c                | 1374 +----------
 drivers/net/ethernet/ti/cpsw_ale.c            |  148 +-
 drivers/net/ethernet/ti/cpsw_ale.h            |   11 +
 drivers/net/ethernet/ti/cpsw_new.c            | 2048 +++++++++++++++++
 drivers/net/ethernet/ti/cpsw_priv.c           | 1246 +++++++++-
 drivers/net/ethernet/ti/cpsw_priv.h           |   79 +-
 drivers/net/ethernet/ti/cpsw_switchdev.c      |  589 +++++
 drivers/net/ethernet/ti/cpsw_switchdev.h      |   15 +
 drivers/phy/ti/Kconfig                        |    4 +-
 20 files changed, 4755 insertions(+), 1340 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
 create mode 100644 Documentation/networking/device_drivers/ti/cpsw_switchdev.txt
 create mode 100644 Documentation/networking/devlink-params-ti-cpsw-switch.txt
 create mode 100644 drivers/net/ethernet/ti/cpsw_new.c
 create mode 100644 drivers/net/ethernet/ti/cpsw_switchdev.c
 create mode 100644 drivers/net/ethernet/ti/cpsw_switchdev.h

-- 
2.17.1

Comments

Tony Lindgren Nov. 11, 2019, 5:26 p.m. UTC | #1
Hi,

* Grygorii Strashko <grygorii.strashko@ti.com> [191109 15:17]:
> +    mac_sw: switch@0 {

> +        compatible = "ti,dra7-cpsw-switch","ti,cpsw-switch";

> +        reg = <0x0 0x4000>;

> +        ranges = <0 0 0x4000>;

> +        clocks = <&gmac_main_clk>;

> +        clock-names = "fck";

> +        #address-cells = <1>;

> +        #size-cells = <1>;

> +        syscon = <&scm_conf>;

> +        inctrl-names = "default", "sleep";

> +

> +        interrupts = <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,

> +                     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,

> +                     <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,

> +                     <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>;

> +        interrupt-names = "rx_thresh", "rx", "tx", "misc";


I think with the ti-sysc managing the interconnect target module as the
parent of this, you should be able add all the modules as direct children
of ti-sysc with minor fixups. This would simplify things, and makes it
easier to update the driver later on when the child modules get
changed/updated/moved around.

The child modules just need to call PM runtime to have access to their
registers, and whatever cpsw control module part could be a separate
driver providing Linux standard services for example for clock gating :)

> +        davinci_mdio_sw: mdio@1000 {

> +                compatible = "ti,cpsw-mdio","ti,davinci_mdio";

> +                reg = <0x1000 0x100>;

> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0>;

> +                clock-names = "fck";

> +                #address-cells = <1>;

> +                #size-cells = <0>;

> +                bus_freq = <1000000>;

> +

> +                ethphy0_sw: ethernet-phy@0 {

> +                        reg = <0>;

> +                };

> +

> +                ethphy1_sw: ethernet-phy@1 {

> +                        reg = <41>;

> +                };

> +        };


And in this case, mdio above would just move up one level.

This goes back to my earlier comments saying the cpsw is really just
a private interconnect with a collection of various mostly independent
modules. Sounds like you're heading that way already though at the
driver level :)

Regards,

Tony
Grygorii Strashko Nov. 12, 2019, 9:53 a.m. UTC | #2
On 11/11/2019 19:26, Tony Lindgren wrote:
> Hi,

> 

> * Grygorii Strashko <grygorii.strashko@ti.com> [191109 15:17]:

>> +    mac_sw: switch@0 {

>> +        compatible = "ti,dra7-cpsw-switch","ti,cpsw-switch";

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

>> +        ranges = <0 0 0x4000>;

>> +        clocks = <&gmac_main_clk>;

>> +        clock-names = "fck";

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

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

>> +        syscon = <&scm_conf>;

>> +        inctrl-names = "default", "sleep";

>> +

>> +        interrupts = <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,

>> +                     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,

>> +                     <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,

>> +                     <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>;

>> +        interrupt-names = "rx_thresh", "rx", "tx", "misc";

> 

> I think with the ti-sysc managing the interconnect target module as the

> parent of this, you should be able add all the modules as direct children

> of ti-sysc with minor fixups. This would simplify things, and makes it

> easier to update the driver later on when the child modules get

> changed/updated/moved around.

> 

> The child modules just need to call PM runtime to have access to their

> registers, and whatever cpsw control module part could be a separate

> driver providing Linux standard services for example for clock gating :)

> 

>> +        davinci_mdio_sw: mdio@1000 {

>> +                compatible = "ti,cpsw-mdio","ti,davinci_mdio";

>> +                reg = <0x1000 0x100>;

>> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0>;

>> +                clock-names = "fck";

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

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

>> +                bus_freq = <1000000>;

>> +

>> +                ethphy0_sw: ethernet-phy@0 {

>> +                        reg = <0>;

>> +                };

>> +

>> +                ethphy1_sw: ethernet-phy@1 {

>> +                        reg = <41>;

>> +                };

>> +        };

> 

> And in this case, mdio above would just move up one level.

> 

> This goes back to my earlier comments saying the cpsw is really just

> a private interconnect with a collection of various mostly independent

> modules. Sounds like you're heading that way already though at the

> driver level :)


No, sorry I do not agree. The MDIO is inseparable part of CPSW and it's enabled when CPSW is enabled
(on interconnect level), more over I want to get rid of platform device in MDIO for most of the cases
as it only introduces boot/probing complexity.

The same is valid for CPTS.

-- 
Best regards,
grygorii
Tony Lindgren Nov. 12, 2019, 4:22 p.m. UTC | #3
Hi,

* Grygorii Strashko <grygorii.strashko@ti.com> [191112 09:54]:
> No, sorry I do not agree. The MDIO is inseparable part of CPSW and it's enabled when CPSW is enabled

> (on interconnect level), more over I want to get rid of platform device in MDIO for most of the cases

> as it only introduces boot/probing complexity.


Well the fact that mdio is enabled at the interconnect level is why
I think the cpsw child modules are independent components :)

So I did the following quick test on pocketbeagle with Linux next,
it has no Ethernet wired up, and by default we have ethernet@0
set to status = "disabled".

Manually enable the target module at 0x4a100000:
# echo on > /sys/devices/platform/ocp/4a000000.interconnect/\
4a000000.interconnect:segment@0/4a101200.target-module/power/control

Dump out mdio registers at offset 0x1000:
# rwmem 0x4a101000+0x100
0x4a101000 = 0x40070106
0x4a101004 = 0x810000ff
0x4a101008 = 0000000000
0x4a10100c = 0000000000
0x4a101010 = 0000000000
0x4a101014 = 0000000000
0x4a101018 = 0000000000
...

So yup, it seems quite independent of the other child devices
on the same interconnect target mdoule. I'm quessing it's the
same story for other modules like cppi_dma and so on, this
should be easy to check.

Hmm and isn't the some version of mdio also used stuffed into
davinci_emac and netcp too?

Anyways, up to you. But my experience is that having separate
driver modules is the way to go than trying to treat any TI
"subsystem" as a single device. This is because the child modules
tend to get updated and changed and moved around over time.

Regards,

Tony
Rob Herring (Arm) Nov. 12, 2019, 7:06 p.m. UTC | #4
On Sat, Nov 09, 2019 at 05:15:18PM +0200, Grygorii Strashko wrote:
> Add bindings for the new TI CPSW switch driver. Comparing to the legacy

> bindings (net/cpsw.txt):

> - ports definition follows DSA bindings (net/dsa/dsa.txt) and ports can be

> marked as "disabled" if not physically wired.

> - all deprecated properties dropped;

> - all legacy propertiies dropped which represent constant HW cpapbilities

> (cpdma_channels, ale_entries, bd_ram_size, mac_control, slaves,

> active_slave)

> - TI CPTS DT properties are reused as is, but grouped in "cpts" sub-node

> - TI Davinci MDIO DT bindings are reused as is, because Davinci MDIO is

> reused.

> 

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

> ---

>  .../bindings/net/ti,cpsw-switch.yaml          | 245 ++++++++++++++++++

>  1 file changed, 245 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml

> 

> diff --git a/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml b/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml

> new file mode 100644

> index 000000000000..afeb6a4f1ada

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml

> @@ -0,0 +1,245 @@

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

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/net/ti,cpsw-switch.yaml#

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

> +

> +title: TI SoC Ethernet Switch Controller (CPSW) Device Tree Bindings

> +

> +maintainers:

> +  - Grygorii Strashko <grygorii.strashko@ti.com>

> +  - Sekhar Nori <nsekhar@ti.com>

> +

> +description:

> +  The 3-port switch gigabit ethernet subsystem provides ethernet packet

> +  communication and can be configured as an ethernet switch. It provides the

> +  gigabit media independent interface (GMII),reduced gigabit media

> +  independent interface (RGMII), reduced media independent interface (RMII),

> +  the management data input output (MDIO) for physical layer device (PHY)

> +  management.

> +

> +properties:

> +  compatible:

> +    oneOf:

> +      - const: ti,cpsw-switch

> +      - items:

> +         - const: ti,am335x-cpsw-switch

> +         - const: ti,cpsw-switch

> +      - items:

> +        - const: ti,am4372-cpsw-switch

> +        - const: ti,cpsw-switch

> +      - items:

> +        - const: ti,dra7-cpsw-switch

> +        - const: ti,cpsw-switch

> +

> +  reg:

> +    maxItems: 1

> +    description:

> +       The physical base address and size of full the CPSW module IO range

> +

> +  ranges: true

> +

> +  clocks:

> +    maxItems: 1

> +    description: CPSW functional clock

> +

> +  clock-names:

> +    maxItems: 1

> +    items:

> +      - const: fck

> +

> +  interrupts:

> +    maxItems: 4


Implied by 'items' list.

> +    items:

> +      - description: RX_THRESH interrupt

> +      - description: RX interrupt

> +      - description: TX interrupt

> +      - description: MISC interrupt

> +

> +  interrupt-names:

> +    maxItems: 4


Implied by 'items' list.

> +    items:

> +      - const: "rx_thresh"

> +      - const: "rx"

> +      - const: "tx"

> +      - const: "misc"

> +

> +  pinctrl-names: true

> +

> +  syscon:

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

> +    maxItems: 1


Not an array, so not needed.

> +    description:

> +      Phandle to the system control device node which provides access to

> +      efuse IO range with MAC addresses

> +

> +

> +  ethernet-ports:

> +    type: object

> +    properties:

> +      '#address-cells':

> +        const: 1

> +      '#size-cells':

> +        const: 0

> +

> +    patternProperties:

> +      "^port@[0-9]+$":

> +          type: object

> +          minItems: 1

> +          maxItems: 2

> +          description: CPSW external ports

> +

> +          allOf:

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

> +

> +          properties:

> +            reg:

> +              maxItems: 1

> +              enum: [1, 2]

> +              description: CPSW port number

> +

> +            phys:

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

> +              maxItems: 1

> +              description:  phandle on phy-gmii-sel PHY

> +

> +            label:

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

> +              maxItems: 1

> +              description: label associated with this port

> +

> +            ti,dual-emac-pvid:

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

> +              maxItems: 1

> +              minimum: 1

> +              maximum: 1024

> +              description:

> +                Specifies default PORT VID to be used to segregate

> +                ports. Default value - CPSW port number.

> +

> +          required:

> +            - reg

> +            - phys

> +

> +  mdio:

> +    type: object

> +    allOf:

> +      - $ref: "ti,davinci-mdio.yaml#"

> +    description:

> +      CPSW MDIO bus.

> +

> +  cpts:

> +    type: object

> +    description:

> +      The Common Platform Time Sync (CPTS) module

> +

> +    properties:

> +      clocks:

> +        maxItems: 1

> +        description: CPTS reference clock

> +

> +      clock-names:

> +        maxItems: 1

> +        items:

> +          - const: cpts

> +

> +      cpts_clock_mult:

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

> +        maxItems: 1


Not an array, so not needed.

Is there a set or range of values you can define?

> +        description:

> +          Numerator to convert input clock ticks into ns

> +

> +      cpts_clock_shift:

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

> +        maxItems: 1


Same comments here.

> +        description:

> +          Denominator to convert input clock ticks into ns.

> +          Mult and shift will be calculated basing on CPTS rftclk frequency if

> +          both cpts_clock_shift and cpts_clock_mult properties are not provided.

> +

> +    required:

> +      - clocks

> +      - clock-names

> +

> +required:

> +  - compatible

> +  - reg

> +  - ranges

> +  - clocks

> +  - clock-names

> +  - interrupts

> +  - interrupt-names

> +  - '#address-cells'

> +  - '#size-cells'

> +

> +examples:

> +  - |

> +    #include <dt-bindings/interrupt-controller/irq.h>

> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

> +    #include <dt-bindings/clock/dra7.h>

> +

> +    mac_sw: switch@0 {

> +        compatible = "ti,dra7-cpsw-switch","ti,cpsw-switch";

> +        reg = <0x0 0x4000>;

> +        ranges = <0 0 0x4000>;

> +        clocks = <&gmac_main_clk>;

> +        clock-names = "fck";

> +        #address-cells = <1>;

> +        #size-cells = <1>;

> +        syscon = <&scm_conf>;

> +        inctrl-names = "default", "sleep";

> +

> +        interrupts = <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,

> +                     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,

> +                     <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,

> +                     <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>;

> +        interrupt-names = "rx_thresh", "rx", "tx", "misc";

> +

> +        ethernet-ports {

> +                #address-cells = <1>;

> +                #size-cells = <0>;

> +

> +                cpsw_port1: port@1 {

> +                        reg = <1>;

> +                        label = "port1";

> +                        mac-address = [ 00 00 00 00 00 00 ];

> +                        phys = <&phy_gmii_sel 1>;

> +                        phy-handle = <&ethphy0_sw>;

> +                        phy-mode = "rgmii";

> +                        ti,dual_emac_pvid = <1>;

> +                };

> +

> +                cpsw_port2: port@2 {

> +                        reg = <2>;

> +                        label = "wan";

> +                        mac-address = [ 00 00 00 00 00 00 ];

> +                        phys = <&phy_gmii_sel 2>;

> +                        phy-handle = <&ethphy1_sw>;

> +                        phy-mode = "rgmii";

> +                        ti,dual_emac_pvid = <2>;

> +                };

> +        };

> +

> +        davinci_mdio_sw: mdio@1000 {

> +                compatible = "ti,cpsw-mdio","ti,davinci_mdio";

> +                reg = <0x1000 0x100>;

> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0>;

> +                clock-names = "fck";

> +                #address-cells = <1>;

> +                #size-cells = <0>;

> +                bus_freq = <1000000>;

> +

> +                ethphy0_sw: ethernet-phy@0 {

> +                        reg = <0>;

> +                };

> +

> +                ethphy1_sw: ethernet-phy@1 {

> +                        reg = <41>;


make dt_binding_check fails:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: 
mdio@1000: ethernet-phy@1:reg:0:0: 41 is greater than the maximum of 31
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/ti,cpsw-switch.example.dt.yaml: 
ethernet-phy@1: reg:0:0: 41 is greater than the maximum of 31

> +                };

> +        };

> +

> +        cpts {

> +                clocks = <&gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 25>;

> +                clock-names = "cpts";

> +        };

> +    };

> -- 

> 2.17.1

>