diff mbox series

[3/3] net: dsa: Add Vitesse VSC73xx DSA router driver

Message ID 20180614123534.8063-4-linus.walleij@linaro.org
State Superseded
Headers show
Series DSA driver for Vitesse VSC73xx | expand

Commit Message

Linus Walleij June 14, 2018, 12:35 p.m. UTC
This adds a DSA driver for:

Vitesse VSC7385 SparX-G5 5-port Integrated Gigabit Ethernet Switch
Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch

These switches have a built-in 8051 CPU and can download and execute
firmware in this CPU. They can also be configured to use an external
CPU handling the switch in a memory-mapped manner by connecting to
that external CPU's memory bus.

This driver (currently) only takes control of the switch chip over
SPI and configures it to route packages around when connected to a
CPU port. The chip has embedded PHYs and VLAN support so we model it
using DSA as a best fit so we can easily add VLAN support and maybe
later also exploit the internal frame header to get more direct
control over the switch.

The four built-in GPIO lines are exposed using a standard GPIO chip.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/net/dsa/Kconfig           |   12 +
 drivers/net/dsa/Makefile          |    1 +
 drivers/net/dsa/vitesse-vsc73xx.c | 1362 +++++++++++++++++++++++++++++
 3 files changed, 1375 insertions(+)
 create mode 100644 drivers/net/dsa/vitesse-vsc73xx.c

-- 
2.17.1

Comments

Florian Fainelli June 14, 2018, 4:51 p.m. UTC | #1
On 06/14/2018 05:35 AM, Linus Walleij wrote:
> This adds a DSA driver for:

> 

> Vitesse VSC7385 SparX-G5 5-port Integrated Gigabit Ethernet Switch

> Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch

> Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch

> Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch

> 

> These switches have a built-in 8051 CPU and can download and execute

> firmware in this CPU. They can also be configured to use an external

> CPU handling the switch in a memory-mapped manner by connecting to

> that external CPU's memory bus.

> 

> This driver (currently) only takes control of the switch chip over

> SPI and configures it to route packages around when connected to a

> CPU port. The chip has embedded PHYs and VLAN support so we model it

> using DSA as a best fit so we can easily add VLAN support and maybe

> later also exploit the internal frame header to get more direct

> control over the switch.


Yes having the internal frame header working would be really great,
DSA_TAG_PROTO_NONE is really difficult to use without knowing all the
DSA details which reminds that we should have the following action items:

- document how DSA_TAG_PROTO_NONE behave differently with respect to
bridges/VLAN configuration and the DSA master device

- possibly introduce DSA_TAG_PROTO_8021Q which would automatically
partition ports by allocating one VLAN ID per-port (e.g: from top to
bottom) that would effectively offer the same features/paradigms as what
a proper header would offer (Port separation, if nothing else) and it
could be made seemingly automatic from within DSA

- get rid of b53's DSA_TAG_PROTO_NONE

> 

> The four built-in GPIO lines are exposed using a standard GPIO chip.


What are those typically used for out of curiosity? Is this to connect
to an EEPROM?

> 

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  drivers/net/dsa/Kconfig           |   12 +

>  drivers/net/dsa/Makefile          |    1 +

>  drivers/net/dsa/vitesse-vsc73xx.c | 1362 +++++++++++++++++++++++++++++

>  3 files changed, 1375 insertions(+)

>  create mode 100644 drivers/net/dsa/vitesse-vsc73xx.c

> 

> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig

> index 2b81b97e994f..2f6207b969e3 100644

> --- a/drivers/net/dsa/Kconfig

> +++ b/drivers/net/dsa/Kconfig

> @@ -76,4 +76,16 @@ config NET_DSA_SMSC_LAN9303_MDIO

>  	  Enable access functions if the SMSC/Microchip LAN9303 is configured

>  	  for MDIO managed mode.

>  

> +config NET_DSA_VITESSE_VSC73XX

> +	tristate "Vitesse VSC7385/7388/7395/7398 support"

> +	depends on OF && SPI

> +	depends on NET_DSA

> +	select FIXED_PHY

> +	select VITESSE_PHY

> +	select NET_DSA_TAG_TRAILER


You advertise DSA_TAG_PROTO_NONE, so that appears to be unnecessary right?

[snip]
> +/**

> + * struct vsc73xx - VSC73xx state container

> + */

> +struct vsc73xx {

> +	struct device		*dev;

> +	struct gpio_desc	*reset;

> +	struct spi_device	*spi;

> +	struct dsa_switch	*ds;

> +	struct gpio_chip	gc;

> +	u16			chipid;

> +	bool			is_vsc7385;

> +	bool			is_vsc7388;

> +	bool			is_vsc7395;

> +	bool			is_vsc7398;


How about having an u16/u32 chip_id instead?

> +	u8			addr[ETH_ALEN];

> +	struct mutex		lock; /* Protects SPI traffic */

> +};


[snip]

> +static enum dsa_tag_protocol vsc73xx_get_tag_protocol(struct dsa_switch *ds,

> +						      int port)

> +{

> +	/* The switch internally uses a 8 byte header with length,

> +	 * source port, tag, LPA and priority. This is supposedly

> +	 * only accessible when operating the switch using the internal

> +	 * CPU or with an external CPU mapping the device in, but not

> +	 * when operating the switch over SPI and putting frames in/out

> +	 * on port 6 (the CPU port). So far we must assume that we

> +	 * cannot access the tag. (See "Internal frame header" section

> +	 * 3.9.1 in the manual.)


I would be really good if we could get this to work even in SPI with the
CPU controlling the switch, I cannot really think of why the 8051 would
have to be involved, because having the 8051 means either the switch is
entirely standalone and runs off an EEPROM (which is additional cost on
your BOM), or the host, through SPI can entirely take over.

Is the datasheet public somehow?

> +	 */

> +	return DSA_TAG_PROTO_NONE;

> +}


[snip]

> +static void vsc73xx_adjust_link(struct dsa_switch *ds, int port,

> +				struct phy_device *phydev)

> +{

> +	struct vsc73xx *vsc = ds->priv;

> +	u32 val;


No is_pseudo_fixed_link() check, you really have to do all of this for
each front-panel port? That is really bad if that is the case... most
switches with front-panel built-in PHYs are at the very least capable of
re-configuring their internal MAC accordingly.

> +

> +	/* Special handling of the CPU-facing port */

> +	if (port == CPU_PORT) {

> +		/* Other ports are already initialized but not this one */

> +		vsc73xx_init_port(vsc, CPU_PORT);

> +		/* Select the external port for this interface (EXT_PORT)

> +		 * Enable the GMII GTX external clock

> +		 * Use double data rate (DDR mode)

> +		 */

> +		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,

> +			      CPU_PORT,

> +			      VSC73XX_ADVPORTM,

> +			      VSC73XX_ADVPORTM_EXT_PORT |

> +			      VSC73XX_ADVPORTM_ENA_GTX |

> +			      VSC73XX_ADVPORTM_DDR_MODE);

> +	}

> +

> +	/* This is the MAC confiuration that always need to happen

> +	 * after a PHY or the CPU port comes up or down.

> +	 */

> +	val = phy_read(phydev, 1);


MII_BMSR

> +	if ((val & 0x0024) != 0x0024) {


BMSR_ANEGCOMPLETE | BMSR_LSTATUS


> +		dev_info(vsc->dev, "port %d: went down\n",

> +			 port);


That would duplicate what PHYLIB already prints, please drop it or
demote it dev_dbg().

> +

> +		/* Disable RX on this port */

> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,

> +				    VSC73XX_MAC_CFG,

> +				    VSC73XX_MAC_CFG_RX_EN, 0);

> +

> +		/* Discard packets */

> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

> +				    VSC73XX_ARBDISC, BIT(port), BIT(port));

> +

> +		/* Wait until queue is empty */

> +		vsc73xx_read(vsc, VSC73XX_BLOCK_ARBITER, 0,

> +			     VSC73XX_ARBEMPTY, &val);

> +		while (!(val & BIT(port))) {

> +			msleep(1);

> +			vsc73xx_read(vsc, VSC73XX_BLOCK_ARBITER, 0,

> +				     VSC73XX_ARBEMPTY, &val);

> +		}


Possibly unbounded loop (if the HW behave incorrectly).

> +

> +		/* Put this port into reset */

> +		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,

> +			      VSC73XX_MAC_CFG_RESET);

> +

> +		/* Accept packets again */

> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

> +				    VSC73XX_ARBDISC, BIT(port), 0);

> +

> +		/* Allow backward dropping of frames from this port */

> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

> +				    VSC73XX_SBACKWDROP, BIT(port), BIT(port));

> +

> +		/* Receive mask (disable forwarding) */

> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,

> +				    VSC73XX_RECVMASK, BIT(port), 0);

> +

> +		return;

> +	}

> +

> +	/* Figure out what speed was negotiated */

> +	val = phy_read(phydev, 0x0a);


MII_STAT1000

> +	if (val & 0x0c00) {


LPA_1000FULL | LPA_1000HALF

> +		dev_info(vsc->dev, "port %d: 1000 Mbit mode full duplex\n",

> +			 port);


Likewise, duplicates PHYLIB messages.

> +

> +		/* Set up default for internal or external RGMII */

> +		if (port == CPU_PORT)

> +			val = VSC73XX_MAC_CFG_1000M_F_RGMII;


You need to look at the CPU port's phy_device->interface for that (which
should be a fixed-link).

> +		else

> +			val = VSC73XX_MAC_CFG_1000M_F_PHY;

> +		vsc73xx_adjust_enable_port(vsc, port, phydev, val);

> +	} else {

> +		val = phy_read(phydev, 0x05);


MII_LPA

> +		val &= 0x05e0;

> +		val >>= 5;

> +		if (val & 0x0c) {

> +			if (val & 0x08) {

> +				val = VSC73XX_MAC_CFG_100_10M_F_PHY;

> +				dev_info(vsc->dev,

> +					 "port %d: 100 Mbit full duplex mode\n",

> +					 port);

> +			} else {

> +				val = VSC73XX_MAC_CFG_100_10M_H_PHY;

> +				dev_info(vsc->dev,

> +					 "port %d: 100 Mbit half duplex mode\n",

> +					 port);

> +			}

> +			vsc73xx_adjust_enable_port(vsc, port, phydev, val);

> +		} else if (val & 0x03) {

> +			if (val & 0x02) {

> +				val = VSC73XX_MAC_CFG_100_10M_F_PHY;

> +				dev_info(vsc->dev,

> +					 "port %d: 10 Mbit full duplex mode\n",

> +					 port);

> +			} else {

> +				val = VSC73XX_MAC_CFG_100_10M_H_PHY;

> +				dev_info(vsc->dev,

> +					 "port %d: 10 Mbit half duplex mode\n",

> +					 port);

> +			}

> +			vsc73xx_adjust_enable_port(vsc, port, phydev, val);

> +		} else {

> +			dev_err(vsc->dev,

> +				"could not adjust link: unknown speed\n");

> +		}

> +	}


A lot of what you are doing here has been dong by genphy_read_status()
and you should be able to extract the link speed/duplex/lpa from the
phy_device itself, and not perform additional MDIO reads.

> +

> +	/* Enable port (forwarding) in the receieve mask */

> +	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,

> +			    VSC73XX_RECVMASK, BIT(port), BIT(port));

> +}

> +

> +static int vsc73xx_port_enable(struct dsa_switch *ds, int port,

> +			       struct phy_device *phy)

> +{

> +	struct vsc73xx *vsc = ds->priv;

> +

> +	dev_info(vsc->dev, "enable port %d\n", port);

> +

> +	/* VSC7385 and VSC7395 have ports 0..4 accessible */

> +	if ((vsc->is_vsc7385 || vsc->is_vsc7395) && port > 4)

> +		return -ENODEV;

> +	if ((vsc->is_vsc7388 || vsc->is_vsc7398) && port > 7)

> +		return -ENODEV;


Humm no, you would not even get there if you told DSA how many ports you
have, see below.

> +

> +	vsc73xx_init_port(vsc, port);

> +

> +	return 0;

> +}

> +

> +static void vsc73xx_port_disable(struct dsa_switch *ds, int port,

> +				 struct phy_device *phy)

> +{

> +	struct vsc73xx *vsc = ds->priv;

> +

> +	/* VSC7385 and VSC7395 have ports 0..4 accessible */

> +	if ((vsc->is_vsc7385 || vsc->is_vsc7395) && port > 4)

> +		return;

> +	if ((vsc->is_vsc7388 || vsc->is_vsc7398) && port > 7)

> +		return;


Likewise.

> +

> +	/* Just put the port into reset */

> +	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port,

> +		      VSC73XX_MAC_CFG, VSC73XX_MAC_CFG_RESET);

> +}

> +

> +const struct vsc73xx_counter *vsc73xx_find_counter(struct vsc73xx *vsc,

> +						   u8 counter,

> +						   bool tx)


Missing static?

[snip]

> +	vsc->gc.label = devm_kasprintf(dev, GFP_KERNEL, "VSC%04x",

> +				       vsc->chipid);

> +	vsc->gc.ngpio = 4;

> +	vsc->gc.owner = THIS_MODULE;

> +	vsc->gc.parent = dev;

> +	vsc->gc.of_node = dev->of_node;

> +	vsc->gc.base = -1;

> +	vsc->gc.get = vsc73xx_gpio_get;

> +	vsc->gc.set = vsc73xx_gpio_set;

> +	vsc->gc.direction_input = vsc73xx_gpio_direction_input;

> +	vsc->gc.direction_output = vsc73xx_gpio_direction_output;

> +	vsc->gc.get_direction = vsc73xx_gpio_get_direction;

> +	vsc->gc.can_sleep = true;

> +	ret = devm_gpiochip_add_data(dev, &vsc->gc, vsc);

> +	if (ret) {

> +		dev_err(dev, "unable to register GPIO chip\n");

> +		dsa_unregister_switch(vsc->ds);

> +		return ret;

> +	}


Would you consider putting this in a separate function so this can be
optionally disabled?

> +

> +	return 0;

> +}

> +

> +static int vsc73xx_remove(struct spi_device *spi)

> +{

> +	struct vsc73xx *vsc = spi_get_drvdata(spi);

> +

> +	dsa_unregister_switch(vsc->ds);

> +	gpiod_set_value(vsc->reset, 1);

> +

> +	return 0;

> +}

> +

> +static const struct of_device_id vsc73xx_of_match[] = {

> +	{

> +		.compatible = "vitesse,vsc7385",


Would not you want to pass additional data here, like the possible port
layout/chip id to be reading?
-- 
Florian
Linus Walleij June 30, 2018, 11:05 a.m. UTC | #2
Hi Florian,

Thanks for your review!

will send a v2 shortly, just some minor feedbacks:

On Thu, Jun 14, 2018 at 6:51 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 06/14/2018 05:35 AM, Linus Walleij wrote:


> > This driver (currently) only takes control of the switch chip over

> > SPI and configures it to route packages around when connected to a

> > CPU port. The chip has embedded PHYs and VLAN support so we model it

> > using DSA as a best fit so we can easily add VLAN support and maybe

> > later also exploit the internal frame header to get more direct

> > control over the switch.

>

> Yes having the internal frame header working would be really great,

> DSA_TAG_PROTO_NONE is really difficult to use without knowing all the

> DSA details which reminds that we should have the following action items:

>

> - document how DSA_TAG_PROTO_NONE behave differently with respect to

> bridges/VLAN configuration and the DSA master device

>

> - possibly introduce DSA_TAG_PROTO_8021Q which would automatically

> partition ports by allocating one VLAN ID per-port (e.g: from top to

> bottom) that would effectively offer the same features/paradigms as what

> a proper header would offer (Port separation, if nothing else) and it

> could be made seemingly automatic from within DSA


To me this makes a lot of sense. I haven't implemented VLAN yet
for this switch chip, but for the Realtek RTL8366RB that I'm also
working with, the vendor driver already does more or less exactly
this by default. It would be great to have the kernel just put this
in place so we have it under control.

The RTL8366 chips does not even seem to have an internal
frame format AFAICT, it seems they don't know what that is.
So they will likely always have to use VLAN for this.

> > The four built-in GPIO lines are exposed using a standard GPIO chip.

>

> What are those typically used for out of curiosity? Is this to connect

> to an EEPROM?


No, there are actually separate address and data lines for connecting
an EEPROM and booting the 8051 CPU from it if need be.

This is likely mostly for the stand-alone usecase. The VSC73xx
and other Vitesse chips are only used with another CPU (like
one running Linux) as an exception, IIUC. The usual usecase is
to put the Vitesse chip with an EEPROM and SW from the vendor
running on the 8051, and box it up as a rack-mounted or small
switch or router.

The vendor provides an SDK for the 8051 CPU including an
SNMP implementation so it can be remotely managed.

If you use this to build a rack-mounted or boxed product,
you likely want to add some "reset" key or a few LEDs to
indicate whatever, and then those 4 GPIO lines come in
handy.

With another OS, like Linux running, these lines can be used
for whatever you want, just an extra GPIO expander
essentially.

> > +     u16                     chipid;

> > +     bool                    is_vsc7385;

> > +     bool                    is_vsc7388;

> > +     bool                    is_vsc7395;

> > +     bool                    is_vsc7398;

>

> How about having an u16/u32 chip_id instead?


OK I already have the u16 chipid there.

So I added some macros like IS_VSC7385() instead.

> > +     /* The switch internally uses a 8 byte header with length,

> > +      * source port, tag, LPA and priority. This is supposedly

> > +      * only accessible when operating the switch using the internal

> > +      * CPU or with an external CPU mapping the device in, but not

> > +      * when operating the switch over SPI and putting frames in/out

> > +      * on port 6 (the CPU port). So far we must assume that we

> > +      * cannot access the tag. (See "Internal frame header" section

> > +      * 3.9.1 in the manual.)

>

> I would be really good if we could get this to work even in SPI with the

> CPU controlling the switch, I cannot really think of why the 8051 would

> have to be involved, because having the 8051 means either the switch is

> entirely standalone and runs off an EEPROM (which is additional cost on

> your BOM), or the host, through SPI can entirely take over.


If you think back on the original usecase in a switch or router it does
make a bit of sense. Another OS is just in the back seat, the 8051
is supposed to do the heavy lifting here.

This is different from devices without an internal CPU where
some "outside OS" (Linux) is supposed to deal with the raw
DSA-tagged frames.

OpenWRT have a bunch of firmware from vendors being uploaded
to the 8051 for different hardware:
http://mirror2.openwrt.org/sources/vsc73x5-ucode.tar.bz2

These look very small, for the internal RAM. I think those are
usually quite silly hacks and unnecessary, the only thing that really
makes sense to load over there is the full SNMP stack that one
can get from the vendor. I haven't disassembled their code though.
That's why I think these devices are better off using this driver
that just take control of the device over SPI.

But it would be neat to figure out if we can get access to the
raw frames. :/

> Is the datasheet public somehow?


Partminer accidentally managed to download and archive it at
one point, but it seems it disappeared from the link:
https://www.buyparts.io/download.php?id=6263858&t=ih

Since Microchips are much more liberal with datasheets I
expect it to become public ASAP, I just got access to all datasheets
when I asked.

> > +static void vsc73xx_adjust_link(struct dsa_switch *ds, int port,

> > +                             struct phy_device *phydev)

> > +{

> > +     struct vsc73xx *vsc = ds->priv;

> > +     u32 val;

>

> No is_pseudo_fixed_link() check, you really have to do all of this for

> each front-panel port? That is really bad if that is the case... most

> switches with front-panel built-in PHYs are at the very least capable of

> re-configuring their internal MAC accordingly.


Sadly this is needed. The manual explicitly says that the MAC
has to be set up depending on speed immediately after the PHY
comes up. I guess the ASIC engineer skipped any integration
of the PHY and MAC and they are just connected, not really
talking to each other.

Luckily the .adjust_link() callback is handy for this.

> > +static const struct of_device_id vsc73xx_of_match[] = {

> > +     {

> > +             .compatible = "vitesse,vsc7385",

>

> Would not you want to pass additional data here, like the possible port

> layout/chip id to be reading?


Since the chip is self-identifying (chip ID) it is not necessary.

If I wanted to be overzealous I could check that the internal
chip ID matches the compatible string (I have seen that some
drivers do that.)

Yours,
Linus Walleij
Vladimir Oltean March 3, 2019, 12:08 p.m. UTC | #3
On Sat, 30 Jun 2018 13:05:47, Linus Walleij wrote:
> On Thu, Jun 14, 2018 at 6:51 PM Florian Fainelli <f.fainelli@gmail.com> wrote:

> > On 06/14/2018 05:35 AM, Linus Walleij wrote:

>

> > > using DSA as a best fit so we can easily add VLAN support and maybe

> > > later also exploit the internal frame header to get more direct

> > > control over the switch.

> >

> > Yes having the internal frame header working would be really great,

> > DSA_TAG_PROTO_NONE is really difficult to use without knowing all the

> > DSA details which reminds that we should have the following action items:

> >

> > - document how DSA_TAG_PROTO_NONE behave differently with respect to

> > bridges/VLAN configuration and the DSA master device

> >

> > - possibly introduce DSA_TAG_PROTO_8021Q which would automatically

> > partition ports by allocating one VLAN ID per-port (e.g: from top to

> > bottom) that would effectively offer the same features/paradigms as what

> > a proper header would offer (Port separation, if nothing else) and it

> > could be made seemingly automatic from within DSA

>

> To me this makes a lot of sense. I haven't implemented VLAN yet

> for this switch chip, but for the Realtek RTL8366RB that I'm also

> working with, the vendor driver already does more or less exactly

> this by default. It would be great to have the kernel just put this

> in place so we have it under control.

>

> The RTL8366 chips does not even seem to have an internal

> frame format AFAICT, it seems they don't know what that is.

> So they will likely always have to use VLAN for this.


Hi Florian, Linus,

I am writing a DSA driver for a switch that doesn't support port separation
through switch tagging, and came across this discussion.
Would you mind to share some thoughts on how port separation with
DSA_TAG_PROTO_NONE would currently work? Thinking about the situations when
(a) ports are standalone
(b) ports are bridged with vlan_filtering=0
(c) ports are bridged with vlan_filtering=1
I am interested in trying to add generic support for DSA_TAG_PROTO_8021Q that
would also cover my hardware. However I do see some potential limitations, so
let me quickly describe what it can and cannot do:
* VLAN filtering cannot be disabled per se in hardware. The switch always
  internally appends the port-configured pvid to all untagged traffic. If
  traffic comes tagged, it enforces VLAN membership on the ingress port, and
  does not append any tag internally but keeps what already was in the frame.
  Each egress port can be either configured to send tagged or untagged frames.
* The switch has a programmable TPID by which it can be told what EtherType it
  should recognize and tag as 802.1Q. Thinking that VLAN filtering disabled can
  be emulated by configuring its TPID as 0x0 instead of 0x8100.
* It is able to recognize double-tagged frames (802.1ad) via a configurable
  TPID2, but it cannot actually be configured to push or pop VLAN headers if
  frames are already tagged on ingress. The only permitted action on frames
  detected as double-tagged is to optionally drop them.
* It cannot retag an ingress VLAN ID on ingress with another one on egress.
  Actually it can, but the feature is for debug purposes and it will actually
  create a frame clone with the new tag. I would like to avoid using this, as I
  would have to drop the original frame and it would look bad in port counters.

For my particular hardware, when vlan_filtering is disabled, this custom
802.1Q tag with a TPID of 0x0 can be used as a make-shift switch tag, given
that a unique pvid is applied to each user port, and the upstream port is
configured in trunk mode (tagged on the egress towards the master).
When vlan_filtering is enabled, obviously I cannot make use of the 0x0 TPID and
I have to restore the 802.1Q one. This means that switch tagging can no longer
work, and port separation has to rely on user configuration.

I did look at RTL8366 and saw that the TPID is not programmable. I'm wondering
whether a generic solution for DSA_TAG_PROTO_8021Q is indeed possible, and
looking for your advice on how it would look like.

Thank you,
-Vladimir
Linus Walleij March 4, 2019, 9:51 a.m. UTC | #4
Hi Vladimir,

On Sun, Mar 3, 2019 at 1:08 PM Vladimir Oltean <olteanv@gmail.com> wrote:

> I am writing a DSA driver for a switch that doesn't support port separation

> through switch tagging, and came across this discussion.

> Would you mind to share some thoughts on how port separation with

> DSA_TAG_PROTO_NONE would currently work? Thinking about the situations when

> (a) ports are standalone

> (b) ports are bridged with vlan_filtering=0

> (c) ports are bridged with vlan_filtering=1


When VLAN filtering is off it kind of becomes broadcast and all
ports see all ports right? The switch is just "one big phy connected
to an external switch" and with "external switch" I mean one
of those boxes you buy and plug into your network as a
network powerstrip. It gets as dumb as that IIUC.

If all ports on the switch are marked "LAN1", "LAN2" etc
on the device this is kind of fine (IIUC) but if one of them
is marked "WAN" this is definitely not good because what
happens is that your bridge your LAN with the WAN
without any firewall (uh-oh). All my devices with the
Vitesse VSC switches have only "LAN" ports, phew.

With VLAN enabled it can at least start to do some kind of
intelligent things if the user knows how to use the "ip"
command (I don't really, lack of experience).

> I am interested in trying to add generic support for DSA_TAG_PROTO_8021Q that

> would also cover my hardware.


This would be nice, I think Andrew Lunn suggested this in the past
(DSA_TAG_PROTO_VLAN or so) and I looked at it briefly but I'm
not smart enough with VLANs to understand what needs to be
done.

> However I do see some potential limitations, so

> let me quickly describe what it can and cannot do:

> * VLAN filtering cannot be disabled per se in hardware. The switch always

>   internally appends the port-configured pvid to all untagged traffic. If

>   traffic comes tagged, it enforces VLAN membership on the ingress port, and

>   does not append any tag internally but keeps what already was in the frame.

>   Each egress port can be either configured to send tagged or untagged frames.

> * The switch has a programmable TPID by which it can be told what EtherType it

>   should recognize and tag as 802.1Q. Thinking that VLAN filtering disabled can

>   be emulated by configuring its TPID as 0x0 instead of 0x8100.


I think I heard about that trick before.

I also heard about using VLAN 0 which has some undefined properties.

> * It is able to recognize double-tagged frames (802.1ad) via a configurable

>   TPID2, but it cannot actually be configured to push or pop VLAN headers if

>   frames are already tagged on ingress. The only permitted action on frames

>   detected as double-tagged is to optionally drop them.

> * It cannot retag an ingress VLAN ID on ingress with another one on egress.

>   Actually it can, but the feature is for debug purposes and it will actually

>   create a frame clone with the new tag. I would like to avoid using this, as I

>   would have to drop the original frame and it would look bad in port counters.

>

> For my particular hardware, when vlan_filtering is disabled, this custom

> 802.1Q tag with a TPID of 0x0 can be used as a make-shift switch tag, given

> that a unique pvid is applied to each user port, and the upstream port is

> configured in trunk mode (tagged on the egress towards the master).

> When vlan_filtering is enabled, obviously I cannot make use of the 0x0 TPID and

> I have to restore the 802.1Q one. This means that switch tagging can no longer

> work, and port separation has to rely on user configuration.

>

> I did look at RTL8366 and saw that the TPID is not programmable.


Right, it's either 0x8100 or Realtek RRCP funnyspeak if I get it right.

> I'm wondering

> whether a generic solution for DSA_TAG_PROTO_8021Q is indeed possible, and

> looking for your advice on how it would look like.


I was hacking on the RTL8366RB the other day.

Currently it probes default by setting up VLAN 1..6 where these correspond
to ports 0..5, port 5 is the CPU port. 4 ports are LAN one port WAN and
one port CPU. It creates one VLAN per port and makes the CPU port
member of all VLANs and all ports members of the CPU port VLAN.

Port 0 gets VLAN 1, Port 1 gets VLAN 2 etc. The port mask for each
port includes that port itself and the CPU port, and the port mask for
the CPU port is special and includes all ports.

This allows all the ports to talk freely to the CPU port and makes for
isolation between WAN and LAN and the different LAN ports by
default, which is neat.

This set-up is just hard-coded at probe()
drivers/net/dsa/rtl8366.c function rtl8366_init_vlan().

When I say it sets up VLAN it just hard-codes that into the
hardware so the network can be brought up, the kernel at
large and userspace is not aware.

(This was what the vendor driver hack was doing, mind that I don't
even have a datasheet for this ASIC, just an unintelligible code
dump...)

I was thinking that this kind of VLAN setup would be what
DSA_TAG_PROTO_VLAN would do by calling the proper API
VLAN setup calls.

This current setup is problematic because userspace "ip" does
not know about it, and e.g. OpenWrt userspace starts to
"disable filtering" on all ports when it initializes and that is not
quite working. Userspace has no idea that it can't use VLAN
1 thru 6 or how to inspect existing VLANs (IIUC). "ip link show"
shows nothing.

I don't even know where the state of existing VLANs are
stored. I guess in the kernel so DSA_TAG_PROTO_VLAN would
register these VLANs properly and userspace would be able to
see them using "ip link show" as eth0.1, eth0.2, eth0.3 etc
(the CPU port is == eth0).

I have an experimental patch where I use VLAN 0 as "filtering off"
VLAN, so that I can make a port member of VLAN 0 and set pvid
accordingly to emulate "VLAN disabled", as there is no way to
actually disable VLAN filtering on this switch. I didn't submit this
patch because I'm not sure it is a good idea.

Yours,
Linus Walleij
Vladimir Oltean March 4, 2019, 12:14 p.m. UTC | #5
On Mon, 4 Mar 2019 at 11:52, Linus Walleij <linus.walleij@linaro.org> wrote:
>

> Hi Vladimir,


Hi Linus,

>

> On Sun, Mar 3, 2019 at 1:08 PM Vladimir Oltean <olteanv@gmail.com> wrote:

>

> > I am writing a DSA driver for a switch that doesn't support port separation

> > through switch tagging, and came across this discussion.

> > Would you mind to share some thoughts on how port separation with

> > DSA_TAG_PROTO_NONE would currently work? Thinking about the situations when

> > (a) ports are standalone

> > (b) ports are bridged with vlan_filtering=0

> > (c) ports are bridged with vlan_filtering=1

>

> When VLAN filtering is off it kind of becomes broadcast and all

> ports see all ports right? The switch is just "one big phy connected

> to an external switch" and with "external switch" I mean one

> of those boxes you buy and plug into your network as a

> network powerstrip. It gets as dumb as that IIUC.

>

> If all ports on the switch are marked "LAN1", "LAN2" etc

> on the device this is kind of fine (IIUC) but if one of them

> is marked "WAN" this is definitely not good because what

> happens is that your bridge your LAN with the WAN

> without any firewall (uh-oh). All my devices with the

> Vitesse VSC switches have only "LAN" ports, phew.

>


I may be in error, but I don't think we have the same understanding of what
VLAN filtering is. As far as I understand, no VLAN filtering means that
hardware is required to not parse, push or pop VLAN tags in whatever frames
it receives or sends (tagged or not). Conversely, VLAN filtering enabled means
hardware is required to parse VLAN information and enforce port membership.

Now, this is orthogonal to forwarding, in a way. I believe that if you want
to isolate two front-panel ports from other two front-panel ports (be they WAN
or LAN), you would want to add them to different bridges (br0 and br1).
But indeed forwarding domain can also be limited using VLAN. In this case a
frame will be passed from one port to another based on a compound decision
(L2 forwarding rules allow AND VLAN port membership domain allows). You would
want to use VLAN when you want a single port to be a member of multiple
domains, because you can't put a single netdevice (front-panel switch port)
in multiple bridges.
All I'm really saying is that I don't think that inside the port_bridge_join
callback, configuring forwarding based on VLAN is a sane thing to do.

I do see that neither vitesse-vsc73xx nor rtl8366 do implement the bridge
membership callbacks, and as I don't happen to know nearly enough about DSA, I
wonder at what moment in time does forwarding get activated between ports.

> With VLAN enabled it can at least start to do some kind of

> intelligent things if the user knows how to use the "ip"

> command (I don't really, lack of experience).

>

> > I am interested in trying to add generic support for DSA_TAG_PROTO_8021Q that

> > would also cover my hardware.

>

> This would be nice, I think Andrew Lunn suggested this in the past

> (DSA_TAG_PROTO_VLAN or so) and I looked at it briefly but I'm

> not smart enough with VLANs to understand what needs to be

> done.

>

> > However I do see some potential limitations, so

> > let me quickly describe what it can and cannot do:

> > * VLAN filtering cannot be disabled per se in hardware. The switch always

> >   internally appends the port-configured pvid to all untagged traffic. If

> >   traffic comes tagged, it enforces VLAN membership on the ingress port, and

> >   does not append any tag internally but keeps what already was in the frame.

> >   Each egress port can be either configured to send tagged or untagged frames.

> > * The switch has a programmable TPID by which it can be told what EtherType it

> >   should recognize and tag as 802.1Q. Thinking that VLAN filtering disabled can

> >   be emulated by configuring its TPID as 0x0 instead of 0x8100.

>

> I think I heard about that trick before.

>

> I also heard about using VLAN 0 which has some undefined properties.


I was really talking about making the TPID 0x0 here and not VID:

+--------+---+---+------- +
|  TPID  |PCP|DEI|   VID  |
+--------+---+---+--------+

As you know, TPID overlaps with EtherType field which has various meanings.
When it's smaller than ETH_P_802_3_MIN (include/uapi/linux/if_ether.h), the
EtherType is taken to mean 'frame length' - an obsolete interpretation of the
field. Hence my proposal was to actually hijack an EtherType that otherwise
means 'this Ethernet frame has a length of zero', and collect some feedback on
the sanity of this idea.

>

> > * It is able to recognize double-tagged frames (802.1ad) via a configurable

> >   TPID2, but it cannot actually be configured to push or pop VLAN headers if

> >   frames are already tagged on ingress. The only permitted action on frames

> >   detected as double-tagged is to optionally drop them.

> > * It cannot retag an ingress VLAN ID on ingress with another one on egress.

> >   Actually it can, but the feature is for debug purposes and it will actually

> >   create a frame clone with the new tag. I would like to avoid using this, as I

> >   would have to drop the original frame and it would look bad in port counters.

> >

> > For my particular hardware, when vlan_filtering is disabled, this custom

> > 802.1Q tag with a TPID of 0x0 can be used as a make-shift switch tag, given

> > that a unique pvid is applied to each user port, and the upstream port is

> > configured in trunk mode (tagged on the egress towards the master).

> > When vlan_filtering is enabled, obviously I cannot make use of the 0x0 TPID and

> > I have to restore the 802.1Q one. This means that switch tagging can no longer

> > work, and port separation has to rely on user configuration.

> >

> > I did look at RTL8366 and saw that the TPID is not programmable.

>

> Right, it's either 0x8100 or Realtek RRCP funnyspeak if I get it right.

>

> > I'm wondering

> > whether a generic solution for DSA_TAG_PROTO_8021Q is indeed possible, and

> > looking for your advice on how it would look like.

>

> I was hacking on the RTL8366RB the other day.

>

> Currently it probes default by setting up VLAN 1..6 where these correspond

> to ports 0..5, port 5 is the CPU port. 4 ports are LAN one port WAN and

> one port CPU. It creates one VLAN per port and makes the CPU port

> member of all VLANs and all ports members of the CPU port VLAN.

>

> Port 0 gets VLAN 1, Port 1 gets VLAN 2 etc. The port mask for each

> port includes that port itself and the CPU port, and the port mask for

> the CPU port is special and includes all ports.

>

> This allows all the ports to talk freely to the CPU port and makes for

> isolation between WAN and LAN and the different LAN ports by

> default, which is neat.

>

> This set-up is just hard-coded at probe()

> drivers/net/dsa/rtl8366.c function rtl8366_init_vlan().

>

> When I say it sets up VLAN it just hard-codes that into the

> hardware so the network can be brought up, the kernel at

> large and userspace is not aware.

>

> (This was what the vendor driver hack was doing, mind that I don't

> even have a datasheet for this ASIC, just an unintelligible code

> dump...)

>

> I was thinking that this kind of VLAN setup would be what

> DSA_TAG_PROTO_VLAN would do by calling the proper API

> VLAN setup calls.

>

> This current setup is problematic because userspace "ip" does

> not know about it, and e.g. OpenWrt userspace starts to

> "disable filtering" on all ports when it initializes and that is not

> quite working. Userspace has no idea that it can't use VLAN

> 1 thru 6 or how to inspect existing VLANs (IIUC). "ip link show"

> shows nothing.

>

> I don't even know where the state of existing VLANs are

> stored. I guess in the kernel so DSA_TAG_PROTO_VLAN would

> register these VLANs properly and userspace would be able to

> see them using "ip link show" as eth0.1, eth0.2, eth0.3 etc

> (the CPU port is == eth0).

>


I don't know with exact certainty but if I were to guess, I'd say the generic
bridge driver keeps VLAN membership information (that you see in 'bridge vlan
show'). And here is where the conundrum is. If one creates a bridge using
'ip link add name br0 type bridge vlan_filtering 0', then when you add the
switch ports to it, the bridge code will never attempt to install VLANs into
it. So switch tagging with 802.1Q would only work when vlan_filtering is
active. With vlan_filtering, I don't see it as a hard thing to achieve given my
current understanding (there need to be as many VLAN sub-interfaces on the
master port as there are front-panel switch ports).
The difficulty is what to do when userspace explicitly requests that switch
should not get smart about VLANs (vlan_filtering 0).
In my previous email, I explained that my hardware's architecture actually
prevents me from avoiding being smart with VLANs (the TPID 0x0 thing). I can
actually make use of that pseudo-VLAN tag even when vlan_filtering is 0
(because technically it's not real VLAN TPID). So the question is not about
myself but about the general case.

> I have an experimental patch where I use VLAN 0 as "filtering off"

> VLAN, so that I can make a port member of VLAN 0 and set pvid

> accordingly to emulate "VLAN disabled", as there is no way to

> actually disable VLAN filtering on this switch. I didn't submit this

> patch because I'm not sure it is a good idea.

>


What happens if you receive tagged frames using one of VLANs 1, 2, 3, 4, 5, 6?
Is the VLAN tag removed on egress, or used in any way? If it is, I don't
believe that vlan_filtering is truly disabled then.
Linus Walleij March 4, 2019, 12:26 p.m. UTC | #6
Hi Vladimir,

On Mon, Mar 4, 2019 at 1:14 PM Vladimir Oltean <olteanv@gmail.com> wrote:

> I may be in error, but I don't think we have the same understanding of what

> VLAN filtering is. As far as I understand, no VLAN filtering means that

> hardware is required to not parse, push or pop VLAN tags in whatever frames

> it receives or sends (tagged or not). Conversely, VLAN filtering enabled means

> hardware is required to parse VLAN information and enforce port membership.


OK I get it now, I hope I don't forget it the next 5 minutes.
I think I have the wrong mental model about a whole lot of things
regarding VLANs.

> Now, this is orthogonal to forwarding, in a way. I believe that if you want

> to isolate two front-panel ports from other two front-panel ports (be they WAN

> or LAN), you would want to add them to different bridges (br0 and br1).

> But indeed forwarding domain can also be limited using VLAN. In this case a

> frame will be passed from one port to another based on a compound decision

> (L2 forwarding rules allow AND VLAN port membership domain allows). You would

> want to use VLAN when you want a single port to be a member of multiple

> domains, because you can't put a single netdevice (front-panel switch port)

> in multiple bridges.

> All I'm really saying is that I don't think that inside the port_bridge_join

> callback, configuring forwarding based on VLAN is a sane thing to do.

>

> I do see that neither vitesse-vsc73xx nor rtl8366 do implement the bridge

> membership callbacks, and as I don't happen to know nearly enough about DSA, I

> wonder at what moment in time does forwarding get activated between ports.


I think I should seriously look at other DSA drivers implementing this
because I think that is what I really want to achieve here.

Yours,
Linus Walleij
Andrew Lunn March 4, 2019, 1:53 p.m. UTC | #7
> I do see that neither vitesse-vsc73xx nor rtl8366 do implement the bridge

> membership callbacks, and as I don't happen to know nearly enough about DSA, I

> wonder at what moment in time does forwarding get activated between ports.


If those callbacks are not implement, it means the software bridge
does the forwarding. The hardware is not used.

The DSA model allows you to start simple and just implement something
like a port multiplexor. All frames get passed to the host, and the
host then decides what to do with them.

You can then add more callbacks and make more use of the hardware,
e.g. get the hardware to bridge between ports.

If the hardware is not capable of supporting vlan filtering, but you
can support simpler modes, you should implement what you can, and
return an error when asked to do something which it cannot support.

       Andrew
Vladimir Oltean March 4, 2019, 2:10 p.m. UTC | #8
On Mon, 4 Mar 2019 at 15:53, Andrew Lunn <andrew@lunn.ch> wrote:
>

> > I do see that neither vitesse-vsc73xx nor rtl8366 do implement the bridge

> > membership callbacks, and as I don't happen to know nearly enough about DSA, I

> > wonder at what moment in time does forwarding get activated between ports.

>

> If those callbacks are not implement, it means the software bridge

> does the forwarding. The hardware is not used.

>

> The DSA model allows you to start simple and just implement something

> like a port multiplexor. All frames get passed to the host, and the

> host then decides what to do with them.

>

> You can then add more callbacks and make more use of the hardware,

> e.g. get the hardware to bridge between ports.

>

> If the hardware is not capable of supporting vlan filtering, but you

> can support simpler modes, you should implement what you can, and

> return an error when asked to do something which it cannot support.

>

>        Andrew


Hi Andrew,

Thank you for the explanation, it does make sense.
Would you mind sharing a thought about the feasability of DSA_TAG_PROTO_8021Q
in light of the required handling that was discussed in the previous emails?

Regards,
-Vladimir
diff mbox series

Patch

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 2b81b97e994f..2f6207b969e3 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -76,4 +76,16 @@  config NET_DSA_SMSC_LAN9303_MDIO
 	  Enable access functions if the SMSC/Microchip LAN9303 is configured
 	  for MDIO managed mode.
 
+config NET_DSA_VITESSE_VSC73XX
+	tristate "Vitesse VSC7385/7388/7395/7398 support"
+	depends on OF && SPI
+	depends on NET_DSA
+	select FIXED_PHY
+	select VITESSE_PHY
+	select NET_DSA_TAG_TRAILER
+	select GPIOLIB
+	---help---
+	  This enables support for the Vitesse VSC7385, VSC7388,
+	  VSC7395 and VSC7398 SparX integrated ethernet switches.
+
 endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 15c2a831edf1..d4f873ae2f6a 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -11,6 +11,7 @@  obj-$(CONFIG_NET_DSA_QCA8K)	+= qca8k.o
 obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
 obj-$(CONFIG_NET_DSA_SMSC_LAN9303_I2C) += lan9303_i2c.o
 obj-$(CONFIG_NET_DSA_SMSC_LAN9303_MDIO) += lan9303_mdio.o
+obj-$(CONFIG_NET_DSA_VITESSE_VSC73XX) += vitesse-vsc73xx.o
 obj-y				+= b53/
 obj-y				+= microchip/
 obj-y				+= mv88e6xxx/
diff --git a/drivers/net/dsa/vitesse-vsc73xx.c b/drivers/net/dsa/vitesse-vsc73xx.c
new file mode 100644
index 000000000000..cf478856e53f
--- /dev/null
+++ b/drivers/net/dsa/vitesse-vsc73xx.c
@@ -0,0 +1,1362 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* DSA driver for:
+ * Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
+ * Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
+ *
+ * These switches have a built-in 8051 CPU and can download and execute a
+ * firmware in this CPU. They can also be configured to use an external CPU
+ * handling the switch in a memory-mapped manner by connecting to that external
+ * CPU's memory bus.
+ *
+ * This driver (currently) only takes control of the switch chip over SPI and
+ * configures it to route packages around when connected to a CPU port. The
+ * chip has embedded PHYs and VLAN support so we model it using DSA.
+ *
+ * Copyright (C) 2018 Linus Wallej <linus.walleij@linaro.org>
+ * Includes portions of code from the firmware uploader by:
+ * Copyright (C) 2009 Gabor Juhos <juhosg@openwrt.org>
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_mdio.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/bitops.h>
+#include <linux/if_bridge.h>
+#include <linux/etherdevice.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/random.h>
+#include <net/dsa.h>
+
+#define VSC73XX_BLOCK_MAC	0x1 /* Subblocks 0-4, 6 (CPU port) */
+#define VSC73XX_BLOCK_ANALYZER	0x2 /* Only subblock 0 */
+#define VSC73XX_BLOCK_MII	0x3 /* Subblocks 0 and 1 */
+#define VSC73XX_BLOCK_MEMINIT	0x3 /* Only subblock 2 */
+#define VSC73XX_BLOCK_CAPTURE	0x4 /* Only subblock 2 */
+#define VSC73XX_BLOCK_ARBITER	0x5 /* Only subblock 0 */
+#define VSC73XX_BLOCK_SYSTEM	0x7 /* Only subblock 0 */
+
+#define CPU_PORT	6 /* CPU port */
+
+/* MAC Block registers */
+#define VSC73XX_MAC_CFG		0x00
+#define VSC73XX_MACHDXGAP	0x02
+#define VSC73XX_FCCONF		0x04
+#define VSC73XX_FCMACHI		0x08
+#define VSC73XX_FCMACLO		0x0c
+#define VSC73XX_MAXLEN		0x10
+#define VSC73XX_ADVPORTM	0x19
+#define VSC73XX_TXUPDCFG	0x24
+#define VSC73XX_TXQ_SELECT_CFG	0x28
+#define VSC73XX_RXOCT		0x50
+#define VSC73XX_TXOCT		0x51
+#define VSC73XX_C_RX0		0x52
+#define VSC73XX_C_RX1		0x53
+#define VSC73XX_C_RX2		0x54
+#define VSC73XX_C_TX0		0x55
+#define VSC73XX_C_TX1		0x56
+#define VSC73XX_C_TX2		0x57
+#define VSC73XX_C_CFG		0x58
+#define VSC73XX_CAT_DROP	0x6e
+#define VSC73XX_CAT_PR_MISC_L2	0x6f
+#define VSC73XX_CAT_PR_USR_PRIO	0x75
+#define VSC73XX_Q_MISC_CONF	0xdf
+
+/* MAC_CFG register bits */
+#define VSC73XX_MAC_CFG_WEXC_DIS	BIT(31)
+#define VSC73XX_MAC_CFG_PORT_RST	BIT(29)
+#define VSC73XX_MAC_CFG_TX_EN		BIT(28)
+#define VSC73XX_MAC_CFG_SEED_LOAD	BIT(27)
+#define VSC73XX_MAC_CFG_SEED_MASK	GENMASK(26, 19)
+#define VSC73XX_MAC_CFG_SEED_OFFSET	19
+#define VSC73XX_MAC_CFG_FDX		BIT(18)
+#define VSC73XX_MAC_CFG_GIGA_MODE	BIT(17)
+#define VSC73XX_MAC_CFG_RX_EN		BIT(16)
+#define VSC73XX_MAC_CFG_VLAN_DBLAWR	BIT(15)
+#define VSC73XX_MAC_CFG_VLAN_AWR	BIT(14)
+#define VSC73XX_MAC_CFG_100_BASE_T	BIT(13) /* Not in manual */
+#define VSC73XX_MAC_CFG_TX_IPG_MASK	GENMASK(10, 6)
+#define VSC73XX_MAC_CFG_TX_IPG_OFFSET	6
+#define VSC73XX_MAC_CFG_TX_IPG_1000M	(6 << VSC73XX_MAC_CFG_TX_IPG_OFFSET)
+#define VSC73XX_MAC_CFG_TX_IPG_100_10M	(17 << VSC73XX_MAC_CFG_TX_IPG_OFFSET)
+#define VSC73XX_MAC_CFG_MAC_RX_RST	BIT(5)
+#define VSC73XX_MAC_CFG_MAC_TX_RST	BIT(4)
+#define VSC73XX_MAC_CFG_CLK_SEL_MASK	GENMASK(2, 0)
+#define VSC73XX_MAC_CFG_CLK_SEL_OFFSET	0
+#define VSC73XX_MAC_CFG_CLK_SEL_1000M	1
+#define VSC73XX_MAC_CFG_CLK_SEL_100M	2
+#define VSC73XX_MAC_CFG_CLK_SEL_10M	3
+#define VSC73XX_MAC_CFG_CLK_SEL_EXT	4
+
+#define VSC73XX_MAC_CFG_1000M_F_PHY	(VSC73XX_MAC_CFG_FDX | \
+					 VSC73XX_MAC_CFG_GIGA_MODE | \
+					 VSC73XX_MAC_CFG_TX_IPG_1000M | \
+					 VSC73XX_MAC_CFG_CLK_SEL_EXT)
+#define VSC73XX_MAC_CFG_100_10M_F_PHY	(VSC73XX_MAC_CFG_FDX | \
+					 VSC73XX_MAC_CFG_TX_IPG_100_10M | \
+					 VSC73XX_MAC_CFG_CLK_SEL_EXT)
+#define VSC73XX_MAC_CFG_100_10M_H_PHY	(VSC73XX_MAC_CFG_TX_IPG_100_10M | \
+					 VSC73XX_MAC_CFG_CLK_SEL_EXT)
+#define VSC73XX_MAC_CFG_1000M_F_RGMII	(VSC73XX_MAC_CFG_FDX | \
+					 VSC73XX_MAC_CFG_GIGA_MODE | \
+					 VSC73XX_MAC_CFG_TX_IPG_1000M | \
+					 VSC73XX_MAC_CFG_CLK_SEL_1000M)
+#define VSC73XX_MAC_CFG_RESET		(VSC73XX_MAC_CFG_PORT_RST | \
+					 VSC73XX_MAC_CFG_MAC_RX_RST | \
+					 VSC73XX_MAC_CFG_MAC_TX_RST)
+
+/* Flow control register bits */
+#define VSC73XX_FCCONF_ZERO_PAUSE_EN	BIT(17)
+#define VSC73XX_FCCONF_FLOW_CTRL_OBEY	BIT(16)
+#define VSC73XX_FCCONF_PAUSE_VAL_MASK	GENMASK(15, 0)
+
+/* ADVPORTM advanced port setup register bits */
+#define VSC73XX_ADVPORTM_IFG_PPM	BIT(7)
+#define VSC73XX_ADVPORTM_EXC_COL_CONT	BIT(6)
+#define VSC73XX_ADVPORTM_EXT_PORT	BIT(5)
+#define VSC73XX_ADVPORTM_INV_GTX	BIT(4)
+#define VSC73XX_ADVPORTM_ENA_GTX	BIT(3)
+#define VSC73XX_ADVPORTM_DDR_MODE	BIT(2)
+#define VSC73XX_ADVPORTM_IO_LOOPBACK	BIT(1)
+#define VSC73XX_ADVPORTM_HOST_LOOPBACK	BIT(0)
+
+/* CAT_DROP categorizer frame dropping register bits */
+#define VSC73XX_CAT_DROP_DROP_MC_SMAC_ENA	BIT(6)
+#define VSC73XX_CAT_DROP_FWD_CTRL_ENA		BIT(4)
+#define VSC73XX_CAT_DROP_FWD_PAUSE_ENA		BIT(3)
+#define VSC73XX_CAT_DROP_UNTAGGED_ENA		BIT(2)
+#define VSC73XX_CAT_DROP_TAGGED_ENA		BIT(1)
+#define VSC73XX_CAT_DROP_NULL_MAC_ENA		BIT(0)
+
+#define VSC73XX_Q_MISC_CONF_EXTENT_MEM		BIT(31)
+#define VSC73XX_Q_MISC_CONF_EARLY_TX_MASK	GENMASK(4, 1)
+#define VSC73XX_Q_MISC_CONF_EARLY_TX_512	(1 << 1)
+#define VSC73XX_Q_MISC_CONF_MAC_PAUSE_MODE	BIT(0)
+
+/* Frame analyzer block 2 registers */
+#define VSC73XX_STORMLIMIT	0x02
+#define VSC73XX_ADVLEARN	0x03
+#define VSC73XX_IFLODMSK	0x04
+#define VSC73XX_VLANMASK	0x05
+#define VSC73XX_MACHDATA	0x06
+#define VSC73XX_MACLDATA	0x07
+#define VSC73XX_ANMOVED		0x08
+#define VSC73XX_ANAGEFIL	0x09
+#define VSC73XX_ANEVENTS	0x0a
+#define VSC73XX_ANCNTMASK	0x0b
+#define VSC73XX_ANCNTVAL	0x0c
+#define VSC73XX_LEARNMASK	0x0d
+#define VSC73XX_UFLODMASK	0x0e
+#define VSC73XX_MFLODMASK	0x0f
+#define VSC73XX_RECVMASK	0x10
+#define VSC73XX_AGGRCTRL	0x20
+#define VSC73XX_AGGRMSKS	0x30 /* Until 0x3f */
+#define VSC73XX_DSTMASKS	0x40 /* Until 0x7f */
+#define VSC73XX_SRCMASKS	0x80 /* Until 0x87 */
+#define VSC73XX_CAPENAB		0xa0
+#define VSC73XX_MACACCESS	0xb0
+#define VSC73XX_IPMCACCESS	0xb1
+#define VSC73XX_MACTINDX	0xc0
+#define VSC73XX_VLANACCESS	0xd0
+#define VSC73XX_VLANTIDX	0xe0
+#define VSC73XX_AGENCTRL	0xf0
+#define VSC73XX_CAPRST		0xff
+
+#define VSC73XX_MACACCESS_CPU_COPY		BIT(14)
+#define VSC73XX_MACACCESS_FWD_KILL		BIT(13)
+#define VSC73XX_MACACCESS_IGNORE_VLAN		BIT(12)
+#define VSC73XX_MACACCESS_AGED_FLAG		BIT(11)
+#define VSC73XX_MACACCESS_VALID			BIT(10)
+#define VSC73XX_MACACCESS_LOCKED		BIT(9)
+#define VSC73XX_MACACCESS_DEST_IDX_MASK		GENMASK(8, 3)
+#define VSC73XX_MACACCESS_CMD_MASK		GENMASK(2, 0)
+#define VSC73XX_MACACCESS_CMD_IDLE		0
+#define VSC73XX_MACACCESS_CMD_LEARN		1
+#define VSC73XX_MACACCESS_CMD_FORGET		2
+#define VSC73XX_MACACCESS_CMD_AGE_TABLE		3
+#define VSC73XX_MACACCESS_CMD_FLUSH_TABLE	4
+#define VSC73XX_MACACCESS_CMD_CLEAR_TABLE	5
+#define VSC73XX_MACACCESS_CMD_READ_ENTRY	6
+#define VSC73XX_MACACCESS_CMD_WRITE_ENTRY	7
+
+#define VSC73XX_VLANACCESS_LEARN_DISABLED	BIT(30)
+#define VSC73XX_VLANACCESS_VLAN_MIRROR		BIT(29)
+#define VSC73XX_VLANACCESS_VLAN_SRC_CHECK	BIT(28)
+#define VSC73XX_VLANACCESS_VLAN_PORT_MASK	GENMASK(9, 2)
+#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_MASK	GENMASK(2, 0)
+#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_IDLE	0
+#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_READ_ENTRY	1
+#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_WRITE_ENTRY	2
+#define VSC73XX_VLANACCESS_VLAN_TBL_CMD_CLEAR_TABLE	3
+
+/* MII block 3 registers */
+#define VSC73XX_MII_STAT	0x0
+#define VSC73XX_MII_CMD		0x1
+#define VSC73XX_MII_DATA	0x2
+
+/* Arbiter block 5 registers */
+#define VSC73XX_ARBEMPTY		0x0c
+#define VSC73XX_ARBDISC			0x0e
+#define VSC73XX_SBACKWDROP		0x12
+#define VSC73XX_DBACKWDROP		0x13
+#define VSC73XX_ARBBURSTPROB		0x15
+
+/* System block 7 registers */
+#define VSC73XX_ICPU_SIPAD		0x01
+#define VSC73XX_GMIIDELAY		0x05
+#define VSC73XX_ICPU_CTRL		0x10
+#define VSC73XX_ICPU_ADDR		0x11
+#define VSC73XX_ICPU_SRAM		0x12
+#define VSC73XX_HWSEM			0x13
+#define VSC73XX_GLORESET		0x14
+#define VSC73XX_ICPU_MBOX_VAL		0x15
+#define VSC73XX_ICPU_MBOX_SET		0x16
+#define VSC73XX_ICPU_MBOX_CLR		0x17
+#define VSC73XX_CHIPID			0x18
+#define VSC73XX_GPIO			0x34
+
+#define VSC73XX_GMIIDELAY_GMII0_GTXDELAY_NONE	0
+#define VSC73XX_GMIIDELAY_GMII0_GTXDELAY_1_4_NS	1
+#define VSC73XX_GMIIDELAY_GMII0_GTXDELAY_1_7_NS	2
+#define VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS	3
+
+#define VSC73XX_GMIIDELAY_GMII0_RXDELAY_NONE	(0 << 4)
+#define VSC73XX_GMIIDELAY_GMII0_RXDELAY_1_4_NS	(1 << 4)
+#define VSC73XX_GMIIDELAY_GMII0_RXDELAY_1_7_NS	(2 << 4)
+#define VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS	(3 << 4)
+
+#define VSC73XX_ICPU_CTRL_WATCHDOG_RST	BIT(31)
+#define VSC73XX_ICPU_CTRL_CLK_DIV_MASK	GENMASK(12, 8)
+#define VSC73XX_ICPU_CTRL_SRST_HOLD	BIT(7)
+#define VSC73XX_ICPU_CTRL_ICPU_PI_EN	BIT(6)
+#define VSC73XX_ICPU_CTRL_BOOT_EN	BIT(3)
+#define VSC73XX_ICPU_CTRL_EXT_ACC_EN	BIT(2)
+#define VSC73XX_ICPU_CTRL_CLK_EN	BIT(1)
+#define VSC73XX_ICPU_CTRL_SRST		BIT(0)
+
+#define VSC73XX_CHIPID_ID_SHIFT		12
+#define VSC73XX_CHIPID_ID_MASK		0xffff
+#define VSC73XX_CHIPID_REV_SHIFT	28
+#define VSC73XX_CHIPID_REV_MASK		0xf
+#define VSC73XX_CHIPID_ID_7385		0x7385
+#define VSC73XX_CHIPID_ID_7388		0x7388
+#define VSC73XX_CHIPID_ID_7395		0x7395
+#define VSC73XX_CHIPID_ID_7398		0x7398
+
+#define VSC73XX_GLORESET_STROBE		BIT(4)
+#define VSC73XX_GLORESET_ICPU_LOCK	BIT(3)
+#define VSC73XX_GLORESET_MEM_LOCK	BIT(2)
+#define VSC73XX_GLORESET_PHY_RESET	BIT(1)
+#define VSC73XX_GLORESET_MASTER_RESET	BIT(0)
+
+#define VSC73XX_CMD_MODE_READ		0
+#define VSC73XX_CMD_MODE_WRITE		1
+#define VSC73XX_CMD_MODE_SHIFT		4
+#define VSC73XX_CMD_BLOCK_SHIFT		5
+#define VSC73XX_CMD_BLOCK_MASK		0x7
+#define VSC73XX_CMD_SUBBLOCK_MASK	0xf
+
+#define VSC7385_CLOCK_DELAY		((3 << 4) | 3)
+#define VSC7385_CLOCK_DELAY_MASK	((3 << 4) | 3)
+
+#define VSC73XX_ICPU_CTRL_STOP	(VSC73XX_ICPU_CTRL_SRST_HOLD | \
+				 VSC73XX_ICPU_CTRL_BOOT_EN | \
+				 VSC73XX_ICPU_CTRL_EXT_ACC_EN)
+
+#define VSC73XX_ICPU_CTRL_START	(VSC73XX_ICPU_CTRL_CLK_DIV | \
+				 VSC73XX_ICPU_CTRL_BOOT_EN | \
+				 VSC73XX_ICPU_CTRL_CLK_EN | \
+				 VSC73XX_ICPU_CTRL_SRST)
+
+/**
+ * struct vsc73xx - VSC73xx state container
+ */
+struct vsc73xx {
+	struct device		*dev;
+	struct gpio_desc	*reset;
+	struct spi_device	*spi;
+	struct dsa_switch	*ds;
+	struct gpio_chip	gc;
+	u16			chipid;
+	bool			is_vsc7385;
+	bool			is_vsc7388;
+	bool			is_vsc7395;
+	bool			is_vsc7398;
+	u8			addr[ETH_ALEN];
+	struct mutex		lock; /* Protects SPI traffic */
+};
+
+struct vsc73xx_counter {
+	u8 counter;
+	const char *name;
+};
+
+/* Counters are named according to the MIB standards where applicable.
+ * Some counters are custom, non-standard. The standard counters are
+ * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex
+ * 30A Counters.
+ */
+static const struct vsc73xx_counter vsc73xx_rx_counters[] = {
+	{ 0, "RxEtherStatsPkts" },
+	{ 1, "RxBroadcast+MulticastPkts" }, /* non-standard counter */
+	{ 2, "RxTotalErrorPackets" }, /* non-standard counter */
+	{ 3, "RxEtherStatsBroadcastPkts" },
+	{ 4, "RxEtherStatsMulticastPkts" },
+	{ 5, "RxEtherStatsPkts64Octets" },
+	{ 6, "RxEtherStatsPkts65to127Octets" },
+	{ 7, "RxEtherStatsPkts128to255Octets" },
+	{ 8, "RxEtherStatsPkts256to511Octets" },
+	{ 9, "RxEtherStatsPkts512to1023Octets" },
+	{ 10, "RxEtherStatsPkts1024to1518Octets" },
+	{ 11, "RxJumboFrames" }, /* non-standard counter */
+	{ 12, "RxaPauseMACControlFramesTransmitted" },
+	{ 13, "RxFIFODrops" }, /* non-standard counter */
+	{ 14, "RxBackwardDrops" }, /* non-standard counter */
+	{ 15, "RxClassifierDrops" }, /* non-standard counter */
+	{ 16, "RxEtherStatsCRCAlignErrors" },
+	{ 17, "RxEtherStatsUndersizePkts" },
+	{ 18, "RxEtherStatsOversizePkts" },
+	{ 19, "RxEtherStatsFragments" },
+	{ 20, "RxEtherStatsJabbers" },
+	{ 21, "RxaMACControlFramesReceived" },
+	/* 22-24 are undefined */
+	{ 25, "RxaFramesReceivedOK" },
+	{ 26, "RxQoSClass0" }, /* non-standard counter */
+	{ 27, "RxQoSClass1" }, /* non-standard counter */
+	{ 28, "RxQoSClass2" }, /* non-standard counter */
+	{ 29, "RxQoSClass3" }, /* non-standard counter */
+};
+
+static const struct vsc73xx_counter vsc73xx_tx_counters[] = {
+	{ 0, "TxEtherStatsPkts" },
+	{ 1, "TxBroadcast+MulticastPkts" }, /* non-standard counter */
+	{ 2, "TxTotalErrorPackets" }, /* non-standard counter */
+	{ 3, "TxEtherStatsBroadcastPkts" },
+	{ 4, "TxEtherStatsMulticastPkts" },
+	{ 5, "TxEtherStatsPkts64Octets" },
+	{ 6, "TxEtherStatsPkts65to127Octets" },
+	{ 7, "TxEtherStatsPkts128to255Octets" },
+	{ 8, "TxEtherStatsPkts256to511Octets" },
+	{ 9, "TxEtherStatsPkts512to1023Octets" },
+	{ 10, "TxEtherStatsPkts1024to1518Octets" },
+	{ 11, "TxJumboFrames" }, /* non-standard counter */
+	{ 12, "TxaPauseMACControlFramesTransmitted" },
+	{ 13, "TxFIFODrops" }, /* non-standard counter */
+	{ 14, "TxDrops" }, /* non-standard counter */
+	{ 15, "TxEtherStatsCollisions" },
+	{ 16, "TxEtherStatsCRCAlignErrors" },
+	{ 17, "TxEtherStatsUndersizePkts" },
+	{ 18, "TxEtherStatsOversizePkts" },
+	{ 19, "TxEtherStatsFragments" },
+	{ 20, "TxEtherStatsJabbers" },
+	/* 21-24 are undefined */
+	{ 25, "TxaFramesReceivedOK" },
+	{ 26, "TxQoSClass0" }, /* non-standard counter */
+	{ 27, "TxQoSClass1" }, /* non-standard counter */
+	{ 28, "TxQoSClass2" }, /* non-standard counter */
+	{ 29, "TxQoSClass3" }, /* non-standard counter */
+};
+
+static int vsc73xx_is_addr_valid(u8 block, u8 subblock)
+{
+	switch (block) {
+	case VSC73XX_BLOCK_MAC:
+		switch (subblock) {
+		case 0 ... 4:
+		case 6:
+			return 1;
+		}
+		break;
+
+	case VSC73XX_BLOCK_ANALYZER:
+	case VSC73XX_BLOCK_SYSTEM:
+		switch (subblock) {
+		case 0:
+			return 1;
+		}
+		break;
+
+	case VSC73XX_BLOCK_MII:
+	case VSC73XX_BLOCK_CAPTURE:
+	case VSC73XX_BLOCK_ARBITER:
+		switch (subblock) {
+		case 0 ... 1:
+			return 1;
+		}
+		break;
+	}
+
+	return 0;
+}
+
+static u8 vsc73xx_make_addr(u8 mode, u8 block, u8 subblock)
+{
+	u8 ret;
+
+	ret = (block & VSC73XX_CMD_BLOCK_MASK) << VSC73XX_CMD_BLOCK_SHIFT;
+	ret |= (mode & 1) << VSC73XX_CMD_MODE_SHIFT;
+	ret |= subblock & VSC73XX_CMD_SUBBLOCK_MASK;
+
+	return ret;
+}
+
+static int vsc73xx_read(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+			u32 *val)
+{
+	struct spi_transfer t[2];
+	struct spi_message m;
+	u8 cmd[4];
+	u8 buf[4];
+	int ret;
+
+	if (!vsc73xx_is_addr_valid(block, subblock))
+		return -EINVAL;
+
+	spi_message_init(&m);
+
+	memset(&t, 0, sizeof(t));
+
+	t[0].tx_buf = cmd;
+	t[0].len = sizeof(cmd);
+	spi_message_add_tail(&t[0], &m);
+
+	t[1].rx_buf = buf;
+	t[1].len = sizeof(buf);
+	spi_message_add_tail(&t[1], &m);
+
+	cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_READ, block, subblock);
+	cmd[1] = reg;
+	cmd[2] = 0;
+	cmd[3] = 0;
+
+	mutex_lock(&vsc->lock);
+	ret = spi_sync(vsc->spi, &m);
+	mutex_unlock(&vsc->lock);
+
+	if (ret)
+		return ret;
+
+	*val = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
+
+	return 0;
+}
+
+static int vsc73xx_write(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,
+			 u32 val)
+{
+	struct spi_transfer t[2];
+	struct spi_message m;
+	u8 cmd[2];
+	u8 buf[4];
+	int ret;
+
+	if (!vsc73xx_is_addr_valid(block, subblock))
+		return -EINVAL;
+
+	spi_message_init(&m);
+
+	memset(&t, 0, sizeof(t));
+
+	t[0].tx_buf = cmd;
+	t[0].len = sizeof(cmd);
+	spi_message_add_tail(&t[0], &m);
+
+	t[1].tx_buf = buf;
+	t[1].len = sizeof(buf);
+	spi_message_add_tail(&t[1], &m);
+
+	cmd[0] = vsc73xx_make_addr(VSC73XX_CMD_MODE_WRITE, block, subblock);
+	cmd[1] = reg;
+
+	buf[0] = (val >> 24) & 0xff;
+	buf[1] = (val >> 16) & 0xff;
+	buf[2] = (val >> 8) & 0xff;
+	buf[3] = val & 0xff;
+
+	mutex_lock(&vsc->lock);
+	ret = spi_sync(vsc->spi, &m);
+	mutex_unlock(&vsc->lock);
+
+	return ret;
+}
+
+static int vsc73xx_update_bits(struct vsc73xx *vsc, u8 block, u8 subblock,
+			       u8 reg, u32 mask, u32 val)
+{
+	u32 tmp, orig;
+	int ret;
+
+	/* Same read-modify-write algorithm as e.g. regmap */
+	ret = vsc73xx_read(vsc, block, subblock, reg, &orig);
+	if (ret)
+		return ret;
+	tmp = orig & ~mask;
+	tmp |= val & mask;
+	return vsc73xx_write(vsc, block, subblock, reg, tmp);
+}
+
+static int vsc73xx_detect(struct vsc73xx *vsc)
+{
+	bool icpu_si_boot_en;
+	bool icpu_pi_en;
+	u32 val;
+	u32 rev;
+	int ret;
+	u32 id;
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+			   VSC73XX_ICPU_MBOX_VAL, &val);
+	if (ret) {
+		dev_err(vsc->dev, "unable to read mailbox (%d)\n", ret);
+		return ret;
+	}
+
+	if (val == 0xffffffff) {
+		dev_info(vsc->dev, "chip seems dead, assert reset\n");
+		gpiod_set_value_cansleep(vsc->reset, 1);
+		/* Reset pulse should be 20ns minimum, according to datasheet
+		 * table 245, so 10us should be fine
+		 */
+		usleep_range(10, 100);
+		gpiod_set_value_cansleep(vsc->reset, 0);
+		/* Wait 20ms according to datasheet table 245 */
+		msleep(20);
+
+		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+				   VSC73XX_ICPU_MBOX_VAL, &val);
+		if (val == 0xffffffff) {
+			dev_err(vsc->dev, "seems not to help, giving up\n");
+			return -ENODEV;
+		}
+	}
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+			   VSC73XX_CHIPID, &val);
+	if (ret) {
+		dev_err(vsc->dev, "unable to read chip id (%d)\n", ret);
+		return ret;
+	}
+
+	id = (val >> VSC73XX_CHIPID_ID_SHIFT) &
+		VSC73XX_CHIPID_ID_MASK;
+	switch (id) {
+	case VSC73XX_CHIPID_ID_7385:
+		vsc->is_vsc7385 = true;
+		break;
+	case VSC73XX_CHIPID_ID_7388:
+		vsc->is_vsc7388 = true;
+		break;
+	case VSC73XX_CHIPID_ID_7395:
+		vsc->is_vsc7395 = true;
+		break;
+	case VSC73XX_CHIPID_ID_7398:
+		vsc->is_vsc7398 = true;
+		break;
+	default:
+		dev_err(vsc->dev, "unsupported chip, id=%04x\n", id);
+		return -ENODEV;
+	}
+
+	vsc->chipid = id;
+	rev = (val >> VSC73XX_CHIPID_REV_SHIFT) &
+		VSC73XX_CHIPID_REV_MASK;
+	dev_info(vsc->dev, "VSC%04X (rev: %d) switch found\n", id, rev);
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+			   VSC73XX_ICPU_CTRL, &val);
+	if (ret) {
+		dev_err(vsc->dev, "unable to read iCPU control\n");
+		return ret;
+	}
+
+	/* The iCPU can always be used but can boot in different ways.
+	 * If it is initially disabled and has no external memory,
+	 * we are in control and can do whatever we like, else we
+	 * are probably in trouble (we need some way to communicate
+	 * with the running firmware) so we bail out for now.
+	 */
+	icpu_pi_en = !!(val & VSC73XX_ICPU_CTRL_ICPU_PI_EN);
+	icpu_si_boot_en = !!(val & VSC73XX_ICPU_CTRL_BOOT_EN);
+	if (icpu_si_boot_en && icpu_pi_en) {
+		dev_err(vsc->dev,
+			"iCPU enabled boots from SI, has external memory\n");
+		dev_err(vsc->dev, "no idea how to deal with this\n");
+		return -ENODEV;
+	}
+	if (icpu_si_boot_en && !icpu_pi_en) {
+		dev_err(vsc->dev,
+			"iCPU enabled boots from SI, no external memory\n");
+		dev_err(vsc->dev, "no idea how to deal with this\n");
+		return -ENODEV;
+	}
+	if (!icpu_si_boot_en && icpu_pi_en) {
+		dev_err(vsc->dev,
+			"iCPU enabled, boots from PI external memory\n");
+		dev_err(vsc->dev, "no idea how to deal with this\n");
+		return -ENODEV;
+	}
+	/* !icpu_si_boot_en && !cpu_pi_en */
+	dev_info(vsc->dev, "iCPU disabled, no external memory\n");
+
+	return 0;
+}
+
+static int vsc73xx_phy_read(struct dsa_switch *ds, int phy, int regnum)
+{
+	struct vsc73xx *vsc = ds->priv;
+	u32 cmd;
+	u32 val;
+	int ret;
+
+	/* Setting bit 26 means "read" */
+	cmd = BIT(26) | (phy << 21) | (regnum << 16);
+	ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd);
+	if (ret)
+		return ret;
+	msleep(2);
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_MII, 0, 2, &val);
+	if (ret)
+		return ret;
+	if (val & BIT(16)) {
+		dev_err(vsc->dev, "reading reg %02x from phy%d failed\n",
+			regnum, phy);
+		return -EIO;
+	}
+	val &= 0xFFFFU;
+
+	dev_dbg(vsc->dev, "read reg %02x from phy%d = %04x\n",
+		regnum, phy, val);
+
+	return val;
+}
+
+static int vsc73xx_phy_write(struct dsa_switch *ds, int phy, int regnum,
+			     u16 val)
+{
+	struct vsc73xx *vsc = ds->priv;
+	u32 cmd;
+	int ret;
+
+	/* It was found through tedious experiments that this router
+	 * chip really hates to have it's PHYs reset. They
+	 * never recover if that happens: autonegotiation stops
+	 * working after a reset. Just filter out this command.
+	 * (Resetting the whole chip is OK.)
+	 */
+	if (regnum == 0 && (val & BIT(15))) {
+		dev_info(vsc->dev, "reset PHY - disallowed\n");
+		return 0;
+	}
+
+	cmd = (phy << 21) | (regnum << 16);
+	ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd);
+	if (ret)
+		return ret;
+
+	dev_dbg(vsc->dev, "write %04x to reg %02x in phy%d\n",
+		val, regnum, phy);
+	return 0;
+}
+
+static enum dsa_tag_protocol vsc73xx_get_tag_protocol(struct dsa_switch *ds,
+						      int port)
+{
+	/* The switch internally uses a 8 byte header with length,
+	 * source port, tag, LPA and priority. This is supposedly
+	 * only accessible when operating the switch using the internal
+	 * CPU or with an external CPU mapping the device in, but not
+	 * when operating the switch over SPI and putting frames in/out
+	 * on port 6 (the CPU port). So far we must assume that we
+	 * cannot access the tag. (See "Internal frame header" section
+	 * 3.9.1 in the manual.)
+	 */
+	return DSA_TAG_PROTO_NONE;
+}
+
+static int vsc73xx_setup(struct dsa_switch *ds)
+{
+	struct vsc73xx *vsc = ds->priv;
+	int i;
+
+	dev_info(vsc->dev, "set up the switch\n");
+
+	/* Issue RESET */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GLORESET,
+		      VSC73XX_GLORESET_MASTER_RESET);
+	usleep_range(125, 200);
+
+	/* Initialize memory, initialize RAM bank 0..15 except 6 and 7
+	 * This sequence appears in the
+	 * VSC7385 SparX-G5 datasheet section 6.6.1
+	 * VSC7395 SparX-G5e datasheet section 6.6.1
+	 * "initialization sequence".
+	 * No explanation is given to the 0x1010400 magic number.
+	 */
+	for (i = 0; i <= 15; i++) {
+		if (i != 6 && i != 7) {
+			vsc73xx_write(vsc, VSC73XX_BLOCK_MEMINIT,
+				      2,
+				      0, 0x1010400 + i);
+			mdelay(1);
+		}
+	}
+	mdelay(30);
+
+	/* Clear MAC table */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+		      VSC73XX_MACACCESS,
+		      VSC73XX_MACACCESS_CMD_CLEAR_TABLE);
+
+	/* Clear VLAN table */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+		      VSC73XX_VLANACCESS,
+		      VSC73XX_VLANACCESS_VLAN_TBL_CMD_CLEAR_TABLE);
+
+	msleep(40);
+
+	/* Use 20KiB buffers on all ports on VSC7395
+	 * The VSC7385 has 16KiB buffers and that is the
+	 * default if we don't set this up explicitly.
+	 * Port "31" is "all ports".
+	 */
+	if (vsc->is_vsc7395 || vsc->is_vsc7398)
+		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, 0x1f,
+			      VSC73XX_Q_MISC_CONF,
+			      VSC73XX_Q_MISC_CONF_EXTENT_MEM);
+
+	/* Put all ports into reset until enabled */
+	for (i = 0; i < 7; i++) {
+		if (i == 5)
+			continue;
+		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, 4,
+			      VSC73XX_MAC_CFG, VSC73XX_MAC_CFG_RESET);
+	}
+
+	/* MII delay, set both GTX and RX delay to 2 ns */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GMIIDELAY,
+		      VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS |
+		      VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS);
+	/* Enable reception of frames on all ports */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_RECVMASK,
+		      0x5f);
+	/* IP multicast flood mask (table 144) */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_IFLODMSK,
+		      0xff);
+
+	mdelay(50);
+
+	/* Release reset from the internal PHYs */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GLORESET,
+		      VSC73XX_GLORESET_PHY_RESET);
+
+	udelay(4);
+
+	return 0;
+}
+
+static void vsc73xx_init_port(struct vsc73xx *vsc, int port)
+{
+	u32 val;
+
+	/* MAC configure, first reset the port and then write defaults */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_MAC_CFG,
+		      VSC73XX_MAC_CFG_RESET);
+
+	/* Take up the port in 1Gbit mode by default, this will be
+	 * augmented after auto-negotiation on the PHY-facing
+	 * ports.
+	 */
+	if (port == CPU_PORT)
+		val = VSC73XX_MAC_CFG_1000M_F_RGMII;
+	else
+		val = VSC73XX_MAC_CFG_1000M_F_PHY;
+
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_MAC_CFG,
+		      val |
+		      VSC73XX_MAC_CFG_TX_EN |
+		      VSC73XX_MAC_CFG_RX_EN);
+
+	/* Max length, we can do up to 9.6 KiB, so allow that.
+	 * According to application not "VSC7398 Jumbo Frames" setting
+	 * up the MTU to 9.6 KB does not affect the performance on standard
+	 * frames, so just enable it. It is clear from the application note
+	 * that "9.6 kilobytes" == 9600 bytes.
+	 */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_MAXLEN, 9600);
+
+	/* Flow control for the CPU port:
+	 * Use a zero delay pause frame when pause condition is left
+	 * Obey pause control frames
+	 */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_FCCONF,
+		      VSC73XX_FCCONF_ZERO_PAUSE_EN |
+		      VSC73XX_FCCONF_FLOW_CTRL_OBEY);
+
+	/* Issue pause control frames on PHY facing ports.
+	 * Allow early initiation of MAC transmission if the amount
+	 * of egress data is below 512 bytes on CPU port.
+	 * FIXME: enable 20KiB buffers?
+	 */
+	if (port == CPU_PORT)
+		val = VSC73XX_Q_MISC_CONF_EARLY_TX_512;
+	else
+		val = VSC73XX_Q_MISC_CONF_MAC_PAUSE_MODE;
+	val |= VSC73XX_Q_MISC_CONF_EXTENT_MEM;
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_Q_MISC_CONF,
+		      val);
+
+	/* Flow control MAC: a MAC address used in flow control frames */
+	val = (vsc->addr[5] << 16) | (vsc->addr[4] << 8) | (vsc->addr[3]);
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_FCMACHI,
+		      val);
+	val = (vsc->addr[2] << 16) | (vsc->addr[1] << 8) | (vsc->addr[0]);
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_FCMACLO,
+		      val);
+
+	/* Tell the categorizer to forward pause frames, not control
+	 * frame. Do not drop anything.
+	 */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port,
+		      VSC73XX_CAT_DROP,
+		      VSC73XX_CAT_DROP_FWD_PAUSE_ENA);
+
+	/* Clear all counters */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+		      port, VSC73XX_C_RX0, 0);
+}
+
+static void vsc73xx_adjust_enable_port(struct vsc73xx *vsc,
+				       int port, struct phy_device *phydev,
+				       u32 initval)
+{
+	u32 val = initval;
+	u8 seed;
+
+	/* Reset this port FIXME: break out subroutine */
+	val |= VSC73XX_MAC_CFG_RESET;
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, val);
+
+	/* Seed the port randomness with randomness */
+	get_random_bytes(&seed, 1);
+	val |= seed << VSC73XX_MAC_CFG_SEED_OFFSET;
+	val |= VSC73XX_MAC_CFG_SEED_LOAD;
+	val |= VSC73XX_MAC_CFG_WEXC_DIS;
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, val);
+
+	/* Flow control for the PHY facing ports:
+	 * Use a zero delay pause frame when pause condition is left
+	 * Obey pause control frames
+	 * When generating pause frames, use 0xff as pause value
+	 */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_FCCONF,
+		      VSC73XX_FCCONF_ZERO_PAUSE_EN |
+		      VSC73XX_FCCONF_FLOW_CTRL_OBEY |
+		      0xff);
+
+	/* Disallow backward dropping of frames from this port */
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
+			    VSC73XX_SBACKWDROP, BIT(port), 0);
+
+	/* Enable TX, RX, deassert reset, stop loading seed */
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+			    VSC73XX_MAC_CFG,
+			    VSC73XX_MAC_CFG_RESET | VSC73XX_MAC_CFG_SEED_LOAD |
+			    VSC73XX_MAC_CFG_TX_EN | VSC73XX_MAC_CFG_RX_EN,
+			    VSC73XX_MAC_CFG_TX_EN | VSC73XX_MAC_CFG_RX_EN);
+}
+
+static void vsc73xx_adjust_link(struct dsa_switch *ds, int port,
+				struct phy_device *phydev)
+{
+	struct vsc73xx *vsc = ds->priv;
+	u32 val;
+
+	/* Special handling of the CPU-facing port */
+	if (port == CPU_PORT) {
+		/* Other ports are already initialized but not this one */
+		vsc73xx_init_port(vsc, CPU_PORT);
+		/* Select the external port for this interface (EXT_PORT)
+		 * Enable the GMII GTX external clock
+		 * Use double data rate (DDR mode)
+		 */
+		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
+			      CPU_PORT,
+			      VSC73XX_ADVPORTM,
+			      VSC73XX_ADVPORTM_EXT_PORT |
+			      VSC73XX_ADVPORTM_ENA_GTX |
+			      VSC73XX_ADVPORTM_DDR_MODE);
+	}
+
+	/* This is the MAC confiuration that always need to happen
+	 * after a PHY or the CPU port comes up or down.
+	 */
+	val = phy_read(phydev, 1);
+	if ((val & 0x0024) != 0x0024) {
+		dev_info(vsc->dev, "port %d: went down\n",
+			 port);
+
+		/* Disable RX on this port */
+		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+				    VSC73XX_MAC_CFG,
+				    VSC73XX_MAC_CFG_RX_EN, 0);
+
+		/* Discard packets */
+		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
+				    VSC73XX_ARBDISC, BIT(port), BIT(port));
+
+		/* Wait until queue is empty */
+		vsc73xx_read(vsc, VSC73XX_BLOCK_ARBITER, 0,
+			     VSC73XX_ARBEMPTY, &val);
+		while (!(val & BIT(port))) {
+			msleep(1);
+			vsc73xx_read(vsc, VSC73XX_BLOCK_ARBITER, 0,
+				     VSC73XX_ARBEMPTY, &val);
+		}
+
+		/* Put this port into reset */
+		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,
+			      VSC73XX_MAC_CFG_RESET);
+
+		/* Accept packets again */
+		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
+				    VSC73XX_ARBDISC, BIT(port), 0);
+
+		/* Allow backward dropping of frames from this port */
+		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
+				    VSC73XX_SBACKWDROP, BIT(port), BIT(port));
+
+		/* Receive mask (disable forwarding) */
+		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+				    VSC73XX_RECVMASK, BIT(port), 0);
+
+		return;
+	}
+
+	/* Figure out what speed was negotiated */
+	val = phy_read(phydev, 0x0a);
+	if (val & 0x0c00) {
+		dev_info(vsc->dev, "port %d: 1000 Mbit mode full duplex\n",
+			 port);
+
+		/* Set up default for internal or external RGMII */
+		if (port == CPU_PORT)
+			val = VSC73XX_MAC_CFG_1000M_F_RGMII;
+		else
+			val = VSC73XX_MAC_CFG_1000M_F_PHY;
+		vsc73xx_adjust_enable_port(vsc, port, phydev, val);
+	} else {
+		val = phy_read(phydev, 0x05);
+		val &= 0x05e0;
+		val >>= 5;
+		if (val & 0x0c) {
+			if (val & 0x08) {
+				val = VSC73XX_MAC_CFG_100_10M_F_PHY;
+				dev_info(vsc->dev,
+					 "port %d: 100 Mbit full duplex mode\n",
+					 port);
+			} else {
+				val = VSC73XX_MAC_CFG_100_10M_H_PHY;
+				dev_info(vsc->dev,
+					 "port %d: 100 Mbit half duplex mode\n",
+					 port);
+			}
+			vsc73xx_adjust_enable_port(vsc, port, phydev, val);
+		} else if (val & 0x03) {
+			if (val & 0x02) {
+				val = VSC73XX_MAC_CFG_100_10M_F_PHY;
+				dev_info(vsc->dev,
+					 "port %d: 10 Mbit full duplex mode\n",
+					 port);
+			} else {
+				val = VSC73XX_MAC_CFG_100_10M_H_PHY;
+				dev_info(vsc->dev,
+					 "port %d: 10 Mbit half duplex mode\n",
+					 port);
+			}
+			vsc73xx_adjust_enable_port(vsc, port, phydev, val);
+		} else {
+			dev_err(vsc->dev,
+				"could not adjust link: unknown speed\n");
+		}
+	}
+
+	/* Enable port (forwarding) in the receieve mask */
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+			    VSC73XX_RECVMASK, BIT(port), BIT(port));
+}
+
+static int vsc73xx_port_enable(struct dsa_switch *ds, int port,
+			       struct phy_device *phy)
+{
+	struct vsc73xx *vsc = ds->priv;
+
+	dev_info(vsc->dev, "enable port %d\n", port);
+
+	/* VSC7385 and VSC7395 have ports 0..4 accessible */
+	if ((vsc->is_vsc7385 || vsc->is_vsc7395) && port > 4)
+		return -ENODEV;
+	if ((vsc->is_vsc7388 || vsc->is_vsc7398) && port > 7)
+		return -ENODEV;
+
+	vsc73xx_init_port(vsc, port);
+
+	return 0;
+}
+
+static void vsc73xx_port_disable(struct dsa_switch *ds, int port,
+				 struct phy_device *phy)
+{
+	struct vsc73xx *vsc = ds->priv;
+
+	/* VSC7385 and VSC7395 have ports 0..4 accessible */
+	if ((vsc->is_vsc7385 || vsc->is_vsc7395) && port > 4)
+		return;
+	if ((vsc->is_vsc7388 || vsc->is_vsc7398) && port > 7)
+		return;
+
+	/* Just put the port into reset */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port,
+		      VSC73XX_MAC_CFG, VSC73XX_MAC_CFG_RESET);
+}
+
+const struct vsc73xx_counter *vsc73xx_find_counter(struct vsc73xx *vsc,
+						   u8 counter,
+						   bool tx)
+{
+	const struct vsc73xx_counter *cnts;
+	int num_cnts;
+	int i;
+
+	if (tx) {
+		cnts = vsc73xx_tx_counters;
+		num_cnts = ARRAY_SIZE(vsc73xx_tx_counters);
+	} else {
+		cnts = vsc73xx_rx_counters;
+		num_cnts = ARRAY_SIZE(vsc73xx_rx_counters);
+	}
+
+	for (i = 0; i < num_cnts; i++) {
+		const struct vsc73xx_counter *cnt;
+
+		cnt = &cnts[i];
+		if (cnt->counter == counter)
+			return cnt;
+	}
+
+	return NULL;
+}
+
+void vsc73xx_get_strings(struct dsa_switch *ds, int port, uint8_t *data)
+{
+	const struct vsc73xx_counter *cnt;
+	struct vsc73xx *vsc = ds->priv;
+	u8 indices[6];
+	int i, j;
+	u32 val;
+	int ret;
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
+			   VSC73XX_C_CFG, &val);
+	if (ret)
+		return;
+
+	indices[0] = (val & 0x1f); /* RX counter 0 */
+	indices[1] = ((val >> 5) & 0x1f); /* RX counter 1 */
+	indices[2] = ((val >> 10) & 0x1f); /* RX counter 2 */
+	indices[3] = ((val >> 16) & 0x1f); /* TX counter 0 */
+	indices[4] = ((val >> 21) & 0x1f); /* TX counter 1 */
+	indices[5] = ((val >> 26) & 0x1f); /* TX counter 2 */
+
+	/* The first counters is the RX octets */
+	j = 0;
+	strncpy(data + j * ETH_GSTRING_LEN,
+		"RxEtherStatsOctets", ETH_GSTRING_LEN);
+	j++;
+
+	/* Each port supports recording 3 RX counters and 3 TX counters,
+	 * figure out what counters we use in this set-up and return the
+	 * names of them. The hardware default counters will be number of
+	 * packets on RX/TX, combined broadcast+multicast packets RX/TX and
+	 * total error packets RX/TX.
+	 */
+	for (i = 0; i < 3; i++) {
+		cnt = vsc73xx_find_counter(vsc, indices[i], false);
+		if (cnt)
+			strncpy(data + j * ETH_GSTRING_LEN,
+				cnt->name, ETH_GSTRING_LEN);
+		j++;
+	}
+
+	/* TX stats begins with the number of TX octets */
+	strncpy(data + j * ETH_GSTRING_LEN,
+		"TxEtherStatsOctets", ETH_GSTRING_LEN);
+	j++;
+
+	for (i = 3; i < 6; i++) {
+		cnt = vsc73xx_find_counter(vsc, indices[i], true);
+		if (cnt)
+			strncpy(data + j * ETH_GSTRING_LEN,
+				cnt->name, ETH_GSTRING_LEN);
+		j++;
+	}
+}
+
+int vsc73xx_get_sset_count(struct dsa_switch *ds, int port)
+{
+	/* RX and TX packets, then 3 RX counters, 3 TX counters */
+	return 8;
+}
+
+void vsc73xx_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
+{
+	struct vsc73xx *vsc = ds->priv;
+	u8 regs[] = {
+		VSC73XX_RXOCT,
+		VSC73XX_C_RX0,
+		VSC73XX_C_RX1,
+		VSC73XX_C_RX2,
+		VSC73XX_TXOCT,
+		VSC73XX_C_TX0,
+		VSC73XX_C_TX1,
+		VSC73XX_C_TX2,
+	};
+	u32 val;
+	int ret;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++) {
+		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port,
+				   regs[i], &val);
+		if (ret) {
+			dev_err(vsc->dev, "error reading counter %d\n", i);
+			return;
+		}
+		data[i] = val;
+	}
+}
+
+static const struct dsa_switch_ops vsc73xx_ds_ops = {
+	.get_tag_protocol = vsc73xx_get_tag_protocol,
+	.setup = vsc73xx_setup,
+	.phy_read = vsc73xx_phy_read,
+	.phy_write = vsc73xx_phy_write,
+	.adjust_link = vsc73xx_adjust_link,
+	.get_strings = vsc73xx_get_strings,
+	.get_ethtool_stats = vsc73xx_get_ethtool_stats,
+	.get_sset_count = vsc73xx_get_sset_count,
+	.port_enable = vsc73xx_port_enable,
+	.port_disable = vsc73xx_port_disable,
+};
+
+static int vsc73xx_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct vsc73xx *vsc = gpiochip_get_data(chip);
+	u32 val;
+	int ret;
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+			   VSC73XX_GPIO, &val);
+	if (ret)
+		return ret;
+
+	return !!(val & BIT(offset));
+}
+
+static void vsc73xx_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int val)
+{
+	struct vsc73xx *vsc = gpiochip_get_data(chip);
+	u32 tmp = val ? BIT(offset) : 0;
+
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+			    VSC73XX_GPIO, BIT(offset), tmp);
+}
+
+static int vsc73xx_gpio_direction_output(struct gpio_chip *chip,
+					 unsigned int offset, int val)
+{
+	struct vsc73xx *vsc = gpiochip_get_data(chip);
+	u32 tmp = val ? BIT(offset) : 0;
+
+	return vsc73xx_update_bits(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+				   VSC73XX_GPIO, BIT(offset + 4) | BIT(offset),
+				   BIT(offset + 4) | tmp);
+}
+
+static int vsc73xx_gpio_direction_input(struct gpio_chip *chip,
+					unsigned int offset)
+{
+	struct vsc73xx *vsc = gpiochip_get_data(chip);
+
+	return  vsc73xx_update_bits(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+				    VSC73XX_GPIO, BIT(offset + 4),
+				    0);
+}
+
+static int vsc73xx_gpio_get_direction(struct gpio_chip *chip,
+				      unsigned int offset)
+{
+	struct vsc73xx *vsc = gpiochip_get_data(chip);
+	u32 val;
+	int ret;
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0,
+			   VSC73XX_GPIO, &val);
+	if (ret)
+		return ret;
+
+	return !(val & BIT(offset + 4));
+}
+
+static int vsc73xx_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct vsc73xx *vsc;
+	int ret;
+
+	vsc = devm_kzalloc(dev, sizeof(*vsc), GFP_KERNEL);
+	if (!vsc)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, vsc);
+	vsc->spi = spi_dev_get(spi);
+	vsc->dev = dev;
+	mutex_init(&vsc->lock);
+
+	/* Release reset, if any */
+	vsc->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(vsc->reset)) {
+		dev_err(dev, "failed to get RESET GPIO\n");
+		return PTR_ERR(vsc->reset);
+	}
+	if (vsc->reset)
+		/* Wait 20ms according to datasheet table 245 */
+		msleep(20);
+
+	spi->mode = SPI_MODE_0;
+	spi->bits_per_word = 8;
+	ret = spi_setup(spi);
+	if (ret < 0) {
+		dev_err(dev, "spi setup failed.\n");
+		return ret;
+	}
+
+	ret = vsc73xx_detect(vsc);
+	if (ret) {
+		dev_err(dev, "no chip found (%d)\n", ret);
+		return -ENODEV;
+	}
+
+	eth_random_addr(vsc->addr);
+	dev_info(vsc->dev,
+		 "MAC for control frames: %02X:%02X:%02X:%02X:%02X:%02X\n",
+		 vsc->addr[0], vsc->addr[1], vsc->addr[2],
+		 vsc->addr[3], vsc->addr[4], vsc->addr[5]);
+
+	/* The VSC7395 switch chips have 5+1 ports which means 5
+	 * ordinary ports and a sixth CPU port facing the processor
+	 * with an RGMII interface. These ports are numbered 0..4
+	 * and 6, so they leave a "hole" in the port map for port 5,
+	 * which is invalid.
+	 *
+	 * The VSC7398 has 8 ports, port 7 is again the CPU port.
+	 *
+	 * We allocate 8 ports and avoid access to the nonexistant
+	 * ports.
+	 */
+	vsc->ds = dsa_switch_alloc(dev, 8);
+	if (!vsc->ds)
+		return -ENOMEM;
+	vsc->ds->priv = vsc;
+
+	vsc->ds->ops = &vsc73xx_ds_ops;
+	ret = dsa_register_switch(vsc->ds);
+	if (ret) {
+		dev_err(dev, "unable to register switch (%d)\n", ret);
+		return ret;
+	}
+
+	vsc->gc.label = devm_kasprintf(dev, GFP_KERNEL, "VSC%04x",
+				       vsc->chipid);
+	vsc->gc.ngpio = 4;
+	vsc->gc.owner = THIS_MODULE;
+	vsc->gc.parent = dev;
+	vsc->gc.of_node = dev->of_node;
+	vsc->gc.base = -1;
+	vsc->gc.get = vsc73xx_gpio_get;
+	vsc->gc.set = vsc73xx_gpio_set;
+	vsc->gc.direction_input = vsc73xx_gpio_direction_input;
+	vsc->gc.direction_output = vsc73xx_gpio_direction_output;
+	vsc->gc.get_direction = vsc73xx_gpio_get_direction;
+	vsc->gc.can_sleep = true;
+	ret = devm_gpiochip_add_data(dev, &vsc->gc, vsc);
+	if (ret) {
+		dev_err(dev, "unable to register GPIO chip\n");
+		dsa_unregister_switch(vsc->ds);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int vsc73xx_remove(struct spi_device *spi)
+{
+	struct vsc73xx *vsc = spi_get_drvdata(spi);
+
+	dsa_unregister_switch(vsc->ds);
+	gpiod_set_value(vsc->reset, 1);
+
+	return 0;
+}
+
+static const struct of_device_id vsc73xx_of_match[] = {
+	{
+		.compatible = "vitesse,vsc7385",
+	},
+	{
+		.compatible = "vitesse,vsc7388",
+	},
+	{
+		.compatible = "vitesse,vsc7395",
+	},
+	{
+		.compatible = "vitesse,vsc7398",
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, vsc73xx_of_match);
+
+static struct spi_driver vsc73xx_driver = {
+	.probe = vsc73xx_probe,
+	.remove = vsc73xx_remove,
+	.driver = {
+		.name = "vsc73xx",
+		.of_match_table = vsc73xx_of_match,
+	},
+};
+module_spi_driver(vsc73xx_driver);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_DESCRIPTION("Vitesse VSC7385/7388/7395/7398 driver");
+MODULE_LICENSE("GPL v2");