DSA support for Micrel KSZ8895

Hi!

I've got hardware with KSZ8895, and I'd like to use switch ports as separate ethernet cards. I believe that means DSA support.

And there are even patches available from microchip... unfortunately they are in strange form and for v3.18.

http://www.microchip.com/SWLibraryWeb/product.aspx?product=KSZ8895%20Software%20Linux%203.18

Is there newer version of the driver available somewhere? Is the driver good starting point, or should I start with something else?

Quoted 9 lines

Hi!

I've got hardware with KSZ8895, and I'd like to use switch ports as separate ethernet cards. I believe that means DSA support.

And there are even patches available from microchip... unfortunately they are in strange form and for v3.18.

http://www.microchip.com/SWLibraryWeb/product.aspx?product=KSZ8895
%20Software%20Linux%203.18

Quoted 3 lines

Is there newer version of the driver available somewhere? Is the driver good starting point, or should I start with something else?

Hi Pavel

Woojung is the expert here. His DSA driver for the 9477 is a nice clean driver.

Have you compared the 8895 to the 9477. Are they similar? Could the existing 9477 be extended to support the 8895?

Andrew

Quoted 6 lines

Woojung is the expert here. His DSA driver for the 9477 is a nice clean driver.

Have you compared the 8895 to the 9477. Are they similar? Could the existing 9477 be extended to support the 8895?

Quoted 1 line

Andrew

Hi Pavel,

I'll forward your email to our support.
AFAIK, KSZ8895 has different register mapping from KSZ9477, it will be more than ID changes in current driver.

Any ideas how to do the work in a way to minimize code duplication are welcome...

Quoted 3 lines

I'll forward your email to our support.
AFAIK, KSZ8895 has different register mapping from KSZ9477, it will be more than ID changes in current driver.

More than ID changes, indeed. As layout is completely different, it looks like different source file will be needed for support.

I'm not nearly there; but I can ifconfig lanX up, already, and perform some pings.

Any ideas how to do the work in a way to minimize code duplication are welcome...

Quoted 7 lines

Quoted 3 lines

I'll forward your email to our support.
AFAIK, KSZ8895 has different register mapping from KSZ9477, it will be more than ID changes in current driver.

More than ID changes, indeed. As layout is completely different, it looks like different source file will be needed for support.

Quoted 5 lines

I'm not nearly there; but I can ifconfig lanX up, already, and perform some pings.

Any ideas how to do the work in a way to minimize code duplication are welcome...

Which version do you use to create patch? Getting error when applying patch to the latest net-next.

I'm working at 4.13-rc.

Hi!

So I fought with the driver a bit more, and now I have something that kind-of-works.

"great great hack" belows worries me.

Yeah, disabled code needs to be removed before merge.

No, tag_ksz part probably is not acceptable. Do you see solution better than just copying it into tag_ksz1 file?

Any more comments, etc?

drivers/net/dsa/microchip/ksz_mdio_emulation.c new file +57 -0

+/**
+ * sw_r_phy - read data from PHY register
+ * @sw:		The switch instance.
+ * @phy:	PHY address to read.
+ * @reg:	PHY register to read.
+ * @val:	Buffer to store the read data.
+ *
+ * This routine reads data from the PHY register.
+ */
+static void sw_r_phy(struct ksz_device *sw, u16 phy, u16 reg, u16 *val)
+{
+	u8 ctrl;
+	u8 restart;
+	u8 link;
+	u8 speed;
+	u8 force;
+	u8 p = phy;
+	u16 data = 0;
+
+	switch (reg) {
+	case PHY_REG_CTRL:
+		ksz_pread8(sw, p, P_LOCAL_CTRL, &ctrl);
+		ksz_pread8(sw, p, P_NEG_RESTART_CTRL, &restart);
+		ksz_pread8(sw, p, P_SPEED_STATUS, &speed);
+		ksz_pread8(sw, p, P_FORCE_CTRL, &force);
+		if (restart & PORT_PHY_LOOPBACK)
+			data |= PHY_LOOPBACK;
+		if (force & PORT_FORCE_100_MBIT)
+			data |= PHY_SPEED_100MBIT;
+		if (!(force & PORT_AUTO_NEG_DISABLE))
+			data |= PHY_AUTO_NEG_ENABLE;
+		if (restart & PORT_POWER_DOWN)
+			data |= PHY_POWER_DOWN;
+		if (restart & PORT_AUTO_NEG_RESTART)
+			data |= PHY_AUTO_NEG_RESTART;
+		if (force & PORT_FORCE_FULL_DUPLEX)
+			data |= PHY_FULL_DUPLEX;
+		if (speed & PORT_HP_MDIX)
+			data |= PHY_HP_MDIX;
+		if (restart & PORT_FORCE_MDIX)
+			data |= PHY_FORCE_MDIX;
+		if (restart & PORT_AUTO_MDIX_DISABLE)
+			data |= PHY_AUTO_MDIX_DISABLE;
+		if (restart & PORT_TX_DISABLE)
+			data |= PHY_TRANSMIT_DISABLE;
+		if (restart & PORT_LED_OFF)
+			data |= PHY_LED_DISABLE;
+		break;
+	case PHY_REG_STATUS:
+		ksz_pread8(sw, p, P_LINK_STATUS, &link);
+		ksz_pread8(sw, p, P_SPEED_STATUS, &speed);
+		data = PHY_100BTX_FD_CAPABLE |
+			PHY_100BTX_CAPABLE |
+			PHY_10BT_FD_CAPABLE |
+			PHY_10BT_CAPABLE |
+			PHY_AUTO_NEG_CAPABLE;
+		if (link & PORT_AUTO_NEG_COMPLETE)

Quoted 9 lines

drivers/net/dsa/microchip/ksz_mdio_emulation.c new file +9 -0

+	case PHY_REG_STATUS:
+		ksz_pread8(sw, p, P_LINK_STATUS, &link);
+		ksz_pread8(sw, p, P_SPEED_STATUS, &speed);
+		data = PHY_100BTX_FD_CAPABLE |
+			PHY_100BTX_CAPABLE |
+			PHY_10BT_FD_CAPABLE |
+			PHY_10BT_CAPABLE |
+			PHY_AUTO_NEG_CAPABLE;
+		if (link & PORT_AUTO_NEG_COMPLETE)

drivers/net/dsa/microchip/ksz_mdio_emulation.c line 77

Quoted 10 lines

drivers/net/dsa/microchip/ksz_mdio_emulation.c new file +10 -0

+			data |= PHY_AUTO_NEG_ACKNOWLEDGE;
+		if (link & PORT_STAT_LINK_GOOD)
+			data |= PHY_LINK_STATUS;
+		break;
+	case PHY_REG_ID_1:
+		data = KSZ8895_ID_HI;
+		break;
+	case PHY_REG_ID_2:
+		data = KSZ8895_ID_LO;
+		break;

According to the datasheet, the PHY has the normal ID registers, which have the value 0x0022, 0x1450. So it should be possible to have a standard PHY driver in drivers/net/phy.

In fact, the IDs suggest it is a micrel phy, and 1430, 1435 are already supported. So it could be you only need minor modifications to the micrel.c.

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Quoted 3 lines

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

Andrew

Micrel has some drivers on their web site to support some chips. For that chips they do virtual mdio over spi.
And driver is available on download page:
http://www.microchip.com/wwwproducts/en/KSZ8895

Documentation->Software library.

Both driver and DSA driver. Driver has to work with some minor fixups related to your kernel version. But I think they are don't care about up-streaming that code.
So you can take their code as a reference.

But the MDIO emaulation code is from their driver, after lots of deletions.

Quoted 2 lines

But the MDIO emaulation code is from their driver, after lots of deletions.

Is this driver supposed to run on lots of different OSs? That would explain why they ignored the Linux MDIO and PHY layers.

If possible, please make use of the Linux infrastructure.

Quoted 5 lines · Andrew Lunn

Quoted 2 lines

But the MDIO emaulation code is from their driver, after lots of deletions.

Is this driver supposed to run on lots of different OSs? That would explain why they ignored the Linux MDIO and PHY layers.

It did not look particulary portable.

Quoted 2 lines

If possible, please make use of the Linux infrastructure.

I did not find any infrastructure I could use instead ksz_mdio_emulation.

Now, drivers/net/phy/spi_ks8995.c can access the PHY registers, and I can not see any translation there, so there may be something I'm missing.

Pointers would be welcome at this point.

Thanks,
Pavel

Quoted 8 lines · Andrew Lunn

Quoted 5 lines

Quoted 2 lines

But the MDIO emaulation code is from their driver, after lots of deletions.

Is this driver supposed to run on lots of different OSs? That would explain why they ignored the Linux MDIO and PHY layers.

It did not look particulary portable.

Part of the problem is that they need to duplicate the standard MII definitions, whereas we could re-use those from include/linux/mii.h and/or mdio.h.

Quoted 5 lines

Quoted 2 lines

If possible, please make use of the Linux infrastructure.

I did not find any infrastructure I could use instead ksz_mdio_emulation.

fixed PHY/swphy.c is as close as it could get, but it is a highly simplified version of this.

Quoted 4 lines

Now, drivers/net/phy/spi_ks8995.c can access the PHY registers, and I can not see any translation there, so there may be something I'm missing.

I don't see anything in that driver that seems to access PHY registers what makes you think it does?

There's got to be a way to perform indirect accesses through SPI, Woojung, do you know?

Quoted 6 lines

Pointers would be welcome at this point.

Thanks,
Pavel

--
Florian

Quoted 3 lines

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

Quoted 5 lines

Quoted 3 lines

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Quoted 8 lines

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

As far as I can tell (and their driver confirms) -- yes, all those registers can be accessed over the SPI, they are just shuffled around... hence MDIO emulation code. I copied it from their code (see the copyrights) so no, I don't believe there's nicer solution.

Best regards,

Pavel

The KSZ8795 driver will be submitted after Labor Day (9/4) if testing reveals no problem. The KSZ8895 driver will be submitted right after that. You should have no problem using the driver right away.

Quoted 15 lines · Andrew Lunn

Quoted 13 lines

Quoted 3 lines

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

Quoted 6 lines

As far as I can tell (and their driver confirms) -- yes, all those registers can be accessed over the SPI, they are just shuffled around... hence MDIO emulation code. I copied it from their code (see the copyrights) so no, I don't believe there's nicer solution.

Best regards,

Can you hold on your developing work on KSZ8895 driver? I am afraid your effort may be in vain. We at Microchip are planning to release DSA drivers for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.

The driver files all follow the structures of the current KSZ9477 DSA driver, and the file tag_ksz.c will be updated to handle the tail tag of different chips, which requires including the ksz_priv.h header. That is required nevertheless to support using the offload_fwd_mark indication.

The KSZ8795 driver will be submitted after Labor Day (9/4) if testing reveals no problem. The KSZ8895 driver will be submitted right after that. You should have no problem using the driver right away.

Hi!

Quoted 20 lines · On Wed 2017-08-30 21:32:07, [email protected] wrote:

Quoted 20 lines · Andrew Lunn

Quoted 5 lines

Quoted 3 lines

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Quoted 8 lines

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

As far as I can tell (and their driver confirms) -- yes, all those registers can be accessed over the SPI, they are just shuffled around... hence MDIO emulation code. I copied it from their code (see the copyrights) so no, I don't believe there's nicer solution.

Quoted 4 lines

Quoted 1 line

Best regards,

Can you hold on your developing work on KSZ8895 driver? I am afraid your effort may be in vain. We at Microchip are planning to release DSA drivers for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.

Well, thanks for heads up... but its too late to stop now. I already have working code, without the advanced features.

I don't know how far away you are with the development. You may want to start from my driver (but its probably too late now).

Quoted 19 lines

Quoted 18 lines

Quoted 9 lines

Quoted 8 lines

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

Quoted 6 lines

As far as I can tell (and their driver confirms) -- yes, all those registers can be accessed over the SPI, they are just shuffled around... hence MDIO emulation code. I copied it from their code (see the copyrights) so no, I don't believe there's nicer solution.

Best regards,

Can you hold on your developing work on KSZ8895 driver? I am afraid your effort may be in vain. We at Microchip are planning to release DSA drivers for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.

Quoted 2 lines

Well, thanks for heads up... but its too late to stop now. I already have working code, without the advanced features.

No driver has landed yet nor has any driver been posted in a proper form or shape, so at this point neither of you are able to make any claims as to which one should be chosen.

Quoted 2 lines

I don't know how far away you are with the development. You may want to start from my driver (but its probably too late now).

I would tend to favor Tristram's submission when we see it because he claims support for more devices and it is likely to be backed and maintained by Microchip in the future.

I am sure there will be opportunity for you to contribute a lot to this driver. Of course, this all depends on the code quality and timing, but having two people work on the same things in parallel is just a complete waste of each other's time so we might as well wait for Tristram to post the said driver and define a plan of action from there?

Quoted 22 lines · Andrew Lunn

Quoted 5 lines

Quoted 3 lines

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Quoted 8 lines

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

As far as I can tell (and their driver confirms) -- yes, all those registers can be accessed over the SPI, they are just shuffled around... hence MDIO emulation code. I copied it from their code (see the copyrights) so no, I don't believe there's nicer solution.

Best regards,

Quoted 1 line

Pavel

Can you hold on your developing work on KSZ8895 driver? I am afraid your effort may be in vain. We at Microchip are planning to release DSA drivers for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.

The driver files all follow the structures of the current KSZ9477 DSA driver, and the file tag_ksz.c will be updated to handle the tail tag of different chips, which requires including the ksz_priv.h header. That is required nevertheless to support using the offload_fwd_mark indication.

The KSZ8795 driver will be submitted after Labor Day (9/4) if testing reveals no problem. The KSZ8895 driver will be submitted right after that. You should have no problem using the driver right away.

Tristram Ha
Principal Software Engineer
Microchip Technology Inc.

-----Original Message-----
From: Maxim Uvarov [mailto:[email protected]]
Sent: Wednesday, September 06, 2017 2:15 AM
To: Tristram Ha - C24268
Cc: Pavel Machek; Woojung Huh - C21699; Nathan Conrad; Vivien Didelot; Florian Fainelli; netdev; [email protected]; Andrew Lunn
Subject: Re: [PATCH] DSA support for Micrel KSZ8895

2017-08-31 0:32 GMT+03:00 <[email protected]>:

Quoted 20 lines

Quoted 20 lines · Andrew Lunn

Quoted 5 lines

Quoted 3 lines

I may be confused here, but AFAICT:

1) Yes, it has standard layout when accessed over MDIO.

Quoted 8 lines

Section 4.8 of the datasheet says:

All the registers defined in this section can be also accessed via the SPI interface.

Meaning all PHY registers can be access via the SPI interface. So you should be able to make a standard Linux MDIO bus driver which performs SPI reads.

As far as I can tell (and their driver confirms) -- yes, all those registers can be accessed over the SPI, they are just shuffled around... hence MDIO emulation code. I copied it from their code (see the copyrights) so no, I don't believe there's nicer solution.

Quoted 6 lines

Quoted 4 lines

Best regards,

Pavel

Can you hold on your developing work on KSZ8895 driver? I am afraid your

effort may be in vain. We at Microchip are planning to release DSA drivers for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.

Quoted 2 lines

The driver files all follow the structures of the current KSZ9477 DSA driver,

and the file tag_ksz.c will be updated to handle the tail tag of different chips, which requires including the ksz_priv.h header. That is required nevertheless to support using the offload_fwd_mark indication.

Quoted 1 line

Quoted 1 line

The KSZ8795 driver will be submitted after Labor Day (9/4) if testing reveals

no problem. The KSZ8895 driver will be submitted right after that. You should have no problem using the driver right away.

Quoted 1 line

Hello Tristram, is there any update for that driver?

Maxim.

The patches are under review internally and will need to be updated and approved by Woojung before formal submission. Problem is although KSZ8795 and KSZ8895 drivers are new code and will be submitted as RFC, they depend on the change of KSZ9477 driver currently in the kernel, which require more rigorous review.

No, tag_ksz part probably is not acceptable. Do you see solution better than just copying it into tag_ksz1 file?

Quoted 2 lines

No, tag_ksz part probably is not acceptable. Do you see solution better than just copying it into tag_ksz1 file?

How about something like this, which needs further work to actually compile, but should give you the idea.

Andrew

index 99e38af85fc5..843e77b7c270 100644

net/dsa/dsa.c +5 -2

@@ -49,8 +49,11 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
 #ifdef CONFIG_NET_DSA_TAG_EDSA
        [DSA_TAG_PROTO_EDSA] = &edsa_netdev_ops,
 #endif
-#ifdef CONFIG_NET_DSA_TAG_KSZ
-       [DSA_TAG_PROTO_KSZ] = &ksz_netdev_ops,
+#ifdef CONFIG_NET_DSA_TAG_KSZ_8K
+       [DSA_TAG_PROTO_KSZ8K] = &ksz8k_netdev_ops,
+#endif
+#ifdef CONFIG_NET_DSA_TAG_KSZ_9K
+       [DSA_TAG_PROTO_KSZ9K] = &ksz9k_netdev_ops,
 #endif
 #ifdef CONFIG_NET_DSA_TAG_LAN9303
        [DSA_TAG_PROTO_LAN9303] = &lan9303_netdev_ops,

net/dsa/tag_ksz.c +17 -3

index de66ca8e6201..398b833889f1 100644
@@ -35,6 +35,9 @@
 static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
 {
        struct dsa_slave_priv *p = netdev_priv(dev);
+       struct dsa_port *dp = p->dp;
+       struct dsa_switch *ds = dp->ds;
+       struct dsa_switch_tree *dst = ds->dst;
        struct sk_buff *nskb;
        int padlen;
        u8 *tag;

@@ -69,8 +72,14 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
        }
 
        tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
-       tag[0] = 0;
-       tag[1] = 1 << p->dp->index; /* destination port */
+       if (dst->tag_ops == ksz8k_netdev_ops) {
+               tag[0] = 1 << p->dp->index; /* destination port */0;
+               tag[1] = 0;
+       }
+
+       if (dst->tag_ops == ksz9k_netdev_ops) {
+               tag[0] = 0;
+               tag[1] = 1 << p->dp->index; /* destination port */
 
        return nskb;
 }

@@ -98,7 +107,12 @@ static struct sk_buff *ksz_rcv(struct sk_buff *skb, struct net_device *dev,
        return skb;
 }
 
-const struct dsa_device_ops ksz_netdev_ops = {
+const struct dsa_device_ops ksz8k_netdev_ops = {
+       .xmit   = ksz_xmit,
+       .rcv    = ksz_rcv,
+};
+
+const struct dsa_device_ops ksz9k_netdev_ops = {
        .xmit   = ksz_xmit,
        .rcv    = ksz_rcv,
 };

Hi!

So I fought with the driver a bit more, and now I have something that kind-of-works.

"great great hack" belows worries me.

Yeah, disabled code needs to be removed before merge.

No, tag_ksz part probably is not acceptable. Do you see solution better than just copying it into tag_ksz1 file?

Any more comments, etc?

Help would be welcome.

Quoted 2 lines

No, tag_ksz part probably is not acceptable. Do you see solution better than just copying it into tag_ksz1 file?

You could have all Micrel tag implementations live under net/dsa/tag_ksz.c and have e.g: DSA_TAG_PROTO_KSZ for the current (newer) switches and DSA_TAG_PROTO_KSZ_LEGACY (or any other name) for the older switches and you would provide two sets of function pointers depending on which protocol is requested by the switch.

Considering the minor difference needed in tagging here, it might be acceptable to actually keep the current functions and just have the xmit() call check what get_tag_protocol returns and use word 1 or 0 based on that. Even though that's a fast path it shouldn't hurt performance too much. If it does, we can always copy the tagging protocol into dsa_slave_priv so you have a fast access to it.

Quoted 2 lines

Any more comments, etc?

The MII emulation bits are interesting, was it not sufficient if you implemented phy_read and phy_write operations that perform the necessary internal PHY accesses or maybe you don't get access to standard MII registers? b53 does such a thing and we merely just need to do a simple shift to access the MII register number, thus avoiding the translation.

Quoted 1 line

Help would be welcome.

I concur with Andrew, try to get a patch series, even an RFC one together so we can review things individually.

How functional is your driver so far? I'd say the basic stuff to get working: counters (debugging), link management (auto-negotiation, forced, etc.) and basic bridging: all ports separate by default and working port to port switching when brought together in a bridge. VLAN, FDB, MDB, other ethtool goodies can be added later on.

Quoted 8 lines

I've got hardware with KSZ8895, and I'd like to use switch ports as separate ethernet cards. I believe that means DSA support.

And there are even patches available from microchip... unfortunately they are in strange form and for v3.18.

http://www.microchip.com/SWLibraryWeb/product.aspx?product=KSZ8895%20Software%20Linux%203.18

Quoted 2 lines

Is there newer version of the driver available somewhere? Is the driver good starting point, or should I start with something else?

Hi Pavel

Woojung is the expert here. His DSA driver for the 9477 is a nice clean driver.

Have you compared the 8895 to the 9477. Are they similar? Could the existing 9477 be extended to support the 8895?