Message-Id: <20211116062328.1949151-1-colin.foster@in-advantage.com>
Date:   Mon, 15 Nov 2021 22:23:05 -0800
From:   Colin Foster <colin.foster@...advantage.com>
To:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-gpio@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        UNGLinuxDriver@...rochip.com, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Lars Povlsen <lars.povlsen@...rochip.com>,
        Steen Hegelund <Steen.Hegelund@...rochip.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: [RFC PATCH v4 net-next 00/23] add support for VSC75XX control over SPI

My apologies for this next RFC taking so long. Life got in the way.


The patch set in general is to add support for the VSC7511, VSC7512,
VSC7513 and VSC7514 devices controlled over SPI. The driver is
relatively functional for the internal phy ports (0-3) on the VSC7512.
As I'll discuss, it is not yet functional for other ports yet.


I still think there are enough updates to bounce by the community
in case I'm terribly off base or doomed to chase my tail.


The main changes for V4 are trying to get pinctrl-ocelot and
pinctrl-microchip-sgpio functional. Without pinctrl-ocelot,
communication to external phys won't work. Without communication to
external phys, PCS ports 4-7 on the dev board won't work. Nor will any
fiber ports. 


The hardware setup I'm using for development is a beaglebone black, with
jumpers from SPI0 to the microchip VSC7512 dev board. The microchip dev 
board has been modified to not boot from flash, but wait for SPI. An
ethernet cable is connected from the beaglebone ethernet to port 0 of
the dev board.


The device tree I'm using for the VSC7512 is below. Note that ports 4-7
are still not expected to work, but left in as placeholders for when
they do.


&spi0 {
	#address-cells = <1>;
	#size-cells = <0>;
	status = "okay";

	ethernet-switch@0{
		 compatible = "mscc,vsc7512";
		 spi-max-frequency = <250000>;
		 reg = <0>;

		 ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@0 {
				reg = <0>;
				label = "cpu";
				status = "okay";
				ethernet = <&mac_sw>;
				phy-handle = <&sw_phy0>;
				phy-mode = "internal";
			};

			port@1 {
				reg = <1>;
				label = "swp1";
				status = "okay";
				phy-handle = <&sw_phy1>;
				phy-mode = "internal";
			};

			port@2 {
				reg = <2>;
				label = "swp2";
				status = "okay";
				phy-handle = <&sw_phy2>;
				phy-mode = "internal";
			};

			port@3 {
				reg = <3>;
				label = "swp3";
				status = "okay";
				phy-handle = <&sw_phy3>;
				phy-mode = "internal";
			};

			port@4 {
				reg = <4>;
				label = "swp4";
				status = "okay";
				phy-handle = <&sw_phy4>;
				phy-mode = "sgmii";
			};

			port@5 {
				reg = <5>;
				label = "swp5";
				status = "okay";
				phy-handle = <&sw_phy5>;
				phy-mode = "sgmii";
			};

			port@6 {
				reg = <6>;
				label = "swp6";
				status = "okay";
				phy-handle = <&sw_phy6>;
				phy-mode = "sgmii";
			};

			port@7 {
				reg = <7>;
				label = "swp7";
				status = "okay";
				phy-handle = <&sw_phy7>;
				phy-mode = "sgmii";
			};
		};

		mdio {
			#address-cells = <1>;
			#size-cells = <0>;

			sw_phy0: ethernet-phy@0 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x0>;
			};

			sw_phy1: ethernet-phy@1 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x1>;
			};

			sw_phy2: ethernet-phy@2 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x2>;
			};

			sw_phy3: ethernet-phy@3 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x3>;
			};

			sw_phy4: ethernet-phy@4 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x4>;
			};

			sw_phy5: ethernet-phy@5 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x5>;
			};

			sw_phy6: ethernet-phy@6 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x6>;
			};

			sw_phy7: ethernet-phy@7 {
				#address-cells = <1>;
				#size-cells = <0>;
				reg = <0x7>;
			};
		};

		gpio: pinctrl {
			compatible = "mscc,ocelot-pinctrl";
			#address-cells = <1>;
			#size-cells = <0>;
			#gpio_cells = <2>;
			gpio-ranges = <&gpio 0 0 22>;

			led_shift_reg_pins: led-shift-reg-pins {
				pins = "GPIO_0", "GPIO_1", "GPIO_2", "GPIO_3";
				function = "sg0";
			};
		};

		sgpio: sgpio {
			compatible = "mscc,ocelot-sgpio";
			#address-cells = <1>;
			#size-cells = <0>;
			bus-frequency=<12500000>;
			clocks = <&ocelot_clock>;
			microchip,sgpio-port-ranges = <0 31>;

			sgpio_in0: sgpio@0 {
				compatible = "microchip,sparx5-sgpio-bank";
				reg = <0>;
				gpio-controller;
				#gpio-cells = <3>;
				ngpios = <32>;
			};

			sgpio_out1: sgpio@1 {
				compatible = "microchip,sparx5-sgpio-bank";
				reg = <1>;
				gpio-controller;
				gpio-cells = <3>;
				ngpios = <32>;
			};
		};
	};
};


My main focus is getting the ocelot-pinctrl driver fully functional. My
current hope is that it would correctly set GPIO pins 0-3 into the "sg0"
state. That is not the case right now, and I'll be looking into why. The
behavior I'm hoping for is to be able to configure the sgpio LEDs for
activity at the very least. Link status would be a bonus.


I do have pinctrl by way of debugfs and sysfs. There aren't any debug
LEDs that are attached to unused pins, unfortunately. That would've been
really helpful. So there's a key takeaway for dev-board manufacturers.


As you'll see, the main changes to the three drivers I'm utilizing
(mscc_miim, pinctrl-ocelot, and pinctrl-microchip-sgpio) follow a
similar path. First, convert everything to regmap. Second, expose
whatever functions are necessary to fully utilize an external regmap.


One thing to note: I've been following a pattern of adding "offset"
variables to these drivers. I'm looking for feedback here, because I
don't like it - however I feel like it is the "least bad" interface I
could come up with. 


Specifically, ocelot has a regmap for GCB. ocelot-pinctrl would create a
smaller regmap at an address of "GCB + 0x34".


There are three options I saw here:
1. Have vsc7512_spi create a new regmap at GCB + 0x34 and pass that to
ocelot-pinctrl
2. Give ocelot-pinctrl the concept of a "parent bus" by which it could
request a regmap. 
3. Keep the same GCB regmap, but pass in 0x34 as an offset.


I will admit that option 2 sounds very enticing, but I don't know if
that type of interaction exists. If not, implementing it is probably
outside the scope of a first patch set. As such, I opted for option 3.


Version 4 also fixes some logic for MDIO probing. It wasn't using the
device tree by way of of_mdiobus_register. Now it is.


The relevant boot log for the switch / MDIO bus is here. As expected,
devices 4-7 are missing. If nothing else, that is telling me that the
device tree is working.

[    4.005195] mdio_bus spi0.0-mii:03: using lookup tables for GPIO lookup
[    4.005205] mdio_bus spi0.0-mii:03: No GPIO consumer reset found
[    4.006586] mdio_bus spi0.0-mii: MDIO device at address 4 is missing.
[    4.014333] mdio_bus spi0.0-mii: MDIO device at address 5 is missing.
[    4.022009] mdio_bus spi0.0-mii: MDIO device at address 6 is missing.
[    4.029573] mdio_bus spi0.0-mii: MDIO device at address 7 is missing.
[    8.386624] vsc7512 spi0.0: PHY [spi0.0-mii:00] driver [Generic PHY] (irq=POLL)
[    8.397222] vsc7512 spi0.0: configuring for phy/internal link mode
[    8.419484] vsc7512 spi0.0 swp1 (uninitialized): PHY [spi0.0-mii:01] driver [Generic PHY] (irq=POLL)
[    8.437278] vsc7512 spi0.0 swp2 (uninitialized): PHY [spi0.0-mii:02] driver [Generic PHY] (irq=POLL)
[    8.452867] vsc7512 spi0.0 swp3 (uninitialized): PHY [spi0.0-mii:03] driver [Generic PHY] (irq=POLL)
[    8.465007] vsc7512 spi0.0 swp4 (uninitialized): no phy at 4
[    8.470721] vsc7512 spi0.0 swp4 (uninitialized): failed to connect to PHY: -ENODEV
[    8.478388] vsc7512 spi0.0 swp4 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 4
[    8.489636] vsc7512 spi0.0 swp5 (uninitialized): no phy at 5
[    8.495371] vsc7512 spi0.0 swp5 (uninitialized): failed to connect to PHY: -ENODEV
[    8.502996] vsc7512 spi0.0 swp5 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 5
[    8.514186] vsc7512 spi0.0 swp6 (uninitialized): no phy at 6
[    8.519882] vsc7512 spi0.0 swp6 (uninitialized): failed to connect to PHY: -ENODEV
[    8.527539] vsc7512 spi0.0 swp6 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 6
[    8.538716] vsc7512 spi0.0 swp7 (uninitialized): no phy at 7
[    8.544451] vsc7512 spi0.0 swp7 (uninitialized): failed to connect to PHY: -ENODEV
[    8.552079] vsc7512 spi0.0 swp7 (uninitialized): error -19 setting up PHY for tree 0, switch 0, port 7
[    8.571962] device eth0 entered promiscuous mode
[    8.576684] DSA: tree 0 setup
[   10.490093] vsc7512 spi0.0: Link is Up - 100Mbps/Full - flow control off


Much later on, I created a bridge with STP (and two ports jumped
together) as a test. Everything seems to be working as expected. 


[59839.920340] cpsw-switch 4a100000.switch: starting ndev. mode: dual_mac
[59840.013636] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
[59840.031444] 8021q: adding VLAN 0 to HW filter on device eth0
[59840.057406] vsc7512 spi0.0 swp1: configuring for phy/internal link mode
[59840.089302] vsc7512 spi0.0 swp2: configuring for phy/internal link mode
[59840.121514] vsc7512 spi0.0 swp3: configuring for phy/internal link mode
[59840.167589] br0: port 1(swp1) entered blocking state
[59840.172818] br0: port 1(swp1) entered disabled state
[59840.191078] device swp1 entered promiscuous mode
[59840.224855] br0: port 2(swp2) entered blocking state
[59840.229893] br0: port 2(swp2) entered disabled state
[59840.245844] device swp2 entered promiscuous mode
[59840.270839] br0: port 3(swp3) entered blocking state
[59840.276003] br0: port 3(swp3) entered disabled state
[59840.291674] device swp3 entered promiscuous mode
[59840.663239] vsc7512 spi0.0: Link is Down
[59841.691641] vsc7512 spi0.0: Link is Up - 100Mbps/Full - flow control off
[59842.167897] cpsw-switch 4a100000.switch eth0: Link is Up - 100Mbps/Full - flow control off
[59842.176481] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[59843.216121] vsc7512 spi0.0 swp1: Link is Up - 1Gbps/Full - flow control rx/tx
[59843.231076] IPv6: ADDRCONF(NETDEV_CHANGE): swp1: link becomes ready
[59843.237593] br0: port 1(swp1) entered blocking state
[59843.242629] br0: port 1(swp1) entered listening state
[59843.301447] vsc7512 spi0.0 swp3: Link is Up - 1Gbps/Full - flow control rx/tx
[59843.309027] IPv6: ADDRCONF(NETDEV_CHANGE): swp3: link becomes ready
[59843.315544] br0: port 3(swp3) entered blocking state
[59843.320545] br0: port 3(swp3) entered listening state
[59845.042058] br0: port 3(swp3) entered blocking state
[59858.401566] br0: port 1(swp1) entered learning state
[59871.841910] br0: received packet on swp1 with own address as source address (addr:24:76:25:76:35:37, vlan:0)
[59873.761495] br0: port 1(swp1) entered forwarding state
[59873.766703] br0: topology change detected, propagating
[59873.776278] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
[59902.561908] br0: received packet on swp1 with own address as source address (addr:24:76:25:76:35:37, vlan:0)
[59926.494446] vsc7512 spi0.0 swp2: Link is Up - 1Gbps/Full - flow control rx/tx
[59926.501959] IPv6: ADDRCONF(NETDEV_CHANGE): swp2: link becomes ready
[59926.508702] br0: port 2(swp2) entered blocking state
[59926.513868] br0: port 2(swp2) entered listening state
[59941.601540] br0: port 2(swp2) entered learning state
[59956.961493] br0: port 2(swp2) entered forwarding state
[59956.966711] br0: topology change detected, propagating
[59968.481839] br0: received packet on swp1 with own address as source address (addr:24:76:25:76:35:37, vlan:0)


In order to make this work, I have modified the cpsw driver, and now the
cpsw_new driver, to allow for frames over 1500 bytes. Otherwise the
tagging protocol will not work between the beaglebone and the VSC7512. I
plan to eventually try to get those changes in mainline, but I don't
want to get distracted from my initial goal.


RFC history:
v1 (accidentally named vN)
	* Initial architecture. Not functional
	* General concepts laid out

v2
	* Near functional. No CPU port communication, but control over all
	external ports
	* Cleaned up regmap implementation from v1

v3
	* Functional
	* Shared MDIO transactions routed through mdio-mscc-miim
	* CPU / NPI port enabled by way of vsc7512_enable_npi_port /
	felix->info->enable_npi_port
	* NPI port tagging functional - Requires a CPU port driver that supports
	frames of 1520 bytes. Verified with a patch to the cpsw driver

v4
    * Functional
    * Device tree fixes
    * Add hooks for pinctrl-ocelot - some functionality by way of sysfs
    * Add hooks for pinctrl-microsemi-sgpio - not yet fully functional
    * Remove lynx_pcs interface for a generic phylink_pcs. The goal here
    is to have an ocelot_pcs that will work for each configuration of
    every port. 



Colin Foster (23):
  net: dsa: ocelot: remove unnecessary pci_bar variables
  net: mdio: mscc-miim: convert to a regmap implementation
  net: dsa: ocelot: seville: utilize of_mdiobus_register
  net: dsa: ocelot: felix: switch to mdio-mscc-miim driver for indirect
    mdio access
  net: dsa: ocelot: felix: Remove requirement for PCS in felix devices
  net: dsa: ocelot: felix: add interface for custom regmaps
  net: dsa: ocelot: felix: add per-device-per-port quirks
  net: mscc: ocelot: split register definitions to a separate file
  net: mscc: ocelot: expose ocelot wm functions
  pinctrl: ocelot: combine get resource and ioremap into single call
  pinctrl: ocelot: update pinctrl to automatic base address
  pinctrl: ocelot: convert pinctrl to regmap
  pinctrl: ocelot: expose ocelot_pinctrl_core_probe interface
  pinctrl: microchip-sgpio: update to support regmap
  device property: add helper function fwnode_get_child_node_count
  pinctrl: microchip-sgpio: change device tree matches to use nodes
    instead of device
  pinctrl: microchip-sgpio: expose microchip_sgpio_core_probe interface
  net: phy: lynx: refactor Lynx PCS module to use generic phylink_pcs
  net: dsa: felix: name change for clarity from pcs to mdio_device
  net: dsa: seville: name change for clarity from pcs to mdio_device
  net: ethernet: enetc: name change for clarity from pcs to mdio_device
  net: pcs: lynx: use a common naming scheme for all lynx_pcs variables
  net: dsa: ocelot: felix: add support for VSC75XX control over SPI

 drivers/base/property.c                       |  20 +-
 drivers/net/dsa/ocelot/Kconfig                |  16 +
 drivers/net/dsa/ocelot/Makefile               |   7 +
 drivers/net/dsa/ocelot/felix.c                |  29 +-
 drivers/net/dsa/ocelot/felix.h                |  10 +-
 drivers/net/dsa/ocelot/felix_mdio.c           |  54 +
 drivers/net/dsa/ocelot/felix_mdio.h           |  13 +
 drivers/net/dsa/ocelot/felix_vsc9959.c        |  38 +-
 drivers/net/dsa/ocelot/ocelot_vsc7512_spi.c   | 946 ++++++++++++++++++
 drivers/net/dsa/ocelot/seville_vsc9953.c      | 136 +--
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  12 +-
 .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |   3 +-
 .../net/ethernet/freescale/enetc/enetc_pf.c   |  27 +-
 .../net/ethernet/freescale/enetc/enetc_pf.h   |   4 +-
 drivers/net/ethernet/mscc/Makefile            |   3 +-
 drivers/net/ethernet/mscc/ocelot.c            |   8 +
 drivers/net/ethernet/mscc/ocelot_devlink.c    |  31 +
 drivers/net/ethernet/mscc/ocelot_vsc7514.c    | 548 +---------
 drivers/net/ethernet/mscc/vsc7514_regs.c      | 522 ++++++++++
 drivers/net/mdio/mdio-mscc-miim.c             | 167 +++-
 drivers/net/pcs/pcs-lynx.c                    |  36 +-
 drivers/pinctrl/pinctrl-microchip-sgpio.c     | 127 ++-
 drivers/pinctrl/pinctrl-ocelot.c              | 207 ++--
 include/linux/mdio/mdio-mscc-miim.h           |  19 +
 include/linux/pcs-lynx.h                      |   9 +-
 include/linux/property.h                      |   1 +
 include/soc/mscc/ocelot.h                     |  60 ++
 include/soc/mscc/vsc7514_regs.h               |  27 +
 28 files changed, 2219 insertions(+), 861 deletions(-)
 create mode 100644 drivers/net/dsa/ocelot/felix_mdio.c
 create mode 100644 drivers/net/dsa/ocelot/felix_mdio.h
 create mode 100644 drivers/net/dsa/ocelot/ocelot_vsc7512_spi.c
 create mode 100644 drivers/net/ethernet/mscc/vsc7514_regs.c
 create mode 100644 include/linux/mdio/mdio-mscc-miim.h
 create mode 100644 include/soc/mscc/vsc7514_regs.h

-- 
2.25.1

Powered by blists - more mailing lists