Skip to contentSkip to conversation

New: sc92031 does not create a device link in sysfs

Bugme-new Applied Andrew Morton 4 participants 12 messages Apr 2, 2008 – Apr 12, 2008 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]/

#1 lore raw

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 37 lines · On Wed, 2 Apr 2008 07:30:35 -0700 (PDT) [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=10380

           Summary: sc92031 does not create a device link in sysfs
           Product: Drivers
           Version: 2.5
     KernelVersion: 2.6.24
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Network
        AssignedTo: [email protected]
        ReportedBy: [email protected]

Latest working kernel version:
Earliest failing kernel version:
Distribution: Gentoo
Hardware Environment: x86, x86_64
Software Environment:
Problem Description:
there is no 'device' link created for the sc92031 nic in sysfs under its respective directory '/sys/class/net/eth?' thus making it impossible to search up the device tree using udev.
In my special case, persistent udev net rules cannot be generated by the respective gentoo udev script due to its dependency upon the udev 'DEVICES' key which cannot be determined because of the impossibility to search up the tree. Steps to reproduce:
1) install silan sc92031 (or realtek 8139D) nic in your pc
2) recompile and install kernel 2.6.24 (sysfs enabled) with sc92031 driver either in-kernel or as a module
3) reboot or load sc92031 module
4) make sure sysfs is mounted on /sys
5) look into the respective '/sys/class/net/eth?' directory

#2 lore raw

PATCH 1/1 sc92031: sysfs link missing

Add missing sysfs device association. Compile tested only -ENOHW.

drivers/net/sc92031.c +1 -0
@@ -1446,6 +1446,7 @@ static int __devinit sc92031_probe(struc
 	}
 
 	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &hw->pdev->dev);
 
 #if SC92031_USE_BAR == 0
 	dev->mem_start = pci_resource_start(pdev, SC92031_USE_BAR);
Applied
#3 lore raw

Re: [PATCH] sc92031: sysfs link missing

Quoted 8 lines · Stephen Hemminger

Add missing sysfs device association. Compile tested only -ENOHW.

drivers/net/sc92031.c static int __devinit sc92031_probe(struc · line 1452

Quoted 7 lines
drivers/net/sc92031.c +1 -0
@@ -1446,6 +1446,7 @@ static int __devinit sc92031_probe(struc
 	}
 
 	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &hw->pdev->dev);
 
 #if SC92031_USE_BAR == 0
 	dev->mem_start = pci_resource_start(pdev, SC92031_USE_BAR);

applied

#4 lore raw

Re: [PATCH] sc92031: sysfs link missing

Quoted 11 lines · On Fri, 04 Apr 2008 01:46:05 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 10 lines · Stephen Hemminger

Add missing sysfs device association. Compile tested only -ENOHW.

drivers/net/sc92031.c +0 -0
@@ -1446,6 +1446,7 @@ static int __devinit sc92031_probe(struc
 	}
 
Quoted 8 lines

drivers/net/sc92031.c static int __devinit sc92031_probe(struc · line 1452

Quoted 5 lines
drivers/net/sc92031.c +1 -0
 	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &hw->pdev->dev);
 
 #if SC92031_USE_BAR == 0
 	dev->mem_start = pci_resource_start(pdev, SC92031_USE_BAR);

applied

That's version 1. There was a super-optimised version 2:

From: Stephen Hemminger <[email protected]>

Add missing sysfs device association. Compile tested only -ENOHW.

Addresses http://bugzilla.kernel.org/show_bug.cgi?id=10380

---

 drivers/net/sc92031.c |    1 +
 1 file changed, 1 insertion(+)
drivers/net/sc92031.c renamed from drivers/net/sc92031.c~sc92031-sysfs-link-missing +1 -0
diff -puN drivers/net/sc92031.c~sc92031-sysfs-link-missing drivers/net/sc92031.c
@@ -1446,6 +1446,7 @@ static int __devinit sc92031_probe(struc
 	}
 
 	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
 
 #if SC92031_USE_BAR == 0
 	dev->mem_start = pci_resource_start(pdev, SC92031_USE_BAR);

_

#5 lore raw

Re: [PATCH] sc92031: sysfs link missing

Quoted 45 lines · Andrew Morton
Quoted 11 lines · On Fri, 04 Apr 2008 01:46:05 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 10 lines · Stephen Hemminger

Add missing sysfs device association. Compile tested only -ENOHW.

drivers/net/sc92031.c +0 -0
@@ -1446,6 +1446,7 @@ static int __devinit sc92031_probe(struc
 	}
 
Quoted 7 lines

drivers/net/sc92031.c static int __devinit sc92031_probe(struc · line 1452

Quoted 5 lines
drivers/net/sc92031.c +1 -0
 	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &hw->pdev->dev);
 
 #if SC92031_USE_BAR == 0
 	dev->mem_start = pci_resource_start(pdev, SC92031_USE_BAR);

applied

That's version 1. There was a super-optimised version 2:

From: Stephen Hemminger <[email protected]>

Add missing sysfs device association. Compile tested only -ENOHW.

Addresses http://bugzilla.kernel.org/show_bug.cgi?id=10380

---

 drivers/net/sc92031.c |    1 +
 1 file changed, 1 insertion(+)

diff -puN drivers/net/sc92031.c~sc92031-sysfs-link-missing drivers/net/sc92031.c

drivers/net/sc92031.c static int __devinit sc92031_probe(struc · line 1452

Quoted 7 lines
drivers/net/sc92031.c renamed from drivers/net/sc92031.c~sc92031-sysfs-link-missing +1 -0
@@ -1446,6 +1446,7 @@ static int __devinit sc92031_probe(struc
 	}
 
 	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
 
 #if SC92031_USE_BAR == 0
 	dev->mem_start = pci_resource_start(pdev, SC92031_USE_BAR);

Thanks!

After I wrote "applied", compile testing kicked it out, so I dropped it from the batch sent to DaveM just now.

Not even compile tested apparently..

Jeff

#6 lore raw
In reply to Andrew Morton

Add missing sysfs device association. Compile tested only -ENOHW.

---
Patch against 2.6.25-rc8, ignore previous patch.

drivers/net/sc92031.c +1 -0
@@ -1446,6 +1446,7 @@ static int __devinit sc92031_probe(struc
 	}
 
 	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
 
 #if SC92031_USE_BAR == 0
 	dev->mem_start = pci_resource_start(pdev, SC92031_USE_BAR);
#7 lore raw

PATCH 1/3 sc92031: use net_device stats

Statistics structure is available for use in net_device structure. Compile tested only.

---
Patch against 2.6.25-rc8 with previous SET_NETDEV_DEV patch applied.

drivers/net/sc92031.c +34 -36
@@ -311,7 +311,6 @@ struct sc92031_priv {
 
 	/* for dev->get_stats */
 	long			rx_value;
-	struct net_device_stats	stats;
 };
 
 /* I don't know which registers can be safely read; however, I can guess
@@ -421,7 +420,7 @@ static void _sc92031_tx_clear(struct net
 
 	while (priv->tx_head - priv->tx_tail > 0) {
 		priv->tx_tail++;
-		priv->stats.tx_dropped++;
+		dev->stats.tx_dropped++;
 	}
 	priv->tx_head = priv->tx_tail = 0;
 }
@@ -676,27 +675,27 @@ static void _sc92031_tx_tasklet(struct n
 		priv->tx_tail++;
 
 		if (tx_status & TxStatOK) {
-			priv->stats.tx_bytes += tx_status & 0x1fff;
-			priv->stats.tx_packets++;
+			dev->stats.tx_bytes += tx_status & 0x1fff;
+			dev->stats.tx_packets++;
 			/* Note: TxCarrierLost is always asserted at 100mbps. */
-			priv->stats.collisions += (tx_status >> 22) & 0xf;
+			dev->stats.collisions += (tx_status >> 22) & 0xf;
 		}
 
 		if (tx_status & (TxOutOfWindow | TxAborted)) {
-			priv->stats.tx_errors++;
+			dev->stats.tx_errors++;
 
 			if (tx_status & TxAborted)
-				priv->stats.tx_aborted_errors++;
+				dev->stats.tx_aborted_errors++;
 
 			if (tx_status & TxCarrierLost)
-				priv->stats.tx_carrier_errors++;
+				dev->stats.tx_carrier_errors++;
 
 			if (tx_status & TxOutOfWindow)
-				priv->stats.tx_window_errors++;
+				dev->stats.tx_window_errors++;
 		}
 
 		if (tx_status & TxUnderrun)
-			priv->stats.tx_fifo_errors++;
+			dev->stats.tx_fifo_errors++;
 	}
 
 	if (priv->tx_tail != old_tx_tail)
@@ -704,27 +703,29 @@ static void _sc92031_tx_tasklet(struct n
 			netif_wake_queue(dev);
 }
 
-static void _sc92031_rx_tasklet_error(u32 rx_status,
-		struct sc92031_priv *priv, unsigned rx_size)
+static void _sc92031_rx_tasklet_error(struct net_device *dev,
+				      u32 rx_status, unsigned rx_size)
 {
 	if(rx_size > (MAX_ETH_FRAME_SIZE + 4) || rx_size < 16) {
-		priv->stats.rx_errors++;
-		priv->stats.rx_length_errors++;
+		dev->stats.rx_errors++;
+		dev->stats.rx_length_errors++;
 	}
 
 	if (!(rx_status & RxStatesOK)) {
-		priv->stats.rx_errors++;
+		dev->stats.rx_errors++;
 
 		if (rx_status & (RxHugeFrame | RxSmallFrame))
-			priv->stats.rx_length_errors++;
+			dev->stats.rx_length_errors++;
 
 		if (rx_status & RxBadAlign)
-			priv->stats.rx_frame_errors++;
+			dev->stats.rx_frame_errors++;
 
 		if (!(rx_status & RxCRCOK))
-			priv->stats.rx_crc_errors++;
-	} else
+			dev->stats.rx_crc_errors++;
+	} else {
+		struct sc92031_priv *priv = netdev_priv(dev);
 		priv->rx_loss++;
+	}
 }
 
 static void _sc92031_rx_tasklet(struct net_device *dev)
@@ -783,7 +784,7 @@ static void _sc92031_rx_tasklet(struct n
 				|| rx_size > (MAX_ETH_FRAME_SIZE + 4)
 				|| rx_size < 16
 				|| !(rx_status & RxStatesOK))) {
-			_sc92031_rx_tasklet_error(rx_status, priv, rx_size);
+			_sc92031_rx_tasklet_error(dev, rx_status, rx_size);
 			break;
 		}
 
@@ -818,11 +819,11 @@ static void _sc92031_rx_tasklet(struct n
 		dev->last_rx = jiffies;
 		netif_rx(skb);
 
-		priv->stats.rx_bytes += pkt_size;
-		priv->stats.rx_packets++;
+		dev->stats.rx_bytes += pkt_size;
+		dev->stats.rx_packets++;
 
 		if (rx_status & Rx_Multicast)
-			priv->stats.multicast++;
+			dev->stats.multicast++;
 
 	next:
 		rx_ring_offset = (rx_ring_offset + rx_size_align) % RX_BUF_LEN;
@@ -835,13 +836,11 @@ static void _sc92031_rx_tasklet(struct n
 
 static void _sc92031_link_tasklet(struct net_device *dev)
 {
-	struct sc92031_priv *priv = netdev_priv(dev);
-
 	if (_sc92031_check_media(dev))
 		netif_wake_queue(dev);
 	else {
 		netif_stop_queue(dev);
-		priv->stats.tx_carrier_errors++;
+		dev->stats.tx_carrier_errors++;
 	}
 }
 
@@ -866,11 +865,11 @@ static void sc92031_tasklet(unsigned lon
 		_sc92031_rx_tasklet(dev);
 
 	if (intr_status & RxOverflow)
-		priv->stats.rx_errors++;
+		dev->stats.rx_errors++;
 
 	if (intr_status & TimeOut) {
-		priv->stats.rx_errors++;
-		priv->stats.rx_length_errors++;
+		dev->stats.rx_errors++;
+		dev->stats.rx_length_errors++;
 	}
 
 	if (intr_status & (LinkFail | LinkOK))
@@ -936,15 +935,14 @@ static struct net_device_stats *sc92031_
 
 		if (temp == 0xffff) {
 			priv->rx_value += temp;
-			priv->stats.rx_fifo_errors = priv->rx_value;
-		} else {
-			priv->stats.rx_fifo_errors = temp + priv->rx_value;
-		}
+			dev->stats.rx_fifo_errors = priv->rx_value;
+		} else
+			dev->stats.rx_fifo_errors = temp + priv->rx_value;
 
 		spin_unlock_bh(&priv->lock);
 	}
 
-	return &priv->stats;
+	return &dev->stats;
 }
 
 static int sc92031_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -959,7 +957,7 @@ static int sc92031_start_xmit(struct sk_
 
 	if (unlikely(skb->len > TX_BUF_SIZE)) {
 		err = -EMSGSIZE;
-		priv->stats.tx_dropped++;
+		dev->stats.tx_dropped++;
 		goto out;
 	}
 
@@ -967,7 +965,7 @@ static int sc92031_start_xmit(struct sk_
 
 	if (unlikely(!netif_carrier_ok(dev))) {
 		err = -ENOLINK;
-		priv->stats.tx_dropped++;
+		dev->stats.tx_dropped++;
 		goto out_unlock;
 	}
 
#8 lore raw

PATCH 1/3 sc92031: use net_device stats

In reply to Stephen Hemminger

Statistics structure is available for use in net_device structure. Compile tested only.

---
Patch against 2.6.25-rc8 with previous SET_NETDEV_DEV patch applied.

drivers/net/sc92031.c +34 -36
@@ -311,7 +311,6 @@ struct sc92031_priv {
 
 	/* for dev->get_stats */
 	long			rx_value;
-	struct net_device_stats	stats;
 };
 
 /* I don't know which registers can be safely read; however, I can guess
@@ -421,7 +420,7 @@ static void _sc92031_tx_clear(struct net
 
 	while (priv->tx_head - priv->tx_tail > 0) {
 		priv->tx_tail++;
-		priv->stats.tx_dropped++;
+		dev->stats.tx_dropped++;
 	}
 	priv->tx_head = priv->tx_tail = 0;
 }
@@ -676,27 +675,27 @@ static void _sc92031_tx_tasklet(struct n
 		priv->tx_tail++;
 
 		if (tx_status & TxStatOK) {
-			priv->stats.tx_bytes += tx_status & 0x1fff;
-			priv->stats.tx_packets++;
+			dev->stats.tx_bytes += tx_status & 0x1fff;
+			dev->stats.tx_packets++;
 			/* Note: TxCarrierLost is always asserted at 100mbps. */
-			priv->stats.collisions += (tx_status >> 22) & 0xf;
+			dev->stats.collisions += (tx_status >> 22) & 0xf;
 		}
 
 		if (tx_status & (TxOutOfWindow | TxAborted)) {
-			priv->stats.tx_errors++;
+			dev->stats.tx_errors++;
 
 			if (tx_status & TxAborted)
-				priv->stats.tx_aborted_errors++;
+				dev->stats.tx_aborted_errors++;
 
 			if (tx_status & TxCarrierLost)
-				priv->stats.tx_carrier_errors++;
+				dev->stats.tx_carrier_errors++;
 
 			if (tx_status & TxOutOfWindow)
-				priv->stats.tx_window_errors++;
+				dev->stats.tx_window_errors++;
 		}
 
 		if (tx_status & TxUnderrun)
-			priv->stats.tx_fifo_errors++;
+			dev->stats.tx_fifo_errors++;
 	}
 
 	if (priv->tx_tail != old_tx_tail)
@@ -704,27 +703,29 @@ static void _sc92031_tx_tasklet(struct n
 			netif_wake_queue(dev);
 }
 
-static void _sc92031_rx_tasklet_error(u32 rx_status,
-		struct sc92031_priv *priv, unsigned rx_size)
+static void _sc92031_rx_tasklet_error(struct net_device *dev,
+				      u32 rx_status, unsigned rx_size)
 {
 	if(rx_size > (MAX_ETH_FRAME_SIZE + 4) || rx_size < 16) {
-		priv->stats.rx_errors++;
-		priv->stats.rx_length_errors++;
+		dev->stats.rx_errors++;
+		dev->stats.rx_length_errors++;
 	}
 
 	if (!(rx_status & RxStatesOK)) {
-		priv->stats.rx_errors++;
+		dev->stats.rx_errors++;
 
 		if (rx_status & (RxHugeFrame | RxSmallFrame))
-			priv->stats.rx_length_errors++;
+			dev->stats.rx_length_errors++;
 
 		if (rx_status & RxBadAlign)
-			priv->stats.rx_frame_errors++;
+			dev->stats.rx_frame_errors++;
 
 		if (!(rx_status & RxCRCOK))
-			priv->stats.rx_crc_errors++;
-	} else
+			dev->stats.rx_crc_errors++;
+	} else {
+		struct sc92031_priv *priv = netdev_priv(dev);
 		priv->rx_loss++;
+	}
 }
 
 static void _sc92031_rx_tasklet(struct net_device *dev)
@@ -783,7 +784,7 @@ static void _sc92031_rx_tasklet(struct n
 				|| rx_size > (MAX_ETH_FRAME_SIZE + 4)
 				|| rx_size < 16
 				|| !(rx_status & RxStatesOK))) {
-			_sc92031_rx_tasklet_error(rx_status, priv, rx_size);
+			_sc92031_rx_tasklet_error(dev, rx_status, rx_size);
 			break;
 		}
 
@@ -818,11 +819,11 @@ static void _sc92031_rx_tasklet(struct n
 		dev->last_rx = jiffies;
 		netif_rx(skb);
 
-		priv->stats.rx_bytes += pkt_size;
-		priv->stats.rx_packets++;
+		dev->stats.rx_bytes += pkt_size;
+		dev->stats.rx_packets++;
 
 		if (rx_status & Rx_Multicast)
-			priv->stats.multicast++;
+			dev->stats.multicast++;
 
 	next:
 		rx_ring_offset = (rx_ring_offset + rx_size_align) % RX_BUF_LEN;
@@ -835,13 +836,11 @@ static void _sc92031_rx_tasklet(struct n
 
 static void _sc92031_link_tasklet(struct net_device *dev)
 {
-	struct sc92031_priv *priv = netdev_priv(dev);
-
 	if (_sc92031_check_media(dev))
 		netif_wake_queue(dev);
 	else {
 		netif_stop_queue(dev);
-		priv->stats.tx_carrier_errors++;
+		dev->stats.tx_carrier_errors++;
 	}
 }
 
@@ -866,11 +865,11 @@ static void sc92031_tasklet(unsigned lon
 		_sc92031_rx_tasklet(dev);
 
 	if (intr_status & RxOverflow)
-		priv->stats.rx_errors++;
+		dev->stats.rx_errors++;
 
 	if (intr_status & TimeOut) {
-		priv->stats.rx_errors++;
-		priv->stats.rx_length_errors++;
+		dev->stats.rx_errors++;
+		dev->stats.rx_length_errors++;
 	}
 
 	if (intr_status & (LinkFail | LinkOK))
@@ -936,15 +935,14 @@ static struct net_device_stats *sc92031_
 
 		if (temp == 0xffff) {
 			priv->rx_value += temp;
-			priv->stats.rx_fifo_errors = priv->rx_value;
-		} else {
-			priv->stats.rx_fifo_errors = temp + priv->rx_value;
-		}
+			dev->stats.rx_fifo_errors = priv->rx_value;
+		} else
+			dev->stats.rx_fifo_errors = temp + priv->rx_value;
 
 		spin_unlock_bh(&priv->lock);
 	}
 
-	return &priv->stats;
+	return &dev->stats;
 }
 
 static int sc92031_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -959,7 +957,7 @@ static int sc92031_start_xmit(struct sk_
 
 	if (unlikely(skb->len > TX_BUF_SIZE)) {
 		err = -EMSGSIZE;
-		priv->stats.tx_dropped++;
+		dev->stats.tx_dropped++;
 		goto out;
 	}
 
@@ -967,7 +965,7 @@ static int sc92031_start_xmit(struct sk_
 
 	if (unlikely(!netif_carrier_ok(dev))) {
 		err = -ENOLINK;
-		priv->stats.tx_dropped++;
+		dev->stats.tx_dropped++;
 		goto out_unlock;
 	}
 
#9 lore raw

Re: [PATCH 1/3] sc92031: use net_device stats

Quoted 7 lines · Stephen Hemminger

Statistics structure is available for use in net_device structure. Compile tested only.

---
Patch against 2.6.25-rc8 with previous SET_NETDEV_DEV patch applied.

applied 1-3

#10 lore raw

PATCH 2/3 sc92031: start transmit return value bugfix

In reply to Stephen Hemminger

Any negative return value from start_xmit is interpreted as NETDEV_TX_LOCK which is not what this driver wants. It should return 0 (NETDEV_TX_OK) when it consumes a packet.

Also, use skb_padto() as the generic way to pad small frames.

drivers/net/sc92031.c +4 -10
@@ -947,16 +947,16 @@ static struct net_device_stats *sc92031_
 
 static int sc92031_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	int err = 0;
 	struct sc92031_priv *priv = netdev_priv(dev);
 	void __iomem *port_base = priv->port_base;
-
 	unsigned len;
 	unsigned entry;
 	u32 tx_status;
 
+	if (skb_padto(skb, ETH_ZLEN))
+		return NETDEV_TX_OK;
+
 	if (unlikely(skb->len > TX_BUF_SIZE)) {
-		err = -EMSGSIZE;
 		dev->stats.tx_dropped++;
 		goto out;
 	}
@@ -964,7 +964,6 @@ static int sc92031_start_xmit(struct sk_
 	spin_lock(&priv->lock);
 
 	if (unlikely(!netif_carrier_ok(dev))) {
-		err = -ENOLINK;
 		dev->stats.tx_dropped++;
 		goto out_unlock;
 	}
@@ -976,11 +975,6 @@ static int sc92031_start_xmit(struct sk_
 	skb_copy_and_csum_dev(skb, priv->tx_bufs + entry * TX_BUF_SIZE);
 
 	len = skb->len;
-	if (unlikely(len < ETH_ZLEN)) {
-		memset(priv->tx_bufs + entry * TX_BUF_SIZE + len,
-				0, ETH_ZLEN - len);
-		len = ETH_ZLEN;
-	}
 
 	wmb();
 
@@ -1007,7 +1001,7 @@ out_unlock:
 out:
 	dev_kfree_skb(skb);
 
-	return err;
+	return NETDEV_TX_OK;
 }
 
 static int sc92031_open(struct net_device *dev)
#11 lore raw

PATCH 3/3 sc92031: use netdev_alloc_skb

In reply to Stephen Hemminger

Use netdev_alloc_skb since it handles any NUMA node memory localtion issues and sets skb->dev. Since device driver was not setting skb->dev, I bet filter rules based on device would not work.

drivers/net/sc92031.c +1 -1
@@ -796,7 +796,7 @@ static void _sc92031_rx_tasklet(struct n
 
 		rx_len -= rx_size_align + 4;
 
-		skb = dev_alloc_skb(pkt_size + NET_IP_ALIGN);
+		skb = netdev_alloc_skb(dev, pkt_size + NET_IP_ALIGN);
 		if (unlikely(!skb)) {
 			if (printk_ratelimit())
 				printk(KERN_ERR "%s: Couldn't allocate a skb_buff for a packet of size %u\n",