All of lore.kernel.org
 help / color / mirror / Atom feed
From: stsp@list.ru (Stas Sergeev)
To: linux-arm-kernel@lists.infradead.org
Subject: mvneta: SGMII fixed-link not so fixed
Date: Fri, 18 Sep 2015 14:29:34 +0300	[thread overview]
Message-ID: <55FBF59E.3010205@list.ru> (raw)
In-Reply-To: <20150917231422.GY21084@n2100.arm.linux.org.uk>

18.09.2015 02:14, Russell King - ARM Linux ?????:
> 3. Having DT specify a fixed-link with parameters along with in-band
>    negotiation results in the fixed-link parameters being ignored.
>    This means if a fixed-link DT declaration specifies a speed, that
>    declaration will be ignored.  What I'm basically saying is that:
> 
> 		phy-mode = "sgmii";
> 		fixed-link {
> 			speed = <1000>;
> 		};
> 
>    specifies a fixed-speed serdes link at 1000mbps, but:
> 
> 		phy-mode = "sgmii";
> 		managed = "in-band-status";
> 		fixed-link {
> 			speed = <1000>;
> 		};
> 
>    does not fix the speed at all.
I think the fixed-link DT parsing should be moved from of_mdio.c
to fixed-phy.c. Then fixed-phy will be able to back up the initial
values and use them whenever needed.
I can even do such a simple patch, but since it is not strictly
speaking a regression from my changes, I'd rather prefer someone
else do. :)


>  _But_ using the in-band status
>    property fundamentally requires this for mvneta to behave correctly:
> 
> 		phy-mode = "sgmii";
> 		managed = "in-band-status";
> 		fixed-link {
> 		};
> 
>    with _no_ phy node.
I don't understand this one.
At least for me it works without empty fixed-link.
There may be some bug.

> 
> 4. Going back to the SFP problem, the link is only up when the SFP
>    module pins indicate that there's no transmitter fault, no loss of
>    signal _and_ the PCS layer at the MAC indicates that it has completed
>    negotiation.  This pretty much rules out trying to emulate a SFP cage
>    as a software-based PHY.  I've code right now doing exactly that, and
>    it results in netif_carrier_on() being called far too early.
> 
> What I don't know is how many generations of the mvneta hardware have
> support for both serdes modes, but what I'm basically saying is that
> the solution we now have seems to be somewhat lacking - maybe it should
> have been "auto", "in-band-sgmii" and "in-band-1000base-x" with the
> ability to add additional modes later.
Well, you need to be quick with such a change, esp now when the patch
was queued to -stable.
What alternatives do we have here?
Will the following work too?
 		phy-mode = "1000base-x";
 		managed = "in-band-status";

WARNING: multiple messages have this Message-ID (diff)
From: Stas Sergeev <stsp@list.ru>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	David Miller <davem@davemloft.net>
Cc: andrew@lunn.ch, linux-arm-kernel@lists.infradead.org,
	netdev@vger.kernel.org
Subject: Re: mvneta: SGMII fixed-link not so fixed
Date: Fri, 18 Sep 2015 14:29:34 +0300	[thread overview]
Message-ID: <55FBF59E.3010205@list.ru> (raw)
In-Reply-To: <20150917231422.GY21084@n2100.arm.linux.org.uk>

18.09.2015 02:14, Russell King - ARM Linux пишет:
> 3. Having DT specify a fixed-link with parameters along with in-band
>    negotiation results in the fixed-link parameters being ignored.
>    This means if a fixed-link DT declaration specifies a speed, that
>    declaration will be ignored.  What I'm basically saying is that:
> 
> 		phy-mode = "sgmii";
> 		fixed-link {
> 			speed = <1000>;
> 		};
> 
>    specifies a fixed-speed serdes link at 1000mbps, but:
> 
> 		phy-mode = "sgmii";
> 		managed = "in-band-status";
> 		fixed-link {
> 			speed = <1000>;
> 		};
> 
>    does not fix the speed at all.
I think the fixed-link DT parsing should be moved from of_mdio.c
to fixed-phy.c. Then fixed-phy will be able to back up the initial
values and use them whenever needed.
I can even do such a simple patch, but since it is not strictly
speaking a regression from my changes, I'd rather prefer someone
else do. :)


>  _But_ using the in-band status
>    property fundamentally requires this for mvneta to behave correctly:
> 
> 		phy-mode = "sgmii";
> 		managed = "in-band-status";
> 		fixed-link {
> 		};
> 
>    with _no_ phy node.
I don't understand this one.
At least for me it works without empty fixed-link.
There may be some bug.

> 
> 4. Going back to the SFP problem, the link is only up when the SFP
>    module pins indicate that there's no transmitter fault, no loss of
>    signal _and_ the PCS layer at the MAC indicates that it has completed
>    negotiation.  This pretty much rules out trying to emulate a SFP cage
>    as a software-based PHY.  I've code right now doing exactly that, and
>    it results in netif_carrier_on() being called far too early.
> 
> What I don't know is how many generations of the mvneta hardware have
> support for both serdes modes, but what I'm basically saying is that
> the solution we now have seems to be somewhat lacking - maybe it should
> have been "auto", "in-band-sgmii" and "in-band-1000base-x" with the
> ability to add additional modes later.
Well, you need to be quick with such a change, esp now when the patch
was queued to -stable.
What alternatives do we have here?
Will the following work too?
 		phy-mode = "1000base-x";
 		managed = "in-band-status";

  parent reply	other threads:[~2015-09-18 11:29 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-14 10:32 mvneta: SGMII fixed-link not so fixed Russell King - ARM Linux
2015-09-14 10:32 ` Russell King - ARM Linux
2015-09-14 11:06 ` Stas Sergeev
2015-09-14 11:06   ` Stas Sergeev
2015-09-14 11:42   ` Russell King - ARM Linux
2015-09-14 11:42     ` Russell King - ARM Linux
2015-09-17 22:12     ` David Miller
2015-09-17 22:12       ` David Miller
2015-09-17 23:02       ` Florian Fainelli
2015-09-17 23:02         ` Florian Fainelli
2015-09-17 23:26         ` David Miller
2015-09-17 23:26           ` David Miller
2015-09-17 23:14       ` Russell King - ARM Linux
2015-09-17 23:14         ` Russell King - ARM Linux
2015-09-17 23:36         ` Florian Fainelli
2015-09-17 23:36           ` Florian Fainelli
2015-09-18  8:14           ` Russell King - ARM Linux
2015-09-18  8:14             ` Russell King - ARM Linux
2015-09-18 11:29         ` Stas Sergeev [this message]
2015-09-18 11:29           ` Stas Sergeev
2015-09-18 12:13           ` Russell King - ARM Linux
2015-09-18 12:13             ` Russell King - ARM Linux
2015-09-18 12:43             ` Stas Sergeev
2015-09-18 12:43               ` Stas Sergeev
2015-09-18 13:12               ` Russell King - ARM Linux
2015-09-18 13:12                 ` Russell King - ARM Linux
2015-09-18 13:43                 ` Stas Sergeev
2015-09-18 13:43                   ` Stas Sergeev
2015-09-18 13:57                   ` Russell King - ARM Linux
2015-09-18 13:57                     ` Russell King - ARM Linux
2015-09-18 14:45                     ` Stas Sergeev
2015-09-18 14:45                       ` Stas Sergeev
2015-09-18 15:43                       ` Russell King - ARM Linux
2015-09-18 15:43                         ` Russell King - ARM Linux
2015-09-18 16:04                         ` Stas Sergeev
2015-09-18 16:04                           ` Stas Sergeev
2015-09-18 17:22                           ` Russell King - ARM Linux
2015-09-18 17:22                             ` Russell King - ARM Linux
2015-09-18 17:30                             ` Florian Fainelli
2015-09-18 17:30                               ` Florian Fainelli
2015-09-18 19:38                               ` Stas Sergeev
2015-09-18 19:38                                 ` Stas Sergeev

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=55FBF59E.3010205@list.ru \
    --to=stsp@list.ru \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.