netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lucas Stach <l.stach@pengutronix.de>
To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Philippe Schenker <philippe.schenker@toradex.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-imx@nxp.com, kernel@pengutronix.de,
	David Jander <david@protonic.nl>,
	Fabio Estevam <festevam@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 0/7] remove different PHY fixups
Date: Tue, 13 Apr 2021 13:10:47 +0200	[thread overview]
Message-ID: <5277e89e497be121aa7d371a434a3f510fa00e4b.camel@pengutronix.de> (raw)
In-Reply-To: <20210413105144.GN1463@shell.armlinux.org.uk>

Hi Russell,

sorry for the noise of this arriving in your inbox twice. Apparently I
messed up and replied in private in my last mail.

Am Dienstag, dem 13.04.2021 um 11:51 +0100 schrieb Russell King - ARM Linux admin:
> On Tue, Apr 13, 2021 at 12:00:45PM +0200, Lucas Stach wrote:
> > I agree with the opinion that those PHY fixups introduce more harm than
> > good. Essentially they are pushing board specific configuration values
> > into the PHY, without any checks that the fixup is even running on the
> > specific board it was targeted at.
> 
> Yes and no. The problem is, that's an easy statement to make when one
> doesn't understand what they're all doing.
> 
> Some are "board specific" in that the normal setup for e.g. iMX6 would
> be to enable clock output from the AR8035 PHY and feed that into the
> iMX6 - as far as I'm aware, that's the only working configuration for
> that SoC and PHY. However, it's also true that this fixup should not
> be applied unconditionally.
> 
> Then there's SmartEEE - it has been found that the PHY defaults for
> this lead to link drops independent of the board and SoC that it is
> connected to. It seems that the PHY is essentially broken - it powers
> up with SmartEEE enabled, and when connected to another SmartEEE
> supporting device, it seems guaranteed that it will result in link
> drops in its default configuration.
> 
> Freescale's approach has apparently been to unconditionally disable
> SmartEEE for all their platforms because of this. With a bit of
> research however (as has been done by Jon and myself) we've found
> that increasing the Tw parameter for 1G connections results in a
> much more stable link.
> 
> So, just saying that these are bad without actually understanding what
> they are doing is _also_ bad.

I'm not saying the fixups are bad per se. What I'm saying is that they
are inherently board specific and the right way to apply them is either
via DT properties, or if absolutely necessary via a fixup that at least
checks that it is running on the specific board it was targeted at.

While SmartEEE disabling will cause no big harm, aside from a bit more
power consumption, a wrong clock configuration can cause major
confusion. Especially if the configuration in DT and values put into
the PHY via fixups differ from each other.

Regards,
Lucas



      reply	other threads:[~2021-04-13 11:11 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-09 11:26 [PATCH v2 0/7] remove different PHY fixups Oleksij Rempel
2021-03-09 11:26 ` [PATCH v2 1/7] ARM: imx6q: remove PHY fixup for KSZ9031 Oleksij Rempel
2021-05-11  1:49   ` Shawn Guo
2021-03-09 11:26 ` [PATCH v2 2/7] ARM: imx6q: remove TX clock delay of ar8031_phy_fixup() Oleksij Rempel
2021-03-09 11:26 ` [PATCH v2 3/7] ARM: imx6q: remove hand crafted PHY power up in ar8035_phy_fixup() Oleksij Rempel
2021-03-09 11:26 ` [PATCH v2 4/7] ARM: imx6q: remove clk-out fixup for the Atheros AR8031 and AR8035 PHYs Oleksij Rempel
2021-03-09 11:26 ` [PATCH v2 5/7] ARM: imx6q: remove Atheros AR8035 SmartEEE fixup Oleksij Rempel
2021-03-09 11:26 ` [PATCH v2 6/7] ARM: imx6sx: remove Atheros AR8031 PHY fixup Oleksij Rempel
2021-03-09 11:26 ` [PATCH v2 7/7] ARM: imx7d: " Oleksij Rempel
2021-03-24  5:54 ` [PATCH v2 0/7] remove different PHY fixups Oleksij Rempel
2021-03-29  0:44   ` Shawn Guo
2021-03-29  8:13     ` Oleksij Rempel
2021-03-30 14:00 ` Fabio Estevam
2021-03-30 14:30   ` Andrew Lunn
2021-03-30 15:04     ` Fabio Estevam
2021-04-13  9:43       ` Oleksij Rempel
2021-04-13 10:00 ` Lucas Stach
2021-04-13 10:51   ` Russell King - ARM Linux admin
2021-04-13 11:10     ` Lucas Stach [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5277e89e497be121aa7d371a434a3f510fa00e4b.camel@pengutronix.de \
    --to=l.stach@pengutronix.de \
    --cc=andrew@lunn.ch \
    --cc=david@protonic.nl \
    --cc=f.fainelli@gmail.com \
    --cc=festevam@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=philippe.schenker@toradex.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).