net: axienet: Fix random driver issues.

PATCH In review Richard Cochran 3 participants 11 messages Dec 16, 2019 – Dec 20, 2019

Subsystems (4)

Networking Network drivers Arm/Zynq architecture Xilinx AXI Ethernet driver

lore

Share & tools

lore

Richard Cochran

author

2019-12-16 18:33 UTC #0 lore raw

cover net: axienet: Fix random driver issues.

While working on the PTP P2P 1-step series, my hardware under test just happened to be connected with an axienet MAC. This series address three unrelated issues in the driver.

Thanks,
Richard

Richard Cochran (3):
net: axienet: Propagate registration errors during probe.
net: axienet: Support software transmit time stamping.
net: axienet: Pass ioctls to the phy.

 .../net/ethernet/xilinx/xilinx_axienet_main.c | 32 ++++++++++++++++---
 1 file changed, 28 insertions(+), 4 deletions(-)

Signature · 7 lines

2.20.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Richard Cochran

author

2019-12-16 18:33 UTC #1 lore raw

PATCH 1/3 net: axienet: Propagate registration errors during probe.

The function, axienet_mdio_setup(), calls of_mdiobus_register() which might return EDEFER_PROBE. However, this error is not propagated to the driver's probe method, and so deferred registration cannot happen. This patch fixes the issue by handling the error code properly.

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +4 -3

index 20746b801959..53644abe52da 100644
@@ -1835,9 +1835,10 @@ static int axienet_probe(struct platform_device *pdev)
 		}
 
 		ret = axienet_mdio_setup(lp);
-		if (ret)
-			dev_warn(&pdev->dev,
-				 "error registering MDIO bus: %d\n", ret);
+		if (ret) {
+			dev_err(&pdev->dev, "error registering MDIO bus\n");
+			goto free_netdev;
+		}
 	}
 
 	lp->phylink_config.dev = &ndev->dev;

Signature · 1 line

2.20.1

Radhey Shyam Pandey

2019-12-17 06:19 UTC #2 lore raw

RE: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 10 lines

-----Original Message-----
From: Richard Cochran <[email protected]>
Sent: Tuesday, December 17, 2019 12:03 AM
To: [email protected]
Cc: [email protected]; David Miller <[email protected]>; Michal Simek <[email protected]>; Radhey Shyam Pandey <[email protected]>
Subject: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 2 lines

The function, axienet_mdio_setup(), calls of_mdiobus_register() which might return EDEFER_PROBE. However, this error is not propagated to

EPROBE_DEFER. In which scenario we are hitting probe_defer?

Quoted 14 lines

the driver's probe method, and so deferred registration cannot happen. This patch fixes the issue by handling the error code properly.
Signed-off-by Richard Cochran <[email protected]>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/net/ethernet/xilinx/xilinx_axienet_main.c +0 -0
index 20746b801959..53644abe52da 100644

drivers/net/ethernet/xilinx/xilinx_axienet_main.c static int axienet_probe(struct platform_device · line 1841

Quoted 10 lines

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +2 -3

@@ -1835,9 +1835,10 @@ static int axienet_probe(struct platform_device
 *pdev)
 		}
 
 		ret = axienet_mdio_setup(lp);
-		if (ret)
-			dev_warn(&pdev->dev,
-				 "error registering MDIO bus: %d\n", ret);
+		if (ret) {
+			dev_err(&pdev->dev, "error registering MDIO
 bus\n");

Error handling for _mdio_setup failure seems ok. Minor nit- skip printing message " error registering MDIO bus" for defer case.

Quoted 7 lines

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +2 -0
+			goto free_netdev;
+		}
 	}
lp->phylink_config.dev = &ndev->dev;
--
2.20.1

Richard Cochran

author

2019-12-17 15:49 UTC #3 lore raw

Re: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 10 lines · Radhey Shyam Pandey

Quoted 10 lines
-----Original Message-----
From: Richard Cochran <[email protected]>
Sent: Tuesday, December 17, 2019 12:03 AM
To: [email protected]
Cc: [email protected]; David Miller <[email protected]>; Michal Simek <[email protected]>; Radhey Shyam Pandey <[email protected]>
Subject: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 3 lines

Quoted 2 lines
The function, axienet_mdio_setup(), calls of_mdiobus_register() which might return EDEFER_PROBE. However, this error is not propagated to
EPROBE_DEFER. In which scenario we are hitting probe_defer?

Did you see the cover letter? I am referring to this series:

16.Dec'19 [PATCH V6 net-next 00/11] Peer to Peer One-Step time stamping

Thanks,
Richard

Radhey Shyam Pandey

2019-12-19 18:13 UTC #4 lore raw

RE: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 20 lines

-----Original Message-----
From: Richard Cochran <[email protected]>
Sent: Tuesday, December 17, 2019 9:20 PM
To: Radhey Shyam Pandey <[email protected]>
Cc: [email protected]; [email protected]; David Miller <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.
Quoted 10 lines · Radhey Shyam Pandey
Quoted 10 lines
-----Original Message-----
From: Richard Cochran <[email protected]>
Sent: Tuesday, December 17, 2019 12:03 AM
To: [email protected]
Cc: [email protected]; David Miller <[email protected]>; Michal Simek <[email protected]>; Radhey Shyam Pandey <[email protected]>
Subject: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 5 lines

Quoted 3 lines
Quoted 2 lines
The function, axienet_mdio_setup(), calls of_mdiobus_register() which might return EDEFER_PROBE. However, this error is not propagated to
EPROBE_DEFER. In which scenario we are hitting probe_defer?
Did you see the cover letter? I am referring to this series:

I mean in which scenario we are hitting of_mdiobus_register defer? The series you mentioned talks about one-step TS.

Quoted 5 lines

16.Dec'19 [PATCH V6 net-next 00/11] Peer to Peer One-Step time stamping
Thanks,
Richard

Richard Cochran

author

2019-12-20 05:19 UTC #5 lore raw

Re: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 1 line · Radhey Shyam Pandey

I mean in which scenario we are hitting of_mdiobus_register defer?

of_mdiobus_register_phy() returns EPROBE_DEFER.

Thanks,
Richard

Radhey Shyam Pandey

2019-12-20 05:51 UTC #6 lore raw

RE: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 9 lines

-----Original Message-----
From: Richard Cochran <[email protected]>
Sent: Friday, December 20, 2019 10:50 AM
To: Radhey Shyam Pandey <[email protected]>
Cc: [email protected]; [email protected]; David Miller <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [PATCH net-next 1/3] net: axienet: Propagate registration errors during probe.

Quoted 4 lines

Quoted 1 line · Radhey Shyam Pandey
I mean in which scenario we are hitting of_mdiobus_register defer?
of_mdiobus_register_phy() returns EPROBE_DEFER.

Thanks. For defer we can skip "error registering MDIO bus" reporting.

Quoted 3 lines

Thanks,
Richard

Richard Cochran

author

2019-12-16 18:33 UTC #7 lore raw

PATCH 2/3 net: axienet: Support software transmit time stamping.

In reply to Richard Cochran

MAC drivers are expected to invoke the transmit time stamping hook in order to support both software time stamping and PHY time stamping. This patch adds the missing hook. In addition, drivers calling netif_rx() should first check for PHY time stamping by calling skb_defer_rx_timestamp().

Signed-off-by Richard Cochran <[email protected]>

---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +6 -1

index 53644abe52da..05fa7371c39a 100644
@@ -675,6 +675,9 @@ axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	cur_p->skb = skb;
 
 	tail_p = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * lp->tx_bd_tail;
+
+	skb_tx_timestamp(skb);
+
 	/* Start the transfer */
 	axienet_dma_out32(lp, XAXIDMA_TX_TDESC_OFFSET, tail_p);
 	if (++lp->tx_bd_tail >= lp->tx_bd_num)

@@ -736,7 +739,8 @@ static void axienet_recv(struct net_device *ndev)
 			skb->ip_summed = CHECKSUM_COMPLETE;
 		}
 
-		netif_rx(skb);
+		if (!skb_defer_rx_timestamp(skb))
+			netif_rx(skb);
 
 		size += length;
 		packets++;

@@ -1367,6 +1371,7 @@ static const struct ethtool_ops axienet_ethtool_ops = {
 	.set_pauseparam = axienet_ethtools_set_pauseparam,
 	.get_coalesce   = axienet_ethtools_get_coalesce,
 	.set_coalesce   = axienet_ethtools_set_coalesce,
+	.get_ts_info	= ethtool_op_get_ts_info,
 	.get_link_ksettings = axienet_ethtools_get_link_ksettings,
 	.set_link_ksettings = axienet_ethtools_set_link_ksettings,
 };

Signature · 1 line

2.20.1

Richard Cochran

author

2019-12-16 18:33 UTC #8 lore raw

PATCH 3/3 net: axienet: Pass ioctls to the phy.

In reply to Richard Cochran

In order to allow PHY drivers to handle ioctls, the MAC driver must pass the calls through. However, the axienet driver does not support ioctls at all. This patch fixes the issue by handing off the invocations to the PHY appropriately.

Signed-off-by Richard Cochran <[email protected]>

---
 .../net/ethernet/xilinx/xilinx_axienet_main.c  | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +18 -0

index 05fa7371c39a..d0b996f220f5 100644
@@ -1067,6 +1067,23 @@ static int axienet_change_mtu(struct net_device *ndev, int new_mtu)
 	return 0;
 }
 
+static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
+{
+	if (!netif_running(dev))
+		return -EINVAL;
+
+	switch (cmd) {
+	case SIOCGMIIPHY:
+	case SIOCGMIIREG:
+	case SIOCSMIIREG:
+	case SIOCSHWTSTAMP:
+	case SIOCGHWTSTAMP:
+		return phy_mii_ioctl(dev->phydev, rq, cmd);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 /**
  * axienet_poll_controller - Axi Ethernet poll mechanism.

@@ -1095,6 +1112,7 @@ static const struct net_device_ops axienet_netdev_ops = {
 	.ndo_set_mac_address = netdev_set_mac_address,
 	.ndo_validate_addr = eth_validate_addr,
 	.ndo_set_rx_mode = axienet_set_multicast_list,
+	.ndo_do_ioctl = axienet_ioctl,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = axienet_poll_controller,
 #endif

Signature · 1 line

2.20.1

Radhey Shyam Pandey

2019-12-17 06:43 UTC #9 lore raw

RE: [PATCH net-next 3/3] net: axienet: Pass ioctls to the phy.

Quoted 29 lines

-----Original Message-----
From: Richard Cochran <[email protected]>
Sent: Tuesday, December 17, 2019 12:03 AM
To: [email protected]
Cc: [email protected]; David Miller <[email protected]>; Michal Simek <[email protected]>; Radhey Shyam Pandey <[email protected]>
Subject: [PATCH net-next 3/3] net: axienet: Pass ioctls to the phy.
In order to allow PHY drivers to handle ioctls, the MAC driver must pass the calls through. However, the axienet driver does not support ioctls at all. This patch fixes the issue by handing off the invocations to the PHY appropriately.
Signed-off-by Richard Cochran <[email protected]>
---
 .../net/ethernet/xilinx/xilinx_axienet_main.c  | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/net/ethernet/xilinx/xilinx_axienet_main.c +0 -0
index 05fa7371c39a..d0b996f220f5 100644
@@ -1067,6 +1067,23 @@ static int axienet_change_mtu(struct net_device
 *ndev, int new_mtu)
 	return 0;
 }
 

drivers/net/ethernet/xilinx/xilinx_axienet_main.c static int axienet_change_mtu(struct net_device · line 1080

Quoted 10 lines

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +10 -0

+static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
+{
+	if (!netif_running(dev))
+		return -EINVAL;
+
+	switch (cmd) {
+	case SIOCGMIIPHY:
+	case SIOCGMIIREG:
+	case SIOCSMIIREG:
+	case SIOCSHWTSTAMP:

For hw timestamp we are passing the request to phy?

drivers/net/ethernet/xilinx/xilinx_axienet_main.c static int axienet_change_mtu(struct net_device *ndev, int new_mtu) · line 1081

Quoted 2 lines

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +2 -0
+	case SIOCGHWTSTAMP:
+		return phy_mii_ioctl(dev->phydev, rq, cmd);

Driver migrated to phylink so now we have to use phylink_mii_ioctl.

Quoted 19 lines

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +5 -0

+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 /**

* axienet_poll_controller - Axi Ethernet poll mechanism.

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +1 -0

@@ -1095,6 +1112,7 @@ static const struct net_device_ops
 axienet_netdev_ops = {
 	.ndo_set_mac_address = netdev_set_mac_address,
 	.ndo_validate_addr = eth_validate_addr,
 	.ndo_set_rx_mode = axienet_set_multicast_list,
+	.ndo_do_ioctl = axienet_ioctl,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = axienet_poll_controller,

#endif
--
2.20.1

Andrew Lunn

2019-12-17 10:59 UTC #10 lore raw

Re: [PATCH net-next 3/3] net: axienet: Pass ioctls to the phy.

In reply to Richard Cochran

Quoted 6 lines

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +6 -0

+static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
+{
+	if (!netif_running(dev))
+		return -EINVAL;
+
+	switch (cmd) {

drivers/net/ethernet/xilinx/xilinx_axienet_main.c static int axienet_change_mtu(struct net_device *ndev, int new_mtu) · line 1085

Quoted 10 lines

drivers/net/ethernet/xilinx/xilinx_axienet_main.c +10 -0

+	case SIOCGMIIPHY:
+	case SIOCGMIIREG:
+	case SIOCSMIIREG:
+	case SIOCSHWTSTAMP:
+	case SIOCGHWTSTAMP:
+		return phy_mii_ioctl(dev->phydev, rq, cmd);
+	default:
+		return -EOPNOTSUPP;
+	}
+}

Hi Richard

You don't need to be conditional. phy_mii_ioctl() and phylink_mii_ioctl() will return -EOPNOTSUPP for anything it does not support.

Andrew