linux-kernel - Re: [net v2] net: phy: mscc: adjust the phy support for PTP and MACsec

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:   Wed, 11 Nov 2020 17:28:54 +0100
From:   Antoine Tenart <atenart@...nel.org>
To:     "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>,
        Bryan Whitehead <Bryan.Whitehead@...rochip.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Steen Hegelund <steen.hegelund@...rochip.com>
Cc:     Steen Hegelund <steen.hegelund@...rochip.com>,
        Microchip UNG Driver List <UNGLinuxDriver@...rochip.com>,
        John Haechten <John.Haechten@...rochip.com>,
        Netdev List <netdev@...r.kernel.org>,
        Linux Kernel List <linux-kernel@...r.kernel.org>
Subject: Re: [net v2] net: phy: mscc: adjust the phy support for PTP and MACsec

Hi Steen,

Quoting Steen Hegelund (2020-11-11 16:17:53)
> The MSCC PHYs selected for PTP and MACSec was not correct
> 
> - PTP
>     - Add VSC8572 and VSC8574
> 
> - MACsec
>     - Removed VSC8575
> 
> The relevant datasheets can be found here:
>   - VSC8572: https://www.microchip.com/wwwproducts/en/VSC8572
>   - VSC8574: https://www.microchip.com/wwwproducts/en/VSC8574
>   - VSC8575: https://www.microchip.com/wwwproducts/en/VSC8575
> 
> History:
> v1 -> v2:
>   - Added "fixes:" tags to the commit message
> 
> Fixes: bb56c016a1257 ("net: phy: mscc: split the driver into separate files")

This commit splitting the driver didn't introduced the issue, it only
moved code around. You can remove this Fixes tag. (You usually/should
have a single Fixes tag per patch).

> Fixes: ab2bf93393571 ("net: phy: mscc: 1588 block initialization")

The PTP and the MACsec support were introduced in separate patches (and
were introduced in different releases of the kernel). This patch is
fixing two different issues then, and its changes can't apply to the
same kernel versions. You should send them in two separate patches.

With the changes sent in two different patches, I would suggest to only
send the MACsec one as a fix for net (it's really fixing something, by
removing a non-compatible PHY from using MACsec) and the PTP one for
net-next as it's adding PTP support for two new PHYs (not fixing
anything).

When you do so, please use the following commands to format the patches,
to end up with the correct prefix in the subject:
git format-patch --subject-prefix='PATCH net' ...
git format-patch --subject-prefix='PATCH net-next' ...

Thanks!
Antoine

> Signed-off-by: Steen Hegelund <steen.hegelund@...rochip.com>
> ---
>  drivers/net/phy/mscc/mscc_macsec.c | 1 -
>  drivers/net/phy/mscc/mscc_ptp.c    | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c
> index 1d4c012194e9..72292bf6c51c 100644
> --- a/drivers/net/phy/mscc/mscc_macsec.c
> +++ b/drivers/net/phy/mscc/mscc_macsec.c
> @@ -981,7 +981,6 @@ int vsc8584_macsec_init(struct phy_device *phydev)
>  
>         switch (phydev->phy_id & phydev->drv->phy_id_mask) {
>         case PHY_ID_VSC856X:
> -       case PHY_ID_VSC8575:
>         case PHY_ID_VSC8582:
>         case PHY_ID_VSC8584:
>                 INIT_LIST_HEAD(&vsc8531->macsec_flows);
> diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
> index b97ee79f3cdf..f0537299c441 100644
> --- a/drivers/net/phy/mscc/mscc_ptp.c
> +++ b/drivers/net/phy/mscc/mscc_ptp.c
> @@ -1510,6 +1510,8 @@ void vsc8584_config_ts_intr(struct phy_device *phydev)
>  int vsc8584_ptp_init(struct phy_device *phydev)
>  {
>         switch (phydev->phy_id & phydev->drv->phy_id_mask) {
> +       case PHY_ID_VSC8572:
> +       case PHY_ID_VSC8574:
>         case PHY_ID_VSC8575:
>         case PHY_ID_VSC8582:
>         case PHY_ID_VSC8584:
> -- 
> 2.29.2
>