1. Patchew
  2. linux
  3. View series

[PATCHv3 net-next] net: modernize ioremap in probe

Rosen Penev posted 1 patch 5 days, 17 hours ago 
drivers/net/dsa/hirschmann/hellcreek.c         | 18 +++---------------
drivers/net/ethernet/atheros/ag71xx.c          | 13 +++++--------
drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  6 ++----
.../net/ethernet/marvell/mvpp2/mvpp2_main.c    | 14 +++++---------
drivers/net/ethernet/renesas/rswitch.c         |  9 +--------
drivers/net/ethernet/renesas/rtsn.c            | 10 ++--------
drivers/net/mdio/mdio-ipq4019.c                |  5 +----
7 files changed, 19 insertions(+), 56 deletions(-)
[PATCHv3 net-next] net: modernize ioremap in probe
Posted by Rosen Penev 5 days, 17 hours ago 
Convert platform_get_resource_bynam + devm_ioremap_resource to
devm_platform_ioremap_resource_byname.

Convert platform_get_resource + devm_ioremap_resource to
devm_platform_ioremap_resource.

resource aquisition and ioremap can be performed in one step.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v3: reworded commit message again. Also removed devm_ioremap
 conversions. Even though they use normal resource, they are not
 the same.
 v2: fixed compilation errors on PPC and reworded commit message
 drivers/net/dsa/hirschmann/hellcreek.c         | 18 +++---------------
 drivers/net/ethernet/atheros/ag71xx.c          | 13 +++++--------
 drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  6 ++----
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c    | 14 +++++---------
 drivers/net/ethernet/renesas/rswitch.c         |  9 +--------
 drivers/net/ethernet/renesas/rtsn.c            | 10 ++--------
 drivers/net/mdio/mdio-ipq4019.c                |  5 +----
 7 files changed, 19 insertions(+), 56 deletions(-)

diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 283ec5a6e23c..940c4fa6a924 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -1932,7 +1932,6 @@ static int hellcreek_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct hellcreek *hellcreek;
-	struct resource *res;
 	int ret, i;
 
 	hellcreek = devm_kzalloc(dev, sizeof(*hellcreek), GFP_KERNEL);
@@ -1982,23 +1981,12 @@ static int hellcreek_probe(struct platform_device *pdev)
 
 	hellcreek->dev = dev;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tsn");
-	if (!res) {
-		dev_err(dev, "No memory region provided!\n");
-		return -ENODEV;
-	}
-
-	hellcreek->base = devm_ioremap_resource(dev, res);
+	hellcreek->base = devm_platform_ioremap_resource_byname(pdev, "tsn");
 	if (IS_ERR(hellcreek->base))
 		return PTR_ERR(hellcreek->base);
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ptp");
-	if (!res) {
-		dev_err(dev, "No PTP memory region provided!\n");
-		return -ENODEV;
-	}
-
-	hellcreek->ptp_base = devm_ioremap_resource(dev, res);
+	hellcreek->ptp_base =
+		devm_platform_ioremap_resource_byname(pdev, "ptp");
 	if (IS_ERR(hellcreek->ptp_base))
 		return PTR_ERR(hellcreek->ptp_base);
 
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 3d4c3d8698e2..928d27b51b2a 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -1798,15 +1798,16 @@ static int ag71xx_probe(struct platform_device *pdev)
 	if (!ndev)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -EINVAL;
-
 	dcfg = of_device_get_match_data(&pdev->dev);
 	if (!dcfg)
 		return -EINVAL;
 
 	ag = netdev_priv(ndev);
+
+	ag->mac_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(ag->mac_base))
+		return PTR_ERR(ag->mac_base);
+
 	ag->mac_idx = -1;
 	for (i = 0; i < ARRAY_SIZE(ar71xx_addr_ar7100); i++) {
 		if (ar71xx_addr_ar7100[i] == res->start)
@@ -1836,10 +1837,6 @@ static int ag71xx_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(ag->mac_reset),
 				     "missing mac reset");
 
-	ag->mac_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(ag->mac_base))
-		return PTR_ERR(ag->mac_base);
-
 	/* ensure that HW is in manual polling mode before interrupts are
 	 * activated. Otherwise ag71xx_interrupt might call napi_schedule
 	 * before it is initialized by netif_napi_add.
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 65e3a0656a4c..420317abe3d2 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -2646,16 +2646,14 @@ static int bcm_enetsw_probe(struct platform_device *pdev)
 	struct bcm_enet_priv *priv;
 	struct net_device *dev;
 	struct bcm63xx_enetsw_platform_data *pd;
-	struct resource *res_mem;
 	int ret, irq_rx, irq_tx;
 
 	if (!bcm_enet_shared_base[0])
 		return -EPROBE_DEFER;
 
-	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq_rx = platform_get_irq(pdev, 0);
 	irq_tx = platform_get_irq(pdev, 1);
-	if (!res_mem || irq_rx < 0)
+	if (irq_rx < 0)
 		return -ENODEV;
 
 	dev = alloc_etherdev(sizeof(*priv));
@@ -2688,7 +2686,7 @@ static int bcm_enetsw_probe(struct platform_device *pdev)
 	if (ret)
 		goto out;
 
-	priv->base = devm_ioremap_resource(&pdev->dev, res_mem);
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base)) {
 		ret = PTR_ERR(priv->base);
 		goto out;
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 571631a30320..faf853edc0db 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -7425,21 +7425,17 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 static int mvpp2_get_sram(struct platform_device *pdev,
 			  struct mvpp2 *priv)
 {
-	struct resource *res;
 	void __iomem *base;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	if (!res) {
+	base = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(base)) {
 		if (has_acpi_companion(&pdev->dev))
 			dev_warn(&pdev->dev, "ACPI is too old, Flow control not supported\n");
 		else
-			dev_warn(&pdev->dev, "DT is too old, Flow control not supported\n");
-		return 0;
-	}
-
-	base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(base))
+			dev_warn(&pdev->dev,
+				 "DT is too old, Flow control not supported\n");
 		return PTR_ERR(base);
+	}
 
 	priv->cm3_base = base;
 	return 0;
diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c
index 8d18dae4d8fb..8ef52fc46a01 100644
--- a/drivers/net/ethernet/renesas/rswitch.c
+++ b/drivers/net/ethernet/renesas/rswitch.c
@@ -2046,15 +2046,8 @@ static int renesas_eth_sw_probe(struct platform_device *pdev)
 {
 	const struct soc_device_attribute *attr;
 	struct rswitch_private *priv;
-	struct resource *res;
 	int ret;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "secure_base");
-	if (!res) {
-		dev_err(&pdev->dev, "invalid resource\n");
-		return -EINVAL;
-	}
-
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -2074,7 +2067,7 @@ static int renesas_eth_sw_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 	priv->pdev = pdev;
-	priv->addr = devm_ioremap_resource(&pdev->dev, res);
+	priv->addr = devm_platform_ioremap_resource_byname(pdev, "secure_base");
 	if (IS_ERR(priv->addr))
 		return PTR_ERR(priv->addr);
 
diff --git a/drivers/net/ethernet/renesas/rtsn.c b/drivers/net/ethernet/renesas/rtsn.c
index 6b3f7fca8d15..bfe08facc707 100644
--- a/drivers/net/ethernet/renesas/rtsn.c
+++ b/drivers/net/ethernet/renesas/rtsn.c
@@ -1297,14 +1297,8 @@ static int rtsn_probe(struct platform_device *pdev)
 	ndev->netdev_ops = &rtsn_netdev_ops;
 	ndev->ethtool_ops = &rtsn_ethtool_ops;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp");
-	if (!res) {
-		dev_err(&pdev->dev, "Can't find gptp resource\n");
-		ret = -EINVAL;
-		goto error_free;
-	}
-
-	priv->ptp_priv->addr = devm_ioremap_resource(&pdev->dev, res);
+	priv->ptp_priv->addr =
+		devm_platform_ioremap_resource_byname(pdev, "gptp");
 	if (IS_ERR(priv->ptp_priv->addr)) {
 		ret = PTR_ERR(priv->ptp_priv->addr);
 		goto error_free;
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index 859302b0d38c..50afef293649 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -327,7 +327,6 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
 {
 	struct ipq4019_mdio_data *priv;
 	struct mii_bus *bus;
-	struct resource *res;
 	int ret;
 
 	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
@@ -351,9 +350,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
 
 	/* The platform resource is provided on the chipset IPQ5018 */
 	/* This resource is optional */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (res)
-		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
+	priv->eth_ldo_rdy = devm_platform_ioremap_resource(pdev, 1);
 
 	bus->name = "ipq4019_mdio";
 	bus->read = ipq4019_mdio_read_c22;
-- 
2.47.0
Re: [PATCHv3 net-next] net: modernize ioremap in probe
Posted by Sergey Shtylyov 3 days, 18 hours ago 
On 11/18/24 12:27 AM, Rosen Penev wrote:

> Convert platform_get_resource_bynam + devm_ioremap_resource to
> devm_platform_ioremap_resource_byname.
> 
> Convert platform_get_resource + devm_ioremap_resource to
> devm_platform_ioremap_resource.
> 
> resource aquisition and ioremap can be performed in one step.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  v3: reworded commit message again. Also removed devm_ioremap
>  conversions. Even though they use normal resource, they are not
>  the same.
>  v2: fixed compilation errors on PPC and reworded commit message
>  drivers/net/dsa/hirschmann/hellcreek.c         | 18 +++---------------
>  drivers/net/ethernet/atheros/ag71xx.c          | 13 +++++--------
>  drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  6 ++----
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c    | 14 +++++---------
>  drivers/net/ethernet/renesas/rswitch.c         |  9 +--------
>  drivers/net/ethernet/renesas/rtsn.c            | 10 ++--------
>  drivers/net/mdio/mdio-ipq4019.c                |  5 +----
>  7 files changed, 19 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> index 283ec5a6e23c..940c4fa6a924 100644
> --- a/drivers/net/dsa/hirschmann/hellcreek.c
> +++ b/drivers/net/dsa/hirschmann/hellcreek.c
[...]
> @@ -1982,23 +1981,12 @@ static int hellcreek_probe(struct platform_device *pdev)
>  
>  	hellcreek->dev = dev;
>  
> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tsn");
> -	if (!res) {
> -		dev_err(dev, "No memory region provided!\n");
> -		return -ENODEV;
> -	}
> -
> -	hellcreek->base = devm_ioremap_resource(dev, res);
> +	hellcreek->base = devm_platform_ioremap_resource_byname(pdev, "tsn");
>  	if (IS_ERR(hellcreek->base))
>  		return PTR_ERR(hellcreek->base);
>  
> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ptp");
> -	if (!res) {
> -		dev_err(dev, "No PTP memory region provided!\n");
> -		return -ENODEV;
> -	}
> -
> -	hellcreek->ptp_base = devm_ioremap_resource(dev, res);
> +	hellcreek->ptp_base = 
> +		devm_platform_ioremap_resource_byname(pdev, "ptp");

   You have full 100 columns now, so doing this with 2 lines doesn't seem necessary --
checkpatch.pl shouldn't complain...

[...]

   Other than that, looks saner now... :-)

MBR, Sergey
Re: [PATCHv3 net-next] net: modernize ioremap in probe
Posted by Rosen Penev 2 days, 19 hours ago 
On Tue, Nov 19, 2024 at 12:02 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote:
>
> On 11/18/24 12:27 AM, Rosen Penev wrote:
>
> > Convert platform_get_resource_bynam + devm_ioremap_resource to
> > devm_platform_ioremap_resource_byname.
> >
> > Convert platform_get_resource + devm_ioremap_resource to
> > devm_platform_ioremap_resource.
> >
> > resource aquisition and ioremap can be performed in one step.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  v3: reworded commit message again. Also removed devm_ioremap
> >  conversions. Even though they use normal resource, they are not
> >  the same.
> >  v2: fixed compilation errors on PPC and reworded commit message
> >  drivers/net/dsa/hirschmann/hellcreek.c         | 18 +++---------------
> >  drivers/net/ethernet/atheros/ag71xx.c          | 13 +++++--------
> >  drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  6 ++----
> >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c    | 14 +++++---------
> >  drivers/net/ethernet/renesas/rswitch.c         |  9 +--------
> >  drivers/net/ethernet/renesas/rtsn.c            | 10 ++--------
> >  drivers/net/mdio/mdio-ipq4019.c                |  5 +----
> >  7 files changed, 19 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
> > index 283ec5a6e23c..940c4fa6a924 100644
> > --- a/drivers/net/dsa/hirschmann/hellcreek.c
> > +++ b/drivers/net/dsa/hirschmann/hellcreek.c
> [...]
> > @@ -1982,23 +1981,12 @@ static int hellcreek_probe(struct platform_device *pdev)
> >
> >       hellcreek->dev = dev;
> >
> > -     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tsn");
> > -     if (!res) {
> > -             dev_err(dev, "No memory region provided!\n");
> > -             return -ENODEV;
> > -     }
> > -
> > -     hellcreek->base = devm_ioremap_resource(dev, res);
> > +     hellcreek->base = devm_platform_ioremap_resource_byname(pdev, "tsn");
> >       if (IS_ERR(hellcreek->base))
> >               return PTR_ERR(hellcreek->base);
> >
> > -     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ptp");
> > -     if (!res) {
> > -             dev_err(dev, "No PTP memory region provided!\n");
> > -             return -ENODEV;
> > -     }
> > -
> > -     hellcreek->ptp_base = devm_ioremap_resource(dev, res);
> > +     hellcreek->ptp_base =
> > +             devm_platform_ioremap_resource_byname(pdev, "ptp");
>
>    You have full 100 columns now, so doing this with 2 lines doesn't seem necessary --
> checkpatch.pl shouldn't complain...
Looks like that's bdc48fa11e46f according to git blame. Reading the
commit message, 80 is still prefered. The reason it's done here is
because of .clang-format, which still lists 80.
>
> [...]
>
>    Other than that, looks saner now... :-)
>
> MBR, Sergey
>
Re: [PATCHv3 net-next] net: modernize ioremap in probe
Posted by Niklas Söderlund 5 days, 16 hours ago 
Hello Rosen,

Thanks for your work.

On 2024-11-17 13:27:11 -0800, Rosen Penev wrote:

> diff --git a/drivers/net/ethernet/renesas/rtsn.c 
> b/drivers/net/ethernet/renesas/rtsn.c
> index 6b3f7fca8d15..bfe08facc707 100644
> --- a/drivers/net/ethernet/renesas/rtsn.c
> +++ b/drivers/net/ethernet/renesas/rtsn.c
> @@ -1297,14 +1297,8 @@ static int rtsn_probe(struct platform_device *pdev)
>  	ndev->netdev_ops = &rtsn_netdev_ops;
>  	ndev->ethtool_ops = &rtsn_ethtool_ops;
>  
> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp");
> -	if (!res) {
> -		dev_err(&pdev->dev, "Can't find gptp resource\n");
> -		ret = -EINVAL;
> -		goto error_free;
> -	}
> -
> -	priv->ptp_priv->addr = devm_ioremap_resource(&pdev->dev, res);
> +	priv->ptp_priv->addr =
> +		devm_platform_ioremap_resource_byname(pdev, "gptp");
>  	if (IS_ERR(priv->ptp_priv->addr)) {
>  		ret = PTR_ERR(priv->ptp_priv->addr);
>  		goto error_free;

You have a similar construct using platform_get_resource_byname() a few 
lines above this one. Please convert both uses, or none, mixing them is 
just confusing IMHO.

-- 
Kind Regards,
Niklas Söderlund
Re: [PATCHv3 net-next] net: modernize ioremap in probe
Posted by Rosen Penev 5 days, 15 hours ago 
On Sun, Nov 17, 2024 at 2:38 PM Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
>
> Hello Rosen,
>
> Thanks for your work.
>
> On 2024-11-17 13:27:11 -0800, Rosen Penev wrote:
>
> > diff --git a/drivers/net/ethernet/renesas/rtsn.c
> > b/drivers/net/ethernet/renesas/rtsn.c
> > index 6b3f7fca8d15..bfe08facc707 100644
> > --- a/drivers/net/ethernet/renesas/rtsn.c
> > +++ b/drivers/net/ethernet/renesas/rtsn.c
> > @@ -1297,14 +1297,8 @@ static int rtsn_probe(struct platform_device *pdev)
> >       ndev->netdev_ops = &rtsn_netdev_ops;
> >       ndev->ethtool_ops = &rtsn_ethtool_ops;
> >
> > -     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp");
> > -     if (!res) {
> > -             dev_err(&pdev->dev, "Can't find gptp resource\n");
> > -             ret = -EINVAL;
> > -             goto error_free;
> > -     }
> > -
> > -     priv->ptp_priv->addr = devm_ioremap_resource(&pdev->dev, res);
> > +     priv->ptp_priv->addr =
> > +             devm_platform_ioremap_resource_byname(pdev, "gptp");
> >       if (IS_ERR(priv->ptp_priv->addr)) {
> >               ret = PTR_ERR(priv->ptp_priv->addr);
> >               goto error_free;
>
> You have a similar construct using platform_get_resource_byname() a few
> lines above this one. Please convert both uses, or none, mixing them is
> just confusing IMHO.
that cannot be converted.

devm_platform_ioremap_resource_byname has no res parameter, which is a
problem as there's this lovely line below it.

ndev->base_addr = res->start;
>
> --
> Kind Regards,
> Niklas Söderlund
Re: [PATCHv3 net-next] net: modernize ioremap in probe
Posted by Niklas Söderlund 3 days, 18 hours ago 
On 2024-11-17 15:07:53 -0800, Rosen Penev wrote:
> On Sun, Nov 17, 2024 at 2:38 PM Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> >
> > Hello Rosen,
> >
> > Thanks for your work.
> >
> > On 2024-11-17 13:27:11 -0800, Rosen Penev wrote:
> >
> > > diff --git a/drivers/net/ethernet/renesas/rtsn.c
> > > b/drivers/net/ethernet/renesas/rtsn.c
> > > index 6b3f7fca8d15..bfe08facc707 100644
> > > --- a/drivers/net/ethernet/renesas/rtsn.c
> > > +++ b/drivers/net/ethernet/renesas/rtsn.c
> > > @@ -1297,14 +1297,8 @@ static int rtsn_probe(struct platform_device *pdev)
> > >       ndev->netdev_ops = &rtsn_netdev_ops;
> > >       ndev->ethtool_ops = &rtsn_ethtool_ops;
> > >
> > > -     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp");
> > > -     if (!res) {
> > > -             dev_err(&pdev->dev, "Can't find gptp resource\n");
> > > -             ret = -EINVAL;
> > > -             goto error_free;
> > > -     }
> > > -
> > > -     priv->ptp_priv->addr = devm_ioremap_resource(&pdev->dev, res);
> > > +     priv->ptp_priv->addr =
> > > +             devm_platform_ioremap_resource_byname(pdev, "gptp");
> > >       if (IS_ERR(priv->ptp_priv->addr)) {
> > >               ret = PTR_ERR(priv->ptp_priv->addr);
> > >               goto error_free;
> >
> > You have a similar construct using platform_get_resource_byname() a few
> > lines above this one. Please convert both uses, or none, mixing them is
> > just confusing IMHO.
> that cannot be converted.
> 
> devm_platform_ioremap_resource_byname has no res parameter, which is a
> problem as there's this lovely line below it.
> 
> ndev->base_addr = res->start;

I see, maybe we can refactor that too? I see not all drivers set 
base_addr, and some even set it to the remapped memory returned by 
devm_platform_ioremap_resource_byname() or such.

The documentation for this field is not crystal clear for me and it is 
marked with a FIXME in the definition.

struct net_device {

	...

	/*
	 *	I/O specific fields
	 *	FIXME: Merge these and struct ifmap into one
	 */
	unsigned long		mem_end;
	unsigned long		mem_start;
	unsigned long		base_addr;

	...

> >
> > --
> > Kind Regards,
> > Niklas Söderlund

-- 
Kind Regards,
Niklas Söderlund
Re: [PATCHv3 net-next] net: modernize ioremap in probe
Posted by Russell King (Oracle) 3 days, 15 hours ago 
On Tue, Nov 19, 2024 at 09:39:16PM +0100, Niklas Söderlund wrote:
> On 2024-11-17 15:07:53 -0800, Rosen Penev wrote:
> > devm_platform_ioremap_resource_byname has no res parameter, which is a
> > problem as there's this lovely line below it.
> > 
> > ndev->base_addr = res->start;
> 
> I see, maybe we can refactor that too? I see not all drivers set 
> base_addr, and some even set it to the remapped memory returned by 
> devm_platform_ioremap_resource_byname() or such.

base_addr carries with it an issue that setting it on every driver is
likely not a good idea.

Namely, that it's "unsigned long", it's reported to userspace, and
on PAE systems, unsigned long is 32-bit but the device address may
be >32-bit.

I haven't checked the user APIs, whether that restricts it to 32-bit
on 32-bit systems.

In any case, whether base_addr is set or not is probably best left
as-is and not have some "we must always / never set base_addr" rule
applied to it.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.