linux-kernel - Re: [PATCH v2] ARM: imx: allow to disable board specific PHY fixups

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: <20200331151503.GO25745@shell.armlinux.org.uk>
Date:   Tue, 31 Mar 2020 16:15:03 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     David Jander <david@...tonic.nl>,
        Florian Fainelli <f.fainelli@...il.com>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        netdev@...r.kernel.org, Sascha Hauer <s.hauer@...gutronix.de>,
        linux-kernel@...r.kernel.org, Fabio Estevam <festevam@...il.com>,
        linux-imx@....com, kernel@...gutronix.de,
        Shawn Guo <shawnguo@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH v2] ARM: imx: allow to disable board specific PHY fixups

On Tue, Mar 31, 2020 at 02:54:33PM +0200, Andrew Lunn wrote:
> >  - Disable the SmartEEE feature of the phy. The comment in the code implies
> >    that for some reason it doesn't work, but the reason itself is not given.
> >    Anyway, disabling SmartEEE should IMHO opinion be controlled by a DT
> >    setting. There is no reason to believe this problem is specific to the
> >    i.MX6. Besides, it is a feature of the phy, so it seems logical to expose
> >    that via the DT. Once that is done, it has no place here.
> 
> The device tree properties are defined:
> 
> bindings/net/ethernet-phy.yaml:  eee-broken-100tx:
> bindings/net/ethernet-phy.yaml:  eee-broken-1000t:
> bindings/net/ethernet-phy.yaml:  eee-broken-10gt:
> bindings/net/ethernet-phy.yaml:  eee-broken-1000kx:
> bindings/net/ethernet-phy.yaml:  eee-broken-10gkx4:
> bindings/net/ethernet-phy.yaml:  eee-broken-10gkr:
> 
> And there is a helper:
> 
> void of_set_phy_eee_broken(struct phy_device *phydev)

Disabling the advertisement may solve it, but that is not known.
What the quirk is doing is disabling the SmartEEE feature only
(which is where the PHY handles the EEE so-called "transparently"
to the MAC).

It's all very well waving arms years later and saying we don't
like code that was merged, but unless someone can prove that an
alternative way is better and doesn't regress anything, there
won't be a way forward.

> >  - Enable TXC delay. To clarify, the RGMII specification version 1 specified
> >    that the RXC and TXC traces should be routed long enough to introduce a
> >    certain delay to the clock signal, or the delay should be introduced via
> >    other means. In a later version of the spec, a provision was given for MAC
> >    or PHY devices to generate this delay internally. The i.MX6 MAC interface
> >    is unable to generate the required delay internally, so it has to be taken
> >    care of either by the board layout, or by the PHY device. This is the
> >    crucial point: The amount of delay set by the PHY delay register depends on
> >    the board layout. It should NEVER be hard-coded in SoC setup code. The
> >    correct way is to specify it in the DT. Needless to say that this too,
> >    isn't i.MX6-specific.
> 
> Correct:
> 
>       # RX and TX delays are added by the MAC when required
>       - rgmii
> 
>       # RGMII with internal RX and TX delays provided by the PHY,
>       # the MAC should not add the RX or TX delays in this case
>       - rgmii-id
> 
>       # RGMII with internal RX delay provided by the PHY, the MAC
>       # should not add an RX delay in this case
>       - rgmii-rxid
> 
>       # RGMII with internal TX delay provided by the PHY, the MAC
>       # should not add an TX delay in this case
>       - rgmii-txid
> 
> The needed properties exist.
> 
> I think part of the issue is that there are iMX6 board which are not
> device tree?

I think it's historical - iMX6 never used to be able to enumerate
anything on the MDIO bus, so the only way to configure stuff on the
PHY was via quirks.  That seems to have changed in v3.17-rc1 without
anyone noticing, which happened after the SolidRun support was merged
(v3.14-rc1).  So, not surprisingly, SolidRun platforms don't make use
of the DT properties - quite how one is supposed to know about this
stuff, I've no idea (short of following almost every damn subsystem
mailing list and reading tonnes of email - that's highly impractical.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up