From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mugunthan V N Subject: Re: [net PATCH 1/1] net: phy: fix phy link up when limiting speed via device tree Date: Fri, 26 Jun 2015 00:32:57 +0530 Message-ID: <558C5061.6090509@ti.com> References: <1435251062-5287-1-git-send-email-mugunthanvnm@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: netdev , David Miller To: Florian Fainelli Return-path: Received: from comal.ext.ti.com ([198.47.26.152]:59726 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751555AbbFYTDD (ORCPT ); Thu, 25 Jun 2015 15:03:03 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Thursday 25 June 2015 11:01 PM, Florian Fainelli wrote: > 2015-06-25 9:51 GMT-07:00 Mugunthan V N : >> > When limiting phy link speed using "max-speed" to 100mbps or less on a >> > giga bit phy, phy never completes auto negotiation and phy state >> > machine is held in PHY_AN. Fixing this issue by comparing the giga >> > bit advertise though phydev->supported doesn't have it but phy has >> > BMSR_ESTATEN set. So that auto negotiation is restarted as old and >> > new advertise are different and link comes up fine. > This looks valid, however, I am curious why this part of the code: > > oldadv = adv; > adv &= ~(ADVERTISE_ALL | ADVERTISE_100BASE4 | ADVERTISE_PAUSE_CAP | > ADVERTISE_PAUSE_ASYM); > adv |= ethtool_adv_to_mii_adv_t(advertise); > > if (adv != oldadv) { > err = phy_write(phydev, MII_ADVERTISE, adv); > > if (err < 0) > return err; > changed = 1; > } > > Is not flagging this as a change already? > This can flag a change when the selected limit is 10mbps, if 100mbps is selected, at this instant oldadv and adv are same so change is not flagged here. Regards Mugunthan V N