lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221021072735.pcx6wlnujhhxgy3q@soft-dev3-1>
Date:   Fri, 21 Oct 2022 09:27:35 +0200
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
CC:     <netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
        <linux-kernel@...r.kernel.org>, <bryan.whitehead@...rochip.com>,
        <hkallweit1@...il.com>, <pabeni@...hat.com>, <edumazet@...gle.com>,
        <linux@...linux.org.uk>, <UNGLinuxDriver@...rochip.com>,
        <andrew@...n.ch>, <Ian.Saturley@...rochip.com>
Subject: Re: [PATCH net-next 2/2] net: phy: micrel: Add PHY Auto/MDI/MDI-X
 set driver for KSZ9131

The 10/21/2022 11:26, Raju Lakkaraju wrote:

Hi Raju,

> Add support for MDI-X status and configuration for KSZ9131 chips
> 
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
> ---
>  drivers/net/phy/micrel.c | 77 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 54a17b576eac..40aa52d442f8 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1295,6 +1295,81 @@ static int ksz9131_config_init(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +#define MII_KSZPHY_AUTO_MDIX		0x1C
> +#define MII_KSZPHY_AUTO_MDI_SET_	BIT(7)
> +#define MII_KSZPHY_AUTO_MDIX_SWAP_OFF_	BIT(6)

Can you please drop the "_" from the end of the macros. The only
macros that have that suffix are those used by PTP.

Are these KSZPHY registers generic for all KSZ phys? Then the functions
'ksz9131_mdix_update' and 'ksz9131_config_mdix' shouldn't be rename to
something like 'kszphy_mdix_update' and 'kszphy_config_mdix'?
Otherwise shoudln't these register contain 9131 in their name?

I know that also lan8841 and lan8804 have the same layout for these
registers.

> +
> +static int ksz9131_mdix_update(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = phy_read(phydev, MII_KSZPHY_AUTO_MDIX);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret & MII_KSZPHY_AUTO_MDIX_SWAP_OFF_) {
> +		if (ret & MII_KSZPHY_AUTO_MDI_SET_)
> +			phydev->mdix_ctrl = ETH_TP_MDI;
> +		else
> +			phydev->mdix_ctrl = ETH_TP_MDI_X;
> +	} else {
> +		phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
> +	}
> +
> +	if (ret & MII_KSZPHY_AUTO_MDI_SET_)
> +		phydev->mdix = ETH_TP_MDI;
> +	else
> +		phydev->mdix = ETH_TP_MDI_X;
> +
> +	return 0;
> +}
> +
> +static int ksz9131_config_mdix(struct phy_device *phydev, u8 ctrl)
> +{
> +	u16 val;
> +
> +	switch (ctrl) {
> +	case ETH_TP_MDI:
> +		val = MII_KSZPHY_AUTO_MDIX_SWAP_OFF_ |
> +		      MII_KSZPHY_AUTO_MDI_SET_;
> +		break;
> +	case ETH_TP_MDI_X:
> +		val = MII_KSZPHY_AUTO_MDIX_SWAP_OFF_;
> +		break;
> +	case ETH_TP_MDI_AUTO:
> +		val = 0;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return phy_modify(phydev, MII_KSZPHY_AUTO_MDIX,
> +			  MII_KSZPHY_AUTO_MDIX_SWAP_OFF_ |
> +			  MII_KSZPHY_AUTO_MDI_SET_, val);
> +}
> +
> +static int ksz9131_read_status(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = ksz9131_mdix_update(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	return genphy_read_status(phydev);
> +}
> +
> +static int ksz9131_config_aneg(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = genphy_config_aneg(phydev);
> +	if (ret)
> +		return ret;
> +
> +	return ksz9131_config_mdix(phydev, phydev->mdix_ctrl);
> +}
> +
>  #define KSZ8873MLL_GLOBAL_CONTROL_4	0x06
>  #define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	BIT(6)
>  #define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	BIT(4)
> @@ -3304,6 +3379,8 @@ static struct phy_driver ksphy_driver[] = {
>  	.probe		= kszphy_probe,
>  	.config_init	= ksz9131_config_init,
>  	.config_intr	= kszphy_config_intr,
> +	.config_aneg	= ksz9131_config_aneg,
> +	.read_status	= ksz9131_read_status,
>  	.handle_interrupt = kszphy_handle_interrupt,
>  	.get_sset_count = kszphy_get_sset_count,
>  	.get_strings	= kszphy_get_strings,
> -- 
> 2.25.1
> 

-- 
/Horatiu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ