[PATCH] net: ethernet: sunplus: Switch to ndo_eth_ioctl

[PATCH] net: ethernet: sunplus: Switch to ndo_eth_ioctl

[Yeking]

From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>

ndo_do_ioctl is no longer called by the device ioctl handler,
so use ndo_eth_ioctl instead.

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
 drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sunplus/spl2sw_driver.c b/drivers/net/ethernet/sunplus/spl2sw_driver.c
index 721d8ed3f302..5e0e4c9ecbb0 100644
--- a/drivers/net/ethernet/sunplus/spl2sw_driver.c
+++ b/drivers/net/ethernet/sunplus/spl2sw_driver.c
@@ -199,7 +199,7 @@ static const struct net_device_ops netdev_ops = {
 	.ndo_start_xmit = spl2sw_ethernet_start_xmit,
 	.ndo_set_rx_mode = spl2sw_ethernet_set_rx_mode,
 	.ndo_set_mac_address = spl2sw_ethernet_set_mac_address,
-	.ndo_do_ioctl = phy_do_ioctl,
+	.ndo_eth_ioctl = phy_do_ioctl,
 	.ndo_tx_timeout = spl2sw_ethernet_tx_timeout,
 };
 
-- 
2.43.0

Re: [PATCH] net: ethernet: sunplus: Switch to ndo_eth_ioctl

[Jakub Kicinski]

On Wed,  8 Jan 2025 14:12:38 +0000 Yeking@Red54.com wrote:
> From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> 
> ndo_do_ioctl is no longer called by the device ioctl handler,
> so use ndo_eth_ioctl instead.

I presume this used to work and now it doesn't?
If so a Fixes tag pointing to the commit that broke it would 
be in order.

Please also mention how you tested this. Is this something you actually
run into on real HW? Or just found by code inspection and only compile
tested?
-- 
pw-bot: cr

[net PATCH v2] net: ethernet: sunplus: Switch to ndo_eth_ioctl

[Yeking]

From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>

ndo_do_ioctl is no longer called by the device ioctl handler,
so use ndo_eth_ioctl instead. (found by code inspection)

Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021")
Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
V1 -> V2: Update commit message

 drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sunplus/spl2sw_driver.c b/drivers/net/ethernet/sunplus/spl2sw_driver.c
index 721d8ed3f302..5e0e4c9ecbb0 100644
--- a/drivers/net/ethernet/sunplus/spl2sw_driver.c
+++ b/drivers/net/ethernet/sunplus/spl2sw_driver.c
@@ -199,7 +199,7 @@ static const struct net_device_ops netdev_ops = {
 	.ndo_start_xmit = spl2sw_ethernet_start_xmit,
 	.ndo_set_rx_mode = spl2sw_ethernet_set_rx_mode,
 	.ndo_set_mac_address = spl2sw_ethernet_set_mac_address,
-	.ndo_do_ioctl = phy_do_ioctl,
+	.ndo_eth_ioctl = phy_do_ioctl,
 	.ndo_tx_timeout = spl2sw_ethernet_tx_timeout,
 };
 
-- 
2.43.0

Re: [net PATCH v2] net: ethernet: sunplus: Switch to ndo_eth_ioctl

[Andrew Lunn]

On Thu, Jan 09, 2025 at 02:05:52AM +0000, Yeking@Red54.com wrote:
> From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> 
> ndo_do_ioctl is no longer called by the device ioctl handler,
> so use ndo_eth_ioctl instead. (found by code inspection)

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

There are a couple of things which would of been nice to add, but the
patch can be accepted anyway.

One is a bit more details in the commit message about when
ndo_do_ioctl is actually used, just to help make it clearer why the
code is wrong and the patch is correct. You could of quoted
a76053707dbf:

    Most users of ndo_do_ioctl are ethernet drivers that implement
    the MII commands SIOCGMIIPHY/SIOCGMIIREG/SIOCSMIIREG, or hardware
    timestamping with SIOCSHWTSTAMP/SIOCGHWTSTAMP.
    
    Separate these from the few drivers that use ndo_do_ioctl to
    implement SIOCBOND, SIOCBR and SIOCWANDEV commands.

Also, if i find a bug like this, i often wounder if there are more
instances of the same bug somewhere else. I did a quick grep and it
does seem to be the only case. If you did the same check, it would be
nice to mention this in the commit message, maybe below the --- marker
so it does not become part of the permanent history in git.

    Andrew

Re: [net PATCH v2] net: ethernet: sunplus: Switch to ndo_eth_ioctl

[Jakub Kicinski]

On Thu,  9 Jan 2025 02:05:52 +0000 Yeking@Red54.com wrote:
> From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> 
> ndo_do_ioctl is no longer called by the device ioctl handler,
> so use ndo_eth_ioctl instead. (found by code inspection)
> 
> Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021")

This tag is unnecessary, Fixes tag should point to the commit which
_broke_ things.

> Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl")

Now that you added this tag you need to run get_maintainer again /
correctly on the patch and CC the authors.
-- 
pw-bot: cr

[PATCH net v3] net: ethernet: sunplus: Switch to ndo_eth_ioctl

[Yeking]

From: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>

The device ioctl handler no longer calls ndo_do_ioctl, but calls
ndo_eth_ioctl to handle mii ioctls. However, sunplus still used
ndo_do_ioctl when it was introduced. So switch to ndo_eth_ioctl. (found
by code inspection)

Fixes: fd3040b9394c ("net: ethernet: Add driver for Sunplus SP7021", 2022-05-08)
Fixes: a76053707dbf ("dev_ioctl: split out ndo_eth_ioctl", 2021-07-27)
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
V2 -> V3: Update commit message again, and add short author date to the
Fixes tag to make it clear at a glance and reduce misunderstandings.
V1 -> V2: Update commit message

To Jakub Kicinski:
The old Fixes tag style is at least 10 years old. It lacks date
information, which can lead to misjudgment. So I added short author date
to avoid this. However, if you disagree, I can change it back to the old
Fixes tag style.

 drivers/net/ethernet/sunplus/spl2sw_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sunplus/spl2sw_driver.c b/drivers/net/ethernet/sunplus/spl2sw_driver.c
index 721d8ed3f302..5e0e4c9ecbb0 100644
--- a/drivers/net/ethernet/sunplus/spl2sw_driver.c
+++ b/drivers/net/ethernet/sunplus/spl2sw_driver.c
@@ -199,7 +199,7 @@ static const struct net_device_ops netdev_ops = {
 	.ndo_start_xmit = spl2sw_ethernet_start_xmit,
 	.ndo_set_rx_mode = spl2sw_ethernet_set_rx_mode,
 	.ndo_set_mac_address = spl2sw_ethernet_set_mac_address,
-	.ndo_do_ioctl = phy_do_ioctl,
+	.ndo_eth_ioctl = phy_do_ioctl,
 	.ndo_tx_timeout = spl2sw_ethernet_tx_timeout,
 };
 
-- 
2.43.0