mbox series

[RFC,v1,0/9] provide cable test support for the ksz886x

Message ID 20210505092025.8785-1-o.rempel@pengutronix.de
Headers show
Series provide cable test support for the ksz886x | expand

Message

Oleksij Rempel May 5, 2021, 9:20 a.m. UTC
This patches provide support for cable testing on the ksz886x switches.
Since it has one special port, we needed to add phylink with validation
and extra quirk for the PHY to signal, that one port will not provide
valid cable testing reports.

Michael Grzeschik (3):
  net: phy: micrel: move phy reg offsets to common header
  net: dsa: microchip: ksz8795: add phylink support
  net: phy: micrel: add patch for erratas on port1

Oleksij Rempel (6):
  net: phy: micrel: use consistent indention after define
  net: phy: micrel: apply resume errata workaround for ksz8873 and
    ksz8863
  net: phy: micrel: ksz886x add MDI-X support
  net: phy: micrel: ksz8081 add MDI-X support
  net: dsa: microchip: ksz8795: add LINK_MD register support
  net: phy: micrel: ksz886x/ksz8081: add cabletest support

 drivers/net/dsa/microchip/ksz8795.c     | 110 +++++++
 drivers/net/dsa/microchip/ksz8795_reg.h |  67 +---
 drivers/net/ethernet/micrel/ksz884x.c   |  70 +---
 drivers/net/phy/micrel.c                | 416 +++++++++++++++++++++++-
 drivers/net/phy/phylink.c               |   2 +-
 include/linux/micrel_phy.h              |  64 ++++
 net/dsa/slave.c                         |   4 +
 7 files changed, 586 insertions(+), 147 deletions(-)

Comments

Andrew Lunn May 5, 2021, 12:37 p.m. UTC | #1
> +/* Device specific MII_BMCR (Reg 0) bits */
> +/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */
> +#define KSZ886X_BMCR_HP_MDIX			BIT(5)
> +/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation
> + * (transmit on TXP/TXM pins)
> + */
> +#define KSZ886X_BMCR_FORCE_MDI			BIT(4)
> +/* 1 = Disable auto MDI-X */
> +#define KSZ886X_BMCR_DISABLE_AUTO_MDIX		BIT(3)
> +#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT	BIT(2)
> +#define KSZ886X_BMCR_DISABLE_TRANSMIT		BIT(1)
> +#define KSZ886X_BMCR_DISABLE_LED		BIT(0)

Do these have the same values as what you added in patch 1?

> +static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)
> +{
> +	u16 val;
> +
> +	switch (ctrl) {
> +	case ETH_TP_MDI:
> +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;
> +		break;
> +	case ETH_TP_MDI_X:
> +		/* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit
> +		 * counter intuitive, the "-X" in "1 = Force MDI" in the data
> +		 * sheet seems to be missing:
> +		 * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)
> +		 * 0 = Normal operation (transmit on TX+/TX- pins)
> +		 */
> +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;
> +		break;
> +	case ETH_TP_MDI_AUTO:
> +		val = 0;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return phy_modify(phydev, MII_BMCR,
> +			  KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |
> +			  KSZ886X_BMCR_DISABLE_AUTO_MDIX,
> +			  KSZ886X_BMCR_HP_MDIX | val);
> +}

Maybe this will also work for the PHY driver embedded in ksz8795.c?
Maybe as another patchset, see if that PHY driver can be moved out of the DSA driver,
and share some code with this driver?

    Andrew
Oleksij Rempel May 10, 2021, 9:10 a.m. UTC | #2
On Wed, May 05, 2021 at 02:37:35PM +0200, Andrew Lunn wrote:
> > +/* Device specific MII_BMCR (Reg 0) bits */

> > +/* 1 = HP Auto MDI/MDI-X mode, 0 = Microchip Auto MDI/MDI-X mode */

> > +#define KSZ886X_BMCR_HP_MDIX			BIT(5)

> > +/* 1 = Force MDI (transmit on RXP/RXM pins), 0 = Normal operation

> > + * (transmit on TXP/TXM pins)

> > + */

> > +#define KSZ886X_BMCR_FORCE_MDI			BIT(4)

> > +/* 1 = Disable auto MDI-X */

> > +#define KSZ886X_BMCR_DISABLE_AUTO_MDIX		BIT(3)

> > +#define KSZ886X_BMCR_DISABLE_FAR_END_FAULT	BIT(2)

> > +#define KSZ886X_BMCR_DISABLE_TRANSMIT		BIT(1)

> > +#define KSZ886X_BMCR_DISABLE_LED		BIT(0)

> 

> Do these have the same values as what you added in patch 1?


ACK, i'll move it

> > +static int ksz886x_config_mdix(struct phy_device *phydev, u8 ctrl)

> > +{

> > +	u16 val;

> > +

> > +	switch (ctrl) {

> > +	case ETH_TP_MDI:

> > +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX;

> > +		break;

> > +	case ETH_TP_MDI_X:

> > +		/* Note: The naming of the bit KSZ886X_BMCR_FORCE_MDI is bit

> > +		 * counter intuitive, the "-X" in "1 = Force MDI" in the data

> > +		 * sheet seems to be missing:

> > +		 * 1 = Force MDI (sic!) (transmit on RX+/RX- pins)

> > +		 * 0 = Normal operation (transmit on TX+/TX- pins)

> > +		 */

> > +		val = KSZ886X_BMCR_DISABLE_AUTO_MDIX | KSZ886X_BMCR_FORCE_MDI;

> > +		break;

> > +	case ETH_TP_MDI_AUTO:

> > +		val = 0;

> > +		break;

> > +	default:

> > +		return 0;

> > +	}

> > +

> > +	return phy_modify(phydev, MII_BMCR,

> > +			  KSZ886X_BMCR_HP_MDIX | KSZ886X_BMCR_FORCE_MDI |

> > +			  KSZ886X_BMCR_DISABLE_AUTO_MDIX,

> > +			  KSZ886X_BMCR_HP_MDIX | val);

> > +}

> 

> Maybe this will also work for the PHY driver embedded in ksz8795.c?

> Maybe as another patchset, see if that PHY driver can be moved out of the DSA driver,

> and share some code with this driver?


Hm, i'm sure it can be done, but right now i have no access to ksz8795
hardware. I'll keep it in mind.

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 |