netdev - Re: [net-next.git 1/4 (v5)] phy: add the EEE support and the way to access to the MMD registers.

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: <4FDAD0C8.6000307@st.com>
Date:	Fri, 15 Jun 2012 08:06:00 +0200
From:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	netdev@...r.kernel.org, eric.dumazet@...il.com,
	rayagond@...avyalabs.com, davem@...emloft.net,
	yuvalmin@...adcom.com
Subject: Re: [net-next.git 1/4 (v5)] phy: add the EEE support and the way
 to access to the MMD registers.

Hello Ben

On 6/14/2012 1:28 AM, Ben Hutchings wrote:

[snip]

>> > + */
>> > +int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
>> > +{
>> > +	int ret = -EPROTONOSUPPORT;
>> > +
>> > +	/* According to 802.3az,the EEE is supported only in full duplex-mode.
>> > +	 * Also EEE feature is active when core is operating with MII, GMII
>> > +	 * or RGMII.
>> > +	 */
>> > +	if ((phydev->duplex == DUPLEX_FULL) &&
>> > +	    ((phydev->interface == PHY_INTERFACE_MODE_MII) ||
>> > +	    (phydev->interface == PHY_INTERFACE_MODE_GMII) ||
>> > +	    (phydev->interface == PHY_INTERFACE_MODE_RGMII))) {
>> > +		int eee_cap, eee_link;
>> > +
>> > +		/* EEE ability must be supported in both local and remote
>> > +		 * PHY devices.
>> > +		 */
>> > +		eee_cap = phy_read_mmd_indirect(phydev->bus, MDIO_AN_EEE_LPABLE,
>> > +						MDIO_MMD_AN, phydev->addr);
>> > +		if (eee_cap < 0)
>> > +			return eee_cap;
>> > +
>> > +		eee_link = phy_read_mmd_indirect(phydev->bus, MDIO_PCS_EEE_ABLE,
>> > +						 MDIO_MMD_PCS, phydev->addr);
>> > +		if (eee_link < 0)
>> > +			return eee_link;
>> > +
>> > +		if (eee_cap && eee_link) {
> I don't see any harm in setting the 'clock stop' bit if requested, even
> if EEE is not supported and therefore we will never receive LPI from the
> link partner.

ok

> But you also use this condition to decide whether to enable TX LPI, so
> it's important that it does match the specification (§78.3) for whether
> EEE is supported - but it doesn't.  You need to work out what mode was
> autonegotiated, then check that the relevant bit is set in both our EEE
> advertising (*not* supported) and the LP EEE advertising masks.

I've some doubts and, before resending the patch, I kindly ask you some
further details just on this point.

In the code, I check if the EEE is supported and on GMII, MII and RGMII
and duplex mode; in case of success the Ethernet driver can enable the
TX LPI.
Indeed, I am only using the 3.20 and 7.61 registers w/o looking at the
7.60. So this should be fixed, shouldn't it?
Am I missing anything else?
What do you mean when say that it doesn't match the specification
(§78.3)? I'm pointing to the '78.3 Capabilities Negotiation' chapter of
the IEEE802-3az, is it ok?..........

....Thanks for your feedback in advance.

peppe
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html