Re: [PATCH] net: mdiobus: Prevent spike on MDIO bus reset signal

On Wed, 27 Jan 2021 08:08:29 +0100 Mike Looijmans wrote:

quoted

Andrew, I don't get what you're saying.

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.

Therefore, to me the patch looks entirely reasonable and correct.
 

Thanks, excellent explanation.

As a bit of background, we were using a Marvell PHY where the datasheet 
states that thou shallt not release the reset within 50 ms of power-up. 
A pull-down on the active-low reset was thus added. Looking at the reset 
signal with a scope revealed a short spike, visible only because it was 
being controlled by an I2C GPIO expander. So it's indeed point "B" that 
we wanted to eliminate.

This is all useful information - can we roll more of it into the commit
message? I'd think that calling out the part and the 50ms value could
make things more "concrete" for a reader down the line?