Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net: mdiobus: Prevent spike on MDIO bus reset signal

The mdio_bus reset code first de-asserted the reset by allocating with
GPIOD_OUT_LOW, then asserted and de-asserted again. In other words, if
the reset signal defaulted to asserted, there'd be a short "spike"
before the reset.

Here is what happens depending on the pre-existing state of the reset
signal:
Reset (previously asserted): ~~~|_|~~~~|_______
Reset (previously deasserted): _____|~~~~|_______
^ ^ ^
A B C

At point A, the low going transition is because the reset line is
requested using GPIOD_OUT_LOW. If the line is successfully requested,
the first thing we do is set it high _without_ any delay. This is
point B. So, a glitch occurs between A and B.

We then fsleep() and finally set the GPIO low at point C.

Requesting the line using GPIOD_OUT_HIGH eliminates the A and B
transitions. Instead we get:

Reset (previously asserted) : ~~~~~~~~~~|______
Reset (previously deasserted): ____|~~~~~|______
^ ^
A C

Where A and C are the points described above in the code. Point B
has been eliminated.

The issue was found when we pulled down the reset signal for the
Marvell 88E1512P PHY (because it requires at least 50ms after POR with
an active clock). Looking at the reset signal with a scope revealed a
short spike, point B in the artwork above.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://lore.kernel.org/r/20210202143239.10714-1-mike.looijmans@topic.nl
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Mike Looijmans and committed by
Jakub Kicinski
e0183b97 4160d9ec

+2 -4
+2 -4
drivers/net/phy/mdio_bus.c
··· 543 543 mutex_init(&bus->mdio_lock); 544 544 mutex_init(&bus->shared_lock); 545 545 546 - /* de-assert bus level PHY GPIO reset */ 547 - gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_LOW); 546 + /* assert bus level PHY GPIO reset */ 547 + gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_HIGH); 548 548 if (IS_ERR(gpiod)) { 549 549 err = dev_err_probe(&bus->dev, PTR_ERR(gpiod), 550 550 "mii_bus %s couldn't get reset GPIO\n", ··· 553 553 return err; 554 554 } else if (gpiod) { 555 555 bus->reset_gpiod = gpiod; 556 - 557 - gpiod_set_value_cansleep(gpiod, 1); 558 556 fsleep(bus->reset_delay_us); 559 557 gpiod_set_value_cansleep(gpiod, 0); 560 558 if (bus->reset_post_delay_us > 0)