New: sc92031 does not create a device link in sysfs
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]/
(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
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 );
Jeff Garzik
AppliedRe: [PATCH] sc92031: sysfs link missing
Quoted 8 lines · Stephen Hemminger
Add missing sysfs device association. Compile tested only
-ENOHW.Signed-off-by Stephen Hemminger <[email protected]>
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
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.Signed-off-by Stephen Hemminger <[email protected]>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
-0pci_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
@@ -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 );
_
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.Signed-off-by Stephen Hemminger <[email protected]>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
-0pci_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
Signed-off-by Stephen Hemminger <[email protected]>Cc Jeff Garzik <[email protected]>Cc Cesar Eduardo Barros <[email protected]>Cc <[email protected]>Signed-off-by Andrew Morton <[email protected]>---
drivers/net/sc92031.c | 1 + 1 file changed, 1 insertion(+)diff -puN
drivers/net/sc92031.c~sc92031-sysfs-link-missingdrivers/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
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 );
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 netwhile (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 nnetif_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 nstatic 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; }
PATCH 1/3 sc92031: use net_device stats
In reply to Stephen HemmingerStatistics 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 netwhile (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 nnetif_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 nstatic 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; }
Re: [PATCH 1/3] sc92031: use net_device stats
Quoted 7 lines · Stephen Hemminger
Statistics structure is available for use in
net_devicestructure. Compile tested only.Signed-off-by Stephen Hemminger <[email protected]>---
Patch against 2.6.25-rc8 with previousSET_NETDEV_DEVpatch applied.
applied 1-3
PATCH 2/3 sc92031: start transmit return value bugfix
In reply to Stephen HemmingerAny 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)
PATCH 3/3 sc92031: use netdev_alloc_skb
In reply to Stephen HemmingerUse 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",
Quoted 7 lines · Stephen Hemminger
Add missing sysfs device association. Compile tested only
-ENOHW.Signed-off-by Stephen Hemminger <[email protected]>---
Patch against 2.6.25-rc8, ignore previous patch....
Works for me!
Thank you, that was fast.