Commit 530fe9d4 authored by David S. Miller's avatar David S. Miller

Merge branch 'Improve-MDIO-Ethernet-PHY-reset'

Bruno Thomsen says:

====================
Improve MDIO Ethernet PHY reset

This patch series is a result of trying to upstream a new device
tree for a TQMa7D based board[1][2]. Initial this DTS used some
deprecated PHY reset properties on the FEC device; NXP Ethernet
MAC also known as Freescale Fast Ethernet Controller.

When switching from FEC properties[3]:
"phy-reset-gpios"
"phy-reset-duration"
"phy-reset-post-delay"

To MDIO PHY properties[4]:
"reset-gpios"
"reset-assert-us"
"reset-deassert-us"

The result was that no Ethernet PHY device was detected on boot.

This issue could be worked around by disabling PHY type ID auto-
detection by using "ethernet-phy-id0022.1560" as compatible
string and not "ethernet-phy-ieee802.3-c22".

Upstreaming a DTS with this workaround was not accepted, so I
digged into the MDIO reset flow and found that it had a few
missing parts compared to the deprecated FEC reset function.
After some more testing and logic analyzer traces it was
revealed that the failed PHY communication was due to missing
initial device reset.

I was suggested[5] in a earlier mail thread to use MDIO bus
reset as that was performed before auto-detection, but current
device tree binding was limited to reset assert in usec.
Microchip/Micrel Ethernet PHYs recommended reset circuit[8],
figure 7-12, is a little "slow" after reset deassert as that
is left to a RC circuit with a tau of ~100ms; using a 10k PU
resistor together with a 10uF decoupling capacitor. The diode
in serie of the reset signal converts the GPIO push-pull output
into a open-drain output. So a post reset delay in the range
of 500-1000ms is needed, depending on component tolerances
and general hardware design margins.

In the first version of this patch series[6] I reused the
"reset-delay-us" property for reset deassert in usec as that
would cause 50/50% duty-cycle, but that would always apply.
The solution in this patch series is to add a new MDIO bus
property, so post reset delay is optional and configured
separately.

MDIO bus properties[7]:
"reset-delay-us"
"reset-post-delay-us" (new)

I have not marked this with "Fixes:" as no single commit is the
cause and historically this code has only supported MDIO devices
that need reset after auto-detection. The patch series also uses
a new flexible sleep helper function that was introduced in
5.8-rc1, so the driver uses the optimal sleep function depending
on value loaded from device tree.

Future work in this area could add new properties on the MDIO
device, so reset points are configurable, e.g. no reset,
before/after auto-detection or both.

[1] https://lore.kernel.org/linux-devicetree/20200629114927.17379-2-bruno.thomsen@gmail.com/
[2] https://lore.kernel.org/linux-devicetree/20200716172611.5349-2-bruno.thomsen@gmail.com/
[3] https://elixir.bootlin.com/linux/v5.7.8/source/Documentation/devicetree/bindings/net/fsl-fec.txt#L44
[4] https://elixir.bootlin.com/linux/v5.8-rc4/source/Documentation/devicetree/bindings/net/mdio.yaml#L78
[5] https://lore.kernel.org/netdev/CAOMZO5DtYDomD8FDCZDwYCSr2AwNT81Ay4==aDxXyBxtyvPiJA@mail.gmail.com/
[6] https://lore.kernel.org/netdev/20200728090203.17313-1-bruno.thomsen@gmail.com/
[7] https://elixir.bootlin.com/linux/v5.8-rc4/source/Documentation/devicetree/bindings/net/mdio.yaml#L36
[8] http://ww1.microchip.com/downloads/en/DeviceDoc/00002202C.pdf
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents bd0b33b2 e4d5efdd
......@@ -39,6 +39,13 @@ properties:
and must therefore be appropriately determined based on all devices
requirements (maximum value of all per-device RESET pulse widths).
reset-post-delay-us:
description:
Delay after reset deassert in microseconds. It applies to all MDIO
devices and it's determined by how fast all devices are ready for
communication. This delay happens just before e.g. Ethernet PHY
type ID auto detection.
clock-frequency:
description:
Desired MDIO bus clock frequency in Hz. Values greater than IEEE 802.3
......
......@@ -554,8 +554,10 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
bus->reset_gpiod = gpiod;
gpiod_set_value_cansleep(gpiod, 1);
udelay(bus->reset_delay_us);
fsleep(bus->reset_delay_us);
gpiod_set_value_cansleep(gpiod, 0);
if (bus->reset_post_delay_us > 0)
fsleep(bus->reset_post_delay_us);
}
if (bus->reset) {
......
......@@ -132,7 +132,7 @@ void mdio_device_reset(struct mdio_device *mdiodev, int value)
d = value ? mdiodev->reset_assert_delay : mdiodev->reset_deassert_delay;
if (d)
usleep_range(d, d + max_t(unsigned int, d / 10, 100));
fsleep(d);
}
EXPORT_SYMBOL(mdio_device_reset);
......
......@@ -268,6 +268,8 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
/* Get bus level PHY reset GPIO details */
mdio->reset_delay_us = DEFAULT_GPIO_RESET_DELAY;
of_property_read_u32(np, "reset-delay-us", &mdio->reset_delay_us);
mdio->reset_post_delay_us = 0;
of_property_read_u32(np, "reset-post-delay-us", &mdio->reset_post_delay_us);
/* Register the MDIO bus */
rc = mdiobus_register(mdio);
......
......@@ -293,6 +293,8 @@ struct mii_bus {
/* GPIO reset pulse width in microseconds */
int reset_delay_us;
/* GPIO reset deassert delay in microseconds */
int reset_post_delay_us;
/* RESET GPIO descriptor pointer */
struct gpio_desc *reset_gpiod;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment