netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Mack <zonque@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, marek.belisko@gmail.com, ujhelyi.m@gmail.com
Subject: Re: [PATCH 1/2] net: phylib: add adjust_state callback to phy device
Date: Fri, 15 Nov 2013 08:35:00 +0100	[thread overview]
Message-ID: <5285CEA4.70207@gmail.com> (raw)
In-Reply-To: <20131114.163207.230147902479371528.davem@davemloft.net>

On 11/14/2013 10:32 PM, David Miller wrote:
> From: Daniel Mack <zonque@gmail.com>
> Date: Wed, 13 Nov 2013 22:07:49 +0100
> 
>> Allow phy drivers to take action when the core does its link adjustment.
>> No change for drivers that do not implement this callback.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
> 
> So you're using this to reset the entire PHY via the reset bit in the
> BMCR register when the link goes down.
> 
> But this is going to break things.
> 
> If the phy library previously programmed a non-autonegotiated static
> link configuration into the BMCR register, your reset is going to
> undo that.
> 
> Now the configuration phylib thinks the chip has and the one it
> acutally does is out of sync.

You're right, thanks for the review. Let me just state that I'm really
unhappy about that approach as well.

However, I currently see no better way than resetting the PHY every time
the link goes down, as we have no way of telling whether the chip has
entered its locked-up state which it seemingly does arbitrarily when the
link changes. This is really not nice, but the only thing that avoided
the effect in our tests.

That means that we either have to tell the PHY core about what we did,
or manually preserve the registers contents across the reset. I'll have
another look next week. Any pointers appreciated :)


Daniel

      reply	other threads:[~2013-11-15  7:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-13 21:07 [PATCH 1/2] net: phylib: add adjust_state callback to phy device Daniel Mack
2013-11-13 21:07 ` [PATCH 2/2] net: phy: at803x: soft-reset PHY when link goes down Daniel Mack
2013-11-15  0:03   ` Sergei Shtylyov
2013-11-14 21:32 ` [PATCH 1/2] net: phylib: add adjust_state callback to phy device David Miller
2013-11-15  7:35   ` Daniel Mack [this message]

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=5285CEA4.70207@gmail.com \
    --to=zonque@gmail.com \
    --cc=davem@davemloft.net \
    --cc=marek.belisko@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=ujhelyi.m@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).