netdev - Re: fec: micrel: Ethernet PHY type ID auto-detection issue

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200722132608.GX1339445@lunn.ch>
Date:   Wed, 22 Jul 2020 15:26:08 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Bruno Thomsen <bth@...strup.com>
Cc:     Fabio Estevam <festevam@...il.com>,
        netdev <netdev@...r.kernel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Lars Alex Pedersen <laa@...strup.com>,
        "bruno.thomsen@...il.com" <bruno.thomsen@...il.com>
Subject: Re: fec: micrel: Ethernet PHY type ID auto-detection issue

Hi Bruno

Is it held in reset, and the reset is released, or is the reset line
toggled active and then inactive?

> > void mdio_device_reset(struct mdio_device *mdiodev, int value)
> > {
> > 	unsigned int d;
> > 
> > 	......
> > 
> > 	d = value ? mdiodev->reset_assert_delay : mdiodev->reset_deassert_delay;
> > 	if (d)
> > 		usleep_range(d, d + max_t(unsigned int, d / 10, 100));
> 
> This is not the recommended way of sleeping if d > 20ms.
> 
> https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
> 
> The deprecated fec code handles this correctly.
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/freescale/fec_main.c#L3381
> 
> if (d)
> 	if (d > 20000)
> 		msleep(d / 1000);
> 	else
> 		usleep_range(d, d + max_t(unsigned int, d / 10, 100));

Patch welcome.

> Micrel recommended reset circuit has a deassert tau of 100ms, e.g. 10k * 10uF.
> So to be sure the signal is deasserted 5 * tau or more, and this brings the value
> up in the 500-1000ms range depending on component tolerances and design
> margin.
> 
> See figure 22 in pdf for reset circuit.
> http://ww1.microchip.com/downloads/en/devicedoc/ksz8081mnx-rnb.pdf
> 
> So my current conclusion is that using generic mdio phy handling does
> not work with Micrel PHYs unless 3 issues has been resolved.
> - Reset PHY before auto type detection.

This has been raised recently. Look back in the mail archive about a
month. For GPIOs this is easier to solve. But regulators pose a
problem.

Part of the problem is the history of this code. It originated from a
PHY which needed to be reset after probe and configuration to make its
clock stable, if i remember correctly. So the PHY would already probe,
without the reset. Something similar was needed for another PHY so the
code got pulled out of the driver and into the PHY core. But the
assumption remained, the PHY will probe, the reset is used after the
probe. This code now needs to be made more generic.

There is one other option, depending on your board design. The PHY
core supports two different resets. There is a per PHY reset, which is
what you are using. And then there is a reset for all devices on the
bus. That is used when multiple PHYs are connected to one reset GPIO.
You might be able to use that reset instead. But you might need to fix
up the sleep code in that case as well.

	Andrew