Re: [RFC PATCH] net:Add basic DWC Ethernet QoS Driver

Anish Khurana
2014-05-10T10:19:14Z

[-- Attachment #1: Type: text/plain, Size: 26332 bytes --]

small suggestions regarding interrupt registration ( request_irq()) call:
1. instead of request_irq() call in probe function , i think interrupt can
be registered in dwceqos_open() call . since packet only come when we make
interface up( ifconfig up) .
2. in request_irq() thier is flag parameter can either be zero or flag as :
IRQF_DISABLED
IRQF_SHARED
...
normally if we have to share the irq line with other device we use
IRQF_SHARED .

On Fri, May 9, 2014 at 8:49 PM, Tobias Klauser <tklauser@distanz.ch> wrote:

> On 2014-05-08 at 14:50:14 +0200, Andreas Irestal <andreas.irestal@axis.com>
> wrote:
> > This is an early version of a driver for the Synopsys DWC Ethernet QoS IP
> > version 4. Unfortunately, version 4.00a and onwards of this IP is totally
> > different from earlier versions used in the STMicroelectronics drivers.
> Both
> > functionality and registers are different. As this is my first network
> driver
> > I am submitting an RFC to catch design flaws and bad coding standards at
> an
> > early stage. Also, others looking at this IP could hopefully be helped
> by this
> > early code.
> >
> > The driver is quite inefficient, yet still functional (Gbit only) and
> uses a
> > polling-driven TX-approach. For the RX side NAPI and interrupts are used.
> > There are still quite a lot of work to do. Link handling, more robust
> > error-handling, HW Checksumming, scatter-gather and TCP Segmentation and
> > checksum offloading to name a few.
> >
> > All code has been developed and tested using an FPGA implementation of
> the IP,
> > with an ARM Cortex A9 as the main CPU, running Linux 3.13.
> >
> > Signed-off-by: Andreas Irestaal <Andreas.Irestal@axis.com>
> > ---
> > drivers/net/ethernet/Kconfig | 1 +
> > drivers/net/ethernet/Makefile | 1 +
> > drivers/net/ethernet/synopsys/Kconfig | 24 +
> > drivers/net/ethernet/synopsys/Makefile | 5 +
> > drivers/net/ethernet/synopsys/dwc_eth_qos.c | 706
> +++++++++++++++++++++++++++
> > drivers/net/ethernet/synopsys/dwc_eth_qos.h | 308 ++++++++++++
> > 6 files changed, 1045 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/net/ethernet/synopsys/Kconfig
> > create mode 100644 drivers/net/ethernet/synopsys/Makefile
> > create mode 100644 drivers/net/ethernet/synopsys/dwc_eth_qos.c
> > create mode 100644 drivers/net/ethernet/synopsys/dwc_eth_qos.h
>
> [...]
>
> > diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c
> b/drivers/net/ethernet/synopsys/dwc_eth_qos.c
> > new file mode 100644
> > index 0000000..20b1a9e
> > --- /dev/null
> > +++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c
> > @@ -0,0 +1,706 @@
>
> [...]
>
> > +struct net_local {
> > + void __iomem *baseaddr;
> > + struct clk *phy_ref_clk;
> > + struct clk *apb_pclk;
> > +
> > + struct net_device *ndev;
> > + struct platform_device *pdev;
> > +
> > + volatile struct dwc_eth_qos_txdesc *tx_descs;
> > + volatile struct dwc_eth_qos_rxdesc *rx_descs;
> > +
> > + void *tx_buf;
> > +
> > + struct ring_info *rx_skb;
> > +
> > + dma_addr_t tx_descs_addr;
> > + dma_addr_t rx_descs_addr;
> > +
> > + u32 next_tx;
> > + u32 next_rx;
> > +
> > + struct napi_struct napi;
> > +
> > + struct net_device_stats stats;
>
> You don't need a private copy of struct net_device_stats, just use the
> one from struct net_device. It looks like the driver is not returning
> these stats currently anyway. If you use ndev->stats, they will be
> picked up by dev_get_stats() and i.e. show up in ifconfig output.
>
> > + spinlock_t rx_lock;
> > +};
> > +
> > +/* Allocate DMA helper structure at position given by index */
> > +static void dwceqos_alloc_rxring_desc(struct net_local *lp, int index)
> > +{
> > + struct sk_buff *new_skb;
> > + u32 new_skb_baddr = 0;
> > + new_skb = netdev_alloc_skb(lp->ndev, DWCEQOS_RX_BUF_SIZE);
> > + if (!new_skb) {
> > + dev_err(&lp->ndev->dev, "alloc_skb error for desc %d\n",
> index);
> > + goto out;
> > + }
> > +
> > + /* Get dma handle of skb->data */
> > + new_skb_baddr = (u32)dma_map_single(lp->ndev->dev.parent,
> > + new_skb->data, DWCEQOS_RX_BUF_SIZE, DMA_FROM_DEVICE);
> > + if (dma_mapping_error(lp->ndev->dev.parent, new_skb_baddr)) {
> > + dev_err(&lp->pdev->dev, "DMA map error\n");
> > + dev_kfree_skb(new_skb);
> > + new_skb = NULL;
> > + }
> > +out:
> > + lp->rx_skb[index].skb = new_skb;
> > + lp->rx_skb[index].mapping = new_skb_baddr;
> > + lp->rx_skb[index].len = DWCEQOS_RX_BUF_SIZE;
> > +
> > + return;
> > +}
> > +
> > +static void dwceqos_mtl_init(struct net_local *lp)
> > +{
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MTL_OP_MODE, 0x00000060);
> > +}
> > +
> > +static void dwceqos_mtl_tx_init(struct net_local *lp)
> > +{
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MTL_TXQ0_OP_MODE,
> > + DWCEQOS_MTL_TXQ_EN);
> > +}
> > +
> > +static void dwceqos_mtl_rx_init(struct net_local *lp)
> > +{
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MTL_RXQ0_OP_MODE,
> > + DWCEQOS_MTL_RXQ_RQS256 | DWCEQOS_MTL_RXQ_DIS_TCP_EF |
> > + DWCEQOS_MTL_RXQ_FEP | DWCEQOS_MTL_RXQ_FUP |
> > + DWCEQOS_MTL_RXQ_RTC32);
> > +}
> > +
> > +static void dwceqos_mac_rx_init(struct net_local *lp)
> > +{
> > + u32 val;
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MAC_RXQ_CTRL0, 2);
> > + val = DWCEQOS_MAC_PKT_FILTER_RX_ALL |
> DWCEQOS_MAC_PKT_FILTER_PCF_ALL |
> > + DWCEQOS_MAC_PKT_FILTER_PROMISCUOUS;
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MAC_PKT_FILTER, val);
> > +}
> > +
> > +static int dwceqos_mac_enable(struct net_local *lp)
> > +{
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MAC_CFG,
> > + DWCEQOS_MAC_CFG_DM | DWCEQOS_MAC_CFG_TE |
> > + DWCEQOS_MAC_CFG_RE);
> > + return 0;
> > +}
> > +
> > +static void dwceqos_dma_wake_rx(struct net_local *lp)
> > +{
> > + u32 offset;
> > + offset = DWCEQOS_RX_DCNT * sizeof(struct dwc_eth_qos_rxdesc);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_RXDESC_TAIL,
> > + (u32)lp->rx_descs_addr + offset);
> > +}
> > +
> > +static inline void dwceqos_dma_rx_start(struct net_local *lp)
> > +{
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_RX_CTRL,
> > + DWCEQOS_DMA_CH0_TXRX_CONTROL_START |
> > + DWCEQOS_DMA_CH0_TXRX_CONTROL_PBL16 |
> > + DWCEQOS_RX_BUF_SIZE);
> > + dwceqos_dma_wake_rx(lp);
> > +}
> > +
> > +static inline void dwceqos_dma_tx_start(struct net_local *lp)
> > +{
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_TX_CTRL,
> > + DWCEQOS_DMA_CH0_TXRX_CONTROL_START |
> > + DWCEQOS_DMA_CH0_TXRX_CONTROL_PBL16);
> > +}
> > +
> > +static void dwceqos_dma_setmode(struct net_local *lp)
> > +{
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_MODE,
> > + DWCEQOS_DMA_MODE_TXPR | DWCEQOS_DMA_MODE_DA);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_SYSBUS_MODE,
> > + DWCEQOS_DMA_SYSBUS_MB);
> > +}
> > +
> > +static void dwceqos_dma_txenable(struct net_local *lp)
> > +{
> > + u32 val;
> > + val = DWCEQOS_DMA_CH0_TXRX_CONTROL_PBL16 |
> > + DWCEQOS_DMA_CH0_TXRX_CONTROL_START;
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_TX_CTRL, val);
> > +}
> > +
> > +static void dwceqos_dma_rx_enable(struct net_local *lp)
> > +{
> > + u32 val;
> > + val = DWCEQOS_DMA_CH0_TXRX_CONTROL_PBL16 |
> > + DWCEQOS_RX_BUF_SIZE | DWCEQOS_DMA_CH0_TXRX_CONTROL_START;
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_RX_CTRL, val);
> > +}
> > +
> > +/* Initialize DMA descriptors to their start values */
> > +static void dwceqos_dma_prepare(struct net_local *lp)
> > +{
> > + int i;
> > + for (i = 0; i < DWCEQOS_TX_DCNT; ++i) {
> > + lp->tx_descs[i].tdes0.raw = 0;
> > + lp->tx_descs[i].tdes1.raw = 0;
> > + lp->tx_descs[i].tdes2.raw = 0;
> > + lp->tx_descs[i].tdes3.raw = 0;
> > + }
> > +
> > + for (i = 0; i < DWCEQOS_RX_DCNT; ++i) {
> > + lp->rx_descs[i].rdes0.rd.buffer1 = lp->rx_skb[i].mapping;
> > + lp->rx_descs[i].rdes1.raw = 0;
> > + lp->rx_descs[i].rdes2.rd.buffer2 = 0;
> > + lp->rx_descs[i].rdes3.raw = 0;
> > + lp->rx_descs[i].rdes3.rd.buf1v = 1;
> > + lp->rx_descs[i].rdes3.rd.inte = 1;
> > + lp->rx_descs[i].rdes3.rd.own = 1;
> > + }
> > + lp->next_tx = 0;
> > + lp->next_rx = 0;
> > +}
> > +
> > +/* Allocate and initiate DMA rings for TX and RX. Also allocates RX
> buffers for
> > + * incoming packets.
> > + */
> > +static int dwceqos_dma_alloc(struct net_local *lp)
> > +{
> > + u32 size;
> > + int i;
> > +
> > + size = DWCEQOS_RX_DCNT * sizeof(struct ring_info);
> > + lp->rx_skb = kzalloc(size, GFP_KERNEL);
> > + if (!lp->rx_skb) {
> > + dev_err(&lp->pdev->dev, "Unable to allocate ring
> descriptor area\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* Allocate DMA descriptors */
> > + size = DWCEQOS_TX_DCNT * sizeof(struct dwc_eth_qos_txdesc);
> > + lp->tx_descs = dma_alloc_coherent(&lp->pdev->dev, size,
> > + &lp->tx_descs_addr, 0);
> > + if (!lp->tx_descs) {
> > + dev_err(&lp->pdev->dev, "Failed to allocate TX DMA
> descriptors\n");
> > + /* Should deallocate memory here */
> > + return -ENOMEM;
> > + }
> > + size = DWCEQOS_RX_DCNT * sizeof(struct dwc_eth_qos_rxdesc);
> > + lp->rx_descs = dma_alloc_coherent(&lp->pdev->dev, size,
> > + &lp->rx_descs_addr, 0);
> > + if (!lp->rx_descs) {
> > + dev_err(&lp->pdev->dev, "Failed to allocate RX DMA
> descriptors\n");
> > + /* Should deallocate memory here */
> > + return -ENOMEM;
> > + }
> > +
> > + for (i = 0; i < DWCEQOS_RX_DCNT; ++i) {
> > + dwceqos_alloc_rxring_desc(lp, i);
> > + if (!(lp->rx_skb[lp->next_rx].skb)) {
> > + dev_err(&lp->pdev->dev, "Unable to map
> descriptor %d to DMA\n",
> > + lp->next_rx);
> > + /* What error code to return for mapping error?
> */
> > + return -1;
> > + }
> > + }
> > +
> > + dwceqos_dma_prepare(lp);
> > +
> > + /* Tell HW where to look*/
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_RXDESC_RING_LEN,
> > + DWCEQOS_RX_DCNT-1);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_TXDESC_RING_LEN,
> > + DWCEQOS_TX_DCNT-1);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_RXDESC_LIST_ADDR,
> > + (u32)lp->rx_descs_addr);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_TXDESC_LIST_ADDR,
> > + (u32)lp->tx_descs_addr);
> > + return 0;
> > +}
> > +
> > +/* Checks if there are any available incoming packets ready for
> processing */
> > +int dwceqos_packet_avail(struct net_local *lp)
>
> Make this function static.
>
> > +{
> > + return !lp->rx_descs[lp->next_rx].rdes3.wr.own;
> > +}
> > +
> > +/* Free DMA descriptor area */
> > +static void dwceqos_dma_free(struct net_local *lp)
> > +{
> > + u32 size;
> > + size = DWCEQOS_TX_DCNT * sizeof(struct dwc_eth_qos_txdesc);
> > + if (lp->tx_descs)
> > + dma_free_coherent(&lp->pdev->dev, size,
> > + lp->tx_descs, lp->tx_descs_addr);
> > + size = DWCEQOS_RX_DCNT * sizeof(struct dwc_eth_qos_rxdesc);
> > + if (lp->rx_descs)
> > + dma_free_coherent(&lp->pdev->dev, size,
> > + lp->rx_descs, lp->rx_descs_addr);
> > +}
> > +
> > +/* HW transmit function. */
> > +static void dwceqos_dma_xmit(struct net_local *lp, void *data, int len)
> > +{
> > + u32 regval;
> > + int i = lp->next_tx;
> > + int dwc_wait;
> > + dma_addr_t dma_handle;
> > +
> > + dma_handle = dma_map_single(lp->ndev->dev.parent,
> > + data, DWCEQOS_RX_BUF_SIZE, DMA_TO_DEVICE);
> > + if (dma_mapping_error(lp->ndev->dev.parent, dma_handle)) {
> > + dev_err(&lp->pdev->dev, "DMA Mapping error\n");
> > + return;
> > + }
> > +
> > + lp->tx_descs[i].tdes0.rd.buffer1 = dma_handle;
> > + lp->tx_descs[i].tdes2.rd.buf1len = len;
> > + lp->tx_descs[i].tdes3.rd.fl = len;
> > + lp->tx_descs[i].tdes3.rd.fd = 1;
> > + lp->tx_descs[i].tdes3.rd.ld = 1;
> > + lp->tx_descs[i].tdes3.rd.own = 1;
> > +
> > + /* Issue Transmit Poll by writing address of next free
> descriptor */
> > + regval = lp->tx_descs_addr + (i+1) * sizeof(struct
> dwc_eth_qos_txdesc);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_TXDESC_TAIL, regval);
> > +
> > + /* Set poll wait timeout to 2 seconds */
> > + dwc_wait = 200;
> > +
> > + while (lp->tx_descs[i].tdes3.wr.own) {
> > + mdelay(10);
> > + if (!dwc_wait--)
> > + break;
> > + }
> > + if (lp->tx_descs[i].tdes3.wr.own)
> > + dev_err(&lp->pdev->dev, "Failed to transmit: Timed
> out\n");
> > +
> > + lp->next_tx = (lp->next_tx + 1) % DWCEQOS_TX_DCNT;
> > + dma_unmap_single(lp->ndev->dev.parent,
> > + dma_handle, DWCEQOS_RX_BUF_SIZE, DMA_TO_DEVICE);
> > +}
> > +
> > +/* Store HW Addr in MAC registers. Source Address Replacement not used
> yet */
> > +static void dwceqos_set_hwaddr(struct net_local *lp)
> > +{
> > + u32 val;
> > +
> > + val = DWCEQOS_MAC_ADDR_HI_EN | (lp->ndev->dev_addr[0] << 8) |
> > + (lp->ndev->dev_addr[1]);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MAC_ADDR_HI, val);
> > + val = (lp->ndev->dev_addr[2] << 24) | (lp->ndev->dev_addr[3] <<
> 16) |
> > + (lp->ndev->dev_addr[4] << 8) | (lp->ndev->dev_addr[5]);
> > + dwceqos_write(lp->baseaddr, DWCEQOS_MAC_ADDR_LO, val);
> > +}
> > +
> > +/* DMA reset. When issued also resets all MTL and MAC registers as well
> */
> > +static void dwceqos_dma_reset(struct net_local *lp)
> > +{
> > + /* Wait 5 seconds for DMA reset*/
> > + int i = 5000;
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_MODE, 1);
> > +
> > + do {
> > + mdelay(1);
> > + } while ((dwceqos_read(lp->baseaddr, DWCEQOS_DMA_MODE) & 0x1) &&
> i--);
> > +}
> > +
> > +/* Interrupt handler. So far only RX interrupts are used */
> > +static irqreturn_t dwceqos_interrupt(int irq, void *dev_id)
> > +{
> > + struct net_device *ndev = dev_id;
> > + struct net_local *lp = netdev_priv(ndev);
> > +
> > + u32 cause;
> > + u32 val;
> > +
> > + cause = dwceqos_read(lp->baseaddr, DWCEQOS_DMA_INT_STAT);
> > + if (cause & 0x00000001) {
> > + /* DMA Channel 0 Interrupt. Assume RX interrupt for now
> */
> > + val = dwceqos_read(lp->baseaddr, DWCEQOS_DMA_CH0_STAT);
> > + /* Ack */
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_STAT, val);
> > + if (val & 0x00000040)
> > + napi_schedule(&lp->napi);
> > + } else if (unlikely(!cause)) {
> > + return IRQ_NONE;
> > + }
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int dwceqos_open(struct net_device *ndev)
> > +{
> > + /* Allocate Buffers & DMA Descr. + Initiate DMA descriptors */
> > + struct net_local *lp = netdev_priv(ndev);
> > +
> > + /* Set up hardware. It looks like order of writes is important
> here.
> > + * Only gigatbit support, no advanced MTL or MAC configuration
> yet.
> > + */
> > + dwceqos_dma_reset(lp);
> > + dwceqos_mtl_init(lp);
> > + dwceqos_mtl_tx_init(lp);
> > + dwceqos_dma_setmode(lp);
> > + dwceqos_dma_alloc(lp);
> > + dwceqos_dma_wake_rx(lp);
> > + dwceqos_dma_txenable(lp);
> > + dwceqos_mtl_rx_init(lp);
> > + dwceqos_mac_rx_init(lp);
> > + dwceqos_dma_rx_enable(lp);
> > + dwceqos_dma_wake_rx(lp);
> > + dwceqos_set_hwaddr(lp);
> > + dwceqos_mac_enable(lp);
> > +
> > + /* Enable RX Interrupts */
> > + dwceqos_write(lp->baseaddr, DWCEQOS_DMA_CH0_INT_EN, 0x00010040);
> > +
> > + napi_enable(&lp->napi);
> > + netif_carrier_on(ndev);
> > + netif_start_queue(ndev);
> > + return 0;
> > +}
> > +
> > +
> > +int dwceqos_stop(struct net_device *ndev)
>
> Make this function static.
>
> > +{
> > + struct net_local *lp = netdev_priv(ndev);
> > + dwceqos_dma_free(lp);
> > + netif_stop_queue(ndev);
> > + napi_disable(&lp->napi);
> > + netif_carrier_off(ndev);
> > + return 0;
> > +}
> > +
> > +/* Receive one packet and return skb */
> > +struct sk_buff *
> > +dwceqos_recv_packet(struct net_local *lp)
>
> Make this function static.
>
> > +{
> > + struct sk_buff *skb;
> > + u32 len;
> > +
> > + skb = lp->rx_skb[lp->next_rx].skb;
> > + len = lp->rx_descs[lp->next_rx].rdes3.wr.length;
> > +
> > + /* Unmap old buffer */
> > + dma_unmap_single(lp->ndev->dev.parent,
> lp->rx_skb[lp->next_rx].mapping,
> > + lp->rx_skb[lp->next_rx].len, DMA_FROM_DEVICE);
> > + skb_put(skb, len);
> > + skb->protocol = eth_type_trans(skb, lp->ndev);
> > + skb->ip_summed = 0;
> > +
> > + /* Initialize new Buffer descriptor */
> > + dwceqos_alloc_rxring_desc(lp, lp->next_rx);
> > + if (!(lp->rx_skb[lp->next_rx].skb)) {
> > + dev_err(&lp->pdev->dev, "Unable to map descriptor %d to
> DMA\n",
> > + lp->next_rx);
> > + return NULL;
> > + }
> > +
> > + /* Initialize new DMA descriptor */
> > + lp->rx_descs[lp->next_rx].rdes0.rd.buffer1 =
> > + lp->rx_skb[lp->next_rx].mapping;
> > + lp->rx_descs[lp->next_rx].rdes1.raw = 0;
> > + lp->rx_descs[lp->next_rx].rdes2.rd.buffer2 = 0;
> > + lp->rx_descs[lp->next_rx].rdes3.raw = 0;
> > + lp->rx_descs[lp->next_rx].rdes3.rd.buf1v = 1;
> > + lp->rx_descs[lp->next_rx].rdes3.rd.inte = 1;
> > + lp->rx_descs[lp->next_rx].rdes3.rd.own = 1;
> > + lp->next_rx = (lp->next_rx + 1) % DWCEQOS_RX_DCNT;
> > + return skb;
> > +}
> > +
> > +
> > +/* NAPI poll routine */
> > +int dwceqos_rx_poll(struct napi_struct *napi, int budget)
> > +{
> > + struct net_local *lp = container_of(napi, struct net_local,
> napi);
> > + int npackets = 0;
> > + struct sk_buff *skb;
> > +
> > + spin_lock(&lp->rx_lock);
> > + while (npackets < budget && dwceqos_packet_avail(lp)) {
> > +
> > + skb = dwceqos_recv_packet(lp);
> > + if (!skb)
> > + break;
> > +
> > + netif_receive_skb(skb);
> > +
> > + lp->stats.rx_packets++;
> > + lp->stats.rx_bytes += skb->len;
> > + npackets++;
> > + }
> > + if (npackets < budget)
> > + napi_complete(napi);
> > +
> > + spin_unlock(&lp->rx_lock);
> > + return npackets;
> > +}
> > +
> > +/* Main transmit function called from kernel */
> > +int dwceqos_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>
> Make this function static.
>
> > +{
> > + int len;
> > + struct net_local *lp = netdev_priv(ndev);
> > + char *data;
> > + data = skb->data;
> > + len = skb->len;
> > +
> > + /* Send packet on wire */
> > + dwceqos_dma_xmit(lp, data, len);
> > +
> > + skb_tx_timestamp(skb);
> > +
> > + ndev->stats.tx_bytes += len;
> > + dev_kfree_skb(skb);
> > +
> > + return 0;
> > +}
> > +
> > +struct net_device_ops dwceq_netdev_ops;
>
> Why not initialize this here?
>
> > +#ifdef CONFIG_OF
> > +static inline int dwceqos_probe_config_dt(struct platform_device *pdev)
> > +{
> > + struct net_device *ndev;
> > + const void *mac_address;
> > +
> > + ndev = platform_get_drvdata(pdev);
> > + /* Set the MAC address. */
> > + mac_address = of_get_mac_address(pdev->dev.of_node);
> > + if (mac_address)
> > + memcpy(ndev->dev_addr, mac_address, ETH_ALEN);
> > + else
> > + dev_warn(&pdev->dev, "No MAC address found\n");
> > + return 0;
> > +}
> > +#else
> > +static inline int dwceqos_probe_config_dt()
> > +{
> > + return -ENOSYS;
> > +}
> > +#endif
> > +
> > +
> > +/**
> > + * dwceqos_probe - Platform driver probe
> > + * @pdev: Pointer to platform device structure
> > + *
> > + * Return 0 on success, negative value if error
> > + */
> > +int dwceqos_probe(struct platform_device *pdev)
>
> Make this function static.
>
> > +{
> > + struct resource *r_mem = NULL;
> > + struct resource *r_irq = NULL;
> > + struct net_device *ndev;
> > + struct net_local *lp;
> > + int ret = -ENXIO;
> > +
> > + r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + r_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > + if (!r_mem || !r_irq) {
> > + dev_err(&pdev->dev, "no IO resource defined.\n");
> > + return -ENXIO;
> > + }
> > + ndev = alloc_etherdev(sizeof(*lp));
> > + if (!ndev) {
> > + dev_err(&pdev->dev, "etherdev allocation failed.\n");
> > + return -ENOMEM;
> > + }
> > +
> > + SET_NETDEV_DEV(ndev, &pdev->dev);
> > +
> > + lp = netdev_priv(ndev);
> > + lp->ndev = ndev;
> > + lp->pdev = pdev;
> > +
> > + lp->baseaddr = devm_ioremap_resource(&pdev->dev, r_mem);
> > + if (IS_ERR(lp->baseaddr)) {
> > + dev_err(&pdev->dev, "failed to map baseaddress.\n");
> > + ret = PTR_ERR(lp->baseaddr);
> > + goto err_out_free_netdev;
> > + }
> > +
> > +
> > + dev_dbg(&lp->pdev->dev, "BASEADDRESS hw: %p virt: %p\n",
> > + (void *)r_mem->start, lp->baseaddr);
> > +
> > + ndev->irq = platform_get_irq(pdev, 0);
> > + ndev->netdev_ops = &dwceq_netdev_ops;
> > + ndev->base_addr = r_mem->start;
> > + ndev->features = 0;
> > +
> > + netif_napi_add(ndev, &lp->napi, dwceqos_rx_poll, 64);
> > +
> > + ret = register_netdev(ndev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Cannot register net device,
> aborting.\n");
> > + goto err_out_free_netdev;
> > + }
> > +
> > + lp->apb_pclk = devm_clk_get(&pdev->dev, "apb_pclk");
> > + if (IS_ERR(lp->apb_pclk)) {
> > + dev_err(&pdev->dev, "apb_pclk clock not found.\n");
> > + ret = PTR_ERR(lp->apb_pclk);
> > + goto err_out_unregister_netdev;
> > + }
> > + lp->phy_ref_clk = devm_clk_get(&pdev->dev, "phy_ref_clk");
> > + if (IS_ERR(lp->phy_ref_clk)) {
> > + dev_err(&pdev->dev, "phy_ref_clk clock not found.\n");
> > + ret = PTR_ERR(lp->phy_ref_clk);
> > + goto err_out_unregister_netdev;
> > + }
> > +
> > + ret = clk_prepare_enable(lp->apb_pclk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Unable to enable APER clock.\n");
> > + goto err_out_unregister_netdev;
> > + }
> > + ret = clk_prepare_enable(lp->phy_ref_clk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Unable to enable device clock.\n");
> > + goto err_out_clk_dis_aper;
> > + }
> > +
> > + platform_set_drvdata(pdev, ndev);
> > + dwceqos_probe_config_dt(pdev);
> > + dev_info(&lp->pdev->dev, "pdev->id %d, baseaddr 0x%08lx, irq
> %d\n",
> > + pdev->id, ndev->base_addr, ndev->irq);
> > +
> > + ret = devm_request_irq(&pdev->dev, ndev->irq,
> &dwceqos_interrupt, 0,
> > + ndev->name, ndev);
> > + if (ret) {
> > + dev_err(&lp->pdev->dev, "Unable to request IRQ %p, error
> %d\n",
> > + r_irq, ret);
> > + goto err_out_unregister_clk_notifier;
> > + }
> > +
> > + return 0;
> > +
> > +err_out_unregister_clk_notifier:
> > + clk_disable_unprepare(lp->phy_ref_clk);
> > +err_out_clk_dis_aper:
> > + clk_disable_unprepare(lp->apb_pclk);
> > +err_out_unregister_netdev:
> > + unregister_netdev(ndev);
> > +err_out_free_netdev:
> > + free_netdev(ndev);
> > + platform_set_drvdata(pdev, NULL);
> > + return ret;
> > +}
> > +
> > +int dwceqos_remove(struct platform_device *pdev)
>
> Make this function static.
>
> > +{
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + unregister_netdev(ndev);
> > + free_netdev(ndev);
> > + return 0;
> > +}
> > +
> > +struct net_device_ops dwceq_netdev_ops = {
> > + .ndo_open = dwceqos_open,
> > + .ndo_stop = dwceqos_stop,
> > + .ndo_start_xmit = dwceqos_start_xmit,
> > +};
>
> Move initialization to where it is declared and make the struct static.
>
> > +
> > +struct of_device_id dwceq_of_match[] = {
> > + { .compatible = "dwc,qos-ethernet", },
> > + {}
> > +};
>
> Make this struct static.
>
> Make this struct static.
> > +MODULE_DEVICE_TABLE(of, dwceq_of_match);
> > +
> > +struct platform_driver dwceqos_driver = {
> > + .probe = dwceqos_probe,
> > + .remove = dwceqos_remove,
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .owner = THIS_MODULE,
> > + .of_match_table = dwceq_of_match,
> > + },
> > +};
>
> Make this struct static.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

[-- Attachment #2: Type: text/html, Size: 34332 bytes --]