linux-kernel - Re: [PATCH V2 net-next] net: marvell: prestera: add phylink 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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Jul 2022 08:56:26 +0000
From:   Oleksandr Mazur <oleksandr.mazur@...ision.eu>
To:     Andrew Lunn <andrew@...n.ch>,
        "Russell King (Oracle)" <linux@...linux.org.uk>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        Yevhen Orlov <yevhen.orlov@...ision.eu>,
        Taras Chornyi <taras.chornyi@...ision.eu>
Subject: Re: [PATCH V2 net-next] net: marvell: prestera: add phylink support

Hello Andrew, Russel,
Thanks for the input and sorry for the inconviniences.

>First question which applies to everything in this patch is - why make
>phylink conditional for this driver?

1. As for the phylink ifdefs:
The original idea was to support mac config on devices (DB boards) that don't have full sfp support on board, however we scrapped out
this idea as we could simply use fixed-link DTS configs; also due to this solution being non-upstream-friendly.
Please note that V2 holds no phylink ifdefs;

>In SGMII mode, it is normal for the advertising
>mask to indicate the media modes on the PHY to advertise
2. As for the SGMII mode, yes, Russel, you're right; V3 will hold a fix for this, and keep the inband enabled for SGMII.

>No way to restart 1000base-X autoneg?
3. As for AN restart, no, it's not yet supported by our FW as of now.

>I think you should be calling phylink_mac_change() here, rather than
>below.

4. V3 is gonna fix this, thanks for the input.