lkml.org 
[lkml]   [2021]   [Jan]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] net: mdiobus: Prevent spike on MDIO bus reset signal
From
Date
Hi Andrew,

Response below...


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topicproducts.com
W: www.topicproducts.com

Please consider the environment before printing this e-mail
On 28-01-2021 02:56, Andrew Lunn wrote:
> On Tue, Jan 26, 2021 at 08:33:37AM +0100, Mike Looijmans wrote:
>> 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.
>>
>> Instead, directly assert the reset signal using GPIOD_OUT_HIGH, this
>> removes the spike and also removes a line of code since the signal
>> is already high.
>
> Hi Mike
>
> Did you look at the per PHY reset? mdiobus_register_gpiod() gets the
> GPIO with GPIOD_OUT_LOW. mdiobus_register_device() then immediately
> sets it high.
>
> So it looks like it suffers from the same problem.

Well, now that I have your attention...

The per PHY reset was more broken, it first probes the MDIO bus to see if the
PHY is there, and only after that it controls the reset line. So if the reset
defaults to "asserted", the PHY will not work because it didn't respond when
the MDIO went looking for it. I haven't quite figured out how this was
supposed to work, but at least for the case of one MDIO bus, one PHY
configured through devicetree it didn't work as one would expect. I added a
few printk statements to reveal that this was indeed the case.

This issue also makes the PHY hardware reset useless - if the PHY is in some
non-responsive state, the MDIO won't get a response and report the PHY as
missing before even attempting to assert/de-assert the reset line.

This was with a 5.4 kernel, but as far as I could see this hasn't changed
since then.

My solution to that was to move to the MDIO bus reset, since that at least
happened before interrogating the devices on the bus. This revealed the issue
with the extra "spike" while evaluating that, which is something that I could
easily solve and upstream.

Probably these issues were never dicovered because usually there's a pull-up
of some kind on the (active-low) reset signal of the PHYs. That hides the
spike and also hides the fact that the per-phy reset doesn't actually work. We
only discovered the issue when we changed that to a pull-down and suddenly the
phy failed to probe.

The way that the MDIO bus is being populated seemed rather complex to me, so
chances of breaking things are quite high there...

\
 
 \ /
  Last update: 2021-01-28 09:49    [W:0.141 / U:0.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site