diff mbox series

[net-next,v3,8/9] net: dsa: dsa_slave_phy_connect(): extend phy's flags with port specific phy flags

Message ID 20210526043037.9830-9-o.rempel@pengutronix.de (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series provide cable test support for the ksz886x switch | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: hkallweit1@gmail.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 18 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Oleksij Rempel May 26, 2021, 4:30 a.m. UTC
This patch extends the flags of the phy that's being connected with the
port specific flags of the switch port.

This is needed to handle a port specific erratum of the KSZ8873 switch,
which is added in a later patch.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phylink.c | 2 +-
 net/dsa/slave.c           | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Russell King (Oracle) May 26, 2021, 3:08 p.m. UTC | #1
On Wed, May 26, 2021 at 06:30:36AM +0200, Oleksij Rempel wrote:
> This patch extends the flags of the phy that's being connected with the
> port specific flags of the switch port.
> 
> This is needed to handle a port specific erratum of the KSZ8873 switch,
> which is added in a later patch.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/phylink.c | 2 +-
>  net/dsa/slave.c           | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 96d8e88b4e46..167c2277814f 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1029,7 +1029,7 @@ static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
>  	if (pl->phydev)
>  		return -EBUSY;
>  
> -	return phy_attach_direct(pl->netdev, phy, 0, interface);
> +	return phy_attach_direct(pl->netdev, phy, phy->dev_flags, interface);

I don't think this has any benefit. phy_attach_direct() does this
internally:

        phydev->dev_flags |= flags;

which means the above change is effectively doing:

        phydev->dev_flags |= phydev->dev_flags;

So, are you sure you need this?
Oleksij Rempel June 10, 2021, 10:04 a.m. UTC | #2
On Wed, May 26, 2021 at 04:08:11PM +0100, Russell King (Oracle) wrote:
> On Wed, May 26, 2021 at 06:30:36AM +0200, Oleksij Rempel wrote:
> > This patch extends the flags of the phy that's being connected with the
> > port specific flags of the switch port.
> > 
> > This is needed to handle a port specific erratum of the KSZ8873 switch,
> > which is added in a later patch.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/net/phy/phylink.c | 2 +-
> >  net/dsa/slave.c           | 4 ++++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 96d8e88b4e46..167c2277814f 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -1029,7 +1029,7 @@ static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
> >  	if (pl->phydev)
> >  		return -EBUSY;
> >  
> > -	return phy_attach_direct(pl->netdev, phy, 0, interface);
> > +	return phy_attach_direct(pl->netdev, phy, phy->dev_flags, interface);
> 
> I don't think this has any benefit. phy_attach_direct() does this
> internally:
> 
>         phydev->dev_flags |= flags;
> 
> which means the above change is effectively doing:
> 
>         phydev->dev_flags |= phydev->dev_flags;
> 
> So, are you sure you need this?

Ah, good point. Back for two years, phy_attach_direct() was doing
	phydev->dev_flags = flags;

I didn't noticed this change.

Regards,
Oleksij
diff mbox series

Patch

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 96d8e88b4e46..167c2277814f 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1029,7 +1029,7 @@  static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
 	if (pl->phydev)
 		return -EBUSY;
 
-	return phy_attach_direct(pl->netdev, phy, 0, interface);
+	return phy_attach_direct(pl->netdev, phy, phy->dev_flags, interface);
 }
 
 /**
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 8c0f3c6ab365..7e208f16f006 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1758,6 +1758,10 @@  static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
 		return -ENODEV;
 	}
 
+	if (ds->ops->get_phy_flags)
+		slave_dev->phydev->dev_flags |=
+			ds->ops->get_phy_flags(ds, dp->index);
+
 	return phylink_connect_phy(dp->pl, slave_dev->phydev);
 }