[PATCH net] net: phy: marvell-88x2222: set proper phydev->port

[PATCH net] net: phy: marvell-88x2222: set proper phydev->port

[Ivan Bornyakov]

phydev->port was not set and always reported as PORT_TP.
Set phydev->port according to inserted SFP module.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/net/phy/marvell-88x2222.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
index ec4f1407a78c..9f971b37ec35 100644
--- a/drivers/net/phy/marvell-88x2222.c
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -603,6 +603,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 	dev = &phydev->mdio.dev;
 
 	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
+	phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
 	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
 
 	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
@@ -639,6 +640,7 @@ static void mv2222_sfp_remove(void *upstream)
 
 	priv->line_interface = PHY_INTERFACE_MODE_NA;
 	linkmode_zero(priv->supported);
+	phydev->port = PORT_OTHER;
 }
 
 static void mv2222_sfp_link_up(void *upstream)
-- 
2.25.1

Re: [PATCH net] net: phy: marvell-88x2222: set proper phydev->port

[Andrew Lunn]

On Tue, Apr 05, 2022 at 06:03:05PM +0300, Ivan Bornyakov wrote:
> phydev->port was not set and always reported as PORT_TP.
> Set phydev->port according to inserted SFP module.

This is definitely something for Russell to review.

But i'm wondering if this is the correct place to do this? What about
at803x and marvell10g? It seems like this should be done once in the
core somewhere, not in each driver.

     Andrew

Re: [PATCH net] net: phy: marvell-88x2222: set proper phydev->port

[Ivan Bornyakov]

On Tue, Apr 05, 2022 at 06:14:11PM +0200, Andrew Lunn wrote:
> On Tue, Apr 05, 2022 at 06:03:05PM +0300, Ivan Bornyakov wrote:
> > phydev->port was not set and always reported as PORT_TP.
> > Set phydev->port according to inserted SFP module.
> 
> This is definitely something for Russell to review.
> 
> But i'm wondering if this is the correct place to do this? What about
> at803x and marvell10g?

I guess them need this too, but I only have HW with Marvell 88X2222.

> It seems like this should be done once in the
> core somewhere, not in each driver.
> 
>      Andrew

Apart from sfp_parse_port(), sfp_parse_support() and
sfp_select_interface() are also present in all
sfp_upstream_ops->module_insert PHY callbacks.

[PATCH net-next RESEND] net: phy: marvell-88x2222: set proper phydev->port

[Ivan Bornyakov]

phydev->port was not set and always reported as PORT_TP.
Set phydev->port according to inserted SFP module.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/net/phy/marvell-88x2222.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
index ec4f1407a78c..9f971b37ec35 100644
--- a/drivers/net/phy/marvell-88x2222.c
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -603,6 +603,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 	dev = &phydev->mdio.dev;
 
 	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
+	phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
 	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
 
 	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
@@ -639,6 +640,7 @@ static void mv2222_sfp_remove(void *upstream)
 
 	priv->line_interface = PHY_INTERFACE_MODE_NA;
 	linkmode_zero(priv->supported);
+	phydev->port = PORT_OTHER;
 }
 
 static void mv2222_sfp_link_up(void *upstream)
-- 
2.25.1

Re: [PATCH net-next RESEND] net: phy: marvell-88x2222: set proper phydev->port

[Russell King (Oracle)]

On Mon, Apr 25, 2022 at 07:16:37AM +0300, Ivan Bornyakov wrote:
> phydev->port was not set and always reported as PORT_TP.
> Set phydev->port according to inserted SFP module.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/net/phy/marvell-88x2222.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
> index ec4f1407a78c..9f971b37ec35 100644
> --- a/drivers/net/phy/marvell-88x2222.c
> +++ b/drivers/net/phy/marvell-88x2222.c
> @@ -603,6 +603,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
>  	dev = &phydev->mdio.dev;
>  
>  	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
> +	phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
>  	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
>  
>  	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
> @@ -639,6 +640,7 @@ static void mv2222_sfp_remove(void *upstream)
>  
>  	priv->line_interface = PHY_INTERFACE_MODE_NA;
>  	linkmode_zero(priv->supported);
> +	phydev->port = PORT_OTHER;

Can this PHY be used in dual-media mode, auto-switching between copper
and fibre? If so, is PORT_OTHER actually appropriate here, or should
the old value be saved when the module is inserted and restored when
it's removed?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next RESEND] net: phy: marvell-88x2222: set proper phydev->port

[Ivan Bornyakov]

On Mon, Apr 25, 2022 at 10:33:06PM +0100, Russell King (Oracle) wrote:
> On Mon, Apr 25, 2022 at 07:16:37AM +0300, Ivan Bornyakov wrote:
> > phydev->port was not set and always reported as PORT_TP.
> > Set phydev->port according to inserted SFP module.
> > 
> > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> > ---
> >  drivers/net/phy/marvell-88x2222.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
> > index ec4f1407a78c..9f971b37ec35 100644
> > --- a/drivers/net/phy/marvell-88x2222.c
> > +++ b/drivers/net/phy/marvell-88x2222.c
> > @@ -603,6 +603,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
> >  	dev = &phydev->mdio.dev;
> >  
> >  	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
> > +	phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
> >  	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
> >  
> >  	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
> > @@ -639,6 +640,7 @@ static void mv2222_sfp_remove(void *upstream)
> >  
> >  	priv->line_interface = PHY_INTERFACE_MODE_NA;
> >  	linkmode_zero(priv->supported);
> > +	phydev->port = PORT_OTHER;
> 
> Can this PHY be used in dual-media mode, auto-switching between copper
> and fibre?

I believe, it can not.

> If so, is PORT_OTHER actually appropriate here, or should
> the old value be saved when the module is inserted and restored when
> it's removed?

Would PORT_NONE be appropriate? Saving old value would be ok also.

[PATCH v2 net-next] net: phy: marvell-88x2222: set proper phydev->port

[Ivan Bornyakov]

phydev->port was not set and always reported as PORT_TP.
Set phydev->port according to inserted SFP module.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
Changelog:
  v1 -> v2: set port as PORT_NONE on SFP removal, instead of PORT_OTHER

 drivers/net/phy/marvell-88x2222.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
index d8b31d4d2a73..f070776ca904 100644
--- a/drivers/net/phy/marvell-88x2222.c
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -490,6 +490,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 	dev = &phydev->mdio.dev;
 
 	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
+	phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
 	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
 
 	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
@@ -526,6 +527,7 @@ static void mv2222_sfp_remove(void *upstream)
 
 	priv->line_interface = PHY_INTERFACE_MODE_NA;
 	linkmode_zero(priv->supported);
+	phydev->port = PORT_NONE;
 }
 
 static void mv2222_sfp_link_up(void *upstream)
-- 
2.35.1

Re: [PATCH v2 net-next] net: phy: marvell-88x2222: set proper phydev->port

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Sun, 12 Jun 2022 21:19:34 +0300 you wrote:
> phydev->port was not set and always reported as PORT_TP.
> Set phydev->port according to inserted SFP module.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
> Changelog:
>   v1 -> v2: set port as PORT_NONE on SFP removal, instead of PORT_OTHER
> 
> [...]

Here is the summary with links:
  - [v2,net-next] net: phy: marvell-88x2222: set proper phydev->port
    https://git.kernel.org/netdev/net-next/c/9794ef5a6843

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html