All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Fedin <p.fedin@samsung.com>
To: "'David Miller'" <davem@davemloft.net>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	steve.glendinning@shawell.net
Subject: RE: PING: [PATCH] net: smsc911x: Reset PHY during initialization
Date: Wed, 11 Nov 2015 10:05:24 +0300	[thread overview]
Message-ID: <009b01d11c4f$57f6ef10$07e4cd30$@samsung.com> (raw)
In-Reply-To: <20151110.115149.604622279273759756.davem@davemloft.net>

 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



  reply	other threads:[~2015-11-11  7:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-10  6:36 PING: [PATCH] net: smsc911x: Reset PHY during initialization Pavel Fedin
2015-11-10 16:51 ` David Miller
2015-11-11  7:05   ` Pavel Fedin [this message]
2015-11-11 16:14     ` David Miller
2015-11-12  7:04       ` Pavel Fedin
2015-11-12 16:20         ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='009b01d11c4f$57f6ef10$07e4cd30$@samsung.com' \
    --to=p.fedin@samsung.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=steve.glendinning@shawell.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.