Changes wrt. v1:
- added some cleanup patches
- simplify handling of 'quirks' flags as suggested by Russell King.
- remove DIV_ROUND_UP() from byte swapping loop as suggested by
Eric Dumazet
Subject:
In-Reply-To:
commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
introduced a regression for i.MX28. The swap_buffer() function doing
the endian conversion of the received data on i.MX28 may access memory
beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
does not copy those bytes, so that the last bytes of a packet may be
filled with invalid data after swapping.
This will likely lead to checksum errors on received packets.
E.g. when trying to mount an NFS rootfs:
UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
Do the byte swapping and copying to the new skb in one go if
necessary.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 404fb9d..b92324c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -339,6 +339,18 @@ static void *swap_buffer(void *bufaddr, int len)
return bufaddr;
}
+static void *swap_buffer2(void *dst_buf, void *src_buf, int len)
+{
+ int i;
+ unsigned int *src = src_buf;
+ unsigned int *dst = dst_buf;
+
+ for (i = 0; i < len; i += 4, src++, dst++)
+ swab32s(src);
+
+ return dst_buf;
+}
+
static void fec_dump(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
@@ -1334,7 +1346,7 @@ fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff
}
static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
- struct bufdesc *bdp, u32 length)
+ struct bufdesc *bdp, u32 length, bool swap)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct sk_buff *new_skb;
@@ -1349,7 +1361,10 @@ static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
FEC_ENET_RX_FRSIZE - fep->rx_align,
DMA_FROM_DEVICE);
- memcpy(new_skb->data, (*skb)->data, length);
+ if (!swap)
+ memcpy(new_skb->data, (*skb)->data, length);
+ else
+ swap_buffer2(new_skb->data, (*skb)->data, length);
*skb = new_skb;
return true;
@@ -1377,6 +1392,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
u16 vlan_tag;
int index = 0;
bool is_copybreak;
+ bool need_swap = fep->quirks & FEC_QUIRK_SWAP_FRAME;
#ifdef CONFIG_M532x
flush_cache_all();
@@ -1440,7 +1456,8 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
* include that when passing upstream as it messes up
* bridging applications.
*/
- is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4);
+ is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4,
+ need_swap);
if (!is_copybreak) {
skb_new = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
if (unlikely(!skb_new)) {
@@ -1455,7 +1472,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
prefetch(skb->data - NET_IP_ALIGN);
skb_put(skb, pkt_len - 4);
data = skb->data;
- if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
+ if (!is_copybreak && need_swap)
swap_buffer(data, pkt_len);
/* Extract the enhanced buffer descriptor */
--
1.7.10.4
fep->bufdesc_ex is used as boolean flag; thus declare it as such.
Also remove an unnecessary initialization to 0.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec.h | 2 +-
drivers/net/ethernet/freescale/fec_main.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 1e65917..2634de2 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -451,7 +451,7 @@ struct fec_enet_private {
int speed;
struct completion mdio_done;
int irq[FEC_IRQ_NUM];
- int bufdesc_ex;
+ bool bufdesc_ex;
int pause_flag;
struct napi_struct napi;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index decea57..f7d344f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3209,8 +3209,6 @@ fec_probe(struct platform_device *pdev)
fep->pdev = pdev;
fep->dev_id = dev_id++;
- fep->bufdesc_ex = 0;
-
platform_set_drvdata(pdev, ndev);
phy_node = of_parse_phandle(np, "phy-handle", 0);
@@ -3268,7 +3266,7 @@ fec_probe(struct platform_device *pdev)
pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
if (IS_ERR(fep->clk_ptp)) {
fep->clk_ptp = NULL;
- fep->bufdesc_ex = 0;
+ fep->bufdesc_ex = false;
}
ret = fec_enet_clk_enable(ndev, true);
--
1.7.10.4
From: Lothar Waßmann
> commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> introduced a regression for i.MX28. The swap_buffer() function doing
> the endian conversion of the received data on i.MX28 may access memory
> beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> does not copy those bytes, so that the last bytes of a packet may be
> filled with invalid data after swapping.
> This will likely lead to checksum errors on received packets.
> E.g. when trying to mount an NFS rootfs:
> UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
>
> Do the byte swapping and copying to the new skb in one go if
> necessary.
>
> Signed-off-by: Lothar Wamann <[email protected]>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 404fb9d..b92324c 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -339,6 +339,18 @@ static void *swap_buffer(void *bufaddr, int len)
> return bufaddr;
> }
>
> +static void *swap_buffer2(void *dst_buf, void *src_buf, int len)
> +{
> + int i;
> + unsigned int *src = src_buf;
> + unsigned int *dst = dst_buf;
> +
> + for (i = 0; i < len; i += 4, src++, dst++)
> + swab32s(src);
This will probably benefit from being unrolled slightly.
Neither 'dst' nor the return value is used.
> +
> + return dst_buf;
> +}
> +
> static void fec_dump(struct net_device *ndev)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
> @@ -1334,7 +1346,7 @@ fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff
> }
>
> static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
> - struct bufdesc *bdp, u32 length)
> + struct bufdesc *bdp, u32 length, bool swap)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
> struct sk_buff *new_skb;
> @@ -1349,7 +1361,10 @@ static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
> dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
> FEC_ENET_RX_FRSIZE - fep->rx_align,
> DMA_FROM_DEVICE);
> - memcpy(new_skb->data, (*skb)->data, length);
> + if (!swap)
> + memcpy(new_skb->data, (*skb)->data, length);
> + else
> + swap_buffer2(new_skb->data, (*skb)->data, length);
> *skb = new_skb;
>
> return true;
> @@ -1377,6 +1392,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> u16 vlan_tag;
> int index = 0;
> bool is_copybreak;
> + bool need_swap = fep->quirks & FEC_QUIRK_SWAP_FRAME;
>
> #ifdef CONFIG_M532x
> flush_cache_all();
> @@ -1440,7 +1456,8 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> * include that when passing upstream as it messes up
> * bridging applications.
> */
> - is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4);
> + is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4,
> + need_swap);
> if (!is_copybreak) {
> skb_new = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
> if (unlikely(!skb_new)) {
> @@ -1455,7 +1472,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> prefetch(skb->data - NET_IP_ALIGN);
> skb_put(skb, pkt_len - 4);
> data = skb->data;
> - if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> + if (!is_copybreak && need_swap)
> swap_buffer(data, pkt_len);
It has to be better to set the 'copybreak' limit to be larger than the
maximum frame size and so always go through the 'copybreak' paths.
>
> /* Extract the enhanced buffer descriptor */
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
From: Lothar Waßmann
> commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> introduced a regression for i.MX28. The swap_buffer() function doing
> the endian conversion of the received data on i.MX28 may access memory
> beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> does not copy those bytes, so that the last bytes of a packet may be
> filled with invalid data after swapping.
> This will likely lead to checksum errors on received packets.
> E.g. when trying to mount an NFS rootfs:
> UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
>
> Do the byte swapping and copying to the new skb in one go if
> necessary.
>
> Signed-off-by: Lothar Wamann <[email protected]>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 404fb9d..b92324c 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -339,6 +339,18 @@ static void *swap_buffer(void *bufaddr, int len)
> return bufaddr;
> }
>
> +static void *swap_buffer2(void *dst_buf, void *src_buf, int len)
> +{
> + int i;
> + unsigned int *src = src_buf;
> + unsigned int *dst = dst_buf;
> +
> + for (i = 0; i < len; i += 4, src++, dst++)
> + swab32s(src);
> +
> + return dst_buf;
> +}
> +
Actually that is completely f*cked....
David
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
when swap_buffer() is being called, we know for sure, that we need to
byte swap the data. Also this function is called for swapping data in
both directions. Thus cpu_to_be32() is semantically not correct for
all use cases. Use swab32s() to reflect this.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 323ae2e..3a103e9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -334,7 +334,7 @@ static void *swap_buffer(void *bufaddr, int len)
unsigned int *buf = bufaddr;
for (i = 0; i < DIV_ROUND_UP(len, 4); i++, buf++)
- *buf = cpu_to_be32(*buf);
+ swab32s(buf);
return bufaddr;
}
--
1.7.10.4
Eliminate the DIV_ROUND_UP() and change the loop counter increment to
4 instead. This results in saving 6 instructions in the functions
assembly code.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 3a103e9..404fb9d 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -333,7 +333,7 @@ static void *swap_buffer(void *bufaddr, int len)
int i;
unsigned int *buf = bufaddr;
- for (i = 0; i < DIV_ROUND_UP(len, 4); i++, buf++)
+ for (i = 0; i < len; i += 4, buf++)
swab32s(buf);
return bufaddr;
--
1.7.10.4
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e364d1f..decea57 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -101,7 +101,7 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
* frames not being transmitted until there is a 0-to-1 transition on
* ENET_TDAR[TDAR].
*/
-#define FEC_QUIRK_ERR006358 (1 << 7)
+#define FEC_QUIRK_ERR006358 (1 << 7)
/* ENET IP hw AVB
*
* i.MX6SX ENET IP add Audio Video Bridging (AVB) feature support.
--
1.7.10.4
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec.h | 1 +
drivers/net/ethernet/freescale/fec_main.c | 105 ++++++++++-------------------
2 files changed, 38 insertions(+), 68 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 2634de2..d43c1d3 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -453,6 +453,7 @@ struct fec_enet_private {
int irq[FEC_IRQ_NUM];
bool bufdesc_ex;
int pause_flag;
+ u32 quirks;
struct napi_struct napi;
int csum_flags;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f7d344f..323ae2e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -392,8 +392,6 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct bufdesc *bdp = txq->cur_tx;
struct bufdesc_ex *ebdp;
int nr_frags = skb_shinfo(skb)->nr_frags;
@@ -429,7 +427,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
}
if (fep->bufdesc_ex) {
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -441,11 +439,11 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
index = fec_enet_get_bd_index(txq->tx_bd_base, bdp, fep);
if (((unsigned long) bufaddr) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], bufaddr, frag_len);
bufaddr = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(bufaddr, frag_len);
}
@@ -481,8 +479,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq,
struct sk_buff *skb, struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
int nr_frags = skb_shinfo(skb)->nr_frags;
struct bufdesc *bdp, *last_bdp;
void *bufaddr;
@@ -521,11 +517,11 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq,
queue = skb_get_queue_mapping(skb);
index = fec_enet_get_bd_index(txq->tx_bd_base, bdp, fep);
if (((unsigned long) bufaddr) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], skb->data, buflen);
bufaddr = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(bufaddr, buflen);
}
@@ -560,7 +556,7 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq,
fep->hwts_tx_en))
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -604,8 +600,6 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
int size, bool last_tcp, bool is_last)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc);
unsigned short queue = skb_get_queue_mapping(skb);
unsigned short status;
@@ -618,11 +612,11 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
if (((unsigned long) data) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], data, size);
data = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(data, size);
}
@@ -638,7 +632,7 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
bdp->cbd_bufaddr = addr;
if (fep->bufdesc_ex) {
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -666,8 +660,6 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
struct bufdesc *bdp, int index)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc);
unsigned short queue = skb_get_queue_mapping(skb);
@@ -683,11 +675,11 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
bufaddr = txq->tso_hdrs + index * TSO_HEADER_SIZE;
dmabuf = txq->tso_hdrs_dma + index * TSO_HEADER_SIZE;
if (((unsigned long)bufaddr) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], skb->data, hdr_len);
bufaddr = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(bufaddr, hdr_len);
dmabuf = dma_map_single(&fep->pdev->dev, bufaddr,
@@ -704,7 +696,7 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
bdp->cbd_datlen = hdr_len;
if (fep->bufdesc_ex) {
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -729,8 +721,6 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
struct tso_t tso;
unsigned int index = 0;
int ret;
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep, txq)) {
dev_kfree_skb_any(skb);
@@ -792,7 +782,7 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
txq->cur_tx = bdp;
/* Trigger transmission start */
- if (!(id_entry->driver_data & FEC_QUIRK_ERR007885) ||
+ if (!(fep->quirks & FEC_QUIRK_ERR007885) ||
!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
@@ -955,8 +945,6 @@ static void
fec_restart(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
u32 val;
u32 temp_mac[2];
u32 rcntl = OPT_FRAME_SIZE | 0x04;
@@ -966,7 +954,7 @@ fec_restart(struct net_device *ndev)
* For i.MX6SX SOC, enet use AXI bus, we use disable MAC
* instead of reset MAC itself.
*/
- if (id_entry && id_entry->driver_data & FEC_QUIRK_HAS_AVB) {
+ if (fep->quirks & FEC_QUIRK_HAS_AVB) {
writel(0, fep->hwp + FEC_ECNTRL);
} else {
writel(1, fep->hwp + FEC_ECNTRL);
@@ -977,7 +965,7 @@ fec_restart(struct net_device *ndev)
* enet-mac reset will reset mac address registers too,
* so need to reconfigure it.
*/
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
writel(cpu_to_be32(temp_mac[0]), fep->hwp + FEC_ADDR_LOW);
writel(cpu_to_be32(temp_mac[1]), fep->hwp + FEC_ADDR_HIGH);
@@ -1023,7 +1011,7 @@ fec_restart(struct net_device *ndev)
* The phy interface and speed need to get configured
* differently on enet-mac.
*/
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
/* Enable flow control and length check */
rcntl |= 0x40000000 | 0x00000020;
@@ -1046,7 +1034,7 @@ fec_restart(struct net_device *ndev)
}
} else {
#ifdef FEC_MIIGSK_ENR
- if (id_entry->driver_data & FEC_QUIRK_USE_GASKET) {
+ if (fep->quirks & FEC_QUIRK_USE_GASKET) {
u32 cfgr;
/* disable the gasket and wait */
writel(0, fep->hwp + FEC_MIIGSK_ENR);
@@ -1099,7 +1087,7 @@ fec_restart(struct net_device *ndev)
writel(0, fep->hwp + FEC_HASH_TABLE_LOW);
#endif
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
/* enable ENET endian swap */
ecntl |= (1 << 8);
/* enable ENET store and forward mode */
@@ -1133,8 +1121,6 @@ static void
fec_stop(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8);
/* We cannot expect a graceful transmit stop without link !!! */
@@ -1149,7 +1135,7 @@ fec_stop(struct net_device *ndev)
* For i.MX6SX SOC, enet use AXI bus, we use disable MAC
* instead of reset MAC itself.
*/
- if (id_entry && id_entry->driver_data & FEC_QUIRK_HAS_AVB) {
+ if (fep->quirks & FEC_QUIRK_HAS_AVB) {
writel(0, fep->hwp + FEC_ECNTRL);
} else {
writel(1, fep->hwp + FEC_ECNTRL);
@@ -1159,7 +1145,7 @@ fec_stop(struct net_device *ndev)
writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
/* We have to keep ENET enabled to have MII interrupt stay working */
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
writel(2, fep->hwp + FEC_ECNTRL);
writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
}
@@ -1378,8 +1364,6 @@ static int
fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct fec_enet_priv_rx_q *rxq;
struct bufdesc *bdp;
unsigned short status;
@@ -1471,7 +1455,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
prefetch(skb->data - NET_IP_ALIGN);
skb_put(skb, pkt_len - 4);
data = skb->data;
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(data, pkt_len);
/* Extract the enhanced buffer descriptor */
@@ -1905,8 +1889,6 @@ failed_clk_ipg:
static int fec_enet_mii_probe(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct phy_device *phy_dev = NULL;
char mdio_bus_id[MII_BUS_ID_SIZE];
char phy_name[MII_BUS_ID_SIZE + 3];
@@ -1952,7 +1934,7 @@ static int fec_enet_mii_probe(struct net_device *ndev)
}
/* mask with MAC supported features */
- if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) {
+ if (fep->quirks & FEC_QUIRK_HAS_GBIT) {
phy_dev->supported &= PHY_GBIT_FEATURES;
phy_dev->supported &= ~SUPPORTED_1000baseT_Half;
#if !defined(CONFIG_M5272)
@@ -1980,8 +1962,6 @@ static int fec_enet_mii_init(struct platform_device *pdev)
static struct mii_bus *fec0_mii_bus;
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct device_node *node;
int err = -ENXIO, i;
@@ -2001,7 +1981,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
* mdio interface in board design, and need to be configured by
* fec0 mii_bus.
*/
- if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
+ if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
/* fec1 uses fec0 mii_bus */
if (mii_cnt && fec0_mii_bus) {
fep->mii_bus = fec0_mii_bus;
@@ -2022,7 +2002,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
* document.
*/
fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk_ipg), 5000000);
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
+ if (fep->quirks & FEC_QUIRK_ENET_MAC)
fep->phy_speed--;
fep->phy_speed <<= 1;
writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
@@ -2064,7 +2044,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
mii_cnt++;
/* save fec0 mii_bus */
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
+ if (fep->quirks & FEC_QUIRK_ENET_MAC)
fec0_mii_bus = fep->mii_bus;
return 0;
@@ -2333,11 +2313,9 @@ static int fec_enet_us_to_itr_clock(struct net_device *ndev, int us)
static void fec_enet_itr_coal_set(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
int rx_itr, tx_itr;
- if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
return;
/* Must be greater than zero to avoid unpredictable behavior */
@@ -2372,10 +2350,8 @@ static int
fec_enet_get_coalesce(struct net_device *ndev, struct ethtool_coalesce *ec)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
- if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
return -EOPNOTSUPP;
ec->rx_coalesce_usecs = fep->rx_time_itr;
@@ -2391,12 +2367,9 @@ static int
fec_enet_set_coalesce(struct net_device *ndev, struct ethtool_coalesce *ec)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
-
unsigned int cycle;
- if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
return -EOPNOTSUPP;
if (ec->rx_max_coalesced_frames > 255) {
@@ -2976,8 +2949,6 @@ static const struct net_device_ops fec_netdev_ops = {
static int fec_enet_init(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct fec_enet_priv_tx_q *txq;
struct fec_enet_priv_rx_q *rxq;
struct bufdesc *cbd_base;
@@ -3056,11 +3027,11 @@ static int fec_enet_init(struct net_device *ndev)
writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK);
netif_napi_add(ndev, &fep->napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT);
- if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN)
+ if (fep->quirks & FEC_QUIRK_HAS_VLAN)
/* enable hw VLAN support */
ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
- if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) {
+ if (fep->quirks & FEC_QUIRK_HAS_CSUM) {
ndev->gso_max_segs = FEC_MAX_TSO_SEGS;
/* enable hw accelerator */
@@ -3069,7 +3040,7 @@ static int fec_enet_init(struct net_device *ndev)
fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
}
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB) {
+ if (fep->quirks & FEC_QUIRK_HAS_AVB) {
fep->tx_align = 0;
fep->rx_align = 0x3f;
}
@@ -3169,10 +3140,6 @@ fec_probe(struct platform_device *pdev)
int num_tx_qs;
int num_rx_qs;
- of_id = of_match_device(fec_dt_ids, &pdev->dev);
- if (of_id)
- pdev->id_entry = of_id->data;
-
fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
/* Init network device */
@@ -3186,13 +3153,16 @@ fec_probe(struct platform_device *pdev)
/* setup board info structure */
fep = netdev_priv(ndev);
+ of_id = of_match_device(fec_dt_ids, &pdev->dev);
+ if (of_id)
+ fep->quirks = (u32)of_id->data;
+
fep->num_rx_queues = num_rx_qs;
fep->num_tx_queues = num_tx_qs;
#if !defined(CONFIG_M5272)
/* default enable pause frame auto negotiation */
- if (pdev->id_entry &&
- (pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT))
+ if (fep->quirks & FEC_QUIRK_HAS_GBIT)
fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG;
#endif
@@ -3261,9 +3231,8 @@ fec_probe(struct platform_device *pdev)
if (IS_ERR(fep->clk_ref))
fep->clk_ref = NULL;
+ fep->bufdesc_ex = fep->quirks & FEC_QUIRK_HAS_BUFDESC_EX;
fep->clk_ptp = devm_clk_get(&pdev->dev, "ptp");
- fep->bufdesc_ex =
- pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
if (IS_ERR(fep->clk_ptp)) {
fep->clk_ptp = NULL;
fep->bufdesc_ex = false;
--
1.7.10.4
Hi,
David Laight wrote:
> From: Lothar Waßmann
> > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > introduced a regression for i.MX28. The swap_buffer() function doing
> > the endian conversion of the received data on i.MX28 may access memory
> > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > does not copy those bytes, so that the last bytes of a packet may be
> > filled with invalid data after swapping.
> > This will likely lead to checksum errors on received packets.
> > E.g. when trying to mount an NFS rootfs:
> > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> >
> > Do the byte swapping and copying to the new skb in one go if
> > necessary.
> >
> > Signed-off-by: Lothar Wamann <[email protected]>
> > ---
> > drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> > 1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> > index 404fb9d..b92324c 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -339,6 +339,18 @@ static void *swap_buffer(void *bufaddr, int len)
> > return bufaddr;
> > }
> >
> > +static void *swap_buffer2(void *dst_buf, void *src_buf, int len)
> > +{
> > + int i;
> > + unsigned int *src = src_buf;
> > + unsigned int *dst = dst_buf;
> > +
> > + for (i = 0; i < len; i += 4, src++, dst++)
> > + swab32s(src);
> > +
> > + return dst_buf;
> > +}
> > +
>
> Actually that is completely f*cked....
>
Yeah, noticed that shortly after sending out. :(
One change too many...
Lothar Waßmann
Hi,
David Laight wrote:
> From: Lothar Waßmann
> > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > introduced a regression for i.MX28. The swap_buffer() function doing
> > the endian conversion of the received data on i.MX28 may access memory
> > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > does not copy those bytes, so that the last bytes of a packet may be
> > filled with invalid data after swapping.
> > This will likely lead to checksum errors on received packets.
> > E.g. when trying to mount an NFS rootfs:
> > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> >
> > Do the byte swapping and copying to the new skb in one go if
> > necessary.
> >
> > Signed-off-by: Lothar Wamann <[email protected]>
> > ---
> > drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> > 1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > @@ -1455,7 +1472,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> > prefetch(skb->data - NET_IP_ALIGN);
> > skb_put(skb, pkt_len - 4);
> > data = skb->data;
> > - if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> > + if (!is_copybreak && need_swap)
> > swap_buffer(data, pkt_len);
>
> It has to be better to set the 'copybreak' limit to be larger than the
> maximum frame size and so always go through the 'copybreak' paths.
>
Since the copybreak support is all about performance optimistation, we
should IMO buy the additional advantage for i.MX28 by not having to
access the buffer twice (once for copying and once again for swapping).
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
From: Lothar Waßmann
> David Laight wrote:
> > From: Lothar Waßmann
> > > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > > introduced a regression for i.MX28. The swap_buffer() function doing
> > > the endian conversion of the received data on i.MX28 may access memory
> > > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > > does not copy those bytes, so that the last bytes of a packet may be
> > > filled with invalid data after swapping.
> > > This will likely lead to checksum errors on received packets.
> > > E.g. when trying to mount an NFS rootfs:
> > > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> > >
> > > Do the byte swapping and copying to the new skb in one go if
> > > necessary.
> > >
> > > Signed-off-by: Lothar Wamann <[email protected]>
> > > ---
> > > drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> > > 1 file changed, 21 insertions(+), 4 deletions(-)
> > >
> > > @@ -1455,7 +1472,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> > > prefetch(skb->data - NET_IP_ALIGN);
> > > skb_put(skb, pkt_len - 4);
> > > data = skb->data;
> > > - if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> > > + if (!is_copybreak && need_swap)
> > > swap_buffer(data, pkt_len);
> >
> > It has to be better to set the 'copybreak' limit to be larger than the
> > maximum frame size and so always go through the 'copybreak' paths.
> >
> Since the copybreak support is all about performance optimistation, we
> should IMO buy the additional advantage for i.MX28 by not having to
> access the buffer twice (once for copying and once again for swapping).
You definitely want to do the byteswap at the same time as the copy.
The point I'm trying to make that if you need to do the byteswap you
probably might as well copy the data to an skb of the correct size at
the same time.
Certainly I'd expect the 'break even' length will be much higher.
David
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
introduced a regression for i.MX28. The swap_buffer() function doing
the endian conversion of the received data on i.MX28 may access memory
beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
does not copy those bytes, so that the last bytes of a packet may be
filled with invalid data after swapping.
This will likely lead to checksum errors on received packets.
E.g. when trying to mount an NFS rootfs:
UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
Do the byte swapping and copying to the new skb in one go if
necessary.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e52864c..5ee4912 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -294,7 +294,16 @@ static void swap_buffer(void *bufaddr, int len)
for (i = 0; i < len; i += 4, buf++)
swab32s(buf);
+}
+
+static void swap_buffer2(void *dst_buf, void *src_buf, int len)
+{
+ int i;
+ unsigned int *src = src_buf;
+ unsigned int *dst = dst_buf;
+ for (i = 0; i < len; i += 4, src++, dst++)
+ *dst = swab32p(src);
}
static void fec_dump(struct net_device *ndev)
@@ -1292,7 +1301,7 @@ fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff
}
static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
- struct bufdesc *bdp, u32 length)
+ struct bufdesc *bdp, u32 length, bool swap)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct sk_buff *new_skb;
@@ -1307,7 +1316,10 @@ static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
FEC_ENET_RX_FRSIZE - fep->rx_align,
DMA_FROM_DEVICE);
- memcpy(new_skb->data, (*skb)->data, length);
+ if (!swap)
+ memcpy(new_skb->data, (*skb)->data, length);
+ else
+ swap_buffer2(new_skb->data, (*skb)->data, length);
*skb = new_skb;
return true;
@@ -1335,6 +1347,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
u16 vlan_tag;
int index = 0;
bool is_copybreak;
+ bool need_swap = fep->quirks & FEC_QUIRK_SWAP_FRAME;
#ifdef CONFIG_M532x
flush_cache_all();
@@ -1398,7 +1411,8 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
* include that when passing upstream as it messes up
* bridging applications.
*/
- is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4);
+ is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4,
+ need_swap);
if (!is_copybreak) {
skb_new = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
if (unlikely(!skb_new)) {
@@ -1413,7 +1427,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
prefetch(skb->data - NET_IP_ALIGN);
skb_put(skb, pkt_len - 4);
data = skb->data;
- if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
+ if (!is_copybreak && need_swap)
swap_buffer(data, pkt_len);
/* Extract the enhanced buffer descriptor */
--
1.7.10.4
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec.h | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index e778b9e..27e75cf 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -279,36 +279,37 @@ struct bufdesc_ex {
#define FEC_ENET_MAX_TX_QS 3
#define FEC_ENET_MAX_RX_QS 3
-#define FEC_R_DES_START(X) ((X == 1) ? FEC_R_DES_START_1 : \
- ((X == 2) ? \
+#define FEC_R_DES_START(X) (((X) == 1) ? FEC_R_DES_START_1 : \
+ (((X) == 2) ? \
FEC_R_DES_START_2 : FEC_R_DES_START_0))
-#define FEC_X_DES_START(X) ((X == 1) ? FEC_X_DES_START_1 : \
- ((X == 2) ? \
+#define FEC_X_DES_START(X) (((X) == 1) ? FEC_X_DES_START_1 : \
+ (((X) == 2) ? \
FEC_X_DES_START_2 : FEC_X_DES_START_0))
-#define FEC_R_DES_ACTIVE(X) ((X == 1) ? FEC_R_DES_ACTIVE_1 : \
- ((X == 2) ? \
+#define FEC_R_DES_ACTIVE(X) (((X) == 1) ? FEC_R_DES_ACTIVE_1 : \
+ (((X) == 2) ? \
FEC_R_DES_ACTIVE_2 : FEC_R_DES_ACTIVE_0))
-#define FEC_X_DES_ACTIVE(X) ((X == 1) ? FEC_X_DES_ACTIVE_1 : \
- ((X == 2) ? \
+#define FEC_X_DES_ACTIVE(X) (((X) == 1) ? FEC_X_DES_ACTIVE_1 : \
+ (((X) == 2) ? \
FEC_X_DES_ACTIVE_2 : FEC_X_DES_ACTIVE_0))
-#define FEC_DMA_CFG(X) ((X == 2) ? FEC_DMA_CFG_2 : FEC_DMA_CFG_1)
+#define FEC_DMA_CFG(X) (((X) == 2) ? FEC_DMA_CFG_2 : FEC_DMA_CFG_1)
#define DMA_CLASS_EN (1 << 16)
-#define FEC_RCMR(X) ((X == 2) ? FEC_RCMR_2 : FEC_RCMR_1)
+#define FEC_RCMR(X) (((X) == 2) ? FEC_RCMR_2 : FEC_RCMR_1)
#define IDLE_SLOPE_MASK 0xffff
#define IDLE_SLOPE_1 0x200 /* BW fraction: 0.5 */
#define IDLE_SLOPE_2 0x200 /* BW fraction: 0.5 */
-#define IDLE_SLOPE(X) ((X == 1) ? (IDLE_SLOPE_1 & IDLE_SLOPE_MASK) : \
+#define IDLE_SLOPE(X) (((X) == 1) ? \
+ (IDLE_SLOPE_1 & IDLE_SLOPE_MASK) : \
(IDLE_SLOPE_2 & IDLE_SLOPE_MASK))
#define RCMR_MATCHEN (0x1 << 16)
-#define RCMR_CMP_CFG(v, n) ((v & 0x7) << (n << 2))
+#define RCMR_CMP_CFG(v, n) (((v) & 0x7) << (n << 2))
#define RCMR_CMP_1 (RCMR_CMP_CFG(0, 0) | RCMR_CMP_CFG(1, 1) | \
RCMR_CMP_CFG(2, 2) | RCMR_CMP_CFG(3, 3))
#define RCMR_CMP_2 (RCMR_CMP_CFG(4, 0) | RCMR_CMP_CFG(5, 1) | \
RCMR_CMP_CFG(6, 2) | RCMR_CMP_CFG(7, 3))
-#define RCMR_CMP(X) ((X == 1) ? RCMR_CMP_1 : RCMR_CMP_2)
-#define FEC_TX_BD_FTYPE(X) ((X & 0xf) << 20)
+#define RCMR_CMP(X) (((X) == 1) ? RCMR_CMP_1 : RCMR_CMP_2)
+#define FEC_TX_BD_FTYPE(X) (((X) & 0xf) << 20)
/* The number of Tx and Rx buffers. These are allocated from the page
* pool. The code may assume these are power of two, so it it best
@@ -359,7 +360,7 @@ struct bufdesc_ex {
/* ENET interrupt coalescing macro define */
#define FEC_ITR_CLK_SEL (0x1 << 30)
#define FEC_ITR_EN (0x1 << 31)
-#define FEC_ITR_ICFT(X) ((X & 0xff) << 20)
+#define FEC_ITR_ICFT(X) (((X) & 0xff) << 20)
#define FEC_ITR_ICTT(X) ((X) & 0xffff)
#define FEC_ITR_ICFT_DEFAULT 200 /* Set 200 frame count threshold */
#define FEC_ITR_ICTT_DEFAULT 1000 /* Set 1000us timer threshold */
--
1.7.10.4
consistently use TABs for indentation
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec.h | 108 +++++++++++++++++-----------------
1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 9af296a..3047db4 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -213,60 +213,60 @@ struct bufdesc_ex {
* The following definitions courtesy of commproc.h, which where
* Copyright (c) 1997 Dan Malek ([email protected]).
*/
-#define BD_SC_EMPTY ((ushort)0x8000) /* Receive is empty */
-#define BD_SC_READY ((ushort)0x8000) /* Transmit is ready */
-#define BD_SC_WRAP ((ushort)0x2000) /* Last buffer descriptor */
-#define BD_SC_INTRPT ((ushort)0x1000) /* Interrupt on change */
-#define BD_SC_CM ((ushort)0x0200) /* Continuous mode */
-#define BD_SC_ID ((ushort)0x0100) /* Rec'd too many idles */
-#define BD_SC_P ((ushort)0x0100) /* xmt preamble */
-#define BD_SC_BR ((ushort)0x0020) /* Break received */
-#define BD_SC_FR ((ushort)0x0010) /* Framing error */
-#define BD_SC_PR ((ushort)0x0008) /* Parity error */
-#define BD_SC_OV ((ushort)0x0002) /* Overrun */
-#define BD_SC_CD ((ushort)0x0001) /* ?? */
+#define BD_SC_EMPTY ((ushort)0x8000) /* Receive is empty */
+#define BD_SC_READY ((ushort)0x8000) /* Transmit is ready */
+#define BD_SC_WRAP ((ushort)0x2000) /* Last buffer descriptor */
+#define BD_SC_INTRPT ((ushort)0x1000) /* Interrupt on change */
+#define BD_SC_CM ((ushort)0x0200) /* Continuous mode */
+#define BD_SC_ID ((ushort)0x0100) /* Rec'd too many idles */
+#define BD_SC_P ((ushort)0x0100) /* xmt preamble */
+#define BD_SC_BR ((ushort)0x0020) /* Break received */
+#define BD_SC_FR ((ushort)0x0010) /* Framing error */
+#define BD_SC_PR ((ushort)0x0008) /* Parity error */
+#define BD_SC_OV ((ushort)0x0002) /* Overrun */
+#define BD_SC_CD ((ushort)0x0001) /* ?? */
/* Buffer descriptor control/status used by Ethernet receive.
-*/
-#define BD_ENET_RX_EMPTY ((ushort)0x8000)
-#define BD_ENET_RX_WRAP ((ushort)0x2000)
-#define BD_ENET_RX_INTR ((ushort)0x1000)
-#define BD_ENET_RX_LAST ((ushort)0x0800)
-#define BD_ENET_RX_FIRST ((ushort)0x0400)
-#define BD_ENET_RX_MISS ((ushort)0x0100)
-#define BD_ENET_RX_LG ((ushort)0x0020)
-#define BD_ENET_RX_NO ((ushort)0x0010)
-#define BD_ENET_RX_SH ((ushort)0x0008)
-#define BD_ENET_RX_CR ((ushort)0x0004)
-#define BD_ENET_RX_OV ((ushort)0x0002)
-#define BD_ENET_RX_CL ((ushort)0x0001)
-#define BD_ENET_RX_STATS ((ushort)0x013f) /* All status bits */
+ */
+#define BD_ENET_RX_EMPTY ((ushort)0x8000)
+#define BD_ENET_RX_WRAP ((ushort)0x2000)
+#define BD_ENET_RX_INTR ((ushort)0x1000)
+#define BD_ENET_RX_LAST ((ushort)0x0800)
+#define BD_ENET_RX_FIRST ((ushort)0x0400)
+#define BD_ENET_RX_MISS ((ushort)0x0100)
+#define BD_ENET_RX_LG ((ushort)0x0020)
+#define BD_ENET_RX_NO ((ushort)0x0010)
+#define BD_ENET_RX_SH ((ushort)0x0008)
+#define BD_ENET_RX_CR ((ushort)0x0004)
+#define BD_ENET_RX_OV ((ushort)0x0002)
+#define BD_ENET_RX_CL ((ushort)0x0001)
+#define BD_ENET_RX_STATS ((ushort)0x013f) /* All status bits */
/* Enhanced buffer descriptor control/status used by Ethernet receive */
-#define BD_ENET_RX_VLAN 0x00000004
+#define BD_ENET_RX_VLAN 0x00000004
/* Buffer descriptor control/status used by Ethernet transmit.
-*/
-#define BD_ENET_TX_READY ((ushort)0x8000)
-#define BD_ENET_TX_PAD ((ushort)0x4000)
-#define BD_ENET_TX_WRAP ((ushort)0x2000)
-#define BD_ENET_TX_INTR ((ushort)0x1000)
-#define BD_ENET_TX_LAST ((ushort)0x0800)
-#define BD_ENET_TX_TC ((ushort)0x0400)
-#define BD_ENET_TX_DEF ((ushort)0x0200)
-#define BD_ENET_TX_HB ((ushort)0x0100)
-#define BD_ENET_TX_LC ((ushort)0x0080)
-#define BD_ENET_TX_RL ((ushort)0x0040)
-#define BD_ENET_TX_RCMASK ((ushort)0x003c)
-#define BD_ENET_TX_UN ((ushort)0x0002)
-#define BD_ENET_TX_CSL ((ushort)0x0001)
-#define BD_ENET_TX_STATS ((ushort)0x0fff) /* All status bits */
-
-/*enhanced buffer descriptor control/status used by Ethernet transmit*/
-#define BD_ENET_TX_INT 0x40000000
-#define BD_ENET_TX_TS 0x20000000
-#define BD_ENET_TX_PINS 0x10000000
-#define BD_ENET_TX_IINS 0x08000000
+ */
+#define BD_ENET_TX_READY ((ushort)0x8000)
+#define BD_ENET_TX_PAD ((ushort)0x4000)
+#define BD_ENET_TX_WRAP ((ushort)0x2000)
+#define BD_ENET_TX_INTR ((ushort)0x1000)
+#define BD_ENET_TX_LAST ((ushort)0x0800)
+#define BD_ENET_TX_TC ((ushort)0x0400)
+#define BD_ENET_TX_DEF ((ushort)0x0200)
+#define BD_ENET_TX_HB ((ushort)0x0100)
+#define BD_ENET_TX_LC ((ushort)0x0080)
+#define BD_ENET_TX_RL ((ushort)0x0040)
+#define BD_ENET_TX_RCMASK ((ushort)0x003c)
+#define BD_ENET_TX_UN ((ushort)0x0002)
+#define BD_ENET_TX_CSL ((ushort)0x0001)
+#define BD_ENET_TX_STATS ((ushort)0x0fff) /* All status bits */
+
+/* enhanced buffer descriptor control/status used by Ethernet transmit */
+#define BD_ENET_TX_INT 0x40000000
+#define BD_ENET_TX_TS 0x20000000
+#define BD_ENET_TX_PINS 0x10000000
+#define BD_ENET_TX_IINS 0x08000000
/* This device has up to three irqs on some platforms */
@@ -301,7 +301,7 @@ struct bufdesc_ex {
#define IDLE_SLOPE_2 0x200 /* BW fraction: 0.5 */
#define IDLE_SLOPE(X) ((X == 1) ? (IDLE_SLOPE_1 & IDLE_SLOPE_MASK) : \
(IDLE_SLOPE_2 & IDLE_SLOPE_MASK))
-#define RCMR_MATCHEN (0x1 << 16)
+#define RCMR_MATCHEN (0x1 << 16)
#define RCMR_CMP_CFG(v, n) ((v & 0x7) << (n << 2))
#define RCMR_CMP_1 (RCMR_CMP_CFG(0, 0) | RCMR_CMP_CFG(1, 1) | \
RCMR_CMP_CFG(2, 2) | RCMR_CMP_CFG(3, 3))
@@ -326,8 +326,8 @@ struct bufdesc_ex {
#define TX_RING_SIZE 512 /* Must be power of two */
#define TX_RING_MOD_MASK 511 /* for this to work */
-#define BD_ENET_RX_INT 0x00800000
-#define BD_ENET_RX_PTP ((ushort)0x0400)
+#define BD_ENET_RX_INT 0x00800000
+#define BD_ENET_RX_PTP ((ushort)0x0400)
#define BD_ENET_RX_ICE 0x00000020
#define BD_ENET_RX_PCR 0x00000010
#define FLAG_RX_CSUM_ENABLED (BD_ENET_RX_ICE | BD_ENET_RX_PCR)
@@ -364,8 +364,8 @@ struct bufdesc_ex {
#define FEC_ITR_ICFT_DEFAULT 200 /* Set 200 frame count threshold */
#define FEC_ITR_ICTT_DEFAULT 1000 /* Set 1000us timer threshold */
-#define FEC_VLAN_TAG_LEN 0x04
-#define FEC_ETHTYPE_LEN 0x02
+#define FEC_VLAN_TAG_LEN 0x04
+#define FEC_ETHTYPE_LEN 0x02
/* Controller is ENET-MAC */
#define FEC_QUIRK_ENET_MAC (1 << 0)
@@ -390,7 +390,7 @@ struct bufdesc_ex {
* frames not being transmitted until there is a 0-to-1 transition on
* ENET_TDAR[TDAR].
*/
-#define FEC_QUIRK_ERR006358 (1 << 7)
+#define FEC_QUIRK_ERR006358 (1 << 7)
/* ENET IP hw AVB
*
* i.MX6SX ENET IP add Audio Video Bridging (AVB) feature support.
--
1.7.10.4
fep->bufdesc_ex is used as boolean flag; thus declare it as such.
Also remove an unnecessary initialization to 0.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec.h | 2 +-
drivers/net/ethernet/freescale/fec_main.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 27e75cf..1418813 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -502,7 +502,7 @@ struct fec_enet_private {
int speed;
struct completion mdio_done;
int irq[FEC_IRQ_NUM];
- int bufdesc_ex;
+ bool bufdesc_ex;
int pause_flag;
struct napi_struct napi;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 50a851d..bb006e1 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3169,8 +3169,6 @@ fec_probe(struct platform_device *pdev)
fep->pdev = pdev;
fep->dev_id = dev_id++;
- fep->bufdesc_ex = 0;
-
platform_set_drvdata(pdev, ndev);
phy_node = of_parse_phandle(np, "phy-handle", 0);
@@ -3228,7 +3226,7 @@ fec_probe(struct platform_device *pdev)
pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
if (IS_ERR(fep->clk_ptp)) {
fep->clk_ptp = NULL;
- fep->bufdesc_ex = 0;
+ fep->bufdesc_ex = false;
}
ret = fec_enet_clk_enable(ndev, true);
--
1.7.10.4
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index ddfc57d..e52864c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -287,7 +287,7 @@ static int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep,
return entries > 0 ? entries : entries + txq->tx_ring_size;
}
-static void *swap_buffer(void *bufaddr, int len)
+static void swap_buffer(void *bufaddr, int len)
{
int i;
unsigned int *buf = bufaddr;
@@ -295,7 +295,6 @@ static void *swap_buffer(void *bufaddr, int len)
for (i = 0; i < len; i += 4, buf++)
swab32s(buf);
- return bufaddr;
}
static void fec_dump(struct net_device *ndev)
--
1.7.10.4
when swap_buffer() is being called, we know for sure, that we need to
byte swap the data. Also this function is called for swapping data in
both directions. Thus cpu_to_be32() is semantically not correct for
all use cases. Use swab32s() to reflect this.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5c1d517..5b76cdc 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -293,7 +293,7 @@ static void *swap_buffer(void *bufaddr, int len)
unsigned int *buf = bufaddr;
for (i = 0; i < DIV_ROUND_UP(len, 4); i++, buf++)
- *buf = cpu_to_be32(*buf);
+ swab32s(buf);
return bufaddr;
}
--
1.7.10.4
Eliminate the DIV_ROUND_UP() and change the loop counter increment to
4 instead. This results in saving 6 instructions in the functions
assembly code.
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5b76cdc..ddfc57d 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -292,7 +292,7 @@ static void *swap_buffer(void *bufaddr, int len)
int i;
unsigned int *buf = bufaddr;
- for (i = 0; i < DIV_ROUND_UP(len, 4); i++, buf++)
+ for (i = 0; i < len; i += 4, buf++)
swab32s(buf);
return bufaddr;
--
1.7.10.4
Changes wrt. v1:
- added some cleanup patches
- simplify handling of 'quirks' flags as suggested by Russell King.
- remove DIV_ROUND_UP() from byte swapping loop as suggested by
Eric Dumazet
Changes wrt. v2:
- rebased against next-20141028
- added some more cleanups in fec.h
- removed unused return value from swap_buffer()
- fixed messed swab32s() call in swap_buffer2()
- fixed messed up setup of fep->quirks
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec.h | 88 +++++++++++++++++-----------------
1 file changed, 44 insertions(+), 44 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 3047db4..e778b9e 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -38,9 +38,9 @@
#define FEC_ADDR_LOW 0x0e4 /* Low 32bits MAC address */
#define FEC_ADDR_HIGH 0x0e8 /* High 16bits MAC address */
#define FEC_OPD 0x0ec /* Opcode + Pause duration */
-#define FEC_TXIC0 0xF0 /* Tx Interrupt Coalescing for ring 0 */
-#define FEC_TXIC1 0xF4 /* Tx Interrupt Coalescing for ring 1 */
-#define FEC_TXIC2 0xF8 /* Tx Interrupt Coalescing for ring 2 */
+#define FEC_TXIC0 0x0f0 /* Tx Interrupt Coalescing for ring 0 */
+#define FEC_TXIC1 0x0f4 /* Tx Interrupt Coalescing for ring 1 */
+#define FEC_TXIC2 0x0f8 /* Tx Interrupt Coalescing for ring 2 */
#define FEC_RXIC0 0x100 /* Rx Interrupt Coalescing for ring 0 */
#define FEC_RXIC1 0x104 /* Rx Interrupt Coalescing for ring 1 */
#define FEC_RXIC2 0x108 /* Rx Interrupt Coalescing for ring 2 */
@@ -62,7 +62,7 @@
#define FEC_R_FIFO_RSEM 0x194 /* Receive FIFO section empty threshold */
#define FEC_R_FIFO_RAEM 0x198 /* Receive FIFO almost empty threshold */
#define FEC_R_FIFO_RAFL 0x19c /* Receive FIFO almost full threshold */
-#define FEC_RACC 0x1C4 /* Receive Accelerator function */
+#define FEC_RACC 0x1c4 /* Receive Accelerator function */
#define FEC_RCMR_1 0x1c8 /* Receive classification match ring 1 */
#define FEC_RCMR_2 0x1cc /* Receive classification match ring 2 */
#define FEC_DMA_CFG_1 0x1d8 /* DMA class configuration for ring 1 */
@@ -82,57 +82,57 @@
#define RMON_T_DROP 0x200 /* Count of frames not cntd correctly */
#define RMON_T_PACKETS 0x204 /* RMON TX packet count */
#define RMON_T_BC_PKT 0x208 /* RMON TX broadcast pkts */
-#define RMON_T_MC_PKT 0x20C /* RMON TX multicast pkts */
+#define RMON_T_MC_PKT 0x20c /* RMON TX multicast pkts */
#define RMON_T_CRC_ALIGN 0x210 /* RMON TX pkts with CRC align err */
#define RMON_T_UNDERSIZE 0x214 /* RMON TX pkts < 64 bytes, good CRC */
#define RMON_T_OVERSIZE 0x218 /* RMON TX pkts > MAX_FL bytes good CRC */
-#define RMON_T_FRAG 0x21C /* RMON TX pkts < 64 bytes, bad CRC */
+#define RMON_T_FRAG 0x21c /* RMON TX pkts < 64 bytes, bad CRC */
#define RMON_T_JAB 0x220 /* RMON TX pkts > MAX_FL bytes, bad CRC */
#define RMON_T_COL 0x224 /* RMON TX collision count */
#define RMON_T_P64 0x228 /* RMON TX 64 byte pkts */
-#define RMON_T_P65TO127 0x22C /* RMON TX 65 to 127 byte pkts */
+#define RMON_T_P65TO127 0x22c /* RMON TX 65 to 127 byte pkts */
#define RMON_T_P128TO255 0x230 /* RMON TX 128 to 255 byte pkts */
#define RMON_T_P256TO511 0x234 /* RMON TX 256 to 511 byte pkts */
#define RMON_T_P512TO1023 0x238 /* RMON TX 512 to 1023 byte pkts */
-#define RMON_T_P1024TO2047 0x23C /* RMON TX 1024 to 2047 byte pkts */
+#define RMON_T_P1024TO2047 0x23c /* RMON TX 1024 to 2047 byte pkts */
#define RMON_T_P_GTE2048 0x240 /* RMON TX pkts > 2048 bytes */
#define RMON_T_OCTETS 0x244 /* RMON TX octets */
#define IEEE_T_DROP 0x248 /* Count of frames not counted crtly */
-#define IEEE_T_FRAME_OK 0x24C /* Frames tx'd OK */
+#define IEEE_T_FRAME_OK 0x24c /* Frames tx'd OK */
#define IEEE_T_1COL 0x250 /* Frames tx'd with single collision */
#define IEEE_T_MCOL 0x254 /* Frames tx'd with multiple collision */
#define IEEE_T_DEF 0x258 /* Frames tx'd after deferral delay */
-#define IEEE_T_LCOL 0x25C /* Frames tx'd with late collision */
+#define IEEE_T_LCOL 0x25c /* Frames tx'd with late collision */
#define IEEE_T_EXCOL 0x260 /* Frames tx'd with excesv collisions */
#define IEEE_T_MACERR 0x264 /* Frames tx'd with TX FIFO underrun */
#define IEEE_T_CSERR 0x268 /* Frames tx'd with carrier sense err */
-#define IEEE_T_SQE 0x26C /* Frames tx'd with SQE err */
+#define IEEE_T_SQE 0x26c /* Frames tx'd with SQE err */
#define IEEE_T_FDXFC 0x270 /* Flow control pause frames tx'd */
#define IEEE_T_OCTETS_OK 0x274 /* Octet count for frames tx'd w/o err */
#define RMON_R_PACKETS 0x284 /* RMON RX packet count */
#define RMON_R_BC_PKT 0x288 /* RMON RX broadcast pkts */
-#define RMON_R_MC_PKT 0x28C /* RMON RX multicast pkts */
+#define RMON_R_MC_PKT 0x28c /* RMON RX multicast pkts */
#define RMON_R_CRC_ALIGN 0x290 /* RMON RX pkts with CRC alignment err */
#define RMON_R_UNDERSIZE 0x294 /* RMON RX pkts < 64 bytes, good CRC */
#define RMON_R_OVERSIZE 0x298 /* RMON RX pkts > MAX_FL bytes good CRC */
-#define RMON_R_FRAG 0x29C /* RMON RX pkts < 64 bytes, bad CRC */
-#define RMON_R_JAB 0x2A0 /* RMON RX pkts > MAX_FL bytes, bad CRC */
-#define RMON_R_RESVD_O 0x2A4 /* Reserved */
-#define RMON_R_P64 0x2A8 /* RMON RX 64 byte pkts */
-#define RMON_R_P65TO127 0x2AC /* RMON RX 65 to 127 byte pkts */
-#define RMON_R_P128TO255 0x2B0 /* RMON RX 128 to 255 byte pkts */
-#define RMON_R_P256TO511 0x2B4 /* RMON RX 256 to 511 byte pkts */
-#define RMON_R_P512TO1023 0x2B8 /* RMON RX 512 to 1023 byte pkts */
-#define RMON_R_P1024TO2047 0x2BC /* RMON RX 1024 to 2047 byte pkts */
-#define RMON_R_P_GTE2048 0x2C0 /* RMON RX pkts > 2048 bytes */
-#define RMON_R_OCTETS 0x2C4 /* RMON RX octets */
-#define IEEE_R_DROP 0x2C8 /* Count frames not counted correctly */
-#define IEEE_R_FRAME_OK 0x2CC /* Frames rx'd OK */
-#define IEEE_R_CRC 0x2D0 /* Frames rx'd with CRC err */
-#define IEEE_R_ALIGN 0x2D4 /* Frames rx'd with alignment err */
-#define IEEE_R_MACERR 0x2D8 /* Receive FIFO overflow count */
-#define IEEE_R_FDXFC 0x2DC /* Flow control pause frames rx'd */
-#define IEEE_R_OCTETS_OK 0x2E0 /* Octet cnt for frames rx'd w/o err */
+#define RMON_R_FRAG 0x29c /* RMON RX pkts < 64 bytes, bad CRC */
+#define RMON_R_JAB 0x2a0 /* RMON RX pkts > MAX_FL bytes, bad CRC */
+#define RMON_R_RESVD_O 0x2a4 /* Reserved */
+#define RMON_R_P64 0x2a8 /* RMON RX 64 byte pkts */
+#define RMON_R_P65TO127 0x2ac /* RMON RX 65 to 127 byte pkts */
+#define RMON_R_P128TO255 0x2b0 /* RMON RX 128 to 255 byte pkts */
+#define RMON_R_P256TO511 0x2b4 /* RMON RX 256 to 511 byte pkts */
+#define RMON_R_P512TO1023 0x2b8 /* RMON RX 512 to 1023 byte pkts */
+#define RMON_R_P1024TO2047 0x2bc /* RMON RX 1024 to 2047 byte pkts */
+#define RMON_R_P_GTE2048 0x2c0 /* RMON RX pkts > 2048 bytes */
+#define RMON_R_OCTETS 0x2c4 /* RMON RX octets */
+#define IEEE_R_DROP 0x2c8 /* Count frames not counted correctly */
+#define IEEE_R_FRAME_OK 0x2cc /* Frames rx'd OK */
+#define IEEE_R_CRC 0x2d0 /* Frames rx'd with CRC err */
+#define IEEE_R_ALIGN 0x2d4 /* Frames rx'd with alignment err */
+#define IEEE_R_MACERR 0x2d8 /* Receive FIFO overflow count */
+#define IEEE_R_FDXFC 0x2dc /* Flow control pause frames rx'd */
+#define IEEE_R_OCTETS_OK 0x2e0 /* Octet cnt for frames rx'd w/o err */
#else
@@ -170,16 +170,16 @@
/* Not existed in real chip
* Just for pass build.
*/
-#define FEC_RCMR_1 0xFFF
-#define FEC_RCMR_2 0xFFF
-#define FEC_DMA_CFG_1 0xFFF
-#define FEC_DMA_CFG_2 0xFFF
-#define FEC_TXIC0 0xFFF
-#define FEC_TXIC1 0xFFF
-#define FEC_TXIC2 0xFFF
-#define FEC_RXIC0 0xFFF
-#define FEC_RXIC1 0xFFF
-#define FEC_RXIC2 0xFFF
+#define FEC_RCMR_1 0xfff
+#define FEC_RCMR_2 0xfff
+#define FEC_DMA_CFG_1 0xfff
+#define FEC_DMA_CFG_2 0xfff
+#define FEC_TXIC0 0xfff
+#define FEC_TXIC1 0xfff
+#define FEC_TXIC2 0xfff
+#define FEC_RXIC0 0xfff
+#define FEC_RXIC1 0xfff
+#define FEC_RXIC2 0xfff
#endif /* CONFIG_M5272 */
@@ -296,7 +296,7 @@ struct bufdesc_ex {
#define DMA_CLASS_EN (1 << 16)
#define FEC_RCMR(X) ((X == 2) ? FEC_RCMR_2 : FEC_RCMR_1)
-#define IDLE_SLOPE_MASK 0xFFFF
+#define IDLE_SLOPE_MASK 0xffff
#define IDLE_SLOPE_1 0x200 /* BW fraction: 0.5 */
#define IDLE_SLOPE_2 0x200 /* BW fraction: 0.5 */
#define IDLE_SLOPE(X) ((X == 1) ? (IDLE_SLOPE_1 & IDLE_SLOPE_MASK) : \
@@ -308,7 +308,7 @@ struct bufdesc_ex {
#define RCMR_CMP_2 (RCMR_CMP_CFG(4, 0) | RCMR_CMP_CFG(5, 1) | \
RCMR_CMP_CFG(6, 2) | RCMR_CMP_CFG(7, 3))
#define RCMR_CMP(X) ((X == 1) ? RCMR_CMP_1 : RCMR_CMP_2)
-#define FEC_TX_BD_FTYPE(X) ((X & 0xF) << 20)
+#define FEC_TX_BD_FTYPE(X) ((X & 0xf) << 20)
/* The number of Tx and Rx buffers. These are allocated from the page
* pool. The code may assume these are power of two, so it it best
@@ -359,8 +359,8 @@ struct bufdesc_ex {
/* ENET interrupt coalescing macro define */
#define FEC_ITR_CLK_SEL (0x1 << 30)
#define FEC_ITR_EN (0x1 << 31)
-#define FEC_ITR_ICFT(X) ((X & 0xFF) << 20)
-#define FEC_ITR_ICTT(X) ((X) & 0xFFFF)
+#define FEC_ITR_ICFT(X) ((X & 0xff) << 20)
+#define FEC_ITR_ICTT(X) ((X) & 0xffff)
#define FEC_ITR_ICFT_DEFAULT 200 /* Set 200 frame count threshold */
#define FEC_ITR_ICTT_DEFAULT 1000 /* Set 1000us timer threshold */
--
1.7.10.4
Signed-off-by: Lothar Waßmann <[email protected]>
---
drivers/net/ethernet/freescale/fec.h | 1 +
drivers/net/ethernet/freescale/fec_main.c | 106 +++++++++++------------------
2 files changed, 39 insertions(+), 68 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 1418813..7aa9388 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -504,6 +504,7 @@ struct fec_enet_private {
int irq[FEC_IRQ_NUM];
bool bufdesc_ex;
int pause_flag;
+ u32 quirks;
struct napi_struct napi;
int csum_flags;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index bb006e1..5c1d517 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -351,8 +351,6 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct bufdesc *bdp = txq->cur_tx;
struct bufdesc_ex *ebdp;
int nr_frags = skb_shinfo(skb)->nr_frags;
@@ -388,7 +386,7 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
}
if (fep->bufdesc_ex) {
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -400,11 +398,11 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
index = fec_enet_get_bd_index(txq->tx_bd_base, bdp, fep);
if (((unsigned long) bufaddr) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], bufaddr, frag_len);
bufaddr = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(bufaddr, frag_len);
}
@@ -440,8 +438,6 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq,
struct sk_buff *skb, struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
int nr_frags = skb_shinfo(skb)->nr_frags;
struct bufdesc *bdp, *last_bdp;
void *bufaddr;
@@ -480,11 +476,11 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq,
queue = skb_get_queue_mapping(skb);
index = fec_enet_get_bd_index(txq->tx_bd_base, bdp, fep);
if (((unsigned long) bufaddr) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], skb->data, buflen);
bufaddr = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(bufaddr, buflen);
}
@@ -519,7 +515,7 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq,
fep->hwts_tx_en))
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -563,8 +559,6 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
int size, bool last_tcp, bool is_last)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc);
unsigned short queue = skb_get_queue_mapping(skb);
unsigned short status;
@@ -577,11 +571,11 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
if (((unsigned long) data) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], data, size);
data = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(data, size);
}
@@ -597,7 +591,7 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
bdp->cbd_bufaddr = addr;
if (fep->bufdesc_ex) {
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -625,8 +619,6 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
struct bufdesc *bdp, int index)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc);
unsigned short queue = skb_get_queue_mapping(skb);
@@ -642,11 +634,11 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
bufaddr = txq->tso_hdrs + index * TSO_HEADER_SIZE;
dmabuf = txq->tso_hdrs_dma + index * TSO_HEADER_SIZE;
if (((unsigned long)bufaddr) & fep->tx_align ||
- id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+ fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], skb->data, hdr_len);
bufaddr = txq->tx_bounce[index];
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(bufaddr, hdr_len);
dmabuf = dma_map_single(&fep->pdev->dev, bufaddr,
@@ -663,7 +655,7 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
bdp->cbd_datlen = hdr_len;
if (fep->bufdesc_ex) {
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -688,8 +680,6 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
struct tso_t tso;
unsigned int index = 0;
int ret;
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep, txq)) {
dev_kfree_skb_any(skb);
@@ -751,7 +741,7 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
txq->cur_tx = bdp;
/* Trigger transmission start */
- if (!(id_entry->driver_data & FEC_QUIRK_ERR007885) ||
+ if (!(fep->quirks & FEC_QUIRK_ERR007885) ||
!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
@@ -914,8 +904,6 @@ static void
fec_restart(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
u32 val;
u32 temp_mac[2];
u32 rcntl = OPT_FRAME_SIZE | 0x04;
@@ -925,7 +913,7 @@ fec_restart(struct net_device *ndev)
* For i.MX6SX SOC, enet use AXI bus, we use disable MAC
* instead of reset MAC itself.
*/
- if (id_entry && id_entry->driver_data & FEC_QUIRK_HAS_AVB) {
+ if (fep->quirks & FEC_QUIRK_HAS_AVB) {
writel(0, fep->hwp + FEC_ECNTRL);
} else {
writel(1, fep->hwp + FEC_ECNTRL);
@@ -936,7 +924,7 @@ fec_restart(struct net_device *ndev)
* enet-mac reset will reset mac address registers too,
* so need to reconfigure it.
*/
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
writel(cpu_to_be32(temp_mac[0]), fep->hwp + FEC_ADDR_LOW);
writel(cpu_to_be32(temp_mac[1]), fep->hwp + FEC_ADDR_HIGH);
@@ -982,7 +970,7 @@ fec_restart(struct net_device *ndev)
* The phy interface and speed need to get configured
* differently on enet-mac.
*/
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
/* Enable flow control and length check */
rcntl |= 0x40000000 | 0x00000020;
@@ -1005,7 +993,7 @@ fec_restart(struct net_device *ndev)
}
} else {
#ifdef FEC_MIIGSK_ENR
- if (id_entry->driver_data & FEC_QUIRK_USE_GASKET) {
+ if (fep->quirks & FEC_QUIRK_USE_GASKET) {
u32 cfgr;
/* disable the gasket and wait */
writel(0, fep->hwp + FEC_MIIGSK_ENR);
@@ -1058,7 +1046,7 @@ fec_restart(struct net_device *ndev)
writel(0, fep->hwp + FEC_HASH_TABLE_LOW);
#endif
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
/* enable ENET endian swap */
ecntl |= (1 << 8);
/* enable ENET store and forward mode */
@@ -1092,8 +1080,6 @@ static void
fec_stop(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8);
/* We cannot expect a graceful transmit stop without link !!! */
@@ -1108,7 +1094,7 @@ fec_stop(struct net_device *ndev)
* For i.MX6SX SOC, enet use AXI bus, we use disable MAC
* instead of reset MAC itself.
*/
- if (id_entry && id_entry->driver_data & FEC_QUIRK_HAS_AVB) {
+ if (fep->quirks & FEC_QUIRK_HAS_AVB) {
writel(0, fep->hwp + FEC_ECNTRL);
} else {
writel(1, fep->hwp + FEC_ECNTRL);
@@ -1118,7 +1104,7 @@ fec_stop(struct net_device *ndev)
writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
/* We have to keep ENET enabled to have MII interrupt stay working */
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
+ if (fep->quirks & FEC_QUIRK_ENET_MAC) {
writel(2, fep->hwp + FEC_ECNTRL);
writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
}
@@ -1337,8 +1323,6 @@ static int
fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct fec_enet_priv_rx_q *rxq;
struct bufdesc *bdp;
unsigned short status;
@@ -1430,7 +1414,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
prefetch(skb->data - NET_IP_ALIGN);
skb_put(skb, pkt_len - 4);
data = skb->data;
- if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
swap_buffer(data, pkt_len);
/* Extract the enhanced buffer descriptor */
@@ -1865,8 +1849,6 @@ failed_clk_ipg:
static int fec_enet_mii_probe(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct phy_device *phy_dev = NULL;
char mdio_bus_id[MII_BUS_ID_SIZE];
char phy_name[MII_BUS_ID_SIZE + 3];
@@ -1912,7 +1894,7 @@ static int fec_enet_mii_probe(struct net_device *ndev)
}
/* mask with MAC supported features */
- if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) {
+ if (fep->quirks & FEC_QUIRK_HAS_GBIT) {
phy_dev->supported &= PHY_GBIT_FEATURES;
phy_dev->supported &= ~SUPPORTED_1000baseT_Half;
#if !defined(CONFIG_M5272)
@@ -1940,8 +1922,6 @@ static int fec_enet_mii_init(struct platform_device *pdev)
static struct mii_bus *fec0_mii_bus;
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct device_node *node;
int err = -ENXIO, i;
@@ -1961,7 +1941,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
* mdio interface in board design, and need to be configured by
* fec0 mii_bus.
*/
- if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
+ if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
/* fec1 uses fec0 mii_bus */
if (mii_cnt && fec0_mii_bus) {
fep->mii_bus = fec0_mii_bus;
@@ -1982,7 +1962,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
* document.
*/
fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk_ipg), 5000000);
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
+ if (fep->quirks & FEC_QUIRK_ENET_MAC)
fep->phy_speed--;
fep->phy_speed <<= 1;
writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
@@ -2024,7 +2004,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
mii_cnt++;
/* save fec0 mii_bus */
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
+ if (fep->quirks & FEC_QUIRK_ENET_MAC)
fec0_mii_bus = fep->mii_bus;
return 0;
@@ -2293,11 +2273,9 @@ static int fec_enet_us_to_itr_clock(struct net_device *ndev, int us)
static void fec_enet_itr_coal_set(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
int rx_itr, tx_itr;
- if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
return;
/* Must be greater than zero to avoid unpredictable behavior */
@@ -2332,10 +2310,8 @@ static int
fec_enet_get_coalesce(struct net_device *ndev, struct ethtool_coalesce *ec)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
- if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
return -EOPNOTSUPP;
ec->rx_coalesce_usecs = fep->rx_time_itr;
@@ -2351,12 +2327,9 @@ static int
fec_enet_set_coalesce(struct net_device *ndev, struct ethtool_coalesce *ec)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
-
unsigned int cycle;
- if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
return -EOPNOTSUPP;
if (ec->rx_max_coalesced_frames > 255) {
@@ -2936,8 +2909,6 @@ static const struct net_device_ops fec_netdev_ops = {
static int fec_enet_init(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- const struct platform_device_id *id_entry =
- platform_get_device_id(fep->pdev);
struct fec_enet_priv_tx_q *txq;
struct fec_enet_priv_rx_q *rxq;
struct bufdesc *cbd_base;
@@ -3016,11 +2987,11 @@ static int fec_enet_init(struct net_device *ndev)
writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK);
netif_napi_add(ndev, &fep->napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT);
- if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN)
+ if (fep->quirks & FEC_QUIRK_HAS_VLAN)
/* enable hw VLAN support */
ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
- if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) {
+ if (fep->quirks & FEC_QUIRK_HAS_CSUM) {
ndev->gso_max_segs = FEC_MAX_TSO_SEGS;
/* enable hw accelerator */
@@ -3029,7 +3000,7 @@ static int fec_enet_init(struct net_device *ndev)
fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
}
- if (id_entry->driver_data & FEC_QUIRK_HAS_AVB) {
+ if (fep->quirks & FEC_QUIRK_HAS_AVB) {
fep->tx_align = 0;
fep->rx_align = 0x3f;
}
@@ -3129,10 +3100,6 @@ fec_probe(struct platform_device *pdev)
int num_tx_qs;
int num_rx_qs;
- of_id = of_match_device(fec_dt_ids, &pdev->dev);
- if (of_id)
- pdev->id_entry = of_id->data;
-
fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
/* Init network device */
@@ -3146,13 +3113,17 @@ fec_probe(struct platform_device *pdev)
/* setup board info structure */
fep = netdev_priv(ndev);
+ of_id = of_match_device(fec_dt_ids, &pdev->dev);
+ if (of_id)
+ pdev->id_entry = of_id->data;
+ fep->quirks = pdev->id_entry->driver_data;
+
fep->num_rx_queues = num_rx_qs;
fep->num_tx_queues = num_tx_qs;
#if !defined(CONFIG_M5272)
/* default enable pause frame auto negotiation */
- if (pdev->id_entry &&
- (pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT))
+ if (fep->quirks & FEC_QUIRK_HAS_GBIT)
fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG;
#endif
@@ -3221,9 +3192,8 @@ fec_probe(struct platform_device *pdev)
if (IS_ERR(fep->clk_ref))
fep->clk_ref = NULL;
+ fep->bufdesc_ex = fep->quirks & FEC_QUIRK_HAS_BUFDESC_EX;
fep->clk_ptp = devm_clk_get(&pdev->dev, "ptp");
- fep->bufdesc_ex =
- pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
if (IS_ERR(fep->clk_ptp)) {
fep->clk_ptp = NULL;
fep->bufdesc_ex = false;
--
1.7.10.4
Hi,
David Laight wrote:
> From: Lothar Waßmann
> > David Laight wrote:
> > > From: Lothar Waßmann
> > > > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > > > introduced a regression for i.MX28. The swap_buffer() function doing
> > > > the endian conversion of the received data on i.MX28 may access memory
> > > > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > > > does not copy those bytes, so that the last bytes of a packet may be
> > > > filled with invalid data after swapping.
> > > > This will likely lead to checksum errors on received packets.
> > > > E.g. when trying to mount an NFS rootfs:
> > > > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> > > >
> > > > Do the byte swapping and copying to the new skb in one go if
> > > > necessary.
> > > >
> > > > Signed-off-by: Lothar Wamann <[email protected]>
> > > > ---
> > > > drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> > > > 1 file changed, 21 insertions(+), 4 deletions(-)
> > > >
> > > > @@ -1455,7 +1472,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> > > > prefetch(skb->data - NET_IP_ALIGN);
> > > > skb_put(skb, pkt_len - 4);
> > > > data = skb->data;
> > > > - if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> > > > + if (!is_copybreak && need_swap)
> > > > swap_buffer(data, pkt_len);
> > >
> > > It has to be better to set the 'copybreak' limit to be larger than the
> > > maximum frame size and so always go through the 'copybreak' paths.
> > >
> > Since the copybreak support is all about performance optimistation, we
> > should IMO buy the additional advantage for i.MX28 by not having to
> > access the buffer twice (once for copying and once again for swapping).
>
> You definitely want to do the byteswap at the same time as the copy.
>
> The point I'm trying to make that if you need to do the byteswap you
> probably might as well copy the data to an skb of the correct size at
> the same time.
> Certainly I'd expect the 'break even' length will be much higher.
>
I didn't implement the copybreak support. I'm only trying to fix a bug
it introduced for i.MX28. So, I won't mess with the copybreak
parameters.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
From: Lothar Wa?mann <[email protected]>
Date: Tue, 28 Oct 2014 14:22:55 +0100
> Changes wrt. v1:
> - added some cleanup patches
> - simplify handling of 'quirks' flags as suggested by Russell King.
> - remove DIV_ROUND_UP() from byte swapping loop as suggested by
> Eric Dumazet
>
> Changes wrt. v2:
> - rebased against next-20141028
> - added some more cleanups in fec.h
> - removed unused return value from swap_buffer()
> - fixed messed swab32s() call in swap_buffer2()
> - fixed messed up setup of fep->quirks
>
It is not appropriate to mix cleanups and bonafide bug fixes.
I want to see only bug fixes targetted at 'net'. You can later
submit the cleanups to 'net-next'.
Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
in swap_buffer() is valid. The whole point is that the current
code handles buffers which have a length which is not a multiple
of 4 properly, after your change it will no longer do so.
Hi,
David Miller wrote:
> From: Lothar Waßmann <[email protected]>
> Date: Tue, 28 Oct 2014 14:22:55 +0100
>
> > Changes wrt. v1:
> > - added some cleanup patches
> > - simplify handling of 'quirks' flags as suggested by Russell King.
> > - remove DIV_ROUND_UP() from byte swapping loop as suggested by
> > Eric Dumazet
> >
> > Changes wrt. v2:
> > - rebased against next-20141028
> > - added some more cleanups in fec.h
> > - removed unused return value from swap_buffer()
> > - fixed messed swab32s() call in swap_buffer2()
> > - fixed messed up setup of fep->quirks
> >
>
> It is not appropriate to mix cleanups and bonafide bug fixes.
>
> I want to see only bug fixes targetted at 'net'. You can later
> submit the cleanups to 'net-next'.
>
OK.
> Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
> in swap_buffer() is valid. The whole point is that the current
> code handles buffers which have a length which is not a multiple
> of 4 properly, after your change it will no longer do so.
>
Do you really think so?
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
From: Lothar Wa?mann <[email protected]>
Date: Thu, 30 Oct 2014 07:51:04 +0100
>> Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
>> in swap_buffer() is valid. The whole point is that the current
>> code handles buffers which have a length which is not a multiple
>> of 4 properly, after your change it will no longer do so.
>>
> Do you really think so?
Yes, because you're rounding down so you'll miss the final
partial word (if any).
Hi,
David Miller wrote:
> From: Lothar Waßmann <[email protected]>
> Date: Thu, 30 Oct 2014 07:51:04 +0100
>
> >> Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
> >> in swap_buffer() is valid. The whole point is that the current
> >> code handles buffers which have a length which is not a multiple
> >> of 4 properly, after your change it will no longer do so.
> >>
> > Do you really think so?
>
> Yes, because you're rounding down so you'll miss the final
> partial word (if any).
>
Nope. DIV_ROUND_UP() would give '1' as upper bound for lengths from 1 to
4, '2' for lengths from 5 to 8 and so on.
The loop with increment 4 and i < len does exactly the same.
Try it for yourself, if you don't believe it.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
Hi Lothar,
> Lothar Waßmann <[email protected]> hat am 28. Oktober 2014 um 14:23
> geschrieben:
>
>
> commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx
> performance")
> introduced a regression for i.MX28. The swap_buffer() function doing
> the endian conversion of the received data on i.MX28 may access memory
> beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> does not copy those bytes, so that the last bytes of a packet may be
> filled with invalid data after swapping.
> This will likely lead to checksum errors on received packets.
> E.g. when trying to mount an NFS rootfs:
> UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
i think i experience the same problem with 3.18-rc2 on my mx28 board. I get
strange warnings about unexpected bytes from ping on my mx28 board and ping to
my mx28 board fails because of no response.
After applying the complete patch series these problems disappear. I'm looking
forward to see V4.
Thanks Stefan
Hi David,
Lothar Waßmann wrote:
> David Miller wrote:
> > From: Lothar Waßmann <[email protected]>
> > Date: Thu, 30 Oct 2014 07:51:04 +0100
> >
> > >> Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
> > >> in swap_buffer() is valid. The whole point is that the current
> > >> code handles buffers which have a length which is not a multiple
> > >> of 4 properly, after your change it will no longer do so.
> > >>
> > > Do you really think so?
> >
> > Yes, because you're rounding down so you'll miss the final
> > partial word (if any).
> >
> Nope. DIV_ROUND_UP() would give '1' as upper bound for lengths from 1 to
> 4, '2' for lengths from 5 to 8 and so on.
>
> The loop with increment 4 and i < len does exactly the same.
> Try it for yourself, if you don't believe it.
>
>
Do you still think, the loop without DIV_ROUND_UP() is incorrect,
or can this patch be applied?
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
From: Lothar Wa?mann <[email protected]>
Date: Tue, 4 Nov 2014 11:29:12 +0100
> Hi David,
>
> Lothar Wa?mann wrote:
>> David Miller wrote:
>> > From: Lothar Wa?mann <[email protected]>
>> > Date: Thu, 30 Oct 2014 07:51:04 +0100
>> >
>> > >> Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
>> > >> in swap_buffer() is valid. The whole point is that the current
>> > >> code handles buffers which have a length which is not a multiple
>> > >> of 4 properly, after your change it will no longer do so.
>> > >>
>> > > Do you really think so?
>> >
>> > Yes, because you're rounding down so you'll miss the final
>> > partial word (if any).
>> >
>> Nope. DIV_ROUND_UP() would give '1' as upper bound for lengths from 1 to
>> 4, '2' for lengths from 5 to 8 and so on.
>>
>> The loop with increment 4 and i < len does exactly the same.
>> Try it for yourself, if you don't believe it.
>>
>>
> Do you still think, the loop without DIV_ROUND_UP() is incorrect,
> or can this patch be applied?
I haven't had the time to fully re-look into the details, I'm busy
with many other things at the moment.
But looking at DIV_ROUND_UP() macro it rounds up. It gives an
upper bound of 4 for any value 1 to 4. Unlike what you claim.
Because it goes "(n + (d - 1)) / d"
Which for 'd' of 4 gives:
1 --> 4
2 --> 4
3 --> 4
4 --> 4
Hi,
David Miller wrote:
> From: Lothar Waßmann <[email protected]>
> Date: Tue, 4 Nov 2014 11:29:12 +0100
>
> > Hi David,
> >
> > Lothar Waßmann wrote:
> >> David Miller wrote:
> >> > From: Lothar Waßmann <[email protected]>
> >> > Date: Thu, 30 Oct 2014 07:51:04 +0100
> >> >
> >> > >> Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
> >> > >> in swap_buffer() is valid. The whole point is that the current
> >> > >> code handles buffers which have a length which is not a multiple
> >> > >> of 4 properly, after your change it will no longer do so.
> >> > >>
> >> > > Do you really think so?
> >> >
> >> > Yes, because you're rounding down so you'll miss the final
> >> > partial word (if any).
> >> >
> >> Nope. DIV_ROUND_UP() would give '1' as upper bound for lengths from 1 to
> >> 4, '2' for lengths from 5 to 8 and so on.
> >>
> >> The loop with increment 4 and i < len does exactly the same.
> >> Try it for yourself, if you don't believe it.
> >>
> >>
> > Do you still think, the loop without DIV_ROUND_UP() is incorrect,
> > or can this patch be applied?
>
> I haven't had the time to fully re-look into the details, I'm busy
> with many other things at the moment.
>
> But looking at DIV_ROUND_UP() macro it rounds up. It gives an
> upper bound of 4 for any value 1 to 4. Unlike what you claim.
>
You're missing the 'DIV' part of DIV_ROUND_UP().
> Because it goes "(n + (d - 1)) / d"
>
> Which for 'd' of 4 gives:
>
> 1 --> 4
> 2 --> 4
> 3 --> 4
> 4 --> 4
>
'1', not '4'.
The loop has to be done once for each (probably partial) WORD of input
data, not for each BYTE.
Without dividing and incrementing by four the loop
counter will be 0 for the first word which is less than any length > 0.
Thus the loop will be run once for any number of bytes from 1 thru 4.
If incremented by 4 after the first loop, it will be run again for any
length > 5 and so forth.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
From: Lothar Wa?mann <[email protected]>
Date: Wed, 5 Nov 2014 06:21:48 +0100
> David Miller wrote:
>> Because it goes "(n + (d - 1)) / d"
>>
>> Which for 'd' of 4 gives:
>>
>> 1 --> 4
>> 2 --> 4
>> 3 --> 4
>> 4 --> 4
>>
> '1', not '4'.
> The loop has to be done once for each (probably partial) WORD of input
> data, not for each BYTE.
You're right, thanks for being so patient with me.
Please repost your patch, I'll integrate it.
Thanks.