diff mbox series

[RFC,v1,2/9] net: dsa: microchip: ksz8795: add phylink support

Message ID 20210505092025.8785-3-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series provide cable test support for the ksz886x | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning CHECK: Alignment should match open parenthesis CHECK: Unnecessary parentheses around 'state->interface != PHY_INTERFACE_MODE_MII' CHECK: Unnecessary parentheses around 'state->interface != PHY_INTERFACE_MODE_RMII' WARNING: else is not generally useful after a break or return WARNING: line length of 81 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Oleksij Rempel May 5, 2021, 9:20 a.m. UTC 
From: Michael Grzeschik <m.grzeschik@pengutronix.de>

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

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz8795.c | 73 +++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

Comments

Vladimir Oltean May 6, 2021, 12:13 p.m. UTC | #1 
Hi Oleksij,

On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> This patch adds the phylink support to the ksz8795 driver, since
> phylib is obsolete for dsa drivers.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  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/20210422094257.1641396-6-prasanna.vengateshan@microchip.com/
What made you submit the patch?
Oleksij Rempel May 6, 2021, 1:05 p.m. UTC | #2 
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 <m.grzeschik@pengutronix.de>
> > 
> > This patch adds the phylink support to the ksz8795 driver, since
> > phylib is obsolete for dsa drivers.
> > 
> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  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/20210422094257.1641396-6-prasanna.vengateshan@microchip.com/
> 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
Vladimir Oltean May 6, 2021, 1:28 p.m. UTC | #3 
On Wed, May 05, 2021 at 11:20:18AM +0200, Oleksij Rempel wrote:
> From: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> This patch adds the phylink support to the ksz8795 driver, since
> phylib is obsolete for dsa drivers.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  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
>
Oleksij Rempel May 10, 2021, 9:02 a.m. UTC | #4 
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 <m.grzeschik@pengutronix.de>
> > 
> > This patch adds the phylink support to the ksz8795 driver, since
> > phylib is obsolete for dsa drivers.
> > 
> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  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
diff mbox series

Patch

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,