2015-07-08 16:37:35

by Sebastien Rannou

[permalink] [raw]
Subject: Re: [5/6] mvneta: implement SGMII-based in-band link state signaling

Hi Stas,

On Fri, 27 Mar 2015, Stas Sergeev wrote:

> When MDIO bus is unavailable (common setup for SGMII), the in-band
> signaling must be used to correctly track link state.
> This patch enables the in-band status delivery and interrupts for
> links state changes, namely:
> - link up/down
> - link speed
> - duplex full/half
> Upon reciving the appropriate interrupt, the driver updates the
> fixed_phy status to match the received status.

I'm seeing a regression with this patch when trying to netboot an Armada
XP board (by reverting this commit it is fine), the network
link stays down:

[ 9.274492] mvneta d0070000.ethernet eth0: Link is Down

(I've added an extra call to phy_print_status() in mvneta_adjust_link() to
get this trace).

I've tried to dig a bit in the code, and it seems that the status.link flag
never gets set in mvneta_fixed_link_update(). If I try to force the
use_inband_status to 0 in mvneta_probe(), it boots properly so I'm not
sure that I need the in-band status/delivery in my case ; I'm using a
custom DTB with a fixed-link:

eth0: ethernet@70000 {
status = "okay";
fixed-link = <1 1 1000 0 0>;
phy-mode = "sgmii";
};

Could there be something missing in the condition that initializes
pp->use_inband_status?

PS: I've also tried to apply this patch without success:
https://lkml.org/lkml/2015/6/18/496

--
Sébastien


2015-07-08 16:55:49

by Stas Sergeev

[permalink] [raw]
Subject: Re: [5/6] mvneta: implement SGMII-based in-band link state signaling

08.07.2015 19:30, Sebastien Rannou пишет:
> Hi Stas,
>
> On Fri, 27 Mar 2015, Stas Sergeev wrote:
>
>> When MDIO bus is unavailable (common setup for SGMII), the in-band
>> signaling must be used to correctly track link state.
>> This patch enables the in-band status delivery and interrupts for
>> links state changes, namely:
>> - link up/down
>> - link speed
>> - duplex full/half
>> Upon reciving the appropriate interrupt, the driver updates the
>> fixed_phy status to match the received status.
> I'm seeing a regression with this patch when trying to netboot an Armada
> XP board (by reverting this commit it is fine), the network
> link stays down:
>
> [ 9.274492] mvneta d0070000.ethernet eth0: Link is Down
>
> (I've added an extra call to phy_print_status() in mvneta_adjust_link() to
> get this trace).
>
> I've tried to dig a bit in the code, and it seems that the status.link flag
> never gets set in mvneta_fixed_link_update(). If I try to force the
> use_inband_status to 0 in mvneta_probe(), it boots properly so I'm not
> sure that I need the in-band status/delivery in my case ; I'm using a
> custom DTB with a fixed-link:
>
> eth0: ethernet@70000 {
> status = "okay";
> fixed-link = <1 1 1000 0 0>;
> phy-mode = "sgmii";
> };
>
> Could there be something missing in the condition that initializes
> pp->use_inband_status?
Hi, use_inband_status is set when fixed-link is used, which is
exactly your case. But it seems something on the other end
is not generating the inband status. What is there? A phy chip,
or something else?
Perhaps some DT property should be added to explicitly
enable the use of the inband status... I just thought in sgmii
protocol it is a mandatory.
I'll try to come up with the patch tomorrow that adds such
property.

2015-07-09 09:03:34

by Sebastien Rannou

[permalink] [raw]
Subject: Re: [5/6] mvneta: implement SGMII-based in-band link state signaling

On Wed, 8 Jul 2015, Stas Sergeev wrote:

> What is there? A phy chip, or something else?

It's "something else", there's a phy which aggregates 4xSGMIIs to
1xQSGMII, we are on the media side here, the MAC side is connected
to the switch through QSGMII.

> Perhaps some DT property should be added to explicitly
> enable the use of the inband status...

Yes, that would be fine.

> I'll try to come up with the patch tomorrow that adds such
> property.

Thanks! I can help for testing.

--
Sébastien

2015-07-09 09:19:58

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [5/6] mvneta: implement SGMII-based in-band link state signaling

Sebastien, Stas,

On Thu, 9 Jul 2015 11:03:26 +0200 (CEST), Sebastien Rannou wrote:
> On Wed, 8 Jul 2015, Stas Sergeev wrote:
>
> > What is there? A phy chip, or something else?
>
> It's "something else", there's a phy which aggregates 4xSGMIIs to
> 1xQSGMII, we are on the media side here, the MAC side is connected
> to the switch through QSGMII.
>
> > Perhaps some DT property should be added to explicitly
> > enable the use of the inband status...
>
> Yes, that would be fine.

Isn't it a bit weird to need a new DT property for this? Shouldn't
fixed-link always imply this inband status thing?

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

2015-07-09 10:11:27

by Stas Sergeev

[permalink] [raw]
Subject: Re: [5/6] mvneta: implement SGMII-based in-band link state signaling

09.07.2015 12:19, Thomas Petazzoni пишет:
> Sebastien, Stas,
>
> On Thu, 9 Jul 2015 11:03:26 +0200 (CEST), Sebastien Rannou wrote:
>> On Wed, 8 Jul 2015, Stas Sergeev wrote:
>>
>>> What is there? A phy chip, or something else?
>>
>> It's "something else", there's a phy which aggregates 4xSGMIIs to
>> 1xQSGMII, we are on the media side here, the MAC side is connected
>> to the switch through QSGMII.
>>
>>> Perhaps some DT property should be added to explicitly
>>> enable the use of the inband status...
>>
>> Yes, that would be fine.
>
> Isn't it a bit weird to need a new DT property for this? Shouldn't
> fixed-link always imply this inband status thing?
That's how it is currently implemented. I thought its safe.
But what if the device on the other end does not generate the
inband status? I think this device is doing the wrong thing,
but nevertheless we have a regression at hands.

Currently the link status cannot be specified for fixed-link,
at all.
What I am going to code up, is the new property, like this:

fixed-link {
link = "up" | "down" | "auto";
};

"auto" will mean the inband status.
Looks like a simple solution.