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

[Steen Hegelund]

On 11.11.2020 17:28, Antoine Tenart wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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).
OK.  I get that.

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

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

I will split the patch as you suggested.

> 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
Thanks for the comments, Antoine,

BR
Steen
> > Signed-off-by: Steen Hegelund <steen.hegelund@xxxxxxxxxxxxx>
> > ---
> >  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

BR
Steen

---------------------------------------
Steen Hegelund
steen.hegelund@xxxxxxxxxxxxx