2021-05-05 09:26:09

by Oleksij Rempel

[permalink] [raw]
Subject: [RFC PATCH v1 2/9] net: dsa: microchip: ksz8795: add phylink support

From: Michael Grzeschik <[email protected]>

This patch adds the phylink support to the ksz8795 driver, since
phylib is obsolete for dsa drivers.

Signed-off-by: Michael Grzeschik <[email protected]>
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 4ca352fbe81c..0ddaf2547f18 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -18,6 +18,7 @@
#include <linux/micrel_phy.h>
#include <net/dsa.h>
#include <net/switchdev.h>
+#include <linux/phylink.h>

#include "ksz_common.h"
#include "ksz8795_reg.h"
@@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)
return 0;
}

+static int ksz_get_state(struct dsa_switch *ds, int port,
+ struct phylink_link_state *state)
+{
+ struct ksz_device *dev = ds->priv;
+ struct ksz8 *ksz8 = dev->priv;
+ const u8 *regs = ksz8->regs;
+ u8 speed, link;
+
+ ksz_pread8(dev, port, regs[P_LINK_STATUS], &link);
+ ksz_pread8(dev, port, regs[P_SPEED_STATUS], &speed);
+
+ state->link = !!(link & PORT_STAT_LINK_GOOD);
+ if (state->link) {
+ state->speed =
+ (speed & PORT_STAT_SPEED_100MBIT) ? SPEED_100 : SPEED_10;
+ state->duplex =
+ (speed & PORT_STAT_FULL_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF;
+ }
+
+ return 0;
+}
+
+static void ksz_validate(struct dsa_switch *ds, int port,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+ struct ksz_device *dev = ds->priv;
+
+ if (port == dev->cpu_port) {
+ if ((state->interface != PHY_INTERFACE_MODE_RMII) &&
+ (state->interface != PHY_INTERFACE_MODE_MII))
+ goto unsupported;
+ } else if (port > dev->port_cnt) {
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ dev_err(ds->dev, "Unsupported port: %i\n", port);
+ return;
+ } else {
+ if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
+ goto unsupported;
+ }
+
+ /* Allow all the expected bits */
+ phylink_set_port_modes(mask);
+ phylink_set(mask, Autoneg);
+
+ phylink_set(mask, Pause);
+ /* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */
+ if (port || !ksz_is_ksz88x3(dev))
+ phylink_set(mask, Asym_Pause);
+
+ /* 10M and 100M are only supported */
+ phylink_set(mask, 10baseT_Half);
+ phylink_set(mask, 10baseT_Full);
+ phylink_set(mask, 100baseT_Half);
+ phylink_set(mask, 100baseT_Full);
+
+ bitmap_and(supported, supported, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(state->advertising, state->advertising, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+
+ return;
+
+unsupported:
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ dev_err(ds->dev, "Unsupported interface: %d, port: %d\n",
+ state->interface, port);
+}
+
static const struct dsa_switch_ops ksz8_switch_ops = {
.get_tag_protocol = ksz8_get_tag_protocol,
.setup = ksz8_setup,
.phy_read = ksz_phy_read16,
.phy_write = ksz_phy_write16,
+ .phylink_validate = ksz_validate,
+ .phylink_mac_link_state = ksz_get_state,
.phylink_mac_link_down = ksz_mac_link_down,
.port_enable = ksz_enable_port,
.get_strings = ksz8_get_strings,
--
2.29.2


2021-05-06 12:16:41

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC PATCH v1 2/9] net: dsa: microchip: ksz8795: add phylink support

Hi Oleksij,

On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
> From: Michael Grzeschik <[email protected]>
>
> This patch adds the phylink support to the ksz8795 driver, since
> phylib is obsolete for dsa drivers.
>
> Signed-off-by: Michael Grzeschik <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 4ca352fbe81c..0ddaf2547f18 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -18,6 +18,7 @@
> #include <linux/micrel_phy.h>
> #include <net/dsa.h>
> #include <net/switchdev.h>
> +#include <linux/phylink.h>
>
> #include "ksz_common.h"
> #include "ksz8795_reg.h"
> @@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)
> return 0;
> }
>
> +static int ksz_get_state(struct dsa_switch *ds, int port,
> + struct phylink_link_state *state)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz8 *ksz8 = dev->priv;
> + const u8 *regs = ksz8->regs;
> + u8 speed, link;
> +
> + ksz_pread8(dev, port, regs[P_LINK_STATUS], &link);
> + ksz_pread8(dev, port, regs[P_SPEED_STATUS], &speed);
> +
> + state->link = !!(link & PORT_STAT_LINK_GOOD);
> + if (state->link) {
> + state->speed =
> + (speed & PORT_STAT_SPEED_100MBIT) ? SPEED_100 : SPEED_10;
> + state->duplex =
> + (speed & PORT_STAT_FULL_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF;
> + }
> +
> + return 0;
> +}
> +
> +static void ksz_validate(struct dsa_switch *ds, int port,
> + unsigned long *supported,
> + struct phylink_link_state *state)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> + struct ksz_device *dev = ds->priv;
> +
> + if (port == dev->cpu_port) {
> + if ((state->interface != PHY_INTERFACE_MODE_RMII) &&
> + (state->interface != PHY_INTERFACE_MODE_MII))
> + goto unsupported;
> + } else if (port > dev->port_cnt) {
> + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + dev_err(ds->dev, "Unsupported port: %i\n", port);
> + return;
> + } else {
> + if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
> + goto unsupported;
> + }
> +
> + /* Allow all the expected bits */
> + phylink_set_port_modes(mask);
> + phylink_set(mask, Autoneg);
> +
> + phylink_set(mask, Pause);
> + /* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */
> + if (port || !ksz_is_ksz88x3(dev))
> + phylink_set(mask, Asym_Pause);
> +
> + /* 10M and 100M are only supported */
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> +
> + bitmap_and(supported, supported, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> + bitmap_and(state->advertising, state->advertising, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> +
> + return;
> +
> +unsupported:
> + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + dev_err(ds->dev, "Unsupported interface: %d, port: %d\n",
> + state->interface, port);
> +}
> +
> static const struct dsa_switch_ops ksz8_switch_ops = {
> .get_tag_protocol = ksz8_get_tag_protocol,
> .setup = ksz8_setup,
> .phy_read = ksz_phy_read16,
> .phy_write = ksz_phy_write16,
> + .phylink_validate = ksz_validate,
> + .phylink_mac_link_state = ksz_get_state,
> .phylink_mac_link_down = ksz_mac_link_down,
> .port_enable = ksz_enable_port,
> .get_strings = ksz8_get_strings,
> --
> 2.29.2
>

I've asked Prasanna about this too, but for one reason or another I am
still not edified. Is this change a compliance thing, or do you actually
gain anything at all from phylink?
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
What made you submit the patch?

2021-05-06 13:08:14

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [RFC PATCH v1 2/9] net: dsa: microchip: ksz8795: add phylink support

Hi Vladimir,

On Thu, May 06, 2021 at 03:13:21PM +0300, Vladimir Oltean wrote:
> Hi Oleksij,
>
> On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
> > From: Michael Grzeschik <[email protected]>
> >
> > This patch adds the phylink support to the ksz8795 driver, since
> > phylib is obsolete for dsa drivers.
> >
> > Signed-off-by: Michael Grzeschik <[email protected]>
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
> > 1 file changed, 73 insertions(+)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> > index 4ca352fbe81c..0ddaf2547f18 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -18,6 +18,7 @@
> > #include <linux/micrel_phy.h>
> > #include <net/dsa.h>
> > #include <net/switchdev.h>
> > +#include <linux/phylink.h>
> >
> > #include "ksz_common.h"
> > #include "ksz8795_reg.h"
> > @@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)
> > return 0;
> > }
> >
> > +static int ksz_get_state(struct dsa_switch *ds, int port,
> > + struct phylink_link_state *state)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + struct ksz8 *ksz8 = dev->priv;
> > + const u8 *regs = ksz8->regs;
> > + u8 speed, link;
> > +
> > + ksz_pread8(dev, port, regs[P_LINK_STATUS], &link);
> > + ksz_pread8(dev, port, regs[P_SPEED_STATUS], &speed);
> > +
> > + state->link = !!(link & PORT_STAT_LINK_GOOD);
> > + if (state->link) {
> > + state->speed =
> > + (speed & PORT_STAT_SPEED_100MBIT) ? SPEED_100 : SPEED_10;
> > + state->duplex =
> > + (speed & PORT_STAT_FULL_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void ksz_validate(struct dsa_switch *ds, int port,
> > + unsigned long *supported,
> > + struct phylink_link_state *state)
> > +{
> > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > + struct ksz_device *dev = ds->priv;
> > +
> > + if (port == dev->cpu_port) {
> > + if ((state->interface != PHY_INTERFACE_MODE_RMII) &&
> > + (state->interface != PHY_INTERFACE_MODE_MII))
> > + goto unsupported;
> > + } else if (port > dev->port_cnt) {
> > + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + dev_err(ds->dev, "Unsupported port: %i\n", port);
> > + return;
> > + } else {
> > + if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
> > + goto unsupported;
> > + }
> > +
> > + /* Allow all the expected bits */
> > + phylink_set_port_modes(mask);
> > + phylink_set(mask, Autoneg);
> > +
> > + phylink_set(mask, Pause);
> > + /* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */
> > + if (port || !ksz_is_ksz88x3(dev))
> > + phylink_set(mask, Asym_Pause);
> > +
> > + /* 10M and 100M are only supported */
> > + phylink_set(mask, 10baseT_Half);
> > + phylink_set(mask, 10baseT_Full);
> > + phylink_set(mask, 100baseT_Half);
> > + phylink_set(mask, 100baseT_Full);
> > +
> > + bitmap_and(supported, supported, mask,
> > + __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + bitmap_and(state->advertising, state->advertising, mask,
> > + __ETHTOOL_LINK_MODE_MASK_NBITS);
> > +
> > + return;
> > +
> > +unsupported:
> > + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + dev_err(ds->dev, "Unsupported interface: %d, port: %d\n",
> > + state->interface, port);
> > +}
> > +
> > static const struct dsa_switch_ops ksz8_switch_ops = {
> > .get_tag_protocol = ksz8_get_tag_protocol,
> > .setup = ksz8_setup,
> > .phy_read = ksz_phy_read16,
> > .phy_write = ksz_phy_write16,
> > + .phylink_validate = ksz_validate,
> > + .phylink_mac_link_state = ksz_get_state,
> > .phylink_mac_link_down = ksz_mac_link_down,
> > .port_enable = ksz_enable_port,
> > .get_strings = ksz8_get_strings,
> > --
> > 2.29.2
> >
>
> I've asked Prasanna about this too, but for one reason or another I am
> still not edified. Is this change a compliance thing, or do you actually
> gain anything at all from phylink?
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
> What made you submit the patch?

This switch has proper flow control only on one port, see
Module 4: Port 1 does not respond to received flow control PAUSE frames:
http://ww1.microchip.com/downloads/en/DeviceDoc/KSZ8873-Errata-DS80000830A.pdf

With phylink we can disable Pause here, instead of bypassing it to the PHY
driver.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2021-05-06 13:30:43

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [RFC PATCH v1 2/9] net: dsa: microchip: ksz8795: add phylink support

On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
> From: Michael Grzeschik <[email protected]>
>
> This patch adds the phylink support to the ksz8795 driver, since
> phylib is obsolete for dsa drivers.
>
> Signed-off-by: Michael Grzeschik <[email protected]>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 4ca352fbe81c..0ddaf2547f18 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -18,6 +18,7 @@
> #include <linux/micrel_phy.h>
> #include <net/dsa.h>
> #include <net/switchdev.h>
> +#include <linux/phylink.h>
>
> #include "ksz_common.h"
> #include "ksz8795_reg.h"
> @@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)
> return 0;
> }
>
> +static int ksz_get_state(struct dsa_switch *ds, int port,
> + struct phylink_link_state *state)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz8 *ksz8 = dev->priv;
> + const u8 *regs = ksz8->regs;
> + u8 speed, link;
> +
> + ksz_pread8(dev, port, regs[P_LINK_STATUS], &link);
> + ksz_pread8(dev, port, regs[P_SPEED_STATUS], &speed);
> +
> + state->link = !!(link & PORT_STAT_LINK_GOOD);
> + if (state->link) {
> + state->speed =
> + (speed & PORT_STAT_SPEED_100MBIT) ? SPEED_100 : SPEED_10;
> + state->duplex =
> + (speed & PORT_STAT_FULL_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF;
> + }
> +
> + return 0;
> +}

How does the port know the speed?

> +
> +static void ksz_validate(struct dsa_switch *ds, int port,
> + unsigned long *supported,
> + struct phylink_link_state *state)

Indentation looks odd.
Also, I expect that not all KSZ PHYs to have the same validation
function, so maybe you should call this ksz8_phylink_validate.

> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> + struct ksz_device *dev = ds->priv;
> +
> + if (port == dev->cpu_port) {
> + if ((state->interface != PHY_INTERFACE_MODE_RMII) &&
> + (state->interface != PHY_INTERFACE_MODE_MII))
> + goto unsupported;

The phylink API says that when .validate is called with state->interface
as PHY_INTERFACE_MODE_NA, you should report all supported capabilities.

> + } else if (port > dev->port_cnt) {
> + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + dev_err(ds->dev, "Unsupported port: %i\n", port);
> + return;
> + } else {
> + if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
> + goto unsupported;
> + }
> +
> + /* Allow all the expected bits */
> + phylink_set_port_modes(mask);
> + phylink_set(mask, Autoneg);
> +
> + phylink_set(mask, Pause);
> + /* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */
> + if (port || !ksz_is_ksz88x3(dev))
> + phylink_set(mask, Asym_Pause);

The code doesn't seem to match the comment? If the switch is a KSZ88x3,
ASM_DIR will be advertised for all ports except port 0, is this what you
want?

> +
> + /* 10M and 100M are only supported */
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> +
> + bitmap_and(supported, supported, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> + bitmap_and(state->advertising, state->advertising, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> +
> + return;
> +
> +unsupported:
> + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + dev_err(ds->dev, "Unsupported interface: %d, port: %d\n",
> + state->interface, port);

%s, phy_modes(state->interface)

> +}
> +
> static const struct dsa_switch_ops ksz8_switch_ops = {
> .get_tag_protocol = ksz8_get_tag_protocol,
> .setup = ksz8_setup,
> .phy_read = ksz_phy_read16,
> .phy_write = ksz_phy_write16,
> + .phylink_validate = ksz_validate,
> + .phylink_mac_link_state = ksz_get_state,
> .phylink_mac_link_down = ksz_mac_link_down,
> .port_enable = ksz_enable_port,
> .get_strings = ksz8_get_strings,
> --
> 2.29.2
>

2021-05-10 09:03:41

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [RFC PATCH v1 2/9] net: dsa: microchip: ksz8795: add phylink support

Hi Vladimir,

On Thu, May 06, 2021 at 04:28:55PM +0300, Vladimir Oltean wrote:
> On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
> > From: Michael Grzeschik <[email protected]>
> >
> > This patch adds the phylink support to the ksz8795 driver, since
> > phylib is obsolete for dsa drivers.
> >
> > Signed-off-by: Michael Grzeschik <[email protected]>
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
> > 1 file changed, 73 insertions(+)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> > index 4ca352fbe81c..0ddaf2547f18 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -18,6 +18,7 @@
> > #include <linux/micrel_phy.h>
> > #include <net/dsa.h>
> > #include <net/switchdev.h>
> > +#include <linux/phylink.h>
> >
> > #include "ksz_common.h"
> > #include "ksz8795_reg.h"
> > @@ -1420,11 +1421,83 @@ static int ksz8_setup(struct dsa_switch *ds)
> > return 0;
> > }
> >
> > +static int ksz_get_state(struct dsa_switch *ds, int port,
> > + struct phylink_link_state *state)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + struct ksz8 *ksz8 = dev->priv;
> > + const u8 *regs = ksz8->regs;
> > + u8 speed, link;
> > +
> > + ksz_pread8(dev, port, regs[P_LINK_STATUS], &link);
> > + ksz_pread8(dev, port, regs[P_SPEED_STATUS], &speed);
> > +
> > + state->link = !!(link & PORT_STAT_LINK_GOOD);
> > + if (state->link) {
> > + state->speed =
> > + (speed & PORT_STAT_SPEED_100MBIT) ? SPEED_100 : SPEED_10;
> > + state->duplex =
> > + (speed & PORT_STAT_FULL_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF;
> > + }
> > +
> > + return 0;
> > +}
>
> How does the port know the speed?

PHY and switch control registers are mixed on this switch, so we have
access to the PHY bits directly over switch control registers.
On other hand, we provide proper PHY abstraction and there is no need to
provide this function at all. I'll remove it.

> > +
> > +static void ksz_validate(struct dsa_switch *ds, int port,
> > + unsigned long *supported,
> > + struct phylink_link_state *state)
>
> Indentation looks odd.
> Also, I expect that not all KSZ PHYs to have the same validation
> function, so maybe you should call this ksz8_phylink_validate.

done

> > +{
> > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > + struct ksz_device *dev = ds->priv;
> > +
> > + if (port == dev->cpu_port) {
> > + if ((state->interface != PHY_INTERFACE_MODE_RMII) &&
> > + (state->interface != PHY_INTERFACE_MODE_MII))
> > + goto unsupported;
>
> The phylink API says that when .validate is called with state->interface
> as PHY_INTERFACE_MODE_NA, you should report all supported capabilities.

done

> > + } else if (port > dev->port_cnt) {
> > + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + dev_err(ds->dev, "Unsupported port: %i\n", port);
> > + return;
> > + } else {
> > + if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
> > + goto unsupported;
> > + }
> > +
> > + /* Allow all the expected bits */
> > + phylink_set_port_modes(mask);
> > + phylink_set(mask, Autoneg);
> > +
> > + phylink_set(mask, Pause);
> > + /* Silicon Errata Sheet (DS80000830A): Asym_Pause limit to port 2 */
> > + if (port || !ksz_is_ksz88x3(dev))
> > + phylink_set(mask, Asym_Pause);
>
> The code doesn't seem to match the comment? If the switch is a KSZ88x3,
> ASM_DIR will be advertised for all ports except port 0, is this what you
> want?

good point, no. Fixed.

> > +
> > + /* 10M and 100M are only supported */
> > + phylink_set(mask, 10baseT_Half);
> > + phylink_set(mask, 10baseT_Full);
> > + phylink_set(mask, 100baseT_Half);
> > + phylink_set(mask, 100baseT_Full);
> > +
> > + bitmap_and(supported, supported, mask,
> > + __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + bitmap_and(state->advertising, state->advertising, mask,
> > + __ETHTOOL_LINK_MODE_MASK_NBITS);
> > +
> > + return;
> > +
> > +unsupported:
> > + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + dev_err(ds->dev, "Unsupported interface: %d, port: %d\n",
> > + state->interface, port);
>
> %s, phy_modes(state->interface)

done

> > +}
> > +
> > static const struct dsa_switch_ops ksz8_switch_ops = {
> > .get_tag_protocol = ksz8_get_tag_protocol,
> > .setup = ksz8_setup,
> > .phy_read = ksz_phy_read16,
> > .phy_write = ksz_phy_write16,
> > + .phylink_validate = ksz_validate,
> > + .phylink_mac_link_state = ksz_get_state,
> > .phylink_mac_link_down = ksz_mac_link_down,
> > .port_enable = ksz_enable_port,
> > .get_strings = ksz8_get_strings,
> > --
> > 2.29.2
> >
>
>

Regards,
Oleksij

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |