Hello!
> If you think I should reconsider the patch, you should resubmit it.
I understand this, of course. But, before doing this i'd like to
clarify your concern, why exactly you think that loopback test will
break. Because the (simplified) algorithm is:
do {
result = loopback_test()
if (result == failed)
reset_phy()
} while (result == ok)
So, if loopback test works for the first time, PHY reset will never
be done. So, the conclusion is: in real situation it is never used at all.
Conclusion no 2, coming from my tests: if loopback test fails, phy reset
actually does not fix it. So, perhaps, it's not needed there at all.
I understand, that some other boards might behave differently, and loopback
test was written this very way for some reason. Also, i
understand that you, as a maintainer, have rights for the final decision. And
in order to rework the patch, i'd like to discuss with
you, which rework you would prefer. I came up with three possibilities:
--- cut ---
a) Leave PHY reset inside loopback test as it is, but add a second routine and
call it only before smsc911x_soft_reset().
b) Reset PHY only conditionally, using the following algorithm:
if smsc911x_soft_reset() {
/* NIC reset failed, kick the PHY and retry */
smsc911x_phy_reset()
if (smsc_911x_soft_reset())
return -ENODEV;
}
c) Do extra PHY reset only if some hint in device tree is specified (or the
machine is known to have the problem)
--- cut ---
I can even try to guess your thoughts (because you never elaborated them for
me):
1. loopback test will break because PHY has been reset before. In this case,
(b) or (c) is a way to go.
2. loopback test will break because of reset method change. In this case (a)
is the way to go.
So, what do you really think?
BTW, where is Steve, whose address is specified in MAINTAINERS for this code?
Is it abandonware?
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
