[PATCH 0/3] Series short description

[PATCH 0/3] Series short description

[Grant Likely]

Hi David,

Here are the fec_mpc52xx patches which should be picked up for 2.6.29.
Patch #1 fixes a dma related BUG_ON() discovered after the merge window
opened.  #2 & #3 were original a single patch posted prior to the merge
window, but not having a s-o-b line from the original author held them up.
In the end I rewrote it from scratch as two patches just so I can get
them merged.  #2 makes the driver use net_device_ops, and #3 fixes a NULL
dereference.

I've tested these patches on several MPC5200 platforms.

 drivers/net/fec_mpc52xx.c |   77 +++++++++++++++++++++++++-------------------
 1 files changed, 44 insertions(+), 33 deletions(-)

Cheers,
g.

--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.

[PATCH 1/3] net/fec_mpc52xx: fix BUG on missing dma_ops

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

The driver triggers a BUG_ON() when allocating DMA buffers because the
arch/powerpc dma_ops aren't in the net_device's struct device.  This
patch fixes the problem by using the parent of_device which does have
the correct dma_ops set.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Reviewed-by: Becky Bruce <beckyb@kernel.crashing.org>
---

 drivers/net/fec_mpc52xx.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)


diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index 049b0a7..f99463f 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -129,7 +129,8 @@ static void mpc52xx_fec_free_rx_buffers(struct net_device *dev, struct bcom_task
 		struct sk_buff *skb;
 
 		skb = bcom_retrieve_buffer(s, NULL, (struct bcom_bd **)&bd);
-		dma_unmap_single(&dev->dev, bd->skb_pa, skb->len, DMA_FROM_DEVICE);
+		dma_unmap_single(dev->dev.parent, bd->skb_pa, skb->len,
+				 DMA_FROM_DEVICE);
 		kfree_skb(skb);
 	}
 }
@@ -150,7 +151,7 @@ static int mpc52xx_fec_alloc_rx_buffers(struct net_device *dev, struct bcom_task
 		bd = (struct bcom_fec_bd *)bcom_prepare_next_buffer(rxtsk);
 
 		bd->status = FEC_RX_BUFFER_SIZE;
-		bd->skb_pa = dma_map_single(&dev->dev, skb->data,
+		bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
 				FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
 
 		bcom_submit_next_buffer(rxtsk, skb);
@@ -388,7 +389,8 @@ static int mpc52xx_fec_hard_start_xmit(struct sk_buff *skb, struct net_device *d
 		bcom_prepare_next_buffer(priv->tx_dmatsk);
 
 	bd->status = skb->len | BCOM_FEC_TX_BD_TFD | BCOM_FEC_TX_BD_TC;
-	bd->skb_pa = dma_map_single(&dev->dev, skb->data, skb->len, DMA_TO_DEVICE);
+	bd->skb_pa = dma_map_single(dev->dev.parent, skb->data, skb->len,
+				    DMA_TO_DEVICE);
 
 	bcom_submit_next_buffer(priv->tx_dmatsk, skb);
 
@@ -430,7 +432,8 @@ static irqreturn_t mpc52xx_fec_tx_interrupt(int irq, void *dev_id)
 		struct bcom_fec_bd *bd;
 		skb = bcom_retrieve_buffer(priv->tx_dmatsk, NULL,
 				(struct bcom_bd **)&bd);
-		dma_unmap_single(&dev->dev, bd->skb_pa, skb->len, DMA_TO_DEVICE);
+		dma_unmap_single(dev->dev.parent, bd->skb_pa, skb->len,
+				 DMA_TO_DEVICE);
 
 		dev_kfree_skb_irq(skb);
 	}
@@ -455,7 +458,8 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
 
 		rskb = bcom_retrieve_buffer(priv->rx_dmatsk, &status,
 				(struct bcom_bd **)&bd);
-		dma_unmap_single(&dev->dev, bd->skb_pa, rskb->len, DMA_FROM_DEVICE);
+		dma_unmap_single(dev->dev.parent, bd->skb_pa, rskb->len,
+				 DMA_FROM_DEVICE);
 
 		/* Test for errors in received frame */
 		if (status & BCOM_FEC_RX_BD_ERRORS) {
@@ -464,7 +468,8 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
 				bcom_prepare_next_buffer(priv->rx_dmatsk);
 
 			bd->status = FEC_RX_BUFFER_SIZE;
-			bd->skb_pa = dma_map_single(&dev->dev, rskb->data,
+			bd->skb_pa = dma_map_single(dev->dev.parent,
+					rskb->data,
 					FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
 
 			bcom_submit_next_buffer(priv->rx_dmatsk, rskb);
@@ -499,7 +504,7 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
 			bcom_prepare_next_buffer(priv->rx_dmatsk);
 
 		bd->status = FEC_RX_BUFFER_SIZE;
-		bd->skb_pa = dma_map_single(&dev->dev, skb->data,
+		bd->skb_pa = dma_map_single(dev->dev.parent, skb->data,
 				FEC_RX_BUFFER_SIZE, DMA_FROM_DEVICE);
 
 		bcom_submit_next_buffer(priv->rx_dmatsk, skb);

[PATCH 2/3] net/fec_mpc52xx: Migrate to net_device_ops.

[Grant Likely]

From: Henk Stegeman <henk.stegeman@gmail.com>

Since not using net_device_ops gets you shunned out the cool crowd,
this patch modifies the fec_mpc52xx Ethernet driver to provide the
management hooks via a struct net_device_ops.

Reported-by: Henk Stegeman <henk.stegeman@gmail.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/net/fec_mpc52xx.c |   36 ++++++++++++++++++++----------------
 1 files changed, 20 insertions(+), 16 deletions(-)


diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index f99463f..e4355d4 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -371,7 +371,7 @@ static int mpc52xx_fec_close(struct net_device *dev)
  * invariant will hold if you make sure that the netif_*_queue()
  * calls are done at the proper times.
  */
-static int mpc52xx_fec_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static int mpc52xx_fec_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct mpc52xx_fec_priv *priv = netdev_priv(dev);
 	struct bcom_fec_bd *bd;
@@ -379,7 +379,7 @@ static int mpc52xx_fec_hard_start_xmit(struct sk_buff *skb, struct net_device *d
 	if (bcom_queue_full(priv->tx_dmatsk)) {
 		if (net_ratelimit())
 			dev_err(&dev->dev, "transmit queue overrun\n");
-		return 1;
+		return NETDEV_TX_BUSY;
 	}
 
 	spin_lock_irq(&priv->lock);
@@ -400,7 +400,7 @@ static int mpc52xx_fec_hard_start_xmit(struct sk_buff *skb, struct net_device *d
 
 	spin_unlock_irq(&priv->lock);
 
-	return 0;
+	return NETDEV_TX_OK;
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -890,6 +890,22 @@ static int mpc52xx_fec_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	return mpc52xx_fec_phy_mii_ioctl(priv, if_mii(rq), cmd);
 }
 
+static const struct net_device_ops mpc52xx_fec_netdev_ops = {
+	.ndo_open = mpc52xx_fec_open,
+	.ndo_stop = mpc52xx_fec_close,
+	.ndo_start_xmit = mpc52xx_fec_start_xmit,
+	.ndo_set_multicast_list = mpc52xx_fec_set_multicast_list,
+	.ndo_set_mac_address = mpc52xx_fec_set_mac_address,
+	.ndo_validate_addr = eth_validate_addr,
+	.ndo_do_ioctl = mpc52xx_fec_ioctl,
+	.ndo_change_mtu = eth_change_mtu,
+	.ndo_tx_timeout = mpc52xx_fec_tx_timeout,
+	.ndo_get_stats = mpc52xx_fec_get_stats,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_poll_controller = mpc52xx_fec_poll_controller,
+#endif
+};
+
 /* ======================================================================== */
 /* OF Driver                                                                */
 /* ======================================================================== */
@@ -934,22 +950,10 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
 		return -EBUSY;
 
 	/* Init ether ndev with what we have */
-	ndev->open		= mpc52xx_fec_open;
-	ndev->stop		= mpc52xx_fec_close;
-	ndev->hard_start_xmit	= mpc52xx_fec_hard_start_xmit;
-	ndev->do_ioctl		= mpc52xx_fec_ioctl;
+	ndev->netdev_ops	= &mpc52xx_fec_netdev_ops;
 	ndev->ethtool_ops	= &mpc52xx_fec_ethtool_ops;
-	ndev->get_stats		= mpc52xx_fec_get_stats;
-	ndev->set_mac_address	= mpc52xx_fec_set_mac_address;
-	ndev->set_multicast_list = mpc52xx_fec_set_multicast_list;
-	ndev->tx_timeout	= mpc52xx_fec_tx_timeout;
 	ndev->watchdog_timeo	= FEC_WATCHDOG_TIMEOUT;
 	ndev->base_addr		= mem.start;
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	ndev->poll_controller = mpc52xx_fec_poll_controller;
-#endif
-
-	priv->t_irq = priv->r_irq = ndev->irq = NO_IRQ; /* IRQ are free for now */
 
 	spin_lock_init(&priv->lock);
 

[PATCH 3/3] net/fec_mpc52xx: Don't dereference phy_device if it is NULL

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

The FEC Ethernet device isn't always attached to a phy.  Be careful
not to dereference phy_device if it is NULL.  Also eliminates an
unnecessary extra function from the ioctl path.

Reported-by: Henk Stegeman <henk.stegeman@gmail.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/net/fec_mpc52xx.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)


diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index e4355d4..8bbe7f6 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -271,15 +271,6 @@ static void mpc52xx_fec_phy_stop(struct net_device *dev)
 	phy_write(priv->phydev, MII_BMCR, BMCR_PDOWN);
 }
 
-static int mpc52xx_fec_phy_mii_ioctl(struct mpc52xx_fec_priv *priv,
-		struct mii_ioctl_data *mii_data, int cmd)
-{
-	if (!priv->phydev)
-		return -ENOTSUPP;
-
-	return phy_mii_ioctl(priv->phydev, mii_data, cmd);
-}
-
 static void mpc52xx_fec_phy_hw_init(struct mpc52xx_fec_priv *priv)
 {
 	struct mpc52xx_fec __iomem *fec = priv->fec;
@@ -852,12 +843,20 @@ static void mpc52xx_fec_get_drvinfo(struct net_device *dev,
 static int mpc52xx_fec_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
 	struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+
+	if (!priv->phydev)
+		return -ENODEV;
+
 	return phy_ethtool_gset(priv->phydev, cmd);
 }
 
 static int mpc52xx_fec_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
 	struct mpc52xx_fec_priv *priv = netdev_priv(dev);
+
+	if (!priv->phydev)
+		return -ENODEV;
+
 	return phy_ethtool_sset(priv->phydev, cmd);
 }
 
@@ -887,7 +886,10 @@ static int mpc52xx_fec_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct mpc52xx_fec_priv *priv = netdev_priv(dev);
 
-	return mpc52xx_fec_phy_mii_ioctl(priv, if_mii(rq), cmd);
+	if (!priv->phydev)
+		return -ENOTSUPP;
+
+	return phy_mii_ioctl(priv->phydev, if_mii(rq), cmd);
 }
 
 static const struct net_device_ops mpc52xx_fec_netdev_ops = {

Re: [PATCH 0/3] Series short description

[David Miller]

From: Grant Likely <grant.likely@secretlab.ca>
Date: Wed, 01 Apr 2009 00:16:46 -0600

> Here are the fec_mpc52xx patches which should be picked up for 2.6.29.
> Patch #1 fixes a dma related BUG_ON() discovered after the merge window
> opened.  #2 & #3 were original a single patch posted prior to the merge
> window, but not having a s-o-b line from the original author held them up.
> In the end I rewrote it from scratch as two patches just so I can get
> them merged.  #2 makes the driver use net_device_ops, and #3 fixes a NULL
> dereference.
> 
> I've tested these patches on several MPC5200 platforms.

All applied, thanks Grant.

Re: [PATCH 0/3] Series short description

[Grant Likely]

On Thu, Apr 2, 2009 at 1:57 AM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Wed, 01 Apr 2009 00:16:46 -0600
>
>> Here are the fec_mpc52xx patches which should be picked up for 2.6.29.
>> Patch #1 fixes a dma related BUG_ON() discovered after the merge window
>> opened.  #2 & #3 were original a single patch posted prior to the merge
>> window, but not having a s-o-b line from the original author held them up.
>> In the end I rewrote it from scratch as two patches just so I can get
>> them merged.  #2 makes the driver use net_device_ops, and #3 fixes a NULL
>> dereference.
>>
>> I've tested these patches on several MPC5200 platforms.
>
> All applied, thanks Grant.

Thanks David!

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.