Re: [PATCH] net: emac: emac gigabit ethernet controller driver

From: Timur Tabi
Date: Wed Dec 09 2015 - 15:09:39 EST

Next message: Joe Perches: "Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err()"
Previous message: Greg Kroah-Hartman: "Re: [PATCH 1/4] usb: host: ehci.h: cleanup header file"
Next in thread: Fabio Estevam: "Re: [PATCH] net: emac: emac gigabit ethernet controller driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

So first of all, thanks for posting this. I know it's missing a bunch
of stuff that's necessary for Qualcomm's Server chip, but it's a
start.

Unfortunately, 6,000 lines is a lot to review at once. Any chance you
can break up the next version into smaller patches?

On Mon, Dec 7, 2015 at 4:58 PM, Gilad Avidov <gavidov@xxxxxxxxxxxxxx> wrote:

> + qcom,emac-gpio-mdc = <&msmgpio 123 0>;
> + qcom,emac-gpio-mdio = <&msmgpio 124 0>;

Is there any chance you can remove all references to "MSM" throughout
the entire driver, since the EMAC exists on non-MSM parts?

> + qcom,emac-tstamp-en;
> + qcom,emac-ptp-frac-ns-adj = <125000000 1>;
> + phy-addr = <0>;
> + };
> diff --git a/drivers/net/ethernet/qualcomm/Kconfig b/drivers/net/ethernet/qualcomm/Kconfig
> index a76e380..ae9442d 100644
> --- a/drivers/net/ethernet/qualcomm/Kconfig
> +++ b/drivers/net/ethernet/qualcomm/Kconfig
> @@ -24,4 +24,11 @@ config QCA7000
> To compile this driver as a module, choose M here. The module
> will be called qcaspi.
>
> +config QCOM_EMAC
> + tristate "MSM EMAC Gigabit Ethernet support"
> + default n

"default n" is redundant

> + select CRC32
> + ---help---
> + This driver supports the Qualcomm EMAC Gigabit Ethernet controller.

I think should be longer, perhaps by adding some more info about the
controller itself?

> +
> endif # NET_VENDOR_QUALCOMM
> diff --git a/drivers/net/ethernet/qualcomm/Makefile b/drivers/net/ethernet/qualcomm/Makefile
> index 9da2d75..b14686e 100644
> --- a/drivers/net/ethernet/qualcomm/Makefile
> +++ b/drivers/net/ethernet/qualcomm/Makefile
> @@ -4,3 +4,5 @@
>
> obj-$(CONFIG_QCA7000) += qcaspi.o
> qcaspi-objs := qca_spi.o qca_framing.o qca_7k.o qca_debug.o
> +
> +obj-$(CONFIG_QCOM_EMAC) += emac/
> \ No newline at end of file

Please fix

> +/* RSS */
> +static void emac_mac_rss_config(struct emac_adapter *adpt)
> +{
> + int key_len_by_u32 = sizeof(adpt->rss_key) / sizeof(u32);
> + int idt_len_by_u32 = sizeof(adpt->rss_idt) / sizeof(u32);

Can you use ARRAY_SIZE here?

> + u32 rxq0;
> + int i;
> +
> + /* Fill out hash function keys */
> + for (i = 0; i < key_len_by_u32; i++) {
> + u32 key, idx_base;
> +
> + idx_base = (key_len_by_u32 - i) * 4;
> + key = ((adpt->rss_key[idx_base - 1]) |
> + (adpt->rss_key[idx_base - 2] << 8) |
> + (adpt->rss_key[idx_base - 3] << 16) |
> + (adpt->rss_key[idx_base - 4] << 24));
> + writel_relaxed(key, adpt->base + EMAC_RSS_KEY(i, u32));
> + }
> +
> + /* Fill out redirection table */
> + for (i = 0; i < idt_len_by_u32; i++)
> + writel_relaxed(adpt->rss_idt[i],
> + adpt->base + EMAC_RSS_TBL(i, u32));
> +
> + writel_relaxed(adpt->rss_base_cpu, adpt->base + EMAC_BASE_CPU_NUMBER);
> +
> + rxq0 = readl_relaxed(adpt->base + EMAC_RXQ_CTRL_0);
> + if (adpt->rss_hstype & EMAC_RSS_HSTYP_IPV4_EN)
> + rxq0 |= RXQ0_RSS_HSTYP_IPV4_EN;
> + else
> + rxq0 &= ~RXQ0_RSS_HSTYP_IPV4_EN;
> +
> + if (adpt->rss_hstype & EMAC_RSS_HSTYP_TCP4_EN)
> + rxq0 |= RXQ0_RSS_HSTYP_IPV4_TCP_EN;
> + else
> + rxq0 &= ~RXQ0_RSS_HSTYP_IPV4_TCP_EN;
> +
> + if (adpt->rss_hstype & EMAC_RSS_HSTYP_IPV6_EN)
> + rxq0 |= RXQ0_RSS_HSTYP_IPV6_EN;
> + else
> + rxq0 &= ~RXQ0_RSS_HSTYP_IPV6_EN;
> +
> + if (adpt->rss_hstype & EMAC_RSS_HSTYP_TCP6_EN)
> + rxq0 |= RXQ0_RSS_HSTYP_IPV6_TCP_EN;
> + else
> + rxq0 &= ~RXQ0_RSS_HSTYP_IPV6_TCP_EN;
> +
> + rxq0 |= ((adpt->rss_idt_size << IDT_TABLE_SIZE_SHFT) &
> + IDT_TABLE_SIZE_BMSK);
> + rxq0 |= RSS_HASH_EN;
> +
> + wmb(); /* ensure all parameters are written before enabling RSS */
> +
> + writel_relaxed(rxq0, adpt->base + EMAC_RXQ_CTRL_0);

Why not just use writel(), which already includes a wmb()

> +/* Power Management */
> +void emac_mac_pm(struct emac_adapter *adpt, u32 speed, bool wol_en, bool rx_en)
> +{
> + u32 dma_mas, mac;
> +
> + dma_mas = readl_relaxed(adpt->base + EMAC_DMA_MAS_CTRL);
> + dma_mas &= ~LPW_CLK_SEL;
> + dma_mas |= LPW_STATE;
> +
> + mac = readl_relaxed(adpt->base + EMAC_MAC_CTRL);
> + mac &= ~(FULLD | RXEN | TXEN);
> + mac = (mac & ~SPEED_BMSK) |
> + (((u32)emac_mac_speed_10_100 << SPEED_SHFT) & SPEED_BMSK);
> +
> + if (wol_en) {
> + if (rx_en)
> + mac |= (RXEN | BROAD_EN);

You don't need the parentheses.

> +/* Config descriptor rings */
> +static void emac_mac_dma_rings_config(struct emac_adapter *adpt)
> +{
> + if (adpt->timestamp_en)
> + emac_reg_update32(adpt->csr + EMAC_EMAC_WRAPPER_CSR1,
> + 0, ENABLE_RRD_TIMESTAMP);
> +
> + /* TPD */

What does TPD stand for?

> + writel_relaxed(EMAC_DMA_ADDR_HI(adpt->tx_q[0].tpd.p_addr),
> + adpt->base + EMAC_DESC_CTRL_1);
> + switch (adpt->tx_q_cnt) {
> + case 4:
> + writel_relaxed(EMAC_DMA_ADDR_LO(adpt->tx_q[3].tpd.p_addr),
> + adpt->base + EMAC_H3TPD_BASE_ADDR_LO);
> + /* fall through */
> + case 3:
> + writel_relaxed(EMAC_DMA_ADDR_LO(adpt->tx_q[2].tpd.p_addr),
> + adpt->base + EMAC_H2TPD_BASE_ADDR_LO);
> + /* fall through */
> + case 2:
> + writel_relaxed(EMAC_DMA_ADDR_LO(adpt->tx_q[1].tpd.p_addr),
> + adpt->base + EMAC_H1TPD_BASE_ADDR_LO);
> + /* fall through */
> + case 1:
> + writel_relaxed(EMAC_DMA_ADDR_LO(adpt->tx_q[0].tpd.p_addr),
> + adpt->base + EMAC_DESC_CTRL_8);
> + break;
> + default:
> + netdev_err(adpt->netdev,
> + "error: invalid number of TX queues (%d)\n",
> + adpt->tx_q_cnt);
> + return;
> + }

This is not time-critical code. Why not just create a for-loop?

> +/* Config transmit parameters */
> +static void emac_mac_tx_config(struct emac_adapter *adpt)
> +{
> + u16 tx_offload_thresh = EMAC_MAX_TX_OFFLOAD_THRESH;
> + u32 val;
> +
> + writel_relaxed((tx_offload_thresh >> 3) &

Why is tx_offload_thresh a u16 if you're going to use writel anyway?
Make it a u32.

> +void emac_mac_reset(struct emac_adapter *adpt)
> +{
> + writel_relaxed(0, adpt->base + EMAC_INT_MASK);
> + writel_relaxed(DIS_INT, adpt->base + EMAC_INT_STATUS);
> +
> + emac_mac_stop(adpt);
> +
> + emac_reg_update32(adpt->base + EMAC_DMA_MAS_CTRL, 0, SOFT_RST);
> + wmb(); /* ensure mac is fully reset */
> + usleep_range(100, 150);

Please add a comment explaiing why this delay is necessary.

> +void emac_mac_stop(struct emac_adapter *adpt)
> +{
> + emac_reg_update32(adpt->base + EMAC_RXQ_CTRL_0, RXQ_EN, 0);
> + emac_reg_update32(adpt->base + EMAC_TXQ_CTRL_0, TXQ_EN, 0);
> + emac_reg_update32(adpt->base + EMAC_MAC_CTRL, (TXEN | RXEN), 0);
> + wmb(); /* ensure mac is stopped before we proceed */
> + usleep_range(1000, 1050);

Same here.

> +/* set MAC address */
> +void emac_mac_addr_clear(struct emac_adapter *adpt, u8 *addr)
> +{
> + u32 sta;
> +
> + /* for example: 00-A0-C6-11-22-33
> + * 0<-->C6112233, 1<-->00A0.
> + */

/*
* Multi-line comments
* look like this.
*/

> +/* Free all descriptors of given transmit queue */
> +static void emac_tx_q_descs_free(struct emac_adapter *adpt,
> + struct emac_tx_queue *tx_q)
> +{
> + unsigned long size;
> + u32 i;

Since 'i' is used as an index, it should be an unsized integer. And
'size' should be a 'size_t'

> +static void emac_tx_q_descs_free_all(struct emac_adapter *adpt)
> +{
> + u8 i;

'int'. Personally, I'd prefer "unsigned int", but 'int' is what you
use elsewhere.

> +/* Free all descriptors of given receive queue */
> +static void emac_rx_q_free_descs(struct emac_adapter *adpt,
> + struct emac_rx_queue *rx_q)
> +{
> + struct device *dev = adpt->netdev->dev.parent;
> + unsigned long size;
> + u32 i;

size_t size;
int i;

> +/* Allocate TX descriptor ring for the given transmit queue */
> +static int emac_tx_q_desc_alloc(struct emac_adapter *adpt,
> + struct emac_tx_queue *tx_q)
> +{
> + struct emac_ring_header *ring_header = &adpt->ring_header;
> + unsigned long size;

size_t

> +
> + size = sizeof(struct emac_buffer) * tx_q->tpd.count;
> + tx_q->tpd.tpbuff = kzalloc(size, GFP_KERNEL);
> + if (!tx_q->tpd.tpbuff)
> + return -ENOMEM;
> +
> + tx_q->tpd.size = tx_q->tpd.count * (adpt->tpd_size * 4);
> + tx_q->tpd.p_addr = ring_header->p_addr + ring_header->used;
> + tx_q->tpd.v_addr = ring_header->v_addr + ring_header->used;
> + ring_header->used += ALIGN(tx_q->tpd.size, 8);
> + tx_q->tpd.produce_idx = 0;
> + tx_q->tpd.consume_idx = 0;
> + return 0;

blank line above "return".

> +}
> +
> +static int emac_tx_q_desc_alloc_all(struct emac_adapter *adpt)
> +{
> + int retval = 0;
> + u8 i;

int i;

> +static void emac_rx_q_free_bufs_all(struct emac_adapter *adpt)
> +{
> + u8 i;

int i;

> +/* Allocate RX descriptor rings for the given receive queue */
> +static int emac_rx_descs_alloc(struct emac_adapter *adpt,
> + struct emac_rx_queue *rx_q)
> +{
> + struct emac_ring_header *ring_header = &adpt->ring_header;
> + unsigned long size;

size_t

> +static int emac_rx_descs_allocs_all(struct emac_adapter *adpt)
> +{
> + int retval = 0;
> + u8 i;

int i;

> +
> + for (i = 0; i < adpt->rx_q_cnt; i++) {
> + retval = emac_rx_descs_alloc(adpt, &adpt->rx_q[i]);
> + if (retval)
> + break;
> + }
> +
> + if (retval) {
> + netdev_err(adpt->netdev, "error: Rx Queue %u alloc failed\n",

%d

> +/* Produce new receive free descriptor */
> +static bool emac_mac_rx_rfd_create(struct emac_adapter *adpt,
> + struct emac_rx_queue *rx_q,
> + union emac_rfd *rfd)
> +{
> + u32 *hw_rfd = EMAC_RFD(rx_q, adpt->rfd_size,
> + rx_q->rfd.produce_idx);
> +
> + *(hw_rfd++) = rfd->word[0];
> + *hw_rfd = rfd->word[1];
> +
> + if (++rx_q->rfd.produce_idx == rx_q->rfd.count)
> + rx_q->rfd.produce_idx = 0;
> +
> + return true;

You never check the return value, so just make this a void function.

> +}
> +
> +/* Fill up receive queue's RFD with preallocated receive buffers */
> +static int emac_mac_rx_descs_refill(struct emac_adapter *adpt,
> + struct emac_rx_queue *rx_q)
> +{
> + struct emac_buffer *curr_rxbuf;
> + struct emac_buffer *next_rxbuf;
> + union emac_rfd rfd;
> + struct sk_buff *skb;
> + void *skb_data = NULL;
> + u32 count = 0;

int count = 0;

The type should match the return value of the function.

> + u32 next_produce_idx;
> +
> + next_produce_idx = rx_q->rfd.produce_idx;
> + if (++next_produce_idx == rx_q->rfd.count)
> + next_produce_idx = 0;
> + curr_rxbuf = GET_RFD_BUFFER(rx_q, rx_q->rfd.produce_idx);
> + next_rxbuf = GET_RFD_BUFFER(rx_q, next_produce_idx);
> +
> + /* this always has a blank rx_buffer*/
> + while (!next_rxbuf->dma) {
> + skb = dev_alloc_skb(adpt->rxbuf_size + NET_IP_ALIGN);
> + if (unlikely(!skb))
> + break;

I don't think this is time-critical code, so don't use unlikely().

> +/* Consume next received packet descriptor */
> +static bool emac_rx_process_rrd(struct emac_adapter *adpt,
> + struct emac_rx_queue *rx_q,
> + union emac_rrd *rrd)
> +{
> + u32 *hw_rrd = EMAC_RRD(rx_q, adpt->rrd_size,
> + rx_q->rrd.consume_idx);
> +
> + /* If time stamping is enabled, it will be added in the beginning of
> + * the hw rrd (hw_rrd). In sw rrd (rrd), 32bit words 4 & 5 are reserved
> + * for the time stamp; hence the conversion.
> + * Also, read the rrd word with update flag first; read rest of rrd
> + * only if update flag is set.
> + */
> + if (adpt->timestamp_en)
> + rrd->word[3] = *(hw_rrd + 5);
> + else
> + rrd->word[3] = *(hw_rrd + 3);
> + rmb(); /* ensure hw receive returned descriptor timestamp is read */
> +
> + if (!rrd->genr.update)
> + return false;
> +
> + if (adpt->timestamp_en) {
> + rrd->word[4] = *(hw_rrd++);
> + rrd->word[5] = *(hw_rrd++);
> + } else {
> + rrd->word[4] = 0;
> + rrd->word[5] = 0;
> + }
> +
> + rrd->word[0] = *(hw_rrd++);
> + rrd->word[1] = *(hw_rrd++);
> + rrd->word[2] = *(hw_rrd++);
> + mb(); /* ensure descriptor is read */

Why do you use mb() here, but rmb() above? The comment is the same.

> +static void emac_rx_rfd_clean(struct emac_rx_queue *rx_q,
> + union emac_rrd *rrd)
> +{
> + struct emac_buffer *rfbuf = rx_q->rfd.rfbuff;
> + u32 consume_idx = rrd->genr.si;
> + u16 i;

int i;

> +static inline bool emac_skb_cb_expired(struct sk_buff *skb)
> +{
> + if (time_is_after_jiffies(EMAC_SKB_CB(skb)->jiffies +
> + msecs_to_jiffies(100)))
> + return false;
> + return true;

return time_is_before_jiffies(EMAC_SKB_CB(skb)->jiffies +
msecs_to_jiffies(100));

> +/* Process receive event */
> +void emac_mac_rx_process(struct emac_adapter *adpt, struct emac_rx_queue *rx_q,
> + int *num_pkts, int max_pkts)
> +{
> + struct net_device *netdev = adpt->netdev;
> +
> + union emac_rrd rrd;
> + struct emac_buffer *rfbuf;
> + struct sk_buff *skb;
> +
> + u32 hw_consume_idx, num_consume_pkts;
> + u32 count = 0;

unsigned int count;

> + u32 proc_idx;
> + u32 reg = readl_relaxed(adpt->base + rx_q->consume_reg);
> +
> + hw_consume_idx = (reg & rx_q->consume_mask) >> rx_q->consume_shft;
> + num_consume_pkts = (hw_consume_idx >= rx_q->rrd.consume_idx) ?
> + (hw_consume_idx - rx_q->rrd.consume_idx) :
> + (hw_consume_idx + rx_q->rrd.count - rx_q->rrd.consume_idx);
> +
> + while (1) {
> + if (!num_consume_pkts)
> + break;
> +
> + if (!emac_rx_process_rrd(adpt, rx_q, &rrd))
> + break;
> +
> + if (likely(rrd.genr.nor == 1)) {
> + /* good receive */
> + rfbuf = GET_RFD_BUFFER(rx_q, rrd.genr.si);
> + dma_unmap_single(adpt->netdev->dev.parent, rfbuf->dma,
> + rfbuf->length, DMA_FROM_DEVICE);
> + rfbuf->dma = 0;
> + skb = rfbuf->skb;
> + } else {
> + netdev_err(adpt->netdev,
> + "error: multi-RFD not support yet!\n");
> + break;
> + }
> + emac_rx_rfd_clean(rx_q, &rrd);
> + num_consume_pkts--;
> + count++;
> +
> + /* Due to a HW issue in L4 check sum detection (UDP/TCP frags
> + * with DF set are marked as error), drop packets based on the
> + * error mask rather than the summary bit (ignoring L4F errors)
> + */
> + if (rrd.word[EMAC_RRD_STATS_DW_IDX] & EMAC_RRD_ERROR) {
> + netif_dbg(adpt, rx_status, adpt->netdev,
> + "Drop error packet[RRD: 0x%x:0x%x:0x%x:0x%x]\n",
> + rrd.word[0], rrd.word[1],
> + rrd.word[2], rrd.word[3]);
> +
> + dev_kfree_skb(skb);
> + continue;
> + }
> +
> + skb_put(skb, rrd.genr.pkt_len - ETH_FCS_LEN);
> + skb->dev = netdev;
> + skb->protocol = eth_type_trans(skb, skb->dev);
> + if (netdev->features & NETIF_F_RXCSUM)
> + skb->ip_summed = ((rrd.genr.l4f) ?
> + CHECKSUM_NONE : CHECKSUM_UNNECESSARY);
> + else
> + skb_checksum_none_assert(skb);
> +
> + if (test_bit(EMAC_STATUS_TS_RX_EN, &adpt->status)) {
> + struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
> +
> + hwts->hwtstamp = ktime_set(rrd.genr.ts_high,
> + rrd.genr.ts_low);
> + }
> +
> + emac_receive_skb(rx_q, skb, (u16)rrd.genr.cvlan_tag,
> + (bool)rrd.genr.cvlan_flag);
> +
> + netdev->last_rx = jiffies;
> + (*num_pkts)++;
> + if (*num_pkts >= max_pkts)
> + break;
> + }

How about

do {
...
} while (*num_pkts < max_pkts);

> +/* Check if enough transmit descriptors are available */
> +static bool emac_tx_has_enough_descs(struct emac_tx_queue *tx_q,
> + const struct sk_buff *skb)
> +{
> + u32 num_required = 1;
> + u16 i;

int i;

Anyway, you got the idea. I think sized integers should be used
sparingly, and general counting and index variable should be unsized
integers, preferably also unsigned.

> +/* Fill up transmit descriptors with TSO and Checksum offload information */
> +static int emac_tso_csum(struct emac_adapter *adpt,
> + struct emac_tx_queue *tx_q,
> + struct sk_buff *skb,
> + union emac_tpd *tpd)
> +{
> + u8 hdr_len;
> + int retval;
> +
> + if (skb_is_gso(skb)) {
> + if (skb_header_cloned(skb)) {
> + retval = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
> + if (unlikely(retval))
> + return retval;
> + }
> +
> + if (skb->protocol == htons(ETH_P_IP)) {
> + u32 pkt_len =
> + ((unsigned char *)ip_hdr(skb) - skb->data) +

Use void* for pointer math, instead of "unsigned char *".

> +/* Transmit the packet using specified transmit queue */
> +int emac_mac_tx_buf_send(struct emac_adapter *adpt, struct emac_tx_queue *tx_q,
> + struct sk_buff *skb)
> +{
> + union emac_tpd tpd;
> + u32 prod_idx;
> +
> + if (test_bit(EMAC_STATUS_DOWN, &adpt->status)) {
> + dev_kfree_skb_any(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + if (!emac_tx_has_enough_descs(tx_q, skb)) {
> + /* not enough descriptors, just stop queue */
> + netif_stop_queue(adpt->netdev);
> + return NETDEV_TX_BUSY;
> + }
> +
> + memset(&tpd, 0, sizeof(tpd));
> +
> + if (emac_tso_csum(adpt, tx_q, skb, &tpd) != 0) {
> + dev_kfree_skb_any(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + if (skb_vlan_tag_present(skb)) {
> + u16 vlan = skb_vlan_tag_get(skb);
> + u16 tag;
> +
> + EMAC_VLAN_TO_TAG(vlan, tag);
> + tpd.genr.cvlan_tag = tag;

Can't you just do EMAC_VLAN_TO_TAG(vlan, tpd.genr.cvlan_tag);

> diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.h b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
> new file mode 100644
> index 0000000..a6761af
> --- /dev/null
> +++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
> @@ -0,0 +1,341 @@
> +/* Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +/* EMAC DMA HW engine uses three rings:
> + * Tx:
> + * TPD: Transmit Packet Descriptor ring.
> + * Rx:
> + * RFD: Receive Free Descriptor ring.
> + * Ring of descriptors with empty buffers to be filled by Rx HW.
> + * RRD: Receive Return Descriptor ring.
> + * Ring of descriptors with buffers filled with received data.
> + */
> +
> +#ifndef _EMAC_HW_H_
> +#define _EMAC_HW_H_
> +
> +/* EMAC_CSR register offsets */
> +#define EMAC_EMAC_WRAPPER_CSR1 0x000000
> +#define EMAC_EMAC_WRAPPER_CSR2 0x000004
> +#define EMAC_EMAC_WRAPPER_CSR3 0x000008
> +#define EMAC_EMAC_WRAPPER_CSR5 0x000010
> +#define EMAC_EMAC_WRAPPER_TX_TS_LO 0x000104
> +#define EMAC_EMAC_WRAPPER_TX_TS_HI 0x000108
> +#define EMAC_EMAC_WRAPPER_TX_TS_INX 0x00010c

Can you move some of the macros into the .c files? For example, I'm
pretty sure that the EMAC_EMAC_WRAPPER_CSRx macros are used only in
emac-sgmii.c.

Anyway, I'm stopping for now. I'll post more later.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Joe Perches: "Re: [PATCH v2] of/address: replace printk() with pr_debug() / pr_err()"
Previous message: Greg Kroah-Hartman: "Re: [PATCH 1/4] usb: host: ehci.h: cleanup header file"
Next in thread: Fabio Estevam: "Re: [PATCH] net: emac: emac gigabit ethernet controller driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]