[PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[shayderrr]

From: Pranav Bajjuri <darknessshayder@gmail.com>

Add netif_carrier_off() to cvm_oct_common_stop(), guard
cvm_oct_adjust_link() against null phydev, replace open-coded
ioctl checks with phy_do_ioctl_running(), and zero priv->link_info
on stop alongside last_link.

Signed-off-by: Pranav Bajjuri <darknessshayder@gmail.com>
---
 drivers/staging/octeon/ethernet-mdio.c | 64 +++++++++-----------------
 1 file changed, 23 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
index 211423059e30..4b5cac324eff 100644
--- a/drivers/staging/octeon/ethernet-mdio.c
+++ b/drivers/staging/octeon/ethernet-mdio.c
@@ -17,6 +17,8 @@
 #include "ethernet-mdio.h"
 #include "ethernet-util.h"
 
+#define CVM_OCT_PHY_FLAGS 0
+
 static void cvm_oct_get_drvinfo(struct net_device *dev,
 				struct ethtool_drvinfo *info)
 {
@@ -36,30 +38,16 @@ static int cvm_oct_nway_reset(struct net_device *dev)
 }
 
 const struct ethtool_ops cvm_oct_ethtool_ops = {
-	.get_drvinfo = cvm_oct_get_drvinfo,
-	.nway_reset = cvm_oct_nway_reset,
-	.get_link = ethtool_op_get_link,
-	.get_link_ksettings = phy_ethtool_get_link_ksettings,
-	.set_link_ksettings = phy_ethtool_set_link_ksettings,
+	.get_drvinfo		= cvm_oct_get_drvinfo,
+	.nway_reset		= cvm_oct_nway_reset,
+	.get_link		= ethtool_op_get_link,
+	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
+	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
 };
 
-/**
- * cvm_oct_ioctl - IOCTL support for PHY control
- * @dev:    Device to change
- * @rq:     the request
- * @cmd:    the command
- *
- * Returns Zero on success
- */
 int cvm_oct_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
-	if (!netif_running(dev))
-		return -EINVAL;
-
-	if (!dev->phydev)
-		return -EINVAL;
-
-	return phy_mii_ioctl(dev->phydev, rq, cmd);
+	return phy_do_ioctl_running(dev, rq, cmd);
 }
 
 void cvm_oct_note_carrier(struct octeon_ethernet *priv,
@@ -81,15 +69,15 @@ void cvm_oct_adjust_link(struct net_device *dev)
 	struct octeon_ethernet *priv = netdev_priv(dev);
 	union cvmx_helper_link_info link_info;
 
+	if (WARN_ON(!dev->phydev))
+		return;
+
 	link_info.u64		= 0;
 	link_info.s.link_up	= dev->phydev->link ? 1 : 0;
-	link_info.s.full_duplex = dev->phydev->duplex ? 1 : 0;
+	link_info.s.full_duplex	= dev->phydev->duplex ? 1 : 0;
 	link_info.s.speed	= dev->phydev->speed;
 	priv->link_info		= link_info.u64;
 
-	/*
-	 * The polling task need to know about link status changes.
-	 */
 	if (priv->poll)
 		priv->poll(dev);
 
@@ -100,13 +88,13 @@ void cvm_oct_adjust_link(struct net_device *dev)
 	}
 }
 
-int cvm_oct_common_stop(struct net_device *dev)
+void cvm_oct_common_stop(struct net_device *dev)
 {
 	struct octeon_ethernet *priv = netdev_priv(dev);
 	int interface = INTERFACE(priv->port);
+	int index = INDEX(priv->port);
 	union cvmx_helper_link_info link_info;
 	union cvmx_gmxx_prtx_cfg gmx_cfg;
-	int index = INDEX(priv->port);
 
 	gmx_cfg.u64 = cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
 	gmx_cfg.s.en = 0;
@@ -114,26 +102,21 @@ int cvm_oct_common_stop(struct net_device *dev)
 
 	priv->poll = NULL;
 
+	netif_carrier_off(dev);
+
 	if (dev->phydev)
 		phy_disconnect(dev->phydev);
 
 	if (priv->last_link) {
-		link_info.u64 = 0;
-		priv->last_link = 0;
+		link_info.u64	= 0;
+		priv->link_info	= 0;
+		priv->last_link	= 0;
 
 		cvmx_helper_link_set(priv->port, link_info);
 		cvm_oct_note_carrier(priv, link_info);
 	}
-	return 0;
 }
 
-/**
- * cvm_oct_phy_setup_device - setup the PHY
- *
- * @dev:    Device to setup
- *
- * Returns Zero on success, negative on failure
- */
 int cvm_oct_phy_setup_device(struct net_device *dev)
 {
 	struct octeon_ethernet *priv = netdev_priv(dev);
@@ -149,8 +132,8 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
 	if (!phy_node)
 		goto no_phy;
 
-	phydev = of_phy_connect(dev, phy_node, cvm_oct_adjust_link, 0,
-				priv->phy_mode);
+	phydev = of_phy_connect(dev, phy_node, cvm_oct_adjust_link,
+				CVM_OCT_PHY_FLAGS, priv->phy_mode);
 	of_node_put(phy_node);
 
 	if (!phydev)
@@ -160,10 +143,9 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
 	phy_start(phydev);
 
 	return 0;
+
 no_phy:
-	/* If there is no phy, assume a direct MAC connection and that
-	 * the link is up.
-	 */
 	netif_carrier_on(dev);
 	return 0;
 }
+
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[Greg KH]

On Sun, May 17, 2026 at 09:19:55AM -0500, shayderrr wrote:
> From: Pranav Bajjuri <darknessshayder@gmail.com>
> 
> Add netif_carrier_off() to cvm_oct_common_stop(), guard
> cvm_oct_adjust_link() against null phydev, replace open-coded
> ioctl checks with phy_do_ioctl_running(), and zero priv->link_info
> on stop alongside last_link.

I am sorry, but I do not understand any of this.  Please always only do
one logical thing per patch.  This should be a patch series, right?

And have you tested this?

thanks,

greg k-h

Re: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[kernel test robot]

Hi shayderrr,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on staging/staging-next staging/staging-linus linus/master v7.1-rc3 next-20260508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/shayderrr/net-octeon-fix-carrier-state-null-guard-and-modernize-phy-ioctl/20260517-222121
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260517141955.79666-1-darknessshayder%40gmail.com
patch subject: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20260518/202605180212.LjnBydnh-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260518/202605180212.LjnBydnh-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605180212.LjnBydnh-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/staging/octeon/ethernet-mdio.c:91:6: error: conflicting types for 'cvm_oct_common_stop'; have 'void(struct net_device *)'
      91 | void cvm_oct_common_stop(struct net_device *dev)
         |      ^~~~~~~~~~~~~~~~~~~
   In file included from drivers/staging/octeon/ethernet-mdio.c:15:
   drivers/staging/octeon/octeon-ethernet.h:90:5: note: previous declaration of 'cvm_oct_common_stop' with type 'int(struct net_device *)'
      90 | int cvm_oct_common_stop(struct net_device *dev);
         |     ^~~~~~~~~~~~~~~~~~~


vim +91 drivers/staging/octeon/ethernet-mdio.c

    90	
  > 91	void cvm_oct_common_stop(struct net_device *dev)
    92	{
    93		struct octeon_ethernet *priv = netdev_priv(dev);
    94		int interface = INTERFACE(priv->port);
    95		int index = INDEX(priv->port);
    96		union cvmx_helper_link_info link_info;
    97		union cvmx_gmxx_prtx_cfg gmx_cfg;
    98	
    99		gmx_cfg.u64 = cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
   100		gmx_cfg.s.en = 0;
   101		cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface), gmx_cfg.u64);
   102	
   103		priv->poll = NULL;
   104	
   105		netif_carrier_off(dev);
   106	
   107		if (dev->phydev)
   108			phy_disconnect(dev->phydev);
   109	
   110		if (priv->last_link) {
   111			link_info.u64	= 0;
   112			priv->link_info	= 0;
   113			priv->last_link	= 0;
   114	
   115			cvmx_helper_link_set(priv->port, link_info);
   116			cvm_oct_note_carrier(priv, link_info);
   117		}
   118	}
   119	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[kernel test robot]

Hi shayderrr,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on staging/staging-next staging/staging-linus linus/master v7.1-rc3 next-20260508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/shayderrr/net-octeon-fix-carrier-state-null-guard-and-modernize-phy-ioctl/20260517-222121
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260517141955.79666-1-darknessshayder%40gmail.com
patch subject: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20260518/202605180508.53ptUezJ-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260518/202605180508.53ptUezJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605180508.53ptUezJ-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/staging/octeon/ethernet-mdio.c:91:6: error: conflicting types for 'cvm_oct_common_stop'
      91 | void cvm_oct_common_stop(struct net_device *dev)
         |      ^
   drivers/staging/octeon/octeon-ethernet.h:90:5: note: previous declaration is here
      90 | int cvm_oct_common_stop(struct net_device *dev);
         |     ^
   1 error generated.


vim +/cvm_oct_common_stop +91 drivers/staging/octeon/ethernet-mdio.c

    90	
  > 91	void cvm_oct_common_stop(struct net_device *dev)
    92	{
    93		struct octeon_ethernet *priv = netdev_priv(dev);
    94		int interface = INTERFACE(priv->port);
    95		int index = INDEX(priv->port);
    96		union cvmx_helper_link_info link_info;
    97		union cvmx_gmxx_prtx_cfg gmx_cfg;
    98	
    99		gmx_cfg.u64 = cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
   100		gmx_cfg.s.en = 0;
   101		cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface), gmx_cfg.u64);
   102	
   103		priv->poll = NULL;
   104	
   105		netif_carrier_off(dev);
   106	
   107		if (dev->phydev)
   108			phy_disconnect(dev->phydev);
   109	
   110		if (priv->last_link) {
   111			link_info.u64	= 0;
   112			priv->link_info	= 0;
   113			priv->last_link	= 0;
   114	
   115			cvmx_helper_link_set(priv->port, link_info);
   116			cvm_oct_note_carrier(priv, link_info);
   117		}
   118	}
   119	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[Greg KH]

On Sun, May 17, 2026 at 11:44:20AM -0500, shxyder wrote:
> Hi Greg,
> 
> Thank you for the feedback. I will split this into separate patches, one
> change per patch, and resubmit as a series.
> 
> Regarding testing: I do not have access to OCTEON hardware, so I have not
> been able to runtime test this. The changes are straightforward — adding a
> missing netif_carrier_off(), a null guard, replacing phy_mii_ioctl() with
> phy_do_ioctl_running(), and zeroing a cached field on stop. I should have
> mentioned this upfront and I apologize for not doing so.

It also looks like you did not test-build your patches, which is
generally always a requirement :)

thanks,

greg k-h

Re: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[Dan Carpenter]

On Sun, May 17, 2026 at 09:19:55AM -0500, shayderrr wrote:
> From: Pranav Bajjuri <darknessshayder@gmail.com>
> 
> Add netif_carrier_off() to cvm_oct_common_stop(),

Why?  Shouldn't this be in it's own patch?  Do we need a Fixes tag?

> guard
> cvm_oct_adjust_link() against null phydev,

Can this really happen or is this AI advice?  Shouldn't this be in
it's own patch?  Do we need a Fixes tag?  You have added a
WARN_ON() which basically means reboot the kernel on most systems
so it's discouraged as much as possible.

> replace open-coded
> ioctl checks with phy_do_ioctl_running(),

Put this in its own patch.

> and zero priv->link_info
> on stop alongside last_link.

Why?  Shouldn't this be in it's own patch?  Do we need a Fixes tag?

regards,
dan carpenter

Re: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[Dan Carpenter]

Oh flip.  This is octeon.  It's a MIPS thing.  No way you are
actually testing this.  I bet this is all AI.

regards,
dan carpenter

Re: [PATCH] net: octeon: fix carrier state, null guard, and modernize phy ioctl

[Greg KH]

On Mon, May 18, 2026 at 08:55:05AM +0300, Dan Carpenter wrote:
> Oh flip.  This is octeon.  It's a MIPS thing.  No way you are
> actually testing this.  I bet this is all AI.

Yeah, it didn't even build :(