* [patch] 2.6.0-test4 - more sis190 nit I/II
2003-08-26 20:51 [bk] 2.6.x net driver queue snapshot Jeff Garzik
@ 2003-08-26 22:07 ` Francois Romieu
2003-08-26 22:24 ` Jeff Garzik
2003-08-26 22:14 ` [patch] 2.6.0-test4 - more sis190 nit II/II Francois Romieu
1 sibling, 1 reply; 4+ messages in thread
From: Francois Romieu @ 2003-08-26 22:07 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
Jeff Garzik <jgarzik@pobox.com> :
[....]
> <romieu@fr.zoreil.com> (03/08/26 1.1293)
> [netdrvr sis190] pass irq argument to synchronize_irq()
>
> Looks like this driver wasn't tested on SMP :)
The following patch should apply on top of this. Compiled, untested.
Driver does not need to enforce 256 byte alignment for data returned
from pci_alloc_consistent().
- {rx/tx}_dma_aligned and {rx/td}_dma_raw are both replaced by {rx/tx}_dma;
- {rx/tx}_desc_raw is replaced by direct use of {Rx/Tx}DescArray;
- SiS190_open()
+ fixup for a lack of kmalloc() failure handling;
+ (return status) there is no need for both retval/rc: merge them;
+ anonymous printk() fixup: the name of the guilty device is printed;
- define {RX/TX}_DESC_TOTAL_SIZE because I am too lazy to read twice the
same lengthy arithmetic expression.
drivers/net/sis190.c | 78 ++++++++++++++++++++++-----------------------------
1 files changed, 35 insertions(+), 43 deletions(-)
diff -puN drivers/net/sis190.c~redundant-alignment-sis190 drivers/net/sis190.c
--- linux-2.6.0-test4/drivers/net/sis190.c~redundant-alignment-sis190 Tue Aug 26 21:55:14 2003
+++ linux-2.6.0-test4-fr/drivers/net/sis190.c Tue Aug 26 22:22:09 2003
@@ -76,6 +76,8 @@ static int multicast_filter_limit = 32;
#define NUM_TX_DESC 64 /* Number of Tx descriptor registers */
#define NUM_RX_DESC 64 /* Number of Rx descriptor registers */
+#define TX_DESC_TOTAL_SIZE ((NUM_TX_DESC * sizeof (struct TxDesc)) + 256)
+#define RX_DESC_TOTAL_SIZE ((NUM_RX_DESC * sizeof (struct RxDesc)) + 256)
#define RX_BUF_SIZE 1536 /* Rx Buffer size */
#define SiS190_MIN_IO_SIZE 0x80
@@ -311,12 +313,8 @@ struct sis190_private {
unsigned long cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
unsigned long cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
unsigned long dirty_tx;
- void *tx_desc_raw; /* Tx descriptor buffer */
- dma_addr_t tx_dma_raw;
- dma_addr_t tx_dma_aligned;
- void *rx_desc_raw; /* Rx descriptor buffer */
- dma_addr_t rx_dma_raw;
- dma_addr_t rx_dma_aligned;
+ dma_addr_t tx_dma;
+ dma_addr_t rx_dma;
struct TxDesc *TxDescArray; /* Index of 256-alignment Tx Descriptor buffer */
struct RxDesc *RxDescArray; /* Index of 256-alignment Rx Descriptor buffer */
unsigned char *RxBufferRings; /* Index of Rx Buffer */
@@ -714,54 +712,50 @@ static int
SiS190_open(struct net_device *dev)
{
struct sis190_private *tp = dev->priv;
- int retval;
- u8 diff;
int rc;
- retval =
- request_irq(dev->irq, SiS190_interrupt, SA_SHIRQ, dev->name, dev);
- if (retval) {
- return retval;
- }
+ rc = request_irq(dev->irq, SiS190_interrupt, SA_SHIRQ, dev->name, dev);
+ if (rc)
+ goto out;
- tp->tx_desc_raw = pci_alloc_consistent(tp->pci_dev,
- (NUM_TX_DESC * sizeof (struct TxDesc)) + 256,
- &tp->tx_dma_raw);
- if (!tp->tx_desc_raw) {
+ /*
+ * Rx and Tx descriptors need 256 bytes alignment.
+ * pci_alloc_consistent() guarantees a stronger alignment.
+ */
+ tp->TxDescArray = pci_alloc_consistent(tp->pci_dev, TX_DESC_TOTAL_SIZE,
+ &tp->tx_dma);
+ if (!tp->TxDescArray) {
rc = -ENOMEM;
goto err_out;
}
- // Tx Desscriptor needs 256 bytes alignment;
- diff = 256 - (tp->tx_dma_raw - ((tp->tx_dma_raw >> 8) << 8));
- tp->tx_dma_aligned = tp->tx_dma_raw + diff;
- tp->TxDescArray = (struct TxDesc *) (tp->tx_desc_raw + diff);
-
- tp->rx_desc_raw = pci_alloc_consistent(tp->pci_dev,
- (NUM_RX_DESC * sizeof (struct RxDesc)) + 256,
- &tp->rx_dma_raw);
- if (!tp->rx_desc_raw) {
+
+ tp->RxDescArray = pci_alloc_consistent(tp->pci_dev, RX_DESC_TOTAL_SIZE,
+ &tp->rx_dma);
+ if (!tp->RxDescArray) {
rc = -ENOMEM;
goto err_out_free_tx;
}
- // Rx Desscriptor needs 256 bytes alignment;
- diff = 256 - (tp->rx_dma_raw - ((tp->rx_dma_raw >> 8) << 8));
- tp->rx_dma_aligned = tp->rx_dma_raw + diff;
- tp->RxDescArray = (struct RxDesc *) (tp->rx_desc_raw + diff);
tp->RxBufferRings = kmalloc(RX_BUF_SIZE * NUM_RX_DESC, GFP_KERNEL);
if (tp->RxBufferRings == NULL) {
- printk(KERN_INFO "Allocate RxBufferRing failed\n");
+ printk(KERN_INFO "%s: allocate RxBufferRing failed\n",
+ dev->name);
+ rc = -ENOMEM;
+ goto err_out_free_rx;
}
SiS190_init_ring(dev);
SiS190_hw_start(dev);
- return 0;
+out:
+ return rc;
+err_out_free_rx:
+ pci_free_consistent(tp->pci_dev, RX_DESC_TOTAL_SIZE, tp->RxDescArray,
+ tp->rx_dma);
err_out_free_tx:
- pci_free_consistent(tp->pci_dev,
- (NUM_TX_DESC * sizeof (struct TxDesc)) + 256,
- tp->tx_desc_raw, tp->tx_dma_raw);
+ pci_free_consistent(tp->pci_dev, TX_DESC_TOTAL_SIZE, tp->TxDescArray,
+ tp->tx_dma);
err_out:
free_irq(dev->irq, dev);
return rc;
@@ -780,10 +774,10 @@ SiS190_hw_start(struct net_device *dev)
SiS_W32(IntrControl, 0x0);
SiS_W32(0x0, 0x01a00);
- SiS_W32(0x4, tp->tx_dma_aligned);
+ SiS_W32(0x4, tp->tx_dma);
SiS_W32(0x10, 0x1a00);
- SiS_W32(0x14, tp->rx_dma_aligned);
+ SiS_W32(0x14, tp->rx_dma);
SiS_W32(0x20, 0xffffffff);
SiS_W32(0x24, 0x0);
@@ -1115,12 +1109,10 @@ SiS190_close(struct net_device *dev)
free_irq(dev->irq, dev);
SiS190_tx_clear(tp);
- pci_free_consistent(tp->pci_dev,
- (NUM_TX_DESC * sizeof (struct TxDesc)) + 256,
- tp->tx_desc_raw, tp->tx_dma_raw);
- pci_free_consistent(tp->pci_dev,
- (NUM_RX_DESC * sizeof (struct RxDesc)) + 256,
- tp->rx_desc_raw, tp->rx_dma_raw);
+ pci_free_consistent(tp->pci_dev, TX_DESC_TOTAL_SIZE, tp->TxDescArray,
+ tp->tx_dma);
+ pci_free_consistent(tp->pci_dev, RX_DESC_TOTAL_SIZE, tp->RxDescArray,
+ tp->rx_dma);
tp->TxDescArray = NULL;
tp->RxDescArray = NULL;
kfree(tp->RxBufferRings);
_
^ permalink raw reply [flat|nested] 4+ messages in thread* [patch] 2.6.0-test4 - more sis190 nit II/II
2003-08-26 20:51 [bk] 2.6.x net driver queue snapshot Jeff Garzik
2003-08-26 22:07 ` [patch] 2.6.0-test4 - more sis190 nit I/II Francois Romieu
@ 2003-08-26 22:14 ` Francois Romieu
1 sibling, 0 replies; 4+ messages in thread
From: Francois Romieu @ 2003-08-26 22:14 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
Jeff Garzik <jgarzik@pobox.com> :
[...]
> <jgarzik@redhat.com> (03/08/26 1.1298)
> [netdrvr sis190] make driver depend on CONFIG_BROKEN
>
> Until RX path is cleaned up to use PCI DMA API and
> not virt_to_bus.
>
> <jgarzik@redhat.com> (03/08/26 1.1297)
> [netdrvr sis190] convert TX path to use PCI DMA API
>
> Also, minor changes:
> * mark ->hard_start_xmit ETH_ZLEN test as unlikely()
> * use cpu_to_le32() and le32_to_cpu() in TX path
> * fix two leak in error path, in ->hard_start_xmit
> * don't test netif_queue_stopped() in TX completion path,
> netif_wake_queue() already does that.
>
Is plain old patch for the previous items available somewhere ? I had just
finished that (no more work on it before 24h):
Missing pieces for DMA-API.
- SiS190_init_ring: the global area for the received data is mapped.
This area is persistent during the whole driver's life.
It only needs to be unmapped in SiS190_close() as no other exit/error
path exists.
- SiS190_rx_interrupt: no map/unmap for received data buffer. A single
sync operation is done. Btw, there is no need to store the same value
in RxDescArray[cur_rx].buf_addr over and over again.
- SiS190_start_xmit: transmitted data buffer is mapped.
- SiS190_tx_clear/SiS190_tx_interrupt: transmitted data buffer is unmapped.
drivers/net/sis190.c | 109 ++++++++++++++++++++++++++-------------------------
1 files changed, 57 insertions(+), 52 deletions(-)
diff -puN drivers/net/sis190.c~sis190-dma-mapping drivers/net/sis190.c
--- linux-2.6.0-test4/drivers/net/sis190.c~sis190-dma-mapping Tue Aug 26 22:28:01 2003
+++ linux-2.6.0-test4-fr/drivers/net/sis190.c Wed Aug 27 00:00:54 2003
@@ -824,19 +824,19 @@ SiS190_init_ring(struct net_device *dev)
tp->Tx_skbuff[i] = NULL;
}
for (i = 0; i < NUM_RX_DESC; i++) {
+ struct RxDesc *desc = tp->RxDescArray + i;
- tp->RxDescArray[i].PSize = 0x0;
+ desc->PSize = 0x0;
if (i == (NUM_RX_DESC - 1))
- tp->RxDescArray[i].buf_Len = BIT_31 + RX_BUF_SIZE; //bit 31 is End bit
+ desc->buf_Len = BIT_31 + RX_BUF_SIZE; //bit 31 is End bit
else
- tp->RxDescArray[i].buf_Len = RX_BUF_SIZE;
-
-#warning Replace virt_to_bus with DMA mapping
- tp->RxBufferRing[i] = &(tp->RxBufferRings[i * RX_BUF_SIZE]);
- tp->RxDescArray[i].buf_addr = virt_to_bus(tp->RxBufferRing[i]);
- tp->RxDescArray[i].status = OWNbit | INTbit;
+ desc->buf_Len = RX_BUF_SIZE;
+ tp->RxBufferRing[i] = tp->RxBufferRings + i * RX_BUF_SIZE;
+ desc->buf_addr = pci_map_single(tp->pci_dev,
+ tp->RxBufferRing[i], RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
+ desc->status = OWNbit | INTbit;
}
}
@@ -848,8 +848,14 @@ SiS190_tx_clear(struct sis190_private *t
tp->cur_tx = 0;
for (i = 0; i < NUM_TX_DESC; i++) {
- if (tp->Tx_skbuff[i] != NULL) {
- dev_kfree_skb(tp->Tx_skbuff[i]);
+ struct sk_buff *skb = tp->Tx_skbuff[i];
+
+ if (skb != NULL) {
+ pci_unmap_single(tp->pci_dev,
+ tp->TxDescArray[i].buf_addr,
+ (skb->len > ETH_ZLEN) ? skb->len : ETH_ZLEN,
+ PCI_DMA_TODEVICE);
+ dev_kfree_skb(skb);
tp->Tx_skbuff[i] = NULL;
tp->stats.tx_dropped++;
}
@@ -888,6 +894,7 @@ SiS190_start_xmit(struct sk_buff *skb, s
struct sis190_private *tp = dev->priv;
void *ioaddr = tp->mmio_addr;
int entry = tp->cur_tx % NUM_TX_DESC;
+ struct TxDesc *desc = tp->TxDescArray + entry;
if (skb->len < ETH_ZLEN) {
skb = skb_padto(skb, ETH_ZLEN);
@@ -897,23 +904,17 @@ SiS190_start_xmit(struct sk_buff *skb, s
spin_lock_irq(&tp->lock);
- if ((tp->TxDescArray[entry].status & OWNbit) == 0) {
-#warning Replace virt_to_bus with DMA mapping
+ if ((desc->status & OWNbit) == 0) {
tp->Tx_skbuff[entry] = skb;
- tp->TxDescArray[entry].buf_addr = virt_to_bus(skb->data);
- tp->TxDescArray[entry].PSize =
- ((skb->len > ETH_ZLEN) ? skb->len : ETH_ZLEN);
-
- if (entry != (NUM_TX_DESC - 1)) {
- tp->TxDescArray[entry].buf_Len =
- tp->TxDescArray[entry].PSize;
- } else {
- tp->TxDescArray[entry].buf_Len =
- tp->TxDescArray[entry].PSize | ENDbit;
- }
+ desc->buf_addr = pci_map_single(tp->pci_dev, skb->data,
+ desc->PSize, PCI_DMA_TODEVICE);
+ desc->PSize = ((skb->len > ETH_ZLEN) ? skb->len : ETH_ZLEN);
+
+ desc->buf_Len = desc->PSize;
+ if (entry == (NUM_TX_DESC - 1))
+ desc->buf_Len |= ENDbit;
- tp->TxDescArray[entry].status |=
- (OWNbit | INTbit | DEFbit | CRCbit | PADbit);
+ desc->status |= (OWNbit | INTbit | DEFbit | CRCbit | PADbit);
SiS_W32(TxControl, 0x1a11); //Start Send
@@ -946,10 +947,17 @@ SiS190_tx_interrupt(struct net_device *d
tx_left = tp->cur_tx - dirty_tx;
while (tx_left > 0) {
- if ((tp->TxDescArray[entry].status & OWNbit) == 0) {
- dev_kfree_skb_irq(tp->
- Tx_skbuff[dirty_tx % NUM_TX_DESC]);
- tp->Tx_skbuff[dirty_tx % NUM_TX_DESC] = NULL;
+ const int dirty = dirty_tx % NUM_TX_DESC;
+ struct TxDesc *desc = tp->TxDescArray + dirty;
+
+ if ((desc->status & OWNbit) == 0) {
+ struct sk_buff *skb = tp->Tx_skbuff[dirty];
+
+ pci_unmap_single(tp->pci_dev, desc->buf_addr,
+ (skb->len > ETH_ZLEN) ? skb->len : ETH_ZLEN,
+ PCI_DMA_TODEVICE);
+ dev_kfree_skb_irq(skb);
+ tp->Tx_skbuff[dirty] = NULL;
tp->stats.tx_packets++;
dirty_tx++;
tx_left--;
@@ -968,29 +976,30 @@ static void
SiS190_rx_interrupt(struct net_device *dev, struct sis190_private *tp,
void *ioaddr)
{
- int cur_rx;
- struct sk_buff *skb;
- int pkt_size = 0;
+ int cur_rx = tp->cur_rx;
+ struct RxDesc *desc = tp->RxDescArray + cur_rx;
assert(dev != NULL);
assert(tp != NULL);
assert(ioaddr != NULL);
- cur_rx = tp->cur_rx;
- while ((tp->RxDescArray[cur_rx].status & OWNbit) == 0) {
+ while ((desc->status & OWNbit) == 0) {
- if (tp->RxDescArray[cur_rx].PSize & 0x0080000) {
+ if (desc->PSize & 0x0080000) {
printk(KERN_INFO "%s: Rx ERROR!!!\n", dev->name);
tp->stats.rx_errors++;
tp->stats.rx_length_errors++;
- } else if (!(tp->RxDescArray[cur_rx].PSize & 0x0010000)) {
+ } else if (!(desc->PSize & 0x0010000)) {
printk(KERN_INFO "%s: Rx ERROR!!!\n", dev->name);
tp->stats.rx_errors++;
tp->stats.rx_crc_errors++;
} else {
- pkt_size =
- (int) (tp->RxDescArray[cur_rx].
- PSize & 0x0000FFFF) - 4;
+ struct sk_buff *skb;
+ int pkt_size;
+
+ pkt_size = (int) (desc->PSize & 0x0000FFFF) - 4;
+ pci_dma_sync_single(tp->pci_dev, desc->buf_addr,
+ RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
skb = dev_alloc_skb(pkt_size + 2);
if (skb != NULL) {
skb->dev = dev;
@@ -1001,24 +1010,18 @@ SiS190_rx_interrupt(struct net_device *d
skb->protocol = eth_type_trans(skb, dev);
netif_rx(skb);
- tp->RxDescArray[cur_rx].PSize = 0x0;
+ desc->PSize = 0x0;
if (cur_rx == (NUM_RX_DESC - 1))
- tp->RxDescArray[cur_rx].buf_Len =
- ENDbit + RX_BUF_SIZE;
+ desc->buf_Len = ENDbit + RX_BUF_SIZE;
else
- tp->RxDescArray[cur_rx].buf_Len =
- RX_BUF_SIZE;
+ desc->buf_Len = RX_BUF_SIZE;
-#warning Replace virt_to_bus with DMA mapping
- tp->RxDescArray[cur_rx].buf_addr =
- virt_to_bus(tp->RxBufferRing[cur_rx]);
dev->last_rx = jiffies;
tp->stats.rx_bytes += pkt_size;
tp->stats.rx_packets++;
- tp->RxDescArray[cur_rx].status =
- OWNbit | INTbit;
+ desc->status = OWNbit | INTbit;
} else {
printk(KERN_WARNING
"%s: Memory squeeze, deferring packet.\n",
@@ -1030,7 +1033,7 @@ SiS190_rx_interrupt(struct net_device *d
}
cur_rx = (cur_rx + 1) % NUM_RX_DESC;
-
+ desc = tp->RxDescArray + cur_rx;
}
tp->cur_rx = cur_rx;
@@ -1114,11 +1117,13 @@ SiS190_close(struct net_device *dev)
pci_free_consistent(tp->pci_dev, RX_DESC_TOTAL_SIZE, tp->RxDescArray,
tp->rx_dma);
tp->TxDescArray = NULL;
- tp->RxDescArray = NULL;
- kfree(tp->RxBufferRings);
for (i = 0; i < NUM_RX_DESC; i++) {
+ pci_unmap_single(tp->pci_dev, tp->RxDescArray[i].buf_addr,
+ RX_BUF_SIZE, PCI_DMA_FROMDEVICE);
tp->RxBufferRing[i] = NULL;
}
+ tp->RxDescArray = NULL;
+ kfree(tp->RxBufferRings);
return 0;
}
_
^ permalink raw reply [flat|nested] 4+ messages in thread