On Tue, 2006-06-20 at 15:30 -0500, Steve Wise wrote:

> +/*
> + * Allocate TX ring elements and chain them together.
> + * One-to-one association of adapter descriptors with ring elements.
> + */
> +static int c2_tx_ring_alloc(struct c2_ring *tx_ring, void *vaddr,
> +                         dma_addr_t base, void __iomem * mmio_txp_ring)
> +{
> +     struct c2_tx_desc *tx_desc;
> +     struct c2_txp_desc __iomem *txp_desc;
> +     struct c2_element *elem;
> +     int i;
> +
> +     tx_ring->start = kmalloc(sizeof(*elem) * tx_ring->count, GFP_KERNEL);

I would think this needs a dma_alloc_coherent() rather than a kmalloc...


> +
> +/* Free all buffers in RX ring, assumes receiver stopped */
> +static void c2_rx_clean(struct c2_port *c2_port)
> +{
> +     struct c2_dev *c2dev = c2_port->c2dev;
> +     struct c2_ring *rx_ring = &c2_port->rx_ring;
> +     struct c2_element *elem;
> +     struct c2_rx_desc *rx_desc;
> +
> +     elem = rx_ring->start;
> +     do {
> +             rx_desc = elem->ht_desc;
> +             rx_desc->len = 0;
> +
> +             __raw_writew(0, elem->hw_desc + C2_RXP_STATUS);
> +             __raw_writew(0, elem->hw_desc + C2_RXP_COUNT);
> +             __raw_writew(0, elem->hw_desc + C2_RXP_LEN);

you seem to be a fan of the __raw_write() functions... any reason why?
__raw_ is not a magic "go faster" prefix....

Also on a related note, have you checked the driver for the needed PCI
posting flushes?

> +
> +     /* Disable IRQs by clearing the interrupt mask */
> +     writel(1, c2dev->regs + C2_IDIS);
> +     writel(0, c2dev->regs + C2_NIMR0);

like here...
> +
> +     elem = tx_ring->to_use;
> +     elem->skb = skb;
> +     elem->mapaddr = mapaddr;
> +     elem->maplen = maplen;
> +
> +     /* Tell HW to xmit */
> +     __raw_writeq(cpu_to_be64(mapaddr), elem->hw_desc + C2_TXP_ADDR);
> +     __raw_writew(cpu_to_be16(maplen), elem->hw_desc + C2_TXP_LEN);
> +     __raw_writew(cpu_to_be16(TXP_HTXD_READY), elem->hw_desc + C2_TXP_FLAGS);

or here

> +static int c2_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +     int ret = 0;
> +
> +     if (new_mtu < ETH_ZLEN || new_mtu > ETH_JUMBO_MTU)
> +             return -EINVAL;
> +
> +     netdev->mtu = new_mtu;
> +
> +     if (netif_running(netdev)) {
> +             c2_down(netdev);
> +
> +             c2_up(netdev);
> +     }

this looks odd...



-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to