netdev - Re: [PATCH net] net: dsa: mv88e6xxx: Disable AN on 2500base-x for Amethyst

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YZ/OV+ang2FW9phY@lunn.ch>
Date:   Thu, 25 Nov 2021 18:56:39 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Marek Behún <kabel@...nel.org>
Cc:     netdev@...r.kernel.org, Russell King <rmk+kernel@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net,
        Tobias Waldekranz <tobias@...dekranz.com>
Subject: Re: [PATCH net] net: dsa: mv88e6xxx: Disable AN on 2500base-x for
 Amethyst

On Thu, Nov 25, 2021 at 06:45:51PM +0100, Marek Behún wrote:
> On Thu, 25 Nov 2021 16:37:40 +0100
> Andrew Lunn <andrew@...n.ch> wrote:
> 
> > On Thu, Nov 25, 2021 at 03:43:59PM +0100, Marek Behún wrote:
> > > Amethyst does not support autonegotiation in 2500base-x mode.  
> > 
> > Hi Marek
> > 
> > I tried to avoid using Marvells internal names for these devices. I
> > don't think these names exist in the datasheet? They are visible in
> > the SDK, but that is not so widely available. So if you do want to use
> > these names, please also reference the name we use in the kernel,
> > mv88e6393x.
> 
> I used these names in previous commit that are already merged (search
> for Amethyst, Topaz, Peridot). But if you want, I can send v2. Please
> let me know if I should send v2.

I'm not against these names, but i don't like them used on there own,
thats all.

> > What happens when changing from say 1000BaseX to 2500BaseX? Do you
> > need to disable the advertisement which 1000BaseX might of enabled?

> 
> I don't need to disable it, it is disabled on it's own by cmode change.

O.K, that is important information to put into the commit message,
since it makes it clear you have thought about this, and a reviewer
does not need to ask the question.

> Moreover I did some experiments on 88E6393X vs 88E6190.
> 
> It is a little weird for 6393x.
> 
> On 6190
> - resetting the SerDes (BMCR_RESET) does not have impact on the
>   BMCR_ANENABLE bit, but changing cmode does
> 
>   when cmode is changed to SGMII or 1000base-x, it is enabled, for
>   2500base-x it is disabled
> 
> - resetting the SerDes (BMCR_RESET) when cmode is
>   - sgmii, changes value of the MV88E6390_SGMII_ADVERTISE to 0x0001
>     automatically
>   - 1000base-x or 2500base-x does not change the value of adv register
> 
>   moreover it seems that changing cmode also resets the the SerDes
> 
> On 6393x:
> - the BMCR behaves the same as in 6190: the switch defaults to AN
>   disabled for 2500base-x, and enabled for 1000base-x and SGMII
> 
> - the difference is that there are weird values written to
>   MV88E6390_SGMII_ADVERTISE on SerDes reset (which is done by switch
>   when changing cmode)
> 
>   for 1000base-x, the value is 0x9120
>   for 2500base-x, the value is 0x9f41
> 
>   I don't understand why they changed it so for 6393x.

Tobias found something which might be relevant here. On the 6390
family, if you change cmode while the SERDES is powered off, bad
things happen. What you could be seeing is another symptom of
that. Tobias has a WIP patch which changes the order of operations
when changing the cmode. It would be interesting to see if that makes
a difference here as well.

  Andrew