> Gesendet: Samstag, 22. Oktober 2022 um 19:05 Uhr
> Von: "Russell King (Oracle)" <[email protected]>
> On Sat, Oct 22, 2022 at 12:52:00PM +0200, Frank Wunderlich wrote:
> > > Gesendet: Samstag, 22. Oktober 2022 um 11:11 Uhr
> > > Von: "Russell King (Oracle)" <[email protected]>
> > this patch breaks connectivity at least on the sfp-port (eth1).
> > pcs_get_state
> > [ 65.522936] offset:0 0x2c1140
> > [ 65.522950] offset:4 0x4d544950
> > [ 65.525914] offset:8 0x40e041a0
> > [ 177.346183] offset:0 0x2c1140
> > [ 177.346202] offset:4 0x4d544950
> > [ 177.349168] offset:8 0x40e041a0
> > [ 177.352477] offset:0 0x2c1140
> > [ 177.356952] offset:4 0x4d544950
>
> Hi,
>
> Thanks. Well, the results suggest that the register at offset 8 is
> indeed the advertisement and link-partner advertisement register. So
> we have a bit of progress and a little more understanding of this
> hardware.
>
> Do you know if your link partner also thinks the link is up?
yes link is up on my switch, cannot enable autoneg for fibre-port, so port is fixed to 1000M/full flowcontrol enabled.
> What I notice is:
>
> mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Unknown - flow control off
>
> The duplex is "unknown" which means you're not filling in the
> state->duplex field in your pcs_get_state() function. Given the
> link parter adverisement is 0x00e0, this means the link partner
> supports PAUSE, 1000base-X/Half and 1000base-X/Full. The resolution
> is therefore full duplex, so can we hack that in to your
> pcs_get_state() so we're getting that right for this testing please?
0xe0 is bits 5-7 are set (in lower byte from upper word)..which one is for duplex?
so i should set state->duplex/pause based on this value (maybe compare with own caps)?
found a documentation where 5=full,6=half, and bits 7+8 are for pause (symetric/asymetric)
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1+8, &val);
partner_advertising = (val & 0x00ff0000) >> 16;
if (partner_advertising & BIT(5)) state->duplex = DUPLEX_FULL;
else if (partner_advertising & BIT(6)) state->duplex = DUPLEX_HALF;
if (partner_advertising & BIT(7)) state->pause = MAC_SYM_PAUSE;
else if (partner_advertising & BIT(8)) state->pause = MAC_ASYM_PAUSE;
> Now, I'm wondering what SGMII_IF_MODE_BIT0 and SGMII_IF_MODE_BIT5 do
> in the SGMSYS_SGMII_MODE register. Does one of these bits set the
> format for the 16-bit control word that's used to convey the
> advertisements. I think the next step would be to play around with
> these and see what effect setting or clearing these bits has -
> please can you give that a go?
these is not clear to me...should i blindly set these and how to verify what they do?
is network broken because of wrong duplex/pause setting? do not fully understand your Patch.
But the timer-change can also break sgmii...
regards Frank
Hi,
On Sat, Oct 22, 2022 at 07:53:16PM +0200, Frank Wunderlich wrote:
> > Gesendet: Samstag, 22. Oktober 2022 um 19:05 Uhr
> > Von: "Russell King (Oracle)" <[email protected]>
> > On Sat, Oct 22, 2022 at 12:52:00PM +0200, Frank Wunderlich wrote:
> > > > Gesendet: Samstag, 22. Oktober 2022 um 11:11 Uhr
> > > > Von: "Russell King (Oracle)" <[email protected]>
>
> > > this patch breaks connectivity at least on the sfp-port (eth1).
>
> > > pcs_get_state
> > > [ 65.522936] offset:0 0x2c1140
> > > [ 65.522950] offset:4 0x4d544950
> > > [ 65.525914] offset:8 0x40e041a0
> > > [ 177.346183] offset:0 0x2c1140
> > > [ 177.346202] offset:4 0x4d544950
> > > [ 177.349168] offset:8 0x40e041a0
> > > [ 177.352477] offset:0 0x2c1140
> > > [ 177.356952] offset:4 0x4d544950
> >
> > Hi,
> >
> > Thanks. Well, the results suggest that the register at offset 8 is
> > indeed the advertisement and link-partner advertisement register. So
> > we have a bit of progress and a little more understanding of this
> > hardware.
> >
> > Do you know if your link partner also thinks the link is up?
>
> yes link is up on my switch, cannot enable autoneg for fibre-port, so port is fixed to 1000M/full flowcontrol enabled.
>
> > What I notice is:
> >
> > mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Unknown - flow control off
> >
> > The duplex is "unknown" which means you're not filling in the
> > state->duplex field in your pcs_get_state() function. Given the
> > link parter adverisement is 0x00e0, this means the link partner
> > supports PAUSE, 1000base-X/Half and 1000base-X/Full. The resolution
> > is therefore full duplex, so can we hack that in to your
> > pcs_get_state() so we're getting that right for this testing please?
>
> 0xe0 is bits 5-7 are set (in lower byte from upper word)..which one is for duplex?
>
> so i should set state->duplex/pause based on this value (maybe compare with own caps)?
>
> found a documentation where 5=full,6=half, and bits 7+8 are for pause (symetric/asymetric)
>
> regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1+8, &val);
> partner_advertising = (val & 0x00ff0000) >> 16;
Not quite :) When we have the link partner's advertisement and the BMSR,
we have a helper function in phylink to do all the gritty work:
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
will do all the work for you without having to care about whether
you're operating at 2500base-X, 1000base-X or SGMII mode.
> > Now, I'm wondering what SGMII_IF_MODE_BIT0 and SGMII_IF_MODE_BIT5 do
> > in the SGMSYS_SGMII_MODE register. Does one of these bits set the
> > format for the 16-bit control word that's used to convey the
> > advertisements. I think the next step would be to play around with
> > these and see what effect setting or clearing these bits has -
> > please can you give that a go?
>
> these is not clear to me...should i blindly set these and how to
> verify what they do?
Yes please - I don't think anyone knows what they do.
> is network broken because of wrong duplex/pause setting? do not
> fully understand your Patch.
I suspect not having the duplex correct _could_ break stuff, but I
also wonder whether the PCS is trying to decode the advertisements
itself and coming out with the wrong settings.
If it's interpreting a link partner advertisement of 0x00e0 using
SGMII rules, then it will be looking at bits 11 and 10 for the
speed, both of which are zero, which means 10Mbps - and 1000base-X
doesn't operate at 10Mbps!
So my hunch is that one of those two IF_MODE_BIT{0,5} _might_ change
the way the PCS interprets the control word, but as we don't have
any documentation to go on, only experimentation will answer this
question.
If these registers are MMIO, you could ensure that you have /dev/mem
access enabled, and use devmem2 to poke at this register which would
probably be quicker than doing a build-boot-test cycle with the
kernel - this is how I do a lot of this kind of discovery when
documentation is lacking.
> But the timer-change can also break sgmii...
SGMII mode should be writing the same value to the link timer, but
looking at it now, I see I ended up with one too many zeros on the
16000000! It should be 1.6ms in nanoseconds, so 1600000. Please
correct for future testing.
Many thanks for your patience.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> Gesendet: Samstag, 22. Oktober 2022 um 21:18 Uhr
> Von: "Russell King (Oracle)" <[email protected]>
> An: "Frank Wunderlich" <[email protected]>
> Cc: "Frank Wunderlich" <[email protected]>, [email protected], "Alexander Couzens" <[email protected]>, "Felix Fietkau" <[email protected]>, "John Crispin" <[email protected]>, "Sean Wang" <[email protected]>, "Mark Lee" <[email protected]>, "David S. Miller" <[email protected]>, "Eric Dumazet" <[email protected]>, "Jakub Kicinski" <[email protected]>, "Paolo Abeni" <[email protected]>, "Matthias Brugger" <[email protected]>, [email protected], [email protected], [email protected]
> Betreff: Re: Re: Re: Re: Re: [PATCH v2] net: mtk_sgmii: implement mtk_pcs_ops
>
> Hi,
>
> On Sat, Oct 22, 2022 at 07:53:16PM +0200, Frank Wunderlich wrote:
> > > Gesendet: Samstag, 22. Oktober 2022 um 19:05 Uhr
> > > Von: "Russell King (Oracle)" <[email protected]>
> > > On Sat, Oct 22, 2022 at 12:52:00PM +0200, Frank Wunderlich wrote:
> > > > > Gesendet: Samstag, 22. Oktober 2022 um 11:11 Uhr
> > > > > Von: "Russell King (Oracle)" <[email protected]>
> >
> > > > this patch breaks connectivity at least on the sfp-port (eth1).
> >
> > > > pcs_get_state
> > > > [ 65.522936] offset:0 0x2c1140
> > > > [ 65.522950] offset:4 0x4d544950
> > > > [ 65.525914] offset:8 0x40e041a0
> > > > [ 177.346183] offset:0 0x2c1140
> > > > [ 177.346202] offset:4 0x4d544950
> > > > [ 177.349168] offset:8 0x40e041a0
> > > > [ 177.352477] offset:0 0x2c1140
> > > > [ 177.356952] offset:4 0x4d544950
> > >
> > > Hi,
> > >
> > > Thanks. Well, the results suggest that the register at offset 8 is
> > > indeed the advertisement and link-partner advertisement register. So
> > > we have a bit of progress and a little more understanding of this
> > > hardware.
> > >
> > > Do you know if your link partner also thinks the link is up?
> >
> > yes link is up on my switch, cannot enable autoneg for fibre-port, so port is fixed to 1000M/full flowcontrol enabled.
> >
> > > What I notice is:
> > >
> > > mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Unknown - flow control off
> > >
> > > The duplex is "unknown" which means you're not filling in the
> > > state->duplex field in your pcs_get_state() function. Given the
> > > link parter adverisement is 0x00e0, this means the link partner
> > > supports PAUSE, 1000base-X/Half and 1000base-X/Full. The resolution
> > > is therefore full duplex, so can we hack that in to your
> > > pcs_get_state() so we're getting that right for this testing please?
> >
> > 0xe0 is bits 5-7 are set (in lower byte from upper word)..which one is for duplex?
> >
> > so i should set state->duplex/pause based on this value (maybe compare with own caps)?
> >
> > found a documentation where 5=full,6=half, and bits 7+8 are for pause (symetric/asymetric)
> >
> > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1+8, &val);
> > partner_advertising = (val & 0x00ff0000) >> 16;
>
> Not quite :) When we have the link partner's advertisement and the BMSR,
> we have a helper function in phylink to do all the gritty work:
>
> regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
> regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
>
> phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
>
> will do all the work for you without having to care about whether
> you're operating at 2500base-X, 1000base-X or SGMII mode.
>
> > > Now, I'm wondering what SGMII_IF_MODE_BIT0 and SGMII_IF_MODE_BIT5 do
> > > in the SGMSYS_SGMII_MODE register. Does one of these bits set the
> > > format for the 16-bit control word that's used to convey the
> > > advertisements. I think the next step would be to play around with
> > > these and see what effect setting or clearing these bits has -
> > > please can you give that a go?
> >
> > these is not clear to me...should i blindly set these and how to
> > verify what they do?
>
> Yes please - I don't think anyone knows what they do.
i guess BIT0 is the SGMII_EN flag like in other sgmii implementations.
Bit5 is "reserved" in all docs i've found....maybe it is related to HSGMII
or for 1G vs. 2G5.
but how to check what has changed...i guess only the register itself changed
and i have to readout another to check whats changed.
do we really need these 2 bits? reading/setting duplex/pause from/to the register
makes sense, but digging into undocumented bits is much work and we still only guess.
so i would first want to get sgmii working again and then strip the pause/duplex from it.
> > is network broken because of wrong duplex/pause setting? do not
> > fully understand your Patch.
>
> I suspect not having the duplex correct _could_ break stuff, but I
> also wonder whether the PCS is trying to decode the advertisements
> itself and coming out with the wrong settings.
>
> If it's interpreting a link partner advertisement of 0x00e0 using
> SGMII rules, then it will be looking at bits 11 and 10 for the
> speed, both of which are zero, which means 10Mbps - and 1000base-X
> doesn't operate at 10Mbps!
so maybe this breaks sgmii? have you changed this behaviour with your Patch?
> So my hunch is that one of those two IF_MODE_BIT{0,5} _might_ change
> the way the PCS interprets the control word, but as we don't have
> any documentation to go on, only experimentation will answer this
> question.
>
> If these registers are MMIO, you could ensure that you have /dev/mem
> access enabled, and use devmem2 to poke at this register which would
> probably be quicker than doing a build-boot-test cycle with the
> kernel - this is how I do a lot of this kind of discovery when
> documentation is lacking.
>
> > But the timer-change can also break sgmii...
>
> SGMII mode should be writing the same value to the link timer, but
> looking at it now, I see I ended up with one too many zeros on the
> 16000000! It should be 1.6ms in nanoseconds, so 1600000. Please
> correct for future testing.
tried removing 1 zero from the 16000000, but same result.
tried also setting duplex with ethtool, but after read it is still unknown.
and i get no traffic working...i wonder because duplex was not set before your
patch too, but interface was working.
> Many thanks for your patience.
i do what i can, but i'm limited in time.
Frank
On Sun, Oct 23, 2022 at 09:26:39AM +0200, Frank Wunderlich wrote:
> > Gesendet: Samstag, 22. Oktober 2022 um 21:18 Uhr
> > Von: "Russell King (Oracle)" <[email protected]>
> > An: "Frank Wunderlich" <[email protected]>
> > Cc: "Frank Wunderlich" <[email protected]>, [email protected], "Alexander Couzens" <[email protected]>, "Felix Fietkau" <[email protected]>, "John Crispin" <[email protected]>, "Sean Wang" <[email protected]>, "Mark Lee" <[email protected]>, "David S. Miller" <[email protected]>, "Eric Dumazet" <[email protected]>, "Jakub Kicinski" <[email protected]>, "Paolo Abeni" <[email protected]>, "Matthias Brugger" <[email protected]>, [email protected], [email protected], [email protected]
> > Betreff: Re: Re: Re: Re: Re: [PATCH v2] net: mtk_sgmii: implement mtk_pcs_ops
> >
> > Hi,
> >
> > On Sat, Oct 22, 2022 at 07:53:16PM +0200, Frank Wunderlich wrote:
> > > > Gesendet: Samstag, 22. Oktober 2022 um 19:05 Uhr
> > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > On Sat, Oct 22, 2022 at 12:52:00PM +0200, Frank Wunderlich wrote:
> > > > > > Gesendet: Samstag, 22. Oktober 2022 um 11:11 Uhr
> > > > > > Von: "Russell King (Oracle)" <[email protected]>
> > >
> > > > > this patch breaks connectivity at least on the sfp-port (eth1).
> > >
> > > > > pcs_get_state
> > > > > [ 65.522936] offset:0 0x2c1140
> > > > > [ 65.522950] offset:4 0x4d544950
> > > > > [ 65.525914] offset:8 0x40e041a0
> > > > > [ 177.346183] offset:0 0x2c1140
> > > > > [ 177.346202] offset:4 0x4d544950
> > > > > [ 177.349168] offset:8 0x40e041a0
> > > > > [ 177.352477] offset:0 0x2c1140
> > > > > [ 177.356952] offset:4 0x4d544950
> > > >
> > > > Hi,
> > > >
> > > > Thanks. Well, the results suggest that the register at offset 8 is
> > > > indeed the advertisement and link-partner advertisement register. So
> > > > we have a bit of progress and a little more understanding of this
> > > > hardware.
> > > >
> > > > Do you know if your link partner also thinks the link is up?
> > >
> > > yes link is up on my switch, cannot enable autoneg for fibre-port, so port is fixed to 1000M/full flowcontrol enabled.
> > >
> > > > What I notice is:
> > > >
> > > > mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Unknown - flow control off
> > > >
> > > > The duplex is "unknown" which means you're not filling in the
> > > > state->duplex field in your pcs_get_state() function. Given the
> > > > link parter adverisement is 0x00e0, this means the link partner
> > > > supports PAUSE, 1000base-X/Half and 1000base-X/Full. The resolution
> > > > is therefore full duplex, so can we hack that in to your
> > > > pcs_get_state() so we're getting that right for this testing please?
> > >
> > > 0xe0 is bits 5-7 are set (in lower byte from upper word)..which one is for duplex?
> > >
> > > so i should set state->duplex/pause based on this value (maybe compare with own caps)?
> > >
> > > found a documentation where 5=full,6=half, and bits 7+8 are for pause (symetric/asymetric)
> > >
> > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1+8, &val);
> > > partner_advertising = (val & 0x00ff0000) >> 16;
> >
> > Not quite :) When we have the link partner's advertisement and the BMSR,
> > we have a helper function in phylink to do all the gritty work:
> >
> > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
> > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
> >
> > phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
> >
> > will do all the work for you without having to care about whether
> > you're operating at 2500base-X, 1000base-X or SGMII mode.
> >
> > > > Now, I'm wondering what SGMII_IF_MODE_BIT0 and SGMII_IF_MODE_BIT5 do
> > > > in the SGMSYS_SGMII_MODE register. Does one of these bits set the
> > > > format for the 16-bit control word that's used to convey the
> > > > advertisements. I think the next step would be to play around with
> > > > these and see what effect setting or clearing these bits has -
> > > > please can you give that a go?
> > >
> > > these is not clear to me...should i blindly set these and how to
> > > verify what they do?
> >
> > Yes please - I don't think anyone knows what they do.
>
> i guess BIT0 is the SGMII_EN flag like in other sgmii implementations.
> Bit5 is "reserved" in all docs i've found....maybe it is related to HSGMII
> or for 1G vs. 2G5.
"other sgmii implementations" ?
If this is the SGMII_EN flag, maybe SGMII_IF_MODE_BIT0 should be
renamed to SGMII_IF_SGMII_EN ? Maybe it needs to be set for SGMII
and clear for base-X ?
> but how to check what has changed...i guess only the register itself changed
> and i have to readout another to check whats changed.
>
> do we really need these 2 bits? reading/setting duplex/pause from/to the register
> makes sense, but digging into undocumented bits is much work and we still only guess.
I don't know - I've no idea about this hardware, or what the PCS is,
and other people over the years I've talked to have said "we're not
using it, we can't help". The mediatek driver has been somewhat of a
pain for phylink as a result.
> so i would first want to get sgmii working again and then strip the pause/duplex from it.
I think I'd need more information on your setup - is this dev 0? Are
you using in-band mode or fixed-link mode?
I don't think you've updated me with register values for this since
the patch. With the link timer adjusted back to 1.6ms, that should
result in it working again, but if not, I think there's some
possibilities.
The addition of SGMII_AN_ENABLE for SGMSYS_PCS_CONTROL_1 could have
broken your setup if there is no actual in-band signalling, which
basically means that your firmware description is wrong - and you've
possibly been led astray by the poor driver implementation.
Can you confirm that the device on the other end for dev 0 does in
actual fact use in-band signalling please?
> > If it's interpreting a link partner advertisement of 0x00e0 using
> > SGMII rules, then it will be looking at bits 11 and 10 for the
> > speed, both of which are zero, which means 10Mbps - and 1000base-X
> > doesn't operate at 10Mbps!
>
> so maybe this breaks sgmii? have you changed this behaviour with your Patch?
Nope, but not setting the duplex properly is yet another buggy and poor
quality of implementation that afficts this driver. I've said about
setting the duplex value when reviewing your patch to add .pcs_get_state
and I'm disappointed that you seemingly haven't yet corrected it in the
code you're testing despite those review comments.
If duplex remains as "unknown", then the MAC will be programmed to
operate in _half_ _duplex_ mode (read mtk_mac_link_up()) which is not
what you likely want. Many MACs don't support half duplex at 1G speed,
so it's likely that without setting state->duplex, the result is that
the MAC hardware is programmed incorrectly.
> > So my hunch is that one of those two IF_MODE_BIT{0,5} _might_ change
> > the way the PCS interprets the control word, but as we don't have
> > any documentation to go on, only experimentation will answer this
> > question.
> >
> > If these registers are MMIO, you could ensure that you have /dev/mem
> > access enabled, and use devmem2 to poke at this register which would
> > probably be quicker than doing a build-boot-test cycle with the
> > kernel - this is how I do a lot of this kind of discovery when
> > documentation is lacking.
> >
> > > But the timer-change can also break sgmii...
> >
> > SGMII mode should be writing the same value to the link timer, but
> > looking at it now, I see I ended up with one too many zeros on the
> > 16000000! It should be 1.6ms in nanoseconds, so 1600000. Please
> > correct for future testing.
>
> tried removing 1 zero from the 16000000, but same result.
> tried also setting duplex with ethtool, but after read it is still unknown.
Honestly this doesn't surprise me given the poor state of the mtk_sgmii
code. There's lots that this implementation gets wrong, but I can't fix
it without either people willing to research and test stuff, or without
the actual hardware in front of me.
This mtk_eth_soc driver has been a right pain for me ever since the
half-hearted switch-over to use phylink.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> Gesendet: Sonntag, 23. Oktober 2022 um 11:43 Uhr
> Von: "Russell King (Oracle)" <[email protected]>
> On Sun, Oct 23, 2022 at 09:26:39AM +0200, Frank Wunderlich wrote:
> > > Gesendet: Samstag, 22. Oktober 2022 um 21:18 Uhr
> > > Von: "Russell King (Oracle)" <[email protected]>
> > > Hi,
> > >
> > > On Sat, Oct 22, 2022 at 07:53:16PM +0200, Frank Wunderlich wrote:
> > > > > Gesendet: Samstag, 22. Oktober 2022 um 19:05 Uhr
> > > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > > On Sat, Oct 22, 2022 at 12:52:00PM +0200, Frank Wunderlich wrote:
> > > > > > > Gesendet: Samstag, 22. Oktober 2022 um 11:11 Uhr
> > > > > > > Von: "Russell King (Oracle)" <[email protected]>
> > > >
> > > > > > this patch breaks connectivity at least on the sfp-port (eth1).
> > > >
> > > > > > pcs_get_state
> > > > > > [ 65.522936] offset:0 0x2c1140
> > > > > > [ 65.522950] offset:4 0x4d544950
> > > > > > [ 65.525914] offset:8 0x40e041a0
> > > > > > [ 177.346183] offset:0 0x2c1140
> > > > > > [ 177.346202] offset:4 0x4d544950
> > > > > > [ 177.349168] offset:8 0x40e041a0
> > > > > > [ 177.352477] offset:0 0x2c1140
> > > > > > [ 177.356952] offset:4 0x4d544950
> > > > >
> > > > > Hi,
> > > > >
> > > > > Thanks. Well, the results suggest that the register at offset 8 is
> > > > > indeed the advertisement and link-partner advertisement register. So
> > > > > we have a bit of progress and a little more understanding of this
> > > > > hardware.
> > > > >
> > > > > Do you know if your link partner also thinks the link is up?
> > > >
> > > > yes link is up on my switch, cannot enable autoneg for fibre-port, so port is fixed to 1000M/full flowcontrol enabled.
> > > >
> > > > > What I notice is:
> > > > >
> > > > > mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Unknown - flow control off
> > > > >
> > > > > The duplex is "unknown" which means you're not filling in the
> > > > > state->duplex field in your pcs_get_state() function. Given the
> > > > > link parter adverisement is 0x00e0, this means the link partner
> > > > > supports PAUSE, 1000base-X/Half and 1000base-X/Full. The resolution
> > > > > is therefore full duplex, so can we hack that in to your
> > > > > pcs_get_state() so we're getting that right for this testing please?
> > > >
> > > > 0xe0 is bits 5-7 are set (in lower byte from upper word)..which one is for duplex?
> > > >
> > > > so i should set state->duplex/pause based on this value (maybe compare with own caps)?
> > > >
> > > > found a documentation where 5=full,6=half, and bits 7+8 are for pause (symetric/asymetric)
> > > >
> > > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1+8, &val);
> > > > partner_advertising = (val & 0x00ff0000) >> 16;
> > >
> > > Not quite :) When we have the link partner's advertisement and the BMSR,
> > > we have a helper function in phylink to do all the gritty work:
> > >
> > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
> > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
> > >
> > > phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
> > >
> > > will do all the work for you without having to care about whether
> > > you're operating at 2500base-X, 1000base-X or SGMII mode.
> > >
> > > > > Now, I'm wondering what SGMII_IF_MODE_BIT0 and SGMII_IF_MODE_BIT5 do
> > > > > in the SGMSYS_SGMII_MODE register. Does one of these bits set the
> > > > > format for the 16-bit control word that's used to convey the
> > > > > advertisements. I think the next step would be to play around with
> > > > > these and see what effect setting or clearing these bits has -
> > > > > please can you give that a go?
> > > >
> > > > these is not clear to me...should i blindly set these and how to
> > > > verify what they do?
> > >
> > > Yes please - I don't think anyone knows what they do.
> >
> > i guess BIT0 is the SGMII_EN flag like in other sgmii implementations.
> > Bit5 is "reserved" in all docs i've found....maybe it is related to HSGMII
> > or for 1G vs. 2G5.
>
> "other sgmii implementations" ?
yes i googled for sgmii and found register definition for different vendor...
i don't know if sgmii is a standard for each vendor, afair trgmii was not.
> If this is the SGMII_EN flag, maybe SGMII_IF_MODE_BIT0 should be
> renamed to SGMII_IF_SGMII_EN ? Maybe it needs to be set for SGMII
> and clear for base-X ?
>
> > but how to check what has changed...i guess only the register itself changed
> > and i have to readout another to check whats changed.
> >
> > do we really need these 2 bits? reading/setting duplex/pause from/to the register
> > makes sense, but digging into undocumented bits is much work and we still only guess.
>
> I don't know - I've no idea about this hardware, or what the PCS is,
> and other people over the years I've talked to have said "we're not
> using it, we can't help". The mediatek driver has been somewhat of a
> pain for phylink as a result.
>
> > so i would first want to get sgmii working again and then strip the pause/duplex from it.
>
> I think I'd need more information on your setup - is this dev 0? Are
> you using in-band mode or fixed-link mode?
i only test with dev1 which is the sfp-port/eth1/gmac1...dev0 is the fixed-link to switch-chip.
> I don't think you've updated me with register values for this since
> the patch. With the link timer adjusted back to 1.6ms, that should
> result in it working again, but if not, I think there's some
> possibilities.
sorry for that, have debugged timing and it was wrong because if-condition had not included 1000baseX and 2500baseX. only sgmii
> The addition of SGMII_AN_ENABLE for SGMSYS_PCS_CONTROL_1 could have
> broken your setup if there is no actual in-band signalling, which
> basically means that your firmware description is wrong - and you've
> possibly been led astray by the poor driver implementation.
disabled it, but makes no change.
but i've noticed that timing is wrong
old value: 0x186a0
new value: 0x98968
so it takes the 10000000 and not the 1600000. so it looks like interface-mode is not (yet) SGMII.
debugged it and got 1000baseX (21),in dts i have
phy-mode = "2500base-x";
but SFP only supports 1G so mode 1000baseX is right
set the old value with your calculation, but still not working, also with disabled AN_ENABLE-flag ;(
root@bpi-r3:~# ip link set eth1 up
[ 44.287442] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/1000be
[ 44.295902] interface-mode 21 (sgmii:4)
root@bpi-r3:~# [ 44.295907] timer 0x186a0
[ 44.352872] offset:0 0x2c1140
[ 44.355507] offset:4 0x4d544950
[ 44.358462] offset:8 0x40e041a0
[ 44.361609] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Full - flf
[ 44.373042] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
root@bpi-r3:~# ip a a 192.168.0.19/24 dev eth1
root@bpi-r3:~# ping 192.168.0.10
PING 192.168.0.10 (192.168.0.10) 56(84) bytes of data.
^C
> Can you confirm that the device on the other end for dev 0 does in
> actual fact use in-band signalling please?
>
> > > If it's interpreting a link partner advertisement of 0x00e0 using
> > > SGMII rules, then it will be looking at bits 11 and 10 for the
> > > speed, both of which are zero, which means 10Mbps - and 1000base-X
> > > doesn't operate at 10Mbps!
> >
> > so maybe this breaks sgmii? have you changed this behaviour with your Patch?
>
> Nope, but not setting the duplex properly is yet another buggy and poor
> quality of implementation that afficts this driver. I've said about
> setting the duplex value when reviewing your patch to add .pcs_get_state
> and I'm disappointed that you seemingly haven't yet corrected it in the
> code you're testing despite those review comments.
sorry, i thought we want to read out the values from registers to set it based on them.
currently i test only with the dev 1 (in-band-managed with 1GBit/s SFP)
[ 1.088310] dev: 0 offset:0 0x40140
[ 1.088331] dev: 0 offset:4 0x4d544950
[ 1.091827] dev: 0 offset:8 0x1
[ 1.095607] dev: 1 offset:0 0x81140
[ 1.098739] dev: 1 offset:4 0x4d544950
[ 1.102214] dev: 1 offset:8 0x1
after bring device up (disabled AN and set duplex to full):
[ 34.615926] timer 0x98968
[ 34.672888] offset:0 0x2c1140
[ 34.675518] offset:4 0x4d544950
[ 34.678473] offset:8 0x40e041a0
codebase:
https://github.com/frank-w/BPI-R2-4.14/commits/6.1-r3-sgmii
> If duplex remains as "unknown", then the MAC will be programmed to
> operate in _half_ _duplex_ mode (read mtk_mac_link_up()) which is not
> what you likely want. Many MACs don't support half duplex at 1G speed,
> so it's likely that without setting state->duplex, the result is that
> the MAC hardware is programmed incorrectly.
wonder why it was working with only my patch which had duplex also not set.
> > > So my hunch is that one of those two IF_MODE_BIT{0,5} _might_ change
> > > the way the PCS interprets the control word, but as we don't have
> > > any documentation to go on, only experimentation will answer this
> > > question.
the bits were in offset 0/4/8? are they now different than before?
if yes maybe these break it.
as offset 4 is the phy-id and 8 is the advertisement from local and far
interface i guesss IF_MODE_* is in offset 0.
> > > If these registers are MMIO, you could ensure that you have /dev/mem
> > > access enabled, and use devmem2 to poke at this register which would
> > > probably be quicker than doing a build-boot-test cycle with the
> > > kernel - this is how I do a lot of this kind of discovery when
> > > documentation is lacking.
> > >
> > > > But the timer-change can also break sgmii...
currently this is no more the case as i set the timer to old value
so something else is breaking it.
> > > SGMII mode should be writing the same value to the link timer, but
> > > looking at it now, I see I ended up with one too many zeros on the
> > > 16000000! It should be 1.6ms in nanoseconds, so 1600000. Please
> > > correct for future testing.
> >
> > tried removing 1 zero from the 16000000, but same result.
> > tried also setting duplex with ethtool, but after read it is still unknown.
>
> Honestly this doesn't surprise me given the poor state of the mtk_sgmii
> code. There's lots that this implementation gets wrong, but I can't fix
> it without either people willing to research and test stuff, or without
> the actual hardware in front of me.
i can test, but i do not fully understand the code nor have the exoerience
to work with devmem2. have used it long time ago but with assistance to read
which register and with expected values.
> This mtk_eth_soc driver has been a right pain for me ever since the
> half-hearted switch-over to use phylink.
i know it is huge as it covers many different SoC. but i'm not able to rewrite it :p
regards Frank
On Sun, Oct 23, 2022 at 05:05:30PM +0200, Frank Wunderlich wrote:
> > Gesendet: Sonntag, 23. Oktober 2022 um 11:43 Uhr
> > Von: "Russell King (Oracle)" <[email protected]>
>
> > On Sun, Oct 23, 2022 at 09:26:39AM +0200, Frank Wunderlich wrote:
> > > > Gesendet: Samstag, 22. Oktober 2022 um 21:18 Uhr
> > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > Hi,
> > > >
> > > > On Sat, Oct 22, 2022 at 07:53:16PM +0200, Frank Wunderlich wrote:
> > > > > > Gesendet: Samstag, 22. Oktober 2022 um 19:05 Uhr
> > > > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > > > On Sat, Oct 22, 2022 at 12:52:00PM +0200, Frank Wunderlich wrote:
> > > > > > > > Gesendet: Samstag, 22. Oktober 2022 um 11:11 Uhr
> > > > > > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > >
> > > > > > > this patch breaks connectivity at least on the sfp-port (eth1).
> > > > >
> > > > > > > pcs_get_state
> > > > > > > [ 65.522936] offset:0 0x2c1140
> > > > > > > [ 65.522950] offset:4 0x4d544950
> > > > > > > [ 65.525914] offset:8 0x40e041a0
> > > > > > > [ 177.346183] offset:0 0x2c1140
> > > > > > > [ 177.346202] offset:4 0x4d544950
> > > > > > > [ 177.349168] offset:8 0x40e041a0
> > > > > > > [ 177.352477] offset:0 0x2c1140
> > > > > > > [ 177.356952] offset:4 0x4d544950
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Thanks. Well, the results suggest that the register at offset 8 is
> > > > > > indeed the advertisement and link-partner advertisement register. So
> > > > > > we have a bit of progress and a little more understanding of this
> > > > > > hardware.
> > > > > >
> > > > > > Do you know if your link partner also thinks the link is up?
> > > > >
> > > > > yes link is up on my switch, cannot enable autoneg for fibre-port, so port is fixed to 1000M/full flowcontrol enabled.
> > > > >
> > > > > > What I notice is:
> > > > > >
> > > > > > mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Unknown - flow control off
> > > > > >
> > > > > > The duplex is "unknown" which means you're not filling in the
> > > > > > state->duplex field in your pcs_get_state() function. Given the
> > > > > > link parter adverisement is 0x00e0, this means the link partner
> > > > > > supports PAUSE, 1000base-X/Half and 1000base-X/Full. The resolution
> > > > > > is therefore full duplex, so can we hack that in to your
> > > > > > pcs_get_state() so we're getting that right for this testing please?
> > > > >
> > > > > 0xe0 is bits 5-7 are set (in lower byte from upper word)..which one is for duplex?
> > > > >
> > > > > so i should set state->duplex/pause based on this value (maybe compare with own caps)?
> > > > >
> > > > > found a documentation where 5=full,6=half, and bits 7+8 are for pause (symetric/asymetric)
> > > > >
> > > > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1+8, &val);
> > > > > partner_advertising = (val & 0x00ff0000) >> 16;
> > > >
> > > > Not quite :) When we have the link partner's advertisement and the BMSR,
> > > > we have a helper function in phylink to do all the gritty work:
> > > >
> > > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
> > > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
> > > >
> > > > phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
> > > >
> > > > will do all the work for you without having to care about whether
> > > > you're operating at 2500base-X, 1000base-X or SGMII mode.
> > > >
> > > > > > Now, I'm wondering what SGMII_IF_MODE_BIT0 and SGMII_IF_MODE_BIT5 do
> > > > > > in the SGMSYS_SGMII_MODE register. Does one of these bits set the
> > > > > > format for the 16-bit control word that's used to convey the
> > > > > > advertisements. I think the next step would be to play around with
> > > > > > these and see what effect setting or clearing these bits has -
> > > > > > please can you give that a go?
> > > > >
> > > > > these is not clear to me...should i blindly set these and how to
> > > > > verify what they do?
> > > >
> > > > Yes please - I don't think anyone knows what they do.
> > >
> > > i guess BIT0 is the SGMII_EN flag like in other sgmii implementations.
> > > Bit5 is "reserved" in all docs i've found....maybe it is related to HSGMII
> > > or for 1G vs. 2G5.
> >
> > "other sgmii implementations" ?
>
> yes i googled for sgmii and found register definition for different vendor...
> i don't know if sgmii is a standard for each vendor, afair trgmii was not.
>
> > If this is the SGMII_EN flag, maybe SGMII_IF_MODE_BIT0 should be
> > renamed to SGMII_IF_SGMII_EN ? Maybe it needs to be set for SGMII
> > and clear for base-X ?
> >
> > > but how to check what has changed...i guess only the register itself changed
> > > and i have to readout another to check whats changed.
> > >
> > > do we really need these 2 bits? reading/setting duplex/pause from/to the register
> > > makes sense, but digging into undocumented bits is much work and we still only guess.
> >
> > I don't know - I've no idea about this hardware, or what the PCS is,
> > and other people over the years I've talked to have said "we're not
> > using it, we can't help". The mediatek driver has been somewhat of a
> > pain for phylink as a result.
> >
> > > so i would first want to get sgmii working again and then strip the pause/duplex from it.
> >
> > I think I'd need more information on your setup - is this dev 0? Are
> > you using in-band mode or fixed-link mode?
>
> i only test with dev1 which is the sfp-port/eth1/gmac1...dev0 is the fixed-link to switch-chip.
Okay, so when you're using SGMII, how are you testing it? With a copper
SFP plugged in?
> > I don't think you've updated me with register values for this since
> > the patch. With the link timer adjusted back to 1.6ms, that should
> > result in it working again, but if not, I think there's some
> > possibilities.
>
> sorry for that, have debugged timing and it was wrong because if-
> condition had not included 1000baseX and 2500baseX. only sgmii
SGMII's link timer is specified to be 1.6ms - the SGMII v1.8 spec
doesn't specify the margins for this.
802.3z (1000base-X) is 10ms +10ms -0s.
This is what we should be using, and what I tried to implement.
The hex values programmed into the register should be 0x186A0 for
SGMII and 0x98968 for 1000base-X and 2500base-X - both values
should fit because the link timer is apparently 20 bits wide.
> > The addition of SGMII_AN_ENABLE for SGMSYS_PCS_CONTROL_1 could have
> > broken your setup if there is no actual in-band signalling, which
> > basically means that your firmware description is wrong - and you've
> > possibly been led astray by the poor driver implementation.
>
> disabled it, but makes no change.
>
> but i've noticed that timing is wrong
>
> old value: 0x186a0
> new value: 0x98968
>
> so it takes the 10000000 and not the 1600000. so it looks like interface-mode is not (yet) SGMII.
>
> debugged it and got 1000baseX (21),in dts i have
> phy-mode = "2500base-x";
> but SFP only supports 1G so mode 1000baseX is right
>
> set the old value with your calculation, but still not working, also with disabled AN_ENABLE-flag ;(
I'm getting the impression that there's some confusing terminology going
on here... can we clear this up please?
SGMII is a proprietary modification of the 802.3z 1000base-X standard
which:
- reduces the link timer from 10ms to 1.6ms
- implements data replication by 10x and 100x to achieve 100M and 10M
speeds over a link operating at a fixed speed of 1.25Gbaud.
- changes the control word format to allow a SGMII PHY to signal to the
MAC in a timely manner which speed it is operating at.
So, if you're using a fibre SFP to another device that is operating in
1000base-X mode, then you're wanting 1000base-X and not SGMII, and
referring to this as SGMII is technically misleading.
> root@bpi-r3:~# ip link set eth1 up
> [ 44.287442] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/1000be
> [ 44.295902] interface-mode 21 (sgmii:4)
> root@bpi-r3:~# [ 44.295907] timer 0x186a0
> [ 44.352872] offset:0 0x2c1140
> [ 44.355507] offset:4 0x4d544950
> [ 44.358462] offset:8 0x40e041a0
> [ 44.361609] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Full - flf
> [ 44.373042] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
>
> root@bpi-r3:~# ip a a 192.168.0.19/24 dev eth1
> root@bpi-r3:~# ping 192.168.0.10
> PING 192.168.0.10 (192.168.0.10) 56(84) bytes of data.
> ^C
This is where I postulated that the PCS is trying to interpret the
advertisements as if they are SGMII formatted control words rather than
1000base-X formatted control words - and by doing so, it is trying to
operate at 10Mbps (100x data replication) with the remote end trying to
operate at 1000Mbps. If that is what it is doing, then you will have
link-up but no communication.
The solution to this is likely trying to find a bit that tells the
PCS whether it should be expecting a 1000base-X (or 802.3z) formatted
control word (aka 1000base-X mode) or a SGMII formatted control word.
You mentioned that bit 0 in SGMSYS_SGMII_MODE is a "SGMII_EN" bit.
Any ideas exactly what this bit does? Does it enable the PCS as a
whole, or could that be the bit which switches between 1000base-X
mode and SGMII mode? (More on this below).
Note that the "old way" used to work because even in 1000base-X
mode, the code would (technically incorrectly) force the PCS to
use a fixed configuration of 1000Mbps and force the duplex bit -
basically no 802.3 specified autonegotiation.
However, 1000base-X with in-band signalling _should_ be using
the autonegotiation - as everything else that uses phylink does.
> > Can you confirm that the device on the other end for dev 0 does in
> > actual fact use in-band signalling please?
> >
> > > > If it's interpreting a link partner advertisement of 0x00e0 using
> > > > SGMII rules, then it will be looking at bits 11 and 10 for the
> > > > speed, both of which are zero, which means 10Mbps - and 1000base-X
> > > > doesn't operate at 10Mbps!
> > >
> > > so maybe this breaks sgmii? have you changed this behaviour with your Patch?
> >
> > Nope, but not setting the duplex properly is yet another buggy and poor
> > quality of implementation that afficts this driver. I've said about
> > setting the duplex value when reviewing your patch to add .pcs_get_state
> > and I'm disappointed that you seemingly haven't yet corrected it in the
> > code you're testing despite those review comments.
>
> sorry, i thought we want to read out the values from registers to set it based on them.
>
> currently i test only with the dev 1 (in-band-managed with 1GBit/s SFP)
>
> [ 1.088310] dev: 0 offset:0 0x40140
> [ 1.088331] dev: 0 offset:4 0x4d544950
> [ 1.091827] dev: 0 offset:8 0x1
> [ 1.095607] dev: 1 offset:0 0x81140
> [ 1.098739] dev: 1 offset:4 0x4d544950
> [ 1.102214] dev: 1 offset:8 0x1
>
> after bring device up (disabled AN and set duplex to full):
>
> [ 34.615926] timer 0x98968
> [ 34.672888] offset:0 0x2c1140
> [ 34.675518] offset:4 0x4d544950
> [ 34.678473] offset:8 0x40e041a0
>
> codebase:
>
> https://github.com/frank-w/BPI-R2-4.14/commits/6.1-r3-sgmii
I think it would also be useful to print the register at offset 32
as well, which is the SGMSYS_SGMII_MODE register, so we can discover
what the initial and current values of these IF_MODE_BITs are. I
may then be able to provide you an updated patch.
> > If duplex remains as "unknown", then the MAC will be programmed to
> > operate in _half_ _duplex_ mode (read mtk_mac_link_up()) which is not
> > what you likely want. Many MACs don't support half duplex at 1G speed,
> > so it's likely that without setting state->duplex, the result is that
> > the MAC hardware is programmed incorrectly.
>
> wonder why it was working with only my patch which had duplex also not set.
It depends entirely on the MAC implementation and why the manufacturer
decides to state that 1000 half-duplex isn't supported by the hardware!
I don't think we can guess. However, configuring the hardware correctly
eliminates potential issues.
It is in entirely possible for devices configured with dissimilar
duplex settings to communicate, but there will be packet loss - since
the end operating in full duplex will transmit while the receiving
end could also be transmitting, and the receving end could interpret
that as a collision.
> > > > So my hunch is that one of those two IF_MODE_BIT{0,5} _might_ change
> > > > the way the PCS interprets the control word, but as we don't have
> > > > any documentation to go on, only experimentation will answer this
> > > > question.
>
> the bits were in offset 0/4/8? are they now different than before?
> if yes maybe these break it.
>
> as offset 4 is the phy-id and 8 is the advertisement from local and far
> interface i guesss IF_MODE_* is in offset 0.
They're in the register at offset 32:
/* Register to control remote fault */
#define SGMSYS_SGMII_MODE 0x20
#define SGMII_IF_MODE_BIT0 BIT(0)
...
#define SGMII_IF_MODE_BIT5 BIT(5)
So, I think the first step should be to print the value of this register
along side the other three you've been providing me and update me with
its value. I'll then provide you a replacement patch to try.
Russell.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> Gesendet: Sonntag, 23. Oktober 2022 um 17:46 Uhr
> Von: "Russell King (Oracle)" <[email protected]>
> On Sun, Oct 23, 2022 at 05:05:30PM +0200, Frank Wunderlich wrote:
> > > Gesendet: Sonntag, 23. Oktober 2022 um 11:43 Uhr
> > > Von: "Russell King (Oracle)" <[email protected]>
> >
> > > On Sun, Oct 23, 2022 at 09:26:39AM +0200, Frank Wunderlich wrote:
> > > > > Gesendet: Samstag, 22. Oktober 2022 um 21:18 Uhr
> > > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > > Hi,
> > > > >
> > > > > On Sat, Oct 22, 2022 at 07:53:16PM +0200, Frank Wunderlich wrote:
> > > > > > > Gesendet: Samstag, 22. Oktober 2022 um 19:05 Uhr
> > > > > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > > > > On Sat, Oct 22, 2022 at 12:52:00PM +0200, Frank Wunderlich wrote:
> > > > > > > > > Gesendet: Samstag, 22. Oktober 2022 um 11:11 Uhr
> > > > > > > > > Von: "Russell King (Oracle)" <[email protected]>
> > > > > >
> > > > > > > > this patch breaks connectivity at least on the sfp-port (eth1).
> > > > > >
> > > > > > > > pcs_get_state
> > > > > > > > [ 65.522936] offset:0 0x2c1140
> > > > > > > > [ 65.522950] offset:4 0x4d544950
> > > > > > > > [ 65.525914] offset:8 0x40e041a0
> > > > > > > > [ 177.346183] offset:0 0x2c1140
> > > > > > > > [ 177.346202] offset:4 0x4d544950
> > > > > > > > [ 177.349168] offset:8 0x40e041a0
> > > > > > > > [ 177.352477] offset:0 0x2c1140
> > > > > > > > [ 177.356952] offset:4 0x4d544950
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > Thanks. Well, the results suggest that the register at offset 8 is
> > > > > > > indeed the advertisement and link-partner advertisement register. So
> > > > > > > we have a bit of progress and a little more understanding of this
> > > > > > > hardware.
> > > > > > >
> > > > > > > Do you know if your link partner also thinks the link is up?
> > > > > >
> > > > > > yes link is up on my switch, cannot enable autoneg for fibre-port, so port is fixed to 1000M/full flowcontrol enabled.
> > > > > >
> > > > > > > What I notice is:
> > > > > > >
> > > > > > > mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Unknown - flow control off
> > > > > > >
> > > > > > > The duplex is "unknown" which means you're not filling in the
> > > > > > > state->duplex field in your pcs_get_state() function. Given the
> > > > > > > link parter adverisement is 0x00e0, this means the link partner
> > > > > > > supports PAUSE, 1000base-X/Half and 1000base-X/Full. The resolution
> > > > > > > is therefore full duplex, so can we hack that in to your
> > > > > > > pcs_get_state() so we're getting that right for this testing please?
> > > > > >
> > > > > > 0xe0 is bits 5-7 are set (in lower byte from upper word)..which one is for duplex?
> > > > > >
> > > > > > so i should set state->duplex/pause based on this value (maybe compare with own caps)?
> > > > > >
> > > > > > found a documentation where 5=full,6=half, and bits 7+8 are for pause (symetric/asymetric)
> > > > > >
> > > > > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1+8, &val);
> > > > > > partner_advertising = (val & 0x00ff0000) >> 16;
> > > > >
> > > > > Not quite :) When we have the link partner's advertisement and the BMSR,
> > > > > we have a helper function in phylink to do all the gritty work:
> > > > >
> > > > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
> > > > > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
> > > > >
> > > > > phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
> > > > >
> > > > > will do all the work for you without having to care about whether
> > > > > you're operating at 2500base-X, 1000base-X or SGMII mode.
> > > > >
> > > > > > > Now, I'm wondering what SGMII_IF_MODE_BIT0 and SGMII_IF_MODE_BIT5 do
> > > > > > > in the SGMSYS_SGMII_MODE register. Does one of these bits set the
> > > > > > > format for the 16-bit control word that's used to convey the
> > > > > > > advertisements. I think the next step would be to play around with
> > > > > > > these and see what effect setting or clearing these bits has -
> > > > > > > please can you give that a go?
> > > > > >
> > > > > > these is not clear to me...should i blindly set these and how to
> > > > > > verify what they do?
> > > > >
> > > > > Yes please - I don't think anyone knows what they do.
> > > >
> > > > i guess BIT0 is the SGMII_EN flag like in other sgmii implementations.
> > > > Bit5 is "reserved" in all docs i've found....maybe it is related to HSGMII
> > > > or for 1G vs. 2G5.
> > >
> > > "other sgmii implementations" ?
> >
> > yes i googled for sgmii and found register definition for different vendor...
> > i don't know if sgmii is a standard for each vendor, afair trgmii was not.
> >
> > > If this is the SGMII_EN flag, maybe SGMII_IF_MODE_BIT0 should be
> > > renamed to SGMII_IF_SGMII_EN ? Maybe it needs to be set for SGMII
> > > and clear for base-X ?
> > >
> > > > but how to check what has changed...i guess only the register itself changed
> > > > and i have to readout another to check whats changed.
> > > >
> > > > do we really need these 2 bits? reading/setting duplex/pause from/to the register
> > > > makes sense, but digging into undocumented bits is much work and we still only guess.
> > >
> > > I don't know - I've no idea about this hardware, or what the PCS is,
> > > and other people over the years I've talked to have said "we're not
> > > using it, we can't help". The mediatek driver has been somewhat of a
> > > pain for phylink as a result.
> > >
> > > > so i would first want to get sgmii working again and then strip the pause/duplex from it.
> > >
> > > I think I'd need more information on your setup - is this dev 0? Are
> > > you using in-band mode or fixed-link mode?
> >
> > i only test with dev1 which is the sfp-port/eth1/gmac1...dev0 is the fixed-link to switch-chip.
>
> Okay, so when you're using SGMII, how are you testing it? With a copper
> SFP plugged in?
>
> > > I don't think you've updated me with register values for this since
> > > the patch. With the link timer adjusted back to 1.6ms, that should
> > > result in it working again, but if not, I think there's some
> > > possibilities.
> >
> > sorry for that, have debugged timing and it was wrong because if-
> > condition had not included 1000baseX and 2500baseX. only sgmii
>
> SGMII's link timer is specified to be 1.6ms - the SGMII v1.8 spec
> doesn't specify the margins for this.
>
> 802.3z (1000base-X) is 10ms +10ms -0s.
>
> This is what we should be using, and what I tried to implement.
>
> The hex values programmed into the register should be 0x186A0 for
> SGMII and 0x98968 for 1000base-X and 2500base-X - both values
> should fit because the link timer is apparently 20 bits wide.
>
> > > The addition of SGMII_AN_ENABLE for SGMSYS_PCS_CONTROL_1 could have
> > > broken your setup if there is no actual in-band signalling, which
> > > basically means that your firmware description is wrong - and you've
> > > possibly been led astray by the poor driver implementation.
> >
> > disabled it, but makes no change.
> >
> > but i've noticed that timing is wrong
> >
> > old value: 0x186a0
> > new value: 0x98968
> >
> > so it takes the 10000000 and not the 1600000. so it looks like interface-mode is not (yet) SGMII.
> >
> > debugged it and got 1000baseX (21),in dts i have
> > phy-mode = "2500base-x";
> > but SFP only supports 1G so mode 1000baseX is right
> >
> > set the old value with your calculation, but still not working, also with disabled AN_ENABLE-flag ;(
>
> I'm getting the impression that there's some confusing terminology going
> on here... can we clear this up please?
>
> SGMII is a proprietary modification of the 802.3z 1000base-X standard
> which:
> - reduces the link timer from 10ms to 1.6ms
> - implements data replication by 10x and 100x to achieve 100M and 10M
> speeds over a link operating at a fixed speed of 1.25Gbaud.
> - changes the control word format to allow a SGMII PHY to signal to the
> MAC in a timely manner which speed it is operating at.
>
> So, if you're using a fibre SFP to another device that is operating in
> 1000base-X mode, then you're wanting 1000base-X and not SGMII, and
> referring to this as SGMII is technically misleading.
>
> > root@bpi-r3:~# ip link set eth1 up
> > [ 44.287442] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/1000be
> > [ 44.295902] interface-mode 21 (sgmii:4)
> > root@bpi-r3:~# [ 44.295907] timer 0x186a0
> > [ 44.352872] offset:0 0x2c1140
> > [ 44.355507] offset:4 0x4d544950
> > [ 44.358462] offset:8 0x40e041a0
> > [ 44.361609] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Full - flf
> > [ 44.373042] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
> >
> > root@bpi-r3:~# ip a a 192.168.0.19/24 dev eth1
> > root@bpi-r3:~# ping 192.168.0.10
> > PING 192.168.0.10 (192.168.0.10) 56(84) bytes of data.
> > ^C
>
> This is where I postulated that the PCS is trying to interpret the
> advertisements as if they are SGMII formatted control words rather than
> 1000base-X formatted control words - and by doing so, it is trying to
> operate at 10Mbps (100x data replication) with the remote end trying to
> operate at 1000Mbps. If that is what it is doing, then you will have
> link-up but no communication.
>
> The solution to this is likely trying to find a bit that tells the
> PCS whether it should be expecting a 1000base-X (or 802.3z) formatted
> control word (aka 1000base-X mode) or a SGMII formatted control word.
>
> You mentioned that bit 0 in SGMSYS_SGMII_MODE is a "SGMII_EN" bit.
> Any ideas exactly what this bit does? Does it enable the PCS as a
> whole, or could that be the bit which switches between 1000base-X
> mode and SGMII mode? (More on this below).
>
> Note that the "old way" used to work because even in 1000base-X
> mode, the code would (technically incorrectly) force the PCS to
> use a fixed configuration of 1000Mbps and force the duplex bit -
> basically no 802.3 specified autonegotiation.
>
> However, 1000base-X with in-band signalling _should_ be using
> the autonegotiation - as everything else that uses phylink does.
>
> > > Can you confirm that the device on the other end for dev 0 does in
> > > actual fact use in-band signalling please?
> > >
> > > > > If it's interpreting a link partner advertisement of 0x00e0 using
> > > > > SGMII rules, then it will be looking at bits 11 and 10 for the
> > > > > speed, both of which are zero, which means 10Mbps - and 1000base-X
> > > > > doesn't operate at 10Mbps!
> > > >
> > > > so maybe this breaks sgmii? have you changed this behaviour with your Patch?
> > >
> > > Nope, but not setting the duplex properly is yet another buggy and poor
> > > quality of implementation that afficts this driver. I've said about
> > > setting the duplex value when reviewing your patch to add .pcs_get_state
> > > and I'm disappointed that you seemingly haven't yet corrected it in the
> > > code you're testing despite those review comments.
> >
> > sorry, i thought we want to read out the values from registers to set it based on them.
> >
> > currently i test only with the dev 1 (in-band-managed with 1GBit/s SFP)
> >
> > [ 1.088310] dev: 0 offset:0 0x40140
> > [ 1.088331] dev: 0 offset:4 0x4d544950
> > [ 1.091827] dev: 0 offset:8 0x1
> > [ 1.095607] dev: 1 offset:0 0x81140
> > [ 1.098739] dev: 1 offset:4 0x4d544950
> > [ 1.102214] dev: 1 offset:8 0x1
> >
> > after bring device up (disabled AN and set duplex to full):
> >
> > [ 34.615926] timer 0x98968
> > [ 34.672888] offset:0 0x2c1140
> > [ 34.675518] offset:4 0x4d544950
> > [ 34.678473] offset:8 0x40e041a0
> >
> > codebase:
> >
> > https://github.com/frank-w/BPI-R2-4.14/commits/6.1-r3-sgmii
>
> I think it would also be useful to print the register at offset 32
> as well, which is the SGMSYS_SGMII_MODE register, so we can discover
> what the initial and current values of these IF_MODE_BITs are. I
> may then be able to provide you an updated patch.
>
> > > If duplex remains as "unknown", then the MAC will be programmed to
> > > operate in _half_ _duplex_ mode (read mtk_mac_link_up()) which is not
> > > what you likely want. Many MACs don't support half duplex at 1G speed,
> > > so it's likely that without setting state->duplex, the result is that
> > > the MAC hardware is programmed incorrectly.
> >
> > wonder why it was working with only my patch which had duplex also not set.
>
> It depends entirely on the MAC implementation and why the manufacturer
> decides to state that 1000 half-duplex isn't supported by the hardware!
> I don't think we can guess. However, configuring the hardware correctly
> eliminates potential issues.
>
> It is in entirely possible for devices configured with dissimilar
> duplex settings to communicate, but there will be packet loss - since
> the end operating in full duplex will transmit while the receiving
> end could also be transmitting, and the receving end could interpret
> that as a collision.
>
> > > > > So my hunch is that one of those two IF_MODE_BIT{0,5} _might_ change
> > > > > the way the PCS interprets the control word, but as we don't have
> > > > > any documentation to go on, only experimentation will answer this
> > > > > question.
> >
> > the bits were in offset 0/4/8? are they now different than before?
> > if yes maybe these break it.
> >
> > as offset 4 is the phy-id and 8 is the advertisement from local and far
> > interface i guesss IF_MODE_* is in offset 0.
>
> They're in the register at offset 32:
>
> /* Register to control remote fault */
> #define SGMSYS_SGMII_MODE 0x20
> #define SGMII_IF_MODE_BIT0 BIT(0)
> ...
> #define SGMII_IF_MODE_BIT5 BIT(5)
>
> So, I think the first step should be to print the value of this register
> along side the other three you've been providing me and update me with
> its value. I'll then provide you a replacement patch to try.
here it is (AN again enabled, changes pushed to tree above):
bootup:
[ 1.098876] dev: 1 offset:0 0x81140
[ 1.102699] dev: 1 offset:4 0x4d544950
[ 1.106180] dev: 1 offset:8 0x1
[ 1.109914] dev: 1 offset:32 0x3112001b
after putting eth1 up:
[ 32.566099] timer 0x186a0
[ 32.623021] offset:0 0x2c1140
[ 32.625653] offset:4 0x4d544950
[ 32.628614] offset:8 0x40e041a0
[ 32.631746] offset:32 0x3112011b
regards Frank
On Sun, Oct 23, 2022 at 06:41:45PM +0200, Frank Wunderlich wrote:
> bootup:
>
> [ 1.098876] dev: 1 offset:0 0x81140
> [ 1.102699] dev: 1 offset:4 0x4d544950
> [ 1.106180] dev: 1 offset:8 0x1
> [ 1.109914] dev: 1 offset:32 0x3112001b
>
> after putting eth1 up:
>
> [ 32.566099] timer 0x186a0
> [ 32.623021] offset:0 0x2c1140
> [ 32.625653] offset:4 0x4d544950
> [ 32.628614] offset:8 0x40e041a0
> [ 32.631746] offset:32 0x3112011b
Hi Frank,
Based on this, could you give the following patch a try - it replaces
my previous patch.
Thanks.
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index b52f3b0177ef..1a3eb3ecf7e3 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -479,7 +479,7 @@
/* Register to programmable link timer, the unit in 2 * 8ns */
#define SGMSYS_PCS_LINK_TIMER 0x18
-#define SGMII_LINK_TIMER_DEFAULT (0x186a0 & GENMASK(19, 0))
+#define SGMII_LINK_TIMER_MASK GENMASK(19, 0)
/* Register to control remote fault */
#define SGMSYS_SGMII_MODE 0x20
diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c
index 736839c84130..63736c52bab2 100644
--- a/drivers/net/ethernet/mediatek/mtk_sgmii.c
+++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c
@@ -20,19 +20,40 @@ static struct mtk_pcs *pcs_to_mtk_pcs(struct phylink_pcs *pcs)
}
/* For SGMII interface mode */
-static int mtk_pcs_setup_mode_an(struct mtk_pcs *mpcs)
+static int mtk_pcs_setup_mode_an(struct mtk_pcs *mpcs,
+ phy_interface_t interface,
+ const unsigned long *advertising)
{
- unsigned int val;
+ unsigned int val, link_timer;
+ int advertise;
+ bool changed;
+
+ advertise = phylink_mii_c22_pcs_encode_advertisement(interface,
+ advertising);
+ if (advertise < 0)
+ return advertise;
+
+ if (interface == PHY_INTERFACE_MODE_SGMII)
+ link_timer = 1600000 / 2 / 8;
+ else
+ link_timer = 10000000 / 2 / 8;
/* Setup the link timer and QPHY power up inside SGMIISYS */
- regmap_write(mpcs->regmap, SGMSYS_PCS_LINK_TIMER,
- SGMII_LINK_TIMER_DEFAULT);
+ regmap_write(mpcs->regmap, SGMSYS_PCS_LINK_TIMER, link_timer);
regmap_read(mpcs->regmap, SGMSYS_SGMII_MODE, &val);
+ if (interface = == PHY_INTERFACE_MODE_SGMII)
+ val |= SGMII_IF_MODE_BIT0;
+ else
+ val &= ~SGMII_IF_MODE_BIT0;
val |= SGMII_REMOTE_FAULT_DIS;
regmap_write(mpcs->regmap, SGMSYS_SGMII_MODE, val);
+ regmap_update_bits_check(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, 0xffff,
+ advertise, &changed);
+
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &val);
+ val |= SGMII_AN_ENABLE;
val |= SGMII_AN_RESTART;
regmap_write(mpcs->regmap, SGMSYS_PCS_CONTROL_1, val);
@@ -40,7 +61,7 @@ static int mtk_pcs_setup_mode_an(struct mtk_pcs *mpcs)
val &= ~SGMII_PHYA_PWD;
regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, val);
- return 0;
+ return changed ? 1 : 0;
}
@@ -52,12 +73,6 @@ static int mtk_pcs_setup_mode_force(struct mtk_pcs *mpcs,
{
unsigned int val;
- regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val);
- val &= ~RG_PHY_SPEED_MASK;
- if (interface == PHY_INTERFACE_MODE_2500BASEX)
- val |= RG_PHY_SPEED_3_125G;
- regmap_write(mpcs->regmap, mpcs->ana_rgc3, val);
-
/* Disable SGMII AN */
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &val);
val &= ~SGMII_AN_ENABLE;
@@ -83,13 +98,22 @@ static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
bool permit_pause_to_mac)
{
struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs);
+ unsigned int val;
int err = 0;
+ if (interface == PHY_INTERFACE_MODE_2500BASEX)
+ val = RG_PHY_SPEED_3_125G;
+ else
+ val = 0;
+
+ regmap_update_bits(mpcs->regmap, mpcs->ana_rgc3,
+ RG_PHY_SPEED_3_125G, val);
+
/* Setup SGMIISYS with the determined property */
- if (interface != PHY_INTERFACE_MODE_SGMII)
+ if (phylink_autoneg_inband(mode))
+ err = mtk_pcs_setup_mode_an(mpcs, interface, advertising);
+ else if (interface != PHY_INTERFACE_MODE_SGMII)
err = mtk_pcs_setup_mode_force(mpcs, interface);
- else if (phylink_autoneg_inband(mode))
- err = mtk_pcs_setup_mode_an(mpcs);
return err;
}
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> Gesendet: Sonntag, 23. Oktober 2022 um 19:52 Uhr
> Von: "Russell King (Oracle)" <[email protected]>
> Hi Frank,
>
> Based on this, could you give the following patch a try - it replaces
> my previous patch.
looks better now:
root@bpi-r3:~# ip link set eth1 up
[ 59.437700] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/1000base-x link mode
root@bpi-r3:~# [ 59.446191] interface-mode: 21 advertise: 0x1a0 link timer:0x98968
[ 59.503145] offset:0 0x2c1140
[ 59.509329] offset:4 0x4d544950
[ 59.512284] offset:8 0x40e041a0
[ 59.515446] offset:32 0x3112011a
[ 59.518598] mtk_soc_eth 15100000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off
[ 59.530096] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
root@bpi-r3:~# ip a a 192.168.0.19/24 dev eth1
root@bpi-r3:~# ping 192.168.0.10
PING 192.168.0.10 (192.168.0.10) 56(84) bytes of data.
64 bytes from 192.168.0.10: icmp_seq=1 ttl=64 time=0.863 ms
64 bytes from 192.168.0.10: icmp_seq=2 ttl=64 time=0.491 ms
^C
--- 192.168.0.10 ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 1071ms
rtt min/avg/max/mdev = 0.491/0.677/0.863/0.186 ms
root@bpi-r3:~# ethtool eth1
[ 128.027246] offset:0 0x2c1140
[ 128.027264] offset:4 0x4d544950
[ 128.030230] offset:8 0x40e041a0
Settings for eth[ 128.033411] offset:32 0x3112011a
1:
Supported p[ 128.036798] offset:0 0x2c1140
[ 128.041287] offset:4 0x4d544950
Supported link[ 128.045636] offset:8 0x40e041a0
modes: 1000baseX/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 1000baseX/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Auto-negotiation: on
Port: FIBRE
PHYAD: 0
Transceiver: internal
Current message level: 0x000000ff (255)
drv probe link timer ifdown ifup rx_err tx_err
Link detected: yes
root@bpi-r3:~#
root@bpi-r3:~# iperf3 -c 192.168.0.21
Connecting to host 192.168.0.21, port 5201
[ 5] local 192.168.0.19 port 50992 connected to 192.168.0.21 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 114 MBytes 960 Mbits/sec 0 450 KBytes
[ 5] 1.00-2.00 sec 112 MBytes 940 Mbits/sec 0 450 KBytes
[ 5] 2.00-3.00 sec 113 MBytes 948 Mbits/sec 0 450 KBytes
[ 5] 3.00-4.00 sec 112 MBytes 940 Mbits/sec 0 450 KBytes
[ 5] 4.00-5.00 sec 112 MBytes 941 Mbits/sec 0 450 KBytes
[ 5] 5.00-6.00 sec 112 MBytes 941 Mbits/sec 0 450 KBytes
[ 5] 6.00-7.00 sec 113 MBytes 948 Mbits/sec 0 450 KBytes
[ 5] 7.00-8.00 sec 112 MBytes 939 Mbits/sec 0 450 KBytes
[ 5] 8.00-9.00 sec 112 MBytes 940 Mbits/sec 0 450 KBytes
[ 5] 9.00-10.00 sec 113 MBytes 947 Mbits/sec 0 450 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.00 sec 1.10 GBytes 944 Mbits/sec 0 sender
[ 5] 0.00-10.06 sec 1.10 GBytes 938 Mbits/sec receiver
iperf Done.
root@bpi-r3:~# iperf3 -c 192.168.0.21 -R
Connecting to host 192.168.0.21, port 5201
Reverse mode, remote host 192.168.0.21 is sending
[ 5] local 192.168.0.19 port 38736 connected to 192.168.0.21 port 5201
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 112 MBytes 937 Mbits/sec
[ 5] 1.00-2.00 sec 112 MBytes 940 Mbits/sec
[ 5] 2.00-3.00 sec 112 MBytes 939 Mbits/sec
[ 5] 3.00-4.00 sec 112 MBytes 940 Mbits/sec
[ 5] 4.00-5.00 sec 112 MBytes 940 Mbits/sec
[ 5] 5.00-6.00 sec 112 MBytes 940 Mbits/sec
[ 5] 6.00-7.00 sec 112 MBytes 940 Mbits/sec
[ 5] 7.00-8.00 sec 112 MBytes 941 Mbits/sec
[ 5] 8.00-9.00 sec 112 MBytes 940 Mbits/sec
[ 5] 9.00-10.00 sec 112 MBytes 940 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.06 sec 1.10 GBytes 936 Mbits/sec 0 sender
[ 5] 0.00-10.00 sec 1.09 GBytes 940 Mbits/sec receiver
iperf Done.
root@bpi-r3:~#
so now checking the first gmac (mt7531 switch-chip, fixed link with 2500baseX - sgmii, but i can only test 1G on a switchport not the full 2g5):
root@bpi-r3:~# ip link set eth1 down
[ 128.050136] offset:32 0x3112011a
[ 266.426857] mtk_soc_eth 15100000.ethernet eth1: Link is Down
root@bpi-r3:~# ip link set eth0 up
[ 277.257578] mtk_soc_eth 15100000.ethernet eth0: configuring for fixed/2500base-x link mode
[ 277.266007] mtk_soc_eth 15100000.ethernet eth0: Link is Up - 2.5Gbps/Full - flow control rx/tx
root@bpi-r3:~#
root@bpi-r3:~# ip a a 192.168.0.19/24 dev wan
root@bpi-r3:~# ip link set wan up
[ 373.687223] mt7530 mdio-bus:1f wan: configuring for phy/gmii link mode
root@bpi-r3:~# [ 373.698593] mt7530 mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
[ 373.706061] IPv6: ADDRCONF(NETDEV_CHANGE): wan: link becomes ready
root@bpi-r3:~# ip a a 192.168.0.19/24 dev wan
root@bpi-r3:~# ip link set wan up
[ 373.687223] mt7530 mdio-bus:1f wan: configuring for phy/gmii link mode
root@bpi-r3:~# [ 373.698593] mt7530 mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
[ 373.706061] IPv6: ADDRCONF(NETDEV_CHANGE): wan: link becomes ready
root@bpi-r3:~# ping 192.168.0.10
PING 192.168.0.10 (192.168.0.10) 56(84) bytes of data.
64 bytes from 192.168.0.10: icmp_seq=1 ttl=64 time=0.964 ms
64 bytes from 192.168.0.10: icmp_seq=2 ttl=64 time=0.523 ms
^C
--- 192.168.0.10 ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 1001ms
rtt min/avg/max/mdev = 0.523/0.743/0.964/0.220 ms
root@bpi-r3:~# iperf3 -c 192.168.0.21
Connecting to host 192.168.0.21, port 5201
[ 5] local 192.168.0.19 port 48332 connected to 192.168.0.21 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 115 MBytes 962 Mbits/sec 0 641 KBytes
[ 5] 1.00-2.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 2.00-3.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 3.00-4.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 4.00-5.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 5.00-6.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 6.00-7.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 7.00-8.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 8.00-9.00 sec 112 MBytes 944 Mbits/sec 0 641 KBytes
[ 5] 9.00-10.00 sec 111 MBytes 933 Mbits/sec 0 641 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.00 sec 1.10 GBytes 944 Mbits/sec 0 sender
[ 5] 0.00-10.06 sec 1.10 GBytes 937 Mbits/sec receiver
iperf Done.
root@bpi-r3:~# iperf3 -c 192.168.0.21 -R
Connecting to host 192.168.0.21, port 5201
Reverse mode, remote host 192.168.0.21 is sending
[ 5] local 192.168.0.19 port 51822 connected to 192.168.0.21 port 5201
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 112 MBytes 937 Mbits/sec
[ 5] 1.00-2.00 sec 112 MBytes 941 Mbits/sec
[ 5] 2.00-3.00 sec 112 MBytes 939 Mbits/sec
[ 5] 3.00-4.00 sec 112 MBytes 940 Mbits/sec
[ 5] 4.00-5.00 sec 112 MBytes 938 Mbits/sec
[ 5] 5.00-6.00 sec 112 MBytes 940 Mbits/sec
[ 5] 6.00-7.00 sec 112 MBytes 938 Mbits/sec
[ 5] 7.00-8.00 sec 112 MBytes 941 Mbits/sec
[ 5] 8.00-9.00 sec 112 MBytes 940 Mbits/sec
[ 5] 9.00-10.00 sec 112 MBytes 940 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.06 sec 1.10 GBytes 936 Mbits/sec 0 sender
[ 5] 0.00-10.00 sec 1.09 GBytes 939 Mbits/sec receiver
iperf Done.
root@bpi-r3:~# ethtool eth0
Settings for eth0:
Supported ports: [ MII ]
Supported link modes: 2500baseT/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 2500baseT/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 2500baseT/Full
Link partner advertised pause frame use: Symmetric
Link partner advertised auto-negotiation: No
Link partner advertised FEC modes: Not reported
Speed: 2500Mb/s
Duplex: Full
Auto-negotiation: on
Port: MII
PHYAD: 0
Transceiver: internal
Current message level: 0x000000ff (255)
drv probe link timer ifdown ifup rx_err tx_err
Link detected: yes
root@bpi-r3:~# ethtool wan
Settings for wan:
Supported ports: [ TP MII ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
Link partner advertised pause frame use: Symmetric
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Auto-negotiation: on
master-slave cfg: preferred slave
master-slave status: master
Port: Twisted Pair
PHYAD: 0
Transceiver: external
MDI-X: Unknown
Supports Wake-on: d
Wake-on: d
Link detected: yes
root@bpi-r3:~#
looks good too :)
if you fix this typo you can send the patch and add my tested-by:
regmap_read(mpcs->regmap, SGMSYS_SGMII_MODE, &val);
- if (interface = == PHY_INTERFACE_MODE_SGMII)
+ if (interface == PHY_INTERFACE_MODE_SGMII)
val |= SGMII_IF_MODE_BIT0;
else
val &= ~SGMII_IF_MODE_BIT0;
should i send an update for my patch including this:
state->duplex = DUPLEX_FULL;
or do you want to read the duplex from the advertisement like stated before?
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
regards Frank
> Gesendet: Sonntag, 23. Oktober 2022 um 21:03 Uhr
> Von: "Frank Wunderlich" <[email protected]>
> if you fix this typo you can send the patch and add my tested-by:
>
> regmap_read(mpcs->regmap, SGMSYS_SGMII_MODE, &val);
> - if (interface = == PHY_INTERFACE_MODE_SGMII)
> + if (interface == PHY_INTERFACE_MODE_SGMII)
> val |= SGMII_IF_MODE_BIT0;
> else
> val &= ~SGMII_IF_MODE_BIT0;
>
> should i send an update for my patch including this:
>
> state->duplex = DUPLEX_FULL;
>
> or do you want to read the duplex from the advertisement like stated before?
>
> regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
> regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
>
> phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
with the phylink-helper it works too
https://github.com/frank-w/BPI-R2-4.14/commits/6.1-r3-sgmii2
regards Frank
On Sun, Oct 23, 2022 at 09:21:37PM +0200, Frank Wunderlich wrote:
> > Gesendet: Sonntag, 23. Oktober 2022 um 21:03 Uhr
> > Von: "Frank Wunderlich" <[email protected]>
>
> > if you fix this typo you can send the patch and add my tested-by:
> >
> > regmap_read(mpcs->regmap, SGMSYS_SGMII_MODE, &val);
> > - if (interface = == PHY_INTERFACE_MODE_SGMII)
> > + if (interface == PHY_INTERFACE_MODE_SGMII)
> > val |= SGMII_IF_MODE_BIT0;
> > else
> > val &= ~SGMII_IF_MODE_BIT0;
> >
> > should i send an update for my patch including this:
> >
> > state->duplex = DUPLEX_FULL;
> >
> > or do you want to read the duplex from the advertisement like stated before?
> >
> > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
> > regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1 + 8, &adv);
> >
> > phylink_mii_c22_pcs_decode_state(state, bm >> 16, adv >> 16);
>
> with the phylink-helper it works too
>
> https://github.com/frank-w/BPI-R2-4.14/commits/6.1-r3-sgmii2
This is amazingly great news - we now know how to configure this
hardware! Let me cook up a proper set of patches for tomorrow - if
that's okay.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Sun, Oct 23, 2022 at 09:09:23PM +0100, Russell King (Oracle) wrote:
> This is amazingly great news - we now know how to configure this
> hardware! Let me cook up a proper set of patches for tomorrow - if
> that's okay.
Here's the combined patch for where I would like mtk_sgmii to get to.
It looks like this PCS is similar to what we know as pcs-lynx.c, but
there do seem to be differences - the duplex bit for example appears
to be inverted.
Please confirm whether this still works for you, thanks.
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index b52f3b0177ef..8ecf97bcfec6 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -466,8 +466,10 @@
#define ETHSYS_DMA_AG_MAP_PPE BIT(2)
/* SGMII subsystem config registers */
-/* Register to auto-negotiation restart */
+/* BMCR (low 16) BMSR (high 16) */
#define SGMSYS_PCS_CONTROL_1 0x0
+#define SGMII_BMCR GENMASK(15, 0)
+#define SGMII_BMSR GENMASK(31, 16)
#define SGMII_AN_RESTART BIT(9)
#define SGMII_ISOLATE BIT(10)
#define SGMII_AN_ENABLE BIT(12)
@@ -477,9 +479,13 @@
#define SGMII_PCS_FAULT BIT(23)
#define SGMII_AN_EXPANSION_CLR BIT(30)
+#define SGMSYS_PCS_ADVERTISE 0x8
+#define SGMII_ADVERTISE GENMASK(15, 0)
+#define SGMII_LPA GENMASK(31, 16)
+
/* Register to programmable link timer, the unit in 2 * 8ns */
#define SGMSYS_PCS_LINK_TIMER 0x18
-#define SGMII_LINK_TIMER_DEFAULT (0x186a0 & GENMASK(19, 0))
+#define SGMII_LINK_TIMER_MASK GENMASK(19, 0)
/* Register to control remote fault */
#define SGMSYS_SGMII_MODE 0x20
diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c
index 736839c84130..e64c02a48449 100644
--- a/drivers/net/ethernet/mediatek/mtk_sgmii.c
+++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c
@@ -19,110 +19,136 @@ static struct mtk_pcs *pcs_to_mtk_pcs(struct phylink_pcs *pcs)
return container_of(pcs, struct mtk_pcs, pcs);
}
-/* For SGMII interface mode */
-static int mtk_pcs_setup_mode_an(struct mtk_pcs *mpcs)
+static void mtk_pcs_get_state(struct phylink_pcs *pcs,
+ struct phylink_link_state *state)
{
- unsigned int val;
-
- /* Setup the link timer and QPHY power up inside SGMIISYS */
- regmap_write(mpcs->regmap, SGMSYS_PCS_LINK_TIMER,
- SGMII_LINK_TIMER_DEFAULT);
-
- regmap_read(mpcs->regmap, SGMSYS_SGMII_MODE, &val);
- val |= SGMII_REMOTE_FAULT_DIS;
- regmap_write(mpcs->regmap, SGMSYS_SGMII_MODE, val);
-
- regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &val);
- val |= SGMII_AN_RESTART;
- regmap_write(mpcs->regmap, SGMSYS_PCS_CONTROL_1, val);
-
- regmap_read(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, &val);
- val &= ~SGMII_PHYA_PWD;
- regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, val);
+ struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs);
+ unsigned int bm, adv;
- return 0;
+ /* Read the BMSR and LPA */
+ regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &bm);
+ regmap_read(mpcs->regmap, SGMSYS_PCS_ADVERTISE, &adv);
+ phylink_mii_c22_pcs_decode_state(state, FIELD_GET(SGMII_BMSR, bm),
+ FIELD_GET(SGMII_LPA, adv));
}
-/* For 1000BASE-X and 2500BASE-X interface modes, which operate at a
- * fixed speed.
- */
-static int mtk_pcs_setup_mode_force(struct mtk_pcs *mpcs,
- phy_interface_t interface)
+static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
+ phy_interface_t interface,
+ const unsigned long *advertising,
+ bool permit_pause_to_mac)
{
- unsigned int val;
+ struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs);
+ unsigned int rgc3, sgm_mode, bmcr;
+ int advertise, link_timer;
+ bool changed, use_an;
- regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val);
- val &= ~RG_PHY_SPEED_MASK;
if (interface == PHY_INTERFACE_MODE_2500BASEX)
- val |= RG_PHY_SPEED_3_125G;
- regmap_write(mpcs->regmap, mpcs->ana_rgc3, val);
+ rgc3 = RG_PHY_SPEED_3_125G;
+ else
+ rgc3 = 0;
+
+ advertise = phylink_mii_c22_pcs_encode_advertisement(interface,
+ advertising);
+ if (advertise < 0)
+ return advertise;
+
+ link_timer = phylink_get_link_timer_ns(interface);
+ if (link_timer < 0)
+ return link_timer;
+
+ /* Clearing IF_MODE_BIT0 switches the PCS to BASE-X mode, and
+ * we assume that fixes it's speed at bitrate = line rate (in
+ * other words, 1000Mbps or 2500Mbps).
+ */
+ if (interface == PHY_INTERFACE_MODE_SGMII) {
+ sgm_mode = SGMII_IF_MODE_BIT0;
+ if (phylink_autoneg_inband(mode)) {
+ sgm_mode |= SGMII_REMOTE_FAULT_DIS |
+ SGMII_SPEED_DUPLEX_AN;
+ use_an = true;
+ } else {
+ use_an = false;
+ }
+ } else if (phylink_autoneg_inband(mode)) {
+ /* 1000base-X or 2500base-X autoneg */
+ sgm_mode = SGMII_REMOTE_FAULT_DIS;
+ use_an = linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ advertising);
+ } else {
+ /* 1000base-X or 2500base-X without autoneg */
+ sgm_mode = 0;
+ use_an = false;
+ }
- /* Disable SGMII AN */
- regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &val);
- val &= ~SGMII_AN_ENABLE;
- regmap_write(mpcs->regmap, SGMSYS_PCS_CONTROL_1, val);
+ if (use_an) {
+ /* FIXME: Do we need to set AN_RESTART here? */
+ bmcr = SGMII_AN_RESTART | SGMII_AN_ENABLE;
+ } else {
+ bmcr = 0;
+ }
- /* Set the speed etc but leave the duplex unchanged */
- regmap_read(mpcs->regmap, SGMSYS_SGMII_MODE, &val);
- val &= SGMII_DUPLEX_FULL | ~SGMII_IF_MODE_MASK;
- val |= SGMII_SPEED_1000;
- regmap_write(mpcs->regmap, SGMSYS_SGMII_MODE, val);
+ /* Configure the underlying interface speed */
+ regmap_update_bits(mpcs->regmap, mpcs->ana_rgc3,
+ RG_PHY_SPEED_3_125G, rgc3);
- /* Release PHYA power down state */
- regmap_read(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, &val);
- val &= ~SGMII_PHYA_PWD;
- regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, val);
+ /* Update the advertisement, noting whether it has changed */
+ regmap_update_bits_check(mpcs->regmap, SGMSYS_PCS_ADVERTISE,
+ SGMII_ADVERTISE, advertise, &changed);
- return 0;
-}
+ /* Setup the link timer and QPHY power up inside SGMIISYS */
+ regmap_write(mpcs->regmap, SGMSYS_PCS_LINK_TIMER, link_timer / 2 / 8);
-static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising,
- bool permit_pause_to_mac)
-{
- struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs);
- int err = 0;
+ /* Update the sgmsys mode register */
+ regmap_update_bits(mpcs->regmap, SGMSYS_SGMII_MODE,
+ SGMII_REMOTE_FAULT_DIS | SGMII_SPEED_DUPLEX_AN |
+ SGMII_IF_MODE_BIT0, sgm_mode);
+
+ /* Update the BMCR */
+ regmap_update_bits(mpcs->regmap, SGMSYS_PCS_CONTROL_1,
+ SGMII_AN_RESTART | SGMII_AN_ENABLE, bmcr);
- /* Setup SGMIISYS with the determined property */
- if (interface != PHY_INTERFACE_MODE_SGMII)
- err = mtk_pcs_setup_mode_force(mpcs, interface);
- else if (phylink_autoneg_inband(mode))
- err = mtk_pcs_setup_mode_an(mpcs);
+ /* Release PHYA power down state */
+ regmap_update_bits(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL,
+ SGMII_PHYA_PWD, 0);
- return err;
+ return changed ? 1 : 0;
}
static void mtk_pcs_restart_an(struct phylink_pcs *pcs)
{
struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs);
- unsigned int val;
- regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &val);
- val |= SGMII_AN_RESTART;
- regmap_write(mpcs->regmap, SGMSYS_PCS_CONTROL_1, val);
+ regmap_update_bits(mpcs->regmap, SGMSYS_PCS_CONTROL_1,
+ SGMII_AN_RESTART, SGMII_AN_RESTART);
}
static void mtk_pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
phy_interface_t interface, int speed, int duplex)
{
struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs);
- unsigned int val;
-
- if (!phy_interface_mode_is_8023z(interface))
- return;
-
- /* SGMII force duplex setting */
- regmap_read(mpcs->regmap, SGMSYS_SGMII_MODE, &val);
- val &= ~SGMII_DUPLEX_FULL;
- if (duplex == DUPLEX_FULL)
- val |= SGMII_DUPLEX_FULL;
-
- regmap_write(mpcs->regmap, SGMSYS_SGMII_MODE, val);
+ unsigned int sgm_mode;
+
+ if (!phylink_autoneg_inband(mode)) {
+ /* Force the speed and duplex setting */
+ if (speed == SPEED_10)
+ sgm_mode = SGMII_SPEED_10;
+ else if (speed == SPEED_100)
+ sgm_mode = SGMII_SPEED_100;
+ else
+ sgm_mode = SGMII_SPEED_1000;
+
+ if (duplex == DUPLEX_FULL)
+ sgm_mode |= SGMII_DUPLEX_FULL;
+
+ regmap_update_bits(mpcs->regmap, SGMSYS_SGMII_MODE,
+ SGMII_DUPLEX_FULL | SGMII_SPEED_MASK,
+ sgm_mode);
+ }
}
static const struct phylink_pcs_ops mtk_pcs_ops = {
+ .pcs_get_state = mtk_pcs_get_state,
.pcs_config = mtk_pcs_config,
.pcs_an_restart = mtk_pcs_restart_an,
.pcs_link_up = mtk_pcs_link_up,
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 63800bf4a7ac..7a3eb46b38c1 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -616,6 +616,22 @@ int phylink_speed_up(struct phylink *pl);
void phylink_set_port_modes(unsigned long *bits);
+static inline int phylink_get_link_timer_ns(phy_interface_t interface)
+{
+ switch (interface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_QSGMII:
+ return 1600000;
+
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ return 10000000;
+
+ default:
+ return -EINVAL;
+ }
+}
+
void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
u16 bmsr, u16 lpa);
void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Oct 24, 2022 at 04:45:40PM +0200, Frank Wunderlich wrote:
> Hi
> > Gesendet: Montag, 24. Oktober 2022 um 11:27 Uhr
> > Von: "Russell King (Oracle)" <[email protected]>
>
> > Here's the combined patch for where I would like mtk_sgmii to get to.
> >
> > It looks like this PCS is similar to what we know as pcs-lynx.c, but
> > there do seem to be differences - the duplex bit for example appears
> > to be inverted.
> >
> > Please confirm whether this still works for you, thanks.
>
> basicly Patch works, but i get some (1-50) retransmitts on iperf3 on first interval in tx-mode (on r3 without -R), other 9 are clean. reverse mode is mostly clean.
> run iperf3 multiple times, every first interval has retransmitts. same for gmac0 (fixed-link 2500baseX)
>
> i notice that you have changed the timer again to 10000000 for 1000/2500baseX...maybe use here the default value too like the older code does?
You obviously missed my explanation. I will instead quote the 802.3
standard which covers 1000base-X:
37.3.1.4 Timers
link_timer
Timer used to ensure Auto-Negotiation protocol stability and
register read/write by the management interface.
Duration: 10 ms, tolerance +10 ms, –0 s.
For SGMII, the situation is different. Here is what the SGMII
specification says:
The link_timer inside the Auto-Negotiation has been changed from 10
msec to 1.6 msec to ensure a prompt update of the link status.
So, 10ms is correct for 1000base-X, and 1.6ms correct for SGMII.
However, feel free to check whether changing it solves that issue, but
also check whether it could be some ARP related issue - remember, if
two endpoints haven't communicated, they need to ARP to get the other
end's ethernet addresses which adds extra latency, and may result in
some packet loss in high packet queuing rate situations.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Hi
> Gesendet: Montag, 24. Oktober 2022 um 11:27 Uhr
> Von: "Russell King (Oracle)" <[email protected]>
> Here's the combined patch for where I would like mtk_sgmii to get to.
>
> It looks like this PCS is similar to what we know as pcs-lynx.c, but
> there do seem to be differences - the duplex bit for example appears
> to be inverted.
>
> Please confirm whether this still works for you, thanks.
basicly Patch works, but i get some (1-50) retransmitts on iperf3 on first interval in tx-mode (on r3 without -R), other 9 are clean. reverse mode is mostly clean.
run iperf3 multiple times, every first interval has retransmitts. same for gmac0 (fixed-link 2500baseX)
i notice that you have changed the timer again to 10000000 for 1000/2500baseX...maybe use here the default value too like the older code does?
regards Frank
> Gesendet: Montag, 24. Oktober 2022 um 16:56 Uhr
> Von: "Russell King (Oracle)" <[email protected]>
> On Mon, Oct 24, 2022 at 04:45:40PM +0200, Frank Wunderlich wrote:
> > Hi
> > > Gesendet: Montag, 24. Oktober 2022 um 11:27 Uhr
> > > Von: "Russell King (Oracle)" <[email protected]>
> >
> > > Here's the combined patch for where I would like mtk_sgmii to get to.
> > >
> > > It looks like this PCS is similar to what we know as pcs-lynx.c, but
> > > there do seem to be differences - the duplex bit for example appears
> > > to be inverted.
> > >
> > > Please confirm whether this still works for you, thanks.
> >
> > basicly Patch works, but i get some (1-50) retransmitts on iperf3 on first interval in tx-mode (on r3 without -R), other 9 are clean. reverse mode is mostly clean.
> > run iperf3 multiple times, every first interval has retransmitts. same for gmac0 (fixed-link 2500baseX)
> >
> > i notice that you have changed the timer again to 10000000 for 1000/2500baseX...maybe use here the default value too like the older code does?
>
> You obviously missed my explanation. I will instead quote the 802.3
> standard which covers 1000base-X:
sorry, right i remember you've already mentioned it
> 37.3.1.4 Timers
>
> link_timer
> Timer used to ensure Auto-Negotiation protocol stability and
> register read/write by the management interface.
>
> Duration: 10 ms, tolerance +10 ms, –0 s.
>
> For SGMII, the situation is different. Here is what the SGMII
> specification says:
>
> The link_timer inside the Auto-Negotiation has been changed from 10
> msec to 1.6 msec to ensure a prompt update of the link status.
>
> So, 10ms is correct for 1000base-X, and 1.6ms correct for SGMII.
>
> However, feel free to check whether changing it solves that issue, but
> also check whether it could be some ARP related issue - remember, if
> two endpoints haven't communicated, they need to ARP to get the other
> end's ethernet addresses which adds extra latency, and may result in
> some packet loss in high packet queuing rate situations.
tried with 1.6ms, same result (or even worse on 1000baseX). i guess arp cache should stay for ~5s?
so at least second round followed directly after the first should be clean when looking on ARP.
apart from this little problem it works much better than it actually is so imho more
people should test it on different platforms.
regards Frank
Frank Wunderlich <[email protected]> writes:
> apart from this little problem it works much better than it actually is so imho more
> people should test it on different platforms.
Hello! I've been banging my head against an MT7986 board with two
Maxlinear GPY211C phys for a while. One of those phys is connected to
port 5 of the MT7531 switch. This is working perfectly.
The other GPY211C is connected to the second MT7986 mac. This one is
giving me a headache...
I can only get the port to work at 2500Mb/s. Changing the speed to
anything lower looks fine in ethtool etc, but traffic is blocked.
Not knowing the first thing about MACs and PHYs and such, my best guess
is that there is something wrong with the PCS config.
Now I am currently testing this on an older kernel (using OpenWrt -
booting mainline is not straight forward). But I have backported all the
patches which came out of this thread. The resulting mtk_sgmii.c is
identical to the one currently in next-next, except for some additional
debug printk's. Since this is a small file, I've attached my current
mtk_sgmii.c copy for reference.
The output of those printks when changing peer speed to to 1000Mb/s is:
[ 363.099410] mtk_soc_eth 15100000.ethernet wan: Link is Down
[ 365.189945] mtk_sgmii_select_pcs: id=1
[ 365.193709] mtk_pcs_config: interface=4
[ 365.197530] offset:0 0x140
[ 365.197533] offset:4 0x4d544950
[ 365.200237] offset:8 0x20
[ 365.203365] mtk_pcs_config: rgc3=0x0, advertise=0x1 (changed), link_timer=1600000, sgm_mode=0x1, bmcr=0x0
[ 365.215601] mtk_pcs_link_up: interface=4
[ 365.219511] offset:0 0x140
[ 365.219513] offset:4 0x4d544950
[ 365.222204] offset:8 0x1
[ 365.225328] mtk_pcs_link_up: sgm_mode=0x18
[ 365.231940] mtk_soc_eth 15100000.ethernet wan: Link is Up - 1Gbps/Full - flow control rx/tx
and when changing peer back to autoneg (i.e. 2500Mb/s):
[ 878.939417] mtk_soc_eth 15100000.ethernet wan: Link is Down
[ 883.099857] mtk_sgmii_select_pcs: id=1
[ 883.103620] mtk_pcs_config: interface=22
[ 883.107527] offset:0 0x140
[ 883.107529] offset:4 0x4d544950
[ 883.110234] offset:8 0x1
[ 883.113363] mtk_pcs_config: rgc3=0x4, advertise=0x20 (changed), link_timer=10000000, sgm_mode=0x0, bmcr=0x0
[ 883.125686] mtk_pcs_link_up: interface=22
[ 883.129683] offset:0 0x40140
[ 883.129685] offset:4 0x4d544950
[ 883.132550] offset:8 0x20
[ 883.135687] mtk_soc_eth 15100000.ethernet wan: Link is Up - 2.5Gbps/Full - flow control rx/tx
ethtool output looks as expected in both cases:
# ethtool wan
Settings for wan:
Supported ports: [ ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
2500baseT/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
2500baseT/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 1000baseT/Full
Link partner advertised pause frame use: Symmetric Receive-only
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Auto-negotiation: on
master-slave cfg: preferred slave
master-slave status: slave
Port: Twisted Pair
PHYAD: 6
Transceiver: external
MDI-X: on (auto)
Current message level: 0x000000ff (255)
drv probe link timer ifdown ifup rx_err tx_err
Link detected: yes
# ethtool wan
Settings for wan:
Supported ports: [ ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
2500baseT/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
2500baseT/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 100baseT/Full
1000baseT/Full
2500baseT/Full
Link partner advertised pause frame use: Symmetric Receive-only
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 2500Mb/s
Duplex: Full
Auto-negotiation: on
master-slave cfg: preferred slave
master-slave status: master
Port: Twisted Pair
PHYAD: 6
Transceiver: external
MDI-X: on (auto)
Current message level: 0x000000ff (255)
drv probe link timer ifdown ifup rx_err tx_err
Link detected: yes
The behaviour looks similar to the GPY211C attached to switch port 5.
Except that the latter works regardless of speed..
I did however notice one difference, which may or may not be
significant, in the VSPEC1_SGMII_STAT register of the phys after
changing to 1000Mb/s. The switch attached phy reports:
root@OpenWrt:/# mdio mdio-bus 5:30 raw 9
0x002e
while the soc mac attached phy reports
root@OpenWrt:/# mdio mdio-bus 6:30 raw 9
0x000e
According to
https://assets.maxlinear.com/web/documents/617810_gpy211b1vc_gpy211c0vc_ds_rev1.4.pdf
bit 5 is "Auto-Negotiation Completed". So it does look like the switch
mac is doing AN on the SGMII link, and the soc mac is not. Is that
correct?
Any hints on where I should look next?
The ethernet part of my device tree looks like this:
ð {
status = "okay";
gmac0: mac@0 {
compatible = "mediatek,eth-mac";
reg = <0>;
phy-mode = "2500base-x";
fixed-link {
speed = <2500>;
full-duplex;
pause;
};
};
mac@1 {
compatible = "mediatek,eth-mac";
reg = <1>;
label = "wan";
phy-mode = "2500base-x";
phy-handle = <&phy6>;
};
mdio: mdio-bus {
#address-cells = <1>;
#size-cells = <0>;
};
};
&mdio {
reset-gpios = <&pio 6 GPIO_ACTIVE_LOW>;
reset-delay-us = <50000>;
reset-post-delay-us = <20000>;
phy5: phy@5 {
compatible = "ethernet-phy-ieee802.3-c45";
reg = <5>;
};
phy6: phy@6 {
compatible = "ethernet-phy-ieee802.3-c45";
reg = <6>;
};
switch: switch@1f {
compatible = "mediatek,mt7531";
reg = <31>;
reset-gpios = <&pio 5 GPIO_ACTIVE_HIGH>;
interrupt-controller;
#interrupt-cells = <1>;
interrupt-parent = <&pio>;
interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
};
};
&switch {
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
label = "lan3";
};
port@1 {
reg = <1>;
label = "lan2";
};
port@2 {
reg = <2>;
label = "lan1";
};
port@5 {
reg = <5>;
label = "lan4";
phy-mode = "2500base-x";
phy-handle = <&phy5>;
};
port@6 {
reg = <6>;
label = "cpu";
ethernet = <&gmac0>;
phy-mode = "2500base-x";
fixed-link {
speed = <2500>;
full-duplex;
pause;
};
};
};
};
Bjørn
On Mon, Jan 16, 2023 at 02:08:30PM +0100, Bj?rn Mork wrote:
> Frank Wunderlich <[email protected]> writes:
>
> > apart from this little problem it works much better than it actually is so imho more
> > people should test it on different platforms.
>
> Hello! I've been banging my head against an MT7986 board with two
> Maxlinear GPY211C phys for a while. One of those phys is connected to
> port 5 of the MT7531 switch. This is working perfectly.
>
> The other GPY211C is connected to the second MT7986 mac. This one is
> giving me a headache...
>
> I can only get the port to work at 2500Mb/s. Changing the speed to
> anything lower looks fine in ethtool etc, but traffic is blocked.
My guess would be that the GPY PHY is using in-band SGMII negotiation
(it sets VSPEC1_SGMII_ANEN_ANRS when entering SGMII mode and clears
it in 2500base-X), but as the link is not using in-band mode on the
PCS side, the PHY never sees its in-band negotiation complete, so the
link between PCS and PHY never comes up.
Both sides need to agree on that detail.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
"Russell King (Oracle)" <[email protected]> writes:
> On Mon, Jan 16, 2023 at 02:08:30PM +0100, Bjørn Mork wrote:
>> Frank Wunderlich <[email protected]> writes:
>>
>> > apart from this little problem it works much better than it actually is so imho more
>> > people should test it on different platforms.
>>
>> Hello! I've been banging my head against an MT7986 board with two
>> Maxlinear GPY211C phys for a while. One of those phys is connected to
>> port 5 of the MT7531 switch. This is working perfectly.
>>
>> The other GPY211C is connected to the second MT7986 mac. This one is
>> giving me a headache...
>>
>> I can only get the port to work at 2500Mb/s. Changing the speed to
>> anything lower looks fine in ethtool etc, but traffic is blocked.
>
> My guess would be that the GPY PHY is using in-band SGMII negotiation
> (it sets VSPEC1_SGMII_ANEN_ANRS when entering SGMII mode and clears
> it in 2500base-X), but as the link is not using in-band mode on the
> PCS side, the PHY never sees its in-band negotiation complete, so the
> link between PCS and PHY never comes up.
>
> Both sides need to agree on that detail.
Any hints on how I would go about doing that? I am a little lost here,
changing arbitrary bits I don't understand the meaning of.
Bjørn
On Mon, Jan 16, 2023 at 03:45:24PM +0100, Bj?rn Mork wrote:
> "Russell King (Oracle)" <[email protected]> writes:
> > On Mon, Jan 16, 2023 at 02:08:30PM +0100, Bj?rn Mork wrote:
> >> Frank Wunderlich <[email protected]> writes:
> >>
> >> > apart from this little problem it works much better than it actually is so imho more
> >> > people should test it on different platforms.
> >>
> >> Hello! I've been banging my head against an MT7986 board with two
> >> Maxlinear GPY211C phys for a while. One of those phys is connected to
> >> port 5 of the MT7531 switch. This is working perfectly.
> >>
> >> The other GPY211C is connected to the second MT7986 mac. This one is
> >> giving me a headache...
> >>
> >> I can only get the port to work at 2500Mb/s. Changing the speed to
> >> anything lower looks fine in ethtool etc, but traffic is blocked.
> >
> > My guess would be that the GPY PHY is using in-band SGMII negotiation
> > (it sets VSPEC1_SGMII_ANEN_ANRS when entering SGMII mode and clears
> > it in 2500base-X), but as the link is not using in-band mode on the
> > PCS side, the PHY never sees its in-band negotiation complete, so the
> > link between PCS and PHY never comes up.
> >
> > Both sides need to agree on that detail.
>
> Any hints on how I would go about doing that? I am a little lost here,
> changing arbitrary bits I don't understand the meaning of.
Hi,
To prove the point, in mtk_pcs_config():
if (interface == PHY_INTERFACE_MODE_SGMII) {
sgm_mode = SGMII_IF_MODE_SGMII;
if (phylink_autoneg_inband(mode)) {
Force the second if() to always be true, and see whether that allows
traffic to pass.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
"Russell King (Oracle)" <[email protected]> writes:
> To prove the point, in mtk_pcs_config():
>
> if (interface == PHY_INTERFACE_MODE_SGMII) {
> sgm_mode = SGMII_IF_MODE_SGMII;
> if (phylink_autoneg_inband(mode)) {
>
> Force the second if() to always be true, and see whether that allows
> traffic to pass.
Changed it with a printk to make sure I didn't mess up:
if (1 || phylink_autoneg_inband(mode)) {
pr_info("forcing AN\n");
But unfortunately without success. Still same failure. Output when
changing peer speed:
[ 54.539438] mtk_soc_eth 15100000.ethernet wan: Link is Down
[ 56.619937] mtk_sgmii_select_pcs: id=1
[ 56.623690] mtk_pcs_config: interface=4
[ 56.627511] offset:0 0x140
[ 56.627513] offset:4 0x4d544950
[ 56.630215] offset:8 0x20
[ 56.633340] forcing AN
[ 56.638292] mtk_pcs_config: rgc3=0x0, advertise=0x1 (changed), link_timer=1600000, sgm_mode=0x103, bmcr=0x1000, use_an=1
[ 56.649226] mtk_pcs_link_up: interface=4
[ 56.653135] offset:0 0x81140
[ 56.653137] offset:4 0x4d544950
[ 56.656001] offset:8 0x1
[ 56.659137] mtk_soc_eth 15100000.ethernet wan: Link is Up - 1Gbps/Full - flow control rx/tx
and the phy still reports this:
root@OpenWrt:/# mdio mdio-bus 6:30 raw 9
0x000e
Bjørn