[PATCH net-next 0/2] altera: simplify probe

[PATCH net-next 0/2] altera: simplify probe

[Rosen Penev]

A small devm change with goto removals and a function simplification.

Rosen Penev (2):
  net: altera: use devm for alloc_etherdev
  net: altera: simplify request_and_map

 drivers/net/ethernet/altera/altera_tse_main.c | 128 ++++++------------
 1 file changed, 43 insertions(+), 85 deletions(-)

-- 
2.47.0

[PATCH net-next 1/2] net: altera: use devm for alloc_etherdev

[Rosen Penev]

Remove a bunch of gotos as a result.

As this driver already uses devm, it makes sense to register
alloc_etherdev with devm before anything else.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 61 ++++++++-----------
 1 file changed, 24 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 3f6204de9e6b..52d4cafec683 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1141,9 +1141,11 @@ static int altera_tse_probe(struct platform_device *pdev)
 	struct mii_bus *pcs_bus;
 	struct net_device *ndev;
 	void __iomem *descmap;
+	struct device *dev;
 	int ret = -ENODEV;
 
-	ndev = alloc_etherdev(sizeof(struct altera_tse_private));
+	dev = &pdev->dev;
+	ndev = devm_alloc_etherdev(dev, sizeof(struct altera_tse_private));
 	if (!ndev) {
 		dev_err(&pdev->dev, "Could not allocate network device\n");
 		return -ENODEV;
@@ -1163,7 +1165,7 @@ static int altera_tse_probe(struct platform_device *pdev)
 		/* Get the mapped address to the SGDMA descriptor memory */
 		ret = request_and_map(pdev, "s1", &dma_res, &descmap);
 		if (ret)
-			goto err_free_netdev;
+			return ret;
 
 		/* Start of that memory is for transmit descriptors */
 		priv->tx_dma_desc = descmap;
@@ -1182,26 +1184,24 @@ static int altera_tse_probe(struct platform_device *pdev)
 		if (upper_32_bits(priv->rxdescmem_busaddr)) {
 			dev_dbg(priv->device,
 				"SGDMA bus addresses greater than 32-bits\n");
-			ret = -EINVAL;
-			goto err_free_netdev;
+			return -EINVAL;
 		}
 		if (upper_32_bits(priv->txdescmem_busaddr)) {
 			dev_dbg(priv->device,
 				"SGDMA bus addresses greater than 32-bits\n");
-			ret = -EINVAL;
-			goto err_free_netdev;
+			return -EINVAL;
 		}
 	} else if (priv->dmaops &&
 		   priv->dmaops->altera_dtype == ALTERA_DTYPE_MSGDMA) {
 		ret = request_and_map(pdev, "rx_resp", &dma_res,
 				      &priv->rx_dma_resp);
 		if (ret)
-			goto err_free_netdev;
+			return ret;
 
 		ret = request_and_map(pdev, "tx_desc", &dma_res,
 				      &priv->tx_dma_desc);
 		if (ret)
-			goto err_free_netdev;
+			return ret;
 
 		priv->txdescmem = resource_size(dma_res);
 		priv->txdescmem_busaddr = dma_res->start;
@@ -1209,44 +1209,40 @@ static int altera_tse_probe(struct platform_device *pdev)
 		ret = request_and_map(pdev, "rx_desc", &dma_res,
 				      &priv->rx_dma_desc);
 		if (ret)
-			goto err_free_netdev;
+			return ret;
 
 		priv->rxdescmem = resource_size(dma_res);
 		priv->rxdescmem_busaddr = dma_res->start;
 
-	} else {
-		ret = -ENODEV;
-		goto err_free_netdev;
-	}
+	} else
+		return -ENODEV;
 
 	if (!dma_set_mask(priv->device, DMA_BIT_MASK(priv->dmaops->dmamask))) {
 		dma_set_coherent_mask(priv->device,
 				      DMA_BIT_MASK(priv->dmaops->dmamask));
 	} else if (!dma_set_mask(priv->device, DMA_BIT_MASK(32))) {
 		dma_set_coherent_mask(priv->device, DMA_BIT_MASK(32));
-	} else {
-		ret = -EIO;
-		goto err_free_netdev;
-	}
+	} else
+		return -EIO;
 
 	/* MAC address space */
 	ret = request_and_map(pdev, "control_port", &control_port,
 			      (void __iomem **)&priv->mac_dev);
 	if (ret)
-		goto err_free_netdev;
+		return ret;
 
 	/* xSGDMA Rx Dispatcher address space */
 	ret = request_and_map(pdev, "rx_csr", &dma_res,
 			      &priv->rx_dma_csr);
 	if (ret)
-		goto err_free_netdev;
+		return ret;
 
 
 	/* xSGDMA Tx Dispatcher address space */
 	ret = request_and_map(pdev, "tx_csr", &dma_res,
 			      &priv->tx_dma_csr);
 	if (ret)
-		goto err_free_netdev;
+		return ret;
 
 	memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
 	memset(&mrc, 0, sizeof(mrc));
@@ -1274,10 +1270,9 @@ static int altera_tse_probe(struct platform_device *pdev)
 	/* Create a regmap for the PCS so that it can be used by the PCS driver */
 	pcs_regmap = devm_regmap_init_mmio(&pdev->dev, priv->pcs_base,
 					   &pcs_regmap_cfg);
-	if (IS_ERR(pcs_regmap)) {
-		ret = PTR_ERR(pcs_regmap);
-		goto err_free_netdev;
-	}
+	if (IS_ERR(pcs_regmap))
+		return PTR_ERR(pcs_regmap);
+
 	mrc.regmap = pcs_regmap;
 	mrc.parent = &pdev->dev;
 	mrc.valid_addr = 0x0;
@@ -1287,31 +1282,27 @@ static int altera_tse_probe(struct platform_device *pdev)
 	priv->rx_irq = platform_get_irq_byname(pdev, "rx_irq");
 	if (priv->rx_irq == -ENXIO) {
 		dev_err(&pdev->dev, "cannot obtain Rx IRQ\n");
-		ret = -ENXIO;
-		goto err_free_netdev;
+		return -ENXIO;
 	}
 
 	/* Tx IRQ */
 	priv->tx_irq = platform_get_irq_byname(pdev, "tx_irq");
 	if (priv->tx_irq == -ENXIO) {
 		dev_err(&pdev->dev, "cannot obtain Tx IRQ\n");
-		ret = -ENXIO;
-		goto err_free_netdev;
+		return -ENXIO;
 	}
 
 	/* get FIFO depths from device tree */
 	if (of_property_read_u32(pdev->dev.of_node, "rx-fifo-depth",
 				 &priv->rx_fifo_depth)) {
 		dev_err(&pdev->dev, "cannot obtain rx-fifo-depth\n");
-		ret = -ENXIO;
-		goto err_free_netdev;
+		return -ENXIO;
 	}
 
 	if (of_property_read_u32(pdev->dev.of_node, "tx-fifo-depth",
 				 &priv->tx_fifo_depth)) {
 		dev_err(&pdev->dev, "cannot obtain tx-fifo-depth\n");
-		ret = -ENXIO;
-		goto err_free_netdev;
+		return -ENXIO;
 	}
 
 	/* get hash filter settings for this instance */
@@ -1354,7 +1345,7 @@ static int altera_tse_probe(struct platform_device *pdev)
 	ret = altera_tse_phy_get_addr_mdio_create(ndev);
 
 	if (ret)
-		goto err_free_netdev;
+		return ret;
 
 	/* initialize netdev */
 	ndev->mem_start = control_port->start;
@@ -1450,8 +1441,6 @@ static int altera_tse_probe(struct platform_device *pdev)
 err_register_netdev:
 	netif_napi_del(&priv->napi);
 	altera_tse_mdio_destroy(ndev);
-err_free_netdev:
-	free_netdev(ndev);
 	return ret;
 }
 
@@ -1467,8 +1456,6 @@ static void altera_tse_remove(struct platform_device *pdev)
 	unregister_netdev(ndev);
 	phylink_destroy(priv->phylink);
 	lynx_pcs_destroy(priv->pcs);
-
-	free_netdev(ndev);
 }
 
 static const struct altera_dmaops altera_dtype_sgdma = {
-- 
2.47.0

[PATCH net-next 2/2] net: altera: simplify request_and_map

[Rosen Penev]

This function is effectively devm_platform_ioremap_resource_byname with
an additional parameter returning the resource. Might as well use modern
helpers.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 81 ++++++-------------
 1 file changed, 26 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 52d4cafec683..66add91e64e0 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1098,33 +1098,11 @@ static const struct phylink_mac_ops alt_tse_phylink_ops = {
 	.mac_select_pcs = alt_tse_select_pcs,
 };
 
-static int request_and_map(struct platform_device *pdev, const char *name,
-			   struct resource **res, void __iomem **ptr)
+static void __iomem *request_and_map(struct platform_device *pdev,
+				     const char *name, struct resource **res)
 {
-	struct device *device = &pdev->dev;
-	struct resource *region;
-
 	*res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
-	if (*res == NULL) {
-		dev_err(device, "resource %s not defined\n", name);
-		return -ENODEV;
-	}
-
-	region = devm_request_mem_region(device, (*res)->start,
-					 resource_size(*res), dev_name(device));
-	if (region == NULL) {
-		dev_err(device, "unable to request %s\n", name);
-		return -EBUSY;
-	}
-
-	*ptr = devm_ioremap(device, region->start,
-				    resource_size(region));
-	if (*ptr == NULL) {
-		dev_err(device, "ioremap of %s failed!", name);
-		return -ENOMEM;
-	}
-
-	return 0;
+	return devm_ioremap_resource(&pdev->dev, *res);
 }
 
 /* Probe Altera TSE MAC device
@@ -1163,9 +1141,9 @@ static int altera_tse_probe(struct platform_device *pdev)
 	if (priv->dmaops &&
 	    priv->dmaops->altera_dtype == ALTERA_DTYPE_SGDMA) {
 		/* Get the mapped address to the SGDMA descriptor memory */
-		ret = request_and_map(pdev, "s1", &dma_res, &descmap);
-		if (ret)
-			return ret;
+		descmap = request_and_map(pdev, "s1", &dma_res);
+		if (IS_ERR(descmap))
+			return PTR_ERR(descmap);
 
 		/* Start of that memory is for transmit descriptors */
 		priv->tx_dma_desc = descmap;
@@ -1193,23 +1171,20 @@ static int altera_tse_probe(struct platform_device *pdev)
 		}
 	} else if (priv->dmaops &&
 		   priv->dmaops->altera_dtype == ALTERA_DTYPE_MSGDMA) {
-		ret = request_and_map(pdev, "rx_resp", &dma_res,
-				      &priv->rx_dma_resp);
-		if (ret)
-			return ret;
+		priv->rx_dma_resp = request_and_map(pdev, "rx_resp", &dma_res);
+		if (IS_ERR(priv->rx_dma_resp))
+			return PTR_ERR(priv->rx_dma_resp);
 
-		ret = request_and_map(pdev, "tx_desc", &dma_res,
-				      &priv->tx_dma_desc);
-		if (ret)
-			return ret;
+		priv->tx_dma_desc = request_and_map(pdev, "tx_desc", &dma_res);
+		if (IS_ERR(priv->tx_dma_desc))
+			return PTR_ERR(priv->tx_dma_desc);
 
 		priv->txdescmem = resource_size(dma_res);
 		priv->txdescmem_busaddr = dma_res->start;
 
-		ret = request_and_map(pdev, "rx_desc", &dma_res,
-				      &priv->rx_dma_desc);
-		if (ret)
-			return ret;
+		priv->rx_dma_desc = request_and_map(pdev, "rx_desc", &dma_res);
+		if (IS_ERR(priv->rx_dma_desc))
+			return PTR_ERR(priv->rx_dma_desc);
 
 		priv->rxdescmem = resource_size(dma_res);
 		priv->rxdescmem_busaddr = dma_res->start;
@@ -1226,23 +1201,19 @@ static int altera_tse_probe(struct platform_device *pdev)
 		return -EIO;
 
 	/* MAC address space */
-	ret = request_and_map(pdev, "control_port", &control_port,
-			      (void __iomem **)&priv->mac_dev);
-	if (ret)
-		return ret;
+	priv->mac_dev = request_and_map(pdev, "control_port", &control_port);
+	if (IS_ERR(priv->mac_dev))
+		return PTR_ERR(priv->mac_dev);
 
 	/* xSGDMA Rx Dispatcher address space */
-	ret = request_and_map(pdev, "rx_csr", &dma_res,
-			      &priv->rx_dma_csr);
-	if (ret)
-		return ret;
-
+	priv->rx_dma_csr = request_and_map(pdev, "rx_csr", &dma_res);
+	if (IS_ERR(priv->rx_dma_csr))
+		return PTR_ERR(priv->rx_dma_csr);
 
 	/* xSGDMA Tx Dispatcher address space */
-	ret = request_and_map(pdev, "tx_csr", &dma_res,
-			      &priv->tx_dma_csr);
-	if (ret)
-		return ret;
+	priv->tx_dma_csr = request_and_map(pdev, "tx_csr", &dma_res);
+	if (IS_ERR(priv->tx_dma_csr))
+		return PTR_ERR(priv->tx_dma_csr);
 
 	memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
 	memset(&mrc, 0, sizeof(mrc));
@@ -1251,8 +1222,8 @@ static int altera_tse_probe(struct platform_device *pdev)
 	 * address space, but if it's not the case, we fallback to the mdiophy0
 	 * from the MAC's address space
 	 */
-	ret = request_and_map(pdev, "pcs", &pcs_res, &priv->pcs_base);
-	if (ret) {
+	priv->pcs_base = request_and_map(pdev, "pcs", &pcs_res);
+	if (IS_ERR(priv->pcs_base)) {
 		/* If we can't find a dedicated resource for the PCS, fallback
 		 * to the internal PCS, that has a different address stride
 		 */
-- 
2.47.0

Re: [PATCH net-next 0/2] altera: simplify probe

[Jakub Kicinski]

On Tue,  3 Dec 2024 15:31:48 -0800 Rosen Penev wrote:
> A small devm change with goto removals and a function simplification.

Please don't post pure devm conversions, unless you're actively
developing the driver and can test on real HW.