lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sat, 10 May 2014 16:08:32 +0530
From:	Anish Khurana <anish.khurana130181@...il.com>
To:	Tobias Klauser <tklauser@...tanz.ch>
Cc:	Andreas Irestal <andreas.irestal@...s.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"maxime.ripard@...e-electrons.com" <maxime.ripard@...e-electrons.com>,
	"abrodkin@...opsys.com" <abrodkin@...opsys.com>,
	"jeffrey.t.kirsher@...el.com" <jeffrey.t.kirsher@...el.com>,
	"ben@...adent.org.uk" <ben@...adent.org.uk>,
	"sr@...x.de" <sr@...x.de>,
	"jonas.jensen@...il.com" <jonas.jensen@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Jesper Nilsson <jespern@...s.com>
Subject: Re: [RFC PATCH] net:Add basic DWC Ethernet QoS Driver

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@...tanz.ch> wrote:
> On 2014-05-08 at 14:50:14 +0200, Andreas Irestal <andreas.irestal@...s.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@...s.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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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