Message-ID: <5d5443650907161042g428afc74jf40f5daf71db7620@mail.gmail.com>
Date:	Thu, 16 Jul 2009 23:12:00 +0530
From:	Trilok Soni <soni.trilok@...il.com>
To:	Wan ZongShun <mcuos.com@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	linux-netdev <netdev@...r.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@...ts.arm.linux.org.uk>,
	"Eric.miao" <eric.y.miao@...il.com>
Subject: Re: [PATCH] Add mac driver for w90p910

Hi Wan,


> diff --git a/drivers/net/arm/Makefile b/drivers/net/arm/Makefile
> index 811a3cc..303171f 100644
> --- a/drivers/net/arm/Makefile
> +++ b/drivers/net/arm/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_ARM_AT91_ETHER)  += at91_ether.o
>  obj-$(CONFIG_ARM_KS8695_ETHER) += ks8695net.o
>  obj-$(CONFIG_EP93XX_ETH)       += ep93xx_eth.o
>  obj-$(CONFIG_IXP4XX_ETH)       += ixp4xx_eth.o
> +obj-$(CONFIG_W90P910_ETH)      += w90p910_ether.o
> diff --git a/drivers/net/arm/w90p910_ether.c b/drivers/net/arm/w90p910_ether.c
> new file mode 100644
> index 0000000..616fb79
> --- /dev/null
> +++ b/drivers/net/arm/w90p910_ether.c
> @@ -0,0 +1,1105 @@
> +/*
> + * Copyright (c) 2008-2009 Nuvoton technology corporation.
> + *
> + * Wan ZongShun <mcuos.com@...il.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation;version 2 of the License.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/mii.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/skbuff.h>
> +#include <linux/ethtool.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +
> +#define DRV_MODULE_NAME                "w90p910-emc"
> +#define DRV_MODULE_VERSION     "0.1"
> +
> +/* Ethernet MAC Registers */
> +#define REG_CAMCMR             0x00
> +#define REG_CAMEN              0x04
> +#define REG_CAMM_BASE          0x08
> +#define REG_CAML_BASE          0x0c
> +#define REG_TXDLSA             0x88
> +#define REG_RXDLSA             0x8C
> +#define REG_MCMDR              0x90
> +#define REG_MIID               0x94
> +#define REG_MIIDA              0x98
> +#define REG_FFTCR              0x9C
> +#define REG_TSDR               0xa0
> +#define REG_RSDR               0xa4
> +#define REG_DMARFC             0xa8
> +#define REG_MIEN               0xac
> +#define REG_MISTA              0xb0
> +#define REG_CTXDSA             0xcc
> +#define REG_CTXBSA             0xd0
> +#define REG_CRXDSA             0xd4
> +#define REG_CRXBSA             0xd8
> +
> +/* mac controller bit */
> +#define MCMDR_RXON             0x01
> +#define MCMDR_ACP              (0x01 << 3)
> +#define MCMDR_SPCRC            (0x01 << 5)
> +#define MCMDR_TXON             (0x01 << 8)
> +#define MCMDR_FDUP             (0x01 << 18)
> +#define MCMDR_ENMDC            (0x01 << 19)
> +#define MCMDR_OPMOD            (0x01 << 20)
> +#define SWR                    (0x01 << 24)
> +
> +/* cam command regiser */
> +#define CAMCMR_AUP             0x01
> +#define CAMCMR_AMP             (0x01 << 1)
> +#define CAMCMR_ABP             (0x01 << 2)
> +#define CAMCMR_CCAM            (0x01 << 3)
> +#define CAMCMR_ECMP            (0x01 << 4)
> +#define CAM0EN                 0x01
> +
> +/* mac mii controller bit */
> +#define MDCCR                  (0x0a << 20)
> +#define PHYAD                  (0x01 << 8)
> +#define PHYWR                  (0x01 << 16)
> +#define PHYBUSY                        (0x01 << 17)
> +#define PHYPRESP               (0x01 << 18)
> +#define CAM_ENTRY_SIZE         0x08
> +
> +/* rx and tx status */
> +#define TXDS_TXCP              (0x01 << 19)
> +#define RXDS_CRCE              (0x01 << 17)
> +#define RXDS_PTLE              (0x01 << 19)
> +#define RXDS_RXGD              (0x01 << 20)
> +#define RXDS_ALIE              (0x01 << 21)
> +#define RXDS_RP                        (0x01 << 22)
> +
> +/* mac interrupt status*/
> +#define MISTA_EXDEF            (0x01 << 19)
> +#define MISTA_TXBERR           (0x01 << 24)
> +#define MISTA_TDU              (0x01 << 23)
> +#define MISTA_RDU              (0x01 << 10)
> +#define MISTA_RXBERR           (0x01 << 11)
> +
> +#define ENSTART                        0x01
> +#define ENRXINTR               0x01
> +#define ENRXGD                 (0x01 << 4)
> +#define ENRXBERR               (0x01 << 11)
> +#define ENTXINTR               (0x01 << 16)
> +#define ENTXCP                 (0x01 << 18)
> +#define ENTXABT                        (0x01 << 21)
> +#define ENTXBERR               (0x01 << 24)
> +#define ENMDC                  (0x01 << 19)
> +#define PHYBUSY                        (0x01 << 17)
> +#define MDCCR_VAL              0xa00000
> +
> +/* rx and tx owner bit */
> +#define RX_OWEN_DMA            (0x01 << 31)
> +#define RX_OWEN_CPU            (~(0x03 << 30))
> +#define TX_OWEN_DMA            (0x01 << 31)
> +#define TX_OWEN_CPU            (~(0x01 << 31))
> +
> +/* tx frame desc controller bit */
> +#define MACTXINTEN             0x04
> +#define CRCMODE                        0x02
> +#define PADDINGMODE            0x01
> +
> +/* fftcr controller bit */
> +#define TXTHD                  (0x03 << 8)
> +#define BLENGTH                        (0x01 << 20)
> +
> +/* global setting for driver */
> +#define RX_DESC_SIZE           50
> +#define TX_DESC_SIZE           10
> +#define MAX_RBUFF_SZ           0x600
> +#define MAX_TBUFF_SZ           0x600
> +#define TX_TIMEOUT             50
> +#define DELAY                  1000
> +#define CAM0                   0x0
> +
> +static int w90p910_mdio_read(struct net_device *dev, int phy_id, int reg);
> +
> +struct w90p910_rxbd {
> +       unsigned int sl;
> +       unsigned int buffer;
> +       unsigned int reserved;
> +       unsigned int next;
> +};
> +
> +struct w90p910_txbd {
> +       unsigned int mode;
> +       unsigned int buffer;
> +       unsigned int sl;
> +       unsigned int next;
> +};
> +
> +struct recv_pdesc {
> +       struct w90p910_rxbd desclist[RX_DESC_SIZE];
> +       char recv_buf[RX_DESC_SIZE][MAX_RBUFF_SZ];
> +};
> +
> +struct tran_pdesc {
> +       struct w90p910_txbd desclist[TX_DESC_SIZE];
> +       char tran_buf[RX_DESC_SIZE][MAX_TBUFF_SZ];
> +};
> +
> +struct  w90p910_ether {
> +       struct recv_pdesc *rdesc;
> +       struct recv_pdesc *rdesc_phys;
> +       struct tran_pdesc *tdesc;
> +       struct tran_pdesc *tdesc_phys;
> +       struct net_device_stats stats;
> +       struct platform_device *pdev;
> +       struct sk_buff *skb;
> +       struct clk *clk;
> +       struct clk *rmiiclk;
> +       struct mii_if_info mii;
> +       struct timer_list check_timer;
> +       void __iomem *reg;
> +       unsigned int rxirq;
> +       unsigned int txirq;
> +       unsigned int cur_tx;
> +       unsigned int cur_rx;
> +       unsigned int finish_tx;
> +       unsigned int rx_packets;
> +       unsigned int rx_bytes;
> +       unsigned int start_tx_ptr;
> +       unsigned int start_rx_ptr;
> +       unsigned int linkflag;
> +       spinlock_t lock;

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists