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] [day] [month] [year] [list]

Message-ID: <20211125192343.20e19849@thinkpad>
Date:   Thu, 25 Nov 2021 19:23:43 +0100
From:   Marek Behún <kabel@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
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, 25 Nov 2021 18:56:39 +0100
Andrew Lunn <andrew@...n.ch> wrote:

> 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.

Thanks. I will try this tomorrow, both on 6190 and 6393x.

For now, ignore this patch.

Marek