* [PATCHv3 net-next] net: modernize ioremap in probe
@ 2024-11-17 21:27 Rosen Penev
2024-11-17 22:38 ` Niklas Söderlund
2024-11-19 20:02 ` Sergey Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Rosen Penev @ 2024-11-17 21:27 UTC (permalink / raw)
To: netdev
Cc: Kurt Kanzenbach, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Chris Snook,
Marcin Wojtas, Russell King, Yoshihiro Shimoda,
Niklas Söderlund, Heiner Kallweit, Richard Cochran,
open list, open list:RENESAS ETHERNET SWITCH DRIVER
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv3 net-next] net: modernize ioremap in probe
2024-11-17 21:27 [PATCHv3 net-next] net: modernize ioremap in probe Rosen Penev
@ 2024-11-17 22:38 ` Niklas Söderlund
2024-11-17 23:07 ` Rosen Penev
2024-11-19 20:02 ` Sergey Shtylyov
1 sibling, 1 reply; 7+ messages in thread
From: Niklas Söderlund @ 2024-11-17 22:38 UTC (permalink / raw)
To: Rosen Penev
Cc: netdev, Kurt Kanzenbach, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Chris Snook, Marcin Wojtas, Russell King, Yoshihiro Shimoda,
Heiner Kallweit, Richard Cochran, open list,
open list:RENESAS ETHERNET SWITCH DRIVER
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3 net-next] net: modernize ioremap in probe
2024-11-17 22:38 ` Niklas Söderlund
@ 2024-11-17 23:07 ` Rosen Penev
2024-11-19 20:39 ` Niklas Söderlund
0 siblings, 1 reply; 7+ messages in thread
From: Rosen Penev @ 2024-11-17 23:07 UTC (permalink / raw)
To: Niklas Söderlund
Cc: netdev, Kurt Kanzenbach, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Chris Snook, Marcin Wojtas, Russell King, Yoshihiro Shimoda,
Heiner Kallweit, Richard Cochran, open list,
open list:RENESAS ETHERNET SWITCH DRIVER
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3 net-next] net: modernize ioremap in probe
2024-11-17 21:27 [PATCHv3 net-next] net: modernize ioremap in probe Rosen Penev
2024-11-17 22:38 ` Niklas Söderlund
@ 2024-11-19 20:02 ` Sergey Shtylyov
2024-11-20 19:29 ` Rosen Penev
1 sibling, 1 reply; 7+ messages in thread
From: Sergey Shtylyov @ 2024-11-19 20:02 UTC (permalink / raw)
To: Rosen Penev, netdev
Cc: Kurt Kanzenbach, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Chris Snook,
Marcin Wojtas, Russell King, Yoshihiro Shimoda,
Niklas Söderlund, Heiner Kallweit, Richard Cochran,
open list, open list:RENESAS ETHERNET SWITCH DRIVER
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3 net-next] net: modernize ioremap in probe
2024-11-17 23:07 ` Rosen Penev
@ 2024-11-19 20:39 ` Niklas Söderlund
2024-11-19 23:40 ` Russell King (Oracle)
0 siblings, 1 reply; 7+ messages in thread
From: Niklas Söderlund @ 2024-11-19 20:39 UTC (permalink / raw)
To: Rosen Penev
Cc: netdev, Kurt Kanzenbach, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Chris Snook, Marcin Wojtas, Russell King, Yoshihiro Shimoda,
Heiner Kallweit, Richard Cochran, open list,
open list:RENESAS ETHERNET SWITCH DRIVER
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3 net-next] net: modernize ioremap in probe
2024-11-19 20:39 ` Niklas Söderlund
@ 2024-11-19 23:40 ` Russell King (Oracle)
0 siblings, 0 replies; 7+ messages in thread
From: Russell King (Oracle) @ 2024-11-19 23:40 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Rosen Penev, netdev, Kurt Kanzenbach, Andrew Lunn,
Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Chris Snook, Marcin Wojtas, Yoshihiro Shimoda,
Heiner Kallweit, Richard Cochran, open list,
open list:RENESAS ETHERNET SWITCH DRIVER
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!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3 net-next] net: modernize ioremap in probe
2024-11-19 20:02 ` Sergey Shtylyov
@ 2024-11-20 19:29 ` Rosen Penev
0 siblings, 0 replies; 7+ messages in thread
From: Rosen Penev @ 2024-11-20 19:29 UTC (permalink / raw)
To: Sergey Shtylyov
Cc: netdev, Kurt Kanzenbach, Andrew Lunn, Vladimir Oltean,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Chris Snook, Marcin Wojtas, Russell King, Yoshihiro Shimoda,
Niklas Söderlund, Heiner Kallweit, Richard Cochran,
open list, open list:RENESAS ETHERNET SWITCH DRIVER
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-11-20 19:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-17 21:27 [PATCHv3 net-next] net: modernize ioremap in probe Rosen Penev
2024-11-17 22:38 ` Niklas Söderlund
2024-11-17 23:07 ` Rosen Penev
2024-11-19 20:39 ` Niklas Söderlund
2024-11-19 23:40 ` Russell King (Oracle)
2024-11-19 20:02 ` Sergey Shtylyov
2024-11-20 19:29 ` Rosen Penev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).