From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1bD3hk-0006vw-Tp for barebox@lists.infradead.org; Wed, 15 Jun 2016 05:49:33 +0000 Date: Wed, 15 Jun 2016 07:49:10 +0200 From: Sascha Hauer Message-ID: <20160615054910.GC9677@pengutronix.de> References: <20160504074331.GM19714@pengutronix.de> <1465838955-24515-1-git-send-email-guille.rodriguez@gmail.com> <20160614063919.GP31666@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "barebox" Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH] Fix genphy_restart_aneg() for Micrel's ksz9031. To: Guillermo Rodriguez Garcia Cc: barebox@lists.infradead.org, Philipp Zabel , Trent Piepho On Tue, Jun 14, 2016 at 09:39:23AM +0200, Guillermo Rodriguez Garcia wrote: > Hi Sascha, > > 2016-06-14 8:39 GMT+02:00 Sascha Hauer : > > Hi Guillermo, > > > > On Mon, Jun 13, 2016 at 07:29:15PM +0200, Guillermo Rodriguez Garcia wrote: > >> From: grodriguez > >> > >> Commit da89ee8f2e04 ("Center FLP timing at 16ms") breaks > >> genphy_restart_aneg() for Micrel's ksz9031. According to the > >> datasheet, the ksz9031 requires a wait of 1ms after clearing > >> the PDOWN bit and before read/write access to any PHY registers. > >> --- > >> drivers/net/phy/phy.c | 9 ++++++++- > >> 1 file changed, 8 insertions(+), 1 deletion(-) > > > > I must say that I am not overly happy with this patch as it leaks in phy > > specific stuff into a somewhat generic function. Anyway, I see the need > > for this patch and so I applied it. > > Yes, I understand perfectly and I feel the same. > The alternative was to create a custom genphy_restart_aneg for micrel > PHYs only add the delay there, however I am not sure it is worth the > trouble just for a 1ms delay which will be virtually unnoticeable > anyway. There's also the case when Micrel phy support is disabled. In this case a Micrel phy is then handled by the generic phy, so we still need the delay in the generic code, or we would have to remove the BMCR_PDOWN setting from the generic code also. > > Also I am not completely sure that this only applies to Micrel. See > this for example in the SMSC911x driver from the Linux kernel: > http://lxr.free-electrons.com/source/drivers/net/ethernet/smsc/smsc911x.c#L1364 > > Anyway if you would prefer this to be moved to micrel.c I would be > happy to prepare a patch for that; just let me know. Your patch is fine for now. Let's see what unforseen problems it generates before changing it again ;) Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox