netdev - Re: [PATCH net-next] net: phy: marvell10g: add downshift tunable support

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: <YVCQlul6Ar4fVtns@lunn.ch>
Date:   Sun, 26 Sep 2021 17:24:06 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Marek Beh__n <kabel@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next] net: phy: marvell10g: add downshift tunable
 support

> So, some further questions: should we be calling the set_downshift
> implementation from the .config_init as the Marvell driver does to
> ensure that downshift is correctly enabled?

The bootloader might of messed it up, so it does not seem unreasonable
to set it somewhere at startup.

> Is .config_init really
> the best place to do this? So many things with Marvell PHYs seem to
> require a reset, which bounces the link. So if one brings up the
> network interface, then sets EEE (you get a link bounce) and then
> set downshift, you get another link bounce. Each link bounce takes
> more than a second, which means the more features that need to be
> configured after bringing the interface up, the longer it takes for
> the network to become usable. Note that Marvell downshift will cause
> the link to bounce even if the values programmed into the register
> were already there - there is no check to see if we actually changed
> anything before calling genphy_soft_reset() which seems suboptimal
> given that we have phy_modify_changed() which can tell us that.

This can clearly be optimized. Add a test if the values are being
changed. Skip the reset if it is being done as part of .config_init
and there is a guarantee a later stage will perform the reset, etc.

    Andrew