Hi Wan,

describe what lock protects.

+};
+

+
+static void w90p910_init_desc(struct net_device *dev)
+{
+       struct w90p910_ether *ether;
+       struct w90p910_txbd  *tdesc, *tdesc_phys;
+       struct w90p910_rxbd  *rdesc, *rdesc_phys;
+       unsigned int i, j;
+
+       ether = netdev_priv(dev);
+
+       ether->tdesc = (struct tran_pdesc *)
+                       dma_alloc_coherent(NULL, sizeof(struct tran_pdesc),
+                               (dma_addr_t *) &ether->tdesc_phys, GFP_KERNEL);

no-need of casting I think.

+
+       ether->rdesc = (struct recv_pdesc *)
+                       dma_alloc_coherent(NULL, sizeof(struct recv_pdesc),
+                               (dma_addr_t *) &ether->rdesc_phys, GFP_KERNEL);

Ditto.

+
+       for (i = 0; i < TX_DESC_SIZE; i++) {
+               tdesc = &(ether->tdesc->desclist[i]);
+
+               j = ((i + 1) / TX_DESC_SIZE);
+
+               if (j != 0) {
+                       tdesc_phys = &(ether->tdesc_phys->desclist[0]);
+                       ether->start_tx_ptr = (unsigned int)tdesc_phys;
+                       tdesc->next = (unsigned int)ether->start_tx_ptr;
+               } else {
+                       tdesc_phys = &(ether->tdesc_phys->desclist[i+1]);
+                       tdesc->next = (unsigned int)tdesc_phys;
+               }
+
+               tdesc->buffer = (unsigned int)ether->tdesc_phys->tran_buf[i];
+               tdesc->sl = 0;
+               tdesc->mode = 0;
+       }
+
+       for (i = 0; i < RX_DESC_SIZE; i++) {
+               rdesc = &(ether->rdesc->desclist[i]);
+
+               j = ((i + 1) / RX_DESC_SIZE);
+
+               if (j != 0) {
+                       rdesc_phys = &(ether->rdesc_phys->desclist[0]);
+                       ether->start_rx_ptr = (unsigned int)rdesc_phys;
+                       rdesc->next = (unsigned int)ether->start_rx_ptr;
+               } else {
+                       rdesc_phys = &(ether->rdesc_phys->desclist[i+1]);
+                       rdesc->next = (unsigned int)rdesc_phys;
+               }
+
+               rdesc->sl = RX_OWEN_DMA;
+               rdesc->buffer = (unsigned int)ether->rdesc_phys->recv_buf[i];
+         }
+}
+
+static void w90p910_set_fifo_threshold(struct net_device *dev)
+{
+       struct w90p910_ether *ether = netdev_priv(dev);
+       unsigned int val;
+
+       val = TXTHD | BLENGTH;
+       __raw_writel(val, ether->reg + REG_FFTCR);
+}
+
+static void w90p910_return_default_idle(struct net_device *dev)
+{
+       struct w90p910_ether *ether = netdev_priv(dev);
+       unsigned int val;
+
+       val = __raw_readl(ether->reg + REG_MCMDR);
+       val |= SWR;
+       __raw_writel(val, ether->reg + REG_MCMDR);
+}
+
+static void w90p910_trigger_rx(struct net_device *dev)
+{
+       struct w90p910_ether *ether = netdev_priv(dev);
+
+       __raw_writel(ENSTART, ether->reg + REG_RSDR);
+}
+
+static void w90p910_trigger_tx(struct net_device *dev)
+{
+       struct w90p910_ether *ether = netdev_priv(dev);
+
+       __raw_writel(ENSTART, ether->reg + REG_TSDR);
+}
+
+static void w90p910_enable_mac_interrupt(struct net_device *dev)
+{
+       struct w90p910_ether *ether = netdev_priv(dev);
+       unsigned int val;
+
+       val = ENTXINTR | ENRXINTR | ENRXGD | ENTXCP;
+       val |= ENTXBERR | ENRXBERR | ENTXABT;
+
+       __raw_writel(val, ether->reg + REG_MIEN);
+}

+
+static int set_mac_address(struct net_device *dev, void *addr)
+{

how about naming it as w90p910_set_mac_address for consistency.

+       struct sockaddr *address = addr;
+
+       if (!is_valid_ether_addr(address->sa_data))
+               return -EADDRNOTAVAIL;
+
+       memcpy(dev->dev_addr, address->sa_data, dev->addr_len);
+       w90p910_write_cam(dev, CAM0, dev->dev_addr);
+
+       return 0;
+}
+

+
+static int w90p910_ether_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+       struct w90p910_ether *ether = netdev_priv(dev);
+
+       if (!(w90p910_send_frame(dev, skb->data, skb->len))) {
+               ether->skb = skb;
+               dev_kfree_skb_irq(skb);
+               return 0;
+       }
+       return -1;

better error code?

+}
+
+static irqreturn_t w90p910_tx_interrupt(int irq, void *dev_id)
+{
+       struct w90p910_ether *ether;
+       struct w90p910_txbd  *txbd;
+       struct platform_device *pdev;
+       struct tran_pdesc *tran_pdesc;
+       struct net_device *dev;
+       unsigned int cur_entry, entry, status;
+
+       dev = (struct net_device *)dev_id;

un-necessary casting.

+       ether = netdev_priv(dev);
+       pdev = ether->pdev;
+
+       spin_lock(&ether->lock);
+
+       w90p910_get_and_clear_int(dev, &status);
+
+       cur_entry = __raw_readl(ether->reg + REG_CTXDSA);
+
+       tran_pdesc = ether->tdesc_phys;
+       entry = (unsigned int)(&tran_pdesc->desclist[ether->finish_tx]);
+
+       while (entry != cur_entry) {
+               txbd = &ether->tdesc->desclist[ether->finish_tx];
+
+               ether->finish_tx = (ether->finish_tx + 1) % TX_DESC_SIZE;
+
+               if (txbd->sl & TXDS_TXCP) {
+                       ether->stats.tx_packets++;
+                       ether->stats.tx_bytes += txbd->sl & 0xFFFF;
+               } else {
+                       ether->stats.tx_errors++;
+               }
+
+               txbd->sl = 0x0;
+               txbd->mode = 0x0;
+
+               if (netif_queue_stopped(dev))
+                       netif_wake_queue(dev);
+
+               entry = (unsigned int)(&tran_pdesc->desclist[ether->finish_tx]);
+       }
+
+       if (status & MISTA_EXDEF) {
+               dev_err(&pdev->dev, "emc defer exceed interrupt\n");
+       } else if (status & MISTA_TXBERR) {
+                       dev_err(&pdev->dev, "emc bus error interrupt\n");
+                       w90p910_reset_mac(dev);
+               } else if (status & MISTA_TDU) {
+                               if (netif_queue_stopped(dev))
+                                       netif_wake_queue(dev);
+                       }

indentation doesn't look correct.

+
+       spin_unlock(&ether->lock);
+
+       return IRQ_HANDLED;
+}
+

+
+static irqreturn_t w90p910_rx_interrupt(int irq, void *dev_id)
+{
+       struct net_device *dev;
+       struct w90p910_ether  *ether;
+       struct platform_device *pdev;
+       unsigned int status;
+
+       dev = (struct net_device *)dev_id;

No need of casting.

+       ether = netdev_priv(dev);
+       pdev = ether->pdev;
+
+       spin_lock(&ether->lock);
+
+       w90p910_get_and_clear_int(dev, &status);
+
+       if (status & MISTA_RDU) {
+               netdev_rx(dev);
+
+               w90p910_trigger_rx(dev);
+
+               spin_unlock(&ether->lock);
+               return IRQ_HANDLED;
+       } else if (status & MISTA_RXBERR) {
+                       dev_err(&pdev->dev, "emc rx bus error\n");
+                       w90p910_reset_mac(dev);
+               }

indentation problem.

+
+       netdev_rx(dev);
+       spin_unlock(&ether->lock);
+       return IRQ_HANDLED;
+}
+
+static int w90p910_ether_open(struct net_device *dev)
+{
+       struct w90p910_ether *ether;
+       struct platform_device *pdev;
+
+       ether = netdev_priv(dev);
+       pdev = ether->pdev;
+
+       w90p910_reset_mac(dev);
+       w90p910_set_fifo_threshold(dev);
+       w90p910_set_curdest(dev);
+       w90p910_enable_cam(dev);
+       w90p910_enable_cam_command(dev);
+       w90p910_enable_mac_interrupt(dev);
+       w90p910_set_global_maccmd(dev);
+       w90p910_enable_rx(dev, 1);
+
+       ether->rx_packets = 0x0;
+       ether->rx_bytes = 0x0;
+
+       if (request_irq(ether->txirq, w90p910_tx_interrupt,
+                                               0x0, pdev->name, dev)) {
+               dev_err(&pdev->dev, "register irq tx failed\n");
+               return -EAGAIN;
+       }
+
+       if (request_irq(ether->rxirq, w90p910_rx_interrupt,
+                                               0x0, pdev->name, dev)) {
+               dev_err(&pdev->dev, "register irq rx failed\n");
+               return -EAGAIN;

error path problem. You should do free_irq(ether->txirq);

+       }
+
+       mod_timer(&ether->check_timer, jiffies + msecs_to_jiffies(1000));
+       netif_start_queue(dev);
+       w90p910_trigger_rx(dev);
+
+       dev_info(&pdev->dev, "%s is OPENED\n", dev->name);
+
+       return 0;
+}
+

+
+static int __devinit w90p910_ether_probe(struct platform_device *pdev)
+{
+       struct w90p910_ether *ether;
+       struct net_device *dev;
+       struct resource *res;
+       int error;
+
+       dev = alloc_etherdev(sizeof(struct w90p910_ether));
+       if (!dev)
+               return -ENOMEM;
+
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (res == NULL) {
+               dev_err(&pdev->dev, "failed to get I/O memory\n");
+               error = -ENXIO;
+               goto failed_free;
+       }
+
+       res = request_mem_region(res->start, resource_size(res), pdev->name);
+       if (res == NULL) {
+               dev_err(&pdev->dev, "failed to request I/O memory\n");
+               error = -EBUSY;
+               goto failed_free;
+       }
+
+       ether = netdev_priv(dev);
+
+       ether->reg = ioremap(res->start, resource_size(res));
+       if (ether->reg == NULL) {
+               dev_err(&pdev->dev, "failed to remap I/O memory\n");
+               error = -ENXIO;
+               goto failed_free_mem;
+       }
+
+       ether->txirq = platform_get_irq(pdev, 0);
+       if (ether->txirq < 0) {
+               dev_err(&pdev->dev, "failed to get ether tx irq\n");
+               error = -ENXIO;
+               goto failed_free_io;
+       }
+
+       ether->rxirq = platform_get_irq(pdev, 1);
+       if (ether->rxirq < 0) {
+               dev_err(&pdev->dev, "failed to get ether rx irq\n");
+               error = -ENXIO;
+               goto failed_free_txirq;
+       }
+
+       platform_set_drvdata(pdev, dev);
+
+       ether->clk = clk_get(&pdev->dev, NULL);
+       if (IS_ERR(ether->clk)) {
+               dev_err(&pdev->dev, "failed to get ether clock\n");
+               error = PTR_ERR(ether->clk);
+               goto failed_free_rxirq;
+       }
+
+       ether->rmiiclk = clk_get(&pdev->dev, "RMII");
+       if (IS_ERR(ether->rmiiclk)) {
+               dev_err(&pdev->dev, "failed to get ether clock\n");
+               error = PTR_ERR(ether->rmiiclk);
+               goto failed_put_clk;
+       }
+
+       ether->pdev = pdev;
+
+       w90p910_ether_setup(dev);
+
+       error = register_netdev(dev);
+       if (error != 0) {
+               dev_err(&pdev->dev, "Regiter EMC w90p910 FAILED\n");
+               error = -ENODEV;
+               goto failed_put_rmiiclk;
+       }
+
+       return 0;
+failed_put_rmiiclk:
+       clk_put(ether->rmiiclk);
+failed_put_clk:
+       clk_put(ether->clk);
+failed_free_rxirq:
+       free_irq(ether->rxirq, pdev);
+       platform_set_drvdata(pdev, NULL);
+failed_free_txirq:
+       free_irq(ether->txirq, pdev);
+failed_free_io:
+       iounmap(ether->reg);
+failed_free_mem:
+       release_mem_region(res->start, resource_size(res));
+failed_free:
+       free_netdev(dev);
+       return error;
+}
+
+static int __devexit w90p910_ether_remove(struct platform_device *pdev)
+{
+       struct net_device *dev = platform_get_drvdata(pdev);
+       struct w90p910_ether *ether = netdev_priv(dev);
+
+       unregister_netdev(dev);
+       clk_put(ether->rmiiclk);
+       clk_put(ether->clk);
+       del_timer_sync(&ether->check_timer);
+       platform_set_drvdata(pdev, NULL);
+       free_netdev(dev);

I don't see free_irq for two irqs.

+       return 0;
+}

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php