[PATCH net-next 0/2] net: systemport: couple fixes

[PATCH net-next 0/2] net: systemport: couple fixes

[Florian Fainelli]

Hi David,

This patch series contains two small fixes for the Broadcom SYSTEMPORT driver.

Thank you!

Florian Fainelli (2):
  net: systemport: only update UMAC_CMD if something changed
  net: systemport: pad packets to a minimum of 64 bytes

 drivers/net/ethernet/broadcom/bcmsysport.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

-- 
1.9.1

[PATCH net-next 1/2] net: systemport: only update UMAC_CMD if something changed

[Florian Fainelli]

The link adjustment callback can be called as frequently as desired by
the PHY library, as such, let's avoid doing a Read/Modify/Write sequence
if nothing changed, which is more than likely since we are interfaced
with a switch device.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 4dc8d1e..e118e74 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -959,15 +959,16 @@ static void bcm_sysport_adj_link(struct net_device *dev)
 	if (!phydev->pause)
 		cmd_bits |= CMD_RX_PAUSE_IGNORE | CMD_TX_PAUSE_IGNORE;
 
-	reg = umac_readl(priv, UMAC_CMD);
-	reg &= ~((CMD_SPEED_MASK << CMD_SPEED_SHIFT) |
-			CMD_HD_EN | CMD_RX_PAUSE_IGNORE |
-			CMD_TX_PAUSE_IGNORE);
-	reg |= cmd_bits;
-	umac_writel(priv, reg, UMAC_CMD);
+	if (changed) {
+		reg = umac_readl(priv, UMAC_CMD);
+		reg &= ~((CMD_SPEED_MASK << CMD_SPEED_SHIFT) |
+				CMD_HD_EN | CMD_RX_PAUSE_IGNORE |
+				CMD_TX_PAUSE_IGNORE);
+		reg |= cmd_bits;
+		umac_writel(priv, reg, UMAC_CMD);
 
-	if (changed)
 		phy_print_status(priv->phydev);
+	}
 }
 
 static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
-- 
1.9.1

[PATCH net-next 2/2] net: systemport: pad packets to a minimum of 64 bytes

[Florian Fainelli]

The switch fabric which is used behind the Broadcom SYSTEMPORT Ethernet
controller will discard any incoming packet that is not 64 bytes or
more. Since the UniMAC hardware automatically pads up to the specified
length, we can simply ensure we instruct it to transmit >= 64 bytes
packets.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index e118e74..a49c381 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -821,6 +821,7 @@ static netdev_tx_t bcm_sysport_xmit(struct sk_buff *skb,
 	struct bcm_sysport_cb *cb;
 	struct netdev_queue *txq;
 	struct dma_desc *desc;
+	unsigned int skb_len;
 	dma_addr_t mapping;
 	u32 len_status;
 	u16 queue;
@@ -848,7 +849,14 @@ static netdev_tx_t bcm_sysport_xmit(struct sk_buff *skb,
 		}
 	}
 
-	mapping = dma_map_single(kdev, skb->data, skb->len, DMA_TO_DEVICE);
+	/* The Ethernet switch we are interfaced with needs packets to be at
+	 * least 64 bytes otherwise they will be discarded when they enter
+	 * the switch port logic. The UniMAC hardware automatically pads if
+	 * instructed to do so.
+	 */
+	skb_len = skb->len < 64 ? 64 : skb->len;
+
+	mapping = dma_map_single(kdev, skb->data, skb_len, DMA_TO_DEVICE);
 	if (dma_mapping_error(kdev, mapping)) {
 		netif_err(priv, tx_err, dev, "DMA map failed at %p (len=%d)\n",
 				skb->data, skb->len);
@@ -860,14 +868,14 @@ static netdev_tx_t bcm_sysport_xmit(struct sk_buff *skb,
 	cb = &ring->cbs[ring->curr_desc];
 	cb->skb = skb;
 	dma_unmap_addr_set(cb, dma_addr, mapping);
-	dma_unmap_len_set(cb, dma_len, skb->len);
+	dma_unmap_len_set(cb, dma_len, skb_len);
 
 	/* Fetch a descriptor entry from our pool */
 	desc = ring->desc_cpu;
 
 	desc->addr_lo = lower_32_bits(mapping);
 	len_status = upper_32_bits(mapping) & DESC_ADDR_HI_MASK;
-	len_status |= (skb->len << DESC_LEN_SHIFT);
+	len_status |= (skb_len << DESC_LEN_SHIFT);
 	len_status |= (DESC_SOP | DESC_EOP | TX_STATUS_APP_CRC) <<
 			DESC_STATUS_SHIFT;
 	if (skb->ip_summed == CHECKSUM_PARTIAL)
-- 
1.9.1

Re: [PATCH net-next 2/2] net: systemport: pad packets to a minimum of 64 bytes

[Ben Hutchings]

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

On Sat, 2014-05-03 at 11:31 -0700, Florian Fainelli wrote:
> The switch fabric which is used behind the Broadcom SYSTEMPORT Ethernet
> controller will discard any incoming packet that is not 64 bytes or
> more. Since the UniMAC hardware automatically pads up to the specified
> length, we can simply ensure we instruct it to transmit >= 64 bytes
> packets.
[...]

No, you have to pad with zeroes (skb_padto()) otherwise you're leaking
potentially sensitive data.

Ben.

-- 
Ben Hutchings
All the simple programs have been written, and all the good names taken.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]