[PATCH 0/7] net: ethernet: marvell: Assorted fixes

[PATCH 0/7] net: ethernet: marvell: Assorted fixes

[Ezequiel Garcia]

This series consists of cleanups and minor improvements on mvneta, mv643xx_eth
and mvmdio drivers. None of the patches imply any functionality change, except
for the patch six "Change the number of default rx queues to one".

This patch reduces the driver's allocated resources and makes the multiqueue
path in the poll function not get taken. In other words, in the poll():

static int mvneta_poll(struct napi_struct *napi, int budget)
{
	/* .. */
        if (rxq_number > 1) {
		/* .. */
        } else {
                rx_done = mvneta_rx(pp, budget, &pp->rxqs[rxq_def]);
                budget -= rx_done;
        }
}

the first block of the 'if' won't be taken. This should be completely harmless,
as the driver only supports the first queue, by requiring 'rxq_def' to be '0'.
This is explained by a comment in the code:

        /* Our multiqueue support is not complete, so for now, only
         * allow the usage of the first RX queue
         */
        if (rxq_def != 0) {
                dev_err(&pdev->dev, "Invalid rxq_def argument: %d\n", rxq_def);
                return -EINVAL;
        }

Willy, Thomas: given you've done some work on these drivers, could you review
the series and comment?

Ezequiel Garcia (7):
  net: mv643xx_eth: Simplify mv643xx_eth_adjust_link()
  net: mvneta: Clean-up mvneta_tx_frag_process()
  net: mvneta: Check tx queue setup error in mvneta_change_mtu()
  net: mvneta: Clean-up mvneta_init()
  net: mvneta: Use prepare/commit API to simplify MAC address setting
  net: mvneta: Change the number of default rx queues to one
  net: mvmdio: Use devm_* API to simplify the code

 drivers/net/ethernet/marvell/mv643xx_eth.c | 15 ++----
 drivers/net/ethernet/marvell/mvmdio.c      | 18 +++----
 drivers/net/ethernet/marvell/mvneta.c      | 86 +++++++++++++-----------------
 3 files changed, 47 insertions(+), 72 deletions(-)

-- 
1.9.1

[PATCH 1/7] net: mv643xx_eth: Simplify mv643xx_eth_adjust_link()

[Ezequiel Garcia]

Currently, mv643xx_eth_adjust_link() is only used to call mv643xx_adjust_pscr().
This commit renames the latter to the former, and therefore removes the extra
and useless function.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/net/ethernet/marvell/mv643xx_eth.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index b7b8d74..1e63c63 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -1010,8 +1010,10 @@ static void txq_set_fixed_prio_mode(struct tx_queue *txq)
 
 
 /* mii management interface *************************************************/
-static void mv643xx_adjust_pscr(struct mv643xx_eth_private *mp)
+static void mv643xx_eth_adjust_link(struct net_device *dev)
 {
+	struct mv643xx_eth_private *mp = netdev_priv(dev);
+
 	u32 pscr = rdlp(mp, PORT_SERIAL_CONTROL);
 	u32 autoneg_disable = FORCE_LINK_PASS |
 	             DISABLE_AUTO_NEG_SPEED_GMII |
@@ -1387,7 +1389,7 @@ mv643xx_eth_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 
 	ret = phy_ethtool_sset(mp->phy, cmd);
 	if (!ret)
-		mv643xx_adjust_pscr(mp);
+		mv643xx_eth_adjust_link(dev);
 	return ret;
 }
 
@@ -2303,7 +2305,7 @@ static int mv643xx_eth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 
 	ret = phy_mii_ioctl(mp->phy, ifr, cmd);
 	if (!ret)
-		mv643xx_adjust_pscr(mp);
+		mv643xx_eth_adjust_link(dev);
 	return ret;
 }
 
@@ -2701,13 +2703,6 @@ static void set_params(struct mv643xx_eth_private *mp,
 	mp->txq_count = pd->tx_queue_count ? : 1;
 }
 
-static void mv643xx_eth_adjust_link(struct net_device *dev)
-{
-	struct mv643xx_eth_private *mp = netdev_priv(dev);
-
-	mv643xx_adjust_pscr(mp);
-}
-
 static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
 				   int phy_addr)
 {
-- 
1.9.1

[PATCH 2/7] net: mvneta: Clean-up mvneta_tx_frag_process()

[Ezequiel Garcia]

A tiny clean-up to improve readability. This commit makes no functionality
change.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 14786c8..d73ed91 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1524,9 +1524,9 @@ static int mvneta_tx_frag_process(struct mvneta_port *pp, struct sk_buff *skb,
 				  struct mvneta_tx_queue *txq)
 {
 	struct mvneta_tx_desc *tx_desc;
-	int i;
+	int i, nr_frags = skb_shinfo(skb)->nr_frags;
 
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+	for (i = 0; i < nr_frags; i++) {
 		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 		void *addr = page_address(frag->page.p) + frag->page_offset;
 
@@ -1543,20 +1543,16 @@ static int mvneta_tx_frag_process(struct mvneta_port *pp, struct sk_buff *skb,
 			goto error;
 		}
 
-		if (i == (skb_shinfo(skb)->nr_frags - 1)) {
+		if (i == nr_frags - 1) {
 			/* Last descriptor */
 			tx_desc->command = MVNETA_TXD_L_DESC | MVNETA_TXD_Z_PAD;
-
 			txq->tx_skb[txq->txq_put_index] = skb;
-
-			mvneta_txq_inc_put(txq);
 		} else {
 			/* Descriptor in the middle: Not First, Not Last */
 			tx_desc->command = 0;
-
 			txq->tx_skb[txq->txq_put_index] = NULL;
-			mvneta_txq_inc_put(txq);
 		}
+		mvneta_txq_inc_put(txq);
 	}
 
 	return 0;
-- 
1.9.1

[PATCH 3/7] net: mvneta: Check tx queue setup error in mvneta_change_mtu()

[Ezequiel Garcia]

This commit checks the return code of mvneta_setup_txq() call
in mvneta_change_mtu(). Also, use the netdevice pointer directly
instead of dereferencing the port structure. While here, let's
fix a tiny comment typo.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index d73ed91..743503a 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2275,24 +2275,28 @@ static int mvneta_change_mtu(struct net_device *dev, int mtu)
 		return 0;
 
 	/* The interface is running, so we have to force a
-	 * reallocation of the RXQs
+	 * reallocation of the queues
 	 */
 	mvneta_stop_dev(pp);
 
 	mvneta_cleanup_txqs(pp);
 	mvneta_cleanup_rxqs(pp);
 
-	pp->pkt_size = MVNETA_RX_PKT_SIZE(pp->dev->mtu);
+	pp->pkt_size = MVNETA_RX_PKT_SIZE(dev->mtu);
 	pp->frag_size = SKB_DATA_ALIGN(MVNETA_RX_BUF_SIZE(pp->pkt_size)) +
 	                SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 
 	ret = mvneta_setup_rxqs(pp);
 	if (ret) {
-		netdev_err(pp->dev, "unable to setup rxqs after MTU change\n");
+		netdev_err(dev, "unable to setup rxqs after MTU change\n");
 		return ret;
 	}
 
-	mvneta_setup_txqs(pp);
+	ret = mvneta_setup_txqs(pp);
+	if (ret) {
+		netdev_err(dev, "unable to setup txqs after MTU change\n");
+		return ret;
+	}
 
 	mvneta_start_dev(pp);
 	mvneta_port_up(pp);
-- 
1.9.1

[PATCH 4/7] net: mvneta: Clean-up mvneta_init()

[Ezequiel Garcia]

This commit cleans-up mvneta_init(), which initializes the hardware
and allocates the rx/qx queues. The queue allocation is simplified
by using devm_kzalloc instead of kzalloc. The unused phy_addr parameter
is removed. While here, the 'hal' references in the comments are removed.
This commit makes no functionality change.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 38 ++++++++++++-----------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 743503a..9a444a0 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1999,7 +1999,7 @@ static void mvneta_tx_reset(struct mvneta_port *pp)
 {
 	int queue;
 
-	/* free the skb's in the hal tx ring */
+	/* free the skb's in the tx ring */
 	for (queue = 0; queue < txq_number; queue++)
 		mvneta_txq_done_force(pp, &pp->txqs[queue]);
 
@@ -2638,7 +2638,7 @@ const struct ethtool_ops mvneta_eth_tool_ops = {
 };
 
 /* Initialize hw */
-static int mvneta_init(struct mvneta_port *pp, int phy_addr)
+static int mvneta_init(struct device *dev, struct mvneta_port *pp)
 {
 	int queue;
 
@@ -2648,8 +2648,9 @@ static int mvneta_init(struct mvneta_port *pp, int phy_addr)
 	/* Set port default values */
 	mvneta_defaults_set(pp);
 
-	pp->txqs = kzalloc(txq_number * sizeof(struct mvneta_tx_queue),
-			   GFP_KERNEL);
+	pp->txqs = devm_kzalloc(dev,
+				txq_number * sizeof(struct mvneta_tx_queue),
+				GFP_KERNEL);
 	if (!pp->txqs)
 		return -ENOMEM;
 
@@ -2661,12 +2662,11 @@ static int mvneta_init(struct mvneta_port *pp, int phy_addr)
 		txq->done_pkts_coal = MVNETA_TXDONE_COAL_PKTS;
 	}
 
-	pp->rxqs = kzalloc(rxq_number * sizeof(struct mvneta_rx_queue),
-			   GFP_KERNEL);
-	if (!pp->rxqs) {
-		kfree(pp->txqs);
+	pp->rxqs = devm_kzalloc(dev,
+				rxq_number * sizeof(struct mvneta_rx_queue),
+				GFP_KERNEL);
+	if (!pp->rxqs)
 		return -ENOMEM;
-	}
 
 	/* Create Rx descriptor rings */
 	for (queue = 0; queue < rxq_number; queue++) {
@@ -2680,12 +2680,6 @@ static int mvneta_init(struct mvneta_port *pp, int phy_addr)
 	return 0;
 }
 
-static void mvneta_deinit(struct mvneta_port *pp)
-{
-	kfree(pp->txqs);
-	kfree(pp->rxqs);
-}
-
 /* platform glue : initialize decoding windows */
 static void mvneta_conf_mbus_windows(struct mvneta_port *pp,
 				     const struct mbus_dram_target_info *dram)
@@ -2768,7 +2762,6 @@ static int mvneta_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct device_node *dn = pdev->dev.of_node;
 	struct device_node *phy_node;
-	u32 phy_addr;
 	struct mvneta_port *pp;
 	struct net_device *dev;
 	const char *dt_mac_addr;
@@ -2864,16 +2857,14 @@ static int mvneta_probe(struct platform_device *pdev)
 	pp->dev = dev;
 	SET_NETDEV_DEV(dev, &pdev->dev);
 
-	err = mvneta_init(pp, phy_addr);
-	if (err < 0) {
-		dev_err(&pdev->dev, "can't init eth hal\n");
+	err = mvneta_init(&pdev->dev, pp);
+	if (err < 0)
 		goto err_free_stats;
-	}
 
 	err = mvneta_port_power_up(pp, phy_mode);
 	if (err < 0) {
 		dev_err(&pdev->dev, "can't power up port\n");
-		goto err_deinit;
+		goto err_free_stats;
 	}
 
 	dram_target_info = mv_mbus_dram_info();
@@ -2890,7 +2881,7 @@ static int mvneta_probe(struct platform_device *pdev)
 	err = register_netdev(dev);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to register\n");
-		goto err_deinit;
+		goto err_free_stats;
 	}
 
 	netdev_info(dev, "Using %s mac address %pM\n", mac_from,
@@ -2900,8 +2891,6 @@ static int mvneta_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_deinit:
-	mvneta_deinit(pp);
 err_free_stats:
 	free_percpu(pp->stats);
 err_clk:
@@ -2920,7 +2909,6 @@ static int mvneta_remove(struct platform_device *pdev)
 	struct mvneta_port *pp = netdev_priv(dev);
 
 	unregister_netdev(dev);
-	mvneta_deinit(pp);
 	clk_disable_unprepare(pp->clk);
 	free_percpu(pp->stats);
 	irq_dispose_mapping(dev->irq);
-- 
1.9.1

[PATCH 5/7] net: mvneta: Use prepare/commit API to simplify MAC address setting

[Ezequiel Garcia]

Use eth_prepare_mac_addr_change and eth_commit_mac_addr_change, instead
of manually checking and storing the MAC address, which makes the
code slightly more robust. This fixes the lack of valid MAC address check
in the driver's .ndo_set_mac_address hook.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 9a444a0..0dcce29 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2323,22 +2323,19 @@ static void mvneta_get_mac_addr(struct mvneta_port *pp, unsigned char *addr)
 static int mvneta_set_mac_addr(struct net_device *dev, void *addr)
 {
 	struct mvneta_port *pp = netdev_priv(dev);
-	u8 *mac = addr + 2;
-	int i;
-
-	if (netif_running(dev))
-		return -EBUSY;
+	struct sockaddr *sockaddr = addr;
+	int ret;
 
+	ret = eth_prepare_mac_addr_change(dev, addr);
+	if (ret < 0)
+		return ret;
 	/* Remove previous address table entry */
 	mvneta_mac_addr_set(pp, dev->dev_addr, -1);
 
 	/* Set new addr in hw */
-	mvneta_mac_addr_set(pp, mac, rxq_def);
-
-	/* Set addr in the device */
-	for (i = 0; i < ETH_ALEN; i++)
-		dev->dev_addr[i] = mac[i];
+	mvneta_mac_addr_set(pp, sockaddr->sa_data, rxq_def);
 
+	eth_commit_mac_addr_change(dev, addr);
 	return 0;
 }
 
@@ -2433,8 +2430,6 @@ static int mvneta_open(struct net_device *dev)
 	struct mvneta_port *pp = netdev_priv(dev);
 	int ret;
 
-	mvneta_mac_addr_set(pp, dev->dev_addr, rxq_def);
-
 	pp->pkt_size = MVNETA_RX_PKT_SIZE(pp->dev->mtu);
 	pp->frag_size = SKB_DATA_ALIGN(MVNETA_RX_BUF_SIZE(pp->pkt_size)) +
 	                SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
-- 
1.9.1

[PATCH 6/7] net: mvneta: Change the number of default rx queues to one

[Ezequiel Garcia]

The driver does not support multiple rx queues, and so it's a waste
of resources to have a default number larger than one (1).

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 0dcce29..ec4f67a 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -441,7 +441,10 @@ struct mvneta_rx_queue {
 	int next_desc_to_proc;
 };
 
-static int rxq_number = 8;
+/* The hardware supports eight (8) rx queues, but we are only allowing
+ * the first one to be used. Therefore, let's just allocate one queue.
+ */
+static int rxq_number = 1;
 static int txq_number = 8;
 
 static int rxq_def;
-- 
1.9.1

[PATCH 7/7] net: mvmdio: Use devm_* API to simplify the code

[Ezequiel Garcia]

This commit makes use of devm_kmalloc() for memory allocation and the
recently introduced devm_mdiobus_alloc() API to simplify driver's code.
While here, remove a redundant out of memory error message.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index b161a52..995e182 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -195,11 +195,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	bus = mdiobus_alloc_size(sizeof(struct orion_mdio_dev));
-	if (!bus) {
-		dev_err(&pdev->dev, "Cannot allocate MDIO bus\n");
+	bus = devm_mdiobus_alloc_size(&pdev->dev,
+				      sizeof(struct orion_mdio_dev));
+	if (!bus)
 		return -ENOMEM;
-	}
 
 	bus->name = "orion_mdio_bus";
 	bus->read = orion_mdio_read;
@@ -208,11 +207,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		 dev_name(&pdev->dev));
 	bus->parent = &pdev->dev;
 
-	bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
-	if (!bus->irq) {
-		mdiobus_free(bus);
+	bus->irq = devm_kmalloc(&pdev->dev,
+				sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+	if (!bus->irq)
 		return -ENOMEM;
-	}
 
 	for (i = 0; i < PHY_MAX_ADDR; i++)
 		bus->irq[i] = PHY_POLL;
@@ -261,8 +259,6 @@ static int orion_mdio_probe(struct platform_device *pdev)
 out_mdio:
 	if (!IS_ERR(dev->clk))
 		clk_disable_unprepare(dev->clk);
-	kfree(bus->irq);
-	mdiobus_free(bus);
 	return ret;
 }
 
@@ -273,8 +269,6 @@ static int orion_mdio_remove(struct platform_device *pdev)
 
 	writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 	mdiobus_unregister(bus);
-	kfree(bus->irq);
-	mdiobus_free(bus);
 	if (!IS_ERR(dev->clk))
 		clk_disable_unprepare(dev->clk);
 
-- 
1.9.1

Re: [PATCH 1/7] net: mv643xx_eth: Simplify mv643xx_eth_adjust_link()

[Sebastian Hesselbarth]

On 05/03/2014 08:26 PM, Ezequiel Garcia wrote:
> Currently, mv643xx_eth_adjust_link() is only used to call mv643xx_adjust_pscr().
> This commit renames the latter to the former, and therefore removes the extra
> and useless function.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Thanks!

> ---
>  drivers/net/ethernet/marvell/mv643xx_eth.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index b7b8d74..1e63c63 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -1010,8 +1010,10 @@ static void txq_set_fixed_prio_mode(struct tx_queue *txq)
>  
>  
>  /* mii management interface *************************************************/
> -static void mv643xx_adjust_pscr(struct mv643xx_eth_private *mp)
> +static void mv643xx_eth_adjust_link(struct net_device *dev)
>  {
> +	struct mv643xx_eth_private *mp = netdev_priv(dev);
> +
>  	u32 pscr = rdlp(mp, PORT_SERIAL_CONTROL);
>  	u32 autoneg_disable = FORCE_LINK_PASS |
>  	             DISABLE_AUTO_NEG_SPEED_GMII |
> @@ -1387,7 +1389,7 @@ mv643xx_eth_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>  
>  	ret = phy_ethtool_sset(mp->phy, cmd);
>  	if (!ret)
> -		mv643xx_adjust_pscr(mp);
> +		mv643xx_eth_adjust_link(dev);
>  	return ret;
>  }
>  
> @@ -2303,7 +2305,7 @@ static int mv643xx_eth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>  
>  	ret = phy_mii_ioctl(mp->phy, ifr, cmd);
>  	if (!ret)
> -		mv643xx_adjust_pscr(mp);
> +		mv643xx_eth_adjust_link(dev);
>  	return ret;
>  }
>  
> @@ -2701,13 +2703,6 @@ static void set_params(struct mv643xx_eth_private *mp,
>  	mp->txq_count = pd->tx_queue_count ? : 1;
>  }
>  
> -static void mv643xx_eth_adjust_link(struct net_device *dev)
> -{
> -	struct mv643xx_eth_private *mp = netdev_priv(dev);
> -
> -	mv643xx_adjust_pscr(mp);
> -}
> -
>  static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
>  				   int phy_addr)
>  {
> 

Re: [PATCH 7/7] net: mvmdio: Use devm_* API to simplify the code

[Sebastian Hesselbarth]

On 05/03/2014 08:27 PM, Ezequiel Garcia wrote:
> This commit makes use of devm_kmalloc() for memory allocation and the
> recently introduced devm_mdiobus_alloc() API to simplify driver's code.
> While here, remove a redundant out of memory error message.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Thanks!

> ---
>  drivers/net/ethernet/marvell/mvmdio.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
> index b161a52..995e182 100644
> --- a/drivers/net/ethernet/marvell/mvmdio.c
> +++ b/drivers/net/ethernet/marvell/mvmdio.c
> @@ -195,11 +195,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	bus = mdiobus_alloc_size(sizeof(struct orion_mdio_dev));
> -	if (!bus) {
> -		dev_err(&pdev->dev, "Cannot allocate MDIO bus\n");
> +	bus = devm_mdiobus_alloc_size(&pdev->dev,
> +				      sizeof(struct orion_mdio_dev));
> +	if (!bus)
>  		return -ENOMEM;
> -	}
>  
>  	bus->name = "orion_mdio_bus";
>  	bus->read = orion_mdio_read;
> @@ -208,11 +207,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  		 dev_name(&pdev->dev));
>  	bus->parent = &pdev->dev;
>  
> -	bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
> -	if (!bus->irq) {
> -		mdiobus_free(bus);
> +	bus->irq = devm_kmalloc(&pdev->dev,
> +				sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
> +	if (!bus->irq)
>  		return -ENOMEM;
> -	}
>  
>  	for (i = 0; i < PHY_MAX_ADDR; i++)
>  		bus->irq[i] = PHY_POLL;
> @@ -261,8 +259,6 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  out_mdio:
>  	if (!IS_ERR(dev->clk))
>  		clk_disable_unprepare(dev->clk);
> -	kfree(bus->irq);
> -	mdiobus_free(bus);
>  	return ret;
>  }
>  
> @@ -273,8 +269,6 @@ static int orion_mdio_remove(struct platform_device *pdev)
>  
>  	writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
>  	mdiobus_unregister(bus);
> -	kfree(bus->irq);
> -	mdiobus_free(bus);
>  	if (!IS_ERR(dev->clk))
>  		clk_disable_unprepare(dev->clk);
>  
> 

Re: [PATCH 1/7] net: mv643xx_eth: Simplify mv643xx_eth_adjust_link()

[David Miller]

From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Date: Sat,  3 May 2014 15:26:56 -0300

> Currently, mv643xx_eth_adjust_link() is only used to call mv643xx_adjust_pscr().
> This commit renames the latter to the former, and therefore removes the extra
> and useless function.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/mv643xx_eth.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index b7b8d74..1e63c63 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -1010,8 +1010,10 @@ static void txq_set_fixed_prio_mode(struct tx_queue *txq)
>  
>  
>  /* mii management interface *************************************************/
> -static void mv643xx_adjust_pscr(struct mv643xx_eth_private *mp)
> +static void mv643xx_eth_adjust_link(struct net_device *dev)
>  {
> +	struct mv643xx_eth_private *mp = netdev_priv(dev);
> +
>  	u32 pscr = rdlp(mp, PORT_SERIAL_CONTROL);
>  	u32 autoneg_disable = FORCE_LINK_PASS |
>  	             DISABLE_AUTO_NEG_SPEED_GMII |

Please do not add empty lines in the middle of the function local variable declarations.

Re: [PATCH 1/7] net: mv643xx_eth: Simplify mv643xx_eth_adjust_link()

[Ezequiel Garcia]

On 05 May 03:40 PM, David Miller wrote:
> From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> Date: Sat,  3 May 2014 15:26:56 -0300
> 
> > Currently, mv643xx_eth_adjust_link() is only used to call mv643xx_adjust_pscr().
> > This commit renames the latter to the former, and therefore removes the extra
> > and useless function.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> >  drivers/net/ethernet/marvell/mv643xx_eth.c | 15 +++++----------
> >  1 file changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> > index b7b8d74..1e63c63 100644
> > --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> > +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> > @@ -1010,8 +1010,10 @@ static void txq_set_fixed_prio_mode(struct tx_queue *txq)
> >  
> >  
> >  /* mii management interface *************************************************/
> > -static void mv643xx_adjust_pscr(struct mv643xx_eth_private *mp)
> > +static void mv643xx_eth_adjust_link(struct net_device *dev)
> >  {
> > +	struct mv643xx_eth_private *mp = netdev_priv(dev);
> > +
> >  	u32 pscr = rdlp(mp, PORT_SERIAL_CONTROL);
> >  	u32 autoneg_disable = FORCE_LINK_PASS |
> >  	             DISABLE_AUTO_NEG_SPEED_GMII |
> 
> Please do not add empty lines in the middle of the function local variable declarations.

Argh, I missed this.

Thanks for the catch,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com